feat(custom-resources): ignore DELETE after failed CREATE (aws#5525)

* feat(custom-resources): ignore DELETE after failed CREATE When a CREATE operation fails, CloudFormation will automatically issue a DELETE operation with the `PhysicalResourceId` submitted by the FAILED response. The provider framework currently does not support customizing the PhysicalResourceId of a failed response (as described in aws#5524), and therefore it makes more sense to have the framework simply ignore this DELETE operation. Otherwise, the user handler will need to special case this somehow, without proper signal. The solution is to use a special marker for the physical resource ID when a CREATE fails, and recognize this marker in the subsequent DELETE. * chore(build): resolve eslint plugins relative to cdk-build-tools plugins are installed centrally under cdk-build-tools and therefore resolution should happen against that module instead of the current module. otherwise, we get an error `ESLint couldn't find the plugin "eslint-plugin-node".` * moved MISSING_MARKER to the last minute * update expectations
parroty · Dec 23, 2019 · 9ab989e · 9ab989e
1 parent 394313e
commit 9ab989e
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 29 deletions.
diff --git a/packages/@aws-cdk/custom-resources/README.md b/packages/@aws-cdk/custom-resources/README.md
@@ -59,7 +59,7 @@ At the minimum, users must define the `onEvent` handler, which is invoked by the
 framework for all resource lifecycle events (`Create`, `Update` and `Delete`)
 and returns a result which is then submitted to CloudFormation.
 
-The following example is a skelaton for a Python implementation of `onEvent`:
+The following example is a skeleton for a Python implementation of `onEvent`:
 
 ```py
 def on_event(event, context):
@@ -96,7 +96,7 @@ where the lifecycle operation cannot be completed immediately. The
 `isComplete` handler will be retried asynchronously after `onEvent` until it
 returns `IsComplete: true`, or until the total provider timeout has expired.
 
-The following example is a skelaton for a Python implementation of `isComplete`:
+The following example is a skeleton for a Python implementation of `isComplete`:
 
 ```py
 def is_complete(event, context):
@@ -219,11 +219,10 @@ When AWS CloudFormation receives a "FAILED" response, it will attempt to roll
 back the stack to it's last state. This has different meanings for different
 lifecycle events:
 
-- If a `Create` event fails, CloudFormation will issue a `Delete` event to allow
-  the provider to clean up any unfinished work related to the creation of the
-  resource. The implication of this is that it is recommended to implement
-  `Delete` in an idempotent way, in order to make sure that the rollback
-  `Delete` operation won't fail if a resource creation has failed.
+- If a `Create` event fails, the resource provider framework will automatically
+  ignore the subsequent `Delete` operation issued by AWS CloudFormation. The
+  framework currently does not support customizing this behavior (see
+  https://github.com/aws/aws-cdk/issues/5524).
 - If an `Update` event fails, CloudFormation will issue an additional `Update`
   with the previous properties.
 - If a `Delete` event fails, CloudFormation will abandon this resource.

diff --git a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/cfn-response.ts b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/cfn-response.ts
@@ -4,6 +4,9 @@ import * as url from 'url';
 import { httpRequest } from './outbound';
 import { log } from './util';
 
+export const CREATE_FAILED_PHYSICAL_ID_MARKER = 'AWSCDK::CustomResourceProviderFramework::CREATE_FAILED';
+export const MISSING_PHYSICAL_ID_MARKER = 'AWSCDK::CustomResourceProviderFramework::MISSING_PHYSICAL_ID';
+
 export interface CloudFormationResponseOptions {
   readonly reason?: string;
   readonly noEcho?: boolean;
@@ -24,7 +27,7 @@ export async function submitResponse(status: 'SUCCESS' | 'FAILED', event: CloudF
     Reason: options.reason || status,
     StackId: event.StackId,
     RequestId: event.RequestId,
-    PhysicalResourceId: event.PhysicalResourceId || event.RequestId,
+    PhysicalResourceId: event.PhysicalResourceId || MISSING_PHYSICAL_ID_MARKER,
     LogicalResourceId: event.LogicalResourceId,
     NoEcho: options.noEcho,
     Data: event.Data
@@ -50,11 +53,40 @@ export let includeStackTraces = true; // for unit tests
 
 export function safeHandler(block: (event: any) => Promise<void>) {
   return async (event: any) => {
+
+    // ignore DELETE event when the physical resource ID is the marker that
+    // indicates that this DELETE is a subsequent DELETE to a failed CREATE
+    // operation.
+    if (event.RequestType === 'Delete' && event.PhysicalResourceId === CREATE_FAILED_PHYSICAL_ID_MARKER) {
+      log(`ignoring DELETE event caused by a failed CREATE event`);
+      await submitResponse('SUCCESS', event);
+      return;
+    }
+
     try {
       await block(event);
     } catch (e) {
       // tell waiter state machine to retry
-      if (e instanceof Retry) { throw e; }
+      if (e instanceof Retry) {
+        log(`retry requested by handler`);
+        throw e;
+      }
+
+      if (!event.PhysicalResourceId) {
+        // special case: if CREATE fails, which usually implies, we usually don't
+        // have a physical resource id. in this case, the subsequent DELETE
+        // operation does not have any meaning, and will likely fail as well. to
+        // address this, we use a marker so the provider framework can simply
+        // ignore the subsequent DELETE.
+        if (event.RequestType === 'Create') {
+          log(`CREATE failed, responding with a marker physical resource id so that the subsequent DELETE will be ignored`);
+          event.PhysicalResourceId = CREATE_FAILED_PHYSICAL_ID_MARKER;
+        } else {
+          // otherwise, if PhysicalResourceId is not specified, something is
+          // terribly wrong because all other events should have an ID.
+          log(`ERROR: Malformed event. "PhysicalResourceId" is required: ${JSON.stringify(event)}`);
+        }
+      }
 
       // this is an actual error, fail the activity altogether and exist.
       await submitResponse('FAILED', event, {

diff --git a/packages/@aws-cdk/custom-resources/test/provider-framework/integ.provider.expected.json b/packages/@aws-cdk/custom-resources/test/provider-framework/integ.provider.expected.json
@@ -200,7 +200,7 @@
       "Properties": {
         "Code": {
           "S3Bucket": {
-            "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3BucketBF5EAA30"
+            "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3BucketE8BB46CE"
           },
           "S3Key": {
             "Fn::Join": [
@@ -213,7 +213,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -226,7 +226,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -579,7 +579,7 @@
       "Properties": {
         "Code": {
           "S3Bucket": {
-            "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3BucketBF5EAA30"
+            "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3BucketE8BB46CE"
           },
           "S3Key": {
             "Fn::Join": [
@@ -592,7 +592,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -605,7 +605,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -721,7 +721,7 @@
       "Properties": {
         "Code": {
           "S3Bucket": {
-            "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3BucketBF5EAA30"
+            "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3BucketE8BB46CE"
           },
           "S3Key": {
             "Fn::Join": [
@@ -734,7 +734,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -747,7 +747,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -860,7 +860,7 @@
       "Properties": {
         "Code": {
           "S3Bucket": {
-            "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3BucketBF5EAA30"
+            "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3BucketE8BB46CE"
           },
           "S3Key": {
             "Fn::Join": [
@@ -873,7 +873,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -886,7 +886,7 @@
                       "Fn::Split": [
                         "||",
                         {
-                          "Ref": "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101"
+                          "Ref": "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8"
                         }
                       ]
                     }
@@ -1038,17 +1038,17 @@
       "Type": "String",
       "Description": "Artifact hash for asset \"865603dd9d562e52d496bad5ef42cafdeb7c05931986d8ad11deb93dc0e436e6\""
     },
-    "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3BucketBF5EAA30": {
+    "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3BucketE8BB46CE": {
       "Type": "String",
-      "Description": "S3 bucket for asset \"75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63f\""
+      "Description": "S3 bucket for asset \"3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0\""
     },
-    "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fS3VersionKeyFEA51101": {
+    "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0S3VersionKeyDFF4B5D8": {
       "Type": "String",
-      "Description": "S3 key for asset version \"75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63f\""
+      "Description": "S3 key for asset version \"3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0\""
     },
-    "AssetParameters75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63fArtifactHash921C6847": {
+    "AssetParameters3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0ArtifactHashDD0FF81A": {
       "Type": "String",
-      "Description": "Artifact hash for asset \"75d866254bf5ba7ec72a947ae03a1304d0b5dbe42d254b461b842ddf64abe63f\""
+      "Description": "Artifact hash for asset \"3e728f777afd6d4a580bc77d99f86194358dca730432b3f4583e544f1e85d2a0\""
     },
     "AssetParametersdb961fc9d087616ad76339bd5135f518cea24001f866a17067a1024235128511S3Bucket776FD46E": {
       "Type": "String",

diff --git a/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts b/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts
@@ -241,6 +241,38 @@ test('fails if user handler returns a non-object response', async () => {
   expectCloudFormationFailed('return values from user-handlers must be JSON objects. got: \"string\"');
 });
 
+describe('if CREATE fails, the subsequent DELETE will be ignored', () => {
+
+  it('FAILED response sets PhysicalResourceId to a special marker', async () => {
+    // WHEN
+    mocks.onEventImplMock = async () => { throw new Error('CREATE FAILED'); };
+
+    // THEN
+    await simulateEvent({
+      RequestType: 'Create'
+    });
+
+    expectCloudFormationFailed('CREATE FAILED', {
+      PhysicalResourceId: cfnResponse.CREATE_FAILED_PHYSICAL_ID_MARKER,
+    });
+  });
+
+  it('DELETE request with the marker succeeds without calling user handler', async () => {
+    // GIVEN
+    // user handler is not assigned
+
+    // WHEN
+    await simulateEvent({
+      RequestType: 'Delete',
+      PhysicalResourceId: cfnResponse.CREATE_FAILED_PHYSICAL_ID_MARKER
+    });
+
+    // THEN
+    expectCloudFormationSuccess();
+  });
+
+});
+
 // -----------------------------------------------------------------------------------------------------------------------
 
 /**
@@ -292,10 +324,11 @@ async function simulateEvent(req: Partial<AWSLambda.CloudFormationCustomResource
   }
 }
 
-function expectCloudFormationFailed(expectedReason: string) {
+function expectCloudFormationFailed(expectedReason: string, resp?: Partial<AWSLambda.CloudFormationCustomResourceResponse>) {
   expectCloudFormationResponse({
     Status: 'FAILED',
-    Reason: expectedReason
+    Reason: expectedReason,
+    ...resp
   });
 }