Skip to content

Commit

Permalink
all sqs props are validated at initialization, instead of failing fast
Browse files Browse the repository at this point in the history
  • Loading branch information
iankhou committed Jan 22, 2025
1 parent c517547 commit 1d716cb
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 35 deletions.
16 changes: 3 additions & 13 deletions packages/aws-cdk-lib/aws-sqs/lib/queue.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Construct } from 'constructs';
import { IQueue, QueueAttributes, QueueBase, QueueEncryption } from './queue-base';
import { CfnQueue } from './sqs.generated';
import { validateProps } from './validate-props';
import { validateQueueProps, validateRedrivePolicy } from './validate-queue-props';
import * as iam from '../../aws-iam';
import * as kms from '../../aws-kms';
import { Duration, RemovalPolicy, Stack, Token, ArnFormat, Annotations } from '../../core';
Expand Down Expand Up @@ -384,20 +384,10 @@ export class Queue extends QueueBase {
physicalName: props.queueName,
});

validateProps(this, props);
validateQueueProps(this, props);

if (props.redriveAllowPolicy) {
const { redrivePermission, sourceQueues } = props.redriveAllowPolicy;
if (redrivePermission === RedrivePermission.BY_QUEUE) {
if (!sourceQueues || sourceQueues.length === 0) {
throw new ValidationError('At least one source queue must be specified when RedrivePermission is set to \'byQueue\'', this);
}
if (sourceQueues && sourceQueues.length > 10) {
throw new ValidationError('Up to 10 sourceQueues can be specified. Set RedrivePermission to \'allowAll\' to specify more', this);
}
} else if (redrivePermission && sourceQueues) {
throw new ValidationError('sourceQueues cannot be configured when RedrivePermission is set to \'allowAll\' or \'denyAll\'', this);
}
validateRedrivePolicy(this, props.redriveAllowPolicy);
}

const redrivePolicy = props.deadLetterQueue
Expand Down
20 changes: 0 additions & 20 deletions packages/aws-cdk-lib/aws-sqs/lib/validate-props.ts

This file was deleted.

59 changes: 59 additions & 0 deletions packages/aws-cdk-lib/aws-sqs/lib/validate-queue-props.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { Construct } from 'constructs';
import { Queue, QueueProps, RedriveAllowPolicy, RedrivePermission } from './index';
import { Token } from '../../core';
import { validateAllProps, ValidationRule } from '../../core/lib/helpers-internal';

function validateRange(value: number | undefined, minValue: number, maxValue: number): boolean {
return value !== undefined && !Token.isUnresolved(value) && (value < minValue || value > maxValue);
}

const queueValidationRules: ValidationRule<QueueProps>[] = [
{
condition: (props) => validateRange(props.deliveryDelay?.toSeconds(), 0, 900),
message: (props) => `delivery delay must be between 0 and 900 seconds, but ${props.deliveryDelay?.toSeconds()} was provided`,
},
{
condition: (props) => validateRange(props.maxMessageSizeBytes, 1_024, 262_144),
message: (props) => `maximum message size must be between 1,024 and 262,144 bytes, but ${props.maxMessageSizeBytes} was provided`,
},
{
condition: (props) => validateRange(props.retentionPeriod?.toSeconds(), 60, 1_209_600),
message: (props) => `message retention period must be between 60 and 1,209,600 seconds, but ${props.retentionPeriod?.toSeconds()} was provided`,
},
{
condition: (props) => validateRange(props.receiveMessageWaitTime?.toSeconds(), 0, 20),
message: (props) => `receive wait time must be between 0 and 20 seconds, but ${props.receiveMessageWaitTime?.toSeconds()} was provided`,
},
{
condition: (props) => validateRange(props.visibilityTimeout?.toSeconds(), 0, 43_200),
message: (props) => `visibility timeout must be between 0 and 43,200 seconds, but ${props.visibilityTimeout?.toSeconds()} was provided`,
},
{
condition: (props) => validateRange(props.deadLetterQueue?.maxReceiveCount, 1, Number.MAX_SAFE_INTEGER),
message: (props) => `dead letter target maximum receive count must be 1 or more, but ${props.deadLetterQueue?.maxReceiveCount} was provided`,
},
];
const redriveValidationRules: ValidationRule<RedriveAllowPolicy>[] = [
{
condition: ({ redrivePermission, sourceQueues }) =>
redrivePermission === RedrivePermission.BY_QUEUE && (!sourceQueues || sourceQueues.length === 0),
message: () => 'At least one source queue must be specified when RedrivePermission is set to \'byQueue\'',
},
{
condition: ({ redrivePermission, sourceQueues }) =>
!!(redrivePermission === RedrivePermission.BY_QUEUE && sourceQueues && sourceQueues.length > 10),
message: () => 'Up to 10 sourceQueues can be specified. Set RedrivePermission to \'allowAll\' to specify more',
},
{
condition: ({ redrivePermission, sourceQueues }) => !!(redrivePermission && sourceQueues),
message: () => 'sourceQueues cannot be configured when RedrivePermission is set to \'allowAll\' or \'denyAll\'',
},
];

export function validateQueueProps(scope: Construct, props: QueueProps) {
validateAllProps(scope, Queue.name, props, queueValidationRules);
}

export function validateRedrivePolicy(scope: Construct, policy: RedriveAllowPolicy) {
validateAllProps(scope, Queue.name, policy, redriveValidationRules);
}
15 changes: 13 additions & 2 deletions packages/aws-cdk-lib/aws-sqs/test/sqs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,29 @@ test('with a dead letter queue', () => {
expect(queue.deadLetterQueue).toEqual(dlqProps);
});

test('multiple prop validation errors are presented to the user (out-of-range retentionPeriod and deliveryDelay)', () => {
// GIVEN
const stack = new Stack();

// THEN
expect(() => new sqs.Queue(stack, 'MyQueue', {
retentionPeriod: Duration.seconds(30),
deliveryDelay: Duration.minutes(16),
})).toThrow('Queue initialization failed due to the following validation error(s):\n- delivery delay must be between 0 and 900 seconds, but 960 was provided\n- message retention period must be between 60 and 1,209,600 seconds, but 30 was provided');
});

test('message retention period must be between 1 minute to 14 days', () => {
// GIVEN
const stack = new Stack();

// THEN
expect(() => new sqs.Queue(stack, 'MyQueue', {
retentionPeriod: Duration.seconds(30),
})).toThrow(/message retention period must be 60 seconds or more/);
})).toThrow('Queue initialization failed due to the following validation error(s):\n- message retention period must be between 60 and 1,209,600 seconds, but 30 was provided');

expect(() => new sqs.Queue(stack, 'AnotherQueue', {
retentionPeriod: Duration.days(15),
})).toThrow(/message retention period must be 1209600 seconds or less/);
})).toThrow('Queue initialization failed due to the following validation error(s):\n- message retention period must be between 60 and 1,209,600 seconds, but 1296000 was provided');
});

test('message retention period can be provided as a parameter', () => {
Expand Down

0 comments on commit 1d716cb

Please sign in to comment.