retain legacy behavior for addTargetGroups

packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md

@@ -768,13 +768,11 @@ const targetGroupMetrics: elbv2.IApplicationTargetGroupMetrics = targetGroup.met
 
 ## logicalIds on ExternalApplicationListener.addTargetGroups() and .addAction()
 
-Legacy behavior for the `addTargetGroups()` method did not follow the standard of adding
-a `Rule` suffix to the logicalId.
-If you have `ListenerRule`s deployed using the legacy behavior and are upgrading,
-you will need to enable the `removeRuleSuffixFromLogicalId: true` property
-so that CloudFormation will not attempt to replace your existing `ListenerRule`s.
-
-Similarly, if you have `ListenerRule`s deployed using the legacy behavior of `addTargetGroups()`,
+By default, the `addTargetGroups()` method does not follow the standard behavior
+of adding a `Rule` suffix to the logicalId of the `ListenerRule` it creates.
+If you are deploying new `ListenerRule`s using `addTargetGroups()` the recommendation
+is to set the `removeRuleSuffixFromLogicalId: false` property.
+If you have `ListenerRule`s deployed using the legacy behavior of `addTargetGroups()`,
 which you need to switch over to being managed by the `addAction()` method,
 then you will need to enable the `removeRuleSuffixFromLogicalId: true` property in the `addAction()` method.
 

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

@@ -620,12 +620,23 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
    *
    * It's possible to add conditions to the TargetGroups added in this way.
    * At least one TargetGroup must be added without conditions.
+   *
+   * NOTE: if you are deploying new infrastructure using this method,
+   * it is recommended to set the `removeRuleSuffixFromLogicalId: false`
+   * property so that the logicalId for the generated `ListenerRule`
+   * is consistent with other methods for managing `ListenerRule`s.
+   *
+   * If you have already deployed a `ListenerRule` using this method
+   * and need to migrate to using the more feature rich `addAction()`
+   * method, then you will need to set the `removeRuleSuffixFromLogicalId: true`
+   * property there to avoid having CloudFormation attempt to replace your resource.
    */
   public addTargetGroups(id: string, props: AddApplicationTargetGroupsProps): void {
     checkAddRuleProps(props);
 
     if (props.priority !== undefined) {
-      const ruleId = props.removeRuleSuffixFromLogicalId ? id : id + 'Rule';
+      const removeRuleSuffixFromLogicalId = props.removeRuleSuffixFromLogicalId ?? true;
+      const ruleId = removeRuleSuffixFromLogicalId ? id : id + 'Rule';
       // New rule
       new ApplicationListenerRule(this, ruleId, {
         listener: this,
@@ -665,6 +676,11 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
    * It is not possible to add a default action to an imported IApplicationListener.
    * In order to add actions to an imported IApplicationListener a `priority`
    * must be provided.
+   *
+   * If you previously deployed a `ListenerRule` using the `addTargetGroups()` method
+   * and need to migrate to using the more feature rich `addAction()`
+   * method, then you will need to set the `removeRuleSuffixFromLogicalId: true`
+   * property here to avoid having CloudFormation attempt to replace your resource.
    */
   public addAction(id: string, props: AddApplicationActionProps): void {
     checkAddRuleProps(props);
@@ -804,9 +820,9 @@ export interface AddApplicationTargetGroupsProps extends AddRuleProps {
    *
    * Legacy behavior did not include the `Rule` suffix on the logicalId of the generated `ListenerRule`
    * when generated by the `addTargetGroups()` method.
-   * This exists to allow backwards compatible behavior with `ListenerRule`s deployed by older versions of aws-cdk.
+   * This exists to allow new `ListenerRule`s to have consistent logicalIds.
    *
-   * @default - do not remove the logicalId `Rule` suffix
+   * @default true remove the logicalId `Rule` suffix
    */
   readonly removeRuleSuffixFromLogicalId?: boolean;
 }

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

@@ -1699,16 +1699,21 @@ describe('tests', () => {
   });
 
   describe('Rule suffix for logicalId', () => {
+    const identifierToken = 'SuperMagicToken';
     interface TestCase {
-      readonly removeRuleSuffixFromLogicalId: boolean;
+      readonly removeRuleSuffixFromLogicalId?: boolean;
+      readonly expectedLogicalId: string;
     };
-    const testCases: TestCase[] = [
-      { removeRuleSuffixFromLogicalId: true },
-      { removeRuleSuffixFromLogicalId: false },
+    const nonDefaultTestCases: TestCase[] = [
+      { removeRuleSuffixFromLogicalId: true, expectedLogicalId: identifierToken },
+      { removeRuleSuffixFromLogicalId: false, expectedLogicalId: identifierToken + 'Rule' },
     ];
-    test.each<TestCase>(testCases)('addTargetGroups %s', ({ removeRuleSuffixFromLogicalId }) => {
+    test.each<TestCase>([
+      // Default is to not have the `Rule` suffix, per legacy behavior.
+      { removeRuleSuffixFromLogicalId: undefined, expectedLogicalId: identifierToken },
+      ...nonDefaultTestCases,
+    ])('addTargetGroups %s', ({ removeRuleSuffixFromLogicalId, expectedLogicalId }) => {
       // GIVEN
-      const identifierToken = 'SuperMagicToken';
       const app = new cdk.App();
       const stack = new cdk.Stack(app, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
       const vpc = new ec2.Vpc(stack, 'Stack');
@@ -1728,15 +1733,17 @@ describe('tests', () => {
       });
 
       // THEN
-      const expectedLogicalId = removeRuleSuffixFromLogicalId ? identifierToken : identifierToken + 'Rule';
       const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
       expect(applicationListenerRule).toBeDefined();
       expect(applicationListenerRule!.node.id).toBe(expectedLogicalId);
     });
 
-    test.each<TestCase>(testCases)('addAction %s', ({ removeRuleSuffixFromLogicalId }) => {
+    test.each<TestCase>([
+      // Default is consistent, which means it has the `Rule` suffix. This means no change from legacy behavior
+      { removeRuleSuffixFromLogicalId: undefined, expectedLogicalId: identifierToken + 'Rule' },
+      ...nonDefaultTestCases,
+    ])('addAction %s', ({ removeRuleSuffixFromLogicalId, expectedLogicalId }) => {
       // GIVEN
-      const identifierToken = 'SuperMagicToken';
       const app = new cdk.App();
       const stack = new cdk.Stack(app, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
       const vpc = new ec2.Vpc(stack, 'Stack');
@@ -1756,7 +1763,6 @@ describe('tests', () => {
       });
 
       // THEN
-      const expectedLogicalId = removeRuleSuffixFromLogicalId ? identifierToken : identifierToken + 'Rule';
       const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
       expect(applicationListenerRule).toBeDefined();
       expect(applicationListenerRule!.node.id).toBe(expectedLogicalId);