address comments

aws · Apr 17, 2024 · 9f5e2e4 · 9f5e2e4
1 parent 8fc296a
commit 9f5e2e4
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 60 deletions.
diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts
@@ -621,24 +621,14 @@ 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.
+ * @deprecated consider using addAction() instead as it generates a consistent logicalId
  */
  public addTargetGroups(id: string, props: AddApplicationTargetGroupsProps): void {
  checkAddRuleProps(props);
 
  if (props.priority !== undefined) {
- const removeRuleSuffixFromLogicalId = props.removeRuleSuffixFromLogicalId ?? true;
- const ruleId = removeRuleSuffixFromLogicalId ? id : id + 'Rule';
  // New rule
- new ApplicationListenerRule(this, ruleId, {
+ new ApplicationListenerRule(this, id, {
  listener: this,
  priority: props.priority,
  ...props,
@@ -686,7 +676,7 @@ abstract class ExternalApplicationListener extends Resource implements IApplicat
  checkAddRuleProps(props);
 
  if (props.priority !== undefined) {
- const ruleId = props.removeRuleSuffixFromLogicalId ? id : id + 'Rule';
+ const ruleId = props.removeSuffix ? id : id + 'Rule';
  // New rule
  //
  // TargetGroup.registerListener is called inside ApplicationListenerRule.
@@ -815,16 +805,6 @@ export interface AddApplicationTargetGroupsProps extends AddRuleProps {
  * Target groups to forward requests to
  */
  readonly targetGroups: IApplicationTargetGroup[];
- /**
- * `ListenerRule`s have a `Rule` suffix on their logicalId by default.
- *
- * 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 new `ListenerRule`s to have consistent logicalIds.
- *
- * @default true remove the logicalId `Rule` suffix
- */
- readonly removeRuleSuffixFromLogicalId?: boolean;
 }
 
 /**
@@ -847,7 +827,7 @@ export interface AddApplicationActionProps extends AddRuleProps {
  *
  * @default - use standard logicalId with the `Rule` suffix
  */
- readonly removeRuleSuffixFromLogicalId?: boolean;
+ readonly removeSuffix?: boolean;
 }
 
 /**

diff --git a/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/listener.test.ts b/packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/listener.test.ts
@@ -1701,48 +1701,18 @@ describe('tests', () => {
  describe('Rule suffix for logicalId', () => {
  const identifierToken = 'SuperMagicToken';
  interface TestCase {
- readonly removeRuleSuffixFromLogicalId?: boolean;
+ readonly removeSuffix?: boolean;
  readonly expectedLogicalId: string;
  };
  const nonDefaultTestCases: TestCase[] = [
- { removeRuleSuffixFromLogicalId: true, expectedLogicalId: identifierToken },
- { removeRuleSuffixFromLogicalId: false, expectedLogicalId: identifierToken + 'Rule' },
+ { removeSuffix: true, expectedLogicalId: identifierToken },
+ { removeSuffix: false, expectedLogicalId: identifierToken + 'Rule' },
  ];
- 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 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');
- const targetGroup = new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', { vpc, port: 80 });
- const listener = elbv2.ApplicationListener.fromLookup(stack, 'a', {
- loadBalancerTags: {
- some: 'tag',
- },
- });
-
- // WHEN
- listener.addTargetGroups(identifierToken, {
- conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
- priority: 42,
- targetGroups: [targetGroup],
- removeRuleSuffixFromLogicalId,
- });
-
- // THEN
- const applicationListenerRule = listener.node.children.find((v)=> v.hasOwnProperty('conditions'));
- expect(applicationListenerRule).toBeDefined();
- expect(applicationListenerRule!.node.id).toBe(expectedLogicalId);
- });
-
  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' },
+ { removeSuffix: undefined, expectedLogicalId: identifierToken + 'Rule' },
  ...nonDefaultTestCases,
- ])('addAction %s', ({ removeRuleSuffixFromLogicalId, expectedLogicalId }) => {
+ ])('addAction %s', ({ removeSuffix, expectedLogicalId }) => {
  // GIVEN
  const app = new cdk.App();
  const stack = new cdk.Stack(app, 'TestStack', { env: { account: '123456789012', region: 'us-east-1' } });
@@ -1759,7 +1729,7 @@ describe('tests', () => {
  action: elbv2.ListenerAction.weightedForward([{ targetGroup, weight: 1 }]),
  conditions: [elbv2.ListenerCondition.pathPatterns(['/fake'])],
  priority: 42,
- removeRuleSuffixFromLogicalId,
+ removeSuffix,
  });
 
  // THEN