Skip to content

Commit

Permalink
fix(pipelines): too many CodeBuild steps inflate policy size
Browse files Browse the repository at this point in the history
(This change has been split off from #20189 because that PR was growing
too big)

Collapse CodeBuild action Roles: each CodeBuild step used to create a
fresh Role to run the CodeBuild action. Change to use one Role for all
CodeBuild actions. This saves a lot of resources and policy space when
using many CodeBuild steps, and doesn't appreciably change the
security posture of the Pipeline (note: this is not about the
Execution Role of the CodeBuild projects, this is about the Role
assumed by the Pipeline to initiate execution of the Project).

Relates to #19276, #19939, #19835.
  • Loading branch information
rix0rrr committed May 18, 2022
1 parent 10df757 commit b539164
Show file tree
Hide file tree
Showing 59 changed files with 1,406 additions and 2,283 deletions.
18 changes: 7 additions & 11 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Expand Up @@ -367,6 +367,7 @@ export class Pipeline extends PipelineBase {
private readonly crossAccountKeys: boolean;
private readonly enableKeyRotation?: boolean;
private readonly reuseCrossRegionSupportStacks: boolean;
private readonly codePipeline: CfnPipeline;

constructor(scope: Construct, id: string, props: PipelineProps = {}) {
super(scope, id, {
Expand Down Expand Up @@ -428,7 +429,7 @@ export class Pipeline extends PipelineBase {
assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'),
});

const codePipeline = new CfnPipeline(this, 'Resource', {
this.codePipeline = new CfnPipeline(this, 'Resource', {
artifactStore: Lazy.any({ produce: () => this.renderArtifactStoreProperty() }),
artifactStores: Lazy.any({ produce: () => this.renderArtifactStoresProperty() }),
stages: Lazy.any({ produce: () => this.renderStages() }),
Expand All @@ -439,11 +440,11 @@ export class Pipeline extends PipelineBase {
});

// this will produce a DependsOn for both the role and the policy resources.
codePipeline.node.addDependency(this.role);
this.codePipeline.node.addDependency(this.role);

this.artifactBucket.grantReadWrite(this.role);
this.pipelineName = this.getResourceNameAttribute(codePipeline.ref);
this.pipelineVersion = codePipeline.attrVersion;
this.pipelineName = this.getResourceNameAttribute(this.codePipeline.ref);
this.pipelineVersion = this.codePipeline.attrVersion;
this.crossRegionBucketsPassed = !!props.crossRegionReplicationBuckets;

for (const [region, replicationBucket] of Object.entries(props.crossRegionReplicationBuckets || {})) {
Expand Down Expand Up @@ -735,13 +736,8 @@ export class Pipeline extends PipelineBase {
}

// the pipeline role needs assumeRole permissions to the action role
if (actionRole) {
this.role.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['sts:AssumeRole'],
resources: [actionRole.roleArn],
}));
}

const grant = actionRole?.grantAssumeRole(this.role);
grant?.applyBefore(this.codePipeline);
return actionRole;
}

Expand Down
7 changes: 7 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/lazy-role.ts
Expand Up @@ -119,6 +119,13 @@ export class LazyRole extends cdk.Resource implements IRole {
return this.instantiate().grantPassRole(identity);
}

/**
* Grant permissions to the given principal to assume this role.
*/
public grantAssumeRole(identity: IPrincipal): Grant {
return this.instantiate().grantAssumeRole(identity);
}

private instantiate(): Role {
if (!this.role) {
const role = new Role(this, 'Default', this.props);
Expand Down
20 changes: 20 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Expand Up @@ -246,6 +246,13 @@ export class Role extends Resource implements IRole {
return this.grant(identity, 'iam:PassRole');
}

/**
* Grant permissions to the given principal to pass this role.
*/
public grantAssumeRole(identity: IPrincipal): Grant {
return this.grant(identity, 'sts:AssumeRole');
}

/**
* Grant the actions defined in actions to the identity Principal on this resource.
*/
Expand Down Expand Up @@ -447,6 +454,14 @@ export class Role extends Resource implements IRole {
return this.grant(identity, 'iam:PassRole');
}

/**
* Grant permissions to the given principal to assume this role.
*/
public grantAssumeRole(identity: IPrincipal) {
return this.grant(identity, 'sts:AssumeRole');
}


/**
* Return a copy of this Role object whose Policies will not be updated
*
Expand Down Expand Up @@ -502,6 +517,11 @@ export interface IRole extends IIdentity {
* Grant permissions to the given principal to pass this role.
*/
grantPassRole(grantee: IPrincipal): Grant;

/**
* Grant permissions to the given principal to assume this role.
*/
grantAssumeRole(grantee: IPrincipal): Grant;
}

function createAssumeRolePolicy(principal: IPrincipal, externalIds: string[]) {
Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/aws-iam/test/role.test.ts
Expand Up @@ -59,6 +59,30 @@ describe('IAM role', () => {
});
});

test('a role can grant AssumeRole permissions', () => {
// GIVEN
const stack = new Stack();
const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('henk.amazonaws.com') });
const user = new User(stack, 'User');

// WHEN
role.grantAssumeRole(user);

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Action: 'sts:AssumeRole',
Effect: 'Allow',
Resource: { 'Fn::GetAtt': ['Role1ABCC5F0', 'Arn'] },
},
],
Version: '2012-10-17',
},
});
});

