From e0aa17b2dc2fdaa5dd3c298cb25aeadbef9ae671 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Thu, 13 Aug 2026 19:37:02 -0700 Subject: [PATCH 1/6] refactor(compute): negotiate external driver behavior Signed-off-by: Drew Newberry --- .../skills/debug-openshell-cluster/SKILL.md | 6 +- architecture/compute-runtimes.md | 10 +- crates/openshell-core/src/driver_utils.rs | 25 ++- crates/openshell-driver-docker/src/lib.rs | 4 + .../openshell-driver-kubernetes/src/driver.rs | 1 + crates/openshell-driver-podman/src/driver.rs | 4 + crates/openshell-driver-vm/src/driver.rs | 13 +- crates/openshell-server/src/cli.rs | 41 ++--- crates/openshell-server/src/compute/mod.rs | 167 ++++++++++++------ .../openshell-server/src/gateway_listener.rs | 41 ++--- crates/openshell-server/src/grpc/policy.rs | 17 +- crates/openshell-server/src/grpc/sandbox.rs | 9 +- .../openshell-server/src/grpc/validation.rs | 69 +++----- crates/openshell-server/src/lib.rs | 38 ++-- crates/openshell-server/src/test_support.rs | 14 ++ docs/reference/gateway-config.mdx | 20 ++- docs/reference/sandbox-compute-drivers.mdx | 54 +++--- proto/compute_driver.proto | 20 ++- 18 files changed, 330 insertions(+), 223 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 7a73131ea0..d32a4bef50 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -71,7 +71,7 @@ Use gateway metadata, deployment values, or the user's setup notes to identify t Before debugging the compute platform, inspect gateway logs for failures in dependencies initialized before the listener becomes ready. -For out-of-tree compute drivers, confirm the custom driver name and socket agree across CLI flags or `gateway.toml`, and that the operator-owned driver is running before the gateway starts: +For out-of-tree compute drivers, confirm the selected driver name and socket agree across CLI flags or `gateway.toml`, and that the operator-owned driver is running before the gateway starts: ```bash rg -n 'compute_drivers|socket_path' /etc/openshell/gateway.toml @@ -80,7 +80,7 @@ journalctl -u --no-pager --lines=200 journalctl -u openshell-gateway --no-pager --lines=200 ``` -The custom driver name must not be a reserved built-in name (`docker`, `podman`, `kubernetes`, or `vm`). The socket must be accessible only to the intended gateway identity. Check gateway logs for connection errors, `GetCapabilities` failures, or an unexpected advertised driver name. The gateway does not create or supervise out-of-tree driver processes or sockets. +Custom names use `[openshell.drivers.].socket_path`. A launch-time `--compute-driver-socket` override may also use `docker`, `podman`, `kubernetes`, or `vm`; the endpoint then takes precedence over built-in construction. The socket must be accessible only to the intended gateway identity. Check gateway logs for connection errors, `GetCapabilities` failures, or an unexpected advertised driver name. The advertised name is diagnostic metadata; negotiated features control optional behavior. The gateway does not create or supervise operator-supplied driver processes or sockets. For configured gateway interceptors, inspect `[[openshell.gateway.interceptors]]`, their Unix or network endpoints, and gateway startup logs: @@ -593,7 +593,7 @@ openshell logs | Binary WebSocket message passes without a middleware RPC | Binary is unsupported by the V1 text-message binding under both `on_error` modes | `WEBSOCKET_MIDDLEWARE_COVERAGE state=unsupported_message_type`; the next text RPC may have a valid sequence gap | | WebSocket messages stop reaching middleware after one failure | A fail-open stage stream was disabled for the rest of the connection | `openshell.middleware.websocket_stage_disabled`; middleware timeout/stream/protocol logs. A per-message capacity bypass alone leaves the stage active. Reconnect to create a fresh stream after a genuine stream failure | | Supervisor repeatedly fails to install middleware after enabling gateway JWT signing | Extension credential minting, distribution, or authenticated service connection failed; last-known-good registry remains active | Gateway `RefreshSandboxToken` logs, sandbox configuration events, service token-verification logs, registration TLS/audience settings | -| Custom compute driver is unavailable | Driver process/socket missing, inaccessible, or configured with a reserved/mismatched name | Socket ownership/mode, driver service logs, gateway `GetCapabilities` logs | +| Custom compute driver is unavailable | Driver process/socket missing, inaccessible, or selected name does not match its endpoint/config key | Socket ownership/mode, driver service logs, gateway `GetCapabilities` logs | | Sandbox remains `Stopping` or `Starting` | Driver stop/start failed, retained resource is missing, or a fresh supervisor has not connected | Gateway and driver logs; `docker inspect`, `podman inspect`, Agent Sandbox status/PVC, or VM state marker and launcher process | | Image pull failure | Gateway or sandbox image cannot be pulled | Runtime events and image pull credentials | | `K8s namespace not ready` with `envoy-gateway-openshell.yaml: the server could not find the requested resource` | Optional Gateway API manifest was applied without Envoy Gateway CRDs, or k3s Helm controller startup exceeded the namespace wait | Apply `deploy/kube/manifests/envoy-gateway-openshell.yaml` manually only after Envoy Gateway is installed and `grpcRoute` is enabled | diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 0b901ef3db..0ed8722dbe 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -23,6 +23,14 @@ references to gateway-internal types. The gateway owns the public `SandboxPhase::Ready` decision. This applies equally to extension drivers implementing `ComputeDriver` out of tree. +`compute_driver.proto` is the supported gateway/driver extension boundary. +At initialization the gateway snapshots additive feature values from +`GetCapabilities`; unknown values are ignored. Post-initialization lifecycle +and policy behavior does not depend on the configured or advertised driver +name. Startup intent reconciliation and native process-identity defaults are +enabled only by their corresponding features. Driver-requested listeners are +structurally validated and remain restricted to sandbox callback RPCs. + Drivers own runtime-specific platform event interpretation. When an event should drive client provisioning UI, the driver attaches the shared `openshell.progress.*` metadata defined in `openshell-core` instead of requiring @@ -152,7 +160,7 @@ delete, reconciliation removes the row; otherwise it can remain `Deleting`. | Podman | Rootless or single-machine deployments. | Container plus nested sandbox namespace. | Uses the Podman REST API and CDI GPU devices when available. Delivers the supervisor via OCI image volume by default; falls back to extracting the binary to a host-side cache and bind-mounting it when `userns` is configured (overlay does not support idmapped mounts). | | Kubernetes | Cluster deployment through Helm. | Pod plus nested sandbox namespace. | Uses Kubernetes API objects, service accounts, secrets, PVC-backed workspace storage, and GPU resources. | | VM | Experimental microVM isolation. | Per-sandbox libkrun VM. | Managed endpoint-backed driver. The gateway spawns `openshell-driver-vm`, waits for its Unix socket, and then consumes it through the same remote `compute_driver.proto` path used by unmanaged endpoint drivers. The VM driver boots a cached bootstrap `rootfs.ext4`, prepares requested OCI images inside a bootstrap VM with `umoci`, attaches the prepared image disk read-only, and gives each sandbox a writable `overlay.ext4` for merged-root changes and runtime material. The driver persists each accepted launch request beside the overlay and restarts those VMs on driver startup without recreating the overlay. | -| Extension | Out-of-tree drivers operated alongside the gateway. | Whatever boundary the driver implements. | Selected by a non-reserved custom `compute_drivers = [""]` entry with `[openshell.drivers.].socket_path`, or at launch time by pairing `--drivers ` with `--compute-driver-socket=`. Reserved built-in names such as `vm`, `docker`, `podman`, and `kubernetes` cannot be used as unmanaged socket endpoints. The gateway connects to a UDS the operator already provisioned, runs `GetCapabilities`, logs the advertised `driver_name`, and dispatches all sandbox lifecycle calls through `compute_driver.proto`. The driver process and socket lifecycle are operator-owned; the gateway does not spawn, supervise, or remove unmanaged extension drivers. The trust boundary is the socket's filesystem permissions: the operator must ensure only the gateway uid can read/write it. | +| Extension | Out-of-tree drivers operated alongside the gateway. | Whatever boundary the driver implements. | Selected by a custom `compute_drivers = [""]` entry with `[openshell.drivers.].socket_path`, or at launch time by pairing `--drivers ` with `--compute-driver-socket=`. A launch-time endpoint may use a canonical built-in name to preserve its driver-config key while replacing in-process construction. The gateway connects to an operator-provisioned UDS, snapshots `GetCapabilities`, and dispatches all sandbox lifecycle calls through `compute_driver.proto`. The driver process and socket lifecycle are operator-owned; the gateway does not spawn, supervise, or remove unmanaged extension drivers. The trust boundary is the socket's filesystem permissions: the operator must ensure only the gateway uid can read/write it. | Per-sandbox CPU and memory values currently enter the driver layer through template resource limits. Docker and Podman apply them as runtime limits. diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index ae621fde08..9ed0f45d04 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; -use crate::proto::compute::v1::{DriverSandbox, GetCapabilitiesResponse}; +use crate::proto::compute::v1::{ComputeDriverFeature, DriverSandbox, GetCapabilitiesResponse}; pub use crate::container_paths::{ SANDBOX_TOKEN_MOUNT_PATH, SUPERVISOR_CONTAINER_BINARY, SUPERVISOR_CONTAINER_DIR, @@ -380,11 +380,13 @@ pub fn build_capabilities_response( driver_name: &str, driver_version: impl Into, default_image: impl Into, + features: impl IntoIterator, ) -> GetCapabilitiesResponse { GetCapabilitiesResponse { driver_name: driver_name.to_string(), driver_version: driver_version.into(), default_image: default_image.into(), + features: features.into_iter().map(i32::from).collect(), } } @@ -580,6 +582,27 @@ pub fn validate_linux_elf_binary(path: &Path) -> Result<(), String> { mod tests { use super::*; + #[test] + fn capabilities_encode_additive_features() { + let capabilities = build_capabilities_response( + "external", + "1.0.0", + "sandbox:latest", + [ + ComputeDriverFeature::GatewayStartReconciliation, + ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, + ], + ); + + assert_eq!( + capabilities.features, + vec![ + i32::from(ComputeDriverFeature::GatewayStartReconciliation), + i32::from(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity), + ] + ); + } + #[test] fn upstream_proxy_url_accepts_http_with_port() { let addr = parse_upstream_proxy_url("http://proxy.corp.com:8080").unwrap(); diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index cd1a0a8c51..f6cf5461c7 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -494,6 +494,10 @@ impl DockerComputeDriver { "docker", &self.config.daemon_version, &self.config.default_image, + [ + openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayStartReconciliation, + openshell_core::proto::compute::v1::ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, + ], ) } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index ddc7fe2a4f..e3addad55f 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -548,6 +548,7 @@ impl KubernetesComputeDriver { "kubernetes", openshell_core::VERSION, &self.config.default_image, + [], )) } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 9f8a62ef29..864851d477 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -468,6 +468,10 @@ impl PodmanComputeDriver { "podman", openshell_core::VERSION, &self.config.default_image, + [ + openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayStartReconciliation, + openshell_core::proto::compute::v1::ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, + ], )) } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 4dd2ea059b..e82182a61b 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -37,12 +37,12 @@ use openshell_core::progress::{ format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail, }; use openshell_core::proto::compute::v1::{ - CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, DeleteSandboxResponse, - DeleteWorkspaceRequest, DeleteWorkspaceResponse, DriverCondition as SandboxCondition, - DriverPlatformEvent as PlatformEvent, DriverSandbox as Sandbox, - DriverSandboxStatus as SandboxStatus, DriverSandboxTemplate as SandboxTemplate, - EnsureWorkspaceRequest, EnsureWorkspaceResponse, GetCapabilitiesRequest, - GetCapabilitiesResponse, GetGatewayListenerRequirementsRequest, + ComputeDriverFeature, CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, + DeleteSandboxResponse, DeleteWorkspaceRequest, DeleteWorkspaceResponse, + DriverCondition as SandboxCondition, DriverPlatformEvent as PlatformEvent, + DriverSandbox as Sandbox, DriverSandboxStatus as SandboxStatus, + DriverSandboxTemplate as SandboxTemplate, EnsureWorkspaceRequest, EnsureWorkspaceResponse, + GetCapabilitiesRequest, GetCapabilitiesResponse, GetGatewayListenerRequirementsRequest, GetGatewayListenerRequirementsResponse, GetSandboxRequest, GetSandboxResponse, ListSandboxesRequest, ListSandboxesResponse, StartSandboxRequest, StartSandboxResponse, StopSandboxRequest, StopSandboxResponse, ValidateSandboxCreateRequest, @@ -520,6 +520,7 @@ impl VmDriver { driver_name: DRIVER_NAME.to_string(), driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), + features: vec![ComputeDriverFeature::GatewayStartReconciliation.into()], } } diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 2e86c3a1b5..fc70284721 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -115,9 +115,9 @@ struct RunArgs { /// implementing `compute_driver.proto`. /// /// When set, the socket is associated with the single driver name supplied - /// by `--drivers` or `OPENSHELL_DRIVERS`. Reserved built-in driver names - /// such as Docker, Podman, Kubernetes, and VM do not accept socket - /// endpoints. + /// by `--drivers` or `OPENSHELL_DRIVERS`. The endpoint overrides built-in + /// construction when the selected name is Docker, Podman, Kubernetes, or + /// VM. #[arg(long, env = "OPENSHELL_COMPUTE_DRIVER_SOCKET")] compute_driver_socket: Option, @@ -755,7 +755,7 @@ fn normalize_compute_driver_socket_args(args: &mut RunArgs, matches: &ArgMatches } if arg_defaulted(matches, "drivers") { return Err(miette::miette!( - "--compute-driver-socket requires --drivers or OPENSHELL_DRIVERS= to select a non-reserved compute driver name" + "--compute-driver-socket requires --drivers or OPENSHELL_DRIVERS= to select a compute driver name" )); } @@ -763,19 +763,6 @@ fn normalize_compute_driver_socket_args(args: &mut RunArgs, matches: &ArgMatches [driver] => { let driver = openshell_core::config::normalize_compute_driver_name(driver) .map_err(|err| miette::miette!("{err}"))?; - if matches!( - driver.parse::().ok(), - Some( - ComputeDriverKind::Docker - | ComputeDriverKind::Podman - | ComputeDriverKind::Kubernetes - | ComputeDriverKind::Vm - ) - ) { - return Err(miette::miette!( - "--compute-driver-socket cannot be combined with reserved built-in compute driver '{driver}'" - )); - } args.drivers[0] = driver; Ok(()) } @@ -1663,7 +1650,7 @@ ssh_session_ttl_secs = 1234 } #[test] - fn compute_driver_socket_rejects_reserved_builtin_drivers() { + fn compute_driver_socket_accepts_canonical_builtin_driver_name() { let _lock = ENV_LOCK .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -1679,16 +1666,12 @@ ssh_session_ttl_secs = 1234 "--compute-driver-socket", "/run/openshell/extension.sock", ]); - let err = super::normalize_compute_driver_socket_args(&mut args, &matches).unwrap_err(); - assert!( - err.to_string() - .contains("cannot be combined with reserved built-in compute driver 'docker'"), - "unexpected error: {err}" - ); + super::normalize_compute_driver_socket_args(&mut args, &matches).unwrap(); + assert_eq!(args.drivers, ["docker"]); } #[test] - fn compute_driver_socket_rejects_vm_endpoint() { + fn compute_driver_socket_accepts_vm_endpoint() { let _lock = ENV_LOCK .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -1704,12 +1687,8 @@ ssh_session_ttl_secs = 1234 "--compute-driver-socket", "/run/openshell/vm.sock", ]); - let err = super::normalize_compute_driver_socket_args(&mut args, &matches).unwrap_err(); - assert!( - err.to_string() - .contains("cannot be combined with reserved built-in compute driver 'vm'"), - "unexpected error: {err}" - ); + super::normalize_compute_driver_socket_args(&mut args, &matches).unwrap(); + assert_eq!(args.drivers, ["vm"]); } #[test] diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index d157616d0b..165a577aa8 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -32,17 +32,18 @@ use futures::{Stream, StreamExt}; use hyper_util::rt::TokioIo; use openshell_core::ComputeDriverKind; use openshell_core::proto::compute::v1::{ - CreateSandboxRequest, DeleteSandboxRequest, DeleteWorkspaceRequest, DeleteWorkspaceResponse, - DriverCondition, DriverPlatformEvent, DriverResourceRequirements, DriverSandbox, - DriverSandboxSpec, DriverSandboxStatus, DriverSandboxTemplate, EnsureWorkspaceRequest, - EnsureWorkspaceResponse, GatewayListenerRequirement as ProtoGatewayListenerRequirement, - GetCapabilitiesRequest, GetGatewayListenerRequirementsRequest, - GetGatewayListenerRequirementsResponse, GetSandboxRequest, - GpuResourceRequirements as DriverGpuResourceRequirements, ListSandboxesRequest, - ResourceRequirements as DriverSandboxResourceRequirements, StartSandboxRequest, - StopSandboxRequest, ValidateSandboxCreateRequest, WatchSandboxesEvent, WatchSandboxesRequest, - compute_driver_client::ComputeDriverClient, compute_driver_server::ComputeDriver, - gateway_listener_requirement::Selector, watch_sandboxes_event, + ComputeDriverFeature, CreateSandboxRequest, DeleteSandboxRequest, DeleteWorkspaceRequest, + DeleteWorkspaceResponse, DriverCondition, DriverPlatformEvent, DriverResourceRequirements, + DriverSandbox, DriverSandboxSpec, DriverSandboxStatus, DriverSandboxTemplate, + EnsureWorkspaceRequest, EnsureWorkspaceResponse, + GatewayListenerRequirement as ProtoGatewayListenerRequirement, GetCapabilitiesRequest, + GetGatewayListenerRequirementsRequest, GetGatewayListenerRequirementsResponse, + GetSandboxRequest, GpuResourceRequirements as DriverGpuResourceRequirements, + ListSandboxesRequest, ResourceRequirements as DriverSandboxResourceRequirements, + StartSandboxRequest, StopSandboxRequest, ValidateSandboxCreateRequest, WatchSandboxesEvent, + WatchSandboxesRequest, compute_driver_client::ComputeDriverClient, + compute_driver_server::ComputeDriver, gateway_listener_requirement::Selector, + watch_sandboxes_event, }; use openshell_core::proto::{ PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxSpec, SandboxStatus, @@ -274,6 +275,8 @@ pub struct ComputeDriverInfoSnapshot { pub driver_name: String, /// Driver-reported implementation version from the startup capability snapshot. pub driver_version: String, + /// Feature values negotiated from the startup capability snapshot. + pub features: Vec, } /// Interval between store-vs-backend reconciliation sweeps. @@ -609,6 +612,7 @@ impl ComputeRuntime { name: driver_name.clone(), driver_name: capabilities.driver_name, driver_version: capabilities.driver_version, + features: capabilities.features, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -826,6 +830,16 @@ impl ComputeRuntime { self.driver_info.name.parse().ok() } + #[must_use] + pub(crate) fn supports_feature(&self, feature: ComputeDriverFeature) -> bool { + self.driver_info.features.contains(&i32::from(feature)) + } + + #[must_use] + pub(crate) fn preserves_unspecified_process_identity(&self) -> bool { + self.supports_feature(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity) + } + #[must_use] pub(crate) fn gateway_listener_requirements(&self) -> &[GatewayListenerRequirement] { &self.gateway_listener_requirements @@ -2128,20 +2142,16 @@ impl ComputeRuntime { /// Reconcile running intent for local compute after a gateway restart. /// - /// Docker and Podman resources can outlive the gateway but may have been - /// stopped by their runtime, while VM compute exits with its gateway-owned - /// driver process. `StartSandbox` is idempotent for all three drivers, so - /// call it for every persisted phase that requires running compute. Stable - /// stopped, deleting, and error states are deliberately left alone. + /// Drivers opt into this sweep through their startup capability snapshot. + /// `StartSandbox` is idempotent, so call it for every persisted phase that + /// requires running compute. Stable stopped, deleting, and error states are + /// deliberately left alone. /// /// Should be called once at gateway startup, before watchers spawn, /// so the watch loop sees the post-start state on its first poll. pub async fn start_persisted_sandboxes(&self) -> Result<(), String> { self.recover_persisted_lifecycle_transitions().await?; - if !matches!( - self.driver_kind(), - Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman | ComputeDriverKind::Vm) - ) { + if !self.supports_feature(ComputeDriverFeature::GatewayStartReconciliation) { return Ok(()); } @@ -3912,6 +3922,7 @@ impl ComputeDriver for NoopTestDriver { driver_name: "noop-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + features: Vec::new(), }, )) } @@ -4046,12 +4057,23 @@ pub async fn new_test_runtime_with_driver( driver_name: &str, driver: Arc, ) -> ComputeRuntime { + let features = match driver_name.parse::().ok() { + Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman) => vec![ + i32::from(ComputeDriverFeature::GatewayStartReconciliation), + i32::from(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity), + ], + Some(ComputeDriverKind::Vm) => { + vec![i32::from(ComputeDriverFeature::GatewayStartReconciliation)] + } + _ => Vec::new(), + }; ComputeRuntime { driver: TracedDriver::new(driver, "test".to_string()), driver_info: ComputeDriverInfoSnapshot { name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), + features, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4214,6 +4236,7 @@ mod tests { driver_name: "test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + features: Vec::new(), })) } @@ -4531,6 +4554,7 @@ mod tests { driver_name: "controlled-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + features: Vec::new(), })) } @@ -4725,6 +4749,14 @@ mod tests { async fn test_runtime_for_driver( driver: SharedComputeDriver, driver_name: &str, + ) -> ComputeRuntime { + test_runtime_for_driver_with_features(driver, driver_name, Vec::new()).await + } + + async fn test_runtime_for_driver_with_features( + driver: SharedComputeDriver, + driver_name: &str, + features: Vec, ) -> ComputeRuntime { let store = Arc::new(Store::connect("sqlite::memory:").await.unwrap()); ComputeRuntime { @@ -4733,6 +4765,7 @@ mod tests { name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), + features: features.into_iter().map(i32::from).collect(), }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -8238,7 +8271,12 @@ mod tests { #[tokio::test] async fn start_persisted_sandboxes_starts_running_phases() { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), "docker").await; + let runtime = test_runtime_for_driver_with_features( + driver.clone(), + "extension", + vec![ComputeDriverFeature::GatewayStartReconciliation], + ) + .await; for (id, name, phase) in [ ("sb-unspecified", "unspecified", SandboxPhase::Unspecified), @@ -8277,7 +8315,12 @@ mod tests { async fn start_persisted_sandboxes_marks_missing_backend_as_error() { let driver = ControlledDriver::new(); driver.set_start_outcome(ControlledLifecycleOutcome::NotFound); - let runtime = test_runtime_for_driver(driver, "podman").await; + let runtime = test_runtime_for_driver_with_features( + driver, + "extension", + vec![ComputeDriverFeature::GatewayStartReconciliation], + ) + .await; let sandbox = sandbox_record("sb-1", "missing", SandboxPhase::Ready); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8307,7 +8350,12 @@ mod tests { async fn start_persisted_sandboxes_marks_failed_start_as_error() { let driver = ControlledDriver::new(); driver.set_start_outcome(ControlledLifecycleOutcome::Error("runtime angry")); - let runtime = test_runtime_for_driver(driver, "vm").await; + let runtime = test_runtime_for_driver_with_features( + driver, + "extension", + vec![ComputeDriverFeature::GatewayStartReconciliation], + ) + .await; let sandbox = sandbox_record("sb-1", "broken", SandboxPhase::Provisioning); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8334,10 +8382,15 @@ mod tests { } #[tokio::test] - async fn start_persisted_sandboxes_runs_for_each_local_driver() { - for driver_name in ["docker", "podman", "vm"] { + async fn start_persisted_sandboxes_runs_for_any_capable_driver() { + for driver_name in ["docker", "podman", "vm", "extension"] { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; + let runtime = test_runtime_for_driver_with_features( + driver.clone(), + driver_name, + vec![ComputeDriverFeature::GatewayStartReconciliation], + ) + .await; let sandbox = sandbox_record("sb-1", "local", SandboxPhase::Ready); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8352,8 +8405,8 @@ mod tests { } #[tokio::test] - async fn start_persisted_sandboxes_skips_kubernetes_and_extension_drivers() { - for driver_name in ["kubernetes", "extension"] { + async fn start_persisted_sandboxes_skips_drivers_without_feature() { + for driver_name in ["kubernetes", "docker", "extension"] { let driver = ControlledDriver::new(); let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; let sandbox = sandbox_record("sb-1", "remote", SandboxPhase::Ready); @@ -8580,13 +8633,17 @@ mod tests { let driver = FakeComputeDriver::new() .with_driver_name("fake-remote-driver") .with_default_image("openshell/sandbox:remote") + .with_features([ + ComputeDriverFeature::GatewayStartReconciliation, + ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, + ]) .with_gateway_listener_requirement( "172.19.0.1:17670", "external driver managed bridge", ); let _server = driver.serve_uds(&socket_path).unwrap(); - let endpoint = connect_remote_compute_driver("external-test", &socket_path) + let endpoint = connect_remote_compute_driver("docker", &socket_path) .await .unwrap(); let store = Arc::new(Store::connect("sqlite::memory:").await.unwrap()); @@ -8600,11 +8657,12 @@ mod tests { ) .await .unwrap(); + assert!(runtime.preserves_unspecified_process_identity()); assert_eq!( runtime.gateway_listener_requirements(), &[GatewayListenerRequirement::Exact { address: "172.19.0.1:17670".parse().unwrap(), - driver_name: "external-test".to_string(), + driver_name: "docker".to_string(), reason: "external driver managed bridge".to_string(), }] ); @@ -8617,11 +8675,11 @@ mod tests { driver_config: Some(prost_types::Struct { fields: [ ( - "external-test".to_string(), + "docker".to_string(), struct_value([("pool", string_value("ci"))]), ), ( - "docker".to_string(), + "kubernetes".to_string(), struct_value([("network_mode", string_value("bridge"))]), ), ] @@ -8635,30 +8693,14 @@ mod tests { runtime.validate_sandbox_create(&sandbox).await.unwrap(); runtime.create_sandbox(sandbox, None).await.unwrap(); - assert!( - runtime - .delete_sandbox("default", "uds-sandbox") - .await - .unwrap() - .deleted - ); - let calls = driver.calls(); - assert_eq!(calls.len(), 5, "unexpected calls: {calls:?}"); - assert!(matches!(calls[0], FakeComputeDriverCall::GetCapabilities)); - assert!(matches!( - calls[1], - FakeComputeDriverCall::GetGatewayListenerRequirements - )); - + assert_eq!(calls.len(), 4, "unexpected calls: {calls:?}"); let validated = match &calls[2] { FakeComputeDriverCall::ValidateSandboxCreate { sandbox: Some(sandbox), } => sandbox, other => panic!("expected ValidateSandboxCreate call, got {other:?}"), }; - assert_eq!(validated.id, "sb-uds"); - assert_eq!(validated.name, "uds-sandbox"); let driver_config = validated .spec .as_ref() @@ -8668,16 +8710,25 @@ mod tests { assert!(driver_config.fields.contains_key("pool")); assert!(!driver_config.fields.contains_key("network_mode")); - let created = match &calls[3] { - FakeComputeDriverCall::CreateSandbox { - sandbox: Some(sandbox), - } => sandbox, - other => panic!("expected CreateSandbox call, got {other:?}"), - }; - assert_eq!(created.id, "sb-uds"); - assert_eq!(created.name, "uds-sandbox"); + driver.clear_calls(); + runtime.start_persisted_sandboxes().await.unwrap(); + assert!(matches!( + driver.calls().as_slice(), + [FakeComputeDriverCall::StartSandbox { sandbox_id, sandbox_name }] + if sandbox_id == "sb-uds" && sandbox_name == "uds-sandbox" + )); + driver.clear_calls(); + assert!( + runtime + .delete_sandbox("default", "uds-sandbox") + .await + .unwrap() + .deleted + ); - match &calls[4] { + let calls = driver.calls(); + assert_eq!(calls.len(), 1, "unexpected calls: {calls:?}"); + match &calls[0] { FakeComputeDriverCall::DeleteSandbox { sandbox_id, sandbox_name, diff --git a/crates/openshell-server/src/gateway_listener.rs b/crates/openshell-server/src/gateway_listener.rs index 4e4c7e80d0..640757bfb3 100644 --- a/crates/openshell-server/src/gateway_listener.rs +++ b/crates/openshell-server/src/gateway_listener.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::compute::GatewayListenerRequirement; -use openshell_core::{ComputeDriverKind, Error, Result}; +use openshell_core::{Error, Result}; use socket2::{Domain, Protocol, Socket, Type}; use std::net::{IpAddr, SocketAddr}; use tokio::net::TcpListener; @@ -193,25 +193,11 @@ fn validate_gateway_listener_requirement( requirement: &GatewayListenerRequirement, ) -> Result<()> { match requirement { - GatewayListenerRequirement::Exact { - address, - driver_name, - .. - } if driver_name == ComputeDriverKind::Docker.as_str() - || driver_name == ComputeDriverKind::Podman.as_str() => - { + GatewayListenerRequirement::Exact { address, .. } => { validate_resolved_gateway_listener(primary_listener, *address) } - GatewayListenerRequirement::DefaultRouteInterface { driver_name, .. } - | GatewayListenerRequirement::LoopbackInterface { driver_name, .. } - if driver_name == ComputeDriverKind::Podman.as_str() => - { - Ok(()) - } - _ => Err(Error::config(format!( - "compute driver '{}' is not authorized to request this gateway listener selector", - requirement.driver_name() - ))), + GatewayListenerRequirement::DefaultRouteInterface { .. } + | GatewayListenerRequirement::LoopbackInterface { .. } => Ok(()), } } @@ -464,7 +450,7 @@ mod tests { } #[test] - fn gateway_listener_specs_reject_unauthorized_external_driver() { + fn gateway_listener_specs_accept_safe_external_driver_requirement() { let primary: SocketAddr = "127.0.0.1:8080".parse().unwrap(); let requirement = GatewayListenerRequirement::Exact { address: "172.18.0.1:8080".parse().unwrap(), @@ -472,8 +458,10 @@ mod tests { reason: "external bridge".to_string(), }; - let err = gateway_listener_specs(primary, &[requirement]).unwrap_err(); - assert!(err.to_string().contains("not authorized")); + let specs = gateway_listener_specs(primary, &[requirement]).unwrap(); + assert_eq!(specs.len(), 2); + assert_eq!(specs[1].address, "172.18.0.1:8080".parse().unwrap()); + assert_eq!(specs[1].scope, GatewayListenerScope::ComputeDriverCallback); } #[test] @@ -622,15 +610,18 @@ mod tests { } #[test] - fn gateway_listener_specs_reject_cross_driver_selector_authority() { - let primary: SocketAddr = "127.0.0.1:8080".parse().unwrap(); + fn gateway_listener_specs_validate_selector_independently_of_driver_name() { + let primary: SocketAddr = "192.168.20.20:8080".parse().unwrap(); let requirement = GatewayListenerRequirement::LoopbackInterface { driver_name: "docker".to_string(), reason: "wrong selector".to_string(), }; - let err = gateway_listener_specs(primary, &[requirement]).unwrap_err(); - assert!(err.to_string().contains("not authorized")); + let specs = gateway_listener_specs(primary, &[requirement]).unwrap(); + assert_eq!(specs.len(), 2); + assert_eq!(specs[0].address, primary); + assert_eq!(specs[1].address, "127.0.0.1:8080".parse().unwrap()); + assert_eq!(specs[1].scope, GatewayListenerScope::ComputeDriverCallback); } #[tokio::test] diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index a63121ae05..5c63fae7a3 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -2429,7 +2429,10 @@ async fn handle_update_config_inner( let mut new_policy = req.policy.ok_or_else(|| { Status::invalid_argument("policy is required for global policy update") })?; - normalize_process_identity_for_driver(&mut new_policy, state.compute.driver_kind()); + normalize_process_identity_for_driver( + &mut new_policy, + state.compute.preserves_unspecified_process_identity(), + ); validate_no_reserved_provider_policy_keys(&new_policy)?; validate_policy_safety(&new_policy)?; crate::middleware::validate_policy(state.middleware_registry.as_ref(), &new_policy) @@ -2738,7 +2741,10 @@ async fn handle_update_config_inner( }; let mut baseline_policy = spec.policy.clone(); if let Some(policy) = baseline_policy.as_mut() { - normalize_process_identity_for_driver(policy, state.compute.driver_kind()); + normalize_process_identity_for_driver( + policy, + state.compute.preserves_unspecified_process_identity(), + ); } let (version, hash, updated_sandbox) = apply_merge_operations_with_retry( state.store.as_ref(), @@ -2814,7 +2820,10 @@ async fn handle_update_config_inner( let mut new_policy = req .policy .ok_or_else(|| Status::invalid_argument("policy is required"))?; - normalize_process_identity_for_driver(&mut new_policy, state.compute.driver_kind()); + normalize_process_identity_for_driver( + &mut new_policy, + state.compute.preserves_unspecified_process_identity(), + ); let global_settings = load_global_settings(state.store.as_ref()).await?; if global_settings.settings.contains_key(POLICY_SETTING_KEY) { @@ -2843,7 +2852,7 @@ async fn handle_update_config_inner( let mut comparable_baseline = baseline_policy.clone(); normalize_process_identity_for_driver( &mut comparable_baseline, - state.compute.driver_kind(), + state.compute.preserves_unspecified_process_identity(), ); validate_static_fields_unchanged(&comparable_baseline, &new_policy)?; None diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 9338956950..0d8c50c378 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -268,10 +268,13 @@ async fn handle_create_sandbox_inner( template.image = state.compute.default_image().to_string(); } - // Docker and Podman preserve omitted identity fields for OCI USER - // fallback. Other drivers retain the legacy persisted sandbox defaults. + // Drivers with native image/runtime identity handling preserve omitted + // fields. Other drivers retain the legacy persisted sandbox defaults. if let Some(ref mut policy) = spec.policy { - normalize_process_identity_for_driver(policy, state.compute.driver_kind()); + normalize_process_identity_for_driver( + policy, + state.compute.preserves_unspecified_process_identity(), + ); validate_no_reserved_provider_policy_keys(policy)?; validate_policy_safety(policy)?; crate::middleware::validate_policy(state.middleware_registry.as_ref(), policy).await?; diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index f71623fa3d..12e184e57f 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -8,7 +8,6 @@ #![allow(clippy::result_large_err)] // Validation returns Result<_, Status> -use openshell_core::ComputeDriverKind; use openshell_core::proto::{ CredentialHandle, ExecSandboxRequest, Provider, SandboxPolicy as ProtoSandboxPolicy, SandboxTemplate, @@ -28,19 +27,14 @@ use super::{ // Exec request validation // --------------------------------------------------------------------------- -/// Preserve process-identity omission only for the local OCI-aware drivers. -/// -/// Kubernetes, VM, and unknown/remote drivers retain the legacy persisted -/// `sandbox:sandbox` defaults so existing policy hashes and live-update -/// workflows do not change. +/// Preserve process-identity omission only when the compute driver advertises +/// native image/runtime identity handling. Drivers without the feature retain +/// the legacy persisted `sandbox:sandbox` defaults. pub(super) fn normalize_process_identity_for_driver( policy: &mut ProtoSandboxPolicy, - driver_kind: Option, + preserves_unspecified_process_identity: bool, ) { - if !matches!( - driver_kind, - Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman) - ) { + if !preserves_unspecified_process_identity { openshell_policy::ensure_sandbox_process_identity(policy); } } @@ -1854,41 +1848,30 @@ mod tests { // ---- Policy safety ---- #[test] - fn process_identity_omission_is_driver_scoped() { + fn process_identity_omission_is_feature_scoped() { use openshell_core::proto::ProcessPolicy; - for driver in [ComputeDriverKind::Docker, ComputeDriverKind::Podman] { - let mut policy = ProtoSandboxPolicy { - process: Some(ProcessPolicy { - run_as_user: "1234".into(), - run_as_group: String::new(), - }), - ..Default::default() - }; - normalize_process_identity_for_driver(&mut policy, Some(driver)); - assert!( - policy.process.unwrap().run_as_group.is_empty(), - "{driver:?} must preserve omission" - ); - } + let mut policy = ProtoSandboxPolicy { + process: Some(ProcessPolicy { + run_as_user: "1234".into(), + run_as_group: String::new(), + }), + ..Default::default() + }; + normalize_process_identity_for_driver(&mut policy, true); + assert!(policy.process.unwrap().run_as_group.is_empty()); - for driver in [ - Some(ComputeDriverKind::Kubernetes), - Some(ComputeDriverKind::Vm), - None, - ] { - let mut policy = ProtoSandboxPolicy { - process: Some(ProcessPolicy { - run_as_user: "1234".into(), - run_as_group: String::new(), - }), - ..Default::default() - }; - normalize_process_identity_for_driver(&mut policy, driver); - let process = policy.process.unwrap(); - assert_eq!(process.run_as_user, "1234"); - assert_eq!(process.run_as_group, "sandbox"); - } + let mut policy = ProtoSandboxPolicy { + process: Some(ProcessPolicy { + run_as_user: "1234".into(), + run_as_group: String::new(), + }), + ..Default::default() + }; + normalize_process_identity_for_driver(&mut policy, false); + let process = policy.process.unwrap(); + assert_eq!(process.run_as_user, "1234"); + assert_eq!(process.run_as_group, "sandbox"); } #[test] diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 979e372084..6ff455bea2 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1249,10 +1249,8 @@ fn resolve_configured_compute_driver( let name = openshell_core::config::normalize_compute_driver_name(driver_name) .map_err(Error::config)?; let driver_kind = builtin_compute_driver(&name); - if driver_kind.is_some() && driver_startup.endpoint_overrides.contains_key(&name) { - return Err(Error::config(format!( - "compute driver '{name}' is a reserved built-in driver and cannot be selected with a socket endpoint" - ))); + if driver_startup.endpoint_overrides.contains_key(&name) { + return Ok(ConfiguredComputeDriver::Remote { name }); } if let Some(kind) = driver_kind { @@ -1877,35 +1875,31 @@ mod tests { } #[test] - fn configured_compute_driver_rejects_vm_endpoint_from_config() { + fn configured_compute_driver_uses_vm_endpoint_override() { let config = Config::new(None) .with_compute_drivers([ComputeDriverKind::Vm]) .with_compute_driver_endpoint("vm", "/run/openshell/vm.sock"); - let err = - configured_compute_driver(&config, test_driver_startup(&config, None)).unwrap_err(); - - assert!( - err.to_string() - .contains("reserved built-in driver and cannot be selected with a socket endpoint"), - "unexpected error: {err}" - ); + let driver = + configured_compute_driver(&config, test_driver_startup(&config, None)).unwrap(); + assert!(matches!( + driver, + ConfiguredComputeDriver::Remote { name } if name == "vm" + )); } #[test] - fn configured_compute_driver_rejects_builtin_endpoint() { + fn configured_compute_driver_uses_builtin_endpoint_override() { let config = Config::new(None) .with_compute_drivers([ComputeDriverKind::Docker]) .with_compute_driver_endpoint("docker", "/run/openshell/docker.sock"); - let err = - configured_compute_driver(&config, test_driver_startup(&config, None)).unwrap_err(); - - assert!( - err.to_string() - .contains("cannot be selected with a socket endpoint"), - "unexpected error: {err}" - ); + let driver = + configured_compute_driver(&config, test_driver_startup(&config, None)).unwrap(); + assert!(matches!( + driver, + ConfiguredComputeDriver::Remote { name } if name == "docker" + )); } #[test] diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index f8124ded6c..742910ce47 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -74,6 +74,7 @@ struct FakeComputeDriverState { driver_name: String, driver_version: String, default_image: String, + features: Vec, gateway_listener_requirements: Vec, gateway_listener_requirements_supported: bool, sandboxes: HashMap, @@ -95,6 +96,7 @@ impl FakeComputeDriver { driver_name: "fake-compute-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + features: Vec::new(), gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, sandboxes: HashMap::new(), @@ -122,6 +124,17 @@ impl FakeComputeDriver { self } + #[must_use] + pub fn with_features( + self, + features: impl IntoIterator, + ) -> Self { + self.with_state(|state| { + state.features = features.into_iter().map(i32::from).collect(); + }); + self + } + #[must_use] pub fn with_gateway_listener_requirement( self, @@ -241,6 +254,7 @@ impl ComputeDriver for FakeComputeDriver { driver_name: state.driver_name.clone(), driver_version: state.driver_version.clone(), default_image: state.default_image.clone(), + features: state.features.clone(), } }); Ok(Response::new(response)) diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 8d74b3d44f..44cca9f687 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -743,10 +743,9 @@ guest_tls_key = "/var/lib/openshell/guest-tls/client-key.pem" ### Extension Driver Extension drivers run outside the gateway and expose the -`compute_driver.proto` gRPC service on a Unix socket. Use a non-reserved driver -name; built-in names such as `vm`, `docker`, `podman`, and `kubernetes` cannot -be selected through unmanaged socket endpoints. The selected driver name is the -key used for driver-owned sandbox config such as `template.driver_config.`. +supported `compute_driver.proto` gRPC service on a Unix socket. The selected +driver name is the key used for driver-owned sandbox config such as +`template.driver_config.`. ```toml [openshell] @@ -760,3 +759,16 @@ compute_drivers = ["kyma"] [openshell.drivers.kyma] socket_path = "/run/openshell/kyma-compute-driver.sock" ``` + +At launch time, `--compute-driver-socket` can override the implementation for +any explicitly selected name, including `docker`, `podman`, `kubernetes`, and +`vm`. The override takes precedence over built-in construction: + +```shell +openshell-gateway --drivers docker \ + --compute-driver-socket /run/openshell/docker-driver.sock +``` + +Do not use `[openshell.drivers.docker].socket_path` for this purpose; that +existing field selects the Docker daemon socket. Operator-supplied driver +processes and Unix sockets are not created or supervised by the gateway. diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 8405c1f3ba..3efae28960 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -20,10 +20,12 @@ remain unavailable. Restarting the gateway preserves this intent. The gateway does not stop Docker or Podman containers during shutdown. At startup it sends idempotent start -requests for Docker, Podman, and MicroVM sandboxes that were intended to run; -already-running resources are unchanged, retained stopped compute is restarted, -and explicitly stopped sandboxes remain stopped. Kubernetes workloads continue -running independently of the gateway process. +requests for drivers that advertise startup reconciliation. The built-in +Docker, Podman, and MicroVM implementations advertise this feature, so +sandboxes that were intended to run are reconciled. Already-running resources +are unchanged, retained stopped compute is restarted, and explicitly stopped +sandboxes remain stopped. Kubernetes workloads continue running independently +of the gateway process. ## Configure a Compute Driver @@ -34,8 +36,8 @@ Configure the compute driver on the gateway. Current releases accept one driver compute_drivers = ["docker"] ``` -Reserved built-in values are `docker`, `podman`, `kubernetes`, and `vm`. -Non-reserved names select an extension driver and require a +Built-in values are `docker`, `podman`, `kubernetes`, and `vm`. +Other names select an extension driver and require a `socket_path` in `[openshell.drivers.]`. When `compute_drivers` is unset, the gateway auto-detects Kubernetes, then Podman, then Docker. Local container runtimes must respond to an API probe before the gateway selects them. The VM driver is never auto-detected; configure it explicitly with `compute_drivers = ["vm"]` or set `OPENSHELL_DRIVERS=vm` in the launch environment. @@ -48,8 +50,8 @@ Common gateway options: Set driver-specific values such as sandbox images, callback endpoints, network names, TLS material, and VM sizing in the gateway TOML file. See the [Gateway Configuration File](./gateway-config) reference for the full `[openshell.drivers.]` schema. -Extension drivers use the same `compute_driver.proto` gRPC surface as the -managed VM driver. For an out-of-tree driver, choose a driver name and point +External drivers use the same supported `compute_driver.proto` gRPC surface as +the managed VM driver. For an out-of-tree driver, choose a driver name and point the gateway at the Unix socket the operator has already provisioned: ```toml @@ -60,16 +62,27 @@ compute_drivers = ["kyma"] socket_path = "/run/openshell/kyma.sock" ``` -For a launch-time socket override, pass the same non-reserved driver name with -the socket path: +For a launch-time socket override, pass the selected driver name with the +socket path: ```shell openshell-gateway --drivers kyma --compute-driver-socket /run/openshell/kyma.sock ``` -The gateway does not spawn, supervise, or delete extension drivers. The +The override also accepts a built-in name. This lets an operator provide, for +example, a Docker-compatible implementation through the external contract +without changing driver-owned config keys: + +```shell +openshell-gateway --drivers docker --compute-driver-socket /run/openshell/docker-driver.sock +``` + +The endpoint override takes precedence over built-in construction. The gateway +does not spawn, supervise, or delete an operator-supplied driver. The operator must protect the socket so only the gateway uid can access it. -Reserved built-in names cannot be selected through unmanaged socket endpoints. +The driver's advertised name is diagnostic metadata and does not authorize +special behavior. Drivers negotiate optional startup and process-identity +behavior through `GetCapabilities`; unknown features are ignored. Sandbox create supports `--cpu` and `--memory` for per-sandbox compute sizing. Docker and Podman apply them as runtime limits. Kubernetes applies them as both @@ -120,15 +133,16 @@ It overrides the gateway's configured default runtime class for that sandbox, while a typed `SandboxTemplate.runtime_class_name` value from the API still takes precedence. -Docker and Podman report the address through which their sandboxes can reach -the gateway. If the primary listener covers that address, the gateway reuses +Compute drivers can report the address through which their sandboxes reach the +gateway. If the primary listener covers a requested address, the gateway reuses it and sandbox JWT authentication restricts the supervisor to its callback RPC -allowlist. If the primary listener is not reachable through that address, the -gateway creates an additional callback-only listener. Use the primary endpoint -for CLI, administrator, health, reflection, inference-route management, and -HTTP requests. A `PermissionDenied` response from an additional callback-only -listener is expected for those requests. Do not broaden the primary listener -to `0.0.0.0` solely to make sandbox callbacks reachable. +allowlist. Otherwise, the gateway creates an additional callback-only listener. +Listener requirements are structurally validated independently of the selected +driver name. Use the primary endpoint for CLI, administrator, health, +reflection, inference-route management, and HTTP requests. A `PermissionDenied` +response from an additional callback-only listener is expected for those +requests. Do not broaden the primary listener to `0.0.0.0` solely to make +sandbox callbacks reachable. ## Docker Driver diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 0ce4f61539..86a66b7b58 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -8,14 +8,17 @@ package openshell.compute.v1; import "google/protobuf/struct.proto"; import "options.proto"; -// Internal compute-driver contract used by the gateway. +// Gateway/compute-driver extension contract. // // Conventions: // - This file owns driver-native request, response, and observation types. // - Compute drivers must not import or return the public `openshell.v1.Sandbox` // resource model. -// - The gateway translates between these internal driver-native messages and +// - The gateway translates between these driver-native messages and // the public OpenShell API resource model. +// - Capability fields are additive. Drivers and gateways must ignore unknown +// feature values so independently versioned external drivers remain +// forward-compatible. service ComputeDriver { // Report driver capabilities and defaults. rpc GetCapabilities(GetCapabilitiesRequest) returns (GetCapabilitiesResponse); @@ -72,6 +75,19 @@ message GetCapabilitiesResponse { string driver_version = 2; // Default sandbox image recommended by the driver. string default_image = 3; + // Optional behavior supported by this driver instance. Unknown values are + // ignored and omitted features retain conservative gateway behavior. + repeated ComputeDriverFeature features = 6; +} + +enum ComputeDriverFeature { + COMPUTE_DRIVER_FEATURE_UNSPECIFIED = 0; + // Reconcile persisted running intent by calling StartSandbox when the + // gateway starts. + COMPUTE_DRIVER_FEATURE_GATEWAY_START_RECONCILIATION = 1; + // Preserve omitted process user/group fields so the runtime can apply its + // native image or OCI identity defaults. + COMPUTE_DRIVER_FEATURE_PRESERVE_UNSPECIFIED_PROCESS_IDENTITY = 2; } message GetGatewayListenerRequirementsRequest {} From 97153a946efdf303b4517e85ccee18794c52c533 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Sun, 16 Aug 2026 23:57:45 -0700 Subject: [PATCH 2/6] docs(compute): revert external driver documentation Signed-off-by: Drew Newberry --- docs/reference/gateway-config.mdx | 20 ++------ docs/reference/sandbox-compute-drivers.mdx | 54 ++++++++-------------- 2 files changed, 24 insertions(+), 50 deletions(-) diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 44cca9f687..8d74b3d44f 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -743,9 +743,10 @@ guest_tls_key = "/var/lib/openshell/guest-tls/client-key.pem" ### Extension Driver Extension drivers run outside the gateway and expose the -supported `compute_driver.proto` gRPC service on a Unix socket. The selected -driver name is the key used for driver-owned sandbox config such as -`template.driver_config.`. +`compute_driver.proto` gRPC service on a Unix socket. Use a non-reserved driver +name; built-in names such as `vm`, `docker`, `podman`, and `kubernetes` cannot +be selected through unmanaged socket endpoints. The selected driver name is the +key used for driver-owned sandbox config such as `template.driver_config.`. ```toml [openshell] @@ -759,16 +760,3 @@ compute_drivers = ["kyma"] [openshell.drivers.kyma] socket_path = "/run/openshell/kyma-compute-driver.sock" ``` - -At launch time, `--compute-driver-socket` can override the implementation for -any explicitly selected name, including `docker`, `podman`, `kubernetes`, and -`vm`. The override takes precedence over built-in construction: - -```shell -openshell-gateway --drivers docker \ - --compute-driver-socket /run/openshell/docker-driver.sock -``` - -Do not use `[openshell.drivers.docker].socket_path` for this purpose; that -existing field selects the Docker daemon socket. Operator-supplied driver -processes and Unix sockets are not created or supervised by the gateway. diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 3efae28960..8405c1f3ba 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -20,12 +20,10 @@ remain unavailable. Restarting the gateway preserves this intent. The gateway does not stop Docker or Podman containers during shutdown. At startup it sends idempotent start -requests for drivers that advertise startup reconciliation. The built-in -Docker, Podman, and MicroVM implementations advertise this feature, so -sandboxes that were intended to run are reconciled. Already-running resources -are unchanged, retained stopped compute is restarted, and explicitly stopped -sandboxes remain stopped. Kubernetes workloads continue running independently -of the gateway process. +requests for Docker, Podman, and MicroVM sandboxes that were intended to run; +already-running resources are unchanged, retained stopped compute is restarted, +and explicitly stopped sandboxes remain stopped. Kubernetes workloads continue +running independently of the gateway process. ## Configure a Compute Driver @@ -36,8 +34,8 @@ Configure the compute driver on the gateway. Current releases accept one driver compute_drivers = ["docker"] ``` -Built-in values are `docker`, `podman`, `kubernetes`, and `vm`. -Other names select an extension driver and require a +Reserved built-in values are `docker`, `podman`, `kubernetes`, and `vm`. +Non-reserved names select an extension driver and require a `socket_path` in `[openshell.drivers.]`. When `compute_drivers` is unset, the gateway auto-detects Kubernetes, then Podman, then Docker. Local container runtimes must respond to an API probe before the gateway selects them. The VM driver is never auto-detected; configure it explicitly with `compute_drivers = ["vm"]` or set `OPENSHELL_DRIVERS=vm` in the launch environment. @@ -50,8 +48,8 @@ Common gateway options: Set driver-specific values such as sandbox images, callback endpoints, network names, TLS material, and VM sizing in the gateway TOML file. See the [Gateway Configuration File](./gateway-config) reference for the full `[openshell.drivers.]` schema. -External drivers use the same supported `compute_driver.proto` gRPC surface as -the managed VM driver. For an out-of-tree driver, choose a driver name and point +Extension drivers use the same `compute_driver.proto` gRPC surface as the +managed VM driver. For an out-of-tree driver, choose a driver name and point the gateway at the Unix socket the operator has already provisioned: ```toml @@ -62,27 +60,16 @@ compute_drivers = ["kyma"] socket_path = "/run/openshell/kyma.sock" ``` -For a launch-time socket override, pass the selected driver name with the -socket path: +For a launch-time socket override, pass the same non-reserved driver name with +the socket path: ```shell openshell-gateway --drivers kyma --compute-driver-socket /run/openshell/kyma.sock ``` -The override also accepts a built-in name. This lets an operator provide, for -example, a Docker-compatible implementation through the external contract -without changing driver-owned config keys: - -```shell -openshell-gateway --drivers docker --compute-driver-socket /run/openshell/docker-driver.sock -``` - -The endpoint override takes precedence over built-in construction. The gateway -does not spawn, supervise, or delete an operator-supplied driver. The +The gateway does not spawn, supervise, or delete extension drivers. The operator must protect the socket so only the gateway uid can access it. -The driver's advertised name is diagnostic metadata and does not authorize -special behavior. Drivers negotiate optional startup and process-identity -behavior through `GetCapabilities`; unknown features are ignored. +Reserved built-in names cannot be selected through unmanaged socket endpoints. Sandbox create supports `--cpu` and `--memory` for per-sandbox compute sizing. Docker and Podman apply them as runtime limits. Kubernetes applies them as both @@ -133,16 +120,15 @@ It overrides the gateway's configured default runtime class for that sandbox, while a typed `SandboxTemplate.runtime_class_name` value from the API still takes precedence. -Compute drivers can report the address through which their sandboxes reach the -gateway. If the primary listener covers a requested address, the gateway reuses +Docker and Podman report the address through which their sandboxes can reach +the gateway. If the primary listener covers that address, the gateway reuses it and sandbox JWT authentication restricts the supervisor to its callback RPC -allowlist. Otherwise, the gateway creates an additional callback-only listener. -Listener requirements are structurally validated independently of the selected -driver name. Use the primary endpoint for CLI, administrator, health, -reflection, inference-route management, and HTTP requests. A `PermissionDenied` -response from an additional callback-only listener is expected for those -requests. Do not broaden the primary listener to `0.0.0.0` solely to make -sandbox callbacks reachable. +allowlist. If the primary listener is not reachable through that address, the +gateway creates an additional callback-only listener. Use the primary endpoint +for CLI, administrator, health, reflection, inference-route management, and +HTTP requests. A `PermissionDenied` response from an additional callback-only +listener is expected for those requests. Do not broaden the primary listener +to `0.0.0.0` solely to make sandbox callbacks reachable. ## Docker Driver From d0f502e889149bc96d871aeb1f48e4dc27bbcd58 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Tue, 18 Aug 2026 00:03:24 -0700 Subject: [PATCH 3/6] refactor(compute): negotiate gateway-managed lifecycle Signed-off-by: Drew Newberry --- .../skills/debug-openshell-cluster/SKILL.md | 3 +- architecture/compute-runtimes.md | 7 +- crates/openshell-core/src/driver_utils.rs | 4 +- crates/openshell-driver-docker/src/lib.rs | 2 +- crates/openshell-driver-podman/src/driver.rs | 2 +- crates/openshell-driver-vm/src/driver.rs | 2 +- crates/openshell-server/src/compute/mod.rs | 101 +++++++++++++----- proto/compute_driver.proto | 6 +- 8 files changed, 87 insertions(+), 40 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index d32a4bef50..3f822a5908 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -199,7 +199,8 @@ running intent should stop before the gateway exits and restart after it returns. Check for `Stopped sandbox during gateway shutdown` and `Started sandbox during gateway startup` in gateway logs. A sandbox explicitly stopped through the CLI remains stopped. Kubernetes sandboxes are cluster-owned and do -not follow this local gateway lifecycle. +not follow this local gateway lifecycle. An external driver follows it only +when `GetCapabilities` advertises `GATEWAY_MANAGED_LIFECYCLE`. ### Step 5: Check Podman-Backed Gateways diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 0ed8722dbe..5bc9bd4260 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -27,9 +27,10 @@ implementing `ComputeDriver` out of tree. At initialization the gateway snapshots additive feature values from `GetCapabilities`; unknown values are ignored. Post-initialization lifecycle and policy behavior does not depend on the configured or advertised driver -name. Startup intent reconciliation and native process-identity defaults are -enabled only by their corresponding features. Driver-requested listeners are -structurally validated and remain restricted to sandbox callback RPCs. +name. Gateway-managed shutdown/start lifecycle and native process-identity +defaults are enabled only by their corresponding features. Driver-requested +listeners are structurally validated and remain restricted to sandbox callback +RPCs. Drivers own runtime-specific platform event interpretation. When an event should drive client provisioning UI, the driver attaches the shared diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index 9ed0f45d04..c1816f4e32 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -589,7 +589,7 @@ mod tests { "1.0.0", "sandbox:latest", [ - ComputeDriverFeature::GatewayStartReconciliation, + ComputeDriverFeature::GatewayManagedLifecycle, ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, ], ); @@ -597,7 +597,7 @@ mod tests { assert_eq!( capabilities.features, vec![ - i32::from(ComputeDriverFeature::GatewayStartReconciliation), + i32::from(ComputeDriverFeature::GatewayManagedLifecycle), i32::from(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity), ] ); diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index f6cf5461c7..d6bb7050b0 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -495,7 +495,7 @@ impl DockerComputeDriver { &self.config.daemon_version, &self.config.default_image, [ - openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayStartReconciliation, + openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayManagedLifecycle, openshell_core::proto::compute::v1::ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, ], ) diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 864851d477..dbca7e6dce 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -469,7 +469,7 @@ impl PodmanComputeDriver { openshell_core::VERSION, &self.config.default_image, [ - openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayStartReconciliation, + openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayManagedLifecycle, openshell_core::proto::compute::v1::ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, ], )) diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index e82182a61b..ef5cfdc79d 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -520,7 +520,7 @@ impl VmDriver { driver_name: DRIVER_NAME.to_string(), driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), - features: vec![ComputeDriverFeature::GatewayStartReconciliation.into()], + features: vec![ComputeDriverFeature::GatewayManagedLifecycle.into()], } } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 165a577aa8..7ad7c6e86e 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -835,6 +835,11 @@ impl ComputeRuntime { self.driver_info.features.contains(&i32::from(feature)) } + #[must_use] + pub(crate) fn uses_gateway_managed_lifecycle(&self) -> bool { + self.supports_feature(ComputeDriverFeature::GatewayManagedLifecycle) + } + #[must_use] pub(crate) fn preserves_unspecified_process_identity(&self) -> bool { self.supports_feature(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity) @@ -2053,14 +2058,10 @@ impl ComputeRuntime { /// persisted lifecycle intent. /// /// An explicit sandbox stop persists `Stopped`; gateway shutdown does not. - /// Docker, Podman, and VM compute is stopped through the same public driver - /// RPC and restarted from the retained running-intent phase on gateway - /// startup. Kubernetes compute remains cluster-owned and is excluded. + /// Drivers opt into stop/start ownership through their public capability + /// snapshot. Kubernetes and legacy extension drivers omit that capability. async fn stop_persisted_sandboxes_on_shutdown(&self) -> Result<(), String> { - if !matches!( - self.driver_kind(), - Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman | ComputeDriverKind::Vm) - ) { + if !self.uses_gateway_managed_lifecycle() { return Ok(()); } @@ -2142,7 +2143,7 @@ impl ComputeRuntime { /// Reconcile running intent for local compute after a gateway restart. /// - /// Drivers opt into this sweep through their startup capability snapshot. + /// Drivers opt into this sweep through their lifecycle capability snapshot. /// `StartSandbox` is idempotent, so call it for every persisted phase that /// requires running compute. Stable stopped, deleting, and error states are /// deliberately left alone. @@ -2151,7 +2152,7 @@ impl ComputeRuntime { /// so the watch loop sees the post-start state on its first poll. pub async fn start_persisted_sandboxes(&self) -> Result<(), String> { self.recover_persisted_lifecycle_transitions().await?; - if !self.supports_feature(ComputeDriverFeature::GatewayStartReconciliation) { + if !self.uses_gateway_managed_lifecycle() { return Ok(()); } @@ -4059,11 +4060,11 @@ pub async fn new_test_runtime_with_driver( ) -> ComputeRuntime { let features = match driver_name.parse::().ok() { Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman) => vec![ - i32::from(ComputeDriverFeature::GatewayStartReconciliation), + i32::from(ComputeDriverFeature::GatewayManagedLifecycle), i32::from(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity), ], Some(ComputeDriverKind::Vm) => { - vec![i32::from(ComputeDriverFeature::GatewayStartReconciliation)] + vec![i32::from(ComputeDriverFeature::GatewayManagedLifecycle)] } _ => Vec::new(), }; @@ -8161,7 +8162,12 @@ mod tests { #[tokio::test] async fn shutdown_stops_running_intent_without_changing_persisted_phase() { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), "docker").await; + let runtime = test_runtime_for_driver_with_features( + driver.clone(), + "extension", + vec![ComputeDriverFeature::GatewayManagedLifecycle], + ) + .await; for (id, name, phase) in [ ("sb-unspecified", "unspecified", SandboxPhase::Unspecified), @@ -8220,7 +8226,12 @@ mod tests { async fn shutdown_stop_sweep_continues_after_driver_errors() { let driver = ControlledDriver::new(); driver.set_stop_outcome(ControlledLifecycleOutcome::Error("runtime angry")); - let runtime = test_runtime_for_driver(driver.clone(), "podman").await; + let runtime = test_runtime_for_driver_with_features( + driver.clone(), + "extension", + vec![ComputeDriverFeature::GatewayManagedLifecycle], + ) + .await; for (id, name) in [("sb-1", "one"), ("sb-2", "two")] { runtime .store @@ -8239,16 +8250,15 @@ mod tests { } #[tokio::test] - async fn shutdown_stop_sweep_runs_for_each_local_driver_only() { - for (driver_name, expected_calls) in [ - ("docker", 1), - ("podman", 1), - ("vm", 1), - ("kubernetes", 0), - ("extension", 0), - ] { + async fn shutdown_stop_sweep_runs_for_any_capable_driver() { + for driver_name in ["docker", "podman", "vm", "extension"] { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; + let runtime = test_runtime_for_driver_with_features( + driver.clone(), + driver_name, + vec![ComputeDriverFeature::GatewayManagedLifecycle], + ) + .await; runtime .store .put_message(&sandbox_record("sb-1", "sandbox", SandboxPhase::Ready)) @@ -8262,19 +8272,43 @@ mod tests { assert_eq!( driver.stop_calls(), - expected_calls, + 1, "unexpected shutdown behavior for {driver_name}" ); } } + #[tokio::test] + async fn shutdown_stop_sweep_skips_drivers_without_feature() { + for driver_name in ["kubernetes", "docker", "extension"] { + let driver = ControlledDriver::new(); + let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; + runtime + .store + .put_message(&sandbox_record("sb-1", "sandbox", SandboxPhase::Ready)) + .await + .unwrap(); + + runtime + .stop_persisted_sandboxes_on_shutdown() + .await + .unwrap(); + + assert_eq!( + driver.stop_calls(), + 0, + "{driver_name} should retain operator-owned lifecycle" + ); + } + } + #[tokio::test] async fn start_persisted_sandboxes_starts_running_phases() { let driver = ControlledDriver::new(); let runtime = test_runtime_for_driver_with_features( driver.clone(), "extension", - vec![ComputeDriverFeature::GatewayStartReconciliation], + vec![ComputeDriverFeature::GatewayManagedLifecycle], ) .await; @@ -8318,7 +8352,7 @@ mod tests { let runtime = test_runtime_for_driver_with_features( driver, "extension", - vec![ComputeDriverFeature::GatewayStartReconciliation], + vec![ComputeDriverFeature::GatewayManagedLifecycle], ) .await; @@ -8353,7 +8387,7 @@ mod tests { let runtime = test_runtime_for_driver_with_features( driver, "extension", - vec![ComputeDriverFeature::GatewayStartReconciliation], + vec![ComputeDriverFeature::GatewayManagedLifecycle], ) .await; @@ -8388,7 +8422,7 @@ mod tests { let runtime = test_runtime_for_driver_with_features( driver.clone(), driver_name, - vec![ComputeDriverFeature::GatewayStartReconciliation], + vec![ComputeDriverFeature::GatewayManagedLifecycle], ) .await; let sandbox = sandbox_record("sb-1", "local", SandboxPhase::Ready); @@ -8634,7 +8668,7 @@ mod tests { .with_driver_name("fake-remote-driver") .with_default_image("openshell/sandbox:remote") .with_features([ - ComputeDriverFeature::GatewayStartReconciliation, + ComputeDriverFeature::GatewayManagedLifecycle, ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, ]) .with_gateway_listener_requirement( @@ -8710,6 +8744,17 @@ mod tests { assert!(driver_config.fields.contains_key("pool")); assert!(!driver_config.fields.contains_key("network_mode")); + driver.clear_calls(); + runtime + .stop_persisted_sandboxes_on_shutdown() + .await + .unwrap(); + assert!(matches!( + driver.calls().as_slice(), + [FakeComputeDriverCall::StopSandbox { sandbox_id, sandbox_name }] + if sandbox_id == "sb-uds" && sandbox_name == "uds-sandbox" + )); + driver.clear_calls(); runtime.start_persisted_sandboxes().await.unwrap(); assert!(matches!( diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 86a66b7b58..437ca685c3 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -82,9 +82,9 @@ message GetCapabilitiesResponse { enum ComputeDriverFeature { COMPUTE_DRIVER_FEATURE_UNSPECIFIED = 0; - // Reconcile persisted running intent by calling StartSandbox when the - // gateway starts. - COMPUTE_DRIVER_FEATURE_GATEWAY_START_RECONCILIATION = 1; + // Let the gateway stop persisted running intent during graceful shutdown + // and restart that intent when the gateway starts. + COMPUTE_DRIVER_FEATURE_GATEWAY_MANAGED_LIFECYCLE = 1; // Preserve omitted process user/group fields so the runtime can apply its // native image or OCI identity defaults. COMPUTE_DRIVER_FEATURE_PRESERVE_UNSPECIFIED_PROCESS_IDENTITY = 2; From 8fef8136574adbc1e755f6073e6d70c0e79b5ee8 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Tue, 18 Aug 2026 18:18:27 -0700 Subject: [PATCH 4/6] refactor(compute): remove driver feature negotiation Signed-off-by: Drew Newberry --- .../skills/debug-openshell-cluster/SKILL.md | 4 +- architecture/compute-runtimes.md | 12 +- crates/openshell-core/src/driver_utils.rs | 25 +-- crates/openshell-driver-docker/src/lib.rs | 4 - .../openshell-driver-kubernetes/src/driver.rs | 1 - crates/openshell-driver-podman/src/driver.rs | 4 - crates/openshell-driver-vm/src/driver.rs | 13 +- crates/openshell-server/src/compute/mod.rs | 154 +++++------------- crates/openshell-server/src/grpc/policy.rs | 35 +--- crates/openshell-server/src/grpc/sandbox.rs | 19 +-- .../openshell-server/src/grpc/validation.rs | 39 ----- crates/openshell-server/src/test_support.rs | 14 -- proto/compute_driver.proto | 13 -- 13 files changed, 67 insertions(+), 270 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 3f822a5908..389626ed5b 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -199,8 +199,8 @@ running intent should stop before the gateway exits and restart after it returns. Check for `Stopped sandbox during gateway shutdown` and `Started sandbox during gateway startup` in gateway logs. A sandbox explicitly stopped through the CLI remains stopped. Kubernetes sandboxes are cluster-owned and do -not follow this local gateway lifecycle. An external driver follows it only -when `GetCapabilities` advertises `GATEWAY_MANAGED_LIFECYCLE`. +not follow this local gateway lifecycle. An external driver configured under a +different name is not included in the local gateway lifecycle sweep. ### Step 5: Check Podman-Backed Gateways diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 5bc9bd4260..567dda118c 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -24,13 +24,11 @@ references to gateway-internal types. The gateway owns the public implementing `ComputeDriver` out of tree. `compute_driver.proto` is the supported gateway/driver extension boundary. -At initialization the gateway snapshots additive feature values from -`GetCapabilities`; unknown values are ignored. Post-initialization lifecycle -and policy behavior does not depend on the configured or advertised driver -name. Gateway-managed shutdown/start lifecycle and native process-identity -defaults are enabled only by their corresponding features. Driver-requested -listeners are structurally validated and remain restricted to sandbox callback -RPCs. +At initialization the gateway snapshots the driver's identity, version, and +default image from `GetCapabilities`. Process-identity omissions are preserved +across this boundary so every driver can apply its native image or runtime +defaults. Driver-requested listeners are structurally validated and remain +restricted to sandbox callback RPCs. Drivers own runtime-specific platform event interpretation. When an event should drive client provisioning UI, the driver attaches the shared diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index c1816f4e32..ae621fde08 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; -use crate::proto::compute::v1::{ComputeDriverFeature, DriverSandbox, GetCapabilitiesResponse}; +use crate::proto::compute::v1::{DriverSandbox, GetCapabilitiesResponse}; pub use crate::container_paths::{ SANDBOX_TOKEN_MOUNT_PATH, SUPERVISOR_CONTAINER_BINARY, SUPERVISOR_CONTAINER_DIR, @@ -380,13 +380,11 @@ pub fn build_capabilities_response( driver_name: &str, driver_version: impl Into, default_image: impl Into, - features: impl IntoIterator, ) -> GetCapabilitiesResponse { GetCapabilitiesResponse { driver_name: driver_name.to_string(), driver_version: driver_version.into(), default_image: default_image.into(), - features: features.into_iter().map(i32::from).collect(), } } @@ -582,27 +580,6 @@ pub fn validate_linux_elf_binary(path: &Path) -> Result<(), String> { mod tests { use super::*; - #[test] - fn capabilities_encode_additive_features() { - let capabilities = build_capabilities_response( - "external", - "1.0.0", - "sandbox:latest", - [ - ComputeDriverFeature::GatewayManagedLifecycle, - ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, - ], - ); - - assert_eq!( - capabilities.features, - vec![ - i32::from(ComputeDriverFeature::GatewayManagedLifecycle), - i32::from(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity), - ] - ); - } - #[test] fn upstream_proxy_url_accepts_http_with_port() { let addr = parse_upstream_proxy_url("http://proxy.corp.com:8080").unwrap(); diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index d6bb7050b0..cd1a0a8c51 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -494,10 +494,6 @@ impl DockerComputeDriver { "docker", &self.config.daemon_version, &self.config.default_image, - [ - openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayManagedLifecycle, - openshell_core::proto::compute::v1::ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, - ], ) } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index e3addad55f..ddc7fe2a4f 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -548,7 +548,6 @@ impl KubernetesComputeDriver { "kubernetes", openshell_core::VERSION, &self.config.default_image, - [], )) } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index dbca7e6dce..9f8a62ef29 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -468,10 +468,6 @@ impl PodmanComputeDriver { "podman", openshell_core::VERSION, &self.config.default_image, - [ - openshell_core::proto::compute::v1::ComputeDriverFeature::GatewayManagedLifecycle, - openshell_core::proto::compute::v1::ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, - ], )) } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index ef5cfdc79d..4dd2ea059b 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -37,12 +37,12 @@ use openshell_core::progress::{ format_bytes, mark_progress_active, mark_progress_complete, mark_progress_detail, }; use openshell_core::proto::compute::v1::{ - ComputeDriverFeature, CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, - DeleteSandboxResponse, DeleteWorkspaceRequest, DeleteWorkspaceResponse, - DriverCondition as SandboxCondition, DriverPlatformEvent as PlatformEvent, - DriverSandbox as Sandbox, DriverSandboxStatus as SandboxStatus, - DriverSandboxTemplate as SandboxTemplate, EnsureWorkspaceRequest, EnsureWorkspaceResponse, - GetCapabilitiesRequest, GetCapabilitiesResponse, GetGatewayListenerRequirementsRequest, + CreateSandboxRequest, CreateSandboxResponse, DeleteSandboxRequest, DeleteSandboxResponse, + DeleteWorkspaceRequest, DeleteWorkspaceResponse, DriverCondition as SandboxCondition, + DriverPlatformEvent as PlatformEvent, DriverSandbox as Sandbox, + DriverSandboxStatus as SandboxStatus, DriverSandboxTemplate as SandboxTemplate, + EnsureWorkspaceRequest, EnsureWorkspaceResponse, GetCapabilitiesRequest, + GetCapabilitiesResponse, GetGatewayListenerRequirementsRequest, GetGatewayListenerRequirementsResponse, GetSandboxRequest, GetSandboxResponse, ListSandboxesRequest, ListSandboxesResponse, StartSandboxRequest, StartSandboxResponse, StopSandboxRequest, StopSandboxResponse, ValidateSandboxCreateRequest, @@ -520,7 +520,6 @@ impl VmDriver { driver_name: DRIVER_NAME.to_string(), driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), - features: vec![ComputeDriverFeature::GatewayManagedLifecycle.into()], } } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 7ad7c6e86e..37d1ce9637 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -32,18 +32,17 @@ use futures::{Stream, StreamExt}; use hyper_util::rt::TokioIo; use openshell_core::ComputeDriverKind; use openshell_core::proto::compute::v1::{ - ComputeDriverFeature, CreateSandboxRequest, DeleteSandboxRequest, DeleteWorkspaceRequest, - DeleteWorkspaceResponse, DriverCondition, DriverPlatformEvent, DriverResourceRequirements, - DriverSandbox, DriverSandboxSpec, DriverSandboxStatus, DriverSandboxTemplate, - EnsureWorkspaceRequest, EnsureWorkspaceResponse, - GatewayListenerRequirement as ProtoGatewayListenerRequirement, GetCapabilitiesRequest, - GetGatewayListenerRequirementsRequest, GetGatewayListenerRequirementsResponse, - GetSandboxRequest, GpuResourceRequirements as DriverGpuResourceRequirements, - ListSandboxesRequest, ResourceRequirements as DriverSandboxResourceRequirements, - StartSandboxRequest, StopSandboxRequest, ValidateSandboxCreateRequest, WatchSandboxesEvent, - WatchSandboxesRequest, compute_driver_client::ComputeDriverClient, - compute_driver_server::ComputeDriver, gateway_listener_requirement::Selector, - watch_sandboxes_event, + CreateSandboxRequest, DeleteSandboxRequest, DeleteWorkspaceRequest, DeleteWorkspaceResponse, + DriverCondition, DriverPlatformEvent, DriverResourceRequirements, DriverSandbox, + DriverSandboxSpec, DriverSandboxStatus, DriverSandboxTemplate, EnsureWorkspaceRequest, + EnsureWorkspaceResponse, GatewayListenerRequirement as ProtoGatewayListenerRequirement, + GetCapabilitiesRequest, GetGatewayListenerRequirementsRequest, + GetGatewayListenerRequirementsResponse, GetSandboxRequest, + GpuResourceRequirements as DriverGpuResourceRequirements, ListSandboxesRequest, + ResourceRequirements as DriverSandboxResourceRequirements, StartSandboxRequest, + StopSandboxRequest, ValidateSandboxCreateRequest, WatchSandboxesEvent, WatchSandboxesRequest, + compute_driver_client::ComputeDriverClient, compute_driver_server::ComputeDriver, + gateway_listener_requirement::Selector, watch_sandboxes_event, }; use openshell_core::proto::{ PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxSpec, SandboxStatus, @@ -275,8 +274,6 @@ pub struct ComputeDriverInfoSnapshot { pub driver_name: String, /// Driver-reported implementation version from the startup capability snapshot. pub driver_version: String, - /// Feature values negotiated from the startup capability snapshot. - pub features: Vec, } /// Interval between store-vs-backend reconciliation sweeps. @@ -612,7 +609,6 @@ impl ComputeRuntime { name: driver_name.clone(), driver_name: capabilities.driver_name, driver_version: capabilities.driver_version, - features: capabilities.features, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -830,21 +826,6 @@ impl ComputeRuntime { self.driver_info.name.parse().ok() } - #[must_use] - pub(crate) fn supports_feature(&self, feature: ComputeDriverFeature) -> bool { - self.driver_info.features.contains(&i32::from(feature)) - } - - #[must_use] - pub(crate) fn uses_gateway_managed_lifecycle(&self) -> bool { - self.supports_feature(ComputeDriverFeature::GatewayManagedLifecycle) - } - - #[must_use] - pub(crate) fn preserves_unspecified_process_identity(&self) -> bool { - self.supports_feature(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity) - } - #[must_use] pub(crate) fn gateway_listener_requirements(&self) -> &[GatewayListenerRequirement] { &self.gateway_listener_requirements @@ -2058,10 +2039,14 @@ impl ComputeRuntime { /// persisted lifecycle intent. /// /// An explicit sandbox stop persists `Stopped`; gateway shutdown does not. - /// Drivers opt into stop/start ownership through their public capability - /// snapshot. Kubernetes and legacy extension drivers omit that capability. + /// Docker, Podman, and VM compute is stopped through the same public driver + /// RPC and restarted from retained running intent. Kubernetes remains + /// cluster-owned. async fn stop_persisted_sandboxes_on_shutdown(&self) -> Result<(), String> { - if !self.uses_gateway_managed_lifecycle() { + if !matches!( + self.driver_kind(), + Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman | ComputeDriverKind::Vm) + ) { return Ok(()); } @@ -2143,16 +2128,18 @@ impl ComputeRuntime { /// Reconcile running intent for local compute after a gateway restart. /// - /// Drivers opt into this sweep through their lifecycle capability snapshot. /// `StartSandbox` is idempotent, so call it for every persisted phase that - /// requires running compute. Stable stopped, deleting, and error states are - /// deliberately left alone. + /// requires running compute on Docker, Podman, and VM. Stable stopped, + /// deleting, and error states are deliberately left alone. /// /// Should be called once at gateway startup, before watchers spawn, /// so the watch loop sees the post-start state on its first poll. pub async fn start_persisted_sandboxes(&self) -> Result<(), String> { self.recover_persisted_lifecycle_transitions().await?; - if !self.uses_gateway_managed_lifecycle() { + if !matches!( + self.driver_kind(), + Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman | ComputeDriverKind::Vm) + ) { return Ok(()); } @@ -3923,7 +3910,6 @@ impl ComputeDriver for NoopTestDriver { driver_name: "noop-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - features: Vec::new(), }, )) } @@ -4058,23 +4044,12 @@ pub async fn new_test_runtime_with_driver( driver_name: &str, driver: Arc, ) -> ComputeRuntime { - let features = match driver_name.parse::().ok() { - Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman) => vec![ - i32::from(ComputeDriverFeature::GatewayManagedLifecycle), - i32::from(ComputeDriverFeature::PreserveUnspecifiedProcessIdentity), - ], - Some(ComputeDriverKind::Vm) => { - vec![i32::from(ComputeDriverFeature::GatewayManagedLifecycle)] - } - _ => Vec::new(), - }; ComputeRuntime { driver: TracedDriver::new(driver, "test".to_string()), driver_info: ComputeDriverInfoSnapshot { name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), - features, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4237,7 +4212,6 @@ mod tests { driver_name: "test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - features: Vec::new(), })) } @@ -4555,7 +4529,6 @@ mod tests { driver_name: "controlled-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - features: Vec::new(), })) } @@ -4750,14 +4723,6 @@ mod tests { async fn test_runtime_for_driver( driver: SharedComputeDriver, driver_name: &str, - ) -> ComputeRuntime { - test_runtime_for_driver_with_features(driver, driver_name, Vec::new()).await - } - - async fn test_runtime_for_driver_with_features( - driver: SharedComputeDriver, - driver_name: &str, - features: Vec, ) -> ComputeRuntime { let store = Arc::new(Store::connect("sqlite::memory:").await.unwrap()); ComputeRuntime { @@ -4766,7 +4731,6 @@ mod tests { name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), - features: features.into_iter().map(i32::from).collect(), }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -8162,12 +8126,7 @@ mod tests { #[tokio::test] async fn shutdown_stops_running_intent_without_changing_persisted_phase() { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver_with_features( - driver.clone(), - "extension", - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver.clone(), "docker").await; for (id, name, phase) in [ ("sb-unspecified", "unspecified", SandboxPhase::Unspecified), @@ -8226,12 +8185,7 @@ mod tests { async fn shutdown_stop_sweep_continues_after_driver_errors() { let driver = ControlledDriver::new(); driver.set_stop_outcome(ControlledLifecycleOutcome::Error("runtime angry")); - let runtime = test_runtime_for_driver_with_features( - driver.clone(), - "extension", - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver.clone(), "podman").await; for (id, name) in [("sb-1", "one"), ("sb-2", "two")] { runtime .store @@ -8250,15 +8204,10 @@ mod tests { } #[tokio::test] - async fn shutdown_stop_sweep_runs_for_any_capable_driver() { - for driver_name in ["docker", "podman", "vm", "extension"] { + async fn shutdown_stop_sweep_runs_for_each_local_driver() { + for driver_name in ["docker", "podman", "vm"] { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver_with_features( - driver.clone(), - driver_name, - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; runtime .store .put_message(&sandbox_record("sb-1", "sandbox", SandboxPhase::Ready)) @@ -8279,8 +8228,8 @@ mod tests { } #[tokio::test] - async fn shutdown_stop_sweep_skips_drivers_without_feature() { - for driver_name in ["kubernetes", "docker", "extension"] { + async fn shutdown_stop_sweep_skips_kubernetes_and_extension_drivers() { + for driver_name in ["kubernetes", "extension"] { let driver = ControlledDriver::new(); let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; runtime @@ -8305,12 +8254,7 @@ mod tests { #[tokio::test] async fn start_persisted_sandboxes_starts_running_phases() { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver_with_features( - driver.clone(), - "extension", - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver.clone(), "docker").await; for (id, name, phase) in [ ("sb-unspecified", "unspecified", SandboxPhase::Unspecified), @@ -8349,12 +8293,7 @@ mod tests { async fn start_persisted_sandboxes_marks_missing_backend_as_error() { let driver = ControlledDriver::new(); driver.set_start_outcome(ControlledLifecycleOutcome::NotFound); - let runtime = test_runtime_for_driver_with_features( - driver, - "extension", - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver, "podman").await; let sandbox = sandbox_record("sb-1", "missing", SandboxPhase::Ready); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8384,12 +8323,7 @@ mod tests { async fn start_persisted_sandboxes_marks_failed_start_as_error() { let driver = ControlledDriver::new(); driver.set_start_outcome(ControlledLifecycleOutcome::Error("runtime angry")); - let runtime = test_runtime_for_driver_with_features( - driver, - "extension", - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver, "vm").await; let sandbox = sandbox_record("sb-1", "broken", SandboxPhase::Provisioning); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8416,15 +8350,10 @@ mod tests { } #[tokio::test] - async fn start_persisted_sandboxes_runs_for_any_capable_driver() { - for driver_name in ["docker", "podman", "vm", "extension"] { + async fn start_persisted_sandboxes_runs_for_each_local_driver() { + for driver_name in ["docker", "podman", "vm"] { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver_with_features( - driver.clone(), - driver_name, - vec![ComputeDriverFeature::GatewayManagedLifecycle], - ) - .await; + let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; let sandbox = sandbox_record("sb-1", "local", SandboxPhase::Ready); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8439,8 +8368,8 @@ mod tests { } #[tokio::test] - async fn start_persisted_sandboxes_skips_drivers_without_feature() { - for driver_name in ["kubernetes", "docker", "extension"] { + async fn start_persisted_sandboxes_skips_kubernetes_and_extension_drivers() { + for driver_name in ["kubernetes", "extension"] { let driver = ControlledDriver::new(); let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; let sandbox = sandbox_record("sb-1", "remote", SandboxPhase::Ready); @@ -8667,10 +8596,6 @@ mod tests { let driver = FakeComputeDriver::new() .with_driver_name("fake-remote-driver") .with_default_image("openshell/sandbox:remote") - .with_features([ - ComputeDriverFeature::GatewayManagedLifecycle, - ComputeDriverFeature::PreserveUnspecifiedProcessIdentity, - ]) .with_gateway_listener_requirement( "172.19.0.1:17670", "external driver managed bridge", @@ -8691,7 +8616,6 @@ mod tests { ) .await .unwrap(); - assert!(runtime.preserves_unspecified_process_identity()); assert_eq!( runtime.gateway_listener_requirements(), &[GatewayListenerRequirement::Exact { diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 5c63fae7a3..de50e107af 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -81,9 +81,8 @@ use tonic::{Request, Response, Status}; use tracing::{debug, info, warn}; use super::validation::{ - level_matches, normalize_process_identity_for_driver, source_matches, validate_annotations, - validate_no_reserved_provider_policy_keys, validate_policy_safety, - validate_static_fields_unchanged, + level_matches, source_matches, validate_annotations, validate_no_reserved_provider_policy_keys, + validate_policy_safety, validate_static_fields_unchanged, }; use super::{MAX_PAGE_SIZE, StoredSettingValue, StoredSettings, clamp_limit}; use crate::persistence::current_time_ms; @@ -2426,13 +2425,9 @@ async fn handle_update_config_inner( "delete_setting cannot be combined with policy payload", )); } - let mut new_policy = req.policy.ok_or_else(|| { + let new_policy = req.policy.ok_or_else(|| { Status::invalid_argument("policy is required for global policy update") })?; - normalize_process_identity_for_driver( - &mut new_policy, - state.compute.preserves_unspecified_process_identity(), - ); validate_no_reserved_provider_policy_keys(&new_policy)?; validate_policy_safety(&new_policy)?; crate::middleware::validate_policy(state.middleware_registry.as_ref(), &new_policy) @@ -2739,13 +2734,7 @@ async fn handle_update_config_inner( provenance: &req.annotations, annotations: &req.annotations, }; - let mut baseline_policy = spec.policy.clone(); - if let Some(policy) = baseline_policy.as_mut() { - normalize_process_identity_for_driver( - policy, - state.compute.preserves_unspecified_process_identity(), - ); - } + let baseline_policy = spec.policy.clone(); let (version, hash, updated_sandbox) = apply_merge_operations_with_retry( state.store.as_ref(), &sandbox_id, @@ -2820,11 +2809,6 @@ async fn handle_update_config_inner( let mut new_policy = req .policy .ok_or_else(|| Status::invalid_argument("policy is required"))?; - normalize_process_identity_for_driver( - &mut new_policy, - state.compute.preserves_unspecified_process_identity(), - ); - let global_settings = load_global_settings(state.store.as_ref()).await?; if global_settings.settings.contains_key(POLICY_SETTING_KEY) { return Err(Status::failed_precondition( @@ -2849,11 +2833,7 @@ async fn handle_update_config_inner( } let backfill_policy = if let Some(baseline_policy) = spec.policy.as_ref() { - let mut comparable_baseline = baseline_policy.clone(); - normalize_process_identity_for_driver( - &mut comparable_baseline, - state.compute.preserves_unspecified_process_identity(), - ); + let comparable_baseline = baseline_policy.clone(); validate_static_fields_unchanged(&comparable_baseline, &new_policy)?; None } else { @@ -14656,6 +14636,7 @@ mod tests { assert_eq!(response.version, 1); // Verify the resource_version incremented and policy was backfilled + // without replacing an omitted process identity component. let updated_sandbox = state .store .get_message_by_name::("default", "test-sandbox") @@ -14667,9 +14648,9 @@ mod tests { .as_ref() .and_then(|spec| spec.policy.as_ref()) .and_then(|policy| policy.process.as_ref()) - .expect("legacy process identity should be persisted"); + .expect("partial process identity should be persisted"); assert_eq!(process.run_as_user, "1234"); - assert_eq!(process.run_as_group, "sandbox"); + assert!(process.run_as_group.is_empty()); assert_eq!( updated_sandbox.metadata.as_ref().unwrap().resource_version, current_version + 1, diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 0d8c50c378..6fa9959564 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -53,9 +53,8 @@ use super::provider::{ get_provider_record, is_valid_env_key, validate_provider_environment_keys_unique, }; use super::validation::{ - level_matches, normalize_process_identity_for_driver, source_matches, - validate_exec_request_fields, validate_no_reserved_provider_policy_keys, - validate_policy_safety, validate_sandbox_spec, + level_matches, source_matches, validate_exec_request_fields, + validate_no_reserved_provider_policy_keys, validate_policy_safety, validate_sandbox_spec, }; use super::{MAX_PAGE_SIZE, MAX_PROVIDERS, MAX_ROUTABLE_NAME_LEN, clamp_limit}; use crate::persistence::current_time_ms; @@ -268,13 +267,7 @@ async fn handle_create_sandbox_inner( template.image = state.compute.default_image().to_string(); } - // Drivers with native image/runtime identity handling preserve omitted - // fields. Other drivers retain the legacy persisted sandbox defaults. - if let Some(ref mut policy) = spec.policy { - normalize_process_identity_for_driver( - policy, - state.compute.preserves_unspecified_process_identity(), - ); + if let Some(ref policy) = spec.policy { validate_no_reserved_provider_policy_keys(policy)?; validate_policy_safety(policy)?; crate::middleware::validate_policy(state.middleware_registry.as_ref(), policy).await?; @@ -3431,7 +3424,7 @@ mod tests { } #[tokio::test] - async fn create_and_get_restore_legacy_identity_defaults_for_non_local_driver() { + async fn create_and_get_preserve_partial_process_identity_for_kubernetes() { let state = test_server_state_with_driver(openshell_core::ComputeDriverKind::Kubernetes.as_str()) .await; @@ -3458,7 +3451,7 @@ mod tests { }), ) .await - .expect("Kubernetes identity defaults should be accepted") + .expect("partial Kubernetes process identity should be accepted") .into_inner(); let process = response @@ -3470,7 +3463,7 @@ mod tests { .unwrap() .process .unwrap(); - assert_eq!(process.run_as_user, "sandbox"); + assert!(process.run_as_user.is_empty()); assert_eq!(process.run_as_group, "1234"); } diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 12e184e57f..f64a539d9d 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -27,18 +27,6 @@ use super::{ // Exec request validation // --------------------------------------------------------------------------- -/// Preserve process-identity omission only when the compute driver advertises -/// native image/runtime identity handling. Drivers without the feature retain -/// the legacy persisted `sandbox:sandbox` defaults. -pub(super) fn normalize_process_identity_for_driver( - policy: &mut ProtoSandboxPolicy, - preserves_unspecified_process_identity: bool, -) { - if !preserves_unspecified_process_identity { - openshell_policy::ensure_sandbox_process_identity(policy); - } -} - /// Maximum number of arguments in the command array. pub(super) const MAX_EXEC_COMMAND_ARGS: usize = 1024; /// Maximum length of a single command argument or environment value (bytes). @@ -1847,33 +1835,6 @@ mod tests { // ---- Policy safety ---- - #[test] - fn process_identity_omission_is_feature_scoped() { - use openshell_core::proto::ProcessPolicy; - - let mut policy = ProtoSandboxPolicy { - process: Some(ProcessPolicy { - run_as_user: "1234".into(), - run_as_group: String::new(), - }), - ..Default::default() - }; - normalize_process_identity_for_driver(&mut policy, true); - assert!(policy.process.unwrap().run_as_group.is_empty()); - - let mut policy = ProtoSandboxPolicy { - process: Some(ProcessPolicy { - run_as_user: "1234".into(), - run_as_group: String::new(), - }), - ..Default::default() - }; - normalize_process_identity_for_driver(&mut policy, false); - let process = policy.process.unwrap(); - assert_eq!(process.run_as_user, "1234"); - assert_eq!(process.run_as_group, "sandbox"); - } - #[test] fn validate_policy_safety_rejects_root_user() { use openshell_core::proto::{FilesystemPolicy, ProcessPolicy}; diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index 742910ce47..f8124ded6c 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -74,7 +74,6 @@ struct FakeComputeDriverState { driver_name: String, driver_version: String, default_image: String, - features: Vec, gateway_listener_requirements: Vec, gateway_listener_requirements_supported: bool, sandboxes: HashMap, @@ -96,7 +95,6 @@ impl FakeComputeDriver { driver_name: "fake-compute-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - features: Vec::new(), gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, sandboxes: HashMap::new(), @@ -124,17 +122,6 @@ impl FakeComputeDriver { self } - #[must_use] - pub fn with_features( - self, - features: impl IntoIterator, - ) -> Self { - self.with_state(|state| { - state.features = features.into_iter().map(i32::from).collect(); - }); - self - } - #[must_use] pub fn with_gateway_listener_requirement( self, @@ -254,7 +241,6 @@ impl ComputeDriver for FakeComputeDriver { driver_name: state.driver_name.clone(), driver_version: state.driver_version.clone(), default_image: state.default_image.clone(), - features: state.features.clone(), } }); Ok(Response::new(response)) diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 437ca685c3..767db5a445 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -75,19 +75,6 @@ message GetCapabilitiesResponse { string driver_version = 2; // Default sandbox image recommended by the driver. string default_image = 3; - // Optional behavior supported by this driver instance. Unknown values are - // ignored and omitted features retain conservative gateway behavior. - repeated ComputeDriverFeature features = 6; -} - -enum ComputeDriverFeature { - COMPUTE_DRIVER_FEATURE_UNSPECIFIED = 0; - // Let the gateway stop persisted running intent during graceful shutdown - // and restart that intent when the gateway starts. - COMPUTE_DRIVER_FEATURE_GATEWAY_MANAGED_LIFECYCLE = 1; - // Preserve omitted process user/group fields so the runtime can apply its - // native image or OCI identity defaults. - COMPUTE_DRIVER_FEATURE_PRESERVE_UNSPECIFIED_PROCESS_IDENTITY = 2; } message GetGatewayListenerRequirementsRequest {} From 46b9a2367aebc542bf7ed92c080ea46ae70724e8 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Tue, 18 Aug 2026 18:58:26 -0700 Subject: [PATCH 5/6] feat(compute): let drivers declare gateway lifecycle Signed-off-by: Drew Newberry --- .../skills/debug-openshell-cluster/SKILL.md | 5 +- architecture/compute-runtimes.md | 15 ++-- crates/openshell-core/src/driver_utils.rs | 2 + crates/openshell-driver-docker/src/lib.rs | 1 + .../openshell-driver-kubernetes/src/driver.rs | 1 + crates/openshell-driver-podman/src/driver.rs | 1 + crates/openshell-driver-vm/src/driver.rs | 1 + crates/openshell-server/src/compute/mod.rs | 72 +++++++++++-------- crates/openshell-server/src/test_support.rs | 9 +++ proto/compute_driver.proto | 3 + 10 files changed, 75 insertions(+), 35 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 389626ed5b..bc35e0d351 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -199,8 +199,9 @@ running intent should stop before the gateway exits and restart after it returns. Check for `Stopped sandbox during gateway shutdown` and `Started sandbox during gateway startup` in gateway logs. A sandbox explicitly stopped through the CLI remains stopped. Kubernetes sandboxes are cluster-owned and do -not follow this local gateway lifecycle. An external driver configured under a -different name is not included in the local gateway lifecycle sweep. +not follow this local gateway lifecycle. Internal and external drivers follow +the same rule: `GetCapabilities.gateway_managed_lifecycle` must be true for the +gateway to run shutdown and startup sweeps. ### Step 5: Check Podman-Backed Gateways diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 567dda118c..33a9b7c2c4 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -24,11 +24,11 @@ references to gateway-internal types. The gateway owns the public implementing `ComputeDriver` out of tree. `compute_driver.proto` is the supported gateway/driver extension boundary. -At initialization the gateway snapshots the driver's identity, version, and -default image from `GetCapabilities`. Process-identity omissions are preserved -across this boundary so every driver can apply its native image or runtime -defaults. Driver-requested listeners are structurally validated and remain -restricted to sandbox callback RPCs. +At initialization the gateway snapshots the driver's identity, version, +default image, and gateway-lifecycle preference from `GetCapabilities`. +Process-identity omissions are preserved across this boundary so every driver +can apply its native image or runtime defaults. Driver-requested listeners are +structurally validated and remain restricted to sandbox callback RPCs. Drivers own runtime-specific platform event interpretation. When an event should drive client provisioning UI, the driver attaches the shared @@ -122,6 +122,11 @@ shared idempotent `StartSandbox` RPC before watch processing begins. Explicitly cluster-owned and continue running without gateway shutdown or startup lifecycle calls. +The driver reports this behavior through +`GetCapabilities.gateway_managed_lifecycle`. The same declaration works for +in-process and external drivers. Older drivers omit the field and retain the +conservative operator-managed behavior. + ## Deletion Lifecycle Lifecycle requests use per-sandbox gates to serialize stop, start, and diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index ae621fde08..523d9ccc42 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -380,11 +380,13 @@ pub fn build_capabilities_response( driver_name: &str, driver_version: impl Into, default_image: impl Into, + gateway_managed_lifecycle: bool, ) -> GetCapabilitiesResponse { GetCapabilitiesResponse { driver_name: driver_name.to_string(), driver_version: driver_version.into(), default_image: default_image.into(), + gateway_managed_lifecycle, } } diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index cd1a0a8c51..1b9fe1f23b 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -494,6 +494,7 @@ impl DockerComputeDriver { "docker", &self.config.daemon_version, &self.config.default_image, + true, ) } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index ddc7fe2a4f..79c1b43c9b 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -548,6 +548,7 @@ impl KubernetesComputeDriver { "kubernetes", openshell_core::VERSION, &self.config.default_image, + false, )) } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 9f8a62ef29..976c67b92d 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -468,6 +468,7 @@ impl PodmanComputeDriver { "podman", openshell_core::VERSION, &self.config.default_image, + true, )) } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 4dd2ea059b..28dc348800 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -520,6 +520,7 @@ impl VmDriver { driver_name: DRIVER_NAME.to_string(), driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), + gateway_managed_lifecycle: true, } } diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 37d1ce9637..0c3a48205b 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -274,6 +274,8 @@ pub struct ComputeDriverInfoSnapshot { pub driver_name: String, /// Driver-reported implementation version from the startup capability snapshot. pub driver_version: String, + /// Whether the driver asks the gateway to reconcile compute across restarts. + pub gateway_managed_lifecycle: bool, } /// Interval between store-vs-backend reconciliation sweeps. @@ -609,6 +611,7 @@ impl ComputeRuntime { name: driver_name.clone(), driver_name: capabilities.driver_name, driver_version: capabilities.driver_version, + gateway_managed_lifecycle: capabilities.gateway_managed_lifecycle, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -2039,14 +2042,9 @@ impl ComputeRuntime { /// persisted lifecycle intent. /// /// An explicit sandbox stop persists `Stopped`; gateway shutdown does not. - /// Docker, Podman, and VM compute is stopped through the same public driver - /// RPC and restarted from retained running intent. Kubernetes remains - /// cluster-owned. + /// Drivers request this sweep through their startup capability snapshot. async fn stop_persisted_sandboxes_on_shutdown(&self) -> Result<(), String> { - if !matches!( - self.driver_kind(), - Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman | ComputeDriverKind::Vm) - ) { + if !self.driver_info.gateway_managed_lifecycle { return Ok(()); } @@ -2129,17 +2127,15 @@ impl ComputeRuntime { /// Reconcile running intent for local compute after a gateway restart. /// /// `StartSandbox` is idempotent, so call it for every persisted phase that - /// requires running compute on Docker, Podman, and VM. Stable stopped, - /// deleting, and error states are deliberately left alone. + /// requires running compute for drivers that request gateway-managed + /// lifecycle. Stable stopped, deleting, and error states are deliberately + /// left alone. /// /// Should be called once at gateway startup, before watchers spawn, /// so the watch loop sees the post-start state on its first poll. pub async fn start_persisted_sandboxes(&self) -> Result<(), String> { self.recover_persisted_lifecycle_transitions().await?; - if !matches!( - self.driver_kind(), - Some(ComputeDriverKind::Docker | ComputeDriverKind::Podman | ComputeDriverKind::Vm) - ) { + if !self.driver_info.gateway_managed_lifecycle { return Ok(()); } @@ -3910,6 +3906,7 @@ impl ComputeDriver for NoopTestDriver { driver_name: "noop-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + gateway_managed_lifecycle: false, }, )) } @@ -4050,6 +4047,7 @@ pub async fn new_test_runtime_with_driver( name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), + gateway_managed_lifecycle: false, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4212,6 +4210,7 @@ mod tests { driver_name: "test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + gateway_managed_lifecycle: false, })) } @@ -4529,6 +4528,7 @@ mod tests { driver_name: "controlled-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + gateway_managed_lifecycle: false, })) } @@ -4731,6 +4731,7 @@ mod tests { name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), + gateway_managed_lifecycle: false, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4746,6 +4747,15 @@ mod tests { } } + async fn test_runtime_with_gateway_managed_lifecycle( + driver: SharedComputeDriver, + driver_name: &str, + ) -> ComputeRuntime { + let mut runtime = test_runtime_for_driver(driver, driver_name).await; + runtime.driver_info.gateway_managed_lifecycle = true; + runtime + } + fn register_test_supervisor_session(runtime: &ComputeRuntime, sandbox_id: &str) { let (tx, _rx) = mpsc::channel(1); let (shutdown_tx, _shutdown_rx) = oneshot::channel(); @@ -8126,7 +8136,8 @@ mod tests { #[tokio::test] async fn shutdown_stops_running_intent_without_changing_persisted_phase() { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), "docker").await; + let runtime = + test_runtime_with_gateway_managed_lifecycle(driver.clone(), "arbitrary").await; for (id, name, phase) in [ ("sb-unspecified", "unspecified", SandboxPhase::Unspecified), @@ -8185,7 +8196,8 @@ mod tests { async fn shutdown_stop_sweep_continues_after_driver_errors() { let driver = ControlledDriver::new(); driver.set_stop_outcome(ControlledLifecycleOutcome::Error("runtime angry")); - let runtime = test_runtime_for_driver(driver.clone(), "podman").await; + let runtime = + test_runtime_with_gateway_managed_lifecycle(driver.clone(), "arbitrary").await; for (id, name) in [("sb-1", "one"), ("sb-2", "two")] { runtime .store @@ -8204,10 +8216,11 @@ mod tests { } #[tokio::test] - async fn shutdown_stop_sweep_runs_for_each_local_driver() { - for driver_name in ["docker", "podman", "vm"] { + async fn shutdown_stop_sweep_runs_for_any_capable_driver() { + for driver_name in ["arbitrary", "docker"] { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; + let runtime = + test_runtime_with_gateway_managed_lifecycle(driver.clone(), driver_name).await; runtime .store .put_message(&sandbox_record("sb-1", "sandbox", SandboxPhase::Ready)) @@ -8228,8 +8241,8 @@ mod tests { } #[tokio::test] - async fn shutdown_stop_sweep_skips_kubernetes_and_extension_drivers() { - for driver_name in ["kubernetes", "extension"] { + async fn shutdown_stop_sweep_skips_drivers_without_capability() { + for driver_name in ["docker", "kubernetes", "extension"] { let driver = ControlledDriver::new(); let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; runtime @@ -8254,7 +8267,8 @@ mod tests { #[tokio::test] async fn start_persisted_sandboxes_starts_running_phases() { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), "docker").await; + let runtime = + test_runtime_with_gateway_managed_lifecycle(driver.clone(), "arbitrary").await; for (id, name, phase) in [ ("sb-unspecified", "unspecified", SandboxPhase::Unspecified), @@ -8293,7 +8307,7 @@ mod tests { async fn start_persisted_sandboxes_marks_missing_backend_as_error() { let driver = ControlledDriver::new(); driver.set_start_outcome(ControlledLifecycleOutcome::NotFound); - let runtime = test_runtime_for_driver(driver, "podman").await; + let runtime = test_runtime_with_gateway_managed_lifecycle(driver, "arbitrary").await; let sandbox = sandbox_record("sb-1", "missing", SandboxPhase::Ready); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8323,7 +8337,7 @@ mod tests { async fn start_persisted_sandboxes_marks_failed_start_as_error() { let driver = ControlledDriver::new(); driver.set_start_outcome(ControlledLifecycleOutcome::Error("runtime angry")); - let runtime = test_runtime_for_driver(driver, "vm").await; + let runtime = test_runtime_with_gateway_managed_lifecycle(driver, "arbitrary").await; let sandbox = sandbox_record("sb-1", "broken", SandboxPhase::Provisioning); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8350,10 +8364,11 @@ mod tests { } #[tokio::test] - async fn start_persisted_sandboxes_runs_for_each_local_driver() { - for driver_name in ["docker", "podman", "vm"] { + async fn start_persisted_sandboxes_runs_for_any_capable_driver() { + for driver_name in ["arbitrary", "docker"] { let driver = ControlledDriver::new(); - let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; + let runtime = + test_runtime_with_gateway_managed_lifecycle(driver.clone(), driver_name).await; let sandbox = sandbox_record("sb-1", "local", SandboxPhase::Ready); runtime.store.put_message(&sandbox).await.unwrap(); @@ -8368,8 +8383,8 @@ mod tests { } #[tokio::test] - async fn start_persisted_sandboxes_skips_kubernetes_and_extension_drivers() { - for driver_name in ["kubernetes", "extension"] { + async fn start_persisted_sandboxes_skips_drivers_without_capability() { + for driver_name in ["docker", "kubernetes", "extension"] { let driver = ControlledDriver::new(); let runtime = test_runtime_for_driver(driver.clone(), driver_name).await; let sandbox = sandbox_record("sb-1", "remote", SandboxPhase::Ready); @@ -8596,6 +8611,7 @@ mod tests { let driver = FakeComputeDriver::new() .with_driver_name("fake-remote-driver") .with_default_image("openshell/sandbox:remote") + .with_gateway_managed_lifecycle() .with_gateway_listener_requirement( "172.19.0.1:17670", "external driver managed bridge", diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index f8124ded6c..a9f16c2116 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -74,6 +74,7 @@ struct FakeComputeDriverState { driver_name: String, driver_version: String, default_image: String, + gateway_managed_lifecycle: bool, gateway_listener_requirements: Vec, gateway_listener_requirements_supported: bool, sandboxes: HashMap, @@ -95,6 +96,7 @@ impl FakeComputeDriver { driver_name: "fake-compute-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), + gateway_managed_lifecycle: false, gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, sandboxes: HashMap::new(), @@ -122,6 +124,12 @@ impl FakeComputeDriver { self } + #[must_use] + pub fn with_gateway_managed_lifecycle(self) -> Self { + self.with_state(|state| state.gateway_managed_lifecycle = true); + self + } + #[must_use] pub fn with_gateway_listener_requirement( self, @@ -241,6 +249,7 @@ impl ComputeDriver for FakeComputeDriver { driver_name: state.driver_name.clone(), driver_version: state.driver_version.clone(), default_image: state.default_image.clone(), + gateway_managed_lifecycle: state.gateway_managed_lifecycle, } }); Ok(Response::new(response)) diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index 767db5a445..e695014dda 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -75,6 +75,9 @@ message GetCapabilitiesResponse { string driver_version = 2; // Default sandbox image recommended by the driver. string default_image = 3; + // Whether the gateway should stop running sandbox compute during graceful + // shutdown and restart the retained running intent on startup. + bool gateway_managed_lifecycle = 6; } message GetGatewayListenerRequirementsRequest {} From e0ea8f4d4b40abff24bed88a0182003b403e3a0d Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Wed, 19 Aug 2026 08:48:20 -0700 Subject: [PATCH 6/6] refactor(compute): clarify lifecycle ownership Signed-off-by: Drew Newberry --- architecture/compute-runtimes.md | 2 +- crates/openshell-core/src/driver_utils.rs | 20 +----------- crates/openshell-driver-docker/src/lib.rs | 12 +++---- .../openshell-driver-kubernetes/src/driver.rs | 12 +++---- crates/openshell-driver-podman/src/driver.rs | 12 +++---- crates/openshell-driver-vm/src/driver.rs | 2 +- crates/openshell-server/src/cli.rs | 7 ++-- crates/openshell-server/src/compute/mod.rs | 22 ++++++------- crates/openshell-server/src/lib.rs | 5 ++- crates/openshell-server/src/test_support.rs | 32 ++++++++----------- proto/compute_driver.proto | 2 +- 11 files changed, 54 insertions(+), 74 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 33a9b7c2c4..3085745304 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -123,7 +123,7 @@ cluster-owned and continue running without gateway shutdown or startup lifecycle calls. The driver reports this behavior through -`GetCapabilities.gateway_managed_lifecycle`. The same declaration works for +`GetCapabilities.gateway_manages_lifecycle`. The same declaration works for in-process and external drivers. Older drivers omit the field and retain the conservative operator-managed behavior. diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index 523d9ccc42..8f88c0cd4a 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; -use crate::proto::compute::v1::{DriverSandbox, GetCapabilitiesResponse}; +use crate::proto::compute::v1::DriverSandbox; pub use crate::container_paths::{ SANDBOX_TOKEN_MOUNT_PATH, SUPERVISOR_CONTAINER_BINARY, SUPERVISOR_CONTAINER_DIR, @@ -372,24 +372,6 @@ pub fn sandbox_token_path( Ok(path.join(sandbox_id).join("sandbox.jwt")) } -/// Build a [`GetCapabilitiesResponse`] from the common driver capability fields. -/// -/// Every compute driver constructs this response with the same fields. Shared -/// here to avoid repeating the struct literal in each driver crate. -pub fn build_capabilities_response( - driver_name: &str, - driver_version: impl Into, - default_image: impl Into, - gateway_managed_lifecycle: bool, -) -> GetCapabilitiesResponse { - GetCapabilitiesResponse { - driver_name: driver_name.to_string(), - driver_version: driver_version.into(), - default_image: default_image.into(), - gateway_managed_lifecycle, - } -} - /// Return the effective log level for a sandbox. /// /// Uses the level from the sandbox spec when non-empty, falling back to diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 1b9fe1f23b..60c625aa26 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -490,12 +490,12 @@ impl DockerComputeDriver { } fn capabilities(&self) -> GetCapabilitiesResponse { - openshell_core::driver_utils::build_capabilities_response( - "docker", - &self.config.daemon_version, - &self.config.default_image, - true, - ) + GetCapabilitiesResponse { + driver_name: "docker".to_string(), + driver_version: self.config.daemon_version.clone(), + default_image: self.config.default_image.clone(), + gateway_manages_lifecycle: true, + } } #[cfg(test)] diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 79c1b43c9b..b9b9de72ad 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -544,12 +544,12 @@ impl KubernetesComputeDriver { } pub fn capabilities(&self) -> Result { - Ok(openshell_core::driver_utils::build_capabilities_response( - "kubernetes", - openshell_core::VERSION, - &self.config.default_image, - false, - )) + Ok(GetCapabilitiesResponse { + driver_name: "kubernetes".to_string(), + driver_version: openshell_core::VERSION.to_string(), + default_image: self.config.default_image.clone(), + gateway_manages_lifecycle: false, + }) } pub fn operator_allowlist(&self) -> Option<&OperatorNamespaceAllowlist> { diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 976c67b92d..f70141f3fb 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -464,12 +464,12 @@ impl PodmanComputeDriver { /// Report driver capabilities. pub fn capabilities(&self) -> Result { - Ok(openshell_core::driver_utils::build_capabilities_response( - "podman", - openshell_core::VERSION, - &self.config.default_image, - true, - )) + Ok(GetCapabilitiesResponse { + driver_name: "podman".to_string(), + driver_version: openshell_core::VERSION.to_string(), + default_image: self.config.default_image.clone(), + gateway_manages_lifecycle: true, + }) } /// Report the gateway exposure needed by Podman's standard local callback aliases. diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 28dc348800..13bcdb10c5 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -520,7 +520,7 @@ impl VmDriver { driver_name: DRIVER_NAME.to_string(), driver_version: openshell_core::VERSION.to_string(), default_image: self.config.default_image.clone(), - gateway_managed_lifecycle: true, + gateway_manages_lifecycle: true, } } diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index fc70284721..02afb3917f 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -115,9 +115,10 @@ struct RunArgs { /// implementing `compute_driver.proto`. /// /// When set, the socket is associated with the single driver name supplied - /// by `--drivers` or `OPENSHELL_DRIVERS`. The endpoint overrides built-in - /// construction when the selected name is Docker, Podman, Kubernetes, or - /// VM. + /// by `--drivers` or `OPENSHELL_DRIVERS` and replaces normal construction + /// for that selected name, including canonical built-in names. The gateway + /// connects to this operator-provided endpoint; it does not provision the + /// remote driver. #[arg(long, env = "OPENSHELL_COMPUTE_DRIVER_SOCKET")] compute_driver_socket: Option, diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 9dbdd605fd..dfc1acf9aa 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -278,7 +278,7 @@ pub struct ComputeDriverInfoSnapshot { /// Driver-reported implementation version from the startup capability snapshot. pub driver_version: String, /// Whether the driver asks the gateway to reconcile compute across restarts. - pub gateway_managed_lifecycle: bool, + pub gateway_manages_lifecycle: bool, } /// Interval between store-vs-backend reconciliation sweeps. @@ -614,7 +614,7 @@ impl ComputeRuntime { name: driver_name.clone(), driver_name: capabilities.driver_name, driver_version: capabilities.driver_version, - gateway_managed_lifecycle: capabilities.gateway_managed_lifecycle, + gateway_manages_lifecycle: capabilities.gateway_manages_lifecycle, }; let default_image = capabilities.default_image; let gateway_listener_requirements = match driver @@ -2047,7 +2047,7 @@ impl ComputeRuntime { /// An explicit sandbox stop persists `Stopped`; gateway shutdown does not. /// Drivers request this sweep through their startup capability snapshot. async fn stop_persisted_sandboxes_on_shutdown(&self) -> Result<(), String> { - if !self.driver_info.gateway_managed_lifecycle { + if !self.driver_info.gateway_manages_lifecycle { return Ok(()); } @@ -2144,7 +2144,7 @@ impl ComputeRuntime { /// so the watch loop sees the post-start state on its first poll. pub async fn start_persisted_sandboxes(&self) -> Result<(), String> { self.recover_persisted_lifecycle_transitions().await?; - if !self.driver_info.gateway_managed_lifecycle { + if !self.driver_info.gateway_manages_lifecycle { return Ok(()); } @@ -3943,7 +3943,7 @@ impl ComputeDriver for NoopTestDriver { driver_name: "noop-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - gateway_managed_lifecycle: false, + gateway_manages_lifecycle: false, }, )) } @@ -4084,7 +4084,7 @@ pub async fn new_test_runtime_with_driver( name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), - gateway_managed_lifecycle: false, + gateway_manages_lifecycle: false, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4247,7 +4247,7 @@ mod tests { driver_name: "test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - gateway_managed_lifecycle: false, + gateway_manages_lifecycle: false, })) } @@ -4565,7 +4565,7 @@ mod tests { driver_name: "controlled-test-driver".to_string(), driver_version: "test".to_string(), default_image: "openshell/sandbox:test".to_string(), - gateway_managed_lifecycle: false, + gateway_manages_lifecycle: false, })) } @@ -4768,7 +4768,7 @@ mod tests { name: driver_name.to_string(), driver_name: driver_name.to_string(), driver_version: "test".to_string(), - gateway_managed_lifecycle: false, + gateway_manages_lifecycle: false, }, driver_process: None, default_image: "openshell/sandbox:test".to_string(), @@ -4789,7 +4789,7 @@ mod tests { driver_name: &str, ) -> ComputeRuntime { let mut runtime = test_runtime_for_driver(driver, driver_name).await; - runtime.driver_info.gateway_managed_lifecycle = true; + runtime.driver_info.gateway_manages_lifecycle = true; runtime } @@ -8792,7 +8792,7 @@ mod tests { let driver = FakeComputeDriver::new() .with_driver_name("fake-remote-driver") .with_default_image("openshell/sandbox:remote") - .with_gateway_managed_lifecycle() + .with_gateway_manages_lifecycle() .with_gateway_listener_requirement( "172.19.0.1:17670", "external driver managed bridge", diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 6ff455bea2..9ea8161a0d 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1248,11 +1248,14 @@ fn resolve_configured_compute_driver( ) -> Result { let name = openshell_core::config::normalize_compute_driver_name(driver_name) .map_err(Error::config)?; - let driver_kind = builtin_compute_driver(&name); + // An operator-provided endpoint replaces normal construction for the + // selected name. The gateway connects to it; it does not provision a + // remote implementation for canonical built-in names. if driver_startup.endpoint_overrides.contains_key(&name) { return Ok(ConfiguredComputeDriver::Remote { name }); } + let driver_kind = builtin_compute_driver(&name); if let Some(kind) = driver_kind { return Ok(ConfiguredComputeDriver::Builtin(kind)); } diff --git a/crates/openshell-server/src/test_support.rs b/crates/openshell-server/src/test_support.rs index a9f16c2116..957db46fab 100644 --- a/crates/openshell-server/src/test_support.rs +++ b/crates/openshell-server/src/test_support.rs @@ -71,10 +71,7 @@ pub struct FakeComputeDriver { #[derive(Debug)] struct FakeComputeDriverState { - driver_name: String, - driver_version: String, - default_image: String, - gateway_managed_lifecycle: bool, + capabilities: GetCapabilitiesResponse, gateway_listener_requirements: Vec, gateway_listener_requirements_supported: bool, sandboxes: HashMap, @@ -93,10 +90,12 @@ impl FakeComputeDriver { pub fn new() -> Self { Self { state: Arc::new(Mutex::new(FakeComputeDriverState { - driver_name: "fake-compute-driver".to_string(), - driver_version: "test".to_string(), - default_image: "openshell/sandbox:test".to_string(), - gateway_managed_lifecycle: false, + capabilities: GetCapabilitiesResponse { + driver_name: "fake-compute-driver".to_string(), + driver_version: "test".to_string(), + default_image: "openshell/sandbox:test".to_string(), + gateway_manages_lifecycle: false, + }, gateway_listener_requirements: Vec::new(), gateway_listener_requirements_supported: true, sandboxes: HashMap::new(), @@ -108,25 +107,25 @@ impl FakeComputeDriver { #[must_use] pub fn with_driver_name(self, driver_name: impl Into) -> Self { - self.with_state(|state| state.driver_name = driver_name.into()); + self.with_state(|state| state.capabilities.driver_name = driver_name.into()); self } #[must_use] pub fn with_driver_version(self, driver_version: impl Into) -> Self { - self.with_state(|state| state.driver_version = driver_version.into()); + self.with_state(|state| state.capabilities.driver_version = driver_version.into()); self } #[must_use] pub fn with_default_image(self, default_image: impl Into) -> Self { - self.with_state(|state| state.default_image = default_image.into()); + self.with_state(|state| state.capabilities.default_image = default_image.into()); self } #[must_use] - pub fn with_gateway_managed_lifecycle(self) -> Self { - self.with_state(|state| state.gateway_managed_lifecycle = true); + pub fn with_gateway_manages_lifecycle(self) -> Self { + self.with_state(|state| state.capabilities.gateway_manages_lifecycle = true); self } @@ -245,12 +244,7 @@ impl ComputeDriver for FakeComputeDriver { self.record_traceparent(request.metadata()); let response = self.with_state(|state| { state.calls.push(FakeComputeDriverCall::GetCapabilities); - GetCapabilitiesResponse { - driver_name: state.driver_name.clone(), - driver_version: state.driver_version.clone(), - default_image: state.default_image.clone(), - gateway_managed_lifecycle: state.gateway_managed_lifecycle, - } + state.capabilities.clone() }); Ok(Response::new(response)) } diff --git a/proto/compute_driver.proto b/proto/compute_driver.proto index e695014dda..76b8d94806 100644 --- a/proto/compute_driver.proto +++ b/proto/compute_driver.proto @@ -77,7 +77,7 @@ message GetCapabilitiesResponse { string default_image = 3; // Whether the gateway should stop running sandbox compute during graceful // shutdown and restart the retained running intent on startup. - bool gateway_managed_lifecycle = 6; + bool gateway_manages_lifecycle = 6; } message GetGatewayListenerRequirementsRequest {}