From aa8484a154baea87f223c4b22135f3a845b836b3 Mon Sep 17 00:00:00 2001 From: yoshi Date: Sat, 2 Mar 2024 04:29:00 +0900 Subject: [PATCH] fix(sqs): `redrivePermission` is set to `byQueue` no matter what value is specified (#29130) ### Issue #29129 Closes #29129. ### Reason for this change When `redriveAllowPolicy.redrivePermission` is specified, any value will be output to template as `byQueue` ### Description of changes 1. Fix the evaluation order by enclosing the ternary operators in parentheses ```typescript ?? (props.redriveAllowPolicy.sourceQueues ? RedrivePermission.BY_QUEUE : RedrivePermission.ALLOW_ALL), ``` 2. Added a test case in `packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts` when redrivePermission is specified other than `BY_QUEUE`. 3. Added an integ test case in `packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.ts` ### Description of how you validated changes Added a test case in `packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts` when redrivePermission is specified other than `BY_QUEUE`. Added an integ test case in `packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.ts` And ran the test case. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-cdk-sqs.template.json | 10 +++++++ .../test/integ.sqs-source-queue-permission.ts | 12 +++++++-- packages/aws-cdk-lib/aws-sqs/lib/queue.ts | 2 +- packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts | 27 +++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.js.snapshot/aws-cdk-sqs.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.js.snapshot/aws-cdk-sqs.template.json index a116bcb48f618..a17bfbfefa270 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.js.snapshot/aws-cdk-sqs.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.js.snapshot/aws-cdk-sqs.template.json @@ -2,11 +2,21 @@ "Resources": { "SourceQueue1F4BBA4BB": { "Type": "AWS::SQS::Queue", + "Properties": { + "RedriveAllowPolicy": { + "redrivePermission": "allowAll" + } + }, "UpdateReplacePolicy": "Delete", "DeletionPolicy": "Delete" }, "SourceQueue22481CB5A": { "Type": "AWS::SQS::Queue", + "Properties": { + "RedriveAllowPolicy": { + "redrivePermission": "denyAll" + } + }, "UpdateReplacePolicy": "Delete", "DeletionPolicy": "Delete" }, diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.ts index e6079ba21f67e..3ff61390fd95d 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sqs/test/integ.sqs-source-queue-permission.ts @@ -7,8 +7,16 @@ const app = new App(); const stack = new Stack(app, 'aws-cdk-sqs'); -const sourceQueue1 = new Queue(stack, 'SourceQueue1'); -const sourceQueue2 = new Queue(stack, 'SourceQueue2'); +const sourceQueue1 = new Queue(stack, 'SourceQueue1', { + redriveAllowPolicy: { + redrivePermission: RedrivePermission.ALLOW_ALL, + }, +}); +const sourceQueue2 = new Queue(stack, 'SourceQueue2', { + redriveAllowPolicy: { + redrivePermission: RedrivePermission.DENY_ALL, + }, +}); new Queue(stack, 'DeadLetterQueue', { redriveAllowPolicy: { diff --git a/packages/aws-cdk-lib/aws-sqs/lib/queue.ts b/packages/aws-cdk-lib/aws-sqs/lib/queue.ts index c8a9c758978a9..f425536f89937 100644 --- a/packages/aws-cdk-lib/aws-sqs/lib/queue.ts +++ b/packages/aws-cdk-lib/aws-sqs/lib/queue.ts @@ -411,7 +411,7 @@ export class Queue extends QueueBase { redrivePermission: props.redriveAllowPolicy.redrivePermission // When `sourceQueues` is provided in `redriveAllowPolicy`, `redrivePermission` defaults to allow specified queues (`BY_QUEUE`); // otherwise, it defaults to allow all queues (`ALLOW_ALL`). - ?? props.redriveAllowPolicy.sourceQueues ? RedrivePermission.BY_QUEUE : RedrivePermission.ALLOW_ALL, + ?? (props.redriveAllowPolicy.sourceQueues ? RedrivePermission.BY_QUEUE : RedrivePermission.ALLOW_ALL), sourceQueueArns: props.redriveAllowPolicy.sourceQueues?.map(q => q.queueArn), } : undefined; diff --git a/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts b/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts index 956c8a77cdd7a..0ae7bc2919ef5 100644 --- a/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts +++ b/packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts @@ -753,6 +753,33 @@ describe('redriveAllowPolicy', () => { }); }); + test.each([ + [sqs.RedrivePermission.ALLOW_ALL, 'allowAll'], + [sqs.RedrivePermission.DENY_ALL, 'denyAll'], + ])('redrive permission can be set to %s', (permission, expected) => { + const stack = new Stack(); + new sqs.Queue(stack, 'Queue', { + redriveAllowPolicy: { + redrivePermission: permission, + }, + }); + + Template.fromStack(stack).templateMatches({ + 'Resources': { + 'Queue4A7E3555': { + 'Type': 'AWS::SQS::Queue', + 'Properties': { + 'RedriveAllowPolicy': { + 'redrivePermission': expected, + }, + }, + 'UpdateReplacePolicy': 'Delete', + 'DeletionPolicy': 'Delete', + }, + }, + }); + }); + test('explicit specification of dead letter source queues', () => { const stack = new Stack(); const sourceQueue1 = new sqs.Queue(stack, 'SourceQueue1');