Feat: Allow certs for PostgreSQL connections

[alaye-ms]

Removes the permissive pg_hba rules that allowed passwordless PostgreSQL access from any address and replaces them with least-privilege rules plus optional certificate-based mutual TLS for replication. Adds a new spec.tls.postgres field so operators can supply their own CA/certificate Secrets for PostgreSQL server and replication connections.

Motivation
Previously the operator emitted:

Anyone with pod-network reachability could connect to PostgreSQL unauthenticated. This PR closes that gap while keeping the gateway sidecar (loopback) and cross-region replication working.

Changes
PostgreSQL access hardening (cnpg_cluster.go)

Base pg_hba is now host all all localhost trust (gateway sidecar over pod loopback) + hostssl replication streaming_replica all cert.
New spec.tls.postgres API (documentdb_types.go)

spec.tls.postgres is now backed by CNPG's CertificatesConfiguration (replaces the empty PostgresTLS placeholder). Supports serverTLSSecret/serverCASecret and replicationTLSSecret/clientCASecret.
A CEL XValidation rule enforces pairing invariants: server and replication secrets must each be provided together, and serverTLSSecret requires replicationTLSSecret.
Certificates are propagated to the CNPG Cluster and diffed/patched in cnpg_sync.go (add/replace/remove).
Replication TLS (physical_replication.go)

When replication client certs are provided, cross-region externalClusters connect with sslmode=require (or verify-full when a server CA is present) and present the streaming_replica client certificate.
When spec.tls.postgres is unset in multi-region deployments, replication falls back to host replication streaming_replica all trust, relying on network-layer security (e.g. an Istio/service-mesh mTLS boundary).
Reworked replication patch logic to diff externalClusters, pg_hba, and synchronous independently (proper add/replace/remove ops) instead of only reacting to cluster-name changes.
Docs & playground

New "PostgreSQL certificates" section in the TLS configuration guide; multi-region setup docs updated.
deploy-multi-region.sh / deploy-fleet-bicep.sh generate replication certs with cross-region SANs; new documentdb-resource-crp.yaml propagates cert Secrets across the fleet.
CHANGELOG entries under Security and Major Features.
Tests

New physical_replication_test.go and certificate_controller_test.go coverage; updated CNPG cluster and backup controller tests.

[Copilot]

Pull request overview

This PR hardens PostgreSQL connectivity for the DocumentDB Kubernetes Operator by tightening default pg_hba rules and introducing an API surface (spec.tls.postgres) to optionally supply CloudNative-PG PostgreSQL server/replication certificate Secrets—supporting mTLS for cross-cluster replication while keeping existing gateway loopback behavior.

Changes:

• Replace permissive PostgreSQL pg_hba entries with least-privilege defaults and replication rules (streaming_replica + cert/TLS-aware behavior).
• Introduce spec.tls.postgres backed by CloudNative-PG CertificatesConfiguration, with CRD CEL validation to enforce secret pairing invariants.
• Propagate certificate configuration into CNPG Cluster spec (including patch/diff support) and update multi-region replication connection parameters to use TLS client certs when provided; update docs/playground and tests accordingly.

