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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const enableNoThrowDefaultErrorIn = [
'aws-ssmcontacts',
'aws-ssmincidents',
'aws-ssmquicksetup',
'aws-apigateway',
];
baseConfig.overrides.push({
files: enableNoThrowDefaultErrorIn.map(m => `./${m}/lib/**`),
Expand Down
3 changes: 2 additions & 1 deletion 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 { UnscopedValidationError } from '../../core/lib/errors';

/**
* Access log destination for a RestApi Stage.
Expand Down Expand Up @@ -51,7 +52,7 @@ export class FirehoseLogDestination implements IAccessLogDestination {
*/
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 UnscopedValidationError(`Firehose delivery stream name for access log destination must begin with 'amazon-apigateway-', got '${this.stream.deliveryStreamName}'`);
}
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 @@ -197,15 +198,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 { UnscopedValidationError } 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 UnscopedValidationError('Cannot attach authorizer to two different rest APIs');
}

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 UnscopedValidationError(`Authorizer (${this.node.path}) must be attached to a RestApi`);
}
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 @@ -80,7 +81,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 @@ -90,7 +91,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 @@ -161,7 +162,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 @@ -275,7 +276,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 { UnscopedValidationError } 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 UnscopedValidationError('Cannot modify the logical ID when the construct is locked');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why can't this be scoped to 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 @@ -144,7 +145,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 @@ -182,10 +183,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 @@ -208,10 +209,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 @@ -237,7 +238,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
15 changes: 8 additions & 7 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 } from '../../core/lib/errors';

export interface IntegrationOptions {
/**
Expand Down Expand Up @@ -199,23 +200,23 @@ 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.');
}
}

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 UnscopedValidationError("'uri' is required when there are more than one NLBs in the VPC Link");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be scoped to method, does that not work for some reason?

} else {
return `http://${targets[0]}`;
}
} else {
throw new Error("'uri' is required when the 'connectionType' is VPC_LINK");
throw new UnscopedValidationError("'uri' is required when the 'connectionType' is VPC_LINK");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feels like here too

}
},
});
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 { UnscopedValidationError } 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 UnscopedValidationError('State Machine must be of type "EXPRESS". Please use StateMachineType.EXPRESS as the stateMachineType');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.stateMachine?

}

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