Skip to content

Commit f334060

Browse files
authored
fix(pipelines): too many CodeBuild steps inflate policy size (#20396)
(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. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent de027e2 commit f334060

File tree

106 files changed

+1797
-2672
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

106 files changed

+1797
-2672
lines changed

packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts

+7-11
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ export class Pipeline extends PipelineBase {
365365
private readonly crossAccountKeys: boolean;
366366
private readonly enableKeyRotation?: boolean;
367367
private readonly reuseCrossRegionSupportStacks: boolean;
368+
private readonly codePipeline: CfnPipeline;
368369

369370
constructor(scope: Construct, id: string, props: PipelineProps = {}) {
370371
super(scope, id, {
@@ -426,7 +427,7 @@ export class Pipeline extends PipelineBase {
426427
assumedBy: new iam.ServicePrincipal('codepipeline.amazonaws.com'),
427428
});
428429

429-
const codePipeline = new CfnPipeline(this, 'Resource', {
430+
this.codePipeline = new CfnPipeline(this, 'Resource', {
430431
artifactStore: Lazy.any({ produce: () => this.renderArtifactStoreProperty() }),
431432
artifactStores: Lazy.any({ produce: () => this.renderArtifactStoresProperty() }),
432433
stages: Lazy.any({ produce: () => this.renderStages() }),
@@ -437,11 +438,11 @@ export class Pipeline extends PipelineBase {
437438
});
438439

439440
// this will produce a DependsOn for both the role and the policy resources.
440-
codePipeline.node.addDependency(this.role);
441+
this.codePipeline.node.addDependency(this.role);
441442

442443
this.artifactBucket.grantReadWrite(this.role);
443-
this.pipelineName = this.getResourceNameAttribute(codePipeline.ref);
444-
this.pipelineVersion = codePipeline.attrVersion;
444+
this.pipelineName = this.getResourceNameAttribute(this.codePipeline.ref);
445+
this.pipelineVersion = this.codePipeline.attrVersion;
445446
this.crossRegionBucketsPassed = !!props.crossRegionReplicationBuckets;
446447

447448
for (const [region, replicationBucket] of Object.entries(props.crossRegionReplicationBuckets || {})) {
@@ -724,13 +725,8 @@ export class Pipeline extends PipelineBase {
724725
}
725726

726727
// the pipeline role needs assumeRole permissions to the action role
727-
if (actionRole) {
728-
this.role.addToPrincipalPolicy(new iam.PolicyStatement({
729-
actions: ['sts:AssumeRole'],
730-
resources: [actionRole.roleArn],
731-
}));
732-
}
733-
728+
const grant = actionRole?.grantAssumeRole(this.role);
729+
grant?.applyBefore(this.codePipeline);
734730
return actionRole;
735731
}
736732

packages/@aws-cdk/aws-iam/lib/lazy-role.ts

+7
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ export class LazyRole extends cdk.Resource implements IRole {
119119
return this.instantiate().grantPassRole(identity);
120120
}
121121

122+
/**
123+
* Grant permissions to the given principal to assume this role.
124+
*/
125+
public grantAssumeRole(identity: IPrincipal): Grant {
126+
return this.instantiate().grantAssumeRole(identity);
127+
}
128+
122129
private instantiate(): Role {
123130
if (!this.role) {
124131
const role = new Role(this, 'Default', this.props);

packages/@aws-cdk/aws-iam/lib/principals.ts

+15
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,21 @@ export interface ServicePrincipalOpts {
429429
* An IAM principal that represents an AWS service (i.e. sqs.amazonaws.com).
430430
*/
431431
export class ServicePrincipal extends PrincipalBase {
432+
/**
433+
* Translate the given service principal name based on the region it's used in.
434+
*
435+
* For example, for Chinese regions this may (depending on whether that's necessary
436+
* for the given service principal) append `.cn` to the name.
437+
*
438+
* The `region-info` module is used to obtain this information.
439+
*
440+
* @example
441+
* const principalName = iam.ServicePrincipal.servicePrincipalName('ec2.amazonaws.com');
442+
*/
443+
public static servicePrincipalName(service: string): string {
444+
return new ServicePrincipalToken(service, {}).toString();
445+
}
446+
432447
/**
433448
*
434449
* @param service AWS service (i.e. sqs.amazonaws.com)

packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts

+4
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,8 @@ export class ImmutableRole extends Resource implements IRole {
6767
public grantPassRole(grantee: IPrincipal): Grant {
6868
return this.role.grantPassRole(grantee);
6969
}
70+
71+
public grantAssumeRole(identity: IPrincipal): Grant {
72+
return this.role.grantAssumeRole(identity);
73+
}
7074
}

packages/@aws-cdk/aws-iam/lib/role.ts

+20
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,13 @@ export class Role extends Resource implements IRole {
246246
return this.grant(identity, 'iam:PassRole');
247247
}
248248

249+
/**
250+
* Grant permissions to the given principal to pass this role.
251+
*/
252+
public grantAssumeRole(identity: IPrincipal): Grant {
253+
return this.grant(identity, 'sts:AssumeRole');
254+
}
255+
249256
/**
250257
* Grant the actions defined in actions to the identity Principal on this resource.
251258
*/
@@ -447,6 +454,14 @@ export class Role extends Resource implements IRole {
447454
return this.grant(identity, 'iam:PassRole');
448455
}
449456

457+
/**
458+
* Grant permissions to the given principal to assume this role.
459+
*/
460+
public grantAssumeRole(identity: IPrincipal) {
461+
return this.grant(identity, 'sts:AssumeRole');
462+
}
463+
464+
450465
/**
451466
* Return a copy of this Role object whose Policies will not be updated
452467
*
@@ -502,6 +517,11 @@ export interface IRole extends IIdentity {
502517
* Grant permissions to the given principal to pass this role.
503518
*/
504519
grantPassRole(grantee: IPrincipal): Grant;
520+
521+
/**
522+
* Grant permissions to the given principal to assume this role.
523+
*/
524+
grantAssumeRole(grantee: IPrincipal): Grant;
505525
}
506526

507527
function createAssumeRolePolicy(principal: IPrincipal, externalIds: string[]) {

packages/@aws-cdk/aws-iam/test/principals.test.ts

+10
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,16 @@ test('AccountPrincipal can specify an organization', () => {
294294
});
295295
});
296296

297+
test('ServicePrincipalName returns just a string representing the principal', () => {
298+
// GIVEN
299+
const usEastStack = new Stack(undefined, undefined, { env: { region: 'us-east-1' } });
300+
const afSouthStack = new Stack(undefined, undefined, { env: { region: 'af-south-1' } });
301+
const principalName = iam.ServicePrincipal.servicePrincipalName('ssm.amazonaws.com');
302+
303+
expect(usEastStack.resolve(principalName)).toEqual('ssm.amazonaws.com');
304+
expect(afSouthStack.resolve(principalName)).toEqual('ssm.af-south-1.amazonaws.com');
305+
});
306+
297307
test('Passing non-string as accountId parameter in AccountPrincipal constructor should throw error', () => {
298308
expect(() => new iam.AccountPrincipal(1234)).toThrowError('accountId should be of type string');
299309
});

packages/@aws-cdk/aws-iam/test/role.test.ts

+24
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,30 @@ describe('IAM role', () => {
5959
});
6060
});
6161

62+
test('a role can grant AssumeRole permissions', () => {
63+
// GIVEN
64+
const stack = new Stack();
65+
const role = new Role(stack, 'Role', { assumedBy: new ServicePrincipal('henk.amazonaws.com') });
66+
const user = new User(stack, 'User');
67+
68+
// WHEN
69+
role.grantAssumeRole(user);
70+
71+
// THEN
72+
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
73+
PolicyDocument: {
74+
Statement: [
75+
{
76+
Action: 'sts:AssumeRole',
77+
Effect: 'Allow',
78+
Resource: { 'Fn::GetAtt': ['Role1ABCC5F0', 'Arn'] },
79+
},
80+
],
81+
Version: '2012-10-17',
82+
},
83+
});
84+
});
85+
6286
testDeprecated('can supply externalId', () => {
6387
// GIVEN
6488
const stack = new Stack();

packages/@aws-cdk/pipelines/lib/codepipeline/private/codebuild-factory.ts

+13-1
Original file line numberDiff line numberDiff line change
@@ -323,15 +323,27 @@ export class CodeBuildFactory implements ICodePipelineActionFactory {
323323
? { _PROJECT_CONFIG_HASH: projectConfigHash }
324324
: {};
325325

326+
327+
// Start all CodeBuild projects from a single (shared) Action Role, so that we don't have to generate an Action Role for each
328+
// individual CodeBuild Project and blow out the pipeline policy size (and potentially # of resources in the stack).
329+
const actionRoleCid = 'CodeBuildActionRole';
330+
const actionRole = this.props.actionRole
331+
?? options.pipeline.node.tryFindChild(actionRoleCid) as iam.IRole
332+
?? new iam.Role(options.pipeline, actionRoleCid, {
333+
assumedBy: new iam.PrincipalWithConditions(new iam.AccountRootPrincipal(), {
334+
Bool: { 'aws:ViaAWSService': iam.ServicePrincipal.servicePrincipalName('codepipeline.amazonaws.com') },
335+
}),
336+
});
337+
326338
stage.addAction(new codepipeline_actions.CodeBuildAction({
327339
actionName: actionName,
328340
input: inputArtifact,
329341
extraInputs: extraInputArtifacts,
330342
outputs: outputArtifacts,
331343
project,
332344
runOrder: options.runOrder,
333-
role: this.props.actionRole,
334345
variablesNamespace: options.variablesNamespace,
346+
role: actionRole,
335347

336348
// Inclusion of the hash here will lead to the pipeline structure for any changes
337349
// made the config of the underlying CodeBuild Project.

packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-graph.test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
/* eslint-disable import/no-extraneous-dependencies */
2-
import '@aws-cdk/assert-internal/jest';
32
import * as cdkp from '../../../lib';
43
import { ManualApprovalStep, Step } from '../../../lib';
54
import { Graph, GraphNode, PipelineGraph } from '../../../lib/helpers-internal';

packages/@aws-cdk/pipelines/test/blueprint/helpers-internal/pipeline-queries.test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
/* eslint-disable import/no-extraneous-dependencies */
2-
import '@aws-cdk/assert-internal/jest';
32
import * as cdkp from '../../../lib';
43
import { PipelineQueries } from '../../../lib/helpers-internal/pipeline-queries';
54
import { AppWithOutput, TestApp } from '../../testhelpers/test-app';

packages/@aws-cdk/pipelines/test/codepipeline/codebuild-step.test.ts

+5-2
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ test('role passed it used for project and code build action', () => {
172172
});
173173

174174
// THEN
175-
Template.fromStack(pipelineStack).hasResourceProperties('AWS::CodeBuild::Project', {
175+
const tpl = Template.fromStack(pipelineStack);
176+
tpl.hasResourceProperties('AWS::CodeBuild::Project', {
176177
ServiceRole: {
177178
'Fn::GetAtt': [
178179
'ProjectRole5B707505',
@@ -181,7 +182,7 @@ test('role passed it used for project and code build action', () => {
181182
},
182183
});
183184

184-
expect(pipelineStack).toHaveResourceLike('AWS::CodePipeline::Pipeline', {
185+
tpl.hasResourceProperties('AWS::CodePipeline::Pipeline', {
185186
Stages: [
186187
// source stage
187188
{},
@@ -203,6 +204,8 @@ test('role passed it used for project and code build action', () => {
203204
},
204205
],
205206
},
207+
// Self-update
208+
{},
206209
],
207210
});
208211
});

packages/@aws-cdk/pipelines/test/codepipeline/codepipeline.test.ts

+32-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
import { Template } from '@aws-cdk/assertions';
12
import * as ccommit from '@aws-cdk/aws-codecommit';
23
import * as sqs from '@aws-cdk/aws-sqs';
34
import * as cdk from '@aws-cdk/core';
45
import { Construct } from 'constructs';
56
import * as cdkp from '../../lib';
6-
import { PIPELINE_ENV, TestApp } from '../testhelpers';
7+
import { PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline } from '../testhelpers';
78

89
let app: TestApp;
910

@@ -52,24 +53,48 @@ describe('CodePipeline support stack reuse', () => {
5253
const supportStackAArtifact = assembly.getStackByName(`PipelineStackA-support-${testStageEnv.region}`);
5354
const supportStackBArtifact = assembly.getStackByName(`PipelineStackB-support-${testStageEnv.region}`);
5455

55-
const supportStackATemplate = supportStackAArtifact.template;
56-
expect(supportStackATemplate).toHaveResourceLike('AWS::S3::Bucket', {
56+
const supportStackATemplate = Template.fromJSON(supportStackAArtifact.template);
57+
supportStackATemplate.hasResourceProperties('AWS::S3::Bucket', {
5758
BucketName: 'pipelinestacka-support-useplicationbucket80db3753a0ebbf052279',
5859
});
59-
expect(supportStackATemplate).toHaveResourceLike('AWS::KMS::Alias', {
60+
supportStackATemplate.hasResourceProperties('AWS::KMS::Alias', {
6061
AliasName: 'alias/pport-ustencryptionalias5cad45754e1ff088476b',
6162
});
6263

63-
const supportStackBTemplate = supportStackBArtifact.template;
64-
expect(supportStackBTemplate).toHaveResourceLike('AWS::S3::Bucket', {
64+
const supportStackBTemplate = Template.fromJSON(supportStackBArtifact.template);
65+
supportStackBTemplate.hasResourceProperties('AWS::S3::Bucket', {
6566
BucketName: 'pipelinestackb-support-useplicationbucket1d556ec7f959b336abf8',
6667
});
67-
expect(supportStackBTemplate).toHaveResourceLike('AWS::KMS::Alias', {
68+
supportStackBTemplate.hasResourceProperties('AWS::KMS::Alias', {
6869
AliasName: 'alias/pport-ustencryptionalias668c7ffd0de17c9867b0',
6970
});
7071
});
7172
});
7273

74+
test('CodeBuild action role has the right AssumeRolePolicyDocument', () => {
75+
const pipelineStack = new cdk.Stack(app, 'PipelineStack', { env: PIPELINE_ENV });
76+
new ModernTestGitHubNpmPipeline(pipelineStack, 'Cdk');
77+
78+
const template = Template.fromStack(pipelineStack);
79+
template.hasResourceProperties('AWS::IAM::Role', {
80+
AssumeRolePolicyDocument: {
81+
Statement: [
82+
{
83+
Action: 'sts:AssumeRole',
84+
Principal: {
85+
AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::123pipeline:root']] },
86+
},
87+
Condition: {
88+
Bool: {
89+
'aws:ViaAWSService': 'codepipeline.amazonaws.com',
90+
},
91+
},
92+
},
93+
],
94+
},
95+
});
96+
});
97+
7398
interface ReuseCodePipelineStackProps extends cdk.StackProps {
7499
reuseCrossRegionSupportStacks?: boolean;
75100
}

packages/@aws-cdk/pipelines/test/compliance/basic-behavior.test.ts

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import * as fs from 'fs';
33
import * as path from 'path';
44
import { Capture, Match, Template } from '@aws-cdk/assertions';
5-
import '@aws-cdk/assert-internal/jest';
65
import { Stack, Stage, StageProps, Tags } from '@aws-cdk/core';
76
import { Construct } from 'constructs';
87
import { behavior, LegacyTestGitHubNpmPipeline, OneStackApp, BucketStack, PIPELINE_ENV, TestApp, ModernTestGitHubNpmPipeline, stringLike } from '../testhelpers';

packages/@aws-cdk/pipelines/test/newpipeline-with-vpc.integ.snapshot/PipelineStack.assets.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
11
{
2-
"version": "17.0.0",
2+
"version": "19.0.0",
33
"files": {
4-
"9220951fe280727c8695ca9cd06f929712e6c44319ad69591a87f7ce6a931b6b": {
4+
"09ed6a107711fc77b4417fe759eedb1920ea48ea07d68490b9973255f017840d": {
55
"source": {
66
"path": "PipelineStack.template.json",
77
"packaging": "file"
88
},
99
"destinations": {
1010
"current_account-current_region": {
1111
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
12-
"objectKey": "9220951fe280727c8695ca9cd06f929712e6c44319ad69591a87f7ce6a931b6b.json",
12+
"objectKey": "09ed6a107711fc77b4417fe759eedb1920ea48ea07d68490b9973255f017840d.json",
1313
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
1414
}
1515
}

0 commit comments

Comments
 (0)