Reviewed changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
operator/src/internal/controller/physical_replication.go	Builds external cluster connection parameters with optional TLS client cert/root CA and updates replication patch logic.
operator/src/internal/controller/physical_replication_test.go	Adds tests covering external cluster detail patching and Postgres cert-related replication behavior.
operator/src/internal/controller/certificate_controller_test.go	Extends cert controller tests to ensure Postgres cert resources are not managed/created unexpectedly.
operator/src/internal/controller/backup_controller_test.go	Test formatting/indentation update (no behavioral change).
operator/src/internal/cnpg/cnpg_sync.go	Adds CNPG Cluster patching support for /spec/certificates changes.
operator/src/internal/cnpg/cnpg_patch.go	Adds JSON patch path constants for certificates and pg_hba.
operator/src/internal/cnpg/cnpg_cluster.go	Sets CNPG spec.certificates from spec.tls.postgres and hardens default pg_hba.
operator/src/internal/cnpg/cnpg_cluster_test.go	Updates/extends tests for Postgres certificate config presence and pg_hba expectations.
operator/src/config/crd/bases/documentdb.io_dbs.yaml	Updates generated CRD schema for spec.tls.postgres and adds CEL validation rule.
operator/src/api/preview/zz_generated.deepcopy.go	Regenerates deep-copies to support CNPG CertificatesConfiguration on TLS spec.
operator/src/api/preview/documentdb_types.go	Replaces placeholder PostgresTLS with CNPG CertificatesConfiguration + XValidation.
operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml	Mirrors CRD schema changes for Helm-managed CRDs.
documentdb-playground/aks-fleet-deployment/documentdb-resource-crp.yaml	Adds cert-manager resources and fleet Secret propagation for cross-region replication TLS.
documentdb-playground/aks-fleet-deployment/documentdb-operator-crp.yaml	Adjusts fleet placement affinity for operator resources.
documentdb-playground/aks-fleet-deployment/deploy-multi-region.sh	Adds SAN computation + more robust apply logic with webhook readiness/retries.
documentdb-playground/aks-fleet-deployment/deploy-fleet-bicep.sh	Hardens shell settings/quoting and pins external repos to tags; adds RBAC bindings and hub CA handling.
docs/operator-public-documentation/preview/multi-region-deployment/setup.md	Documents replication TLS options and SAN requirements; updates backup link.
docs/operator-public-documentation/preview/multi-region-deployment/overview.md	Updates replication traffic guidance to reference spec.tls.postgres behavior.
docs/operator-public-documentation/preview/getting-started/deploy-on-aks.md	Fixes/updates doc links for storage and TLS configuration.
docs/operator-public-documentation/preview/configuration/tls.md	Adds PostgreSQL certificate configuration documentation and examples.
docs/operator-public-documentation/preview/architecture/overview.md	Updates backup doc link location.
docs/operator-public-documentation/preview/api-reference.md	Updates API reference to reflect CertificatesConfiguration and removes placeholder PostgresTLS.
CHANGELOG.md	Adds security and feature entries describing pg_hba hardening and new Postgres TLS support.

Files not reviewed (1)

• operator/src/api/preview/zz_generated.deepcopy.go: Generated file

[documentdb-triage-tool]

🤖 Auto-triaged by documentdb-triage-tool.

Applied: documentation, go, ecosystem, test, enhancement
Project fields suggested: Component docs · Priority P0 · Effort XL · Status Needs Review
Confidence: 0.95 (deterministic)

Reasoning

component from path globs (docs, controllers, playground, test, api, manifests); P0 keyword "security" in body; effort from diff stats (1190+172 LOC, 24 files)

If a label is wrong, remove it manually and ping @patty-chow so the rules can be tuned. The bot will not re-label items that already have component labels.

[WentingWu666666]

Thanks for tackling replication TLS — the hardened pg_hba and cross-cluster wiring look solid. I have a few questions about the public API shape and defaults that I think are worth thinking through before this locks in. None of these are blockers from me — more things I'd love your take on.

1. Is spec.tls.postgres the right name?

Its siblings under spec.tls (gateway, globalEndpoints) name the communication surface, whereas postgres names the software. As far as I can tell this field only secures replication (intra-cluster + cross-region) — apps reach Postgres through the gateway over loopback trust, not this path.

Worth considering: could a user read tls.postgres as securing their app → DB traffic and get a false sense of coverage? Would something like spec.tls.replication describe the actual scope more honestly? Curious whether you see a future where this widens beyond replication (e.g. direct Postgres access), which might justify keeping postgres.

2. Should we expose cnpgv1.CertificatesConfiguration directly?

The API currently surfaces the CNPG type as-is (4 fields: serverTLSSecret, serverCASecret, replicationTLSSecret, clientCASecret). Two things worth weighing:

• Does coupling our public CRD to a CNPG struct risk a CNPG-side change becoming a breaking change to our API?

• Since a kubernetes.io/tls Secret already bundles cert + key + ca.crt, could the four fields collapse to two? Your own example reuses one secret name per pair, which made me wonder if the split is redundant:

  spec:
    tls:
      replication:
        serverCertSecret: <secret>   # → serverTLSSecret + serverCASecret
        clientCertSecret: <secret>   # → replicationTLSSecret + clientCASecret

  Would deriving the CNPG fields from two bundled secrets be simpler for users?

3. What happens on the partial-cert path?

