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

Servers are not showing up in openapi 3.0.1 #1139

Open
caffeinatedMike opened this issue Jan 25, 2020 · 9 comments
Open

Servers are not showing up in openapi 3.0.1 #1139

caffeinatedMike opened this issue Jan 25, 2020 · 9 comments

Comments

@caffeinatedMike
Copy link

Description

I've defined three servers with urls and descriptions to have easy access during all phases of development. However, the servers are not showing up when viewing the swagger ui served by connexion. If I use the same yaml below on https://editor.swagger.io/ the Servers dropdown is correctly populated.

Expected behaviour

The servers should show up in the Servers dropdown.

Actual behaviour

The Servers dropdown is empty.

Steps to reproduce

  1. Use the following app settings
import connexion
from connexion.resolver import MethodViewResolver

app = connexion.FlaskApp(__name__, specification_dir='static/')
options = {"swagger_url": "/"}
app.add_api('swagger.yaml',
                options=options,
                resolver=MethodViewResolver('entities'), strict_validation=True)
app.run(debug=True)
  1. Create /entities folder with brand.py and __init__.py

brand.py

from flask_restful import Resource
from flask import request

class BrandView(Resource):
    # Get an existing brand
    def get(self, id):
        return '', 200

init.py

from .brand import BrandView
  1. Use the following code in /static/swagger.yaml
openapi: 3.0.1
info:
  title: POC API
  description: 'Proof-of-Concept'
  termsOfService: https://url.com/terms/
  contact:
    email: mhill@url.com
  license:
    name: Apache 2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
  version: 1.0.0
externalDocs:
  description: Find out more about Swagger
  url: http://swagger.io
servers:
- url: http://127.0.0.1:5000/
  description: Dev Server
- url: http://...QA.server.../...api/
  description: Staging Server
- url: https://api.livesiteurl.com/
  description: Live Server
tags:
- name: Brands
  description: Operations to manage brand info and availability
paths:
  /brand:
    get:
      tags:
      - Brands
      summary: Get all available brands
      responses:
        200:
          description: Brands retrieved
          content:
            application/json:
              schema:
                type: array
                items:
                  $ref: '#/components/schemas/Brand'
        401:
          description: Invalid token supplied
          content: {}
components:
  schemas:
    Brand:
      type: object
      required:
      - name
      - status
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
        status:
          type: string
          description: Brand Status
          enum:
          - active
          - disabled
      xml:
        name: Brand

Additional info:

Output of the commands:

  • python --version => Python 3.7.6
  • pip show connexion | grep "^Version\:" => Version: 2.5.1
@dtkav
Copy link
Collaborator

dtkav commented Jan 25, 2020

Hey @caffeinatedMike - In order to support path-altering reverse proxies (and multiple servers with various paths), there is code that uses url_for (in Flask) to look up the current host and path, and then serves up a spec with a computed base path that points back to the API. This has the side effect of overwriting the servers list. On the plus side it should always point to your API.

You can find the relevant code here:
https://github.com/zalando/connexion/blob/master/connexion/apis/flask_api.py#L304-L310
https://github.com/zalando/connexion/blob/master/connexion/spec.py#L156

Connexion also uses the spec to define the base path for the API, so if we have multiple servers in the servers list (with different paths), it's not clear which one the connexion instance should be. At the moment it assumes the first server in the list, but can answer to multiple hosts or prefixes if headers are set by a reverse proxy.

I'd love to support your use-case also, unfortunately I haven't figured out a good way to do so without breaking reverse-proxy support.

Please let me know if you have any ideas. While the OpenAPI spec is very clear about the servers block for clients, it's unclear how to implement it correctly on the server side. I've tried to get clarity on the #swagger IRC multiple times, but it's quite a difficult problem to grasp and communicate.

@dtkav
Copy link
Collaborator

dtkav commented Jan 25, 2020

You should be able to override the behavior like this

class MyInternalHandlers(InternalHandlers):
    def _spec_for_prefix(self):
        return self.specification.raw

