Lambda-http: vary type of response based on request origin #269
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ApiGatewayV2, ApiGateway and Alb all expect different types of responses to be returned from the invoked lambda function. Thus, it makes sense to pass the request origin to the creation of the response, so that the correct type of LambdaResponse is returned from the function. This commit also adds support for the "cookies" attribute which can be used for returning multiple Set-cookie headers from a lambda invoked via ApiGatewayV2, since ApiGatewayV2 no longer seems to recognize the "multiValueHeaders" attribute. Closes: awslabs#267.
softprops
approved these changes
Nov 25, 2020
There was a problem hiding this comment.
This is excellent.
At first there was only rest apis ... and then albs.. and then http apis.
When we got to supporting three types the current impl didn't feel right but this feels a lot cleaner and provides an way forward when the inevitable 4th http like trigger comes along
| @@ -149,7 +152,7 @@ where | |||
|
|
|||
| #[doc(hidden)] | |||
| pub struct TransformResponse<R, E> { | |||
| is_alb: bool, | |||
| request_origin: RequestOrigin, | |||
softprops
approved these changes
Nov 25, 2020
calavera
approved these changes
Nov 30, 2020
There was a problem hiding this comment.
It'd be awesome if this got merged into master @davidbarsky.
|
An honest question: why is this runtime maintaining its own implementation of the AWS Events where one could get them from the |
bahildebrand
approved these changes
Feb 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#267
Description of changes:
ApiGatewayV2, ApiGateway and Alb all expect different types of responses to be returned from the invoked lambda function. Thus, it makes sense to pass the request origin to the creation of the response, so that the correct type of LambdaResponse is returned from the function.
This PR also adds support for the "cookies" attribute which can be used for returning multiple Set-cookie headers from a lambda invoked via ApiGatewayV2, since ApiGatewayV2 no longer seems to recognize the "multiValueHeaders" attribute.
On lines 129-132 of
response.rstheSET_COOKIEheaders are removed from theheadersreturned to API Gateway v2, because otherwise API Gateway v2 would return threeset-cookieheaders when just returning two cookies incookies, because one of the cookies would be duplicated in theheaders. I tried using thehttp::header::OccupiedEntry::remove_entry_multfunction for removing theSET_COOKIEheaders from theheaders, but I got some panicking from the http crate as described in hyperium/http#446. So I went with the solution where I first fetch theSET_COOKIEheaders returned withheaders.get_alland insert them cloned into thecookies, and then remove them withheaders.remove(SET_COOKIE). However, any ideas and suggestions as to improve this are warmly welcome!By submitting this pull request