Skip to content

Commit

Permalink
fix(codepipeline): large cross-region CodePipeline exceed IAM policy …
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@skinny85
skinny85 committed Sep 2, 2021
1 parent dbfebb4 commit f6b47a5
Show file tree
Hide file tree
Showing 37 changed files with 768 additions and 873 deletions.
30 changes: 14 additions & 16 deletions packages/@aws-cdk/app-delivery/test/integ.cicd.expected.json
View file
Expand Up @@ -63,22 +63,20 @@
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"CodePipelineDeployExecuteCodePipelineActionRoleAE36AF49",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"CodePipelineDeployChangeSetCodePipelineActionRoleB3BCDD8A",
"Arn"
]
}
"Resource": [
{
"Fn::GetAtt": [
"CodePipelineDeployExecuteCodePipelineActionRoleAE36AF49",
"Arn"
]
},
{
"Fn::GetAtt": [
"CodePipelineDeployChangeSetCodePipelineActionRoleB3BCDD8A",
"Arn"
]
}
]
}
],
"Version": "2012-10-17"
Expand Down
89 changes: 27 additions & 62 deletions packages/@aws-cdk/aws-codepipeline-actions/lib/cloudformation/pipeline-actions.ts
View file
Expand Up @@ -522,7 +522,7 @@ export class CloudFormationDeleteStackAction extends CloudFormationDeployAction
* Statements created outside of this class are not considered when adding new
* permissions.
*/
class SingletonPolicy extends Construct implements iam.IGrantable {
class SingletonPolicy extends iam.GroupingByActionsPrincipal {
/**
* Obtain a SingletonPolicy for a given role.
* @param role the Role this policy is bound to.
Expand All @@ -535,97 +535,69 @@ class SingletonPolicy extends Construct implements iam.IGrantable {

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

public readonly grantPrincipal: iam.IPrincipal;

private statements: { [key: string]: iam.PolicyStatement } = {};

private constructor(private readonly role: iam.IRole) {
super(role as unknown as cdk.Construct, SingletonPolicy.UUID);
this.grantPrincipal = role;
private constructor(role: iam.IRole) {
super(role, SingletonPolicy.UUID);
}

public grantExecuteChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void {
this.statementFor({
this.addToPrincipalPolicy(new iam.PolicyStatement({
actions: [
'cloudformation:DescribeStacks',
'cloudformation:DescribeChangeSet',
'cloudformation:DescribeStacks',
'cloudformation:ExecuteChangeSet',
],
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
}).addResources(this.stackArnFromProps(props));
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
resources: [this.stackArnFromProps(props)],
}));
}

public grantCreateReplaceChangeSet(props: { stackName: string, changeSetName: string, region?: string }): void {
this.statementFor({
this.addToPrincipalPolicy(new iam.PolicyStatement({
actions: [
'cloudformation:CreateChangeSet',
'cloudformation:DeleteChangeSet',
'cloudformation:DescribeChangeSet',
'cloudformation:DescribeStacks',
],
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
}).addResources(this.stackArnFromProps(props));
resources: [this.stackArnFromProps(props)],
}));
}

public grantCreateUpdateStack(props: { stackName: string, replaceOnFailure?: boolean, region?: string }): void {
const actions = [
'cloudformation:DescribeStack*',
'cloudformation:CreateStack',
'cloudformation:UpdateStack',
'cloudformation:GetTemplate*',
'cloudformation:ValidateTemplate',
'cloudformation:DescribeStack*',
'cloudformation:GetStackPolicy',
'cloudformation:GetTemplate*',
'cloudformation:SetStackPolicy',
'cloudformation:UpdateStack',
'cloudformation:ValidateTemplate',
];
if (props.replaceOnFailure) {
actions.push('cloudformation:DeleteStack');
}
this.statementFor({ actions }).addResources(this.stackArnFromProps(props));
this.addToPrincipalPolicy(new iam.PolicyStatement({
actions,
resources: [this.stackArnFromProps(props)],
}));
}

public grantDeleteStack(props: { stackName: string, region?: string }): void {
this.statementFor({
this.addToPrincipalPolicy(new iam.PolicyStatement({
actions: [
'cloudformation:DescribeStack*',
'cloudformation:DeleteStack',
'cloudformation:DescribeStack*',
],
}).addResources(this.stackArnFromProps(props));
resources: [this.stackArnFromProps(props)],
}));
}

public grantPassRole(role: iam.IRole): void {
this.statementFor({ actions: ['iam:PassRole'] }).addResources(role.roleArn);
}

private statementFor(template: StatementTemplate): iam.PolicyStatement {
const key = keyFor(template);
if (!(key in this.statements)) {
this.statements[key] = new iam.PolicyStatement({ actions: template.actions });
if (template.conditions) {
this.statements[key].addConditions(template.conditions);
}
this.role.addToPolicy(this.statements[key]);
}
return this.statements[key];

function keyFor(props: StatementTemplate): string {
const actions = `${props.actions.sort().join('\x1F')}`;
const conditions = formatConditions(props.conditions);
return `${actions}\x1D${conditions}`;

function formatConditions(cond?: StatementCondition): string {
if (cond == null) { return ''; }
let result = '';
for (const op of Object.keys(cond).sort()) {
result += `${op}\x1E`;
const condition = cond[op];
for (const attribute of Object.keys(condition).sort()) {
const value = condition[attribute];
result += `${value}\x1F`;
}
}
return result;
}
}
this.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['iam:PassRole'],
resources: [role.roleArn],
}));
}

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

interface StatementTemplate {
actions: string[];
conditions?: StatementCondition;
}

type StatementCondition = { [op: string]: { [attribute: string]: string } };

function parseCapabilities(capabilities: cdk.CfnCapabilities[] | undefined): string | undefined {
if (capabilities === undefined) {
return undefined;
Expand Down
9 changes: 7 additions & 2 deletions packages/@aws-cdk/aws-codepipeline-actions/test/cloudformation/pipeline-actions.test.ts
View file
Expand Up @@ -442,10 +442,15 @@ class RoleDouble extends iam.Role {
}

public addToPolicy(statement: iam.PolicyStatement): boolean {
super.addToPolicy(statement);
this.statements.push(statement);
this.addToPrincipalPolicy(statement);
return true;
}

public addToPrincipalPolicy(statement: iam.PolicyStatement): iam.AddToPrincipalPolicyResult {
const ret = super.addToPrincipalPolicy(statement);
this.statements.push(statement);
return ret;
}
}

class BucketDouble extends s3.Bucket {
Expand Down
62 changes: 26 additions & 36 deletions ...ges/@aws-cdk/aws-codepipeline-actions/test/integ.cfn-template-from-repo.lit.expected.json
View file
Expand Up @@ -157,42 +157,32 @@
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineDeployPrepareChangesCodePipelineActionRole41931444",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineDeployApproveChangesCodePipelineActionRole5AA6E21B",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineDeployExecuteChangesCodePipelineActionRole6AA2756F",
"Arn"
]
}
"Resource": [
{
"Fn::GetAtt": [
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineDeployPrepareChangesCodePipelineActionRole41931444",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineDeployApproveChangesCodePipelineActionRole5AA6E21B",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineDeployExecuteChangesCodePipelineActionRole6AA2756F",
"Arn"
]
}
]
}
],
"Version": "2012-10-17"
Expand Down
78 changes: 32 additions & 46 deletions ...ws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json
View file
Expand Up @@ -151,52 +151,38 @@
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineSourceCdkCodeSourceCodePipelineActionRole237947B8",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineSourceLambdaCodeSourceCodePipelineActionRole4E89EF60",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineBuildCDKBuildCodePipelineActionRole15F4B424",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineBuildLambdaBuildCodePipelineActionRole2DAE39E9",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineDeployLambdaCFNDeployCodePipelineActionRoleF8A74488",
"Arn"
]
}
"Resource": [
{
"Fn::GetAtt": [
"PipelineSourceCdkCodeSourceCodePipelineActionRole237947B8",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineSourceLambdaCodeSourceCodePipelineActionRole4E89EF60",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineBuildCDKBuildCodePipelineActionRole15F4B424",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineBuildLambdaBuildCodePipelineActionRole2DAE39E9",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineDeployLambdaCFNDeployCodePipelineActionRoleF8A74488",
"Arn"
]
}
]
}
],
"Version": "2012-10-17"
Expand Down
30 changes: 14 additions & 16 deletions packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json
View file
Expand Up @@ -151,22 +151,20 @@
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
"Arn"
]
}
},
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Resource": {
"Fn::GetAtt": [
"PipelineLambdaCodePipelineActionRoleC6032822",
"Arn"
]
}
"Resource": [
{
"Fn::GetAtt": [
"PipelineSourceCodePipelineActionRoleC6F9E7F5",
"Arn"
]
},
{
"Fn::GetAtt": [
"PipelineLambdaCodePipelineActionRoleC6032822",
"Arn"
]
}
]
}
],
"Version": "2012-10-17"
Expand Down

0 comments on commit f6b47a5

Please sign in to comment.