Skip to content

Commit 16b74f3

Browse files
authored
fix(pipelines): "Node with duplicate id" on duplicate stack names (#31328)
When having 2 stacks with the same name in the same stage (which makes sense when deploying them to different environments), the CodePipeline Action name is derived from the stack name, and will be duplicated. Detect if an graph node name is already being used and if so, use environment information to try and make the name unique. Closes #30960. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent abd1768 commit 16b74f3

File tree

3 files changed

+61
-1
lines changed

3 files changed

+61
-1
lines changed

Diff for: packages/aws-cdk-lib/pipelines/lib/helpers-internal/graph.ts

+4
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,10 @@ export class Graph<A> extends GraphNode<A> {
230230
return this.children.get(name);
231231
}
232232

233+
public containsId(id: string) {
234+
return this.tryGetChild(id) !== undefined;
235+
}
236+
233237
public contains(node: GraphNode<A>) {
234238
return this.nodes.has(node);
235239
}

Diff for: packages/aws-cdk-lib/pipelines/lib/helpers-internal/pipeline-graph.ts

+16-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,12 @@ export class PipelineGraph {
134134
const stackGraphs = new Map<StackDeployment, AGraph>();
135135

136136
for (const stack of stage.stacks) {
137-
const stackGraph: AGraph = Graph.of(this.simpleStackName(stack.stackName, stage.stageName), { type: 'stack-group', stack });
137+
const stackGraphName = findUniqueName(retGraph, [
138+
this.simpleStackName(stack.stackName, stage.stageName),
139+
...stack.account ? [stack.account] : [],
140+
...stack.region ? [stack.region] : [],
141+
]);
142+
const stackGraph: AGraph = Graph.of(stackGraphName, { type: 'stack-group', stack });
138143
const prepareNode: AGraphNode | undefined = this.prepareStep ? aGraphNode('Prepare', { type: 'prepare', stack }) : undefined;
139144
const deployNode: AGraphNode = aGraphNode('Deploy', {
140145
type: 'execute',
@@ -412,4 +417,14 @@ function aGraphNode(id: string, x: GraphAnnotation): AGraphNode {
412417

413418
function stripPrefix(s: string, prefix: string) {
414419
return s.startsWith(prefix) ? s.slice(prefix.length) : s;
420+
}
421+
422+
function findUniqueName(parent: Graph<any>, parts: string[]): string {
423+
for (let i = 1; i <= parts.length; i++) {
424+
const candidate = parts.slice(0, i).join('.');
425+
if (!parent.containsId(candidate)) {
426+
return candidate;
427+
}
428+
}
429+
return parts.join('.');
415430
}

Diff for: packages/aws-cdk-lib/pipelines/test/compliance/basic-behavior.test.ts

+41
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,33 @@ test('overridden stack names are respected', () => {
8181
});
8282
});
8383

84+
test('two stacks can have the same name', () => {
85+
const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', { useChangeSets: false });
86+
pipeline.addStage(new TwoStacksApp(app, 'App'));
87+
88+
Template.fromStack(pipelineStack).hasResourceProperties('AWS::CodePipeline::Pipeline', {
89+
Stages: Match.arrayWith([
90+
{
91+
Name: 'App',
92+
Actions: Match.arrayWith([
93+
Match.objectLike({
94+
Name: stringLike('MyFancyStack.Deploy'),
95+
Configuration: Match.objectLike({
96+
StackName: 'MyFancyStack',
97+
}),
98+
}),
99+
Match.objectLike({
100+
Name: stringLike('MyFancyStack.eu-west-2.Deploy'),
101+
Configuration: Match.objectLike({
102+
StackName: 'MyFancyStack',
103+
}),
104+
}),
105+
]),
106+
},
107+
]),
108+
});
109+
});
110+
84111
test('changing CLI version leads to a different pipeline structure (restarting it)', () => {
85112

86113
// GIVEN
@@ -154,3 +181,17 @@ class OneStackAppWithCustomName extends Stage {
154181
});
155182
}
156183
}
184+
185+
class TwoStacksApp extends Stage {
186+
constructor(scope: Construct, id: string, props?: StageProps) {
187+
super(scope, id, props);
188+
new BucketStack(this, 'Stack1', {
189+
env: { region: 'eu-west-1' },
190+
stackName: 'MyFancyStack',
191+
});
192+
new BucketStack(this, 'Stack2', {
193+
env: { region: 'eu-west-2' },
194+
stackName: 'MyFancyStack',
195+
});
196+
}
197+
}

0 commit comments

Comments
 (0)