Skip to content

Commit bf441fa

Browse files
authored
fix(sqs): encryptionType is incorrect when encryptionMasterKey is provided (#26886)
In #19796 we added additional validation to sns subscription. For that purpose the Queue `encryptionType` was exposed as a public property. However the PR forgot to take into account that the provided `encryption` property is automatically changed when a `encryptionMasterKey` is provided. This PR ensures that the public `encryptionType` has the correct value. Additionally, adds a warning for an incorrect configuration scenario where `encryptionMasterKey` is provided together with an `encryption` other than QueueEncryption.KMS. This feature was supposed to allow users to simply provide an encryption key and have the encryption type being selected automatically. However it now unintentionally allows for wrong configurations that are silently fixed, e.g. setting QueueEncryption.UNENCRYPTED and providing an encryption key. The warning keeps backwards compatibility, but instructs users to fix their configuration. Closes #26719 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent d152d61 commit bf441fa

File tree

3 files changed

+65
-5
lines changed

3 files changed

+65
-5
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { Template } from '../../assertions';
2+
import * as kms from '../../aws-kms';
3+
import * as sns from '../../aws-sns';
4+
import * as sqs from '../../aws-sqs';
5+
import { Stack } from '../../core';
6+
import * as subscriptions from '../lib';
7+
8+
test('can add subscription to queue that has encryptionType auto changed', () => {
9+
// GIVEN
10+
const stack = new Stack();
11+
const key = new kms.Key(stack, 'CustomKey');
12+
const queue = new sqs.Queue(stack, 'Queue', {
13+
encryption: sqs.QueueEncryption.KMS_MANAGED,
14+
encryptionMasterKey: key,
15+
});
16+
17+
const someTopic = new sns.Topic(stack, 'Topic');
18+
someTopic.addSubscription(
19+
new subscriptions.SqsSubscription(queue, {
20+
rawMessageDelivery: true,
21+
}),
22+
);
23+
24+
// THEN
25+
Template.fromStack(stack).hasResourceProperties('AWS::SNS::Subscription', {
26+
Endpoint: {
27+
'Fn::GetAtt': ['Queue4A7E3555', 'Arn'],
28+
},
29+
Protocol: 'sqs',
30+
});
31+
});

packages/aws-cdk-lib/aws-sqs/lib/queue.ts

+20-5
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { CfnQueue } from './sqs.generated';
44
import { validateProps } from './validate-props';
55
import * as iam from '../../aws-iam';
66
import * as kms from '../../aws-kms';
7-
import { Duration, RemovalPolicy, Stack, Token, ArnFormat } from '../../core';
7+
import { Duration, RemovalPolicy, Stack, Token, ArnFormat, Annotations } from '../../core';
88

99
/**
1010
* Properties for creating a new Queue
@@ -336,7 +336,7 @@ export class Queue extends QueueBase {
336336
}
337337
: undefined;
338338

339-
const { encryptionMasterKey, encryptionProps } = _determineEncryptionProps.call(this);
339+
const { encryptionMasterKey, encryptionProps, encryptionType } = _determineEncryptionProps.call(this);
340340

341341
const fifoProps = this.determineFifoProps(props);
342342
this.fifo = fifoProps.fifoQueue || false;
@@ -362,25 +362,37 @@ export class Queue extends QueueBase {
362362
this.encryptionMasterKey = encryptionMasterKey;
363363
this.queueUrl = queue.ref;
364364
this.deadLetterQueue = props.deadLetterQueue;
365-
this.encryptionType = props.encryption;
365+
this.encryptionType = encryptionType;
366366

367-
function _determineEncryptionProps(this: Queue): { encryptionProps: EncryptionProps, encryptionMasterKey?: kms.IKey } {
367+
function _determineEncryptionProps(this: Queue): {
368+
encryptionProps: EncryptionProps,
369+
encryptionMasterKey?: kms.IKey,
370+
encryptionType: QueueEncryption | undefined
371+
} {
368372
let encryption = props.encryption;
369373

370374
if (encryption === QueueEncryption.SQS_MANAGED && props.encryptionMasterKey) {
371375
throw new Error("'encryptionMasterKey' is not supported if encryption type 'SQS_MANAGED' is used");
372376
}
373377

374378
if (encryption !== QueueEncryption.KMS && props.encryptionMasterKey) {
379+
if (encryption !== undefined) {
380+
Annotations.of(this).addWarningV2('@aws-cdk/aws-sqs:queueEncryptionChangedToKMS', [
381+
`encryption: Automatically changed to QueueEncryption.KMS, was: QueueEncryption.${Object.keys(QueueEncryption)[Object.values(QueueEncryption).indexOf(encryption)]}`,
382+
'When encryptionMasterKey is provided, always set `encryption: QueueEncryption.KMS`',
383+
].join('\n'));
384+
}
385+
375386
encryption = QueueEncryption.KMS; // KMS is implied by specifying an encryption key
376387
}
377388

378389
if (!encryption) {
379-
return { encryptionProps: {} };
390+
return { encryptionProps: {}, encryptionType: encryption };
380391
}
381392

382393
if (encryption === QueueEncryption.UNENCRYPTED) {
383394
return {
395+
encryptionType: encryption,
384396
encryptionProps: {
385397
sqsManagedSseEnabled: false,
386398
},
@@ -389,6 +401,7 @@ export class Queue extends QueueBase {
389401

390402
if (encryption === QueueEncryption.KMS_MANAGED) {
391403
return {
404+
encryptionType: encryption,
392405
encryptionProps: {
393406
kmsMasterKeyId: 'alias/aws/sqs',
394407
kmsDataKeyReusePeriodSeconds: props.dataKeyReuse && props.dataKeyReuse.toSeconds(),
@@ -402,6 +415,7 @@ export class Queue extends QueueBase {
402415
});
403416

404417
return {
418+
encryptionType: encryption,
405419
encryptionMasterKey: masterKey,
406420
encryptionProps: {
407421
kmsMasterKeyId: masterKey.keyArn,
@@ -412,6 +426,7 @@ export class Queue extends QueueBase {
412426

413427
if (encryption === QueueEncryption.SQS_MANAGED) {
414428
return {
429+
encryptionType: encryption,
415430
encryptionProps: {
416431
sqsManagedSseEnabled: true,
417432
},

packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts

+14
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ describe('queue encryption', () => {
358358
const queue = new sqs.Queue(stack, 'Queue', { encryptionMasterKey: key });
359359

360360
expect(queue.encryptionMasterKey).toEqual(key);
361+
expect(queue.encryptionType).toEqual(sqs.QueueEncryption.KMS);
361362
Template.fromStack(stack).hasResourceProperties('AWS::SQS::Queue', {
362363
'KmsMasterKeyId': { 'Fn::GetAtt': ['CustomKey1E6D0D07', 'Arn'] },
363364
});
@@ -492,6 +493,19 @@ describe('queue encryption', () => {
492493
encryptionMasterKey: key,
493494
})).toThrow(/'encryptionMasterKey' is not supported if encryption type 'SQS_MANAGED' is used/);
494495
});
496+
497+
test('encryptionType is always KMS, when an encryptionMasterKey is provided', () => {
498+
// GIVEN
499+
const stack = new Stack();
500+
const key = new kms.Key(stack, 'CustomKey');
501+
const queue = new sqs.Queue(stack, 'Queue', {
502+
encryption: sqs.QueueEncryption.KMS_MANAGED,
503+
encryptionMasterKey: key,
504+
});
505+
506+
// THEN
507+
expect(queue.encryptionType).toBe(sqs.QueueEncryption.KMS);
508+
});
495509
});
496510

497511
describe('encryption in transit', () => {

0 commit comments

Comments
 (0)