feat(aws-cloudformation): add permission management to CreateUpdate and Delete Stack CodePipeline Actions. (#880)

skinny85 · web-flow · commit 8b3ae433558c · 2018-10-11T14:55:18.000-07:00
diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts
@@ -211,6 +211,12 @@ export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFo
         this.role.addToPolicy(new iam.PolicyStatement().addAction('*').addAllResources());
       }
     }
+
+    // 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));
   }
 
   /**
@@ -265,10 +271,6 @@ export class PipelineCreateReplaceChangeSetAction extends PipelineCloudFormation
       .addActions('cloudformation:CreateChangeSet', 'cloudformation:DeleteChangeSet', 'cloudformation:DescribeChangeSet')
       .addResource(stackArn)
       .addCondition('StringEquals', { 'cloudformation:ChangeSetName': props.changeSetName }));
-    // Allow the pipeline to pass this actions' role to CloudFormation
-    props.stage.pipelineRole.addToPolicy(new iam.PolicyStatement()
-      .addAction('iam:PassRole')
-      .addResource(this.role.roleArn));
   }
 }
 
@@ -318,6 +320,23 @@ export class PipelineCreateUpdateStackAction extends PipelineCloudFormationDeplo
       TemplatePath: props.templatePath.location
     });
     this.addInputArtifact(props.templatePath.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));
   }
 }
 
@@ -339,6 +358,13 @@ 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));
   }
 }
 
diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts
@@ -6,7 +6,7 @@ import nodeunit = require('nodeunit');
 import cloudformation = require('../lib');
 
 export = nodeunit.testCase({
-  CreateReplaceChangeSet: {
+  'CreateReplaceChangeSet': {
     works(test: nodeunit.Test) {
       const stack = new cdk.Stack();
       const pipelineRole = new RoleDouble(stack, 'PipelineRole');
@@ -21,11 +21,7 @@ export = nodeunit.testCase({
 
       _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
 
-      const stackArn = cdk.ArnUtils.fromComponents({
-        service: 'cloudformation',
-        resource: 'stack',
-        resourceName: 'MyStack/*'
-      });
+      const stackArn = _stackArn('MyStack');
       const changeSetCondition = { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } };
       _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStacks', stackArn);
       _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeChangeSet', stackArn, changeSetCondition);
@@ -44,7 +40,7 @@ export = nodeunit.testCase({
       test.done();
     }
   },
-  ExecuteChangeSet: {
+  'ExecuteChangeSet': {
     works(test: nodeunit.Test) {
       const stack = new cdk.Stack();
       const pipelineRole = new RoleDouble(stack, 'PipelineRole');
@@ -55,11 +51,7 @@ export = nodeunit.testCase({
         stackName: 'MyStack',
       });
 
-      const stackArn = cdk.ArnUtils.fromComponents({
-        service: 'cloudformation',
-        resource: 'stack',
-        resourceName: 'MyStack/*'
-      });
+      const stackArn = _stackArn('MyStack');
       _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:ExecuteChangeSet', stackArn,
                                { StringEquals: { 'cloudformation:ChangeSetName': 'MyChangeSet' } });
 
@@ -71,7 +63,44 @@ export = nodeunit.testCase({
 
       test.done();
     }
-  }
+  },
+
+  'the CreateUpdateStack Action sets the DescribeStack*, Create/Update/DeleteStack & PassRole permissions'(test: nodeunit.Test) {
+    const stack = new cdk.Stack();
+    const pipelineRole = new RoleDouble(stack, 'PipelineRole');
+    const action = new cloudformation.PipelineCreateUpdateStackAction(stack, 'Action', {
+      stage: new StageDouble({ pipelineRole }),
+      templatePath: new cpapi.Artifact(stack as any, 'TestArtifact').atPath('some/file'),
+      stackName: 'MyStack',
+    });
+    const stackArn = _stackArn('MyStack');
+
+    _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStack*', stackArn);
+    _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:CreateStack', stackArn);
+    _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:UpdateStack', stackArn);
+    _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteStack', stackArn);
+
+    _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
+
+    test.done();
+  },
+
+  'the DeleteStack Action sets the DescribeStack*, DeleteStack & PassRole permissions'(test: nodeunit.Test) {
+    const stack = new cdk.Stack();
+    const pipelineRole = new RoleDouble(stack, 'PipelineRole');
+    const action = new cloudformation.PipelineDeleteStackAction(stack, 'Action', {
+      stage: new StageDouble({ pipelineRole }),
+      stackName: 'MyStack',
+    });
+    const stackArn = _stackArn('MyStack');
+
+    _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStack*', stackArn);
+    _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteStack', stackArn);
+
+    _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
+
+    test.done();
+  },
 });
 
 interface PolicyStatementJson {
@@ -121,7 +150,7 @@ function _assertPermissionGranted(test: nodeunit.Test, statements: PolicyStateme
                      : '';
   const statementsStr = JSON.stringify(cdk.resolve(statements), null, 2);
   test.ok(_grantsPermission(statements, action, resource, conditions),
-          `Expected to find a statement granting ${action} on ${cdk.resolve(resource)}${conditionStr}, found:\n${statementsStr}`);
+          `Expected to find a statement granting ${action} on ${JSON.stringify(cdk.resolve(resource))}${conditionStr}, found:\n${statementsStr}`);
 }
 
 function _grantsPermission(statements: PolicyStatementJson[], action: string, resource: string, conditions?: any) {
@@ -145,6 +174,14 @@ function _isOrContains(entity: string | string[], value: string): boolean {
   return false;
 }
 
+function _stackArn(stackName: string): string {
+  return cdk.ArnUtils.fromComponents({
+    service: 'cloudformation',
+    resource: 'stack',
+    resourceName: `${stackName}/*`,
+  });
+}
+
 class StageDouble implements cpapi.IStage, cpapi.IInternalStage {
   public readonly name: string;
   public readonly pipelineArn: string;
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
@@ -76,6 +76,16 @@
                 ]
               }
             },
+            {
+              "Action": "iam:PassRole",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::GetAtt": [
+                  "PipelineDeployPrepareChangesRoleD28C853C",
+                  "Arn"
+                ]
+              }
+            },
             {
               "Action": "cloudformation:DescribeStacks",
               "Effect": "Allow",
@@ -145,16 +155,6 @@
                 ]
               }
             },
-            {
-              "Action": "iam:PassRole",
-              "Effect": "Allow",
-              "Resource": {
-                "Fn::GetAtt": [
-                  "PipelineDeployPrepareChangesRoleD28C853C",
-                  "Arn"
-                ]
-              }
-            },
             {
               "Action": "cloudformation:ExecuteChangeSet",
               "Condition": {
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
@@ -91,6 +91,16 @@
                 }
               ]
             },
+            {
+              "Action": "iam:PassRole",
+              "Effect": "Allow",
+              "Resource": {
+                "Fn::GetAtt": [
+                  "CfnChangeSetRole6F05F6FC",
+                  "Arn"
+                ]
+              }
+            },
             {
               "Action": "cloudformation:DescribeStacks",
               "Effect": "Allow",
@@ -159,16 +169,6 @@
                   ]
                 ]
               }
-            },
-            {
-              "Action": "iam:PassRole",
-              "Effect": "Allow",
-              "Resource": {
-                "Fn::GetAtt": [
-                  "CfnChangeSetRole6F05F6FC",
-                  "Arn"
-                ]
-              }
             }
           ],
           "Version": "2012-10-17"

Original file line number	Diff line number	Diff line change
`@@ -76,6 +76,16 @@`
`76`	`76`	`]`
`77`	`77`	`}`
`78`	`78`	`},`
	`79`	`+ {`
	`80`	`+ "Action": "iam:PassRole",`
	`81`	`+ "Effect": "Allow",`
	`82`	`+ "Resource": {`
	`83`	`+ "Fn::GetAtt": [`
	`84`	`+ "PipelineDeployPrepareChangesRoleD28C853C",`
	`85`	`+ "Arn"`
	`86`	`+ ]`
	`87`	`+ }`
	`88`	`+ },`
`79`	`89`	`{`
`80`	`90`	`"Action": "cloudformation:DescribeStacks",`
`81`	`91`	`"Effect": "Allow",`
`@@ -145,16 +155,6 @@`
`145`	`155`	`]`
`146`	`156`	`}`
`147`	`157`	`},`
`148`		`- {`
`149`		`- "Action": "iam:PassRole",`
`150`		`- "Effect": "Allow",`
`151`		`- "Resource": {`
`152`		`- "Fn::GetAtt": [`
`153`		`- "PipelineDeployPrepareChangesRoleD28C853C",`
`154`		`- "Arn"`
`155`		`- ]`
`156`		`- }`
`157`		`- },`
`158`	`158`	`{`
`159`	`159`	`"Action": "cloudformation:ExecuteChangeSet",`
`160`	`160`	`"Condition": {`
Original file line number	Diff line number	Diff line change
`@@ -91,6 +91,16 @@`
`91`	`91`	`}`
`92`	`92`	`]`
`93`	`93`	`},`
	`94`	`+ {`
	`95`	`+ "Action": "iam:PassRole",`
	`96`	`+ "Effect": "Allow",`
	`97`	`+ "Resource": {`
	`98`	`+ "Fn::GetAtt": [`
	`99`	`+ "CfnChangeSetRole6F05F6FC",`
	`100`	`+ "Arn"`
	`101`	`+ ]`
	`102`	`+ }`
	`103`	`+ },`
`94`	`104`	`{`
`95`	`105`	`"Action": "cloudformation:DescribeStacks",`
`96`	`106`	`"Effect": "Allow",`
`@@ -159,16 +169,6 @@`
`159`	`169`	`]`
`160`	`170`	`]`
`161`	`171`	`}`
`162`		`- },`
`163`		`- {`
`164`		`- "Action": "iam:PassRole",`
`165`		`- "Effect": "Allow",`
`166`		`- "Resource": {`
`167`		`- "Fn::GetAtt": [`
`168`		`- "CfnChangeSetRole6F05F6FC",`
`169`		`- "Arn"`
`170`		`- ]`
`171`		`- }`
`172`	`172`	`}`
`173`	`173`	`],`
`174`	`174`	`"Version": "2012-10-17"`