-
Notifications
You must be signed in to change notification settings - Fork 406
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(event_handler): improved support for headers and cookies in v2 #1455
Merged
heitorlessa
merged 35 commits into
aws-powertools:v2
from
rubenfonseca:feat/api-gateway-cookies-v2
Aug 29, 2022
Merged
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
0c1f2f0
feat(event_handler): add support for setting cookies
rubenfonseca 0d9dae3
feat(event_handler): format headers and cookies according to the requ…
rubenfonseca e28c8ca
chore(tests): move load_event helper to global utils
rubenfonseca 38f1d92
chore(tests): add tests for header serializer
rubenfonseca 8178362
chore(tests): fix all tests
rubenfonseca d7dea93
chore(tests): typo
rubenfonseca 9089df7
chore(event_handler): move header serializer logic
rubenfonseca c7edb6d
Revert "chore(tests): move load_event helper to global utils"
rubenfonseca 8ddce93
fix(event_handler): simplified tests
rubenfonseca 66f1c41
docs(event_handler): update example
rubenfonseca a240f02
fix(event_handler): don't be smart about multiple headers
rubenfonseca 5263a2d
fix(docs): simplified wording
rubenfonseca 7ad8b88
feat(event_handler): add support for multiple headers with same key
rubenfonseca a705d26
chore(tests): fix headers test
rubenfonseca 8bb2658
chore(tests): move headers test to the correct place
rubenfonseca e702ab8
tests(event_handler): add first e2e test
rubenfonseca 7e169a9
chore(docs): initial upgrade guide for v2
rubenfonseca 6faeb91
chore(tests): move load_event helper to global utils
rubenfonseca ba039f8
Revert "chore(tests): move load_event helper to global utils"
rubenfonseca e2b64d5
feat(event_handler): add e2e tests
rubenfonseca 7646079
chore(deps): add latest cdk packages
rubenfonseca 3271d75
chore(event_handler): address review comments
rubenfonseca e5ceac6
chore(deps): dropped python 3.6 from pyproject
rubenfonseca 2aca4f0
chore(event_handler): applied suggestions from code review
rubenfonseca e6ccc69
chore(deps): remove old python version guard
rubenfonseca bed825d
chore(deps): upgrade black
rubenfonseca 70f61dc
chore(test): move cookie name to GIVEN block
rubenfonseca 1b4b18d
chore(docs): move constant to GIVEN block
rubenfonseca 8fe8398
chore(tests): use table-testing
rubenfonseca 56b0d2c
chore: re-added sorting of CORS headers
rubenfonseca fbac728
chore: simplified tests
rubenfonseca 840e9c4
chore: change test name to ease scanning
heitorlessa 60d07f7
chore: revert parametrize into single tests like Ruben had
heitorlessa 8ce6c50
chore: revert parametrize into single tests like Ruben had pt2
heitorlessa a27d674
feat(event_handler): supppor headers with either str or List[str] values
rubenfonseca File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -305,5 +305,8 @@ site/ | |
!404.html | ||
!docs/overrides/*.html | ||
|
||
# CDK | ||
.cdk | ||
|
||
!.github/workflows/lib | ||
examples/**/sam/.aws-sam |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,15 @@ | ||
# -*- coding: utf-8 -*- | ||
|
||
"""Top-level package for Lambda Python Powertools.""" | ||
|
||
from pathlib import Path | ||
|
||
"""Top-level package for Lambda Python Powertools.""" | ||
from .logging import Logger # noqa: F401 | ||
from .metrics import Metrics, single_metric # noqa: F401 | ||
from .package_logger import set_package_logger_handler | ||
from .tracing import Tracer # noqa: F401 | ||
|
||
__author__ = """Amazon Web Services""" | ||
|
||
PACKAGE_PATH = Path(__file__).parent | ||
|
||
set_package_logger_handler() |
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
import warnings | ||
from collections import defaultdict | ||
from typing import Any, Dict, List, Union | ||
|
||
|
||
class BaseHeadersSerializer: | ||
""" | ||
Helper class to correctly serialize headers and cookies for Amazon API Gateway, | ||
ALB and Lambda Function URL response payload. | ||
""" | ||
|
||
def serialize(self, headers: Dict[str, Union[str, List[str]]], cookies: List[str]) -> Dict[str, Any]: | ||
""" | ||
Serializes headers and cookies according to the request type. | ||
Returns a dict that can be merged with the response payload. | ||
|
||
Parameters | ||
---------- | ||
headers: Dict[str, List[str]] | ||
A dictionary of headers to set in the response | ||
cookies: List[str] | ||
A list of cookies to set in the response | ||
""" | ||
raise NotImplementedError() | ||
|
||
|
||
class HttpApiHeadersSerializer(BaseHeadersSerializer): | ||
def serialize(self, headers: Dict[str, Union[str, List[str]]], cookies: List[str]) -> Dict[str, Any]: | ||
""" | ||
When using HTTP APIs or LambdaFunctionURLs, everything is taken care automatically for us. | ||
We can directly assign a list of cookies and a dict of headers to the response payload, and the | ||
runtime will automatically serialize them correctly on the output. | ||
|
||
https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html#http-api-develop-integrations-lambda.proxy-format | ||
https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-develop-integrations-lambda.html#http-api-develop-integrations-lambda.response | ||
""" | ||
|
||
# Format 2.0 doesn't have multiValueHeaders or multiValueQueryStringParameters fields. | ||
# Duplicate headers are combined with commas and included in the headers field. | ||
combined_headers: Dict[str, str] = {} | ||
for key, values in headers.items(): | ||
if isinstance(values, str): | ||
combined_headers[key] = values | ||
else: | ||
combined_headers[key] = ", ".join(values) | ||
|
||
return {"headers": combined_headers, "cookies": cookies} | ||
|
||
|
||
class MultiValueHeadersSerializer(BaseHeadersSerializer): | ||
def serialize(self, headers: Dict[str, Union[str, List[str]]], cookies: List[str]) -> Dict[str, Any]: | ||
""" | ||
When using REST APIs, headers can be encoded using the `multiValueHeaders` key on the response. | ||
This is also the case when using an ALB integration with the `multiValueHeaders` option enabled. | ||
The solution covers headers with just one key or multiple keys. | ||
|
||
https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-output-format | ||
https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#multi-value-headers-response | ||
""" | ||
payload: Dict[str, List[str]] = defaultdict(list) | ||
|
||
for key, values in headers.items(): | ||
if isinstance(values, str): | ||
payload[key].append(values) | ||
else: | ||
for value in values: | ||
payload[key].append(value) | ||
|
||
if cookies: | ||
payload.setdefault("Set-Cookie", []) | ||
for cookie in cookies: | ||
payload["Set-Cookie"].append(cookie) | ||
|
||
return {"multiValueHeaders": payload} | ||
|
||
|
||
class SingleValueHeadersSerializer(BaseHeadersSerializer): | ||
def serialize(self, headers: Dict[str, Union[str, List[str]]], cookies: List[str]) -> Dict[str, Any]: | ||
""" | ||
The ALB integration has `multiValueHeaders` disabled by default. | ||
If we try to set multiple headers with the same key, or more than one cookie, print a warning. | ||
|
||
https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#respond-to-load-balancer | ||
""" | ||
payload: Dict[str, Dict[str, str]] = {} | ||
payload.setdefault("headers", {}) | ||
|
||
if cookies: | ||
if len(cookies) > 1: | ||
warnings.warn( | ||
"Can't encode more than one cookie in the response. Sending the last cookie only. " | ||
"Did you enable multiValueHeaders on the ALB Target Group?" | ||
) | ||
|
||
# We can only send one cookie, send the last one | ||
payload["headers"]["Set-Cookie"] = cookies[-1] | ||
|
||
for key, values in headers.items(): | ||
if isinstance(values, str): | ||
payload["headers"][key] = values | ||
else: | ||
if len(values) > 1: | ||
warnings.warn( | ||
f"Can't encode more than one header value for the same key ('{key}') in the response. " | ||
"Did you enable multiValueHeaders on the ALB Target Group?" | ||
) | ||
|
||
# We can only set one header per key, send the last one | ||
payload["headers"][key] = values[-1] | ||
|
||
return payload |
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
--- | ||
title: Upgrade guide | ||
description: Guide to update between major Powertools versions | ||
--- | ||
|
||
<!-- markdownlint-disable MD043 --> | ||
|
||
## Migrate to v2 from v1 | ||
|
||
The transition from Powertools for Python v1 to v2 is as painless as possible, as we aimed for minimal breaking changes. | ||
Changes at a glance: | ||
|
||
* The API for **event handler's `Response`** has minor changes to support multi value headers and cookies. | ||
|
||
???+ important | ||
Powertools for Python v2 drops suport for Python 3.6, following the Python 3.6 End-Of-Life (EOL) reached on December 23, 2021. | ||
|
||
### Initial Steps | ||
|
||
Before you start, we suggest making a copy of your current working project or create a new branch with git. | ||
|
||
1. **Upgrade** Python to at least v3.7 | ||
|
||
2. **Ensure** you have the latest `aws-lambda-powertools` | ||
|
||
```bash | ||
pip install aws-lambda-powertools -U | ||
``` | ||
|
||
3. **Review** the following sections to confirm whether they affect your code | ||
|
||
## Event Handler Response (headers and cookies) | ||
|
||
The `Response` class of the event handler utility changed slightly: | ||
|
||
1. The `headers` parameter now expects either a value or list of values per header (type `Union[str, Dict[str, List[str]]]`) | ||
2. We introduced a new `cookies` parameter (type `List[str]`) | ||
|
||
???+ note | ||
Code that set headers as `Dict[str, str]` will still work unchanged. | ||
|
||
```python hl_lines="6 12 13" | ||
@app.get("/todos") | ||
def get_todos(): | ||
# Before | ||
return Response( | ||
# ... | ||
headers={"Content-Type": "text/plain"} | ||
) | ||
|
||
# After | ||
return Response( | ||
# ... | ||
headers={"Content-Type": ["text/plain"]}, | ||
cookies=["CookieName=CookieValue"] | ||
) | ||
``` |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@heitorlessa I'm seeing
TypeError: object of type 'NoneType' has no len()
when I have the following header set:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasmarc could you create a bug report issue on this please?
We can make a patch release as soon as we reproduce it.
Thanks a lot!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we await for a bug report and a fix by EOD for headers with
None
value, you can safely use an empty string instead ofNone
.Because we provide native support for CORS in Event Handler, you can do as follows:
If your intent is to set an explicit
null
in the Allow-Origin...W3C doesn't recommend having an Allow Origin explicitly set to
null
due to potential data leak, sincedata:
andfile:
schemes can accessnull
origins - that's not the case with an empty Allow Origin (''
). Unless I'm misunderstanding both your intent (bug report helps!) or W3C docs.With an explicit
null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leandrodamascena when you're back online, feel free to create a bug report if we don't hear from the customer within an hour. Leaving the tests and my endpoint above to save you from setting it all up besides E2E.
These tests reproduce the issue. What needs to be done is an E2E for each resolver to confirm whether the final value becomes
''
or fails (ALB is always the odd one) - this needs to be documented too later.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @heitorlessa sorry for the delay. Yes, I worked around it and I did also notice that it was not recommended, so I almost hesitated to bring it up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have filed #1791