Skip to content

Commit 8f4c2ab

Browse files
RomainMullerRomainMuller
authored
feat(codepipeline/cfn): Use fewer statements for pipeline permissions (#1009)
When trying to make minimal-permission IAM policies, it can be necessary to ensure the policy remains as compact as possible. In certain cases, the same permissions will be extended to multiple resources separately, and those can be represented using a single statement, instead of one per each resource. This feature uses a role-local singleton construct to ensure only one statement is created for a given permission template, so as to minimize the size of the resulting policy. The feature is being used in order to avoid creating extremely large policy documents when adding CodePipeline actions to deploy a number of CloudFormation stacks using the same ChangeSet name (using a single statement instead of one per stack).
1 parent 67f7fa1 commit 8f4c2ab

10 files changed

+256
-109
lines changed

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

Lines changed: 122 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,8 @@ export class PipelineExecuteChangeSetAction extends PipelineCloudFormationAction
9292
ChangeSetName: props.changeSetName,
9393
});
9494

95-
props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
96-
.addAction('cloudformation:ExecuteChangeSet')
97-
.addResource(stackArnFromName(props.stackName))
98-
.addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName }));
95+
SingletonPolicy.forRole(props.stage.pipelineRole)
96+
.grantExecuteChangeSet(props);
9997
}
10098
}
10199

@@ -212,11 +210,7 @@ export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFo
212210
}
213211
}
214212

215-
// Allow the pipeline to pass this actions' role to CloudFormation
216-
// Required by all Actions that perform CFN deployments
217-
props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
218-
.addAction('iam:PassRole')
219-
.addResource(this.role.roleArn));
213+
SingletonPolicy.forRole(props.stage.pipelineRole).grantPassRole(this.role);
220214
}
221215

