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

Docs: In the SAM template, BinaryMediaTypes and CORS do not work together #3373

Closed
1 task done
martinber opened this issue Nov 20, 2023 · 8 comments
Closed
1 task done
Assignees
Labels
documentation Improvements or additions to documentation event_handlers

Comments

@martinber
Copy link

What were you searching in the docs?

How to configure CORS

Is this related to an existing documentation section?

https://docs.powertools.aws.dev/lambda/python/latest/core/event_handler/api_gateway/#required-resources

How can we improve?

If I'm not wrong, when configuring CORS in the template.yml of AWS SAM, CORS doesn't work if BinaryMediaTypes is set. See this issue.

The example in the documentation has both CORS and BinaryMediaTypes configured. I just copied this example without thinking much and I was surprised to see that it doesn't work. I think that at least a note should be added somewhere to indicate that these two are incompatible.

Got a suggestion in mind?

No response

Acknowledgment

  • I understand the final update might be different from my proposed suggestion, or refused.
@martinber martinber added documentation Improvements or additions to documentation triage Pending triage from maintainers labels Nov 20, 2023
Copy link

boring-cyborg bot commented Nov 20, 2023

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@heitorlessa
Copy link
Contributor

heitorlessa commented Nov 21, 2023

hey @martinber sad to hear you bumped into a known issue in SAM exacerbated by our incorrect docs. We'll add a note and push a doc rebuild by EOD, and give the SAM team a heads up about it.

Thank you @martinber for flagging this to us, truly.

NOTE: I'll read the entire thread to make sure I fully understand whether this is an API Gateway, SAM or SAM CLI limitation, and document accordingly. If there's a known workaround, we'll update the template too.

@heitorlessa heitorlessa removed the triage Pending triage from maintainers label Nov 21, 2023
@heitorlessa heitorlessa self-assigned this Nov 21, 2023
@heitorlessa heitorlessa moved this from Triage to Backlog in Powertools for AWS Lambda (Python) Nov 21, 2023
@heitorlessa
Copy link
Contributor

heitorlessa commented Nov 21, 2023

NOTE: This is not an issue in Powertools for AWS Lambda. We handle runtime.

Tested and here's a summary for anyone coming in later on the issue:

  1. API Gateway Pre-flight (OPTIONS) gets confused with */* binary and believes all responses are binary
  2. Setting each individual mime types work however this can be problematic for customers receiving arbitrary set of binaries

Action. Adding a comment in the template and in the Binary section to highlight this limitation in API Gateway.

workaround

You could use API Gateway as a passthrough for pre-flight requests by configuring it to send OPTIONS requests to Lambda function. You can use @app.route(".*", method="OPTIONS") - that way you have full control of pre-flight logic.

We don't document it yet because by default as ~90% of the time API Gateway handles just fine, and this can create confusion. This is typically used as a workaround for another issue - e.g., multi-origin.

@heitorlessa
Copy link
Contributor

PR created. Added the following notes in three places, along with an example of a stricter mime type if CORS must be used:

  1. Warning in Binary response section
  2. Getting started SAM template
  3. Micro-function SAM template

Releasing shortly along with other bugfixes today.

image image

@martinber
Copy link
Author

Thank you! This is more than enough for me

@github-actions github-actions bot added the pending-release Fix or implementation already in dev waiting to be released label Nov 21, 2023
@heitorlessa
Copy link
Contributor

Merged, gonna prep a release now so nobody else bumps into this (hopefully). If we get yet another customer, I'll personally create a complete workaround example to handle CORS along with caveats.

Thank you one more time @martinber for taking the time to report this. We appreciate your care on making everyone's experience better!

Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

Copy link
Contributor

This is now released under 2.27.1 version!

@github-actions github-actions bot removed the pending-release Fix or implementation already in dev waiting to be released label Nov 21, 2023
@heitorlessa heitorlessa moved this from Coming soon to Shipped in Powertools for AWS Lambda (Python) Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation event_handlers
Projects
Status: Shipped
Development

No branches or pull requests

2 participants