From 31cea549bcf9a1a590bb217a3967c660c4084bf3 Mon Sep 17 00:00:00 2001 From: Akash Askoolum Date: Wed, 5 May 2021 08:02:17 +0100 Subject: [PATCH] feat: Log when any stateful resource is in stack A resource is a raw CloudFormation item. A construct is CDK's L1 or L2 abstraction of a resource. A stateful resource can be defined as something that holds state. This could be a database, a bucket, load balancer, message queue etc. This change will, upon stack synthesis, walk the tree of resources and log a warning for all the stateful resources we have identified. This does mean we end up keeping a list of these resources, which is not ideal... The `GuStatefulMigratableConstruct` mixin performs a similar role here, however that only operates against the constructs that exist in the library. Ideally we'd be able to use Stack Policies to protect these resources. However they are not currently supported in CDK. See: - https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.Construct.html#protected-prepare - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/protect-stack-resources.html - https://github.com/aws/aws-cdk-rfcs/issues/72 --- src/constants/stateful-resource-types.ts | 23 +++++++++++++++++++ src/constructs/core/stack.test.ts | 25 +++++++++++++++++++++ src/constructs/core/stack.ts | 28 ++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 src/constants/stateful-resource-types.ts diff --git a/src/constants/stateful-resource-types.ts b/src/constants/stateful-resource-types.ts new file mode 100644 index 0000000000..54a68997c9 --- /dev/null +++ b/src/constants/stateful-resource-types.ts @@ -0,0 +1,23 @@ +/** + * A list of resource types that should be considered stateful + * and care should be taken when updating them to ensure they + * are not accidentally replaced as this could lead to downtime. + * + * For example, if a load balancer is accidentally replaced, + * any CNAME DNS entry for it would now be invalid and downtime + * will be incurred for the TTL of the DNS entry. + * + * Currently, this list is used to generate warnings at synth time. + * Ideally we'd add a stack policy to stop the resource being deleted, + * however this isn't currently supported in CDK. + * + * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/protect-stack-resources.html + * @see https://github.com/aws/aws-cdk-rfcs/issues/72 + */ +export const StatefulResourceTypes: string[] = [ + "AWS::CertificateManager::Certificate", + "AWS::DynamoDB::Table", + "AWS::ElasticLoadBalancing::LoadBalancer", + "AWS::ElasticLoadBalancingV2::LoadBalancer", + "AWS::S3::Bucket", +]; diff --git a/src/constructs/core/stack.test.ts b/src/constructs/core/stack.test.ts index 7af6836932..00c3e7bc1d 100644 --- a/src/constructs/core/stack.test.ts +++ b/src/constructs/core/stack.test.ts @@ -2,15 +2,25 @@ import "@aws-cdk/assert/jest"; import { SynthUtils } from "@aws-cdk/assert"; import { Role, ServicePrincipal } from "@aws-cdk/aws-iam"; +import { Bucket } from "@aws-cdk/aws-s3"; import { App } from "@aws-cdk/core"; import { Stage, Stages } from "../../constants"; import { TrackingTag } from "../../constants/tracking-tag"; +import { Logger } from "../../utils/logger"; import { alphabeticalTags, simpleGuStackForTesting } from "../../utils/test"; import type { SynthedStack } from "../../utils/test"; import { GuParameter } from "./parameters"; import { GuStack } from "./stack"; describe("The GuStack construct", () => { + const info = jest.spyOn(Logger, "info"); + const warn = jest.spyOn(Logger, "warn"); + + afterEach(() => { + warn.mockReset(); + info.mockReset(); + }); + it("requires passing the stack value as props", function () { const stack = simpleGuStackForTesting({ stack: "some-stack" }); expect(stack.stack).toEqual("some-stack"); @@ -71,4 +81,19 @@ describe("The GuStack construct", () => { "Attempting to read parameter i-do-not-exist which does not exist" ); }); + + it("During the synthesise process, should advise updating with caution when it contains a stateful resource", () => { + const stack = simpleGuStackForTesting(); + const bucket = new Bucket(stack, "MyBucket"); + SynthUtils.toCloudFormation(stack); + + // `defaultChild can technically be `undefined`. + // We know a `Bucket` has a `defaultChild` so the coalescing is just appeasing the compiler. + const cfnBucketResourcePath = bucket.node.defaultChild?.node.path ?? ""; + + expect(warn).toHaveBeenCalledTimes(1); + expect(warn).toHaveBeenCalledWith( + `The resource '${cfnBucketResourcePath}' of type AWS::S3::Bucket is considered stateful by @guardian/cdk. Care should be taken when updating this resource to avoid accidental replacement as this could lead to downtime` + ); + }); }); diff --git a/src/constructs/core/stack.ts b/src/constructs/core/stack.ts index 7b0aeecdeb..3c6e6be256 100644 --- a/src/constructs/core/stack.ts +++ b/src/constructs/core/stack.ts @@ -1,7 +1,9 @@ -import type { App, StackProps } from "@aws-cdk/core"; -import { Stack, Tags } from "@aws-cdk/core"; +import type { App, IConstruct, StackProps } from "@aws-cdk/core"; +import { CfnResource, Stack, Tags } from "@aws-cdk/core"; import { Stage } from "../../constants"; +import { StatefulResourceTypes } from "../../constants/stateful-resource-types"; import { TrackingTag } from "../../constants/tracking-tag"; +import { Logger } from "../../utils/logger"; import type { StackStageIdentity } from "./identity"; import type { GuStageDependentValue } from "./mappings"; import { GuStageMapping } from "./mappings"; @@ -120,4 +122,26 @@ export class GuStack extends Stack implements StackStageIdentity, GuMigratingSta this.addTag("Stack", this.stack); this.addTag("Stage", this.stage); } + + protected prepare(): void { + super.prepare(); + + /* + Log a message whenever a stateful resource is encountered in the stack. + + Ideally we'd add a stack policy to stop the resource being deleted, + however this isn't currently supported in CDK. + + See: + - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/protect-stack-resources.html + - https://github.com/aws/aws-cdk-rfcs/issues/72 + */ + this.node.findAll().forEach((construct: IConstruct) => { + if (CfnResource.isCfnResource(construct) && StatefulResourceTypes.includes(construct.cfnResourceType)) { + Logger.warn( + `The resource '${construct.node.path}' of type ${construct.cfnResourceType} is considered stateful by @guardian/cdk. Care should be taken when updating this resource to avoid accidental replacement as this could lead to downtime` + ); + } + }); + } }