From 0709a3647f6362b9fbfc2d40d5e81dcda05cd936 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 5 Sep 2024 14:48:13 +0200 Subject: [PATCH] fix(pipelines): "Node with duplicate id" on duplicate stack names 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. --- .../pipelines/lib/helpers-internal/graph.ts | 4 ++ .../lib/helpers-internal/pipeline-graph.ts | 17 +++++++- .../test/compliance/basic-behavior.test.ts | 41 +++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/pipelines/lib/helpers-internal/graph.ts b/packages/aws-cdk-lib/pipelines/lib/helpers-internal/graph.ts index 4dc58664dc406..109a447514af2 100644 --- a/packages/aws-cdk-lib/pipelines/lib/helpers-internal/graph.ts +++ b/packages/aws-cdk-lib/pipelines/lib/helpers-internal/graph.ts @@ -230,6 +230,10 @@ export class Graph extends GraphNode { return this.children.get(name); } + public containsId(id: string) { + return this.tryGetChild(id) !== undefined; + } + public contains(node: GraphNode) { return this.nodes.has(node); } diff --git a/packages/aws-cdk-lib/pipelines/lib/helpers-internal/pipeline-graph.ts b/packages/aws-cdk-lib/pipelines/lib/helpers-internal/pipeline-graph.ts index cad9fe7769c1a..dbe61b3640dc8 100644 --- a/packages/aws-cdk-lib/pipelines/lib/helpers-internal/pipeline-graph.ts +++ b/packages/aws-cdk-lib/pipelines/lib/helpers-internal/pipeline-graph.ts @@ -134,7 +134,12 @@ export class PipelineGraph { const stackGraphs = new Map(); 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', @@ -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, 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('.'); } \ No newline at end of file diff --git a/packages/aws-cdk-lib/pipelines/test/compliance/basic-behavior.test.ts b/packages/aws-cdk-lib/pipelines/test/compliance/basic-behavior.test.ts index 8ca4d83650a8f..2b51dd914c429 100644 --- a/packages/aws-cdk-lib/pipelines/test/compliance/basic-behavior.test.ts +++ b/packages/aws-cdk-lib/pipelines/test/compliance/basic-behavior.test.ts @@ -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 @@ -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', + }); + } +} \ No newline at end of file