Skip to content

Commit

Permalink
fix(core): apply overrides after rendering properties
Browse files Browse the repository at this point in the history
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')`.
  • Loading branch information
Elad Ben-Israel committed May 30, 2019
1 parent c52bcfc commit b5f2a32
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
8 changes: 5 additions & 3 deletions packages/@aws-cdk/cdk/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
}
};
Expand Down
32 changes: 32 additions & 0 deletions packages/@aws-cdk/cdk/test/test.resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit b5f2a32

Please sign in to comment.