When only a client cert is present, the code uses sslmode=require, which encrypts the connection but doesn't verify the server's identity. That leaves it open to a man-in-the-middle (MITM) attack — an attacker on the network between the replica and primary can impersonate the primary, terminate the replica's TLS, and relay to the real primary while reading or altering the replication stream in between. The connection is encrypted, but potentially to the attacker. This path is currently described as "mTLS" in the docs.

Worth thinking through: in bidirectional replication (either cluster can become primary after failover), each cluster has to act as both server and client — so is there actually any coherent single-sided config? If not, would it make sense to require the two secrets as a pair (both-or-neither) so we always land on verify-full (which verifies the server cert against a trusted CA and closes the MITM gap)? And should the "mTLS" wording be reserved for the verify-full path only?

Note the two certs generally aren't interchangeable: the client cert needs CN=streaming_replica (Postgres maps the CN to that role) plus clientAuth EKU, while the server cert needs the endpoint host in its SAN plus serverAuth EKU. So a normal server cert can't double as the client cert. A single secret can serve both only if it's a purpose-built cert (CN=streaming_replica, host in SAN, both EKUs) — which is exactly why the two-field shape (serverCertSecret + clientCertSecret, each mapped by the operator onto the CNPG fields) fits the underlying reality better than a single field.

4. Is the plaintext fallback intentional?

When multi-region replication is set up without certs, pg_hba becomes host replication streaming_replica all trust cluster-wide — plaintext and unauthenticated across regions, and it also seems to relax intra-cluster enforcement. The code comment suggests this assumes an Istio mesh.

Is that assumption safe as a silent default? Would it be worth making the mesh-only path an explicit opt-in so the security posture is visible in the spec rather than implied by omission?

---

Happy to pair on any of these or prototype the type changes if that helps. Mostly wanted to surface them while the API is still easy to shape.

[WentingWu666666]

Duplicate — please see the request-changes review below.

[WentingWu666666]

Marking this as request-changes — not because I think any specific answer is required, but because these four points concern the public API surface (hard to change once released) and the security posture of cross-region replication, so I'd like us to work through them deliberately before merging rather than after.

I've laid out the details and rationale in my earlier comment. The four things I'd ask you to think carefully about:

1. Naming — does spec.tls.postgres describe the real scope (replication only), or could it imply app→DB coverage it doesn't provide?
2. CNPG type leakage — is exposing cnpgv1.CertificatesConfiguration directly in our CRD a coupling we want to commit to long-term?
3. verify-full guarantee — is the sslmode=require partial-cert path (MITM-exposed) something we should ship and call "mTLS," or should the two secrets be required as a pair?
4. Silent plaintext default — is the cert-less trust fallback (and its Istio-mesh assumption) safe as an implicit default, or should it be an explicit opt-in?

No rush to agree with my leanings — I'd just like your reasoning on each before we lock the API in. Happy to pair or prototype if that helps move it forward.

Duplicate — superseded by the request-changes review directly below.

[alaye-ms]

To address your questions from the comment above:

1. The spec.tls.postgres name was already present as a placeholder in the code, and I think changing that name is out of scope for this PR, though I do agree with the sentiment.

2. A change to the CNPG implementation of certificates would in either case require a change from our side due to the direct implementation. I think simple pass throughs are better here and should cnpg make a change, we could address whether to keep our current surface or add an extra layer then.

3. Good catch on the wording, I'll make sure to edit the docs to make clear that the client-only certs are not mutual TLS. As far as removing the feature altogether, I don't think that's needed once the documentation accurately reflects the reality of the path.

4. I think explicit opt-in for the mesh path is a good idea, I'll add that now.

[WentingWu666666]

Approving this — the functionality is correct and the security items from my earlier review (the mTLS wording and the explicit disableTLS opt-in) are addressed. Nice work. I have two non-blocking things I'd love the team to keep in mind.

1. Let's rethink the tls.postgres shape while it's still easy to change.

You mentioned tls.postgres was already there as a placeholder, so renaming feels out of scope. I'd gently flip that around: because it was only an empty placeholder before this PR, this PR is the first time it gets a real shape and real fields. Nobody depends on it yet, so right now is the cheapest moment to get the name and structure right — changing it later, once people use it, is a breaking change.

Two things I'd love you to consider:

