feat(aws-codepipeline): pipeline action’s service role - rename CF ro…

…le attribute Renamed * `PipelineCloudFormationDeployActionProps.role`; * `PipelineCloudFormationDeployAction.role`; * `PipelineDeployStackActionProps.role` to `deploymentRole` as `role` could cause a lot of ambiguity with more generic action’s roles. BREAKING CHANGE: `PipelineCloudFormationDeployActionProps.role`, `PipelineCloudFormationDeployAction.role`, `PipelineDeployStackActionProps.role` renamed to `deploymentRole`
aws · Jan 7, 2019 · aab545d · aab545d
1 parent fa9d42e
commit aab545d
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 30 deletions.
diff --git a/packages/@aws-cdk/app-delivery/lib/pipeline-deploy-stack-action.ts b/packages/@aws-cdk/app-delivery/lib/pipeline-deploy-stack-action.ts
@@ -97,7 +97,7 @@ export class PipelineDeployStackAction extends cdk.Construct {
   /**
    * The role used by CloudFormation for the deploy action
    */
-  public readonly role: iam.IRole;
+  public readonly deploymentRole: iam.IRole;
 
   private readonly stack: cdk.Stack;
 
@@ -127,10 +127,10 @@ export class PipelineDeployStackAction extends cdk.Construct {
       stage: props.stage,
       templatePath: props.inputArtifact.atPath(`${props.stack.name}.template.yaml`),
       adminPermissions: props.adminPermissions,
-      role: props.role,
+      deploymentRole: props.role,
       capabilities,
     });
