fix(cloudformation-diff): DependsOn singleton arrays aren't equal to …

…string values (#9814)

----

Closes #9813 

This allows

`DependsOn: "A"` and `DependsOn: ["A"]`

and

`DependsOn: ["A", "B"]` and `DependsOn: ["B", "A"]` to be equivalent.

----

It also fixes a bug that would cause 

`BucketName: { 'Fn::Select': [0, ['name2', 'name1']] }`

and

`BucketName: { 'Fn::Select': [0, ['name1', 'name2']] }`

to be equal.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts

@@ -17,7 +17,9 @@ export function deepEqual(lvalue: any, rvalue: any): boolean {
   if (lvalue === rvalue) { return true; }
   // allows a numeric 10 and a literal "10" to be equivalent;
   // this is consistent with CloudFormation.
-  if (parseFloat(lvalue) === parseFloat(rvalue)) { return true; }
+  if (((typeof lvalue === 'string') || (typeof rvalue === 'string')) && (parseFloat(lvalue) === parseFloat(rvalue))) {
+    return true;
+  }
   if (typeof lvalue !== typeof rvalue) { return false; }
   if (Array.isArray(lvalue) !== Array.isArray(rvalue)) { return false; }
   if (Array.isArray(lvalue) /* && Array.isArray(rvalue) */) {
@@ -36,6 +38,7 @@ export function deepEqual(lvalue: any, rvalue: any): boolean {
     if (keys.length !== Object.keys(rvalue).length) { return false; }
     for (const key of keys) {
       if (!rvalue.hasOwnProperty(key)) { return false; }
+      if (key === 'DependsOn') { return dependsOnEqual(lvalue[key], rvalue[key]); }
       if (!deepEqual(lvalue[key], rvalue[key])) { return false; }
     }
     return true;
@@ -45,6 +48,43 @@ export function deepEqual(lvalue: any, rvalue: any): boolean {
   return false;
 }
 
+/**
+ * Compares two arguments to DependsOn for equality.
+ *
+ * @param lvalue the left operand of the equality comparison.
+ * @param rvalue the right operand of the equality comparison.
+ *
+ * @returns +true+ if both +lvalue+ and +rvalue+ are equivalent to each other.
+ */
+function dependsOnEqual(lvalue: any, rvalue: any): boolean {
+  // allows ['Value'] and 'Value' to be equal
+  if (Array.isArray(lvalue) !== Array.isArray(rvalue)) {
+    const array = Array.isArray(lvalue) ? lvalue : rvalue;
+    const nonArray = Array.isArray(lvalue) ? rvalue : lvalue;
+
+    if (array.length === 1 && deepEqual(array[0], nonArray)) {
+      return true;
+    }
+    return false;
+  }
+
+  // allows arrays passed to DependsOn to be equivalent irrespective of element order
+  if (Array.isArray(lvalue) && Array.isArray(rvalue)) {
+    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;
+      }
+    }
+    return true;
+  }
+
+  return false;
+}
+
 /**
  * Produce the differences between two maps, as a map, using a specified diff function.
  *

packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts

@@ -374,3 +374,116 @@ test('adding and removing quotes from a numeric property causes no changes', ()
   differences = diffTemplate(newTemplate, currentTemplate);
   expect(differences.resources.differenceCount).toBe(0);
 });
+
+test('single element arrays are equivalent to the single element in DependsOn expressions', () => {
+  // GIVEN
+  const currentTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        DependsOn: ['SomeResource'],
+      },
+    },
+  };
+
+  // WHEN
+  const newTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        DependsOn: 'SomeResource',
+      },
+    },
+  };
+
+  let differences = diffTemplate(currentTemplate, newTemplate);
+  expect(differences.resources.differenceCount).toBe(0);
+
+  differences = diffTemplate(newTemplate, currentTemplate);
+  expect(differences.resources.differenceCount).toBe(0);
+});
+
+
+test('array equivalence is independent of element order in DependsOn expressions', () => {
+  // GIVEN
+  const currentTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        DependsOn: ['SomeResource', 'AnotherResource'],
+      },
+    },
+  };
+
+  // WHEN
+  const newTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        DependsOn: ['AnotherResource', 'SomeResource'],
+      },
+    },
+  };
+
+  let differences = diffTemplate(currentTemplate, newTemplate);
+  expect(differences.resources.differenceCount).toBe(0);
+
+  differences = diffTemplate(newTemplate, currentTemplate);
+  expect(differences.resources.differenceCount).toBe(0);
+});
+
+test('arrays of different length are considered unequal in DependsOn expressions', () => {
+  // GIVEN
+  const currentTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        DependsOn: ['SomeResource', 'AnotherResource', 'LastResource'],
+      },
+    },
+  };
+
+  // WHEN
+  const newTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        DependsOn: ['AnotherResource', 'SomeResource'],
+      },
+    },
+  };
+
+  let differences = diffTemplate(currentTemplate, newTemplate);
+  expect(differences.resources.differenceCount).toBe(1);
+
+  differences = diffTemplate(newTemplate, currentTemplate);
+  expect(differences.resources.differenceCount).toBe(1);
+});
+
+test('arrays that differ only in element order are considered unequal outside of DependsOn expressions', () => {
+  // GIVEN
+  const currentTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        BucketName: { 'Fn::Select': [0, ['name1', 'name2']] },
+      },
+    },
+  };
+
+  // WHEN
+  const newTemplate = {
+    Resources: {
+      BucketResource: {
+        Type: 'AWS::S3::Bucket',
+        BucketName: { 'Fn::Select': [0, ['name2', 'name1']] },
+      },
+    },
+  };
+
+  let differences = diffTemplate(currentTemplate, newTemplate);
+  expect(differences.resources.differenceCount).toBe(1);
+
+  differences = diffTemplate(newTemplate, currentTemplate);
+  expect(differences.resources.differenceCount).toBe(1);
+});