fix(core): cannot use the same stack name for multiple stacks (under …

…feature flag) (#4895)

Since we used the stack name as the template file name, if users wanted to use the same stack name in two environments, the emitted templates overwrote each other.

Furthermore, the CLI used the artifact ID as the stack name, so this became a bit more complex. This means that `assembly.getStack()` is now ambiguous, so I renamed it `getStackByName` which fails if there are two stacks with the same name (legitimate) and added `getStackArtifact` which uses the artifact ID.

The core library will effectively generate identical cloud assemblies if the stack name and artifact IDs are the same, and to ensure backwards compat, no existing tests have been changed (albeit it would have been more correct to replace all `getStackByName` with `getStackArtifact`, but effectively this is the same thing if they are equal).

We want the template file name to use the artifact ID instead of the physical stack name but this can break users that depend on this behaviour (despite the fact that it's a formal API). To avoid this, we will only enable this new behaviour behind a [feature flag](#4925) which means that it will only be enabled for new projects created through `cdk init`, but old projects will still get the old behaviour.

RFC for feature flags: #4925

Fixes #4412

BREAKING CHANGE: template file names in `cdk.out` for new projects created by `cdk init` will use `stack.artifactId` instead of the physical stack name to enable multiple stacks to use the same name. In most cases the artifact ID is the same as the stack name. To enable this fix for old projects, add the context key `@aws-cdk/core:enableStackNameDuplicates: true` in your `cdk.json` file.

packages/@aws-cdk/assert/lib/inspector.ts

@@ -63,7 +63,7 @@ export class StackPathInspector extends Inspector {
     // then try with the stack name preprended for backwards compat with most tests that happen to give
     // their stack an ID that's the same as the stack name.
     const metadata = this.stack.manifest.metadata || {};
-    const md = metadata[this.path] || metadata[`/${this.stack.name}${this.path}`];
+    const md = metadata[this.path] || metadata[`/${this.stack.id}${this.path}`];
     if (md === undefined) { return undefined; }
     const resourceMd = md.find(entry => entry.type === api.LOGICAL_ID_METADATA_KEY);
     if (resourceMd === undefined) { return undefined; }

packages/@aws-cdk/assert/lib/synth-utils.ts

@@ -11,7 +11,7 @@ export class SynthUtils {
     // if the root is an app, invoke "synth" to avoid double synthesis
     const assembly = root instanceof App ? root.synth() : ConstructNode.synth(root.node, options);
 
-    return assembly.getStack(stack.stackName);
+    return assembly.getStackArtifact(stack.artifactId);
   }
 
   /**
@@ -61,7 +61,7 @@ export class SynthUtils {
       return JSON.parse(fs.readFileSync(path.join(assembly.directory, stack.templateFile)).toString('utf-8'));
     }
 
-    return assembly.getStack(stack.stackName);
+    return assembly.getStackArtifact(stack.artifactId);
   }
 }
 

packages/@aws-cdk/assert/test/test.assertions.ts

@@ -265,7 +265,7 @@ function synthesizedStack(fn: (stack: cdk.Stack) => void): cx.CloudFormationStac
   fn(stack);
 
   const assembly = app.synth();
-  return assembly.getStack(stack.stackName);
+  return assembly.getStackArtifact(stack.artifactId);
 }
 
 interface TestResourceProps extends cdk.CfnResourceProps {

packages/@aws-cdk/assert/test/test.have-resource.ts

@@ -103,5 +103,5 @@ function mkStack(template: any): cxapi.CloudFormationStackArtifact {
   });
 
   writeFileSync(join(assembly.outdir, 'template.json'), JSON.stringify(template));
-  return assembly.buildAssembly().getStack('test');
+  return assembly.buildAssembly().getStackByName('test');
 }

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

@@ -84,7 +84,7 @@ export = {
     const assembly = app.synth();
 
     // THEN
-    test.deepEqual(assembly.getStack(parent.stackName).assets, [{
+    test.deepEqual(assembly.getStackByName(parent.stackName).assets, [{
       path: 'parentstacknestedstack844892C0.nested.template.json',
       id: 'c639c0a5e7320758aa22589669ecebc98f185b711300b074f53998c8f9a45096',
       packaging: 'file',
@@ -418,8 +418,8 @@ export = {
     });
 
     // verify a depedency was established between the parents
-    const stack1Artifact = assembly.getStack(stack1.stackName);
-    const stack2Artifact = assembly.getStack(stack2.stackName);
+    const stack1Artifact = assembly.getStackByName(stack1.stackName);
+    const stack2Artifact = assembly.getStackByName(stack2.stackName);
     test.deepEqual(stack1Artifact.dependencies.length, 1);
     test.deepEqual(stack2Artifact.dependencies.length, 0);
     test.same(stack1Artifact.dependencies[0], stack2Artifact);
@@ -465,7 +465,7 @@ export = {
     });
 
     // parent stack (stack1) should export this value
-    test.deepEqual(assembly.getStack(stack1.stackName).template.Outputs, {
+    test.deepEqual(assembly.getStackByName(stack1.stackName).template.Outputs, {
       ExportsOutputFnGetAttNestedUnderStack1NestedStackNestedUnderStack1NestedStackResourceF616305BOutputsStack1NestedUnderStack1ResourceInNestedStack6EE9DCD2MyAttribute564EECF3: {
         Value: { 'Fn::GetAtt': ['NestedUnderStack1NestedStackNestedUnderStack1NestedStackResourceF616305B', 'Outputs.Stack1NestedUnderStack1ResourceInNestedStack6EE9DCD2MyAttribute'] },
         Export: { Name: 'Stack1:ExportsOutputFnGetAttNestedUnderStack1NestedStackNestedUnderStack1NestedStackResourceF616305BOutputsStack1NestedUnderStack1ResourceInNestedStack6EE9DCD2MyAttribute564EECF3' }
@@ -487,8 +487,8 @@ export = {
     });
 
     test.deepEqual(assembly.stacks.length, 2);
-    const stack1Artifact = assembly.getStack(stack1.stackName);
-    const stack2Artifact = assembly.getStack(stack2.stackName);
+    const stack1Artifact = assembly.getStackByName(stack1.stackName);
+    const stack2Artifact = assembly.getStackByName(stack2.stackName);
     test.deepEqual(stack1Artifact.dependencies.length, 0);
     test.deepEqual(stack2Artifact.dependencies.length, 1);
     test.same(stack2Artifact.dependencies[0], stack1Artifact);
@@ -718,7 +718,7 @@ export = {
     }));
 
     // parent stack should have 2 assets
-    test.deepEqual(assembly.getStack(parent.stackName).assets.length, 2);
+    test.deepEqual(assembly.getStackByName(parent.stackName).assets.length, 2);
     test.done();
   },
 
@@ -764,7 +764,7 @@ export = {
     }));
 
     // parent stack should have 2 assets
-    test.deepEqual(assembly.getStack(parent.stackName).assets.length, 2);
+    test.deepEqual(assembly.getStackByName(parent.stackName).assets.length, 2);
     test.done();
   },
 
@@ -819,8 +819,8 @@ export = {
     // THEN: the first non-nested stack records the assembly metadata
     const asm = app.synth();
     test.deepEqual(asm.stacks.length, 2); // only one stack is defined as an artifact
-    test.deepEqual(asm.getStack(parent.stackName).findMetadataByType('foo'), []);
-    test.deepEqual(asm.getStack(child.stackName).findMetadataByType('foo'), [
+    test.deepEqual(asm.getStackByName(parent.stackName).findMetadataByType('foo'), []);
+    test.deepEqual(asm.getStackByName(child.stackName).findMetadataByType('foo'), [
       {
         path: '/parent/child/nested/resource',
         type: 'foo',

packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts

@@ -655,7 +655,7 @@ export = {
 
     // THEN
     const assembly = app.synth();
-    const template = assembly.getStack(stack.stackName).template;
+    const template = assembly.getStackByName(stack.stackName).template;
     expect(stack).to(haveResource("AWS::AutoScaling::LaunchConfiguration", {
       ImageId: {
         Ref: "SsmParameterValueawsserviceecsoptimizedamiamazonlinux2gpurecommendedimageidC96584B6F00A464EAD1953AFF4B05118Parameter"
@@ -709,7 +709,7 @@ export = {
 
     // THEN
     const assembly = app.synth();
-    const template = assembly.getStack(stack.stackName).template;
+    const template = assembly.getStackByName(stack.stackName).template;
     test.deepEqual(template.Parameters, {
       SsmParameterValueawsserviceecsoptimizedamiwindowsserver2019englishfullrecommendedimageidC96584B6F00A464EAD1953AFF4B05118Parameter: {
         Type: "AWS::SSM::Parameter::Value<AWS::EC2::Image::Id>",
@@ -846,7 +846,7 @@ export = {
 
     // THEN
     const assembly = app.synth();
-    const template = assembly.getStack(stack.stackName).template;
+    const template = assembly.getStackByName(stack.stackName).template;
     expect(stack).to(haveResource("AWS::AutoScaling::LaunchConfiguration", {
       ImageId: {
         Ref: "SsmParameterValueawsserviceecsoptimizedamiamazonlinux2gpurecommendedimageidC96584B6F00A464EAD1953AFF4B05118Parameter"
@@ -877,7 +877,7 @@ export = {
 
     // THEN
     const assembly = app.synth();
-    const template = assembly.getStack(stack.stackName).template;
+    const template = assembly.getStackByName(stack.stackName).template;
     expect(stack).to(haveResource("AWS::AutoScaling::LaunchConfiguration", {
       ImageId: {
         Ref: "SsmParameterValueawsserviceecsoptimizedamiamazonlinuxrecommendedimageidC96584B6F00A464EAD1953AFF4B05118Parameter"
@@ -908,7 +908,7 @@ export = {
 
     // THEN
     const assembly = app.synth();
-    const template = assembly.getStack(stack.stackName).template;
+    const template = assembly.getStackByName(stack.stackName).template;
     test.deepEqual(template.Parameters, {
       SsmParameterValueawsserviceecsoptimizedamiwindowsserver2019englishfullrecommendedimageidC96584B6F00A464EAD1953AFF4B05118Parameter: {
         Type: "AWS::SSM::Parameter::Value<AWS::EC2::Image::Id>",

packages/@aws-cdk/aws-eks/test/test.cluster.ts

@@ -311,7 +311,7 @@ export = {
 
       // THEN
       const assembly = app.synth();
-      const template = assembly.getStack(stack.stackName).template;
+      const template = assembly.getStackByName(stack.stackName).template;
       test.deepEqual(template.Outputs, {
         ClusterConfigCommand43AAE40F: { Value: { 'Fn::Join': [ '', [ 'aws eks update-kubeconfig --name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1' ] ] } },
         ClusterGetTokenCommand06AE992E: { Value: { 'Fn::Join': [ '', [ 'aws eks get-token --cluster-name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1' ] ] } }
@@ -329,7 +329,7 @@ export = {
 
       // THEN
       const assembly = app.synth();
-      const template = assembly.getStack(stack.stackName).template;
+      const template = assembly.getStackByName(stack.stackName).template;
       test.deepEqual(template.Outputs, {
         ClusterConfigCommand43AAE40F: { Value: { 'Fn::Join': [ '', [ 'aws eks update-kubeconfig --name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1 --role-arn ', { 'Fn::GetAtt': [ 'masters0D04F23D', 'Arn' ] } ] ] } },
         ClusterGetTokenCommand06AE992E: { Value: { 'Fn::Join': [ '', [ 'aws eks get-token --cluster-name ', { Ref: 'Cluster9EE0221C' }, ' --region us-east-1 --role-arn ', { 'Fn::GetAtt': [ 'masters0D04F23D', 'Arn' ] } ] ] } }
@@ -350,7 +350,7 @@ export = {
 
       // THEN
       const assembly = app.synth();
-      const template = assembly.getStack(stack.stackName).template;
+      const template = assembly.getStackByName(stack.stackName).template;
       test.ok(!template.Outputs); // no outputs
       test.done();
     },
@@ -367,7 +367,7 @@ export = {
 
       // THEN
       const assembly = app.synth();
-      const template = assembly.getStack(stack.stackName).template;
+      const template = assembly.getStackByName(stack.stackName).template;
       test.deepEqual(template.Outputs, {
         ClusterClusterNameEB26049E: { Value: { Ref: 'Cluster9EE0221C' } }
       });
@@ -387,7 +387,7 @@ export = {
 
       // THEN
       const assembly = app.synth();
-      const template = assembly.getStack(stack.stackName).template;
+      const template = assembly.getStackByName(stack.stackName).template;
       test.deepEqual(template.Outputs, {
         ClusterMastersRoleArnB15964B1: { Value: { 'Fn::GetAtt': [ 'masters0D04F23D', 'Arn' ] } }
       });
@@ -406,7 +406,7 @@ export = {
 
       // THEN
       const assembly = app.synth();
-      const template = assembly.getStack(stack.stackName).template;
+      const template = assembly.getStackByName(stack.stackName).template;
       test.deepEqual(template.Outputs, {
         ClusterDefaultCapacityInstanceRoleARN7DADF219: {
           Value: { 'Fn::GetAtt': [ 'ClusterDefaultCapacityInstanceRole3E209969', 'Arn' ] }
@@ -427,7 +427,7 @@ export = {
       cluster.addCapacity('MyCapcity', { instanceType: new ec2.InstanceType('m3.xlargs') });
 
       // THEN
-      const template = app.synth().getStack(stack.stackName).template;
+      const template = app.synth().getStackByName(stack.stackName).template;
       const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData;
       test.deepEqual(userData, { 'Fn::Base64': { 'Fn::Join': [ '', [ '#!/bin/bash\nset -o xtrace\n/etc/eks/bootstrap.sh ', { Ref: 'Cluster9EE0221C' }, ' --kubelet-extra-args "--node-labels lifecycle=OnDemand" --use-max-pods true\n/opt/aws/bin/cfn-signal --exit-code $? --stack Stack --resource ClusterMyCapcityASGD4CD8B97 --region us-east-1' ] ] } });
       test.done();
@@ -445,7 +445,7 @@ export = {
       });
 
       // THEN
-      const template = app.synth().getStack(stack.stackName).template;
+      const template = app.synth().getStackByName(stack.stackName).template;
       const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData;
       test.deepEqual(userData, { "Fn::Base64": "#!/bin/bash" });
       test.done();
@@ -466,7 +466,7 @@ export = {
       });
 
       // THEN
-      const template = app.synth().getStack(stack.stackName).template;
+      const template = app.synth().getStackByName(stack.stackName).template;
       const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData;
       test.deepEqual(userData, { 'Fn::Base64': { 'Fn::Join': [ '', [ '#!/bin/bash\nset -o xtrace\n/etc/eks/bootstrap.sh ', { Ref: 'Cluster9EE0221C' }, ' --kubelet-extra-args "--node-labels lifecycle=OnDemand  --node-labels FOO=42" --use-max-pods true\n/opt/aws/bin/cfn-signal --exit-code $? --stack Stack --resource ClusterMyCapcityASGD4CD8B97 --region us-east-1' ] ] } });
       test.done();
@@ -486,7 +486,7 @@ export = {
         });
 
         // THEN
-        const template = app.synth().getStack(stack.stackName).template;
+        const template = app.synth().getStackByName(stack.stackName).template;
         const userData = template.Resources.ClusterMyCapcityLaunchConfig58583345.Properties.UserData;
         test.deepEqual(userData, { 'Fn::Base64': { 'Fn::Join': [ '', [ '#!/bin/bash\nset -o xtrace\n/etc/eks/bootstrap.sh ', { Ref: 'Cluster9EE0221C' }, ' --kubelet-extra-args "--node-labels lifecycle=Ec2Spot --register-with-taints=spotInstance=true:PreferNoSchedule" --use-max-pods true\n/opt/aws/bin/cfn-signal --exit-code $? --stack Stack --resource ClusterMyCapcityASGD4CD8B97 --region us-east-1' ] ] } });
         test.done();

packages/@aws-cdk/aws-s3-assets/test/test.asset.ts

@@ -60,7 +60,7 @@ export = {
       path: dirPath
     });
 
-    const synth = app.synth().getStack(stack.stackName);
+    const synth = app.synth().getStackByName(stack.stackName);
     const meta = synth.manifest.metadata || {};
     test.ok(meta['/my-stack']);
     test.ok(meta['/my-stack'][0]);
@@ -340,7 +340,7 @@ export = {
 
       // WHEN
       const session = app.synth();
-      const artifact = session.getStack(stack.stackName);
+      const artifact = session.getStackByName(stack.stackName);
       const metadata = artifact.manifest.metadata || {};
       const md = Object.values(metadata)[0]![0]!.data;
       test.deepEqual(md.path, 'asset.6b84b87243a4a01c592d78e1fd3855c4bfef39328cd0a450cc97e81717fea2a2');

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

@@ -172,6 +172,11 @@ export class Stack extends Construct implements ITaggable {
    */
   public readonly templateFile: string;
 
+  /**
+   * The ID of the cloud assembly artifact for this stack.
+   */
+  public readonly artifactId: string;
+
   /**
    * Logical ID generation strategy
    */
@@ -201,12 +206,14 @@ export class Stack extends Construct implements ITaggable {
    * Creates a new stack.
    *
    * @param scope Parent of this stack, usually a Program instance.
-   * @param name The name of the CloudFormation stack. Defaults to "Stack".
+   * @param id The construct ID of this stack. If `stackName` is not explicitly
+   * defined, this id (and any parent IDs) will be used to determine the
+   * physical ID of the stack.
    * @param props Stack properties.
    */
-  public constructor(scope?: Construct, name?: string, props: StackProps = {}) {
+  public constructor(scope?: Construct, id?: string, props: StackProps = {}) {
     // For unit test convenience parents are optional, so bypass the type check when calling the parent.
-    super(scope!, name!);
+    super(scope!, id!);
 
     Object.defineProperty(this, STACK_SYMBOL, { value: true });
 
@@ -227,14 +234,26 @@ export class Stack extends Construct implements ITaggable {
       this.templateOptions.description = props.description;
     }
 
-    this._stackName = props.stackName !== undefined ? props.stackName : this.calculateStackName();
+    this._stackName = props.stackName !== undefined ? props.stackName : this.generateUniqueStackName();
     this.tags = new TagManager(TagType.KEY_VALUE, 'aws:cdk:stack', props.tags);
 
     if (!VALID_STACK_NAME_REGEX.test(this.stackName)) {
-      throw new Error(`Stack name must match the regular expression: ${VALID_STACK_NAME_REGEX.toString()}, got '${name}'`);
+      throw new Error(`Stack name must match the regular expression: ${VALID_STACK_NAME_REGEX.toString()}, got '${id}'`);
     }
 
-    this.templateFile = `${this.stackName}.template.json`;
+    // we use `generateUniqueStackName` here as the artifact ID. This will
+    // ensure that in case where `stackName` is not explicitly configured,
+    // artifact ID and stack name will be the same and therefore the template
+    // file name will be the same as `<stackName>.template.json` (for backwards
+    // compatibility with the behavior before we
+    // ENABLE_STACK_NAME_DUPLICATES_CONTEXT was introduced).
+    this.artifactId = this.generateUniqueStackName();
+
+    const templateFileName = this.node.tryGetContext(cxapi.ENABLE_STACK_NAME_DUPLICATES_CONTEXT)
+      ? this.artifactId
+      : this.stackName;
+
+    this.templateFile = `${templateFileName}.template.json`;
     this.templateUrl = Lazy.stringValue({ produce: () => this._templateUrl || '<unresolved>' });
   }
 
@@ -704,15 +723,27 @@ export class Stack extends Construct implements ITaggable {
       return;
     }
 
-    const deps = this.dependencies.map(s => s.stackName);
+    const deps = this.dependencies.map(s => s.artifactId);
     const meta = this.collectMetadata();
 
+    // backwards compatibility since originally artifact ID was always equal to
+    // stack name the stackName attribute is optional and if it is not specified
+    // the CLI will use the artifact ID as the stack name. we *could have*
+    // always put the stack name here but wanted to minimize the risk around
+    // changes to the assembly manifest. so this means that as long as stack
+    // name and artifact ID are the same, the cloud assembly manifest will not
+    // change.
+    const stackNameProperty = this.stackName === this.artifactId
+      ? { }
+      : { stackName: this.stackName };
+
     const properties: cxapi.AwsCloudFormationStackProperties = {
-      templateFile: this.templateFile
+      templateFile: this.templateFile,
+      ...stackNameProperty
     };
 
     // add an artifact that represents this stack
-    builder.addArtifact(this.stackName, {
+    builder.addArtifact(this.artifactId, {
       type: cxapi.ArtifactType.AWS_CLOUDFORMATION_STACK,
       environment: this.environment,
       properties,
@@ -892,7 +923,7 @@ export class Stack extends Construct implements ITaggable {
   /**
    * Calculcate the stack name based on the construct path
    */
-  private calculateStackName() {
+  private generateUniqueStackName() {
     // In tests, it's possible for this stack to be the root object, in which case
     // we need to use it as part of the root path.
     const rootPath = this.node.scope !== undefined ? this.node.scopes.slice(1) : [this];