class MyFlaskApi(FlaskApi):
    @property
    def _handlers(self):
        # type: () -> InternalHandlers
        if not hasattr(self, '_internal_handlers'):
            self._internal_handlers = MyInternalHandlers(self.base_path, self.options, self.specification)
        return self._internal_handlers

@caffeinatedMike
Copy link
Author

@dtkav thanks for the quick and verbose response. It was actually much more complicated of a response than I had expected to be honest (not a bad thing!). I think I might be able to hopefully offer some help in accomplishing this by pointing you towards the flasgger Flask plugin that I've used to accomplish this in past. At least I didn't seem to have any problem with the multiple servers. The only test I can remember doing was testing the QA server baseurl from my local (127.0.0.0) instance and it worked. Let me know if this helps at all.

@caffeinatedMike
Copy link
Author

@dtkav I was thinking, I'm not sure using url_for is the best option here because it doesn't make sense in the event more than a single server is provided. Shouldn't it be assumed that the server paths provided are all absolute and not relative paths? That's how the servers are meant function. I get why you tried to make it possible to accommodate relative paths, but I think that goes against the original spec.

@dtkav
Copy link
Collaborator

dtkav commented Jan 26, 2020

hey @caffeinatedMike -
Here are some totally reasonable example of the servers block that need to be handled.

- /
- /api/v0/
- https://www.mysite.tld/
- http://www.mysite.tld/
- /asdf/
- /asdf/1234/

Unfortunately we can't treat them as absolute paths, because any of these servers could be hosted behind a path-altering reverse proxy (with a basepath like /api/gateway/). See #823

For each of these servers, we need to serve up a templated swagger-ui that is correctly configured to hit the API.

Hopefully this sheds some light on why it's necessary to inspect the requests as they come in, and to use url_for to adjust the path.

@caffeinatedMike
Copy link
Author

@dtkav that does make sense. However, I think adding some additional logic to only utilize url_for in the case of an incomplete/relative path would be an easy fix that would service both use-cases. So, for your examples, the /api/v0, /asdf/, and /asdf/1234/ servers would prompt the usage of url_for, while the https://www.mysite.tld and http://www.mysite.tld servers, being absolute paths, would avoid the usage of url_for.

@ktarplee
Copy link

ktarplee commented Apr 1, 2020

Why would these not be considered valid urls in the servers block:

- .
- ../
- ../api

See my comment here. The OpenAPI spec allows relative paths. They are meant to be relative to where the openapi.json file is served. Handling relative paths properly solves all of these issues (at least in my mind).

@Halkcyon
Copy link

Halkcyon commented Aug 22, 2020

@dtkav

In order to support path-altering reverse proxies (and multiple servers with various paths), there is code that uses url_for (in Flask) to look up the current host and path, and then serves up a spec with a computed base path that points back to the API. This has the side effect of overwriting the servers list.

Is this a hard requirement? In my experience, I use a relative url as supported by the spec so I can run requests against the current host and then have an enumeration that can split my hosts by environment or something of that nature (which also doesn't work with the swagger-ui extras package).

Example:

servers:
  - url: /v1
  - url: https://load-balanced-name{env}.domain.com/v1
    variables:
      env:
        default: ''
        enum:
          - ''
          - .staging
          - .development

Maybe have enforcement by the package to enforce basePath to be equal across all url variants if that's required?

@vemonet
Copy link

vemonet commented Dec 11, 2020

We are using https://github.com/nginx-proxy/nginx-proxy to serve our OpenAPI 3.0.1 YAML with Connexion

But Connexion override the server path and server a openapi.json with a empty list of server, preventing to have any servers shown in the served API. And contradicting the Connexion documentation to define servers: https://connexion.readthedocs.io/en/latest/routing.html#api-versioning-and-basepath

When using connexion I usually expect the behavior I defined in the openapi.yml as ground truth for the final API rendering

Would it make sense for Connexion to not override the servers knowingly defined by the user in the openapi.yml file? And providing an argument to allowing override?

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

No branches or pull requests

5 participants