• It doesn't match our own conventions. Most of our API is DocumentDB-native — we rarely expose CNPG types directly. tls.postgres is an exception: it passes CNPG's CertificatesConfiguration straight through. It's also different from its sibling tls.gateway, which already uses a clean, native shape. And DocumentDB isn't CNPG — users reach us through the gateway (Mongo protocol) and cross-cluster replication, not as generic Postgres clients, so the surface we expose should reflect that.

• A simpler shape. Since a kubernetes.io/tls secret already bundles cert + key + ca.crt, we could collapse the four CNPG fields into two of our own:

  spec:
    tls:
      replication:
        serverCertSecret: <secret>   # -> serverTLSSecret + serverCASecret
        clientCertSecret: <secret>   # -> replicationTLSSecret + clientCASecret

  The operator maps these onto CNPG under the hood. (One open question worth a quick think: keep the CA bundled inside each secret like above, or expose a shared caSecret separately — since the CA is really the one shared piece across clusters.)

Since we're in preview and don't have many customers yet, it's totally fine to ship this as-is for now. I just think it's worth revisiting before GA. Could we open a tracking issue so it isn't forgotten?

2. Add an e2e test for the cert path.

The unit tests are solid, but they only check that the operator writes the right CNPG spec — they don't prove replication actually connects over TLS. The e2e replication path currently sets disableTLS: true, so the cert paths this PR adds aren't exercised end-to-end. It would be good to add an e2e test (even single-cluster) covering the cert path — ideally a positive case (certs provided -> replication healthy) and a negative case (bad cert -> replication fails).

Thanks again for the work here — happy to help with either follow-up.

[alaye-ms]

Added tracking issues:
#427
#428

[xgerman]

Reviewed and tested locally on this branch: go build, go test ./... (all packages), go vet, gofmt -l, and make manifests generate (no diff) all pass. The change correctly closes the trust-everywhere pg_hba gap for new clusters, the CEL pairing invariants are sound, and splitting externalClusters/pg_hba/synchronous into independent add/replace/remove ops is a real improvement over the old name-only trigger.

One functional gap worth addressing before merge (Major 1 below), plus a couple of lower-severity notes. Details inline.

[xgerman]

🟠 Major — the hardened pg_hba never reaches existing non-replication clusters on upgrade. SyncCnpgCluster diffs PostgresConfiguration.Parameters (just above) and now Certificates, but not PostgresConfiguration.PgHBA. The only place that patches pg_hba is getReplicasChangePatchOps (physical_replication.go:541), reachable solely via syncReplicationChanges, which early-returns when ReplicaCluster == nil (physical_replication.go:378-380).

Net effect: a single-instance cluster created before this upgrade keeps host all all 0.0.0.0/0 trust indefinitely — the headline vulnerability stays open for existing installs. New clusters are fine (rules are set at bootstrap in buildPostgresConfiguration).

Suggest adding a PostgresConfiguration.PgHBA diff/patch here (mirroring the Parameters block, using PatchPathPostgresPgHBA) so all clusters converge regardless of the replication path.

[xgerman]

🟡 Minor — verify-full is auto-selected whenever a server CA is present. verify-full validates the server hostname against the certificate SAN, but the connection host is the internal -rw.<ns>.svc (or fleet) DNS name. User-provided server certs whose SANs don't cover that exact host will make replication silently fail with no operator-side signal. The deploy scripts generate correct SANs, but user-supplied serverCASecret/serverTLSSecret won't necessarily. Consider making sslmode configurable (or documenting the exact SAN requirement prominently next to the API field).

[xgerman]

🟡 Minor — this reflect.DeepEqual on the full ExternalClusters slice replaces the old order-insensitive name-set compare. Ordering is deterministic here (slice-backed OtherCNPGClusterNames), so no churn today. The residual risk is server-side defaulting: if CNPG ever echoes back a normalized/defaulted ExternalCluster form (e.g. an added connection param), current vs freshly-built desired would differ every reconcile → perpetual replace patches. Low risk given current CNPG behavior, but a field-scoped compare (name + connection params + SSL selectors) would be more robust.

[xgerman]

🟢 Nitpick — localhost here is matched by Postgres as a hostname (reverse-DNS lookup on the client IP), not a literal address. It works inside the pod, but host all all 127.0.0.1/32 trust + host all all ::1/128 trust is the more explicit/robust form and avoids depending on reverse-DNS resolution.