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 23, 2025
1 parent d6e3c61 commit 9cc1efe
Show file tree
Hide file tree
Showing 10 changed files with 275 additions and 72 deletions.
31 changes: 3 additions & 28 deletions packages/aws-cdk-lib/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, R
import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
import { CfnDBCluster, CfnDBClusterProps, CfnDBInstance } from './rds.generated';
import { ISubnetGroup, SubnetGroup } from './subnet-group';
import { validateDatabaseClusterProps } from './validate-database-cluster-props';
import * as cloudwatch from '../../aws-cloudwatch';
import * as ec2 from '../../aws-ec2';
import * as iam from '../../aws-iam';
Expand Down Expand Up @@ -830,36 +831,10 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
});
}

validateDatabaseClusterProps(this, props);

const enablePerformanceInsights = props.enablePerformanceInsights
|| props.performanceInsightRetention !== undefined || props.performanceInsightEncryptionKey !== undefined;
if (enablePerformanceInsights && props.enablePerformanceInsights === false) {
throw new ValidationError('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set', this);
}

if (props.clusterScalabilityType === ClusterScalabilityType.LIMITLESS || props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {
if (!props.enablePerformanceInsights) {
throw new ValidationError('Performance Insights must be enabled for Aurora Limitless Database.', this);
}
if (!props.performanceInsightRetention || props.performanceInsightRetention < PerformanceInsightRetention.MONTHS_1) {
throw new ValidationError('Performance Insights retention period must be set at least 31 days for Aurora Limitless Database.', this);
}
if (!props.monitoringInterval || !props.enableClusterLevelEnhancedMonitoring) {
throw new ValidationError('Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'.', this);
}
if (props.writer || props.readers) {
throw new ValidationError('Aurora Limitless Database does not support readers or writer instances.', this);
}
if (!props.engine.engineVersion?.fullVersion?.endsWith('limitless')) {
throw new ValidationError(`Aurora Limitless Database requires an engine version that supports it, got ${props.engine.engineVersion?.fullVersion}`, this);
}
if (props.storageType !== DBClusterStorageType.AURORA_IOPT1) {
throw new ValidationError(`Aurora Limitless Database requires I/O optimized storage type, got: ${props.storageType}`, this);
}
if (props.cloudwatchLogsExports === undefined || props.cloudwatchLogsExports.length === 0) {
throw new ValidationError('Aurora Limitless Database requires CloudWatch Logs exports to be set.', this);
}
}

this.performanceInsightsEnabled = enablePerformanceInsights;
this.performanceInsightRetention = enablePerformanceInsights
? (props.performanceInsightRetention || PerformanceInsightRetention.DEFAULT)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { Construct } from 'constructs';
import { ClusterScailabilityType, DatabaseCluster, DatabaseClusterProps, DBClusterStorageType } from './cluster';
import { PerformanceInsightRetention } from './props';
import { validateAllProps, ValidationRule } from '../../core/lib/helpers-internal';

const standardDatabaseRules: ValidationRule<DatabaseClusterProps>[] = [
{
condition: (props) => props.enablePerformanceInsights === false &&
(props.performanceInsightRetention !== undefined || props.performanceInsightEncryptionKey !== undefined),
message: () => '`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set',

},
];

const limitlessDatabaseRules: ValidationRule<DatabaseClusterProps>[] = [
{
condition: (props) => !props.enablePerformanceInsights,
message: () => 'Performance Insights must be enabled for Aurora Limitless Database',
},
{
condition: (props) => !props.performanceInsightRetention
|| props.performanceInsightRetention < PerformanceInsightRetention.MONTHS_1,
message: () => 'Performance Insights retention period must be set to at least 31 days for Aurora Limitless Database',
},
{
condition: (props) => !props.monitoringInterval || !props.enableClusterLevelEnhancedMonitoring,
message: () => 'Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'',
},
{
condition: (props) => !!(props.writer || props.readers),
message: () => 'Aurora Limitless Database does not support reader or writer instances',
},
{
condition: (props) => !props.engine.engineVersion?.fullVersion?.endsWith('limitless'),
message: (props) => `Aurora Limitless Database requires an engine version that supports it, got: ${props.engine.engineVersion?.fullVersion}`,
},
{
condition: (props) => props.storageType !== DBClusterStorageType.AURORA_IOPT1,
message: (props) => `Aurora Limitless Database requires I/O optimized storage type, got: ${props.storageType}`,
},
{
condition: (props) => props.cloudwatchLogsExports === undefined || props.cloudwatchLogsExports.length === 0,
message: () => 'Aurora Limitless Database requires CloudWatch Logs exports to be set',
},
];

export function validateDatabaseClusterProps(scope: Construct, props: DatabaseClusterProps): void {
const isLimitlessCluster = props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS;
const applicableRules = isLimitlessCluster
? [...standardDatabaseRules, ...limitlessDatabaseRules]
: standardDatabaseRules;

validateAllProps(scope, DatabaseCluster.name, props, applicableRules);
}
18 changes: 9 additions & 9 deletions packages/aws-cdk-lib/aws-rds/test/cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ describe('cluster new api', () => {
storageType: DBClusterStorageType.AURORA_IOPT1,
cloudwatchLogsExports: ['postgresql'],
});
}).toThrow('Performance Insights must be enabled for Aurora Limitless Database.');
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- Performance Insights must be enabled for Aurora Limitless Database\n- Performance Insights retention period must be set to at least 31 days for Aurora Limitless Database');
});

