Skip to content

Commit

Permalink
fix(pipelines): "Node with duplicate id" on duplicate stack names (#3…
Browse files Browse the repository at this point in the history
…1328)

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*
  • Loading branch information
rix0rrr authored Sep 25, 2024
1 parent abd1768 commit 16b74f3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 1 deletion.
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/pipelines/lib/helpers-internal/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ export class Graph<A> extends GraphNode<A> {
return this.children.get(name);
}

public containsId(id: string) {
return this.tryGetChild(id) !== undefined;
}

public contains(node: GraphNode<A>) {
return this.nodes.has(node);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,12 @@ export class PipelineGraph {
const stackGraphs = new Map<StackDeployment, AGraph>();

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

function stripPrefix(s: string, prefix: string) {
return s.startsWith(prefix) ? s.slice(prefix.length) : s;
}

function findUniqueName(parent: Graph<any>, parts: string[]): string {
for (let i = 1; i <= parts.length; i++) {
const candidate = parts.slice(0, i).join('.');
if (!parent.containsId(candidate)) {
return candidate;
}
}
return parts.join('.');
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,33 @@ test('overridden stack names are respected', () => {
});
});

test('two stacks can have the same name', () => {
const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', { useChangeSets: false });
pipeline.addStage(new TwoStacksApp(app, 'App'));

Template.fromStack(pipelineStack).hasResourceProperties('AWS::CodePipeline::Pipeline', {
Stages: Match.arrayWith([
{
Name: 'App',
Actions: Match.arrayWith([
Match.objectLike({
Name: stringLike('MyFancyStack.Deploy'),
Configuration: Match.objectLike({
StackName: 'MyFancyStack',
}),
}),
Match.objectLike({
Name: stringLike('MyFancyStack.eu-west-2.Deploy'),
Configuration: Match.objectLike({
StackName: 'MyFancyStack',
}),
}),
]),
},
]),
});
});

test('changing CLI version leads to a different pipeline structure (restarting it)', () => {

// GIVEN
Expand Down Expand Up @@ -154,3 +181,17 @@ class OneStackAppWithCustomName extends Stage {
});
}
}

class TwoStacksApp extends Stage {
constructor(scope: Construct, id: string, props?: StageProps) {
super(scope, id, props);
new BucketStack(this, 'Stack1', {
env: { region: 'eu-west-1' },
stackName: 'MyFancyStack',
});
new BucketStack(this, 'Stack2', {
env: { region: 'eu-west-2' },
stackName: 'MyFancyStack',
});
}
}

0 comments on commit 16b74f3

Please sign in to comment.