Skip to content

feat(network): support additional destination CAs - #3292

Open
jhjaggars wants to merge 6 commits into
NVIDIA:mainfrom
jhjaggars:feat/network-supervisor-additional-ca
Open

jhjaggars wants to merge 6 commits into
NVIDIA:mainfrom
jhjaggars:feat/network-supervisor-additional-ca

Conversation

@jhjaggars

@jhjaggars jhjaggars commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add a global network-supervisor configuration for additional destination CA certificates so sandbox egress can trust private PKI without replacing default roots or changing gateway control-plane trust. Deliver the normalized trust bundle consistently through Docker, Podman, Kubernetes combined/sidecar, and VM compute paths, including lifecycle-safe certificate rollover for stopped sandboxes.

Related Issue

No linked accepted issue (process discrepancy): this feature was implemented and published by direct user request. An accepted issue is still required before this PR is ready to merge.

Changes

  • Add [openshell.supervisor.network].additional_ca_cert_paths with strict, bounded startup validation, canonicalization, redacted metadata, and fail-closed capability-based driver handling.
  • Keep effective-config preflight side-effect-free; stage gateway-owned trust material only after the selected driver passes compatibility checks.
  • Extend the shared network supervisor with additive rustls and child-process trust while preserving user TLS variables in direct mode and keeping proxy CA and gateway mTLS trust separate.
  • Stage the normalized bundle through read-only Docker/Podman mounts, Kubernetes immutable content-addressed ConfigMaps, and VM overlays.
  • Record and validate the expected trust generation at the supervisor boundary so mutable or stale runtime material fails closed.
  • Reconcile stopped sandboxes whose recorded trust generation differs from the restarted gateway snapshot while retaining sandbox identity and supported durable workspace contents.
  • Restrict Kubernetes ConfigMap RBAC to the operations required for immutable generation creation and validation; do not grant list, watch, patch, update, or delete for this feature.
  • Enforce one deployable, bounded trust-bundle limit across gateway, Kubernetes, VM, and supervisor consumers.
  • Add shared cross-driver additional-CA coverage for private/public trust, hostname enforcement, invalid staged material, callback isolation, and removal/restart behavior.
  • Update gateway reference, architecture, RFC, Helm, compute-driver, and cluster-debugging documentation.

Why lifecycle reconciliation is included

Additional CA sources are startup-only gateway configuration. Updating a source file or Helm source ConfigMap does not hot-reload running supervisors. To roll certificates, an operator updates the source, restarts or redeploys the gateway, and explicitly stop/starts each affected sandbox.

Each runtime records the immutable trust generation it was created with. Without reconciliation, ordinary stop/start cannot reliably move an existing sandbox to the new generation: Docker and Podman retain old container metadata, Kubernetes retains the old content-addressed ConfigMap mount, and VM retains the old writable overlay. Replacing only the staged file would conflict with the supervisor's recorded digest and correctly fail closed.

The only existing manual alternative is to delete and recreate each affected sandbox, or create a replacement sandbox and move work to it. Sandbox deletion normally removes its driver-owned mutable filesystem storage—Docker/Podman workspace volumes, the Kubernetes sandbox PVC, or the VM writable overlay. Operators would therefore need to back up and restore required /sandbox contents, use external persistent storage, and re-establish any sandbox-specific state. The reconciliation implemented here avoids that migration by rebuilding stale runtime resources during stop/start while preserving the sandbox record and supported durable workspace data.

This lifecycle work can be split into a follow-up only if destructive replacement plus explicit data migration is an acceptable interim certificate-rotation procedure.

Testing

  • mise run pre-commit passes
  • Workspace formatting and Clippy with warnings denied pass
  • Targeted Rust tests pass across core, sandbox, supervisor-network, Docker, Kubernetes, Podman, VM, server, and gateway packages
  • Helm tests pass: 164 tests across 15 main-chart suites plus 4 workspace-chart tests
  • Unit and process-level tests added/updated
  • Cross-driver E2E coverage added and feature combinations compile
  • Rootless Podman manually verified against a private-PKI GitLab endpoint: TLS failed without the additional CA and succeeded with it while default public trust remained enabled
  • Full mise run test task run as a single validation command
  • Docker, Kubernetes combined/sidecar, and VM E2E runtime lanes (configured for CI; not run locally)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture and user-facing configuration documentation updated

@copy-pr-bot

copy-pr-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@jhjaggars

Copy link
Copy Markdown
Contributor Author
 [openshell.supervisor.network]
   additional_ca_cert_paths = [
     "/tmp/openshell-e2e-podman.McovxM/additional-ca/ca.crt",
   ]

@politerealism politerealism 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.

Summary

The design is sound and directly addresses both #3781 (Podman TLS-trust regression) and #3295 (general additional-destination-CA feature): trust material is scoped to the supervisor container only, root-store construction is fixed for both the bundled-ca-roots and native-store variants, validation is fail-closed end-to-end, and the gateway-mTLS trust boundary is provably untouched (new destination_ca_cannot_authenticate_gateway regression test). However, the branch as submitted (eb2db3f65) doesn't compile its own test suite, so CI can't pass yet.

Blocking

cargo test -p openshell-driver-podman -p openshell-driver-docker -p openshell-server (and mise run pre-commit's rust:lint step) fail to compile, all four attributable to this PR's own commits:

  • crates/openshell-driver-podman/src/container.rs:1833 — new test additional_destination_trust_is_delivered_only_to_supervisor_with_digest is missing the pre-existing required field rootless on IsolationSpecInput.
  • crates/openshell-driver-podman/src/container.rs:2020 — existing rootful_specs = build_isolation_specs(...) wasn't updated with the new required field network_trust.
  • crates/openshell-driver-docker/src/tests.rs:135 — sets daemon_version: "28.0.0".to_string(), but DockerDriverRuntimeConfig has no such field.
  • crates/openshell-server/src/compute/mod.rs:13576 — new test start_sandbox_request_uses_optional_wire_field_six_for_durable_snapshot is missing the pre-existing required field expected_runtime_identity on StartSandboxRequest.

openshell-driver-kubernetes, openshell-driver-vm, and openshell-gateway build and test cleanly in isolation, so this is localized to four struct literals, not systemic.

Non-blocking notes

  • deploy/helm/openshell/templates/clusterrole.yaml:82-97 / role.yaml:65-70: RBAC grants unscoped get on ConfigMaps (in addition to create), since resourceNames can't restrict dynamically digest-named generations. That lets the gateway service account read any ConfigMap in the bound scope, not just its own CA generations. Reasoning is documented in-line and in the debug-openshell-cluster skill update, but worth a second look given it's a real (if bounded) privilege widening.
  • crates/openshell-supervisor-network/src/run.rs:212-219: the additional-CA load path fails closed correctly but doesn't emit a ConfigStateChangeBuilder OCSF event the way the adjacent TLS-termination code does at run.rs:401-429. Not required by AGENTS.md's OCSF scope (that's openshell-sandbox-specific), but would be a nice consistency improvement for operator visibility.

Testing

cargo fmt --all -- --check passes. cargo test -p openshell-supervisor-network -p openshell-core -p openshell-supervisor passes (1344/1345, one pre-existing unrelated flake). Docker/Kubernetes/VM e2e lanes haven't been run (not available in my review environment) and remain genuinely unverified — recommend running those before merge, in addition to fixing the compile errors above.

Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>
@jhjaggars
jhjaggars force-pushed the feat/network-supervisor-additional-ca branch from eb2db3f to 60826fb Compare September 29, 2026 13:21
Signed-off-by: Jesse Jaggars <jjaggars@redhat.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants