Skip to content

Commit

Permalink
fix(pipelines): unable to publish assets inside VPC (#12331)
Browse files Browse the repository at this point in the history
The CodeBuild projects for the asset publishing could not be
created inside a VPC, as the Exeuction Roles assigned to it
were missing the required permissions.

Normally the CodeBuild project would take care of these permissions,
but since we are creating a single role that is shared between all
CodeBuild projects (in order to not explode the number of roles in the
account) and the role we manage is immutable, the pipeline library
itself is responsible for adding the permissions.

Fixes #11815


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Jan 26, 2021
1 parent 532c91f commit a16f09c
Show file tree
Hide file tree
Showing 4 changed files with 377 additions and 314 deletions.
36 changes: 35 additions & 1 deletion packages/@aws-cdk/pipelines/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as path from 'path';
import * as codepipeline from '@aws-cdk/aws-codepipeline';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import { Annotations, App, CfnOutput, PhysicalName, Stack, Stage } from '@aws-cdk/core';
import { Annotations, App, Aws, CfnOutput, PhysicalName, Stack, Stage } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { AssetType, DeployCdkStackAction, PublishAssetsAction, UpdatePipelineAction } from './actions';
import { appOf, assemblyBuilderOf } from './private/construct-internals';
Expand Down Expand Up @@ -516,6 +516,40 @@ class AssetPublishing extends CoreConstruct {
// Artifact access
this.pipeline.artifactBucket.grantRead(assetRole);

// VPC permissions required for CodeBuild
// Normally CodeBuild itself takes care of this but we're creating a singleton role so now
// we need to do this.
if (this.props.vpc) {
assetRole.attachInlinePolicy(new iam.Policy(assetRole, 'VpcPolicy', {
statements: [
new iam.PolicyStatement({
resources: [`arn:${Aws.PARTITION}:ec2:${Aws.REGION}:${Aws.ACCOUNT_ID}:network-interface/*`],
actions: ['ec2:CreateNetworkInterfacePermission'],
conditions: {
StringEquals: {
'ec2:Subnet': this.props.vpc
.selectSubnets(this.props.subnetSelection).subnetIds
.map(si => `arn:${Aws.PARTITION}:ec2:${Aws.REGION}:${Aws.ACCOUNT_ID}:subnet/${si}`),
'ec2:AuthorizedService': 'codebuild.amazonaws.com',
},
},
}),
new iam.PolicyStatement({
resources: ['*'],
actions: [
'ec2:CreateNetworkInterface',
'ec2:DescribeNetworkInterfaces',
'ec2:DeleteNetworkInterface',
'ec2:DescribeSubnets',
'ec2:DescribeSecurityGroups',
'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs',
],
}),
],
}));
}

this.assetRoles[assetType] = assetRole.withoutPolicyUpdates();
return this.assetRoles[assetType];
}
Expand Down
39 changes: 21 additions & 18 deletions packages/@aws-cdk/pipelines/test/builds.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,27 +331,27 @@ test('Standard (NPM) synth can run in a VPC', () => {
expect(pipelineStack).toHaveResourceLike('AWS::CodeBuild::Project', {
VpcConfig: {
SecurityGroupIds: [
{
'Fn::GetAtt': [
'CdkPipelineBuildSynthCdkBuildProjectSecurityGroupEA44D7C2',
'GroupId',
],
},
{ 'Fn::GetAtt': ['CdkPipelineBuildSynthCdkBuildProjectSecurityGroupEA44D7C2', 'GroupId'] },
],
Subnets: [
{
Ref: 'NpmSynthTestVpcPrivateSubnet1Subnet81E3AA56',
},
{
Ref: 'NpmSynthTestVpcPrivateSubnet2SubnetC1CA3EF0',
},
{
Ref: 'NpmSynthTestVpcPrivateSubnet3SubnetA04163EE',
},
{ Ref: 'NpmSynthTestVpcPrivateSubnet1Subnet81E3AA56' },
{ Ref: 'NpmSynthTestVpcPrivateSubnet2SubnetC1CA3EF0' },
{ Ref: 'NpmSynthTestVpcPrivateSubnet3SubnetA04163EE' },
],
VpcId: {
Ref: 'NpmSynthTestVpc5E703F25',
},
VpcId: { Ref: 'NpmSynthTestVpc5E703F25' },
},
});

expect(pipelineStack).toHaveResourceLike('AWS::IAM::Policy', {
Roles: [
{ Ref: 'CdkPipelineBuildSynthCdkBuildProjectRole5E173C62' },
],
PolicyDocument: {
Statement: arrayWith({
Action: arrayWith('ec2:DescribeSecurityGroups'),
Effect: 'Allow',
Resource: '*',
}),
},
});
});
Expand Down Expand Up @@ -520,6 +520,9 @@ test('SimpleSynthAction can reference an imported ECR repo', () => {
},
}),
});

// THEN -- no exception (necessary for linter)
expect(true).toBeTruthy();
});

function npmYarnBuild(npmYarn: string) {
Expand Down
Loading

0 comments on commit a16f09c

Please sign in to comment.