Skip to content

feat(helm): add optional PostgreSQL backing store#1579

Open
sauagarwa wants to merge 3 commits into
NVIDIA:mainfrom
sauagarwa:feat/helm-postgres-secret
Open

feat(helm): add optional PostgreSQL backing store#1579
sauagarwa wants to merge 3 commits into
NVIDIA:mainfrom
sauagarwa:feat/helm-postgres-secret

Conversation

@sauagarwa
Copy link
Copy Markdown
Contributor

@sauagarwa sauagarwa commented May 26, 2026

Summary

  • Add optional PostgreSQL support to the Helm chart with postgres.enabled (use PostgreSQL) and postgres.deploy (deploy bundled Bitnami subchart) flags
  • Support both internal (bundled subchart) and external PostgreSQL with credentials managed via Kubernetes Secrets
  • Add postgres.external.existingSecret to let users bring their own pre-existing Secret (e.g. from external-secrets-operator or GitOps)
  • When postgres.deploy=true, reference the Bitnami service-binding secret directly instead of duplicating credentials
  • Externalize JWT signing key file mode via sandboxJwt.secretDefaultMode with 0400 default
  • Add validation, helm unit tests, and README documentation for all configuration variants

Closes #1599

Changes

  • values.yaml: Add postgres.* values block including external.existingSecret and sandboxJwt.secretDefaultMode
  • Chart.yaml: Add Bitnami PostgreSQL optional subchart dependency (condition: postgres.deploy)
  • templates/db-secret.yaml: Chart-managed Opaque Secret with individual credential fields + uri key (only created when deploy=false and no existingSecret)
  • templates/_helpers.tpl: openshell.dbSecretName and openshell.postgresFullname helpers for secret resolution across all modes
  • templates/statefulset.yaml: Conditional OPENSHELL_DB_URL env var from Secret uri key, scoped checksum annotation
  • templates/gateway-config.yaml: Omit db_url from TOML when PostgreSQL is enabled
  • tests/gateway_config_test.yaml: Comprehensive test cases covering bundled, external, existingSecret, fullnameOverride, and nameOverride paths
  • README.md.gotmpl: Restructured database backend section with existingSecret-first flow, separate Kubernetes/OpenShift examples
  • tasks/helm.toml: Add helm dependency build before lint and unittest
  • .gitignore: Ignore subchart tarballs

Test plan

  • mise run helm:test — all tests pass
  • mise run pre-commit — all lint checks pass
  • Deploy with internal PostgreSQL (postgres.enabled=true, postgres.deploy=true)
  • Deploy with external PostgreSQL (postgres.enabled=true, external host)
  • Deploy with postgres.external.existingSecret pointing to a pre-existing Secret
  • Verify postgres.deploy=true without postgres.enabled=true fails with clear error
  • Verify default SQLite path unchanged when postgres.enabled=false

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 26, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@sauagarwa sauagarwa force-pushed the feat/helm-postgres-secret branch 3 times, most recently from 1fc9bef to 253051f Compare May 26, 2026 22:07
@sauagarwa sauagarwa marked this pull request as draft May 26, 2026 22:07
@sauagarwa sauagarwa force-pushed the feat/helm-postgres-secret branch from 253051f to d8d0be7 Compare May 26, 2026 22:15
@TaylorMutch TaylorMutch self-assigned this May 27, 2026
…redentials

- Add postgres.enabled and postgres.deploy values to control database
  backend (SQLite vs PostgreSQL) and subchart deployment independently.
- Introduce db-secret.yaml template for Opaque Secret with assembled
  postgresql:// connection string injected via OPENSHELL_DB_URL env var.
- Add Bitnami PostgreSQL as optional subchart dependency keyed on
  postgres.deploy to prevent subchart deployment in external mode.
- Externalize JWT signing key file mode via sandboxJwt.secretDefaultMode
  with 0400 default matching upstream.
- Add validation guard for postgres.deploy=true without postgres.enabled.
- Add helm unit tests covering internal, external, URL-override, special
  character encoding, and misconfiguration error paths.
- Update README with Kubernetes and OpenShift install examples for
  bundled and external PostgreSQL configurations.
- Add helm dependency build to lint and unittest tasks.
@sauagarwa sauagarwa force-pushed the feat/helm-postgres-secret branch from 4ba4f67 to b71c3a7 Compare May 28, 2026 00:04
@sauagarwa sauagarwa changed the title feat(helm): add optional PostgreSQL backing store with Secret-based credentials feat(helm): add optional PostgreSQL backing store May 28, 2026
@sauagarwa sauagarwa marked this pull request as ready for review May 28, 2026 00:09
@TaylorMutch
Copy link
Copy Markdown
Collaborator

/ok to test b71c3a7

@TaylorMutch TaylorMutch added the test:e2e Requires end-to-end coverage label May 28, 2026
@github-actions
Copy link
Copy Markdown

Label test:e2e applied for b71c3a7. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

The helm-docs CI check failed because the Database backend section was
added directly to README.md instead of README.md.gotmpl. Move the
content to the template and regenerate so the check passes.
@TaylorMutch
Copy link
Copy Markdown
Collaborator

/ok to test 6a2b2ee

Comment thread deploy/helm/openshell/README.md.gotmpl Outdated
Comment thread deploy/helm/openshell/README.md.gotmpl Outdated
Comment on lines +123 to +125
helm install openshell oci://ghcr.io/nvidia/openshell/helm-chart --version <version> \
--set postgres.enabled=true \
--set postgres.external.url="postgres://user:pass@host:5432/db?sslmode=require"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't want to include the connection string directly, since it will contain plaintext credentials. We should prefer to use an existing kube secret with those credentials and not do any of our own templating.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment above.

Replace the inline db-url stringData pattern with a proper Secret
containing individual fields plus a uri key.  When postgres.deploy=true
the Bitnami service-binding secret is referenced directly; when
deploy=false users can supply postgres.external.existingSecret to
bring their own Secret, or let the chart generate one from the external
field values.

Also restructures the README database section for clarity, adds
helm-unittest coverage for the new secret resolution paths, and
fixes a markdown lint issue in the root README.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(helm): add optional PostgreSQL backing store for gateway persistence

2 participants