Skip to content

Commit

Permalink
fix(codepipeline-actions): CodeCommit source action fails when it's c…
Browse files Browse the repository at this point in the history
…ross-account (#14260)

Apparently, when removing the s3:PutObject* permissions in #12391,
we broke the CodeCommitSourceAction when it's cross-account.

Not entirely sure why is that permission required only when the action is cross-account,
but I have confirmed this fixes the problem,
so add an explicit call to `Bucket.grantPutAcl()`
when the actions is cross-account.

Fixes #14156

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
skinny85 authored Apr 21, 2021
1 parent 283ed02 commit 1508e60
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as codecommit from '@aws-cdk/aws-codecommit';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as targets from '@aws-cdk/aws-events-targets';
import * as iam from '@aws-cdk/aws-iam';
import { Names, Token } from '@aws-cdk/core';
import { Names, Stack, Token, TokenComparison } from '@aws-cdk/core';
import { Action } from '../action';
import { sourceArtifactBounds } from '../common';

Expand Down Expand Up @@ -171,6 +171,11 @@ export class CodeCommitSourceAction extends Action {
// the Action will write the contents of the Git repository to the Bucket,
// so its Role needs write permissions to the Pipeline Bucket
options.bucket.grantReadWrite(options.role);
// when this action is cross-account,
// the Role needs the s3:PutObjectAcl permission for some not yet fully understood reason
if (Token.compareStrings(this.props.repository.env.account, Stack.of(stage.pipeline).account) === TokenComparison.DIFFERENT) {
options.bucket.grantPutAcl(options.role);
}

// https://docs.aws.amazon.com/codecommit/latest/userguide/auth-and-access-control-permissions-reference.html#aa-acp
options.role.addToPrincipalPolicy(new iam.PolicyStatement({
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-codepipeline-actions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"@aws-cdk/aws-events": "0.0.0",
"@aws-cdk/aws-events-targets": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-sns": "0.0.0",
Expand All @@ -110,6 +111,7 @@
"@aws-cdk/aws-events": "0.0.0",
"@aws-cdk/aws-events-targets": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-sns": "0.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import * as codebuild from '@aws-cdk/aws-codebuild';
import * as codecommit from '@aws-cdk/aws-codecommit';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as iam from '@aws-cdk/aws-iam';
import { Stack, Lazy } from '@aws-cdk/core';
import * as kms from '@aws-cdk/aws-kms';
import * as s3 from '@aws-cdk/aws-s3';
import { Stack, Lazy, App } from '@aws-cdk/core';
import { nodeunitShim, Test } from 'nodeunit-shim';
import * as cpactions from '../../lib';

Expand Down Expand Up @@ -429,6 +431,60 @@ nodeunitShim({

test.done();
},

'grants explicit s3:PutObjectAcl permissions when the Actions is cross-account'(test: Test) {
const app = new App();

const repoStack = new Stack(app, 'RepoStack', {
env: { account: '123', region: 'us-east-1' },
});
const repoFomAnotherAccount = codecommit.Repository.fromRepositoryName(repoStack, 'Repo', 'my-repo');

const pipelineStack = new Stack(app, 'PipelineStack', {
env: { account: '456', region: 'us-east-1' },
});
new codepipeline.Pipeline(pipelineStack, 'Pipeline', {
artifactBucket: s3.Bucket.fromBucketAttributes(pipelineStack, 'PipelineBucket', {
bucketName: 'pipeline-bucket',
encryptionKey: kms.Key.fromKeyArn(pipelineStack, 'PipelineKey',
'arn:aws:kms:us-east-1:456:key/my-key'),
}),
stages: [
{
stageName: 'Source',
actions: [new cpactions.CodeCommitSourceAction({
actionName: 'Source',
output: new codepipeline.Artifact(),
repository: repoFomAnotherAccount,
})],
},
{
stageName: 'Approve',
actions: [new cpactions.ManualApprovalAction({
actionName: 'Approve',
})],
},
],
});

expect(repoStack).to(haveResourceLike('AWS::IAM::Policy', {
PolicyDocument: {
Statement: arrayWith({
'Action': 's3:PutObjectAcl',
'Effect': 'Allow',
'Resource': {
'Fn::Join': ['', [
'arn:',
{ 'Ref': 'AWS::Partition' },
':s3:::pipeline-bucket/*',
]],
},
}),
},
}));

test.done();
},
},
});

Expand Down

0 comments on commit 1508e60

Please sign in to comment.