Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pipelines): Secrets Manager permissions not added to asset projects #15718

Merged
merged 5 commits into from
Jul 23, 2021
Merged

fix(pipelines): Secrets Manager permissions not added to asset projects #15718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2d25c27
fix(pipelines): Secrets Manager permissions not added to asset projects
rix0rrr Jul 22, 2021
b8b0160
Fix linter errors
rix0rrr Jul 23, 2021
c65359d
Make sure VPC policies don't get added multiple times
rix0rrr Jul 23, 2021
6b781ff
Add test to prove that it works
rix0rrr Jul 23, 2021
1f14242
Merge branch 'master' into huijbers/asset-role-permissions
mergify[bot] Jul 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 29 additions & 17 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import { CODEPIPELINE_SOURCE_ARTIFACTS_TYPE, NO_SOURCE_TYPE } from './source-typ
// eslint-disable-next-line
import { Construct as CoreConstruct } from '@aws-cdk/core';

const VPC_POLICY_SYM = Symbol.for('@aws-cdk/aws-codebuild.roleVpcPolicy');

/**
* The type returned from {@link IProject#enableBatchBuilds}.
*/
Expand Down Expand Up @@ -1437,23 +1439,33 @@ export class Project extends ProjectBase {
},
}));

const policy = new iam.Policy(this, 'PolicyDocument', {
statements: [
new iam.PolicyStatement({
resources: ['*'],
actions: [
'ec2:CreateNetworkInterface',
'ec2:DescribeNetworkInterfaces',
'ec2:DeleteNetworkInterface',
'ec2:DescribeSubnets',
'ec2:DescribeSecurityGroups',
'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs',
],
}),
],
});
this.role.attachInlinePolicy(policy);
// If the same Role is used for multiple Projects, always creating a new `iam.Policy`
// will attach the same policy multiple times, probably exceeding the maximum size of the
// Role policy. Make sure we only do it once for the same role.
//
// This deduplication could be a feature of the Role itself, but that feels risky and
// is hard to implement (what with Tokens and all). Safer to fix it locally for now.
let policy: iam.Policy | undefined = (this.role as any)[VPC_POLICY_SYM];
if (!policy) {
policy = new iam.Policy(this, 'PolicyDocument', {
statements: [
new iam.PolicyStatement({
resources: ['*'],
actions: [
'ec2:CreateNetworkInterface',
'ec2:DescribeNetworkInterfaces',
'ec2:DeleteNetworkInterface',
'ec2:DescribeSubnets',
'ec2:DescribeSecurityGroups',
'ec2:DescribeDhcpOptions',
'ec2:DescribeVpcs',
],
}),
],
});
this.role.attachInlinePolicy(policy);
(this.role as any)[VPC_POLICY_SYM] = policy;
}

// add an explicit dependency between the EC2 Policy and this Project -
// otherwise, creating the Project fails, as it requires these permissions
Expand Down
35 changes: 35 additions & 0 deletions packages/@aws-cdk/aws-codebuild/test/test.project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,41 @@ export = {
test.done();
},

'if a role is shared between projects in a VPC, the VPC Policy is only attached once'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const vpc = new ec2.Vpc(stack, 'Vpc');
const role = new iam.Role(stack, 'Role', {
assumedBy: new iam.ServicePrincipal('codebuild.amazonaws.com'),
});
const source = codebuild.Source.gitHubEnterprise({
httpsCloneUrl: 'https://mygithub-enterprise.com/myuser/myrepo',
});

// WHEN
new codebuild.Project(stack, 'Project1', { source, role, vpc, projectName: 'P1' });
new codebuild.Project(stack, 'Project2', { source, role, vpc, projectName: 'P2' });

// THEN
// - 1 is for `ec2:CreateNetworkInterfacePermission`, deduplicated as they're part of a single policy
// - 1 is for `ec2:CreateNetworkInterface`, this is the separate Policy we're deduplicating
// We would have found 3 if the deduplication didn't work.
expect(stack).to(countResources('AWS::IAM::Policy', 2));

// THEN - both Projects have a DependsOn on the same policy
expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
Properties: { Name: 'P1' },
DependsOn: ['Project1PolicyDocumentF9761562'],
}, ResourcePart.CompleteDefinition));

expect(stack).to(haveResourceLike('AWS::CodeBuild::Project', {
Properties: { Name: 'P1' },
DependsOn: ['Project1PolicyDocumentF9761562'],
}, ResourcePart.CompleteDefinition));

test.done();
},

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

Expand Down
113 changes: 9 additions & 104 deletions packages/@aws-cdk/pipelines/lib/codepipeline/codepipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import * as cp from '@aws-cdk/aws-codepipeline';
import * as cpa from '@aws-cdk/aws-codepipeline-actions';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import { Aws, Fn, IDependable, Lazy, PhysicalName, Stack } from '@aws-cdk/core';
import { Aws, Fn, Lazy, PhysicalName, Stack } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Construct, Node } from 'constructs';
import { AssetType, FileSet, IFileSetProducer, ManualApprovalStep, ShellStep, StackAsset, StackDeployment, Step } from '../blueprint';
import { DockerCredential, dockerCredentialsInstallCommands, DockerCredentialUsage } from '../docker-credentials';
import { GraphNode, GraphNodeCollection, isGraph, AGraphNode, PipelineGraph } from '../helpers-internal';
import { PipelineBase } from '../main';
import { AssetSingletonRole } from '../private/asset-singleton-role';
import { appOf, assemblyBuilderOf, embeddedAsmPath, obtainScope } from '../private/construct-internals';
import { toPosixPath } from '../private/fs';
import { enumerate, flatten, maybeSuffix, noUndefined } from '../private/javascript';
Expand Down Expand Up @@ -254,11 +255,6 @@ export class CodePipeline extends PipelineBase {
*/
private readonly assetCodeBuildRoles: Record<string, iam.IRole> = {};

/**
* Policies created for the build projects that they have to depend on
*/
private readonly assetAttachedPolicies: Record<string, iam.Policy> = {};

/**
* Per asset type, the target role ARNs that need to be assumed
*/
Expand Down Expand Up @@ -635,7 +631,7 @@ export class CodePipeline extends PipelineBase {
}
}

