Skip to content

feat(pipelines): allow switching to one CodeBuild action for same-typed assets #13803

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
May 31, 2021
Merged

feat(pipelines): allow switching to one CodeBuild action for same-typed assets #13803

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3dc0a85
feat: single asset publish action per type
hoegertn Mar 25, 2021
2b9202d
progress
hoegertn Mar 25, 2021
a12601b
Merge branch 'master' into hoegertn-pipelineassets
hoegertn Mar 25, 2021
ba2253a
tests
hoegertn Mar 25, 2021
a48c6f0
Merge branch 'master' into hoegertn-pipelineassets
hoegertn Apr 26, 2021
272e1d0
progress
hoegertn Apr 26, 2021
bb95222
fix tests
hoegertn Apr 27, 2021
0eb667c
fix whitespace
hoegertn Apr 27, 2021
1a2f234
Merge branch 'master' into hoegertn-pipelineassets
hoegertn May 6, 2021
11cd9db
Merge branch 'master' into hoegertn-pipelineassets
hoegertn May 31, 2021
04a0e60
add unit test
hoegertn May 31, 2021
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
32 changes: 7 additions & 25 deletions packages/@aws-cdk/pipelines/lib/pipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,8 +364,6 @@ interface AssetPublishingProps {
* Add appropriate publishing actions to the asset publishing stage
*/
class AssetPublishing extends CoreConstruct {
// CodePipelines has a hard limit of 50 actions per stage. See https://github.com/aws/aws-cdk/issues/9353
private readonly MAX_PUBLISHERS_PER_STAGE = 50;

private readonly publishers: Record<string, PublishAssetsAction> = {};
private readonly assetRoles: Record<string, iam.IRole> = {};
Expand All @@ -375,9 +373,6 @@ class AssetPublishing extends CoreConstruct {
private readonly stages: codepipeline.IStage[] = [];
private readonly pipeline: codepipeline.Pipeline;

private _fileAssetCtr = 0;
private _dockerAssetCtr = 0;

constructor(scope: Construct, id: string, private readonly props: AssetPublishingProps) {
super(scope, id);
this.myCxAsmRoot = path.resolve(assemblyBuilderOf(appOf(this)).outdir);
Expand Down Expand Up @@ -412,30 +407,17 @@ class AssetPublishing extends CoreConstruct {
this.generateAssetRole(command.assetType);
}

let action = this.publishers[command.assetId];
let action = this.publishers[command.assetType.toString()];
if (!action) {
// Dynamically create new stages as needed, with `MAX_PUBLISHERS_PER_STAGE` assets per stage.
const stageIndex = Math.floor((this._fileAssetCtr + this._dockerAssetCtr) / this.MAX_PUBLISHERS_PER_STAGE);
if (stageIndex >= this.stages.length) {
const previousStage = this.stages.slice(-1)[0] ?? this.lastStageBeforePublishing;
if (this.stages.length == 0) {
this.stages.push(this.pipeline.addStage({
stageName: `Assets${stageIndex > 0 ? stageIndex + 1 : ''}`,
placement: { justAfter: previousStage },
stageName: 'Assets',
placement: { justAfter: this.lastStageBeforePublishing },
}));
}
const id = command.assetType === AssetType.FILE ? 'FileAsset' : 'DockerAsset';

// The asset ID would be a logical candidate for the construct path and project names, but if the asset
// changes it leads to recreation of a number of Role/Policy/Project resources which is slower than
// necessary. Number sequentially instead.
//
// FIXME: The ultimate best solution is probably to generate a single Project per asset type
// and reuse that for all assets.
const id = command.assetType === AssetType.FILE ? `FileAsset${++this._fileAssetCtr}` : `DockerAsset${++this._dockerAssetCtr}`;

// NOTE: It's important that asset changes don't force a pipeline self-mutation.
// This can cause an infinite loop of updates (see https://github.com/aws/aws-cdk/issues/9080).
// For that reason, we use the id as the actionName below, rather than the asset hash.
action = this.publishers[command.assetId] = new PublishAssetsAction(this, id, {
action = this.publishers[command.assetType.toString()] = new PublishAssetsAction(this, id, {
actionName: id,
cloudAssemblyInput: this.props.cloudAssemblyInput,
cdkCliVersion: this.props.cdkCliVersion,
Expand All @@ -444,7 +426,7 @@ class AssetPublishing extends CoreConstruct {
vpc: this.props.vpc,
subnetSelection: this.props.subnetSelection,
});
this.stages[stageIndex].addAction(action);
this.stages[0].addAction(action);
}

action.addPublishCommand(relativePath, command.assetSelector);
Expand Down
65 changes: 4 additions & 61 deletions packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,41 +362,15 @@
},
"Configuration": {
"ProjectName": {
"Ref": "PipelineAssetsFileAsset185A67CB4"
"Ref": "PipelineAssetsFileAsset5D8C5DA6"
}
},
"InputArtifacts": [
{
"Name": "CloudAsm"
}
],
"Name": "FileAsset1",
"RoleArn": {
"Fn::GetAtt": [
"PipelineAssetsFileRole59943A77",
"Arn"
]
},
"RunOrder": 1
},
{
"ActionTypeId": {
"Category": "Build",
"Owner": "AWS",
"Provider": "CodeBuild",
"Version": "1"
},
"Configuration": {
"ProjectName": {
"Ref": "PipelineAssetsFileAsset24D2D639B"
}
},
"InputArtifacts": [
{
"Name": "CloudAsm"
}
],
"Name": "FileAsset2",
"Name": "FileAsset",
"RoleArn": {
"Fn::GetAtt": [
"PipelineAssetsFileRole59943A77",
Expand Down Expand Up @@ -1445,38 +1419,7 @@
]
}
},
"PipelineAssetsFileAsset185A67CB4": {
"Type": "AWS::CodeBuild::Project",
"Properties": {
"Artifacts": {
"Type": "CODEPIPELINE"
},
"Environment": {
"ComputeType": "BUILD_GENERAL1_SMALL",
"Image": "aws/codebuild/standard:4.0",
"ImagePullCredentialsType": "CODEBUILD",
"PrivilegedMode": false,
"Type": "LINUX_CONTAINER"
},
"ServiceRole": {
"Fn::GetAtt": [
"PipelineAssetsFileRole59943A77",
"Arn"
]
},
"Source": {
"BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"8289faf53c7da377bb2b90615999171adef5e1d8f6b88810e5fef75e6ca09ba5:12345678-test-region\\\"\"\n ]\n }\n }\n}",
"Type": "CODEPIPELINE"
},
"EncryptionKey": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKeyF5BF0670",
"Arn"
]
}
}
},
"PipelineAssetsFileAsset24D2D639B": {
"PipelineAssetsFileAsset5D8C5DA6": {
"Type": "AWS::CodeBuild::Project",
"Properties": {
"Artifacts": {
Expand All @@ -1496,7 +1439,7 @@
]
},
"Source": {
"BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"ac76997971c3f6ddf37120660003f1ced72b4fc58c498dfd99c78fa77e721e0e:12345678-test-region\\\"\"\n ]\n }\n }\n}",
"BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"8289faf53c7da377bb2b90615999171adef5e1d8f6b88810e5fef75e6ca09ba5:12345678-test-region\\\"\",\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"ac76997971c3f6ddf37120660003f1ced72b4fc58c498dfd99c78fa77e721e0e:12345678-test-region\\\"\"\n ]\n }\n }\n}",
"Type": "CODEPIPELINE"
},
"EncryptionKey": {
Expand Down
108 changes: 2 additions & 106 deletions packages/@aws-cdk/pipelines/test/pipeline-assets.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,57 +91,6 @@ describe('basic pipeline', () => {
],
});
});

test('up to 50 assets fit in a single stage', () => {
// WHEN
pipeline.addApplicationStage(new MegaAssetsApp(app, 'App', { numAssets: 50 }));

// THEN
expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
Stages: [
objectLike({ Name: 'Source' }),
objectLike({ Name: 'Build' }),
objectLike({ Name: 'UpdatePipeline' }),
objectLike({ Name: 'Assets' }),
objectLike({ Name: 'App' }),
],
});
});

test('51 assets triggers a second stage', () => {
// WHEN
pipeline.addApplicationStage(new MegaAssetsApp(app, 'App', { numAssets: 51 }));

// THEN
expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
Stages: [
objectLike({ Name: 'Source' }),
objectLike({ Name: 'Build' }),
objectLike({ Name: 'UpdatePipeline' }),
objectLike({ Name: 'Assets' }),
objectLike({ Name: 'Assets2' }),
objectLike({ Name: 'App' }),
],
});
});

