Allow use of certs for PG connections#347
Open
alaye-ms wants to merge 3 commits intodocumentdb:mainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to tighten PostgreSQL access controls for DocumentDB deployments by adjusting CNPG PgHBA rules (gateway-to-Postgres and physical replication) and by introducing new clusterReplication fields to reference TLS materials for replication, with corresponding CRD and playground updates.
Changes:
- Add
replicationTLSSecretandclientCASecretto the DocumentDBclusterReplicationAPI/CRDs and wire them into replication context. - Adjust CNPG cluster PgHBA defaults and physical replication external cluster configuration (including optional SSL cert/key selectors).
- Update playground deployment manifests/scripts to include cert-manager resources and propagate the new secret.
Reviewed changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| operator/src/internal/utils/replication_context.go | Carries new replication TLS/CA secret names through replication context. |
| operator/src/internal/controller/physical_replication.go | Configures external cluster replication user and optional CNPG certificate references; adds trust fallback PgHBA. |
| operator/src/internal/cnpg/cnpg_cluster.go | Updates default PgHBA rules for the CNPG cluster spec. |
| operator/src/api/preview/documentdb_types.go | Adds replicationTLSSecret / clientCASecret fields to the API type. |
| operator/src/config/crd/bases/documentdb.io_dbs.yaml | Adds the new fields and expands schemaVersion description text. |
| operator/documentdb-helm-chart/crds/documentdb.io_dbs.yaml | Mirrors CRD changes into Helm-shipped CRDs. |
| documentdb-playground/aks-fleet-deployment/documentdb-resource-crp.yaml | Adds cert-manager Issuer/Certificate and sets new replication secret fields; places the secret via Fleet. |
| documentdb-playground/aks-fleet-deployment/deploy-fleet-bicep.sh | Updates fleet deployment flow (adds hub CA env injection) and removes a join script patch step. |
| operator/src/internal/utils/pv_recovery.go | Formatting-only constant alignment. |
| operator/src/internal/utils/constants.go | Formatting-only constant alignment. |
| operator/src/internal/controller/documentdb_controller.go | Removes stray whitespace. |
| operator/src/internal/cnpg/cnpg_sync.go | Formatting-only alignment. |
Signed-off-by: Alexander Laye <alaye@microsoft.com>
Signed-off-by: Alexander Laye <alaye@microsoft.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
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.
This PR modifies the PgHBA so that the localhost connections (only within the pod, i.e. gateway to pg connections) are inherently trusted.
It also changes the replication connection which is used by local and remote replicas to require a cert. This is already used for local HA replicas and so no change is needed. However, for remote replicas the cert would need to be propagated. If provided, we pass in a cert from the user to validate client connections. If none is provided, we will set it to trust all instead. This should change to a default cert being provided upon the creation of our higher level, managing operator.