fix(codepipeline): correctly pass the replication Buckets to Action.b…

…ind() (aws#3131)

With the change to generate a Role per Pipeline Action,
I realized we were incorrectly setting permissions on cross-region Actions.
We were always passing the Pipeline Bucket to the Action.bind() method,
while in reality they need permissions to their replication Buckets.

I also changed the strategy of generating the names for the replication Buckets to use our new PhysicalName.GENERATE_IF_NEEDED.

As this is not a strictly backwards-compatible change
(it is for customers of `IAction`, but not for Action vendors),
I added a mechanism for exempting changes from our compatibility checker.
You drop a file called `allowed-breaking-changes-${version}.txt` to the root of your package,
where `${version}` is the version of your package (for example, `0.36.1`),
and the checker will exempt the violations present in that file.

packages/@aws-cdk/app-delivery/allowed-breaking-changes-0.36.1.txt

@@ -0,0 +1 @@
+incompatible-argument:@aws-cdk/app-delivery.PipelineDeployStackAction.bind

packages/@aws-cdk/aws-codepipeline-actions/allowed-breaking-changes-0.36.1.txt

@@ -0,0 +1,18 @@
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.Action.bind
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.Action.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.AlexaSkillDeployAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CodeDeployServerDeployAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CloudFormationCreateReplaceChangeSetAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CloudFormationCreateUpdateStackAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CloudFormationDeleteStackAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CloudFormationExecuteChangeSetAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CodeBuildAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.CodeCommitSourceAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.EcrSourceAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.EcsDeployAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.GitHubSourceAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.JenkinsAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.LambdaInvokeAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.ManualApprovalAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.S3DeployAction.bound
+incompatible-argument:@aws-cdk/aws-codepipeline-actions.S3SourceAction.bound

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

@@ -75,14 +75,14 @@ abstract class CloudFormationAction extends Action {
     this.props = props;
   }
 
-  protected bound(_scope: cdk.Construct, stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
+  protected bound(_scope: cdk.Construct, _stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
       codepipeline.ActionConfig {
     const singletonPolicy = SingletonPolicy.forRole(options.role);
 
     if ((this.actionProperties.outputs || []).length > 0) {
-      stage.pipeline.artifactBucket.grantReadWrite(singletonPolicy);
+      options.bucket.grantReadWrite(singletonPolicy);
     } else if ((this.actionProperties.inputs || []).length > 0) {
-      stage.pipeline.artifactBucket.grantRead(singletonPolicy);
+      options.bucket.grantRead(singletonPolicy);
     }
 
     return {

packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts

@@ -83,7 +83,7 @@ export class CodeBuildAction extends Action {
     this.props = props;
   }
 
-  protected bound(_scope: cdk.Construct, stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
+  protected bound(_scope: cdk.Construct, _stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
       codepipeline.ActionConfig {
     // grant the Pipeline role the required permissions to this Project
     options.role.addToPolicy(new iam.PolicyStatement({
@@ -97,9 +97,9 @@ export class CodeBuildAction extends Action {
 
     // allow the Project access to the Pipeline's artifact Bucket
     if ((this.actionProperties.outputs || []).length > 0) {
-      stage.pipeline.artifactBucket.grantReadWrite(this.props.project);
+      options.bucket.grantReadWrite(this.props.project);
     } else {
-      stage.pipeline.artifactBucket.grantRead(this.props.project);
+      options.bucket.grantRead(this.props.project);
     }
 
     const configuration: any = {

packages/@aws-cdk/aws-codepipeline-actions/lib/codecommit/source-action.ts

@@ -91,7 +91,7 @@ export class CodeCommitSourceAction extends Action {
 
     // the Action will write the contents of the Git repository to the Bucket,
     // so its Role needs write permissions to the Pipeline Bucket
-    stage.pipeline.artifactBucket.grantWrite(options.role);
+    options.bucket.grantWrite(options.role);
 
     // https://docs.aws.amazon.com/codecommit/latest/userguide/auth-and-access-control-permissions-reference.html#aa-acp
     options.role.addToPolicy(new iam.PolicyStatement({

packages/@aws-cdk/aws-codepipeline-actions/lib/codedeploy/server-deploy-action.ts

@@ -35,7 +35,7 @@ export class CodeDeployServerDeployAction extends Action {
     this.deploymentGroup = props.deploymentGroup;
   }
 
-  protected bound(_scope: Construct, stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
+  protected bound(_scope: Construct, _stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
       codepipeline.ActionConfig {
     // permissions, based on:
     // https://docs.aws.amazon.com/codedeploy/latest/userguide/auth-and-access-control-permissions-reference.html
@@ -57,11 +57,11 @@ export class CodeDeployServerDeployAction extends Action {
 
     // grant the ASG Role permissions to read from the Pipeline Bucket
     for (const asg of this.deploymentGroup.autoScalingGroups || []) {
-      stage.pipeline.artifactBucket.grantRead(asg.role);
+      options.bucket.grantRead(asg.role);
     }
 
     // the Action's Role needs to read from the Bucket to get artifacts
-    stage.pipeline.artifactBucket.grantRead(options.role);
+    options.bucket.grantRead(options.role);
 
     return {
       configuration: {

packages/@aws-cdk/aws-codepipeline-actions/lib/ecr/source-action.ts

@@ -63,7 +63,7 @@ export class EcrSourceAction extends Action {
     });
 
     // the Action Role also needs to write to the Pipeline's bucket
-    stage.pipeline.artifactBucket.grantWrite(options.role);
+    options.bucket.grantWrite(options.role);
 
     return {
       configuration: {

packages/@aws-cdk/aws-codepipeline-actions/lib/ecs/deploy-action.ts

@@ -61,7 +61,7 @@ export class EcsDeployAction extends Action {
     this.props = props;
   }
 
-  protected bound(_scope: Construct, stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
+  protected bound(_scope: Construct, _stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
       codepipeline.ActionConfig {
     // permissions based on CodePipeline documentation:
     // https://docs.aws.amazon.com/codepipeline/latest/userguide/how-to-custom-role.html#how-to-update-role-new-services
@@ -90,7 +90,7 @@ export class EcsDeployAction extends Action {
       }
     }));
 
-    stage.pipeline.artifactBucket.grantRead(options.role);
+    options.bucket.grantRead(options.role);
 
     return {
       configuration: {

packages/@aws-cdk/aws-codepipeline-actions/lib/s3/deploy-action.ts

@@ -49,13 +49,13 @@ export class S3DeployAction extends Action {
     this.props = props;
   }
 
-  protected bound(_scope: Construct, stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
+  protected bound(_scope: Construct, _stage: codepipeline.IStage, options: codepipeline.ActionBindOptions):
       codepipeline.ActionConfig {
     // pipeline needs permissions to write to the S3 bucket
     this.props.bucket.grantWrite(options.role);
 
     // the Action Role also needs to read from the Pipeline's bucket
-    stage.pipeline.artifactBucket.grantRead(options.role);
+    options.bucket.grantRead(options.role);
 
     return {
       configuration: {

packages/@aws-cdk/aws-codepipeline-actions/lib/s3/source-action.ts

@@ -96,7 +96,7 @@ export class S3SourceAction extends Action {
     this.props.bucket.grantRead(options.role);
 
     // ...and write to the Pipeline bucket
-    stage.pipeline.artifactBucket.grantWrite(options.role);
+    options.bucket.grantWrite(options.role);
 
     return {
       configuration: {

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

@@ -352,6 +352,7 @@ class StageDouble implements codepipeline.IStage {
       const actionParent = new cdk.Construct(stageParent, action.actionProperties.actionName);
       fullActions.push(new FullAction(action.actionProperties, action.bind(actionParent, this, {
         role: pipeline.role,
+        bucket: pipeline.artifactBucket,
       })));
     }
     this.actions = fullActions;

packages/@aws-cdk/aws-codepipeline-actions/test/test.pipeline.ts

@@ -619,7 +619,7 @@ export = {
             "Region": "us-east-1",
             "ArtifactStore": {
               "Type": "S3",
-              "Location": "cdk-cross-region-codepipeline-replication-bucket-685c6feea5fb",
+              "Location": "teststack-support-us-easteplicationbucket1a8063b3cdac6e7e0e73",
             },
           },
           {

packages/@aws-cdk/aws-codepipeline/allowed-breaking-changes-0.36.1.txt

@@ -0,0 +1,2 @@
+incompatible-argument:@aws-cdk/aws-codepipeline.IAction.bind
+removed:@aws-cdk/aws-codepipeline.IPipeline.artifactBucket

packages/@aws-cdk/aws-codepipeline/lib/action.ts

@@ -77,6 +77,8 @@ export interface ActionProperties {
 
 export interface ActionBindOptions {
   readonly role: iam.IRole;
+
+  readonly bucket: s3.IBucket;
 }
 
 export interface ActionConfig {
@@ -111,8 +113,6 @@ export interface IPipeline extends IResource {
    */
   readonly pipelineArn: string;
 
-  readonly artifactBucket: s3.IBucket;
-
   /**
    * Define an event rule triggered by this CodePipeline.
    *

packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts

@@ -1,6 +1,5 @@
 import s3 = require('@aws-cdk/aws-s3');
 import cdk = require('@aws-cdk/core');
-import crypto = require('crypto');
 
 /**
  * Construction properties for {@link CrossRegionSupportStack}.
@@ -45,27 +44,12 @@ export class CrossRegionSupportStack extends cdk.Stack {
       },
     });
 
-    const replicationBucketName = generateUniqueName('cdk-cross-region-codepipeline-replication-bucket-',
-      props.region, props.account, false, 12);
-
     this.replicationBucket = new s3.Bucket(this, 'CrossRegionCodePipelineReplicationBucket', {
-      bucketName: replicationBucketName,
+      bucketName: cdk.PhysicalName.GENERATE_IF_NEEDED,
     });
   }
 }
 
 function generateStackName(props: CrossRegionSupportStackProps): string {
   return `${props.pipelineStackName}-support-${props.region}`;
 }
-
-function generateUniqueName(baseName: string, region: string, account: string,
-                            toUpperCase: boolean, hashPartLen: number = 8): string {
-  const sha256 = crypto.createHash('sha256')
-    .update(baseName)
-    .update(region)
-    .update(account);
-
-  const hash = sha256.digest('hex').slice(0, hashPartLen);
-
-  return baseName + (toUpperCase ? hash.toUpperCase() : hash.toLowerCase());
-}

packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts

@@ -104,7 +104,6 @@ export interface PipelineProps {
 abstract class PipelineBase extends Resource implements IPipeline {
   public abstract pipelineName: string;
   public abstract pipelineArn: string;
-  public abstract artifactBucket: s3.IBucket;
 
   /**
    * Defines an event rule triggered by this CodePipeline.
@@ -305,14 +304,15 @@ export class Pipeline extends PipelineBase {
   /** @internal */
   public _attachActionToPipeline(stage: Stage, action: IAction, actionScope: Construct): FullActionDescriptor {
     // handle cross-region actions here
-    this.ensureReplicationBucketExistsFor(action.actionProperties.region);
+    const bucket = this.ensureReplicationBucketExistsFor(action.actionProperties.region);
 
     // get the role for the given action
     const actionRole = this.getRoleForAction(stage, action, actionScope);
 
     // bind the Action
     const actionDescriptor = action.bind(actionScope, stage, {
       role: actionRole ? actionRole : this.role,
+      bucket,
     });
 
     return new FullActionDescriptor(action, actionDescriptor, actionRole);
@@ -335,17 +335,17 @@ export class Pipeline extends PipelineBase {
     ];
   }
 
-  private ensureReplicationBucketExistsFor(region?: string) {
+  private ensureReplicationBucketExistsFor(region?: string): s3.IBucket {
     if (!region) {
-      return;
+      return this.artifactBucket;
     }
 
     // get the region the Pipeline itself is in
     const pipelineRegion = this.requireRegion();
 
     // if we already have an ArtifactStore generated for this region, or it's the Pipeline's region, nothing to do
-    if (this.artifactStores[region] || region === pipelineRegion) {
-      return;
+    if (region === pipelineRegion) {
+      return this.artifactBucket;
     }
 
     let replicationBucket = this.crossRegionReplicationBuckets[region];
@@ -371,13 +371,16 @@ export class Pipeline extends PipelineBase {
         stack: crossRegionScaffoldStack,
         replicationBucket,
       };
+      this.crossRegionReplicationBuckets[region] = replicationBucket;
     }
     replicationBucket.grantReadWrite(this.role);
 
     this.artifactStores[region] = {
       location: replicationBucket.bucketName,
       type: 'S3',
     };
+
+    return replicationBucket;
   }
 
   /**

scripts/check-api-compatibility.sh

@@ -44,12 +44,16 @@ fi
 
 #----------------------------------------------------------------------
 
+# get the current version from Lerna
+current_version=$(npx lerna ls -pl | head -n 1 | cut -d ':' -f 3)
+
 echo "Checking compatibility..." >&2
 success=true
 for i in ${!package_dirs[*]}; do
     if [[ ! -d $tmpdir/node_modules/${package_names[$i]} ]]; then continue; fi
     echo -n "${package_names[$i]}... "
-    if npx jsii-diff --experimental-errors $tmpdir/node_modules/${package_names[$i]} ${package_dirs[$i]} 2>$tmpdir/output.txt; then
+    if npx jsii-diff --experimental-errors $tmpdir/node_modules/${package_names[$i]} ${package_dirs[$i]} \
+            --ignore-file ${package_dirs[$i]}/allowed-breaking-changes-${current_version}.txt 2>$tmpdir/output.txt; then
         echo "OK."
     else
         success=false