From 13937299596d0b858d56e9116bf7a7dbe039d4b4 Mon Sep 17 00:00:00 2001 From: Lukas Fruntke Date: Fri, 21 Jan 2022 04:14:18 +0100 Subject: [PATCH] fix(ecs-patterns): Fix Network Load Balancer Port assignments in ECS Patterns (#18157) This PR introduces changeable ports as a regression fix for the hardcoded port 80 in both NLB constructs bases. Closes #18073 Additionally it seems like the regression reported in the linked issue was not spotted in the integration tests either. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../network-load-balanced-service-base.ts | 2 +- ...ork-multiple-target-groups-service-base.ts | 2 +- ...oad-balanced-fargate-service.expected.json | 2 +- .../integ.special-listener.expected.json | 2 +- .../load-balanced-fargate-service-v2.test.ts | 74 ++++++++++++++++++- 5 files changed, 77 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts index 0857b99ee8cfd..942f13e3439aa 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts @@ -347,7 +347,7 @@ export abstract class NetworkLoadBalancedServiceBase extends CoreConstruct { const loadBalancer = props.loadBalancer ?? new NetworkLoadBalancer(this, 'LB', lbProps); const listenerPort = props.listenerPort ?? 80; const targetProps = { - port: 80, + port: props.taskImageOptions?.containerPort ?? 80, }; this.listener = loadBalancer.addListener('PublicListener', { port: listenerPort }); diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts index 8eb751b07d13e..677caf8c2df9f 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts @@ -374,7 +374,7 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends CoreConstru protected registerECSTargets(service: BaseService, container: ContainerDefinition, targets: NetworkTargetProps[]): NetworkTargetGroup { for (const targetProps of targets) { const targetGroup = this.findListener(targetProps.listener).addTargets(`ECSTargetGroup${container.containerName}${targetProps.containerPort}`, { - port: 80, + port: targetProps.containerPort ?? 80, targets: [ service.loadBalancerTarget({ containerName: container.containerName, diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.multiple-network-load-balanced-fargate-service.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.multiple-network-load-balanced-fargate-service.expected.json index 643ff38905d69..91413b0f0fd6f 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.multiple-network-load-balanced-fargate-service.expected.json +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.multiple-network-load-balanced-fargate-service.expected.json @@ -458,7 +458,7 @@ "myServicelb2listener2ECSTargetGroupweb90Group6841F924": { "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", "Properties": { - "Port": 80, + "Port": 90, "Protocol": "TCP", "TargetType": "ip", "VpcId": { diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.special-listener.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.special-listener.expected.json index 91c1f4e575e23..1af60f7eaf55c 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.special-listener.expected.json +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.special-listener.expected.json @@ -404,7 +404,7 @@ "FargateNlbServiceLBPublicListenerECSGroup7501571D": { "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", "Properties": { - "Port": 80, + "Port": 2015, "Protocol": "TCP", "TargetType": "ip", "VpcId": { diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service-v2.test.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service-v2.test.ts index 191dbd4430236..b196f4b0616b1 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service-v2.test.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service-v2.test.ts @@ -1,9 +1,10 @@ import { Match, Template } from '@aws-cdk/assertions'; import { Vpc } from '@aws-cdk/aws-ec2'; import * as ecs from '@aws-cdk/aws-ecs'; +import { ContainerImage } from '@aws-cdk/aws-ecs'; import { CompositePrincipal, Role, ServicePrincipal } from '@aws-cdk/aws-iam'; import { Duration, Stack } from '@aws-cdk/core'; -import { ApplicationMultipleTargetGroupsFargateService, NetworkMultipleTargetGroupsFargateService, ApplicationLoadBalancedFargateService } from '../../lib'; +import { ApplicationLoadBalancedFargateService, ApplicationMultipleTargetGroupsFargateService, NetworkLoadBalancedFargateService, NetworkMultipleTargetGroupsFargateService } from '../../lib'; describe('When Application Load Balancer', () => { test('test Fargate loadbalanced construct with default settings', () => { @@ -661,4 +662,75 @@ describe('When Network Load Balancer', () => { }); }).toThrow(/You must specify one of: taskDefinition or image/); }); + + test('test Fargate networkloadbalanced construct with custom Port', () => { + // GIVEN + const stack = new Stack(); + const vpc = new Vpc(stack, 'VPC'); + const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); + + new NetworkLoadBalancedFargateService(stack, 'NLBService', { + cluster: cluster, + memoryLimitMiB: 1024, + cpu: 512, + taskImageOptions: { + image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'), + containerPort: 81, + }, + listenerPort: 8181, + }); + + Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', { + Port: 81, + Protocol: 'TCP', + TargetType: 'ip', + VpcId: { + Ref: 'VPCB9E5F0B4', + }, + }); + }); + + test('test Fargate multinetworkloadbalanced construct with custom Port', () => { + // GIVEN + const stack = new Stack(); + const vpc = new Vpc(stack, 'VPC'); + const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); + + new NetworkMultipleTargetGroupsFargateService(stack, 'Service', { + cluster, + taskImageOptions: { + image: ecs.ContainerImage.fromRegistry('test'), + }, + }); + + + new NetworkMultipleTargetGroupsFargateService(stack, 'NLBService', { + cluster: cluster, + memoryLimitMiB: 1024, + cpu: 512, + taskImageOptions: { + image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'), + }, + loadBalancers: [ + { + name: 'lb1', + listeners: [ + { name: 'listener1', port: 8181 }, + ], + }, + ], + targetGroups: [{ + containerPort: 81, + }], + }); + + Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', { + Port: 81, + Protocol: 'TCP', + TargetType: 'ip', + VpcId: { + Ref: 'VPCB9E5F0B4', + }, + }); + }); });