test('101 assets triggers a third stage', () => {
// WHEN
pipeline.addApplicationStage(new MegaAssetsApp(app, 'App', { numAssets: 101 }));

// THEN
expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
Stages: [
objectLike({ Name: 'Source' }),
objectLike({ Name: 'Build' }),
objectLike({ Name: 'UpdatePipeline' }),
objectLike({ Name: 'Assets' }),
objectLike({ Name: 'Assets2' }),
objectLike({ Name: 'Assets3' }),
objectLike({ Name: 'App' }),
],
});
});
});

test('command line properly locates assets in subassembly', () => {
Expand All @@ -165,22 +114,6 @@ describe('basic pipeline', () => {
});
});

test('multiple assets are published in parallel', () => {
// WHEN
pipeline.addApplicationStage(new TwoFileAssetsApp(app, 'FileAssetApp'));

// THEN
expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
Stages: arrayWith({
Name: 'Assets',
Actions: [
objectLike({ RunOrder: 1 }),
objectLike({ RunOrder: 1 }),
],
}),
});
});

test('assets are also published when using the lower-level addStackArtifactDeployment', () => {
// GIVEN
const asm = new FileAssetApp(app, 'FileAssetApp').synth();
Expand All @@ -194,7 +127,7 @@ describe('basic pipeline', () => {
Name: 'Assets',
Actions: [
objectLike({
Name: 'FileAsset1',
Name: 'FileAsset',
RunOrder: 1,
}),
],
Expand Down Expand Up @@ -372,7 +305,7 @@ describe('pipeline with VPC', () => {
expect(pipelineStack).toHaveResourceLike('AWS::CodeBuild::Project', {
VpcConfig: objectLike({
SecurityGroupIds: [
{ 'Fn::GetAtt': ['CdkAssetsDockerAsset1SecurityGroup078F5C66', 'GroupId'] },
{ 'Fn::GetAtt': ['CdkAssetsDockerAssetSecurityGroup59B832C3', 'GroupId'] },
],
Subnets: [
{ Ref: 'VpcPrivateSubnet1Subnet536B997A' },
Expand Down Expand Up @@ -423,19 +356,6 @@ class FileAssetApp extends Stage {
}
}

class TwoFileAssetsApp extends Stage {
constructor(scope: Construct, id: string, props?: StageProps) {
super(scope, id, props);
const stack = new Stack(this, 'Stack');
new s3_assets.Asset(stack, 'Asset1', {
path: path.join(__dirname, 'test-file-asset.txt'),
});
new s3_assets.Asset(stack, 'Asset2', {
path: path.join(__dirname, 'test-file-asset-two.txt'),
});
}
}

class DockerAssetApp extends Stage {
constructor(scope: Construct, id: string, props?: StageProps) {
super(scope, id, props);
Expand All @@ -446,31 +366,7 @@ class DockerAssetApp extends Stage {
}
}

interface MegaAssetsAppProps extends StageProps {
readonly numAssets: number;
}

// Creates a mix of file and image assets, up to a specified count
class MegaAssetsApp extends Stage {
constructor(scope: Construct, id: string, props: MegaAssetsAppProps) {
super(scope, id, props);
const stack = new Stack(this, 'Stack');

let assetCount = 0;
for (; assetCount < props.numAssets / 2; assetCount++) {
new s3_assets.Asset(stack, `Asset${assetCount}`, {
path: path.join(__dirname, 'test-file-asset.txt'),
assetHash: `FileAsset${assetCount}`,
});
}
for (; assetCount < props.numAssets; assetCount++) {
new ecr_assets.DockerImageAsset(stack, `Asset${assetCount}`, {
directory: path.join(__dirname, 'test-docker-asset'),
extraHash: `FileAsset${assetCount}`,
});
}
}
}

function expectedAssetRolePolicy(assumeRolePattern: string, attachedRole: string) {
return {
Expand Down