diff --git a/packages/@aws-cdk/core/lib/cfn-element.ts b/packages/@aws-cdk/core/lib/cfn-element.ts index 84126add4e13c..feac7bd320dac 100644 --- a/packages/@aws-cdk/core/lib/cfn-element.ts +++ b/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; } /** diff --git a/packages/@aws-cdk/core/lib/stack.ts b/packages/@aws-cdk/core/lib/stack.ts index 4e6287f72fc2b..6a9274b4bc1f0 100644 --- a/packages/@aws-cdk/core/lib/stack.ts +++ b/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); diff --git a/packages/@aws-cdk/core/test/stack.test.ts b/packages/@aws-cdk/core/test/stack.test.ts index d55352601503b..f0f1637ed54ad 100644 --- a/packages/@aws-cdk/core/test/stack.test.ts +++ b/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();