feat(postgres): add TLS support via CNPG operator-managed certificates - #2686

Code Review

This pull request introduces a tri-state TLS configuration for the PostgreSQL package, enabling automatic TLS for external connections while allowing users to explicitly force the setting. The feedback highlights a documentation error regarding the certificate management mechanism and notes that the implementation for forcing TLS off is currently incomplete, as it lacks the necessary PostgreSQL parameter overrides. Additionally, a naming convention update was requested for the Go API types to correctly use the TLS initialism.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/postgres/README.md`:
- Around line 163-171: Update the README text to remove references to
cert-manager resources and the CNPG fields spec.certificates.serverCASecret and
spec.certificates.serverTLSSecret (these describe the old cert-manager flow) and
instead document the current operator-managed certificate flow: state that the
operator manages the CA and leaf certificates internally, that the chart no
longer creates <release>-selfsigned / <release>-ca / <release>-tls cert-manager
resources, and that when external TLS is enabled the operator injects only
spec.certificates.serverAltDNSNames; also adjust verification guidance to show
how to validate the operator-managed chain (e.g., inspect the CNPG Cluster
status/conditions or the operator logs) rather than looking for cert-manager
Secrets.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/postgres/Makefile`:
- Around line 5-8: Add a precondition check that validates the generator's
original output before mutating values.schema.json: use jq -e to assert that
.properties.tls.properties.enabled.type is exactly ["boolean"] (the expected
cozyvalues-gen output) and fail early if not, then perform the jq assignment
that changes it to ["boolean","null"], and finally keep the existing post-patch
equality check; refer to the jq expression
'.properties.tls.properties.enabled.type' and the file values.schema.json to
locate where to insert the precondition check.

---

Duplicate comments:
In `@packages/apps/postgres/README.md`:
- Around line 163-171: Update the README section that currently claims the chart
creates cert-manager resources and that the CNPG Cluster references
spec.certificates.serverCASecret and spec.certificates.serverTLSSecret: change
it to describe the operator-managed flow where the CNPG operator auto-generates
the CA and leaf certificates (so cert-manager resources like
<release>-selfsigned / <release>-ca / <release>-tls are not created by the
chart), and note that the chart only sets spec.certificates.serverAltDNSNames
when external TLS is enabled; replace references to
serverCASecret/serverTLSSecret with the operator auto-generation behavior and
the use of spec.certificates.serverAltDNSNames.

LGTM — postgres now extends the CNPG operator-managed cert with the external hostname via spec.certificates.serverAltDNSNames when external+TLS; tri-state tls.enabled is well-tested; the chart correctly does NOT expose the operator-managed <release>-ca Secret (which holds the CA private key) to tenant use access. Two non-blocking follow-ups below — neither is unique to this PR.

Business context: the earlier iteration tried supplying serverCASecret / serverTLSSecret from a chart-rendered cert-manager chain, which failed the CNPG admission webhook (missing ca.key — cert-manager writes the private key under tls.key, not ca.key). This PR shifts to operator-managed mode, where the chart only contributes the external hostname SAN. CNPG keeps its built-in TLS on the wire regardless of tls.enabled; the toggle controls SAN injection only, which is correctly documented in values.yaml and the new README section.

Verified:

