Skip to content

Commit

Permalink
fix(sqs): redrivePermission is set to byQueue no matter what valu…
Browse files Browse the repository at this point in the history
…e 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*
  • Loading branch information
yoshi-d-24 committed Mar 1, 2024
1 parent 9156b13 commit aa8484a
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 3 deletions.
Expand Up @@ -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"
},
Expand Down
Expand Up @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-sqs/lib/queue.ts
Expand Up @@ -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;

Expand Down
27 changes: 27 additions & 0 deletions packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts
Expand Up @@ -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');
Expand Down

0 comments on commit aa8484a

Please sign in to comment.