Skip to content

Commit 0416e68

Browse files
committed
fix(codepipeline): large cross-region CodePipeline exceed IAM policy size limit
When we generate CodePipelines, we need to add an `sts:AssumeRole` statement for each Action in the pipeline, and a `Bucket.grantReadWrite()` statement for each region the pipeline is in, to the policy statement of the pipeline's Role. For pipelines with many Actions and/or regions, this makes the policy exceed IAM limit of 10240 bytes. Extract a new class from the CodePipeline CloudFormation Actions that caches the statements added to a given Principal by the 'Action' field, and groups the statements with the same 'Actions' by adding elements to the 'Resource' field. This dramatically reduces the duplication in the statement, and increases the chances of it being smaller than the limit. Use this new class in the `Pipeline` construct. Fixes #16244
1 parent dbfebb4 commit 0416e68

File tree

36 files changed

+744
-847
lines changed

36 files changed

+744
-847
lines changed

packages/@aws-cdk/app-delivery/test/integ.cicd.expected.json

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -63,22 +63,20 @@
6363
{
6464
"Action": "sts:AssumeRole",
6565
"Effect": "Allow",
66-
"Resource": {
67-
"Fn::GetAtt": [
68-
"CodePipelineDeployExecuteCodePipelineActionRoleAE36AF49",
69-
"Arn"
70-
]
71-
}
72-
},
73-
{
74-
"Action": "sts:AssumeRole",
75-
"Effect": "Allow",
76-
"Resource": {
77-
"Fn::GetAtt": [
78-
"CodePipelineDeployChangeSetCodePipelineActionRoleB3BCDD8A",
79-
"Arn"
80-
]
81-
}
66+
"Resource": [
67+
{
68+
"Fn::GetAtt": [
69+
"CodePipelineDeployExecuteCodePipelineActionRoleAE36AF49",
70+
"Arn"
71+
]
72+
},
73+
{
74+
"Fn::GetAtt": [
75+
"CodePipelineDeployChangeSetCodePipelineActionRoleB3BCDD8A",
76+
"Arn"
77+
]
78+
}
79+
]
8280
}
8381
],
8482
"Version": "2012-10-17"

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

