feat(rds): change the default retention policy of Cluster and DB Inst…

…ance to Snapshot (#8023) The 'Snapshot' retention policy is a special one used only for RDS. It deletes the underlying resource, but before doing that, creates a snapshot of it, so that the data is not lost. Use the 'Snapshot' policy instead of 'Retain', for the DatabaseCluster and DbInstance resources. Fixes #3298 BREAKING CHANGE: the default retention policy for RDS Cluster and DbInstance is now 'Snapshot'
aws · Jun 1, 2020 · 2d83328 · 2d83328
1 parent a8b1815
commit 2d83328
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 65 deletions.
diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts
@@ -3,7 +3,7 @@ import { IRole, ManagedPolicy, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
-import { Construct, Duration, RemovalPolicy, Resource, Token } from '@aws-cdk/core';
+import { CfnDeletionPolicy, Construct, Duration, RemovalPolicy, Resource, Token } from '@aws-cdk/core';
 import { DatabaseClusterAttributes, IDatabaseCluster } from './cluster-ref';
 import { DatabaseSecret } from './database-secret';
 import { Endpoint } from './endpoint';
@@ -124,9 +124,9 @@ export interface DatabaseClusterProps {
    * The removal policy to apply when the cluster and its instances are removed
    * from the stack or replaced during an update.
    *
-   * @default - Retain cluster.
+   * @default - RemovalPolicy.SNAPSHOT (remove the cluster and instances, but retain a snapshot of the data)
    */
-  readonly removalPolicy?: RemovalPolicy
+  readonly removalPolicy?: RemovalPolicy;
 
   /**
    * The interval, in seconds, between points when Amazon RDS collects enhanced
@@ -461,9 +461,16 @@ export class DatabaseCluster extends DatabaseClusterBase {
       storageEncrypted: props.kmsKey ? true : props.storageEncrypted,
     });
 
-    cluster.applyRemovalPolicy(props.removalPolicy, {
-      applyToUpdateReplacePolicy: true,
-    });
+    // if removalPolicy was not specified,
+    // leave it as the default, which is Snapshot
+    if (props.removalPolicy) {
+      cluster.applyRemovalPolicy(props.removalPolicy);
+    } else {
+      // The CFN default makes sense for DeletionPolicy,
+      // but doesn't cover UpdateReplacePolicy.
+      // Fix that here.
+      cluster.cfnOptions.updateReplacePolicy = CfnDeletionPolicy.SNAPSHOT;
+    }
 
     this.clusterIdentifier = cluster.ref;
 
@@ -519,9 +526,13 @@ export class DatabaseCluster extends DatabaseClusterBase {
         monitoringRoleArn: monitoringRole && monitoringRole.roleArn,
       });
 
-      instance.applyRemovalPolicy(props.removalPolicy, {
-        applyToUpdateReplacePolicy: true,
-      });
+      // If removalPolicy isn't explicitly set,
+      // it's Snapshot for Cluster.
+      // Because of that, in this case,
+      // we can safely use the CFN default of Delete for DbInstances with dbClusterIdentifier set.
+      if (props.removalPolicy) {
+        instance.applyRemovalPolicy(props.removalPolicy);
+      }
 
       // We must have a dependency on the NAT gateway provider here to create
       // things in the right order.

diff --git a/packages/@aws-cdk/aws-rds/lib/instance.ts b/packages/@aws-cdk/aws-rds/lib/instance.ts
@@ -5,7 +5,7 @@ import * as kms from '@aws-cdk/aws-kms';
 import * as lambda from '@aws-cdk/aws-lambda';
 import * as logs from '@aws-cdk/aws-logs';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
-import { Construct, Duration, IResource, Lazy, RemovalPolicy, Resource, SecretValue, Stack, Token } from '@aws-cdk/core';
+import { CfnDeletionPolicy, Construct, Duration, IResource, Lazy, RemovalPolicy, Resource, SecretValue, Stack, Token } from '@aws-cdk/core';
 import { DatabaseSecret } from './database-secret';
 import { Endpoint } from './endpoint';
 import { IOptionGroup } from './option-group';
@@ -536,9 +536,9 @@ export interface DatabaseInstanceNewProps {
    * The CloudFormation policy to apply when the instance is removed from the
    * stack or replaced during an update.
    *
-   * @default RemovalPolicy.Retain
+   * @default - RemovalPolicy.SNAPSHOT (remove the resource, but retain a snapshot of the data)
    */
-  readonly removalPolicy?: RemovalPolicy
+  readonly removalPolicy?: RemovalPolicy;
 
   /**
    * Upper limit to which RDS can scale the storage in GiB(Gibibyte).
@@ -886,9 +886,7 @@ export class DatabaseInstance extends DatabaseInstanceSource implements IDatabas
     const portAttribute = Token.asNumber(instance.attrEndpointPort);
     this.instanceEndpoint = new Endpoint(instance.attrEndpointAddress, portAttribute);
 
-    instance.applyRemovalPolicy(props.removalPolicy, {
-      applyToUpdateReplacePolicy: true,
-    });
+    applyInstanceDeletionPolicy(instance, props.removalPolicy);
 
     if (secret) {
       this.secret = secret.attach(this);
@@ -984,9 +982,7 @@ export class DatabaseInstanceFromSnapshot extends DatabaseInstanceSource impleme
     const portAttribute = Token.asNumber(instance.attrEndpointPort);
     this.instanceEndpoint = new Endpoint(instance.attrEndpointAddress, portAttribute);
 
-    instance.applyRemovalPolicy(props.removalPolicy, {
-      applyToUpdateReplacePolicy: true,
-    });
+    applyInstanceDeletionPolicy(instance, props.removalPolicy);
 
     if (secret) {
       this.secret = secret.attach(this);
@@ -1054,9 +1050,7 @@ export class DatabaseInstanceReadReplica extends DatabaseInstanceNew implements
     const portAttribute = Token.asNumber(instance.attrEndpointPort);
     this.instanceEndpoint = new Endpoint(instance.attrEndpointAddress, portAttribute);
 
-    instance.applyRemovalPolicy(props.removalPolicy, {
-      applyToUpdateReplacePolicy: true,
-    });
+    applyInstanceDeletionPolicy(instance, props.removalPolicy);
 
     this.setLogRetention();
   }
@@ -1072,3 +1066,14 @@ function renderProcessorFeatures(features: ProcessorFeatures): CfnDBInstance.Pro
 
   return featuresList.length === 0 ? undefined : featuresList;
 }
+
+function applyInstanceDeletionPolicy(cfnDbInstance: CfnDBInstance, removalPolicy: RemovalPolicy | undefined): void {
+  if (!removalPolicy) {
+    // the default DeletionPolicy is 'Snapshot', which is fine,
+    // but we should also make it 'Snapshot' for UpdateReplace policy
+    cfnDbInstance.cfnOptions.updateReplacePolicy = CfnDeletionPolicy.SNAPSHOT;
+  } else {
+    // just apply whatever removal policy the customer explicitly provided
+    cfnDbInstance.applyRemovalPolicy(removalPolicy);
+  }
+}
diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster-rotation.lit.expected.json b/packages/@aws-cdk/aws-rds/test/integ.cluster-rotation.lit.expected.json
@@ -706,8 +706,7 @@
           }
         ]
       },
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      "UpdateReplacePolicy": "Snapshot"
     },
     "DatabaseInstance1844F58FD": {
       "Type": "AWS::RDS::DBInstance",
@@ -725,9 +724,7 @@
         "VPCPrivateSubnet1DefaultRouteAE1D6490",
         "VPCPrivateSubnet2DefaultRouteF4F5CFD2",
         "VPCPrivateSubnet3DefaultRoute27F311AE"
-      ],
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      ]
     },
     "DatabaseInstance2AA380DEE": {
       "Type": "AWS::RDS::DBInstance",
@@ -745,9 +742,7 @@
         "VPCPrivateSubnet1DefaultRouteAE1D6490",
         "VPCPrivateSubnet2DefaultRouteF4F5CFD2",
         "VPCPrivateSubnet3DefaultRoute27F311AE"
-      ],
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      ]
     },
     "DatabaseRotationSingleUserSecurityGroupAC6E0E73": {
       "Type": "AWS::EC2::SecurityGroup",
@@ -817,4 +812,4 @@
       }
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster-s3.expected.json b/packages/@aws-cdk/aws-rds/test/integ.cluster-s3.expected.json
@@ -668,8 +668,7 @@
           }
         ]
       },
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      "UpdateReplacePolicy": "Snapshot"
     },
     "DatabaseInstance1844F58FD": {
       "Type": "AWS::RDS::DBInstance",
@@ -687,9 +686,7 @@
       "DependsOn": [
         "VPCPublicSubnet1DefaultRoute91CEF279",
         "VPCPublicSubnet2DefaultRouteB7481BBA"
-      ],
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      ]
     },
     "DatabaseInstance2AA380DEE": {
       "Type": "AWS::RDS::DBInstance",
@@ -707,9 +704,7 @@
       "DependsOn": [
         "VPCPublicSubnet1DefaultRoute91CEF279",
         "VPCPublicSubnet2DefaultRouteB7481BBA"
-      ],
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      ]
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json b/packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json
@@ -500,8 +500,7 @@
           }
         ]
       },
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      "UpdateReplacePolicy": "Snapshot"
     },
     "DatabaseInstance1844F58FD": {
       "Type": "AWS::RDS::DBInstance",
@@ -519,9 +518,7 @@
       "DependsOn": [
         "VPCPublicSubnet1DefaultRoute91CEF279",
         "VPCPublicSubnet2DefaultRouteB7481BBA"
-      ],
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      ]
     },
     "DatabaseInstance2AA380DEE": {
       "Type": "AWS::RDS::DBInstance",
@@ -539,9 +536,7 @@
       "DependsOn": [
         "VPCPublicSubnet1DefaultRoute91CEF279",
         "VPCPublicSubnet2DefaultRouteB7481BBA"
-      ],
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      ]
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json b/packages/@aws-cdk/aws-rds/test/integ.instance.lit.expected.json
@@ -694,8 +694,7 @@
           }
         ]
       },
-      "UpdateReplacePolicy": "Retain",
-      "DeletionPolicy": "Retain"
+      "UpdateReplacePolicy": "Snapshot"
     },
     "InstanceLogRetentiontrace487771C8": {
       "Type": "Custom::LogRetention",
@@ -1122,4 +1121,4 @@
       "Description": "Artifact hash for asset \"82c54bfa7c42ba410d6d18dad983ba51c93a5ea940818c5c20230f8b59c19d4e\""
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-rds/test/test.cluster.ts b/packages/@aws-cdk/aws-rds/test/test.cluster.ts
@@ -1,4 +1,4 @@
-import { expect, haveResource, ResourcePart, SynthUtils } from '@aws-cdk/assert';
+import { ABSENT, countResources, expect, haveResource, ResourcePart, SynthUtils } from '@aws-cdk/assert';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import { ManagedPolicy, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
@@ -8,7 +8,7 @@ import { Test } from 'nodeunit';
 import { ClusterParameterGroup, DatabaseCluster, DatabaseClusterEngine, ParameterGroup } from '../lib';
 
 export = {
-  'check that instantiation works'(test: Test) {
+  'creating a Cluster also creates 2 DB Instances'(test: Test) {
     // GIVEN
     const stack = testStack();
     const vpc = new ec2.Vpc(stack, 'VPC');
@@ -35,17 +35,19 @@ export = {
         MasterUserPassword: 'tooshort',
         VpcSecurityGroupIds: [ {'Fn::GetAtt': ['DatabaseSecurityGroup5C91FDCB', 'GroupId']}],
       },
-      DeletionPolicy: 'Retain',
-      UpdateReplacePolicy: 'Retain',
+      DeletionPolicy: ABSENT,
+      UpdateReplacePolicy: 'Snapshot',
     }, ResourcePart.CompleteDefinition));
 
+    expect(stack).to(countResources('AWS::RDS::DBInstance', 2));
     expect(stack).to(haveResource('AWS::RDS::DBInstance', {
-      DeletionPolicy: 'Retain',
-      UpdateReplacePolicy: 'Retain',
+      DeletionPolicy: ABSENT,
+      UpdateReplacePolicy: ABSENT,
     }, ResourcePart.CompleteDefinition));
 
     test.done();
   },
+
   'can create a cluster with a single instance'(test: Test) {
     // GIVEN
     const stack = testStack();

diff --git a/packages/@aws-cdk/aws-rds/test/test.instance.ts b/packages/@aws-cdk/aws-rds/test/test.instance.ts
@@ -1,4 +1,4 @@
-import { countResources, expect, haveResource, ResourcePart } from '@aws-cdk/assert';
+import { ABSENT, countResources, expect, haveResource, ResourcePart } from '@aws-cdk/assert';
 import * as ec2 from '@aws-cdk/aws-ec2';
 import * as targets from '@aws-cdk/aws-events-targets';
 import { ManagedPolicy, Role, ServicePrincipal } from '@aws-cdk/aws-iam';
@@ -105,13 +105,8 @@ export = {
           },
         ],
       },
-      DeletionPolicy: 'Retain',
-      UpdateReplacePolicy: 'Retain',
-    }, ResourcePart.CompleteDefinition));
-
-    expect(stack).to(haveResource('AWS::RDS::DBInstance', {
-      DeletionPolicy: 'Retain',
-      UpdateReplacePolicy: 'Retain',
+      DeletionPolicy: ABSENT,
+      UpdateReplacePolicy: 'Snapshot',
     }, ResourcePart.CompleteDefinition));
 
     expect(stack).to(haveResource('AWS::RDS::DBSubnetGroup', {

diff --git a/packages/@aws-cdk/core/lib/cfn-resource.ts b/packages/@aws-cdk/core/lib/cfn-resource.ts
@@ -116,6 +116,10 @@ export class CfnResource extends CfnRefElement {
         deletionPolicy = CfnDeletionPolicy.RETAIN;
         break;
 
+      case RemovalPolicy.SNAPSHOT:
+        deletionPolicy = CfnDeletionPolicy.SNAPSHOT;
+        break;
+
       default:
         throw new Error(`Invalid removal policy: ${policy}`);
     }

diff --git a/packages/@aws-cdk/core/lib/removal-policy.ts b/packages/@aws-cdk/core/lib/removal-policy.ts
@@ -10,6 +10,17 @@ export enum RemovalPolicy {
    * in the account, but orphaned from the stack.
    */
   RETAIN = 'retain',
+
+  /**
+   * This retention policy deletes the resource,
+   * but saves a snapshot of its data before deleting,
+   * so that it can be re-created later.
+   * Only available for some stateful resources,
+   * like databases, EFS volumes, etc.
+   *
+   * @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html#aws-attribute-deletionpolicy-options
+   */
+  SNAPSHOT = 'snapshot',
 }
 
 export interface RemovalPolicyOptions {