Add api_gateway_response config #1270
Conversation
Not sure if this is the best way to do this. Might want to edit swagger file at a higher level? Issue I had with that was that SwaggerfileGenerator classes don't have access to the config, as far as I can tell anyways.
Add param for api_gateway_responses to genereate_swagger. This can handle adding the same swagger to all of the TemplateGenerator subclasses as needed.
Now it can rely on the SwaggerGenerator changes made in the previous commit.
We'll just make the swagger definition be the definition input in the config.json. I'll write the docs to reflect this. Makes it easier in the code, and easier for users to find examples since AWS had docs on the swagger definitions of API gateway responses.
Codecov Report
@@ Coverage Diff @@
## master #1270 +/- ##
==========================================
+ Coverage 96.03% 96.04% +<.01%
==========================================
Files 28 28
Lines 5275 5281 +6
Branches 672 673 +1
==========================================
+ Hits 5066 5072 +6
Misses 135 135
Partials 74 74
Continue to review full report at Codecov.
|
|
Thanks for the pull request @felipe-lee this features seems like it would be nice to have. To your point about not knowing where to put it, the config or somewhere in the app code. The distinction for us is that the config file is where properties that would typically vary between stages live. Also they typically don't affect the behavior of any AWS resources. Examples being the size of your lambda function, timeout etc. Those don't really affect how the resource behaves, just how it does it. The application code is where we expect most things related to the application itself, this feature in particular changes the default behavior of every route, so I would consider that to be something that is inherent to your application, and not something that would vary between a beta/gamma/prod stage. I made another pr (#1276) that implements a similar feature that you should take a look at. Its similar in that it is an API gateway global property that affects how the swagger file is generated, so the implementations should be somewhat similar. I'd be happy to review the PR for you if you'd like to update it, it might also be a good idea to open a tracking issue where we can discuss what the API should look like. |
|
@stealthycoin ah that's a good point, I hadn't made that distinction before between the config vs app level settings. I can definitely rewrite this to follow your pr example more. I can open a tracking issue if that's preferred. I wasn't sure if #787 was considered a tracking issue, or if there was a more specific format to track new features. |
|
The main two interface items I can think of to discuss are:
What do you think? |
|
Yeah, I'll open a new issue to discuss what the API should look like. I'll include a link to the values that are possible (or the ones that AWS shows as examples). |
|
+1 I use chalice as backend and angular as frontend for my apps. Recently started using chalice authorizer. For unauthorized requests, Chalice authorizer always returns status 0 (CORS error) while expected to get Http 401 or 403 status code. As of now, I can fix the issue by enabling cors with DEFAULT_4xx Gateway Response in AWS API Gateway console and redeploying the apis. Though fix might exists via chalice package, I would like to continue using chalice deploy. Having this api config to customize API Gateway response is a must need for all Angular/React/web apps to receive 4xx (Unauthorized) errors when using chalice authorizer in chalice apps. I tried achieving the same via awscli and boto3 without any success. Like to know when will this feature be available in chalice. |
Issue #, if available:
Mainly addresses #787
Could potentially address #960 if the CORS docs are updated to indicate you need to set
api_gateway_responsesin theconfig.jsonfile (which I'd be happy to do, but not sure if that issue should be addressed separately, or in a different way).Description of changes:
Added a key/value pair mapping to
config.jsonthat extends the swagger docs that are generated for both packaging and deploying. This mapping is specifically to enable customization of API Gateway responses such as setting the "Access-Control-Allow-Origin" header for Default 4XX/5XX responses, which is needed for CORS to work properly.I'm not sure if this is the best way to do this based on how the code is set up. I went through a few iterations (hence the many commits), before settling on modifying the swagger doc since that's the common piece for the deploy command as well as both types of packaging (CFN/Terraform). Let me know if you'd rather this be implemented in a different way.
I ran all the commands mentioned in the contributing guide to check that everything still works ok. I did get a few errors in pylint, but I get those even just on the master branch so I assume they already exist or might be because of how some things are set up on my computer (e.g. watchdog import error):
I also get two test errors, but the same with these, I get them even on the master branch, so again I think it's based on my local setup. I'm hoping that when the CI gets triggered by this PR I can see for sure if there are errors.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.