Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(validation): shared interface to notify user of all validation issues when initializing a new RDS DB Cluster or SQS Queue #32841

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -829,36 +830,10 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
});
}

validateDatabaseClusterProps(scope, props);

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

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

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 { validateProps, 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;

validateProps(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
4 changes: 2 additions & 2 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 } 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 @@ -383,7 +383,7 @@ export class Queue extends QueueBase {
physicalName: props.queueName,
});

validateProps(props);
validateQueueProps(scope, props);

if (props.redriveAllowPolicy) {
const { redrivePermission, sourceQueues } = props.redriveAllowPolicy;
Expand Down
18 changes: 0 additions & 18 deletions packages/aws-cdk-lib/aws-sqs/lib/validate-props.ts

This file was deleted.

39 changes: 39 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,39 @@
import { Construct } from 'constructs';
import { Queue, QueueProps } from './index';
import { Token } from '../../core';
import { validateProps, 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`,
},
];

export function validateQueueProps(scope: Construct, props: QueueProps) {
validateProps(scope, Queue.name, props, queueValidationRules);
}
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-props';
export { constructInfoFromConstruct, constructInfoFromStack } from '../private/runtime-info';
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { Construct } from 'constructs';
import { ValidationError } from '../errors';

/**
* Represents a validation rule for props of type T.
* @template T The type of the props being validated.
*/
export type ValidationRule<T> = {
/**
* A function that checks if the validation rule condition is met.
* @param {T} props - The props to validate.
* @returns {boolean} True if the condition is met (i.e., validation fails), false otherwise.
*/
condition: (props: T) => boolean;

/**
* A function that returns an error message if the validation fails.
* @param {T} props - The props that failed validation.
* @returns {string} The error message.
*/
message: (props: T) => string;
};

/**
* Validates props against a set of rules and throws an error if any validations fail.
*
* @template T The type of the props being validated.
* @param {string} className - The name of the class being validated, used in the error message.
* @param {T} props - The props to validate.
* @param {ValidationRule<T>[]} rules - An array of validation rules to apply.
* @throws {Error} If any validation rules fail, with a message detailing all failures.
*/
export function validateProps<T>(scope: Construct, className: string, props: T, rules: ValidationRule<T>[]): void {
const validationErrors = rules
.filter(rule => rule.condition(props))
.map(rule => rule.message(props));

if (validationErrors.length > 0) {
const errorMessage = `${className} initialization failed due to the following validation error(s):\n${validationErrors.map(error => `- ${error}`).join('\n')}`;
throw new ValidationError(errorMessage, scope);
}
}
Loading
Loading