fix(aws-dynamodb): Propogate DeletionPolicy to replicas

[TheSPD]

Motivation: When a Global DDB Table is created with multiple regions, the retention policy is not propagated to the Custom::DynamoDBReplica resources that get created under cloudformation.

Say if we want the retentionpolicy to be "Retain", when we delete the CDK object, the table in main region is retained whereas all the replicas are deleted. This is not ideal.

Fixes #17011

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io]

[skinny85]

Thanks for the PR @TheSPD! Two small comments from me.

[skinny85]

Any reason for this assertion here? I don't really see it having much to do with this test... I would just remove it:

Suggested change

	expect(SynthUtils.toCloudFormation(stack).Conditions).toEqual({
	TableStackRegionNotEqualseuwest2A03859E7: {
	'Fn::Not': [
	{ 'Fn::Equals': ['eu-west-2', { Ref: 'AWS::Region' }] },
	],
	},
	TableStackRegionNotEqualseucentral199D46FC0: {
	'Fn::Not': [
	{ 'Fn::Equals': ['eu-central-1', { Ref: 'AWS::Region' }] },
	],
	},
	});
	});

[skinny85]

What about the default removalPolicy?

The default RemovalPolicy for Table is RETAIN. It seems like we should also make it RETAIN for the Custom Resource in that case as well?

[TheSPD]

passing in the default in the method parameters

Pull request has been modified.

[TheSPD]

Testing for default behavior

[TheSPD]

passing in the default in the method parameters

[skinny85]

Looks like some linter errors snuck through 🙂:

@aws-cdk/aws-dynamodb: /codebuild/output/src556108923/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-dynamodb/lib/table.ts
@aws-cdk/aws-dynamodb:   1513:1  error  This line has a length of 161. Maximum allowed is 150  max-len
@aws-cdk/aws-dynamodb: /codebuild/output/src556108923/src/github.com/aws/aws-cdk/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts
@aws-cdk/aws-dynamodb:   9:1  warning  `@aws-cdk/cdk-build-tools/lib/feature-flag` import should occur before import of `@aws-cdk/core`  import/order
@aws-cdk/aws-dynamodb: ✖ 2 problems (1 error, 1 warning)

[skinny85]

@TheSPD I actually had another thought: does making the RemovalPolicy of the Custom Resource Retain also make the Lambda functions used to implement it remain after the Stack has been deleted? Because if that's the case, I'm not a huge fan of this change - I don't want to leave junk in the customer's account after they delete the Stack.

[TheSPD]

Hmm, I understand your concerns here @skinny85, I think it'll leave all those lambdas and step functions hanging around.

However, my 2 cents in trying to change your opinions on this.

1. After creating a replica for the first time, the lambdas don't do anything, so technically they're junk currently as well.
2. I also believe, it's more valuable for customers to retain DB replicas(if that's what they're specifying) vs the cost of leaving junk around (that could be cleaned up manually).
3. If stacks are being deleted, CFn does mention all the resources that were left around as DELETE_SKIPPED and customers can be expected to perform additional manual cleanup if the need to.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 2e8ce06
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[skinny85]

Actually @TheSPD, I just realized you're propagating the Retain Removal Policy only to the Custom::DynamoDBReplica resource, and not to the actual Lambdas that implement it... Given that, I wonder of the actual Lambdas maybe do get cleaned up, and it's only the Custom Resources that remain (which is fine, since I think they don't actually consist of any resources).

Could you do a test, and let me know if it works like that?

[skinny85]

I'm closing this one due to inactivity. @TheSPD let me know if you want to resume working on this.