Skip to content

Commit a67c3f5

Browse files
GavinZZGavinZZ
authored
chore: retain replica table (under feature flag) (#33970)
Follow up to this PR to add feature flag #33953
1 parent 2a8a8a3 commit a67c3f5

16 files changed

+171
-33
lines changed

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global-replicas-provisioned.js.snapshot/aws-cdk-dynamodb-global-replicas-provisioned.assets.json

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global-replicas-provisioned.js.snapshot/aws-cdk-dynamodb-global-replicas-provisioned.template.json

+2-4
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,7 @@
166166
"TableName": {
167167
"Ref": "TableCD117FA1"
168168
},
169-
"Region": "us-east-2",
170-
"SkipReplicaDeletion": false
169+
"Region": "us-east-2"
171170
},
172171
"DependsOn": [
173172
"TableSourceTableAttachedManagedPolicyawscdkdynamodbglobalreplicasprovisionedawscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRoleBE2B1C1A5DC546D2",
@@ -191,8 +190,7 @@
191190
"TableName": {
192191
"Ref": "TableCD117FA1"
193192
},
194-
"Region": "eu-west-3",
195-
"SkipReplicaDeletion": false
193+
"Region": "eu-west-3"
196194
},
197195
"DependsOn": [
198196
"TableSourceTableAttachedManagedPolicyawscdkdynamodbglobalreplicasprovisionedawscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRoleBE2B1C1A5DC546D2",

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global-replicas-provisioned.js.snapshot/manifest.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global-replicas-provisioned.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
66
const app = new cdk.App({
77
postCliContext: {
88
'@aws-cdk/aws-lambda:createNewPoliciesWithAddToRolePolicy': false,
9+
'@aws-cdk/aws-dynamodb:retainTableReplica': false,
910
},
1011
});
1112
const stack = new cdk.Stack(app, 'aws-cdk-dynamodb-global-replicas-provisioned');

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global.js.snapshot/cdk-dynamodb-global-20191121.assets.json

+3-3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global.js.snapshot/cdk-dynamodb-global-20191121.template.json

+2-4
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,7 @@
195195
"TableName": {
196196
"Ref": "TableCD117FA1"
197197
},
198-
"Region": "eu-west-2",
199-
"SkipReplicaDeletion": false
198+
"Region": "eu-west-2"
200199
},
201200
"DependsOn": [
202201
"TableSourceTableAttachedManagedPolicycdkdynamodbglobal20191121awscdkawsdynamodbReplicaProviderIsCompleteHandlerServiceRole3971612857304880",
@@ -217,8 +216,7 @@
217216
"TableName": {
218217
"Ref": "TableCD117FA1"
219218
},
220-
"Region": "eu-central-1",
221-
"SkipReplicaDeletion": false
219+
"Region": "eu-central-1"
222220
},
223221
"DependsOn": [
224222
"TableReplicaeuwest290D3CD3A",

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global.js.snapshot/manifest.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.global.ts

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class TestStack extends Stack {
3434
const app = new App({
3535
postCliContext: {
3636
'@aws-cdk/aws-lambda:createNewPoliciesWithAddToRolePolicy': false,
37+
'@aws-cdk/aws-dynamodb:retainTableReplica': false,
3738
},
3839
});
3940
const stack = new TestStack(app, 'cdk-dynamodb-global-20191121', { env: { region: 'eu-west-1' } });

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.table-skip-replica-deletion.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,11 @@ class TestStack extends Stack {
2424
}
2525
}
2626

27-
const app = new App();
27+
const app = new App({
28+
postCliContext: {
29+
'@aws-cdk/aws-dynamodb:retainTableReplica': true,
30+
},
31+
});
2832
const stack = new TestStack(app, 'cdk-dynamodb-skip-replica-deletion');
2933

3034
new IntegTest(

Diff for: ‎packages/@aws-cdk-testing/framework-integ/test/custom-resources/test/custom-resource-config/integ.custom-resource-config-lambda-node-runtime.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@ import * as dynamodb from 'aws-cdk-lib/aws-dynamodb';
44
import * as lambda from 'aws-cdk-lib/aws-lambda';
55
import { CustomResourceConfig } from 'aws-cdk-lib/custom-resources';
66

7-
const app = new cdk.App();
7+
const app = new cdk.App({
8+
postCliContext: {
9+
'@aws-cdk/aws-dynamodb:retainTableReplica': true,
10+
},
11+
});
812
const stack = new cdk.Stack(app, 'MyStack');
913

1014
new dynamodb.Table(stack, 'Table', {

Diff for: ‎packages/aws-cdk-lib/aws-dynamodb/lib/table.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ import {
2121
Aws, CfnCondition, CfnCustomResource, CfnResource, Duration,
2222
Fn, Lazy, Names, RemovalPolicy, Stack, Token, CustomResource,
2323
CfnDeletionPolicy,
24+
FeatureFlags,
2425
} from '../../core';
2526
import { UnscopedValidationError, ValidationError } from '../../core/lib/errors';
2627
import { addConstructMetadata, MethodMetadata } from '../../core/lib/metadata-resource';
28+
import { DYNAMODB_TABLE_RETAIN_TABLE_REPLICA } from '../../cx-api';
2729

2830
const HASH_KEY_TYPE = 'HASH';
2931
const RANGE_KEY_TYPE = 'RANGE';
@@ -1691,14 +1693,19 @@ export class Table extends TableBase {
16911693

16921694
// Replica table's removal policy will default to DynamoDB Table's removal policy
16931695
// unless replica removal policy is specified.
1694-
const skipReplicaDeletion = Lazy.any({
1696+
const retainReplica = FeatureFlags.of(this).isEnabled(DYNAMODB_TABLE_RETAIN_TABLE_REPLICA);
1697+
1698+
// If feature flag is disabled, never retain replica to maintain backward compatibility
1699+
const skipReplicaDeletion = retainReplica ? Lazy.any({
16951700
produce: () => {
1701+
// If feature flag is enabled, prioritize replica removal policy
16961702
if (replicaRemovalPolicy) {
16971703
return replicaRemovalPolicy == RemovalPolicy.RETAIN;
16981704
}
1705+
// Otherwise fall back to source table's removal policy
16991706
return (this.node.defaultChild as CfnResource).cfnOptions.deletionPolicy === CfnDeletionPolicy.RETAIN;
17001707
},
1701-
});
1708+
}) : false;
17021709

17031710
for (const region of new Set(regions)) { // Remove duplicates
17041711
// Use multiple custom resources because multiple create/delete
@@ -1709,7 +1716,7 @@ export class Table extends TableBase {
17091716
properties: {
17101717
TableName: this.tableName,
17111718
Region: region,
1712-
SkipReplicaDeletion: skipReplicaDeletion,
1719+
...skipReplicaDeletion && { SkipReplicaDeletion: skipReplicaDeletion },
17131720
SkipReplicationCompletedWait: waitForReplicationToFinish == null
17141721
? undefined
17151722
// CFN changes Custom Resource properties to strings anyways,

Diff for: ‎packages/aws-cdk-lib/aws-dynamodb/test/dynamodb.test.ts

+90-10
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,12 @@ testDeprecated('when specifying every property', () => {
446446
});
447447

448448
test('when replica removal policy is not specified', () => {
449-
const stack = new Stack();
449+
const app = new App({
450+
context: {
451+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
452+
},
453+
});
454+
const stack = new Stack(app);
450455
new Table(stack, CONSTRUCT_NAME, {
451456
tableName: TABLE_NAME,
452457
partitionKey: TABLE_PARTITION_KEY,
@@ -460,7 +465,12 @@ test('when replica removal policy is not specified', () => {
460465
});
461466

462467
test('when replica removal policy is not specified', () => {
463-
const stack = new Stack();
468+
const app = new App({
469+
context: {
470+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
471+
},
472+
});
473+
const stack = new Stack(app);
464474
const table = new Table(stack, CONSTRUCT_NAME, {
465475
tableName: TABLE_NAME,
466476
partitionKey: TABLE_PARTITION_KEY,
@@ -474,8 +484,13 @@ test('when replica removal policy is not specified', () => {
474484
});
475485

476486
test('when replica and table removal policy is not specified', () => {
477-
const stack = new Stack();
478-
const table = new Table(stack, CONSTRUCT_NAME, {
487+
const app = new App({
488+
context: {
489+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
490+
},
491+
});
492+
const stack = new Stack(app);
493+
new Table(stack, CONSTRUCT_NAME, {
479494
tableName: TABLE_NAME,
480495
partitionKey: TABLE_PARTITION_KEY,
481496
replicationRegions: ['eu-west-2', 'eu-west-3'],
@@ -486,9 +501,69 @@ test('when replica and table removal policy is not specified', () => {
486501
});
487502
});
488503

489-
test('when replica and table removal policy is not specified', () => {
490-
const stack = new Stack();
491-
const table = new Table(stack, CONSTRUCT_NAME, {
504+
test('when replica and table removal policy is not specified with feature flag true', () => {
505+
const app = new App({
506+
context: {
507+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
508+
},
509+
});
510+
const stack = new Stack(app);
511+
new Table(stack, CONSTRUCT_NAME, {
512+
tableName: TABLE_NAME,
513+
partitionKey: TABLE_PARTITION_KEY,
514+
replicationRegions: ['eu-west-2', 'eu-west-3'],
515+
});
516+
517+
Template.fromStack(stack).hasResourceProperties('Custom::DynamoDBReplica', {
518+
'SkipReplicaDeletion': true,
519+
});
520+
});
521+
522+
test('when table removal policy is specified with feature flag true', () => {
523+
const app = new App({
524+
context: {
525+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
526+
},
527+
});
528+
const stack = new Stack(app);
529+
new Table(stack, CONSTRUCT_NAME, {
530+
tableName: TABLE_NAME,
531+
partitionKey: TABLE_PARTITION_KEY,
532+
removalPolicy: RemovalPolicy.DESTROY,
533+
replicationRegions: ['eu-west-2', 'eu-west-3'],
534+
});
535+
536+
Template.fromStack(stack).hasResourceProperties('Custom::DynamoDBReplica', {
537+
'SkipReplicaDeletion': false,
538+
});
539+
});
540+
541+
test('when replica and table removal policy is not specified with feature flag false', () => {
542+
const app = new App({
543+
context: {
544+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: false,
545+
},
546+
});
547+
const stack = new Stack(app);
548+
new Table(stack, CONSTRUCT_NAME, {
549+
tableName: TABLE_NAME,
550+
partitionKey: TABLE_PARTITION_KEY,
551+
replicationRegions: ['eu-west-2', 'eu-west-3'],
552+
});
553+
554+
Template.fromStack(stack).hasResourceProperties('Custom::DynamoDBReplica', {
555+
'SkipReplicaDeletion': Match.absent(),
556+
});
557+
});
558+
559+
test('when replica is retain and table is destroy', () => {
560+
const app = new App({
561+
context: {
562+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
563+
},
564+
});
565+
const stack = new Stack(app);
566+
new Table(stack, CONSTRUCT_NAME, {
492567
tableName: TABLE_NAME,
493568
partitionKey: TABLE_PARTITION_KEY,
494569
removalPolicy: RemovalPolicy.DESTROY,
@@ -501,9 +576,14 @@ test('when replica and table removal policy is not specified', () => {
501576
});
502577
});
503578

504-
test('when replica and table removal policy is not specified', () => {
505-
const stack = new Stack();
506-
const table = new Table(stack, CONSTRUCT_NAME, {
579+
test('when replica is destory and table is retain', () => {
580+
const app = new App({
581+
context: {
582+
[cxapi.DYNAMODB_TABLE_RETAIN_TABLE_REPLICA]: true,
583+
},
584+
});
585+
const stack = new Stack(app);
586+
new Table(stack, CONSTRUCT_NAME, {
507587
tableName: TABLE_NAME,
508588
partitionKey: TABLE_PARTITION_KEY,
509589
removalPolicy: RemovalPolicy.RETAIN,

Diff for: ‎packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md

+17-1
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ Flags come in three types:
9393
| [@aws-cdk/aws-s3:setUniqueReplicationRoleName](#aws-cdkaws-s3setuniquereplicationrolename) | When enabled, CDK will automatically generate a unique role name that is used for s3 object replication. | 2.182.0 | (fix) |
9494
| [@aws-cdk/pipelines:reduceStageRoleTrustScope](#aws-cdkpipelinesreducestageroletrustscope) | Remove the root account principal from Stage addActions trust policy | 2.184.0 | (default) |
9595
| [@aws-cdk/aws-events:requireEventBusPolicySid](#aws-cdkaws-eventsrequireeventbuspolicysid) | When enabled, grantPutEventsTo() will use resource policies with Statement IDs for service principals. | 2.186.0 | (fix) |
96+
| [@aws-cdk/aws-dynamodb:retainTableReplica](#aws-cdkaws-dynamodbretaintablereplica) | When enabled, table replica will be default to the removal policy of source table unless specified otherwise. | V2NEXT | (fix) |
9697

9798
<!-- END table -->
9899

@@ -172,7 +173,8 @@ The following json shows the current recommended set of flags, as `cdk init` wou
172173
"@aws-cdk/core:enableAdditionalMetadataCollection": true,
173174
"@aws-cdk/aws-lambda:createNewPoliciesWithAddToRolePolicy": true,
174175
"@aws-cdk/aws-s3:setUniqueReplicationRoleName": true,
175-
"@aws-cdk/aws-events:requireEventBusPolicySid": true
176+
"@aws-cdk/aws-events:requireEventBusPolicySid": true,
177+
"@aws-cdk/aws-dynamodb:retainTableReplica": true
176178
}
177179
}
178180
```
@@ -1767,4 +1769,18 @@ This fixes the issue where permissions were silently not being added for service
17671769
| 2.186.0 | `false` | `true` |
17681770

17691771

1772+
### @aws-cdk/aws-dynamodb:retainTableReplica
1773+
1774+
*When enabled, table replica will be default to the removal policy of source table unless specified otherwise.* (fix)
1775+
1776+
Currently, table replica will always be deleted when stack deletes regardless of source table's deletion policy.
1777+
When enabled, table replica will be default to the removal policy of source table unless specified otherwise.
1778+
1779+
1780+
| Since | Default | Recommended |
1781+
| ----- | ----- | ----- |
1782+
| (not in v1) | | |
1783+
| V2NEXT | `false` | `true` |
1784+
1785+
17701786
<!-- END details -->

Diff for: ‎packages/aws-cdk-lib/cx-api/README.md

+15
Original file line numberDiff line numberDiff line change
@@ -679,3 +679,18 @@ _cdk.json_
679679
}
680680
}
681681
```
682+
683+
* `@aws-cdk/aws-dynamodb:retainTableReplica`
684+
685+
Currently, table replica will always be deleted when stack deletes regardless of source table's deletion policy.
686+
When enabled, table replica will be default to the removal policy of source table unless specified otherwise.
687+
688+
_cdk.json_
689+
690+
```json
691+
{
692+
"context": {
693+
"@aws-cdk/aws-dynamodb:retainTableReplica": true
694+
}
695+
}
696+
```

0 commit comments

Comments
 (0)