-    this.role = changeSetAction.role;
+    this.deploymentRole = changeSetAction.deploymentRole;
 
     new cfn.PipelineExecuteChangeSetAction(this, 'Execute', {
       changeSetName,
@@ -159,8 +159,8 @@ export class PipelineDeployStackAction extends cdk.Construct {
    * `adminPermissions` you need to identify the proper statements to add to
    * this role based on the CloudFormation Resources in your stack.
    */
-  public addToRolePolicy(statement: iam.PolicyStatement) {
-    this.role.addToPolicy(statement);
+  public addToDeploymentRolePolicy(statement: iam.PolicyStatement) {
+    this.deploymentRole.addToPolicy(statement);
   }
 }
 

diff --git a/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts b/packages/@aws-cdk/app-delivery/test/test.pipeline-deploy-stack-action.ts
@@ -188,7 +188,7 @@ export = nodeunit.testCase({
       adminPermissions: false,
       role
     });
-    test.same(deployAction.role, role);
+    test.same(deployAction.deploymentRole, role);
     test.done();
   },
   'users can specify IAM permissions for the deploy action'(test: nodeunit.Test) {
@@ -211,7 +211,7 @@ export = nodeunit.testCase({
       adminPermissions: false,
     });
     // we might need to add permissions
-    deployAction.addToRolePolicy( new iam.PolicyStatement().
+    deployAction.addToDeploymentRolePolicy( new iam.PolicyStatement().
       addActions(
         'ec2:AuthorizeSecurityGroupEgress',
         'ec2:AuthorizeSecurityGroupIngress',

diff --git a/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts
@@ -122,7 +122,7 @@ export interface PipelineCloudFormationDeployActionProps extends PipelineCloudFo
    *
    * @default A fresh role with full or no permissions (depending on the value of `adminPermissions`).
    */
-  role?: iam.IRole;
+  deploymentRole?: iam.IRole;
 
   /**
    * Acknowledge certain changes made as part of deployment
@@ -193,40 +193,40 @@ export interface PipelineCloudFormationDeployActionProps extends PipelineCloudFo
  * Base class for all CloudFormation actions that execute or stage deployments.
  */
 export abstract class PipelineCloudFormationDeployAction extends PipelineCloudFormationAction {
-  public readonly role: iam.IRole;
+  public readonly deploymentRole: iam.IRole;
 
   constructor(scope: cdk.Construct, id: string, props: PipelineCloudFormationDeployActionProps, configuration: any) {
     const capabilities = props.adminPermissions && props.capabilities === undefined ? CloudFormationCapabilities.NamedIAM : props.capabilities;
     super(scope, id, props, {
       ...configuration,
       // None evaluates to empty string which is falsey and results in undefined
       Capabilities: (capabilities && capabilities.toString()) || undefined,
-      RoleArn: new cdk.Token(() => this.role.roleArn),
+      RoleArn: new cdk.Token(() => this.deploymentRole.roleArn),
       ParameterOverrides: cdk.CloudFormationJSON.stringify(props.parameterOverrides),
       TemplateConfiguration: props.templateConfiguration ? props.templateConfiguration.location : undefined,
       StackName: props.stackName,
     });
 
-    if (props.role) {
-      this.role = props.role;
+    if (props.deploymentRole) {
+      this.deploymentRole = props.deploymentRole;
     } else {
-      this.role = new iam.Role(this, 'Role', {
+      this.deploymentRole = new iam.Role(this, 'Role', {
         assumedBy: new iam.ServicePrincipal('cloudformation.amazonaws.com')
       });
 
       if (props.adminPermissions) {
-        this.role.addToPolicy(new iam.PolicyStatement().addAction('*').addAllResources());
+        this.deploymentRole.addToPolicy(new iam.PolicyStatement().addAction('*').addAllResources());
       }
     }
 
-    SingletonPolicy.forRole(props.stage.pipeline.role).grantPassRole(this.role);
+    SingletonPolicy.forRole(props.stage.pipeline.role).grantPassRole(this.deploymentRole);
   }
 
   /**
    * Add statement to the service role assumed by CloudFormation while executing this action.
    */
-  public addToRolePolicy(statement: iam.PolicyStatement) {
-    return this.role.addToPolicy(statement);
+  public addToDeploymentRolePolicy(statement: iam.PolicyStatement) {
+    return this.deploymentRole.addToPolicy(statement);
   }
 }
 

diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts b/packages/@aws-cdk/aws-cloudformation/test/test.pipeline-actions.ts
@@ -21,7 +21,7 @@ export = nodeunit.testCase({
         adminPermissions: false,
       });
 
-      _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
+      _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.deploymentRole.roleArn);
 
       const stackArn = _stackArn('MyStack');
       const changeSetCondition = { StringEqualsIfExists: { 'cloudformation:ChangeSetName': 'MyChangeSet' } };
@@ -175,7 +175,7 @@ export = nodeunit.testCase({
     _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:UpdateStack', stackArn);
     _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteStack', stackArn);
 
-    _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
+    _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.deploymentRole.roleArn);
 
     test.done();
   },
@@ -193,7 +193,7 @@ export = nodeunit.testCase({
     _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DescribeStack*', stackArn);
     _assertPermissionGranted(test, pipelineRole.statements, 'cloudformation:DeleteStack', stackArn);
 
-    _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.role.roleArn);
+    _assertPermissionGranted(test, pipelineRole.statements, 'iam:PassRole', action.deploymentRole.roleArn);
 
     test.done();
   },

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
@@ -147,12 +147,6 @@
     "PipelineC660917D": {
       "Type": "AWS::CodePipeline::Pipeline",
       "Properties": {
-        "ArtifactStore": {
-          "Location": {
-            "Ref": "PipelineArtifactsBucket22248F97"
-          },
-          "Type": "S3"
-        },
         "RoleArn": {
           "Fn::GetAtt": [
             "PipelineRoleD68726F7",
@@ -221,7 +215,13 @@
             ],
             "Name": "CFN"
           }
-        ]
+        ],
+        "ArtifactStore": {
+          "Location": {
+            "Ref": "PipelineArtifactsBucket22248F97"
+          },
+          "Type": "S3"
+        }
       },
       "DependsOn": [
         "PipelineRoleD68726F7",
@@ -254,4 +254,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts b/packages/@aws-cdk/aws-codepipeline/test/integ.pipeline-cfn.ts
@@ -36,7 +36,7 @@ new cfn.PipelineCreateReplaceChangeSetAction(stack, 'DeployCFN', {
   stage: cfnStage,
   changeSetName,
   stackName,
-  role,
+  deploymentRole: role,
   templatePath: source.outputArtifact.atPath('test.yaml'),
   adminPermissions: false,
 });

diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.cloudformation-pipeline-actions.ts b/packages/@aws-cdk/aws-codepipeline/test/test.cloudformation-pipeline-actions.ts
@@ -63,7 +63,6 @@ export = {
     stackName,
     changeSetName,
     runOrder: 321,
-    role: changeSetExecRole,
     deploymentRole: changeSetExecRole,
     templatePath: new ArtifactPath(buildAction.outputArtifact, 'template.yaml'),
     templateConfiguration: new ArtifactPath(buildAction.outputArtifact, 'templateConfig.json'),