From c84498f7250544cc1975a96c9d8bb08fc291d8a5 Mon Sep 17 00:00:00 2001 From: Akash Askoolum Date: Wed, 31 Mar 2021 16:23:28 +0100 Subject: [PATCH] refactor: encapsulate logicalId logic in an interface In an attempt to keep things DRY, encapsulate the logic around overriding a construct's logicalId in one place and add tests. --- src/constructs/core/migrating.test.ts | 111 ++++++++++++++++++++++++++ src/constructs/core/migrating.ts | 55 +++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 src/constructs/core/migrating.test.ts create mode 100644 src/constructs/core/migrating.ts diff --git a/src/constructs/core/migrating.test.ts b/src/constructs/core/migrating.test.ts new file mode 100644 index 0000000000..212701f265 --- /dev/null +++ b/src/constructs/core/migrating.test.ts @@ -0,0 +1,111 @@ +import "@aws-cdk/assert/jest"; +import { SynthUtils } from "@aws-cdk/assert"; +import type { BucketProps } from "@aws-cdk/aws-s3"; +import { Bucket } from "@aws-cdk/aws-s3"; +import type { SynthedStack } from "../../utils/test"; +import { simpleGuStackForTesting } from "../../utils/test"; +import type { GuStatefulConstruct } from "./migrating"; +import { GuMigratingResource } from "./migrating"; +import type { GuStack } from "./stack"; + +class TestGuStatefulConstruct extends Bucket implements GuStatefulConstruct { + isStatefulConstruct: true; + + constructor(scope: GuStack, id: string, props?: BucketProps) { + super(scope, id, props); + this.isStatefulConstruct = true; + } +} + +/* +NOTE: In reality, we'd never directly call `GuMigratingResource.setLogicalId` as the GuConstruct will do that. +We're calling it here to test the function in isolation. + */ + +describe("GuMigratingResource", () => { + // eslint-disable-next-line @typescript-eslint/no-empty-function -- we are testing `console.info` being called, we don't need to see the message + const info = jest.spyOn(console, "info").mockImplementation(() => {}); + + // eslint-disable-next-line @typescript-eslint/no-empty-function -- we are testing `console.warn` being called, we don't need to see the message + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + + afterEach(() => { + warn.mockReset(); + info.mockReset(); + }); + + test("Creating a new resource in a new stack produces a new auto-generated ID", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: false }); + const construct = new Bucket(stack, "MyBucket"); + + GuMigratingResource.setLogicalId(construct, stack, {}); + + expect(warn).toHaveBeenCalledTimes(0); + expect(info).toHaveBeenCalledTimes(0); + + const json = SynthUtils.toCloudFormation(stack) as SynthedStack; + const resourceKeys = Object.keys(json.Resources); + + expect(resourceKeys).toHaveLength(1); + expect(resourceKeys[0]).toMatch(/^MyBucket[A-Z0-9]+$/); + }); + + test("Keeping a resource's logicalId when migrating a stack", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true }); + const construct = new Bucket(stack, "MyBucket"); + + GuMigratingResource.setLogicalId(construct, stack, { existingLogicalId: "my-pre-existing-bucket" }); + + expect(warn).toHaveBeenCalledTimes(0); + expect(info).toHaveBeenCalledTimes(0); + + const json = SynthUtils.toCloudFormation(stack) as SynthedStack; + expect(Object.keys(json.Resources)).toContain("my-pre-existing-bucket"); + }); + + test("Creating a construct in a migrating stack, w/out setting existingLogicalId", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true }); + const construct = new Bucket(stack, "MyBucket"); + + GuMigratingResource.setLogicalId(construct, stack, {}); + + expect(info).toHaveBeenCalledTimes(0); + expect(warn).toHaveBeenCalledTimes(0); + + const json = SynthUtils.toCloudFormation(stack) as SynthedStack; + const resourceKeys = Object.keys(json.Resources); + + expect(resourceKeys).toHaveLength(1); + expect(resourceKeys[0]).toMatch(/^MyBucket[A-Z0-9]+$/); + }); + + test("Specifying a construct's existingLogicalId in a new stack", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: false }); + const construct = new Bucket(stack, "MyBucket"); + + GuMigratingResource.setLogicalId(construct, stack, { existingLogicalId: "my-pre-existing-bucket" }); + + expect(info).toHaveBeenCalledTimes(0); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + "GuStack has 'migratedFromCloudFormation' set to false. MyBucket has an 'existingLogicalId' set to my-pre-existing-bucket. This will have no effect - the logicalId will be auto-generated. Set 'migratedFromCloudFormation' to true for 'existingLogicalId' to be observed." + ); + + const json = SynthUtils.toCloudFormation(stack) as SynthedStack; + const resourceKeys = Object.keys(json.Resources); + + expect(resourceKeys).toHaveLength(1); + expect(resourceKeys[0]).toMatch(/^MyBucket[A-Z0-9]+$/); + }); + + test("Creating a stateful construct in a migrating stack, w/out setting existingLogicalId", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true }); + const construct = new TestGuStatefulConstruct(stack, "MyBucket"); + + GuMigratingResource.setLogicalId(construct, stack, {}); + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + "GuStack has 'migratedFromCloudFormation' set to true. MyBucket is a stateful construct and 'existingLogicalId' has not been set. MyBucket's logicalId will be auto-generated. This is not advised as downstream services, such as DNS, will likely need updating." + ); + }); +}); diff --git a/src/constructs/core/migrating.ts b/src/constructs/core/migrating.ts new file mode 100644 index 0000000000..4f10dadfb0 --- /dev/null +++ b/src/constructs/core/migrating.ts @@ -0,0 +1,55 @@ +import type { CfnElement, IConstruct } from "@aws-cdk/core"; +import type { GuStack } from "./stack"; + +export interface GuMigratingResource { + existingLogicalId?: string; +} + +export interface GuStatefulConstruct extends IConstruct { + isStatefulConstruct: true; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any -- user defined type guard +function isGuStatefulConstruct(construct: any): construct is GuStatefulConstruct { + return "isStatefulConstruct" in construct; +} + +export const GuMigratingResource = { + setLogicalId( + construct: T, + { migratedFromCloudFormation }: GuStack, + { existingLogicalId }: GuMigratingResource + ): void { + const overrideLogicalId = (logicalId: string) => { + const defaultChild = construct.node.defaultChild as CfnElement; + defaultChild.overrideLogicalId(logicalId); + }; + + const id = construct.node.id; + const isStateful = isGuStatefulConstruct(construct); + + if (migratedFromCloudFormation) { + if (existingLogicalId) { + return overrideLogicalId(existingLogicalId); + } + + if (isStateful) { + console.warn( + `GuStack has 'migratedFromCloudFormation' set to true. ${id} is a stateful construct and 'existingLogicalId' has not been set. ${id}'s logicalId will be auto-generated. This is not advised as downstream services, such as DNS, will likely need updating.` + ); + } + } else { + if (existingLogicalId) { + console.warn( + `GuStack has 'migratedFromCloudFormation' set to false. ${id} has an 'existingLogicalId' set to ${existingLogicalId}. This will have no effect - the logicalId will be auto-generated. Set 'migratedFromCloudFormation' to true for 'existingLogicalId' to be observed.` + ); + } + + if (isStateful) { + console.info( + `GuStack has 'migratedFromCloudFormation' set to false. ${id} is a stateful construct, it's logicalId will be new and auto-generated.` + ); + } + } + }, +};