testDeprecated('can supply externalId', () => {
// GIVEN
const stack = new Stack();
Expand Down
Expand Up @@ -315,6 +315,16 @@ export class CodeBuildFactory implements ICodePipelineActionFactory {
? { _PROJECT_CONFIG_HASH: projectConfigHash }
: {};


// Start all CodeBuild projects from a single (shared) Action Role, so that we don't have to generate an Action Role for each
// individual CodeBuild Project and blow out the pipeline policy size (and potentially # of resources in the stack).
const actionRoleCid = 'CodeBuildActionRole';
const actionRole = options.pipeline.node.tryFindChild(actionRoleCid) as iam.IRole ?? new iam.Role(options.pipeline, actionRoleCid, {
assumedBy: new iam.PrincipalWithConditions(new iam.AccountRootPrincipal(), {
Bool: { 'aws:ViaAWSService': iam.ServicePrincipal.servicePrincipalName('codepipeline.amazonaws.com') },
}),
});

stage.addAction(new codepipeline_actions.CodeBuildAction({
actionName: actionName,
input: inputArtifact,
Expand All @@ -323,6 +333,7 @@ export class CodeBuildFactory implements ICodePipelineActionFactory {
project,
runOrder: options.runOrder,
variablesNamespace: options.variablesNamespace,
role: actionRole,

// Inclusion of the hash here will lead to the pipeline structure for any changes
// made the config of the underlying CodeBuild Project.
Expand Down
@@ -1,5 +1,4 @@
/* eslint-disable import/no-extraneous-dependencies */
import '@aws-cdk/assert-internal/jest';
import * as cdkp from '../../../lib';
import { ManualApprovalStep, Step } from '../../../lib';
import { Graph, GraphNode, PipelineGraph } from '../../../lib/helpers-internal';
Expand Down
@@ -1,5 +1,4 @@
/* eslint-disable import/no-extraneous-dependencies */
import '@aws-cdk/assert-internal/jest';
import * as cdkp from '../../../lib';
import { PipelineQueries } from '../../../lib/helpers-internal/pipeline-queries';
import { AppWithOutput, TestApp } from '../../testhelpers/test-app';
Expand Down
37 changes: 31 additions & 6 deletions packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts
@@ -1,3 +1,4 @@
import { Template, Annotations, Match } from '@aws-cdk/assertions';
import * as ccommit from '@aws-cdk/aws-codecommit';
import * as sqs from '@aws-cdk/aws-sqs';
import * as cdk from '@aws-cdk/core';
Expand Down Expand Up @@ -52,24 +53,48 @@ describe('CodePipeline support stack reuse', () => {
const supportStackAArtifact = assembly.getStackByName(`PipelineStackA-support-${testStageEnv.region}`);
const supportStackBArtifact = assembly.getStackByName(`PipelineStackB-support-${testStageEnv.region}`);

const supportStackATemplate = supportStackAArtifact.template;
expect(supportStackATemplate).toHaveResourceLike('AWS::S3::Bucket', {
const supportStackATemplate = Template.fromJSON(supportStackAArtifact.template);
supportStackATemplate.hasResourceProperties('AWS::S3::Bucket', {
BucketName: 'pipelinestacka-support-useplicationbucket80db3753a0ebbf052279',
});
expect(supportStackATemplate).toHaveResourceLike('AWS::KMS::Alias', {
supportStackATemplate.hasResourceProperties('AWS::KMS::Alias', {
AliasName: 'alias/pport-ustencryptionalias5cad45754e1ff088476b',
});

const supportStackBTemplate = supportStackBArtifact.template;
expect(supportStackBTemplate).toHaveResourceLike('AWS::S3::Bucket', {
const supportStackBTemplate = Template.fromJSON(supportStackBArtifact.template);
supportStackBTemplate.hasResourceProperties('AWS::S3::Bucket', {
BucketName: 'pipelinestackb-support-useplicationbucket1d556ec7f959b336abf8',
});
expect(supportStackBTemplate).toHaveResourceLike('AWS::KMS::Alias', {
supportStackBTemplate.hasResourceProperties('AWS::KMS::Alias', {
AliasName: 'alias/pport-ustencryptionalias668c7ffd0de17c9867b0',
});
});
});

test('CodeBuild action role has the right AssumeRolePolicyDocument', () => {
const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV });
new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk');

const template = Template.fromStack(pipelineStack);
template.hasResourceProperties('AWS::IAM::Role', {
AssumeRolePolicyDocument: {
Statement: [
{
Action: 'sts:AssumeRole',
Principal: {
AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::123pipeline:root']] },
},
Condition: {
Bool: {
'aws:ViaAWSService': 'codepipeline.amazonaws.com',
},
},
},
],
},
});
});

interface ReuseCodePipelineStackProps extends cdk.StackProps {
reuseCrossRegionSupportStacks?: boolean;
}
Expand Down
Expand Up @@ -2,7 +2,6 @@
import * as fs from 'fs';
import * as path from 'path';
import { Capture, Match, Template } from '@aws-cdk/assertions';
import '@aws-cdk/assert-internal/jest';
import { Stack, Stage, StageProps, Tags } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { behavior, LegacyTestGitHubNpmPipeline, OneStackApp, BucketStack, PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, stringLike } from '../testhelpers';
Expand Down
@@ -1,15 +1,15 @@
{
"version": "17.0.0",
"version": "18.0.0",
"files": {
"9220951fe280727c8695ca9cd06f929712e6c44319ad69591a87f7ce6a931b6b": {
"bc7ded943a77087531e601616d23ec39dbeeffbdcba901ae9d014dc028f53c2b": {
"source": {
"path": "PipelineStack.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "9220951fe280727c8695ca9cd06f929712e6c44319ad69591a87f7ce6a931b6b.json",
"objectKey": "bc7ded943a77087531e601616d23ec39dbeeffbdcba901ae9d014dc028f53c2b.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
Expand Down

0 comments on commit b539164

Please sign in to comment.