fix(cfn-include): handle numbers expressed as strings in templates (#…

…9525)

Closes #9524

----

*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

@@ -4,13 +4,20 @@
  * object prototype into account for the purpose of the comparison, only the values of
  * properties reported by +Object.keys+.
  *
+ * If both operands can be parsed to equivalent numbers, will return true.
+ * This makes diff consistent with CloudFormation, where a numeric 10 and a literal "10"
+ * are considered equivalent.
+ *
  * @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.
  */
 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 !== typeof rvalue) { return false; }
   if (Array.isArray(lvalue) !== Array.isArray(rvalue)) { return false; }
   if (Array.isArray(lvalue) /* && Array.isArray(rvalue) */) {

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

@@ -321,3 +321,56 @@ test('resource replacement is tracked through references', () => {
   // THEN
   expect(differences.resources.differenceCount).toBe(3);
 });
+
+test('adding and removing quotes from a numeric property causes no changes', () => {
+  const currentTemplate = {
+    Resources: {
+      Bucket: {
+        Type: 'AWS::S3::Bucket',
+        Properties: {
+          CorsConfiguration: {
+            CorsRules: [
+              {
+                AllowedMethods: [
+                  'GET',
+                ],
+                AllowedOrigins: [
+                  '*',
+                ],
+                MaxAge: 10,
+              },
+            ],
+          },
+        },
+      },
+    },
+  };
+
+  const newTemplate = {
+    Resources: {
+      Bucket: {
+        Type: 'AWS::S3::Bucket',
+        Properties: {
+          CorsConfiguration: {
+            CorsRules: [
+              {
+                AllowedMethods: [
+                  'GET',
+                ],
+                AllowedOrigins: [
+                  '*',
+                ],
+                MaxAge: '10',
+              },
+            ],
+          },
+        },
+      },
+    },
+  };
+  let differences = diffTemplate(currentTemplate, newTemplate);
+  expect(differences.resources.differenceCount).toBe(0);
+
+  differences = diffTemplate(newTemplate, currentTemplate);
+  expect(differences.resources.differenceCount).toBe(0);
+});

packages/@aws-cdk/cloudformation-include/test/invalid-templates.test.ts

@@ -112,6 +112,14 @@ describe('CDK Include', () => {
       includeTestTemplate(stack, 'fn-sub-${}-only.json');
     }).toThrow(/Element referenced in Fn::Sub expression with logical ID: '' was not found in the template/);
   });
+
+  test('throws an error when a template supplies an invalid string to a number parameter', () => {
+    includeTestTemplate(stack, 'alphabetical-string-passed-to-number.json');
+
+    expect(() => {
+      SynthUtils.synthesize(stack);
+    }).toThrow(/"abc" should be a number/);
+  });
 });
 
 function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {

packages/@aws-cdk/cloudformation-include/test/test-templates/invalid/alphabetical-string-passed-to-number.json

@@ -0,0 +1,18 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "CorsConfiguration": {
+          "CorsRules": [
+            {
+              "AllowedMethods": ["GET"],
+              "AllowedOrigins": ["*"],
+              "MaxAge": "abc"
+            }
+          ]
+        }
+      }
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/test-templates/parsing-as-numbers.json

@@ -0,0 +1,18 @@
+{
+  "Resources": {
+    "Bucket": {
+      "Type": "AWS::S3::Bucket",
+      "Properties": {
+        "CorsConfiguration": {
+          "CorsRules": [
+            {
+              "AllowedMethods": ["GET"],
+              "AllowedOrigins": ["*"],
+              "MaxAge": "10"
+            }
+          ]
+        }
+      }
+    }
+  }
+}

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts

@@ -98,6 +98,14 @@ describe('CDK Include', () => {
     );
   });
 
+  test('correctly parse strings as integers as needed', () => {
+    includeTestTemplate(stack, 'parsing-as-numbers.json');
+
+    expect(stack).toMatchTemplate(
+      loadTestFileToJsObject('parsing-as-numbers.json'),
+    );
+  });
+
   xtest('correctly changes the logical IDs, including references, if imported with preserveLogicalIds=false', () => {
     const cfnTemplate = includeTestTemplate(stack, 'bucket-with-encryption-key.json', {
       preserveLogicalIds: false,

packages/@aws-cdk/core/lib/cfn-parse.ts

@@ -51,6 +51,8 @@ export class FromCloudFormation {
     return value;
   }
 
+  // won't always return a string; if the input can't be resolved to a string,
+  // the input will be returned.
   public static getString(value: any): string {
     // if the string is a deploy-time value, serialize it to a Token
     if (isResolvableObject(value)) {
@@ -62,13 +64,24 @@ export class FromCloudFormation {
     return value;
   }
 
+  // won't always return a number; if the input can't be parsed to a number,
+  // the input will be returned.
   public static getNumber(value: any): number {
     // if the string is a deploy-time value, serialize it to a Token
     if (isResolvableObject(value)) {
       return Token.asNumber(value);
     }
 
-    // in all other cases, just return the input,
+    // return a number, if the input can be parsed as one
+    let parsedValue;
+    if (typeof value === 'string') {
+      parsedValue = parseFloat(value);
+      if (!isNaN(parsedValue)) {
+        return parsedValue;
+      }
+    }
+
+    // otherwise return the input,
     // and let a validator handle it if it's not a number
     return value;
   }