Skip to content

Commit

Permalink
feat(elbv2): throw ValidationError intsead of untyped errors (#33111)
Browse files Browse the repository at this point in the history
### Issue 

`aws-elasticloadbalancing*` 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 24, 2025
1 parent b1dfdd2 commit cc1988a
Show file tree
Hide file tree
Showing 19 changed files with 125 additions and 103 deletions.
22 changes: 13 additions & 9 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,29 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [

// no-throw-default-error
const enableNoThrowDefaultErrorIn = [
'aws-apigatewayv2-integrations',
'aws-apigatewayv2-authorizers',
'aws-apigatewayv2-integrations',
'aws-elasticloadbalancing',
'aws-elasticloadbalancingv2',
'aws-elasticloadbalancingv2-actions',
'aws-elasticloadbalancingv2-targets',
'aws-lambda',
'aws-rds',
'aws-s3',
'aws-sns',
'aws-sqs',
'aws-ssm',
'aws-ssmcontacts',
'aws-ssmincidents',
'aws-ssmquicksetup',
'aws-synthetics',
'aws-route53',
'aws-route53-patterns',
'aws-route53-targets',
'aws-route53profiles',
'aws-route53recoverycontrol',
'aws-route53recoveryreadiness',
'aws-route53resolver',
'aws-sns',
'aws-sqs',
'aws-ssm',
'aws-ssmcontacts',
'aws-ssmincidents',
'aws-ssmquicksetup',
'aws-synthetics',
'aws-s3',
'aws-s3-assets',
'aws-s3-deployment',
'aws-s3-notifications',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
SecurityGroup, SelectedSubnets, SubnetSelection, SubnetType,
} from '../../aws-ec2';
import { Duration, Lazy, Resource } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* Construction properties for a LoadBalancer
Expand Down Expand Up @@ -289,9 +290,9 @@ export class LoadBalancer extends Resource implements IConnectable {
*/
public addListener(listener: LoadBalancerListener): ListenerPort {
if (listener.sslCertificateArn && listener.sslCertificateId) {
throw new Error('"sslCertificateId" is deprecated, please use "sslCertificateArn" only.');
throw new ValidationError('"sslCertificateId" is deprecated, please use "sslCertificateArn" only.', this);
}
const protocol = ifUndefinedLazy(listener.externalProtocol, () => wellKnownProtocol(listener.externalPort));
const protocol = ifUndefinedLazy(listener.externalProtocol, () => wellKnownProtocol(this, listener.externalPort));
const instancePort = listener.internalPort || listener.externalPort;
const sslCertificateArn = listener.sslCertificateArn || listener.sslCertificateId;
const instanceProtocol = ifUndefined(listener.internalProtocol,
Expand Down Expand Up @@ -449,10 +450,10 @@ export class ListenerPort implements IConnectable {
}
}

function wellKnownProtocol(port: number): LoadBalancingProtocol {
function wellKnownProtocol(scope: Construct, port: number): LoadBalancingProtocol {
const proto = tryWellKnownProtocol(port);
if (!proto) {
throw new Error(`Please supply protocol to go with port ${port}`);
throw new ValidationError(`Please supply protocol to go with port ${port}`, scope);
}
return proto;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { IApplicationListener } from './application-listener';
import { IApplicationTargetGroup } from './application-target-group';
import { Port } from '../../../aws-ec2';
import { Duration, SecretValue, Token, Tokenization } from '../../../core';
import { UnscopedValidationError } from '../../../core/lib/errors';
import { CfnListener, CfnListenerRule } from '../elasticloadbalancingv2.generated';
import { IListenerAction } from '../shared/listener-action';

Expand Down Expand Up @@ -39,7 +40,7 @@ export class ListenerAction implements IListenerAction {
*/
public static forward(targetGroups: IApplicationTargetGroup[], options: ForwardOptions = {}): ListenerAction {
if (targetGroups.length === 0) {
throw new Error('Need at least one targetGroup in a ListenerAction.forward()');
throw new UnscopedValidationError('Need at least one targetGroup in a ListenerAction.forward()');
}
if (targetGroups.length === 1 && options.stickinessDuration === undefined) {
// Render a "simple" action for backwards compatibility with old templates
Expand All @@ -59,7 +60,7 @@ export class ListenerAction implements IListenerAction {
*/
public static weightedForward(targetGroups: WeightedTargetGroup[], options: ForwardOptions = {}): ListenerAction {
if (targetGroups.length === 0) {
throw new Error('Need at least one targetGroup in a ListenerAction.weightedForward()');
throw new UnscopedValidationError('Need at least one targetGroup in a ListenerAction.weightedForward()');
}

return new TargetGroupListenerAction(targetGroups.map(g => g.targetGroup), {
Expand Down Expand Up @@ -113,10 +114,10 @@ export class ListenerAction implements IListenerAction {
*/
public static redirect(options: RedirectOptions): ListenerAction {
if ([options.host, options.path, options.port, options.protocol, options.query].findIndex(x => x !== undefined) === -1) {
throw new Error('To prevent redirect loops, set at least one of \'protocol\', \'host\', \'port\', \'path\', or \'query\'.');
throw new UnscopedValidationError('To prevent redirect loops, set at least one of \'protocol\', \'host\', \'port\', \'path\', or \'query\'.');
}
if (options.path && !Token.isUnresolved(options.path) && !options.path.startsWith('/')) {
throw new Error(`Redirect path must start with a \'/\', got: ${options.path}`);
throw new UnscopedValidationError(`Redirect path must start with a \'/\', got: ${options.path}`);
}

return new ListenerAction({
Expand Down Expand Up @@ -200,7 +201,7 @@ export class ListenerAction implements IListenerAction {
*/
protected addRuleAction(actionJson: CfnListenerRule.ActionProperty) {
if (this._actionJson) {
throw new Error('rule action is already set');
throw new UnscopedValidationError('rule action is already set');
}
this._actionJson = actionJson;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Construct } from 'constructs';
import { IApplicationListener } from './application-listener';
import { ValidationError } from '../../../core/lib/errors';
import { CfnListenerCertificate } from '../elasticloadbalancingv2.generated';
import { IListenerCertificate } from '../shared/listener-certificate';

Expand Down Expand Up @@ -40,7 +41,7 @@ export class ApplicationListenerCertificate extends Construct {
super(scope, id);

if (!props.certificateArns && !props.certificates) {
throw new Error('At least one of \'certificateArns\' or \'certificates\' is required');
throw new ValidationError('At least one of \'certificateArns\' or \'certificates\' is required', this);
}

const certificates = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ListenerAction } from './application-listener-action';
import { IApplicationTargetGroup } from './application-target-group';
import { ListenerCondition } from './conditions';
import * as cdk from '../../../core';
import { UnscopedValidationError, ValidationError } from '../../../core/lib/errors';
import { CfnListenerRule } from '../elasticloadbalancingv2.generated';
import { IListenerAction } from '../shared/listener-action';

Expand Down Expand Up @@ -216,17 +217,17 @@ export class ApplicationListenerRule extends Construct {

const hasPathPatterns = props.pathPatterns || props.pathPattern;
if (this.conditions.length === 0 && !props.hostHeader && !hasPathPatterns) {
throw new Error('At least one of \'conditions\', \'hostHeader\', \'pathPattern\' or \'pathPatterns\' is required when defining a load balancing rule.');
throw new ValidationError('At least one of \'conditions\', \'hostHeader\', \'pathPattern\' or \'pathPatterns\' is required when defining a load balancing rule.', this);
}

const possibleActions: Array<keyof ApplicationListenerRuleProps> = ['action', 'targetGroups', 'fixedResponse', 'redirectResponse'];
const providedActions = possibleActions.filter(action => props[action] !== undefined);
if (providedActions.length > 1) {
throw new Error(`'${providedActions}' specified together, specify only one`);
throw new ValidationError(`'${providedActions}' specified together, specify only one`, this);
}

if (!cdk.Token.isUnresolved(props.priority) && props.priority <= 0) {
throw new Error('Priority must have value greater than or equal to 1');
throw new ValidationError('Priority must have value greater than or equal to 1', this);
}

this.listener = props.listener;
Expand All @@ -244,7 +245,7 @@ export class ApplicationListenerRule extends Construct {

if (hasPathPatterns) {
if (props.pathPattern && props.pathPatterns) {
throw new Error('Both `pathPatterns` and `pathPattern` are specified, specify only one');
throw new ValidationError('Both `pathPatterns` and `pathPattern` are specified, specify only one', this);
}
const pathPattern = props.pathPattern ? [props.pathPattern] : props.pathPatterns;
this.setCondition('path-pattern', pathPattern);
Expand Down Expand Up @@ -393,11 +394,11 @@ export class ApplicationListenerRule extends Construct {
*/
function validateFixedResponse(fixedResponse: FixedResponse) {
if (fixedResponse.statusCode && !/^(2|4|5)\d\d$/.test(fixedResponse.statusCode)) {
throw new Error('`statusCode` must be 2XX, 4XX or 5XX.');
throw new UnscopedValidationError('`statusCode` must be 2XX, 4XX or 5XX.');
}

if (fixedResponse.messageBody && fixedResponse.messageBody.length > 1024) {
throw new Error('`messageBody` cannot have more than 1024 characters.');
throw new UnscopedValidationError('`messageBody` cannot have more than 1024 characters.');
}
}

Expand All @@ -408,10 +409,10 @@ function validateFixedResponse(fixedResponse: FixedResponse) {
*/
function validateRedirectResponse(redirectResponse: RedirectResponse) {
if (redirectResponse.protocol && !/^(HTTPS?|#\{protocol\})$/i.test(redirectResponse.protocol)) {
throw new Error('`protocol` must be HTTP, HTTPS, or #{protocol}.');
throw new UnscopedValidationError('`protocol` must be HTTP, HTTPS, or #{protocol}.');
}

if (!redirectResponse.statusCode || !/^HTTP_30[12]$/.test(redirectResponse.statusCode)) {
throw new Error('`statusCode` must be HTTP_301 or HTTP_302.');
throw new UnscopedValidationError('`statusCode` must be HTTP_301 or HTTP_302.');
}
}
Loading

0 comments on commit cc1988a

Please sign in to comment.