fix(shard): dial multipoolers with mTLS when internalTLS is enabled - #626
Merged
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 4, 2026 12:04
99e6b3c to
fe00b88
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 4, 2026 14:39
e9d660d to
dc410aa
Compare
This comment has been minimized.
This comment has been minimized.
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 4, 2026 16:28
dc410aa to
0d12ab8
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/e2e |
This comment has been minimized.
This comment has been minimized.
|
✅ E2E tests success — View run |
This comment has been minimized.
This comment has been minimized.
niconosenzo
marked this pull request as ready for review
September 7, 2026 08:09
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 7, 2026 09:29
d541b44 to
629943b
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/e2e |
|
✅ E2E tests success — View run |
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
4 times, most recently
from
September 7, 2026 10:45
f026f17 to
cb626f6
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/e2e |
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 7, 2026 10:49
cb626f6 to
6473597
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/e2e |
This comment has been minimized.
This comment has been minimized.
|
❌ E2E tests failure — View run |
|
❌ E2E tests failure — View run |
Clusters with spec.internalTLS.enabled run multipooler with gRPC mTLS, but the operator continued dialing with insecure credentials. Status, GetBackups, and ReloadConfig calls were therefore rejected, leaving posture UNKNOWN and internal-TLS shards permanently Progressing. Have the MultigresCluster controller issue a cluster-owned operator client certificate using the cluster's configured issuer. Resolve and cache a TLS MultipoolerClient per cluster, using the exact multipooler.<cluster>.<namespace>.multigres.internal server identity with standard certificate verification. Refresh credentials periodically and rebuild clients only when the Secret changes. Keep the last working client during transient read or rotation failures, and retain replaced clients briefly so in-flight RPCs can finish. When credentials are not initially available, mark posture Unknown, emit PoolerClientUnavailable, and requeue after 10 seconds without triggering controller error backoff. Clusters without internal TLS continue using the shared insecure client. Signed-off-by: Nicolas Nosenzo <niconosenzo@gmail.com>
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 7, 2026 11:35
6473597 to
364b8a4
Compare
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
/e2e |
|
✅ E2E tests success — View run |
This comment has been minimized.
This comment has been minimized.
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 7, 2026 14:17
2b2c5d9 to
d466635
Compare
This comment has been minimized.
This comment has been minimized.
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 7, 2026 14:42
d466635 to
b953040
Compare
metav1.Time serializes at second precision through the fake client, so compare against a second-truncated fixture. The resolver-failure test also only drained one event before the second reconcile and then read the stale PoolEmpty warning; drain the recorder fully and assert on the PoolerClientUnavailable reason instead. Signed-off-by: Nicolas Nosenzo <niconosenzo@gmail.com>
niconosenzo
force-pushed
the
multipooler-mTLS-dial
branch
from
September 7, 2026 14:42
b953040 to
6694dbd
Compare
This comment has been minimized.
This comment has been minimized.
1 similar comment
🔬 Go Test Coverage ReportSummary
Status✅ PASS DetailShow New Coverage |
Contributor
Author
|
/e2e |
|
✅ E2E tests success — View run |
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.
Summary
Fix operator-to-multipooler RPCs for clusters with
spec.internalTLS.enabled: true.The operator now uses a cluster-specific mTLS client when calling
Status,GetBackups, andReloadConfig. Clusters without internal TLS continue using the existing insecure client.Root cause
InternalTLSclusters configure multipooler as an mTLS gRPC server. However, the shard controller continued using a client configured withinsecure.NewCredentials().Multipooler therefore rejected every operator RPC with a connection reset. This prevented the operator from observing pooler posture and backup state or applying reload-safe configuration changes. Posture remained unknown,
PostureConsistentbecameUnknown, and affected shards remainedProgressing.The posture debounce remains useful for transient observation failures, but it cannot recover from a permanently incompatible transport.
Solution
The
MultigresClustercertificate controller now creates a dedicated operator client certificate for every internal-TLS cluster.The certificate:
MultigresClusterand follows its lifecycle.The new
poolerclient.Resolver:MultipoolerClientper cluster.multipooler.<cluster>.<namespace>.multigres.internal.ResourceVersionchanges.This removes the need for runtime Certificate creation, issuer-specific certificate names, a separate issuer flag, and a
MultigresClusterAPI lookup during every resolution.Failure behavior
If the client certificate has not been issued yet, the shard controller:
PostureConsistent=Unknownwith reasonPoolerClientUnavailable.Progressing.PoolerClientUnavailablewarning event.The existing Healthy/no-primary safety guard remains unchanged.
Security
TLS verification is bound to the shard's exact cluster and namespace. A certificate for another cluster, even when signed by the same issuer, is rejected.
The operator client certificate is cluster-owned, so deletion or certificate reconciliation cleans it up together with the other cluster certificates.