Skip to content

Commit

Permalink
fix(core): unable to find stack by name using the cli in legacy mode (#…
Browse files Browse the repository at this point in the history
…4998)

Legacy mode for #4895 still used the uniquely generated id instead of the stack name as the artifact ID in the cloud assembly. The implications were that even if users were not opted-in to the new behavior (through the feature flag), they could not use the stack name in the CLI because the stack artifact ID was still new.

This fix ensures that if the feature flag is not enabled, the artifact ID itself uses the stack name, hence allowing users to query by stack name as long as they are not opted in to the new behavior.

Fixes #4997
  • Loading branch information
Elad Ben-Israel authored and RomainMuller committed Nov 13, 2019
1 parent 6bb29b5 commit e9a305c
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 26 deletions.
23 changes: 10 additions & 13 deletions packages/@aws-cdk/core/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,26 +234,23 @@ export class Stack extends Construct implements ITaggable {
this.templateOptions.description = props.description;
}

this._stackName = props.stackName !== undefined ? props.stackName : this.generateUniqueStackName();
this._stackName = props.stackName !== undefined ? props.stackName : this.generateUniqueId();
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 '${id}'`);
}

// 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
// the preferred behavior is to generate a unique id for this stack and use
// it as the artifact ID in the assembly. this allows multiple stacks to use
// the same name. however, this behavior is breaking for 1.x so it's only
// applied under a feature flag which is applied automatically for new
// projects created using `cdk init`.
this.artifactId = this.node.tryGetContext(cxapi.ENABLE_STACK_NAME_DUPLICATES_CONTEXT)
? this.generateUniqueId()
: this.stackName;

this.templateFile = `${templateFileName}.template.json`;
this.templateFile = `${this.artifactId}.template.json`;
this.templateUrl = Lazy.stringValue({ produce: () => this._templateUrl || '<unresolved>' });
}

Expand Down Expand Up @@ -923,7 +920,7 @@ export class Stack extends Construct implements ITaggable {
/**
* Calculcate the stack name based on the construct path
*/
private generateUniqueStackName() {
private generateUniqueId() {
// 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];
Expand Down
65 changes: 52 additions & 13 deletions packages/@aws-cdk/core/test/test.stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,21 +672,60 @@ export = {
test.done();
},

'allow using the same stack name for two stacks (i.e. in different regions)'(test: Test) {
// GIVEN
const app = new App({ context: cxapi.FUTURE_FLAGS });
'@aws-cdk/core:enableStackNameDuplicates': {

// WHEN
const stack1 = new Stack(app, 'MyStack1', { stackName: 'thestack' });
const stack2 = new Stack(app, 'MyStack2', { stackName: 'thestack' });
const assembly = app.synth();
'disabled (default)': {

'artifactId and templateFile use the stack name'(test: Test) {
// GIVEN
const app = new App();

// WHEN
const stack1 = new Stack(app, 'MyStack1', { stackName: 'thestack' });
const assembly = app.synth();

// THEN
test.deepEqual(stack1.artifactId, 'thestack');
test.deepEqual(stack1.templateFile, 'thestack.template.json');
test.deepEqual(assembly.getStackArtifact(stack1.artifactId).templateFile, 'thestack.template.json');
test.done();
}
},

'enabled': {
'allows using the same stack name for two stacks (i.e. in different regions)'(test: Test) {
// GIVEN
const app = new App({ context: { [cxapi.ENABLE_STACK_NAME_DUPLICATES_CONTEXT]: 'true' } });

// WHEN
const stack1 = new Stack(app, 'MyStack1', { stackName: 'thestack' });
const stack2 = new Stack(app, 'MyStack2', { stackName: 'thestack' });
const assembly = app.synth();

// THEN
test.deepEqual(assembly.getStackArtifact(stack1.artifactId).templateFile, 'MyStack1.template.json');
test.deepEqual(assembly.getStackArtifact(stack2.artifactId).templateFile, 'MyStack2.template.json');
test.deepEqual(stack1.templateFile, 'MyStack1.template.json');
test.deepEqual(stack2.templateFile, 'MyStack2.template.json');
test.done();
},

'artifactId and templateFile use the unique id and not the stack name'(test: Test) {
// GIVEN
const app = new App({ context: { [cxapi.ENABLE_STACK_NAME_DUPLICATES_CONTEXT]: 'true' } });

// WHEN
const stack1 = new Stack(app, 'MyStack1', { stackName: 'thestack' });
const assembly = app.synth();

// THEN
test.deepEqual(stack1.artifactId, 'MyStack1');
test.deepEqual(stack1.templateFile, 'MyStack1.template.json');
test.deepEqual(assembly.getStackArtifact(stack1.artifactId).templateFile, 'MyStack1.template.json');
test.done();
}
}

// THEN
test.deepEqual(assembly.getStackArtifact(stack1.artifactId).templateFile, 'MyStack1.template.json');
test.deepEqual(assembly.getStackArtifact(stack2.artifactId).templateFile, 'MyStack2.template.json');
test.deepEqual(stack1.templateFile, 'MyStack1.template.json');
test.deepEqual(stack2.templateFile, 'MyStack2.template.json');
test.done();
},

'metadata is collected at the stack boundary'(test: Test) {
Expand Down

0 comments on commit e9a305c

Please sign in to comment.