Skip to content

Commit

Permalink
feat(cloudfront): throw ValidationErrors instead of untyped Errors (#…
Browse files Browse the repository at this point in the history
…33438)

### Issue 

Relates to #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 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 Feb 13, 2025
1 parent 9a6e5cc commit c08c7f0
Show file tree
Hide file tree
Showing 18 changed files with 99 additions and 92 deletions.
2 changes: 2 additions & 0 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ const enableNoThrowDefaultErrorIn = [
'aws-backup',
'aws-batch',
'aws-cognito',
'aws-cloudfront',
'aws-cloudfront-origins',
'aws-elasticloadbalancing',
'aws-elasticloadbalancingv2',
'aws-elasticloadbalancingv2-actions',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class FunctionUrlOriginWithOAC extends cloudfront.OriginBase {
if (!this.originAccessControl) {
this.originAccessControl = new cloudfront.FunctionUrlOriginAccessControl(scope, 'FunctionUrlOriginAccessControl');
}
this.validateAuthType();
this.validateAuthType(scope);

this.addInvokePermission(scope, options);

Expand All @@ -142,7 +142,7 @@ class FunctionUrlOriginWithOAC extends cloudfront.OriginBase {
/**
* Validation method: Ensures that when the OAC signing method is SIGV4_ALWAYS, the authType is set to AWS_IAM.
*/
private validateAuthType() {
private validateAuthType(scope: Construct) {
const cfnOriginAccessControl = this.originAccessControl?.node.children.find(
(child) => child instanceof cloudfront.CfnOriginAccessControl,
) as cloudfront.CfnOriginAccessControl;
Expand All @@ -156,7 +156,7 @@ class FunctionUrlOriginWithOAC extends cloudfront.OriginBase {
const isAuthTypeIsNone: boolean = this.functionUrl.authType !== lambda.FunctionUrlAuthType.AWS_IAM;

if (isAlwaysSigning && isAuthTypeIsNone) {
throw new Error('The authType of the Function URL must be set to AWS_IAM when origin access control signing method is SIGV4_ALWAYS.');
throw new cdk.ValidationError('The authType of the Function URL must be set to AWS_IAM when origin access control signing method is SIGV4_ALWAYS.', scope);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Construct } from 'constructs';
import * as cloudfront from '../../aws-cloudfront';
import { ValidationError } from '../../core';

/** Construction properties for `OriginGroup`. */
export interface OriginGroupProps {
Expand Down Expand Up @@ -44,7 +45,7 @@ export class OriginGroup implements cloudfront.IOrigin {
public bind(scope: Construct, options: cloudfront.OriginBindOptions): cloudfront.OriginBindConfig {
const primaryOriginConfig = this.props.primaryOrigin.bind(scope, options);
if (primaryOriginConfig.failoverConfig) {
throw new Error('An OriginGroup cannot use an Origin with its own failover configuration as its primary origin!');
throw new ValidationError('An OriginGroup cannot use an Origin with its own failover configuration as its primary origin!', scope);
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ export function validateSecondsInRangeOrUndefined(name: string, min: number, max
if (duration === undefined) { return; }
const value = duration.toSeconds();
if (!Number.isInteger(value) || value < min || value > max) {
throw new Error(`${name}: Must be an int between ${min} and ${max} seconds (inclusive); received ${value}.`);
throw new cdk.UnscopedValidationError(`${name}: Must be an int between ${min} and ${max} seconds (inclusive); received ${value}.`);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { AccessLevel } from '../../aws-cloudfront';
import * as iam from '../../aws-iam';
import { IKey } from '../../aws-kms';
import { IBucket } from '../../aws-s3';
import { Annotations, Aws, Names, Stack } from '../../core';
import { Annotations, Aws, Names, Stack, UnscopedValidationError } from '../../core';

interface BucketPolicyAction {
readonly action: string;
Expand Down Expand Up @@ -262,7 +262,7 @@ class S3BucketOriginWithOAI extends S3BucketOrigin {

protected renderS3OriginConfig(): cloudfront.CfnDistribution.S3OriginConfigProperty | undefined {
if (!this.originAccessIdentity) {
throw new Error('Origin access identity cannot be undefined');
throw new UnscopedValidationError('Origin access identity cannot be undefined');
}
return { originAccessIdentity: `origin-access-identity/cloudfront/${this.originAccessIdentity.originAccessIdentityId}` };
}
Expand Down
18 changes: 9 additions & 9 deletions packages/aws-cdk-lib/aws-cloudfront/lib/cache-policy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Construct } from 'constructs';
import { CfnCachePolicy } from './cloudfront.generated';
import { Duration, Names, Resource, Stack, Token, withResolved } from '../../core';
import { Duration, Names, Resource, Stack, Token, UnscopedValidationError, ValidationError, withResolved } from '../../core';
import { addConstructMetadata } from '../../core/lib/metadata-resource';

/**
Expand Down Expand Up @@ -148,15 +148,15 @@ export class CachePolicy extends Resource implements ICachePolicy {
const cachePolicyName = props.cachePolicyName ?? `${Names.uniqueId(this).slice(0, 110)}-${Stack.of(this).region}`;

if (!Token.isUnresolved(cachePolicyName) && !cachePolicyName.match(/^[\w-]+$/i)) {
throw new Error(`'cachePolicyName' can only include '-', '_', and alphanumeric characters, got: '${cachePolicyName}'`);
throw new ValidationError(`'cachePolicyName' can only include '-', '_', and alphanumeric characters, got: '${cachePolicyName}'`, this);
}

if (cachePolicyName.length > 128) {
throw new Error(`'cachePolicyName' cannot be longer than 128 characters, got: '${cachePolicyName.length}'`);
throw new ValidationError(`'cachePolicyName' cannot be longer than 128 characters, got: '${cachePolicyName.length}'`, this);
}

if (props.comment && !Token.isUnresolved(props.comment) && props.comment.length > 128) {
throw new Error(`'comment' cannot be longer than 128 characters, got: ${props.comment.length}`);
throw new ValidationError(`'comment' cannot be longer than 128 characters, got: ${props.comment.length}`, this);
}

const minTtl = (props.minTtl ?? Duration.seconds(0)).toSeconds();
Expand Down Expand Up @@ -229,7 +229,7 @@ export class CacheCookieBehavior {
*/
public static allowList(...cookies: string[]) {
if (cookies.length === 0) {
throw new Error('At least one cookie to allow must be provided');
throw new UnscopedValidationError('At least one cookie to allow must be provided');
}
return new CacheCookieBehavior('whitelist', cookies);
}
Expand All @@ -240,7 +240,7 @@ export class CacheCookieBehavior {
*/
public static denyList(...cookies: string[]) {
if (cookies.length === 0) {
throw new Error('At least one cookie to deny must be provided');
throw new UnscopedValidationError('At least one cookie to deny must be provided');
}
return new CacheCookieBehavior('allExcept', cookies);
}
Expand All @@ -265,7 +265,7 @@ export class CacheHeaderBehavior {
/** Listed headers are included in the cache key and are automatically included in requests that CloudFront sends to the origin. */
public static allowList(...headers: string[]) {
if (headers.length === 0) {
throw new Error('At least one header to allow must be provided');
throw new UnscopedValidationError('At least one header to allow must be provided');
}
return new CacheHeaderBehavior('whitelist', headers);
}
Expand Down Expand Up @@ -302,7 +302,7 @@ export class CacheQueryStringBehavior {
*/
public static allowList(...queryStrings: string[]) {
if (queryStrings.length === 0) {
throw new Error('At least one query string to allow must be provided');
throw new UnscopedValidationError('At least one query string to allow must be provided');
}
return new CacheQueryStringBehavior('whitelist', queryStrings);
}
Expand All @@ -313,7 +313,7 @@ export class CacheQueryStringBehavior {
*/
public static denyList(...queryStrings: string[]) {
if (queryStrings.length === 0) {
throw new Error('At least one query string to deny must be provided');
throw new UnscopedValidationError('At least one query string to deny must be provided');
}
return new CacheQueryStringBehavior('allExcept', queryStrings);
}
Expand Down
36 changes: 18 additions & 18 deletions packages/aws-cdk-lib/aws-cloudfront/lib/distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as cloudwatch from '../../aws-cloudwatch';
import * as iam from '../../aws-iam';
import * as lambda from '../../aws-lambda';
import * as s3 from '../../aws-s3';
import { ArnFormat, IResource, Lazy, Resource, Stack, Token, Duration, Names, FeatureFlags, Annotations } from '../../core';
import { ArnFormat, IResource, Lazy, Resource, Stack, Token, Duration, Names, FeatureFlags, Annotations, ValidationError } from '../../core';
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
import { CLOUDFRONT_DEFAULT_SECURITY_POLICY_TLS_V1_2_2021 } from '../../cx-api';

Expand Down Expand Up @@ -332,7 +332,7 @@ export class Distribution extends Resource implements IDistribution {
if (props.certificate) {
const certificateRegion = Stack.of(this).splitArn(props.certificate.certificateArn, ArnFormat.SLASH_RESOURCE_NAME).region;
if (!Token.isUnresolved(certificateRegion) && certificateRegion !== 'us-east-1') {
throw new Error(`Distribution certificates must be in the us-east-1 region and the certificate you provided is in ${certificateRegion}.`);
throw new ValidationError(`Distribution certificates must be in the us-east-1 region and the certificate you provided is in ${certificateRegion}.`, this);
}

if ((props.domainNames ?? []).length === 0) {
Expand Down Expand Up @@ -491,7 +491,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metricOriginLatency(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('Origin latency metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('Origin latency metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('OriginLatency', props);
}
Expand All @@ -508,7 +508,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metricCacheHitRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('Cache hit rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('Cache hit rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('CacheHitRate', props);
}
Expand All @@ -523,7 +523,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metric401ErrorRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('401 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('401 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('401ErrorRate', props);
}
Expand All @@ -538,7 +538,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metric403ErrorRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('403 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('403 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('403ErrorRate', props);
}
Expand All @@ -553,7 +553,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metric404ErrorRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('404 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('404 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('404ErrorRate', props);
}
Expand All @@ -568,7 +568,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metric502ErrorRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('502 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('502 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('502ErrorRate', props);
}
Expand All @@ -583,7 +583,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metric503ErrorRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('503 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('503 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('503ErrorRate', props);
}
Expand All @@ -598,7 +598,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public metric504ErrorRate(props?: cloudwatch.MetricOptions): cloudwatch.Metric {
if (this.publishAdditionalMetrics !== true) {
throw new Error('504 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'');
throw new ValidationError('504 error rate metric is only available if \'publishAdditionalMetrics\' is set \'true\'', this);
}
return this.metric('504ErrorRate', props);
}
Expand All @@ -613,7 +613,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public addBehavior(pathPattern: string, origin: IOrigin, behaviorOptions: AddBehaviorOptions = {}) {
if (pathPattern === '*') {
throw new Error('Only the default behavior can have a path pattern of \'*\'');
throw new ValidationError('Only the default behavior can have a path pattern of \'*\'', this);
}
const originId = this.addOrigin(origin);
this.additionalBehaviors.push(new CacheBehavior(originId, { pathPattern, ...behaviorOptions }));
Expand Down Expand Up @@ -651,7 +651,7 @@ export class Distribution extends Resource implements IDistribution {
@MethodMetadata()
public attachWebAclId(webAclId: string) {
if (this.webAclId) {
throw new Error('A WebACL has already been attached to this distribution');
throw new ValidationError('A WebACL has already been attached to this distribution', this);
}
this.validateWebAclId(webAclId);
this.webAclId = webAclId;
Expand All @@ -661,7 +661,7 @@ export class Distribution extends Resource implements IDistribution {
if (webAclId.startsWith('arn:')) {
const webAclRegion = Stack.of(this).splitArn(webAclId, ArnFormat.SLASH_RESOURCE_NAME).region;
if (!Token.isUnresolved(webAclRegion) && webAclRegion !== 'us-east-1') {
throw new Error(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${webAclRegion}`);
throw new ValidationError(`WebACL for CloudFront distributions must be created in the us-east-1 region; received ${webAclRegion}`, this);
}
}
}
Expand All @@ -681,13 +681,13 @@ export class Distribution extends Resource implements IDistribution {
const originId = originBindConfig.originProperty?.id ?? generatedId;
const duplicateId = this.boundOrigins.find(boundOrigin => boundOrigin.originProperty?.id === originBindConfig.originProperty?.id);
if (duplicateId) {
throw new Error(`Origin with id ${duplicateId.originProperty?.id} already exists. OriginIds must be unique within a distribution`);
throw new ValidationError(`Origin with id ${duplicateId.originProperty?.id} already exists. OriginIds must be unique within a distribution`, this);
}
if (!originBindConfig.failoverConfig) {
this.boundOrigins.push({ origin, originId, distributionId, ...originBindConfig });
} else {
if (isFailoverOrigin) {
throw new Error('An Origin cannot use an Origin with its own failover configuration as its fallback origin!');
throw new ValidationError('An Origin cannot use an Origin with its own failover configuration as its fallback origin!', this);
}
const groupIndex = this.originGroups.length + 1;
const originGroupId = Names.uniqueId(new Construct(this, `OriginGroup${groupIndex}`)).slice(-ORIGIN_ID_MAX_LENGTH);
Expand Down Expand Up @@ -716,7 +716,7 @@ export class Distribution extends Resource implements IDistribution {
): void {
statusCodes = statusCodes ?? [500, 502, 503, 504];
if (statusCodes.length === 0) {
throw new Error('fallbackStatusCodes cannot be empty');
throw new ValidationError('fallbackStatusCodes cannot be empty', this);
}
this.originGroups.push({
failoverCriteria: {
Expand Down Expand Up @@ -766,7 +766,7 @@ export class Distribution extends Resource implements IDistribution {

return this.errorResponses.map(errorConfig => {
if (!errorConfig.responseHttpStatus && !errorConfig.ttl && !errorConfig.responsePagePath) {
throw new Error('A custom error response without either a \'responseHttpStatus\', \'ttl\' or \'responsePagePath\' is not valid.');
throw new ValidationError('A custom error response without either a \'responseHttpStatus\', \'ttl\' or \'responsePagePath\' is not valid.', this);
}

return {
Expand All @@ -783,7 +783,7 @@ export class Distribution extends Resource implements IDistribution {
private renderLogging(props: DistributionProps): CfnDistribution.LoggingProperty | undefined {
if (!props.enableLogging && !props.logBucket) { return undefined; }
if (props.enableLogging === false && props.logBucket) {
throw new Error('Explicitly disabled logging but provided a logging bucket.');
throw new ValidationError('Explicitly disabled logging but provided a logging bucket.', this);
}

const bucket = props.logBucket ?? new s3.Bucket(this, 'LoggingBucket', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Stack,
Stage,
Token,
ValidationError,
} from '../../../core';
import { addConstructMetadata, MethodMetadata } from '../../../core/lib/metadata-resource';
import { CrossRegionStringParamReaderProvider } from '../../../custom-resource-handlers/dist/aws-cloudfront/cross-region-string-param-reader-provider.generated';
Expand Down Expand Up @@ -104,10 +105,10 @@ export class EdgeFunction extends Resource implements lambda.IVersion {
* Not supported. Connections are only applicable to VPC-enabled functions.
*/
public get connections(): ec2.Connections {
throw new Error('Lambda@Edge does not support connections');
throw new ValidationError('Lambda@Edge does not support connections', this);
}
public get latestVersion(): lambda.IVersion {
throw new Error('$LATEST function version cannot be used for Lambda@Edge');
throw new ValidationError('$LATEST function version cannot be used for Lambda@Edge', this);
}

@MethodMetadata()
Expand Down Expand Up @@ -188,7 +189,7 @@ export class EdgeFunction extends Resource implements lambda.IVersion {
private createCrossRegionFunction(id: string, props: EdgeFunctionProps): FunctionConfig {
const parameterNamePrefix = 'cdk/EdgeFunctionArn';
if (Token.isUnresolved(this.env.region)) {
throw new Error('stacks which use EdgeFunctions must have an explicitly set region');
throw new ValidationError('stacks which use EdgeFunctions must have an explicitly set region', this);
}
// SSM parameter names must only contain letters, numbers, ., _, -, or /.
const sanitizedPath = this.node.path.replace(/[^\/\w.-]/g, '_');
Expand Down Expand Up @@ -254,7 +255,7 @@ export class EdgeFunction extends Resource implements lambda.IVersion {
private edgeStack(stackId?: string): Stack {
const stage = Stage.of(this);
if (!stage) {
throw new Error('stacks which use EdgeFunctions must be part of a CDK app or stage');
throw new ValidationError('stacks which use EdgeFunctions must be part of a CDK app or stage', this);
}

const edgeStackId = stackId ?? `edge-lambda-stack-${this.stack.node.addr}`;
Expand Down
Loading

0 comments on commit c08c7f0

Please sign in to comment.