Conversation
secret key value is given, the secret is randomly generated.
libraries/models/cloudharness_model/models/ingress_global_config_all_of_letsencrypt.py
Fixed
Show fixed
Hide fixed
libraries/models/cloudharness_model/models/ingress_global_config_all_of_letsencrypt.py
Fixed
Show fixed
Hide fixed
libraries/models/cloudharness_model/models/ingress_global_config_all_of_letsencrypt.py
Fixed
Show fixed
Hide fixed
libraries/models/cloudharness_model/models/ingress_global_config_all_of_letsencrypt.py
Fixed
Show fixed
Hide fixed
libraries/models/cloudharness_model/models/database_config.py
Dismissed
Show dismissed
Hide dismissed
libraries/models/cloudharness_model/models/database_config.py
Dismissed
Show dismissed
Hide dismissed
applications/samples/backend/samples/controllers/database_controller.py
Dismissed
Show dismissed
Hide dismissed
applications/samples/backend/samples/controllers/database_controller.py
Dismissed
Show dismissed
Hide dismissed
applications/samples/backend/samples/controllers/database_controller.py
Dismissed
Show dismissed
Hide dismissed
applications/samples/backend/samples/controllers/database_controller.py
Dismissed
Show dismissed
Hide dismissed
libraries/models/cloudharness_model/models/database_config.py
Dismissed
Show dismissed
Hide dismissed
libraries/models/cloudharness_model/models/database_config.py
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Pull request overview
This PR updates CloudHarness “gateway/ingress” and database external connection-string handling across Helm templates, the deployment CLI (Codefresh), and the generated models/docs, and adds a sample API endpoint for retrieving the DB connection string.
Changes:
- Add support for injecting
database.external_connect_stringvia CI/CD secrets (Helm secret + Codefreshcustom_values) and reading it at runtime from a mounted file. - Restructure ingress/gateway configuration (new
gatewayapp-level config,ingressClasssupport, Traefik middlewares, per-app Ingress/TLS Secret generation). - Regenerate/extend OpenAPI models/docs to include Gateway/DB configs and update sample OpenAPI + controllers.
Reviewed changes
Copilot reviewed 62 out of 63 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/deployment-cli-tools/tests/test_codefresh.py | Adds test coverage for Codefresh external DB connect-string secret injection; minor formatting fix. |
| tools/deployment-cli-tools/tests/resources/applications/myapp/deploy/values.yaml | Test fixture updated to include database.external_connect_string. |
| tools/deployment-cli-tools/ch_cli_tools/codefresh.py | Adds custom_values injection for empty external_connect_string to be provided via env var at deploy time. |
| libraries/models/docs/PortConfig.md | Adds generated model documentation for PortConfig. |
| libraries/models/docs/IngressGlobalConfigAllOfLetsencrypt.md | Adds generated model documentation for global ingress letsencrypt config. |
| libraries/models/docs/IngressGlobalConfig.md | Adds generated model documentation for global ingress config. |
| libraries/models/docs/IngressConfig.md | Updates generated ingress config docs (now path-related fields). |
| libraries/models/docs/HarnessMainConfig.md | Updates ingress schema reference to GatewayGlobalConfig. |
| libraries/models/docs/GatewayGlobalConfigAllOfLetsencrypt.md | Adds generated model documentation for gateway letsencrypt config. |
| libraries/models/docs/GatewayGlobalConfig.md | Adds generated model documentation for gateway global config. |
| libraries/models/docs/GatewayConfig.md | Adds generated model documentation for per-app gateway config. |
| libraries/models/docs/GatekeeperConf.md | Adds docs for gatekeeper resources and secret. |
| libraries/models/docs/DatabaseDeploymentConfig.md | Adds external_connect_string to DB deployment config docs (contains a typo). |
| libraries/models/docs/DatabaseConfig.md | Adds generated model documentation for DatabaseConfig. |
| libraries/models/docs/ApplicationHarnessConfig.md | Documents new gateway field on app harness config. |
| libraries/models/cloudharness_model/models/port_config.py | Adds generated PortConfig model. |
| libraries/models/cloudharness_model/models/ingress_global_config_all_of_letsencrypt.py | Adds generated model. |
| libraries/models/cloudharness_model/models/ingress_global_config.py | Adds generated model. |
| libraries/models/cloudharness_model/models/ingress_config.py | Updates generated ingress config model fields. |
| libraries/models/cloudharness_model/models/harness_main_config.py | Switches ingress field type to GatewayGlobalConfig. |
| libraries/models/cloudharness_model/models/gateway_global_config_all_of_letsencrypt.py | Adds generated model. |
| libraries/models/cloudharness_model/models/gateway_global_config.py | Adds generated model. |
| libraries/models/cloudharness_model/models/gateway_config.py | Adds generated model. |
| libraries/models/cloudharness_model/models/gatekeeper_conf.py | Adds resources and secret fields and (de)serialization. |
| libraries/models/cloudharness_model/models/database_deployment_config.py | Adds external_connect_string field (contains a typo in description). |
| libraries/models/cloudharness_model/models/database_config.py | Adds generated DatabaseConfig model. |
| libraries/models/cloudharness_model/models/application_harness_config.py | Adds gateway field and (de)serialization. |
| libraries/models/cloudharness_model/models/init.py | Exposes new generated models (Gateway*, DatabaseConfig, PortConfig). |
| libraries/models/api/openapi.yaml | Updates schemas (Gateway*), adds external_connect_string, removes IngressConfig schema, tweaks RegistrySecretConfig. |
| libraries/models/README.md | Updates model list to include new Gateway/DB models. |
| libraries/cloudharness-common/cloudharness/applications.py | Reads external DB connect string from mounted file before falling back to auto-db behavior. |
| docs/model/PortConfig.md | Adds generated public docs for PortConfig. |
| docs/model/IngressConfigAllOfLetsencrypt.md | Removes generated doc file. |
| docs/model/IngressConfig.md | Removes generated doc file. |
| docs/model/HarnessMainConfig.md | Updates public docs to reference GatewayGlobalConfig. |
| docs/model/GatewayGlobalConfigAllOfLetsencrypt.md | Adds generated public docs. |
| docs/model/GatewayGlobalConfig.md | Adds generated public docs. |
| docs/model/GatewayConfig.md | Adds generated public docs. |
| docs/model/GatekeeperConf.md | Updates public docs for gatekeeper resources/secret. |
| docs/model/DatabaseDeploymentConfig.md | Updates public docs for external_connect_string. |
| docs/model/DatabaseConfig.md | Adds generated public docs. |
| docs/model/ApplicationHarnessConfig.md | Documents app-level gateway. |
| docs/ingress-domains-proxies.md | Adds “route pattern” documentation (currently mismatched with chart keys) and updates a gatekeeper link. |
| deployment/codefresh-test.yaml | Reorders/adjusts Codefresh test pipeline steps and rollout order. |
| deployment-configuration/value-template.yaml | Adds database.external_connect_string comment and app-level gateway config template. |
| deployment-configuration/helm/values.yaml | Adds ingress class/annotations/path/pathType defaults; adds proxy gatekeeper secret; formatting tweaks. |
| deployment-configuration/helm/templates/traefik-middlewares.yaml | Adds Traefik middlewares for rewrite/body-size/ssl-redirect/proxy rewrite. |
| deployment-configuration/helm/templates/tls-secret.yaml | Changes local TLS secret generation to per-app secrets. |
| deployment-configuration/helm/templates/ingress.yaml | Refactors ingress generation for ingressClass + per-app ingresses + proxy ingresses + gateway path/pathType. |
| deployment-configuration/helm/templates/configmap.yaml | Removes external_connect_string from rendered all-values configmap. |
| deployment-configuration/helm/templates/certs/letsencrypt.yaml | Uses configurable ingress class instead of hardcoded nginx. |
| deployment-configuration/helm/templates/auto-secrets.yaml | Adds generation of per-app DB external-connect-string Secrets when provided. |
| deployment-configuration/helm/templates/auto-gatekeepers.yaml | Removes hardcoded encryption key; introduces Secret + env var wiring; bumps default gatekeeper image. |
| deployment-configuration/helm/templates/auto-deployments.yaml | Mounts DB external secret to /opt/cloudharness/resources/db when configured. |
| applications/samples/deploy/values.yaml | Adds route_pattern and DB external connect string for samples app. |
| applications/samples/backend/samples/util.py | Formatting/indentation changes in deserialization helpers. |
| applications/samples/backend/samples/openapi/openapi.yaml | Adds /db-connect-string endpoint; changes cookie auth info func hook. |
| applications/samples/backend/samples/models/get_db_connect_string200_response.py | Adds generated response model for new endpoint. |
| applications/samples/backend/samples/models/init.py | Exposes the new generated model. |
| applications/samples/backend/samples/controllers/database_controller.py | Adds controller for /db-connect-string. |
| applications/samples/backend/requirements.txt | Replaces requirements with Connexion 2.x-style pins and Flask pin. |
| applications/samples/backend/.openapi-generator-ignore | Stops ignoring controllers so generated controllers can be included. |
| applications/samples/api/openapi.yaml | Adds /db-connect-string endpoint to the hand-authored API spec. |
| # -- Set to "" to set the set the string in the CI/CD as a secret. Only set the full value for dev/testing | ||
| external_connect_string: |
There was a problem hiding this comment.
Comment has duplicated words: “set the set the”. Please correct the wording so it reads clearly.
| {{ $app := get .root.Values.apps .service_name}} | ||
| - path: /proxy/{{ $app.harness.service.name }}/(.*) | ||
| {{ $routePattern := default .root.Values.ingress.path $app.harness.gateway.path }} | ||
| - path: /proxy/{{ $app.harness.service.name }}/{{- if eq .root.Values.ingress.ingressClass "nginx" }}(.*){{- else }}.*{{- end }} |
There was a problem hiding this comment.
deploy_utils.ingress.service defines $routePattern using $app.harness.gateway.path, which will fail when gateway is unset, and $routePattern is unused. Remove this assignment or compute it safely via get ... | default dict only if needed.
| {{- $gkSecretName := printf "%s-gk" .subdomain }} | ||
| {{- $existingGkSecret := (lookup "v1" "Secret" .root.Values.namespace $gkSecretName) }} | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ $gkSecretName }} | ||
| namespace: {{ .root.Values.namespace }} | ||
| labels: | ||
| app: {{ $gkSecretName }} | ||
| type: Opaque | ||
| stringData: | ||
| updated: {{ now | quote }} | ||
| {{- if .root.Values.proxy.gatekeeper.secret }} | ||
| encryption-key: {{ .root.Values.proxy.gatekeeper.secret | quote }} | ||
| {{- else }} | ||
| {{- $hasExisting := false }} | ||
| {{- if $existingGkSecret }} | ||
| {{- if eq (typeOf $existingGkSecret.data) (typeOf dict) }} | ||
| {{- if hasKey $existingGkSecret.data "encryption-key" }} | ||
| {{- $hasExisting = true }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- if not $hasExisting }} | ||
| encryption-key: {{ randAlphaNum 32 | quote }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
When an existing *-gk Secret is found, this template omits encryption-key from stringData. On helm upgrade this can clear the key from the Secret, breaking gatekeeper startup (the Deployment expects encryption-key). Prefer always rendering the key: use the provided .Values.proxy.gatekeeper.secret, else reuse the looked-up key (e.g. from $existingGkSecret.data["encryption-key"]), else generate a new one.
| /db-connect-string: | ||
| get: | ||
| description: Returns the database connection string for the current application. | ||
| operationId: get_db_connect_string | ||
| responses: | ||
| "200": | ||
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: '#/components/schemas/get_db_connect_string_200_response' | ||
| description: Database connection string returned successfully | ||
| "500": | ||
| description: Error retrieving database connection string | ||
| summary: Get database connection string | ||
| tags: | ||
| - database | ||
| x-openapi-router-controller: samples.controllers.database_controller |
There was a problem hiding this comment.
This endpoint returns the database connection string but has no security requirement. That makes the DB connect string publicly accessible by default. Add an appropriate auth requirement (e.g. bearerAuth/cookieAuth) or remove the endpoint if it’s intended only for debugging.
| external_connect_string: Optional[StrictStr] = Field(default=None, description="Specify if the database is external. If not null, auto deployment if set will not be used. Leva it as an empty string and the connect string will be provided as a secret to be provided at CI/CD (recommended)") | ||
| additional_properties: Dict[str, Any] = {} |
There was a problem hiding this comment.
Typo in field description: “Leva it as an empty string…” should be “Leave it as an empty string…”. This description is exposed in generated docs/clients.
| paths: | ||
| {{- if and $app.harness.secured $secured_gatekeepers $app.harness.uri_role_mapping }} | ||
| {{- range $mapping := $app.harness.uri_role_mapping }} | ||
| {{- if and (hasKey $mapping "white-listed") (index $mapping "white-listed") }} | ||
| {{- $uri := $mapping.uri }} | ||
| {{- if eq $uri "/" }} | ||
| - path: /() | ||
| pathType: ImplementationSpecific | ||
| {{- if and $app.harness.secured $secured_gatekeepers $app.harness.uri_role_mapping (eq $app.harness.gateway.pathType "Prefix") }} | ||
| {{- range $mapping := $app.harness.uri_role_mapping }} |
There was a problem hiding this comment.
$app.harness.gateway.pathType / .path is accessed directly. Since gateway is optional, Helm will error when gateway is not set (can’t evaluate field on nil). Please assign a safe local (e.g. $gateway := (get $app.harness "gateway" | default dict)) and use $gateway.pathType/$gateway.path with fallbacks to .Values.ingress.*.
zsinnema
left a comment
There was a problem hiding this comment.
comment: @filippomc wdyt of always create the database connect string secret? I mean insterad of the current database connection method we could create the secret when installing the chart when it's not configured in the values.yaml of the microservice. Imho this is cleaner than having 2 different ways of getting the correct database connection in the service
@zsinnema I like the secrets idea as a chance to get rid of open and configured database passwords and databases, but have to make it backwards compatible. All considered I lean towards leaving as it is for this PR scope. |
@filippomc yeah I understand and share your concers. What about doing this:
wdyt? |
@zoran-sinnema your suggestion is introducing a new scope to the initial implementation content, which is only about giving a new option to use an external database. Will do the rename within this scope to make it future proof, but let's open another card for the rest. It's additional effort that can be prioritized separately. |
my only concern that remains for now is that this is a global "system" setting and not a per (micro)service setting. this is the follow up ticket: https://metacell.atlassian.net/browse/CH-249 |
…arness into feature/gateway-updates
| from typing import Tuple | ||
| from typing import Union | ||
|
|
||
| from samples.models.get_db_connect_string200_response import GetDbConnectString200Response # noqa: E501 |
| from typing import Union | ||
|
|
||
| from samples.models.get_db_connect_string200_response import GetDbConnectString200Response # noqa: E501 | ||
| from samples import util |
This implementation is per microservice already, but of course could set value-template to be inherited by all microservices. |
Closes CH-243 - Customizable Ingress entry points. Took over from #838, with some fixes and default for cross compatible configuration (Prefix/Exact) and support for Traefik.
Closes CH-224 - Gatekeeper update. Took over from #839, handles the encryption key on a auto generated secret
Closes CH-248 - external database support. Adds a new parameter
harness.external_connect_stringon applications, which points the database route there. Uses a secret to store the connect string.How to test this PR
Currently deployed at https://samples.test-ch.dev.metacell.us/
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):