feat(cloudformation): Add removalPolicy on CustomResource (#2770)

Allows controlling the removal policy of custom resources, as these can occasionally be stateful (depending on the handler implementation).
aws · Jun 19, 2019 · 859248a · 859248a
1 parent 83eee09
commit 859248a
Show file tree

Hide file tree

Showing 19 changed files with 154 additions and 56 deletions.
diff --git a/packages/@aws-cdk/aws-cloudformation/lib/aws-custom-resource-provider/index.ts b/packages/@aws-cdk/aws-cloudformation/lib/aws-custom-resource-provider/index.ts
@@ -79,7 +79,7 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
     await respond('SUCCESS', 'OK', physicalResourceId, data);
   } catch (e) {
     console.log(e);
-    await respond('FAILED', e.message, context.logStreamName, {});
+    await respond('FAILED', e.message || 'Internal Error', context.logStreamName, {});
   }
 
   function respond(responseStatus: string, reason: string, physicalResourceId: string, data: any) {

diff --git a/packages/@aws-cdk/aws-cloudformation/lib/custom-resource.ts b/packages/@aws-cdk/aws-cloudformation/lib/custom-resource.ts
@@ -1,6 +1,6 @@
 import lambda = require('@aws-cdk/aws-lambda');
 import sns = require('@aws-cdk/aws-sns');
-import { CfnResource, Construct, Resource } from '@aws-cdk/cdk';
+import { CfnResource, Construct, RemovalPolicy, Resource } from '@aws-cdk/cdk';
 import { CfnCustomResource } from './cloudformation.generated';
 
 /**
@@ -21,9 +21,7 @@ export class CustomResourceProvider {
    */
   public static topic(topic: sns.ITopic) { return new CustomResourceProvider(topic.topicArn); }
 
-  private constructor(public readonly serviceToken: string) {
-
-  }
+  private constructor(public readonly serviceToken: string) {}
 }
 
 /**
@@ -67,6 +65,13 @@ export interface CustomResourceProps {
    * @default - AWS::CloudFormation::CustomResource
    */
   readonly resourceType?: string;
