fix(pipelines): repos with dashes cannot be used as additionalInputs (#…

…16017) Cause: dashes are valid in *artifact names* in CodePipeline, but are *not* valid in *secondary source names* in CodeBuild. Do a sanitization to the intersection of requirements between CodePipeline and CodeBuild. Fixes #15753. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Aug 12, 2021 · 400a59d · 400a59d
1 parent 97fc290
commit 400a59d
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 13 deletions.
diff --git a/packages/@aws-cdk/pipelines/lib/codepipeline/_codebuild-factory.ts b/packages/@aws-cdk/pipelines/lib/codepipeline/_codebuild-factory.ts
@@ -326,12 +326,12 @@ function generateInputArtifactLinkCommands(artifacts: ArtifactMap, inputs: FileS
     const fragments = [];
 
     if (!['.', '..'].includes(path.dirname(input.directory))) {
-      fragments.push(`mkdir -p "${input.directory}"`);
+      fragments.push(`mkdir -p -- "${input.directory}"`);
     }
 
     const artifact = artifacts.toCodePipeline(input.fileSet);
 
-    fragments.push(`ln -s "$CODEBUILD_SRC_DIR_${artifact.artifactName}" "${input.directory}"`);
+    fragments.push(`ln -s -- "$CODEBUILD_SRC_DIR_${artifact.artifactName}" "${input.directory}"`);
 
     return fragments.join(' && ');
   });

diff --git a/packages/@aws-cdk/pipelines/lib/codepipeline/artifact-map.ts b/packages/@aws-cdk/pipelines/lib/codepipeline/artifact-map.ts
@@ -1,3 +1,4 @@
+import * as crypto from 'crypto';
 import * as cp from '@aws-cdk/aws-codepipeline';
 import { FileSet } from '../blueprint';
 import { PipelineGraph } from '../helpers-internal';
@@ -38,9 +39,27 @@ export class ArtifactMap {
   }
 }
 
