Skip to content

Commit 344d916

Browse files
authored
fix(rds): does not print all failed validations for DatabaseCluster props (#32841)
### Issue #32840 Closes #32840 ### Reason for this change When initializing a new RDS DB Cluster, the [current implementation](https://github.com/aws/aws-cdk/pull/32151/files#diff-49b4a9e1bf0b7db3ab71f4f08580da0cb2191d84605dc82a70c324bd122d5cf7R805-R828) fails on the first validation error, making it possible for the user to encounter another failure after fixing known validation issues. ### Description of changes Implemented a [validation function](https://github.com/aws/aws-cdk/pull/32841/files#diff-5d08d37e744e173239879212c59fd45cb9a279349f3dfb1c66923cb015ed3a3a) that collects all validation errors and presents them to the user. Used this function in RDS Database Cluster initialization. I will implement this separately for SQS Queue initialization as a POC for usability. There are several other places that can make use of this shared code, to show users all validation errors at once. Here's a non-exhaustive list: - [aws-ec2](https://github.com/aws/aws-cdk/blob/3e4f3773bfa48b75bf0adc7d53d46bbec7714a9e/packages/aws-cdk-lib/aws-ec2/lib/volume.ts#L672-L743) - [eventbridge-scheduler](https://github.com/aws/aws-cdk/blob/3e4f3773bfa48b75bf0adc7d53d46bbec7714a9e/packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/eventbridge-scheduler/create-schedule.ts#L324-L362) - [aws-fsx](https://github.com/aws/aws-cdk/blob/3e4f3773bfa48b75bf0adc7d53d46bbec7714a9e/packages/aws-cdk-lib/aws-fsx/lib/lustre-file-system.ts#L360-L380) ### Describe any new or updated permissions being added No permissions changes. ### Description of how you validated changes Added unit tests and modified existing unit tests. <img width="698" alt="Screenshot 2025-01-16 at 14 51 47" src="https://github.com/user-attachments/assets/a724a16a-7ccc-43b6-8fee-599ec007566d" /> ### 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*
1 parent ac90399 commit 344d916

File tree

6 files changed

+199
-37
lines changed

6 files changed

+199
-37
lines changed

packages/aws-cdk-lib/aws-rds/lib/cluster.ts

+3-28
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { BackupProps, Credentials, InstanceProps, PerformanceInsightRetention, R
1111
import { DatabaseProxy, DatabaseProxyOptions, ProxyTarget } from './proxy';
1212
import { CfnDBCluster, CfnDBClusterProps, CfnDBInstance } from './rds.generated';
1313
import { ISubnetGroup, SubnetGroup } from './subnet-group';
14+
import { validateDatabaseClusterProps } from './validate-database-cluster-props';
1415
import * as cloudwatch from '../../aws-cloudwatch';
1516
import * as ec2 from '../../aws-ec2';
1617
import * as iam from '../../aws-iam';
@@ -830,36 +831,10 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase {
830831
});
831832
}
832833

