Skip to content

fix(namespace): reload rotated mTLS certificates in the REST client - #8456

Open
maswin wants to merge 1 commit into
lance-format:mainfrom
maswin:feat/rest-namespace-tls-cert-reload
Open

fix(namespace): reload rotated mTLS certificates in the REST client#8456
maswin wants to merge 1 commit into
lance-format:mainfrom
maswin:feat/rest-namespace-tls-cert-reload

Conversation

@maswin

@maswin maswin commented Aug 10, 2026

Copy link
Copy Markdown

Problem

Trino x Lance plugin started throwing CertificateExpired exception when trying to talk to iceberg rest catalog and the certs in trino co-ordinator instance got rotated.

RestNamespace::from_builder reads the mTLS certificate, key and CA once at construction
and bakes them into a single long-lived reqwest::Client. A reqwest::Identity cannot be
hot-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 client
sees a received TLS alert:

reqwest::Error { kind: Request, url: "https://<catalog-host>:7004/api/lance/<ns>/v1/namespace/%24/list?delimiter=%24",
  source: hyper::Error(Io, Custom { kind: InvalidData, error: "received fatal alert: CertificateExpired" }) }

(received matters: the peer is rejecting our client cert. A stale CA bundle or bad server
cert would surface locally as InvalidCertificate(Expired) / UnknownIssuer.)

Fix

RestClient now holds the client in an ArcSwap<reqwest::Client> and hands it out by value
(cheap — reqwest::Client is a handle around an Arc). Alongside it, the cert/key/CA paths,
the digest of the last-loaded content and the last-checked Instant are kept so the client
can be rebuilt.

The check is lazy, on the request path, gated by an interval:

  • If the interval has elapsed, the PEM files are re-read and the client is rebuilt only if
    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.
  • Rebuilding drops the connection pool, which is correct (pooled connections were
    authenticated with the old certificate) and exactly why it is gated on a real change.
  • Material that fails to read or build keeps the current client and retries on the next
    check, so a truncated read mid-rotation can't replace a working identity with one the
    server would reject.
  • No background thread or task. A few-KB std::fs::read at most once per interval.
  • arc-swap was already a workspace dependency; no new dependencies.

New property tls.reload_interval_seconds, parsed alongside the other tls.* keys, with a
matching builder setter. 0 disables reloading. Properties pass through the Python and Java
bindings opaquely, so both pick this up without binding changes; the tls.* list in
RestNamespace's javadoc is updated.

Verification

cargo build -p lance-namespace -p lance-namespace-impls --features rest -p lance
cargo test -p lance-namespace-impls --features rest
cargo fmt --all
cargo clippy --all --tests --benches -- -D warnings

`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>
@github-actions github-actions Bot added bug Something isn't working A-python Python bindings A-java Java bindings + JNI A-deps Dependency updates A-namespace Namespace impls and removed bug Something isn't working labels Aug 10, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Gate recommendation: approve with a non-blocking risk.

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.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-deps Dependency updates A-java Java bindings + JNI A-namespace Namespace impls A-python Python bindings bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant