From ba2f5c862cf1d1a52d399ba82639650d61c7d549 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Fri, 31 Jan 2025 05:47:32 -0500 Subject: [PATCH] feat(appmesh): throw `ValidationError` istead of untyped Errors (#33245) ### Issue `aws-appmesh` for #32569 ### Description of changes ValidationErrors everywhere ### Describe any new or updated permissions being added n/a ### Description of how you validated changes Existing tests. Exemptions granted as this is basically a refactor of existing code. ### 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* --- packages/aws-cdk-lib/.eslintrc.js | 1 + .../aws-appmesh/lib/gateway-route-spec.ts | 4 ++-- .../aws-appmesh/lib/health-checks.ts | 8 ++++---- .../aws-appmesh/lib/http-route-path-match.ts | 17 ++++++++-------- .../aws-appmesh/lib/private/utils.ts | 20 +++++++++---------- .../aws-cdk-lib/aws-appmesh/lib/route-spec.ts | 12 +++++------ .../aws-appmesh/lib/shared-interfaces.ts | 2 +- .../aws-appmesh/lib/tls-validation.ts | 5 +++-- .../aws-appmesh/lib/virtual-node.ts | 2 +- 9 files changed, 37 insertions(+), 34 deletions(-) diff --git a/packages/aws-cdk-lib/.eslintrc.js b/packages/aws-cdk-lib/.eslintrc.js index e94dc717175d0..46e5780301ecd 100644 --- a/packages/aws-cdk-lib/.eslintrc.js +++ b/packages/aws-cdk-lib/.eslintrc.js @@ -24,6 +24,7 @@ const enableNoThrowDefaultErrorIn = [ 'aws-apigatewayv2-integrations', 'aws-applicationautoscaling', 'aws-appsync', + 'aws-appmesh', 'aws-cognito', 'aws-elasticloadbalancing', 'aws-elasticloadbalancingv2', diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/gateway-route-spec.ts b/packages/aws-cdk-lib/aws-appmesh/lib/gateway-route-spec.ts index dffd856f1698b..d63398183ca50 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/gateway-route-spec.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/gateway-route-spec.ts @@ -364,8 +364,8 @@ class GrpcGatewayRouteSpec extends GatewayRouteSpec { public bind(scope: Construct): GatewayRouteSpecConfig { const metadataMatch = this.match.metadata; - validateGrpcGatewayRouteMatch(this.match); - validateGrpcMatchArrayLength(metadataMatch); + validateGrpcGatewayRouteMatch(scope, this.match); + validateGrpcMatchArrayLength(scope, metadataMatch); return { grpcSpecConfig: { diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/health-checks.ts b/packages/aws-cdk-lib/aws-appmesh/lib/health-checks.ts index 94bed38de5356..bd4f74b4492d0 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/health-checks.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/health-checks.ts @@ -138,19 +138,19 @@ class HealthCheckImpl extends HealthCheck { private readonly path?: string) { super(); if (healthyThreshold < 2 || healthyThreshold > 10) { - throw new Error('healthyThreshold must be between 2 and 10'); + throw new cdk.UnscopedValidationError('healthyThreshold must be between 2 and 10'); } if (unhealthyThreshold < 2 || unhealthyThreshold > 10) { - throw new Error('unhealthyThreshold must be between 2 and 10'); + throw new cdk.UnscopedValidationError('unhealthyThreshold must be between 2 and 10'); } if (interval.toMilliseconds() < 5000 || interval.toMilliseconds() > 300_000) { - throw new Error('interval must be between 5 seconds and 300 seconds'); + throw new cdk.UnscopedValidationError('interval must be between 5 seconds and 300 seconds'); } if (timeout.toMilliseconds() < 2000 || timeout.toMilliseconds() > 60_000) { - throw new Error('timeout must be between 2 seconds and 60 seconds'); + throw new cdk.UnscopedValidationError('timeout must be between 2 seconds and 60 seconds'); } // Default to / for HTTP Health Checks diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/http-route-path-match.ts b/packages/aws-cdk-lib/aws-appmesh/lib/http-route-path-match.ts index 3446e1ab63be4..bed7d25f6753e 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/http-route-path-match.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/http-route-path-match.ts @@ -1,5 +1,6 @@ import { Construct } from 'constructs'; import { CfnGatewayRoute, CfnRoute } from './appmesh.generated'; +import { UnscopedValidationError } from '../../core/lib/errors'; /** * The type returned from the `bind()` method in `HttpRoutePathMatch`. @@ -66,7 +67,7 @@ class HttpRoutePrefixPathMatch extends HttpRoutePathMatch { super(); if (prefix && prefix[0] !== '/') { - throw new Error(`Prefix Path for the match must start with \'/\', got: ${prefix}`); + throw new UnscopedValidationError(`Prefix Path for the match must start with \'/\', got: ${prefix}`); } } @@ -82,7 +83,7 @@ class HttpRouteWholePathMatch extends HttpRoutePathMatch { super(); if (match.exact && match.exact[0] !== '/') { - throw new Error(`Exact Path for the match must start with \'/\', got: ${match.exact}`); + throw new UnscopedValidationError(`Exact Path for the match must start with \'/\', got: ${match.exact}`); } } @@ -184,17 +185,17 @@ class HttpGatewayRoutePrefixPathMatch extends HttpGatewayRoutePathMatch { super(); if (prefixPathMatch[0] !== '/') { - throw new Error('Prefix path for the match must start with \'/\', ' + throw new UnscopedValidationError('Prefix path for the match must start with \'/\', ' + `got: ${prefixPathMatch}`); } if (rewriteTo) { if (prefixPathMatch[prefixPathMatch.length - 1] !== '/') { - throw new Error('When prefix path for the rewrite is specified, prefix path for the match must end with \'/\', ' + throw new UnscopedValidationError('When prefix path for the rewrite is specified, prefix path for the match must end with \'/\', ' + `got: ${prefixPathMatch}`); } if (rewriteTo[0] !== '/' || rewriteTo[rewriteTo.length - 1] !== '/') { - throw new Error('Prefix path for the rewrite must start and end with \'/\', ' + throw new UnscopedValidationError('Prefix path for the rewrite must start and end with \'/\', ' + `got: ${rewriteTo}`); } } @@ -221,13 +222,13 @@ class HttpGatewayRouteWholePathMatch extends HttpGatewayRoutePathMatch { super(); if (wholePathMatch.exact && wholePathMatch.exact[0] !== '/') { - throw new Error(`Exact Path for the match must start with \'/\', got: ${ wholePathMatch.exact }`); + throw new UnscopedValidationError(`Exact Path for the match must start with \'/\', got: ${ wholePathMatch.exact }`); } if (exactPathRewrite === '') { - throw new Error('Exact Path for the rewrite cannot be empty. Unlike startsWith() method, no automatic rewrite on whole path match'); + throw new UnscopedValidationError('Exact Path for the rewrite cannot be empty. Unlike startsWith() method, no automatic rewrite on whole path match'); } if (exactPathRewrite && exactPathRewrite[0] !== '/') { - throw new Error(`Exact Path for the rewrite must start with \'/\', got: ${ exactPathRewrite }`); + throw new UnscopedValidationError(`Exact Path for the rewrite must start with \'/\', got: ${ exactPathRewrite }`); } } diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/private/utils.ts b/packages/aws-cdk-lib/aws-appmesh/lib/private/utils.ts index 3493fabe78d37..b65d2d4e86efa 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/private/utils.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/private/utils.ts @@ -1,5 +1,5 @@ import { Construct } from 'constructs'; -import { Token, TokenComparison } from '../../../core'; +import { Token, TokenComparison, ValidationError } from '../../../core'; import { CfnVirtualNode } from '../appmesh.generated'; import { GrpcGatewayRouteMatch } from '../gateway-route-spec'; import { HeaderMatch } from '../header-match'; @@ -97,45 +97,45 @@ export function renderMeshOwner(resourceAccount: string, meshAccount: string) : /** * This is the helper method to validate the length of HTTP match array when it is specified. */ -export function validateHttpMatchArrayLength(headers?: HeaderMatch[], queryParameters?: QueryParameterMatch[]) { +export function validateHttpMatchArrayLength(scope: Construct, headers?: HeaderMatch[], queryParameters?: QueryParameterMatch[]) { const MIN_LENGTH = 1; const MAX_LENGTH = 10; if (headers && (headers.length < MIN_LENGTH || headers.length > MAX_LENGTH)) { - throw new Error(`Number of headers provided for matching must be between ${MIN_LENGTH} and ${MAX_LENGTH}, got: ${headers.length}`); + throw new ValidationError(`Number of headers provided for matching must be between ${MIN_LENGTH} and ${MAX_LENGTH}, got: ${headers.length}`, scope); } if (queryParameters && (queryParameters.length < MIN_LENGTH || queryParameters.length > MAX_LENGTH)) { - throw new Error(`Number of query parameters provided for matching must be between ${MIN_LENGTH} and ${MAX_LENGTH}, got: ${queryParameters.length}`); + throw new ValidationError(`Number of query parameters provided for matching must be between ${MIN_LENGTH} and ${MAX_LENGTH}, got: ${queryParameters.length}`, scope); } } /** * This is the helper method to validate the length of gRPC match array when it is specified. */ -export function validateGrpcMatchArrayLength(metadata?: HeaderMatch[]): void { +export function validateGrpcMatchArrayLength(scope: Construct, metadata?: HeaderMatch[]): void { const MIN_LENGTH = 1; const MAX_LENGTH = 10; if (metadata && (metadata.length < MIN_LENGTH || metadata.length > MAX_LENGTH)) { - throw new Error(`Number of metadata provided for matching must be between ${MIN_LENGTH} and ${MAX_LENGTH}, got: ${metadata.length}`); + throw new ValidationError(`Number of metadata provided for matching must be between ${MIN_LENGTH} and ${MAX_LENGTH}, got: ${metadata.length}`, scope); } } /** * This is the helper method to validate at least one of gRPC route match type is defined. */ -export function validateGrpcRouteMatch(match: GrpcRouteMatch): void { +export function validateGrpcRouteMatch(scope: Construct, match: GrpcRouteMatch): void { if (match.serviceName === undefined && match.metadata === undefined && match.methodName === undefined && match.port === undefined) { - throw new Error('At least one gRPC route match property must be provided'); + throw new ValidationError('At least one gRPC route match property must be provided', scope); } } /** * This is the helper method to validate at least one of gRPC gateway route match type is defined. */ -export function validateGrpcGatewayRouteMatch(match: GrpcGatewayRouteMatch): void { +export function validateGrpcGatewayRouteMatch(scope: Construct, match: GrpcGatewayRouteMatch): void { if (match.serviceName === undefined && match.metadata === undefined && match.hostname === undefined && match.port === undefined) { - throw new Error('At least one gRPC gateway route match property beside rewriteRequestHostname must be provided'); + throw new ValidationError('At least one gRPC gateway route match property beside rewriteRequestHostname must be provided', scope); } } diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/route-spec.ts b/packages/aws-cdk-lib/aws-appmesh/lib/route-spec.ts index 2204e8aa30a9f..57ced3d7b36b9 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/route-spec.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/route-spec.ts @@ -449,7 +449,7 @@ class HttpRouteSpec extends RouteSpec { const tcpRetryEvents = props.retryPolicy.tcpRetryEvents ?? []; if (httpRetryEvents.length + tcpRetryEvents.length === 0) { - throw new Error('You must specify one value for at least one of `httpRetryEvents` or `tcpRetryEvents`'); + throw new cdk.UnscopedValidationError('You must specify one value for at least one of `httpRetryEvents` or `tcpRetryEvents`'); } this.retryPolicy = { @@ -467,7 +467,7 @@ class HttpRouteSpec extends RouteSpec { const headers = this.match?.headers; const queryParameters = this.match?.queryParameters; - validateHttpMatchArrayLength(headers, queryParameters); + validateHttpMatchArrayLength(scope, headers, queryParameters); const httpConfig: CfnRoute.HttpRouteProperty = { action: { @@ -557,7 +557,7 @@ class GrpcRouteSpec extends RouteSpec { const tcpRetryEvents = props.retryPolicy.tcpRetryEvents ?? []; if (grpcRetryEvents.length + httpRetryEvents.length + tcpRetryEvents.length === 0) { - throw new Error('You must specify one value for at least one of `grpcRetryEvents`, `httpRetryEvents` or `tcpRetryEvents`'); + throw new cdk.UnscopedValidationError('You must specify one value for at least one of `grpcRetryEvents`, `httpRetryEvents` or `tcpRetryEvents`'); } this.retryPolicy = { @@ -575,11 +575,11 @@ class GrpcRouteSpec extends RouteSpec { const metadata = this.match.metadata; const port = this.match.port; - validateGrpcRouteMatch(this.match); - validateGrpcMatchArrayLength(metadata); + validateGrpcRouteMatch(scope, this.match); + validateGrpcMatchArrayLength(scope, metadata); if (methodName && !serviceName) { - throw new Error('If you specify a method name, you must also specify a service name'); + throw new cdk.ValidationError('If you specify a method name, you must also specify a service name', scope); } return { diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/shared-interfaces.ts b/packages/aws-cdk-lib/aws-appmesh/lib/shared-interfaces.ts index 4753ef5f7f959..2fa12050a3d56 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/shared-interfaces.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/shared-interfaces.ts @@ -199,7 +199,7 @@ export abstract class LoggingFormat { */ public static fromJson(jsonLoggingFormat :{[key:string]: string}): LoggingFormat { if (Object.keys(jsonLoggingFormat).length == 0) { - throw new Error('Json key pairs cannot be empty.'); + throw new cdk.UnscopedValidationError('Json key pairs cannot be empty.'); } return new JsonLoggingFormat(jsonLoggingFormat); diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/tls-validation.ts b/packages/aws-cdk-lib/aws-appmesh/lib/tls-validation.ts index b2baf9b99ca07..fb8236a8472ed 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/tls-validation.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/tls-validation.ts @@ -1,6 +1,7 @@ import { Construct } from 'constructs'; import { CfnVirtualNode } from './appmesh.generated'; import * as acmpca from '../../aws-acmpca'; +import { ValidationError } from '../../core/lib/errors'; /** * Represents the properties needed to define TLS Validation context @@ -98,9 +99,9 @@ class TlsValidationAcmTrust extends TlsValidationTrust { this.certificateAuthorities = certificateAuthorities; } - public bind(_scope: Construct): TlsValidationTrustConfig { + public bind(scope: Construct): TlsValidationTrustConfig { if (this.certificateAuthorities.length === 0) { - throw new Error('you must provide at least one Certificate Authority when creating an ACM Trust ClientPolicy'); + throw new ValidationError('you must provide at least one Certificate Authority when creating an ACM Trust ClientPolicy', scope); } else { return { tlsValidationTrust: { diff --git a/packages/aws-cdk-lib/aws-appmesh/lib/virtual-node.ts b/packages/aws-cdk-lib/aws-appmesh/lib/virtual-node.ts index 0e6b6f6ddb3d0..903885ded6cf9 100644 --- a/packages/aws-cdk-lib/aws-appmesh/lib/virtual-node.ts +++ b/packages/aws-cdk-lib/aws-appmesh/lib/virtual-node.ts @@ -233,7 +233,7 @@ export class VirtualNode extends VirtualNodeBase { */ public addListener(listener: VirtualNodeListener) { if (!this.serviceDiscoveryConfig) { - throw new Error('Service discovery information is required for a VirtualNode with a listener.'); + throw new cdk.ValidationError('Service discovery information is required for a VirtualNode with a listener.', this); } this.listeners.push(listener.bind(this)); }