Skip to content

Commit b7ca082

Browse files
authored
fix(ecs-patterns): target group ID changes without setting feature flag (#36199)
### Issue # (if applicable) Closes #36149 ### Reason for this change The feature flag did not stop the target group ID of the ALB to change if the load balancer was private. ### Description of changes The target group ID will not change if the feature flag is not set and remain as `ECS`. ### Describe any new or updated permissions being added No new permissions are added. ### Description of how you validated changes Unit test added. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7e31eb6 commit b7ca082

File tree

8 files changed

+79
-83
lines changed

8 files changed

+79
-83
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-ecs-patterns/test/fargate/integ.alb-fargate-service-public-private-switch.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,13 @@ import * as ecs from 'aws-cdk-lib/aws-ecs';
33
import * as cdk from 'aws-cdk-lib';
44
import * as integ from '@aws-cdk/integ-tests-alpha';
55
import * as ecsPatterns from 'aws-cdk-lib/aws-ecs-patterns';
6+
import { ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID } from 'aws-cdk-lib/cx-api';
67

7-
const app = new cdk.App();
8+
const app = new cdk.App({
9+
postCliContext: {
10+
[ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID]: true,
11+
},
12+
});
813
const stack = new cdk.Stack(app, 'aws-ecs-integ-alb-fargate-public-private-switch');
914

1015
const vpc = new ec2.Vpc(stack, 'Vpc', { maxAzs: 2, restrictDefaultSecurityGroup: false });

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2-targets/test/integ.alb-target.js.snapshot/TestStack.assets.json

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2-targets/test/integ.alb-target.js.snapshot/TestStack.template.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@
477477
"DefaultActions": [
478478
{
479479
"TargetGroupArn": {
480-
"Ref": "ServiceLBPublicListenerECSPrivateGroup93D5832E"
480+
"Ref": "ServiceLBPublicListenerECSGroup0CC8688C"
481481
},
482482
"Type": "forward"
483483
}
@@ -489,7 +489,7 @@
489489
"Protocol": "HTTP"
490490
}
491491
},
492-
"ServiceLBPublicListenerECSPrivateGroup93D5832E": {
492+
"ServiceLBPublicListenerECSGroup0CC8688C": {
493493
"Type": "AWS::ElasticLoadBalancingV2::TargetGroup",
494494
"Properties": {
495495
"Port": 80,
@@ -524,7 +524,7 @@
524524
"ContainerName": "nginx",
525525
"ContainerPort": 80,
526526
"TargetGroupArn": {
527-
"Ref": "ServiceLBPublicListenerECSPrivateGroup93D5832E"
527+
"Ref": "ServiceLBPublicListenerECSGroup0CC8688C"
528528
}
529529
}
530530
],
@@ -554,7 +554,7 @@
554554
}
555555
},
556556
"DependsOn": [
557-
"ServiceLBPublicListenerECSPrivateGroup93D5832E",
557+
"ServiceLBPublicListenerECSGroup0CC8688C",
558558
"ServiceLBPublicListener46709EAA",
559559
"TaskTaskRoleE98524A1"
560560
]
@@ -674,7 +674,7 @@
674674
}
675675
},
676676
"DependsOn": [
677-
"ServiceLBPublicListenerECSPrivateGroup93D5832E",
677+
"ServiceLBPublicListenerECSGroup0CC8688C",
678678
"ServiceLBPublicListener46709EAA"
679679
]
680680
}

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2-targets/test/integ.alb-target.js.snapshot/integ.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2-targets/test/integ.alb-target.js.snapshot/manifest.json

Lines changed: 31 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2-targets/test/integ.alb-target.js.snapshot/tree.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -526,8 +526,8 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
526526
// Include both internetFacing and loadBalancerName in target group ID
527527
targetGroupId = `ECS${props.loadBalancerName ?? ''}${internetFacing ? '' : 'Private'}`;
528528
} else {
529-
// Legacy behavior: only include internetFacing
530-
targetGroupId = internetFacing ? 'ECS' : 'ECSPrivate';
529+
// Legacy behavior
530+
targetGroupId = 'ECS';
531531
}
532532

533533
this.targetGroup = this.listener.addTargets(targetGroupId, targetProps);

packages/aws-cdk-lib/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts

Lines changed: 30 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -2450,71 +2450,6 @@ describe('NetworkLoadBalancedFargateService', () => {
24502450
});
24512451
});
24522452