+
+  /**
+   * The policy to apply when this resource is removed from the application.
+   *
+   * @default cdk.RemovalPolicy.Destroy
+   */
+  readonly removalPolicy?: RemovalPolicy;
 }
 
 /**
@@ -91,6 +96,8 @@ export class CustomResource extends Resource {
         ...uppercaseProperties(props.properties || {})
       }
     });
+
+    this.resource.applyRemovalPolicy(props.removalPolicy, { default: RemovalPolicy.Destroy });
   }
 
   public getAtt(attributeName: string) {

diff --git a/packages/@aws-cdk/aws-cloudformation/package.json b/packages/@aws-cdk/aws-cloudformation/package.json
@@ -65,6 +65,7 @@
   "devDependencies": {
     "@aws-cdk/assert": "^0.34.0",
     "@aws-cdk/aws-events": "^0.34.0",
+    "@aws-cdk/aws-ssm": "^0.34.0",
     "@types/aws-lambda": "^8.10.26",
     "@types/nock": "^10.0.3",
     "@types/sinon": "^7.0.12",
@@ -104,4 +105,4 @@
     ]
   },
   "stability": "experimental"
-}
+}
diff --git a/packages/@aws-cdk/aws-cloudformation/test/integ.aws-custom-resource.expected.json b/packages/@aws-cdk/aws-cloudformation/test/integ.aws-custom-resource.expected.json
@@ -38,7 +38,8 @@
             "Ref": "TopicBFC7AF6E"
           }
         }
-      }
+      },
+      "DeletionPolicy": "Delete"
     },
     "AWS679f53fac002430cb0da5b7982bd2287ServiceRoleC1EA0FF2": {
       "Type": "AWS::IAM::Role",
@@ -186,6 +187,17 @@
           "action": "listTopics",
           "physicalResourceIdPath": "Topics.0.TopicArn"
         }
+      },
+      "DependsOn": [
+        "TopicBFC7AF6E"
+      ],
+      "DeletionPolicy": "Delete"
+    },
+    "DummyParameter53662B67": {
+      "Type": "AWS::SSM::Parameter",
+      "Properties": {
+        "Type": "String",
+        "Value": "1337"
       }
     },
     "GetParameter42B0A00E": {
@@ -201,7 +213,9 @@
           "service": "SSM",
           "action": "getParameter",
           "parameters": {
-            "Name": "my-parameter",
+            "Name": {
+              "Ref": "DummyParameter53662B67"
+            },
             "WithDecryption": true
           },
           "physicalResourceIdPath": "Parameter.ARN"
@@ -210,12 +224,15 @@
           "service": "SSM",
           "action": "getParameter",
           "parameters": {
-            "Name": "my-parameter",
+            "Name": {
+              "Ref": "DummyParameter53662B67"
+            },
             "WithDecryption": true
           },
           "physicalResourceIdPath": "Parameter.ARN"
         }
-      }
+      },
+      "DeletionPolicy": "Delete"
     }
   },
   "Parameters": {

diff --git a/packages/@aws-cdk/aws-cloudformation/test/integ.aws-custom-resource.ts b/packages/@aws-cdk/aws-cloudformation/test/integ.aws-custom-resource.ts
@@ -1,5 +1,6 @@
 #!/usr/bin/env node
 import sns = require('@aws-cdk/aws-sns');
+import ssm = require('@aws-cdk/aws-ssm');
 import cdk = require('@aws-cdk/cdk');
 import { AwsCustomResource } from '../lib';
 
@@ -28,13 +29,17 @@ const listTopics = new AwsCustomResource(stack, 'ListTopics', {
     physicalResourceIdPath: 'Topics.0.TopicArn'
   }
 });
+listTopics.node.addDependency(topic);
 
+const ssmParameter = new ssm.StringParameter(stack, 'DummyParameter', {
+  stringValue: '1337',
+});
 const getParameter = new AwsCustomResource(stack, 'GetParameter', {
   onUpdate: {
     service: 'SSM',
     action: 'getParameter',
     parameters: {
-      Name: 'my-parameter',
+      Name: ssmParameter.parameterName,
       WithDecryption: true
     },
     physicalResourceIdPath: 'Parameter.ARN'

diff --git a/packages/@aws-cdk/aws-cloudformation/test/integ.trivial-lambda-resource.expected.json b/packages/@aws-cdk/aws-cloudformation/test/integ.trivial-lambda-resource.expected.json
@@ -10,7 +10,8 @@
           ]
         },
         "Message": "CustomResource says hello"
-      }
+      },
+      "DeletionPolicy": "Delete"
     },
     "SingletonLambdaf7d4f7304ee111e89c2dfa7ae01bbebcServiceRoleFE9ABB04": {
       "Type": "AWS::IAM::Role",

diff --git a/packages/@aws-cdk/aws-cloudformation/test/test.resource.ts b/packages/@aws-cdk/aws-cloudformation/test/test.resource.ts
@@ -1,16 +1,65 @@
-import { expect, haveResource } from '@aws-cdk/assert';
+import { expect, haveResource, ResourcePart } from '@aws-cdk/assert';
 import lambda = require('@aws-cdk/aws-lambda');
 import sns = require('@aws-cdk/aws-sns');
 import cdk = require('@aws-cdk/cdk');
-import { Test } from 'nodeunit';
+import { Test, testCase } from 'nodeunit';
 import { CustomResource, CustomResourceProvider } from '../lib';
 
 // tslint:disable:object-literal-key-quotes
 
-export = {
+export = testCase({
+  'custom resources honor removalPolicy': {
+    'unspecified (aka .Destroy)'(test: Test) {
+      // GIVEN
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'Test');
+
+      // WHEN
+      new TestCustomResource(stack, 'Custom');
+
+      // THEN
+      expect(stack).to(haveResource('AWS::CloudFormation::CustomResource', {}, ResourcePart.CompleteDefinition));
+      test.equal(app.synth().tryGetArtifact(stack.stackName)!.findMetadataByType('aws:cdk:protected').length, 0);
+
+      test.done();
+    },
+
+    '.Destroy'(test: Test) {
+      // GIVEN
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'Test');
+
+      // WHEN
+      new TestCustomResource(stack, 'Custom', { removalPolicy: cdk.RemovalPolicy.Destroy });
+
+      // THEN
+      expect(stack).to(haveResource('AWS::CloudFormation::CustomResource', {}, ResourcePart.CompleteDefinition));
+      test.equal(app.synth().tryGetArtifact(stack.stackName)!.findMetadataByType('aws:cdk:protected').length, 0);
+
+      test.done();
+    },
+
+    '.Retain'(test: Test) {
+      // GIVEN
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'Test');
+
+      // WHEN
+      new TestCustomResource(stack, 'Custom', {  removalPolicy: cdk.RemovalPolicy.Retain });
+
+      // THEN
+      expect(stack).to(haveResource('AWS::CloudFormation::CustomResource', {
+        DeletionPolicy: 'Retain',
+      }, ResourcePart.CompleteDefinition));
+
+      test.done();
+    },
+  },
+
   'custom resource is added twice, lambda is added once'(test: Test) {
     // GIVEN
-    const stack = new cdk.Stack();
+    const app = new cdk.App();
+    const stack = new cdk.Stack(app, 'Test');
 
     // WHEN
     new TestCustomResource(stack, 'Custom1');
@@ -61,34 +110,37 @@ export = {
         ]
         },
         "Custom1D319B237": {
-        "Type": "AWS::CloudFormation::CustomResource",
-        "Properties": {
-          "ServiceToken": {
-          "Fn::GetAtt": [
-            "SingletonLambdaTestCustomResourceProviderA9255269",
-            "Arn"
-          ]
+          "Type": "AWS::CloudFormation::CustomResource",
+          "DeletionPolicy": "Delete",
+          "Properties": {
+            "ServiceToken": {
+              "Fn::GetAtt": [
+                "SingletonLambdaTestCustomResourceProviderA9255269",
+                "Arn"
+              ]
+            }
           }
-        }
         },
         "Custom2DD5FB44D": {
-        "Type": "AWS::CloudFormation::CustomResource",
-        "Properties": {
-          "ServiceToken": {
-          "Fn::GetAtt": [
-            "SingletonLambdaTestCustomResourceProviderA9255269",
-            "Arn"
-          ]
+          "Type": "AWS::CloudFormation::CustomResource",
+          "DeletionPolicy": "Delete",
+          "Properties": {
+            "ServiceToken": {
+              "Fn::GetAtt": [
+                "SingletonLambdaTestCustomResourceProviderA9255269",
+                "Arn"
+              ]
+            }
           }
         }
-        }
       }
     });
     test.done();
   },
 
   'custom resources can specify a resource type that starts with Custom::'(test: Test) {
-    const stack = new cdk.Stack();
+    const app = new cdk.App();
+    const stack = new cdk.Stack(app, 'Test');
     new CustomResource(stack, 'MyCustomResource', {
       resourceType: 'Custom::MyCustomResourceType',
       provider: CustomResourceProvider.topic(new sns.Topic(stack, 'Provider'))
@@ -99,7 +151,8 @@ export = {
 
   'fails if custom resource type is invalid': {
     'does not start with "Custom::"'(test: Test) {
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'Test');
 
       test.throws(() => {
         new CustomResource(stack, 'MyCustomResource', {
@@ -112,7 +165,8 @@ export = {
     },
 
     'has invalid characters'(test: Test) {
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'Test');
 
       test.throws(() => {
         new CustomResource(stack, 'MyCustomResource', {
@@ -125,7 +179,8 @@ export = {
     },
 
     'is longer than 60 characters'(test: Test) {
-      const stack = new cdk.Stack();
+      const app = new cdk.App();
+      const stack = new cdk.Stack(app, 'Test');
 
       test.throws(() => {
         new CustomResource(stack, 'MyCustomResource', {
@@ -138,10 +193,10 @@ export = {
     },
 
   },
-};
+});
 
 class TestCustomResource extends cdk.Construct {
-  constructor(scope: cdk.Construct, id: string) {
+  constructor(scope: cdk.Construct, id: string, opts: { removalPolicy?: cdk.RemovalPolicy } = {}) {
     super(scope, id);
 
     const singletonLambda = new lambda.SingletonFunction(this, 'Lambda', {
@@ -153,7 +208,8 @@ class TestCustomResource extends cdk.Construct {
     });
 
     new CustomResource(this, 'Resource', {
-      provider: CustomResourceProvider.lambda(singletonLambda)
+      ...opts,
+      provider: CustomResourceProvider.lambda(singletonLambda),
     });
   }
 }
diff --git a/packages/@aws-cdk/aws-codebuild/test/integ.docker-asset.lit.expected.json b/packages/@aws-cdk/aws-codebuild/test/integ.docker-asset.lit.expected.json
@@ -71,7 +71,8 @@
       "DependsOn": [
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleDefaultPolicy6BC8737C",
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17"
-      ]
+      ],
+      "DeletionPolicy": "Delete"
     },
     "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17": {
       "Type": "AWS::IAM::Role",

diff --git a/packages/@aws-cdk/aws-dynamodb-global/test/integ.dynamodb.global.expected.json b/packages/@aws-cdk/aws-dynamodb-global/test/integ.dynamodb.global.expected.json
@@ -232,7 +232,8 @@
           ],
           "ResourceType": "Custom::DynamoGlobalTableCoordinator",
           "TableName": "integrationtest"
-        }
+        },
+        "DeletionPolicy": "Delete"
       }
     },
     "Parameters": {

diff --git a/packages/@aws-cdk/aws-ecr-assets/test/integ.assets-docker.expected.json b/packages/@aws-cdk/aws-ecr-assets/test/integ.assets-docker.expected.json
@@ -44,7 +44,8 @@
       "DependsOn": [
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleDefaultPolicy6BC8737C",
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17"
-      ]
+      ],
+      "DeletionPolicy": "Delete"
     },
     "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17": {
       "Type": "AWS::IAM::Role",

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.asset-image.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.asset-image.expected.json
@@ -657,7 +657,8 @@
       "DependsOn": [
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleDefaultPolicy6BC8737C",
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17"
-      ]
+      ],
+      "DeletionPolicy": "Delete"
     },
     "FargateServiceTaskDefExecutionRole9194820E": {
       "Type": "AWS::IAM::Role",

diff --git a/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json b/packages/@aws-cdk/aws-events-targets/test/ecs/integ.event-ec2-task.lit.expected.json
@@ -813,7 +813,8 @@
       "DependsOn": [
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleDefaultPolicy6BC8737C",
         "AdoptEcrRepositorydbc60defc59544bcaa5c28c95d68f62cServiceRoleD788AA17"
-      ]
+      ],
+      "DeletionPolicy": "Delete"
     },
     "TaskDefExecutionRoleB4775C97": {
       "Type": "AWS::IAM::Role",