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

Support New AWS APIGW Binary Responses #2797

Closed
jthomerson opened this issue Nov 25, 2016 · 50 comments
Closed

Support New AWS APIGW Binary Responses #2797

jthomerson opened this issue Nov 25, 2016 · 50 comments
Assignees
Milestone

Comments

@jthomerson
Copy link
Contributor

@jthomerson jthomerson commented Nov 25, 2016

This is a Feature Proposal

Description

Previously, AWS API Gateway did not support binary responses, making it impossible to return images from your serverless API. Now they do (see https://aws.amazon.com/blogs/compute/binary-support-for-api-integrations-with-amazon-api-gateway/). We need to be able to configure HTTP endpoints/events in serverless to use this new functionality.

@pmuens pmuens added the feature label Nov 26, 2016
@vladgolubev
Copy link
Contributor

@vladgolubev vladgolubev commented Dec 11, 2016

Does it mean we will be able to gzip response?

@adambiggs
Copy link
Contributor

@adambiggs adambiggs commented Dec 17, 2016

My team is eagerly anticipating this feature in Serverless. We have an image resizing service that currently needs to proxy responses through a traditional server to return images... 😫

Does anybody know of a workaround we could use until this is added to Serverless?

@vladgolubev
Copy link
Contributor

@vladgolubev vladgolubev commented Dec 17, 2016

@adambiggs
Copy link
Contributor

@adambiggs adambiggs commented Dec 17, 2016

@vladgolubev our image resizing service resizes the images on-demand based on query string params. The lambda first checks if the requested image size already exists on S3. If it does, it returns the existing image, and if not it generates it, stores it on S3 and then returns it. So direct S3 links won't work for us.

Our current workaround is to run a simple Node.js reverse proxy that calls the Lambda image resize service, gets the direct S3 link, and then returns the binary response to the client...

But this is a temporary solution. When Serverless adds binary response support (or if we can find a workaround, like making changes directly in AWS after deploying with Serverless) we can get rid of the reverse proxy without changing any previously generated image URLs.

@bbilger
Copy link
Contributor

@bbilger bbilger commented Dec 21, 2016

Note: there's some thread in the forum about this: http://forum.serverless.com/t/returning-binary-data-jpg-from-lambda-via-api-gateway/796
The current issue - as far as I know - is that the binaryMediaTypes cannot be set in CloudFormation.
So your only option right now is to configure them manually (if I remember correctly it survives a re-deployment).
See here: https://github.com/bbilger/jrestless-examples/tree/master/aws/gateway/aws-gateway-binary
or here: https://github.com/krisgholson/serverless-thumbnail
or in the blog post: https://aws.amazon.com/blogs/compute/binary-support-for-api-integrations-with-amazon-api-gateway/

and sure you can also gzip: https://github.com/bustlelabs/gziptest/ (not using serverless)

I would, however, not expect too much from it since
a) API Gateway has a size limit of 10MB (+ I guess you have to substract the base64 overhead from it)
b) only if a proper Accept header (for the response) or Content-Type header (for the request) i.e. one registered as binary media type is set, you'll get binary data, else you'll end up with base64 encoded content.
c) If I remember correctly there are issues with multiple accept headers like "Accept: image/png,image/gif"

@pmuens pmuens added help wanted and removed help wanted labels Mar 21, 2017
@nikgraf
Copy link
Contributor

@nikgraf nikgraf commented Apr 4, 2017

@pmuens
Copy link
Contributor

@pmuens pmuens commented Apr 4, 2017

@pmuens seems like somebody got it to work

Wow that's pretty cool! Thanks for sharing! 👍

/cc @brianneisler @eahefnawy

@brianneisler brianneisler added this to the 1.14 milestone May 8, 2017
@ryanmurakami ryanmurakami mentioned this issue May 11, 2017
2 of 7 tasks complete
@brianneisler brianneisler removed this from the 1.14 milestone May 15, 2017
@brianneisler
Copy link
Contributor

@brianneisler brianneisler commented May 15, 2017

Removed from the 1.14 milestone since we're still waiting on CloudFormation support.

