From 8911f63c9b17b6ca4f2e6d0bc40fcab7b002df46 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 17 Aug 2026 14:59:58 +0200 Subject: [PATCH 1/3] feat(docker): supply CDI context for GPU sandboxes Signed-off-by: Evan Lezar --- architecture/compute-runtimes.md | 11 + crates/openshell-core/src/cdi.rs | 9 + crates/openshell-driver-docker/Cargo.toml | 2 +- crates/openshell-driver-docker/README.md | 24 ++ crates/openshell-driver-docker/src/lib.rs | 294 ++++++++++++++---- crates/openshell-driver-docker/src/tests.rs | 245 ++++++++++----- .../src/process.rs | 3 + docs/reference/sandbox-compute-drivers.mdx | 9 + 8 files changed, 470 insertions(+), 127 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 831be067ab..f6fdae7cec 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -169,6 +169,17 @@ Resource requirements enter the driver layer through `SandboxSpec.resource_requi can request a specific number of GPUs or the driver-specific default behaviour. For all in-tree drivers, this is equivalent to selecting a single GPU. +For Docker GPU sandboxes, the driver treats CDI specs as runtime metadata for +both outer injection and inner sandbox policy. It selects opaque CDI device IDs, +passes them to Docker, mounts daemon-reported CDI spec directories into +supervisor-only paths, and uploads a versioned CDI context before starting the +container. The supervisor resolves that context inside the sandbox and derives +Landlock paths and supplemental groups from CDI `containerEdits`. Host-side CDI +spec paths are diagnostic only and are never treated as sandbox policy paths. +Kubernetes must not infer CDI device IDs from the `nvidia.com/gpu` resource +request; it needs a node-local selected-device handoff before using the same +supervisor resolver. + VM runtime state paths are derived only from driver-validated sandbox IDs matching `[A-Za-z0-9._-]{1,128}`. The gateway-owned VM driver socket uses a private `run/` directory plus Unix peer UID/PID checks. Standalone diff --git a/crates/openshell-core/src/cdi.rs b/crates/openshell-core/src/cdi.rs index e07d2beef3..ecc6c2205d 100644 --- a/crates/openshell-core/src/cdi.rs +++ b/crates/openshell-core/src/cdi.rs @@ -9,12 +9,21 @@ use serde::{Deserialize, Serialize}; pub const CDI_CONTEXT_VERSION: u32 = 1; +/// File name used for the serialized CDI context. +pub const CDI_CONTEXT_FILE_NAME: &str = "cdi-context.json"; + /// Absolute supervisor path for the CDI context file mounted by a compute driver. pub const CDI_CONTEXT_PATH: &str = "/run/openshell/supervisor/cdi-context.json"; /// Base supervisor path under which compute drivers mount CDI specification directories. pub const CDI_SPEC_DIR_BASE: &str = "/run/openshell/supervisor/cdi-specs"; +/// Return the supervisor path used for a CDI specification directory. +#[must_use] +pub fn cdi_spec_mount_path(index: usize) -> String { + format!("{CDI_SPEC_DIR_BASE}/{index}") +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct CdiContext { pub version: u32, diff --git a/crates/openshell-driver-docker/Cargo.toml b/crates/openshell-driver-docker/Cargo.toml index 92a32c6d20..61519ec47c 100644 --- a/crates/openshell-driver-docker/Cargo.toml +++ b/crates/openshell-driver-docker/Cargo.toml @@ -24,10 +24,10 @@ serde_json = { workspace = true } prost-types = { workspace = true } bollard = { version = "0.20" } url = { workspace = true } +tar = "0.4" [dev-dependencies] prost-types = { workspace = true } -tar = "0.4" temp-env = "0.3" tempfile = "3" diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index 48e56de7bc..7167297cd9 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -86,9 +86,33 @@ contract: | `restart_policy = unless-stopped` | Keeps managed sandboxes resumable across daemon or gateway restarts. | | `PidsLimit` | Enforces the sandbox PID budget at the Docker cgroup layer. Set `[openshell.drivers.docker].sandbox_pids_limit = 0` to inherit the Docker/runtime default. | | CDI GPU request | Uses opaque `driver_config.cdi_devices` values when set; otherwise selects the requested count of NVIDIA CDI GPUs in round-robin order when daemon CDI support is detected. Docker daemon `/info` can permit `nvidia.com/gpu=all` as a WSL2 all-only compatibility fallback, where it counts as one selectable device. Exact CDI device lists must not contain duplicates and must match the effective GPU count. | +| CDI context upload | For GPU/CDI sandboxes only, mounts daemon-reported CDI spec directories read-only under `/run/openshell/supervisor/cdi-specs/` and uploads `/run/openshell/supervisor/cdi-context.json` after container create and before start. | The agent child process does not retain these supervisor privileges. +## CDI GPU Metadata + +Docker remains the source of truth for GPU injection. The driver selects opaque +CDI device IDs from `driver_config.cdi_devices` or the daemon's discovered CDI +inventory, then passes the same IDs to Docker with a CDI `DeviceRequest`. + +When a GPU/CDI request is present, the driver also mounts the Docker +daemon-reported `Info.CDISpecDirs` into supervisor-only paths and uploads a +small versioned CDI context through Docker's container archive API. The context +uses container-side spec paths for resolution and keeps host-side spec sources +diagnostic-only. If the upload fails, the driver removes the created container +and sandbox token file before reporting the failure. + +The sandbox supervisor resolves the selected IDs from those mounted specs +before it launches agent processes. CDI device nodes become read-write +Landlock paths, mount destinations default to read-only paths, and +`additionalGids` become supplemental groups for the entrypoint and SSH child +processes. Writable CDI mount destinations are accepted only for exact +single-file paths already listed in the sandbox policy `read_write` list; +writable CDI directory mounts fail closed. Kubernetes, Podman, WSL2 hardware +validation, and Tegra/Jetson hardware validation are separate follow-up +targets. + ## Driver Config Mounts The gateway forwards the `docker` block from `--driver-config-json` to this diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index b1fb5ec224..beced9f240 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -5,7 +5,6 @@ #![allow(clippy::result_large_err)] -use bollard::Docker; use bollard::errors::Error as BollardError; use bollard::models::{ ContainerCreateBody, ContainerState, ContainerStateStatusEnum, ContainerSummary, @@ -16,9 +15,12 @@ use bollard::models::{ use bollard::query_parameters::{ CreateContainerOptionsBuilder, CreateImageOptions, DownloadFromContainerOptionsBuilder, ListContainersOptionsBuilder, RemoveContainerOptionsBuilder, StopContainerOptionsBuilder, + UploadToContainerOptionsBuilder, }; +use bollard::{Docker, body_full}; use bytes::Bytes; use futures::{Stream, StreamExt}; +use openshell_core::cdi::{CdiContext, CdiSpecDirectory, cdi_spec_mount_path}; use openshell_core::config::{ DEFAULT_DOCKER_NETWORK_NAME, DEFAULT_SANDBOX_PIDS_LIMIT, DEFAULT_STOP_TIMEOUT_SECS, }; @@ -56,6 +58,7 @@ use openshell_core::proto_struct::{ }; use openshell_core::{Config, Error, Result as CoreResult}; use std::collections::{HashMap, HashSet}; +use std::io::Cursor; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -185,12 +188,64 @@ struct DockerDriverRuntimeConfig { supervisor_bin: PathBuf, guest_tls: Option, daemon_version: String, - supports_gpu: bool, - allow_all_default_gpu: bool, + gpu: DockerGpuRuntimeConfig, sandbox_pids_limit: i64, enable_bind_mounts: bool, } +#[derive(Debug, Clone, Default)] +struct DockerGpuRuntimeConfig { + cdi_spec_dirs: Vec, + allow_all_default: bool, +} + +impl DockerGpuRuntimeConfig { + fn supports_gpu(&self) -> bool { + !self.cdi_spec_dirs.is_empty() + } + + fn cdi_context(&self, gpu_device_ids: Option<&[String]>) -> Result, Status> { + let Some(gpu_device_ids) = gpu_device_ids.filter(|device_ids| !device_ids.is_empty()) + else { + return Ok(None); + }; + self.require_cdi_spec_dirs()?; + Ok(Some(CdiContext::new( + gpu_device_ids.to_vec(), + self.cdi_spec_dirs + .iter() + .enumerate() + .map(|(index, source)| CdiSpecDirectory::new(cdi_spec_mount_path(index), source)) + .collect(), + ))) + } + + fn cdi_spec_bind_strings( + &self, + gpu_device_ids: Option<&[String]>, + ) -> Result, Status> { + let Some(_) = gpu_device_ids.filter(|device_ids| !device_ids.is_empty()) else { + return Ok(Vec::new()); + }; + self.require_cdi_spec_dirs()?; + Ok(self + .cdi_spec_dirs + .iter() + .enumerate() + .map(|(index, source)| format!("{source}:{}:ro,z", cdi_spec_mount_path(index))) + .collect()) + } + + fn require_cdi_spec_dirs(&self) -> Result<(), Status> { + if self.cdi_spec_dirs.is_empty() { + return Err(Status::failed_precondition( + "docker GPU sandboxes require Docker CDI spec directories reported by the daemon", + )); + } + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] enum DockerGatewayRoute { Bridge { @@ -412,12 +467,11 @@ impl DockerComputeDriver { let info = docker.info().await.map_err(|err| { Error::execution(format!("failed to query Docker daemon info: {err}")) })?; - let supports_gpu = info - .cdi_spec_dirs - .as_ref() - .is_some_and(|dirs| !dirs.is_empty()); + let gpu = DockerGpuRuntimeConfig { + cdi_spec_dirs: info.cdi_spec_dirs.clone().unwrap_or_default(), + allow_all_default: docker_info_reports_wsl2(&info), + }; let cdi_gpu_inventory = docker_cdi_gpu_inventory(&info); - let allow_all_default_gpu = docker_info_reports_wsl2(&info); validate_sandbox_pids_limit(docker_config.sandbox_pids_limit)?; let gateway_port = config.bind_address.port(); if gateway_port == 0 { @@ -467,8 +521,7 @@ impl DockerComputeDriver { supervisor_bin, guest_tls, daemon_version: version.version.unwrap_or_else(|| "unknown".to_string()), - supports_gpu, - allow_all_default_gpu, + gpu: gpu.clone(), sandbox_pids_limit: docker_config.sandbox_pids_limit, enable_bind_mounts: docker_config.enable_bind_mounts, }, @@ -476,7 +529,7 @@ impl DockerComputeDriver { pending: Arc::new(Mutex::new(HashMap::new())), gpu_selector: Arc::new(CdiGpuDefaultSelector::new( cdi_gpu_inventory, - allow_all_default_gpu, + gpu.allow_all_default, )), lifecycle_event_fences: DockerLifecycleEventFences::default(), }; @@ -525,7 +578,7 @@ impl DockerComputeDriver { DockerSandboxDriverConfig::from_template(template).map_err(Status::invalid_argument)?; validate_docker_driver_mounts(&driver_config.mounts, config.enable_bind_mounts)?; let gpu_requirements = driver_gpu_requirements(spec.resource_requirements.as_ref()); - Self::validate_gpu_request(gpu_requirements, config.supports_gpu, &driver_config)?; + Self::validate_gpu_request(gpu_requirements, config.gpu.supports_gpu(), &driver_config)?; Ok(ValidatedDockerSandbox { template, driver_config, @@ -633,7 +686,7 @@ impl DockerComputeDriver { .map_err(|err| internal_status("query Docker daemon info", err))?; self.gpu_selector.refresh( docker_cdi_gpu_inventory(&info), - self.config.allow_all_default_gpu, + self.config.gpu.allow_all_default, ); Ok(()) } @@ -779,11 +832,6 @@ impl DockerComputeDriver { .map_err(|status| { DockerProvisioningFailure::new("ImagePullFailed", status.message()) })?; - let token_file_created = write_sandbox_token_file(sandbox, &self.config) - .await - .map_err(|status| { - DockerProvisioningFailure::new("SandboxTokenWriteFailed", status.message()) - })?; let container_name = container_name_for_sandbox(sandbox); let gpu_devices = self @@ -794,9 +842,13 @@ impl DockerComputeDriver { ) .await .map_err(|status| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } + DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) + })?; + let cdi_context = self + .config + .gpu + .cdi_context(gpu_devices.as_deref()) + .map_err(|status| { DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; let create_body = build_container_create_body_for_image( @@ -807,12 +859,15 @@ impl DockerComputeDriver { &image, ) .map_err(|status| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; - self.docker + write_sandbox_token_file(sandbox, &self.config) + .await + .map_err(|status| { + DockerProvisioningFailure::new("SandboxTokenWriteFailed", status.message()) + })?; + if let Err(err) = self + .docker .create_container( Some( CreateContainerOptionsBuilder::default() @@ -822,15 +877,13 @@ impl DockerComputeDriver { create_body, ) .await - .map_err(|err| { - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } - DockerProvisioningFailure::from_status( - "ContainerCreateFailed", - create_status_from_docker_error("create docker sandbox container", err), - ) - })?; + { + cleanup_sandbox_token_file(sandbox, &self.config); + return Err(DockerProvisioningFailure::from_status( + "ContainerCreateFailed", + create_status_from_docker_error("create docker sandbox container", err), + )); + } self.publish_docker_progress( &sandbox.id, "Created", @@ -838,25 +891,30 @@ impl DockerComputeDriver { HashMap::from([("container_name".to_string(), container_name.clone())]), ); + if let Some(cdi_context) = cdi_context + && let Err(err) = self.upload_cdi_context(&container_name, &cdi_context).await + { + self.cleanup_created_container_after_failure( + &sandbox.id, + &container_name, + "CDI context upload failure", + ) + .await; + cleanup_sandbox_token_file(sandbox, &self.config); + return Err(DockerProvisioningFailure::from_status( + "CdiContextUploadFailed", + err, + )); + } + if let Err(err) = self.docker.start_container(&container_name, None).await { - let cleanup = self - .docker - .remove_container( - &container_name, - Some(RemoveContainerOptionsBuilder::default().force(true).build()), - ) - .await; - if let Err(cleanup_err) = cleanup { - warn!( - sandbox_id = %sandbox.id, - container_name, - error = %cleanup_err, - "Failed to clean up Docker container after start failure" - ); - } - if token_file_created { - cleanup_sandbox_token_file(sandbox, &self.config); - } + self.cleanup_created_container_after_failure( + &sandbox.id, + &container_name, + "container start failure", + ) + .await; + cleanup_sandbox_token_file(sandbox, &self.config); return Err(DockerProvisioningFailure::from_status( "ContainerStartFailed", create_status_from_docker_error("start docker sandbox container", err), @@ -882,6 +940,51 @@ impl DockerComputeDriver { Ok(()) } + async fn cleanup_created_container_after_failure( + &self, + sandbox_id: &str, + container_name: &str, + phase: &'static str, + ) { + let cleanup = self + .docker + .remove_container( + container_name, + Some(RemoveContainerOptionsBuilder::default().force(true).build()), + ) + .await; + if let Err(cleanup_err) = cleanup { + warn!( + sandbox_id = %sandbox_id, + container_name = %container_name, + phase, + error = %cleanup_err, + "Failed to clean up Docker container after provisioning failure" + ); + } + } + + async fn upload_cdi_context( + &self, + container_name: &str, + context: &CdiContext, + ) -> Result<(), Status> { + let archive = build_cdi_context_archive(context).map_err(Status::internal)?; + self.docker + .upload_to_container( + container_name, + Some( + UploadToContainerOptionsBuilder::default() + .path("/run") + .no_overwrite_dir_non_dir("true") + .build(), + ), + body_full(Bytes::from(archive)), + ) + .await + .map_err(|err| internal_status("upload CDI context to Docker container", err)) + } + async fn delete_sandbox_inner( &self, sandbox_id: &str, @@ -2396,14 +2499,19 @@ fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRunti } #[cfg(test)] -fn build_environment(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) -> Vec { - build_environment_for_oci_user(sandbox, config, "") +fn build_environment( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + include_cdi_context: bool, +) -> Vec { + build_environment_for_oci_user(sandbox, config, "", include_cdi_context) } fn build_environment_for_oci_user( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, oci_user: &str, + include_cdi_context: bool, ) -> Vec { let mut environment = HashMap::from([ ("HOME".to_string(), "/root".to_string()), @@ -2456,6 +2564,14 @@ fn build_environment_for_oci_user( openshell_core::sandbox_env::TELEMETRY_ENABLED.to_string(), openshell_core::telemetry::enabled_env_value().to_string(), ); + environment.insert( + openshell_core::sandbox_env::CDI_CONTEXT.to_string(), + if include_cdi_context { + openshell_core::cdi::CDI_CONTEXT_PATH.to_string() + } else { + String::new() + }, + ); // The root supervisor executes namespace helpers during bootstrap; keep // their search path driver-owned even when the template/spec set PATH. environment.insert("PATH".to_string(), SUPERVISOR_PATH.to_string()); @@ -2543,6 +2659,68 @@ fn docker_gpu_selection_status(err: CdiGpuSelectionError) -> Status { Status::failed_precondition(err.to_string()) } +fn cdi_context_requested(gpu_device_ids: Option<&[String]>) -> bool { + gpu_device_ids.is_some_and(|device_ids| !device_ids.is_empty()) +} + +fn build_cdi_context_archive(context: &CdiContext) -> Result, String> { + let json = serde_json::to_vec_pretty(context).map_err(|err| err.to_string())?; + let mut archive = DockerTarArchiveBuilder::new(); + archive.append_dir("openshell", 0o700)?; + archive.append_dir("openshell/supervisor", 0o700)?; + archive.append_file( + &format!( + "openshell/supervisor/{}", + openshell_core::cdi::CDI_CONTEXT_FILE_NAME + ), + 0o600, + json, + )?; + archive.into_inner() +} + +struct DockerTarArchiveBuilder { + inner: tar::Builder>, +} + +impl DockerTarArchiveBuilder { + fn new() -> Self { + Self { + inner: tar::Builder::new(Vec::new()), + } + } + + fn append_dir(&mut self, path: &str, mode: u32) -> Result<(), String> { + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Directory); + header.set_size(0); + header.set_mode(mode); + header.set_uid(0); + header.set_gid(0); + header.set_cksum(); + self.inner + .append_data(&mut header, path, std::io::empty()) + .map_err(|err| err.to_string()) + } + + fn append_file(&mut self, path: &str, mode: u32, contents: Vec) -> Result<(), String> { + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Regular); + header.set_size(u64::try_from(contents.len()).map_err(|err| err.to_string())?); + header.set_mode(mode); + header.set_uid(0); + header.set_gid(0); + header.set_cksum(); + self.inner + .append_data(&mut header, path, Cursor::new(contents)) + .map_err(|err| err.to_string()) + } + + fn into_inner(self) -> Result, String> { + self.inner.into_inner().map_err(|err| err.to_string()) + } +} + #[cfg(test)] fn build_container_create_body( sandbox: &DriverSandbox, @@ -2679,7 +2857,12 @@ fn build_container_create_body_for_image( // The image workspace may need to be created or rejected by the // supervisor, so do not let the OCI runtime chdir there first. working_dir: Some("/".to_string()), - env: Some(build_environment_for_oci_user(sandbox, config, &image.user)), + env: Some(build_environment_for_oci_user( + sandbox, + config, + &image.user, + cdi_context_requested(gpu_device_ids), + )), entrypoint: Some(vec![SUPERVISOR_MOUNT_PATH.to_string()]), // Replace the image CMD with the supervisor's resolved workspace // argument so Docker cannot append inherited image arguments. @@ -2692,6 +2875,7 @@ fn build_container_create_body_for_image( device_requests, binds: { let mut binds = build_binds(sandbox, config)?; + binds.extend(config.gpu.cdi_spec_bind_strings(gpu_device_ids)?); binds.extend(user_bind_strings); Some(binds) }, diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index eddfd778bd..a63c2297c0 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -88,7 +88,10 @@ fn gpu_resources(count: Option) -> ResourceRequirements { } } -fn runtime_config() -> DockerDriverRuntimeConfig { +const TEST_CDI_SPEC_DIR: &str = "/opt/openshell-test/cdi"; +const TEST_CDI_SPEC_DIR_ALT: &str = "/srv/openshell-test/cdi"; + +fn runtime_config(supports_gpu: bool) -> DockerDriverRuntimeConfig { DockerDriverRuntimeConfig { default_image: "image:latest".to_string(), image_pull_policy: String::new(), @@ -116,13 +119,32 @@ fn runtime_config() -> DockerDriverRuntimeConfig { key: PathBuf::from("/tmp/tls.key"), }), daemon_version: "28.0.0".to_string(), - supports_gpu: false, - allow_all_default_gpu: false, + gpu: gpu_runtime_config(supports_gpu), sandbox_pids_limit: DEFAULT_SANDBOX_PIDS_LIMIT, enable_bind_mounts: false, } } +fn runtime_config_with_cdi_spec_dirs(cdi_spec_dirs: &[&str]) -> DockerDriverRuntimeConfig { + let mut config = runtime_config(false); + config.gpu.cdi_spec_dirs = cdi_spec_dirs + .iter() + .map(|path| (*path).to_string()) + .collect(); + config +} + +fn gpu_runtime_config(supports_gpu: bool) -> DockerGpuRuntimeConfig { + if supports_gpu { + DockerGpuRuntimeConfig { + cdi_spec_dirs: vec![TEST_CDI_SPEC_DIR.to_string()], + ..Default::default() + } + } else { + DockerGpuRuntimeConfig::default() + } +} + fn json_struct(value: serde_json::Value) -> prost_types::Struct { let serde_json::Value::Object(object) = value else { panic!("expected JSON object"); @@ -147,7 +169,7 @@ fn inspected_volume(driver: &str, options: HashMap) -> bollard:: } fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDriver { - let allow_all_default_gpu = config.allow_all_default_gpu; + let allow_all_default_gpu = config.gpu.allow_all_default; DockerComputeDriver { docker: Arc::new( Docker::connect_with_http("http://127.0.0.1:2375", 1, bollard::API_DEFAULT_VERSION) @@ -166,7 +188,7 @@ fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDr #[tokio::test] async fn gateway_listener_requirements_report_managed_bridge_address() { - let config = runtime_config(); + let config = runtime_config(false); let expected_address = match config.gateway_route { DockerGatewayRoute::Bridge { bind_address, .. } => bind_address, DockerGatewayRoute::HostGateway => panic!("test config must use a managed bridge"), @@ -188,7 +210,7 @@ async fn gateway_listener_requirements_report_managed_bridge_address() { #[tokio::test] async fn gateway_listener_requirements_are_empty_for_host_gateway_route() { - let mut config = runtime_config(); + let mut config = runtime_config(false); config.gateway_route = DockerGatewayRoute::HostGateway; config.gateway_callback_bind_address = None; let driver = test_driver_with_config(config); @@ -204,7 +226,7 @@ async fn gateway_listener_requirements_are_empty_for_host_gateway_route() { #[tokio::test] async fn host_gateway_route_reports_ipv4_loopback_callback_listener() { - let mut config = runtime_config(); + let mut config = runtime_config(false); config.gateway_route = DockerGatewayRoute::HostGateway; config.gateway_callback_bind_address = Some("127.0.0.1:17670".parse().unwrap()); let driver = test_driver_with_config(config); @@ -605,14 +627,14 @@ fn docker_compute_config_disables_bind_mounts_by_default() { #[test] fn container_create_body_sets_driver_owned_pids_limit() { - let body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); let host_config = body.host_config.expect("host config"); assert_eq!(host_config.pids_limit, Some(DEFAULT_SANDBOX_PIDS_LIMIT)); } #[test] fn build_environment_sets_docker_tls_paths() { - let env = build_environment(&test_sandbox(), &runtime_config()); + let env = build_environment(&test_sandbox(), &runtime_config(false), false); assert!(env.contains(&format!("OPENSHELL_TLS_CA={TLS_CA_MOUNT_PATH}"))); assert!(env.contains(&format!("OPENSHELL_TLS_CERT={TLS_CERT_MOUNT_PATH}"))); assert!(env.contains(&format!("OPENSHELL_TLS_KEY={TLS_KEY_MOUNT_PATH}"))); @@ -633,7 +655,7 @@ fn build_environment_protects_oci_identity_metadata() { spec.environment.insert(key.to_string(), value.to_string()); } - let env = build_environment_for_oci_user(&sandbox, &runtime_config(), "app:staff"); + let env = build_environment_for_oci_user(&sandbox, &runtime_config(false), "app:staff", false); assert!(env.contains(&format!( "{}=app:staff", @@ -654,7 +676,7 @@ fn build_environment_strips_gateway_tls_server_name() { "evil.attacker.example.com".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); assert!( !env.iter().any(|entry| entry.starts_with(&format!( @@ -676,7 +698,7 @@ fn container_creation_uses_inspected_immutable_image() { }; let body = build_container_create_body_for_image( &sandbox, - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -706,7 +728,7 @@ fn container_creation_rejects_invalid_oci_working_dir() { }; let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -727,7 +749,7 @@ fn container_creation_rejects_openshell_control_path_working_dir() { }; let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -750,7 +772,7 @@ fn container_creation_rejects_image_volume_that_masks_working_dir() { let error = build_container_create_body_for_image( &sandbox, - &runtime_config(), + &runtime_config(false), &DockerSandboxDriverConfig::default(), None, &metadata, @@ -772,7 +794,7 @@ fn container_creation_rejects_image_volume_over_configured_ssh_socket() { working_dir: "/workspace".to_string(), volumes: vec!["/custom-runtime".to_string()], }; - let mut config = runtime_config(); + let mut config = runtime_config(false); config.ssh_socket_path = "/custom-runtime/ssh.sock".to_string(); let error = build_container_create_body_for_image( @@ -801,7 +823,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( .unwrap(); let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &root_mount, None, &metadata, @@ -823,7 +845,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( }; let err = build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &ancestor_mount, None, &nested_metadata, @@ -840,7 +862,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( .unwrap(); build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &nested_mount, None, &metadata, @@ -854,7 +876,7 @@ fn container_creation_reserves_resolved_workspace_root_but_allows_nested_mounts( .unwrap(); build_container_create_body_for_image( &test_sandbox(), - &runtime_config(), + &runtime_config(false), &compatibility_path_mount, None, &metadata, @@ -874,7 +896,7 @@ fn build_environment_keeps_path_driver_controlled() { .environment .insert("PATH".to_string(), "/malicious/template/bin".to_string()); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); let path_entries = env .iter() .filter(|entry| entry.starts_with("PATH=")) @@ -900,7 +922,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { "true".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); let telemetry_entries = env .iter() .filter(|entry| { @@ -922,7 +944,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { #[test] fn build_binds_uses_docker_tls_directory() { - let binds = build_binds(&test_sandbox(), &runtime_config()).unwrap(); + let binds = build_binds(&test_sandbox(), &runtime_config(false)).unwrap(); let targets = binds .iter() .filter_map(|bind| bind.split(':').nth(1).map(String::from)) @@ -961,7 +983,7 @@ fn build_container_create_body_includes_driver_config_mounts() { ] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -1009,7 +1031,7 @@ fn driver_config_defaults_volume_mounts_to_read_only() { }] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -1038,7 +1060,7 @@ fn driver_config_allows_explicit_writable_volume_mounts() { }] }))); - let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let body = build_container_create_body(&sandbox, &runtime_config(false)).unwrap(); let mounts = body .host_config .unwrap() @@ -1072,7 +1094,7 @@ fn driver_config_rejects_duplicate_mount_targets() { ] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!( @@ -1099,7 +1121,7 @@ fn driver_config_rejects_bind_mounts_unless_enabled() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("enable_bind_mounts = true")); @@ -1125,7 +1147,7 @@ fn build_container_create_body_includes_bind_mounts_when_enabled() { "read_only": true }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1170,7 +1192,7 @@ fn driver_config_defaults_enabled_bind_mounts_to_read_only() { "target": "/sandbox/host" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1208,7 +1230,7 @@ fn bind_mount_selinux_shared_label() { "selinux_label": "shared" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1246,7 +1268,7 @@ fn bind_mount_selinux_private_label() { "selinux_label": "private" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1283,7 +1305,7 @@ fn bind_mount_without_selinux_label() { "read_only": false }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let body = build_container_create_body(&sandbox, &config).unwrap(); @@ -1317,7 +1339,7 @@ fn driver_config_rejects_missing_bind_source() { "target": "/sandbox/data" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let err = build_container_create_body(&sandbox, &config).unwrap_err(); @@ -1347,7 +1369,7 @@ fn driver_config_rejects_relative_bind_sources_when_enabled() { "target": "/sandbox/host" }] }))); - let mut config = runtime_config(); + let mut config = runtime_config(false); config.enable_bind_mounts = true; let err = build_container_create_body(&sandbox, &config).unwrap_err(); @@ -1377,7 +1399,7 @@ fn driver_config_rejects_image_mounts() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("invalid docker driver_config")); @@ -1401,7 +1423,7 @@ fn driver_config_rejects_reserved_mount_targets() { }] }))); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("reserved OpenShell path")); @@ -1422,7 +1444,7 @@ fn driver_config_rejects_mount_over_configured_ssh_socket() { working_dir: "/workspace".to_string(), volumes: Vec::new(), }; - let mut config = runtime_config(); + let mut config = runtime_config(false); config.ssh_socket_path = "/custom-runtime/ssh.sock".to_string(); let error = build_container_create_body_for_image( @@ -1499,7 +1521,7 @@ fn build_environment_uses_token_file_without_raw_token_env() { "user-provided-token".to_string(), ); - let env = build_environment(&sandbox, &runtime_config()); + let env = build_environment(&sandbox, &runtime_config(false), false); assert!(!env.iter().any(|entry| { entry.starts_with(&format!("{}=", openshell_core::sandbox_env::SANDBOX_TOKEN)) @@ -1523,7 +1545,7 @@ fn managed_container_label_filters_include_gateway_namespace() { #[test] fn build_container_create_body_replaces_inherited_cmd_with_workspace_arg() { - let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let create_body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); assert_eq!( create_body.entrypoint, @@ -1572,7 +1594,7 @@ fn build_container_create_body_replaces_inherited_cmd_with_workspace_arg() { #[test] fn validate_sandbox_rejects_gpu_when_cdi_unavailable() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1584,7 +1606,7 @@ fn validate_sandbox_rejects_gpu_when_cdi_unavailable() { #[test] fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1598,7 +1620,7 @@ fn validate_sandbox_rejects_missing_gpu_support_before_request_shape() { #[test] fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1613,7 +1635,7 @@ fn validate_sandbox_rejects_invalid_cdi_devices_before_gpu_capability() { #[test] fn validate_sandbox_rejects_unknown_driver_config_fields() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1628,8 +1650,7 @@ fn validate_sandbox_rejects_unknown_driver_config_fields() { #[test] fn validate_sandbox_accepts_gpu_count_request_shape() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(Some(2))); @@ -1639,8 +1660,7 @@ fn validate_sandbox_accepts_gpu_count_request_shape() { #[test] fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1655,8 +1675,7 @@ fn validate_sandbox_accepts_gpu_count_matching_cdi_devices() { #[test] fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1668,8 +1687,7 @@ fn validate_sandbox_accepts_single_cdi_device_without_gpu_count() { #[test] fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1689,8 +1707,7 @@ fn validate_sandbox_rejects_multiple_cdi_devices_without_gpu_count() { #[test] fn validate_sandbox_rejects_cdi_devices_without_gpu_request() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox .spec @@ -1709,8 +1726,7 @@ fn validate_sandbox_rejects_cdi_devices_without_gpu_request() { #[test] fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1727,7 +1743,7 @@ fn validate_sandbox_rejects_gpu_count_mismatched_cdi_devices() { #[test] fn validate_sandbox_rejects_template_errors_before_device_config() { - let config = runtime_config(); + let config = runtime_config(false); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1765,8 +1781,7 @@ fn validate_sandbox_auth_accepts_gateway_token() { #[test] fn build_container_create_body_maps_default_gpu_to_selected_cdi_device() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1793,10 +1808,101 @@ fn build_container_create_body_maps_default_gpu_to_selected_cdi_device() { ); } +#[test] +fn build_container_create_body_adds_cdi_context_env_and_spec_mounts_for_gpu() { + let config = runtime_config_with_cdi_spec_dirs(&[TEST_CDI_SPEC_DIR, TEST_CDI_SPEC_DIR_ALT]); + let mut sandbox = test_sandbox(); + sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); + + let driver_config = DockerSandboxDriverConfig::default(); + let gpu_devices = vec!["nvidia.com/gpu=1".to_string()]; + let create_body = build_container_create_body_with_gpu_devices( + &sandbox, + &config, + &driver_config, + Some(&gpu_devices), + ) + .unwrap(); + + let env = create_body.env.expect("env should be set"); + assert!(env.iter().any(|entry| { + entry + == &format!( + "{}={}", + openshell_core::sandbox_env::CDI_CONTEXT, + openshell_core::cdi::CDI_CONTEXT_PATH + ) + })); + + let binds = create_body + .host_config + .expect("host config") + .binds + .expect("binds should be set"); + assert!( + binds.iter().any(|bind| { + bind == &format!("{TEST_CDI_SPEC_DIR}:{}:ro,z", cdi_spec_mount_path(0)) + }) + ); + assert!(binds.iter().any(|bind| { + bind == &format!("{TEST_CDI_SPEC_DIR_ALT}:{}:ro,z", cdi_spec_mount_path(1)) + })); +} + +#[test] +fn build_container_create_body_clears_cdi_context_for_non_gpu() { + let mut config = runtime_config(false); + config.gpu.cdi_spec_dirs = vec![TEST_CDI_SPEC_DIR.to_string()]; + let create_body = build_container_create_body(&test_sandbox(), &config).unwrap(); + + let env = create_body.env.expect("env should be set"); + assert!( + env.iter() + .any(|entry| { entry == &format!("{}=", openshell_core::sandbox_env::CDI_CONTEXT) }) + ); + + let binds = create_body + .host_config + .expect("host config") + .binds + .expect("binds should be set"); + assert!( + !binds + .iter() + .any(|bind| bind.contains(openshell_core::cdi::CDI_SPEC_DIR_BASE)) + ); +} + +#[test] +fn build_cdi_context_archive_contains_context_json() { + use std::io::Read as _; + + let context = CdiContext::new( + vec!["nvidia.com/gpu=0".to_string()], + vec![CdiSpecDirectory::new( + cdi_spec_mount_path(0), + TEST_CDI_SPEC_DIR, + )], + ); + let bytes = build_cdi_context_archive(&context).unwrap(); + let mut archive = tar::Archive::new(Cursor::new(bytes)); + let mut found = false; + for entry in archive.entries().unwrap() { + let mut entry = entry.unwrap(); + if entry.path().unwrap().as_ref() == Path::new("openshell/supervisor/cdi-context.json") { + let mut payload = String::new(); + entry.read_to_string(&mut payload).unwrap(); + let parsed: CdiContext = serde_json::from_str(&payload).unwrap(); + assert_eq!(parsed, context); + found = true; + } + } + assert!(found, "archive must include cdi-context.json"); +} + #[test] fn build_container_create_body_omits_devices_without_resolved_default_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); sandbox.spec.as_mut().unwrap().resource_requirements = Some(gpu_resources(None)); @@ -1813,8 +1919,7 @@ fn build_container_create_body_omits_devices_without_resolved_default_cdi_device #[test] fn build_container_create_body_passes_explicit_cdi_device_id_through() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(None)); @@ -1837,8 +1942,7 @@ fn build_container_create_body_passes_explicit_cdi_device_id_through() { #[test] fn build_container_create_body_rejects_gpu_count_mismatched_cdi_devices() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let mut sandbox = test_sandbox(); let spec = sandbox.spec.as_mut().unwrap(); spec.resource_requirements = Some(gpu_resources(Some(2))); @@ -1865,7 +1969,7 @@ fn build_container_create_body_rejects_cdi_devices_without_gpu_request() { .unwrap() .driver_config = Some(cdi_devices_config(&["nvidia.com/gpu=0"])); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::InvalidArgument); assert!(err.message().contains("requires a gpu request")); } @@ -1877,15 +1981,14 @@ fn build_container_create_body_rejects_empty_cdi_devices() { spec.resource_requirements = Some(gpu_resources(None)); spec.template.as_mut().unwrap().driver_config = Some(cdi_devices_config(&[])); - let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + let err = build_container_create_body(&sandbox, &runtime_config(false)).unwrap_err(); assert_eq!(err.code(), tonic::Code::InvalidArgument); assert!(err.message().contains("non-empty list")); } #[test] fn driver_default_gpu_selection_consumes_distinct_devices_for_creates() { - let mut config = runtime_config(); - config.supports_gpu = true; + let config = runtime_config(true); let driver = test_driver_with_config(config); driver.gpu_selector.refresh( CdiGpuInventory::new(["nvidia.com/gpu=0", "nvidia.com/gpu=1"]), @@ -2017,7 +2120,7 @@ fn require_sandbox_identifier_rejects_when_id_and_name_are_empty() { #[test] fn build_container_create_body_uses_bridge_network() { - let create_body = build_container_create_body(&test_sandbox(), &runtime_config()).unwrap(); + let create_body = build_container_create_body(&test_sandbox(), &runtime_config(false)).unwrap(); let host_config = create_body.host_config.expect("host_config is populated"); assert_eq!( @@ -2043,7 +2146,7 @@ fn build_container_create_body_uses_runtime_namespace_label() { // with that empty value would not match subsequent list/get/find // queries (which filter on `config.sandbox_namespace`), leaking // sandboxes that the driver itself cannot observe. - let mut config = runtime_config(); + let mut config = runtime_config(false); config.sandbox_namespace = "tenant-a".to_string(); let mut sandbox = test_sandbox(); sandbox.namespace = "ignored-by-driver".to_string(); diff --git a/crates/openshell-supervisor-process/src/process.rs b/crates/openshell-supervisor-process/src/process.rs index 14ff86ea3b..b8f9cab059 100644 --- a/crates/openshell-supervisor-process/src/process.rs +++ b/crates/openshell-supervisor-process/src/process.rs @@ -2072,6 +2072,9 @@ pub fn drop_privileges_with_identity( target_os = "redox" )))] if target_uid != nix::unistd::geteuid() { + // Resolve the name for initgroups only for the existing explicit-policy + // path. OCI-derived users carry a numeric UID from the bounded parser and + // must not be looked up again through NSS. let user_name_is_numeric = user_name.is_some_and(|n| n.parse::().is_ok()); let initgroups_name = if user_name.is_some() && !user_name_is_numeric && resolved_identity.uid().is_none() { diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 897e780c39..df0de711ea 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -145,6 +145,15 @@ responsible for removing it. For GPU-backed Docker sandboxes, configure Docker CDI before starting the gateway so OpenShell can detect the daemon capability. +For Docker GPU/CDI sandboxes, OpenShell uses Docker's selected CDI device IDs +and daemon-reported CDI spec directories to build a supervisor-only CDI +context. The driver mounts the spec directories read-only into the sandbox +container, uploads `cdi-context.json` before starting the container, and removes +the created container if that upload fails. The supervisor resolves the context +inside the sandbox and derives the inner filesystem and supplemental group +requirements from CDI specs. Non-GPU Docker sandboxes do not receive the CDI +context, spec mounts, or CDI-derived policy changes. + ### Docker Driver Config Mounts Docker driver config accepts user-supplied `volume` and `tmpfs` mounts. It also From 483cf0437a49510b35b25258fe48d16dcea02369 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Tue, 18 Aug 2026 14:49:48 +0200 Subject: [PATCH 2/3] fix(docker): bind CDI context read-only Signed-off-by: Evan Lezar --- crates/openshell-driver-docker/src/lib.rs | 246 +++++++++++--------- crates/openshell-driver-docker/src/tests.rs | 41 ++-- 2 files changed, 157 insertions(+), 130 deletions(-) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index beced9f240..5e03d7b83a 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -5,6 +5,7 @@ #![allow(clippy::result_large_err)] +use bollard::Docker; use bollard::errors::Error as BollardError; use bollard::models::{ ContainerCreateBody, ContainerState, ContainerStateStatusEnum, ContainerSummary, @@ -15,9 +16,7 @@ use bollard::models::{ use bollard::query_parameters::{ CreateContainerOptionsBuilder, CreateImageOptions, DownloadFromContainerOptionsBuilder, ListContainersOptionsBuilder, RemoveContainerOptionsBuilder, StopContainerOptionsBuilder, - UploadToContainerOptionsBuilder, }; -use bollard::{Docker, body_full}; use bytes::Bytes; use futures::{Stream, StreamExt}; use openshell_core::cdi::{CdiContext, CdiSpecDirectory, cdi_spec_mount_path}; @@ -58,7 +57,6 @@ use openshell_core::proto_struct::{ }; use openshell_core::{Config, Error, Result as CoreResult}; use std::collections::{HashMap, HashSet}; -use std::io::Cursor; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -861,11 +859,22 @@ impl DockerComputeDriver { .map_err(|status| { DockerProvisioningFailure::new("ContainerCreateFailed", status.message()) })?; - write_sandbox_token_file(sandbox, &self.config) - .await - .map_err(|status| { - DockerProvisioningFailure::new("SandboxTokenWriteFailed", status.message()) - })?; + if let Some(cdi_context) = cdi_context.as_ref() + && let Err(status) = write_cdi_context_file(sandbox, &self.config, cdi_context) + { + cleanup_cdi_context_file(sandbox, &self.config); + return Err(DockerProvisioningFailure::new( + "CdiContextWriteFailed", + status.message(), + )); + } + if let Err(status) = write_sandbox_token_file(sandbox, &self.config).await { + cleanup_cdi_context_file(sandbox, &self.config); + return Err(DockerProvisioningFailure::new( + "SandboxTokenWriteFailed", + status.message(), + )); + } if let Err(err) = self .docker .create_container( @@ -878,7 +887,7 @@ impl DockerComputeDriver { ) .await { - cleanup_sandbox_token_file(sandbox, &self.config); + cleanup_sandbox_state_files(sandbox, &self.config); return Err(DockerProvisioningFailure::from_status( "ContainerCreateFailed", create_status_from_docker_error("create docker sandbox container", err), @@ -891,22 +900,6 @@ impl DockerComputeDriver { HashMap::from([("container_name".to_string(), container_name.clone())]), ); - if let Some(cdi_context) = cdi_context - && let Err(err) = self.upload_cdi_context(&container_name, &cdi_context).await - { - self.cleanup_created_container_after_failure( - &sandbox.id, - &container_name, - "CDI context upload failure", - ) - .await; - cleanup_sandbox_token_file(sandbox, &self.config); - return Err(DockerProvisioningFailure::from_status( - "CdiContextUploadFailed", - err, - )); - } - if let Err(err) = self.docker.start_container(&container_name, None).await { self.cleanup_created_container_after_failure( &sandbox.id, @@ -914,7 +907,7 @@ impl DockerComputeDriver { "container start failure", ) .await; - cleanup_sandbox_token_file(sandbox, &self.config); + cleanup_sandbox_state_files(sandbox, &self.config); return Err(DockerProvisioningFailure::from_status( "ContainerStartFailed", create_status_from_docker_error("start docker sandbox container", err), @@ -964,27 +957,6 @@ impl DockerComputeDriver { } } - async fn upload_cdi_context( - &self, - container_name: &str, - context: &CdiContext, - ) -> Result<(), Status> { - let archive = build_cdi_context_archive(context).map_err(Status::internal)?; - self.docker - .upload_to_container( - container_name, - Some( - UploadToContainerOptionsBuilder::default() - .path("/run") - .no_overwrite_dir_non_dir("true") - .build(), - ), - body_full(Bytes::from(archive)), - ) - .await - .map_err(|err| internal_status("upload CDI context to Docker container", err)) - } - async fn delete_sandbox_inner( &self, sandbox_id: &str, @@ -1012,11 +984,11 @@ impl DockerComputeDriver { .await { Ok(()) => { - cleanup_sandbox_token_file(&record.sandbox, &self.config); + cleanup_sandbox_state_files(&record.sandbox, &self.config); return Ok(true); } Err(err) if is_not_found_error(&err) => { - cleanup_sandbox_token_file(&record.sandbox, &self.config); + cleanup_sandbox_state_files(&record.sandbox, &self.config); return Ok(true); } Err(err) => { @@ -1039,11 +1011,11 @@ impl DockerComputeDriver { .await { Ok(()) => { - cleanup_sandbox_token_file_for_delete(sandbox_id, pending.as_ref(), &self.config); + cleanup_sandbox_state_files_for_delete(sandbox_id, pending.as_ref(), &self.config); Ok(true) } Err(err) if is_not_found_error(&err) => { - cleanup_sandbox_token_file_for_delete(sandbox_id, pending.as_ref(), &self.config); + cleanup_sandbox_state_files_for_delete(sandbox_id, pending.as_ref(), &self.config); Ok(pending.is_some()) } Err(err) => Err(internal_status("delete docker sandbox container", err)), @@ -1059,7 +1031,7 @@ impl DockerComputeDriver { if let Some(task) = record.task { task.abort(); } - cleanup_sandbox_token_file(&record.sandbox, &self.config); + cleanup_sandbox_state_files(&record.sandbox, &self.config); self.publish_deleted(record.sandbox.id); return Ok(()); } @@ -1287,7 +1259,7 @@ impl DockerComputeDriver { sandbox: &DriverSandbox, failure: &DockerProvisioningFailure, ) { - cleanup_sandbox_token_file(sandbox, &self.config); + cleanup_sandbox_state_files(sandbox, &self.config); let snapshot = pending_sandbox_snapshot( sandbox, &self.config.sandbox_namespace, @@ -2375,6 +2347,7 @@ fn docker_volume_is_bind_backed(volume: &bollard::models::Volume) -> bool { fn build_binds( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, + gpu_device_ids: Option<&[String]>, ) -> Result, Status> { let mut binds = vec![format!( "{}:{}:ro,z", @@ -2401,6 +2374,13 @@ fn build_binds( SANDBOX_TOKEN_MOUNT_PATH )); } + if cdi_context_requested(gpu_device_ids) { + binds.push(format!( + "{}:{}:ro,z", + cdi_context_host_path(sandbox, config)?.display(), + openshell_core::cdi::CDI_CONTEXT_PATH + )); + } Ok(binds) } @@ -2427,6 +2407,57 @@ fn sandbox_token_host_path_by_id( }) } +fn cdi_context_host_path( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, +) -> Result { + cdi_context_host_path_by_id(&sandbox.id, config) +} + +fn cdi_context_host_path_by_id( + sandbox_id: &str, + config: &DockerDriverRuntimeConfig, +) -> Result { + openshell_core::driver_utils::sandbox_token_path( + "docker-cdi-contexts", + Some(&config.sandbox_namespace), + sandbox_id, + ) + .map(|path| path.with_file_name(openshell_core::cdi::CDI_CONTEXT_FILE_NAME)) + .map_err(|err| Status::internal(format!("resolve CDI context state directory failed: {err}"))) +} + +fn write_cdi_context_file( + sandbox: &DriverSandbox, + config: &DockerDriverRuntimeConfig, + context: &CdiContext, +) -> Result<(), Status> { + let path = cdi_context_host_path(sandbox, config)?; + if let Some(parent) = path.parent() { + openshell_core::paths::create_dir_restricted(parent).map_err(|err| { + Status::internal(format!( + "create CDI context directory {} failed: {err}", + parent.display() + )) + })?; + } + let json = serde_json::to_vec(context) + .map_err(|err| Status::internal(format!("encode CDI context failed: {err}")))?; + std::fs::write(&path, json).map_err(|err| { + Status::internal(format!( + "write CDI context file {} failed: {err}", + path.display() + )) + })?; + openshell_core::paths::set_file_owner_only(&path).map_err(|err| { + Status::internal(format!( + "restrict CDI context file {} failed: {err}", + path.display() + )) + })?; + Ok(()) +} + async fn write_sandbox_token_file( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, @@ -2467,6 +2498,15 @@ fn cleanup_sandbox_token_file(sandbox: &DriverSandbox, config: &DockerDriverRunt cleanup_sandbox_token_file_by_id(&sandbox.id, config); } +fn cleanup_cdi_context_file(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) { + cleanup_cdi_context_file_by_id(&sandbox.id, config); +} + +fn cleanup_sandbox_state_files(sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig) { + cleanup_sandbox_token_file(sandbox, config); + cleanup_cdi_context_file(sandbox, config); +} + fn cleanup_sandbox_token_file_for_delete( sandbox_id: &str, pending: Option<&PendingSandboxRecord>, @@ -2479,6 +2519,27 @@ fn cleanup_sandbox_token_file_for_delete( } } +fn cleanup_cdi_context_file_for_delete( + sandbox_id: &str, + pending: Option<&PendingSandboxRecord>, + config: &DockerDriverRuntimeConfig, +) { + if !sandbox_id.is_empty() { + cleanup_cdi_context_file_by_id(sandbox_id, config); + } else if let Some(record) = pending { + cleanup_cdi_context_file(&record.sandbox, config); + } +} + +fn cleanup_sandbox_state_files_for_delete( + sandbox_id: &str, + pending: Option<&PendingSandboxRecord>, + config: &DockerDriverRuntimeConfig, +) { + cleanup_sandbox_token_file_for_delete(sandbox_id, pending, config); + cleanup_cdi_context_file_for_delete(sandbox_id, pending, config); +} + fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRuntimeConfig) { let Ok(path) = sandbox_token_host_path_by_id(sandbox_id, config) else { return; @@ -2498,6 +2559,25 @@ fn cleanup_sandbox_token_file_by_id(sandbox_id: &str, config: &DockerDriverRunti } } +fn cleanup_cdi_context_file_by_id(sandbox_id: &str, config: &DockerDriverRuntimeConfig) { + let Ok(path) = cdi_context_host_path_by_id(sandbox_id, config) else { + return; + }; + if let Err(err) = std::fs::remove_file(&path) + && err.kind() != std::io::ErrorKind::NotFound + { + warn!( + sandbox_id = %sandbox_id, + path = %path.display(), + error = %err, + "Failed to remove Docker CDI context file" + ); + } + if let Some(dir) = path.parent() { + let _ = std::fs::remove_dir(dir); + } +} + #[cfg(test)] fn build_environment( sandbox: &DriverSandbox, @@ -2663,64 +2743,6 @@ fn cdi_context_requested(gpu_device_ids: Option<&[String]>) -> bool { gpu_device_ids.is_some_and(|device_ids| !device_ids.is_empty()) } -fn build_cdi_context_archive(context: &CdiContext) -> Result, String> { - let json = serde_json::to_vec_pretty(context).map_err(|err| err.to_string())?; - let mut archive = DockerTarArchiveBuilder::new(); - archive.append_dir("openshell", 0o700)?; - archive.append_dir("openshell/supervisor", 0o700)?; - archive.append_file( - &format!( - "openshell/supervisor/{}", - openshell_core::cdi::CDI_CONTEXT_FILE_NAME - ), - 0o600, - json, - )?; - archive.into_inner() -} - -struct DockerTarArchiveBuilder { - inner: tar::Builder>, -} - -impl DockerTarArchiveBuilder { - fn new() -> Self { - Self { - inner: tar::Builder::new(Vec::new()), - } - } - - fn append_dir(&mut self, path: &str, mode: u32) -> Result<(), String> { - let mut header = tar::Header::new_gnu(); - header.set_entry_type(tar::EntryType::Directory); - header.set_size(0); - header.set_mode(mode); - header.set_uid(0); - header.set_gid(0); - header.set_cksum(); - self.inner - .append_data(&mut header, path, std::io::empty()) - .map_err(|err| err.to_string()) - } - - fn append_file(&mut self, path: &str, mode: u32, contents: Vec) -> Result<(), String> { - let mut header = tar::Header::new_gnu(); - header.set_entry_type(tar::EntryType::Regular); - header.set_size(u64::try_from(contents.len()).map_err(|err| err.to_string())?); - header.set_mode(mode); - header.set_uid(0); - header.set_gid(0); - header.set_cksum(); - self.inner - .append_data(&mut header, path, Cursor::new(contents)) - .map_err(|err| err.to_string()) - } - - fn into_inner(self) -> Result, String> { - self.inner.into_inner().map_err(|err| err.to_string()) - } -} - #[cfg(test)] fn build_container_create_body( sandbox: &DriverSandbox, @@ -2874,7 +2896,7 @@ fn build_container_create_body_for_image( pids_limit: docker_pids_limit(config.sandbox_pids_limit)?, device_requests, binds: { - let mut binds = build_binds(sandbox, config)?; + let mut binds = build_binds(sandbox, config, gpu_device_ids)?; binds.extend(config.gpu.cdi_spec_bind_strings(gpu_device_ids)?); binds.extend(user_bind_strings); Some(binds) diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index a63c2297c0..a1bc44d1d6 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -944,7 +944,7 @@ fn build_environment_keeps_telemetry_toggle_driver_controlled() { #[test] fn build_binds_uses_docker_tls_directory() { - let binds = build_binds(&test_sandbox(), &runtime_config(false)).unwrap(); + let binds = build_binds(&test_sandbox(), &runtime_config(false), None).unwrap(); let targets = binds .iter() .filter_map(|bind| bind.split(':').nth(1).map(String::from)) @@ -1847,6 +1847,13 @@ fn build_container_create_body_adds_cdi_context_env_and_spec_mounts_for_gpu() { assert!(binds.iter().any(|bind| { bind == &format!("{TEST_CDI_SPEC_DIR_ALT}:{}:ro,z", cdi_spec_mount_path(1)) })); + assert!(binds.iter().any(|bind| { + bind == &format!( + "{}:{}:ro,z", + cdi_context_host_path(&sandbox, &config).unwrap().display(), + openshell_core::cdi::CDI_CONTEXT_PATH + ) + })); } #[test] @@ -1874,9 +1881,11 @@ fn build_container_create_body_clears_cdi_context_for_non_gpu() { } #[test] -fn build_cdi_context_archive_contains_context_json() { - use std::io::Read as _; - +fn write_cdi_context_file_materializes_owned_host_context() { + let _guard = ENV_LOCK.lock().unwrap(); + let state_dir = tempfile::tempdir().unwrap(); + let sandbox = test_sandbox(); + let config = runtime_config(true); let context = CdiContext::new( vec!["nvidia.com/gpu=0".to_string()], vec![CdiSpecDirectory::new( @@ -1884,20 +1893,16 @@ fn build_cdi_context_archive_contains_context_json() { TEST_CDI_SPEC_DIR, )], ); - let bytes = build_cdi_context_archive(&context).unwrap(); - let mut archive = tar::Archive::new(Cursor::new(bytes)); - let mut found = false; - for entry in archive.entries().unwrap() { - let mut entry = entry.unwrap(); - if entry.path().unwrap().as_ref() == Path::new("openshell/supervisor/cdi-context.json") { - let mut payload = String::new(); - entry.read_to_string(&mut payload).unwrap(); - let parsed: CdiContext = serde_json::from_str(&payload).unwrap(); - assert_eq!(parsed, context); - found = true; - } - } - assert!(found, "archive must include cdi-context.json"); + + temp_env::with_var("XDG_STATE_HOME", Some(state_dir.path()), || { + write_cdi_context_file(&sandbox, &config, &context).expect("write CDI context"); + let path = cdi_context_host_path(&sandbox, &config).expect("context path"); + let contents = fs::read(&path).expect("read CDI context"); + let parsed: CdiContext = serde_json::from_slice(&contents).expect("parse CDI context"); + assert_eq!(parsed, context); + cleanup_cdi_context_file(&sandbox, &config); + assert!(!path.exists()); + }); } #[test] From 9f1a3ef8dba171cd06f9329dc2d03c934b43bf52 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Wed, 19 Aug 2026 09:50:40 +0200 Subject: [PATCH 3/3] fix(docker): document CDI context mount Signed-off-by: Evan Lezar --- architecture/compute-runtimes.md | 9 +++++---- crates/openshell-driver-docker/Cargo.toml | 2 +- crates/openshell-driver-docker/README.md | 14 ++++++++------ docs/reference/sandbox-compute-drivers.mdx | 12 +++++++----- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index f6fdae7cec..794e7fd6c2 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -172,10 +172,11 @@ For all in-tree drivers, this is equivalent to selecting a single GPU. For Docker GPU sandboxes, the driver treats CDI specs as runtime metadata for both outer injection and inner sandbox policy. It selects opaque CDI device IDs, passes them to Docker, mounts daemon-reported CDI spec directories into -supervisor-only paths, and uploads a versioned CDI context before starting the -container. The supervisor resolves that context inside the sandbox and derives -Landlock paths and supplemental groups from CDI `containerEdits`. Host-side CDI -spec paths are diagnostic only and are never treated as sandbox policy paths. +supervisor-only paths, and bind-mounts a gateway-owned versioned CDI context +read-only before creating the container. The supervisor resolves that context +inside the sandbox and derives Landlock paths and supplemental groups from CDI +`containerEdits`. Host-side CDI spec paths are diagnostic only and are never +treated as sandbox policy paths. Kubernetes must not infer CDI device IDs from the `nvidia.com/gpu` resource request; it needs a node-local selected-device handoff before using the same supervisor resolver. diff --git a/crates/openshell-driver-docker/Cargo.toml b/crates/openshell-driver-docker/Cargo.toml index 61519ec47c..92a32c6d20 100644 --- a/crates/openshell-driver-docker/Cargo.toml +++ b/crates/openshell-driver-docker/Cargo.toml @@ -24,10 +24,10 @@ serde_json = { workspace = true } prost-types = { workspace = true } bollard = { version = "0.20" } url = { workspace = true } -tar = "0.4" [dev-dependencies] prost-types = { workspace = true } +tar = "0.4" temp-env = "0.3" tempfile = "3" diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index 7167297cd9..27225b9dff 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -86,7 +86,7 @@ contract: | `restart_policy = unless-stopped` | Keeps managed sandboxes resumable across daemon or gateway restarts. | | `PidsLimit` | Enforces the sandbox PID budget at the Docker cgroup layer. Set `[openshell.drivers.docker].sandbox_pids_limit = 0` to inherit the Docker/runtime default. | | CDI GPU request | Uses opaque `driver_config.cdi_devices` values when set; otherwise selects the requested count of NVIDIA CDI GPUs in round-robin order when daemon CDI support is detected. Docker daemon `/info` can permit `nvidia.com/gpu=all` as a WSL2 all-only compatibility fallback, where it counts as one selectable device. Exact CDI device lists must not contain duplicates and must match the effective GPU count. | -| CDI context upload | For GPU/CDI sandboxes only, mounts daemon-reported CDI spec directories read-only under `/run/openshell/supervisor/cdi-specs/` and uploads `/run/openshell/supervisor/cdi-context.json` after container create and before start. | +| CDI context mount | For GPU/CDI sandboxes only, creates a gateway-owned context file and bind-mounts it read-only at `/run/openshell/supervisor/cdi-context.json`; daemon-reported CDI spec directories are mounted read-only under `/run/openshell/supervisor/cdi-specs/`. | The agent child process does not retain these supervisor privileges. @@ -97,11 +97,13 @@ CDI device IDs from `driver_config.cdi_devices` or the daemon's discovered CDI inventory, then passes the same IDs to Docker with a CDI `DeviceRequest`. When a GPU/CDI request is present, the driver also mounts the Docker -daemon-reported `Info.CDISpecDirs` into supervisor-only paths and uploads a -small versioned CDI context through Docker's container archive API. The context -uses container-side spec paths for resolution and keeps host-side spec sources -diagnostic-only. If the upload fails, the driver removes the created container -and sandbox token file before reporting the failure. +daemon-reported `Info.CDISpecDirs` into supervisor-only paths. Before container +creation, it writes a small versioned CDI context in gateway-owned state and +bind-mounts it read-only into the supervisor. The context uses container-side +spec paths for resolution and keeps host-side spec sources diagnostic-only. If +context or token creation fails, the driver removes any created state files; if +container creation or start fails, it also removes the container and state +files before reporting the failure. The sandbox supervisor resolves the selected IDs from those mounted specs before it launches agent processes. CDI device nodes become read-write diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index df0de711ea..0aa1eeed85 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -148,11 +148,13 @@ For GPU-backed Docker sandboxes, configure Docker CDI before starting the gatewa For Docker GPU/CDI sandboxes, OpenShell uses Docker's selected CDI device IDs and daemon-reported CDI spec directories to build a supervisor-only CDI context. The driver mounts the spec directories read-only into the sandbox -container, uploads `cdi-context.json` before starting the container, and removes -the created container if that upload fails. The supervisor resolves the context -inside the sandbox and derives the inner filesystem and supplemental group -requirements from CDI specs. Non-GPU Docker sandboxes do not receive the CDI -context, spec mounts, or CDI-derived policy changes. +container. Before creation, it writes a gateway-owned `cdi-context.json` and +bind-mounts it read-only into the supervisor. If context or token creation +fails, the driver removes the created state files; if container creation or +start fails, it also removes the container and state files. The supervisor +resolves the context inside the sandbox and derives the inner filesystem and +supplemental group requirements from CDI specs. Non-GPU Docker sandboxes do not +receive the CDI context, spec mounts, or CDI-derived policy changes. ### Docker Driver Config Mounts