Skip to content

Commit

Permalink
Assign a physical name to the S3 Bucket in CodePipeline if a customer…
Browse files Browse the repository at this point in the history
… did not pass an explicit one and the Pipeline is cross-account.
  • Loading branch information
skinny85 committed Mar 2, 2019
1 parent 299e151 commit 35c00c0
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 10 deletions.
9 changes: 9 additions & 0 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Stage>();
private eventsRole?: iam.Role;
private readonly pipelineResource: CfnPipeline;
Expand All @@ -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;

Expand Down Expand Up @@ -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', {
Expand Down
8 changes: 3 additions & 5 deletions packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,17 +600,15 @@ 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({
actionName: 'S3',
bucketKey: 'path/to/file.zip',
});
new codepipeline.Pipeline(pipelineStack, 'Pipeline', {
artifactBucket: bucket,
stages: [
{
name: 'Source',
Expand Down Expand Up @@ -684,7 +682,7 @@ export = {
{
"Ref": "AWS::Partition",
},
`:s3:::${bucketPhysicalName}`,
`:s3:::pipelinestack-artifactsbucket-5a14952fd11d`,
],
],
},
Expand All @@ -696,7 +694,7 @@ export = {
{
"Ref": "AWS::Partition",
},
`:s3:::${bucketPhysicalName}/*`,
`:s3:::pipelinestack-artifactsbucket-5a14952fd11d/*`,
],
],
},
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/cdk/lib/core/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export interface IConstruct extends IDependable {
readonly physicalName: string;

readonly tryPhysicalName?: string

assignPhysicalNameIfNotSet(name?: string): boolean;
}

/**
Expand Down
14 changes: 9 additions & 5 deletions packages/@aws-cdk/cdk/lib/util/physical-name-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand All @@ -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;
}
}

Expand Down

0 comments on commit 35c00c0

Please sign in to comment.