feat(runtime): add TLS support to TCP request plane - #10921
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 5277065 | Docs | Datadog PR Page | Give us feedback! |
WalkthroughAdds optional TLS support to the Dynamo runtime's TCP networking layer. A new ChangesTCP TLS Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
|
@grahamking — this is PR 1 from the TLS contribution request (#10809) you approved. Scope is TCP request-plane TLS only (no mTLS, no NATS). Happy to address any feedback! |
|
@walkoss Great! Can you start with the red tests. Look like clippy, maybe some others. Then I'll trigger the bigger unit tests, and schedule a review. Have you done a second round of agent work, where you ask it to simplify and remove unnecessary tests? Can usually slim down an agentic PR a fair bit like that. |
|
@grahamking thank you, all checks are green now. |
|
/ok to test 69d75de |
Encrypt frontend ↔ worker TCP connections using rustls (ring provider). TLS is opt-in via environment variables; existing deployments require no changes. New env vars (all optional): - DYN_TCP_TLS_CERT_PATH / DYN_TCP_TLS_KEY_PATH — server-side TLS - DYN_TCP_TLS_CA_CERT_PATH — client CA verification - DYN_TCP_TLS_INSECURE — skip cert verify (dev only) - DYN_TCP_TLS_SERVER_NAME — SNI override for IP addresses Key design decisions: - BoxRead/BoxWrite unify TLS and plaintext streams through all inner functions with zero branching after accept/connect - TLS acceptor spawned per-connection so the accept loop is never blocked; handshake timeout is 10s on both server and client - ClientConfig cached via OnceLock — built once, Arc-cloned per connection - Warn at startup when server/client TLS env vars are mismatched - Empty CA PEM detected at build time rather than failing at handshake Signed-off-by: Walid El Bouchikhi <walid.elbouchikhi@datadoghq.com>
Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>
ziqifan617
left a comment
There was a problem hiding this comment.
only sign off for dynamo-kv-memory-codeowners, since the related part is kvbm cargo.lock
Per review feedback from nealvaidya: TLS configuration is not tied to Kubernetes or the operator, so it belongs alongside the other runtime reference docs instead of under the Kubernetes Operator section. Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>
The request-plane page documents all other DYN_TCP_* environment variables, so add a pointer to the TLS reference for the encryption options as well (per review feedback). Signed-off-by: Walid <walid.elbouchikhi@datadoghq.com>
|
/ok to test 78ba919 |
|
/ok to test 5277065 |
dyn-3691-extract-shared-target-pid-cuda-customstorage-operation-layer * 'main' of https://github.com/ai-dynamo/dynamo: (50 commits) docs(cli): correct removed vLLM prefill-worker flag reference (#12581) docs(operator): reserve webhook Ignore for emergencies (#12563) ci(docs): make previews and checks match what actually publishes (#12339) refactor(vllm): organize custom encoder modules (#12416) feat(llm): Select reasoning output field via env var (#11464) feat(runtime): add TLS support to TCP request plane (#10921) fix: convert conditional disagg sglang warning to httperror 400 (#12578) feat(operator): add runtime feature gates (#12421) refactor(runtime): extract PushRouter transport seam behind StreamingDispatch trait (#12447) feat(replay): add deterministic canonical offline reports (#12363) build: bump ModelExpress to 0.5.0(OPS-7978) (#12455) fix(mocker): use logical KV tokens for decode timing (#12583) fix(examples): update Triton example for CUDA 13 + fix libdcgm copy (DYN-3697) (#12577) refactor(operator): implement composition-first DGD reconciliation (#12283) feat(frontend): add basetenkenizer backend (#12376) fix(profiler): configure rapid mocker without planner (#12573) docs(vllm): correct worker-role flags and document --kv-transfer-config (#12568) ci: add Kubernetes deploy test to nightly (#12090) fix(container): reuse pinned protoc in runtime image (#12535) feat(self-host): flip DYN_SELF_HOST_METADATA default to ON (gh-8749) (#11417) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Overview
Adds opt-in TLS encryption to the TCP request plane (frontend ↔ worker) using rustls with the
ringcrypto provider. When no TLS env vars are set the transport behaves exactly as before — fully backward compatible.Details
New shared helper (
lib/runtime/src/tls_utils.rs):server_tls_config(cert, key)— builds arustls::ServerConfigclient_tls_config(ca, insecure)— builds arustls::ClientConfig; emits a prominent warning wheninsecure=trueTCP server (
tcp/server.rs):DYN_TCP_TLS_CERT_PATH+DYN_TCP_TLS_KEY_PATHat startup to optionally build aTlsAcceptorBoxRead/BoxWrite(Box<dyn AsyncRead/AsyncWrite + Unpin + Send>) unify TLS and plaintext streams through all inner functions with no branching after acceptCallHomeHandshakeread and response-stream prologue read are also bounded by 10s timeoutsTCP client (
tcp/client.rs):OnceLock<Option<TlsConnector>>— connector built once from env on first connection,Arc-cloned per connectionconnect_and_split()upgrades to TLS when configured, with a 10s handshake timeoutDYN_TCP_TLS_SERVER_NAMEoverride for IP-addressed serversEnvironment variables (all optional, plaintext when unset):
DYN_TCP_TLS_CERT_PATHDYN_TCP_TLS_KEY_PATHDYN_TCP_TLS_CA_CERT_PATHDYN_TCP_TLS_INSECUREDYN_TCP_TLS_SERVER_NAMEWhere should the reviewer start?
lib/runtime/src/tls_utils.rs— new file, self-contained rustls helperslib/runtime/src/pipeline/network/tcp/server.rs—build_tls_acceptor()and the accept loop spawnlib/runtime/src/pipeline/network/tcp/client.rs—build_tls_connector_from_env()andconnect_and_split()lib/runtime/src/config/environment_names.rs— the 5 new constants undertcp_response_stream::tlsRelated Issues
Summary by CodeRabbit