Skip to content

Commit

Permalink
fix(custom-resources): Custom resource provider framework not passing…
Browse files Browse the repository at this point in the history
… `ResponseURL` to user function (backport #21117) (#21123)

This is an automatic backport of pull request #21117 done by [Mergify](https://mergify.com).
Cherry-pick of ddfca48 has failed:
```
On branch mergify/bp/v1-main/pr-21117
Your branch is up to date with 'origin/v1-main'.

You are currently cherry-picking commit ddfca48.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   packages/@aws-cdk/custom-resources/test/provider-framework/runtime.test.ts

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   packages/@aws-cdk/custom-resources/lib/provider-framework/runtime/framework.ts

```


To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com/) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>
  • Loading branch information
mergify[bot] authored Jul 13, 2022
1 parent 8201b4e commit e280dfe
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -96,7 +96,7 @@ async function onTimeout(timeoutEvent: any) {
});
}

async function invokeUserFunction<A extends { ResponseURL: '...' }>(functionArnEnv: string, sanitizedPayload: A) {
async function invokeUserFunction<A extends { ResponseURL: '...' }>(functionArnEnv: string, sanitizedPayload: A, responseUrl: string) {
const functionArn = getEnv(functionArnEnv);
log(`executing user function ${functionArn} with payload`, sanitizedPayload);

Expand All @@ -105,7 +105,9 @@ async function invokeUserFunction<A extends { ResponseURL: '...' }>(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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}"`),
}));
});
});

// -----------------------------------------------------------------------------------------------------------------------

/**
Expand Down

0 comments on commit e280dfe

Please sign in to comment.