fix: Update GuClassicLoadBalancer 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 `GuClassicLoadBalancer` construct to adopt the new logic. As of #418 it's as simple as using the `GuStatefulMigratableConstruct` mixin!
guardian · Apr 13, 2021 · 61649ce · 61649ce
1 parent 73ef8ec
commit 61649ce
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 30 deletions.
diff --git a/src/constructs/loadbalancing/elb.test.ts b/src/constructs/loadbalancing/elb.test.ts
@@ -1,4 +1,5 @@
 import "@aws-cdk/assert/jest";
+import "../../utils/test/jest";
 import { SynthUtils } from "@aws-cdk/assert/lib/synth-utils";
 import { Vpc } from "@aws-cdk/aws-ec2";
 import { Stack } from "@aws-cdk/core";
@@ -14,36 +15,21 @@ describe("The GuClassicLoadBalancer class", () => {
     privateSubnetIds: [""],
   });
 
-  test("overrides the id with the overrideId prop", () => {
-    const stack = simpleGuStackForTesting();
-    new GuClassicLoadBalancer(stack, "ClassicLoadBalancer", { vpc, overrideId: true });
+  test("overrides the logicalId when existingLogicalId is set in a migrating stack", () => {
+    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
+    new GuClassicLoadBalancer(stack, "ClassicLoadBalancer", { vpc, existingLogicalId: "MyCLB" });
 
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).toContain("ClassicLoadBalancer");
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::ElasticLoadBalancing::LoadBalancer", "MyCLB");
   });
 
-  test("has an auto-generated ID by default", () => {
+  test("auto-generates the logicalId by default", () => {
     const stack = simpleGuStackForTesting();
     new GuClassicLoadBalancer(stack, "ClassicLoadBalancer", { vpc });
 
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).not.toContain("ClassicLoadBalancer");
-  });
-
-  test("overrides the id if the stack migrated value is true", () => {
-    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
-    new GuClassicLoadBalancer(stack, "ClassicLoadBalancer", { vpc });
-
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).toContain("ClassicLoadBalancer");
-  });
-
-  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 GuClassicLoadBalancer(stack, "ClassicLoadBalancer", { vpc, overrideId: false });
-
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-    expect(Object.keys(json.Resources)).not.toContain("ClassicLoadBalancer");
+    expect(stack).toHaveResourceOfTypeAndLogicalId(
+      "AWS::ElasticLoadBalancing::LoadBalancer",
+      /^ClassicLoadBalancer.+$/
+    );
   });
 
   test("overrides any properties as required", () => {

diff --git a/src/constructs/loadbalancing/elb.ts b/src/constructs/loadbalancing/elb.ts
@@ -6,16 +6,17 @@ import type {
 } from "@aws-cdk/aws-elasticloadbalancing";
 import { LoadBalancer, LoadBalancingProtocol } from "@aws-cdk/aws-elasticloadbalancing";
 import { Duration } from "@aws-cdk/core";
+import { GuStatefulMigratableConstruct } from "../../utils/mixin";
 import type { GuStack } from "../core";
 import { GuArnParameter } from "../core";
+import type { GuMigratingResource } from "../core/migrating";
 
-interface GuClassicLoadBalancerProps extends Omit<LoadBalancerProps, "healthCheck"> {
-  overrideId?: boolean;
+interface GuClassicLoadBalancerProps extends Omit<LoadBalancerProps, "healthCheck">, GuMigratingResource {
   propertiesToOverride?: Record<string, unknown>;
   healthCheck?: Partial<HealthCheck>;
 }
 
-export class GuClassicLoadBalancer extends LoadBalancer {
+export class GuClassicLoadBalancer extends GuStatefulMigratableConstruct(LoadBalancer) {
   static DefaultHealthCheck = {
     port: 9000,
     path: "/healthcheck",
@@ -36,9 +37,6 @@ export class GuClassicLoadBalancer extends LoadBalancer {
 
     const cfnLb = this.node.defaultChild as CfnLoadBalancer;
 
-    if (mergedProps.overrideId || (scope.migratedFromCloudFormation && mergedProps.overrideId !== false))
-      cfnLb.overrideLogicalId(id);
-
     mergedProps.propertiesToOverride &&
       Object.entries(mergedProps.propertiesToOverride).forEach(([key, value]) => cfnLb.addPropertyOverride(key, value));
   }