From 8483221ae653619bcb9c474a73b66dcb18fa6d93 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 5 Jun 2026 13:55:44 -0700 Subject: [PATCH 1/3] feat(drivers): support docker and podman config mounts Signed-off-by: Drew Newberry --- Cargo.lock | 3 + architecture/compute-runtimes.md | 6 + crates/openshell-driver-docker/Cargo.toml | 2 + crates/openshell-driver-docker/README.md | 35 ++ crates/openshell-driver-docker/src/lib.rs | 372 ++++++++++- crates/openshell-driver-docker/src/tests.rs | 162 +++++ crates/openshell-driver-podman/Cargo.toml | 1 + crates/openshell-driver-podman/README.md | 37 ++ crates/openshell-driver-podman/src/client.rs | 17 + .../openshell-driver-podman/src/container.rs | 584 +++++++++++++++++- crates/openshell-driver-podman/src/driver.rs | 43 +- crates/openshell-driver-podman/src/grpc.rs | 1 + docs/reference/sandbox-compute-drivers.mdx | 98 +++ docs/sandboxes/manage-sandboxes.mdx | 23 + 14 files changed, 1362 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4bc657be3..04b561899 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3481,7 +3481,9 @@ dependencies = [ "bytes", "futures", "openshell-core", + "prost-types", "serde", + "serde_json", "tar", "temp-env", "tempfile", @@ -3528,6 +3530,7 @@ dependencies = [ "miette", "nix", "openshell-core", + "prost-types", "rustix 1.1.4", "serde", "serde_json", diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index b70a2fccc..585c13e48 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -40,6 +40,12 @@ template resource limits. Docker and Podman apply them as runtime limits. Kubernetes mirrors each limit into the matching request. VM accepts the fields but currently ignores them. +Docker and Podman also accept per-sandbox driver-config mounts for existing +runtime-managed named volumes and tmpfs mounts. Podman additionally accepts +image mounts through its image-volume API. User-supplied host bind mounts are +excluded from the driver-config contract; bind mounts remain reserved for +driver-owned supervisor, token, and TLS material. + Kubernetes deployments may set an AppArmor profile on sandbox agent containers through the driver configuration. The Helm chart defaults sandbox agents to `Unconfined` so runtime/default AppArmor profiles do not block supervisor diff --git a/crates/openshell-driver-docker/Cargo.toml b/crates/openshell-driver-docker/Cargo.toml index 0cdc205ed..bc0680304 100644 --- a/crates/openshell-driver-docker/Cargo.toml +++ b/crates/openshell-driver-docker/Cargo.toml @@ -20,6 +20,8 @@ tokio-stream = { workspace = true } tracing = { workspace = true } bytes = { workspace = true } serde = { workspace = true } +serde_json = { workspace = true } +prost-types = { workspace = true } bollard = { version = "0.20" } tar = "0.4" tempfile = "3" diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index ea57f44e4..d71b18a84 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -36,6 +36,41 @@ contract: The agent child process does not retain these supervisor privileges. +## Driver Config Mounts + +The gateway forwards the `docker` block from `--driver-config-json` to this +driver. The driver accepts user-supplied `mounts` entries with these Docker +mount types: + +- `volume`: mounts an existing Docker named volume. The driver validates that + the volume exists before provisioning and never creates or removes it. +- `tmpfs`: mounts an in-memory filesystem with optional `options`, + `size_bytes`, and `mode`. + +Host bind mounts and image mounts are intentionally not part of the Docker +driver-config schema. The driver still uses internal bind mounts for +OpenShell-owned supervisor, token, and TLS material. + +Docker `volume` mounts may include `subpath`. Mount targets must be absolute +container paths and must not replace the workspace root (`/sandbox`) or overlap +OpenShell supervisor files, auth material, TLS material, or `/run/netns`. + +Example NFS usage relies on Docker's named-volume support rather than a host +bind: + +```shell +docker volume create \ + --driver local \ + --opt type=nfs \ + --opt o=addr=10.0.0.10,rw,nfsvers=4 \ + --opt device=:/exports/work \ + work-nfs + +openshell sandbox create \ + --driver-config-json '{"docker":{"mounts":[{"type":"volume","source":"work-nfs","target":"/sandbox/work"}]}}' \ + -- claude +``` + ## Supervisor Binary Resolution The Docker driver bind-mounts a host-side Linux `openshell-sandbox` binary into diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 864d91f22..7e4f2bc40 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -9,8 +9,9 @@ use bollard::Docker; use bollard::errors::Error as BollardError; use bollard::models::{ ContainerCreateBody, ContainerSummary, ContainerSummaryStateEnum, CreateImageInfo, - DeviceRequest, EndpointSettings, HostConfig, NetworkCreateRequest, NetworkingConfig, - ProgressDetail, RestartPolicy, RestartPolicyNameEnum, SystemInfo, + DeviceRequest, EndpointSettings, HostConfig, Mount, MountTmpfsOptions, MountTypeEnum, + MountVolumeOptions, NetworkCreateRequest, NetworkingConfig, ProgressDetail, RestartPolicy, + RestartPolicyNameEnum, SystemInfo, }; use bollard::query_parameters::{ CreateContainerOptionsBuilder, CreateImageOptions, DownloadFromContainerOptionsBuilder, @@ -41,7 +42,7 @@ use openshell_core::proto::compute::v1::{ watch_sandboxes_event, }; use openshell_core::{Config, Error, Result as CoreResult}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io::Read; use std::net::{IpAddr, SocketAddr}; use std::path::{Path, PathBuf}; @@ -258,6 +259,34 @@ struct DockerResourceLimits { memory_bytes: Option, } +#[derive(Debug, Clone, Default, serde::Deserialize)] +#[serde(default, deny_unknown_fields)] +struct DockerSandboxDriverConfig { + mounts: Vec, +} + +#[derive(Debug, Clone, serde::Deserialize)] +#[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)] +enum DockerDriverMountConfig { + Volume { + source: String, + target: String, + #[serde(default)] + read_only: bool, + #[serde(default)] + subpath: Option, + }, + Tmpfs { + target: String, + #[serde(default)] + options: Vec, + #[serde(default)] + size_bytes: Option, + #[serde(default)] + mode: Option, + }, +} + type WatchStream = Pin> + Send + 'static>>; @@ -391,6 +420,7 @@ impl DockerComputeDriver { )); } + let _ = docker_driver_config(template)?; let _ = docker_resource_limits(template)?; Ok(()) } @@ -418,6 +448,35 @@ impl DockerComputeDriver { Ok(()) } + async fn validate_user_volume_mounts_available( + &self, + sandbox: &DriverSandbox, + ) -> Result<(), Status> { + let template = sandbox + .spec + .as_ref() + .and_then(|spec| spec.template.as_ref()) + .ok_or_else(|| Status::invalid_argument("sandbox.spec.template is required"))?; + let config = docker_driver_config(template)?; + for mount in config.mounts { + if let DockerDriverMountConfig::Volume { source, .. } = mount { + match self.docker.inspect_volume(source.trim()).await { + Ok(_) => {} + Err(err) if is_not_found_error(&err) => { + return Err(Status::failed_precondition(format!( + "docker volume '{}' does not exist", + source.trim() + ))); + } + Err(err) => { + return Err(internal_status("inspect docker volume", err)); + } + } + } + } + Ok(()) + } + async fn get_sandbox_snapshot( &self, sandbox_id: &str, @@ -455,6 +514,7 @@ impl DockerComputeDriver { async fn create_sandbox_inner(&self, sandbox: &DriverSandbox) -> Result<(), Status> { Self::validate_sandbox(sandbox, &self.config)?; Self::validate_sandbox_auth(sandbox)?; + self.validate_user_volume_mounts_available(sandbox).await?; let _ = build_container_create_body(sandbox, &self.config)?; if self @@ -1165,6 +1225,7 @@ impl ComputeDriver for DockerComputeDriver { .sandbox .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; Self::validate_sandbox(&sandbox, &self.config)?; + self.validate_user_volume_mounts_available(&sandbox).await?; Ok(Response::new(ValidateSandboxCreateResponse {})) } @@ -1505,6 +1566,309 @@ fn attach_docker_progress_metadata( } } +fn docker_driver_config( + template: &DriverSandboxTemplate, +) -> Result { + let Some(config) = template.driver_config.as_ref() else { + return Ok(DockerSandboxDriverConfig::default()); + }; + + let json = serde_json::Value::Object(proto_struct_to_json_object(config)); + let config: DockerSandboxDriverConfig = serde_json::from_value(json).map_err(|err| { + Status::failed_precondition(format!("invalid docker driver_config: {err}")) + })?; + validate_docker_driver_mounts(&config.mounts)?; + Ok(config) +} + +fn docker_driver_mounts(template: &DriverSandboxTemplate) -> Result, Status> { + let config = docker_driver_config(template)?; + config.mounts.iter().map(docker_mount_from_config).collect() +} + +fn docker_mount_from_config(config: &DockerDriverMountConfig) -> Result { + match config { + DockerDriverMountConfig::Volume { + source, + target, + read_only, + subpath, + } => Ok(Mount { + typ: Some(MountTypeEnum::VOLUME), + source: Some(validate_mount_source(source, "volume source")?), + target: Some(validate_container_mount_target(target)?), + read_only: Some(*read_only), + volume_options: subpath + .as_ref() + .map(|subpath| { + Ok::(MountVolumeOptions { + subpath: Some(validate_mount_subpath(subpath)?), + ..Default::default() + }) + }) + .transpose()?, + ..Default::default() + }), + DockerDriverMountConfig::Tmpfs { + target, + options, + size_bytes, + mode, + } => Ok(Mount { + typ: Some(MountTypeEnum::TMPFS), + target: Some(validate_container_mount_target(target)?), + tmpfs_options: Some(MountTmpfsOptions { + size_bytes: validate_optional_positive_integral_i64( + *size_bytes, + "tmpfs size_bytes", + )?, + mode: validate_optional_nonnegative_integral_i64(*mode, "tmpfs mode")?, + options: (!options.is_empty()) + .then(|| { + options + .iter() + .map(|option| docker_tmpfs_option(option)) + .collect::, _>>() + }) + .transpose()?, + }), + ..Default::default() + }), + } +} + +fn validate_docker_driver_mounts(mounts: &[DockerDriverMountConfig]) -> Result<(), Status> { + let mut targets = HashSet::new(); + for mount in mounts { + let target = match mount { + DockerDriverMountConfig::Volume { + source, + target, + subpath, + .. + } => { + validate_mount_source(source, "volume source")?; + if let Some(subpath) = subpath { + validate_mount_subpath(subpath)?; + } + target + } + DockerDriverMountConfig::Tmpfs { + target, + options, + size_bytes, + mode, + } => { + validate_optional_positive_integral_i64(*size_bytes, "tmpfs size_bytes")?; + validate_optional_nonnegative_integral_i64(*mode, "tmpfs mode")?; + for option in options { + docker_tmpfs_option(option)?; + } + target + } + }; + let target = validate_container_mount_target(target)?; + if !targets.insert(target.clone()) { + return Err(Status::failed_precondition(format!( + "duplicate docker driver_config mount target '{target}'" + ))); + } + } + Ok(()) +} + +fn validate_mount_source(source: &str, field: &str) -> Result { + let source = source.trim(); + if source.is_empty() { + return Err(Status::failed_precondition(format!( + "{field} must not be empty" + ))); + } + if source.as_bytes().contains(&0) { + return Err(Status::failed_precondition(format!( + "{field} must not contain NUL bytes" + ))); + } + Ok(source.to_string()) +} + +fn validate_mount_subpath(subpath: &str) -> Result { + let subpath = subpath.trim(); + if subpath.is_empty() { + return Err(Status::failed_precondition( + "mount subpath must not be empty", + )); + } + let path = Path::new(subpath); + if path.is_absolute() + || path + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)) + { + return Err(Status::failed_precondition( + "mount subpath must be relative and must not contain '..'", + )); + } + Ok(subpath.to_string()) +} + +fn validate_optional_positive_integral_i64( + value: Option, + field: &str, +) -> Result, Status> { + let Some(value) = validate_optional_integral_i64(value, field)? else { + return Ok(None); + }; + if value <= 0 { + return Err(Status::failed_precondition(format!( + "{field} must be positive" + ))); + } + Ok(Some(value)) +} + +fn validate_optional_nonnegative_integral_i64( + value: Option, + field: &str, +) -> Result, Status> { + let Some(value) = validate_optional_integral_i64(value, field)? else { + return Ok(None); + }; + if value < 0 { + return Err(Status::failed_precondition(format!( + "{field} must be zero or greater" + ))); + } + Ok(Some(value)) +} + +fn validate_optional_integral_i64(value: Option, field: &str) -> Result, Status> { + let Some(value) = value else { + return Ok(None); + }; + if !value.is_finite() || value.fract() != 0.0 { + return Err(Status::failed_precondition(format!( + "{field} must be an integer" + ))); + } + value.to_string().parse::().map(Some).map_err(|_| { + Status::failed_precondition(format!("{field} must be representable as an i64")) + }) +} + +fn docker_tmpfs_option(option: &str) -> Result, Status> { + let option = option.trim(); + if option.is_empty() { + return Err(Status::failed_precondition( + "tmpfs options must not contain empty values", + )); + } + if let Some((key, value)) = option.split_once('=') { + let key = key.trim(); + let value = value.trim(); + if key.is_empty() || value.is_empty() { + return Err(Status::failed_precondition( + "tmpfs key=value options must include both key and value", + )); + } + Ok(vec![key.to_string(), value.to_string()]) + } else { + Ok(vec![option.to_string()]) + } +} + +fn validate_container_mount_target(target: &str) -> Result { + let target = normalize_container_mount_target(target); + if target.is_empty() { + return Err(Status::failed_precondition( + "mount target must not be empty", + )); + } + if target.as_bytes().contains(&0) { + return Err(Status::failed_precondition( + "mount target must not contain NUL bytes", + )); + } + if !target.starts_with('/') { + return Err(Status::failed_precondition( + "mount target must be an absolute container path", + )); + } + if target == "/" { + return Err(Status::failed_precondition( + "mount target must not be the container root", + )); + } + let path = Path::new(&target); + if path + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)) + { + return Err(Status::failed_precondition( + "mount target must not contain '..'", + )); + } + if target == "/sandbox" { + return Err(Status::failed_precondition( + "mount target '/sandbox' is reserved for the OpenShell workspace", + )); + } + for reserved in [ + "/opt/openshell", + "/etc/openshell/auth", + "/etc/openshell/tls", + "/run/netns", + ] { + if path_is_or_under(&target, reserved) { + return Err(Status::failed_precondition(format!( + "mount target '{target}' conflicts with reserved OpenShell path '{reserved}'" + ))); + } + } + Ok(target) +} + +fn normalize_container_mount_target(target: &str) -> String { + let target = target.trim(); + if target == "/" { + return target.to_string(); + } + target.trim_end_matches('/').to_string() +} + +fn path_is_or_under(path: &str, parent: &str) -> bool { + path == parent + || path + .strip_prefix(parent) + .is_some_and(|rest| rest.starts_with('/')) +} + +fn proto_struct_to_json_object( + config: &prost_types::Struct, +) -> serde_json::Map { + config + .fields + .iter() + .map(|(key, value)| (key.clone(), proto_value_to_json(value))) + .collect() +} + +fn proto_value_to_json(value: &prost_types::Value) -> serde_json::Value { + match value.kind.as_ref() { + Some(prost_types::value::Kind::NumberValue(num)) => serde_json::Number::from_f64(*num) + .map_or(serde_json::Value::Null, serde_json::Value::Number), + Some(prost_types::value::Kind::StringValue(val)) => serde_json::Value::String(val.clone()), + Some(prost_types::value::Kind::BoolValue(val)) => serde_json::Value::Bool(*val), + Some(prost_types::value::Kind::StructValue(val)) => { + serde_json::Value::Object(proto_struct_to_json_object(val)) + } + Some(prost_types::value::Kind::ListValue(list)) => { + serde_json::Value::Array(list.values.iter().map(proto_value_to_json).collect()) + } + Some(prost_types::value::Kind::NullValue(_)) | None => serde_json::Value::Null, + } +} + fn build_binds( sandbox: &DriverSandbox, config: &DockerDriverRuntimeConfig, @@ -1736,6 +2100,7 @@ fn build_container_create_body( .as_ref() .ok_or_else(|| Status::invalid_argument("sandbox.spec.template is required"))?; let resource_limits = docker_resource_limits(template)?; + let user_mounts = docker_driver_mounts(template)?; let mut labels = template.labels.clone(); labels.insert( LABEL_MANAGED_BY.to_string(), @@ -1767,6 +2132,7 @@ fn build_container_create_body( pids_limit: docker_pids_limit(config.sandbox_pids_limit)?, device_requests: docker_gpu_device_requests(spec.gpu, &spec.gpu_device), binds: Some(build_binds(sandbox, config)?), + mounts: (!user_mounts.is_empty()).then_some(user_mounts), restart_policy: Some(RestartPolicy { name: Some(RestartPolicyNameEnum::UNLESS_STOPPED), maximum_retry_count: None, diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 4a902a48b..3864217ea 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -78,6 +78,43 @@ fn runtime_config() -> DockerDriverRuntimeConfig { } } +fn json_struct(value: serde_json::Value) -> prost_types::Struct { + match json_value(value).kind { + Some(prost_types::value::Kind::StructValue(value)) => value, + _ => panic!("expected JSON object"), + } +} + +fn json_value(value: serde_json::Value) -> prost_types::Value { + match value { + serde_json::Value::Null => prost_types::Value { kind: None }, + serde_json::Value::Bool(value) => prost_types::Value { + kind: Some(prost_types::value::Kind::BoolValue(value)), + }, + serde_json::Value::Number(value) => prost_types::Value { + kind: value.as_f64().map(prost_types::value::Kind::NumberValue), + }, + serde_json::Value::String(value) => prost_types::Value { + kind: Some(prost_types::value::Kind::StringValue(value)), + }, + serde_json::Value::Array(values) => prost_types::Value { + kind: Some(prost_types::value::Kind::ListValue( + prost_types::ListValue { + values: values.into_iter().map(json_value).collect(), + }, + )), + }, + serde_json::Value::Object(values) => prost_types::Value { + kind: Some(prost_types::value::Kind::StructValue(prost_types::Struct { + fields: values + .into_iter() + .map(|(key, value)| (key, json_value(value))) + .collect(), + })), + }, + } +} + #[test] fn container_visible_endpoint_rewrites_loopback_hosts() { assert_eq!( @@ -522,6 +559,131 @@ fn build_binds_uses_docker_tls_directory() { ); } +#[test] +fn build_container_create_body_includes_driver_config_mounts() { + let mut sandbox = test_sandbox(); + let template = sandbox.spec.as_mut().unwrap().template.as_mut().unwrap(); + template.driver_config = Some(json_struct(serde_json::json!({ + "mounts": [ + { + "type": "volume", + "source": "work-nfs", + "target": "/sandbox/work", + "read_only": true, + "subpath": "project-a" + }, + { + "type": "tmpfs", + "target": "/sandbox/cache", + "options": ["nosuid", "size=1048576"], + "size_bytes": 1_048_576, + "mode": 511 + } + ] + }))); + + let body = build_container_create_body(&sandbox, &runtime_config()).unwrap(); + let mounts = body + .host_config + .unwrap() + .mounts + .expect("driver config mounts should be set"); + + assert_eq!(mounts.len(), 2); + assert_eq!(mounts[0].typ, Some(MountTypeEnum::VOLUME)); + assert_eq!(mounts[0].source.as_deref(), Some("work-nfs")); + assert_eq!(mounts[0].target.as_deref(), Some("/sandbox/work")); + assert_eq!(mounts[0].read_only, Some(true)); + assert_eq!( + mounts[0] + .volume_options + .as_ref() + .and_then(|options| options.subpath.as_deref()), + Some("project-a") + ); + assert_eq!(mounts[1].typ, Some(MountTypeEnum::TMPFS)); + assert_eq!(mounts[1].target.as_deref(), Some("/sandbox/cache")); + assert_eq!( + mounts[1] + .tmpfs_options + .as_ref() + .and_then(|options| options.size_bytes), + Some(1_048_576) + ); +} + +#[test] +fn driver_config_rejects_bind_mounts() { + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": "/host/path", + "target": "/sandbox/host" + }] + }))); + + let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + + assert_eq!(err.code(), tonic::Code::FailedPrecondition); + assert!(err.message().contains("invalid docker driver_config")); +} + +#[test] +fn driver_config_rejects_image_mounts() { + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "image", + "source": "ghcr.io/acme/tools:latest", + "target": "/opt/tools" + }] + }))); + + let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + + assert_eq!(err.code(), tonic::Code::FailedPrecondition); + assert!(err.message().contains("invalid docker driver_config")); +} + +#[test] +fn driver_config_rejects_reserved_mount_targets() { + let mut sandbox = test_sandbox(); + sandbox + .spec + .as_mut() + .unwrap() + .template + .as_mut() + .unwrap() + .driver_config = Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "volume", + "source": "work-nfs", + "target": "/etc/openshell/auth/custom" + }] + }))); + + let err = build_container_create_body(&sandbox, &runtime_config()).unwrap_err(); + + assert_eq!(err.code(), tonic::Code::FailedPrecondition); + assert!(err.message().contains("reserved OpenShell path")); +} + #[test] fn build_environment_uses_token_file_without_raw_token_env() { let mut sandbox = test_sandbox(); diff --git a/crates/openshell-driver-podman/Cargo.toml b/crates/openshell-driver-podman/Cargo.toml index 6f2963d92..4a3196697 100644 --- a/crates/openshell-driver-podman/Cargo.toml +++ b/crates/openshell-driver-podman/Cargo.toml @@ -26,6 +26,7 @@ hyper-util = { workspace = true } http-body-util = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +prost-types = { workspace = true } clap = { workspace = true } nix = { workspace = true } rustix = { workspace = true } diff --git a/crates/openshell-driver-podman/README.md b/crates/openshell-driver-podman/README.md index 77b42ba37..2f238821f 100644 --- a/crates/openshell-driver-podman/README.md +++ b/crates/openshell-driver-podman/README.md @@ -50,6 +50,43 @@ The container spec in `container.rs` sets these security-critical fields: The restricted agent child does not retain these supervisor privileges. +## Driver Config Mounts + +The gateway forwards the `podman` block from `--driver-config-json` to this +driver. The driver accepts user-supplied `mounts` entries with these Podman +mount types: + +- `volume`: mounts an existing Podman named volume. The driver validates that + the volume exists before provisioning and never creates or removes it. +- `tmpfs`: mounts an in-memory filesystem with optional `options`, + `size_bytes`, and `mode`. +- `image`: mounts an OCI image through Podman's image-volume API. The driver + pulls the image during provisioning using the sandbox image pull policy. + +Host bind mounts are intentionally not part of the driver-config schema. The +driver still uses internal bind mounts for OpenShell-owned token and TLS +material. + +Podman image and volume mounts do not support `subpath` in OpenShell driver +config. Mount targets must be absolute container paths and must not replace the +workspace root (`/sandbox`) or overlap OpenShell supervisor files, auth +material, TLS material, or `/run/netns`. + +Example NFS usage relies on Podman's named-volume support rather than a host +bind: + +```shell +podman volume create \ + --opt type=nfs \ + --opt o=addr=10.0.0.10,rw,nfsvers=4 \ + --opt device=:/exports/work \ + work-nfs + +openshell sandbox create \ + --driver-config-json '{"podman":{"mounts":[{"type":"volume","source":"work-nfs","target":"/sandbox/work"}]}}' \ + -- claude +``` + ### Capability Breakdown | Capability | Purpose | diff --git a/crates/openshell-driver-podman/src/client.rs b/crates/openshell-driver-podman/src/client.rs index 834fb21a2..a9d36b842 100644 --- a/crates/openshell-driver-podman/src/client.rs +++ b/crates/openshell-driver-podman/src/client.rs @@ -497,6 +497,23 @@ impl PodmanClient { } } + /// Return whether a named volume exists. Does not create the volume. + pub async fn volume_exists(&self, name: &str) -> Result { + validate_name(name)?; + match self + .request_json::( + hyper::Method::GET, + &format!("/libpod/volumes/{name}/json"), + None, + ) + .await + { + Ok(_) => Ok(true), + Err(PodmanApiError::NotFound(_)) => Ok(false), + Err(e) => Err(e), + } + } + // ── Network operations ─────────────────────────────────────────────── /// Create a bridge network with DNS enabled. Idempotent. diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index 13f053e93..fa9d6c466 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -5,10 +5,11 @@ use crate::config::PodmanComputeConfig; use openshell_core::gpu::cdi_gpu_device_ids; -use openshell_core::proto::compute::v1::DriverSandbox; +use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate}; use serde::Serialize; use serde_json::Value; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; +use std::path::Path; /// Returns `true` when `SELinux` is enabled (enforcing or permissive). /// @@ -52,6 +53,46 @@ const TLS_CERT_MOUNT_PATH: &str = "/etc/openshell/tls/client/tls.crt"; const TLS_KEY_MOUNT_PATH: &str = "/etc/openshell/tls/client/tls.key"; const SANDBOX_TOKEN_MOUNT_PATH: &str = "/etc/openshell/auth/sandbox.jwt"; +#[derive(Debug, Clone, Default, serde::Deserialize)] +#[serde(default, deny_unknown_fields)] +struct PodmanSandboxDriverConfig { + mounts: Vec, +} + +#[derive(Debug, Clone, serde::Deserialize)] +#[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)] +enum PodmanDriverMountConfig { + Volume { + source: String, + target: String, + #[serde(default)] + read_only: bool, + #[serde(default)] + subpath: Option, + }, + Tmpfs { + target: String, + #[serde(default)] + options: Vec, + #[serde(default)] + size_bytes: Option, + #[serde(default)] + mode: Option, + }, + Image { + source: String, + target: String, + #[serde(default = "default_true")] + read_only: bool, + #[serde(default)] + subpath: Option, + }, +} + +fn default_true() -> bool { + true +} + /// Build a Podman container name from the sandbox name. #[must_use] pub fn container_name(sandbox_name: &str) -> String { @@ -166,6 +207,13 @@ struct NamedVolume { options: Vec, } +#[derive(Default)] +struct PodmanUserMounts { + volumes: Vec, + image_volumes: Vec, + mounts: Vec, +} + #[derive(Serialize)] struct HealthConfig { test: Vec, @@ -388,6 +436,325 @@ fn build_devices(sandbox: &DriverSandbox) -> Option> { }) } +pub fn podman_driver_volume_mount_sources(sandbox: &DriverSandbox) -> Result, String> { + let template = sandbox + .spec + .as_ref() + .and_then(|spec| spec.template.as_ref()); + let Some(template) = template else { + return Ok(Vec::new()); + }; + let config = podman_driver_config(template)?; + Ok(config + .mounts + .into_iter() + .filter_map(|mount| match mount { + PodmanDriverMountConfig::Volume { source, .. } => Some(source.trim().to_string()), + _ => None, + }) + .collect()) +} + +pub fn podman_driver_image_mount_sources(sandbox: &DriverSandbox) -> Result, String> { + let template = sandbox + .spec + .as_ref() + .and_then(|spec| spec.template.as_ref()); + let Some(template) = template else { + return Ok(Vec::new()); + }; + let config = podman_driver_config(template)?; + Ok(config + .mounts + .into_iter() + .filter_map(|mount| match mount { + PodmanDriverMountConfig::Image { source, .. } => Some(source.trim().to_string()), + _ => None, + }) + .collect()) +} + +fn podman_user_mounts(sandbox: &DriverSandbox) -> Result { + let template = sandbox + .spec + .as_ref() + .and_then(|spec| spec.template.as_ref()); + let Some(template) = template else { + return Ok(PodmanUserMounts::default()); + }; + let config = podman_driver_config(template)?; + let mut result = PodmanUserMounts::default(); + for mount in config.mounts { + match mount { + PodmanDriverMountConfig::Volume { + source, + target, + read_only, + subpath, + } => { + reject_subpath(subpath.as_deref(), "podman volume mounts")?; + result.volumes.push(NamedVolume { + name: validate_mount_source(&source, "volume source")?, + dest: validate_container_mount_target(&target)?, + options: vec![if read_only { "ro" } else { "rw" }.to_string()], + }); + } + PodmanDriverMountConfig::Tmpfs { + target, + options, + size_bytes, + mode, + } => { + let mut options = validate_tmpfs_options(&options)?; + if options.is_empty() { + options.push("rw".to_string()); + } + if let Some(size_bytes) = + validate_optional_positive_integral_i64(size_bytes, "tmpfs size_bytes")? + { + options.push(format!("size={size_bytes}")); + } + if let Some(mode) = validate_optional_nonnegative_integral_i64(mode, "tmpfs mode")? + { + options.push(format!("mode={mode:o}")); + } + result.mounts.push(Mount { + kind: "tmpfs".into(), + source: "tmpfs".into(), + destination: validate_container_mount_target(&target)?, + options, + }); + } + PodmanDriverMountConfig::Image { + source, + target, + read_only, + subpath, + } => { + reject_subpath(subpath.as_deref(), "podman image mounts")?; + result.image_volumes.push(ImageVolume { + source: validate_mount_source(&source, "image source")?, + destination: validate_container_mount_target(&target)?, + rw: !read_only, + }); + } + } + } + Ok(result) +} + +fn podman_driver_config( + template: &DriverSandboxTemplate, +) -> Result { + let Some(config) = template.driver_config.as_ref() else { + return Ok(PodmanSandboxDriverConfig::default()); + }; + let json = Value::Object(proto_struct_to_json_object(config)); + let config: PodmanSandboxDriverConfig = serde_json::from_value(json) + .map_err(|err| format!("invalid podman driver_config: {err}"))?; + validate_podman_driver_mounts(&config.mounts)?; + Ok(config) +} + +fn validate_podman_driver_mounts(mounts: &[PodmanDriverMountConfig]) -> Result<(), String> { + let mut targets = HashSet::new(); + for mount in mounts { + let target = match mount { + PodmanDriverMountConfig::Volume { + source, + target, + subpath, + .. + } => { + validate_mount_source(source, "volume source")?; + reject_subpath(subpath.as_deref(), "podman volume mounts")?; + target + } + PodmanDriverMountConfig::Tmpfs { + target, + options, + size_bytes, + mode, + } => { + validate_tmpfs_options(options)?; + validate_optional_positive_integral_i64(*size_bytes, "tmpfs size_bytes")?; + validate_optional_nonnegative_integral_i64(*mode, "tmpfs mode")?; + target + } + PodmanDriverMountConfig::Image { + source, + target, + subpath, + .. + } => { + validate_mount_source(source, "image source")?; + reject_subpath(subpath.as_deref(), "podman image mounts")?; + target + } + }; + let target = validate_container_mount_target(target)?; + if !targets.insert(target.clone()) { + return Err(format!( + "duplicate podman driver_config mount target '{target}'" + )); + } + } + Ok(()) +} + +fn validate_mount_source(source: &str, field: &str) -> Result { + let source = source.trim(); + if source.is_empty() { + return Err(format!("{field} must not be empty")); + } + if source.as_bytes().contains(&0) { + return Err(format!("{field} must not contain NUL bytes")); + } + Ok(source.to_string()) +} + +fn reject_subpath(subpath: Option<&str>, mount_type: &str) -> Result<(), String> { + let Some(subpath) = subpath else { + return Ok(()); + }; + if subpath.trim().is_empty() { + return Err("mount subpath must not be empty".to_string()); + } + Err(format!("{mount_type} do not support subpath")) +} + +fn validate_optional_positive_integral_i64( + value: Option, + field: &str, +) -> Result, String> { + let Some(value) = validate_optional_integral_i64(value, field)? else { + return Ok(None); + }; + if value <= 0 { + return Err(format!("{field} must be positive")); + } + Ok(Some(value)) +} + +fn validate_optional_nonnegative_integral_i64( + value: Option, + field: &str, +) -> Result, String> { + let Some(value) = validate_optional_integral_i64(value, field)? else { + return Ok(None); + }; + if value < 0 { + return Err(format!("{field} must be zero or greater")); + } + Ok(Some(value)) +} + +fn validate_optional_integral_i64(value: Option, field: &str) -> Result, String> { + let Some(value) = value else { + return Ok(None); + }; + if !value.is_finite() || value.fract() != 0.0 { + return Err(format!("{field} must be an integer")); + } + value + .to_string() + .parse::() + .map(Some) + .map_err(|_| format!("{field} must be representable as an i64")) +} + +fn validate_tmpfs_options(options: &[String]) -> Result, String> { + options + .iter() + .map(|option| { + let option = option.trim(); + if option.is_empty() { + return Err("tmpfs options must not contain empty values".to_string()); + } + Ok(option.to_string()) + }) + .collect() +} + +fn validate_container_mount_target(target: &str) -> Result { + let target = normalize_container_mount_target(target); + if target.is_empty() { + return Err("mount target must not be empty".to_string()); + } + if target.as_bytes().contains(&0) { + return Err("mount target must not contain NUL bytes".to_string()); + } + if !target.starts_with('/') { + return Err("mount target must be an absolute container path".to_string()); + } + if target == "/" { + return Err("mount target must not be the container root".to_string()); + } + let path = Path::new(&target); + if path + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)) + { + return Err("mount target must not contain '..'".to_string()); + } + if target == "/sandbox" { + return Err("mount target '/sandbox' is reserved for the OpenShell workspace".to_string()); + } + for reserved in [ + "/opt/openshell", + "/etc/openshell/auth", + "/etc/openshell/tls", + "/run/netns", + ] { + if path_is_or_under(&target, reserved) { + return Err(format!( + "mount target '{target}' conflicts with reserved OpenShell path '{reserved}'" + )); + } + } + Ok(target) +} + +fn normalize_container_mount_target(target: &str) -> String { + let target = target.trim(); + if target == "/" { + return target.to_string(); + } + target.trim_end_matches('/').to_string() +} + +fn path_is_or_under(path: &str, parent: &str) -> bool { + path == parent + || path + .strip_prefix(parent) + .is_some_and(|rest| rest.starts_with('/')) +} + +fn proto_struct_to_json_object(config: &prost_types::Struct) -> serde_json::Map { + config + .fields + .iter() + .map(|(key, value)| (key.clone(), proto_value_to_json(value))) + .collect() +} + +fn proto_value_to_json(value: &prost_types::Value) -> Value { + match value.kind.as_ref() { + Some(prost_types::value::Kind::NumberValue(num)) => { + serde_json::Number::from_f64(*num).map_or(Value::Null, Value::Number) + } + Some(prost_types::value::Kind::StringValue(val)) => Value::String(val.clone()), + Some(prost_types::value::Kind::BoolValue(val)) => Value::Bool(*val), + Some(prost_types::value::Kind::StructValue(val)) => { + Value::Object(proto_struct_to_json_object(val)) + } + Some(prost_types::value::Kind::ListValue(list)) => { + Value::Array(list.values.iter().map(proto_value_to_json).collect()) + } + Some(prost_types::value::Kind::NullValue(_)) | None => Value::Null, + } +} + /// Build the Podman container creation JSON spec. #[cfg(test)] #[must_use] @@ -396,11 +763,21 @@ pub fn build_container_spec(sandbox: &DriverSandbox, config: &PodmanComputeConfi } #[must_use] +#[cfg(test)] pub fn build_container_spec_with_token( sandbox: &DriverSandbox, config: &PodmanComputeConfig, - token_host_path: Option<&std::path::Path>, + token_host_path: Option<&Path>, ) -> Value { + try_build_container_spec_with_token(sandbox, config, token_host_path) + .expect("valid Podman container spec") +} + +pub fn try_build_container_spec_with_token( + sandbox: &DriverSandbox, + config: &PodmanComputeConfig, + token_host_path: Option<&Path>, +) -> Result { let image = resolve_image(sandbox, config); let name = container_name(&sandbox.name); let vol = volume_name(&sandbox.id); @@ -409,6 +786,7 @@ pub fn build_container_spec_with_token( let labels = build_labels(sandbox); let resource_limits = build_resource_limits(sandbox, config); let devices = build_devices(sandbox); + let user_mounts = podman_user_mounts(sandbox)?; // Network configuration -- always bridge mode. // Matches libpod's network spec format `{name: {opts}}`; the unit-struct @@ -417,27 +795,33 @@ pub fn build_container_spec_with_token( let mut networks = BTreeMap::new(); networks.insert(config.network_name.clone(), NetworkAttachment {}); + let mut volumes = vec![NamedVolume { + name: vol, + dest: "/sandbox".into(), + options: vec!["rw".into()], + }]; + volumes.extend(user_mounts.volumes); + + let mut image_volumes = vec![ImageVolume { + source: config.supervisor_image.clone(), + destination: "/opt/openshell/bin".into(), + rw: false, + }]; + image_volumes.extend(user_mounts.image_volumes); + let container_spec = ContainerSpec { name, image: image.to_string(), labels, env, - volumes: vec![NamedVolume { - name: vol, - dest: "/sandbox".into(), - options: vec!["rw".into()], - }], + volumes, // Side-load the supervisor binary from a standalone OCI image. // Podman resolves image_volumes at the libpod layer, mounting the // image's filesystem at the destination path without starting a // container from it. The supervisor image is FROM scratch with just // the binary at /openshell-sandbox, so it appears at // /opt/openshell/bin/openshell-sandbox. - image_volumes: vec![ImageVolume { - source: config.supervisor_image.clone(), - destination: "/opt/openshell/bin".into(), - rw: false, - }], + image_volumes, hostname: format!("sandbox-{}", sandbox.name), // Override the image's ENTRYPOINT so the supervisor binary runs // directly. Sandbox images (e.g. the community base image) set @@ -608,6 +992,7 @@ pub fn build_container_spec_with_token( options: ro, }); } + m.extend(user_mounts.mounts); m }, // Publish the SSH port with host_port=0 to get an ephemeral host port. @@ -620,7 +1005,7 @@ pub fn build_container_spec_with_token( }], }; - serde_json::to_value(container_spec).expect("ContainerSpec serialization cannot fail") + Ok(serde_json::to_value(container_spec).expect("ContainerSpec serialization cannot fail")) } fn hostadd_entries(config: &PodmanComputeConfig) -> Vec { @@ -699,6 +1084,43 @@ mod tests { static ENV_LOCK: std::sync::LazyLock> = std::sync::LazyLock::new(|| std::sync::Mutex::new(())); + fn json_struct(value: Value) -> prost_types::Struct { + match json_value(value).kind { + Some(prost_types::value::Kind::StructValue(value)) => value, + _ => panic!("expected JSON object"), + } + } + + fn json_value(value: Value) -> prost_types::Value { + match value { + Value::Null => prost_types::Value { kind: None }, + Value::Bool(value) => prost_types::Value { + kind: Some(prost_types::value::Kind::BoolValue(value)), + }, + Value::Number(value) => prost_types::Value { + kind: value.as_f64().map(prost_types::value::Kind::NumberValue), + }, + Value::String(value) => prost_types::Value { + kind: Some(prost_types::value::Kind::StringValue(value)), + }, + Value::Array(values) => prost_types::Value { + kind: Some(prost_types::value::Kind::ListValue( + prost_types::ListValue { + values: values.into_iter().map(json_value).collect(), + }, + )), + }, + Value::Object(values) => prost_types::Value { + kind: Some(prost_types::value::Kind::StructValue(prost_types::Struct { + fields: values + .into_iter() + .map(|(key, value)| (key, json_value(value))) + .collect(), + })), + }, + } + } + #[test] fn parse_cpu_millicore() { assert_eq!(parse_cpu_to_microseconds("500m"), Some(50_000)); @@ -1164,6 +1586,138 @@ mod tests { ); } + #[test] + fn container_spec_includes_driver_config_mounts() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate { + driver_config: Some(json_struct(serde_json::json!({ + "mounts": [ + { + "type": "volume", + "source": "work-nfs", + "target": "/sandbox/work", + "read_only": true + }, + { + "type": "tmpfs", + "target": "/sandbox/cache", + "options": ["nosuid", "nodev"], + "size_bytes": 1_048_576, + "mode": 511 + }, + { + "type": "image", + "source": "ghcr.io/acme/tools:latest", + "target": "/opt/tools", + "read_only": true + } + ] + }))), + ..Default::default() + }), + ..Default::default() + }); + let config = test_config(); + let spec = build_container_spec(&sandbox, &config); + + let volumes = spec["volumes"] + .as_array() + .expect("volumes should be an array"); + assert!(volumes.iter().any(|volume| { + volume["name"].as_str() == Some("openshell-sandbox-test-id-workspace") + && volume["dest"].as_str() == Some("/sandbox") + })); + assert!(volumes.iter().any(|volume| { + volume["name"].as_str() == Some("work-nfs") + && volume["dest"].as_str() == Some("/sandbox/work") + && volume["options"].as_array().is_some_and(|options| { + options.iter().any(|option| option.as_str() == Some("ro")) + }) + })); + + let mounts = spec["mounts"] + .as_array() + .expect("mounts should be an array"); + assert!(mounts.iter().any(|mount| { + mount["type"].as_str() == Some("tmpfs") + && mount["destination"].as_str() == Some("/sandbox/cache") + && mount["options"].as_array().is_some_and(|options| { + options + .iter() + .any(|option| option.as_str() == Some("size=1048576")) + && options + .iter() + .any(|option| option.as_str() == Some("mode=777")) + }) + })); + + let image_volumes = spec["image_volumes"] + .as_array() + .expect("image_volumes should be an array"); + assert!(image_volumes.iter().any(|volume| { + volume["source"].as_str() == Some("ghcr.io/nvidia/openshell/supervisor:latest") + && volume["destination"].as_str() == Some("/opt/openshell/bin") + })); + assert!(image_volumes.iter().any(|volume| { + volume["source"].as_str() == Some("ghcr.io/acme/tools:latest") + && volume["destination"].as_str() == Some("/opt/tools") + && volume["rw"].as_bool() == Some(false) + })); + } + + #[test] + fn driver_config_rejects_bind_mounts() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate { + driver_config: Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "bind", + "source": "/host/path", + "target": "/sandbox/host" + }] + }))), + ..Default::default() + }), + ..Default::default() + }); + let config = test_config(); + + let err = try_build_container_spec_with_token(&sandbox, &config, None).unwrap_err(); + + assert!(err.contains("invalid podman driver_config")); + } + + #[test] + fn driver_config_rejects_reserved_mount_targets() { + use openshell_core::proto::compute::v1::{DriverSandboxSpec, DriverSandboxTemplate}; + + let mut sandbox = test_sandbox("test-id", "test-name"); + sandbox.spec = Some(DriverSandboxSpec { + template: Some(DriverSandboxTemplate { + driver_config: Some(json_struct(serde_json::json!({ + "mounts": [{ + "type": "volume", + "source": "work-nfs", + "target": "/etc/openshell/tls/custom" + }] + }))), + ..Default::default() + }), + ..Default::default() + }); + let config = test_config(); + + let err = try_build_container_spec_with_token(&sandbox, &config, None).unwrap_err(); + + assert!(err.contains("reserved OpenShell path")); + } + #[test] fn container_spec_uses_configured_host_gateway_ip() { let sandbox = test_sandbox("test-id", "test-name"); @@ -1266,7 +1820,7 @@ mod tests { ..Default::default() }); let config = test_config(); - let token_path = std::path::Path::new("/host/token.jwt"); + let token_path = Path::new("/host/token.jwt"); let spec = build_container_spec_with_token(&sandbox, &config, Some(token_path)); diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 1358d8945..0d3dcccb0 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -277,12 +277,14 @@ impl PodmanComputeDriver { } /// Validate a sandbox before creation. - pub fn validate_sandbox_create( + pub async fn validate_sandbox_create( &self, sandbox: &DriverSandbox, ) -> Result<(), ComputeDriverError> { let gpu_requested = sandbox.spec.as_ref().is_some_and(|s| s.gpu); - Self::validate_gpu_request(gpu_requested) + Self::validate_gpu_request(gpu_requested)?; + self.validate_user_volume_mounts_available(sandbox).await?; + Ok(()) } fn validate_gpu_request(gpu_requested: bool) -> Result<(), ComputeDriverError> { @@ -294,6 +296,27 @@ impl PodmanComputeDriver { Ok(()) } + async fn validate_user_volume_mounts_available( + &self, + sandbox: &DriverSandbox, + ) -> Result<(), ComputeDriverError> { + let volumes = container::podman_driver_volume_mount_sources(sandbox) + .map_err(ComputeDriverError::Precondition)?; + for volume in volumes { + let exists = self + .client + .volume_exists(&volume) + .await + .map_err(ComputeDriverError::from)?; + if !exists { + return Err(ComputeDriverError::Precondition(format!( + "podman volume '{volume}' does not exist" + ))); + } + } + Ok(()) + } + /// Create a sandbox container. pub async fn create_sandbox(&self, sandbox: &DriverSandbox) -> Result<(), ComputeDriverError> { if sandbox.name.is_empty() { @@ -311,6 +334,7 @@ impl PodmanComputeDriver { // resources (volume), so we don't leave orphans when the name is // invalid. let name = validated_container_name(&sandbox.name)?; + self.validate_sandbox_create(sandbox).await?; let vol_name = container::volume_name(&sandbox.id); @@ -352,6 +376,16 @@ impl PodmanComputeDriver { .await .map_err(ComputeDriverError::from)?; + for image in container::podman_driver_image_mount_sources(sandbox) + .map_err(ComputeDriverError::Precondition)? + { + info!(image = %image, policy = %pull_policy, "Ensuring image mount source"); + self.client + .pull_image(&image, pull_policy) + .await + .map_err(ComputeDriverError::from)?; + } + // 2. Create workspace volume. if let Err(e) = self.client.create_volume(&vol_name).await { return Err(ComputeDriverError::from(e)); @@ -365,11 +399,12 @@ impl PodmanComputeDriver { }; // 3. Create container. - let spec = container::build_container_spec_with_token( + let spec = container::try_build_container_spec_with_token( sandbox, &self.config, token_host_path.as_deref(), - ); + ) + .map_err(ComputeDriverError::Precondition)?; match self.client.create_container(&spec).await { Ok(_) => {} Err(PodmanApiError::Conflict(_)) => { diff --git a/crates/openshell-driver-podman/src/grpc.rs b/crates/openshell-driver-podman/src/grpc.rs index 0c6015776..4840ee281 100644 --- a/crates/openshell-driver-podman/src/grpc.rs +++ b/crates/openshell-driver-podman/src/grpc.rs @@ -50,6 +50,7 @@ impl ComputeDriver for ComputeDriverService { .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; self.driver .validate_sandbox_create(&sandbox) + .await .map_err(Status::from)?; Ok(Response::new(ValidateSandboxCreateResponse {})) } diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 229bb1bdb..eacac85ef 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -72,6 +72,42 @@ Select Docker with `compute_drivers = ["docker"]` in `[openshell.gateway]`. Conf For GPU-backed Docker sandboxes, configure Docker CDI before starting the gateway so OpenShell can detect the daemon capability. +### Docker Driver Config Mounts + +Docker driver config accepts user-supplied `volume` and `tmpfs` mounts. Host +bind mounts and image mounts are not accepted in Docker sandbox driver config. +Docker's [storage documentation](https://docs.docker.com/engine/storage/) +documents container storage mounts as volumes, bind mounts, tmpfs mounts, and +named pipes. OpenShell intentionally exposes only Docker-managed volumes and +tmpfs mounts for sandbox create. + +Use a `volume` mount for existing Docker named volumes. This includes NFS +volumes created with Docker's local volume driver: + +```shell +docker volume create \ + --driver local \ + --opt type=nfs \ + --opt o=addr=10.0.0.10,rw,nfsvers=4 \ + --opt device=:/exports/work \ + work-nfs + +openshell sandbox create \ + --driver-config-json '{"docker":{"mounts":[{"type":"volume","source":"work-nfs","target":"/sandbox/work","read_only":false}]}}' \ + -- claude +``` + +Docker mount schema: + +| Type | Fields | +|---|---| +| `volume` | `source`, `target`, optional `read_only` (`false` by default), optional `subpath`. The named volume must already exist. | +| `tmpfs` | `target`, optional `options`, optional `size_bytes`, optional `mode`. | + +OpenShell rejects mount targets that replace the workspace root, container root, +supervisor files, TLS material, authentication material, or network namespace +paths. Mounted paths remain subject to sandbox filesystem policy. + ## Podman Driver [Podman](https://podman.io/)-backed sandboxes run as rootless containers on the gateway host. Use Podman for Linux workstation workflows that avoid a rootful Docker daemon. @@ -84,6 +120,49 @@ Select Podman with `compute_drivers = ["podman"]` in `[openshell.gateway]`. Conf On macOS with `podman machine`, the driver uses gvproxy's host-loopback IP, `192.168.127.254`, for sandbox host aliases by default. Set `host_gateway_ip` only when your Podman machine uses a non-standard host-loopback address. On Linux, an empty `host_gateway_ip` keeps Podman's `host-gateway` resolver behavior. +### Podman Driver Config Mounts + +Podman driver config accepts user-supplied `volume`, `tmpfs`, and `image` +mounts. Host bind mounts are not accepted in Podman sandbox driver config. + +Use a `volume` mount for existing Podman named volumes. This includes NFS +volumes created with Podman's local volume driver: + +```shell +podman volume create \ + --opt type=nfs \ + --opt o=addr=10.0.0.10,rw,nfsvers=4 \ + --opt device=:/exports/work \ + work-nfs + +openshell sandbox create \ + --driver-config-json '{"podman":{"mounts":[{"type":"volume","source":"work-nfs","target":"/sandbox/work","read_only":false}]}}' \ + -- claude +``` + +Use an `image` mount to mount an OCI image filesystem through Podman's +image-volume API: + +```shell +openshell sandbox create \ + --driver-config-json '{"podman":{"mounts":[{"type":"image","source":"ghcr.io/acme/tools:latest","target":"/opt/tools"}]}}' \ + -- claude +``` + +Podman mount schema: + +| Type | Fields | +|---|---| +| `volume` | `source`, `target`, optional `read_only` (`false` by default). The named volume must already exist. | +| `tmpfs` | `target`, optional `options`, optional `size_bytes`, optional `mode`. | +| `image` | `source`, `target`, optional `read_only` (`true` by default). | + +Podman `volume` and `image` mounts do not support `subpath` in OpenShell driver +config. OpenShell rejects mount targets that replace the workspace root, +container root, supervisor files, TLS material, authentication material, or +network namespace paths. Mounted paths remain subject to sandbox filesystem +policy. + ## MicroVM Driver MicroVM-backed sandboxes run inside VM-backed isolation instead of a container boundary. Use MicroVM when workloads need a VM boundary instead of a local container boundary. @@ -115,6 +194,13 @@ Configure VM driver values such as `grpc_endpoint`, `driver_dir`, `state_dir`, ` The gateway starts `openshell-driver-vm` over a private Unix socket and passes its process ID so the driver can reject unexpected local clients. The driver's standalone TCP listener is disabled unless `--allow-unauthenticated-tcp` is set for local development. +### VM Driver Config Mounts + +The VM driver does not currently support user-supplied mounts through +`--driver-config-json`. Each sandbox VM boots from a cached read-only image disk +plus a per-sandbox writable overlay disk, and the driver owns the guest mount +layout. + ### Local image resolution The VM driver resolves sandbox images from a local container engine before falling back to registry pulls. It tries Docker first, then falls back to the Podman socket (Docker-compatible API). On Linux with Podman, enable the API socket so the driver can find local images: @@ -156,6 +242,18 @@ For maintainer-level implementation details, refer to the [Kubernetes driver REA The Kubernetes driver creates namespaced `agents.x-k8s.io/v1alpha1` `Sandbox` resources from the Kubernetes SIG Apps [agent-sandbox](https://github.com/kubernetes-sigs/agent-sandbox) project. The Agent Sandbox controller turns those resources into sandbox pods and related storage. +### Kubernetes Driver Config Mounts + +The Kubernetes driver does not currently support Docker or Podman-style +`mounts` entries in `--driver-config-json`. Kubernetes driver config is limited +to pod scheduling and agent resource fields such as `pod.node_selector`, +`pod.tolerations`, `pod.runtime_class_name`, `pod.priority_class_name`, +`containers.agent.resources.requests`, and `containers.agent.resources.limits`. + +Kubernetes sandbox workspace storage uses Agent Sandbox +`volumeClaimTemplates`. The gateway injects the default workspace claim when the +template does not provide one. + `Sandbox.spec.volumeClaimTemplates` is immutable after creation. To change storage configuration, delete the sandbox and create a new one with the updated spec. diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index 0db6d7678..655900821 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -62,6 +62,29 @@ Use this only for driver-specific fields that do not have a stable CLI flag. Prefer stable flags such as `--cpu`, `--memory`, and `--gpu` when they cover the same behavior. +Docker accepts `volume` and `tmpfs` mounts in driver config. Podman accepts +`volume`, `tmpfs`, and `image` mounts. Host bind mounts are not supported +through `--driver-config-json`. Create NFS storage as a Docker or Podman named +volume first, then mount that existing volume into the sandbox: + +```shell +docker volume create \ + --driver local \ + --opt type=nfs \ + --opt o=addr=10.0.0.10,rw,nfsvers=4 \ + --opt device=:/exports/work \ + work-nfs + +openshell sandbox create \ + --driver-config-json '{"docker":{"mounts":[{"type":"volume","source":"work-nfs","target":"/sandbox/work"}]}}' \ + -- claude +``` + +Use the `podman` envelope key and `podman volume create` for Podman-backed +gateways. The volume must already exist before sandbox creation. Mounted paths +remain subject to the sandbox filesystem policy, so allow the mount target when +the agent needs access outside the default policy. + ### GPU Resources To request GPU resources, add `--gpu`: From 50e3715621110886322a73ca2bbfdc5dc4d94bae Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 5 Jun 2026 14:04:57 -0700 Subject: [PATCH 2/3] docs(drivers): trim mount docs Signed-off-by: Drew Newberry --- docs/reference/sandbox-compute-drivers.mdx | 19 ------------------ docs/sandboxes/manage-sandboxes.mdx | 23 ---------------------- 2 files changed, 42 deletions(-) diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index eacac85ef..a15833497 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -194,13 +194,6 @@ Configure VM driver values such as `grpc_endpoint`, `driver_dir`, `state_dir`, ` The gateway starts `openshell-driver-vm` over a private Unix socket and passes its process ID so the driver can reject unexpected local clients. The driver's standalone TCP listener is disabled unless `--allow-unauthenticated-tcp` is set for local development. -### VM Driver Config Mounts - -The VM driver does not currently support user-supplied mounts through -`--driver-config-json`. Each sandbox VM boots from a cached read-only image disk -plus a per-sandbox writable overlay disk, and the driver owns the guest mount -layout. - ### Local image resolution The VM driver resolves sandbox images from a local container engine before falling back to registry pulls. It tries Docker first, then falls back to the Podman socket (Docker-compatible API). On Linux with Podman, enable the API socket so the driver can find local images: @@ -242,18 +235,6 @@ For maintainer-level implementation details, refer to the [Kubernetes driver REA The Kubernetes driver creates namespaced `agents.x-k8s.io/v1alpha1` `Sandbox` resources from the Kubernetes SIG Apps [agent-sandbox](https://github.com/kubernetes-sigs/agent-sandbox) project. The Agent Sandbox controller turns those resources into sandbox pods and related storage. -### Kubernetes Driver Config Mounts - -The Kubernetes driver does not currently support Docker or Podman-style -`mounts` entries in `--driver-config-json`. Kubernetes driver config is limited -to pod scheduling and agent resource fields such as `pod.node_selector`, -`pod.tolerations`, `pod.runtime_class_name`, `pod.priority_class_name`, -`containers.agent.resources.requests`, and `containers.agent.resources.limits`. - -Kubernetes sandbox workspace storage uses Agent Sandbox -`volumeClaimTemplates`. The gateway injects the default workspace claim when the -template does not provide one. - `Sandbox.spec.volumeClaimTemplates` is immutable after creation. To change storage configuration, delete the sandbox and create a new one with the updated spec. diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index 655900821..0db6d7678 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -62,29 +62,6 @@ Use this only for driver-specific fields that do not have a stable CLI flag. Prefer stable flags such as `--cpu`, `--memory`, and `--gpu` when they cover the same behavior. -Docker accepts `volume` and `tmpfs` mounts in driver config. Podman accepts -`volume`, `tmpfs`, and `image` mounts. Host bind mounts are not supported -through `--driver-config-json`. Create NFS storage as a Docker or Podman named -volume first, then mount that existing volume into the sandbox: - -```shell -docker volume create \ - --driver local \ - --opt type=nfs \ - --opt o=addr=10.0.0.10,rw,nfsvers=4 \ - --opt device=:/exports/work \ - work-nfs - -openshell sandbox create \ - --driver-config-json '{"docker":{"mounts":[{"type":"volume","source":"work-nfs","target":"/sandbox/work"}]}}' \ - -- claude -``` - -Use the `podman` envelope key and `podman volume create` for Podman-backed -gateways. The volume must already exist before sandbox creation. Mounted paths -remain subject to the sandbox filesystem policy, so allow the mount target when -the agent needs access outside the default policy. - ### GPU Resources To request GPU resources, add `--gpu`: From 3e692822c604862e3b6f0a5ab5101d7d476c573b Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Fri, 5 Jun 2026 14:45:18 -0700 Subject: [PATCH 3/3] test(e2e): cover local driver volume mounts Signed-off-by: Drew Newberry --- e2e/rust/Cargo.toml | 10 +- e2e/rust/tests/driver_config_volume.rs | 159 +++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 e2e/rust/tests/driver_config_volume.rs diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index 31c6a3347..2ff799c98 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -23,11 +23,12 @@ e2e = [] # `server.hostGatewayIP` is set, so `e2e-kubernetes` does NOT imply this and # the helm wrapper opts in explicitly when it has resolved an IP. e2e-host-gateway = ["e2e"] -e2e-docker = ["e2e", "e2e-host-gateway"] +e2e-local-container-driver = ["e2e"] +e2e-docker = ["e2e", "e2e-host-gateway", "e2e-local-container-driver"] e2e-gpu = ["e2e"] e2e-docker-gpu = ["e2e-docker", "e2e-gpu"] e2e-kubernetes = ["e2e"] -e2e-podman = ["e2e", "e2e-host-gateway"] +e2e-podman = ["e2e", "e2e-host-gateway", "e2e-local-container-driver"] e2e-podman-gpu = ["e2e-podman", "e2e-gpu"] e2e-vm = ["e2e"] @@ -41,6 +42,11 @@ name = "docker_preflight" path = "tests/docker_preflight.rs" required-features = ["e2e-docker"] +[[test]] +name = "driver_config_volume" +path = "tests/driver_config_volume.rs" +required-features = ["e2e-local-container-driver"] + [[test]] name = "gateway_resume" path = "tests/gateway_resume.rs" diff --git a/e2e/rust/tests/driver_config_volume.rs b/e2e/rust/tests/driver_config_volume.rs new file mode 100644 index 000000000..7124cb0ab --- /dev/null +++ b/e2e/rust/tests/driver_config_volume.rs @@ -0,0 +1,159 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-local-container-driver")] + +use std::process::Output; +use std::time::{SystemTime, UNIX_EPOCH}; + +use openshell_e2e::harness::container::{ContainerEngine, e2e_driver}; +use openshell_e2e::harness::sandbox::SandboxGuard; + +const TEST_IMAGE: &str = "ghcr.io/nvidia/openshell-community/sandboxes/base:latest"; +const VOLUME_TARGET: &str = "/sandbox/e2e-volume"; + +struct VolumeGuard { + engine: ContainerEngine, + name: String, +} + +impl VolumeGuard { + fn create(engine: ContainerEngine, driver: &str) -> Result { + let name = unique_volume_name(driver); + run_engine(&engine, &["volume", "create", &name])?; + Ok(Self { engine, name }) + } +} + +impl Drop for VolumeGuard { + fn drop(&mut self) { + let _ = self + .engine + .command() + .args(["volume", "rm", "-f", &self.name]) + .output(); + } +} + +#[tokio::test] +async fn sandbox_mounts_existing_driver_config_volume() { + let driver = e2e_driver().expect("OPENSHELL_E2E_DRIVER must be set by the e2e wrapper"); + assert!( + matches!(driver.as_str(), "docker" | "podman"), + "driver_config volume e2e requires docker or podman, got {driver}" + ); + + let engine = ContainerEngine::from_env(); + let volume = VolumeGuard::create(engine, &driver).expect("create named test volume"); + + seed_volume(&volume).expect("seed named test volume"); + + let driver_config = format!( + r#"{{"{driver}":{{"mounts":[{{"type":"volume","source":"{}","target":"{VOLUME_TARGET}","read_only":false}}]}}}}"#, + volume.name + ); + let mut sandbox = SandboxGuard::create(&[ + "--no-keep", + "--driver-config-json", + &driver_config, + "--", + "sh", + "-lc", + "set -eu; test \"$(cat /sandbox/e2e-volume/input.txt)\" = host-volume-ok; printf sandbox-volume-ok > /sandbox/e2e-volume/output.txt; cat /sandbox/e2e-volume/output.txt", + ]) + .await + .expect("sandbox create with driver-config volume"); + + assert!( + sandbox.create_output.contains("sandbox-volume-ok"), + "sandbox should read and write the mounted volume:\n{}", + sandbox.create_output + ); + + sandbox.cleanup().await; + verify_volume(&volume).expect("verify sandbox wrote to named test volume"); +} + +fn seed_volume(volume: &VolumeGuard) -> Result<(), String> { + run_engine( + &volume.engine, + &[ + "run", + "--rm", + "--user", + "0:0", + "--volume", + &format!("{}:/vol", volume.name), + "--entrypoint", + "sh", + TEST_IMAGE, + "-lc", + "set -eu; chmod 0777 /vol; printf host-volume-ok > /vol/input.txt", + ], + )?; + Ok(()) +} + +fn verify_volume(volume: &VolumeGuard) -> Result<(), String> { + let output = run_engine( + &volume.engine, + &[ + "run", + "--rm", + "--user", + "0:0", + "--volume", + &format!("{}:/vol:ro", volume.name), + "--entrypoint", + "sh", + TEST_IMAGE, + "-lc", + "set -eu; test \"$(cat /vol/input.txt)\" = host-volume-ok; test \"$(cat /vol/output.txt)\" = sandbox-volume-ok; echo volume-ok", + ], + )?; + if !output.contains("volume-ok") { + return Err(format!( + "volume verification did not print expected marker:\n{output}" + )); + } + Ok(()) +} + +fn run_engine(engine: &ContainerEngine, args: &[&str]) -> Result { + let output = engine + .command() + .args(args) + .output() + .map_err(|err| format!("spawn {} {}: {err}", engine.name(), args.join(" ")))?; + engine_output(engine, args, &output) +} + +fn engine_output( + engine: &ContainerEngine, + args: &[&str], + output: &Output, +) -> Result { + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let combined = format!("{stdout}{stderr}"); + if output.status.success() { + return Ok(combined); + } + Err(format!( + "{} {} failed (exit {:?}):\n{combined}", + engine.name(), + args.join(" "), + output.status.code() + )) +} + +fn unique_volume_name(driver: &str) -> String { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock should be after Unix epoch") + .as_nanos(); + format!( + "openshell-e2e-driver-config-volume-{driver}-{}-{nanos}", + std::process::id() + ) +}