Lines changed: 27 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ export class CloudFormationDeleteStackAction extends CloudFormationDeployAction
522522
* Statements created outside of this class are not considered when adding new
523523
* permissions.
524524
*/
525-
class SingletonPolicy extends Construct implements iam.IGrantable {
525+
class SingletonPolicy extends iam.GroupingByActionsPrincipal {
526526
/**
527527
* Obtain a SingletonPolicy for a given role.
528528
* @param role the Role this policy is bound to.
@@ -535,97 +535,69 @@ class SingletonPolicy extends Construct implements iam.IGrantable {
535535

536536
private static readonly UUID = '8389e75f-0810-4838-bf64-d6f85a95cf83';
537537

538-
public readonly grantPrincipal: iam.IPrincipal;
539-
540-
private statements: { [key: string]: iam.PolicyStatement } = {};
541-
542-
private constructor(private readonly role: iam.IRole) {
543-
super(role as unknown as cdk.Construct, SingletonPolicy.UUID);
544-
this.grantPrincipal = role;
538+
private constructor(role: iam.IRole) {
539+
super(role, SingletonPolicy.UUID);
545540
}
546541

547542
public grantExecuteChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void {
548-
this.statementFor({
543+
this.addToPrincipalPolicy(new iam.PolicyStatement({
549544
actions: [
550-
'cloudformation:DescribeStacks',
551545
'cloudformation:DescribeChangeSet',
546+
'cloudformation:DescribeStacks',
552547
'cloudformation:ExecuteChangeSet',
553548
],
554-
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
555-
}).addResources(this.stackArnFromProps(props));
549+
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
550+
resources: [this.stackArnFromProps(props)],
551+
}));
556552
}
557553

558554
public grantCreateReplaceChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void {
559-
this.statementFor({
555+
this.addToPrincipalPolicy(new iam.PolicyStatement({
560556
actions: [
561557
'cloudformation:CreateChangeSet',
562558
'cloudformation:DeleteChangeSet',
563559
'cloudformation:DescribeChangeSet',
564560
'cloudformation:DescribeStacks',
565561
],
566562
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
567-
}).addResources(this.stackArnFromProps(props));
563+
resources: [this.stackArnFromProps(props)],
564+
}));
568565
}
569566

570567
public grantCreateUpdateStack(props: { stackName: string, replaceOnFailure?: boolean, region?: string }): void {
571568
const actions = [
572-
'cloudformation:DescribeStack*',
573569
'cloudformation:CreateStack',
574-
'cloudformation:UpdateStack',
575-
'cloudformation:GetTemplate*',
576-
'cloudformation:ValidateTemplate',
570+
'cloudformation:DescribeStack*',
577571
'cloudformation:GetStackPolicy',
572+
'cloudformation:GetTemplate*',
578573
'cloudformation:SetStackPolicy',
574+
'cloudformation:UpdateStack',
575+
'cloudformation:ValidateTemplate',
579576
];
580577
if (props.replaceOnFailure) {
581578
actions.push('cloudformation:DeleteStack');
582579
}
583-
this.statementFor({ actions }).addResources(this.stackArnFromProps(props));
580+
this.addToPrincipalPolicy(new iam.PolicyStatement({
581+
actions,
582+
resources: [this.stackArnFromProps(props)],
583+
}));
584584
}
585585

586586
public grantDeleteStack(props: { stackName: string, region?: string }): void {
587-
this.statementFor({
587+
this.addToPrincipalPolicy(new iam.PolicyStatement({
588588
actions: [
589-
'cloudformation:DescribeStack*',
590589
'cloudformation:DeleteStack',
590+
'cloudformation:DescribeStack*',
591591
],
592-
}).addResources(this.stackArnFromProps(props));
592+
resources: [this.stackArnFromProps(props)],
593+
}));
593594
}
594595

595596
public grantPassRole(role: iam.IRole): void {
596-
this.statementFor({ actions: ['iam:PassRole'] }).addResources(role.roleArn);
597-
}
598-
599-
private statementFor(template: StatementTemplate): iam.PolicyStatement {
600-
const key = keyFor(template);
601-
if (!(key in this.statements)) {
602-
this.statements[key] = new iam.PolicyStatement({ actions: template.actions });
603-
if (template.conditions) {
604-
this.statements[key].addConditions(template.conditions);
605-
}
606-
this.role.addToPolicy(this.statements[key]);
607-
}
608-
return this.statements[key];
609-
610-
function keyFor(props: StatementTemplate): string {
611-
const actions = `${props.actions.sort().join('\x1F')}`;
612-
const conditions = formatConditions(props.conditions);
613-
return `${actions}\x1D${conditions}`;
614-
615-
function formatConditions(cond?: StatementCondition): string {
616-
if (cond == null) { return ''; }
617-
let result = '';
618-
for (const op of Object.keys(cond).sort()) {
619-
result += `${op}\x1E`;
620-
const condition = cond[op];
621-
for (const attribute of Object.keys(condition).sort()) {
622-
const value = condition[attribute];
623-
result += `${value}\x1F`;
624-
}
625-
}
626-
return result;
627-
}
628-
}
597+
this.addToPrincipalPolicy(new iam.PolicyStatement({
598+
actions: ['iam:PassRole'],
599+
resources: [role.roleArn],
600+
}));
629601
}
630602

631603
private stackArnFromProps(props: { stackName: string, region?: string }): string {
@@ -638,13 +610,6 @@ class SingletonPolicy extends Construct implements iam.IGrantable {
638610
}
639611
}
640612

641-
interface StatementTemplate {
642-
actions: string[];
643-
conditions?: StatementCondition;
644-
}
645-
646-
type StatementCondition = { [op: string]: { [attribute: string]: string } };
647-
648613
function parseCapabilities(capabilities: cdk.CfnCapabilities[] | undefined): string | undefined {
649614
if (capabilities === undefined) {
650615
return undefined;

packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/pipeline-actions.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,15 @@ class RoleDouble extends iam.Role {
442442
}
443443

444444
public addToPolicy(statement: iam.PolicyStatement): boolean {
445-
super.addToPolicy(statement);
446-
this.statements.push(statement);
445+
this.addToPrincipalPolicy(statement);
447446
return true;
448447
}
448+
449+
public addToPrincipalPolicy(statement: iam.PolicyStatement): iam.AddToPrincipalPolicyResult {
450+
const ret = super.addToPrincipalPolicy(statement);
451+
this.statements.push(statement);
452+
return ret;
453+
}
449454
}
450455

451456
class BucketDouble extends s3.Bucket {

packages/@aws-cdk/aws-codepipeline-actions/test/integ.cfn-template-from-repo.lit.expected.json

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -157,42 +157,32 @@
157157
{
158158
"Action": "sts:AssumeRole",
159159
"Effect": "Allow",
160-
"Resource": {
161-
"Fn::GetAtt": [
162-
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
163-
"Arn"
164-
]
165-
}
166-
},
167-
{
168-
"Action": "sts:AssumeRole",
169-
"Effect": "Allow",
170-
"Resource": {
171-
"Fn::GetAtt": [
172-
"PipelineDeployPrepareChangesCodePipelineActionRole41931444",
173-
"Arn"
174-
]
175-
}
176-
},
177-
{
178-
"Action": "sts:AssumeRole",
179-
"Effect": "Allow",
180-
"Resource": {
181-
"Fn::GetAtt": [
182-
"PipelineDeployApproveChangesCodePipelineActionRole5AA6E21B",
183-
"Arn"
184-
]
185-
}
186-
},
187-
{
188-
"Action": "sts:AssumeRole",
189-
"Effect": "Allow",
190-
"Resource": {
191-
"Fn::GetAtt": [
192-
"PipelineDeployExecuteChangesCodePipelineActionRole6AA2756F",
193-
"Arn"
194-
]
195-
}
160+
"Resource": [
161+
{
162+
"Fn::GetAtt": [
163+
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
164+
"Arn"
165+
]
166+
},
167+
{
168+
"Fn::GetAtt": [
169+
"PipelineDeployPrepareChangesCodePipelineActionRole41931444",
170+
"Arn"
171+
]
172+
},
173+
{
174+
"Fn::GetAtt": [
175+
"PipelineDeployApproveChangesCodePipelineActionRole5AA6E21B",
176+
"Arn"
177+
]
178+
},
179+
{
180+
"Fn::GetAtt": [
181+
"PipelineDeployExecuteChangesCodePipelineActionRole6AA2756F",
182+
"Arn"
183+
]
184+
}
185+
]
196186
}
197187
],
198188
"Version": "2012-10-17"

packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json

Lines changed: 32 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -151,52 +151,38 @@
151151
{
152152
"Action": "sts:AssumeRole",
153153
"Effect": "Allow",
154-
"Resource": {
155-
"Fn::GetAtt": [
156-
"PipelineSourceCdkCodeSourceCodePipelineActionRole237947B8",
157-
"Arn"
158-
]
159-
}
160-
},
161-
{
162-
"Action": "sts:AssumeRole",
163-
"Effect": "Allow",
164-
"Resource": {
165-
"Fn::GetAtt": [
166-
"PipelineSourceLambdaCodeSourceCodePipelineActionRole4E89EF60",
167-
"Arn"
168-
]
169-
}
170-
},
171-
{
172-
"Action": "sts:AssumeRole",
173-
"Effect": "Allow",
174-
"Resource": {
175-
"Fn::GetAtt": [
176-
"PipelineBuildCDKBuildCodePipelineActionRole15F4B424",
177-
"Arn"
178-
]
179-
}
180-
},
181-
{
182-
"Action": "sts:AssumeRole",
183-
"Effect": "Allow",
184-
"Resource": {
185-
"Fn::GetAtt": [
186-
"PipelineBuildLambdaBuildCodePipelineActionRole2DAE39E9",
187-
"Arn"
188-
]
189-
}
190-
},
191-
{
192-
"Action": "sts:AssumeRole",
193-
"Effect": "Allow",
194-
"Resource": {
195-
"Fn::GetAtt": [
196-
"PipelineDeployLambdaCFNDeployCodePipelineActionRoleF8A74488",
197-
"Arn"
198-
]
199-
}
154+
"Resource": [
155+
{
156+
"Fn::GetAtt": [
157+
"PipelineSourceCdkCodeSourceCodePipelineActionRole237947B8",
158+
"Arn"
159+
]
160+
},
161+
{
162+
"Fn::GetAtt": [
163+
"PipelineSourceLambdaCodeSourceCodePipelineActionRole4E89EF60",
164+
"Arn"
165+
]
166+
},
167+
{
168+
"Fn::GetAtt": [
169+
"PipelineBuildCDKBuildCodePipelineActionRole15F4B424",
170+
"Arn"
171+
]
172+
},
173+
{
174+
"Fn::GetAtt": [
175+
"PipelineBuildLambdaBuildCodePipelineActionRole2DAE39E9",
176+
"Arn"
177+
]
178+
},
179+
{
180+
"Fn::GetAtt": [
181+
"PipelineDeployLambdaCFNDeployCodePipelineActionRoleF8A74488",
182+
"Arn"
183+
]
184+
}
185+
]
200186
}
201187
],
202188
"Version": "2012-10-17"

packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -151,22 +151,20 @@
151151
{
152152
"Action": "sts:AssumeRole",
153153
"Effect": "Allow",
154-
"Resource": {
155-
"Fn::GetAtt": [
156-
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
157-
"Arn"
158-
]
159-
}
160-
},
161-
{
162-
"Action": "sts:AssumeRole",
163-
"Effect": "Allow",
164-
"Resource": {
165-
"Fn::GetAtt": [
166-
"PipelineLambdaCodePipelineActionRoleC6032822",
167-
"Arn"
168-
]
169-
}
154+
"Resource": [
155+
{
156+
"Fn::GetAtt": [
157+
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
158+
"Arn"
159+
]
160+
},
161+
{
162+
"Fn::GetAtt": [
163+
"PipelineLambdaCodePipelineActionRoleC6032822",
164+
"Arn"
165+
]
166+
}
167+
]
170168
}
171169
],
172170
"Version": "2012-10-17"

0 commit comments

Comments
 (0)