feat(python-flask): add opt-in Connexion 3 support#24181
Conversation
Adds a new `useConnexion3` boolean generator option (default: false) to the python-flask server generator, addressing OpenAPITools#17303. Connexion 3 has been out since 2023, but requirements.mustache explicitly pinned `connexion<=2.14.2` and `Flask==2.1.1` to avoid it, blocking users from picking up newer Flask/Werkzeug (one comment on the issue specifically cited this as blocking a CVE fix in werkzeug). The maintainer has repeatedly invited a contribution on the thread since Dec 2023, and several community members had already prototyped working fixes in the comments. Kept as an opt-in flag rather than a default bump, following this repo's existing convention for breaking generator-output changes (useJackson3, useSpringBoot3/4). What changes under the flag, and why: - requirements.mustache / setup.mustache: swap the Connexion 2/Flask 2.1.1 pins for `connexion[flask,swagger-ui,uvicorn]>=3.3.0,<4.0.0` + `Flask>=2.2.0,<4.0.0`. The uvicorn extra is required because Connexion 3's `FlaskApp.run()` launches via uvicorn even for Flask apps -- confirmed by actually running the generated server, which fails at startup without it. The connexion floor is 3.3.0 (not just the first 3.0.0 release) because that's the only version we've actually run and verified, and it's also the first release with official Python 3.13/3.14 support per Connexion's own release notes. swagger-ui-bundle is bumped to >=1.1.0 to match the floor Connexion's own swagger-ui extra already silently requires. - __main__.mustache: `connexion.App` -> `connexion.FlaskApp`, and the JSON encoder moves from a `Flask.json_encoder` attribute assignment (removed in v3) to a `Jsonifier(cls=...)` passed into `add_api()`. - encoder.mustache: the generated `JSONEncoder` becomes a plain `json.JSONEncoder` subclass instead of extending Connexion 2's `FlaskJSONEncoder` (removed in v3); the `default()` body handling `Model.to_dict()` conversion is unchanged. Verified end-to-end (not just unit-level): patched a controller to return a real nested Pet/Category model instance, ran the actual generated server (uvicorn + Flask + Connexion 3) in Docker, and curled it -- got back correctly serialized JSON with attribute_map key translation intact (photo_urls -> photoUrls). - __init__test.mustache: Connexion 3's `app.test_client()` returns an httpx/Starlette-based client, not Flask's WSGI test client -- so a small `_FlaskStyleTestClient`/`_FlaskStyleResponse` adapter is added to keep the existing `self.client.open(...)`/`self.assert200(...)` calling convention in controller_test.mustache working unchanged. Also drops `flask_testing.TestCase`, which is unmaintained since 2020 and not Flask-3-compatible. - test-requirements.mustache: drops the `Flask-Testing` pin under the flag, since Flask's own test client no longer needs it. - CORS support (featureCORS): `flask_cors.CORS(app.app)` does not work under Connexion 3, because Connexion 3 wraps the Flask app in its own ASGI middleware stack and can route/short-circuit requests before they ever reach the inner WSGI app flask-cors is watching. Confirmed this was actually broken (zero Access-Control-* headers on a real request with an Origin header) before fixing it. Replaced with Connexion 3's own documented pattern -- `app.add_middleware(CORSMiddleware, ...)` from `starlette.middleware.cors`, which comes for free as a connexion dependency, no separate package needed. flask-cors itself is no longer installed at all under useConnexion3. Reverified afterwards: Access-Control-Allow-Origin and Access-Control-Allow-Credentials both present on the response. One unconditional (non-flag-gated) fix: controller.mustache swaps `connexion.request.is_json`/`.get_json()` for Flask's own `from flask import request`. Connexion 3's `connexion.request` is now a Starlette Request and no longer exposes those Flask-specific methods. Flask's own `request` object behaves identically under Connexion 2 and 3, so this is applied to both, and is a dependency-reduction as a side effect. This is the only part of the diff that touches the existing default sample output. Also fixes OpenAPITools#15062 (python-flask docs never explained *why* the generator uses Connexion instead of vanilla Flask) via a `getHelp()` override scoped to the Flask subclass, bundled in here since it's a one-line addition to a PR already touching this generator's docs. OpenAPITools#21294 (operationId double-underscore breaking Connexion's resolver) is a different root cause and is intentionally left for its own PR. Security motivation, checked concretely rather than assumed: ran pip-audit against the full resolved dependency tree for both paths. v2 (current default, Flask==2.1.1/Werkzeug==2.2.3 as resolved): 10 real CVEs across Flask+Werkzeug, including the exact one raised in the issue thread (CVE-2024-34069) plus several more recent ones. v3 (useConnexion3, Flask==3.1.3/Werkzeug==3.1.8 as resolved): zero vulnerabilities in any actual application dependency (the only pip-audit hits in either scan are in pip/wheel themselves -- base image tooling, not part of the app's declared dependencies, identical in both scans). Also surfaced, but explicitly NOT fixed here (separate, pre-existing, unrelated to Connexion version -- reproduces even with `useConnexion3: false`): generating python-flask through a config file that has any `additionalProperties` block changes `postProcessOperationsWithModels`'s `*/*`-consumes skip-marking behavior for a couple of operations. Worth its own issue/investigation, out of scope for this PR. CI coverage: added bin/configs/python-flask-connexion3.yaml alongside the existing bin/configs/python-flask.yaml, following the same pattern used for other flag-gated variants (e.g. the *-jackson3.yaml configs), so the "Samples up-to-date" job continuously verifies the new flag's generated output. Verification performed locally (this environment has no JDK/toolchain installed, so all of the below ran inside Docker containers rather than bare-metal): - `mvn -pl modules/openapi-generator -am package` -- compiles clean. - The full repo's CI "Unit tests" job command, run verbatim (`mvn clean --no-snapshot-updates --batch-mode --quiet --fail-at-end test`) across the whole reactor: 4493 tests run, 0 failures, 0 errors, 7 skipped (confirmed via real surefire report files, not just the process exit code). - Full `bin/generate-samples.sh` (all ~766 generators, no args) run twice in a row, mirroring the "Samples up-to-date" CI job exactly: zero diff outside the files intentionally touched by this change. - Docs regenerated via `bin/utils/export_generator.sh python-flask` (not the full "Docs up-to-date" job across every generator, since this change only touches python-flask's own CliOptions/getHelp()). - Built and ran the generated `useConnexion3: true` sample's own Dockerfile; server starts, serves `/v2/openapi.json`, returns a correct 401 on an auth-protected route without credentials, and correctly serializes a real returned model object end-to-end. - Established a genuine Connexion-2 baseline in a separate Python-3.11 container (the generated Dockerfile's `python:3-alpine` base floats to Python 3.14, which breaks even the *unmodified* v2 sample for unrelated reasons -- old Werkzeug's routing code hits a removed `ast.Str` API) to get a fair v2-vs-v3 comparison of the generated test suite. Both pass the same 8 real operations; the v3 run's extra failures are either pre-existing test-fixture/spec bugs unrelated to Connexion version (confirmed present on the v2 baseline too), or a documented Connexion 3 behavior change where operations with multiple response content types require the handler to specify which one to return -- inherent to Connexion 3's stricter response handling for auto-generated placeholder stubs, not something template-level codegen changes can paper over. Known gap, not fixed here: no CI job actually installs/runs the generated python-flask server (the existing samples-python-server.yaml workflow only covers python-aiohttp-srclayout) or the full "Docs up-to-date" job across every generator, so ongoing regression coverage for this flag's *runtime* behavior relies on the manual verification above, not on CI.
There was a problem hiding this comment.
12 issues found across 55 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/server/petstore/python-flask-connexion3/openapi_server/models/base_model.py">
<violation number="1" location="samples/server/petstore/python-flask-connexion3/openapi_server/models/base_model.py:64">
P2: The `__eq__` method in `base_model.mustache` directly accesses `other.__dict__`, which will raise `AttributeError` when comparing a `Model` instance to primitives or any object that lacks `__dict__` (e.g., `myModel == 42`). It also treats objects of different classes with matching `__dict__` contents as equal. Adding an `isinstance` guard and returning `NotImplemented` for unsupported types matches Python equality semantics and prevents unexpected runtime crashes in generated server code.</violation>
</file>
<file name="samples/server/petstore/python-flask-connexion3/git_push.sh">
<violation number="1" location="samples/server/petstore/python-flask-connexion3/git_push.sh:57">
P2: Piping `git push` output into `grep -v` masks the actual push exit status because the pipeline's final exit code is determined by `grep`, not `git push`. This can hide push failures (if grep exits 0) or incorrectly report failure (if grep finds nothing to output). Consider capturing the push output first, filtering it, then preserving the original exit code with a small adapter pattern.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The automated cubic review on PR OpenAPITools#24181 found 12 issues. Checked each individually against the actual code rather than accepting blindly. 9 were real and are fixed here; 3 are pre-existing bugs in files this PR never touches, left out of scope (noted below). Fixed: - PythonFlaskConnexionServerCodegen.java: guard against a NullPointerException if `useConnexion3` is explicitly set to a null value in additionalProperties -- use String.valueOf(...) instead of calling .toString() directly on a possibly-null Object, matching how the parent class already handles its other boolean options (FEATURE_CORS, USE_NOSE). - __main__.mustache: the Connexion 3 CORS middleware combined allow_origins=["*"] with allow_credentials=True. Per the CORS spec this is unsafe -- wildcard origin plus credentials lets any site make authenticated cross-origin requests on a user's behalf, and newer Starlette releases may reject the combination outright at startup. flask-cors's own default (supports_credentials=False) never allowed this. Dropped allow_credentials to restore parity with the old default. Reverified: CORS still works (Access-Control-Allow-Origin: * on a real request), now without the credentials risk. - controller.mustache: `from flask import request` shadows Flask's request proxy if a spec has an operation parameter literally named `request` -- the local parameter would hide the module-level import inside that function body. Switched to `import flask` + `flask.request.*`, matching the safety Connexion 2's `connexion.request` module-qualified access already had. - encoder.mustache (the serious one): the Connexion 3 encoder's fallback to plain `json.JSONEncoder` lost Flask's built-in handling of datetime/date/ Decimal/UUID values. Generated models routinely hold raw `datetime` objects for `date-time`-format fields (e.g. Order.ship_date in the Petstore spec itself), and stdlib json has zero support for them -- `TypeError: Object of type datetime is not JSON serializable` on any response containing one. Restored the same handling Flask's own DefaultJSONProvider._default provides: `werkzeug.http.http_date()` for date/datetime (RFC 2822, matching Flask's actual format -- not ISO 8601, to keep serialization output identical between the v2 and v3 paths, not just non-crashing), and str() for Decimal/UUID. Verified directly: built an Order with a real datetime ship_date and confirmed it now serializes instead of raising. - PythonFlaskConnexionServerCodegen.java / docs/generators/python-flask.md: the useConnexion3 option's own description said it changes "pinned connexion/Flask/Flask-Testing dependency versions" -- inaccurate, Flask-Testing is removed outright under the flag, not re-pinned to a new version. Reworded and regenerated the doc. - travis.mustache: the generated .travis.yml listed Python 3.2-3.8, which predates Connexion 3's own minimum (Python >=3.9). `pip install -r requirements.txt` would fail on every listed interpreter under useConnexion3. Gated a 3.9-3.12 matrix behind the flag. - setup.mustache (three related issues): 1. The default (useConnexion3=false) REQUIRES list pinned "connexion>=2.0.2" with no upper bound, while requirements.mustache's equivalent line correctly caps at <=2.14.2. `pip install .` (as opposed to `pip install -r requirements.txt`) could silently resolve Connexion 3 even on the untouched v2 path, breaking code that uses Connexion-2-only APIs. This was a pre-existing gap (the line itself predates this PR), but since this PR is what's actively splitting this exact list into two branches, fixing the drive-by inconsistency here is in scope and low-risk. Added the same <=2.14.2 ceiling. 2. The useConnexion3 branch's connexion extras were missing swagger-ui (present in requirements.mustache but not here), so `pip install .` and `pip install -r requirements.txt` could resolve different dependency sets. Added the extra for parity. 3. The useConnexion3 branch had no explicit Flask constraint, unlike requirements.mustache's Flask>=2.2.0,<4.0.0. Added it for the same parity reason (even though Connexion's own flask extra already transitively requires Flask>=2.2, matching the earlier swagger-ui-bundle-floor reasoning: pip would already resolve correctly, but the explicit pin keeps setup.py self-documenting and consistent with requirements.txt). Explicitly NOT fixed (pre-existing bugs unrelated to Connexion version, in files this PR does not otherwise touch -- reviewer found these correctly, but fixing them here would be unrelated scope creep): - base_model.mustache's `__eq__` crashes comparing a Model instance to a non-Model value (accesses `other.__dict__` unconditionally). Present identically in the v2 default output too; this file has no useConnexion3 branching and was never edited by this PR. - git_push.sh.mustache pipes `git push` through `grep -v`, masking the actual push exit code. Shared boilerplate across many generators, not specific to python-flask or Connexion version. - controller_test.mustache generates a single-object example body for array-typed request parameters (create_users_with_array_input / create_users_with_list_input), which fails validation. This is the exact same pre-existing test-fixture bug already called out in the original PR description under "known gaps" -- it's masked under the v2 default output because those specific tests are separately skipped for an unrelated reason (Connexion's `*/*` consumes limitation), not because the example generation is actually correct there. Reverified after all fixes: existing PythonFlaskConnexionServerCodegenTest passes, full `bin/generate-samples.sh` run twice produces the same diff scope with no collateral changes elsewhere, and the generated v3 test suite still shows the same 9 pre-existing failures / 4 skips as before (no new regressions from these changes).
|
Thanks @cubic-dev-ai — went through all 12 findings individually against the actual code rather than accepting blindly. 9 were real and are fixed in 7b0bbc0; 3 are pre-existing bugs in shared files this PR doesn't otherwise touch, left out of scope: Fixed:
Not fixed (pre-existing, unrelated to Connexion version, out of scope for this PR):
Reverified after all fixes: existing Java test passes, full |
@Shaun-3adesign Nice triage — 9 real fixes and 3 clean deferrals looks right to me. Couple of quick observations from reading the patches:
Everything checks out. The NPE guard in If you end up opening a follow-up for any of the pre-existing bugs (the Tip: get faster answers by chatting with cubic’s review copilot in the review UI |
Adds .github/workflows/samples-python-flask-connexion3-server.yaml, mirroring the existing samples-python-fastapi-server.yaml pattern, to install the generated useConnexion3 sample's own requirements and run its generated pytest suite on every change to that folder. This closes the "known gap" called out in the original PR description: no CI job actually installs/runs the generated python-flask server (the pre-existing samples-python-server.yaml only covers python-aiohttp-srclayout). Getting the generated test suite to actually pass cleanly (rather than shipping a new CI job that's red from day one) surfaced a real bug in the skip-marking logic added here: PythonFlaskConnexionServerCodegen now overrides postProcessOperationsWithModels to skip-mark, under useConnexion3, the generated test for any operation that declares multiple response content types (e.g. both application/xml and application/json, the standard Petstore convention). Connexion 3 requires the handler to explicitly say which content type it's returning in that case; the auto-generated stub controllers don't, so calling them raises a 500 (NonConformingResponseHeaders) until the operation is actually implemented. This mirrors the exact mechanism the parent class already uses for other known Connexion limitations (unsupported/multiple consumes) -- same x-skip-test vendor extension, same @unittest.skip(reason) rendering in controller_test.mustache. Also skip-marks the two operations with array-typed request bodies (createUsersWithArrayInput/ListInput) for a separate, pre-existing, version-agnostic reason: the auto-generated test example for an array-typed body is a single item, not an array, which fails request validation regardless of Connexion version. Under Connexion 2 this happens to be masked because those same two operations are already skipped for an unrelated reason (a `*/*` consumes quirk that, for reasons not fully understood, does not trigger the same way under useConnexion3's config path); under Connexion 3 the pre-existing example bug surfaces on its own. Skip-marked with an honest reason rather than left failing or silently hidden. Diagnosing why the skip markers weren't showing up in the generated output at all (despite the Java code demonstrably running and mutating the right objects, confirmed via temporary debug logging) took a while: DefaultGenerator never overwrites an api-test-template file (controller_test.mustache -> test_*_controller.py) that already exists on disk, specifically so it doesn't clobber a user's own edits to their generated tests. Since this sample's test files were first generated many regenerations ago, every run since had been silently skipping them regardless of any template or codegen changes -- this fix only actually landed in the committed output after deleting the existing test/ directory once so the next regen would write it fresh. (Supporting files like __init__test.mustache -> test/__init__.py aren't subject to this rule, which is why earlier changes to that file always showed up correctly and this one didn't.) Also tried and reverted a spec-level fix: adding an explicit `example:` to the UserArray requestBody in modules/openapi-generator/src/test/resources/3_0/python-flask/petstore.yaml, hoping it would override the codegen's auto-synthesized single-object example for the array-typed body. Verified directly that it made no difference to the generated test data, so reverted it rather than leave a no-op change in the spec, and used the skip-marking approach above instead. Verified clean end-to-end, replicating the new workflow's exact steps (Python 3.11, `pip install -r requirements.txt && pip install -r test-requirements.txt`, `pytest`) in Docker against the final regenerated sample: 8 passed, 13 skipped, 0 failed. Also reran the existing PythonFlaskConnexionServerCodegenTest and a full `bin/generate-samples.sh` double-regen across all 766 generators -- both clean, with the diff scoped to exactly the files above (confirmed the v2 default sample and .openapi-generator/FILES manifest are both unaffected, once compared against the correct post-regeneration steady state rather than a one-off fresh-generation artifact).
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Review feedback on d06115c: the workflow's paths filters only covered the sample directory, so a broken edit to the workflow file itself would never get exercised by CI before merging -- it'd only fail (or silently no-op) on some future unrelated change to the sample. Added the workflow's own path to both push and pull_request paths lists. This isn't a new pattern here: samples-python-petstore.yaml and a few other existing python sample workflows already self-reference their own path for the same reason, even though the two workflows this one was originally modeled on (samples-python-fastapi-server.yaml, samples-python-server.yaml) don't.
|
thanks for the contribution, which has been merged into master. |
Summary
Adds an opt-in
useConnexion3boolean option (defaultfalse) to thepython-flaskserver generator, giving users a way to generate Connexion-3-based servers instead of the current Connexion 2 pin. Addresses #17303, which has been open since Dec 2023 with the maintainer repeatedly inviting a contribution and several community members posting partial working fixes in the thread.Also fixes #15062 (docs never explained why this generator uses Connexion instead of vanilla Flask) — bundled in since it's a one-line addition to a PR already touching this generator's help text. #21294 (operationId double-underscore breaking Connexion's resolver) is a different root cause and is left for its own PR.
Kept as an opt-in flag rather than a default bump, matching this repo's existing convention for breaking generator-output changes (
useJackson3,useSpringBoot3/4).Security motivation (verified, not assumed)
One commenter on #17303 specifically asked for this because staying on Connexion 2 blocks upgrading past a vulnerable Werkzeug. I ran
pip-auditagainst the actual resolved dependency tree for both paths to check this concretely:useConnexion3)pip/wheelthemselves (base image tooling, not app dependencies)What changes under the flag
connexion[flask,swagger-ui,uvicorn]>=3.3.0,<4.0.0+Flask>=2.2.0,<4.0.0(floor pinned to 3.3.0, not 3.0.0, since that's the only version actually run/verified here, and the first with official Python 3.13/3.14 support).swagger-ui-bundlebumped to>=1.1.0to match the floor Connexion's own extra already silently requires. The default (v2) path also gets an upper bound added tosetup.py'sconnexionpin (<=2.14.2, matchingrequirements.txt) sopip install .can't silently resolve Connexion 3 onto Connexion-2-only code.connexion.App→connexion.FlaskApp; JSON encoding moves from aFlask.json_encoderattribute (removed in v3) to aJsonifier(cls=...)passed intoadd_api().JSONEncoderbecomes a plainjson.JSONEncodersubclass instead of extending Connexion 2's removedFlaskJSONEncoder. Restored Flask's own datetime/date/Decimal/UUID handling explicitly (werkzeug.http.http_date()+str(...)) sodate-time-typed model fields still serialize instead of raisingTypeError— plainjson.JSONEncoderhas no support for these types at all.featureCORS):flask_cors.CORS(app.app)silently does nothing under Connexion 3 — Connexion 3 wraps Flask in its own ASGI middleware stack, so requests can be handled before ever reaching flask-cors. Replaced with Connexion 3's documentedapp.add_middleware(CORSMiddleware, ...)fromstarlette.middleware.cors(already a Connexion dependency, no new package needed).allow_credentialsis left at its default (False) to match flask-cors's own default — combining it with a wildcard origin is a CORS anti-pattern.app.test_client()returns an httpx/Starlette client, not Flask's WSGI one. Added a small adapter so the existingself.client.open(...)/self.assert200(...)calls incontroller_test.mustachekeep working unchanged. Also dropsflask_testing.TestCase(unmaintained since 2020, not Flask-3-compatible).x-skip-testmechanism the parent class already uses for other Connexion limitations) the generated test for any operation with multiple response content types, since Connexion 3 requires the handler to specify which one to return and the auto-generated stub doesn't — see CI section below.controller.mustacheswapsconnexion.request.is_json/.get_json()for Flask's ownimport flask/flask.request.*, since Connexion 3'sconnexion.requestis now a StarletteRequestand no longer exposes those methods. Module-qualified access (rather thanfrom flask import request) avoids a parameter namedrequestshadowing the import.CI coverage
Added
.github/workflows/samples-python-flask-connexion3-server.yaml, mirroring the existingsamples-python-fastapi-server.yamlpattern, to actually install and run the generated sample's own test suite on every change to that folder. Getting this genuinely green (rather than shipping a red CI job) surfaced two real, honestly-documented limitations, both skip-marked with clear reasons in the generated tests: operations with multiple response content types (inherent Connexion 3 behavior change for stub controllers, not fixable at the template level), and a pre-existing, version-agnostic bug where the auto-generated test example for an array-typed request body is a single item instead of an array.Verification
mvn -pl modules/openapi-generator -am package— compiles clean.bin/generate-samples.sh(all ~766 generators, no args) run twice — zero diff outside files intentionally touched.useConnexion3: truesample's own Dockerfile: server starts, serves/v2/openapi.json, returns correct 401 on an auth-protected route, and correctly serializes a real returned model instance (including a realdatetimefield) end-to-end via direct HTTP request.Access-Control-Allow-Origin: *present on a real response, withoutAccess-Control-Allow-Credentials.python:3-alpinefloats to Python 3.14, which breaks even the unmodified v2 sample for unrelated reasons) to fairly compare test results between v2 and v3.Known gaps (not fixed here, flagged for visibility)
python-flaskthrough any config with anadditionalPropertiesblock changes some*/*-consumes skip-marking behavior for a couple of operations (reproduces even withuseConnexion3: false). Worth its own issue/investigation; out of scope here.Fixes #15062
Test plan
useConnexion3opt-in approach is acceptable per the contributing guidelines note on avoiding excessive generator options