fix(diff): allow strings to be passed for boolean properties (#10378)

In CloudFormation, it's legal to pass as string where a boolean value is expected.
Take that into account when performing diffs,
and allow that case in `@aws-cdk/cloudformation-include`.

----

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

Author: skinny85

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

@@ -15,6 +15,12 @@
  */
 export function deepEqual(lvalue: any, rvalue: any): boolean {
   if (lvalue === rvalue) { return true; }
+  // CloudFormation allows passing strings into boolean-typed fields
+  if (((typeof lvalue === 'string' && typeof rvalue === 'boolean') ||
+      (typeof lvalue === 'boolean' && typeof rvalue === 'string')) &&
+      lvalue.toString() === rvalue.toString()) {
+    return true;
+  }
   // allows a numeric 10 and a literal "10" to be equivalent;
   // this is consistent with CloudFormation.
   if (((typeof lvalue === 'string') || (typeof rvalue === 'string')) && (parseFloat(lvalue) === parseFloat(rvalue))) {

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

@@ -403,7 +403,6 @@ test('single element arrays are equivalent to the single element in DependsOn ex
   expect(differences.resources.differenceCount).toBe(0);
 });
 
-
 test('array equivalence is independent of element order in DependsOn expressions', () => {
   // GIVEN
   const currentTemplate = {
@@ -487,3 +486,37 @@ test('arrays that differ only in element order are considered unequal outside of
   differences = diffTemplate(newTemplate, currentTemplate);
   expect(differences.resources.differenceCount).toBe(1);
 });
+
+test('boolean properties are considered equal with their stringified counterparts', () => {
+  // GIVEN
+  const currentTemplate = {
+    Resources: {
+      Bucket: {
+        Type: 'AWS::S3::Bucket',
+        Properties: {
+          PublicAccessBlockConfiguration: {
+            BlockPublicAcls: 'true',
+          },
+        },
+      },
+    },
+  };
+  const newTemplate = {
+    Resources: {
+      Bucket: {
+        Type: 'AWS::S3::Bucket',
+        Properties: {
+          PublicAccessBlockConfiguration: {
+            BlockPublicAcls: true,
+          },
+        },
+      },
+    },
+  };
+
+  // WHEN
+  const differences = diffTemplate(currentTemplate, newTemplate);
+
+  // THEN
+  expect(differences.differenceCount).toBe(0);
+});

packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-map-dotted-attributes.json

@@ -14,6 +14,7 @@
         "AvailabilityZones": [
           "us-east-1a"
         ],
+        "CrossZone": "true",
         "Listeners": [{
           "LoadBalancerPort": "80",
           "InstancePort": "80",

packages/@aws-cdk/cloudformation-include/test/test-templates/resource-attribute-update-policy.json

@@ -53,7 +53,7 @@
           "BeforeAllowTrafficHook" : "Lambda2",
           "DeploymentGroupName" : { "Ref": "CodeDeployDg" }
         },
-        "EnableVersionUpgrade": true,
+        "EnableVersionUpgrade": "true",
         "UseOnlineResharding": false
       }
     }

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

@@ -32,9 +32,20 @@ export class FromCloudFormation {
   // nothing to for any but return it
   public static getAny(value: any) { return value; }
 
-  // nothing to do - if 'value' is not a boolean or a Token,
-  // a validator should report that at runtime
-  public static getBoolean(value: any): boolean | IResolvable { return value; }
+  public static getBoolean(value: any): boolean | IResolvable {
+    if (typeof value === 'string') {
+      // CloudFormation allows passing strings as boolean
+      switch (value) {
+        case 'true': return true;
+        case 'false': return false;
+        default: throw new Error(`Expected 'true' or 'false' for boolean value, got: '${value}'`);
+      }
+    }
+
+    // in all other cases, just return the value,
+    // and let a validator handle if it's not a boolean
+    return value;
+  }
 
   public static getDate(value: any): Date | IResolvable {
     // if the date is a deploy-time value, just return it
@@ -80,9 +91,8 @@ export class FromCloudFormation {
     }
 
     // return a number, if the input can be parsed as one
-    let parsedValue;
     if (typeof value === 'string') {
-      parsedValue = parseFloat(value);
+      const parsedValue = parseFloat(value);
       if (!isNaN(parsedValue)) {
         return parsedValue;
       }
@@ -338,8 +348,8 @@ export class CfnParser {
       autoScalingRollingUpdate: parseAutoScalingRollingUpdate(policy.AutoScalingRollingUpdate),
       autoScalingScheduledAction: parseAutoScalingScheduledAction(policy.AutoScalingScheduledAction),
       codeDeployLambdaAliasUpdate: parseCodeDeployLambdaAliasUpdate(policy.CodeDeployLambdaAliasUpdate),
-      enableVersionUpgrade: policy.EnableVersionUpgrade,
-      useOnlineResharding: policy.UseOnlineResharding,
+      enableVersionUpgrade: FromCloudFormation.getBoolean(policy.EnableVersionUpgrade),
+      useOnlineResharding: FromCloudFormation.getBoolean(policy.UseOnlineResharding),
     });
 
     function parseAutoScalingReplacingUpdate(p: any): CfnAutoScalingReplacingUpdate | undefined {
@@ -359,7 +369,7 @@ export class CfnParser {
         minSuccessfulInstancesPercent: FromCloudFormation.getNumber(p.MinSuccessfulInstancesPercent),
         pauseTime: FromCloudFormation.getString(p.PauseTime),
         suspendProcesses: FromCloudFormation.getStringArray(p.SuspendProcesses),
-        waitOnResourceSignals: p.WaitOnResourceSignals,
+        waitOnResourceSignals: FromCloudFormation.getBoolean(p.WaitOnResourceSignals),
       });
     }