fix(core): logicalId is consumed prior to being overridden

packages/@aws-cdk/core/lib/cfn-element.ts

@@ -47,6 +47,14 @@ export abstract class CfnElement extends CoreConstruct {
    */
   private _logicalIdOverride?: string;
 
+  /**
+   * If the logicalId is locked then it can no longer be overridden.
+   * This is needed for cases where the logicalId is consumed prior to synthesis
+   * (i.e. Stack.exportValue).
+   */
+  private _logicalIdLocked?: boolean;
+
+
   /**
    * Creates an entity and binds it to a tree.
    * Note that the root of the tree must be a Stack object (not just any Root).
@@ -75,7 +83,37 @@ export abstract class CfnElement extends CoreConstruct {
    * @param newLogicalId The new logical ID to use for this stack element.
    */
   public overrideLogicalId(newLogicalId: string) {
-    this._logicalIdOverride = newLogicalId;
+    if (this._logicalIdLocked) {
+      throw new Error(`The logicalId for resource at path ${Node.of(this).path} has been locked and cannot be overridden\n` +
+        'Make sure you are calling "overrideLogicalId" before Stack.exportValue');
+    } else {
+      this._logicalIdOverride = newLogicalId;
+    }
+  }
+
+  /**
+   * Lock the logicalId of the element and do not allow
+   * any updates (e.g. via overrideLogicalId)
+   *
+   * This is needed in cases where you are consuming the LogicalID
+   * of an element prior to synthesis and you need to not allow future
+   * changes to the id since doing so would cause the value you just
+   * consumed to differ from the synth time value of the logicalId.
+   *
+   * For example:
+   *
+   * const bucket = new Bucket(stack, 'Bucket');
+   * stack.exportValue(bucket.bucketArn) <--- consuming the logicalId
+   * bucket.overrideLogicalId('NewLogicalId') <--- updating logicalId
+   *
+   * You should most likely never need to use this method, and if
+   * you are implementing a feature that requires this, make sure
+   * you actually require it.
+   *
+   * @internal
+   */
+  public _lockLogicalId(): void {
+    this._logicalIdLocked = true;
   }
 
   /**

packages/@aws-cdk/core/lib/stack.ts

@@ -907,6 +907,14 @@ export class Stack extends CoreConstruct implements ITaggable {
       throw new Error('exportValue: either supply \'name\' or make sure to export a resource attribute (like \'bucket.bucketName\')');
     }
 
+    // if exportValue is being called manually (which is pre onPrepare) then the logicalId
+    // could potentially be changed by a call to overrideLogicalId. This would cause our Export/Import
+    // to have an incorrect id. For a better user experience, lock the logicalId and throw an error
+    // if the user tries to override the id _after_ calling exportValue
+    if (CfnElement.isCfnElement(resolvable.target)) {
+      resolvable.target._lockLogicalId();
+    }
+
     // "teleport" the value here, in case it comes from a nested stack. This will also
     // ensure the value is from our own scope.
     const exportable = referenceNestedStackValueInParent(resolvable, this);

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

@@ -576,6 +576,52 @@ describe('stack', () => {
     expect(templateA).toEqual(templateM);
   });
 
+  test('throw error if overrideLogicalId is used and logicalId is locked', () => {
+    // GIVEN: manual
+    const appM = new App();
+    const producerM = new Stack(appM, 'Producer');
+    const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
+    producerM.exportValue(resourceM.getAtt('Att'));
+
+    // THEN - producers are the same
+    expect(() => {
+      resourceM.overrideLogicalId('OVERRIDE_LOGICAL_ID');
+    }).toThrow(/The logicalId for resource at path Producer\/ResourceXXX has been locked and cannot be overridden/);
+  });
+
+  test('do not throw error if overrideLogicalId is used and logicalId is not locked', () => {
+    // GIVEN: manual
+    const appM = new App();
+    const producerM = new Stack(appM, 'Producer');
+    const resourceM = new CfnResource(producerM, 'ResourceXXX', { type: 'AWS::Resource' });
+
+    // THEN - producers are the same
+    resourceM.overrideLogicalId('OVERRIDE_LOGICAL_ID');
+    producerM.exportValue(resourceM.getAtt('Att'));
+
+    const template = appM.synth().getStackByName(producerM.stackName).template;
+    expect(template).toEqual({
+      Outputs: {
+        ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019: {
+          Export: {
+            Name: 'Producer:ExportsOutputFnGetAttOVERRIDELOGICALIDAtt2DD28019',
+          },
+          Value: {
+            'Fn::GetAtt': [
+              'OVERRIDE_LOGICAL_ID',
+              'Att',
+            ],
+          },
+        },
+      },
+      Resources: {
+        OVERRIDE_LOGICAL_ID: {
+          Type: 'AWS::Resource',
+        },
+      },
+    });
+  });
+
   test('automatic cross-stack references and manual exports look the same: nested stack edition', () => {
     // GIVEN: automatic
     const appA = new App();