From 5aaa11fd947b8dd27f22963ac556b096350d7fa2 Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Sat, 1 Jul 2023 15:30:44 +0200 Subject: [PATCH 1/3] fix(ecs-patterns): minHealthyPercent and maxHealthyPercent props validation --- ...plication-load-balanced-fargate-service.ts | 13 +++++++ .../load-balanced-fargate-service.test.ts | 34 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts b/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts index 39131198893dd..dc34eb01cd341 100644 --- a/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts +++ b/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts @@ -96,6 +96,9 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc throw new Error('You must specify one of: taskDefinition or image'); } + this.validateHealthyPercentage('minHealthyPercent', props.minHealthyPercent); + this.validateHealthyPercentage('maxHealthyPercent', props.maxHealthyPercent); + const desiredCount = FeatureFlags.of(this).isEnabled(cxapi.ECS_REMOVE_DEFAULT_DESIRED_COUNT) ? this.internalDesiredCount : this.desiredCount; this.service = new FargateService(this, 'Service', { @@ -120,4 +123,14 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc }); this.addServiceAsTarget(this.service); } + + /** + * Throws an error if the specified percent is not an integer or negative. + */ + private validateHealthyPercentage(name: string, value?: number) { + if (value === undefined) { return; } + if (!Number.isInteger(value) || value < 0) { + throw new Error(`${name}: Must be a non-negative integer; received ${value}`); + } + } } diff --git a/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts b/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts index cf1425bb5fa3e..b2344741c9152 100644 --- a/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts +++ b/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts @@ -1208,3 +1208,37 @@ test('NetworkLoadBalancedFargateService multiple capacity provider strategies ar ]), }); }); + +test('should validate minHealthyPercent', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'VPC'); + const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); + + // WHEN + expect(() => new ecsPatterns.ApplicationLoadBalancedFargateService(stack, 'Service', { + cluster, + taskImageOptions: { + image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), + dockerLabels: { label1: 'labelValue1', label2: 'labelValue2' }, + }, + minHealthyPercent: 0.5, + })).toThrow(/Must be a non-negative integer/); +}); + +test('should validate maxHealthyPercent', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'VPC'); + const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); + + // WHEN + expect(() => new ecsPatterns.ApplicationLoadBalancedFargateService(stack, 'Service', { + cluster, + taskImageOptions: { + image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), + dockerLabels: { label1: 'labelValue1', label2: 'labelValue2' }, + }, + maxHealthyPercent: 0.5, + })).toThrow(/Must be a non-negative integer/); +}); From 46b8fafe527c04925317aded458b7554432cc778 Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Sat, 1 Jul 2023 16:19:38 +0200 Subject: [PATCH 2/3] added min < max validation --- ...pplication-load-balanced-fargate-service.ts | 4 ++++ .../load-balanced-fargate-service.test.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts b/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts index dc34eb01cd341..1aaa447c493db 100644 --- a/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts +++ b/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts @@ -99,6 +99,10 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc this.validateHealthyPercentage('minHealthyPercent', props.minHealthyPercent); this.validateHealthyPercentage('maxHealthyPercent', props.maxHealthyPercent); + if (props.minHealthyPercent && props.maxHealthyPercent && props.minHealthyPercent >= props.maxHealthyPercent) { + throw new Error('Minimum healthy percent must be less than maximum healthy percent.'); + } + const desiredCount = FeatureFlags.of(this).isEnabled(cxapi.ECS_REMOVE_DEFAULT_DESIRED_COUNT) ? this.internalDesiredCount : this.desiredCount; this.service = new FargateService(this, 'Service', { diff --git a/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts b/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts index b2344741c9152..66bf2bb589dcb 100644 --- a/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts +++ b/packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts @@ -1242,3 +1242,21 @@ test('should validate maxHealthyPercent', () => { maxHealthyPercent: 0.5, })).toThrow(/Must be a non-negative integer/); }); + +test('minHealthyPercent must be less than maxHealthyPercent', () => { + // GIVEN + const stack = new cdk.Stack(); + const vpc = new ec2.Vpc(stack, 'VPC'); + const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); + + // WHEN + expect(() => new ecsPatterns.ApplicationLoadBalancedFargateService(stack, 'Service', { + cluster, + taskImageOptions: { + image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'), + dockerLabels: { label1: 'labelValue1', label2: 'labelValue2' }, + }, + minHealthyPercent: 100, + maxHealthyPercent: 70, + })).toThrow(/must be less than/); +}); From d9090d5b92d80b4c22c4a52965935510a97a7aac Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Tue, 25 Jul 2023 09:05:06 +0200 Subject: [PATCH 3/3] added token resolution checks --- .../application-load-balanced-fargate-service.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts b/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts index 1aaa447c493db..c135666d47fcc 100644 --- a/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts +++ b/packages/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts @@ -1,7 +1,7 @@ import { Construct } from 'constructs'; import { ISecurityGroup, SubnetSelection } from '../../../aws-ec2'; import { FargateService, FargateTaskDefinition } from '../../../aws-ecs'; -import { FeatureFlags } from '../../../core'; +import { FeatureFlags, Token } from '../../../core'; import * as cxapi from '../../../cx-api'; import { ApplicationLoadBalancedServiceBase, ApplicationLoadBalancedServiceBaseProps } from '../base/application-load-balanced-service-base'; import { FargateServiceBaseProps } from '../base/fargate-service-base'; @@ -99,7 +99,13 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc this.validateHealthyPercentage('minHealthyPercent', props.minHealthyPercent); this.validateHealthyPercentage('maxHealthyPercent', props.maxHealthyPercent); - if (props.minHealthyPercent && props.maxHealthyPercent && props.minHealthyPercent >= props.maxHealthyPercent) { + if ( + props.minHealthyPercent && + !Token.isUnresolved(props.minHealthyPercent) && + props.maxHealthyPercent && + !Token.isUnresolved(props.maxHealthyPercent) && + props.minHealthyPercent >= props.maxHealthyPercent + ) { throw new Error('Minimum healthy percent must be less than maximum healthy percent.'); } @@ -132,7 +138,7 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc * Throws an error if the specified percent is not an integer or negative. */ private validateHealthyPercentage(name: string, value?: number) { - if (value === undefined) { return; } + if (value === undefined || Token.isUnresolved(value)) { return; } if (!Number.isInteger(value) || value < 0) { throw new Error(`${name}: Must be a non-negative integer; received ${value}`); }