Skip to content

Commit

Permalink
fix(codepipeline): do not retain the default bucket key and alias (#4400
Browse files Browse the repository at this point in the history
)

Currently, the KMS key and alias used for the default CodePipeline artifact bucket are created with RemovalPolicy.RETAIN.
That is problematic when trying to re-deploy a stack after running `cdk destroy`,
as the alias name will already be taken.
Because of that, change the removal policy of both the key and the alias to RemovalPolicy.DESTROY -
there is a grace period of a few days on the key before it's removed permanently,
so that should be good enough if anyone needs it,
and it doesn't seem like directly reading the artifacts of the pipeline is an important use case anyway,
especially after it has been deleted.

Fixes #4336
  • Loading branch information
skinny85 authored and mergify[bot] committed Oct 10, 2019
1 parent 123c594 commit 9740ed3
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -142,8 +142,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -193,8 +193,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -118,8 +118,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -128,8 +128,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -151,8 +151,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -366,8 +366,8 @@
]
}
},
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
"UpdateReplacePolicy": "Delete",
"DeletionPolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -191,8 +191,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"MyPipelineArtifactsBucket727923DD": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -139,8 +139,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"MyPipelineRoleC0D47CA4": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineArtifactsBucket22248F97": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -149,8 +149,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"PipelineRoleD68726F7": {
"Type": "AWS::IAM::Role",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ export class CrossRegionSupportConstruct extends cdk.Construct {
constructor(scope: cdk.Construct, id: string) {
super(scope, id);

const encryptionKey = new kms.Key(this, 'CrossRegionCodePipelineReplicationBucketEncryptionKey');
const encryptionKey = new kms.Key(this, 'CrossRegionCodePipelineReplicationBucketEncryptionKey', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
});
const encryptionAlias = new AliasWithShorterGeneratedName(this, 'CrossRegionCodePipelineReplicationBucketEncryptionAlias', {
targetKey: encryptionKey,
aliasName: cdk.PhysicalName.GENERATE_IF_NEEDED,
removalPolicy: cdk.RemovalPolicy.RETAIN,
removalPolicy: cdk.RemovalPolicy.DESTROY,
});
this.replicationBucket = new s3.Bucket(this, 'CrossRegionCodePipelineReplicationBucket', {
bucketName: cdk.PhysicalName.GENERATE_IF_NEEDED,
Expand Down
8 changes: 6 additions & 2 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,11 @@ export class Pipeline extends PipelineBase {
// If a bucket has been provided, use it - otherwise, create a bucket.
let propsBucket = this.getArtifactBucketFromProps(props);
if (!propsBucket) {
const encryptionKey = new kms.Key(this, 'ArtifactsBucketEncryptionKey');
const encryptionKey = new kms.Key(this, 'ArtifactsBucketEncryptionKey', {
// remove the key - there is a grace period of a few days before it's gone for good,
// that should be enough for any emergency access to the bucket artifacts
removalPolicy: RemovalPolicy.DESTROY,
});
propsBucket = new s3.Bucket(this, 'ArtifactsBucket', {
bucketName: PhysicalName.GENERATE_IF_NEEDED,
encryptionKey,
Expand All @@ -234,7 +238,7 @@ export class Pipeline extends PipelineBase {
new kms.Alias(this, 'ArtifactsBucketEncryptionKeyAlias', {
aliasName: this.generateNameForDefaultBucketKeyAlias(),
targetKey: encryptionKey,
removalPolicy: RemovalPolicy.RETAIN, // alias should be retained, like the key
removalPolicy: RemovalPolicy.DESTROY, // destroy the alias along with the key
});
}
this.artifactBucket = propsBucket;
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ export = {
}));

expect(pipeline.crossRegionSupport[replicationRegion].stack).to(haveResourceLike('AWS::KMS::Alias', {
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete",
}, ResourcePart.CompleteDefinition));

test.done();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@
"Version": "2012-10-17"
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"pipelinePipeline22F2A91DArtifactsBucketC1799DCD": {
"Type": "AWS::S3::Bucket",
Expand Down Expand Up @@ -107,8 +107,8 @@
]
}
},
"DeletionPolicy": "Retain",
"UpdateReplacePolicy": "Retain"
"DeletionPolicy": "Delete",
"UpdateReplacePolicy": "Delete"
},
"pipelinePipeline22F2A91DRole58B7B05E": {
"Type": "AWS::IAM::Role",
Expand Down
8 changes: 4 additions & 4 deletions packages/decdk/test/__snapshots__/synth.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1891,7 +1891,7 @@ Object {
"UpdateReplacePolicy": "Retain",
},
"PipelineArtifactsBucketEncryptionKey01D58D69": Object {
"DeletionPolicy": "Retain",
"DeletionPolicy": "Delete",
"Properties": Object {
"KeyPolicy": Object {
"Statement": Array [
Expand Down Expand Up @@ -2010,10 +2010,10 @@ Object {
},
},
"Type": "AWS::KMS::Key",
"UpdateReplacePolicy": "Retain",
"UpdateReplacePolicy": "Delete",
},
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": Object {
"DeletionPolicy": "Retain",
"DeletionPolicy": "Delete",
"Properties": Object {
"AliasName": "alias/codepipeline-pipelinepipeline22f2a91d",
"TargetKeyId": Object {
Expand All @@ -2024,7 +2024,7 @@ Object {
},
},
"Type": "AWS::KMS::Alias",
"UpdateReplacePolicy": "Retain",
"UpdateReplacePolicy": "Delete",
},
"PipelineBuildCodePipelineActionRoleD77A08E6": Object {
"Properties": Object {
Expand Down

0 comments on commit 9740ed3

Please sign in to comment.