fix(ecs-patterns): minHealthyPercent and maxHealthyPercent props …

…validation (aws#26193) Setting `maxHealthyPercent` to a non-integer value was not raising synth-time errors, but was generating invalid CFN templates. This fix adds validation for both `maxHealthyPercent` and `minHealthyPercent`. Closes aws#26158. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
bmoffatt · Jul 28, 2023 · 978cbbb · 978cbbb
1 parent bff062d
commit 978cbbb
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 1 deletion.
diff --git a/...ges/aws-cdk-lib/aws-ecs-patterns/lib/fargate/application-load-balanced-fargate-service.ts b/...ges/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';
@@ -96,6 +96,19 @@ 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);
+
+    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.');
+    }
+
     const desiredCount = FeatureFlags.of(this).isEnabled(cxapi.ECS_REMOVE_DEFAULT_DESIRED_COUNT) ? this.internalDesiredCount : this.desiredCount;
 
     this.service = new FargateService(this, 'Service', {
@@ -120,4 +133,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 || Token.isUnresolved(value)) { 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
@@ -1208,3 +1208,55 @@ 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/);
+});
+
+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/);
+});