Skip to content

Commit

Permalink
feat: Log when any stateful resource is in stack
Browse files Browse the repository at this point in the history
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
  - aws/aws-cdk-rfcs#72
  • Loading branch information
akash1810 committed May 5, 2021
1 parent 46252b0 commit 74124e3
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 2 deletions.
23 changes: 23 additions & 0 deletions src/constants/stateful-resource-types.ts
Original file line number Diff line number Diff line change
@@ -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",
];
23 changes: 23 additions & 0 deletions src/constructs/core/stack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,23 @@ 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 warn = jest.spyOn(Logger, "warn");

afterEach(() => {
warn.mockReset();
});

it("requires passing the stack value as props", function () {
const stack = simpleGuStackForTesting({ stack: "some-stack" });
expect(stack.stack).toEqual("some-stack");
Expand Down Expand Up @@ -71,4 +79,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.`
);
});
});
28 changes: 26 additions & 2 deletions src/constructs/core/stack.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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.`
);
}
});
}
}

0 comments on commit 74124e3

Please sign in to comment.