fix(namespace): reload rotated mTLS certificates in the REST client - #8456
Open
maswin wants to merge 1 commit into
Open
fix(namespace): reload rotated mTLS certificates in the REST client#8456maswin wants to merge 1 commit into
maswin wants to merge 1 commit into
Conversation
`RestNamespace::from_builder` read the mTLS certificate, key and CA once and baked them into a long-lived `reqwest::Client`. A `reqwest::Identity` cannot be swapped inside a live client, so a process kept presenting the certificate it read at startup forever. Environments that rotate short-lived certificates on disk - SPIFFE/Istio, Vault PKI, cert-manager, Netflix Metatron - therefore broke once the loaded certificate passed its `notAfter`, with every request failing on a received `CertificateExpired` alert until the process restarted. The client is now published through an `ArcSwap` and rebuilt when the content of the configured PEM files changes. The check runs on the request path, gated by `tls.reload_interval_seconds` (default 300, `0` disables reloading), and compares the digest of the file content rather than modification times, since rotation is commonly an atomic rename or a symlink swap. Unchanged content leaves the client and its connection pool untouched, and material that fails to load or build keeps the current client instead of downgrading it mid-rotation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
The request-path reload, content fingerprinting, last-known-good fallback, and atomic client swap address the stale-identity root cause without replaying requests. The implementation also rejects mismatched certificate/key generations before publication.
The remaining risk is verification breadth: current tests prove that rotated PEM content rebuilds the client, but they do not exercise a peer-observed mTLS handshake, CA-only rotation, or a concurrent cutover. A local mTLS rotation test would make that operational contract durable.
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.
Problem
Trino x Lance plugin started throwing
CertificateExpiredexception when trying to talk to iceberg rest catalog and the certs in trino co-ordinator instance got rotated.RestNamespace::from_builderreads the mTLS certificate, key and CA once at constructionand bakes them into a single long-lived
reqwest::Client. Areqwest::Identitycannot behot-swapped inside a live client, so the process keeps presenting the certificate it read at
startup forever.
Any environment that rotates short-lived certificates on disk therefore breaks once the
loaded certificate passes its
notAfter. The server rejects the handshake and the clientsees a received TLS alert:
(
receivedmatters: the peer is rejecting our client cert. A stale CA bundle or bad servercert would surface locally as
InvalidCertificate(Expired)/UnknownIssuer.)Fix
RestClientnow holds the client in anArcSwap<reqwest::Client>and hands it out by value(cheap —
reqwest::Clientis a handle around anArc). Alongside it, the cert/key/CA paths,the digest of the last-loaded content and the last-checked
Instantare kept so the clientcan be rebuilt.
The check is lazy, on the request path, gated by an interval:
the content changed. Content, not mtime: rotation is often a symlink swap or an atomic
rename. A digest keeps the comparison cheap without retaining private key bytes.
authenticated with the old certificate) and exactly why it is gated on a real change.
check, so a truncated read mid-rotation can't replace a working identity with one the
server would reject.
std::fs::readat most once per interval.arc-swapwas already a workspace dependency; no new dependencies.New property
tls.reload_interval_seconds, parsed alongside the othertls.*keys, with amatching builder setter.
0disables reloading. Properties pass through the Python and Javabindings opaquely, so both pick this up without binding changes; the
tls.*list inRestNamespace's javadoc is updated.Verification