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

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.

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 as any,
     LogicalResourceId: event.LogicalResourceId,
     NoEcho: options.noEcho,
     Data: event.Data
@@ -50,11 +53,41 @@ 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)}`);
+          event.PhysicalResourceId = MISSING_PHYSICAL_ID_MARKER;
+        }
+      }
 
       // this is an actual error, fail the activity altogether and exist.
       await submitResponse('FAILED', event, {

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
   });
 }