Skip to content

Commit

Permalink
feat(core): verify CfnOutput has a value and fix VPC export (#2219)
Browse files Browse the repository at this point in the history
Throw an error if `value` is undefined when defining a `CfnOutput`.

Fixes #2012 such that a VPC can be exported without a VPN gateway (there
is already unit test coverage).
  • Loading branch information
Elad Ben-Israel authored Apr 10, 2019
1 parent 109625d commit 9e87661
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 12 deletions.
6 changes: 5 additions & 1 deletion packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,9 +483,13 @@ export class VpcNetwork extends VpcNetworkBase {
const priv = new ExportSubnetGroup(this, 'PrivateSubnetIDs', this.privateSubnets, SubnetType.Private, this.availabilityZones.length);
const iso = new ExportSubnetGroup(this, 'IsolatedSubnetIDs', this.isolatedSubnets, SubnetType.Isolated, this.availabilityZones.length);

const vpnGatewayId = this.vpnGatewayId
? new cdk.CfnOutput(this, 'VpnGatewayId', { value: this.vpnGatewayId }).makeImportValue().toString()
: undefined;

return {
vpcId: new cdk.CfnOutput(this, 'VpcId', { value: this.vpcId }).makeImportValue().toString(),
vpnGatewayId: new cdk.CfnOutput(this, 'VpnGatewayId', { value: this.vpnGatewayId }).makeImportValue().toString(),
vpnGatewayId,
availabilityZones: this.availabilityZones,
publicSubnetIds: pub.ids,
publicSubnetNames: pub.names,
Expand Down
8 changes: 6 additions & 2 deletions packages/@aws-cdk/cdk/lib/cfn-output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface CfnOutputProps {
* The value of an output can include literals, parameter references, pseudo-parameters,
* a mapping value, or intrinsic functions.
*/
readonly value?: any;
readonly value: any;

/**
* The name used to export the value of this output across stacks.
Expand Down Expand Up @@ -75,9 +75,13 @@ export class CfnOutput extends CfnElement {
* @param parent The parent construct.
* @param props CfnOutput properties.
*/
constructor(scope: Construct, id: string, props: CfnOutputProps = {}) {
constructor(scope: Construct, id: string, props: CfnOutputProps) {
super(scope, id);

if (props.value === undefined) {
throw new Error(`Missing value for CloudFormation output at path "${this.node.path}"`);
}

this.description = props.description;
this._value = props.value;
this.condition = props.condition;
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/cdk/test/test.include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export = {

new Include(stack, 'T1', { template: clone(template) });
new CfnResource(stack, 'MyResource3', { type: 'ResourceType3', properties: { P3: 'Hello' } });
new CfnOutput(stack, 'MyOutput', { description: 'Out!', disableExport: true });
new CfnOutput(stack, 'MyOutput', { description: 'Out!', disableExport: true, value: 'hey' });
new CfnParameter(stack, 'MyParam2', { type: 'Integer' });

test.deepEqual(stack._toCloudFormation(), {
Expand All @@ -33,7 +33,7 @@ export = {
MyResource2: { Type: 'ResourceType2' },
MyResource3: { Type: 'ResourceType3', Properties: { P3: 'Hello' } } },
Outputs: {
MyOutput: { Description: 'Out!' } } });
MyOutput: { Description: 'Out!', Value: 'hey' } } });

test.done();
},
Expand All @@ -43,7 +43,7 @@ export = {

new Include(stack, 'T1', { template });
new CfnResource(stack, 'MyResource3', { type: 'ResourceType3', properties: { P3: 'Hello' } });
new CfnOutput(stack, 'MyOutput', { description: 'Out!' });
new CfnOutput(stack, 'MyOutput', { description: 'Out!', value: 'in' });
new CfnParameter(stack, 'MyParam', { type: 'Integer' }); // duplicate!

test.throws(() => stack._toCloudFormation());
Expand Down
12 changes: 7 additions & 5 deletions packages/@aws-cdk/cdk/test/test.output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,41 +23,43 @@ export = {

'outputs cannot be referenced'(test: Test) {
const stack = new Stack();
const output = new CfnOutput(stack, 'MyOutput', { description: 'My CfnOutput' });
const output = new CfnOutput(stack, 'MyOutput', { description: 'My CfnOutput', value: 'boom' });
test.throws(() => output.ref);
test.done();
},

'disableExport can be used to disable the auto-export behavior'(test: Test) {
const stack = new Stack();
const output = new CfnOutput(stack, 'MyOutput', { disableExport: true });
const output = new CfnOutput(stack, 'MyOutput', { disableExport: true, value: 'boom' });

test.equal(output.export, null);

// cannot specify `export` and `disableExport` at the same time.
test.throws(() => new CfnOutput(stack, 'YourOutput', {
disableExport: true,
export: 'bla'
export: 'bla',
value: 'boom'
}), /Cannot set `disableExport` and specify an export name/);

test.done();
},

'if stack name is undefined, we will only use the logical ID for the export name'(test: Test) {
const stack = new Stack();
const output = new CfnOutput(stack, 'MyOutput');
const output = new CfnOutput(stack, 'MyOutput', { value: 'boom' });
test.deepEqual(stack.node.resolve(output.makeImportValue()), { 'Fn::ImportValue': 'Stack:MyOutput' });
test.done();
},

'makeImportValue can be used to create an Fn::ImportValue from an output'(test: Test) {
const stack = new Stack(undefined, 'MyStack');
const output = new CfnOutput(stack, 'MyOutput');
const output = new CfnOutput(stack, 'MyOutput', { value: 'boom' });
test.deepEqual(stack.node.resolve(output.makeImportValue()), { 'Fn::ImportValue': 'MyStack:MyOutput' });

test.deepEqual(stack._toCloudFormation(), {
Outputs: {
MyOutput: {
Value: 'boom',
Export: { Name: 'MyStack:MyOutput' }
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/test/test.stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export = {
const stack = new Stack();

const p = new CfnParameter(stack, 'MyParam', { type: 'String' });
const o = new CfnOutput(stack, 'MyOutput');
const o = new CfnOutput(stack, 'MyOutput', { value: 'boom' });
const c = new CfnCondition(stack, 'MyCondition');

test.equal(stack.node.findChild(p.node.path), p);
Expand Down

0 comments on commit 9e87661

Please sign in to comment.