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 = {