From 98afdeb5270100c1c0882f95c34f305fb2abc445 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Fri, 21 Jun 2019 16:14:15 -0700 Subject: [PATCH] refactor(codepipeline): change the API of cross-region replication Buckets. (#2977) Changed the name and type of the Pipeline.crossRegionScaffold property. Users are now supposed to pass instances of IBucket instead of just Bucket names. BREAKING CHANGE: the Pipeline construction property crossRegionReplicationBuckets now takes values of type IBucket instead of string. * The property Pipeline.crossRegionScaffoldStacks has been renamed to crossRegionSupport, and its type changed from CrossRegionScaffoldStack to CrossRegionSupport. --- .../test/test.pipeline.ts | 106 +++++++++++++++--- packages/@aws-cdk/aws-codepipeline/README.md | 20 ++-- ...stack.ts => cross-region-support-stack.ts} | 26 +++-- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 93 +++++++++------ 4 files changed, 175 insertions(+), 70 deletions(-) rename packages/@aws-cdk/aws-codepipeline/lib/{cross-region-scaffold-stack.ts => cross-region-support-stack.ts} (64%) diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/test.pipeline.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/test.pipeline.ts index f5525fbb65b80..34684325644d3 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/test.pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/test.pipeline.ts @@ -1,4 +1,4 @@ -import { expect, haveResource, haveResourceLike, not, SynthUtils } from '@aws-cdk/assert'; +import { countResources, expect, haveResource, haveResourceLike, not, SynthUtils } from '@aws-cdk/assert'; import codebuild = require('@aws-cdk/aws-codebuild'); import codecommit = require('@aws-cdk/aws-codecommit'); import codepipeline = require('@aws-cdk/aws-codepipeline'); @@ -567,19 +567,19 @@ export = { account: pipelineAccount, }, }); - const bucket = new s3.Bucket(stack, 'MyBucket'); const pipeline = new codepipeline.Pipeline(stack, 'MyPipeline', { crossRegionReplicationBuckets: { - 'us-west-1': 'sfo-replication-bucket', + 'us-west-1': s3.Bucket.fromBucketName(stack, 'ImportedBucket', 'sfo-replication-bucket'), }, }); + const sourceBucket = new s3.Bucket(stack, 'MyBucket'); const sourceOutput = new codepipeline.Artifact('SourceOutput'); const sourceAction = new cpactions.S3SourceAction({ actionName: 'BucketSource', bucketKey: '/some/key', output: sourceOutput, - bucket, + bucket: sourceBucket, }); pipeline.addStage({ stageName: 'Stage1', @@ -619,19 +619,25 @@ export = { "Region": "us-east-1", "ArtifactStore": { "Type": "S3", + "Location": "cdk-cross-region-codepipeline-replication-bucket-685c6feea5fb", }, }, { "Region": "us-west-1", "ArtifactStore": { - "Location": "sfo-replication-bucket", "Type": "S3", + "Location": "sfo-replication-bucket", }, }, { "Region": "us-west-2", "ArtifactStore": { "Type": "S3", + "EncryptionKey": { + "Type": "KMS", + "Id": { + }, + }, }, }, ], @@ -656,18 +662,90 @@ export = { }, ], }, - ] + ], })); - test.equal(pipeline.crossRegionScaffolding[pipelineRegion], undefined); - test.equal(pipeline.crossRegionScaffolding['us-west-1'], undefined); + test.equal(pipeline.crossRegionSupport[pipelineRegion], undefined); + test.equal(pipeline.crossRegionSupport['us-west-1'], undefined); + + const usEast1Support = pipeline.crossRegionSupport['us-east-1']; + test.notEqual(usEast1Support, undefined); + test.equal(usEast1Support.stack.region, 'us-east-1'); + test.equal(usEast1Support.stack.account, pipelineAccount); + test.ok(usEast1Support.stack.node.id.indexOf('us-east-1') !== -1, + `expected '${usEast1Support.stack.node.id}' to contain 'us-east-1'`); + + test.done(); + }, + + 'allows specifying only one of artifactBucket and crossRegionReplicationBuckets'(test: Test) { + const stack = new Stack(); + + test.throws(() => { + new codepipeline.Pipeline(stack, 'Pipeline', { + artifactBucket: new s3.Bucket(stack, 'Bucket'), + crossRegionReplicationBuckets: { + // even an empty map should trigger this validation... + }, + }); + }, /Only one of artifactBucket and crossRegionReplicationBuckets can be specified!/); + test.done(); + }, + + 'does not create a new artifact Bucket if one was provided in the cross-region Buckets for the Pipeline region'(test: Test) { + const pipelineRegion = 'us-west-2'; + + const stack = new Stack(undefined, undefined, { + env: { + region: pipelineRegion, + }, + }); + const sourceOutput = new codepipeline.Artifact(); + new codepipeline.Pipeline(stack, 'Pipeline', { + crossRegionReplicationBuckets: { + [pipelineRegion]: new s3.Bucket(stack, 'Bucket', { + bucketName: PhysicalName.of('my-pipeline-bucket'), + }) + }, + stages: [ + { + stageName: 'Source', + actions: [ + new cpactions.CodeCommitSourceAction({ + actionName: 'Source', + output: sourceOutput, + repository: new codecommit.Repository(stack, 'Repo', { repositoryName: 'Repo' }), + }), + ], + }, + { + stageName: 'Build', + actions: [ + new cpactions.CodeBuildAction({ + actionName: 'Build', + input: sourceOutput, + project: new codebuild.PipelineProject(stack, 'Project'), + }), + ], + }, + ], + }); + + expect(stack).to(countResources('AWS::S3::Bucket', 1)); - const usEast1ScaffoldStack = pipeline.crossRegionScaffolding['us-east-1']; - test.notEqual(usEast1ScaffoldStack, undefined); - test.equal(usEast1ScaffoldStack.region, 'us-east-1'); - test.equal(usEast1ScaffoldStack.account, pipelineAccount); - test.ok(usEast1ScaffoldStack.node.id.indexOf('us-east-1') !== -1, - `expected '${usEast1ScaffoldStack.node.id}' to contain 'us-east-1'`); + expect(stack).to(haveResourceLike('AWS::CodePipeline::Pipeline', { + "ArtifactStores": [ + { + "Region": pipelineRegion, + "ArtifactStore": { + "Type": "S3", + "Location": { + "Ref": "Bucket83908E77", + }, + }, + }, + ], + })); test.done(); }, diff --git a/packages/@aws-cdk/aws-codepipeline/README.md b/packages/@aws-cdk/aws-codepipeline/README.md index b9e4014fdc72f..fb5db152b99d7 100644 --- a/packages/@aws-cdk/aws-codepipeline/README.md +++ b/packages/@aws-cdk/aws-codepipeline/README.md @@ -100,7 +100,9 @@ It works like this: const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', { // ... crossRegionReplicationBuckets: { - 'us-west-1': 'my-us-west-1-replication-bucket', + // note that a physical name of the replication Bucket must be known at synthesis time + 'us-west-1': s3.Bucket.fromBucketName(this, 'UsWest1ReplicationBucket', + 'my-us-west-1-replication-bucket'), }, }); @@ -115,22 +117,20 @@ new codepipeline_actions.CloudFormationCreateUpdateStackAction({ This way, the `CFN_US_West_1` Action will operate in the `us-west-1` region, regardless of which region your Pipeline is in. -If you don't provide a bucket name for a region (other than the Pipeline's region) -that you're using for an Action with the `crossRegionReplicationBuckets` property, -there will be a new Stack, named `aws-cdk-codepipeline-cross-region-scaffolding-`, +If you don't provide a bucket for a region (other than the Pipeline's region) +that you're using for an Action, +there will be a new Stack, called `-support-`, defined for you, containing a replication Bucket. -Note that you have to make sure to `cdk deploy` all of these automatically created Stacks -before you can deploy your main Stack (the one containing your Pipeline). -Use the `cdk ls` command to see all of the Stacks comprising your CDK application. +This new Stack will depend on your Pipeline Stack, +so deploying the Pipeline Stack will deploy the support Stack(s) first. Example: ```bash $ cdk ls MyMainStack -aws-cdk-codepipeline-cross-region-scaffolding-us-west-1 -$ cdk deploy aws-cdk-codepipeline-cross-region-scaffolding-us-west-1 -# output of cdk deploy here... +MyMainStack-support-us-west-1 $ cdk deploy MyMainStack +# output of cdk deploy here... ``` See [the AWS docs here](https://docs.aws.amazon.com/codepipeline/latest/userguide/actions-create-cross-region.html) diff --git a/packages/@aws-cdk/aws-codepipeline/lib/cross-region-scaffold-stack.ts b/packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts similarity index 64% rename from packages/@aws-cdk/aws-codepipeline/lib/cross-region-scaffold-stack.ts rename to packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts index 36667a8a1d9a2..16c397e66a998 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/cross-region-scaffold-stack.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts @@ -1,12 +1,18 @@ import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/cdk'); import crypto = require('crypto'); -import { CrossRegionScaffolding } from './pipeline'; /** - * Construction properties for {@link CrossRegionScaffoldStack}. + * Construction properties for {@link CrossRegionSupportStack}. + * This interface is private to the aws-codepipeline package. */ -export interface CrossRegionScaffoldStackProps { +export interface CrossRegionSupportStackProps { + /** + * The name of the Stack the Pipeline itself belongs to. + * Used to generate a more friendly name for the support Stack. + */ + readonly pipelineStackName: string; + /** * The AWS region this Stack resides in. */ @@ -22,14 +28,15 @@ export interface CrossRegionScaffoldStackProps { /** * A Stack containing resources required for the cross-region CodePipeline functionality to work. + * This class is private to the aws-codepipeline package. */ -export class CrossRegionScaffoldStack extends CrossRegionScaffolding { +export class CrossRegionSupportStack extends cdk.Stack { /** * The name of the S3 Bucket used for replicating the Pipeline's artifacts into the region. */ - public readonly replicationBucketName: string; + public readonly replicationBucket: s3.IBucket; - constructor(scope: cdk.Construct, id: string, props: CrossRegionScaffoldStackProps) { + constructor(scope: cdk.Construct, id: string, props: CrossRegionSupportStackProps) { super(scope, id, { stackName: generateStackName(props), env: { @@ -41,15 +48,14 @@ export class CrossRegionScaffoldStack extends CrossRegionScaffolding { const replicationBucketName = generateUniqueName('cdk-cross-region-codepipeline-replication-bucket-', props.region, props.account, false, 12); - new s3.Bucket(this, 'CrossRegionCodePipelineReplicationBucket', { + this.replicationBucket = new s3.Bucket(this, 'CrossRegionCodePipelineReplicationBucket', { bucketName: cdk.PhysicalName.of(replicationBucketName), }); - this.replicationBucketName = replicationBucketName; } } -function generateStackName(props: CrossRegionScaffoldStackProps): string { - return `aws-cdk-codepipeline-cross-region-scaffolding-${props.region}`; +function generateStackName(props: CrossRegionSupportStackProps): string { + return `${props.pipelineStackName}-support-${props.region}`; } function generateUniqueName(baseName: string, region: string, account: string, diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 9d23f108dcaa8..1e2932030cf3e 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -5,6 +5,7 @@ import s3 = require('@aws-cdk/aws-s3'); import { App, Construct, Lazy, PhysicalName, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/cdk'; import { Action, IPipeline, IStage } from "./action"; import { CfnPipeline } from './codepipeline.generated'; +import { CrossRegionSupportStack } from './cross-region-support-stack'; import { Stage } from './stage'; import { validateName, validateSourceAction } from "./validation"; @@ -84,13 +85,11 @@ export interface PipelineProps { * A map of region to S3 bucket name used for cross-region CodePipeline. * For every Action that you specify targeting a different region than the Pipeline itself, * if you don't provide an explicit Bucket for that region using this property, - * the construct will automatically create a scaffold Stack containing an S3 Bucket in that region. - * Note that you will have to `cdk deploy` that Stack before you can deploy your Pipeline-containing Stack. - * You can query the generated Stacks using the {@link Pipeline#crossRegionScaffoldStacks} property. + * the construct will automatically create a Stack containing an S3 Bucket in that region. * * @default - None. */ - readonly crossRegionReplicationBuckets?: { [region: string]: string }; + readonly crossRegionReplicationBuckets?: { [region: string]: s3.IBucket }; /** * The list of Stages, in order, @@ -214,9 +213,10 @@ export class Pipeline extends PipelineBase { public readonly artifactBucket: s3.IBucket; private readonly stages = new Array(); - private readonly crossRegionReplicationBuckets: { [region: string]: string }; + private readonly crossRegionReplicationBuckets: { [region: string]: s3.IBucket }; + private readonly crossRegionBucketsPassed: boolean; private readonly artifactStores: { [region: string]: CfnPipeline.ArtifactStoreProperty }; - private readonly _crossRegionScaffoldStacks: { [region: string]: CrossRegionScaffoldStack } = {}; + private readonly _crossRegionSupport: { [region: string]: CrossRegionSupport } = {}; constructor(scope: Construct, id: string, props: PipelineProps = {}) { super(scope, id, { @@ -225,8 +225,13 @@ export class Pipeline extends PipelineBase { validateName('Pipeline', this.physicalName); + // only one of artifactBucket and crossRegionReplicationBuckets can be supplied + if (props.artifactBucket && props.crossRegionReplicationBuckets) { + throw new Error('Only one of artifactBucket and crossRegionReplicationBuckets can be specified!'); + } + // If a bucket has been provided, use it - otherwise, create a bucket. - let propsBucket = props.artifactBucket; + let propsBucket = this.getArtifactBucketFromProps(props); if (!propsBucket) { const encryptionKey = new kms.Key(this, 'ArtifactsBucketEncryptionKey'); propsBucket = new s3.Bucket(this, 'ArtifactsBucket', { @@ -268,6 +273,7 @@ export class Pipeline extends PipelineBase { this.pipelineName = resourceIdentifiers.name; this.pipelineVersion = codePipeline.attrVersion; this.crossRegionReplicationBuckets = props.crossRegionReplicationBuckets || {}; + this.crossRegionBucketsPassed = !!props.crossRegionReplicationBuckets; this.artifactStores = {}; // Does not expose a Fn::GetAtt for the ARN so we'll have to make it ourselves @@ -327,13 +333,13 @@ export class Pipeline extends PipelineBase { } /** - * Returns all of the {@link CrossRegionScaffoldStack}s that were generated automatically + * Returns all of the {@link CrossRegionSupportStack}s that were generated automatically * when dealing with Actions that reside in a different region than the Pipeline itself. */ - public get crossRegionScaffolding(): { [region: string]: CrossRegionScaffolding } { - const ret: { [region: string]: CrossRegionScaffoldStack } = {}; - Object.keys(this._crossRegionScaffoldStacks).forEach((key) => { - ret[key] = this._crossRegionScaffoldStacks[key]; + public get crossRegionSupport(): { [region: string]: CrossRegionSupport } { + const ret: { [region: string]: CrossRegionSupport } = {}; + Object.keys(this._crossRegionSupport).forEach((key) => { + ret[key] = this._crossRegionSupport[key]; }); return ret; } @@ -374,7 +380,7 @@ export class Pipeline extends PipelineBase { }); } - private requireRegion() { + private requireRegion(): string { const region = Stack.of(this).region; if (Token.isUnresolved(region)) { throw new Error(`You need to specify an explicit region when using CodePipeline's cross-region support`); @@ -395,9 +401,10 @@ export class Pipeline extends PipelineBase { return; } - let replicationBucketName = this.crossRegionReplicationBuckets[region]; - if (!replicationBucketName) { - const pipelineAccount = Stack.of(this).account; + let replicationBucket = this.crossRegionReplicationBuckets[region]; + if (!replicationBucket) { + const pipelineStack = Stack.of(this); + const pipelineAccount = pipelineStack.account; if (Token.isUnresolved(pipelineAccount)) { throw new Error("You need to specify an explicit account when using CodePipeline's cross-region support"); } @@ -406,17 +413,18 @@ export class Pipeline extends PipelineBase { if (!app || !App.isApp(app)) { throw new Error(`Pipeline stack which uses cross region actions must be part of a CDK app`); } - const crossRegionScaffoldStack = new CrossRegionScaffoldStack(this, `cross-region-stack-${pipelineAccount}:${region}`, { + const crossRegionScaffoldStack = new CrossRegionSupportStack(app, `cross-region-stack-${pipelineAccount}:${region}`, { + pipelineStackName: pipelineStack.stackName, region, account: pipelineAccount, }); - this._crossRegionScaffoldStacks[region] = crossRegionScaffoldStack; - replicationBucketName = crossRegionScaffoldStack.replicationBucketName; + replicationBucket = crossRegionScaffoldStack.replicationBucket; + pipelineStack.addDependency(crossRegionScaffoldStack); + this._crossRegionSupport[region] = { + stack: crossRegionScaffoldStack, + replicationBucket, + }; } - - const replicationBucket = s3.Bucket.fromBucketAttributes(this, 'CrossRegionCodePipelineReplicationBucket-' + region, { - bucketName: replicationBucketName, - }); replicationBucket.grantReadWrite(this.role); this.artifactStores[region] = { @@ -473,6 +481,17 @@ export class Pipeline extends PipelineBase { return actionRole; } + private getArtifactBucketFromProps(props: PipelineProps): s3.IBucket | undefined { + if (props.artifactBucket) { + return props.artifactBucket; + } + if (props.crossRegionReplicationBuckets) { + const pipelineRegion = this.requireRegion(); + return props.crossRegionReplicationBuckets[pipelineRegion]; + } + return undefined; + } + private calculateInsertIndexFromPlacement(placement: StagePlacement): number { // check if at most one placement property was provided const providedPlacementProps = ['rightBefore', 'justAfter', 'atIndex'] @@ -576,11 +595,7 @@ export class Pipeline extends PipelineBase { // add the Pipeline's artifact store const primaryStore = this.renderPrimaryArtifactStore(); const primaryRegion = this.requireRegion(); - this.artifactStores[primaryRegion] = { - location: primaryStore.location, - type: primaryStore.type, - encryptionKey: primaryStore.encryptionKey, - }; + this.artifactStores[primaryRegion] = primaryStore; return Object.entries(this.artifactStores).map(([region, artifactStore]) => ({ region, artifactStore @@ -615,8 +630,8 @@ export class Pipeline extends PipelineBase { } private get crossRegion(): boolean { + if (this.crossRegionBucketsPassed) { return true; } return this.stages.some(stage => stage.actions.some(action => action.region !== undefined)); - // this.pipelineResource.addPropertyOverride(`Stages.${i}.Actions.${j}.Region`, action.region); } private renderStages(): CfnPipeline.StageDeclarationProperty[] { @@ -625,14 +640,20 @@ export class Pipeline extends PipelineBase { } /** - * A Stack containing resources required for the cross-region CodePipeline functionality to work. + * An interface representing resources generated in order to support + * the cross-region capabilities of CodePipeline. + * You get instances of this interface from the {@link Pipeline#crossRegionSupport} property. */ -export abstract class CrossRegionScaffolding extends Stack { +export interface CrossRegionSupport { /** - * The name of the S3 Bucket used for replicating the Pipeline's artifacts into the region. + * The Stack that has been created to house the replication Bucket + * required for this region. */ - public abstract readonly replicationBucketName: string; -} + readonly stack: Stack; -// cyclic dependency -import { CrossRegionScaffoldStack } from './cross-region-scaffold-stack'; + /** + * The replication Bucket used by CodePipeline to operate in this region. + * Belongs to {@link stack}. + */ + readonly replicationBucket: s3.IBucket; +}