Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades Connexion from version 2.14.2 to version 3.x across the CloudHarness framework and its applications, addressing compatibility issues with the newer API framework version.
Key changes:
- Migrates from Connexion 2.x to 3.x with corresponding Flask compatibility updates
- Replaces Flask-CORS with custom CORS handling compatible with Connexion 3.x
- Updates error handling to work with Connexion 3.x's new exception handling paradigm
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| libraries/cloudharness-common/cloudharness/utils/server.py | Core server initialization logic updated for Connexion 3.x compatibility, including JSON encoder changes, CORS setup, and error handling refactoring |
| infrastructure/common-images/cloudharness-flask/requirements.txt | Updated dependency versions for Connexion 3.x, Flask 2.x+, and added uvicorn |
| infrastructure/cluster-configuration/storageclass.yaml | Fixed Kubernetes StorageClass field name from deprecated volumeProvisioner to provisioner |
| infrastructure/cluster-configuration/storageclass-dockerdesktop.yaml | Added new StorageClass configuration for Docker Desktop environments |
| deployment/codefresh-test.yaml | Reorganized CI/CD build steps and updated build order for application images |
| deployment-configuration/helm/templates/ingress.yaml | Added support for whitelisted URI mappings in secured applications and proxy buffering configuration |
| applications/*/requirements.txt | Updated Connexion and related dependencies to version 3.x across all applications |
| applications/samples/tasks/sum/Dockerfile | Changed base image from CLOUDHARNESS_BASE to SAMPLES |
| applications/samples/backend/samples/controllers/test_controller.py | Removed environment variable validation logic from ping endpoint |
| applications/common/server/common/main.py | Removed Flask-CORS initialization as CORS is now handled centrally |
| application-templates/flask-server/backend/requirements.txt | Updated template dependencies to Connexion 3.x |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| try: | ||
| import flask | ||
| request = flask.request if flask.has_request_context() else None | ||
| except: |
There was a problem hiding this comment.
Bare except: clause catches all exceptions including system-exiting exceptions. Use except Exception: to catch only standard exceptions.
| except: | |
| except Exception: |
infrastructure/common-images/cloudharness-flask/requirements.txt
Outdated
Show resolved
Hide resolved
| @@ -1,4 +1,4 @@ | |||
| FROM quay.io/keycloak/keycloak:26.3.5 | |||
| FROM quay.io/keycloak/keycloak:26.4 | |||
There was a problem hiding this comment.
This automatically updates to the latest minor version. It could lead to having different versions across envs for the same app.
There was a problem hiding this comment.
It should be no harm to use the latest patch update for keycloak, they respect semver quite well and release quite often
| python_dateutil >= 2.9.0 | ||
| setuptools >= 21.0.0 | ||
|
|
||
| gunicorn |
There was a problem hiding this comment.
Should we fix a specific version?
| swagger-ui-bundle>=1.1.0 | ||
| python_dateutil>=2.9.0 | ||
| setuptools>=21.0.0 | ||
| uvicorn |
There was a problem hiding this comment.
Should we fix a specific version?
There was a problem hiding this comment.
Used to do pip freeze but now I prefer not to fix specific versions on cloud harness because getting security updates requires a lot of work in pinning every single requirement and we end up with outdated libraries. The risk of this simple microservice breaking is pretty low and we are now monitoring vulnerabilities that should catch bad guys coming in
| @@ -1,3 +1,3 @@ | |||
| FROM node:20 | |||
| FROM node:22-alpine | |||
There was a problem hiding this comment.
For consistency should we use 22-trixie-slim?
There was a problem hiding this comment.
this is just for building, and alpine has less reported vulnerabilities than trixie (easily 0)
| Pillow>=9.2.0 | ||
| python-keycloak | ||
| django-prometheus | ||
| uvicorn No newline at end of file |
There was a problem hiding this comment.
leaving that open for the common images so inheritors can pin versions as needed
|
|
||
| # # keycloak kafka listener plugin | ||
| COPY plugins/metacell-admin-event-listener-module-1.0.0.jar /opt/keycloak/providers/ | ||
| COPY plugins/* /opt/keycloak/providers/ |
There was a problem hiding this comment.
I would specifically list the plugins we want to install. Currently there is only one anyway.
There was a problem hiding this comment.
I preferred this option so applications can add plugins without necessarily overriding the docker file
Closes CH-226
Closes CH-229
Also fixes unrelated Closes CH-207
Some of the changes included in this PR are low impact breaking changes:
Implemented solution
CH-226 Applications and template based on Connexion (python/flask serve openapi generator) are now supporting Connexion 3+
CH-229 base images have all been updated; minimal debian trixie is now used as a base
CH-207 base image names changed to better scope vulnerabilities in applications vs cloud harness
How to test this PR
https://samples.test-ch.dev.metacell.us/ https://workflows.test-ch.dev.metacell.us/ https://common.test-ch.dev.metacell.us/ are deployed with the test pipeline. Might also be useful to test a new generated application.
...
Sanity checks:
Breaking changes (select one):
breaking-changeand the migration procedure is well described abovePossible deployment updates issues (select one):
alert:deploymentTest coverage (select one):
Documentation (select one):
Nice to have (if relevant):