834+
validateDatabaseClusterProps(this, props);
835+
833836
const enablePerformanceInsights = props.enablePerformanceInsights
834837
|| props.performanceInsightRetention !== undefined || props.performanceInsightEncryptionKey !== undefined;
835-
if (enablePerformanceInsights && props.enablePerformanceInsights === false) {
836-
throw new ValidationError('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set', this);
837-
}
838-
839-
if (props.clusterScalabilityType === ClusterScalabilityType.LIMITLESS || props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS) {
840-
if (!props.enablePerformanceInsights) {
841-
throw new ValidationError('Performance Insights must be enabled for Aurora Limitless Database.', this);
842-
}
843-
if (!props.performanceInsightRetention || props.performanceInsightRetention < PerformanceInsightRetention.MONTHS_1) {
844-
throw new ValidationError('Performance Insights retention period must be set at least 31 days for Aurora Limitless Database.', this);
845-
}
846-
if (!props.monitoringInterval || !props.enableClusterLevelEnhancedMonitoring) {
847-
throw new ValidationError('Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'.', this);
848-
}
849-
if (props.writer || props.readers) {
850-
throw new ValidationError('Aurora Limitless Database does not support readers or writer instances.', this);
851-
}
852-
if (!props.engine.engineVersion?.fullVersion?.endsWith('limitless')) {
853-
throw new ValidationError(`Aurora Limitless Database requires an engine version that supports it, got ${props.engine.engineVersion?.fullVersion}`, this);
854-
}
855-
if (props.storageType !== DBClusterStorageType.AURORA_IOPT1) {
856-
throw new ValidationError(`Aurora Limitless Database requires I/O optimized storage type, got: ${props.storageType}`, this);
857-
}
858-
if (props.cloudwatchLogsExports === undefined || props.cloudwatchLogsExports.length === 0) {
859-
throw new ValidationError('Aurora Limitless Database requires CloudWatch Logs exports to be set.', this);
860-
}
861-
}
862-
863838
this.performanceInsightsEnabled = enablePerformanceInsights;
864839
this.performanceInsightRetention = enablePerformanceInsights
865840
? (props.performanceInsightRetention || PerformanceInsightRetention.DEFAULT)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { Construct } from 'constructs';
2+
import { ClusterScailabilityType, DatabaseCluster, DatabaseClusterProps, DBClusterStorageType } from './cluster';
3+
import { PerformanceInsightRetention } from './props';
4+
import { validateAllProps, ValidationRule } from '../../core/lib/helpers-internal';
5+
6+
const standardDatabaseRules: ValidationRule<DatabaseClusterProps>[] = [
7+
{
8+
condition: (props) => props.enablePerformanceInsights === false &&
9+
(props.performanceInsightRetention !== undefined || props.performanceInsightEncryptionKey !== undefined),
10+
message: () => '`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set',
11+
12+
},
13+
];
14+
15+
const limitlessDatabaseRules: ValidationRule<DatabaseClusterProps>[] = [
16+
{
17+
condition: (props) => !props.enablePerformanceInsights,
18+
message: () => 'Performance Insights must be enabled for Aurora Limitless Database',
19+
},
20+
{
21+
condition: (props) => !props.performanceInsightRetention
22+
|| props.performanceInsightRetention < PerformanceInsightRetention.MONTHS_1,
23+
message: () => 'Performance Insights retention period must be set to at least 31 days for Aurora Limitless Database',
24+
},
25+
{
26+
condition: (props) => !props.monitoringInterval || !props.enableClusterLevelEnhancedMonitoring,
27+
message: () => 'Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'',
28+
},
29+
{
30+
condition: (props) => !!(props.writer || props.readers),
31+
message: () => 'Aurora Limitless Database does not support reader or writer instances',
32+
},
33+
{
34+
condition: (props) => !props.engine.engineVersion?.fullVersion?.endsWith('limitless'),
35+
message: (props) => `Aurora Limitless Database requires an engine version that supports it, got: ${props.engine.engineVersion?.fullVersion}`,
36+
},
37+
{
38+
condition: (props) => props.storageType !== DBClusterStorageType.AURORA_IOPT1,
39+
message: (props) => `Aurora Limitless Database requires I/O optimized storage type, got: ${props.storageType}`,
40+
},
41+
{
42+
condition: (props) => props.cloudwatchLogsExports === undefined || props.cloudwatchLogsExports.length === 0,
43+
message: () => 'Aurora Limitless Database requires CloudWatch Logs exports to be set',
44+
},
45+
];
46+
47+
export function validateDatabaseClusterProps(scope: Construct, props: DatabaseClusterProps): void {
48+
const isLimitlessCluster = props.clusterScailabilityType === ClusterScailabilityType.LIMITLESS;
49+
const applicableRules = isLimitlessCluster
50+
? [...standardDatabaseRules, ...limitlessDatabaseRules]
51+
: standardDatabaseRules;
52+
53+
validateAllProps(scope, DatabaseCluster.name, props, applicableRules);
54+
}

packages/aws-cdk-lib/aws-rds/test/cluster.test.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ describe('cluster new api', () => {
268268
storageType: DBClusterStorageType.AURORA_IOPT1,
269269
cloudwatchLogsExports: ['postgresql'],
270270
});
271-
}).toThrow('Performance Insights must be enabled for Aurora Limitless Database.');
271+
}).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');
272272
});
273273

274274
test('throw error for invalid performance insights retention period', () => {
@@ -292,7 +292,7 @@ describe('cluster new api', () => {
292292
storageType: DBClusterStorageType.AURORA_IOPT1,
293293
cloudwatchLogsExports: ['postgresql'],
294294
});
295-
}).toThrow('Performance Insights retention period must be set at least 31 days for Aurora Limitless Database.');
295+
}).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');
296296
});
297297

298298
test('throw error for not specifying monitoring interval', () => {
@@ -316,7 +316,7 @@ describe('cluster new api', () => {
316316
storageType: DBClusterStorageType.AURORA_IOPT1,
317317
cloudwatchLogsExports: ['postgresql'],
318318
});
319-
}).toThrow('Cluster level enhanced monitoring must be set for Aurora Limitless Database. Please set \'monitoringInterval\' and enable \'enableClusterLevelEnhancedMonitoring\'.');
319+
}).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\'');
320320
});
321321

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

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

