Skip to content

Commit 7441418

Browse files
fix(s3-notifications): notifications allowed with imported kms keys (#18989)
---- fixes: #18988 If you add an sqs queue as notification target to an s3 bucket, and this sqs queue is encrypted with an imported kms IKey, the stack won't synthesize. Instead of failing, it should warn the user, that it can not ensure the correct kms key policy permissions. This fix will solve this. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 6ec1005 commit 7441418

File tree

4 files changed

+50
-3
lines changed

4 files changed

+50
-3
lines changed

packages/@aws-cdk/aws-s3-notifications/README.md

+12
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ const topic = new sns.Topic(this, 'Topic');
2626
bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SnsDestination(topic));
2727
```
2828

29+
The following example shows how to send a notification to an SQS queue
30+
when an object is created in an S3 bucket:
31+
32+
```ts
33+
import * as sqs from '@aws-cdk/aws-sqs';
34+
35+
const bucket = new s3.Bucket(this, 'Bucket');
36+
const queue = new sqs.Queue(this, 'Queue');
37+
38+
bucket.addEventNotification(s3.EventType.OBJECT_CREATED_PUT, new s3n.SqsDestination(queue));
39+
```
40+
2941
The following example shows how to send a notification to a Lambda function when an object is created in an S3 bucket:
3042

3143
```ts

packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts

+10-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import * as iam from '@aws-cdk/aws-iam';
22
import * as s3 from '@aws-cdk/aws-s3';
33
import * as sqs from '@aws-cdk/aws-sqs';
4+
import { Annotations } from '@aws-cdk/core';
5+
6+
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
7+
// eslint-disable-next-line no-duplicate-imports, import/order
48
import { Construct } from '@aws-cdk/core';
59

610
/**
@@ -24,11 +28,15 @@ export class SqsDestination implements s3.IBucketNotificationDestination {
2428
// if this queue is encrypted, we need to allow S3 to read messages since that's how
2529
// it verifies that the notification destination configuration is valid.
2630
if (this.queue.encryptionMasterKey) {
27-
this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement({
31+
const statement = new iam.PolicyStatement({
2832
principals: [new iam.ServicePrincipal('s3.amazonaws.com')],
2933
actions: ['kms:GenerateDataKey*', 'kms:Decrypt'],
3034
resources: ['*'],
31-
}), /* allowNoOp */ false);
35+
});
36+
const addResult = this.queue.encryptionMasterKey.addToResourcePolicy(statement, /* allowNoOp */ true);
37+
if (!addResult.statementAdded) {
38+
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)}`);
39+
}
3240
}
3341

3442
return {

packages/@aws-cdk/aws-s3-notifications/package.json

+2
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
},
8181
"dependencies": {
8282
"@aws-cdk/aws-iam": "0.0.0",
83+
"@aws-cdk/aws-kms": "0.0.0",
8384
"@aws-cdk/aws-lambda": "0.0.0",
8485
"@aws-cdk/aws-s3": "0.0.0",
8586
"@aws-cdk/aws-sns": "0.0.0",
@@ -90,6 +91,7 @@
9091
"homepage": "https://github.com/aws/aws-cdk",
9192
"peerDependencies": {
9293
"@aws-cdk/aws-iam": "0.0.0",
94+
"@aws-cdk/aws-kms": "0.0.0",
9395
"@aws-cdk/aws-lambda": "0.0.0",
9496
"@aws-cdk/aws-s3": "0.0.0",
9597
"@aws-cdk/aws-sns": "0.0.0",

packages/@aws-cdk/aws-s3-notifications/test/queue.test.ts

+26-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { Match, Template } from '@aws-cdk/assertions';
1+
import { Match, Template, Annotations } from '@aws-cdk/assertions';
2+
import * as kms from '@aws-cdk/aws-kms';
23
import * as s3 from '@aws-cdk/aws-s3';
34
import * as sqs from '@aws-cdk/aws-sqs';
45
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
9697
},
9798
});
9899
});
100+
101+
test('if the queue is encrypted with a imported kms key, printout warning', () => {
102+
const stack = new Stack();
103+
const bucket = new s3.Bucket(stack, 'Bucket');
104+
const key = kms.Key.fromKeyArn(stack, 'ImportedKey', 'arn:aws:kms:us-west-2:111122223333:key/1234abcd-12ab-34cd-56ef-1234567890ab');
105+
const queue = new sqs.Queue(stack, 'Queue', {
106+
encryption: sqs.QueueEncryption.KMS,
107+
encryptionMasterKey: key,
108+
});
109+
110+
bucket.addObjectCreatedNotification(new notif.SqsDestination(queue));
111+
112+
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({
113+
Action: [
114+
'kms:GenerateDataKey*',
115+
'kms:Decrypt',
116+
],
117+
Effect: 'Allow',
118+
Principal: {
119+
Service: 's3.amazonaws.com',
120+
},
121+
Resource: '*',
122+
}, null, 2)}`);
123+
});

0 commit comments

Comments
 (0)