• Tri-state evaluation in templates/_tls.tpl:11-17 uses kindIs "invalid" for null-detection and inherits from .Values.external. tests/tls_test.yaml covers all four cells of the truth table.
• templates/db.yaml:9 correctly gates the certificates block on both tlsEnabled and external — without an external hostname there is nothing to append to the SAN list, and the operator's default SAN set already covers the rw/r/ro DNS forms.
• templates/dashboard-resourcemap.yaml intentionally does NOT grant read access on <release>-ca. The operator-managed CA Secret carries both ca.crt and ca.key; exposing it to tenant use access would leak the CA private key (the inverse of the mariadb #2680 blocker). README directs users to retrieve the CA via kubectl get secret <release>-ca, which is the right path.
• README's sslmode=verify-full + sslrootcert=ca.crt example matches CNPG's documented client-trust workflow.
• All CodeRabbit / Gemini threads on this PR are resolved.

Non-blocking follow-ups (both apply across the TLS series, not just here):

1. packages/system/postgres-rd/cozyrds/postgres.yaml:42 lists postgres-{{ .name }}-ca under secrets.include, but the chart's dashboard-resources Role does not grant read access on it. The dashboard will advertise the secret name and tenant users will get RBAC-denied on read. The chart's restrictive RBAC is the right call — drop -ca from cozyrds's secrets.include so the discovery layer stops promising a read that the chart deliberately doesn't grant.

2. Both values.schema.json and the cozyrds openAPISchema use "type":["boolean","null"] for tls.enabled. pkg/registry/apps/application/rest.go:103-105 unmarshals the schema into apiextv1.JSONSchemaProps, whose Type field is a single string — I reproduced json: cannot unmarshal array into Go struct field JSONSchemaProps.properties.type of type string against v0.34.1. The error is logged and specSchema stays nil, so server-side defaulting via rest_defaulting.go is silently disabled for this app; Helm-side defaults still apply, so user-visible breakage is minimal but the app loses schema-driven defaulting. Same pattern in #2681 kafka and likely the rest of the TLS series. Worth a separate fix to switch to the apiextv1 idiom (nullable: true) or to drop type entirely on the tri-state property.

Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/postgres/tests/tls_test.yaml`:
- Around line 7-10: Update the commented test header to accurately reflect
behavior: change the `tls.enabled: true` bullet to note that while TLS is forced
on, chart-side SAN injection into `spec.certificates` only occurs when an
`external` hostname is present, and clarify that `tls.enabled: false` forces TLS
off regardless of `external`; reference the keys `tls.enabled`, `external`,
`spec.certificates`, and the phrase “chart-side SAN injection” so the comment
matches the actual assertions in the test.

In `@packages/apps/postgres/values.schema.json`:
- Around line 127-128: The schema description for the boolean property
incorrectly mentions “null” while the generated "type": "boolean" disallows
null; update the source text used by cozyvalues-gen to replace “omitted (null)”
with “omitted/absent” (or similar non-null wording) for that property’s
description, then re-run the cozyvalues-gen pipeline and `make generate` so the
values.schema.json is regenerated consistently; locate the original description
string in the values source used by cozyvalues-gen (the property whose generated
schema contains the shown description) and change only the wording, not the
schema type.

LGTM on the new SHA. Both non-blocking follow-ups from my prior LGTM are now closed:

• edcb656bb fix(postgres): drop CA Secret from cozyrds — tenant retrieves ca.crt from user-credentials Secret — packages/system/postgres-rd/cozyrds/postgres.yaml now lists only postgres-{{ .name }}-credentials under secrets.include, and packages/apps/postgres/templates/dashboard-resourcemap.yaml matches (only <release>-credentials). The dashboard no longer advertises a CA Secret that RBAC didn't actually grant, AND CNPG's operator-managed <release>-ca (which would carry ca.key) stays unexposed to tenant use. Both layers consistent now.
• 184be891d refactor(postgres): drop null from tls.enabled schema, use kindIs invalid pattern — same schema convention as the rest of the batch; "type":["boolean","null"] would fail apiextv1.JSONSchemaProps unmarshal. Resolved.

d63f82ed9 build(postgres): assert schema shape before patching tls.enabled.type adds a defensive grep before the jq patch in the Makefile so a future cozyvalues-gen format change fails the build loud instead of producing a malformed schema silently. Good belt-and-suspenders.

Operator-managed CNPG chain still owns the cert lifecycle end-to-end; chart only injects external-hostname SAN when external+TLS — same shape as before, just cleaner cozyrds advertisement.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/postgres/README.md`:
- Line 217: The `version` parameter is incorrectly listed under the TLS table in
packages/apps/postgres README; update the source that generates the `##
Parameters` block so `version` is defined in the general/app parameters section
(not in the TLS group), then run the doc generator (cozyvalues-gen via `make
generate`) to regenerate the `## Parameters` block in README.md; target the
parameter named `version` and the README's `## Parameters` generation logic so
the entry appears in the general parameters table rather than the TLS table.

LGTM — operator-managed CNPG TLS is correctly wired, the tri-state toggle is honestly documented as controlling SAN injection rather than on-the-wire TLS, generated artifacts are in sync, and tests pass.

Business context: managed Postgres on CNPG already serves TLS by default; this PR's tls.enabled tri-state does not turn TLS on or off — it controls whether the chart appends the external hostname (<release>.<_namespace.host>) to the operator-generated server cert via spec.certificates.serverAltDNSNames, replacing an earlier cert-manager chain that failed CNPG's admission webhook (missing ca.key).

Non-blocking follow-ups

1. _namespace.host is dereferenced unconditionally when external=true and tls.enabled is unset (packages/apps/postgres/templates/db.yaml:25). This matches the platform contract every tenant app relies on (harbor, kubernetes, vpn all read .Values._namespace.host unconditionally), and the chart cannot render under bare helm template regardless — not a real-path regression. The genuine residual: when a tenant has no apex domain, _namespace.host is empty and the SAN renders as <release>. (trailing dot) rather than failing. The gateway package fails on empty host; a required/fail guard here would match that precedent.
2. The field is named tls.enabled but its effect is "inject external hostname into the cert SAN", not an on/off TLS switch. Now thoroughly documented (values.yaml, schema, README each spell out that CNPG keeps TLS on the wire regardless and point to postgresql.parameters.ssl=off for a true disable). Noting as a design observation.