222216
/**
@@ -261,16 +255,7 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation
261255
this.addInputArtifact(props.templateConfiguration.artifact);
262256
}
263257

264-
const stackArn = stackArnFromName(props.stackName);
265-
// Allow the pipeline to check for Stack & ChangeSet existence
266-
props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
267-
.addAction('cloudformation:DescribeStacks')
268-
.addResource(stackArn));
269-
// Allow the pipeline to create & delete the specified ChangeSet
270-
props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
271-
.addActions('cloudformation:CreateChangeSet', 'cloudformation:DeleteChangeSet', 'cloudformation:DescribeChangeSet')
272-
.addResource(stackArn)
273-
.addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName }));
258+
SingletonPolicy.forRole(props.stage.pipelineRole).grantCreateReplaceChangeSet(props);
274259
}
275260
}
276261

@@ -325,22 +310,7 @@ export class PipelineCreateUpdateStackAction extends PipelineCloudFormationDeplo
325310
this.addInputArtifact(props.templateConfiguration.artifact);
326311
}
327312

328-
// permissions are based on best-guess from
329-
// https://docs.aws.amazon.com/codepipeline/latest/userguide/how-to-custom-role.html
330-
// and https://docs.aws.amazon.com/IAM/latest/UserGuide/list_awscloudformation.html
331-
const stackArn = stackArnFromName(props.stackName);
332-
props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
333-
.addActions(
334-
'cloudformation:DescribeStack*',
335-
'cloudformation:CreateStack',
336-
'cloudformation:UpdateStack',
337-
'cloudformation:DeleteStack', // needed when props.replaceOnFailure is true
338-
'cloudformation:GetTemplate*',
339-
'cloudformation:ValidateTemplate',
340-
'cloudformation:GetStackPolicy',
341-
'cloudformation:SetStackPolicy',
342-
)
343-
.addResource(stackArn));
313+
SingletonPolicy.forRole(props.stage.pipelineRole).grantCreateUpdateStack(props);
344314
}
345315
}
346316

@@ -362,13 +332,7 @@ export class PipelineDeleteStackAction extends PipelineCloudFormationDeployActio
362332
super(parent, id, props, {
363333
ActionMode: 'DELETE_ONLY',
364334
});
365-
const stackArn = stackArnFromName(props.stackName);
366-
props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
367-
.addActions(
368-
'cloudformation:DescribeStack*',
369-
'cloudformation:DeleteStack',
370-
)
371-
.addResource(stackArn));
335+
SingletonPolicy.forRole(props.stage.pipelineRole).grantDeleteStack(props);
372336
}
373337
}
374338

@@ -401,3 +365,119 @@ function stackArnFromName(stackName: string): string {
401365
resourceName: `${stackName}/*`
402366
});
403367
}
368+
369+
/**
370+
* Manages a bunch of singleton-y statements on the policy of an IAM Role.
371+
* Dedicated methods can be used to add specific permissions to the role policy
372+
* using as few statements as possible (adding resources to existing compatible
373+
* statements instead of adding new statements whenever possible).
374+
*
375+
* Statements created outside of this class are not considered when adding new
376+
* permissions.
377+
*/
378+
class SingletonPolicy extends cdk.Construct {
379+
/**
380+
* Obtain a SingletonPolicy for a given role.
381+
* @param role the Role this policy is bound to.
382+
* @returns the SingletonPolicy for this role.
383+
*/
384+
public static forRole(role: iam.Role): SingletonPolicy {
385+
const found = role.tryFindChild(SingletonPolicy.UUID);
386+
return (found as SingletonPolicy) || new SingletonPolicy(role);
387+
}
388+
389+
private static readonly UUID = '8389e75f-0810-4838-bf64-d6f85a95cf83';
390+
391+
private statements: { [key: string]: iam.PolicyStatement } = {};
392+
393+
private constructor(private readonly role: iam.Role) {
394+
super(role, SingletonPolicy.UUID);
395+
}
396+
397+
public grantCreateUpdateStack(props: { stackName: string, replaceOnFailure?: boolean }): void {
398+
const actions = [
399+
'cloudformation:DescribeStack*',
400+
'cloudformation:CreateStack',
401+
'cloudformation:UpdateStack',
402+
'cloudformation:GetTemplate*',
403+
'cloudformation:ValidateTemplate',
404+
'cloudformation:GetStackPolicy',
405+
'cloudformation:SetStackPolicy',
406+
];
407+
if (props.replaceOnFailure) {
408+
actions.push('cloudformation:DeleteStack');
409+
}
410+
this.statementFor({ actions }).addResource(stackArnFromName(props.stackName));
411+
}
412+
413+
public grantCreateReplaceChangeSet(props: { stackName: string, changeSetName: string }): void {
414+
this.statementFor({
415+
actions: [
416+
'cloudformation:CreateChangeSet',
417+
'cloudformation:DeleteChangeSet',
418+
'cloudformation:DescribeChangeSet',
419+
'cloudformation:DescribeStacks',
420+
],
421+
conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } },
422+
}).addResource(stackArnFromName(props.stackName));
423+
}
424+
425+
public grantExecuteChangeSet(props: { stackName: string, changeSetName: string }): void {
426+
this.statementFor({
427+
actions: ['cloudformation:ExecuteChangeSet'],
428+
conditions: { StringEquals: { 'cloudformation:ChangeSetName': props.changeSetName } },
429+
}).addResource(stackArnFromName(props.stackName));
430+
}
431+
432+
public grantDeleteStack(props: { stackName: string }): void {
433+
this.statementFor({
434+
actions: [
435+
'cloudformation:DescribeStack*',
436+
'cloudformation:DeleteStack',
437+
]
438+
}).addResource(stackArnFromName(props.stackName));
439+
}
440+
441+
public grantPassRole(role: iam.Role): void {
442+
this.statementFor({ actions: ['iam:PassRole'] }).addResource(role.roleArn);
443+
}
444+
445+
private statementFor(template: StatementTemplate): iam.PolicyStatement {
446+
const key = keyFor(template);
447+
if (!(key in this.statements)) {
448+
this.statements[key] = new iam.PolicyStatement().addActions(...template.actions);
449+
if (template.conditions) {
450+
this.statements[key].addConditions(template.conditions);
451+
}
452+
this.role.addToPolicy(this.statements[key]);
453+
}
454+
return this.statements[key];
455+
456+
function keyFor(props: StatementTemplate): string {
457+
const actions = `${props.actions.sort().join('\x1F')}`;
458+
const conditions = formatConditions(props.conditions);
459+
return `${actions}\x1D${conditions}`;
460+
461+
function formatConditions(cond?: StatementCondition): string {
462+
if (cond == null) { return ''; }
463+
let result = '';
464+
for (const op of Object.keys(cond).sort()) {
465+
result += `${op}\x1E`;
466+
const condition = cond[op];
467+
for (const attribute of Object.keys(condition).sort()) {
468+
const value = condition[attribute];
469+
result += `${value}\x1F`;
470+
}
471+
}
472+
return result;
473+
}
474+
}
475+
}
476+
}
477+
478+
interface StatementTemplate {
479+
actions: string[];
480+
conditions?: StatementCondition;
481+
}
482+
483+
type StatementCondition = { [op: string]: { [attribute: string]: string } };

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

