Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(rds): change the default retention policy of Cluster and DB Instance #8023

Merged
merged 1 commit into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions packages/@aws-cdk/aws-rds/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down
29 changes: 17 additions & 12 deletions packages/@aws-cdk/aws-rds/lib/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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).
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -706,8 +706,7 @@
}
]
},
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
"UpdateReplacePolicy": "Snapshot"
},
"DatabaseInstance1844F58FD": {
"Type": "AWS::RDS::DBInstance",
Expand All @@ -725,9 +724,7 @@
"VPCPrivateSubnet1DefaultRouteAE1D6490",
"VPCPrivateSubnet2DefaultRouteF4F5CFD2",
"VPCPrivateSubnet3DefaultRoute27F311AE"
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
]
},
"DatabaseInstance2AA380DEE": {
"Type": "AWS::RDS::DBInstance",
Expand All @@ -745,9 +742,7 @@
"VPCPrivateSubnet1DefaultRouteAE1D6490",
"VPCPrivateSubnet2DefaultRouteF4F5CFD2",
"VPCPrivateSubnet3DefaultRoute27F311AE"
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
]
},
"DatabaseRotationSingleUserSecurityGroupAC6E0E73": {
"Type": "AWS::EC2::SecurityGroup",
Expand Down Expand Up @@ -817,4 +812,4 @@
}
}
}
}
}
13 changes: 4 additions & 9 deletions packages/@aws-cdk/aws-rds/test/integ.cluster-s3.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -668,8 +668,7 @@
}
]
},
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
"UpdateReplacePolicy": "Snapshot"
},
"DatabaseInstance1844F58FD": {
"Type": "AWS::RDS::DBInstance",
Expand All @@ -687,9 +686,7 @@
"DependsOn": [
"VPCPublicSubnet1DefaultRoute91CEF279",
"VPCPublicSubnet2DefaultRouteB7481BBA"
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
]
},
"DatabaseInstance2AA380DEE": {
"Type": "AWS::RDS::DBInstance",
Expand All @@ -707,9 +704,7 @@
"DependsOn": [
"VPCPublicSubnet1DefaultRoute91CEF279",
"VPCPublicSubnet2DefaultRouteB7481BBA"
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
]
}
}
}
}
13 changes: 4 additions & 9 deletions packages/@aws-cdk/aws-rds/test/integ.cluster.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,7 @@
}
]
},
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
"UpdateReplacePolicy": "Snapshot"
},
"DatabaseInstance1844F58FD": {
"Type": "AWS::RDS::DBInstance",
Expand All @@ -519,9 +518,7 @@
"DependsOn": [
"VPCPublicSubnet1DefaultRoute91CEF279",
"VPCPublicSubnet2DefaultRouteB7481BBA"
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
]
},
"DatabaseInstance2AA380DEE": {
"Type": "AWS::RDS::DBInstance",
Expand All @@ -539,9 +536,7 @@
"DependsOn": [
"VPCPublicSubnet1DefaultRoute91CEF279",
"VPCPublicSubnet2DefaultRouteB7481BBA"
],
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
]
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -694,8 +694,7 @@
}
]
},
"UpdateReplacePolicy": "Retain",
"DeletionPolicy": "Retain"
"UpdateReplacePolicy": "Snapshot"
},
"InstanceLogRetentiontrace487771C8": {
"Type": "Custom::LogRetention",
Expand Down Expand Up @@ -1122,4 +1121,4 @@
"Description": "Artifact hash for asset \"82c54bfa7c42ba410d6d18dad983ba51c93a5ea940818c5c20230f8b59c19d4e\""
}
}
}
}
14 changes: 8 additions & 6 deletions packages/@aws-cdk/aws-rds/test/test.cluster.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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');
Expand All @@ -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();
Expand Down
11 changes: 3 additions & 8 deletions packages/@aws-cdk/aws-rds/test/test.instance.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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', {
Expand Down
4 changes: 4 additions & 0 deletions packages/@aws-cdk/core/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
11 changes: 11 additions & 0 deletions packages/@aws-cdk/core/lib/removal-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
*
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html#aws-attribute-deletionpolicy-options
*/
SNAPSHOT = 'snapshot',
}

export interface RemovalPolicyOptions {
Expand Down