2453-
test('target group has different logical ID for public vs private load balancer', () => {
2454-
// GIVEN
2455-
const stack1 = new cdk.Stack();
2456-
const stack2 = new cdk.Stack();
2457-
2458-
// WHEN - Create public load balancer service
2459-
new ecsPatterns.ApplicationLoadBalancedFargateService(stack1, 'Service', {
2460-
taskImageOptions: {
2461-
image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'),
2462-
},
2463-
publicLoadBalancer: true,
2464-
});
2465-
2466-
// WHEN - Create private load balancer service
2467-
new ecsPatterns.ApplicationLoadBalancedFargateService(stack2, 'Service', {
2468-
taskImageOptions: {
2469-
image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'),
2470-
},
2471-
publicLoadBalancer: false,
2472-
});
2473-
2474-
// THEN - Target groups should have different logical IDs
2475-
const template1 = Template.fromStack(stack1);
2476-
const template2 = Template.fromStack(stack2);
2477-
2478-
// Public load balancer should create target group with 'ECS' suffix
2479-
template1.hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', {
2480-
Port: 80,
2481-
Protocol: 'HTTP',
2482-
TargetType: 'ip',
2483-
});
2484-
2485-
// Private load balancer should create target group with 'ECSPrivate' suffix
2486-
template2.hasResourceProperties('AWS::ElasticLoadBalancingV2::TargetGroup', {
2487-
Port: 80,
2488-
Protocol: 'HTTP',
2489-
TargetType: 'ip',
2490-
});
2491-
2492-
// Verify the logical IDs are different by checking the generated CloudFormation
2493-
const resources1 = template1.toJSON().Resources;
2494-
const resources2 = template2.toJSON().Resources;
2495-
2496-
const targetGroup1Keys = Object.keys(resources1).filter(key =>
2497-
resources1[key].Type === 'AWS::ElasticLoadBalancingV2::TargetGroup',
2498-
);
2499-
const targetGroup2Keys = Object.keys(resources2).filter(key =>
2500-
resources2[key].Type === 'AWS::ElasticLoadBalancingV2::TargetGroup',
2501-
);
2502-
2503-
// Should have exactly one target group each
2504-
expect(targetGroup1Keys).toHaveLength(1);
2505-
expect(targetGroup2Keys).toHaveLength(1);
2506-
2507-
// The logical IDs should be different
2508-
expect(targetGroup1Keys[0]).not.toEqual(targetGroup2Keys[0]);
2509-
2510-
// Public should contain 'ECS' but not 'ECSPrivate'
2511-
expect(targetGroup1Keys[0]).toContain('ECS');
2512-
expect(targetGroup1Keys[0]).not.toContain('ECSPrivate');
2513-
2514-
// Private should contain 'ECSPrivate'
2515-
expect(targetGroup2Keys[0]).toContain('ECSPrivate');
2516-
});
2517-
25182453
describe('ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID feature flag', () => {
25192454
test('with feature flag enabled - generates unique target group IDs', () => {
25202455
// GIVEN
@@ -2577,6 +2512,36 @@ describe('NetworkLoadBalancedFargateService', () => {
25772512
// Should use legacy naming (contains 'ECS' but not unique identifier)
25782513
expect(targetGroupKeys).toHaveLength(1);
25792514
expect(targetGroupKeys[0]).toContain('ECS');
2515+
expect(targetGroupKeys[0]).not.toContain('ECSPrivate');
2516+
});
2517+
2518+
test('with feature flag disabled - uses legacy target group naming for private load balancer', () => {
2519+
// GIVEN
2520+
const featureFlag = { [cxapi.ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID]: false };
2521+
const app = new cdk.App({
2522+
context: featureFlag,
2523+
});
2524+
const stack = new cdk.Stack(app, 'TestStack');
2525+
2526+
// WHEN
2527+
new ecsPatterns.ApplicationLoadBalancedFargateService(stack, 'Service', {
2528+
taskImageOptions: {
2529+
image: ecs.ContainerImage.fromRegistry('/aws/aws-example-app'),
2530+
},
2531+
publicLoadBalancer: false,
2532+
});
2533+
2534+
// THEN
2535+
const template = Template.fromStack(stack);
2536+
const resources = template.toJSON().Resources;
2537+
const targetGroupKeys = Object.keys(resources).filter(key =>
2538+
resources[key].Type === 'AWS::ElasticLoadBalancingV2::TargetGroup',
2539+
);
2540+
2541+
// Should use legacy naming (contains 'ECS' but not unique identifier)
2542+
expect(targetGroupKeys).toHaveLength(1);
2543+
expect(targetGroupKeys[0]).toContain('ECS');
2544+
expect(targetGroupKeys[0]).not.toContain('ECSPrivate');
25802545
});
25812546

25822547
test('without feature flag - uses default behavior', () => {

0 commit comments

Comments
 (0)