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(plugin/aws-lambda)accept string type statusCode as return when working in proxy integration mode #8765

Merged
merged 6 commits into from
May 17, 2022

Conversation

windmgc
Copy link
Member

@windmgc windmgc commented May 7, 2022

Summary

Hi, I would like to add some compatibility to Kong's aws-lambda plugin so that it can receive string type status code when working in the proxy integration mode.

When AWS Lambda function is working as a proxy integration, the statusCode returned by lambda function will become the HTTP status code returned by API gateway.

Currently, we only accept number type statusCode,

if type(response.statusCode) ~= "number" then

After some tests I found that AWS API gateway works fine if the type of statusCode is either number or string. And if the statusCode is an invalid HTTP status code, such as 600(smaller than 100 or larger than 599) or 200a(containing non-numeric characters), it will throw an "internal server error".

It would be good for us to be compatible with a string type status code, according to FTI-4014.

Full changelog

  • Add a feature that make aws-lambda handler be compatible with string type statusCode
  • Add validation function to check if the statusCode is legal

Issue reference

Fix FTI-4014

@windmgc windmgc requested a review from a team as a code owner May 7, 2022 09:16
@CLAassistant
Copy link

CLAassistant commented May 7, 2022

CLA assistant check
All committers have signed the CLA.

@windmgc windmgc requested review from ms2008, outsinre, VicYP and fffonion and removed request for outsinre May 7, 2022 09:20
@pull-request-size pull-request-size bot added size/M and removed size/S labels May 9, 2022
Copy link
Contributor

@fffonion fffonion left a comment

Choose a reason for hiding this comment

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

🎉 First PR!

@windmgc windmgc requested a review from a team May 9, 2022 07:30
Copy link
Contributor

@ms2008 ms2008 left a comment

Choose a reason for hiding this comment

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

LGTM

@windmgc
Copy link
Member Author

windmgc commented May 17, 2022

@dndx Could you review the PR again? Thanks!

Comment on lines 75 to 77
if not status_code then
return false
end
Copy link
Member

Choose a reason for hiding this comment

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

This should be moved inside the if block above, if the status_code is not string then there is no reason to check it again here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

I wonder if it needs more strict check on types, like

  if type(status_code) == "string" or type(status_code) == "number" then
    status_code = tonumber(status_code)

    if not status_code then
      return false
    end

  else
    -- Validation failed if status code is not string or number
    return false
  end
  
  end

@dndx dndx merged commit 86f67e5 into Kong:master May 17, 2022
@windmgc windmgc deleted the FTI-4014 branch May 17, 2022 06:11
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.

5 participants