+/**
+ * Sanitize a string to be a valid artifact name
+ *
+ * This must comport to both the rules of artifacts in CodePipeline, as well
+ * as the names of Source Identifiers in CodeBuild.
+ *
+ * Artifact Name limits aren't documented.
+ *
+ * Source Identifier limits are documented here:
+ * https://docs.aws.amazon.com/codebuild/latest/APIReference/API_ProjectSource.html#CodeBuild-Type-ProjectSource-sourceIdentifier
+ */
 function sanitizeArtifactName(x: string): string {
-  // FIXME: Does this REALLY not allow '.'? The docs don't mention it, but action names etc. do!
-  return x.replace(/[^A-Za-z0-9@\-_]/g, '_');
+  let sani = x.replace(/[^A-Za-z0-9_]/g, '_'); // Charset requirement is imposed by CodeBuild
+  const maxLength = 100; // Max length of 100 is imposed by CodePipeline library
+
+  if (sani.length > maxLength) {
+    const fingerprint = crypto.createHash('sha256').update(sani).digest('hex').substr(0, 8);
+    sani = sani.substr(0, maxLength - fingerprint.length) + fingerprint;
+  }
+
+  return sani;
 }
 
 /**

diff --git a/packages/@aws-cdk/pipelines/lib/codepipeline/codepipeline.ts b/packages/@aws-cdk/pipelines/lib/codepipeline/codepipeline.ts
@@ -830,7 +830,7 @@ enum CodeBuildProjectType {
 
 function actionName<A>(node: GraphNode<A>, parent: GraphNode<A>) {
   const names = node.ancestorPath(parent).map(n => n.id);
-  return names.map(sanitizeName).join('.');
+  return names.map(sanitizeName).join('.').substr(0, 100); // Cannot exceed 100 chars
 }
 
 function sanitizeName(x: string): string {

diff --git a/packages/@aws-cdk/pipelines/test/codepipeline/codepipeline-sources.test.ts b/packages/@aws-cdk/pipelines/test/codepipeline/codepipeline-sources.test.ts
@@ -1,4 +1,4 @@
-import { anything, arrayWith, objectLike } from '@aws-cdk/assert-internal';
+import { anything, arrayWith, Capture, objectLike } from '@aws-cdk/assert-internal';
 import '@aws-cdk/assert-internal/jest';
 import * as ccommit from '@aws-cdk/aws-codecommit';
 import { CodeCommitTrigger, GitHubTrigger } from '@aws-cdk/aws-codepipeline-actions';
@@ -140,5 +140,43 @@ test('GitHub source does not accept unresolved identifiers', () => {
   }).toThrow(/Step id cannot be unresolved/);
 });
 
+test('Dashes in repo names are removed from artifact names', () => {
+  new ModernTestGitHubNpmPipeline(pipelineStack, 'Pipeline', {
+    input: cdkp.CodePipelineSource.gitHub('owner/my-repo', 'main'),
+  });
+
+  expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
+    Stages: arrayWith({
+      Name: 'Source',
+      Actions: [
+        objectLike({
+          OutputArtifacts: [
+            { Name: 'owner_my_repo_Source' },
+          ],
+        }),
+      ],
+    }),
+  });
+});
+
+test('artifact names are never longer than 128 characters', () => {
+  new ModernTestGitHubNpmPipeline(pipelineStack, 'Pipeline', {
+    input: cdkp.CodePipelineSource.gitHub('owner/' + 'my-repo'.repeat(100), 'main'),
+  });
+
+  const artifactId = Capture.aString();
+  expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
+    Stages: arrayWith({
+      Name: 'Source',
+      Actions: [
+        objectLike({
+          OutputArtifacts: [
+            { Name: artifactId.capture() },
+          ],
+        }),
+      ],
+    }),
+  });
 
-// a-z0-9.@-_
+  expect(artifactId.capturedValue.length).toBeLessThanOrEqual(128);
+});
diff --git a/packages/@aws-cdk/pipelines/test/compliance/synths.test.ts b/packages/@aws-cdk/pipelines/test/compliance/synths.test.ts
@@ -948,8 +948,8 @@ behavior('Multiple input sources in side-by-side directories', (suite) => {
           phases: {
             install: {
               commands: [
-                'ln -s "$CODEBUILD_SRC_DIR_foo_bar_Source" "../sibling"',
-                'ln -s "$CODEBUILD_SRC_DIR_Prebuild_Output" "sub"',
+                'ln -s -- "$CODEBUILD_SRC_DIR_foo_bar_Source" "../sibling"',
+                'ln -s -- "$CODEBUILD_SRC_DIR_Prebuild_Output" "sub"',
               ],
             },
             build: {

diff --git a/packages/@aws-cdk/pipelines/test/integ.newpipeline-with-vpc.expected.json b/packages/@aws-cdk/pipelines/test/integ.newpipeline-with-vpc.expected.json
@@ -766,7 +766,7 @@
                 "Name": "rix0rrr_cdk-pipelines-demo",
                 "OutputArtifacts": [
                   {
-                    "Name": "rix0rrr_cdk-pipelines-demo_Source"
+                    "Name": "rix0rrr_cdk_pipelines_demo_Source"
                   }
                 ],
                 "RunOrder": 1
@@ -791,7 +791,7 @@
                 },
                 "InputArtifacts": [
                   {
-                    "Name": "rix0rrr_cdk-pipelines-demo_Source"
+                    "Name": "rix0rrr_cdk_pipelines_demo_Source"
                   }
                 ],
                 "Name": "Synth",

diff --git a/packages/@aws-cdk/pipelines/test/integ.newpipeline.expected.json b/packages/@aws-cdk/pipelines/test/integ.newpipeline.expected.json
@@ -232,7 +232,7 @@
                 "Name": "rix0rrr_cdk-pipelines-demo",
                 "OutputArtifacts": [
                   {
-                    "Name": "rix0rrr_cdk-pipelines-demo_Source"
+                    "Name": "rix0rrr_cdk_pipelines_demo_Source"
                   }
                 ],
                 "RunOrder": 1
@@ -257,7 +257,7 @@
                 },
                 "InputArtifacts": [
                   {
-                    "Name": "rix0rrr_cdk-pipelines-demo_Source"
+                    "Name": "rix0rrr_cdk_pipelines_demo_Source"
                   }
                 ],
                 "Name": "Synth",