From d2d4b87252182726ee29c17d089b8c3cd991c1ec Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Tue, 28 May 2019 14:24:01 -0700 Subject: [PATCH] fix(codebuild): correctly handle permissions for Projects inside VPC. 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 --- packages/@aws-cdk/aws-codebuild/README.md | 23 ++++-- .../@aws-cdk/aws-codebuild/lib/project.ts | 78 +++++++++++-------- .../test/integ.project-vpc.expected.json | 5 +- .../aws-codebuild/test/test.project.ts | 23 ++++++ 4 files changed, 87 insertions(+), 42 deletions(-) diff --git a/packages/@aws-cdk/aws-codebuild/README.md b/packages/@aws-cdk/aws-codebuild/README.md index 0b23c72829556..e87c2ed3d5ac7 100644 --- a/packages/@aws-cdk/aws-codebuild/README.md +++ b/packages/@aws-cdk/aws-codebuild/README.md @@ -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', }), }); @@ -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')) }); ``` @@ -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) }); ``` @@ -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: { @@ -285,7 +290,7 @@ const project = new codebuild.Project(this, 'MyProject', { }, }, }, - }, + }), }); ``` @@ -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 }); diff --git a/packages/@aws-cdk/aws-codebuild/lib/project.ts b/packages/@aws-cdk/aws-codebuild/lib/project.ts index 053ed6c824fed..5f128eef889c1 100644 --- a/packages/@aws-cdk/aws-codebuild/lib/project.ts +++ b/packages/@aws-cdk/aws-codebuild/lib/project.ts @@ -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'; @@ -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, @@ -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. * @@ -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) { diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.expected.json b/packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.expected.json index 55fbb665cbe1b..e8427767496d6 100644 --- a/packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.expected.json +++ b/packages/@aws-cdk/aws-codebuild/test/integ.project-vpc.expected.json @@ -423,7 +423,10 @@ "Ref": "MyVPCAFB07A31" } } - } + }, + "DependsOn": [ + "MyProjectPolicyDocument646EE0F2" + ] } } } diff --git a/packages/@aws-cdk/aws-codebuild/test/test.project.ts b/packages/@aws-cdk/aws-codebuild/test/test.project.ts index 6d87439a9e2e6..65ea194a8b3e5 100644 --- a/packages/@aws-cdk/aws-codebuild/test/test.project.ts +++ b/packages/@aws-cdk/aws-codebuild/test/test.project.ts @@ -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'; @@ -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(); + }, };