diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts index 776d3fa694afa..74c3b453d1256 100644 --- a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts @@ -92,10 +92,8 @@ export class PipelineExecuteChangeSetAction extends PipelineCloudFormationAction ChangeSetName: props.changeSetName, }); - props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement() - .addAction('cloudformation:ExecuteChangeSet') - .addResource(stackArnFromName(props.stackName)) - .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName })); + SingletonPolicy.forRole(props.stage.pipelineRole) + .grantExecuteChangeSet(props); } } @@ -212,11 +210,7 @@ export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFo } } - // Allow the pipeline to pass this actions' role to CloudFormation - // Required by all Actions that perform CFN deployments - props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement() - .addAction('iam:PassRole') - .addResource(this.role.roleArn)); + SingletonPolicy.forRole(props.stage.pipelineRole).grantPassRole(this.role); } /** @@ -261,16 +255,7 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation this.addInputArtifact(props.templateConfiguration.artifact); } - const stackArn = stackArnFromName(props.stackName); - // Allow the pipeline to check for Stack & ChangeSet existence - props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement() - .addAction('cloudformation:DescribeStacks') - .addResource(stackArn)); - // Allow the pipeline to create & delete the specified ChangeSet - props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement() - .addActions('cloudformation:CreateChangeSet', 'cloudformation:DeleteChangeSet', 'cloudformation:DescribeChangeSet') - .addResource(stackArn) - .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName })); + SingletonPolicy.forRole(props.stage.pipelineRole).grantCreateReplaceChangeSet(props); } } @@ -325,22 +310,7 @@ export class PipelineCreateUpdateStackAction extends PipelineCloudFormationDeplo this.addInputArtifact(props.templateConfiguration.artifact); } - // permissions are based on best-guess from - // https://docs.aws.amazon.com/codepipeline/latest/userguide/how-to-custom-role.html - // and https://docs.aws.amazon.com/IAM/latest/UserGuide/list_awscloudformation.html - const stackArn = stackArnFromName(props.stackName); - props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement() - .addActions( - 'cloudformation:DescribeStack*', - 'cloudformation:CreateStack', - 'cloudformation:UpdateStack', - 'cloudformation:DeleteStack', // needed when props.replaceOnFailure is true - 'cloudformation:GetTemplate*', - 'cloudformation:ValidateTemplate', - 'cloudformation:GetStackPolicy', - 'cloudformation:SetStackPolicy', - ) - .addResource(stackArn)); + SingletonPolicy.forRole(props.stage.pipelineRole).grantCreateUpdateStack(props); } } @@ -362,13 +332,7 @@ export class PipelineDeleteStackAction extends PipelineCloudFormationDeployActio super(parent, id, props, { ActionMode: 'DELETE_ONLY', }); - const stackArn = stackArnFromName(props.stackName); - props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement() - .addActions( - 'cloudformation:DescribeStack*', - 'cloudformation:DeleteStack', - ) - .addResource(stackArn)); + SingletonPolicy.forRole(props.stage.pipelineRole).grantDeleteStack(props); } } @@ -401,3 +365,119 @@ function stackArnFromName(stackName: string): string { resourceName: `${stackName}/*` }); } + +/** + * Manages a bunch of singleton-y statements on the policy of an IAM Role. + * Dedicated methods can be used to add specific permissions to the role policy + * using as few statements as possible (adding resources to existing compatible + * statements instead of adding new statements whenever possible). + * + * Statements created outside of this class are not considered when adding new + * permissions. + */ +class SingletonPolicy extends cdk.Construct { + /** + * Obtain a SingletonPolicy for a given role. + * @param role the Role this policy is bound to. + * @returns the SingletonPolicy for this role. + */ + public static forRole(role: iam.Role): SingletonPolicy { + const found = role.tryFindChild(SingletonPolicy.UUID); + return (found as SingletonPolicy) || new SingletonPolicy(role); + } + + private static readonly UUID = '8389e75f-0810-4838-bf64-d6f85a95cf83'; + + private statements: { [key: string]: iam.PolicyStatement } = {}; + + private constructor(private readonly role: iam.Role) { + super(role, SingletonPolicy.UUID); + } + + public grantCreateUpdateStack(props: { stackName: string, replaceOnFailure?: boolean }): void { + const actions = [ + 'cloudformation:DescribeStack*', + 'cloudformation:CreateStack', + 'cloudformation:UpdateStack', + 'cloudformation:GetTemplate*', + 'cloudformation:ValidateTemplate', + 'cloudformation:GetStackPolicy', + 'cloudformation:SetStackPolicy', + ]; + if (props.replaceOnFailure) { + actions.push('cloudformation:DeleteStack'); + } + this.statementFor({ actions }).addResource(stackArnFromName(props.stackName)); + } + + public grantCreateReplaceChangeSet(props: { stackName: string, changeSetName: string }): void { + this.statementFor({ + actions: [ + 'cloudformation:CreateChangeSet', + 'cloudformation:DeleteChangeSet', + 'cloudformation:DescribeChangeSet', + 'cloudformation:DescribeStacks', + ], + conditions: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': props.changeSetName } }, + }).addResource(stackArnFromName(props.stackName)); + } + + public grantExecuteChangeSet(props: { stackName: string, changeSetName: string }): void { + this.statementFor({ + actions: ['cloudformation:ExecuteChangeSet'], + conditions: { StringEquals: { 'cloudformation:ChangeSetName': props.changeSetName } }, + }).addResource(stackArnFromName(props.stackName)); + } + + public grantDeleteStack(props: { stackName: string }): void { + this.statementFor({ + actions: [ + 'cloudformation:DescribeStack*', + 'cloudformation:DeleteStack', + ] + }).addResource(stackArnFromName(props.stackName)); + } + + public grantPassRole(role: iam.Role): void { + this.statementFor({ actions: ['iam:PassRole'] }).addResource(role.roleArn); + } + + private statementFor(template: StatementTemplate): iam.PolicyStatement { + const key = keyFor(template); + if (!(key in this.statements)) { + this.statements[key] = new iam.PolicyStatement().addActions(...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; + } + } + } +} + +interface StatementTemplate { + actions: string[]; + conditions?: StatementCondition; +} + +type StatementCondition = { [op: string]: { [attribute: string]: string } }; diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts index 071e80fc76253..11db184b50963 100644 --- a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts +++ b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts @@ -7,7 +7,7 @@ import cloudformation = require('../lib'); export = nodeunit.testCase({ 'CreateReplaceChangeSet': { - works(test: nodeunit.Test) { + 'works'(test: nodeunit.Test) { const stack = new cdk.Stack(); const pipelineRole = new RoleDouble(stack, 'PipelineRole'); const stage = new StageDouble({ pipelineRole }); @@ -22,8 +22,8 @@ export = nodeunit.testCase({ _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn); const stackArn = _stackArn('MyStack'); - const changeSetCondition = { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }; - _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn); + const changeSetCondition = { StringEqualsIfExists: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }; + _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn, changeSetCondition); _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeChangeSet', stackArn, changeSetCondition); _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:CreateChangeSet', stackArn, changeSetCondition); _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteChangeSet', stackArn, changeSetCondition); @@ -37,11 +37,64 @@ export = nodeunit.testCase({ ChangeSetName: 'MyChangeSet' }); + test.done(); + }, + + 'uses a single permission statement if the same ChangeSet name is used'(test: nodeunit.Test) { + const stack = new cdk.Stack(); + const pipelineRole = new RoleDouble(stack, 'PipelineRole'); + const stage = new StageDouble({ pipelineRole }); + const artifact = new cpapi.Artifact(stack as any, 'TestArtifact'); + new cloudformation.PipelineCreateReplaceChangeSetAction(stack, 'ActionA', { + stage, + changeSetName: 'MyChangeSet', + stackName: 'StackA', + templatePath: artifact.atPath('path/to/file') + }); + + new cloudformation.PipelineCreateReplaceChangeSetAction(stack, 'ActionB', { + stage, + changeSetName: 'MyChangeSet', + stackName: 'StackB', + templatePath: artifact.atPath('path/to/other/file') + }); + + test.deepEqual( + cdk.resolve(pipelineRole.statements), + [ + { + Action: 'iam:PassRole', + Effect: 'Allow', + Resource: [ + { 'Fn::GetAtt': [ 'ActionARole72759154', 'Arn' ] }, + { 'Fn::GetAtt': [ 'ActionBRole6A2F6804', 'Arn' ] } + ], + }, + { + Action: [ + 'cloudformation:CreateChangeSet', + 'cloudformation:DeleteChangeSet', + 'cloudformation:DescribeChangeSet', + 'cloudformation:DescribeStacks' + ], + Condition: { StringEqualsIfExists: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }, + Effect: 'Allow', + Resource: [ + // tslint:disable-next-line:max-line-length + { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackA/*' ] ] }, + // tslint:disable-next-line:max-line-length + { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackB/*' ] ] } + ], + } + ] + ); + test.done(); } }, + 'ExecuteChangeSet': { - works(test: nodeunit.Test) { + 'works'(test: nodeunit.Test) { const stack = new cdk.Stack(); const pipelineRole = new RoleDouble(stack, 'PipelineRole'); const stage = new StageDouble({ pipelineRole }); @@ -61,6 +114,42 @@ export = nodeunit.testCase({ ChangeSetName: 'MyChangeSet' }); + test.done(); + }, + + 'uses a single permission statement if the same ChangeSet name is used'(test: nodeunit.Test) { + const stack = new cdk.Stack(); + const pipelineRole = new RoleDouble(stack, 'PipelineRole'); + const stage = new StageDouble({ pipelineRole }); + new cloudformation.PipelineExecuteChangeSetAction(stack, 'ActionA', { + stage, + changeSetName: 'MyChangeSet', + stackName: 'StackA', + }); + + new cloudformation.PipelineExecuteChangeSetAction(stack, 'ActionB', { + stage, + changeSetName: 'MyChangeSet', + stackName: 'StackB', + }); + + test.deepEqual( + cdk.resolve(pipelineRole.statements), + [ + { + Action: 'cloudformation:ExecuteChangeSet', + Condition: { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } }, + Effect: 'Allow', + Resource: [ + // tslint:disable-next-line:max-line-length + { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackA/*' ] ] }, + // tslint:disable-next-line:max-line-length + { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':cloudformation:', { Ref: 'AWS::Region' }, ':', { Ref: 'AWS::AccountId' }, ':stack/StackB/*' ] ] } + ], + } + ] + ); + test.done(); } }, @@ -72,6 +161,7 @@ export = nodeunit.testCase({ stage: new StageDouble({ pipelineRole }), templatePath: new cpapi.Artifact(stack as any, 'TestArtifact').atPath('some/file'), stackName: 'MyStack', + replaceOnFailure: true, }); const stackArn = _stackArn('MyStack'); @@ -144,12 +234,13 @@ function _hasAction(actions: cpapi.Action[], owner: string, provider: string, ca return false; } -function _assertPermissionGranted(test: nodeunit.Test, statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) { +function _assertPermissionGranted(test: nodeunit.Test, statements: iam.PolicyStatement[], action: string, resource: string, conditions?: any) { const conditionStr = conditions ? ` with condition(s) ${JSON.stringify(cdk.resolve(conditions))}` : ''; - const statementsStr = JSON.stringify(cdk.resolve(statements), null, 2); - test.ok(_grantsPermission(statements, action, resource, conditions), + const resolvedStatements = cdk.resolve(statements); + const statementsStr = JSON.stringify(resolvedStatements, null, 2); + test.ok(_grantsPermission(resolvedStatements, action, resource, conditions), `Expected to find a statement granting ${action} on ${JSON.stringify(cdk.resolve(resource))}${conditionStr}, found:\n${statementsStr}`); } @@ -218,7 +309,7 @@ class StageDouble implements cpapi.IStage, cpapi.IInternalStage { } class RoleDouble extends iam.Role { - public readonly statements = new Array(); + public readonly statements = new Array(); constructor(parent: cdk.Construct, id: string, props: iam.RoleProps = { assumedBy: new iam.ServicePrincipal('test') }) { super(parent, id, props); @@ -226,6 +317,6 @@ class RoleDouble extends iam.Role { public addToPolicy(statement: iam.PolicyStatement) { super.addToPolicy(statement); - this.statements.push(statement.toJson()); + this.statements.push(statement); } } diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json index 2b78411b8811d..552b0ee5903aa 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.cfn-template-from-repo.lit.expected.json @@ -85,38 +85,15 @@ ] } }, - { - "Action": "cloudformation:DescribeStacks", - "Effect": "Allow", - "Resource": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":cloudformation:", - { - "Ref": "AWS::Region" - }, - ":", - { - "Ref": "AWS::AccountId" - }, - ":stack/OurStack/*" - ] - ] - } - }, { "Action": [ "cloudformation:CreateChangeSet", "cloudformation:DeleteChangeSet", - "cloudformation:DescribeChangeSet" + "cloudformation:DescribeChangeSet", + "cloudformation:DescribeStacks" ], "Condition": { - "StringEquals": { + "StringEqualsIfExists": { "cloudformation:ChangeSetName": "StagedChangeSet" } }, diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.expected.json index c4eba9bf3313a..c8b43657dfa5c 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.lambda-pipeline.expected.json @@ -272,4 +272,4 @@ ] } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json index ac56d1f1bbb6a..2481e51936f1f 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.expected.json @@ -99,38 +99,15 @@ ] } }, - { - "Action": "cloudformation:DescribeStacks", - "Effect": "Allow", - "Resource": { - "Fn::Join": [ - "", - [ - "arn:", - { - "Ref": "AWS::Partition" - }, - ":cloudformation:", - { - "Ref": "AWS::Region" - }, - ":", - { - "Ref": "AWS::AccountId" - }, - ":stack/IntegTest-TestActionStack/*" - ] - ] - } - }, { "Action": [ "cloudformation:CreateChangeSet", "cloudformation:DeleteChangeSet", - "cloudformation:DescribeChangeSet" + "cloudformation:DescribeChangeSet", + "cloudformation:DescribeStacks" ], "Condition": { - "StringEquals": { + "StringEqualsIfExists": { "cloudformation:ChangeSetName": "ChangeSetIntegTest" } }, diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit-build.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit-build.expected.json index 5ff0eed026a50..c0083e6923c60 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit-build.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit-build.expected.json @@ -403,4 +403,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit.expected.json index f00592f0ee917..52f99453d0776 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-commit.expected.json @@ -165,4 +165,4 @@ ] } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-deploy.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-deploy.expected.json index 4dc1147ef370d..85cde39cd5645 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-deploy.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-code-deploy.expected.json @@ -319,4 +319,4 @@ ] } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.expected.json b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.expected.json index 4667b873e5679..e9e1f3be44016 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.expected.json +++ b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-events.expected.json @@ -531,4 +531,4 @@ } } } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts index 20cb222e33032..f4d8407c6868c 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/test.dynamodb.ts @@ -2031,4 +2031,4 @@ function testGrant(test: Test, expectedActions: string[], invocation: (user: iam "Users": [ { "Ref": "user2C2B57AE" } ] })); test.done(); -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 1853f28952f7d..30c31e666e95a 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -21,6 +21,15 @@ export interface RoleProps { */ managedPolicyArns?: string[]; + /** + * A list of named policies to inline into this role. These policies will be + * created with the role, whereas those added by ``addToPolicy`` are added + * using a separate CloudFormation resource (allowing a way around circular + * dependencies that could otherwise be introduced). + * @default No policy is inlined in the Role resource. + */ + inlinePolicies?: { [name: string]: PolicyDocument }; + /** * The path associated with this role. For information about IAM paths, see * Friendly Names and Paths in IAM User Guide. @@ -112,15 +121,28 @@ export class Role extends Construct implements IIdentityResource, IPrincipal, ID const role = new cloudformation.RoleResource(this, 'Resource', { assumeRolePolicyDocument: this.assumeRolePolicy as any, managedPolicyArns: undefinedIfEmpty(() => this.managedPolicyArns), + policies: _flatten(props.inlinePolicies), path: props.path, roleName: props.roleName, - maxSessionDuration: props.maxSessionDurationSec + maxSessionDuration: props.maxSessionDurationSec, }); this.roleArn = role.roleArn; this.principal = new ArnPrincipal(this.roleArn); this.roleName = role.roleName; this.dependencyElements = [ role ]; + + function _flatten(policies?: { [name: string]: PolicyDocument }) { + if (policies == null || Object.keys(policies).length === 0) { + return undefined; + } + const result = new Array(); + for (const policyName of Object.keys(policies)) { + const policyDocument = policies[policyName]; + result.push({ policyName, policyDocument }); + } + return result; + } } /**