fix(core): apply overrides after rendering properties (#2685)

Resource overrides (`addOverride` and `addPropertyOverride`) should be
applied after rendering properties at the L1 level. Otherwise, validation
and capitalization changes would be applied to overrides and this
contradicts the idea of being able to specify arbitrary overrides as
"patches" to the synthesized resource.

The previous behavior had two adverse effects:
1. If a property was unknown, it would be omitted from the resource
2. Properties names would need to be capitalized in camel case instead of 1:1 with the CFN schema.

Fixes #2677

BREAKING CHANGE: Properties passed to `addPropertyOverride` should match in capitalization to the CloudFormation schema (normally pascal case). For example, `addPropertyOverride('accessControl', 'xxx')` should now be `addPropertyOverride('AccessControl', 'xxx')`.

Author: Elad Ben-Israel

packages/@aws-cdk/aws-iam/test/test.escape-hatch.ts

@@ -0,0 +1,98 @@
+// tests for the L1 escape hatches (overrides). those are in the IAM module
+// because we want to verify them end-to-end, as a complement to the unit
+// tests in the @aws-cdk/cdk module
+import { expect } from '@aws-cdk/assert';
+import { Stack } from '@aws-cdk/cdk';
+import { Test } from 'nodeunit';
+import iam = require('../lib');
+
+// tslint:disable:object-literal-key-quotes
+
+export = {
+  'addPropertyOverride should allow overriding supported properties'(test: Test) {
+    const stack = new Stack();
+    const user = new iam.User(stack, 'user', {
+      userName: 'MyUserName'
+    });
+
+    const cfn = user.node.findChild('Resource') as iam.CfnUser;
+    cfn.addPropertyOverride('UserName', 'OverriddenUserName');
+
+    expect(stack).toMatch({
+      "Resources": {
+        "user2C2B57AE": {
+          "Type": "AWS::IAM::User",
+          "Properties": {
+            "UserName": "OverriddenUserName"
+          }
+        }
+      }
+    });
+    test.done();
+  },
+  'addPropertyOverrides should allow specifying arbitrary properties'(test: Test) {
+    // GIVEN
+    const stack = new Stack();
+    const user = new iam.User(stack, 'user', { userName: 'MyUserName' });
+    const cfn = user.node.findChild('Resource') as iam.CfnUser;
+
+    // WHEN
+    cfn.addPropertyOverride('Hello.World', 'Boom');
+
+    // THEN
+    expect(stack).toMatch({
+      "Resources": {
+        "user2C2B57AE": {
+          "Type": "AWS::IAM::User",
+          "Properties": {
+            "UserName": "MyUserName",
+            "Hello": {
+              "World": "Boom"
+            }
+          }
+        }
+      }
+    });
+
+    test.done();
+  },
+  'addOverride should allow overriding properties'(test: Test) {
+    // GIVEN
+    const stack = new Stack();
+    const user = new iam.User(stack, 'user', { userName: 'MyUserName' });
+    const cfn = user.node.findChild('Resource') as iam.CfnUser;
+    cfn.options.updatePolicy = { useOnlineResharding: true };
+
+    // WHEN
+    cfn.addOverride('Properties.Hello.World', 'Bam');
+    cfn.addOverride('Properties.UserName', 'HA!');
+    cfn.addOverride('Joob.Jab', 'Jib');
+    cfn.addOverride('Joob.Jab', 'Jib');
+    cfn.addOverride('UpdatePolicy.UseOnlineResharding.Type', 'None');
+
+    // THEN
+    expect(stack).toMatch({
+      "Resources": {
+        "user2C2B57AE": {
+          "Type": "AWS::IAM::User",
+          "Properties": {
+            "UserName": "HA!",
+            "Hello": {
+              "World": "Bam"
+            }
+          },
+          "Joob": {
+            "Jab": "Jib"
+          },
+          "UpdatePolicy": {
+            "UseOnlineResharding": {
+              "Type": "None"
+            }
+          }
+        }
+      }
+    });
+
+    test.done();
+  }
+};

packages/@aws-cdk/cdk/lib/cfn-resource.ts

@@ -234,9 +234,11 @@ export class CfnResource extends CfnRefElement {
             Metadata: ignoreEmpty(this.options.metadata),
             Condition: this.options.condition && this.options.condition.logicalId
           }, props => {
-            const r = deepMerge(props, this.rawOverrides);
-            r.Properties = this.renderProperties(r.Properties);
-            return r;
+            // let derived classes to influence how properties are rendered (e.g. change capitalization)
+            props.Properties = this.renderProperties(props.Properties);
+
+            // merge overrides *after* rendering
+            return deepMerge(props, this.rawOverrides);
           })
         }
       };

packages/@aws-cdk/cdk/test/test.resource.ts

@@ -549,6 +549,38 @@ export = {
       test.done();
     },
 
+    'overrides are applied after render'(test: Test) {
+      // GIVEN
+      class MyResource extends CfnResource {
+        public renderProperties() {
+          return { Fixed: 123 };
+        }
+      }
+      const stack = new Stack();
+      const cfn = new MyResource(stack, 'rr', { type: 'AWS::Resource::Type' });
+
+      // WHEN
+      cfn.addPropertyOverride('Boom', 'Hi');
+      cfn.addOverride('Properties.Foo.Bar', 'Bar');
+
+      // THEN
+      test.deepEqual(stack._toCloudFormation(), {
+        Resources: {
+          rr: {
+            Type: 'AWS::Resource::Type',
+            Properties: {
+              Fixed: 123,
+              Boom: 'Hi',
+              Foo: {
+                Bar: 'Bar'
+              }
+            }
+          }
+        }
+      });
+      test.done();
+    },
+
     'untypedPropertyOverrides': {
 
       'can be used by derived classes to specify overrides before render()'(test: Test) {