From 54e42130e659e67e196e992405d44425717cf1ea Mon Sep 17 00:00:00 2001 From: Mina Asham Date: Mon, 14 Mar 2022 11:36:09 +0000 Subject: [PATCH 1/2] feat(elbv2): add name validation for target group and load balancer names - Target group naming rules: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-name - Load balancer naming rules: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-loadbalancer.html#cfn-elasticloadbalancingv2-loadbalancer-name --- .../lib/shared/base-load-balancer.ts | 4 ++ .../lib/shared/base-target-group.ts | 4 ++ .../test/nlb/load-balancer.test.ts | 70 +++++++++++++++++++ .../test/nlb/target-group.test.ts | 60 ++++++++++++++++ 4 files changed, 138 insertions(+) diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts index 0fcb0f4916c87..def53aa86bacf 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts @@ -216,6 +216,10 @@ export abstract class BaseLoadBalancer extends Resource { this.vpc = baseProps.vpc; + if (!Token.isUnresolved(baseProps.loadBalancerName) && baseProps.loadBalancerName !== undefined && (baseProps.loadBalancerName.length > 32 || baseProps.loadBalancerName.startsWith('internal-') || !/^[0-9a-z]+[0-9a-z-]*[0-9a-z]+$/i.test(baseProps.loadBalancerName))) { + throw new Error(`Load balancer name: "${baseProps.loadBalancerName}" is not allowed. Load balancer name can have a maximum of 32 characters, must contain only alphanumeric characters or hyphens, must not begin or end with a hyphen, and must not begin with "internal-".`); + } + const resource = new CfnLoadBalancer(this, 'Resource', { name: this.physicalName, subnets: subnetIds, diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts index 8897540d7910c..069b0dc006764 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts @@ -245,6 +245,10 @@ export abstract class TargetGroupBase extends CoreConstruct implements ITargetGr this.setAttribute('deregistration_delay.timeout_seconds', baseProps.deregistrationDelay.toSeconds().toString()); } + if (!cdk.Token.isUnresolved(baseProps.targetGroupName) && baseProps.targetGroupName !== undefined && (baseProps.targetGroupName.length > 32 || !/^[0-9a-z]+[0-9a-z-]*[0-9a-z]+$/i.test(baseProps.targetGroupName))) { + throw new Error(`Target group name: "${baseProps.targetGroupName}" is not allowed. Target group name can have a maximum of 32 characters, must contain only alphanumeric characters or hyphens, and must not begin or end with a hyphen.`); + } + this.healthCheck = baseProps.healthCheck || {}; this.vpc = baseProps.vpc; this.targetType = baseProps.targetType; diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts index 2454cd1ccfb01..ed1d27267c5ea 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts @@ -241,6 +241,76 @@ describe('tests', () => { }); }); + test('loadBalancerName unallowed: starts with "internal-"', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'internal-myLoadBalancer', + vpc, + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: more than 32 characters', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'a'.repeat(33), + vpc, + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: unallowed characters', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'my load balancer', + vpc, + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: ends with hyphen', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'myLoadBalancer-', + vpc, + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: starts with hyphen', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: '-myLoadBalancer', + vpc, + }), + ).toThrow(); + }); + test('imported network load balancer with no vpc specified throws error when calling addTargets', () => { // GIVEN const stack = new cdk.Stack(); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts index 7caaf465b045d..c241f95f771ee 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts @@ -144,6 +144,66 @@ describe('tests', () => { }).toThrow(/Health check interval '5' not supported. Must be one of the following values '10,30'./); }); + test('loadBalancerName unallowed: more than 32 characters', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: 'a'.repeat(33), + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: unallowed characters', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: 'my target group', + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: ends with hyphen', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: 'myTargetGroup-', + }), + ).toThrow(); + }); + + test('loadBalancerName unallowed: starts with hyphen', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'Stack'); + + // WHEN/THEN + expect(() => + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: '-myTargetGroup', + }), + ).toThrow(); + }); + test.each([elbv2.Protocol.UDP, elbv2.Protocol.TCP_UDP, elbv2.Protocol.TLS])( 'Throws validation error, when `healthCheck` has `protocol` set to %s', (protocol) => { From f129252baa8374117fd6590df12aef8a8d3e7a15 Mon Sep 17 00:00:00 2001 From: Mina Asham Date: Mon, 14 Mar 2022 15:32:01 +0000 Subject: [PATCH 2/2] Move name validation into "validate" method and split error checks and messages into granular ones --- .../lib/shared/base-load-balancer.ts | 27 ++++- .../lib/shared/base-target-group.ts | 18 ++- .../test/nlb/load-balancer.test.ts | 108 +++++++++++------- .../test/nlb/target-group.test.ts | 96 +++++++++------- 4 files changed, 157 insertions(+), 92 deletions(-) diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts index def53aa86bacf..b7e13aafae83d 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts @@ -216,10 +216,6 @@ export abstract class BaseLoadBalancer extends Resource { this.vpc = baseProps.vpc; - if (!Token.isUnresolved(baseProps.loadBalancerName) && baseProps.loadBalancerName !== undefined && (baseProps.loadBalancerName.length > 32 || baseProps.loadBalancerName.startsWith('internal-') || !/^[0-9a-z]+[0-9a-z-]*[0-9a-z]+$/i.test(baseProps.loadBalancerName))) { - throw new Error(`Load balancer name: "${baseProps.loadBalancerName}" is not allowed. Load balancer name can have a maximum of 32 characters, must contain only alphanumeric characters or hyphens, must not begin or end with a hyphen, and must not begin with "internal-".`); - } - const resource = new CfnLoadBalancer(this, 'Resource', { name: this.physicalName, subnets: subnetIds, @@ -304,4 +300,27 @@ export abstract class BaseLoadBalancer extends Resource { public removeAttribute(key: string) { this.setAttribute(key, undefined); } + + protected validate(): string[] { + const ret = super.validate(); + + // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-loadbalancer.html#cfn-elasticloadbalancingv2-loadbalancer-name + const loadBalancerName = this.physicalName; + if (!Token.isUnresolved(loadBalancerName) && loadBalancerName !== undefined) { + if (loadBalancerName.length > 32) { + ret.push(`Load balancer name: "${loadBalancerName}" can have a maximum of 32 characters.`); + } + if (loadBalancerName.startsWith('internal-')) { + ret.push(`Load balancer name: "${loadBalancerName}" must not begin with "internal-".`); + } + if (loadBalancerName.startsWith('-') || loadBalancerName.endsWith('-')) { + ret.push(`Load balancer name: "${loadBalancerName}" must not begin or end with a hyphen.`); + } + if (!/^[0-9a-z-]+$/i.test(loadBalancerName)) { + ret.push(`Load balancer name: "${loadBalancerName}" must contain only alphanumeric characters or hyphens.`); + } + } + + return ret; + } } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts index 069b0dc006764..0fa47d8670e05 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-target-group.ts @@ -245,10 +245,6 @@ export abstract class TargetGroupBase extends CoreConstruct implements ITargetGr this.setAttribute('deregistration_delay.timeout_seconds', baseProps.deregistrationDelay.toSeconds().toString()); } - if (!cdk.Token.isUnresolved(baseProps.targetGroupName) && baseProps.targetGroupName !== undefined && (baseProps.targetGroupName.length > 32 || !/^[0-9a-z]+[0-9a-z-]*[0-9a-z]+$/i.test(baseProps.targetGroupName))) { - throw new Error(`Target group name: "${baseProps.targetGroupName}" is not allowed. Target group name can have a maximum of 32 characters, must contain only alphanumeric characters or hyphens, and must not begin or end with a hyphen.`); - } - this.healthCheck = baseProps.healthCheck || {}; this.vpc = baseProps.vpc; this.targetType = baseProps.targetType; @@ -343,6 +339,20 @@ export abstract class TargetGroupBase extends CoreConstruct implements ITargetGr ret.push("'vpc' is required for a non-Lambda TargetGroup"); } + // https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-elasticloadbalancingv2-targetgroup.html#cfn-elasticloadbalancingv2-targetgroup-name + const targetGroupName = this.resource.name; + if (!cdk.Token.isUnresolved(targetGroupName) && targetGroupName !== undefined) { + if (targetGroupName.length > 32) { + ret.push(`Target group name: "${targetGroupName}" can have a maximum of 32 characters.`); + } + if (targetGroupName.startsWith('-') || targetGroupName.endsWith('-')) { + ret.push(`Target group name: "${targetGroupName}" must not begin or end with a hyphen.`); + } + if (!/^[0-9a-z-]+$/i.test(targetGroupName)) { + ret.push(`Target group name: "${targetGroupName}" must contain only alphanumeric characters or hyphens.`); + } + } + return ret; } } diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts index ed1d27267c5ea..b0509e9a4e894 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/load-balancer.test.ts @@ -241,74 +241,94 @@ describe('tests', () => { }); }); - test('loadBalancerName unallowed: starts with "internal-"', () => { + test('loadBalancerName unallowed: more than 32 characters', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkLoadBalancer(stack, 'NLB', { - loadBalancerName: 'internal-myLoadBalancer', - vpc, - }), - ).toThrow(); + // WHEN + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'a'.repeat(33), + vpc, + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Load balancer name: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" can have a maximum of 32 characters.'); }); - test('loadBalancerName unallowed: more than 32 characters', () => { + test('loadBalancerName unallowed: starts with "internal-"', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkLoadBalancer(stack, 'NLB', { - loadBalancerName: 'a'.repeat(33), - vpc, - }), - ).toThrow(); + // WHEN + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'internal-myLoadBalancer', + vpc, + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Load balancer name: "internal-myLoadBalancer" must not begin with "internal-".'); }); - test('loadBalancerName unallowed: unallowed characters', () => { + test('loadBalancerName unallowed: starts with hyphen', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkLoadBalancer(stack, 'NLB', { - loadBalancerName: 'my load balancer', - vpc, - }), - ).toThrow(); + // WHEN + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: '-myLoadBalancer', + vpc, + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Load balancer name: "-myLoadBalancer" must not begin or end with a hyphen.'); }); test('loadBalancerName unallowed: ends with hyphen', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkLoadBalancer(stack, 'NLB', { - loadBalancerName: 'myLoadBalancer-', - vpc, - }), - ).toThrow(); + // WHEN + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'myLoadBalancer-', + vpc, + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Load balancer name: "myLoadBalancer-" must not begin or end with a hyphen.'); }); - test('loadBalancerName unallowed: starts with hyphen', () => { + test('loadBalancerName unallowed: unallowed characters', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkLoadBalancer(stack, 'NLB', { - loadBalancerName: '-myLoadBalancer', - vpc, - }), - ).toThrow(); + // WHEN + new elbv2.NetworkLoadBalancer(stack, 'NLB', { + loadBalancerName: 'my load balancer', + vpc, + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Load balancer name: "my load balancer" must contain only alphanumeric characters or hyphens.'); }); test('imported network load balancer with no vpc specified throws error when calling addTargets', () => { diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts index c241f95f771ee..d7ac75aa2f78b 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/test/nlb/target-group.test.ts @@ -144,64 +144,80 @@ describe('tests', () => { }).toThrow(/Health check interval '5' not supported. Must be one of the following values '10,30'./); }); - test('loadBalancerName unallowed: more than 32 characters', () => { + test('targetGroupName unallowed: more than 32 characters', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkTargetGroup(stack, 'Group', { - vpc, - port: 80, - targetGroupName: 'a'.repeat(33), - }), - ).toThrow(); + // WHEN + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: 'a'.repeat(33), + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Target group name: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" can have a maximum of 32 characters.'); }); - test('loadBalancerName unallowed: unallowed characters', () => { + test('targetGroupName unallowed: starts with hyphen', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkTargetGroup(stack, 'Group', { - vpc, - port: 80, - targetGroupName: 'my target group', - }), - ).toThrow(); + // WHEN + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: '-myTargetGroup', + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Target group name: "-myTargetGroup" must not begin or end with a hyphen.'); }); - test('loadBalancerName unallowed: ends with hyphen', () => { + test('targetGroupName unallowed: ends with hyphen', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkTargetGroup(stack, 'Group', { - vpc, - port: 80, - targetGroupName: 'myTargetGroup-', - }), - ).toThrow(); + // WHEN + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: 'myTargetGroup-', + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Target group name: "myTargetGroup-" must not begin or end with a hyphen.'); }); - test('loadBalancerName unallowed: starts with hyphen', () => { + test('targetGroupName unallowed: unallowed characters', () => { // GIVEN - const stack = new cdk.Stack(); + const app = new cdk.App(); + const stack = new cdk.Stack(app); const vpc = new ec2.Vpc(stack, 'Stack'); - // WHEN/THEN - expect(() => - new elbv2.NetworkTargetGroup(stack, 'Group', { - vpc, - port: 80, - targetGroupName: '-myTargetGroup', - }), - ).toThrow(); + // WHEN + new elbv2.NetworkTargetGroup(stack, 'Group', { + vpc, + port: 80, + targetGroupName: 'my target group', + }); + + // THEN + expect(() => { + app.synth(); + }).toThrow('Target group name: "my target group" must contain only alphanumeric characters or hyphens.'); }); test.each([elbv2.Protocol.UDP, elbv2.Protocol.TCP_UDP, elbv2.Protocol.TLS])(