jhthorsen / mojolicious-plugin-openapi

OpenAPI / Swagger plugin for Mojolicious
54 stars 44 forks source link

M:P:OpenAPI must not move routes under OpenAPI basePath #116

Closed augensalat closed 5 years ago

augensalat commented 5 years ago

Moving application routes under /basePath (v2) / /servers/0/url (v3) is an error. This information in the OpenAPI spec file is for the client only.

Assume your Mojo OpenAPI application has a /foo resource. In your OpenAPI3 spec.yaml /servers/0/url is /. "GET /foo" works when you run your application alone.

Now you deploy the API application. In production the API is mounted under /api. Example config for Apache/FastCGI:

ScriptAlias /api /var/www/my_app/script/my_app.fcgi
<Location /api>
    SetHandler fcgid-script
    Options ExecCGI
    Require all granted
</Location>

Of course in your OpenAPI3 spec.yaml /servers/0/url has to be /api now. M:P:OpenAPI parses the OpenAPI3 yaml file, finds /servers/0/url(1) and moves all API-routes under /api:

script/my_app routes
/api                                     *        api
  +/                                     GET
  +/foo                                  GET      "get_foo"

So the API application would expect the request under "GET /api/foo".

Good? NOOO!

Because Mojo::Message::Request does right the opposite. To let the API application be deployment-agnostic Mojo::Message::Request removes the SCRIPT_NAME(2) ("/api") from the REQUEST_URI(2) ("/api/foo"), hence what the application router actually sees is GET /foo.

This is neither an Apache nor a proxy issue!

Prove: Mojolicious::Plugin::Mount Placing these two files in my API application project:

# lib/Mcp.pm
package Mcp;
use Mojo::Base 'Mojolicious';
use FindBin qw($Bin);

sub startup { shift->plugin(Mount => {'/prefix' => "$Bin/my_app"}) }

1;
# script/mcp
#!/usr/bin/env perl

use FindBin;
BEGIN { unshift @INC, "$FindBin::Bin/../lib" }
use Mojolicious::Commands;

# Start command line interface for application
Mojolicious::Commands->start_app('Mcp');

And since we mount our API under /prefix we also have to adjust the OpenAPI spec:

servers:
  - url: /prefix

curl http://localhost:3000/prefix/foo will give you an error! (3) You have to curl http://localhost:3000/prefix/prefix/foo for the foo resource which is obiously wrong.

My understanding is that the problem is in Mojolicious::Plugin::OpenAPI::_build_route https://github.com/jhthorsen/mojolicious-plugin-openapi/blob/master/lib/Mojolicious/Plugin/OpenAPI.pm#L158 The plugin must not move the application routes under the $base_path.

WDYT?

(1) This implementation is limited/incomplete of course (2) SCRIPT_NAME and REQUEST_URI are CGI/PSGI environment variables, and also handled by Mojolicious. (3) For OpenAPI3 the error is wrong ("No responses rules defined for..."), but that's another (yet unreported) issue.

plk commented 5 years ago

This may be related to what I am seeing in a v2->v3 migration. I get a lot of "No responses rules defined for..." errors ...

augensalat commented 5 years ago

In the meantime I learned that I can just leave the basePath at "/" and everything works even if the application is mounted under (e.g.) "/api" - as long as we we are talking about OpenAPI2. The Specrenderer converts the basePath with url_for(). This is unexpected and afaics undocumented behaviour, but OTOH convenient, since it is not needed to change (the basePath in) the OpenAPI specification when deploying the application und some path. So I rest my case and close this issue, because it turns out to be a lack of both, documentation and OpenAPI3 support.

jhthorsen commented 5 years ago

@augensalat: Your fix is not technically wrong, but it would be quite confusing from a developer that is used to working with a reverse proxy that is configured correctly.

See also https://github.com/jhthorsen/mojolicious-plugin-openapi/issues/114#issuecomment-472341835