Conversation
Signed-off-by: Emilien Macchi <emacchi@redhat.com>
Signed-off-by: Emilien Macchi <emacchi@redhat.com>
Signed-off-by: Emilien Macchi <emacchi@redhat.com>
…peer CA Signed-off-by: Emilien Macchi <emacchi@redhat.com>
Signed-off-by: Emilien Macchi <emacchi@redhat.com>
Signed-off-by: Emilien Macchi <emacchi@redhat.com>
Signed-off-by: Emilien Macchi <emacchi@redhat.com>
EmilienM
requested review from
a team,
derekwaynecarr,
mrunalp and
sjenning
as code owners
September 29, 2026 02:26
This branch has not been deployed
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
Gateway replicas on PostgreSQL relay whole supervisor sessions to each other and authenticate with a projected ServiceAccount token. Until now the dialer picked TLS from the scheme the remote owner advertised, fell back to platform trust roots when no peer CA was set, and a plaintext gateway accepted
http://peers with only a warning. This PR makes peer transport fail closed: HTTPS with an explicitly pinned peer CA, or plaintext only through one explicit, loud opt-out on a plaintext gateway. The chart refuses the insecure combination at render time. The PR also documents which operations work through a non-owner replica and adds an HA e2e suite that drives them through non-owner replicas across scale-up, scale-down, graceful and forced owner loss, and a rolling restart, over HTTPS peers.The PR was drafted and adversarially reviewed by Opus 5.5 through multi-agent workflows (research, planning, per-commit review, and a final review gate). The design section and "How this was built" below describe that process and what it changed.
Related Issue
Closes #3529. It addresses all seven acceptance criteria, with one deliberate deviation on AC1 described under Design: plaintext is rejected by default, and one explicit opt-out remains for plaintext gateways (edge TLS termination, dev and CI).
The issue has no acceptance or agent-workflow labels; it is on the 0.1.5 milestone. Implementation was directly requested; labels are unchanged.
Breaking changes and upgrade
server.disableTls=truestops rendering. Setserver.peer.allowInsecureTransport=trueto keep the current behavior on a trusted network. Do not turn on gateway TLS in the same upgrade: existing sandboxes keep the plaintext endpoint they received at creation and would be stranded.ca.crtin that Secret, and the server certificate must chain to it and carry<fullname>.<namespace>.svc.cluster.local. Otherwise the gateway refuses to start. Chart-generated PKI and cert-manager installs need no change.http://peer endpoint needOPENSHELL_PEER_ALLOW_INSECURE_TRANSPORT=trueor TLS. HTTPS peers needOPENSHELL_PEER_TLS_CA_FILE; platform roots are no longer used.Release note: Gateways on PostgreSQL now require HTTPS peer transport with an explicit peer CA. Plaintext peers need
server.peer.allowInsecureTransport=true(Helm) orOPENSHELL_PEER_ALLOW_INSECURE_TRANSPORT=true.Design
Where enforcement lives. Every peer RPC and relay goes through one dialer,
PeerRouteCache::channel→build_peer_channel. The newpeer_transportmodule owns an immutablePeerTransportPolicy, installed once at startup, andbuild_peer_channelchecks it before any I/O. Enforcing at startup alone would not help: the replica that sends the token is the dialer, and it dials endpoints that other replicas wrote into the owner records (for example old plaintext pods during adisableTlsflip). Rejecting on the receiving side cannot help either, because by then the token is already on the wire.Dial rule.
https://requiresOPENSHELL_PEER_TLS_CA_FILE; the native-roots fallback is deleted. That fallback mattered more than it looked: rustls-native-certs replaces the platform store withSSL_CERT_FILE, which the chart sets for the OIDC CA, so peers could end up trusting only the OIDC CA.http://is dialed only with the opt-out on a gateway that itself serves plaintext.unix:,local://and anything else are refused. Unit-test builds (cfg(test)) also dial numeric loopback so in-process fake peers keep working; production builds never do.Startup. When peer routing is expected (PostgreSQL plus a peer endpoint, even with one replica, because a Deployment rollout surges a second pod), startup rejects a plaintext own endpoint without the opt-out. It parses the CA (absent, empty and non-PEM all fail), checks the client cert/key pair and the server name, and requires a peer client identity when the listener requires client certificates. It also verifies that this gateway's own server certificate, the one SNI actually selects for peers, would pass peer verification. Every replica mounts the same Secret, so a bad custom certificate becomes a clear boot error instead of a cross-replica handshake failure. Expired or not-yet-valid certificates only warn, and are still checked for chain and name inside their validity window, so certificate expiry never adds a crash class.
Why an opt-out instead of mandatory pod TLS. With
server.disableTls=truebehind edge TLS, the ingress-to-pod hop and the supervisor-to-gateway hop already carry sandbox JWTs, user bearer tokens and relay bytes in cleartext on the same pod network. Mandatory peer TLS without mandatory pod TLS buys little, and mandatory pod TLS would break the documented edge-termination recipe and single-replica PostgreSQL installs. The opt-out follows theproxy_auth_allow_insecureprecedent: rejected by default, honored only on a plaintext gateway and only forhttp://, never derived fromdisableTls, ignored with a warning on a TLS gateway, and logged as a warning at every startup on PostgreSQL.Relays fail fast on local refusals.
open_routed_relay_with_messageretried every error for 15 seconds, so a policy refusal would have looked like a stall ending insupervisor session not connected. One new match arm returns immediately when the local policy refuses the owner endpoint. It keys on the immutable local policy (refuses_dial), never on a status code a remote peer returned, so a peer cannot trigger it. Handshake failures (wrong CA, wrong name) keep retrying, since a certificate can be rotated, and the dialer now reports the rustls cause (UnknownIssuer,certificate not valid for name) instead of tonic's bare "transport error".Peer identity. Two layers, now both tested. Transport: the owner certificate must chain to the pinned CA and carry the configured server name, so the token is never sent to an unverified peer. Application: the receiver checks TokenReview, audience, ServiceAccount, live pod UID, release labels,
x-openshell-peer-replicaagainst the token's pod, and PeerRelay requester against the principal. The peer TLS client certificate is not an identity: it is the sharedopenshell-clientcertificate, and it is absent withenableMtls=false.Rejected alternatives. A dedicated peer listener (large, leaves the other hops plaintext, breaks #3661's redirect targeting). Mandatory
x-openshell-peer-replica(adds nothing, since the principal comes from TokenReview). Envoy + BackendTLSPolicy for the TLS HA lane (TLS pods behind a plaintext Envoy listener break SSH-based upload, download and forward, becauseCreateSshSessionderives the scheme from pod TLS; it also needs Gateway API CRDs only the Envoy chart installs and would not exercise the peer client certificate).Changes
feat(server)!). Newcrates/openshell-server/src/peer_transport.rs(policy, dial rule, startup validation, SNI-aware own-certificate check, error chains, IPv6 endpoint bracketing for the chart's$(OPENSHELL_POD_IP)).PeerTlsClientConfigloses its native-roots branch and gains CA, identity and server-name checks.PeerRouteCachecarries the policy;build_peer_channelenforces it. New envOPENSHELL_PEER_ALLOW_INSECURE_TRANSPORT.feat(server)). One arm in the relay retry loop,PeerTransportPolicy::preflight, and in-process two-replica TLS tests (PeerRelay, endpoint-status and provider-readiness forwarding over verified TLS with client certificates; replica and requester mismatch; missing identity).test(server)).validate_live_peer_podextracted as a pure function with unchanged order, codes and messages; tests for audience, pod binding, pod UID, pod ServiceAccount, release labels and replica header mismatches.feat(helm)!). New_peer-transport.tplguard: PostgreSQL (externalDbSecretor apostgres:///postgresql://dbUrl) plusdisableTlswithoutserver.peer.allowInsecureTransportfails to render, including--reuse-valuesupgrades; non-boolean values fail.OPENSHELL_PEER_TLS_CA_FILEis always wired when TLS is on. The opt-out env renders only on plaintext gateways;ci/values-skaffold.yamlsets it for dev and CI. Newci/values-high-availability-tls.yaml. Newtests/peer_transport_test.yaml(23 tests) also asserts the whole peer env block and the peer client TLS mount.test(e2e)). Newkubernetes_ha_operationse2e binary. One long-lived sandbox goes through setup, baseline, scale-up, scale-down, graceful owner loss, forced owner loss (--grace-period=0 --forceon the identified owner) andkubectl rollout restart. After each step a recovery gate waits for Ready, then operations run through a replica that does not own the session: continuity check, exec (including more than 1 MiB of piped stdin, which switches toExecSandboxInteractive), upload/download with checksums,forward start,forward service,policy update --wait,sandbox provider attach/detach --wait, and a fresh create/delete. The owner is read from its PostgreSQL record before and after each block; if it moved, the block runs once more, so a pass proves the operations crossed a peer. Retries are bounded and transient-only, non-idempotent operations reconcile before resending, and the test prints one summary line per phase.ci(e2e)). HarnessOPENSHELL_E2E_KUBE_POD_TLSmode (Service port-forward, verified mTLS handshake with the chart CA and client certificate, mTLS CLI registration; rejects Envoy and the DB scenarios). Newmise run e2e:kubernetes:ha-tlsand label-gatedkubernetes-ha-tls-e2eCI job in the HA result gate.docs): the peer transport contract in the gateway configuration reference; "Secure Peer Transport" (chart PKI, custom Secrets, ingress termination, the opt-out and its risk, upgrade guidance) and "Supported Operations" (routing class, non-owner availability, behavior after interruptions, HA test coverage) in the HA guide; architecture, Helm README and values docs, ingress and setup notes; public debug skill troubleshooting rows with the exact error strings; helm-dev skill TLS section.Operational limits:
logsreads only the serving replica's buffer). The HA guide documents this; making logs shared is out of scope.sandbox connectdoes not retryFAILED_PRECONDITION"sandbox is not ready" during an owner move; the HA guide says to run it again.supervisor session not connectedafter about 15 seconds, with the TLS cause in the gateway logs.Testing
Rebased on
mainat1358941b.mise run pre-commit: passed.peer_transporttests (in-process TLS gateway with rcgen PKI: rogue CA, wrong server name, absent/empty/non-PEM CA, key mismatch, intermediate chains, SNI-selected external certificate, client identity, IPv6, opt-out on TLS and plaintext gateways, scheme refusals, two-replica relay and unary forwarding) and 8 newauth::peertests. The C2 fail-fast tests ran 20 times in a row, including under 32 CPU burners on 16 cores.postgres://chart branch, or the peer client TLS mount fails a test.helm:lint(all variants),helm:test226 gateway tests and 6 workspace tests, split ownership,helm:docs:check.markdown:lint0 errors,docs:build:strict0 errors (3 warnings, same asmain).kubernetes_ha_operations, 7 unit tests,shellcheck.mise run e2e:kubernetes:ha-tls(pod TLS + mTLS, HTTPS peers): all standalone conformance runs passed over mTLS;kubernetes_ha_operationspassed in 125 seconds (8 tests: the lifecycle test plus 7 unit tests), every operation on its first attempt.values-skaffoldopt-out): passed in 149 seconds.mise run e2e:kubernetes:ha-rebalancing(plaintext, unchanged tests frommain): the final run passed conformance and both HA tests (2/2 in 236 seconds). Across the runs on this branch,sandbox_exec_rebalances_across_gateway_scale_and_rollouttook 213-221 seconds against its 300-second limit and timed out once while a parallel workspace build loaded the host.sandbox_file_sync_survives_gateway_pod_rollsfailed once withFAILED_PRECONDITION "sandbox is not ready": a CLI sync retry landed in the window where the sandbox readsProvisioningafter its owner pod is replaced, andsync_error_is_retryable(crates/openshell-cli/src/ssh.rs:1709) does not retry that status. This PR does not touch the CLI or that path; it is a pre-existing race.forced_path=released): the kubelet still sent SIGTERM, so the stale-owner-record branch (node failure) was not exercised end to end. The docs do not claim it.cargo check --workspace --all-targets, 2,003 server tests (including feat(server): add HA capacity metrics, scoped locks, and graceful drain #3710's fake-peer metrics tests), feat(server): add HA capacity metrics, scoped locks, and graceful drain #3710's PostgreSQL suite (17/17), 249 Helm tests and markdownlint. feat(server): place supervisor sessions by consistent hash #3661, feat(streams)!: define forwarding and relay lifecycle #3669 and test(conformance): cover relay reconnect recovery #3766 each compile with this branch and pass the peer transport,auth::peerandsupervisor_sessiontests. fix(helm): scope OIDC CA trust to the OIDC client #3673, perf(kubernetes): use a TCP readiness probe for the supervisor #3700, perf(server): drop per-connection session tokens for TCP forwards #3734 and test(conformance): cover sandbox policy lifecycle #3777 merge cleanly.Known environmental failures on this host, unchanged by this PR:
openshell-binary-identitytests that read/proc/<pid>/exe.The HA lanes need the
test:e2e-kuberneteslabel and do not run in merge groups. Please add the label.mise run pre-commitpasses.Checklist
!).How this was built
Each phase ran as a separate multi-agent workflow on Opus 5.5, with the lead session reading every result, committing, and deciding. Nothing was posted on the issue.
SSL_CERT_FILEinteraction, that "multi-replica" in code means PostgreSQL, that logs are replica-local, and that feat(helm): migrate gateway configuration to gatewayConfig #3384 silently drops the peer env block.--fixupand autosquash.In total about 170 agents and 19M subagent tokens. Every verification number above comes from commands run on this branch.
Merge coordination
http://127.0.0.1through the default route cache and pass because the loopback exception exists in test builds. Whoever lands second should trim the HA guide's owner-loss bullets to link feat(server): add HA capacity metrics, scoped locks, and graceful drain #3710's drain section instead of repeating it.dd0d7b98removes the wholeOPENSHELL_PEER_*env block, the peer token volume and the peer tests, and againstmainthat removal merges without a conflict. With this PR it becomes a real conflict in_gateway-workload.tpl(the always-on CA pin and the opt-out branch) and in the debug skill. Whoever lands second must keep the peer wiring;tests/peer_transport_test.yamlfails if the block or the peer client TLS mount disappears.kubernetes-ha-e2e-result.needslist; rerunhelm-docsfor thegrpcRoute.gateway.listener.protocolREADME row (values.yamlmerges on its own); in the debug skill, take feat(helm): add agentgateway ingress support #3714's rewritten ingress rows and keep this PR's SQLite/PostgreSQL split in the "HTTPS ingress connection resets" row. KeepOPENSHELL_E2E_KUBE_USE_ENVOY=0in the TLS task and extend the pod-TLS guard to reject agentgateway. An agentgateway TLS-termination overlay withdisableTlsplus HA now fails the chart guard by design; point HA users at the backend-TLS overlay.Ok(relay) => return Ok(relay),;routed_relay_retries_remote_failed_preconditionpins that remote status codes keep retrying. The combined tree compiles and the relay tests pass.SSL_CERT_FILEno longer affects peer trust after this PR.Follow-ups worth tracking: shared sandbox logs;
sandbox connectand CLI upload/download retrying "sandbox is not ready" during an owner move; deterministic forced owner loss (crictl stop) in e2e; service URL andsandbox connectHA e2e; the plaintext HA lane also running the operation test (after #3714); movingPeerTlsClientConfigintopeer_transport.rsand foldingvalidate_peer_endpoint_schemeinto it (after #3710); owner lookup through #3710's session gauge; feature-gated e2e clippy in CI.🤖 Generated with Claude Code