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
…#2662)

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.

BREAKING CHANGE: the method addToRoleInlinePolicy in CodeBuild's Project class has been removed.

Fixes #2651
Fixes #2652
  • Loading branch information
skinny85 authored Jun 18, 2019
1 parent d229836 commit 390baf1
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 42 deletions.
23 changes: 15 additions & 8 deletions packages/@aws-cdk/aws-codebuild/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ import s3 = require('@aws-cdk/aws-s3');
const bucket = new s3.Bucket(this, 'MyBucket');
new codebuild.Project(this, 'MyProject', {
source: codebuild.Source.s3({
bucket: bucket,
bucket,
path: 'path/to/file.zip',
}),
});
Expand Down Expand Up @@ -138,7 +138,10 @@ With S3 caching, the cache is stored in an S3 bucket which is available from mul

```typescript
new codebuild.Project(this, 'Project', {
source: new codebuild.CodePipelineSource(),
source: codebuild.Source.bitBucket({
owner: 'awslabs',
repo: 'aws-cdk',
}),
cache: codebuild.Cache.bucket(new Bucket(this, 'Bucket'))
});
```
Expand All @@ -153,7 +156,9 @@ With local caching, the cache is stored on the codebuild instance itself. CodeBu

```typescript
new codebuild.Project(this, 'Project', {
source: new codebuild.CodePipelineSource(),
source: codebuild.Source.gitHubEnterprise({
httpsCloneUrl: 'https://my-github-enterprise.com/owner/repo',
}),
cache: codebuild.Cache.local(LocalCacheMode.DockerLayer, LocalCacheMode.Custom)
});
```
Expand Down Expand Up @@ -262,10 +267,10 @@ with their identifier.

So, a buildspec for the above Project could look something like this:

```ts
```typescript
const project = new codebuild.Project(this, 'MyProject', {
// secondary sources and artifacts as above...
buildSpec: {
buildSpec: codebuild.BuildSpec.fromObject({
version: '0.2',
phases: {
build: {
Expand All @@ -285,7 +290,7 @@ const project = new codebuild.Project(this, 'MyProject', {
},
},
},
},
}),
});
```

Expand Down Expand Up @@ -330,8 +335,10 @@ const securityGroup = new ec2.SecurityGroup(stack, 'SecurityGroup1', {
groupName: 'MySecurityGroup',
vpc: vpc,
});
new Project(stack, 'MyProject', {
buildScript: new assets.ZipDirectoryAsset(stack, 'Bundle', { path: 'script_bundle' }),
new codebuild.Project(stack, 'MyProject', {
buildSpec: codebuild.BuildSpec.fromObject({
// ...
}),
securityGroups: [securityGroup],
vpc: vpc
});
Expand Down
78 changes: 45 additions & 33 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,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, Lazy, PhysicalName, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk';
import { Aws, CfnResource, Construct, IResource, Lazy, PhysicalName, Resource, ResourceIdentifiers, Stack } from '@aws-cdk/cdk';
import { IArtifacts } from './artifacts';
import { BuildSpec } from './build-spec';
import { Cache } from './cache';
Expand Down Expand Up @@ -699,6 +699,8 @@ export class Project extends ProjectBase {
vpcConfig: this.configureVpc(props),
});

this.addVpcRequiredPermissions(props, resource);

const resourceIdentifiers = new ResourceIdentifiers(this, {
arn: resource.attrArn,
name: resource.refAsString,
Expand All @@ -718,20 +720,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 @@ -869,31 +857,55 @@ export class Project extends ProjectBase {
}
this._connections = new ec2.Connections({ securityGroups });

this.addToRoleInlinePolicy(new iam.PolicyStatement({
resources: ['*'],
actions: [
'ec2:CreateNetworkInterface', 'ec2:DescribeNetworkInterfaces',
'ec2:DeleteNetworkInterface', 'ec2:DescribeSubnets',
'ec2:DescribeSecurityGroups', 'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs']
}));
this.addToRolePolicy(new iam.PolicyStatement({
return {
vpcId: props.vpc.vpcId,
subnets: props.vpc.selectSubnets(props.subnetSelection).subnetIds,
securityGroupIds: this.connections.securityGroups.map(s => s.securityGroupId)
};
}

private addVpcRequiredPermissions(props: ProjectProps, project: CfnProject): void {
if (!props.vpc || !this.role) {
return;
}

this.role.addToPolicy(new iam.PolicyStatement({
resources: [`arn:aws:ec2:${Aws.region}:${Aws.accountId}:network-interface/*`],
actions: ['ec2:CreateNetworkInterfacePermission'],
conditions: {
StringEquals: {
"ec2:Subnet": props.vpc
'ec2:Subnet': props.vpc
.selectSubnets(props.subnetSelection).subnetIds
.map(si => `arn:aws:ec2:${Aws.region}:${Aws.accountId}:subnet/${si}`),
"ec2:AuthorizedService": "codebuild.amazonaws.com"
}
}
'ec2:AuthorizedService': 'codebuild.amazonaws.com'
},
},
}));
return {
vpcId: props.vpc.vpcId,
subnets: props.vpc.selectSubnets(props.subnetSelection).subnetIds,
securityGroupIds: this.connections.securityGroups.map(s => s.securityGroupId)
};

const policy = new iam.Policy(this, 'PolicyDocument', {
policyName: 'CodeBuildEC2Policy',
statements: [
new iam.PolicyStatement({
resources: ['*'],
actions: [
'ec2:CreateNetworkInterface',
'ec2:DescribeNetworkInterfaces',
'ec2:DeleteNetworkInterface',
'ec2:DescribeSubnets',
'ec2:DescribeSecurityGroups',
'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs',
],
}),
],
});
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;
project.addDependsOn(cfnPolicy);
}

private validateCodePipelineSettings(artifacts: IArtifacts) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,10 @@
"Ref": "MyVPCAFB07A31"
}
}
}
},
"DependsOn": [
"MyProjectPolicyDocument646EE0F2"
]
}
}
}
23 changes: 23 additions & 0 deletions packages/@aws-cdk/aws-codebuild/test/test.project.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { expect, haveResource, haveResourceLike, not } from '@aws-cdk/assert';
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 @@ -232,4 +234,25 @@ 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: codebuild.Source.gitHubEnterprise({
httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
}),
role: importedRole,
vpc,
});

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

test.done();
},
};

0 comments on commit 390baf1

Please sign in to comment.