fix(pipelines): can't have the same asset display name 3 times

packages/aws-cdk-lib/core/lib/stage.ts

@@ -163,7 +163,7 @@ export class Stage extends Construct {
   constructor(scope: Construct, id: string, props: StageProps = {}) {
     super(scope, id);
 
-    if (id !== '' && !/^[a-z][a-z0-9\-\_\.]+$/i.test(id)) {
+    if (id !== '' && !/^[a-z][a-z0-9\-\_\.]*$/i.test(id)) {
       throw new Error(`invalid stage name "${id}". Stage name must start with a letter and contain only alphanumeric characters, hypens ('-'), underscores ('_') and periods ('.')`);
     }
 

packages/aws-cdk-lib/core/test/stage.test.ts

@@ -288,6 +288,7 @@ describe('stage', () => {
   test('stage name validation', () => {
     const app = new App();
 
+    new Stage(app, 'a');
     new Stage(app, 'abcd');
     new Stage(app, 'abcd123');
     new Stage(app, 'abcd123-588dfjjk');
@@ -298,7 +299,6 @@ describe('stage', () => {
     expect(() => new Stage(app, 'abcd123-588dfjjk.sss_ajsid/dfo')).toThrow(/invalid stage name "abcd123-588dfjjk.sss_ajsid\/dfo"/);
     expect(() => new Stage(app, '&')).toThrow(/invalid stage name "&"/);
     expect(() => new Stage(app, '45hello')).toThrow(/invalid stage name "45hello"/);
-    expect(() => new Stage(app, 'f')).toThrow(/invalid stage name "f"/);
   });
 
   test('outdir cannot be specified for nested stages', () => {

packages/aws-cdk-lib/pipelines/lib/codepipeline/codepipeline.ts

@@ -529,7 +529,7 @@ export class CodePipeline extends PipelineBase {
         const sharedParent = new GraphNodeCollection(flatten(tranches)).commonAncestor();
 
         // If we produce the same action name for some actions, append a unique number
-        let namesCtrs = new Map<string, number>();
+        let namesUsed = new Set<string>();
 
         let runOrder = 1;
         for (const tranche of tranches) {
@@ -540,12 +540,16 @@ export class CodePipeline extends PipelineBase {
             const nodeType = this.nodeTypeFromNode(node);
 
             // Come up with a unique name for this action, incrementing a counter if necessary
-            let name = actionName(node, sharedParent);
-            const nameCount = namesCtrs.get(name) ?? 0;
-            if (nameCount > 0) {
-              name += `${nameCount + 1}`;
+            const baseName = actionName(node, sharedParent);
+            let name = baseName;
+            for (let ctr = 1; ; ctr++) {
+              const candidate = ctr > 1 ? `${name}${ctr}` : name;
+              if (!namesUsed.has(candidate)) {
+                name = candidate;
+                break;
+              }
             }
-            namesCtrs.set(name, nameCount + 1);
+            namesUsed.add(name);
 
             const variablesNamespace = node.data?.type === 'step'
               ? namespaceStepOutputs(node.data.step, pipelineStage, name)

packages/aws-cdk-lib/pipelines/test/codepipeline/codepipeline.test.ts

@@ -10,7 +10,7 @@ import { Stack } from '../../../core';
 import * as cxapi from '../../../cx-api';
 import * as cdkp from '../../lib';
 import { CodePipeline } from '../../lib';
-import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, FileAssetApp, TwoStackApp, StageWithStackOutput, TwoFileAssetsApp } from '../testhelpers';
+import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, FileAssetApp, TwoStackApp, StageWithStackOutput, MultipleFileAssetsApp } from '../testhelpers';
 
 let app: TestApp;
 
@@ -438,31 +438,55 @@ test('display name can contain illegal characters which are sanitized for the pi
   });
 });
 
-test('two assets can have same display name, which are reflected in the pipeline', () => {
+test.each([2, 3])('%p assets can have same display name, which are reflected in the pipeline', (n) => {
   const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV });
   const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', {
     crossAccountKeys: true,
     useChangeSets: false,
   });
-  pipeline.addStage(new TwoFileAssetsApp(pipelineStack, 'App', {
-    displayName1: 'asdf',
-    displayName2: 'asdf',
+  pipeline.addStage(new MultipleFileAssetsApp(pipelineStack, 'App', {
+    n,
+    displayNames: Array.from({ length: n }, () => 'asdf'),
   }));
 
   // THEN
   const template = Template.fromStack(pipelineStack);
 
-  // We expect `asdf` and `asdf2` Actions in the pipeline
+  // We expect at least `asdf` and `asdf2` Actions in the pipeline,
   template.hasResourceProperties('AWS::CodePipeline::Pipeline', {
     Stages: Match.arrayWith([{
       Name: 'Assets',
       Actions: Match.arrayWith([
-        Match.objectLike({
-          Name: 'asdf',
-        }),
-        Match.objectLike({
-          Name: 'asdf2',
-        }),
+        Match.objectLike({ Name: 'asdf' }),
+        Match.objectLike({ Name: 'asdf2' }),
+        ...(n == 3 ? [Match.objectLike({ Name: 'asdf3' })] : []),
+      ]),
+    }]),
+  });
+});
+
+test('assets can have display names that conflict with calculated action names', () => {
+  const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV });
+  const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', {
+    crossAccountKeys: true,
+    useChangeSets: false,
+  });
+  pipeline.addStage(new MultipleFileAssetsApp(pipelineStack, 'App', {
+    n: 3,
+    displayNames: ['asdf', 'asdf', 'asdf2'], // asdf2 will conflict with the second generated name which will also be asdf2
+  }));
+
+  // THEN
+  const template = Template.fromStack(pipelineStack);
+
+  // We expect `asdf`, `asdf2` and `asdf22` Actions in the pipeline
+  template.hasResourceProperties('AWS::CodePipeline::Pipeline', {
+    Stages: Match.arrayWith([{
+      Name: 'Assets',
+      Actions: Match.arrayWith([
+        Match.objectLike({ Name: 'asdf' }),
+        Match.objectLike({ Name: 'asdf2' }),
+        Match.objectLike({ Name: 'asdf22' }),
       ]),
     }]),
   });

packages/aws-cdk-lib/pipelines/test/compliance/assets.test.ts

@@ -5,7 +5,7 @@ import * as cb from '../../../aws-codebuild';
 import * as ec2 from '../../../aws-ec2';
 import { Stack, Stage } from '../../../core';
 import { CDKP_DEFAULT_CODEBUILD_IMAGE } from '../../lib/private/default-codebuild-image';
-import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, FileAssetApp, MegaAssetsApp, TwoFileAssetsApp, DockerAssetApp, PlainStackApp, stringLike } from '../testhelpers';
+import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, FileAssetApp, MegaAssetsApp, MultipleFileAssetsApp, DockerAssetApp, PlainStackApp, stringLike } from '../testhelpers';
 
 const FILE_ASSET_SOURCE_HASH = '8289faf53c7da377bb2b90615999171adef5e1d8f6b88810e5fef75e6ca09ba5';
 const FILE_ASSET_SOURCE_HASH2 = 'ac76997971c3f6ddf37120660003f1ced72b4fc58c498dfd99c78fa77e721e0e';
@@ -183,7 +183,7 @@ describe('basic pipeline', () => {
 
   test('multiple assets are published in parallel', () => {
     const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk');
-    pipeline.addStage(new TwoFileAssetsApp(app, 'FileAssetApp'));
+    pipeline.addStage(new MultipleFileAssetsApp(app, 'FileAssetApp', { n: 2 }));
 
     Template.fromStack(pipelineStack).hasResourceProperties('AWS::CodePipeline::Pipeline', {
       Stages: Match.arrayWith([{
@@ -519,13 +519,13 @@ describe('pipeline with single asset publisher', () => {
     const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk-1', {
       publishAssetsInParallel: false,
     });
-    pipeline.addStage(new TwoFileAssetsApp(app, 'FileAssetApp'));
+    pipeline.addStage(new MultipleFileAssetsApp(app, 'FileAssetApp', { n: 2 }));
 
     const pipelineStack2 = new Stack(app, 'PipelineStack2', { env: PIPELINE_ENV });
     const otherPipeline = new ModernTestGitHubNpmPipeline(pipelineStack2, 'Cdk-2', {
       publishAssetsInParallel: false,
     });
-    otherPipeline.addStage(new TwoFileAssetsApp(app, 'OtherFileAssetApp'));
+    otherPipeline.addStage(new MultipleFileAssetsApp(app, 'OtherFileAssetApp', { n: 2 }));
     // THEN
     const buildSpecName1 = new Capture(stringLike('buildspec-*.yaml'));
     const buildSpecName2 = new Capture(stringLike('buildspec-*.yaml'));
@@ -585,7 +585,7 @@ describe('pipeline with single asset publisher', () => {
     const pipeline = new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk', {
       publishAssetsInParallel: false,
     });
-    pipeline.addStage(new TwoFileAssetsApp(app, 'FileAssetApp'));
+    pipeline.addStage(new MultipleFileAssetsApp(app, 'FileAssetApp', { n: 2 }));
 
     // THEN
     const buildSpecName = new Capture(stringLike('buildspec-*.yaml'));
@@ -634,7 +634,7 @@ describe('pipeline with custom asset publisher BuildSpec', () => {
         }),
       },
     });
-    pipeline.addStage(new TwoFileAssetsApp(app, 'FileAssetApp'));
+    pipeline.addStage(new MultipleFileAssetsApp(app, 'FileAssetApp', { n: 2 }));
 
     const buildSpecName = new Capture(stringLike('buildspec-*'));
 

packages/aws-cdk-lib/pipelines/test/testhelpers/assets/test-file-asset-three.txt

@@ -0,0 +1 @@
+And a third.

packages/aws-cdk-lib/pipelines/test/testhelpers/test-app.ts

@@ -4,7 +4,7 @@ import { Construct } from 'constructs';
 import * as ecr_assets from '../../../aws-ecr-assets';
 import * as s3 from '../../../aws-s3';
 import * as s3_assets from '../../../aws-s3-assets';
-import { App, AppProps, Environment, CfnOutput, Stage, StageProps, Stack, StackProps } from '../../../core';
+import { App, AppProps, Environment, CfnOutput, Stage, StageProps, Stack, StackProps, ValidationError } from '../../../core';
 import { assemblyBuilderOf } from '../../lib/private/construct-internals';
 
 export const PIPELINE_ENV: Environment = {
@@ -185,23 +185,40 @@ export class FileAssetApp extends Stage {
   }
 }
 
-export interface TwoFileAssetsAppProps extends StageProps {
-  readonly displayName1?: string;
-  readonly displayName2?: string;
+export interface MultipleFileAssetsProps extends StageProps {
+  readonly n: number;
+
+  /**
+   * Up to 3 display names for equally many assets
+   */
+  readonly displayNames?: string[];
 }
 
-export class TwoFileAssetsApp extends Stage {
-  constructor(scope: Construct, id: string, props?: TwoFileAssetsAppProps) {
+/**
+ * Supports up to 3 file assets
+ */
+export class MultipleFileAssetsApp extends Stage {
+  constructor(scope: Construct, id: string, props: MultipleFileAssetsProps) {
     super(scope, id, props);
     const stack = new Stack(this, 'Stack');
-    new s3_assets.Asset(stack, 'Asset1', {
-      path: path.join(__dirname, 'assets', 'test-file-asset.txt'),
-      displayName: props?.displayName1,
-    });
-    new s3_assets.Asset(stack, 'Asset2', {
-      path: path.join(__dirname, 'assets', 'test-file-asset-two.txt'),
-      displayName: props?.displayName2,
-    });
+
+    const fileNames = ['test-file-asset.txt', 'test-file-asset-two.txt', 'test-file-asset-three.txt'];
+    if (props.displayNames && props.displayNames.length !== props.n) {
+      throw new Error('Incorrect displayNames lenght');
+    }
+
+    for (let i = 0; i < props.n; i++) {
+      const displayName = props.displayNames ? props.displayNames[i] : undefined;
+      const fn = fileNames[i];
+      if (!fn) {
+        throw new ValidationError(`Got more displayNames than we have fileNames: ${i + 1}`, this);
+      }
+
+      new s3_assets.Asset(stack, `Asset${i + 1}`, {
+        path: path.join(__dirname, 'assets', fn),
+        displayName,
+      });
+    }
   }
 }