Skip to content

Commit

Permalink
fix(codebuild): correctly handle permissions for Projects inside VPC.
Browse files Browse the repository at this point in the history
A CodeBuild Project needs to have appropriate EC2 permissions on creation
when it uses a VPC. However, the default Policy that a Project Role has
depends on the Project itself (for CloudWatch Logs permissions).
Because of that, add a dependency between the Policy containing the EC2
permissions and the Project.

Also correctly handle the case when the Project's Role is imported.

Fixes #2651
Fixes #2652
  • Loading branch information
skinny85 committed May 28, 2019
1 parent 5fe0af5 commit 2eef183
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 33 deletions.
81 changes: 50 additions & 31 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import ecr = require('@aws-cdk/aws-ecr');
import events = require('@aws-cdk/aws-events');
import iam = require('@aws-cdk/aws-iam');
import kms = require('@aws-cdk/aws-kms');
import { Aws, Construct, IResource, Resource, Token } from '@aws-cdk/cdk';
import { Aws, CfnResource, Construct, IResource, Resource, Token } from '@aws-cdk/cdk';
import { BuildArtifacts, CodePipelineBuildArtifacts, NoBuildArtifacts } from './artifacts';
import { Cache } from './cache';
import { CfnProject } from './codebuild.generated';
Expand Down Expand Up @@ -707,6 +707,8 @@ export class Project extends ProjectBase {
vpcConfig: this.configureVpc(props),
});

this.addVpcRequiredPermissions(props);

this.projectArn = resource.projectArn;
this.projectName = resource.projectName;

Expand All @@ -727,20 +729,6 @@ export class Project extends ProjectBase {
}
}

/**
* Add a permission only if there's a policy attached.
* @param statement The permissions statement to add
*/
public addToRoleInlinePolicy(statement: iam.PolicyStatement) {
if (this.role) {
const policy = new iam.Policy(this, 'PolicyDocument', {
policyName: 'CodeBuildEC2Policy',
statements: [statement]
});
this.role.attachInlinePolicy(policy);
}
}

/**
* Adds a secondary source to the Project.
*
Expand Down Expand Up @@ -882,7 +870,29 @@ export class Project extends ProjectBase {
});
this._securityGroups = [securityGroup];
}
this.addToRoleInlinePolicy(new iam.PolicyStatement()

return {
vpcId: props.vpc.vpcId,
subnets: props.vpc.selectSubnets(props.subnetSelection).subnetIds,
securityGroupIds: this._securityGroups.map(s => s.securityGroupId)
};
}

private addVpcRequiredPermissions(props: ProjectProps): void {
if (!props.vpc) {
return;
}

this.addToRolePolicy(new iam.PolicyStatement()
.addResource(`arn:aws:ec2:${Aws.region}:${Aws.accountId}:network-interface/*`)
.addAction('ec2:CreateNetworkInterfacePermission')
.addCondition('StringEquals', {
'ec2:Subnet': props.vpc
.selectSubnets(props.subnetSelection).subnetIds
.map(si => `arn:aws:ec2:${Aws.region}:${Aws.accountId}:subnet/${si}`),
'ec2:AuthorizedService': "codebuild.amazonaws.com",
}));
this.addInlineEc2Policy(new iam.PolicyStatement()
.addAllResources()
.addActions(
'ec2:CreateNetworkInterface',
Expand All @@ -891,22 +901,31 @@ export class Project extends ProjectBase {
'ec2:DescribeSubnets',
'ec2:DescribeSecurityGroups',
'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs'
'ec2:DescribeVpcs',
));
this.addToRolePolicy(new iam.PolicyStatement()
.addResource(`arn:aws:ec2:${Aws.region}:${Aws.accountId}:network-interface/*`)
.addCondition('StringEquals', {
"ec2:Subnet": props.vpc
.selectSubnets(props.subnetSelection).subnetIds
.map(si => `arn:aws:ec2:${Aws.region}:${Aws.accountId}:subnet/${si}`),
"ec2:AuthorizedService": "codebuild.amazonaws.com"
})
.addAction('ec2:CreateNetworkInterfacePermission'));
return {
vpcId: props.vpc.vpcId,
subnets: props.vpc.selectSubnets(props.subnetSelection).subnetIds,
securityGroupIds: this._securityGroups.map(s => s.securityGroupId)
};
}

private addInlineEc2Policy(statement: iam.PolicyStatement) {
if (this.role) {
// If the Role is imported, the Policy will be left dangling,
// not attached to any IAM entity, which is invalid in CFN.
// Because of that, check for that case before creating the Policy
const cfnRole = this.role.node.tryFindChild('Resource') as CfnResource;
if (cfnRole) {
const policy = new iam.Policy(this, 'PolicyDocument', {
policyName: 'CodeBuildEC2Policy',
statements: [statement]
});
this.role.attachInlinePolicy(policy);

// add an explicit dependency between the EC2 Policy and this Project -
// otherwise, creating the Project fails,
// as it requires these permissions to be already attached to the Project's Role
const cfnPolicy = policy.node.findChild('Resource') as CfnResource;
const cfnProject = this.node.findChild('Resource') as CfnResource;
cfnProject.addDependsOn(cfnPolicy);
}
}
}

private parseArtifacts(props: ProjectProps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,10 @@
"Ref": "MyVPCAFB07A31"
}
}
}
},
"DependsOn": [
"MyProjectPolicyDocument646EE0F2"
]
}
},
"Parameters": {
Expand All @@ -541,4 +544,4 @@
"Description": "Artifact hash for asset \"aws-cdk-codebuild-project-vpc/Bundle\""
}
}
}
}
24 changes: 24 additions & 0 deletions packages/@aws-cdk/aws-codebuild/test/test.project.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
import assets = require('@aws-cdk/assets');
import ec2 = require('@aws-cdk/aws-ec2');
import iam = require('@aws-cdk/aws-iam');
import { Bucket } from '@aws-cdk/aws-s3';
import cdk = require('@aws-cdk/cdk');
import { Test } from 'nodeunit';
Expand Down Expand Up @@ -239,4 +241,26 @@ export = {

test.done();
},

'can use an imported Role for a Project within a VPC'(test: Test) {
const stack = new cdk.Stack();

const importedRole = iam.Role.fromRoleArn(stack, 'Role', 'arn:aws:iam::1234567890:role/service-role/codebuild-bruiser-service-role');
const vpc = new ec2.Vpc(stack, 'Vpc');

new codebuild.Project(stack, 'Project', {
source: new codebuild.GitHubEnterpriseSource({
httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
}),
artifacts: new codebuild.NoBuildArtifacts(),
role: importedRole,
vpc,
});

expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
// no need to do any assertions
}));

test.done();
},
};

0 comments on commit 2eef183

Please sign in to comment.