feat(event_handler): add support to download OpenAPI spec file

[Thomas-McKanna]

Issue number: #3560

Summary

This feature allows for setting the enable_download_spec in the enable_swagger function so that the OpenAPI specification file can be download for use generating client libraries, hosted documentation external, or for another reason.

Usage would look like:

app.enable_swagger(path="/_swagger", enable_download_spec=True)

As I was working on this task, I came to think that it might be better to just expose the JSON endpoint by default rather
than require the user to opt-in. I don't see how adding this feature as default would break existing deployments, and would cut down the number of steps for a user to use the feature from two steps (adding enable_download_spec parameter and adding endpoint in SAM template) to one step (adding endpoint in SAM template). What are thoughts on this?

Changes

• A new conditional endpoint for downloading the JSON was created.
• Creation of the specification JSON was moved into a separate function to avoid duplicate code.
• Documentation was updated to reflect new feature.

Update - 09/01/2024

We decided to drop the enable_download_spec=True flag. We added a querystring parameter (?format=json) along the /swagger route.

User experience

Additional flexibility of the OpenAPI feature.

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change? No.

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e34f719) 95.78% compared to head (cb4d0e8) 95.78%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3571   +/-   ##
========================================
  Coverage    95.78%   95.78%           
========================================
  Files          210      210           
  Lines         9686     9688    +2     
  Branches      1774     1775    +1     
========================================
+ Hits          9278     9280    +2     
  Misses         294      294           
  Partials       114      114           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leandrodamascena]

As I was working on this task, I came to think that it might be better to just expose the JSON endpoint by default rather than require the user to opt-in. I don't see how adding this feature as default would break existing deployments, and would cut down the number of steps for a user to use the feature from two steps (adding enable_download_spec parameter and adding endpoint in SAM template) to one step (adding endpoint in SAM template). What are thoughts on this?

Hi @Thomas-McKanna! I'm unsure if we would set this parameter to True by default. I'm thinking of cases where customers are already using this feature, just explicitly exposing /swagger, /swagger.js, and /swagger.css instead of using {proxy+} and then Swagger won't work because the path /swagger.json is missing/not defined.

We can keep it as False by default and hear from others before merging.

[leandrodamascena]

Hi @Thomas-McKanna! Thanks a lot for sending this PR, I left some comments.

We need to create some additional tests to check if we propagate/use this new parameter. Can you add this, please?

[Thomas-McKanna]

@leandrodamascena I added a couple of tests to ensure this feature is working with the default path (/swagger) and a custom path.

[leandrodamascena]

Hi @Thomas-McKanna! Comment here instead of the initial conversation because it is more visible to everyone.

I had a conversation with @rubenfonseca - who created the OpenAPI feature in Powertools - and the main reason we inject OpenAPI spec into the configuration (using spec field) instead of requesting a URL is because it's faster and more cost-effective. So, we can keep the field spec in the SwaggerUI configuration, but we can add a new line to update the URL and enable the link to download:

<script>
  var swaggerUIOptions = {{
    dom_id: "#swagger-ui",
	.
	.
    spec: JSON.parse(`
                {spec}
            `.trim()),
	.
	.    
  }}

  var ui = SwaggerUIBundle(swaggerUIOptions)
  ui.specActions.updateUrl('{URL}/{PATH}.json'); // NEW LINE
</script>

We agree that enabling it with a conditional query string is a good approach and we can remove the enable_download_spec parameter. I tested it with a real deployment in API Gateway and it works perfectly. Please go ahead with this implementation.

Thanks for addressing all the feedback and participating in this discussion, Thomas! We always try to find the best experience for our customers and avoid breaking anything.

[Thomas-McKanna]

Sounds good @leandrodamascena, I will get these changes committed soon!

[Thomas-McKanna]

@leandrodamascena I have implemented the requested changes. Now the OpenAPI spec JSON can be fetched by setting the ?format=json query string on the documentation path. No more need for enabled_download_spec and no additional paths need to be created in the SAM template - it should just work for everyone!

I also removed the documentation section I had added.

Let me know if you'd like me to change anything.

[leandrodamascena]

@leandrodamascena I have implemented the requested changes. Now the OpenAPI spec JSON can be fetched by setting the ?format=json query string on the documentation path. No more need for enabled_download_spec and no additional paths need to be created in the SAM template - it should just work for everyone!

I also removed the documentation section I had added.

Let me know if you'd like me to change anything.

Hey @Thomas-McKanna! Thanks for addressing the feedback! I'll review this tomorrow and I expect to merge this before our next release!

[sonarcloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[leandrodamascena]

@Thomas-McKanna I resolved a conflict we had due to a PR we merged today and made a small change, but nothing that changes your logic.

@rubenfonseca please review this PR.

[boring-cyborg]

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!