From f00f95213cbe31d0e78a01b3ada8a68eeda55efa Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Wed, 13 Jul 2022 14:13:00 +0100 Subject: [PATCH] fix(custom-resources): Custom resource provider framework not passing `ResponseURL` to user function (#21117) aws#20889 included a change that broke the custom resource framework by not including the necessary presigned URL. We attempted to fix this with aws#21065 but it didn't resolve the issue and aws#21109 reverted the attempted fix. This changes explicitly includes the presigned URL, as well as adding tests to ensure the URL is passed to the downstream Lambda Function. Closes aws#21058 ---- * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../provider-framework/runtime/framework.ts | 10 +++-- .../test/provider-framework/runtime.test.ts | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts index f9586e032ae5f..20c0144311b94 100644 --- a/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts +++ b/packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts @@ -29,7 +29,7 @@ async function onEvent(cfnRequest: AWSLambda.CloudFormationCustomResourceEvent) cfnRequest.ResourceProperties = cfnRequest.ResourceProperties || { }; - const onEventResult = await invokeUserFunction(consts.USER_ON_EVENT_FUNCTION_ARN_ENV, sanitizedRequest) as OnEventResponse; + const onEventResult = await invokeUserFunction(consts.USER_ON_EVENT_FUNCTION_ARN_ENV, sanitizedRequest, cfnRequest.ResponseURL) as OnEventResponse; log('onEvent returned:', onEventResult); // merge the request and the result from onEvent to form the complete resource event @@ -61,7 +61,7 @@ async function isComplete(event: AWSCDKAsyncCustomResource.IsCompleteRequest) { const sanitizedRequest = { ...event, ResponseURL: '...' } as const; log('isComplete', sanitizedRequest); - const isCompleteResult = await invokeUserFunction(consts.USER_IS_COMPLETE_FUNCTION_ARN_ENV, sanitizedRequest) as IsCompleteResponse; + const isCompleteResult = await invokeUserFunction(consts.USER_IS_COMPLETE_FUNCTION_ARN_ENV, sanitizedRequest, event.ResponseURL) as IsCompleteResponse; log('user isComplete returned:', isCompleteResult); // if we are not complete, return false, and don't send a response back. @@ -96,7 +96,7 @@ async function onTimeout(timeoutEvent: any) { }); } -async function invokeUserFunction(functionArnEnv: string, sanitizedPayload: A) { +async function invokeUserFunction(functionArnEnv: string, sanitizedPayload: A, responseUrl: string) { const functionArn = getEnv(functionArnEnv); log(`executing user function ${functionArn} with payload`, sanitizedPayload); @@ -105,7 +105,9 @@ async function invokeUserFunction(functionArnE // automatically by the JavaScript SDK. const resp = await invokeFunction({ FunctionName: functionArn, - Payload: JSON.stringify(sanitizedPayload), + + // Cannot strip 'ResponseURL' here as this would be a breaking change even though the downstream CR doesn't need it + Payload: JSON.stringify({ ...sanitizedPayload, ResponseURL: responseUrl }), }); log('user function response:', resp, typeof(resp)); 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 index 751e33e6408e0..2d35a9e63eac9 100644 --- a/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts +++ b/packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts @@ -18,7 +18,10 @@ outbound.httpRequest = mocks.httpRequestMock; outbound.invokeFunction = mocks.invokeFunctionMock; outbound.startExecution = mocks.startExecutionMock; +const invokeFunctionSpy = jest.spyOn(outbound, 'invokeFunction'); + beforeEach(() => mocks.setup()); +afterEach(() => invokeFunctionSpy.mockClear()); test('async flow: isComplete returns true only after 3 times', async () => { let isCompleteCalls = 0; @@ -346,6 +349,41 @@ describe('if CREATE fails, the subsequent DELETE will be ignored', () => { }); +describe('ResponseURL is passed to user function', () => { + test('for onEvent', async () => { + // GIVEN + mocks.onEventImplMock = async () => ({ PhysicalResourceId: MOCK_PHYSICAL_ID }); + + // WHEN + await simulateEvent({ + RequestType: 'Create', + }); + + // THEN + expect(invokeFunctionSpy).toHaveBeenCalledTimes(1); + expect(invokeFunctionSpy).toBeCalledWith(expect.objectContaining({ + Payload: expect.stringContaining(`"ResponseURL":"${mocks.MOCK_REQUEST.ResponseURL}"`), + })); + }); + + test('for isComplete', async () => { + // GIVEN + mocks.onEventImplMock = async () => ({ PhysicalResourceId: MOCK_PHYSICAL_ID }); + mocks.isCompleteImplMock = async () => ({ IsComplete: true }); + + // WHEN + await simulateEvent({ + RequestType: 'Create', + }); + + // THEN + expect(invokeFunctionSpy).toHaveBeenCalledTimes(2); + expect(invokeFunctionSpy).toHaveBeenLastCalledWith(expect.objectContaining({ + Payload: expect.stringContaining(`"ResponseURL":"${mocks.MOCK_REQUEST.ResponseURL}"`), + })); + }); +}); + // ----------------------------------------------------------------------------------------------------------------------- /**