Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(' && ');
});
Expand Down
23 changes: 21 additions & 2 deletions packages/@aws-cdk/pipelines/lib/codepipeline/artifact-map.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to this change, but this function can cause name clashes. Is this a problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it but didn't think so.

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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
});
4 changes: 2 additions & 2 deletions packages/@aws-cdk/pipelines/test/compliance/synths.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@
"Name": "rix0rrr_cdk-pipelines-demo",
"OutputArtifacts": [
{
"Name": "rix0rrr_cdk-pipelines-demo_Source"
"Name": "rix0rrr_cdk_pipelines_demo_Source"
}
],
"RunOrder": 1
Expand All @@ -791,7 +791,7 @@
},
"InputArtifacts": [
{
"Name": "rix0rrr_cdk-pipelines-demo_Source"
"Name": "rix0rrr_cdk_pipelines_demo_Source"
}
],
"Name": "Synth",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@
"Name": "rix0rrr_cdk-pipelines-demo",
"OutputArtifacts": [
{
"Name": "rix0rrr_cdk-pipelines-demo_Source"
"Name": "rix0rrr_cdk_pipelines_demo_Source"
}
],
"RunOrder": 1
Expand All @@ -257,7 +257,7 @@
},
"InputArtifacts": [
{
"Name": "rix0rrr_cdk-pipelines-demo_Source"
"Name": "rix0rrr_cdk_pipelines_demo_Source"
}
],
"Name": "Synth",
Expand Down