Merge pull request #433 from guardian/aa-GuDatabaseInstance-logicalId

fix: Update `GuDatabaseInstance` with revised logicalId logic
guardian · Apr 13, 2021 · 060d18e · 060d18e
2 parents 7cc5efc + d335034
commit 060d18e
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 53 deletions.
diff --git a/src/constructs/rds/instance.test.ts b/src/constructs/rds/instance.test.ts
@@ -1,11 +1,10 @@
 import "@aws-cdk/assert/jest";
-import { SynthUtils } from "@aws-cdk/assert/lib/synth-utils";
+import "../../utils/test/jest";
 import { Vpc } from "@aws-cdk/aws-ec2";
 import { DatabaseInstanceEngine, ParameterGroup, PostgresEngineVersion } from "@aws-cdk/aws-rds";
 import { Stack } from "@aws-cdk/core";
 import { TrackingTag } from "../../constants/library-info";
 import { alphabeticalTags, simpleGuStackForTesting } from "../../utils/test";
-import type { SynthedStack } from "../../utils/test";
 import { GuDatabaseInstance } from "./instance";
 
 describe("The GuDatabaseInstance class", () => {
@@ -107,70 +106,33 @@ describe("The GuDatabaseInstance class", () => {
     });
   });
 
-  it("overrides the id if the prop is true", () => {
-    const stack = simpleGuStackForTesting();
-    new GuDatabaseInstance(stack, "DatabaseInstance", {
-      vpc,
-      overrideId: true,
-      instanceType: "t3.small",
-      engine: DatabaseInstanceEngine.postgres({
-        version: PostgresEngineVersion.VER_11_8,
-      }),
-      app: "testing",
-    });
-
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-
-    expect(Object.keys(json.Resources)).toContain("DatabaseInstance");
-  });
-
-  it("does not override the id if the prop is false", () => {
-    const stack = simpleGuStackForTesting();
-    new GuDatabaseInstance(stack, "DatabaseInstance", {
-      vpc,
-      instanceType: "t3.small",
-      engine: DatabaseInstanceEngine.postgres({
-        version: PostgresEngineVersion.VER_11_8,
-      }),
-      app: "testing",
-    });
-
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-
-    expect(Object.keys(json.Resources)).not.toContain("DatabaseInstance");
-  });
-
-  it("overrides the id if the stack migrated value is true", () => {
+  it("overrides the logicalId when existingLogicalId is set in a migrating stack", () => {
     const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
     new GuDatabaseInstance(stack, "DatabaseInstance", {
       vpc,
+      existingLogicalId: "MyDb",
       instanceType: "t3.small",
       engine: DatabaseInstanceEngine.postgres({
         version: PostgresEngineVersion.VER_11_8,
       }),
       app: "testing",
     });
 
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-
-    expect(Object.keys(json.Resources)).toContain("DatabaseInstance");
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::RDS::DBInstance", "MyDb");
   });
 
-  it("does not override the id if the stack migrated value is true but the override id value is false", () => {
-    const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true });
+  test("auto-generates the logicalId by default", () => {
+    const stack = simpleGuStackForTesting();
     new GuDatabaseInstance(stack, "DatabaseInstance", {
       vpc,
-      overrideId: false,
       instanceType: "t3.small",
       engine: DatabaseInstanceEngine.postgres({
         version: PostgresEngineVersion.VER_11_8,
       }),
       app: "testing",
     });
 
-    const json = SynthUtils.toCloudFormation(stack) as SynthedStack;
-
-    expect(Object.keys(json.Resources)).not.toContain("DatabaseInstance");
+    expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::RDS::DBInstance", /^DatabaseInstance.+$/);
   });
 
   test("sets the deletion protection value to true by default", () => {

diff --git a/src/constructs/rds/instance.ts b/src/constructs/rds/instance.ts
@@ -1,17 +1,21 @@
 import { InstanceType } from "@aws-cdk/aws-ec2";
-import type { CfnDBInstance, DatabaseInstanceProps, IParameterGroup } from "@aws-cdk/aws-rds";
+import type { DatabaseInstanceProps, IParameterGroup } from "@aws-cdk/aws-rds";
 import { DatabaseInstance, ParameterGroup } from "@aws-cdk/aws-rds";
 import { Fn } 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 GuDatabaseInstanceProps extends Omit<DatabaseInstanceProps, "instanceType">, AppIdentity {
-  overrideId?: boolean;
+export interface GuDatabaseInstanceProps
+  extends Omit<DatabaseInstanceProps, "instanceType">,
+    AppIdentity,
+    GuMigratingResource {
   instanceType: string;
   parameters?: Record<string, string>;
 }
 
-export class GuDatabaseInstance extends DatabaseInstance {
+export class GuDatabaseInstance extends GuStatefulMigratableConstruct(DatabaseInstance) {
   constructor(scope: GuStack, id: string, props: GuDatabaseInstanceProps) {
     // CDK just wants "t3.micro" format, whereas
     // some CFN yaml might have the older "db.t3.micro" with the "db." prefix
@@ -35,10 +39,6 @@ export class GuDatabaseInstance extends DatabaseInstance {
       ...(parameterGroup && { parameterGroup }),
     });
 
-    if (props.overrideId || (scope.migratedFromCloudFormation && props.overrideId !== false)) {
-      (this.node.defaultChild as CfnDBInstance).overrideLogicalId(id);
-    }
-
     parameterGroup && AppIdentity.taggedConstruct(props, parameterGroup);
     AppIdentity.taggedConstruct(props, this);
   }