Skip to content

Commit

Permalink
fix(core): properties set to false are not rendered in the template (#…
Browse files Browse the repository at this point in the history
…10539)

Any CloudFormation resource that defines a single boolean property set
to false is not rendered to the CloudFormation template.

The bug is in implementation of `_toCloudFormation()` API in
`CfnResource`. It treated `false` and `undefined` the same way.

fixes #10455


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Niranjan Jayakar authored Sep 30, 2020
1 parent 9aea4ae commit b42d4e9
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
5 changes: 4 additions & 1 deletion packages/@aws-cdk/core/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,10 @@ export class CfnResource extends CfnRefElement {
Condition: this.cfnOptions.condition && this.cfnOptions.condition.logicalId,
}, props => {
const renderedProps = this.renderProperties(props.Properties || {});
props.Properties = renderedProps && (Object.values(renderedProps).find(v => !!v) ? renderedProps : undefined);
if (renderedProps) {
const hasDefined = Object.values(renderedProps).find(v => v !== undefined);
props.Properties = hasDefined !== undefined ? renderedProps : undefined;
}
return deepMerge(props, this.rawOverrides);
}),
},
Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/core/test/test.cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,30 @@ export = nodeunit.testCase({

test.done();
},

'renders "Properties" for a resource that has only properties set to "false"'(test: nodeunit.Test) {
const app = new core.App();
const stack = new core.Stack(app, 'TestStack');
new core.CfnResource(stack, 'Resource', {
type: 'Test::Resource::Fake',
properties: {
FakeProperty: false,
},
});

test.deepEqual(app.synth().getStackByName(stack.stackName).template, {
Resources: {
Resource: {
Type: 'Test::Resource::Fake',
Properties: {
FakeProperty: false,
},
},
},
});

test.done();
},
},

'applyRemovalPolicy default includes Update policy'(test: nodeunit.Test) {
Expand Down

0 comments on commit b42d4e9

Please sign in to comment.