@pmuens pmuens removed the needs feedback label May 19, 2017
@schickling
Copy link
Contributor

@schickling schickling commented May 28, 2017

Is there a timeline from AWS for CF support?

@vangorra
Copy link

@vangorra vangorra commented May 28, 2017

We ended up defining our function through serverless but defined the API gateway in the resources section with swagger yml. Works great.

@pmuens
Copy link
Contributor

@pmuens pmuens commented May 29, 2017

Is there a timeline from AWS for CF support?

@schickling unfortunately not yet.

We ended up defining our function through serverless but defined the API gateway in the resources section with swagger yml. Works great.

That sounds like a good workaround. Thanks for sharing @vangorra 👍


For everyone else who wants to use this now. A new Serverless plugin for this was published recently: https://github.com/ryanmurakami/serverless-apigwy-binary

@rupakg rupakg modified the milestone: 1.15 Jun 1, 2017
@amv
Copy link

@amv amv commented Jul 2, 2017

Any word on the CF support?

I am having some troubles with the mentioned plugin, mainly because it forces me to change the integration from lambda-proxy to lambda. I can't seem to get my Content-Types remaining as they are supposed to, as the plugin seems to have a side effect of clearing the Content-Type header mapping for the default 200 pattern.

I was however able to make my binary endpoint work by using the lambda-proxy, manually adding */* to "Binary Support" through the AWS Api Gateway GUI, and sending my data as base64 like this:

callback(null, {
  statusCode: 200,
  body: filebuffer.toString('base64'),
  isBase64Encoded: true,
  headers: { "Content-Type" : "image/png" }
} );`

Does anyone know if this be the approach for the official serverless support, or will we have to work with lambda integration? Could both options be allowed through configuration?

@ajkerr
Copy link

@ajkerr ajkerr commented Jul 5, 2017

@amv
Copy link

@amv amv commented Jul 6, 2017

This is great news!

I have been using manual instructions on how to set the api binary types on my serverless phantomJS screenshot project README file, but I would be more than happy to remove the guides once we get a version of serverless out which properly supports this :)

Ping @brianneisler for the good news and expedited future roadmap inclusion 👍

@pmuens
Copy link
Contributor

@pmuens pmuens commented Jul 6, 2017

Nice! Thanks for posting the update @ajkerr 👍

@amv thanks for your comment! This has been on the roadmap for a long time and has a pretty high priority. Unfortunately it was blocked by the lack of CloudFormation support (until now 🙏).

Anyone here who would like to jump into an implementation / WIP PR? We're more than happy to help out when problems come up!

This way we can get it ready for v1.18 or v1.19.

@rcoh
Copy link

@rcoh rcoh commented Jul 7, 2017

@pmuens I'd like to give this a go tomorrow! First contribution to severless so I'd appreciate being pointed in the right direction.

@pmuens
Copy link
Contributor

@pmuens pmuens commented Jul 7, 2017

@pmuens I'd like to give this a go tomorrow! First contribution to severless so I'd appreciate being pointed in the right direction.

Awesome @rcoh! That's super nice 🎉 🙌 👍

Let me see...

So it looks like support for the BinaryMediaTypes config parameter needs to be added to the AWS::ApiGateway::RestApi resource (according to this documentation).

You can find the code for the compilation of the RestApi here. The rest of the plugin which compiles all of the API Gateway resources can be found here.

Other than that it looks like this documentation describes how everything should work together.

The only thing I haven't found yet is the config for ContentHandling in the CloudFormation resource definition for an IntegrationResponse.

🤔 not sure if it's not added yet or if it's undocumented.

Other than that there's also this forum link which might shed some lights into the way this works in general.

Thanks again for looking into this @rcoh 👍 Let us know if you need anything else! Happy to help you get this into master!

@rcoh
Copy link

@rcoh rcoh commented Jul 17, 2017

Started to work on this. Was hoping to be able to test my changes to serverless by using npm link but when I do that, serverless stops working:

The AWS security token included in the request is invalid / expired.

Unrelated to that, what were you think of for the API from the serverless/end user side of things @pmuens ?

@pmuens
Copy link
Contributor

@pmuens pmuens commented Jul 18, 2017

Started to work on this.

Great @rcoh 🎉 👍 Really looking forward to this feature!

Was hoping to be able to test my changes to serverless by using npm link but when I do that, serverless stops working

😬 That was a bug we recently introduced in master, but it was reverted a few days ago.

Which version of Serverless are you using @rcoh? Can you pull the most recent master? This should fix the issue.

Let us know if you need help with this or anything else!

@rcoh
Copy link

@rcoh rcoh commented Jul 18, 2017

@eraserfusion
Copy link

@eraserfusion eraserfusion commented Apr 9, 2018

@mvayngrib, Can you make a pull request for your change here: mvayngrib/serverless@e796fb5. This fixes an issue that I have had for several days and I think it would benefit the serverless community.

@mvayngrib
Copy link
Contributor

@mvayngrib mvayngrib commented Apr 9, 2018

@eraserfusion np, see #4895, though without tests i doubt it'll be merged any time soon :)

@ronkorving
Copy link
Contributor

@ronkorving ronkorving commented May 18, 2018

When #4895 got merged, it closed this issue. But as that PR describes, it only solves this issue partially. Can it be reopened until binary support is a full first-class citizen? Or did I miss something and is it actually supported out of the box now?

cc @HyperBrain

@HyperBrain
Copy link
Member

@HyperBrain HyperBrain commented May 18, 2018

@ronkorving Thanks for the hint.

@mvayngrib @eraserfusion I'll reopen the issue. Can you elaborate on the merged PR and maybe tell what exactly is needed additionally now to have full support of binary responses?
We might have to adjust the issue subject then.

@HyperBrain HyperBrain reopened this May 18, 2018
@mvayngrib
Copy link
Contributor

@mvayngrib mvayngrib commented May 22, 2018

@HyperBrain see #2797 (comment) , I don't really have anything to add :)

@ronkorving
Copy link
Contributor

@ronkorving ronkorving commented May 22, 2018

@mvayngrib Do you think there's anything against that just being the default setting?

@mvayngrib
Copy link
Contributor

@mvayngrib mvayngrib commented May 22, 2018

@ronkorving not sure i understood, which thing being the default setting?

@ronkorving
Copy link
Contributor

@ronkorving ronkorving commented May 23, 2018

@mvayngrib Is there any reason not to just have serverless configure support for binary by default, without users having to be explicit about it? It doesn't hurt non-binary responses in any way, does it? I'm a total serverless noob, so I may be misunderstanding some of the philosophies at play here completely.

@eahefnawy eahefnawy modified the milestones: 1.39.0, 1.40.0 Feb 20, 2019
@eahefnawy eahefnawy self-assigned this Feb 20, 2019
@dschep dschep modified the milestones: 1.40.0, 1.41.0 Mar 26, 2019
@pmuens pmuens modified the milestones: 1.41.0, 1.42.0 Apr 17, 2019
@pmuens pmuens assigned pmuens and unassigned eahefnawy Apr 29, 2019
@pmuens pmuens mentioned this issue Apr 29, 2019
7 of 7 tasks complete
@pmuens pmuens closed this in #6063 May 8, 2019
@perrin4869
Copy link

@perrin4869 perrin4869 commented May 13, 2019

Just tried this feature. cat .serverless/cloudformation-template-update-stack.json will output the correct stack with the added binary types, however, during deployment, the cloudformation stack does not reflect this change in my case. Any clues how to troubleshoot?

@aprilmintacpineda
Copy link

@aprilmintacpineda aprilmintacpineda commented May 2, 2020

Is there a way for per function binaryMediaTypes?

@jschulenklopper
Copy link

@jschulenklopper jschulenklopper commented May 21, 2020

Is there a way for per function binaryMediaTypes?

Exactly, since one Serverless project might have different functions responding in different formats; text-based formats with MIME type application/json and binary formats such as images. Seting the Binary Media Types on the provider, and not on the individual functions makes that scenario much more difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

You can’t perform that action at this time.