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(rds): throw ValidationError instead of untyped errors #33042

Merged
merged 5 commits into from
Jan 22, 2025
Merged
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
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [


// no-throw-default-error
const modules = ['aws-s3', 'aws-lambda'];
const modules = ['aws-s3', 'aws-lambda', 'aws-rds'];
baseConfig.overrides.push({
files: modules.map(m => `./${m}/lib/**`),
rules: { "@cdklabs/no-throw-default-error": ['error'] },
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/aws-rds/lib/aurora-cluster-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as ec2 from '../../aws-ec2';
import { IRole } from '../../aws-iam';
import * as kms from '../../aws-kms';
import { IResource, Resource, Duration, RemovalPolicy, ArnFormat, FeatureFlags } from '../../core';
import { ValidationError } from '../../core/lib/errors';
import { AURORA_CLUSTER_CHANGE_SCOPE_OF_INSTANCE_PARAMETER_GROUP_WITH_EACH_PARAMETERS } from '../../cx-api';

/**
Expand Down Expand Up @@ -476,7 +477,7 @@ class AuroraClusterInstance extends Resource implements IAuroraClusterInstance {
});
this.tier = props.promotionTier ?? 2;
if (this.tier > 15) {
throw new Error('promotionTier must be between 0-15');
throw new ValidationError('promotionTier must be between 0-15', this);
}

const isOwnedResource = Resource.isOwnedResource(props.cluster);
Expand All @@ -499,7 +500,7 @@ class AuroraClusterInstance extends Resource implements IAuroraClusterInstance {
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');
throw new ValidationError('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set', this);
}

this.performanceInsightsEnabled = enablePerformanceInsights;
Expand Down
9 changes: 5 additions & 4 deletions packages/aws-cdk-lib/aws-rds/lib/cluster-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { EngineVersion } from './engine-version';
import { IParameterGroup, ParameterGroup } from './parameter-group';
import * as iam from '../../aws-iam';
import * as secretsmanager from '../../aws-secretsmanager';
import { ValidationError } from '../../core/lib/errors';

/**
* The extra options passed to the `IClusterEngine.bindToCluster` method.
Expand Down Expand Up @@ -1179,19 +1180,19 @@ class AuroraPostgresClusterEngine extends ClusterEngineBase {
// skip validation for unversioned as it might be supported/unsupported. we cannot reliably tell at compile-time
if (this.engineVersion?.fullVersion) {
if (options.s3ImportRole && !(config.features?.s3Import)) {
throw new Error(`s3Import is not supported for Postgres version: ${this.engineVersion.fullVersion}. Use a version that supports the s3Import feature.`);
throw new ValidationError(`s3Import is not supported for Postgres version: ${this.engineVersion.fullVersion}. Use a version that supports the s3Import feature.`, scope);
}
if (options.s3ExportRole && !(config.features?.s3Export)) {
throw new Error(`s3Export is not supported for Postgres version: ${this.engineVersion.fullVersion}. Use a version that supports the s3Export feature.`);
throw new ValidationError(`s3Export is not supported for Postgres version: ${this.engineVersion.fullVersion}. Use a version that supports the s3Export feature.`, scope);
}
}
return config;
}

protected defaultParameterGroup(scope: Construct): IParameterGroup | undefined {
if (!this.parameterGroupFamily) {
throw new Error('Could not create a new ParameterGroup for an unversioned aurora-postgresql cluster engine. ' +
'Please either use a versioned engine, or pass an explicit ParameterGroup when creating the cluster');
throw new ValidationError('Could not create a new ParameterGroup for an unversioned aurora-postgresql cluster engine. ' +
'Please either use a versioned engine, or pass an explicit ParameterGroup when creating the cluster', scope);
}
return ParameterGroup.fromParameterGroupName(scope, 'AuroraPostgreSqlDatabaseClusterEngineDefaultParameterGroup',
`default.${this.parameterGroupFamily}`);
Expand Down
79 changes: 40 additions & 39 deletions packages/aws-cdk-lib/aws-rds/lib/cluster.ts

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions packages/aws-cdk-lib/aws-rds/lib/instance-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { EngineVersion } from './engine-version';
import { IOptionGroup, OptionGroup } from './option-group';
import * as iam from '../../aws-iam';
import * as secretsmanager from '../../aws-secretsmanager';
import { ValidationError } from '../../core/lib/errors';

/**
* The options passed to `IInstanceEngine.bind`.
Expand Down Expand Up @@ -142,9 +143,9 @@ abstract class InstanceEngineBase implements IInstanceEngine {
this.engineFamily = props.engineFamily;
}

public bindToInstance(_scope: Construct, options: InstanceEngineBindOptions): InstanceEngineConfig {
public bindToInstance(scope: Construct, options: InstanceEngineBindOptions): InstanceEngineConfig {
if (options.timezone && !this.supportsTimezone) {
throw new Error(`timezone property can not be configured for ${this.engineType}`);
throw new ValidationError(`timezone property can not be configured for ${this.engineType}`, scope);
}
return {
features: this.features,
Expand Down Expand Up @@ -621,7 +622,7 @@ class MariaDbInstanceEngine extends InstanceEngineBase {

public bindToInstance(scope: Construct, options: InstanceEngineBindOptions): InstanceEngineConfig {
if (options.domain) {
throw new Error(`domain property cannot be configured for ${this.engineType}`);
throw new ValidationError(`domain property cannot be configured for ${this.engineType}`, scope);
}
return super.bindToInstance(scope, options);
}
Expand Down Expand Up @@ -2828,7 +2829,7 @@ abstract class SqlServerInstanceEngineBase extends InstanceEngineBase {
const s3Role = options.s3ImportRole ?? options.s3ExportRole;
if (s3Role) {
if (options.s3ImportRole && options.s3ExportRole && options.s3ImportRole !== options.s3ExportRole) {
throw new Error('S3 import and export roles must be the same for SQL Server engines');
throw new ValidationError('S3 import and export roles must be the same for SQL Server engines', scope);
}

if (!optionGroup) {
Expand Down
35 changes: 18 additions & 17 deletions packages/aws-cdk-lib/aws-rds/lib/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import * as logs from '../../aws-logs';
import * as s3 from '../../aws-s3';
import * as secretsmanager from '../../aws-secretsmanager';
import { ArnComponents, ArnFormat, Duration, FeatureFlags, IResource, Lazy, RemovalPolicy, Resource, Stack, Token, Tokenization } from '../../core';
import { ValidationError } from '../../core/lib/errors';
import * as cxapi from '../../cx-api';

/**
Expand Down Expand Up @@ -180,15 +181,15 @@ export abstract class DatabaseInstanceBase extends Resource implements IDatabase

public grantConnect(grantee: iam.IGrantable, dbUser?: string): iam.Grant {
if (this.enableIamAuthentication === false) {
throw new Error('Cannot grant connect when IAM authentication is disabled');
throw new ValidationError('Cannot grant connect when IAM authentication is disabled', this);
}

if (!this.instanceResourceId) {
throw new Error('For imported Database Instances, instanceResourceId is required to grantConnect()');
throw new ValidationError('For imported Database Instances, instanceResourceId is required to grantConnect()', this);
}

if (!dbUser) {
throw new Error('For imported Database Instances, the dbUser is required to grantConnect()');
throw new ValidationError('For imported Database Instances, the dbUser is required to grantConnect()', this);
}

this.enableIamAuthentication = true;
Expand Down Expand Up @@ -784,12 +785,12 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData

this.vpc = props.vpc;
if (props.vpcSubnets && props.vpcPlacement) {
throw new Error('Only one of `vpcSubnets` or `vpcPlacement` can be specified');
throw new ValidationError('Only one of `vpcSubnets` or `vpcPlacement` can be specified', this);
}
this.vpcPlacement = props.vpcSubnets ?? props.vpcPlacement;

if (props.multiAz === true && props.availabilityZone) {
throw new Error('Requesting a specific availability zone is not valid for Multi-AZ instances');
throw new ValidationError('Requesting a specific availability zone is not valid for Multi-AZ instances', this);
}

const subnetGroup = props.subnetGroup ?? new SubnetGroup(this, 'SubnetGroup', {
Expand Down Expand Up @@ -820,12 +821,12 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData
const storageType = props.storageType ?? StorageType.GP2;
const iops = defaultIops(storageType, props.iops);
if (props.storageThroughput && storageType !== StorageType.GP3) {
throw new Error(`The storage throughput can only be specified with GP3 storage type. Got ${storageType}.`);
throw new ValidationError(`The storage throughput can only be specified with GP3 storage type. Got ${storageType}.`, this);
}
if (storageType === StorageType.GP3 && props.storageThroughput && iops
&& !Token.isUnresolved(props.storageThroughput) && !Token.isUnresolved(iops)
&& props.storageThroughput/iops > 0.25) {
throw new Error(`The maximum ratio of storage throughput to IOPS is 0.25. Got ${props.storageThroughput/iops}.`);
throw new ValidationError(`The maximum ratio of storage throughput to IOPS is 0.25. Got ${props.storageThroughput/iops}.`, this);
}

this.cloudwatchLogGroups = {};
Expand All @@ -837,7 +838,7 @@ abstract class DatabaseInstanceNew extends DatabaseInstanceBase implements IData
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');
throw new ValidationError('`enablePerformanceInsights` disabled, but `performanceInsightRetention` or `performanceInsightEncryptionKey` was set', this);
}

if (props.domain) {
Expand Down Expand Up @@ -1019,13 +1020,13 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
const engineFeatures = engineConfig.features;
if (s3ImportRole) {
if (!engineFeatures?.s3Import) {
throw new Error(`Engine '${engineDescription(props.engine)}' does not support S3 import`);
throw new ValidationError(`Engine '${engineDescription(props.engine)}' does not support S3 import`, this);
}
instanceAssociatedRoles.push({ roleArn: s3ImportRole.roleArn, featureName: engineFeatures?.s3Import });
}
if (s3ExportRole) {
if (!engineFeatures?.s3Export) {
throw new Error(`Engine '${engineDescription(props.engine)}' does not support S3 export`);
throw new ValidationError(`Engine '${engineDescription(props.engine)}' does not support S3 export`, this);
}
// only add the export feature if it's different from the import feature
if (engineFeatures.s3Import !== engineFeatures?.s3Export) {
Expand All @@ -1036,7 +1037,7 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
this.instanceType = props.instanceType ?? ec2.InstanceType.of(ec2.InstanceClass.M5, ec2.InstanceSize.LARGE);

if (props.parameterGroup && props.parameters) {
throw new Error('You cannot specify both parameterGroup and parameters');
throw new ValidationError('You cannot specify both parameterGroup and parameters', this);
}

const dbParameterGroupName = props.parameters
Expand Down Expand Up @@ -1069,13 +1070,13 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
*/
public addRotationSingleUser(options: RotationSingleUserOptions = {}): secretsmanager.SecretRotation {
if (!this.secret) {
throw new Error('Cannot add single user rotation for an instance without secret.');
throw new ValidationError('Cannot add single user rotation for an instance without secret.', this);
}

const id = 'RotationSingleUser';
const existing = this.node.tryFindChild(id);
if (existing) {
throw new Error('A single user rotation was already added to this instance.');
throw new ValidationError('A single user rotation was already added to this instance.', this);
}

return new secretsmanager.SecretRotation(this, id, {
Expand All @@ -1092,7 +1093,7 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
*/
public addRotationMultiUser(id: string, options: RotationMultiUserOptions): secretsmanager.SecretRotation {
if (!this.secret) {
throw new Error('Cannot add multi user rotation for an instance without secret.');
throw new ValidationError('Cannot add multi user rotation for an instance without secret.', this);
}

return new secretsmanager.SecretRotation(this, id, {
Expand All @@ -1115,7 +1116,7 @@ abstract class DatabaseInstanceSource extends DatabaseInstanceNew implements IDa
public grantConnect(grantee: iam.IGrantable, dbUser?: string): iam.Grant {
if (!dbUser) {
if (!this.secret) {
throw new Error('A secret or dbUser is required to grantConnect()');
throw new ValidationError('A secret or dbUser is required to grantConnect()', this);
}

dbUser = this.secret.secretValueFromJson('username').unsafeUnwrap();
Expand Down Expand Up @@ -1248,7 +1249,7 @@ export class DatabaseInstanceFromSnapshot extends DatabaseInstanceSource impleme
let secret = credentials?.secret;
if (!secret && credentials?.generatePassword) {
if (!credentials.username) {
throw new Error('`credentials` `username` must be specified when `generatePassword` is set to true');
throw new ValidationError('`credentials` `username` must be specified when `generatePassword` is set to true', this);
}

secret = new DatabaseSecret(this, 'Secret', {
Expand Down Expand Up @@ -1351,7 +1352,7 @@ export class DatabaseInstanceReadReplica extends DatabaseInstanceNew implements
if (props.sourceDatabaseInstance.engine
&& !props.sourceDatabaseInstance.engine.supportsReadReplicaBackups
&& props.backupRetention) {
throw new Error(`Cannot set 'backupRetention', as engine '${engineDescription(props.sourceDatabaseInstance.engine)}' does not support automatic backups for read replicas`);
throw new ValidationError(`Cannot set 'backupRetention', as engine '${engineDescription(props.sourceDatabaseInstance.engine)}' does not support automatic backups for read replicas`, this);
}

// The read replica instance always uses the same engine as the source instance
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/aws-rds/lib/option-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { IInstanceEngine } from './instance-engine';
import { CfnOptionGroup } from './rds.generated';
import * as ec2 from '../../aws-ec2';
import { IResource, Lazy, Resource } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* An option group
Expand Down Expand Up @@ -126,7 +127,7 @@ export class OptionGroup extends Resource implements IOptionGroup {

const majorEngineVersion = props.engine.engineVersion?.majorVersion;
if (!majorEngineVersion) {
throw new Error("OptionGroup cannot be used with an engine that doesn't specify a version");
throw new ValidationError("OptionGroup cannot be used with an engine that doesn't specify a version", this);
}

props.configurations.forEach(config => this.addConfiguration(config));
Expand All @@ -146,7 +147,7 @@ export class OptionGroup extends Resource implements IOptionGroup {

if (configuration.port) {
if (!configuration.vpc) {
throw new Error('`port` and `vpc` must be specified together.');
throw new ValidationError('`port` and `vpc` must be specified together.', this);
}

const securityGroups = configuration.securityGroups && configuration.securityGroups.length > 0
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk-lib/aws-rds/lib/parameter-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Construct } from 'constructs';
import { IEngine } from './engine';
import { CfnDBClusterParameterGroup, CfnDBParameterGroup } from './rds.generated';
import { IResource, Lazy, RemovalPolicy, Resource } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* Options for `IParameterGroup.bindToCluster`.
Expand Down Expand Up @@ -143,7 +144,7 @@ export class ParameterGroup extends Resource implements IParameterGroup {

const family = props.engine.parameterGroupFamily;
if (!family) {
throw new Error("ParameterGroup cannot be used with an engine that doesn't specify a version");
throw new ValidationError("ParameterGroup cannot be used with an engine that doesn't specify a version", this);
}
this.family = family;
this.description = props.description;
Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk-lib/aws-rds/lib/private/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as ec2 from '../../../aws-ec2';
import * as iam from '../../../aws-iam';
import * as s3 from '../../../aws-s3';
import { RemovalPolicy } from '../../../core';
import { ValidationError } from '../../../core/lib/errors';
import { DatabaseSecret } from '../database-secret';
import { IEngine } from '../engine';
import { CommonRotationUserOptions, Credentials, SnapshotCredentials } from '../props';
Expand Down Expand Up @@ -43,7 +44,7 @@ export function setupS3ImportExport(

if (props.s3ImportBuckets && props.s3ImportBuckets.length > 0) {
if (props.s3ImportRole) {
throw new Error('Only one of s3ImportRole or s3ImportBuckets must be specified, not both.');
throw new ValidationError('Only one of s3ImportRole or s3ImportBuckets must be specified, not both.', scope);
}

s3ImportRole = (combineRoles && s3ExportRole) ? s3ExportRole : new iam.Role(scope, 'S3ImportRole', {
Expand All @@ -56,7 +57,7 @@ export function setupS3ImportExport(

if (props.s3ExportBuckets && props.s3ExportBuckets.length > 0) {
if (props.s3ExportRole) {
throw new Error('Only one of s3ExportRole or s3ExportBuckets must be specified, not both.');
throw new ValidationError('Only one of s3ExportRole or s3ExportBuckets must be specified, not both.', scope);
}

s3ExportRole = (combineRoles && s3ImportRole) ? s3ImportRole : new iam.Role(scope, 'S3ExportRole', {
Expand Down Expand Up @@ -117,7 +118,7 @@ export function renderSnapshotCredentials(scope: Construct, credentials?: Snapsh
let secret = renderedCredentials?.secret;
if (!secret && renderedCredentials?.generatePassword) {
if (!renderedCredentials.username) {
throw new Error('`snapshotCredentials` `username` must be specified when `generatePassword` is set to true');
throw new ValidationError('`snapshotCredentials` `username` must be specified when `generatePassword` is set to true', scope);
}

renderedCredentials = SnapshotCredentials.fromSecret(
Expand Down
Loading
Loading