From d2a4a92d2cfc0499e915eb46d1336de40fa063c3 Mon Sep 17 00:00:00 2001 From: Aditya Jain Date: Wed, 12 Aug 2026 01:13:07 -0700 Subject: [PATCH] fix(cloudformation-diff): DependsOn changes are invisible for lists of two or more `dependsOnEqual` intends to compare two DependsOn arrays irrespective of element order, but the inner loop ends in an unconditional `break`, so it only ever runs for j = 0. The `return false` is guarded by `j === lvalue.length - 1`, which for any array longer than one element is never true on that single iteration. Two same-length arrays therefore always compare as equal, whatever they contain. The effect is that `cdk diff` reports no change for a resource whose DependsOn list is completely rewritten, as long as the number of entries stays the same. Replace the loop with a multiplicity-preserving match of each element on the left against a distinct element on the right. Neither input array is reordered, keeping the property that #1602 established. Co-Authored-By: Claude Opus 5 --- .../cloudformation-diff/lib/diff/util.ts | 15 +++-- ...template-and-changeset-diff-merger.test.ts | 59 +++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts index 07c6c5c84..b8018af12 100644 --- a/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts +++ b/packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts @@ -105,13 +105,16 @@ function dependsOnEqual(lvalue: any, rvalue: any): boolean { if (lvalue.length !== rvalue.length) { return false; } - for (let i = 0 ; i < lvalue.length ; i++) { - for (let j = 0 ; j < lvalue.length ; j++) { - if ((!deepEqual(lvalue[i], rvalue[j])) && (j === lvalue.length - 1)) { - return false; - } - break; + // Match each element of lvalue with a distinct element of rvalue. Elements are + // consumed as they are matched, so duplicates are compared by multiplicity. + // Neither input is mutated: only the local copy of indices is. + const unmatched = rvalue.map((_, i) => i); + for (const l of lvalue) { + const at = unmatched.findIndex((i) => deepEqual(l, rvalue[i])); + if (at === -1) { + return false; } + unmatched.splice(at, 1); } return true; } diff --git a/packages/@aws-cdk/cloudformation-diff/test/template-and-changeset-diff-merger.test.ts b/packages/@aws-cdk/cloudformation-diff/test/template-and-changeset-diff-merger.test.ts index bff3c2ba5..eeb177dbc 100644 --- a/packages/@aws-cdk/cloudformation-diff/test/template-and-changeset-diff-merger.test.ts +++ b/packages/@aws-cdk/cloudformation-diff/test/template-and-changeset-diff-merger.test.ts @@ -402,6 +402,65 @@ describe('fullDiff tests that include changeset', () => { expect(differences.resources.differenceCount).toBe(1); }); + test.each([ + [['SomeResource', 'AnotherResource'], ['ThirdResource', 'FourthResource']], + [['SomeResource', 'AnotherResource'], ['SomeResource', 'FourthResource']], + [['SomeResource', 'AnotherResource', 'ThirdResource'], ['AnotherResource', 'ThirdResource', 'FourthResource']], + [['SomeResource', 'SomeResource'], ['SomeResource', 'AnotherResource']], + ])('same-length DependsOn arrays with different elements are unequal: %j vs %j', (before, after) => { + // GIVEN + const currentTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: before, + }, + }, + }; + + // WHEN + const newTemplate = { + Resources: { + BucketResource: { + Type: 'AWS::S3::Bucket', + DependsOn: after, + }, + }, + }; + + // THEN + let differences = fullDiff(currentTemplate, newTemplate, {}); + expect(differences.resources.differenceCount).toBe(1); + + differences = fullDiff(newTemplate, currentTemplate, {}); + expect(differences.resources.differenceCount).toBe(1); + }); + + test('DependsOn arrays with the same elements in a different order are equal, and inputs are not mutated', () => { + // GIVEN + const before = ['SomeResource', 'AnotherResource', 'SomeResource']; + const after = ['SomeResource', 'SomeResource', 'AnotherResource']; + const currentTemplate = { + Resources: { + BucketResource: { Type: 'AWS::S3::Bucket', DependsOn: before }, + }, + }; + const newTemplate = { + Resources: { + BucketResource: { Type: 'AWS::S3::Bucket', DependsOn: after }, + }, + }; + + // WHEN + const differences = fullDiff(currentTemplate, newTemplate, {}); + + // THEN + expect(differences.resources.differenceCount).toBe(0); + // the comparison must not reorder the caller's arrays (see #1575) + expect(before).toEqual(['SomeResource', 'AnotherResource', 'SomeResource']); + expect(after).toEqual(['SomeResource', 'SomeResource', 'AnotherResource']); + }); + test('arrays that differ only in element order are considered unequal outside of DependsOn expressions', () => { // GIVEN const currentTemplate = {