Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(core): RemovalPolicy.SNAPSHOT can be added to resources that do not support it #20668

Merged
merged 6 commits into from
Jun 13, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
30 changes: 29 additions & 1 deletion packages/@aws-cdk/core/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as cxapi from '@aws-cdk/cx-api';
import { Annotations } from './annotations';
import { CfnCondition } from './cfn-condition';
// import required to be here, otherwise causes a cycle when running the generated JavaScript
/* eslint-disable import/order */
Expand All @@ -13,6 +14,7 @@ import { RemovalPolicy, RemovalPolicyOptions } from './removal-policy';
import { TagManager } from './tag-manager';
import { Tokenization } from './token';
import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from './util';
import { FeatureFlags } from './feature-flags';

export interface CfnResourceProps {
/**
Expand Down Expand Up @@ -108,7 +110,12 @@ export class CfnResource extends CfnRefElement {
* to be replaced.
*
* The resource can be deleted (`RemovalPolicy.DESTROY`), or left in your AWS
* account for data recovery and cleanup later (`RemovalPolicy.RETAIN`).
* account for data recovery and cleanup later (`RemovalPolicy.RETAIN`). In some
* cases, a snapshot can be taken of the resource prior to deletion
* (`RemovalPolicy.SNAPSHOT`). A list of resources that support this policy
* can be found in the following link:
*
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html#aws-attribute-deletionpolicy-options
*/
public applyRemovalPolicy(policy: RemovalPolicy | undefined, options: RemovalPolicyOptions = {}) {
policy = policy || options.default || RemovalPolicy.RETAIN;
Expand All @@ -125,6 +132,27 @@ export class CfnResource extends CfnRefElement {
break;

case RemovalPolicy.SNAPSHOT:
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html
const snapshottableResourceTypes = [
'AWS::EC2::Volume',
'AWS::ElastiCache::CacheCluster',
'AWS::ElastiCache::ReplicationGroup',
'AWS::Neptune::DBCluster',
'AWS::RDS::DBCluster',
'AWS::RDS::DBInstance',
'AWS::Redshift::Cluster',
];

// error if flag is set, warn if flag is not
const problematicSnapshotPolicy = !snapshottableResourceTypes.includes(this.cfnResourceType);
if (problematicSnapshotPolicy) {
if (FeatureFlags.of(this).isEnabled(cxapi.VALIDATE_SNAPSHOT_REMOVAL_POLICY) ) {
throw new Error(`${this.cfnResourceType} does not support snapshot removal policy`);
} else {
Annotations.of(this).addWarning(`${this.cfnResourceType} does not support snapshot removal policy. This policy will be ignored.`);
}
}

deletionPolicy = CfnDeletionPolicy.SNAPSHOT;
break;

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/core/lib/removal-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export enum RemovalPolicy {
* 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.
* like databases, EC2 volumes, etc.
*
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-attribute-deletionpolicy.html#aws-attribute-deletionpolicy-options
*/
Expand Down
16 changes: 1 addition & 15 deletions packages/@aws-cdk/core/test/annotations.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { CloudAssembly } from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import { App, Stack } from '../lib';
import { Annotations } from '../lib/annotations';
import { getWarnings } from './util';

const restore = process.env.CDK_BLOCK_DEPRECATIONS;

Expand Down Expand Up @@ -75,17 +75,3 @@ describe('annotations', () => {
expect(() => Annotations.of(c1).addDeprecation('foo', 'bar')).toThrow(/MyStack\/Hello: The API foo is deprecated: bar\. This API will be removed in the next major release/);
});
});

function getWarnings(casm: CloudAssembly) {
const result = new Array<{ path: string, message: string }>();
for (const stack of Object.values(casm.manifest.artifacts ?? {})) {
for (const [path, md] of Object.entries(stack.metadata ?? {})) {
for (const x of md) {
if (x.type === 'aws:cdk:warning') {
result.push({ path, message: x.data as string });
}
}
}
}
return result;
}
92 changes: 92 additions & 0 deletions packages/@aws-cdk/core/test/cfn-resource.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { VALIDATE_SNAPSHOT_REMOVAL_POLICY } from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import * as core from '../lib';
import { getWarnings } from './util';

describe('cfn resource', () => {
describe('._toCloudFormation', () => {
Expand Down Expand Up @@ -43,6 +45,96 @@ describe('cfn resource', () => {
});
});

describe('snapshot removal policy', () => {
const supportedResources = [
'AWS::EC2::Volume',
'AWS::ElastiCache::CacheCluster',
'AWS::ElastiCache::ReplicationGroup',
'AWS::Neptune::DBCluster',
'AWS::RDS::DBCluster',
'AWS::RDS::DBInstance',
'AWS::Redshift::Cluster',
];

test.each(supportedResources) (
'works as expected when used on supported resources (old behavior)', (resourceType) => {
// GIVEN
const app = new core.App();
const stack = new core.Stack(app, 'TestStack');
const resource = new core.CfnResource(stack, 'Resource', {
type: resourceType,
});

// WHEN
resource.applyRemovalPolicy(core.RemovalPolicy.SNAPSHOT);

// THEN
expect(app.synth().getStackByName(stack.stackName).template?.Resources).toEqual({
Resource: {
Type: resourceType,
DeletionPolicy: 'Snapshot',
UpdateReplacePolicy: 'Snapshot',
},
});
},
);

test.each(supportedResources) (
'works as expected when used on supported resources (under feature flag)', (resourceType) => {
// GIVEN
const app = new core.App({ context: { [VALIDATE_SNAPSHOT_REMOVAL_POLICY]: true } });
const stack = new core.Stack(app, 'TestStack');
const resource = new core.CfnResource(stack, 'Resource', {
type: resourceType,
});

// WHEN
resource.applyRemovalPolicy(core.RemovalPolicy.SNAPSHOT);

// THEN
expect(app.synth().getStackByName(stack.stackName).template?.Resources).toEqual({
Resource: {
Type: resourceType,
DeletionPolicy: 'Snapshot',
UpdateReplacePolicy: 'Snapshot',
},
});
},
);

test('warns on unsupported resources (without feature flag)', () => {
// GIVEN
const app = new core.App();
const stack = new core.Stack(app);
const resource = new core.CfnResource(stack, 'Resource', {
type: 'AWS::Lambda::Function',
});

// WHEN
resource.applyRemovalPolicy(core.RemovalPolicy.SNAPSHOT);

// THEN
expect(getWarnings(app.synth())).toEqual([
{
path: '/Default/Resource',
message: 'AWS::Lambda::Function does not support snapshot removal policy. This policy will be ignored.',
},
]);
});

test('fails on unsupported resources (under feature flag)', () => {
// GIVEN
const app = new core.App({ context: { [VALIDATE_SNAPSHOT_REMOVAL_POLICY]: true } });
const stack = new core.Stack(app);
const resource = new core.CfnResource(stack, 'Resource', {
type: 'AWS::Lambda::Function',
});

// THEN
expect(() => resource.applyRemovalPolicy(core.RemovalPolicy.SNAPSHOT)).toThrowError('AWS::Lambda::Function does not support snapshot removal policy');
});
});

test('applyRemovalPolicy default includes Update policy', () => {
// GIVEN
const app = new core.App();
Expand Down
15 changes: 15 additions & 0 deletions packages/@aws-cdk/core/test/util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { CloudAssembly } from '@aws-cdk/cx-api';
import { Stack } from '../lib';
import { CDK_DEBUG } from '../lib/debug';
import { synthesize } from '../lib/private/synthesis';
Expand Down Expand Up @@ -30,3 +31,17 @@ export function restoreStackTraceColection(previousValue: string | undefined): v
process.env.CDK_DISABLE_STACK_TRACE = previousValue;
delete process.env[CDK_DEBUG];
}

export function getWarnings(casm: CloudAssembly) {
const result = new Array<{ path: string, message: string }>();
for (const stack of Object.values(casm.manifest.artifacts ?? {})) {
for (const [path, md] of Object.entries(stack.metadata ?? {})) {
for (const x of md) {
if (x.type === 'aws:cdk:warning') {
result.push({ path, message: x.data as string });
}
}
}
}
return result;
}
11 changes: 11 additions & 0 deletions packages/@aws-cdk/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,16 @@ export const EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME = '@aws-cdk/aws-ec2:uniqueIm
*/
export const IAM_MINIMIZE_POLICIES = '@aws-cdk/aws-iam:minimizePolicies';

/**
* Makes sure we do not allow snapshot removal policy on resources that do not support it.
* If supplied on an unsupported resource, CloudFormation ignores the policy altogether.
* This flag will reduce confusion and unexpected loss of data when erroneously supplying
* the snapshot removal policy.
*
* [PERMANENT]
*/
export const VALIDATE_SNAPSHOT_REMOVAL_POLICY = '@aws-cdk/core:validateSnapshotRemovalPolicy';

/**
* Flag values that should apply for new projects
*
Expand Down Expand Up @@ -273,6 +283,7 @@ export const FUTURE_FLAGS: { [key: string]: boolean } = {
[EC2_UNIQUE_IMDSV2_LAUNCH_TEMPLATE_NAME]: true,
[CHECK_SECRET_USAGE]: true,
[IAM_MINIMIZE_POLICIES]: true,
[VALIDATE_SNAPSHOT_REMOVAL_POLICY]: true,
};

/**
Expand Down