Skip to content

Commit

Permalink
feat(appmesh): throw ValidationError istead of untyped Errors (#33245)
Browse files Browse the repository at this point in the history
### 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*
  • Loading branch information
kaizencc authored Jan 31, 2025
1 parent 2e7c3e8 commit ba2f5c8
Show file tree
Hide file tree
Showing 9 changed files with 37 additions and 34 deletions.
1 change: 1 addition & 0 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const enableNoThrowDefaultErrorIn = [
'aws-apigatewayv2-integrations',
'aws-applicationautoscaling',
'aws-appsync',
'aws-appmesh',
'aws-cognito',
'aws-elasticloadbalancing',
'aws-elasticloadbalancingv2',
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/aws-appmesh/lib/gateway-route-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
8 changes: 4 additions & 4 deletions packages/aws-cdk-lib/aws-appmesh/lib/health-checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions packages/aws-cdk-lib/aws-appmesh/lib/http-route-path-match.ts
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down Expand Up @@ -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}`);
}
}

Expand All @@ -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}`);
}
}

Expand Down Expand Up @@ -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}`);
}
}
Expand All @@ -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 }`);
}
}

Expand Down
20 changes: 10 additions & 10 deletions packages/aws-cdk-lib/aws-appmesh/lib/private/utils.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
}
}
12 changes: 6 additions & 6 deletions packages/aws-cdk-lib/aws-appmesh/lib/route-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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: {
Expand Down Expand Up @@ -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 = {
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-appmesh/lib/shared-interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/aws-appmesh/lib/tls-validation.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-appmesh/lib/virtual-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down

0 comments on commit ba2f5c8

Please sign in to comment.