Skip to content

Commit

Permalink
feat(apigateway): throw ValidationError instead of untyped errors (#…
Browse files Browse the repository at this point in the history
…33075)

### Issue 

`aws-apigateway` 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
HBobertz authored Jan 27, 2025
1 parent 1beaf83 commit 04efe6c
Show file tree
Hide file tree
Showing 20 changed files with 103 additions and 84 deletions.
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [
const enableNoThrowDefaultErrorIn = [
'aws-amplify',
'aws-amplifyuibuilder',
'aws-apigateway',
'aws-apigatewayv2',
'aws-apigatewayv2-authorizers',
'aws-apigatewayv2-integrations',
'aws-cognito',
Expand All @@ -34,8 +36,6 @@ const enableNoThrowDefaultErrorIn = [
'aws-ssmcontacts',
'aws-ssmincidents',
'aws-ssmquicksetup',
'aws-apigatewayv2',
'aws-apigatewayv2-authorizers',
'aws-synthetics',
'aws-route53',
'aws-route53-patterns',
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/aws-apigateway/lib/access-log.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { IStage } from './stage';
import * as firehose from '../../aws-kinesisfirehose';
import { ILogGroup } from '../../aws-logs';
import { ValidationError } from '../../core/lib/errors';

/**
* Access log destination for a RestApi Stage.
Expand Down Expand Up @@ -49,9 +50,9 @@ export class FirehoseLogDestination implements IAccessLogDestination {
/**
* Binds this destination to the Firehose delivery stream.
*/
public bind(_stage: IStage): AccessLogDestinationConfig {
public bind(stage: IStage): AccessLogDestinationConfig {
if (!this.stream.deliveryStreamName?.startsWith('amazon-apigateway-')) {
throw new Error(`Firehose delivery stream name for access log destination must begin with 'amazon-apigateway-', got '${this.stream.deliveryStreamName}'`);
throw new ValidationError(`Firehose delivery stream name for access log destination must begin with 'amazon-apigateway-', got '${this.stream.deliveryStreamName}'`, stage);
}
return {
destinationArn: this.stream.attrArn,
Expand Down
11 changes: 6 additions & 5 deletions packages/aws-cdk-lib/aws-apigateway/lib/api-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { CfnRestApi } from './apigateway.generated';
import { IRestApi } from './restapi';
import * as s3 from '../../aws-s3';
import * as s3_assets from '../../aws-s3-assets';
import { UnscopedValidationError, ValidationError } from '../../core/lib/errors';
import * as cxapi from '../../cx-api';

/**
Expand Down Expand Up @@ -137,7 +138,7 @@ export class S3ApiDefinition extends ApiDefinition {
super();

if (!bucket.bucketName) {
throw new Error('bucketName is undefined for the provided bucket');
throw new ValidationError('bucketName is undefined for the provided bucket', bucket);
}

this.bucketName = bucket.bucketName;
Expand All @@ -162,11 +163,11 @@ export class InlineApiDefinition extends ApiDefinition {
super();

if (typeof(definition) !== 'object') {
throw new Error('definition should be of type object');
throw new UnscopedValidationError('definition should be of type object');
}

if (Object.keys(definition).length === 0) {
throw new Error('JSON definition cannot be empty');
throw new UnscopedValidationError('JSON definition cannot be empty');
}
}

Expand Down Expand Up @@ -197,7 +198,7 @@ export class AssetApiDefinition extends ApiDefinition {
}

if (this.asset.isZipArchive) {
throw new Error(`Asset cannot be a .zip file or a directory (${this.path})`);
throw new ValidationError(`Asset cannot be a .zip file or a directory (${this.path})`, scope);
}

return {
Expand All @@ -214,7 +215,7 @@ export class AssetApiDefinition extends ApiDefinition {
}

if (!this.asset) {
throw new Error('bindToResource() must be called after bind()');
throw new ValidationError('bindToResource() must be called after bind()', scope);
}

const child = Node.of(restApi).defaultChild as CfnRestApi;
Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk-lib/aws-apigateway/lib/api-key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { IStage } from './stage';
import { QuotaSettings, ThrottleSettings, UsagePlan, UsagePlanPerApiStage } from './usage-plan';
import * as iam from '../../aws-iam';
import { ArnFormat, IResource as IResourceBase, Resource, Stack } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* API keys are alphanumeric string values that you distribute to
Expand Down Expand Up @@ -196,15 +197,15 @@ export class ApiKey extends ApiKeyBase {
}

if (resources && stages) {
throw new Error('Only one of "resources" or "stages" should be provided');
throw new ValidationError('Only one of "resources" or "stages" should be provided', this);
}

return resources
? resources.map((resource: IRestApi) => {
const restApi = resource;
if (!restApi.deploymentStage) {
throw new Error('Cannot add an ApiKey to a RestApi that does not contain a "deploymentStage".\n'+
'Either set the RestApi.deploymentStage or create an ApiKey from a Stage');
throw new ValidationError('Cannot add an ApiKey to a RestApi that does not contain a "deploymentStage".\n'+
'Either set the RestApi.deploymentStage or create an ApiKey from a Stage', this);
}
const restApiId = restApi.restApiId;
const stageName = restApi.deploymentStage!.stageName.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Construct } from 'constructs';
import { IdentitySource } from './identity-source';
import * as cognito from '../../../aws-cognito';
import { Duration, FeatureFlags, Lazy, Names, Stack } from '../../../core';
import { ValidationError } from '../../../core/lib/errors';
import { APIGATEWAY_AUTHORIZER_CHANGE_DEPLOYMENT_LOGICAL_ID } from '../../../cx-api';
import { CfnAuthorizer, CfnAuthorizerProps } from '../apigateway.generated';
import { Authorizer, IAuthorizer } from '../authorizer';
Expand Down Expand Up @@ -102,7 +103,7 @@ export class CognitoUserPoolsAuthorizer extends Authorizer implements IAuthorize
*/
public _attachToApi(restApi: IRestApi): void {
if (this.restApiId && this.restApiId !== restApi.restApiId) {
throw new Error('Cannot attach authorizer to two different rest APIs');
throw new ValidationError('Cannot attach authorizer to two different rest APIs', restApi);
}

this.restApiId = restApi.restApiId;
Expand All @@ -126,7 +127,7 @@ export class CognitoUserPoolsAuthorizer extends Authorizer implements IAuthorize
return Lazy.string({
produce: () => {
if (!this.restApiId) {
throw new Error(`Authorizer (${this.node.path}) must be attached to a RestApi`);
throw new ValidationError(`Authorizer (${this.node.path}) must be attached to a RestApi`, this);
}
return this.restApiId;
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { UnscopedValidationError } from '../../../core/lib/errors';

/**
* Represents an identity source.
*
Expand Down Expand Up @@ -47,7 +49,7 @@ export class IdentitySource {

private static toString(source: string, type: string) {
if (!source.trim()) {
throw new Error('IdentitySources must be a non-empty string.');
throw new UnscopedValidationError('IdentitySources must be a non-empty string.');
}

return `${type}.${source}`;
Expand Down
9 changes: 5 additions & 4 deletions packages/aws-cdk-lib/aws-apigateway/lib/authorizers/lambda.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { IdentitySource } from './identity-source';
import * as iam from '../../../aws-iam';
import * as lambda from '../../../aws-lambda';
import { Arn, ArnFormat, Duration, FeatureFlags, Lazy, Names, Stack } from '../../../core';
import { ValidationError } from '../../../core/lib/errors';
import { APIGATEWAY_AUTHORIZER_CHANGE_DEPLOYMENT_LOGICAL_ID } from '../../../cx-api';
import { CfnAuthorizer, CfnAuthorizerProps } from '../apigateway.generated';
import { Authorizer, IAuthorizer } from '../authorizer';
Expand Down Expand Up @@ -79,7 +80,7 @@ abstract class LambdaAuthorizer extends Authorizer implements IAuthorizer {
this.role = props.assumeRole;

if (props.resultsCacheTtl && props.resultsCacheTtl?.toSeconds() > 3600) {
throw new Error('Lambda authorizer property \'resultsCacheTtl\' must not be greater than 3600 seconds (1 hour)');
throw new ValidationError('Lambda authorizer property \'resultsCacheTtl\' must not be greater than 3600 seconds (1 hour)', scope);
}
}

Expand All @@ -89,7 +90,7 @@ abstract class LambdaAuthorizer extends Authorizer implements IAuthorizer {
*/
public _attachToApi(restApi: IRestApi) {
if (this.restApiId && this.restApiId !== restApi.restApiId) {
throw new Error('Cannot attach authorizer to two different rest APIs');
throw new ValidationError('Cannot attach authorizer to two different rest APIs', this);
}

this.restApiId = restApi.restApiId;
Expand Down Expand Up @@ -160,7 +161,7 @@ abstract class LambdaAuthorizer extends Authorizer implements IAuthorizer {
return Lazy.string({
produce: () => {
if (!this.restApiId) {
throw new Error(`Authorizer (${this.node.path}) must be attached to a RestApi`);
throw new ValidationError(`Authorizer (${this.node.path}) must be attached to a RestApi`, this);
}
return this.restApiId;
},
Expand Down Expand Up @@ -272,7 +273,7 @@ export class RequestAuthorizer extends LambdaAuthorizer {
super(scope, id, props);

if ((props.resultsCacheTtl === undefined || props.resultsCacheTtl.toSeconds() !== 0) && props.identitySources.length === 0) {
throw new Error('At least one Identity Source is required for a REQUEST-based Lambda authorizer if caching is enabled.');
throw new ValidationError('At least one Identity Source is required for a REQUEST-based Lambda authorizer if caching is enabled.', scope);
}

const restApiId = this.lazyRestApiId();
Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk-lib/aws-apigateway/lib/base-path-mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { IDomainName } from './domain-name';
import { IRestApi, RestApiBase } from './restapi';
import { Stage } from './stage';
import { Resource, Token } from '../../core';
import { ValidationError } from '../../core/lib/errors';

export interface BasePathMappingOptions {
/**
Expand Down Expand Up @@ -57,13 +58,13 @@ export class BasePathMapping extends Resource {

if (props.basePath && !Token.isUnresolved(props.basePath)) {
if (props.basePath.startsWith('/') || props.basePath.endsWith('/')) {
throw new Error(`A base path cannot start or end with /", received: ${props.basePath}`);
throw new ValidationError(`A base path cannot start or end with /", received: ${props.basePath}`, scope);
}
if (props.basePath.match(/\/{2,}/)) {
throw new Error(`A base path cannot have more than one consecutive /", received: ${props.basePath}`);
throw new ValidationError(`A base path cannot have more than one consecutive /", received: ${props.basePath}`, scope);
}
if (!props.basePath.match(/^[a-zA-Z0-9$_.+!*'()-/]+$/)) {
throw new Error(`A base path may only contain letters, numbers, and one of "$-_.+!*'()/", received: ${props.basePath}`);
throw new ValidationError(`A base path may only contain letters, numbers, and one of "$-_.+!*'()/", received: ${props.basePath}`, scope);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk-lib/aws-apigateway/lib/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { CfnDeployment } from './apigateway.generated';
import { Method } from './method';
import { IRestApi, RestApi, SpecRestApi, RestApiBase } from './restapi';
import { Lazy, RemovalPolicy, Resource, CfnResource } from '../../core';
import { ValidationError } from '../../core/lib/errors';
import { md5hash } from '../../core/lib/helpers-internal';

export interface DeploymentProps {
Expand Down Expand Up @@ -168,7 +169,7 @@ class LatestDeploymentResource extends CfnDeployment {
// if the construct is locked, it means we are already synthesizing and then
// we can't modify the hash because we might have already calculated it.
if (this.node.locked) {
throw new Error('Cannot modify the logical ID when the construct is locked');
throw new ValidationError('Cannot modify the logical ID when the construct is locked', this);
}

this.hashComponents.push(data);
Expand Down
13 changes: 7 additions & 6 deletions packages/aws-cdk-lib/aws-apigateway/lib/domain-name.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as apigwv2 from '../../aws-apigatewayv2';
import * as acm from '../../aws-certificatemanager';
import { IBucket } from '../../aws-s3';
import { IResource, Names, Resource, Token } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* Options for creating an api mapping
Expand Down Expand Up @@ -143,7 +144,7 @@ export class DomainName extends Resource implements IDomainName {
this.securityPolicy = props.securityPolicy;

if (!Token.isUnresolved(props.domainName) && /[A-Z]/.test(props.domainName)) {
throw new Error(`Domain name does not support uppercase letters. Got: ${props.domainName}`);
throw new ValidationError(`Domain name does not support uppercase letters. Got: ${props.domainName}`, scope);
}

const mtlsConfig = this.configureMTLS(props.mtls);
Expand Down Expand Up @@ -181,10 +182,10 @@ export class DomainName extends Resource implements IDomainName {
private validateBasePath(path?: string): boolean {
if (this.isMultiLevel(path)) {
if (this.endpointType === EndpointType.EDGE) {
throw new Error('multi-level basePath is only supported when endpointType is EndpointType.REGIONAL');
throw new ValidationError('multi-level basePath is only supported when endpointType is EndpointType.REGIONAL', this);
}
if (this.securityPolicy && this.securityPolicy !== SecurityPolicy.TLS_1_2) {
throw new Error('securityPolicy must be set to TLS_1_2 if multi-level basePath is provided');
throw new ValidationError('securityPolicy must be set to TLS_1_2 if multi-level basePath is provided', this);
}
return true;
}
Expand All @@ -207,10 +208,10 @@ export class DomainName extends Resource implements IDomainName {
*/
public addBasePathMapping(targetApi: IRestApi, options: BasePathMappingOptions = { }): BasePathMapping {
if (this.basePaths.has(options.basePath)) {
throw new Error(`DomainName ${this.node.id} already has a mapping for path ${options.basePath}`);
throw new ValidationError(`DomainName ${this.node.id} already has a mapping for path ${options.basePath}`, this);
}
if (this.isMultiLevel(options.basePath)) {
throw new Error('BasePathMapping does not support multi-level paths. Use "addApiMapping instead.');
throw new ValidationError('BasePathMapping does not support multi-level paths. Use "addApiMapping instead.', this);
}

this.basePaths.add(options.basePath);
Expand All @@ -236,7 +237,7 @@ export class DomainName extends Resource implements IDomainName {
*/
public addApiMapping(targetStage: IStage, options: ApiMappingOptions = {}): void {
if (this.basePaths.has(options.basePath)) {
throw new Error(`DomainName ${this.node.id} already has a mapping for path ${options.basePath}`);
throw new ValidationError(`DomainName ${this.node.id} already has a mapping for path ${options.basePath}`, this);
}
this.validateBasePath(options.basePath);
this.basePaths.add(options.basePath);
Expand Down
17 changes: 9 additions & 8 deletions packages/aws-cdk-lib/aws-apigateway/lib/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Method } from './method';
import { IVpcLink, VpcLink } from './vpc-link';
import * as iam from '../../aws-iam';
import { Lazy, Duration } from '../../core';
import { UnscopedValidationError, ValidationError } from '../../core/lib/errors';

export interface IntegrationOptions {
/**
Expand Down Expand Up @@ -199,31 +200,31 @@ export class Integration {
constructor(private readonly props: IntegrationProps) {
const options = this.props.options || { };
if (options.credentialsPassthrough !== undefined && options.credentialsRole !== undefined) {
throw new Error('\'credentialsPassthrough\' and \'credentialsRole\' are mutually exclusive');
throw new UnscopedValidationError('\'credentialsPassthrough\' and \'credentialsRole\' are mutually exclusive');
}

if (options.connectionType === ConnectionType.VPC_LINK && options.vpcLink === undefined) {
throw new Error('\'connectionType\' of VPC_LINK requires \'vpcLink\' prop to be set');
throw new UnscopedValidationError('\'connectionType\' of VPC_LINK requires \'vpcLink\' prop to be set');
}

if (options.connectionType === ConnectionType.INTERNET && options.vpcLink !== undefined) {
throw new Error('cannot set \'vpcLink\' where \'connectionType\' is INTERNET');
throw new UnscopedValidationError('cannot set \'vpcLink\' where \'connectionType\' is INTERNET');
}

if (options.timeout && !options.timeout.isUnresolved() && options.timeout.toMilliseconds() < 50) {
throw new Error('Integration timeout must be greater than 50 milliseconds.');
throw new UnscopedValidationError('Integration timeout must be greater than 50 milliseconds.');
}

if (props.type !== IntegrationType.MOCK && !props.integrationHttpMethod) {
throw new Error('integrationHttpMethod is required for non-mock integration types.');
throw new UnscopedValidationError('integrationHttpMethod is required for non-mock integration types.');
}
}

/**
* Can be overridden by subclasses to allow the integration to interact with the method
* being integrated, access the REST API object, method ARNs, etc.
*/
public bind(_method: Method): IntegrationConfig {
public bind(method: Method): IntegrationConfig {
let uri = this.props.uri;
const options = this.props.options;

Expand All @@ -235,12 +236,12 @@ export class Integration {
if (vpcLink instanceof VpcLink) {
const targets = vpcLink._targetDnsNames;
if (targets.length > 1) {
throw new Error("'uri' is required when there are more than one NLBs in the VPC Link");
throw new ValidationError("'uri' is required when there are more than one NLBs in the VPC Link", method);
} else {
return `http://${targets[0]}`;
}
} else {
throw new Error("'uri' is required when the 'connectionType' is VPC_LINK");
throw new ValidationError("'uri' is required when the 'connectionType' is VPC_LINK", method);
}
},
});
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk-lib/aws-apigateway/lib/integrations/aws.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { IConstruct } from 'constructs';
import * as cdk from '../../../core';
import { ArnFormat } from '../../../core';
import { UnscopedValidationError } from '../../../core/lib/errors';
import { Integration, IntegrationConfig, IntegrationOptions, IntegrationType } from '../integration';
import { Method } from '../method';
import { parseAwsApiCall } from '../util';
Expand Down Expand Up @@ -89,7 +90,7 @@ export class AwsIntegration extends Integration {
integrationHttpMethod: props.integrationHttpMethod || 'POST',
uri: cdk.Lazy.string({
produce: () => {
if (!this.scope) { throw new Error('AwsIntegration must be used in API'); }
if (!this.scope) { throw new UnscopedValidationError('AwsIntegration must be used in API'); }
return cdk.Stack.of(this.scope).formatArn({
service: 'apigateway',
account: backend,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { AwsIntegration } from './aws';
import * as iam from '../../../aws-iam';
import * as sfn from '../../../aws-stepfunctions';
import { Token } from '../../../core';
import { ValidationError } from '../../../core/lib/errors';
import { IntegrationConfig, IntegrationOptions, PassthroughBehavior } from '../integration';
import { Method } from '../method';
import { Model } from '../model';
Expand Down Expand Up @@ -150,7 +151,7 @@ class StepFunctionsExecutionIntegration extends AwsIntegration {
if (this.stateMachine instanceof sfn.StateMachine) {
const stateMachineType = (this.stateMachine as sfn.StateMachine).stateMachineType;
if (stateMachineType !== sfn.StateMachineType.EXPRESS) {
throw new Error('State Machine must be of type "EXPRESS". Please use StateMachineType.EXPRESS as the stateMachineType');
throw new ValidationError('State Machine must be of type "EXPRESS". Please use StateMachineType.EXPRESS as the stateMachineType', method);
}

// if not imported, extract the name from the CFN layer to reach the
Expand Down
Loading

0 comments on commit 04efe6c

Please sign in to comment.