test('throw error for invalid performance insights retention period', () => {
Expand All @@ -292,7 +292,7 @@ describe('cluster new api', () => {
storageType: DBClusterStorageType.AURORA_IOPT1,
cloudwatchLogsExports: ['postgresql'],
});
}).toThrow('Performance Insights retention period must be set at least 31 days for Aurora Limitless Database.');
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- Performance Insights retention period must be set to at least 31 days for Aurora Limitless Database');
});

test('throw error for not specifying monitoring interval', () => {
Expand All @@ -316,7 +316,7 @@ describe('cluster new api', () => {
storageType: DBClusterStorageType.AURORA_IOPT1,
cloudwatchLogsExports: ['postgresql'],
});
}).toThrow('Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'.');
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'');
});

test.each([false, undefined])('throw error for configuring enhanced monitoring at the instance level', (enableClusterLevelEnhancedMonitoring) => {
Expand All @@ -341,7 +341,7 @@ describe('cluster new api', () => {
cloudwatchLogsExports: ['postgresql'],
instances: 1,
});
}).toThrow('Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'.');
}).toThrow('Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'');
});

test('throw error for specifying writer instance', () => {
Expand All @@ -366,7 +366,7 @@ describe('cluster new api', () => {
cloudwatchLogsExports: ['postgresql'],
writer: ClusterInstance.serverlessV2('writer'),
});
}).toThrow('Aurora Limitless Database does not support readers or writer instances.');
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- Aurora Limitless Database does not support reader or writer instances');
});

test.each([
Expand Down Expand Up @@ -395,7 +395,7 @@ describe('cluster new api', () => {
storageType: DBClusterStorageType.AURORA_IOPT1,
cloudwatchLogsExports: ['postgresql'],
});
}).toThrow(`Aurora Limitless Database requires an engine version that supports it, got ${engine.engineVersion?.fullVersion}`);
}).toThrow(`DatabaseCluster initialization failed due to the following validation error(s):\n- Aurora Limitless Database requires an engine version that supports it, got: ${engine.engineVersion?.fullVersion}`);
});

test('throw error for invalid storage type', () => {
Expand Down Expand Up @@ -443,7 +443,7 @@ describe('cluster new api', () => {
storageType: DBClusterStorageType.AURORA_IOPT1,
cloudwatchLogsExports,
});
}).toThrow('Aurora Limitless Database requires CloudWatch Logs exports to be set.');
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- Aurora Limitless Database requires CloudWatch Logs exports to be set');
});
});

Expand Down Expand Up @@ -2130,7 +2130,7 @@ describe('cluster', () => {
enablePerformanceInsights: false,
performanceInsightRetention: PerformanceInsightRetention.DEFAULT,
});
}).toThrow(/`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set/);
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- `enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set');
});

test('throws if performanceInsightEncryptionKey is set but performance insights is disabled', () => {
Expand All @@ -2142,7 +2142,7 @@ describe('cluster', () => {
enablePerformanceInsights: false,
performanceInsightRetention: PerformanceInsightRetention.DEFAULT,
});
}).toThrow(/`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set/);
}).toThrow('DatabaseCluster initialization failed due to the following validation error(s):\n- `enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set');
});

test('warn if performance insights is enabled at cluster level but disabled on writer and reader instances', () => {
Expand Down
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.

60 changes: 60 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,60 @@
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 !== RedrivePermission.BY_QUEUE && 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
1 change: 1 addition & 0 deletions packages/aws-cdk-lib/core/lib/helpers-internal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export * from './cfn-parse';
export { md5hash } from '../private/md5';
export * from './customize-roles';
export * from './string-specializer';
export * from './validate-all-props';
export { constructInfoFromConstruct, constructInfoFromStack } from '../private/runtime-info';
Loading

0 comments on commit 9cc1efe

Please sign in to comment.