Skip to content

Commit b149b02

Browse files
skinny85mergify[bot]
authored andcommitted
fix(codepipeline): work around CodeBuild's pipeline key bug (#4183)
CodeBuild has a bug where they ignore the encryption key of the pipeline's artifact bucket, instead always using the project's key (the account's default S3 key if the project key has not been set). This makes the CodeBuild actions unusable in a cross-account pipeline, as subsequent actions will get an 'Access Denied' error when trying to download the incorrectly encrypted artifacts. The fix is to always set the project's key to be the same as the pipeline key in the CodeBuild action. Fixes #4033
1 parent b9a1703 commit b149b02

File tree

7 files changed

+233
-8
lines changed

7 files changed

+233
-8
lines changed

packages/@aws-cdk/aws-codebuild/lib/project.ts

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { DockerImageAsset, DockerImageAssetProps } from '@aws-cdk/aws-ecr-assets
55
import events = require('@aws-cdk/aws-events');
66
import iam = require('@aws-cdk/aws-iam');
77
import kms = require('@aws-cdk/aws-kms');
8+
import s3 = require('@aws-cdk/aws-s3');
89
import secretsmanager = require('@aws-cdk/aws-secretsmanager');
910
import { Aws, CfnResource, Construct, Duration, IResource, Lazy, PhysicalName, Resource, Stack } from '@aws-cdk/core';
1011
import { IArtifacts } from './artifacts';
@@ -546,6 +547,16 @@ export interface ProjectProps extends CommonProjectProps {
546547
readonly secondaryArtifacts?: IArtifacts[];
547548
}
548549

550+
/**
551+
* The extra options passed to the {@link IProject.bindToCodePipeline} method.
552+
*/
553+
export interface BindToCodePipelineOptions {
554+
/**
555+
* The artifact bucket that will be used by the action that invokes this project.
556+
*/
557+
readonly artifactBucket: s3.IBucket;
558+
}
559+
549560
/**
550561
* A representation of a CodeBuild Project.
551562
*/
@@ -627,6 +638,7 @@ export class Project extends ProjectBase {
627638
private readonly buildImage: IBuildImage;
628639
private readonly _secondarySources: CfnProject.SourceProperty[];
629640
private readonly _secondaryArtifacts: CfnProject.ArtifactsProperty[];
641+
private _encryptionKey?: kms.IKey;
630642

631643
constructor(scope: Construct, id: string, props: ProjectProps) {
632644
super(scope, id, {
@@ -689,7 +701,8 @@ export class Project extends ProjectBase {
689701
artifacts: artifactsConfig.artifactsProperty,
690702
serviceRole: this.role.roleArn,
691703
environment: this.renderEnvironment(props.environment, environmentVariables),
692-
encryptionKey: props.encryptionKey && props.encryptionKey.keyArn,
704+
// lazy, because we have a setter for it in setEncryptionKey
705+
encryptionKey: Lazy.stringValue({ produce: () => this._encryptionKey && this._encryptionKey.keyArn }),
693706
badgeEnabled: props.badge,
694707
cache: cache._toCloudFormation(),
695708
name: this.physicalName,
@@ -712,7 +725,7 @@ export class Project extends ProjectBase {
712725
this.addToRolePolicy(this.createLoggingPermission());
713726

714727
if (props.encryptionKey) {
715-
props.encryptionKey.grantEncryptDecrypt(this);
728+
this.encryptionKey = props.encryptionKey;
716729
}
717730
}
718731

@@ -742,6 +755,32 @@ export class Project extends ProjectBase {
742755
this._secondaryArtifacts.push(secondaryArtifact.bind(this, this).artifactsProperty);
743756
}
744757

758+
/**
759+
* A callback invoked when the given project is added to a CodePipeline.
760+
*
761+
* @param _scope the construct the binding is taking place in
762+
* @param options additional options for the binding
763+
*/
764+
public bindToCodePipeline(_scope: Construct, options: BindToCodePipelineOptions): void {
765+
if (this.source.type !== CODEPIPELINE_SOURCE_ARTIFACTS_TYPE) {
766+
throw new Error('Only a PipelineProject can be added to a CodePipeline');
767+
}
768+
769+
// work around a bug in CodeBuild: it ignores the KMS key set on the pipeline,
770+
// and always uses its own, project-level key
771+
if (options.artifactBucket.encryptionKey && !this._encryptionKey) {
772+
// we cannot safely do this assignment if the key is of type kms.Key,
773+
// and belongs to a stack in a different account or region than the project
774+
// (that would cause an illegal reference, as KMS keys don't have physical names)
775+
const keyStack = Stack.of(options.artifactBucket.encryptionKey);
776+
const projectStack = Stack.of(this);
777+
if (!(options.artifactBucket.encryptionKey instanceof kms.Key &&
778+
(keyStack.account !== projectStack.account || keyStack.region !== projectStack.region))) {
779+
this.encryptionKey = options.artifactBucket.encryptionKey;
780+
}
781+
}
782+
}
783+
745784
/**
746785
* @override
747786
*/
@@ -760,6 +799,11 @@ export class Project extends ProjectBase {
760799
return ret;
761800
}
762801

802+
private set encryptionKey(encryptionKey: kms.IKey) {
803+
this._encryptionKey = encryptionKey;
804+
encryptionKey.grantEncryptDecrypt(this);
805+
}
806+
763807
private createLoggingPermission() {
764808
const logGroupArn = Stack.of(this).formatArn({
765809
service: 'logs',

packages/@aws-cdk/aws-codebuild/test/test.project.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
22
import ec2 = require('@aws-cdk/aws-ec2');
33
import iam = require('@aws-cdk/aws-iam');
4+
import s3 = require('@aws-cdk/aws-s3');
45
import { Bucket } from '@aws-cdk/aws-s3';
56
import cdk = require('@aws-cdk/core');
67
import { Test } from 'nodeunit';
@@ -148,6 +149,24 @@ export = {
148149

149150
test.done();
150151
},
152+
153+
'cannot have bindToCodePipeline() be called on it'(test: Test) {
154+
const stack = new cdk.Stack();
155+
const project = new codebuild.Project(stack, 'Project', {
156+
source: codebuild.Source.gitHub({
157+
owner: 'testowner',
158+
repo: 'testrepo',
159+
}),
160+
});
161+
162+
test.throws(() => {
163+
project.bindToCodePipeline(project, {
164+
artifactBucket: new s3.Bucket(stack, 'Bucket'),
165+
});
166+
}, /Only a PipelineProject can be added to a CodePipeline/);
167+
168+
test.done();
169+
},
151170
},
152171

153172
'project with s3 cache bucket'(test: Test) {

packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ export class CodeBuildAction extends Action {
113113
options.bucket.grantRead(this.props.project);
114114
}
115115

116+
if (this.props.project instanceof codebuild.Project) {
117+
this.props.project.bindToCodePipeline(scope, {
118+
artifactBucket: options.bucket,
119+
});
120+
}
121+
116122
const configuration: any = {
117123
ProjectName: this.props.project.projectName,
118124
};

packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,24 @@
118118
},
119119
"Resource": "*"
120120
},
121+
{
122+
"Action": [
123+
"kms:Decrypt",
124+
"kms:Encrypt",
125+
"kms:ReEncrypt*",
126+
"kms:GenerateDataKey*"
127+
],
128+
"Effect": "Allow",
129+
"Principal": {
130+
"AWS": {
131+
"Fn::GetAtt": [
132+
"CdkBuildProjectRoleE0B6FEB0",
133+
"Arn"
134+
]
135+
}
136+
},
137+
"Resource": "*"
138+
},
121139
{
122140
"Action": [
123141
"kms:Decrypt",
@@ -137,6 +155,24 @@
137155
},
138156
"Resource": "*"
139157
},
158+
{
159+
"Action": [
160+
"kms:Decrypt",
161+
"kms:Encrypt",
162+
"kms:ReEncrypt*",
163+
"kms:GenerateDataKey*"
164+
],
165+
"Effect": "Allow",
166+
"Principal": {
167+
"AWS": {
168+
"Fn::GetAtt": [
169+
"LambdaBuildProjectRoleD0C4F982",
170+
"Arn"
171+
]
172+
}
173+
},
174+
"Resource": "*"
175+
},
140176
{
141177
"Action": [
142178
"kms:Decrypt",
@@ -1386,6 +1422,21 @@
13861422
"Arn"
13871423
]
13881424
}
1425+
},
1426+
{
1427+
"Action": [
1428+
"kms:Decrypt",
1429+
"kms:Encrypt",
1430+
"kms:ReEncrypt*",
1431+
"kms:GenerateDataKey*"
1432+
],
1433+
"Effect": "Allow",
1434+
"Resource": {
1435+
"Fn::GetAtt": [
1436+
"PipelineArtifactsBucketEncryptionKey01D58D69",
1437+
"Arn"
1438+
]
1439+
}
13891440
}
13901441
],
13911442
"Version": "2012-10-17"
@@ -1402,7 +1453,13 @@
14021453
"Type": "AWS::CodeBuild::Project",
14031454
"Properties": {
14041455
"Artifacts": {
1405-
"Type": "NO_ARTIFACTS"
1456+
"Type": "CODEPIPELINE"
1457+
},
1458+
"EncryptionKey": {
1459+
"Fn::GetAtt": [
1460+
"PipelineArtifactsBucketEncryptionKey01D58D69",
1461+
"Arn"
1462+
]
14061463
},
14071464
"Environment": {
14081465
"ComputeType": "BUILD_GENERAL1_SMALL",
@@ -1418,7 +1475,7 @@
14181475
},
14191476
"Source": {
14201477
"BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install\"\n },\n \"build\": {\n \"commands\": [\n \"npm run build\",\n \"npm run cdk synth LambdaStack -- -o .\"\n ]\n }\n },\n \"artifacts\": {\n \"files\": \"LambdaStack.template.yaml\"\n }\n}",
1421-
"Type": "NO_SOURCE"
1478+
"Type": "CODEPIPELINE"
14221479
}
14231480
}
14241481
},
@@ -1549,6 +1606,21 @@
15491606
"Arn"
15501607
]
15511608
}
1609+
},
1610+
{
1611+
"Action": [
1612+
"kms:Decrypt",
1613+
"kms:Encrypt",
1614+
"kms:ReEncrypt*",
1615+
"kms:GenerateDataKey*"
1616+
],
1617+
"Effect": "Allow",
1618+
"Resource": {
1619+
"Fn::GetAtt": [
1620+
"PipelineArtifactsBucketEncryptionKey01D58D69",
1621+
"Arn"
1622+
]
1623+
}
15521624
}
15531625
],
15541626
"Version": "2012-10-17"
@@ -1565,7 +1637,13 @@
15651637
"Type": "AWS::CodeBuild::Project",
15661638
"Properties": {
15671639
"Artifacts": {
1568-
"Type": "NO_ARTIFACTS"
1640+
"Type": "CODEPIPELINE"
1641+
},
1642+
"EncryptionKey": {
1643+
"Fn::GetAtt": [
1644+
"PipelineArtifactsBucketEncryptionKey01D58D69",
1645+
"Arn"
1646+
]
15691647
},
15701648
"Environment": {
15711649
"ComputeType": "BUILD_GENERAL1_SMALL",
@@ -1581,7 +1659,7 @@
15811659
},
15821660
"Source": {
15831661
"BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install\"\n },\n \"build\": {\n \"commands\": \"npm run build\"\n }\n },\n \"artifacts\": {\n \"files\": [\n \"index.js\",\n \"node_modules/**/*\"\n ]\n }\n}",
1584-
"Type": "NO_SOURCE"
1662+
"Type": "CODEPIPELINE"
15851663
}
15861664
}
15871665
}

packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pipeline.addStage({
4747
// synthesize the Lambda CDK template, using CodeBuild
4848
// the below values are just examples, assuming your CDK code is in TypeScript/JavaScript -
4949
// adjust the build environment and/or commands accordingly
50-
const cdkBuildProject = new codebuild.Project(pipelineStack, 'CdkBuildProject', {
50+
const cdkBuildProject = new codebuild.PipelineProject(pipelineStack, 'CdkBuildProject', {
5151
environment: {
5252
buildImage: codebuild.LinuxBuildImage.UBUNTU_14_04_NODEJS_10_1_0,
5353
},
@@ -80,7 +80,7 @@ const cdkBuildAction = new codepipeline_actions.CodeBuildAction({
8080
// build your Lambda code, using CodeBuild
8181
// again, this example assumes your Lambda is written in TypeScript/JavaScript -
8282
// make sure to adjust the build environment and/or commands if they don't match your specific situation
83-
const lambdaBuildProject = new codebuild.Project(pipelineStack, 'LambdaBuildProject', {
83+
const lambdaBuildProject = new codebuild.PipelineProject(pipelineStack, 'LambdaBuildProject', {
8484
environment: {
8585
buildImage: codebuild.LinuxBuildImage.UBUNTU_14_04_NODEJS_10_1_0,
8686
},

packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit-build.expected.json

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,21 @@
134134
]
135135
}
136136
},
137+
{
138+
"Action": [
139+
"kms:Decrypt",
140+
"kms:Encrypt",
141+
"kms:ReEncrypt*",
142+
"kms:GenerateDataKey*"
143+
],
144+
"Effect": "Allow",
145+
"Resource": {
146+
"Fn::GetAtt": [
147+
"PipelineArtifactsBucketEncryptionKey01D58D69",
148+
"Arn"
149+
]
150+
}
151+
},
137152
{
138153
"Action": [
139154
"s3:GetObject*",
@@ -194,6 +209,12 @@
194209
"Artifacts": {
195210
"Type": "CODEPIPELINE"
196211
},
212+
"EncryptionKey": {
213+
"Fn::GetAtt": [
214+
"PipelineArtifactsBucketEncryptionKey01D58D69",
215+
"Arn"
216+
]
217+
},
197218
"Environment": {
198219
"ComputeType": "BUILD_GENERAL1_SMALL",
199220
"Image": "aws/codebuild/standard:1.0",
@@ -310,6 +331,24 @@
310331
},
311332
"Resource": "*"
312333
},
334+
{
335+
"Action": [
336+
"kms:Decrypt",
337+
"kms:Encrypt",
338+
"kms:ReEncrypt*",
339+
"kms:GenerateDataKey*"
340+
],
341+
"Effect": "Allow",
342+
"Principal": {
343+
"AWS": {
344+
"Fn::GetAtt": [
345+
"MyBuildProjectRole6B7E2258",
346+
"Arn"
347+
]
348+
}
349+
},
350+
"Resource": "*"
351+
},
313352
{
314353
"Action": [
315354
"kms:Decrypt",

0 commit comments

Comments
 (0)