diff --git a/eslint/rules/valid-constructors.js b/eslint/rules/valid-constructors.js index 9285f59e32..ce03135758 100644 --- a/eslint/rules/valid-constructors.js +++ b/eslint/rules/valid-constructors.js @@ -40,7 +40,7 @@ const lintParameter = (param, node, context, { name, type, allowOptional, allowD }); } - const currentName = param.name ?? param.left.name; + const currentName = param.name ?? param.left?.name ?? param.argument.name; if (currentName !== name) { return context.report({ diff --git a/src/constructs/acm/certificate.ts b/src/constructs/acm/certificate.ts index 89ee29c8cf..2ac50250e1 100644 --- a/src/constructs/acm/certificate.ts +++ b/src/constructs/acm/certificate.ts @@ -3,10 +3,10 @@ import type { CertificateProps } from "@aws-cdk/aws-certificatemanager/lib/certi import { HostedZone } from "@aws-cdk/aws-route53"; import { RemovalPolicy } from "@aws-cdk/core"; import { Stage } from "../../constants"; +import { GuStatefulMigratableConstruct } from "../../utils/mixin"; import type { GuStack } from "../core"; import { AppIdentity } from "../core/identity"; -import { GuMigratingResource } from "../core/migrating"; -import type { GuStatefulConstruct } from "../core/migrating"; +import type { GuMigratingResource } from "../core/migrating"; export type GuCertificateProps = Record & GuMigratingResource & AppIdentity; @@ -57,8 +57,7 @@ export interface GuDnsValidatedCertificateProps { * }); *``` */ -export class GuCertificate extends Certificate implements GuStatefulConstruct { - isStatefulConstruct: true; +export class GuCertificate extends GuStatefulMigratableConstruct(Certificate) { constructor(scope: GuStack, id: string, props: GuCertificateProps) { const maybeHostedZone = props.CODE.hostedZoneId && props.PROD.hostedZoneId @@ -71,17 +70,16 @@ export class GuCertificate extends Certificate implements GuStatefulConstruct { }) ) : undefined; - const awsCertificateProps: CertificateProps = { + const awsCertificateProps: CertificateProps & GuMigratingResource = { domainName: scope.withStageDependentValue({ variableName: "domainName", stageValues: { [Stage.CODE]: props.CODE.domainName, [Stage.PROD]: props.PROD.domainName }, }), validation: CertificateValidation.fromDns(maybeHostedZone), + existingLogicalId: props.existingLogicalId, }; super(scope, id, awsCertificateProps); this.applyRemovalPolicy(RemovalPolicy.RETAIN); - this.isStatefulConstruct = true; - GuMigratingResource.setLogicalId(this, scope, { existingLogicalId: props.existingLogicalId }); AppIdentity.taggedConstruct({ app: props.app }, this); } } diff --git a/src/constructs/core/migrating.test.ts b/src/constructs/core/migrating.test.ts index 7bd69c0604..c7b51db049 100644 --- a/src/constructs/core/migrating.test.ts +++ b/src/constructs/core/migrating.test.ts @@ -3,9 +3,9 @@ import { SynthUtils } from "@aws-cdk/assert"; import type { BucketProps } from "@aws-cdk/aws-s3"; import { Bucket } from "@aws-cdk/aws-s3"; import { Logger } from "../../utils/logger"; +import type { GuStatefulConstruct } from "../../utils/mixin"; 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"; diff --git a/src/constructs/core/migrating.ts b/src/constructs/core/migrating.ts index 1705452fb4..7ad1ffc601 100644 --- a/src/constructs/core/migrating.ts +++ b/src/constructs/core/migrating.ts @@ -1,5 +1,6 @@ import type { CfnElement, IConstruct } from "@aws-cdk/core"; import { Logger } from "../../utils/logger"; +import { isGuStatefulConstruct } from "../../utils/mixin"; export interface GuMigratingStack { /** @@ -10,7 +11,12 @@ export interface GuMigratingStack { * @see GuMigratingResource * @see GuStack */ - migratedFromCloudFormation?: boolean; + migratedFromCloudFormation: boolean; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -- user defined type guard +export function isGuMigratingStack(construct: any): construct is GuMigratingStack { + return "migratedFromCloudFormation" in construct; } export interface GuMigratingResource { @@ -29,17 +35,8 @@ 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( + setLogicalId( construct: T, { migratedFromCloudFormation }: GuMigratingStack, { existingLogicalId }: GuMigratingResource diff --git a/src/constructs/core/stack.ts b/src/constructs/core/stack.ts index 1191647511..dbe57eb900 100644 --- a/src/constructs/core/stack.ts +++ b/src/constructs/core/stack.ts @@ -9,7 +9,7 @@ import type { GuMigratingStack } from "./migrating"; import type { GuParameter } from "./parameters"; import { GuStageParameter } from "./parameters"; -export interface GuStackProps extends StackProps, GuMigratingStack { +export interface GuStackProps extends StackProps, Partial { stack: string; } @@ -40,7 +40,7 @@ export interface GuStackProps extends StackProps, GuMigratingStack { * } * ``` */ -export class GuStack extends Stack implements StackStageIdentity { +export class GuStack extends Stack implements StackStageIdentity, GuMigratingStack { private readonly _stack: string; private readonly _stage: string; diff --git a/src/utils/mixin/index.ts b/src/utils/mixin/index.ts new file mode 100644 index 0000000000..619e97c2c8 --- /dev/null +++ b/src/utils/mixin/index.ts @@ -0,0 +1,2 @@ +export * from "./migratable-construct"; +export * from "./migratable-construct-stateful"; diff --git a/src/utils/mixin/migratable-construct-stateful.test.ts b/src/utils/mixin/migratable-construct-stateful.test.ts new file mode 100644 index 0000000000..5dc41b7c20 --- /dev/null +++ b/src/utils/mixin/migratable-construct-stateful.test.ts @@ -0,0 +1,42 @@ +import type { BucketProps } from "@aws-cdk/aws-s3"; +import "../test/jest"; +import { Bucket } from "@aws-cdk/aws-s3"; +import type { GuStack } from "../../constructs/core"; +import type { GuMigratingResource } from "../../constructs/core/migrating"; +import { Logger } from "../logger"; +import { simpleGuStackForTesting } from "../test"; +import { GuStatefulMigratableConstruct } from "./migratable-construct-stateful"; + +interface TestGuMigratableConstructProps extends BucketProps, GuMigratingResource {} + +class StatefulTestGuMigratableConstruct extends GuStatefulMigratableConstruct(Bucket) { + constructor(scope: GuStack, id: string, props: TestGuMigratableConstructProps) { + super(scope, id, props); + } +} + +describe("The GuStatefulMigratableConstruct mixin", () => { + const info = jest.spyOn(Logger, "info"); + + beforeEach(() => { + info.mockReset(); + }); + + it("should add the `isStatefulConstruct` property when used", () => { + const stack = simpleGuStackForTesting(); + const bucket = new StatefulTestGuMigratableConstruct(stack, "MyBucket", {}); + + expect(bucket.isStatefulConstruct).toBe(true); + }); + + it("should trigger a warning when creating a stateful construct in a new stack", () => { + const stack = simpleGuStackForTesting(); + new StatefulTestGuMigratableConstruct(stack, "MyBucket", {}); + + expect(info).toHaveBeenCalledTimes(1); + expect(info).toHaveBeenCalledWith( + "GuStack has 'migratedFromCloudFormation' set to false. MyBucket is a stateful construct, it's logicalId will be auto-generated and AWS will create a new resource." + ); + expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::S3::Bucket", /^MyBucket.+/); + }); +}); diff --git a/src/utils/mixin/migratable-construct-stateful.ts b/src/utils/mixin/migratable-construct-stateful.ts new file mode 100644 index 0000000000..1ff7066d8d --- /dev/null +++ b/src/utils/mixin/migratable-construct-stateful.ts @@ -0,0 +1,57 @@ +import { GuMigratableConstruct } from "./migratable-construct"; +import type { AnyConstructor } from "./types"; + +export interface GuStatefulConstruct { + /** + * A flag to signal to `GuMigratingResource` that a construct is stateful and care should be taken when migrating to GuCDK. + * If one accidentally replaces a stateful resource, downstream services such as DNS may be impacted. + */ + isStatefulConstruct: true; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types -- user defined type guard +export function isGuStatefulConstruct(construct: any): construct is GuStatefulConstruct { + return "isStatefulConstruct" in construct; +} + +/** + * A mixin to add the property `isStatefulConstruct` to a class and execute logic to conditionally override a construct's logicalId when synthesised. + * + * If one accidentally replaces a stateful resource, downstream services such as DNS may be impacted. + * + * Usage: + * ```typescript + * class MyClass extends GuStatefulMigratableConstruct(SomeAwsConstruct) { } + * ``` + * + * This results in any new instance of `MyClass` having an `isStatefulConstruct` accessor field: + * + * ```typescript + * const resource = new MyClass(); + * console.log(resource.isStatefulConstruct) // true + * ``` + * + * Note, if you have `experimentalDecorators` enabled, you can use it as such: + * + * ```typescript + * @GuStatefulMigratableConstruct + * class MyClass extends SomeAwsConstruct { } + * ``` + * As the name suggest, decorators are experimental. + * + * @param BaseClass the class to apply the mixin to + * @constructor + * + * @see GuMigratableConstruct + * @see GuMigratingResource + * @see https://www.typescriptlang.org/docs/handbook/mixins.html + * @see https://www.typescriptlang.org/docs/handbook/decorators.html + */ +// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -- mixin +export function GuStatefulMigratableConstruct(BaseClass: TBase) { + return class extends GuMigratableConstruct(BaseClass) implements GuStatefulConstruct { + get isStatefulConstruct(): true { + return true; + } + }; +} diff --git a/src/utils/mixin/migratable-construct.test.ts b/src/utils/mixin/migratable-construct.test.ts new file mode 100644 index 0000000000..41d824021d --- /dev/null +++ b/src/utils/mixin/migratable-construct.test.ts @@ -0,0 +1,47 @@ +import type { BucketProps } from "@aws-cdk/aws-s3"; +import "../test/jest"; +import { Bucket } from "@aws-cdk/aws-s3"; +import type { GuStack } from "../../constructs/core"; +import { GuMigratingResource } from "../../constructs/core/migrating"; +import { simpleGuStackForTesting } from "../test"; +import { GuMigratableConstruct } from "./migratable-construct"; + +interface TestGuMigratableConstructProps extends BucketProps, GuMigratingResource {} + +class TestGuMigratableConstruct extends GuMigratableConstruct(Bucket) { + constructor(scope: GuStack, id: string, props: TestGuMigratableConstructProps) { + super(scope, id, props); + } +} + +describe("The GuMigratableConstruct mixin", () => { + const spy = jest.spyOn(GuMigratingResource, "setLogicalId"); + + afterEach(() => { + spy.mockReset(); + }); + + it("should call GuMigratingResource.setLogicalId when the stack is being migrated and existingLogicalId is set", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: true }); + new TestGuMigratableConstruct(stack, "MyBucket", { existingLogicalId: "Hello" }); + + expect(spy).toHaveBeenCalledTimes(1); + expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::S3::Bucket", "Hello"); + }); + + it("should call GuMigratingResource.setLogicalId when the stack is not being migrated and existingLogicalId is set", () => { + const stack = simpleGuStackForTesting({ migratedFromCloudFormation: false }); + new TestGuMigratableConstruct(stack, "MyBucket", { existingLogicalId: "Hello" }); + + expect(spy).toHaveBeenCalledTimes(1); + expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::S3::Bucket", /^MyBucket.+/); + }); + + it("should call GuMigratingResource.setLogicalId even when existingLogicalId is undefined", () => { + const stack = simpleGuStackForTesting(); + new TestGuMigratableConstruct(stack, "MyBucket", {}); + + expect(spy).toHaveBeenCalledTimes(1); + expect(stack).toHaveResourceOfTypeAndLogicalId("AWS::S3::Bucket", /^MyBucket.+/); + }); +}); diff --git a/src/utils/mixin/migratable-construct.ts b/src/utils/mixin/migratable-construct.ts new file mode 100644 index 0000000000..e5b91733fc --- /dev/null +++ b/src/utils/mixin/migratable-construct.ts @@ -0,0 +1,90 @@ +import { Construct } from "@aws-cdk/core"; +import type { GuMigratingStack } from "../../constructs/core/migrating"; +import { GuMigratingResource, isGuMigratingStack } from "../../constructs/core/migrating"; +import type { AnyConstructor } from "./types"; + +/** + * A mixin to add the property `isStatefulConstruct` to a class and execute logic to conditionally override a construct's logicalId when synthesised. + * + * Overriding the logicalId makes a migration easier as the resulting CloudFormation template has a smaller difference from that of a running stack. + * + * For example, say you have a stack defined in YAML that creates a bucket: + * + * ```yaml + * MyBucket: + * Type: AWS::S3::Bucket + * ``` + * + * By default, the CDK library will auto-generate the logicalId of resources. That is, by default a bucket would look like: + * + * ```yaml + * MyBucketA0B1C2D: + * Type: AWS::S3::Bucket + * ``` + * + * Override the logicalId to keep it to `MyBucket`. + * + * Usage: + * ```typescript + * class MyClass extends GuMigratableConstruct(SomeAwsConstruct) { } + *``` + * + * Note, if you have `experimentalDecorators` enabled, you can use it as such: + * + * ```typescript + * @GuMigratableConstruct + * class MyClass extends SomeAwsConstruct { + * + * } + * ``` + * As the name suggest, decorators are experimental. + * + * @param BaseClass the class to apply the mixin to + * @constructor + * + * @see GuStatefulMigratableConstruct + * @see GuMigratingResource + * @see https://www.typescriptlang.org/docs/handbook/mixins.html + * @see https://www.typescriptlang.org/docs/handbook/decorators.html + */ +// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types -- mixin +export function GuMigratableConstruct(BaseClass: TBase) { + return class extends BaseClass { + // eslint-disable-next-line custom-rules/valid-constructors, @typescript-eslint/no-explicit-any -- mixin + constructor(...args: any[]) { + // `super` is the parent AWS constructor here + super(...args); + + // Constrained mixins could be used here but couldn't get it to work + // See https://www.typescriptlang.org/docs/handbook/mixins.html#constrained-mixins + if (Construct.isConstruct(this)) { + // the parent AWS constructor presents a common signature of `scope`, `id`, `props` + if (args.length === 3) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- mixin + const [scope, id, props] = args; + + const isAMigratingStack = isGuMigratingStack(scope); + const isIdAString = typeof id === "string"; + + /* + A test for `props` would be to check if it's a `GuMigratingResource`. + That is, is `existingLogicalId` present. + However, this would be `false` if `existingLogicalId` is `undefined` + and `GuMigratingResource.setLogicalId` won't get called. + */ + const looksLikeAConstruct = isAMigratingStack && isIdAString; + + if (looksLikeAConstruct) { + GuMigratingResource.setLogicalId( + this, + { + migratedFromCloudFormation: (scope as GuMigratingStack).migratedFromCloudFormation, + }, + props + ); + } + } + } + } + }; +} diff --git a/src/utils/mixin/types.ts b/src/utils/mixin/types.ts new file mode 100644 index 0000000000..8c9b6e176f --- /dev/null +++ b/src/utils/mixin/types.ts @@ -0,0 +1,2 @@ +// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/ban-types -- mixin +export type AnyConstructor = new (...args: any[]) => {};