feat(core): verify CfnOutput has a value and fix VPC export (#2219)

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).

Author: Elad Ben-Israel

packages/@aws-cdk/aws-ec2/lib/vpc.ts

@@ -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,

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

@@ -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.
@@ -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;

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

@@ -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(), {
@@ -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();
   },
@@ -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());

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

@@ -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' }
         }
       }

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

@@ -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);