fix: Update GuApplicationTargetGroup with revised logicalId logic

In #364 we placed the logic of overriding a construct's logicalId into a single place. In this change, we're updating the `GuApplicationTargetGroup` construct to adopt the new logic. As of #418 it's as simple as using the `GuStatefulMigratableConstruct` mixin!
guardian · Apr 13, 2021 · 1ad1e73 · 1ad1e73
1 parent d69bdbe
commit 1ad1e73
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 36 deletions.
diff --git a/src/constructs/autoscaling/asg.test.ts b/src/constructs/autoscaling/asg.test.ts
@@ -152,7 +152,6 @@ describe("The GuAutoScalingGroup", () => {
       ...app,
       vpc: vpc,
       protocol: ApplicationProtocol.HTTP,
-      overrideId: true,
     });
 
     new GuAutoScalingGroup(stack, "AutoscalingGroup", {
@@ -163,7 +162,7 @@ describe("The GuAutoScalingGroup", () => {
     expect(stack).toHaveResource("AWS::AutoScaling::AutoScalingGroup", {
       TargetGroupARNs: [
         {
-          Ref: "TargetGroup",
+          Ref: "TargetGroupTesting29B71ABC",
         },
       ],
     });

diff --git a/src/constructs/loadbalancing/alb/application-target-group.test.ts b/src/constructs/loadbalancing/alb/application-target-group.test.ts
@@ -1,11 +1,9 @@
 import "@aws-cdk/assert/jest";
 import "../../../utils/test/jest";
-import { SynthUtils } from "@aws-cdk/assert";
 import { Vpc } from "@aws-cdk/aws-ec2";
 import { ApplicationProtocol } from "@aws-cdk/aws-elasticloadbalancingv2";
 import { Stack } from "@aws-cdk/core";
 import { TrackingTag } from "../../../constants/library-info";
-import type { SynthedStack } from "../../../utils/test";
 import { alphabeticalTags, simpleGuStackForTesting } from "../../../utils/test";
 import type { AppIdentity } from "../../core/identity";
 import { GuApplicationTargetGroup } from "./application-target-group";
@@ -53,36 +51,21 @@ describe("The GuApplicationTargetGroup class", () => {
     });
   });
 
-  test("overrides the id if the prop is true", () => {
-    const stack = simpleGuStackForTesting();
-    new GuApplicationTargetGroup(stack, "ApplicationTargetGroup", { ...app, vpc, overrideId: true });
+  test("overrides the logicalId when existingLogicalId is set in a migrating stack", () => {
+    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
+    new GuApplicationTargetGroup(stack, "ApplicationTargetGroup", { ...app, vpc, existingLogicalId: "MyTargetGroup" });
 
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).toContain("ApplicationTargetGroup");
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::ElasticLoadBalancingV2::TargetGroup", "MyTargetGroup");
   });
 
-  test("does not override the id if the prop is false", () => {
+  test("auto-generates the logicalId by default", () => {
     const stack = simpleGuStackForTesting();
     new GuApplicationTargetGroup(stack, "ApplicationTargetGroup", { ...app, vpc });
 
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).not.toContain("ApplicationTargetGroup");
-  });
-
-  test("overrides the id if the stack migrated value is true", () => {
-    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
-    new GuApplicationTargetGroup(stack, "ApplicationTargetGroup", { ...app, vpc });
-
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).toContain("ApplicationTargetGroup");
-  });
-
-  test("does not override the id if the stack migrated value is true but the override id value is false", () => {
-    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
-    new GuApplicationTargetGroup(stack, "ApplicationTargetGroup", { ...app, vpc, overrideId: false });
-
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).not.toContain("ApplicationTargetGroup");
+    expect(stack).toHaveResourceOfTypeAndLogicalId(
+      "AWS::ElasticLoadBalancingV2::TargetGroup",
+      /^ApplicationTargetGroup.+$/
+    );
   });
 
   test("uses default health check properties", () => {

diff --git a/src/constructs/loadbalancing/alb/application-target-group.ts b/src/constructs/loadbalancing/alb/application-target-group.ts
@@ -1,14 +1,14 @@
-import type { ApplicationTargetGroupProps, CfnTargetGroup } from "@aws-cdk/aws-elasticloadbalancingv2";
+import type { ApplicationTargetGroupProps } from "@aws-cdk/aws-elasticloadbalancingv2";
 import { ApplicationProtocol, ApplicationTargetGroup, Protocol } from "@aws-cdk/aws-elasticloadbalancingv2";
 import { Duration } from "@aws-cdk/core";
+import { GuStatefulMigratableConstruct } from "../../../utils/mixin";
 import type { GuStack } from "../../core";
 import { AppIdentity } from "../../core/identity";
+import type { GuMigratingResource } from "../../core/migrating";
 
-export interface GuApplicationTargetGroupProps extends ApplicationTargetGroupProps, AppIdentity {
-  overrideId?: boolean;
-}
+export interface GuApplicationTargetGroupProps extends ApplicationTargetGroupProps, AppIdentity, GuMigratingResource {}
 
-export class GuApplicationTargetGroup extends ApplicationTargetGroup {
+export class GuApplicationTargetGroup extends GuStatefulMigratableConstruct(ApplicationTargetGroup) {
   static DefaultHealthCheck = {
     path: "/healthcheck",
     protocol: Protocol.HTTP,
@@ -29,9 +29,6 @@ export class GuApplicationTargetGroup extends ApplicationTargetGroup {
 
     super(scope, AppIdentity.suffixText({ app }, id), mergedProps);
 
-    if (mergedProps.overrideId || (scope.migratedFromCloudFormation && mergedProps.overrideId !== false))
-      (this.node.defaultChild as CfnTargetGroup).overrideLogicalId(id);
-
     AppIdentity.taggedConstruct({ app }, this);
   }
 }