Skip to content

Commit 3a11317

Browse files
ChriscbrChriscbr
authored
chore(ecs-patterns): revert "feature flag missing for breaking change passing container port for target group port (#20284)" (#20430)
This reverts #20284 since its tests fail to pass in CDK v2, blocking the next CDK release. The root cause of failure looks as though it may be the same as #20427 - I've included the test logs below: <details> ``` @aws-cdk/aws-ecs-patterns: FAIL test/fargate/load-balanced-fargate-service-v2.test.js (11.703 s) @aws-cdk/aws-ecs-patterns: � When Network Load Balancer › Fargate networkloadbalanced construct uses custom Port for target group when feature flag is enabled @aws-cdk/aws-ecs-patterns: Template has 1 resources with type AWS::ElasticLoadBalancingV2::TargetGroup, but none match as expected. @aws-cdk/aws-ecs-patterns: The closest result is: @aws-cdk/aws-ecs-patterns: { @aws-cdk/aws-ecs-patterns: "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", @aws-cdk/aws-ecs-patterns: "Properties": { @aws-cdk/aws-ecs-patterns: "Port": 80, @aws-cdk/aws-ecs-patterns: "Protocol": "TCP", @aws-cdk/aws-ecs-patterns: "TargetType": "ip", @aws-cdk/aws-ecs-patterns: "VpcId": { @aws-cdk/aws-ecs-patterns: "Ref": "VPCB9E5F0B4" @aws-cdk/aws-ecs-patterns: } @aws-cdk/aws-ecs-patterns: } @aws-cdk/aws-ecs-patterns: } @aws-cdk/aws-ecs-patterns: with the following mismatches: @aws-cdk/aws-ecs-patterns: Expected 81 but received 80 at /Properties/Port (using objectLike matcher) @aws-cdk/aws-ecs-patterns: 83 | const matchError = hasResourceProperties(this.template, type, props); @aws-cdk/aws-ecs-patterns: 84 | if (matchError) { @aws-cdk/aws-ecs-patterns: > 85 | throw new Error(matchError); @aws-cdk/aws-ecs-patterns: | ^ @aws-cdk/aws-ecs-patterns: 86 | } @aws-cdk/aws-ecs-patterns: 87 | } @aws-cdk/aws-ecs-patterns: 88 | @aws-cdk/aws-ecs-patterns: at Template.hasResourceProperties (../assertions/lib/template.ts:85:13) @aws-cdk/aws-ecs-patterns: at fn (test/fargate/load-balanced-fargate-service-v2.test.ts:709:31) @aws-cdk/aws-ecs-patterns: at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35) @aws-cdk/aws-ecs-patterns: � When Network Load Balancer › test Fargate multinetworkloadbalanced construct uses custom Port for target group when feature flag is enabled @aws-cdk/aws-ecs-patterns: Template has 2 resources with type AWS::ElasticLoadBalancingV2::TargetGroup, but none match as expected. @aws-cdk/aws-ecs-patterns: The closest result is: @aws-cdk/aws-ecs-patterns: { @aws-cdk/aws-ecs-patterns: "Type": "AWS::ElasticLoadBalancingV2::TargetGroup", @aws-cdk/aws-ecs-patterns: "Properties": { @aws-cdk/aws-ecs-patterns: "Port": 80, @aws-cdk/aws-ecs-patterns: "Protocol": "TCP", @aws-cdk/aws-ecs-patterns: "TargetType": "ip", @aws-cdk/aws-ecs-patterns: "VpcId": { @aws-cdk/aws-ecs-patterns: "Ref": "VPCB9E5F0B4" @aws-cdk/aws-ecs-patterns: } @aws-cdk/aws-ecs-patterns: } @aws-cdk/aws-ecs-patterns: } @aws-cdk/aws-ecs-patterns: with the following mismatches: @aws-cdk/aws-ecs-patterns: Expected 81 but received 80 at /Properties/Port (using objectLike matcher) @aws-cdk/aws-ecs-patterns: 83 | const matchError = hasResourceProperties(this.template, type, props); @aws-cdk/aws-ecs-patterns: 84 | if (matchError) { @aws-cdk/aws-ecs-patterns: > 85 | throw new Error(matchError); @aws-cdk/aws-ecs-patterns: | ^ @aws-cdk/aws-ecs-patterns: 86 | } @aws-cdk/aws-ecs-patterns: 87 | } @aws-cdk/aws-ecs-patterns: 88 | @aws-cdk/aws-ecs-patterns: at Template.hasResourceProperties (../assertions/lib/template.ts:85:13) @aws-cdk/aws-ecs-patterns: at fn (test/fargate/load-balanced-fargate-service-v2.test.ts:823:31) @aws-cdk/aws-ecs-patterns: at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35) ``` </details> ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 484cfb2 commit 3a11317

File tree

4 files changed

+12
-164
lines changed

4 files changed

+12
-164
lines changed

Diff for: ‎packages/@aws-cdk/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts

+5-6
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import { INetworkLoadBalancer, NetworkListener, NetworkLoadBalancer, NetworkTarg
77
import { IRole } from '@aws-cdk/aws-iam';
88
import { ARecord, CnameRecord, IHostedZone, RecordTarget } from '@aws-cdk/aws-route53';
99
import { LoadBalancerTarget } from '@aws-cdk/aws-route53-targets';
10-
import { CfnOutput, Duration, FeatureFlags, Stack } from '@aws-cdk/core';
11-
import { ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT } from '@aws-cdk/cx-api';
10+
import * as cdk from '@aws-cdk/core';
1211
import { Construct } from 'constructs';
1312

1413
// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
@@ -104,7 +103,7 @@ export interface NetworkLoadBalancedServiceBaseProps {
104103
*
105104
* @default - defaults to 60 seconds if at least one load balancer is in-use and it is not already set
106105
*/
107-
readonly healthCheckGracePeriod?: Duration;
106+
readonly healthCheckGracePeriod?: cdk.Duration;
108107

109108
/**
110109
* The maximum number of tasks, specified as a percentage of the Amazon ECS
@@ -348,7 +347,7 @@ export abstract class NetworkLoadBalancedServiceBase extends CoreConstruct {
348347
const loadBalancer = props.loadBalancer ?? new NetworkLoadBalancer(this, 'LB', lbProps);
349348
const listenerPort = props.listenerPort ?? 80;
350349
const targetProps = {
351-
port: FeatureFlags.of(this).isEnabled(ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT) ? props.taskImageOptions?.containerPort ?? 80 : 80,
350+
port: props.taskImageOptions?.containerPort ?? 80,
352351
};
353352

354353
this.listener = loadBalancer.addListener('PublicListener', { port: listenerPort });
@@ -385,7 +384,7 @@ export abstract class NetworkLoadBalancedServiceBase extends CoreConstruct {
385384
}
386385

387386
if (props.loadBalancer === undefined) {
388-
new CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName });
387+
new cdk.CfnOutput(this, 'LoadBalancerDNS', { value: this.loadBalancer.loadBalancerDnsName });
389388
}
390389
}
391390

@@ -395,7 +394,7 @@ export abstract class NetworkLoadBalancedServiceBase extends CoreConstruct {
395394
protected getDefaultCluster(scope: CoreConstruct, vpc?: IVpc): Cluster {
396395
// magic string to avoid collision with user-defined constructs
397396
const DEFAULT_CLUSTER_ID = `EcsDefaultClusterMnL3mNNYN${vpc ? vpc.node.id : ''}`;
398-
const stack = Stack.of(scope);
397+
const stack = cdk.Stack.of(scope);
399398
return stack.node.tryFindChild(DEFAULT_CLUSTER_ID) as Cluster || new Cluster(stack, DEFAULT_CLUSTER_ID, { vpc });
400399
}
401400

Diff for: ‎packages/@aws-cdk/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ import { NetworkListener, NetworkLoadBalancer, NetworkTargetGroup } from '@aws-c
77
import { IRole } from '@aws-cdk/aws-iam';
88
import { ARecord, IHostedZone, RecordTarget } from '@aws-cdk/aws-route53';
99
import { LoadBalancerTarget } from '@aws-cdk/aws-route53-targets';
10-
import { CfnOutput, Duration, FeatureFlags, Stack } from '@aws-cdk/core';
11-
import { ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT } from '@aws-cdk/cx-api';
10+
import { CfnOutput, Duration, Stack } from '@aws-cdk/core';
1211
import { Construct } from 'constructs';
1312

1413
// 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
375374
protected registerECSTargets(service: BaseService, container: ContainerDefinition, targets: NetworkTargetProps[]): NetworkTargetGroup {
376375
for (const targetProps of targets) {
377376
const targetGroup = this.findListener(targetProps.listener).addTargets(`ECSTargetGroup${container.containerName}${targetProps.containerPort}`, {
378-
port: FeatureFlags.of(this).isEnabled(ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT) ? targetProps.containerPort ?? 80 : 80,
377+
port: targetProps.containerPort ?? 80,
379378
targets: [
380379
service.loadBalancerTarget({
381380
containerName: container.containerName,

Diff for: ‎packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service-v2.test.ts

+5-145
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ import { Vpc } from '@aws-cdk/aws-ec2';
33
import * as ecs from '@aws-cdk/aws-ecs';
44
import { ContainerImage } from '@aws-cdk/aws-ecs';
55
import { CompositePrincipal, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
6-
import { testFutureBehavior, testLegacyBehavior } from '@aws-cdk/cdk-build-tools';
7-
import { App, Duration, Stack } from '@aws-cdk/core';
8-
import { ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT } from '@aws-cdk/cx-api';
6+
import { Duration, Stack } from '@aws-cdk/core';
97
import { ApplicationLoadBalancedFargateService, ApplicationMultipleTargetGroupsFargateService, NetworkLoadBalancedFargateService, NetworkMultipleTargetGroupsFargateService } from '../../lib';
108

119
describe('When Application Load Balancer', () => {
@@ -665,36 +663,9 @@ describe('When Network Load Balancer', () => {
665663
}).toThrow(/You must specify one of: taskDefinition or image/);
666664
});
667665

668-
testLegacyBehavior('Fargate neworkloadbalanced construct uses Port 80 for target group when feature flag is not enabled', App, (app) => {
666+
test('test Fargate networkloadbalanced construct with custom Port', () => {
669667
// GIVEN
670-
const stack = new Stack(app);
671-
const vpc = new Vpc(stack, 'VPC');
672-
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
673-
674-
new NetworkLoadBalancedFargateService(stack, 'NLBService', {
675-
cluster: cluster,
676-
memoryLimitMiB: 1024,
677-
cpu: 512,
678-
taskImageOptions: {
679-
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
680-
containerPort: 81,
681-
},
682-
listenerPort: 8181,
683-
});
684-
685-
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', {
686-
Port: 80,
687-
Protocol: 'TCP',
688-
TargetType: 'ip',
689-
VpcId: {
690-
Ref: 'VPCB9E5F0B4',
691-
},
692-
});
693-
});
694-
695-
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) => {
696-
// GIVEN
697-
const stack = new Stack(app);
668+
const stack = new Stack();
698669
const vpc = new Vpc(stack, 'VPC');
699670
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
700671

@@ -719,79 +690,9 @@ describe('When Network Load Balancer', () => {
719690
});
720691
});
721692

722-
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) => {
723-
// GIVEN
724-
const stack = new Stack(app);
725-
const vpc = new Vpc(stack, 'VPC');
726-
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
727-
728-
new NetworkLoadBalancedFargateService(stack, 'NLBService', {
729-
cluster: cluster,
730-
memoryLimitMiB: 1024,
731-
cpu: 512,
732-
taskImageOptions: {
733-
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
734-
},
735-
listenerPort: 8181,
736-
});
737-
738-
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', {
739-
Port: 80,
740-
Protocol: 'TCP',
741-
TargetType: 'ip',
742-
VpcId: {
743-
Ref: 'VPCB9E5F0B4',
744-
},
745-
});
746-
});
747-
748-
testLegacyBehavior('Fargate multinetworkloadbalanced construct uses Port 80 for target group when feature flag is not enabled', App, (app) => {
749-
// GIVEN
750-
const stack = new Stack(app);
751-
const vpc = new Vpc(stack, 'VPC');
752-
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
753-
754-
new NetworkMultipleTargetGroupsFargateService(stack, 'Service', {
755-
cluster,
756-
taskImageOptions: {
757-
image: ecs.ContainerImage.fromRegistry('test'),
758-
},
759-
});
760-
761-
762-
new NetworkMultipleTargetGroupsFargateService(stack, 'NLBService', {
763-
cluster: cluster,
764-
memoryLimitMiB: 1024,
765-
cpu: 512,
766-
taskImageOptions: {
767-
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
768-
},
769-
loadBalancers: [
770-
{
771-
name: 'lb1',
772-
listeners: [
773-
{ name: 'listener1', port: 8181 },
774-
],
775-
},
776-
],
777-
targetGroups: [{
778-
containerPort: 81,
779-
}],
780-
});
781-
782-
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', {
783-
Port: 80,
784-
Protocol: 'TCP',
785-
TargetType: 'ip',
786-
VpcId: {
787-
Ref: 'VPCB9E5F0B4',
788-
},
789-
});
790-
});
791-
792-
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) => {
693+
test('test Fargate multinetworkloadbalanced construct with custom Port', () => {
793694
// GIVEN
794-
const stack = new Stack(app);
695+
const stack = new Stack();
795696
const vpc = new Vpc(stack, 'VPC');
796697
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
797698

@@ -832,45 +733,4 @@ describe('When Network Load Balancer', () => {
832733
},
833734
});
834735
});
835-
836-
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) => {
837-
// GIVEN
838-
const stack = new Stack(app);
839-
const vpc = new Vpc(stack, 'VPC');
840-
const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
841-
842-
new NetworkMultipleTargetGroupsFargateService(stack, 'Service', {
843-
cluster,
844-
taskImageOptions: {
845-
image: ecs.ContainerImage.fromRegistry('test'),
846-
},
847-
});
848-
849-
850-
new NetworkMultipleTargetGroupsFargateService(stack, 'NLBService', {
851-
cluster: cluster,
852-
memoryLimitMiB: 1024,
853-
cpu: 512,
854-
taskImageOptions: {
855-
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
856-
},
857-
loadBalancers: [
858-
{
859-
name: 'lb1',
860-
listeners: [
861-
{ name: 'listener1', port: 8181 },
862-
],
863-
},
864-
],
865-
});
866-
867-
Template.fromStack(stack).hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', {
868-
Port: 80,
869-
Protocol: 'TCP',
870-
TargetType: 'ip',
871-
VpcId: {
872-
Ref: 'VPCB9E5F0B4',
873-
},
874-
});
875-
});
876736
});

Diff for: ‎packages/@aws-cdk/cx-api/lib/features.ts

-10
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,6 @@ export const EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME = '@aws-cdk/aws-ec2:uniqueIm
233233
*/
234234
export const IAM_MINIMIZE_POLICIES = '@aws-cdk/aws-iam:minimizePolicies';
235235

236-
/**
237-
* Enable this feature flag to pass through the `NetworkLoadBalanced<Ec2|Fargate>ServiceProps.taskImageOptions.containerPort`
238-
* and the `NetworkMultipleTargetGroups<Ec2|Fargate>ServiceProps.targetGroups[X].containerPort` to the generated
239-
* `ElasticLoadBalancingV2::TargetGroup`'s `Port` property.
240-
*
241-
* This is a feature flag because updating `Port` causes a replacement of the target groups, which is a breaking change.
242-
*/
243-
export const ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT = '@aws-cdk/aws-ecs-patterns:containerPortToTargetGroupPort';
244-
245236
/**
246237
* Flag values that should apply for new projects
247238
*
@@ -269,7 +260,6 @@ export const FUTURE_FLAGS: { [key: string]: boolean } = {
269260
[EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: true,
270261
[CHECK_SECRET_USAGE]: true,
271262
[IAM_MINIMIZE_POLICIES]: true,
272-
[ECS_PATTERNS_TARGET_GROUP_PORT_FROM_CONTAINER_PORT]: true,
273263
};
274264

275265
/**

0 commit comments

Comments
 (0)