From 6d7184e862d4fb16df6517874fdcaf2d56a5bfa6 Mon Sep 17 00:00:00 2001 From: Christopher Rybicki Date: Thu, 19 May 2022 15:50:21 -0700 Subject: [PATCH 1/2] chore(ecs-patterns): revert "feature flag missing for breaking change passing container port for target group port (#20284)" This reverts commit dc7533cab37b69ac8d3b2c7a6b093da90bcbb911. --- .../network-load-balanced-service-base.ts | 11 +- ...ork-multiple-target-groups-service-base.ts | 5 +- .../load-balanced-fargate-service-v2.test.ts | 150 +----------------- packages/@aws-cdk/cx-api/lib/features.ts | 10 -- 4 files changed, 12 insertions(+), 164 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 fc71eed3c45d1..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 @@ -7,8 +7,7 @@ import { INetworkLoadBalancer, NetworkListener, NetworkLoadBalancer, NetworkTarg import { IRole } from '@aws-cdk/aws-iam'; import { ARecord, CnameRecord, IHostedZone, RecordTarget } from '@aws-cdk/aws-route53'; import { LoadBalancerTarget } from '@aws-cdk/aws-route53-targets'; -import { CfnOutput, Duration, FeatureFlags, Stack } from '@aws-cdk/core'; -import { ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT } from '@aws-cdk/cx-api'; +import * as cdk from '@aws-cdk/core'; import { Construct } from 'constructs'; // keep this import separate from other imports to reduce chance for merge conflicts with v2-main @@ -104,7 +103,7 @@ export interface NetworkLoadBalancedServiceBaseProps { * * @default - defaults to 60 seconds if at least one load balancer is in-use and it is not already set */ - readonly healthCheckGracePeriod?: Duration; + readonly healthCheckGracePeriod?: cdk.Duration; /** * The maximum number of tasks, specified as a percentage of the Amazon ECS @@ -348,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: FeatureFlags.of(this).isEnabled(ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT) ? props.taskImageOptions?.containerPort ?? 80 : 80, + port: props.taskImageOptions?.containerPort ?? 80, }; this.listener = loadBalancer.addListener('PublicListener', { port: listenerPort }); @@ -385,7 +384,7 @@ export abstract class NetworkLoadBalancedServiceBase extends CoreConstruct { } if (props.loadBalancer === undefined) { - new CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName }); + new cdk.CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName }); } } @@ -395,7 +394,7 @@ export abstract class NetworkLoadBalancedServiceBase extends CoreConstruct { protected getDefaultCluster(scope: CoreConstruct, vpc?: IVpc): Cluster { // magic string to avoid collision with user-defined constructs const DEFAULT_CLUSTER_ID = `EcsDefaultClusterMnL3mNNYN${vpc ? vpc.node.id : ''}`; - const stack = Stack.of(scope); + const stack = cdk.Stack.of(scope); return stack.node.tryFindChild(DEFAULT_CLUSTER_ID) as Cluster || new Cluster(stack, DEFAULT_CLUSTER_ID, { vpc }); } 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 3febc79520079..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 @@ -7,8 +7,7 @@ import { NetworkListener, NetworkLoadBalancer, NetworkTargetGroup } from '@aws-c import { IRole } from '@aws-cdk/aws-iam'; import { ARecord, IHostedZone, RecordTarget } from '@aws-cdk/aws-route53'; import { LoadBalancerTarget } from '@aws-cdk/aws-route53-targets'; -import { CfnOutput, Duration, FeatureFlags, Stack } from '@aws-cdk/core'; -import { ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT } from '@aws-cdk/cx-api'; +import { CfnOutput, Duration, Stack } from '@aws-cdk/core'; import { Construct } from 'constructs'; // v2 - keep this import as a separate section to reduce merge conflict when forward merging with the v2 branch. @@ -375,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: FeatureFlags.of(this).isEnabled(ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT) ? targetProps.containerPort ?? 80 : 80, + port: targetProps.containerPort ?? 80, targets: [ service.loadBalancerTarget({ containerName: container.containerName, 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 8ab4b8b8ab34a..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 @@ -3,9 +3,7 @@ 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 { testFutureBehavior, testLegacyBehavior } from '@aws-cdk/cdk-build-tools'; -import { App, Duration, Stack } from '@aws-cdk/core'; -import { ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT } from '@aws-cdk/cx-api'; +import { Duration, Stack } from '@aws-cdk/core'; import { ApplicationLoadBalancedFargateService, ApplicationMultipleTargetGroupsFargateService, NetworkLoadBalancedFargateService, NetworkMultipleTargetGroupsFargateService } from '../../lib'; describe('When Application Load Balancer', () => { @@ -665,36 +663,9 @@ describe('When Network Load Balancer', () => { }).toThrow(/You must specify one of: taskDefinition or image/); }); - testLegacyBehavior('Fargate neworkloadbalanced construct uses Port 80 for target group when feature flag is not enabled', App, (app) => { + test('test Fargate networkloadbalanced construct with custom Port', () => { // GIVEN - const stack = new Stack(app); - 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: 80, - Protocol: 'TCP', - TargetType: 'ip', - VpcId: { - Ref: 'VPCB9E5F0B4', - }, - }); - }); - - testFutureBehavior('Fargate networkloadbalanced construct uses custom Port for target group when feature flag is enabled', { [ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT]: true }, App, (app) => { - // GIVEN - const stack = new Stack(app); + const stack = new Stack(); const vpc = new Vpc(stack, 'VPC'); const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); @@ -719,79 +690,9 @@ describe('When Network Load Balancer', () => { }); }); - testFutureBehavior('Fargate networkloadbalanced construct uses 80 for target group when feature flag is enabled but container port is not provided', { [ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT]: true }, App, (app) => { - // GIVEN - const stack = new Stack(app); - 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'), - }, - listenerPort: 8181, - }); - - Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', { - Port: 80, - Protocol: 'TCP', - TargetType: 'ip', - VpcId: { - Ref: 'VPCB9E5F0B4', - }, - }); - }); - - testLegacyBehavior('Fargate multinetworkloadbalanced construct uses Port 80 for target group when feature flag is not enabled', App, (app) => { - // GIVEN - const stack = new Stack(app); - 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: 80, - Protocol: 'TCP', - TargetType: 'ip', - VpcId: { - Ref: 'VPCB9E5F0B4', - }, - }); - }); - - testFutureBehavior('test Fargate multinetworkloadbalanced construct uses custom Port for target group when feature flag is enabled', { [ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT]: true }, App, (app) => { + test('test Fargate multinetworkloadbalanced construct with custom Port', () => { // GIVEN - const stack = new Stack(app); + const stack = new Stack(); const vpc = new Vpc(stack, 'VPC'); const cluster = new ecs.Cluster(stack, 'Cluster', { vpc }); @@ -832,45 +733,4 @@ describe('When Network Load Balancer', () => { }, }); }); - - testFutureBehavior('test Fargate multinetworkloadbalanced construct uses 80 for target group when feature flag is enabled but container port is not provided', { [ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT]: true }, App, (app) => { - // GIVEN - const stack = new Stack(app); - 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 }, - ], - }, - ], - }); - - Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', { - Port: 80, - Protocol: 'TCP', - TargetType: 'ip', - VpcId: { - Ref: 'VPCB9E5F0B4', - }, - }); - }); }); diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index ad3341853eaf2..21918585ae183 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -233,15 +233,6 @@ export const EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME = '@aws-cdk/aws-ec2:uniqueIm */ export const IAM_MINIMIZE_POLICIES = '@aws-cdk/aws-iam:minimizePolicies'; -/** - * Enable this feature flag to pass through the `NetworkLoadBalancedServiceProps.taskImageOptions.containerPort` - * and the `NetworkMultipleTargetGroupsServiceProps.targetGroups[X].containerPort` to the generated - * `ElasticLoadBalancingV2::TargetGroup`'s `Port` property. - * - * This is a feature flag because updating `Port` causes a replacement of the target groups, which is a breaking change. - */ -export const ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT = '@aws-cdk/aws-ecs-patterns:containerPortToTargetGroupPort'; - /** * Flag values that should apply for new projects * @@ -269,7 +260,6 @@ export const FUTURE_FLAGS: { [key: string]: boolean } = { [EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: true, [CHECK_SECRET_USAGE]: true, [IAM_MINIMIZE_POLICIES]: true, - [ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT]: true, }; /** From d9d9abfca4ea29e6b7b69ac855d67721678fa2fb Mon Sep 17 00:00:00 2001 From: Christopher Rybicki Date: Thu, 19 May 2022 16:29:16 -0700 Subject: [PATCH 2/2] flakey test perhaps? (empty commit)