feat(codepipeline/cfn): Use fewer statements for pipeline permissions

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 } };

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,14 +309,14 @@ class StageDouble implements cpapi.IStage, cpapi.IInternalStage {
 }
 
 class RoleDouble extends iam.Role {
-  public readonly statements = new Array<PolicyStatementJson>();
+  public readonly statements = new Array<iam.PolicyStatement>();
 
   constructor(parent: cdk.Construct, id: string, props: iam.RoleProps = { assumedBy: new iam.ServicePrincipal('test') }) {
     super(parent, id, props);
   }
 
   public addToPolicy(statement: iam.PolicyStatement) {
     super.addToPolicy(statement);
-    this.statements.push(statement.toJson());
+    this.statements.push(statement);
   }
 }