Lines changed: 100 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import cloudformation = require('../lib');
77

88
export = nodeunit.testCase({
99
'CreateReplaceChangeSet': {
10-
works(test: nodeunit.Test) {
10+
'works'(test: nodeunit.Test) {
1111
const stack = new cdk.Stack();
1212
const pipelineRole = new RoleDouble(stack, 'PipelineRole');
1313
const stage = new StageDouble({ pipelineRole });
@@ -22,8 +22,8 @@ export = nodeunit.testCase({
2222
_assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
2323

2424
const stackArn = _stackArn('MyStack');
25-
const changeSetCondition = { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } };
26-
_assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn);
25+
const changeSetCondition = { StringEqualsIfExists: { 'cloudformation:ChangeSetName': 'MyChangeSet' } };
26+
_assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn, changeSetCondition);
2727
_assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeChangeSet', stackArn, changeSetCondition);
2828
_assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:CreateChangeSet', stackArn, changeSetCondition);
2929
_assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteChangeSet', stackArn, changeSetCondition);
@@ -37,11 +37,64 @@ export = nodeunit.testCase({
3737
ChangeSetName: 'MyChangeSet'
3838
});
3939

40+
test.done();
41+
},
42+
43+
'uses a single permission statement if the same ChangeSet name is used'(test: nodeunit.Test) {
44+
const stack = new cdk.Stack();
45+
const pipelineRole = new RoleDouble(stack, 'PipelineRole');
46+
const stage = new StageDouble({ pipelineRole });
47+
const artifact = new cpapi.Artifact(stack as any, 'TestArtifact');
48+
new cloudformation.PipelineCreateReplaceChangeSetAction(stack, 'ActionA', {
49+
stage,
50+
changeSetName: 'MyChangeSet',
51+
stackName: 'StackA',
52+
templatePath: artifact.atPath('path/to/file')
53+
});
54+
55+
new cloudformation.PipelineCreateReplaceChangeSetAction(stack, 'ActionB', {
56+
stage,
57+
changeSetName: 'MyChangeSet',
58+
stackName: 'StackB',
59+
templatePath: artifact.atPath('path/to/other/file')
60+
});
61+
62+
test.deepEqual(
63+
cdk.resolve(pipelineRole.statements),
64+
[
65+
{
66+
Action: 'iam:PassRole',
67+
Effect: 'Allow',
68+
Resource: [
69+
{ 'Fn::GetAtt': [ 'ActionARole72759154', 'Arn' ] },
70+
{ 'Fn::GetAtt': [ 'ActionBRole6A2F6804', 'Arn' ] }
71+
],
72+
},
73+
{
74+
Action: [
75+
'cloudformation:CreateChangeSet',
76+
'cloudformation:DeleteChangeSet',
77+
'cloudformation:DescribeChangeSet',
78+
'cloudformation:DescribeStacks'
79+
],
80+
Condition: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': 'MyChangeSet' } },
81+
Effect: 'Allow',
82+
Resource: [
83+
// tslint:disable-next-line:max-line-length
84+
{ 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackA/*' ] ] },
85+
// tslint:disable-next-line:max-line-length
86+
{ 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackB/*' ] ] }
87+
],
88+
}
89+
]
90+
);
91+
4092
test.done();
4193
}
4294
},
95+
4396
'ExecuteChangeSet': {
44-
works(test: nodeunit.Test) {
97+
'works'(test: nodeunit.Test) {
4598
const stack = new cdk.Stack();
4699
const pipelineRole = new RoleDouble(stack, 'PipelineRole');
47100
const stage = new StageDouble({ pipelineRole });
@@ -61,6 +114,42 @@ export = nodeunit.testCase({
61114
ChangeSetName: 'MyChangeSet'
62115
});
63116

117+
test.done();
118+
},
119+
120+
'uses a single permission statement if the same ChangeSet name is used'(test: nodeunit.Test) {
121+
const stack = new cdk.Stack();
122+
const pipelineRole = new RoleDouble(stack, 'PipelineRole');
123+
const stage = new StageDouble({ pipelineRole });
124+
new cloudformation.PipelineExecuteChangeSetAction(stack, 'ActionA', {
125+
stage,
126+
changeSetName: 'MyChangeSet',
127+
stackName: 'StackA',
128+
});
129+
130+
new cloudformation.PipelineExecuteChangeSetAction(stack, 'ActionB', {
131+
stage,
132+
changeSetName: 'MyChangeSet',
133+
stackName: 'StackB',
134+
});
135+
136+
test.deepEqual(
137+
cdk.resolve(pipelineRole.statements),
138+
[
139+
{
140+
Action: 'cloudformation:ExecuteChangeSet',
141+
Condition: { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } },
142+
Effect: 'Allow',
143+
Resource: [
144+
// tslint:disable-next-line:max-line-length
145+
{ 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackA/*' ] ] },
146+
// tslint:disable-next-line:max-line-length
147+
{ 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackB/*' ] ] }
148+
],
149+
}
150+
]
151+
);
152+
64153
test.done();
65154
}
66155
},
@@ -72,6 +161,7 @@ export = nodeunit.testCase({
72161
stage: new StageDouble({ pipelineRole }),
73162
templatePath: new cpapi.Artifact(stack as any, 'TestArtifact').atPath('some/file'),
74163
stackName: 'MyStack',
164+
replaceOnFailure: true,
75165
});
76166
const stackArn = _stackArn('MyStack');
77167

@@ -144,12 +234,13 @@ function _hasAction(actions: cpapi.Action[], owner: string, provider: string, ca
144234
return false;
145235
}
146236

147-
function _assertPermissionGranted(test: nodeunit.Test, statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) {
237+
function _assertPermissionGranted(test: nodeunit.Test, statements: iam.PolicyStatement[], action: string, resource: string, conditions?: any) {
148238
const conditionStr = conditions
149239
? ` with condition(s) ${JSON.stringify(cdk.resolve(conditions))}`
150240
: '';
151-
const statementsStr = JSON.stringify(cdk.resolve(statements), null, 2);
152-
test.ok(_grantsPermission(statements, action, resource, conditions),
241+
const resolvedStatements = cdk.resolve(statements);
242+
const statementsStr = JSON.stringify(resolvedStatements, null, 2);
243+
test.ok(_grantsPermission(resolvedStatements, action, resource, conditions),
153244
`Expected to find a statement granting ${action} on ${JSON.stringify(cdk.resolve(resource))}${conditionStr}, found:\n${statementsStr}`);
154245
}
155246

@@ -218,14 +309,14 @@ class StageDouble implements cpapi.IStage, cpapi.IInternalStage {
218309
}
219310

220311
class RoleDouble extends iam.Role {
221-
public readonly statements = new Array<PolicyStatementJson>();
312+
public readonly statements = new Array<iam.PolicyStatement>();
222313

223314
constructor(parent: cdk.Construct, id: string, props: iam.RoleProps = { assumedBy: new iam.ServicePrincipal('test') }) {
224315
super(parent, id, props);
225316
}
226317

227318
public addToPolicy(statement: iam.PolicyStatement) {
228319
super.addToPolicy(statement);
229-
this.statements.push(statement.toJson());
320+
this.statements.push(statement);
230321
}
231322
}

0 commit comments

Comments
 (0)