Skip to content

Commit

Permalink
fix(code): make CfnResource#_toCloudFormation null-safe (#3121)
Browse files Browse the repository at this point in the history
The `CfnResource#_toCloudFormation` method creates a `PostResolveToken`
with a post-processor that was not ready to handle the absence of
`Properties` on the resolved value. It is however possible that
`Properties` are missing when an object is created with the default
configuration (e.g: by `new sqs.CfnQueue(this, 'Queue');`).

This change makes the post-processor function correctly handle
`undefined` in this case.

Related #3093
  • Loading branch information
RomainMuller authored Jul 1, 2019
1 parent d40fd05 commit 71cb421
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
3 changes: 2 additions & 1 deletion packages/@aws-cdk/core/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ export class CfnResource extends CfnRefElement {
Metadata: ignoreEmpty(this.cfnOptions.metadata),
Condition: this.cfnOptions.condition && this.cfnOptions.condition.logicalId
}, props => {
props.Properties = this.renderProperties(props.Properties);
const renderedProps = this.renderProperties(props.Properties || {});
props.Properties = renderedProps && (Object.values(renderedProps).find(v => !!v) ? renderedProps : undefined);
return deepMerge(props, this.rawOverrides);
})
}
Expand Down
29 changes: 29 additions & 0 deletions packages/@aws-cdk/core/test/test.cfn-resource.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import nodeunit = require('nodeunit');
import core = require('../lib');

export = nodeunit.testCase({
'._toCloudFormation': {
'does not call renderProperties with an undefined value'(test: nodeunit.Test) {
const app = new core.App();
const stack = new core.Stack(app, 'TestStack');
const resource = new core.CfnResource(stack, 'DefaultResource', { type: 'Test::Resource::Fake' });

let called = false;
(resource as any).renderProperties = (val: any) => {
called = true;
test.notEqual(val, null);
};

test.deepEqual(app.synth().getStack(stack.stackName).template, {
Resources: {
DefaultResource: {
Type: 'Test::Resource::Fake'
}
}
});
test.ok(called, `renderProperties must be called called`);

test.done();
}
}
});

0 comments on commit 71cb421

Please sign in to comment.