From ccb805f44b21974eada69bc6e4a8820770243ea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Wed, 6 Feb 2019 11:03:11 +0100 Subject: [PATCH 1/3] feat(kms): Allow opting out of "Retain" deletion policy Gives the user control over whether the key should be retained or scheduled for deletion when it is removed from the stack (or the stack is deleted). This is convenient in particular for integration tests, to avoid accumulating garbage over successive runs. --- packages/@aws-cdk/aws-kms/lib/key.ts | 12 +++- .../test/integ.key-sharing.lit.expected.json | 2 +- .../aws-kms/test/integ.key-sharing.lit.ts | 2 +- .../aws-kms/test/integ.key.expected.json | 2 +- packages/@aws-cdk/aws-kms/test/integ.key.ts | 2 +- packages/@aws-cdk/aws-kms/test/test.key.ts | 60 +++++++++++++++++++ 6 files changed, 75 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index 7b259bfb8fc25..58dabf0a1d807 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -111,6 +111,14 @@ export interface EncryptionKeyProps { * The AWS resource tags to associate with the KMS key. */ tags?: Tags; + + /** + * Whether the encryption key should be retained when it is removed from the Stack. This is useful when one wants to + * retain access to data that was encrypted with a key that is being retired. + * + * @default true + */ + retainKey?: boolean; } /** @@ -168,7 +176,9 @@ export class EncryptionKey extends EncryptionKeyBase { }); this.keyArn = resource.keyArn; - resource.options.deletionPolicy = DeletionPolicy.Retain; + resource.options.deletionPolicy = props.retainKey === false + ? DeletionPolicy.Delete + : DeletionPolicy.Retain; } /** diff --git a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.expected.json b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.expected.json index 6f327ba4ab49f..3d64434cc7749 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.expected.json +++ b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.expected.json @@ -46,7 +46,7 @@ "Version": "2012-10-17" } }, - "DeletionPolicy": "Retain" + "DeletionPolicy": "Delete" }, "MyKeyAlias1B45D9DA": { "Type": "AWS::KMS::Alias", diff --git a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts index de46032822e13..37d86ed166e79 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts @@ -13,7 +13,7 @@ class KeyStack extends cdk.Stack { constructor(scope: cdk.App, id: string, props?: cdk.StackProps) { super(scope, id, props); - this.key = new kms.EncryptionKey(this, 'MyKey'); + this.key = new kms.EncryptionKey(this, 'MyKey', { retainKey: false }); } } diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.expected.json b/packages/@aws-cdk/aws-kms/test/integ.key.expected.json index 5394998c556aa..a1e41a2eca89e 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key.expected.json +++ b/packages/@aws-cdk/aws-kms/test/integ.key.expected.json @@ -55,7 +55,7 @@ "Version": "2012-10-17" } }, - "DeletionPolicy": "Retain" + "DeletionPolicy": "Delete" }, "MyKeyAlias1B45D9DA": { "Type": "AWS::KMS::Alias", diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.ts b/packages/@aws-cdk/aws-kms/test/integ.key.ts index 370a696fbaab0..dabf943198593 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key.ts @@ -6,7 +6,7 @@ const app = new App(); const stack = new Stack(app, `aws-cdk-kms-1`); -const key = new EncryptionKey(stack, 'MyKey'); +const key = new EncryptionKey(stack, 'MyKey', { retainKey: false }); key.addToResourcePolicy(new PolicyStatement() .addAllResources() diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts index 155347aa7a225..0db41657026db 100644 --- a/packages/@aws-cdk/aws-kms/test/test.key.ts +++ b/packages/@aws-cdk/aws-kms/test/test.key.ts @@ -65,6 +65,66 @@ export = { test.done(); }, + 'default with no retention'(test: Test) { + const app = new App(); + const stack = new Stack(app, 'TestStack'); + + new EncryptionKey(stack, 'MyKey', { retainKey: false }); + + expect(app.synthesizeStack(stack.name)).to(exactlyMatchTemplate({ + Resources: { + MyKey6AB29FA6: { + Type: "AWS::KMS::Key", + Properties: { + KeyPolicy: { + Statement: [ + { + Action: [ + "kms:Create*", + "kms:Describe*", + "kms:Enable*", + "kms:List*", + "kms:Put*", + "kms:Update*", + "kms:Revoke*", + "kms:Disable*", + "kms:Get*", + "kms:Delete*", + "kms:ScheduleKeyDeletion", + "kms:CancelKeyDeletion" + ], + Effect: "Allow", + Principal: { + AWS: { + "Fn::Join": [ + "", + [ + "arn:", + { + Ref: "AWS::Partition" + }, + ":iam::", + { + Ref: "AWS::AccountId" + }, + ":root" + ] + ] + } + }, + Resource: "*" + } + ], + Version: "2012-10-17" + } + }, + DeletionPolicy: "Delete", + } + } + })); + test.done(); + }, + 'default with some permission'(test: Test) { const app = new App(); const stack = new Stack(app, 'Test'); From 23a71b6572d631277d7ff37dc5ec9ef3ca04f6f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Wed, 6 Feb 2019 15:43:05 +0100 Subject: [PATCH 2/3] PR feedback --- packages/@aws-cdk/aws-kms/lib/key.ts | 4 +- .../aws-kms/test/integ.key-sharing.lit.ts | 2 +- packages/@aws-cdk/aws-kms/test/integ.key.ts | 2 +- packages/@aws-cdk/aws-kms/test/test.key.ts | 56 +------------------ 4 files changed, 7 insertions(+), 57 deletions(-) diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index e24b689c71900..2e444de50166e 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -118,7 +118,7 @@ export interface EncryptionKeyProps { * * @default true */ - retainKey?: boolean; + retain?: boolean; } /** @@ -168,7 +168,7 @@ export class EncryptionKey extends EncryptionKeyBase { }); this.keyArn = resource.keyArn; - resource.options.deletionPolicy = props.retainKey === false + resource.options.deletionPolicy = props.retain === false ? DeletionPolicy.Delete : DeletionPolicy.Retain; } diff --git a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts index 37d86ed166e79..ce830b9530e33 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key-sharing.lit.ts @@ -13,7 +13,7 @@ class KeyStack extends cdk.Stack { constructor(scope: cdk.App, id: string, props?: cdk.StackProps) { super(scope, id, props); - this.key = new kms.EncryptionKey(this, 'MyKey', { retainKey: false }); + this.key = new kms.EncryptionKey(this, 'MyKey', { retain: false }); } } diff --git a/packages/@aws-cdk/aws-kms/test/integ.key.ts b/packages/@aws-cdk/aws-kms/test/integ.key.ts index dabf943198593..47d05f7df73d0 100644 --- a/packages/@aws-cdk/aws-kms/test/integ.key.ts +++ b/packages/@aws-cdk/aws-kms/test/integ.key.ts @@ -6,7 +6,7 @@ const app = new App(); const stack = new Stack(app, `aws-cdk-kms-1`); -const key = new EncryptionKey(stack, 'MyKey', { retainKey: false }); +const key = new EncryptionKey(stack, 'MyKey', { retain: false }); key.addToResourcePolicy(new PolicyStatement() .addAllResources() diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts index e9089e9d0c007..255ad5eb8a4fb 100644 --- a/packages/@aws-cdk/aws-kms/test/test.key.ts +++ b/packages/@aws-cdk/aws-kms/test/test.key.ts @@ -1,4 +1,4 @@ -import { exactlyMatchTemplate, expect } from '@aws-cdk/assert'; +import { exactlyMatchTemplate, expect, haveResource, ResourcePart } from '@aws-cdk/assert'; import { PolicyDocument, PolicyStatement } from '@aws-cdk/aws-iam'; import { App, Stack, Tag } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; @@ -68,59 +68,9 @@ export = { const app = new App(); const stack = new Stack(app, 'TestStack'); - new EncryptionKey(stack, 'MyKey', { retainKey: false }); + new EncryptionKey(stack, 'MyKey', { retain: false }); - expect(app.synthesizeStack(stack.name)).to(exactlyMatchTemplate({ - Resources: { - MyKey6AB29FA6: { - Type: "AWS::KMS::Key", - Properties: { - KeyPolicy: { - Statement: [ - { - Action: [ - "kms:Create*", - "kms:Describe*", - "kms:Enable*", - "kms:List*", - "kms:Put*", - "kms:Update*", - "kms:Revoke*", - "kms:Disable*", - "kms:Get*", - "kms:Delete*", - "kms:ScheduleKeyDeletion", - "kms:CancelKeyDeletion" - ], - Effect: "Allow", - Principal: { - AWS: { - "Fn::Join": [ - "", - [ - "arn:", - { - Ref: "AWS::Partition" - }, - ":iam::", - { - Ref: "AWS::AccountId" - }, - ":root" - ] - ] - } - }, - Resource: "*" - } - ], - Version: "2012-10-17" - } - }, - DeletionPolicy: "Delete", - } - } - })); + expect(app.synthesizeStack(stack.name)).to(haveResource('AWS::KMS::Key', { DeletionPolicy: "Delete" }, ResourcePart.CompleteDefinition)); test.done(); }, From eb3d07cbcc260a1f17c85818afb18f3385b0f8a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=91=A8=F0=9F=8F=BC=E2=80=8D=F0=9F=92=BB=20Romain=20M?= =?UTF-8?q?arcadier-Muller?= Date: Fri, 8 Feb 2019 16:42:30 +0100 Subject: [PATCH 3/3] Fix merge artifact --- packages/@aws-cdk/aws-kms/lib/key.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index 2e444de50166e..9284a03aeccdc 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -107,11 +107,6 @@ export interface EncryptionKeyProps { */ policy?: PolicyDocument; - /** - * The AWS resource tags to associate with the KMS key. - */ - tags?: Tags; - /** * Whether the encryption key should be retained when it is removed from the Stack. This is useful when one wants to * retain access to data that was encrypted with a key that is being retired.