const assetBuildConfig = this.obtainAssetCodeBuildRole(assets[0].assetType);
const role = this.obtainAssetCodeBuildRole(assets[0].assetType);

// The base commands that need to be run
const script = new CodeBuildStep(node.id, {
Expand All @@ -647,13 +643,12 @@ export class CodePipeline extends PipelineBase {
buildEnvironment: {
privileged: assets.some(asset => asset.assetType === AssetType.DOCKER_IMAGE),
},
role: assetBuildConfig.role,
role,
});

// Customizations that are not accessible to regular users
return CodeBuildFactory.fromCodeBuildStep(node.id, script, {
additionalConstructLevel: false,
additionalDependable: assetBuildConfig.dependable,

// If we use a single publisher, pass buildspec via file otherwise it'll
// grow too big.
Expand Down Expand Up @@ -775,64 +770,22 @@ export class CodePipeline extends PipelineBase {
* Modeled after the CodePipeline role and 'CodePipelineActionRole' roles.
* Generates one role per asset type to separate file and Docker/image-based permissions.
*/
private obtainAssetCodeBuildRole(assetType: AssetType): AssetCodeBuildRole {
private obtainAssetCodeBuildRole(assetType: AssetType): iam.IRole {
if (this.assetCodeBuildRoles[assetType]) {
return {
role: this.assetCodeBuildRoles[assetType],
dependable: this.assetAttachedPolicies[assetType],
};
return this.assetCodeBuildRoles[assetType];
}

const stack = Stack.of(this);

const rolePrefix = assetType === AssetType.DOCKER_IMAGE ? 'Docker' : 'File';
const assetRole = new iam.Role(this.assetsScope, `${rolePrefix}Role`, {
const assetRole = new AssetSingletonRole(this.assetsScope, `${rolePrefix}Role`, {
roleName: PhysicalName.GENERATE_IF_NEEDED,
assumedBy: new iam.CompositePrincipal(
new iam.ServicePrincipal('codebuild.amazonaws.com'),
new iam.AccountPrincipal(stack.account),
),
});

// Logging permissions
const logGroupArn = stack.formatArn({
service: 'logs',
resource: 'log-group',
sep: ':',
resourceName: '/aws/codebuild/*',
});
assetRole.addToPolicy(new iam.PolicyStatement({
resources: [logGroupArn],
actions: ['logs:CreateLogGroup', 'logs:CreateLogStream', 'logs:PutLogEvents'],
}));

// CodeBuild report groups
const codeBuildArn = stack.formatArn({
service: 'codebuild',
resource: 'report-group',
resourceName: '*',
});
assetRole.addToPolicy(new iam.PolicyStatement({
actions: [
'codebuild:CreateReportGroup',
'codebuild:CreateReport',
'codebuild:UpdateReport',
'codebuild:BatchPutTestCases',
'codebuild:BatchPutCodeCoverages',
],
resources: [codeBuildArn],
}));

// CodeBuild start/stop
assetRole.addToPolicy(new iam.PolicyStatement({
resources: ['*'],
actions: [
'codebuild:BatchGetBuilds',
'codebuild:StartBuild',
'codebuild:StopBuild',
],
}));

// Publishing role access
// The ARNs include raw AWS pseudo parameters (e.g., ${AWS::Partition}), which need to be substituted.
// Lazy-evaluated so all asset publishing roles are included.
Expand All @@ -846,51 +799,8 @@ export class CodePipeline extends PipelineBase {
this.dockerCredentials.forEach(reg => reg.grantRead(assetRole, DockerCredentialUsage.ASSET_PUBLISHING));
}

// 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.
const assetCodeBuildOptions = this.codeBuildDefaultsFor(CodeBuildProjectType.ASSETS);
if (assetCodeBuildOptions?.vpc) {
const vpcPolicy = 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': assetCodeBuildOptions.vpc
.selectSubnets(assetCodeBuildOptions.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',
],
}),
],
});
assetRole.attachInlinePolicy(vpcPolicy);
this.assetAttachedPolicies[assetType] = vpcPolicy;
}

this.assetCodeBuildRoles[assetType] = assetRole.withoutPolicyUpdates();
return {
role: this.assetCodeBuildRoles[assetType],
dependable: this.assetAttachedPolicies[assetType],
};
this.assetCodeBuildRoles[assetType] = assetRole;
return assetRole;
}
}

Expand All @@ -903,11 +813,6 @@ function dockerUsageFromCodeBuild(cbt: CodeBuildProjectType): DockerCredentialUs
}
}

interface AssetCodeBuildRole {
readonly role: iam.IRole;
readonly dependable?: IDependable;
}

enum CodeBuildProjectType {
SYNTH = 'SYNTH',
ASSETS = 'ASSETS',
Expand Down
Loading