Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
jsell-rh
marked this pull request as ready for review
September 15, 2026 19:23
Collaborator
Amber reviewStatus: Complete |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jsell-rh
changed the title
HyperShell environment updating to commit
|
| Fact | Value |
|---|---|
| Namespaces | Platform: hypershell-ci-pr-286 Keycloak: hypershell-ci-pr-286-keycloak |
| OpenShift console | https://console-openshift-console.apps.rosa.hysh-aws-01.c6uk.p3.openshiftapps.com |
| API | https://hypershell-api-hypershell-ci-pr-286.apps.rosa.hysh-aws-01.c6uk.p3.openshiftapps.com |
| Web console | https://hypershell-web-console-hypershell-ci-pr-286.apps.rosa.hysh-aws-01.c6uk.p3.openshiftapps.com |
Log in through the web console with your GitHub account (you must be a member of
the configured organization or on its allowlist). The environment is time-boxed
and refreshed on every new commit.
CLI access
oc login --server=https://api.hysh-aws-01.c6uk.p3.openshiftapps.com:443 --web
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
amber-review-bot
left a comment
amber-review-bot
left a comment
Collaborator
There was a problem hiding this comment.
Verdict
The controller-local admin-Secret provisioning path remains well-scoped and defensively coded: the admin Secret name/namespace are DNS-validated at load, verify-full plus a valid PEM CA is enforced before any SQL runs, cleanup returns catalog/DDL errors to the retry queue instead of masking them, and no admin credentials or secret values leak into logs, errors, or the tenant Secret. The only maintainer-action items are cross-PR design/ordering decisions, not defects in this change.
Since the previously reviewed commit, the only delta is a merge from main; the feature code under internal/config, internal/gateway, cmd/hypershell-controller, and manifests/gateway/deployment.yaml is unchanged. Re-verification against the current tree confirms the prior assessment still holds.
Strengths
GATEWAY_DATABASE_ADMIN_SECRET_NAMEis validated at load time (IsDNS1123Subdomainfor the name,IsDNS1123LabelforHYPERSHELL_NAMESPACE) so an invalid reference fails fast before any read (internal/config/config.go:120-126).- The override is a hardened production path:
readExternalAdminCredentialsforcesverify-full, rejects any weakersslmode, requires a parseable PEMsslrootcert, and validates the port range before opening a connection (internal/gateway/external_db.go:148-172). The legacy API-reference path keeps its reserved-prefix / fixed-name restriction. - TLS is verified end to end: the CA is projected read-only into the gateway pod at
/etc/openshell-db/ca.crt, mounting only the publicsslrootcertkey (not admin credentials), and the tenant DSN references that path (external_db.go:636-654,manifests/gateway/deployment.yaml). - Cleanup correctness:
deleteExternalSQLResourcesreturns existence-query errors instead of treating a failed probe as "absent"; in override modeDeletereturns the error to the live retry queue and otherwise records an orphan warning naming the gateway and DB without credential values (external_db.go:48-60,595-634). - No silent fallback: failures to read or validate the admin Secret abort provisioning and cleanup.
Test Diff Scrutiny
The modified pre-existing test assertions are additive, not weakened. managedDatabaseWatchEligible(..., "") in main_test.go adds the new parameter with an empty value to preserve original behavior, with a companion test for the new branch. external_db_test.go, manifests_test.go, config_test.go, and reconciler_test.go only add new cases. No prior guarantee (accepted -> rejected, optional -> required, success -> error) was silently flipped.
Cross-PR coordination
Two other open pull requests make design or structural choices that must be reconciled with this change before both can land; these are decisions/ordering items, not mechanical merge conflicts:
- A competing spec pull request proposes the opposite, intentionally breaking resolution of the same "controller-local gateway database" problem: it deletes
specs/platform/openshell-gateway-database-external.spec.md(the exact file this PR extends), removes theManagedDatabaseentity fromdata-model.spec.md, and dropsdatabase_idfrom the Gateway API. This PR is built on the opposite premise - it keepsManagedDatabase/database_id, has the API still assign them, and has the controller ignore and retain those records. Maintainers must decide which model is the target design and the merge order: if this PR lands first, the other will delete the section added here; if the other lands first, this PR's spec extension and its ManagedDatabase-preserving code path have no data model to attach to. - A pull request that adopts the upstream OpenShell Helm chart for gateway deployments deletes
components/control-plane/manifests/gateway/deployment.yaml, the same manifest this PR edits to add thedatabase-cavolume and read-only mount that the verify-full tenant TLS feature depends on. If that pull request merges after this one, the CA projection is silently lost unless it is re-expressed in the Helm chart values. Maintainers should decide merge order and who ports the CA projection into the Helm path.
Previous concerns
The prior Amber review (review 5215111487) raised no blocking, critical, or major code-level findings; its only maintainer item was the cross-PR design decision about the competing controller-local-database spec and the Helm-chart manifest deletion. That item is still present and is re-stated above: the competing pull request still deletes the external-database spec file this PR extends and still removes the ManagedDatabase entity and database_id field that this PR's design retains, and the Helm-adoption pull request still deletes the gateway deployment.yaml this PR modifies for the CA mount.
Findings Summary
No blocking, critical, or major code defects were found. The only maintainer-action items are the cross-PR design/ordering decisions noted above.
Convention Checklist
| Convention | Result |
|---|---|
No panic() in production code |
Pass |
Errors wrapped with fmt.Errorf context |
Pass |
errors.IsNotFound handled for 404 scenarios |
Pass |
| No secrets in logs or error messages | Pass |
| Input validated (Secret name / namespace DNS format, port, PEM CA) | Pass |
| SecurityContext / read-only mount for projected CA | Pass |
| Reconcile pattern, no create-or-skip | Pass |
| No silent fallback on error paths | Pass |
| Spec updated alongside behavior change | Pass |
| Test diff scrutiny (no silently removed guarantees) | Pass |
| Conventional commit messages | Pass |
Collaborator
Author
|
@rh-amarin will fold this into his work |
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 16, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback, no API-side database inventory, and TLS is verify-full everywhere with no downgrade path. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - Tenant Secret openshell-gateway-db-credentials now carries sslmode and sslrootcert; the gateway Deployment mounts the CA at /etc/openshell-db/ca.crt and both admin and gateway connections verify the server certificate and hostname. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is now the tenant Secret's sslmode=verify-full and non-empty sslrootcert. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5 of 8 tasks
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 16, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback, no API-side database inventory, and TLS is verify-full everywhere with no downgrade path. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - Tenant Secret openshell-gateway-db-credentials now carries sslmode and sslrootcert; the gateway Deployment mounts the CA at /etc/openshell-db/ca.crt and both admin and gateway connections verify the server certificate and hostname. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is now the tenant Secret's sslmode=verify-full and non-empty sslrootcert. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 16, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback, no API-side database inventory, and TLS is verify-full everywhere with no downgrade path. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - Tenant Secret openshell-gateway-db-credentials now carries sslmode and sslrootcert; the gateway Deployment mounts the CA at /etc/openshell-db/ca.crt and both admin and gateway connections verify the server certificate and hostname. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is now the tenant Secret's sslmode=verify-full and non-empty sslrootcert. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rh-amarin
marked this pull request as draft
September 16, 2026 20:38
4 of 7 tasks
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 17, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback, no API-side database inventory, and TLS is verify-full everywhere with no downgrade path. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - Tenant Secret openshell-gateway-db-credentials now carries sslmode and sslrootcert; the gateway Deployment mounts the CA at /etc/openshell-db/ca.crt and both admin and gateway connections verify the server certificate and hostname. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is now the tenant Secret's sslmode=verify-full and non-empty sslrootcert. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
|
closing in-favor of #300 |
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 18, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback, no API-side database inventory, and TLS is verify-full everywhere with no downgrade path. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - Tenant Secret openshell-gateway-db-credentials now carries sslmode and sslrootcert; the gateway Deployment mounts the CA at /etc/openshell-db/ca.crt and both admin and gateway connections verify the server certificate and hostname. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is now the tenant Secret's sslmode=verify-full and non-empty sslrootcert. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 18, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback, no API-side database inventory, and TLS is verify-full everywhere with no downgrade path. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - Tenant Secret openshell-gateway-db-credentials now carries sslmode and sslrootcert; the gateway Deployment mounts the CA at /etc/openshell-db/ca.crt and both admin and gateway connections verify the server certificate and hostname. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is now the tenant Secret's sslmode=verify-full and non-empty sslrootcert. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rh-amarin
pushed a commit
that referenced
this pull request
Sep 20, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR #286 (jsell-rh): no dual-provider fallback and no API-side database inventory. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - The admin connection is always sslmode=verify-full: the control plane mounts the CA itself and verifies the server certificate and hostname. - The tenant Secret openshell-gateway-db-credentials carries sslmode=require and no sslrootcert. The gateway workload is deployed by the upstream OpenShell Helm chart, which reads only this Secret's uri key (server.externalDbSecret) and has no mechanism to mount a database CA into the gateway pod, so the gateway connection is encrypted but not certificate-verified. See specs/platform/openshell-gateway-helm-adoption.spec.md § Gap: No Database CA Mount. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). - GatewayReconciler takes the DatabaseConfig alongside the Helm client added by PR #194 and refuses to start without an admin credentials directory. API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Dropped the ManagedDatabase tombstone-replay branch from the gRPC RBAC stream interceptor and its test. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. - Dropped the managed-database-status widget from the default layout, added it to REMOVED_WIDGET_TYPES so saved grids shed it, widened the cluster regions donut back across columns 2-3 and bumped the layout persistence key to v42. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is the tenant Secret's sslmode=require, a uri that requests TLS, and the absence of an sslrootcert key. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. - Dashboard users must reset their layout or clear the v41 localStorage key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rh-amarin
pushed a commit
that referenced
this pull request
Sep 21, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR #286 (jsell-rh): no dual-provider fallback and no API-side database inventory. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - The admin connection is always sslmode=verify-full: the control plane mounts the CA itself and verifies the server certificate and hostname. - The tenant Secret openshell-gateway-db-credentials carries sslmode=require and no sslrootcert. The gateway workload is deployed by the upstream OpenShell Helm chart, which reads only this Secret's uri key (server.externalDbSecret) and has no mechanism to mount a database CA into the gateway pod, so the gateway connection is encrypted but not certificate-verified. See specs/platform/openshell-gateway-helm-adoption.spec.md § Gap: No Database CA Mount. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). - GatewayReconciler takes the DatabaseConfig alongside the Helm client added by PR #194 and refuses to start without an admin credentials directory. API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Dropped the ManagedDatabase tombstone-replay branch from the gRPC RBAC stream interceptor and its test. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. - Dropped the managed-database-status widget from the default layout, added it to REMOVED_WIDGET_TYPES so saved grids shed it, widened the cluster regions donut back across columns 2-3 and bumped the layout persistence key to v42. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is the tenant Secret's sslmode=require, a uri that requests TLS, and the absence of an sslrootcert key. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. - Dashboard users must reset their layout or clear the v41 localStorage key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 21, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback and no API-side database inventory. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - The admin connection is always sslmode=verify-full: the control plane mounts the CA itself and verifies the server certificate and hostname. - The tenant Secret openshell-gateway-db-credentials carries sslmode=require and no sslrootcert. The gateway workload is deployed by the upstream OpenShell Helm chart, which reads only this Secret's uri key (server.externalDbSecret) and has no mechanism to mount a database CA into the gateway pod, so the gateway connection is encrypted but not certificate-verified. See specs/platform/openshell-gateway-helm-adoption.spec.md § Gap: No Database CA Mount. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). - GatewayReconciler takes the DatabaseConfig alongside the Helm client added by PR openshift-online#194 and refuses to start without an admin credentials directory. API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Dropped the ManagedDatabase tombstone-replay branch from the gRPC RBAC stream interceptor and its test. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. - Dropped the managed-database-status widget from the default layout, added it to REMOVED_WIDGET_TYPES so saved grids shed it, widened the cluster regions donut back across columns 2-3 and bumped the layout persistence key to v42. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is the tenant Secret's sslmode=require, a uri that requests TLS, and the absence of an sslrootcert key. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. - Dashboard users must reset their layout or clear the v41 localStorage key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rh-amarin
pushed a commit
to rh-amarin/hypershell
that referenced
this pull request
Sep 21, 2026
Remove the ManagedDatabase resource and Gateway.database_id entirely and replace them with a single PostgreSQL admin credential Secret mounted into the control plane. Incorporates and simplifies the idea from PR openshift-online#286 (jsell-rh): no dual-provider fallback and no API-side database inventory. Control plane: - New internal/gateway/database.go reads the admin Secret from files at GATEWAY_DATABASE_ADMIN_DIR (default /etc/hypershell/gateway-database): host, port, user, password, sslrootcert required; dbname optional; sslmode optional but must be verify-full if present. Re-read on every operation so a rotated admin password needs no controller restart. - gateway.ValidateAdminCredentialsDir is a hard startup precondition: the controller refuses to start on a missing/malformed Secret, without connecting to the server. Reachability and privileges are checked at reconcile time with retries. - Per gateway: role and database gw_<id>, GRANT gw_<id> TO <admin user> so a non-superuser admin with CREATEDB+CREATEROLE can own the database, REVOKE CONNECT FROM PUBLIC, GRANT CONNECT. Password reused from the existing tenant Secret or generated with crypto/rand; ALTER ROLE only as repair, never rotation. - The admin connection is always sslmode=verify-full: the control plane mounts the CA itself and verifies the server certificate and hostname. - The tenant Secret openshell-gateway-db-credentials carries sslmode=require and no sslrootcert. The gateway workload is deployed by the upstream OpenShell Helm chart, which reads only this Secret's uri key (server.externalDbSecret) and has no mechanism to mount a database CA into the gateway pod, so the gateway connection is encrypted but not certificate-verified. See specs/platform/openshell-gateway-helm-adoption.spec.md § Gap: No Database CA Mount. - A failed cleanup on gateway delete returns an error to the retry queue and records a PostgreSQLDatabase IncompleteFinalization Event instead of logging and moving on. - Removed the ManagedDatabase watch, reconciler and gRPC service consumer entirely (watcher.go, reconciler.go). - GatewayReconciler takes the DatabaseConfig alongside the Helm client added by PR openshift-online#194 and refuses to start without an admin credentials directory. API server, CLI, SDKs: - Deleted the managedDatabases plugin, its OpenAPI/proto/gRPC surface, and the CLI managedDatabase(s) commands. - Removed database_id from Gateway (model, OpenAPI, proto field reserved by number and name, presenters, handlers, CLI, both SDKs) and the gateway placement code that assigned it. - Dropped the ManagedDatabase tombstone-replay branch from the gRPC RBAC stream interceptor and its test. - Migrations: drop gateways.database_id and the managed_databases table. UI: - Removed the operational dashboard's database inventory widget and metric, the BFF's managed-database Prometheus queries, and the gateway detail page's database ID field. - Dropped the managed-database-status widget from the default layout, added it to REMOVED_WIDGET_TYPES so saved grids shed it, widened the cluster regions donut back across columns 2-3 and bumped the layout persistence key to v42. Deploy, scripts, CI: - New scripts/gen-postgres-tls.sh shared by kind and OpenShift dev environments to mint a CA and serve the stand-in PostgreSQL over TLS. - Both dev drivers create the hypershell-gateway-database-admin Secret with sslmode=verify-full before deploying the controller; the hypershell-managed-db-* credentials namespaces and openshell-db seed are gone. - New optional Kustomize component deploy/components/gateway-database-admin-secret shows External Secrets Operator syncing the admin Secret from a secret manager. - e2e proof of database provisioning is the tenant Secret's sslmode=require, a uri that requests TLS, and the absence of an sslrootcert key. Specs and docs rewritten to match: specs/platform/openshell-gateway-database.spec.md is the single source of truth for the new design; platform-inventory, data-model, rbac-enforcement, naming-multitenancy and the web-console specs updated to remove the ManagedDatabase surface. Upgrade notes: - The migrations are destructive: existing gateway database_id values and the managed_databases table are dropped with no backfill. - Existing openshell-db-* namespaces lose their GC exclusion and become eligible for garbage collection. - OpenShift dev environments need make openshift-down && make openshift-up because the credentials project and TLS Secret shape changed. - Dashboard users must reset their layout or clear the v41 localStorage key. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GATEWAY_DATABASE_ADMIN_SECRET_NAMEselects a PostgreSQL admin Secret inHYPERSHELL_NAMESPACE. The controller creates one SQL database and login role per gateway on that server. It skips the ManagedDatabase watch, database lookup, and CNPG startup check. The unset path keeps its current behavior.The override requires
verify-fulland a PEM CA bundle. The gateway receives its own credentials and the public CA through a read-only mount. The controller grants the role membership needed for a non-superuser admin account. Failed cleanup queries and SQL operations return errors to the live retry queue and record anIncompleteFinalizationwarning Event.The specification now has named requirements and Given/When/Then scenarios. A Kustomize component shows how External Secrets Operator can synchronize a secret-manager entry. The documentation covers credential updates, CA updates, and manual cleanup after a controller restart.
The API, schema, SDKs, and migrations do not change. The API still assigns
database_idand creates or selects its current ManagedDatabase records. The controller ignores and retains these records in override mode. This change does not migrate existing gateway data.Validation:
Operational limits: the cleanup retry queue does not survive a controller restart. Admin password updates are read on each SQL operation. CA changes for running gateways need the tenant CA update and deployment restart described in the runbook.