Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(aws-lambda): add new configuration field empty_arrays_mode to control empty array decoding behavior #13084

Merged
merged 5 commits into from
Jun 4, 2024

Conversation

windmgc
Copy link
Member

@windmgc windmgc commented May 27, 2024

Summary

This PR adds a new configuration field empty_arrays_mode to control the behaviour that whether Kong will transform an empty array [](returned by Lambda function) into an empty object {} when enabled aws-lambda plugin in the response.

Essentially this PR is just a small bugfix for not previously enabling decode_array_with_array_mt when decoding Lambda function's response. However, enabling decode_array_with_array_mt may bring unexpected behaviour changes on the user side(since the wrong behaviour itself has existed for long), so we decided to add a new field to keep backward compatibility, and remove this field in the next major version to let the correct behaviour(keep empty arrrays as is) to become the default behaviour in the next major version.

Checklist

Issue reference

FTI-5937
KAG-4622
KAG-4615

@windmgc windmgc marked this pull request as ready for review May 27, 2024 03:27
@@ -116,6 +116,13 @@ return {
default = "v1",
one_of = { "v1", "v2" }
} },
{ empty_arrays_mode = { -- TODO: this config field is added for backward compatibility and will be removed in next major version
description = "An optional value that defines whether Kong should send empty arrays(returned by Lambda function) as `[]` arrays or `{}` objects in JSON responses. The value `legacy` means Kong will send empty arrays as `{}` objects in response",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this?

Suggested change
description = "An optional value that defines whether Kong should send empty arrays(returned by Lambda function) as `[]` arrays or `{}` objects in JSON responses. The value `legacy` means Kong will send empty arrays as `{}` objects in response",
description = "An optional value that defines whether Kong should send empty arrays(returned by Lambda function) as `[]` arrays or `{}` objects in JSON responses. The value `legacy` means Kong will send empty arrays `[]` as `{}` objects in response",

@windmgc windmgc requested a review from vm-001 June 3, 2024 09:01
Copy link
Contributor

@vm-001 vm-001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

I would love to see if we have a compatibility test for that new field.

Copy link
Member

@bungle bungle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small comment.

kong/plugins/aws-lambda/handler.lua Outdated Show resolved Hide resolved
@windmgc windmgc force-pushed the feat-aws-lambda-empty-json-array branch from 3657ea2 to 5ace553 Compare June 4, 2024 03:33
@windmgc windmgc force-pushed the feat-aws-lambda-empty-json-array branch from 5ace553 to 159fd91 Compare June 4, 2024 04:51
@windmgc windmgc requested a review from bungle June 4, 2024 05:50
@bungle bungle merged commit 76f2612 into master Jun 4, 2024
25 checks passed
@bungle bungle deleted the feat-aws-lambda-empty-json-array branch June 4, 2024 11:08
@github-actions github-actions bot added the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Jun 4, 2024
@kikito kikito removed the incomplete-cherry-pick A cherry-pick was incomplete and needs manual intervention label Jun 4, 2024
@Kong Kong deleted a comment from team-gateway-bot Jun 4, 2024
locao pushed a commit that referenced this pull request Jun 21, 2024
…control empty array decoding behavior (#13084)

Co-authored-by: Yusheng Li <leeys.top@gmail.com>
Co-authored-by: Zachary Hu <6426329+outsinre@users.noreply.github.com>
windmgc added a commit that referenced this pull request Jul 22, 2024
…oxy integration and legacy empty arrays mode (#13381)

This PR fixes a bug that introduced by #13084, that when is_proxy_integration mode is set to true with empty_arrays_mode set to legacy, and lambda function returns multiValueHeader that contains a Content-Type with single value array, the plugin cannot work correctly.

The multiValueHeaders field in the proxy integration response can be an array even if the header has only one value. I've tested AWS API gateway and the Kong gateway before #13084 and confirmed that both would work fine with Content-Type single value array inside the mutliValueHeader response field.

The reason of #13084 brings in this issue is that before #13084 we did not have any logic about reading Content-Type so no problem existed. The single value array will goes into kong.response.exit and normalized automatically.

This PR fixes the problem that when Content-Type returned by the lambda function is ["application-json"], the plugin cannot execute the if-block correctly due to not being able to call the lower function on the array(table) object.

Although there is a tiny possibility that the user will encounter this issue(because I think mostly people will define Content-Type inside the headers field instead of the mutliValueHeaders since it should be only one value), it is still good to fix this issue and let it behave like before.

https://konghq.atlassian.net/browse/FTI-6100

---------

Co-authored-by: Zachary Hu <6426329+outsinre@users.noreply.github.com>
oowl pushed a commit that referenced this pull request Aug 15, 2024
…oxy integration and legacy empty arrays mode (#13381)

This PR fixes a bug that introduced by #13084, that when is_proxy_integration mode is set to true with empty_arrays_mode set to legacy, and lambda function returns multiValueHeader that contains a Content-Type with single value array, the plugin cannot work correctly.

The multiValueHeaders field in the proxy integration response can be an array even if the header has only one value. I've tested AWS API gateway and the Kong gateway before #13084 and confirmed that both would work fine with Content-Type single value array inside the mutliValueHeader response field.

The reason of #13084 brings in this issue is that before #13084 we did not have any logic about reading Content-Type so no problem existed. The single value array will goes into kong.response.exit and normalized automatically.

This PR fixes the problem that when Content-Type returned by the lambda function is ["application-json"], the plugin cannot execute the if-block correctly due to not being able to call the lower function on the array(table) object.

Although there is a tiny possibility that the user will encounter this issue(because I think mostly people will define Content-Type inside the headers field instead of the mutliValueHeaders since it should be only one value), it is still good to fix this issue and let it behave like before.

https://konghq.atlassian.net/browse/FTI-6100

---------

Co-authored-by: Zachary Hu <6426329+outsinre@users.noreply.github.com>
(cherry picked from commit 6f5684d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants