feat: Log when any stateful resource is in stack #530
Conversation
| * @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[] = [ |
There was a problem hiding this comment.
PR is in draft whilst we identify items in this list (and also discuss if this PR is a good idea, as I'm not entirely sold...).
There was a problem hiding this comment.
A few suggestions:
AWS::SNS::Topic (when used as an API for other services)
AWS::SQS::Queue
AWS::Kinesis::Stream
AWS::RDS::DBInstance
There must also be an API Gateway resource which should be included in this list (i.e. the resource which maintains the stable URL which is part of the CNAME DNS entry), but I'm struggling to figure out which resource that is!
| @@ -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 { | |||
There was a problem hiding this comment.
The docs are a little scary, but we're only logging so should be ok?!
Perform final modifications before synthesis.
This method can be implemented by derived constructs in order to perform final changes before synthesis. prepare() will be called after child constructs have been prepared.
This is an advanced framework feature. Only use this if you understand the implications.
| */ | ||
| this.node.findAll().forEach((construct: IConstruct) => { | ||
| if (CfnResource.isCfnResource(construct) && StatefulResourceTypes.includes(construct.cfnResourceType)) { | ||
| Logger.warn( |
There was a problem hiding this comment.
We could call addDeletionOverride here, as we do for the GuAutoScalingGroup.
akash1810
force-pushed
the
aa-stateful
branch
3 times, most recently
from
74124e3 to
6f2426b
Compare
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
|
This change has a risk of introducing noise which one can become blind to. Closing in favour of guardian/riff-raff#623 which makes RiffRaff protect resources via stack policy before performing it's cloudformation deploy. |
What does this change?
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.
The resource '<PATH IN TREE>' of type <TYPE> is considered stateful by @guardian/cdk. Care should be taken when updating this resource to avoid accidental replacement as this could lead to downtime.This does mean we end up keeping a list of these resources, which is not ideal...
The
GuStatefulMigratableConstructmixin 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:
Does this change require changes to existing projects or CDK CLI?
No.
Does this change require changes to the library documentation?
Yes and added.
How to test
See added test.
How can we measure success?
We warn about updating any stateful resource, even ones not covered by
GuStatefulMigratableConstruct.Have we considered potential risks?
The risk is the
StatefulResourceTypesis incomplete or becomes stale.