fix(cdk): only make Outputs Exports when necessary

Export names must be unique and can conflict, so automatically turning
every Output into an Export can lead to problems for customers who
deploy the same template multiple times. Especially when the outputs
are created for them and they have no control over them.

We'll turn Outputs into exports on-demand (when .makeImportValue() is
called).

Fixes #903, fixes #1611.

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

@@ -16,21 +16,22 @@ export interface OutputProps {
   value?: any;
 
   /**
-   * The name used to export the value of this output across stacks. To import
-   * the value from another stack, use `FnImportValue(export)`. You can create
-   * an import value token by calling `output.makeImportValue()`.
+   * The name used to export the value of this output across stacks.
    *
-   * @default The default behavior is to automatically allocate an export name
-   * for outputs based on the stack name and the output's logical ID. To
-   * create an output without an export, set `disableExport: true`.
+   * To import the value from another stack, use `FnImportValue(export)`. You
+   * can create an import value token by calling `output.makeImportValue()`.
+   *
+   * @default Automatically allocate a name when `makeImportValue()`  is
+   * called.
    */
   export?: string;
 
   /**
    * Disables the automatic allocation of an export name for this output.
    *
-   * @default false, which means that an export name is either explicitly
-   * specified or allocated based on the output's logical ID and stack name.
+   * This disables use of `makeImportValue()` if `export` is  not given.
+   *
+   * @default false
    */
   disableExport?: boolean;
 
@@ -52,8 +53,10 @@ export class Output extends StackElement {
   /**
    * The name of the resource output to be exported for a cross-stack reference.
    * By default, the logical ID of the Output element is used as it's export name.
+   *
+   * May be undefined if the Output hasn't been exported yet.
    */
-  public readonly export?: string;
+  public export?: string;
 
   /**
    * A condition from the "Conditions" section to associate with this output
@@ -64,6 +67,8 @@ export class Output extends StackElement {
 
   private _value?: any;
 
+  private disableExport: boolean;
+
   /**
    * Creates an Output value for this stack.
    * @param parent The parent construct.
@@ -76,16 +81,16 @@ export class Output extends StackElement {
     this._value = props.value;
     this.condition = props.condition;
 
+    this.disableExport = props.disableExport !== undefined ? props.disableExport : false;
+
+    if (props.export && this.disableExport) {
+      throw new Error('Cannot set `disableExport` and specify an export name');
+    }
+
+    this.export = props.export;
+
     if (props.export) {
-      if (props.disableExport) {
-        throw new Error('Cannot set `disableExport` and specify an export name');
-      }
       this.export = props.export;
-    } else if (!props.disableExport) {
-      // prefix export name with stack name since exports are global within account + region.
-      const stackName = require('./stack').Stack.find(this).node.id;
-      this.export = stackName ? stackName + ':' : '';
-      this.export += this.logicalId;
     }
   }
 
@@ -102,8 +107,11 @@ export class Output extends StackElement {
    * Returns an FnImportValue bound to this export name.
    */
   public makeImportValue() {
+    if (!this.export && this.disableExport) {
+      throw new Error('Cannot create an ImportValue; `disableExport` has been set.');
+    }
     if (!this.export) {
-      throw new Error('Cannot create an ImportValue without an export name');
+      this.export = this.uniqueOutputName();
     }
     return fn().importValue(this.export);
   }
@@ -124,6 +132,19 @@ export class Output extends StackElement {
   public get ref(): string {
     throw new Error('Outputs cannot be referenced');
   }
+
+  /**
+   * Automatically determine an output name for use with FnImportValue
+   *
+   * This gets called in case the user hasn't specified an export name but is
+   * taking an action that requires exporting. We namespace with the stack name
+   * to reduce chances of collissions between CDK apps.
+   */
+  private uniqueOutputName() {
+    // prefix export name with stack name since exports are global within account + region.
+    const stackName = require('./stack').Stack.find(this).name;
+    return (stackName ? stackName + ':' : '') + this.logicalId;
+  }
 }
 
 /**

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

@@ -1,5 +1,5 @@
 import { Test } from 'nodeunit';
-import { Construct, Output, Ref, Resource, Stack } from '../../lib';
+import { Output, Ref, Resource, Stack } from '../../lib';
 
 export = {
   'outputs can be added to the stack'(test: Test) {
@@ -28,16 +28,6 @@ export = {
     test.done();
   },
 
-  'outputs have a default unique export name'(test: Test) {
-    const stack = new Stack(undefined, 'MyStack');
-    const output = new Output(stack, 'MyOutput');
-    const child = new Construct(stack, 'MyConstruct');
-    const output2 = new Output(child, 'MyOutput2');
-    test.equal(output.export, 'MyStack:MyOutput');
-    test.equal(output2.export, 'MyStack:MyConstructMyOutput255322D15');
-    test.done();
-  },
-
   'disableExport can be used to disable the auto-export behavior'(test: Test) {
     const stack = new Stack();
     const output = new Output(stack, 'MyOutput', { disableExport: true });
@@ -56,14 +46,41 @@ export = {
   'is stack name is undefined, we will only use the logical ID for the export name'(test: Test) {
     const stack = new Stack();
     const output = new Output(stack, 'MyOutput');
-    test.equal(output.export, 'MyOutput');
+    test.deepEqual(stack.node.resolve(output.makeImportValue()), { 'Fn::ImportValue': '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 Output(stack, 'MyOutput');
     test.deepEqual(stack.node.resolve(output.makeImportValue()), { 'Fn::ImportValue': 'MyStack:MyOutput' });
+
+    test.deepEqual(stack.toCloudFormation(), {
+      Outputs: {
+        MyOutput: {
+          Export: { Name: 'MyStack:MyOutput' }
+        }
+      }
+    });
+    test.done();
+  },
+
+  'No export is created by default'(test: Test) {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    new Output(stack, 'SomeOutput', { value: 'x' });
+
+    // THEN
+    test.deepEqual(stack.toCloudFormation(), {
+      Outputs: {
+        SomeOutput: {
+          Value: 'x'
+        }
+      }
+    });
+
     test.done();
-  }
+  },
 };