372372
test.each([
@@ -395,7 +395,7 @@ describe('cluster new api', () => {
395395
storageType: DBClusterStorageType.AURORA_IOPT1,
396396
cloudwatchLogsExports: ['postgresql'],
397397
});
398-
}).toThrow(`Aurora Limitless Database requires an engine version that supports it, got ${engine.engineVersion?.fullVersion}`);
398+
}).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}`);
399399
});
400400

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

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

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

21482148
test('warn if performance insights is enabled at cluster level but disabled on writer and reader instances', () => {

packages/aws-cdk-lib/core/lib/helpers-internal/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ export * from './cfn-parse';
33
export { md5hash } from '../private/md5';
44
export * from './customize-roles';
55
export * from './string-specializer';
6+
export * from './validate-all-props';
67
export { constructInfoFromConstruct, constructInfoFromStack } from '../private/runtime-info';
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { Construct } from 'constructs';
2+
import { ValidationError } from '../errors';
3+
4+
/**
5+
* Represents a validation rule for props of type T.
6+
* @template T The type of the props being validated.
7+
*/
8+
export type ValidationRule<T> = {
9+
/**
10+
* A function that checks if the validation rule condition is met.
11+
* @param {T} props - The props object to validate.
12+
* @returns {boolean} True if the condition is met (i.e., validation fails), false otherwise.
13+
*/
14+
condition: (props: T) => boolean;
15+
16+
/**
17+
* A function that returns an error message if the validation fails.
18+
* @param {T} props - The props that failed validation.
19+
* @returns {string} The error message.
20+
*/
21+
message: (props: T) => string;
22+
};
23+
24+
/**
25+
* Validates props against a set of rules and throws an error if any validations fail.
26+
*
27+
* @template T The type of the props being validated.
28+
* @param {string} className - The name of the class being validated, used in the error message. Ex. for SQS, might be Queue.name
29+
* @param {T} props - The props object to validate.
30+
* @param {ValidationRule<T>[]} rules - An array of validation rules to apply.
31+
* @throws {Error} If any validation rules fail, with a message detailing all failures.
32+
*/
33+
export function validateAllProps<T>(scope: Construct, className: string, props: T, rules: ValidationRule<T>[]): void {
34+
const validationErrors = rules
35+
.filter(rule => rule.condition(props))
36+
.map(rule => rule.message(props));
37+
38+
if (validationErrors.length > 0) {
39+
const errorMessage = `${className} initialization failed due to the following validation error(s):\n${validationErrors.map(error => `- ${error}`).join('\n')}`;
40+
throw new ValidationError(errorMessage, scope);
41+
}
42+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
import { Construct } from 'constructs';
2+
import { ValidationError } from '../../lib/errors';
3+
import { validateAllProps, ValidationRule } from '../../lib/helpers-internal/validate-all-props';
4+
5+
class TestConstruct extends Construct {
6+
constructor() {
7+
super(undefined as any, 'TestConstruct');
8+
}
9+
}
10+
11+
describe('validateAllProps', () => {
12+
let testScope: Construct;
13+
14+
beforeEach(() => {
15+
testScope = new TestConstruct();
16+
});
17+
18+
it('should not throw an error when all validations pass', () => {
19+
const props = { value: 5 };
20+
const rules: ValidationRule<typeof props>[] = [
21+
{
22+
condition: (p) => p.value < 0,
23+
message: (p) => `Value ${p.value} should be non-negative`,
24+
},
25+
];
26+
27+
expect(() => validateAllProps(testScope, 'TestClass', props, rules)).not.toThrow();
28+
});
29+
30+
it('should throw a ValidationError when a validation fails', () => {
31+
const props = { value: -5 };
32+
const rules: ValidationRule<typeof props>[] = [
33+
{
34+
condition: (p) => p.value < 0,
35+
message: (p) => `Value ${p.value} should be non-negative`,
36+
},
37+
];
38+
39+
expect(() => validateAllProps(testScope, 'TestClass', props, rules)).toThrow(ValidationError);
40+
});
41+
42+
it('should include all failed validation messages in the error', () => {
43+
const props = { value1: -5, value2: 15 };
44+
const rules: ValidationRule<typeof props>[] = [
45+
{
46+
condition: (p) => p.value1 < 0,
47+
message: (p) => `Value1 ${p.value1} should be non-negative`,
48+
},
49+
{
50+
condition: (p) => p.value2 > 10,
51+
message: (p) => `Value2 ${p.value2} should be 10 or less`,
52+
},
53+
];
54+
55+
expect(() => validateAllProps(testScope, 'TestClass', props, rules)).toThrow(ValidationError);
56+
try {
57+
validateAllProps(testScope, 'TestClass', props, rules);
58+
} catch (error) {
59+
if (error instanceof ValidationError) {
60+
expect(error.message).toBe(
61+
'TestClass initialization failed due to the following validation error(s):\n' +
62+
'- Value1 -5 should be non-negative\n' +
63+
'- Value2 15 should be 10 or less',
64+
);
65+
}
66+
}
67+
});
68+
69+
it('should work with complex object structures', () => {
70+
const props = { nested: { value: 'invalid' } };
71+
const rules: ValidationRule<typeof props>[] = [
72+
{
73+
condition: (p) => p.nested.value !== 'valid',
74+
message: (p) => `Nested value "${p.nested.value}" is not valid`,
75+
},
76+
];
77+
78+
expect(() => validateAllProps(testScope, 'TestClass', props, rules)).toThrow(ValidationError);
79+
try {
80+
validateAllProps(testScope, 'TestClass', props, rules);
81+
} catch (error) {
82+
if (error instanceof ValidationError) {
83+
expect(error.message).toBe(
84+
'TestClass initialization failed due to the following validation error(s):\n' +
85+
'- Nested value "invalid" is not valid',
86+
);
87+
}
88+
}
89+
});
90+
});

0 commit comments

Comments
 (0)