Connexion: Finalize 2.0 Release

[dtkav]

Description

We created the dev-2.0 branch to introduce a handful of major breaking changes to connexion.
https://github.com/zalando/connexion/milestone/10

At the time, the two major changes were:

• #420 openapi 3 support
• #406 removing vendored swagger-ui

Both of these changes are now in dev-2.0

Next Steps

What other things should be in the 2.0 release? Is it large enough as is? What's could be better?
It would be great to finalize the feature set for Connexion 2.0 and get it shipped! :ship:

Let me know what you think!

[dtkav]

The PR for merging dev-2.0 -> master is #619

[ioggstream]

I successfully run this development connexion with a non-trivial yaml. Good work!

I now have some issues with

connexion/jsonref.py:from jsonschema import RefResolver

because it can't parse yaml schema files like https://opensource.zalando.com/problem/schema.yaml

I workaround'd it in the jsonschema module

+++ b/jsonschema/validators.py
@@ -2,6 +2,7 @@ from __future__ import division

 import contextlib
 import json
+import yaml
 import numbers

 try:
@@ -466,12 +467,15 @@ class RefResolver(object):
             # Requests has support for detecting the correct encoding of
             # json over http
             if callable(requests.Response.json):
-                result = requests.get(uri).json()
-            else:
-                result = requests.get(uri).json
+                try:
+                    result = requests.get(uri).json()
+                except ValueError:
+                    result = None
+            if not result:
+               result = yaml.load(requests.get(uri).content)
         else:
             # Otherwise, pass off to urllib and assume utf-8
-            result = json.loads(urlopen(uri).read().decode("utf-8"))
+           result = yaml.load(urlopen(uri).read().decode("utf-8"))

         if self.cache_remote:

But on https://github.com/Julian/jsonschema/issues/420 it seems yaml is currently out of scope.

Can we fix it in connexion subclassing RefResolver?

[hjacobs]

@dtkav thanks for the great work!

[dtkav]

Thanks for the bug report @ioggstream ! I'll follow that up in a separate ticket. I'd like to keep this one a bit more of a high-level discussion on what features / breaking changes should make it into Connexion 2.0. (I've hidden your comment, now that its' in #681 - hope you don't mind!)

@hjacobs and @jmcs , I would love your thoughts especially on high-level next steps!

Thanks to everyone in the community who helped make these features possible :heart:

[hjacobs]

@dtkav I think the 2.0 release should not be bigger as the OpenAPI 3 support is already a monster change. Let's get some feedback from people trying it out and then releasing it with any additional blocker bug fixes we encounter.

@jmcs WDYT? Do we know who could/would try it out and give feedback?

[dtkav]

@hjacobs - I tend to agree. The only other thing I can think might be useful (and a breaking change) is to validate requests/responses depending on mimetype, and not only if the endpoint is purely json (i.e. #593).
Everything else I can think of can be incremental improvement.

[cziebuhr]

I'm a quite new user of connexion, but I really like the project.
OpenAPI 3 uses JSON Schema Specification Wright Draft 00 (= Draft 5), which is mostly compatible to JSON-Schema Draft 4.
But it points out that "$ref" should only be used wherever a schema is expected. This allows "$ref" to be a valid key for the "properties" keyword. The new resolve_refs function breaks that feature.
Future versions of OpenAPI may use Draft 6 or later, which introduce more incompabilities, e.g. exclusiveMinimum/exclusiveMaximum being a number instead of a boolean.
I therefore want to discuss if it makes sense to have a validator_map per api (add parameter to add_api) instead of having it globally.

In our setup, we already have some extensions to Draft4Validator, regarding x-nullable and readOnly (the first one with a different implementation than here). I will open seperate issues/PRs for discussion about these, but that shouldn't block the 2.0 release.

[ioggstream]

@dtkav I'm running the specs against some files. I'll file separate issues like you did with the yaml stuff, ok?

PS: Thank You for the flash fix!

[cziebuhr]

Can you have a look at https://github.com/zalando/connexion/compare/dev-2.0...cziebuhr:nullable ? According to OpenAPI 3, nullable "allows sending a null value for the defined schema", so it should also work outside of properties.

[cziebuhr]

Depending on using swagger 2 / openapi 3, ConnexionOptions sets swagger_ui_local_path to swagger_ui_2_path / swagger_ui_3_path. IMHO this is only the major version of swagger-ui, swagger-ui 3 supports both swagger 2 and openapi 3.

[dtkav]

Can you have a look at dev-2.0...cziebuhr:nullable ? According to OpenAPI 3, nullable "allows sending a null value for the defined schema", so it should also work outside of properties.

@cziebuhr please make a pull request! Those changes look reasonable to me.

Depending on using swagger 2 / openapi 3, ConnexionOptions sets swagger_ui_local_path to swagger_ui_2_path / swagger_ui_3_path. IMHO this is only the major version of swagger-ui, swagger-ui 3 supports both swagger 2 and openapi 3.

I couldn't quite tell from your comment - are you suggesting we always serve swagger-ui-3? I'm open to that. I am aware that swagger-ui-3 is backwards compatible, but I purposefully tried to avoid changing too much about the Swagger2 development experience when adding OpenAPI3 support. Since the path is easily override-able, my thinking was to not force a major-version bump by default.

I'm totally open to feedback, and happy to make the change based on what the community thinks is best.

[cziebuhr]

PR for nullable is at https://github.com/zalando/connexion/pull/684.
I changed it a bit, so I don't need to overwrite iter_errors.

And I have another suggestion, regarding changes in security handling: https://github.com/zalando/connexion/compare/dev-2.0...cziebuhr:security
It addresses some issues I had when trying to use connexion for a production-ready product.

• Use token_info['sub'] instead of token_info['uid'] to better align with RFC7662 (oauth token introspection)
• x-tokenInfoUrl is only kept for compability reasons. IMHO it's only useful for a small amount of people, because it hardcodes bearer authentication and the response format is not standardized. I can add an example later on how to use RFC7662 with x-tokenInfoFunc.
• validate_token_info was not customizable and simply compared two sets of strings.
  In our setup, we want to add more logic, e.g. scope 'ressource' is superior to 'ressource:read'. One can now set a custom scope validator.
• If security definitons don't match the connexion implementation (only one definition supported), connexion silently ignors ALL definitions and allows full access to the api
• Basic authentication had to be done manully as function decorator instead of using the security definitions from the swagger definition. Now one can define callbacks similar to x-tokenInfoFunc.
  Bonus: If both, oauth and basic auth is specified for an operation, the new x-basicInfoFunc also receives the required oauth scopes to use them for authorization.
• Also added support for apiKey for completeness, I don't use it myself

Sorry for "misusing" the dev-2.0 issue here, but I think it would be huge benefit for all connexion users and would fit into a major version bump.
It's not yet fully tested and documentation/examples are not updated yet, but I would like to get some general feedback on that approach.

[dtkav]

Thank @cziebuhr - I'll try to take a look at #684 tonight.
Can you make a pull request for your other proposed changes as well? That's the best way to get specific feedback. We'll likely need some more in-depth review from Zalando folks because your proposal changes the security flow.

[cziebuhr]

Opened https://github.com/zalando/connexion/pull/686 for the security rework

[cbornet]

Speaking in the name of OpenAPI-generator, a community fork of swagger-codegen, we are very eager to see the v2 of connexion. Our current flask-connexion generator is broken because we can only output a v3 version of the spec in the templates (https://github.com/OpenAPITools/openapi-generator/issues/323).
So :100: to release as soon as possible. Do you have an ETA for the release or for a RC ?

[Lawouach]

It's a big plus one for us as well! Looking forward to it :)

[dtkav]

A bit of an update:
PR's merged:

1. #681 allows references to yaml files (thanks @ioggstream )
2. #683 adds x-openapi-router-controller for OpenAPI-generator compatibility. (thanks @wheelsandmetal)
3. #684 added (proper) support for jsonschema nullable, readOnly, and writeOnly (great work @cziebuhr )
4. #686 changes the security flow, so probably needs proper review from someone at zalando (Thanks @cziebuhr @jmcs and @hjacobs)
5. #698 setting base_url does not update the spec that is served to swagger-ui (thanks @czchen @juxhindb @jmcs)

Issues on hold:

1. #691 oneOf bug (thanks @czchen)
   I don't think this should block the release (what do you think?), because:
   
   
   
   1. The bug is pretty well understood now.
   
   
   2. It will take some time to fix, but won't be a breaking change.
   
   

Thanks to everyone who has been testing the dev-2.0 branch and reporting issues.
Everyone please test the dev-2.0 branch if you can!

It's easy to install it with:

pip install git+https://github.com/zalando/[email protected]#egg=connexion[swagger-ui]

[jmcs]

I released version 2.0.0rc1 this can be installed with pip --pre ... (or pipenv install --pre ...).

[dtkav]

FYI: Due to #707 if you want to install RC1 with pip install --pre, you'll need to prevent jsonschema from installing their alpha release as well:
pip install --pre connexion[swagger-ui] 'jsonschema<3.0.0'

[jmcs]

I released version 2.0.0rc2 with the jsonschema version fix.

[jmcs]

I released version 2.0.0rc3 with https://github.com/zalando/connexion/pull/712

[lsorber]

Feature request: it would be great to have JWT support in v2.0, which seems to have been asked many times before (#607, #389, #124) and is part of the OpenAPI 3 spec.

[dtkav]

hey @lsorber , feel free to make a pull request! I'm happy to review it.

We've already closed the merge window on 2.0, and I think we should get it out without any more changes (unless they are bugfixes). JWT support should be backwards compatible though right? No reason why we can't merge it in for 2.1 or some other minor release.

[dtkav]

FYI @lsorber , @krise3k put up this great PR (#732) for JWT support.

[dtkav]

@jmcs has released 2.0.0rc5 :tada:
pip install --pre connexion[swagger-ui] to install it.

[JuxhinDB]

Big well done guys--sorry I haven't been active but I'm definitely keeping an eye. @dtkav Would you consider rc5 to be somewhat stable? I can start using it soon if so

[dtkav]

@JuxhinDB yes, I think it's fairly stable. Please bang on it and see if you find any issues. I've recommended that we push RC5 as the 2.0 release (and put JWT support in 2.1, which could easily be soon after).

[hjacobs]

2.0.0 was released :tada: https://github.com/zalando/connexion/releases/tag/2.0.0

Thanks to all contributors!