fix(elasticloadbalancerV2): consistent logicalId for addTargetGroups (under feature flag)

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

@@ -625,8 +625,9 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
     checkAddRuleProps(props);
 
     if (props.priority !== undefined) {
+      const idSuffix = FeatureFlags.of(this).isEnabled(cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID) ? '' : 'Rule';
       // New rule
-      new ApplicationListenerRule(this, id, {
+      new ApplicationListenerRule(this, id + idSuffix, {
         listener: this,
         priority: props.priority,
         ...props,

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/listener.test.ts

@@ -1714,6 +1714,38 @@ describe('tests', () => {
       expect(applicationListenerRule).toBeDefined();
       expect(applicationListenerRule!.node.id).toBe(identifierToken); // Should not have `Rule` suffix
     });
+
+    test('backwards compatibility', () => {
+      // GIVEN
+      const context = { [cxapi.ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID]: true };
+      const app = new cdk.App({ context });
+      const stack = new cdk.Stack(app, 'stack', {
+        env: {
+          account: '123456789012',
+          region: 'us-west-2',
+        },
+      });
+      const vpc = new ec2.Vpc(stack, 'Stack');
+      const targetGroup = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 });
+      const listener = elbv2.ApplicationListener.fromLookup(stack, 'a', {
+        loadBalancerTags: {
+          some: 'tag',
+        },
+      });
+
+      // WHEN
+      const identifierToken = 'SuperMagicToken';
+      listener.addTargetGroups(identifierToken, {
+        conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
+        priority: 42,
+        targetGroups: [targetGroup],
+      });
+
+      // THEN
+      const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
+      expect(applicationListenerRule).toBeDefined();
+      expect(applicationListenerRule!.node.id).toBe(identifierToken); // Should NOT have `Rule` suffix
+    });
   });
 
   test('not allowed to specify defaultTargetGroups and defaultAction together', () => {

packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

@@ -69,6 +69,7 @@ Flags come in three types:
 | [@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope](#aws-cdkaws-kmsreducecrossaccountregionpolicyscope) | When enabled, IAM Policy created from KMS key grant will reduce the resource scope to this key only. | 2.134.0 | (fix) |
 | [@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction](#aws-cdkaws-elasticloadbalancingv2externalapplicationlistener-norulesuffixforaddaction) | When enabled, you can switch from the `addTargetGroups()` method of declaring a `ListenerRule` to the `addAction()` method,
 without changing the logicalId and replacing your resource. | V2NEXT | (fix) |
+| [@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId](#aws-cdkaws-elasticloadbalancingv2externalapplicationlistener-addtargetgrouplegacylogicalid) | When enabled, the `addTargetGroups()` method will use the legacy naming conventions (no `Rule` suffix) for `ListenerRule`s. | V2NEXT | (fix) |
 
 <!-- END table -->
 
@@ -1282,4 +1283,18 @@ This allows you to switch from the `addTargetGroups()` method without having Clo
 | V2NEXT | `false` | `false` |
 
 
+### aws-cdkaws-elasticloadbalancingv2externalapplicationlistener-addtargetgrouplegacylogicalid
+
+*When enabled, the `addTargetGroups()` method will use the legacy naming conventions (no `Rule` suffix) for `ListenerRule`s.* (fix)
+
+Setting this feature flag will case the `addTargetGroups()` method to revert to the legacy behavior and not add the `Rule` suffix on the logicalId for the `ListenerRule` it creates.
+This allows you to upgrade to the latest version of the CDK without having CloudFormation deadlock while attempting to replace the resource.
+
+
+| Since | Default | Recommended |
+| ----- | ----- | ----- |
+| (not in v1) |  |  |
+| V2NEXT | `false` | `false` |
+
+
 <!-- END details -->

packages/aws-cdk-lib/cx-api/README.md

@@ -320,18 +320,13 @@ This will prevent `Rule` from being added as a suffix to the logicalId so that t
 Do not enable this if you have already deployed `ListenerRule` resources using the
 `addAction()` method.
 Instead consider the [cdk-logical-id-mapper](https://github.com/mbonig/cdk-logical-id-mapper),
-possibly in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupsConsistentLogicalId` (see below).
+possibly in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId` (see below).
 
-* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupsConsistentLogicalId`
+* `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId`
 
-Enable this feature flag to ensure that the logicalIds of `ListenerRule`s created
-on a `ExternalApplicationListener` by the `addTargetGroups()` method are consistent
-with logicalIds for `ListenerRules` generated by other methods.
-This will allow you to migrate between the different methods
-without causing a replacement of the `ListenerRule` resource.
+Enable this feature flag if you have deployed a `ListenerRule` using the `addTargetGroups()`
+method against an `ExternalApplicationListener` and you want to upgrade to using the
+latest version of aws-cdk. This will prevent cdk from adding the `Rule` suffix to the logicalId
+of the `ListenerRule` involved so that CloudFormation will not deadlock while attempting to replace the resource.
 
-You should enable this on new apps, before creating any resources.
-If you have already created resources with the previous behavior,
-you may still enable this flag, but will need to use something like the
-[cdk-logical-id-mapper](https://github.com/mbonig/cdk-logical-id-mapper).
-Alternatively, do not enable this feature flag and instead consider the `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction` as necessary.
+Do not enable this feature flag for new deployments. May be used in conjunction with `@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction`.

packages/aws-cdk-lib/cx-api/lib/features.ts

@@ -102,6 +102,7 @@ export const CODEPIPELINE_CROSS_ACCOUNT_KEYS_DEFAULT_VALUE_TO_FALSE = '@aws-cdk/
 export const CODEPIPELINE_DEFAULT_PIPELINE_TYPE_TO_V2 = '@aws-cdk/aws-codepipeline:defaultPipelineTypeToV2';
 export const KMS_REDUCE_CROSS_ACCOUNT_REGION_POLICY_SCOPE = '@aws-cdk/aws-kms:reduceCrossAccountRegionPolicyScope';
 export const ALBV2_EXTERNALAPPLICATIONLISTENER_SWITCH_FROM_ADDTARGETGROUP_TO_ADDACTION = '@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-noRuleSuffixForAddAction';
+export const ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID = '@aws-cdk/aws-elasticloadbalancingv2:ExternalApplicationListener-addTargetGroupLegacyLogicalId';
 
 export const FLAGS: Record<string, FlagInfo> = {
   //////////////////////////////////////////////////////////////////////
@@ -1056,6 +1057,25 @@ export const FLAGS: Record<string, FlagInfo> = {
     introducedIn: { v2: 'V2NEXT' },
     recommendedValue: false,
   },
+
+  //////////////////////////////////////////////////////////////////////
+  [ALBV2_EXTERNALAPPLICATIONLISTENER_LEGACY_ADDTARGETGROUP_LOGICALID]: {
+    type: FlagType.ApiDefault,
+    summary: 'When enabled, reverts to the legacy logicalId naming for \`ListenerRule\`s created by the \`addTargetGroups()\` method.',
+    compatibilityWithOldBehaviorMd: `Enable this feature flag to revert to the old behavior.`,
+    detailsMd: `
+      The legacy behavior for the \`addTargetGroups()\` method did not add the \`Rule\` suffix to the logicalId of the \`ListenerRule\`
+      which it generated. This was inconsistent with all the other methods which create \`ListenerRule\`s.
+
+      Replacing \`ListenerRule\`s is disruptive since they have a unique \`priority\`.
+      This means that when Cfn tries to create the new replacement, it will fail, blocking the upgrade path.
+
+      Setting this feature flag will cause the \`addTargetGroups()\` method to not add the \`Rule\` suffix on the logicalId.
+      This should only be set to support deployed infrastructure.
+    `,
+    introducedIn: { v2: 'V2NEXT' },
+    recommendedValue: false,
+  }
 };
 
 const CURRENT_MV = 'v2';