Skip to content

Commit 5f36f6b

Browse files
authored
fix(core): Stacks render CloudFormation elements in nested Stages (#10156)
In current recommended CDK Pipelines usage, the construct tree looks like this: ``` App -> Stack1 -> Stage -> Stack2 -> Resource ``` It's an easy mistake to forget `Stack2` (or make it a generic `Construct` instead), in which case the hierachy looks like the one below and `Stack1` will render Resource in its CloudFormation template: ``` App -> Stack1 -> Stage -> Construct -> Resource ``` This case should not have been allowed in the first place: `Stage`s define a new assembly scope and so `Stack1` does not exist from the point of view of `Resource`. Change `Stack.of()` to fail to find `Stack1` in this case, so that it becomes illegal to define `Resource` in this position. Fixes #9792, relates to #9669. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 35b23a0 commit 5f36f6b

File tree

3 files changed

+32
-3
lines changed

3 files changed

+32
-3
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ export class Stack extends Construct implements ITaggable {
169169
return c;
170170
}
171171

172-
if (!c.node.scope) {
173-
throw new Error(`No stack could be identified for the construct at path ${construct.node.path}`);
172+
if (Stage.isStage(c) || !c.node.scope) {
173+
throw new Error(`${construct.constructor?.name ?? 'Construct'} at '${construct.node.path}' should be created in the scope of a Stack, but no Stack found`);
174174
}
175175

176176
return _lookup(c.node.scope);

packages/@aws-cdk/core/test/test.cfn-resource.ts

+29
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,35 @@ export = nodeunit.testCase({
124124
// No DependsOn!
125125
});
126126

127+
test.done();
128+
},
129+
130+
'CfnResource cannot be created outside Stack'(test: nodeunit.Test) {
131+
const app = new core.App();
132+
test.throws(() => {
133+
new core.CfnResource(app, 'Resource', {
134+
type: 'Some::Resource',
135+
});
136+
}, /should be created in the scope of a Stack, but no Stack found/);
137+
138+
139+
test.done();
140+
},
141+
142+
/**
143+
* Stages start a new scope, which does not count as a Stack anymore
144+
*/
145+
'CfnResource cannot be in Stage in Stack'(test: nodeunit.Test) {
146+
const app = new core.App();
147+
const stack = new core.Stack(app, 'Stack');
148+
const stage = new core.Stage(stack, 'Stage');
149+
test.throws(() => {
150+
new core.CfnResource(stage, 'Resource', {
151+
type: 'Some::Resource',
152+
});
153+
}, /should be created in the scope of a Stack, but no Stack found/);
154+
155+
127156
test.done();
128157
},
129158
});

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ export = {
747747
'Stack.of() throws when there is no parent Stack'(test: Test) {
748748
const root = new Construct(undefined as any, 'Root');
749749
const construct = new Construct(root, 'Construct');
750-
test.throws(() => Stack.of(construct), /No stack could be identified for the construct at path/);
750+
test.throws(() => Stack.of(construct), /should be created in the scope of a Stack, but no Stack found/);
751751
test.done();
752752
},
753753

0 commit comments

Comments
 (0)