From 7db9ee06cacbea9a5b2122f04669062790e11ac0 Mon Sep 17 00:00:00 2001 From: Markus Date: Wed, 16 Feb 2022 13:59:38 +0100 Subject: [PATCH 1/7] fix: notifications allowed with imported kms keys --- .../@aws-cdk/aws-s3-notifications/README.md | 13 +++++++++ .../@aws-cdk/aws-s3-notifications/lib/sqs.ts | 14 ++++++++-- .../aws-s3-notifications/package.json | 2 ++ .../aws-s3-notifications/test/queue.test.ts | 27 ++++++++++++++++++- 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/README.md b/packages/@aws-cdk/aws-s3-notifications/README.md index 0b57126001cf8..edf449ebb53b5 100644 --- a/packages/@aws-cdk/aws-s3-notifications/README.md +++ b/packages/@aws-cdk/aws-s3-notifications/README.md @@ -26,6 +26,19 @@ const topic = new sns.Topic(this, 'Topic'); bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SnsDestination(topic)); ``` +The following example shows how to send a notification to an SQS queue +when an object is created in an S3 bucket: + +```ts +import * as sqs from '@aws-cdk/aws-sqs'; + +const bucket = new s3.Bucket(this, 'Bucket'); +const queue = new sqs.Queue(stack, 'Queue'); + + +bucket.addObjectCreatedNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(topic)); +``` + The following example shows how to send a notification to a Lambda function when an object is created in an S3 bucket: ```ts diff --git a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts index 330a941a9ae86..7cfe85ffd1748 100644 --- a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts +++ b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts @@ -1,6 +1,11 @@ import * as iam from '@aws-cdk/aws-iam'; +import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import * as sqs from '@aws-cdk/aws-sqs'; +import { Annotations } from '@aws-cdk/core'; + +// keep this import separate from other imports to reduce chance for merge conflicts with v2-main +// eslint-disable-next-line no-duplicate-imports, import/order import { Construct } from '@aws-cdk/core'; /** @@ -24,11 +29,16 @@ export class SqsDestination implements s3.IBucketNotificationDestination { // if this queue is encrypted, we need to allow S3 to read messages since that's how // it verifies that the notification destination configuration is valid. if (this.queue.encryptionMasterKey) { - this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement({ + const statement = new iam.PolicyStatement({ principals: [new iam.ServicePrincipal('s3.amazonaws.com')], actions: ['kms:GenerateDataKey*', 'kms:Decrypt'], resources: ['*'], - }), /* allowNoOp */ false); + }); + if (this.queue.encryptionMasterKey instanceof kms.Key) { + this.queue.encryptionMasterKey.addToResourcePolicy(statement, /* allowNoOp */ false); + } else { + Annotations.of(this.queue.encryptionMasterKey).addWarning(`Can not change key policy of imported kms key. Ensure hat your key policy contains the following permissions: \n${JSON.stringify(statement.toJSON(), null, 2)}`); + } } return { diff --git a/packages/@aws-cdk/aws-s3-notifications/package.json b/packages/@aws-cdk/aws-s3-notifications/package.json index 966572d8350ea..3b004ef015a21 100644 --- a/packages/@aws-cdk/aws-s3-notifications/package.json +++ b/packages/@aws-cdk/aws-s3-notifications/package.json @@ -80,6 +80,7 @@ }, "dependencies": { "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", "@aws-cdk/aws-s3": "0.0.0", "@aws-cdk/aws-sns": "0.0.0", @@ -90,6 +91,7 @@ "homepage": "https://github.com/aws/aws-cdk", "peerDependencies": { "@aws-cdk/aws-iam": "0.0.0", + "@aws-cdk/aws-kms": "0.0.0", "@aws-cdk/aws-lambda": "0.0.0", "@aws-cdk/aws-s3": "0.0.0", "@aws-cdk/aws-sns": "0.0.0", diff --git a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts index b1ae15e895a7c..e99a138b61bd0 100644 --- a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts +++ b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts @@ -1,4 +1,5 @@ -import { Match, Template } from '@aws-cdk/assertions'; +import { Match, Template, Annotations } from '@aws-cdk/assertions'; +import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import * as sqs from '@aws-cdk/aws-sqs'; import { Stack } from '@aws-cdk/core'; @@ -96,3 +97,27 @@ test('if the queue is encrypted with a custom kms key, the key resource policy i }, }); }); + +test('if the queue is encrypted with a imported kms key, printout warning', () => { + const stack = new Stack(); + const bucket = new s3.Bucket(stack, 'Bucket'); + const key = kms.Key.fromKeyArn(stack, 'ImportedKey', 'arn:aws:kms:us-west-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab'); + const queue = new sqs.Queue(stack, 'Queue', { + encryption: sqs.QueueEncryption.KMS, + encryptionMasterKey: key, + }); + + bucket.addObjectCreatedNotification(new notif.SqsDestination(queue)); + + Annotations.fromStack(stack).hasWarning('/Default/ImportedKey', `Can not change key policy of imported kms key. Ensure hat your key policy contains the following permissions: \n${JSON.stringify({ + Action: [ + 'kms:GenerateDataKey*', + 'kms:Decrypt', + ], + Effect: 'Allow', + Principal: { + Service: 's3.amazonaws.com', + }, + Resource: '*', + }, null, 2)}`); +}); From 42a4fcf2073a7b67e3c9511606db707ac28a5d51 Mon Sep 17 00:00:00 2001 From: markussiebert Date: Wed, 16 Feb 2022 14:49:36 +0100 Subject: [PATCH 2/7] Update README.md --- packages/@aws-cdk/aws-s3-notifications/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/README.md b/packages/@aws-cdk/aws-s3-notifications/README.md index edf449ebb53b5..7bd4d6c2c090b 100644 --- a/packages/@aws-cdk/aws-s3-notifications/README.md +++ b/packages/@aws-cdk/aws-s3-notifications/README.md @@ -36,7 +36,7 @@ const bucket = new s3.Bucket(this, 'Bucket'); const queue = new sqs.Queue(stack, 'Queue'); -bucket.addObjectCreatedNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(topic)); +bucket.addObjectCreatedNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(queue)); ``` The following example shows how to send a notification to a Lambda function when an object is created in an S3 bucket: From 215c4817766260608f1b18605914006e9dd09476 Mon Sep 17 00:00:00 2001 From: markussiebert Date: Wed, 16 Feb 2022 16:23:51 +0100 Subject: [PATCH 3/7] Update README.md --- packages/@aws-cdk/aws-s3-notifications/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/README.md b/packages/@aws-cdk/aws-s3-notifications/README.md index 7bd4d6c2c090b..3087a25c946cf 100644 --- a/packages/@aws-cdk/aws-s3-notifications/README.md +++ b/packages/@aws-cdk/aws-s3-notifications/README.md @@ -35,8 +35,7 @@ import * as sqs from '@aws-cdk/aws-sqs'; const bucket = new s3.Bucket(this, 'Bucket'); const queue = new sqs.Queue(stack, 'Queue'); - -bucket.addObjectCreatedNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(queue)); +bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(queue)); ``` The following example shows how to send a notification to a Lambda function when an object is created in an S3 bucket: From 781f3e3335b2f0ea691ed74a69d29f6033859af7 Mon Sep 17 00:00:00 2001 From: Markus Date: Wed, 16 Feb 2022 19:03:10 +0100 Subject: [PATCH 4/7] fix: example --- packages/@aws-cdk/aws-s3-notifications/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/README.md b/packages/@aws-cdk/aws-s3-notifications/README.md index 3087a25c946cf..75fba9152e2b8 100644 --- a/packages/@aws-cdk/aws-s3-notifications/README.md +++ b/packages/@aws-cdk/aws-s3-notifications/README.md @@ -33,7 +33,7 @@ when an object is created in an S3 bucket: import * as sqs from '@aws-cdk/aws-sqs'; const bucket = new s3.Bucket(this, 'Bucket'); -const queue = new sqs.Queue(stack, 'Queue'); +const queue = new sqs.Queue(this, 'Queue'); bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(queue)); ``` From 875b25191903280d01ed8c2c091a918edda6ada4 Mon Sep 17 00:00:00 2001 From: markussiebert Date: Thu, 17 Feb 2022 08:53:50 +0100 Subject: [PATCH 5/7] Update queue.test.ts --- packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts index e99a138b61bd0..44404de1b1d79 100644 --- a/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts +++ b/packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts @@ -109,7 +109,7 @@ test('if the queue is encrypted with a imported kms key, printout warning', () = bucket.addObjectCreatedNotification(new notif.SqsDestination(queue)); - Annotations.fromStack(stack).hasWarning('/Default/ImportedKey', `Can not change key policy of imported kms key. Ensure hat your key policy contains the following permissions: \n${JSON.stringify({ + Annotations.fromStack(stack).hasWarning('/Default/ImportedKey', `Can not change key policy of imported kms key. Ensure that your key policy contains the following permissions: \n${JSON.stringify({ Action: [ 'kms:GenerateDataKey*', 'kms:Decrypt', From a47d8d382caf1068f78828fd73d97cf9b2c05650 Mon Sep 17 00:00:00 2001 From: markussiebert Date: Thu, 17 Feb 2022 08:54:03 +0100 Subject: [PATCH 6/7] Update sqs.ts --- packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts index 7cfe85ffd1748..ac27c01a54693 100644 --- a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts +++ b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts @@ -37,7 +37,7 @@ export class SqsDestination implements s3.IBucketNotificationDestination { if (this.queue.encryptionMasterKey instanceof kms.Key) { this.queue.encryptionMasterKey.addToResourcePolicy(statement, /* allowNoOp */ false); } else { - Annotations.of(this.queue.encryptionMasterKey).addWarning(`Can not change key policy of imported kms key. Ensure hat your key policy contains the following permissions: \n${JSON.stringify(statement.toJSON(), null, 2)}`); + Annotations.of(this.queue.encryptionMasterKey).addWarning(`Can not change key policy of imported kms key. Ensure that your key policy contains the following permissions: \n${JSON.stringify(statement.toJSON(), null, 2)}`); } } From b66baa2e8de033f7facfcfdaa9ef25ca3a6c86d5 Mon Sep 17 00:00:00 2001 From: Markus Date: Fri, 18 Feb 2022 12:39:19 +0100 Subject: [PATCH 7/7] feat: small improvement --- packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts index ac27c01a54693..5562a07c5182a 100644 --- a/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts +++ b/packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts @@ -1,5 +1,4 @@ import * as iam from '@aws-cdk/aws-iam'; -import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import * as sqs from '@aws-cdk/aws-sqs'; import { Annotations } from '@aws-cdk/core'; @@ -34,9 +33,8 @@ export class SqsDestination implements s3.IBucketNotificationDestination { actions: ['kms:GenerateDataKey*', 'kms:Decrypt'], resources: ['*'], }); - if (this.queue.encryptionMasterKey instanceof kms.Key) { - this.queue.encryptionMasterKey.addToResourcePolicy(statement, /* allowNoOp */ false); - } else { + const addResult = this.queue.encryptionMasterKey.addToResourcePolicy(statement, /* allowNoOp */ true); + if (!addResult.statementAdded) { Annotations.of(this.queue.encryptionMasterKey).addWarning(`Can not change key policy of imported kms key. Ensure that your key policy contains the following permissions: \n${JSON.stringify(statement.toJSON(), null, 2)}`); } }