From 35c00c00344212c5af35d8bc34a56f48f9376799 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Fri, 1 Mar 2019 18:00:41 -0800 Subject: [PATCH] Assign a physical name to the S3 Bucket in CodePipeline if a customer did not pass an explicit one and the Pipeline is cross-account. --- packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts | 9 +++++++++ .../aws-codepipeline/test/test.pipeline.ts | 8 +++----- packages/@aws-cdk/aws-s3/lib/bucket.ts | 4 ++++ packages/@aws-cdk/cdk/lib/core/construct.ts | 2 ++ .../cdk/lib/util/physical-name-generator.ts | 14 +++++++++----- 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 87acea2d3a56a..b28cb286fbe24 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -150,6 +150,7 @@ export class Pipeline extends cdk.Construct implements cpapi.IPipeline { */ public readonly artifactBucket: s3.IBucket; + private readonly managedBucket: boolean; private readonly stages = new Array(); private eventsRole?: iam.Role; private readonly pipelineResource: CfnPipeline; @@ -174,6 +175,9 @@ export class Pipeline extends cdk.Construct implements cpapi.IPipeline { encryptionKey, encryption: s3.BucketEncryption.Kms, }); + this.managedBucket = true; + } else { + this.managedBucket = false; } this.artifactBucket = propsBucket; @@ -416,6 +420,11 @@ export class Pipeline extends cdk.Construct implements cpapi.IPipeline { return action.role; } + // make sure the Bucket has a physical name if the Construct itself is managing it + if (this.managedBucket) { + this.artifactBucket.assignPhysicalNameIfNotSet(); + } + let actionRole = this.crossAccountActionRoles.get(resourceStack); if (!actionRole) { actionRole = new iam.Role(resourceStack, this.node.id + 'ActionRole', { diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts b/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts index 338389f6604b2..ea78098a6f783 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts @@ -600,9 +600,8 @@ export = { const pipelineStack = new cdk.Stack(undefined, 'PipelineStack', { env: { account: '123456789012' }, }); - const bucketPhysicalName = 'artifact-bucket'; const bucket = new s3.Bucket(pipelineStack, 'ArtifactBucket', { - bucketName: bucketPhysicalName, + bucketName: 'artifact-bucket', encryption: s3.BucketEncryption.Kms, }); const sourceAction = bucket.toCodePipelineSourceAction({ @@ -610,7 +609,6 @@ export = { bucketKey: 'path/to/file.zip', }); new codepipeline.Pipeline(pipelineStack, 'Pipeline', { - artifactBucket: bucket, stages: [ { name: 'Source', @@ -684,7 +682,7 @@ export = { { "Ref": "AWS::Partition", }, - `:s3:::${bucketPhysicalName}`, + `:s3:::pipelinestack-artifactsbucket-5a14952fd11d`, ], ], }, @@ -696,7 +694,7 @@ export = { { "Ref": "AWS::Partition", }, - `:s3:::${bucketPhysicalName}/*`, + `:s3:::pipelinestack-artifactsbucket-5a14952fd11d/*`, ], ], }, diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index cc33b7d4e009b..7cbf77ff8b4d2 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -849,6 +849,10 @@ export class Bucket extends BucketBase { return this.onEvent(EventType.ObjectRemoved, dest, ...filters); } + public physicalNameGenerator(): cdk.PhysicalNameGenerator { + return super.physicalNameGenerator().toLowerCase(); + } + /** * Set up key properties and return the Bucket encryption property from the * user's configuration. diff --git a/packages/@aws-cdk/cdk/lib/core/construct.ts b/packages/@aws-cdk/cdk/lib/core/construct.ts index 3930b8fef7c47..bab6da3507e20 100644 --- a/packages/@aws-cdk/cdk/lib/core/construct.ts +++ b/packages/@aws-cdk/cdk/lib/core/construct.ts @@ -19,6 +19,8 @@ export interface IConstruct extends IDependable { readonly physicalName: string; readonly tryPhysicalName?: string + + assignPhysicalNameIfNotSet(name?: string): boolean; } /** diff --git a/packages/@aws-cdk/cdk/lib/util/physical-name-generator.ts b/packages/@aws-cdk/cdk/lib/util/physical-name-generator.ts index f78f67c173e1b..992c75c0c01a6 100644 --- a/packages/@aws-cdk/cdk/lib/util/physical-name-generator.ts +++ b/packages/@aws-cdk/cdk/lib/util/physical-name-generator.ts @@ -2,10 +2,6 @@ import crypto = require('crypto'); import { Stack } from '../cloudformation/stack'; import { IConstruct } from '../core/construct'; -// interface { -// -// } - export class PhysicalNameGenerator { public static from(construct: IConstruct): PhysicalNameGenerator { return new PhysicalNameGenerator( @@ -18,6 +14,7 @@ export class PhysicalNameGenerator { private readonly idPart: NamePart; private readonly stack: Stack; private readonly hashLength: number; + private changeToLowerCase = false; private constructor(stackPart: NamePart, idPart: NamePart, stack: Stack) { this.stackPart = stackPart; @@ -38,9 +35,16 @@ export class PhysicalNameGenerator { const parts = [this.stackPart, this.idPart] .map(part => part.generate()); - return [...parts, hash] + const ret = [...parts, hash] .filter(part => part.length > 0) .join('-'); + + return this.changeToLowerCase ? ret.toLowerCase() : ret; + } + + public toLowerCase(): PhysicalNameGenerator { + this.changeToLowerCase = true; + return this; } }