fix(ecs-patterns): gate target group ID changes behind ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID (#36149)

[jbazkar]

Issue #36149

Closes #36149

Reason for this change

The ECS patterns construct started generating different Target Group logical IDs (for example, ECSGroup → ECSPrivateGroup) even when the ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID feature flag was not enabled, causing unexpected CloudFormation diffs and unnecessary Target Group replacements for existing services that had not opted into the new naming behavior.

Description of changes

I updated ApplicationLoadBalancedServiceBase so that the new Target Group naming logic is only used when ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID is enabled; when the feature flag is disabled, the construct now always uses the legacy ECSGroup ID, restoring backward-compatible behavior. This directly addresses the issue by preventing unintended logical ID changes for existing stacks while still allowing users who opt in via the feature flag to benefit from the more specific naming. I considered unconditionally reverting to the old naming or introducing a new flag, but chose to reuse the existing ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID feature flag to keep the behavior simple, explicit, and aligned with CDK’s existing feature-flag design.

Describe any new or updated permissions being added

No new or updated IAM permissions are required; this change only affects how the Target Group logical ID is chosen inside the ECS patterns construct.

Description of how you validated changes

I reproduced this with a minimal stack using ApplicationLoadBalancedFargateService and publicLoadBalancer: false:
aws-cdk-lib 2.220.0 → synth → baseline template
upgrade to aws-cdk-lib 2.221.0 (no other changes, no @aws-cdk/aws-ecs-patterns:uniqueTargetGroupId in context) → synth → new template diff of the templates shows the target group logical ID changing from ServiceLBPublicListenerECSGroup... to ServiceLBPublicListenerECSPrivateGroup..., and all references updated accordingly, which causes CloudFormation to replace the target group and leads to downtime during deployment. This confirms that the more specific ECSPrivateGroup naming behavior is applied even when the @aws-cdk/aws-ecs-patterns:uniqueTargetGroupId feature flag is not enabled.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

✅ A exemption request has been requested. Please wait for a maintainer's review.

[jbazkar]

Exemption Request

This PR restores backward-compatible Target Group logical ID behavior in ecs-patterns by ensuring the newer naming scheme is only applied when the ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID feature flag is enabled. The change is small and localized to the ApplicationLoadBalancedServiceBase target group ID selection and is fully guarded by an existing feature flag.

I validated the behavior manually by synthesizing a minimal ApplicationLoadBalancedFargateService stack before and after the change and diffing the resulting CloudFormation templates. With the flag disabled, the Target Group ID remains stable (ECSGroup), and with the flag enabled, the updated naming is applied as expected.

Adding a new integration test and snapshot here would mainly encode specific logical IDs into snapshots (which are already quite brittle) rather than exercise new functional behavior, so I’m requesting an exemption from the “tests required” rule for this regression fix. If you would prefer a unit or integration test instead of an exemption, I’m happy to add one in the location you recommend.

[jbazkar]

Pushed a small follow-up commit to remove the trailing whitespace flagged by ESLint. CI should pass after this.

[jbazkar]

Added the required unit test updates for both load-balanced-fargate-service.test.ts and load-balanced-fargate-service-v2.test.ts

These tests now reflect the corrected target group logical ID behavior based on the ECS_PATTERNS_UNIQUE_TARGET_GROUP_ID flag. The PR should now satisfy the linter requirements for test coverage. Please let me know if additional adjustments are needed.

[Abogical]

Hi @jbazkar ! Thank you for your contribution! However, this isn't the correct approach:

• The legacy behaviour did not have "Group" in the ID variable, it was implicitly added somewhere else.
• This removes the uniqueness of the ID when having multiple loadbalancer names.

Regardless, I already made a fix for this so I'm closing this PR in favor of #36199 .

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.