From 03e4cdbfa8e4158d653a2831e177a48a43fe2654 Mon Sep 17 00:00:00 2001 From: Derek Carr Date: Fri, 7 Aug 2026 17:37:02 -0400 Subject: [PATCH 1/4] feat(k8s): add namespace-per-workspace support (RFC 0011 Phase 3) Implement three workspace namespace modes for the Kubernetes compute driver: shared (default, preserves current single-namespace behavior), managed (auto-creates/deletes namespaces per workspace), and operator (pre-provisioned namespaces with dynamic discovery via label selector or drop-in allowlist file). Key changes: - WorkspaceMode enum and namespace resolution in driver config - Managed namespace lifecycle with ServiceAccount and OpenShift SCC annotation propagation - Cluster-wide sandbox CR watchers for managed/operator modes - NamespaceValidator (Exact/Prefix/Allowlist) for SA token auth - Workspace-aware credential secret storage - Helm ClusterRole for multi-namespace RBAC - Gateway config, architecture, and reference docs Signed-off-by: Derek Carr --- architecture/compute-runtimes.md | 63 ++ crates/openshell-core/src/driver_utils.rs | 3 + .../src/lib.rs | 74 ++- crates/openshell-driver-kubernetes/README.md | 14 +- .../openshell-driver-kubernetes/src/config.rs | 539 ++++++++++++++++++ .../openshell-driver-kubernetes/src/driver.rs | 532 ++++++++++++++--- crates/openshell-driver-kubernetes/src/lib.rs | 5 +- .../openshell-driver-kubernetes/src/main.rs | 26 +- crates/openshell-server/src/auth/k8s_sa.rs | 256 ++++++--- crates/openshell-server/src/lib.rs | 26 +- deploy/helm/openshell/README.md | 3 + .../helm/openshell/templates/clusterrole.yaml | 52 ++ .../openshell/templates/gateway-config.yaml | 10 + deploy/helm/openshell/templates/role.yaml | 3 + .../helm/openshell/templates/rolebinding.yaml | 3 + deploy/helm/openshell/values.yaml | 14 + docs/reference/gateway-config.mdx | 15 + 17 files changed, 1466 insertions(+), 172 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 646b6320bd..f22da681a1 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -286,5 +286,68 @@ Standalone local deployments start the gateway with a selected runtime such as Docker, Podman, or VM. The CLI can register multiple gateways and switch between them without changing the sandbox architecture. +## Workspace Namespace Modes (Kubernetes) + +The Kubernetes driver maps workspaces to namespaces through the `workspace_mode` +configuration field (`WorkspaceMode` in `crates/openshell-driver-kubernetes/src/config.rs`). +The mode controls namespace resolution, resource naming, sandbox CR watching, SA +token authentication, and RBAC requirements. + +| Mode | Namespace resolution | Resource name | Namespace lifecycle | +|---|---|---|---| +| **Shared** (default) | Single static namespace from config | `{workspace}--{name}` | None | +| **Managed** | `openshell-{gateway_id}-{workspace}` | bare sandbox name | Driver creates and deletes | +| **Operator** | Workspace name maps 1:1 to a pre-provisioned namespace | bare sandbox name | External (platform team) | + +**Shared** renders all sandboxes into one configured namespace. Resource names +embed the workspace prefix for collision avoidance. No namespace lifecycle +management. RBAC uses a namespace-scoped Role. + +**Managed** auto-creates a K8s namespace per workspace on first sandbox create. +Each new namespace receives a ServiceAccount and copies OpenShift SCC UID-range +and supplemental-group annotations from the gateway namespace when present. The +driver deletes the namespace when the last sandbox in it is removed +(`delete_namespace_if_empty`). Requires a non-empty `gateway_id` (validated as a +DNS-1123 label at startup) so the namespace prefix fits within the K8s 63-character +limit. RBAC promotes sandbox CRD permissions to a ClusterRole and adds namespace +`create`/`delete` and ServiceAccount `create`/`get` permissions. + +**Operator** uses pre-provisioned namespaces discovered through two optional +sources: a K8s label selector (`operator_namespace_label`) and a drop-in +allowlist file (`operator_namespace_file`). At least one must be configured. +The `OperatorNamespaceAllowlist` (`Arc>>`) is populated +at runtime by background watchers and read by the namespace resolver. Sandbox +creation fails closed if the workspace is not in the current allowlist. Platform +teams manage namespace lifecycle externally. RBAC uses the same ClusterRole as +managed mode but without namespace `create`/`delete` or ServiceAccount +permissions. + +### Watching and Querying + +Managed and operator modes set `is_multi_namespace() == true`, which switches +sandbox CR watchers from namespace-scoped `Api::namespaced` to cluster-wide +`Api::all_with`. In managed mode the driver scopes cluster-wide queries with a +`LABEL_GATEWAY_ID` label selector to support multiple gateways on the same +cluster. K8s Events are not watched in cluster-wide mode — the cluster-wide +watcher emits only sandbox CR changes, not platform events. + +### SA Token Authentication + +The gateway's `K8sServiceAccountAuthenticator` adapts its `NamespaceValidator` +per mode (`crates/openshell-server/src/auth/k8s_sa.rs`): + +- **Shared:** `Exact` — accepts only the single configured namespace. +- **Managed:** `Prefix` — accepts any namespace starting with `openshell-{gateway_id}-`. +- **Operator:** `Allowlist` — accepts namespaces present in the dynamic + `BTreeSet` populated by the label/file watchers. Starts empty (fail-closed) + until the first watcher update. + +### Credential Driver Integration + +The Kubernetes Secrets credential driver (`openshell-driver-kubernetes-secrets`) +stores secrets in workspace-specific namespaces when `workspace_mode` is managed +or operator. In shared mode, all secrets render into the single configured +namespace. + When runtime infrastructure changes, validate the relevant sandbox e2e path and update the matching driver README if a maintainer-facing constraint changes. diff --git a/crates/openshell-core/src/driver_utils.rs b/crates/openshell-core/src/driver_utils.rs index 9bcca9f11d..6cc547b263 100644 --- a/crates/openshell-core/src/driver_utils.rs +++ b/crates/openshell-core/src/driver_utils.rs @@ -35,6 +35,9 @@ pub const LABEL_SANDBOX_NAMESPACE: &str = "openshell.ai/sandbox-namespace"; /// Container/pod label carrying the sandbox workspace. pub const LABEL_SANDBOX_WORKSPACE: &str = "openshell.ai/sandbox-workspace"; +/// Label carrying the gateway identity on managed namespaces. +pub const LABEL_GATEWAY_ID: &str = "openshell.ai/gateway-id"; + /// Label selector that matches all OpenShell-managed resources which carry a /// sandbox ID label. Used by list and watch operations to exclude foreign /// resources from the same namespace. diff --git a/crates/openshell-driver-kubernetes-secrets/src/lib.rs b/crates/openshell-driver-kubernetes-secrets/src/lib.rs index 65c655be16..1c59401cb1 100644 --- a/crates/openshell-driver-kubernetes-secrets/src/lib.rs +++ b/crates/openshell-driver-kubernetes-secrets/src/lib.rs @@ -48,10 +48,33 @@ impl CredentialDriverService { } } +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Deserialize)] +#[serde(rename_all = "snake_case")] +enum WorkspaceMode { + #[default] + Shared, + Managed, + Operator, +} + #[derive(Debug, Clone, PartialEq, Eq)] struct KubernetesSecretsDriverSettings { namespace: String, allow_reference_namespace: bool, + workspace_mode: WorkspaceMode, + gateway_id: String, +} + +impl KubernetesSecretsDriverSettings { + fn target_namespace(&self, workspace: &str) -> String { + match self.workspace_mode { + WorkspaceMode::Shared => self.namespace.clone(), + WorkspaceMode::Managed => { + format!("openshell-{}-{}", self.gateway_id, workspace) + } + WorkspaceMode::Operator => workspace.to_string(), + } + } } #[derive(Debug, Clone, Default, serde::Deserialize)] @@ -59,6 +82,8 @@ struct KubernetesSecretsDriverSettings { struct KubernetesSecretsDriverConfig { namespace: Option, allow_reference_namespace: bool, + workspace_mode: WorkspaceMode, + gateway_id: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -134,7 +159,10 @@ impl KubernetesSecretsCredentialDriver { credential_key: &str, ) -> Result { let reference = Self::parse_handle(handle, credential_key)?; - if reference.namespace != self.settings.namespace + // In managed/operator modes secrets live in workspace-specific + // namespaces so cross-namespace handles are expected. + if self.settings.workspace_mode == WorkspaceMode::Shared + && reference.namespace != self.settings.namespace && !self.settings.allow_reference_namespace { return Err(Status::permission_denied(format!( @@ -175,7 +203,7 @@ impl KubernetesSecretsCredentialDriver { reference } else { KubernetesSecretReference { - namespace: self.settings.namespace.clone(), + namespace: self.settings.target_namespace(&request.workspace), secret_name: managed_secret_name( &request.workspace, &request.provider_id, @@ -508,6 +536,8 @@ impl KubernetesSecretsDriverSettings { Ok(Self { namespace, allow_reference_namespace: config.allow_reference_namespace, + workspace_mode: config.workspace_mode, + gateway_id: config.gateway_id.unwrap_or_default(), }) } } @@ -842,6 +872,8 @@ mod tests { let settings = KubernetesSecretsDriverSettings { namespace: "openshell".to_string(), allow_reference_namespace: false, + workspace_mode: WorkspaceMode::Shared, + gateway_id: String::new(), }; let reference = KubernetesSecretsCredentialDriver::parse_handle( &handle("v1:other-namespace:provider-secret"), @@ -865,6 +897,8 @@ mod tests { let settings = KubernetesSecretsDriverSettings { namespace: "openshell".to_string(), allow_reference_namespace: true, + workspace_mode: WorkspaceMode::Shared, + gateway_id: String::new(), }; let reference = KubernetesSecretsCredentialDriver::parse_handle( &handle("v1:other-namespace:provider-secret"), @@ -1065,4 +1099,40 @@ mod tests { assert_eq!(err.code(), Code::FailedPrecondition); assert!(err.message().contains("is not managed by OpenShell")); } + + #[test] + fn target_namespace_shared_returns_static_namespace() { + let settings = KubernetesSecretsDriverSettings { + namespace: "openshell".to_string(), + allow_reference_namespace: false, + workspace_mode: WorkspaceMode::Shared, + gateway_id: String::new(), + }; + assert_eq!(settings.target_namespace("team-a"), "openshell"); + assert_eq!(settings.target_namespace("team-b"), "openshell"); + } + + #[test] + fn target_namespace_managed_computes_from_workspace() { + let settings = KubernetesSecretsDriverSettings { + namespace: "openshell".to_string(), + allow_reference_namespace: false, + workspace_mode: WorkspaceMode::Managed, + gateway_id: "gw1".to_string(), + }; + assert_eq!(settings.target_namespace("team-a"), "openshell-gw1-team-a"); + assert_eq!(settings.target_namespace("team-b"), "openshell-gw1-team-b"); + } + + #[test] + fn target_namespace_operator_uses_workspace_name() { + let settings = KubernetesSecretsDriverSettings { + namespace: "openshell".to_string(), + allow_reference_namespace: false, + workspace_mode: WorkspaceMode::Operator, + gateway_id: String::new(), + }; + assert_eq!(settings.target_namespace("team-a"), "team-a"); + assert_eq!(settings.target_namespace("prod-ns"), "prod-ns"); + } } diff --git a/crates/openshell-driver-kubernetes/README.md b/crates/openshell-driver-kubernetes/README.md index 1356e2d932..c985e0b776 100644 --- a/crates/openshell-driver-kubernetes/README.md +++ b/crates/openshell-driver-kubernetes/README.md @@ -3,8 +3,18 @@ Kubernetes-backed compute driver for OpenShell cluster deployments. The driver uses the Kubernetes API to create, delete, fetch, and watch sandbox -custom resources in the configured namespace. It runs in-process with the -gateway server. +custom resources. It runs in-process with the gateway server and supports three +workspace namespace modes via `workspace_mode`: + +- **Shared** (default): All sandboxes render into a single static namespace. + Resource names use `{workspace}--{name}` for collision avoidance. +- **Managed**: The driver auto-creates/deletes a K8s namespace per workspace + (`openshell-{gateway_id}-{workspace_name}`), creates a ServiceAccount in each, + and copies OpenShift SCC annotations from the gateway namespace when present. +- **Operator**: Workspace names map 1:1 to pre-provisioned namespaces discovered + via label selector (`operator_namespace_label`) and/or drop-in allowlist file + (`operator_namespace_file`). Sandbox creation fails closed if the workspace + namespace is not in the current allowlist. ## Runtime Model diff --git a/crates/openshell-driver-kubernetes/src/config.rs b/crates/openshell-driver-kubernetes/src/config.rs index 5311f56436..21d3aea851 100644 --- a/crates/openshell-driver-kubernetes/src/config.rs +++ b/crates/openshell-driver-kubernetes/src/config.rs @@ -3,8 +3,13 @@ use openshell_core::config; use serde::{Deserialize, Deserializer, Serialize}; +use std::collections::BTreeSet; use std::path::Path; use std::str::FromStr; +use std::sync::{Arc, RwLock}; + +/// Default gateway identity used in managed-mode namespace naming. +pub const DEFAULT_GATEWAY_ID: &str = "openshell"; /// Default Kubernetes namespace for sandbox resources. pub const DEFAULT_K8S_NAMESPACE: &str = "openshell"; @@ -88,6 +93,48 @@ impl FromStr for SupervisorTopology { } } +/// How workspaces map to Kubernetes namespaces. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum WorkspaceMode { + /// All sandboxes render into a single statically-configured namespace. + /// Resource names use `{workspace}--{name}` for collision avoidance. + #[default] + Shared, + /// The driver creates and deletes K8s namespaces on demand using the + /// convention `openshell-{gateway_id}-{workspace_name}`. + Managed, + /// Sandboxes render into pre-existing K8s namespaces. The driver has no + /// namespace create/delete permissions. Platform teams manage namespaces + /// via their existing tooling. + Operator, +} + +impl std::fmt::Display for WorkspaceMode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Shared => f.write_str("shared"), + Self::Managed => f.write_str("managed"), + Self::Operator => f.write_str("operator"), + } + } +} + +impl FromStr for WorkspaceMode { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "shared" => Ok(Self::Shared), + "managed" => Ok(Self::Managed), + "operator" => Ok(Self::Operator), + other => Err(format!( + "unknown workspace mode '{other}'; expected 'shared', 'managed', or 'operator'" + )), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default, deny_unknown_fields)] pub struct KubernetesSidecarConfig { @@ -232,7 +279,24 @@ where #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default, deny_unknown_fields)] pub struct KubernetesComputeConfig { + /// How workspaces map to Kubernetes namespaces. `"shared"` (default) + /// renders all sandboxes into `namespace`; `"managed"` creates per-workspace + /// namespaces on demand; `"operator"` uses pre-provisioned namespaces. + pub workspace_mode: WorkspaceMode, + /// Stable gateway identity used in managed-mode namespace naming + /// (`openshell-{gateway_id}-{workspace}`). Propagated from + /// `gateway_jwt.gateway_id`. + pub gateway_id: String, pub namespace: String, + /// K8s label selector for operator-mode namespace discovery (e.g., + /// `"openshell.ai/workspace=true"`). The driver watches namespaces matching + /// this label and builds the allowlist dynamically. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub operator_namespace_label: Option, + /// Path to a drop-in JSON file mapping workspace names to namespace names. + /// Hot-reloaded on change. Delivered via `ConfigMap` volume mount. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub operator_namespace_file: Option, /// Kubernetes `ServiceAccount` assigned to sandbox pods and accepted by /// the gateway's `TokenReview` bootstrap authenticator. pub service_account_name: String, @@ -332,7 +396,11 @@ pub const ANNOTATION_SCC_SUPPLEMENTAL_GROUPS: &str = "openshift.io/sa.scc.supple impl Default for KubernetesComputeConfig { fn default() -> Self { Self { + workspace_mode: WorkspaceMode::default(), + gateway_id: DEFAULT_GATEWAY_ID.to_string(), namespace: DEFAULT_K8S_NAMESPACE.to_string(), + operator_namespace_label: None, + operator_namespace_file: None, service_account_name: DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME.to_string(), default_image: openshell_core::image::default_sandbox_image(), // Default empty so the gateway omits `imagePullPolicy` from pod @@ -473,6 +541,214 @@ impl KubernetesComputeConfig { } Ok(()) } + + /// Resolve the K8s namespace for a workspace. + /// + /// - **Shared:** returns the static `namespace` config field. + /// - **Managed:** computes `openshell-{gateway_id}-{workspace_name}`. + /// - **Operator:** looks up `workspace` in the dynamic allowlist. Fails + /// closed if the workspace is not found. + pub fn namespace_for_workspace( + &self, + workspace: &str, + operator_allowlist: Option<&OperatorNamespaceAllowlist>, + ) -> Result { + match self.workspace_mode { + WorkspaceMode::Shared => Ok(self.namespace.clone()), + WorkspaceMode::Managed => Ok(managed_namespace(&self.gateway_id, workspace)), + WorkspaceMode::Operator => { + let allowlist = + operator_allowlist.ok_or("operator mode requires a namespace allowlist")?; + let namespaces = allowlist.read(); + if namespaces.contains(workspace) { + Ok(workspace.to_string()) + } else { + Err(format!( + "workspace '{workspace}' is not in the operator namespace allowlist" + )) + } + } + } + } + + /// Whether the driver operates across multiple namespaces. + #[must_use] + pub fn is_multi_namespace(&self) -> bool { + !matches!(self.workspace_mode, WorkspaceMode::Shared) + } + + /// Compute the K8s resource name for a sandbox. + /// + /// - **Shared:** `{workspace}--{name}` (namespace doesn't provide isolation). + /// - **Managed/Operator:** bare sandbox name (namespace provides isolation). + #[must_use] + pub fn kube_resource_name(&self, workspace: &str, name: &str) -> String { + match self.workspace_mode { + WorkspaceMode::Shared => format!("{workspace}--{name}"), + WorkspaceMode::Managed | WorkspaceMode::Operator => name.to_string(), + } + } + + /// Validate workspace-mode-specific configuration at startup. + pub fn validate_workspace_mode(&self) -> Result<(), String> { + match self.workspace_mode { + WorkspaceMode::Shared => Ok(()), + WorkspaceMode::Managed => { + if self.gateway_id.is_empty() { + return Err("managed workspace mode requires a non-empty gateway_id".into()); + } + if !is_dns_1123_label(&self.gateway_id) { + return Err(format!( + "gateway_id '{}' is not a valid DNS-1123 label", + self.gateway_id + )); + } + // Workspace names can be up to 19 chars (MAX_ROUTABLE_NAME_LEN + // in the server crate). The managed namespace prefix + + // workspace must fit within 63 chars. + let prefix = managed_namespace_prefix(&self.gateway_id); + if prefix.len() + 19 > 63 { + return Err(format!( + "gateway_id '{}' is too long for managed mode; \ + the namespace prefix '{}' ({} chars) plus the \ + maximum workspace name (19 chars) exceeds the \ + 63-char K8s namespace limit", + self.gateway_id, + prefix, + prefix.len() + )); + } + Ok(()) + } + WorkspaceMode::Operator => { + if self.operator_namespace_label.is_none() && self.operator_namespace_file.is_none() + { + return Err("operator workspace mode requires at least one of \ + operator_namespace_label or operator_namespace_file" + .into()); + } + if let Some(ref label) = self.operator_namespace_label + && label.is_empty() + { + return Err("operator_namespace_label must not be empty when set".into()); + } + if let Some(ref file) = self.operator_namespace_file + && file.is_empty() + { + return Err("operator_namespace_file must not be empty when set".into()); + } + Ok(()) + } + } + } +} + +/// Compute the managed-mode namespace name for a workspace. +#[must_use] +pub fn managed_namespace(gateway_id: &str, workspace: &str) -> String { + format!("openshell-{gateway_id}-{workspace}") +} + +/// The managed-mode namespace prefix used for SA token validation. +#[must_use] +pub fn managed_namespace_prefix(gateway_id: &str) -> String { + format!("openshell-{gateway_id}-") +} + +/// Check whether a string is a valid DNS-1123 label (lowercase alphanumeric +/// and hyphens, 1-63 chars, must start and end with alphanumeric). +#[must_use] +pub fn is_dns_1123_label(s: &str) -> bool { + let len = s.len(); + if len == 0 || len > 63 { + return false; + } + let bytes = s.as_bytes(); + if !bytes[0].is_ascii_lowercase() && !bytes[0].is_ascii_digit() { + return false; + } + if !bytes[len - 1].is_ascii_lowercase() && !bytes[len - 1].is_ascii_digit() { + return false; + } + bytes + .iter() + .all(|&b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') +} + +/// Validate that a workspace name produces a valid K8s namespace name in +/// managed mode (combined length <= 63, DNS-1123 compliant). +pub fn validate_managed_namespace_name(gateway_id: &str, workspace: &str) -> Result<(), String> { + let ns = managed_namespace(gateway_id, workspace); + if !is_dns_1123_label(&ns) { + return Err(format!( + "managed namespace '{ns}' (from workspace '{workspace}') is not a valid DNS-1123 label" + )); + } + Ok(()) +} + +/// Thread-safe dynamic allowlist of valid operator-mode namespaces. +/// +/// Backed by an `Arc>>` that is updated by background +/// tasks (label selector watcher, drop-in file watcher) and read by the SA +/// authenticator and namespace resolver. +#[derive(Debug, Clone)] +pub struct OperatorNamespaceAllowlist { + inner: Arc>>, +} + +impl OperatorNamespaceAllowlist { + #[must_use] + pub fn new() -> Self { + Self { + inner: Arc::new(RwLock::new(BTreeSet::new())), + } + } + + #[must_use] + pub fn from_set(set: BTreeSet) -> Self { + Self { + inner: Arc::new(RwLock::new(set)), + } + } + + /// Replace the entire allowlist (used by background watchers on refresh). + pub fn replace(&self, new_set: BTreeSet) { + let mut guard = self.inner.write().expect("allowlist lock poisoned"); + *guard = new_set; + } + + /// Merge additional namespaces into the allowlist. + pub fn merge(&self, additional: &BTreeSet) { + let mut guard = self.inner.write().expect("allowlist lock poisoned"); + guard.extend(additional.iter().cloned()); + } + + /// Read the current allowlist snapshot. + pub fn read(&self) -> std::sync::RwLockReadGuard<'_, BTreeSet> { + self.inner.read().expect("allowlist lock poisoned") + } + + /// Check whether a namespace is in the allowlist. + #[must_use] + pub fn contains(&self, namespace: &str) -> bool { + self.inner + .read() + .expect("allowlist lock poisoned") + .contains(namespace) + } + + /// Return a clone of the inner `Arc` for sharing with background tasks. + #[must_use] + pub fn shared(&self) -> Arc>> { + Arc::clone(&self.inner) + } +} + +impl Default for OperatorNamespaceAllowlist { + fn default() -> Self { + Self::new() + } } fn validate_provider_spiffe_workload_api_socket_path_value( @@ -966,4 +1242,267 @@ mod tests { let uid = cfg.resolve_sandbox_uid(None); assert_eq!(cfg.resolve_sandbox_gid(uid, None), uid); } + + // -- WorkspaceMode tests -- + + #[test] + fn default_workspace_mode_is_shared() { + let cfg = KubernetesComputeConfig::default(); + assert_eq!(cfg.workspace_mode, WorkspaceMode::Shared); + } + + #[test] + fn serde_override_workspace_mode_managed() { + let json = serde_json::json!({ "workspace_mode": "managed" }); + let cfg: KubernetesComputeConfig = serde_json::from_value(json).unwrap(); + assert_eq!(cfg.workspace_mode, WorkspaceMode::Managed); + } + + #[test] + fn serde_override_workspace_mode_operator() { + let json = serde_json::json!({ "workspace_mode": "operator" }); + let cfg: KubernetesComputeConfig = serde_json::from_value(json).unwrap(); + assert_eq!(cfg.workspace_mode, WorkspaceMode::Operator); + } + + #[test] + fn serde_rejects_invalid_workspace_mode() { + let json = serde_json::json!({ "workspace_mode": "invalid" }); + let err = serde_json::from_value::(json).unwrap_err(); + assert!(err.to_string().contains("unknown variant")); + } + + #[test] + fn workspace_mode_display_roundtrips() { + for mode in [ + WorkspaceMode::Shared, + WorkspaceMode::Managed, + WorkspaceMode::Operator, + ] { + assert_eq!(mode.to_string().parse::().unwrap(), mode); + } + } + + #[test] + fn namespace_for_workspace_shared() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Shared, + namespace: "sandbox-ns".to_string(), + ..KubernetesComputeConfig::default() + }; + assert_eq!( + cfg.namespace_for_workspace("team-a", None).unwrap(), + "sandbox-ns" + ); + assert_eq!( + cfg.namespace_for_workspace("team-b", None).unwrap(), + "sandbox-ns" + ); + } + + #[test] + fn namespace_for_workspace_managed() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + gateway_id: "gw1".to_string(), + ..KubernetesComputeConfig::default() + }; + assert_eq!( + cfg.namespace_for_workspace("team-a", None).unwrap(), + "openshell-gw1-team-a" + ); + } + + #[test] + fn namespace_for_workspace_operator() { + let allowlist = OperatorNamespaceAllowlist::from_set(BTreeSet::from(["prod".to_string()])); + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Operator, + operator_namespace_label: Some("openshell.ai/workspace=true".to_string()), + ..KubernetesComputeConfig::default() + }; + assert_eq!( + cfg.namespace_for_workspace("prod", Some(&allowlist)) + .unwrap(), + "prod" + ); + assert!( + cfg.namespace_for_workspace("unknown", Some(&allowlist)) + .is_err() + ); + } + + #[test] + fn kube_resource_name_shared_prefixes_workspace() { + let cfg = KubernetesComputeConfig::default(); + assert_eq!(cfg.kube_resource_name("ws", "box1"), "ws--box1"); + } + + #[test] + fn kube_resource_name_managed_uses_bare_name() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + ..KubernetesComputeConfig::default() + }; + assert_eq!(cfg.kube_resource_name("ws", "box1"), "box1"); + } + + #[test] + fn kube_resource_name_operator_uses_bare_name() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Operator, + operator_namespace_label: Some("x=y".to_string()), + ..KubernetesComputeConfig::default() + }; + assert_eq!(cfg.kube_resource_name("ws", "box1"), "box1"); + } + + #[test] + fn is_multi_namespace() { + assert!(!KubernetesComputeConfig::default().is_multi_namespace()); + assert!( + KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + ..KubernetesComputeConfig::default() + } + .is_multi_namespace() + ); + assert!( + KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Operator, + operator_namespace_label: Some("x=y".to_string()), + ..KubernetesComputeConfig::default() + } + .is_multi_namespace() + ); + } + + #[test] + fn validate_workspace_mode_shared_always_ok() { + let cfg = KubernetesComputeConfig::default(); + cfg.validate_workspace_mode().unwrap(); + } + + #[test] + fn validate_workspace_mode_managed_requires_gateway_id() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + gateway_id: String::new(), + ..KubernetesComputeConfig::default() + }; + assert!(cfg.validate_workspace_mode().is_err()); + } + + #[test] + fn validate_workspace_mode_managed_rejects_invalid_gateway_id() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + gateway_id: "INVALID".to_string(), + ..KubernetesComputeConfig::default() + }; + assert!(cfg.validate_workspace_mode().is_err()); + } + + #[test] + fn validate_workspace_mode_managed_rejects_long_gateway_id() { + // prefix = "openshell-{id}-" = 11 + id.len() + // 11 + 34 + 19 = 64 > 63 → rejected + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + gateway_id: "a".repeat(34), + ..KubernetesComputeConfig::default() + }; + let err = cfg.validate_workspace_mode().unwrap_err(); + assert!(err.contains("too long for managed mode"), "{err}"); + } + + #[test] + fn validate_workspace_mode_managed_accepts_max_gateway_id() { + // 11 + 33 + 19 = 63 → accepted + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Managed, + gateway_id: "a".repeat(33), + ..KubernetesComputeConfig::default() + }; + cfg.validate_workspace_mode().unwrap(); + } + + #[test] + fn validate_workspace_mode_operator_requires_discovery() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Operator, + ..KubernetesComputeConfig::default() + }; + assert!(cfg.validate_workspace_mode().is_err()); + } + + #[test] + fn validate_workspace_mode_operator_accepts_label_only() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Operator, + operator_namespace_label: Some("openshell.ai/workspace=true".to_string()), + ..KubernetesComputeConfig::default() + }; + cfg.validate_workspace_mode().unwrap(); + } + + #[test] + fn validate_workspace_mode_operator_accepts_file_only() { + let cfg = KubernetesComputeConfig { + workspace_mode: WorkspaceMode::Operator, + operator_namespace_file: Some("/etc/openshell/namespaces.json".to_string()), + ..KubernetesComputeConfig::default() + }; + cfg.validate_workspace_mode().unwrap(); + } + + #[test] + fn dns_1123_label_validation() { + assert!(is_dns_1123_label("openshell")); + assert!(is_dns_1123_label("my-gateway-1")); + assert!(is_dns_1123_label("a")); + assert!(!is_dns_1123_label("")); + assert!(!is_dns_1123_label("UPPER")); + assert!(!is_dns_1123_label("-starts-with-dash")); + assert!(!is_dns_1123_label("ends-with-dash-")); + assert!(!is_dns_1123_label("has_underscore")); + assert!(!is_dns_1123_label(&"a".repeat(64))); + } + + #[test] + fn managed_namespace_naming() { + assert_eq!( + managed_namespace("openshell", "default"), + "openshell-openshell-default" + ); + assert_eq!(managed_namespace("gw1", "team-a"), "openshell-gw1-team-a"); + } + + #[test] + fn validate_managed_namespace_name_accepts_valid() { + validate_managed_namespace_name("gw1", "team-a").unwrap(); + } + + #[test] + fn validate_managed_namespace_name_rejects_too_long() { + let long_workspace = "a".repeat(50); + assert!(validate_managed_namespace_name("openshell", &long_workspace).is_err()); + } + + #[test] + fn operator_allowlist_operations() { + let al = OperatorNamespaceAllowlist::new(); + assert!(!al.contains("ns1")); + + al.replace(BTreeSet::from(["ns1".to_string(), "ns2".to_string()])); + assert!(al.contains("ns1")); + assert!(al.contains("ns2")); + assert!(!al.contains("ns3")); + + al.merge(&BTreeSet::from(["ns3".to_string()])); + assert!(al.contains("ns3")); + + al.replace(BTreeSet::new()); + assert!(!al.contains("ns1")); + } } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 2f1ea72a32..53f8443d28 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -7,11 +7,12 @@ use super::AppArmorProfile; use crate::config::{ DEFAULT_PROXY_UID, DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, DEFAULT_SANDBOX_UID, DEFAULT_WORKSPACE_STORAGE_SIZE, KubernetesComputeConfig, SupervisorSideloadMethod, - SupervisorTopology, + SupervisorTopology, WorkspaceMode, managed_namespace, }; use futures::{Stream, StreamExt, TryStreamExt}; use k8s_openapi::api::core::v1::{ - Event as KubeEventObj, Namespace, Node, PersistentVolumeClaimVolumeSource, Volume, VolumeMount, + Event as KubeEventObj, Namespace, Node, PersistentVolumeClaimVolumeSource, ServiceAccount, + Volume, VolumeMount, }; use kube::api::{Api, ApiResource, DeleteParams, ListParams, PostParams, Preconditions}; use kube::core::gvk::GroupVersionKind; @@ -20,8 +21,9 @@ use kube::runtime::watcher::{self, Event}; use kube::{Client, Error as KubeError}; use openshell_core::driver_mounts; use openshell_core::driver_utils::{ - LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, - LABEL_SANDBOX_WORKSPACE, SUPERVISOR_IMAGE_BINARY_PATH, openshell_sandbox_label_selector, + LABEL_GATEWAY_ID, LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, + LABEL_SANDBOX_NAME, LABEL_SANDBOX_WORKSPACE, SUPERVISOR_IMAGE_BINARY_PATH, + openshell_sandbox_label_selector, }; use openshell_core::gpu::{driver_gpu_requirements, effective_driver_gpu_count}; use openshell_core::progress::{ @@ -450,6 +452,9 @@ impl std::fmt::Debug for KubernetesComputeDriver { impl KubernetesComputeDriver { pub async fn new(config: KubernetesComputeConfig) -> Result { + config + .validate_workspace_mode() + .map_err(KubernetesDriverError::Precondition)?; config .validate_provider_spiffe_workload_api_socket_path() .map_err(KubernetesDriverError::Precondition)?; @@ -508,6 +513,169 @@ impl KubernetesComputeDriver { &self.config.ssh_socket_path } + pub fn workspace_mode(&self) -> WorkspaceMode { + self.config.workspace_mode + } + + /// Ensure the K8s namespace for a workspace exists (managed mode only). + /// + /// Idempotent: returns the namespace name whether it was just created or + /// already existed. Also creates the sandbox `ServiceAccount` in the + /// namespace. + pub async fn ensure_namespace(&self, workspace: &str) -> Result { + let ns_name = managed_namespace(&self.config.gateway_id, workspace); + let ns_api: Api = Api::all(self.client.clone()); + + let gateway_ns_api: Api = Api::all(self.client.clone()); + let gateway_ns_annotations = match tokio::time::timeout( + KUBE_API_TIMEOUT, + gateway_ns_api.get(&self.config.namespace), + ) + .await + { + Ok(Ok(ns)) => ns.metadata.annotations.unwrap_or_default(), + _ => BTreeMap::new(), + }; + + let mut labels = BTreeMap::new(); + labels.insert( + LABEL_MANAGED_BY.to_string(), + LABEL_MANAGED_BY_VALUE.to_string(), + ); + labels.insert(LABEL_GATEWAY_ID.to_string(), self.config.gateway_id.clone()); + labels.insert(LABEL_SANDBOX_WORKSPACE.to_string(), workspace.to_string()); + + let mut annotations = BTreeMap::new(); + for key in [ + crate::config::ANNOTATION_SCC_UID_RANGE, + crate::config::ANNOTATION_SCC_SUPPLEMENTAL_GROUPS, + ] { + if let Some(val) = gateway_ns_annotations.get(key) { + annotations.insert(key.to_string(), val.clone()); + } + } + + let ns = Namespace { + metadata: ObjectMeta { + name: Some(ns_name.clone()), + labels: Some(labels), + annotations: if annotations.is_empty() { + None + } else { + Some(annotations) + }, + ..Default::default() + }, + ..Default::default() + }; + + match tokio::time::timeout(KUBE_API_TIMEOUT, ns_api.create(&PostParams::default(), &ns)) + .await + { + Ok(Ok(_)) => { + info!(namespace = %ns_name, workspace = %workspace, "created managed namespace"); + } + Ok(Err(KubeError::Api(api))) if api.code == 409 => { + debug!(namespace = %ns_name, "managed namespace already exists"); + } + Ok(Err(e)) => return Err(KubernetesDriverError::from_kube(e)), + Err(_) => { + return Err(KubernetesDriverError::Message(format!( + "timeout creating namespace {ns_name}" + ))); + } + } + + self.ensure_service_account(&ns_name).await?; + + Ok(ns_name) + } + + async fn ensure_service_account(&self, namespace: &str) -> Result<(), KubernetesDriverError> { + let sa_api: Api = Api::namespaced(self.client.clone(), namespace); + let sa = ServiceAccount { + metadata: ObjectMeta { + name: Some(self.config.service_account_name.clone()), + labels: Some(BTreeMap::from([( + LABEL_MANAGED_BY.to_string(), + LABEL_MANAGED_BY_VALUE.to_string(), + )])), + ..Default::default() + }, + ..Default::default() + }; + + match tokio::time::timeout(KUBE_API_TIMEOUT, sa_api.create(&PostParams::default(), &sa)) + .await + { + Ok(Ok(_)) => { + info!(namespace = %namespace, sa = %self.config.service_account_name, "created service account"); + } + Ok(Err(KubeError::Api(api))) if api.code == 409 => {} + Ok(Err(e)) => return Err(KubernetesDriverError::from_kube(e)), + Err(_) => { + return Err(KubernetesDriverError::Message(format!( + "timeout creating service account in {namespace}" + ))); + } + } + + Ok(()) + } + + /// Delete the managed namespace if it contains no sandboxes (managed mode + /// only). Called after sandbox deletion. + pub async fn delete_namespace_if_empty( + &self, + workspace: &str, + ) -> Result<(), KubernetesDriverError> { + let ns_name = managed_namespace(&self.config.gateway_id, workspace); + + let sandbox_api_version = self + .supported_sandbox_api_version(self.client.clone()) + .await + .map_err(KubernetesDriverError::Message)?; + let agent_api = Self::agent_sandbox_api(self.client.clone(), sandbox_api_version, &ns_name); + + let lp = ListParams::default() + .labels(&openshell_sandbox_label_selector()) + .limit(1); + let list = tokio::time::timeout(KUBE_API_TIMEOUT, agent_api.api.list(&lp)) + .await + .map_err(|_| { + KubernetesDriverError::Message(format!("timeout listing sandboxes in {ns_name}")) + })? + .map_err(KubernetesDriverError::from_kube)?; + + if !list.items.is_empty() { + debug!(namespace = %ns_name, "namespace still has sandboxes, skipping delete"); + return Ok(()); + } + + let ns_api: Api = Api::all(self.client.clone()); + match tokio::time::timeout( + KUBE_API_TIMEOUT, + ns_api.delete(&ns_name, &DeleteParams::default()), + ) + .await + { + Ok(Ok(_)) => { + info!(namespace = %ns_name, workspace = %workspace, "deleted empty managed namespace"); + } + Ok(Err(KubeError::Api(api))) if api.code == 404 => { + debug!(namespace = %ns_name, "managed namespace already deleted"); + } + Ok(Err(e)) => return Err(KubernetesDriverError::from_kube(e)), + Err(_) => { + return Err(KubernetesDriverError::Message(format!( + "timeout deleting namespace {ns_name}" + ))); + } + } + + Ok(()) + } + fn validate_driver_config_for_sandbox( &self, sandbox: &Sandbox, @@ -522,16 +690,70 @@ impl KubernetesComputeDriver { ) } - fn agent_sandbox_api(&self, client: Client, sandbox_api_version: &str) -> AgentSandboxApi { + fn agent_sandbox_api( + client: Client, + sandbox_api_version: &str, + namespace: &str, + ) -> AgentSandboxApi { + let gvk = GroupVersionKind::gvk(SANDBOX_GROUP, sandbox_api_version, SANDBOX_KIND); + let resource = ApiResource::from_gvk(&gvk); + let api = Api::namespaced_with(client, namespace, &resource); + AgentSandboxApi { api, resource } + } + + fn cluster_wide_sandbox_api(client: Client, sandbox_api_version: &str) -> AgentSandboxApi { let gvk = GroupVersionKind::gvk(SANDBOX_GROUP, sandbox_api_version, SANDBOX_KIND); let resource = ApiResource::from_gvk(&gvk); - let api = Api::namespaced_with(client, &self.config.namespace, &resource); + let api = Api::all_with(client, &resource); AgentSandboxApi { api, resource } } - async fn supported_agent_sandbox_api(&self, client: Client) -> Result { + async fn supported_agent_sandbox_api( + &self, + client: Client, + namespace: &str, + ) -> Result { let sandbox_api_version = self.supported_sandbox_api_version(client.clone()).await?; - Ok(self.agent_sandbox_api(client, sandbox_api_version)) + Ok(Self::agent_sandbox_api( + client, + sandbox_api_version, + namespace, + )) + } + + async fn supported_sandbox_api_for_lookup( + &self, + client: Client, + ) -> Result { + let sandbox_api_version = self.supported_sandbox_api_version(client.clone()).await?; + if self.config.is_multi_namespace() { + Ok(Self::cluster_wide_sandbox_api(client, sandbox_api_version)) + } else { + Ok(Self::agent_sandbox_api( + client, + sandbox_api_version, + &self.config.namespace, + )) + } + } + + fn sandbox_lookup_selector(&self, sandbox_id: &str) -> String { + let mut selector = + format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={sandbox_id}"); + if self.config.workspace_mode == WorkspaceMode::Managed { + use std::fmt::Write; + write!(selector, ",{LABEL_GATEWAY_ID}={}", self.config.gateway_id).unwrap(); + } + selector + } + + fn openshell_sandbox_selector(&self) -> String { + let mut selector = openshell_sandbox_label_selector(); + if self.config.workspace_mode == WorkspaceMode::Managed { + use std::fmt::Write; + write!(selector, ",{LABEL_GATEWAY_ID}={}", self.config.gateway_id).unwrap(); + } + selector } async fn supported_sandbox_api_version(&self, client: Client) -> Result<&'static str, String> { @@ -548,7 +770,11 @@ impl KubernetesComputeDriver { client: Client, ) -> Result<&'static str, String> { for sandbox_api_version in SANDBOX_VERSIONS { - let agent_sandbox_api = self.agent_sandbox_api(client.clone(), sandbox_api_version); + let agent_sandbox_api = Self::agent_sandbox_api( + client.clone(), + sandbox_api_version, + &self.config.namespace, + ); match tokio::time::timeout( KUBE_API_TIMEOUT, agent_sandbox_api.api.list(&ListParams::default().limit(1)), @@ -586,39 +812,27 @@ impl KubernetesComputeDriver { )) } - /// Resolve sandbox UID/GID from config or `OpenShift` SCC namespace annotations. - /// - /// Returns `(uid, gid, ns_annotations_map)`: - /// - If `sandbox_uid` is set in config, returns that (with fallback GID) - /// - Otherwise fetches the target namespace and checks for - /// `openshift.io/sa.scc.uid-range` / `openshift.io/sa.scc.supplemental-groups` - /// annotations. - /// - If neither config nor `OpenShift` is found, returns `(1000, 1000, {})` as defaults. - async fn resolve_sandbox_identity(&self) -> (u32, u32, BTreeMap) { - // Explicit config takes priority — skip namespace lookup entirely. + async fn resolve_sandbox_identity_in_namespace( + &self, + namespace: &str, + ) -> (u32, u32, BTreeMap) { if self.config.sandbox_uid.is_some() { let uid = self.config.resolve_sandbox_uid(None); let gid = self.config.resolve_sandbox_gid(uid, None); return (uid, gid, BTreeMap::new()); } - // Try to read namespace annotations for OpenShift SCC. - // Namespace is namespaced so Api::all works (it's cluster-scoped but - // can list all namespaces) and we filter by name, or use Api::namespaced. let ns_api: Api = Api::all(self.client.clone()); - match tokio::time::timeout(KUBE_API_TIMEOUT, ns_api.get(self.config.namespace.as_str())) - .await - { + match tokio::time::timeout(KUBE_API_TIMEOUT, ns_api.get(namespace)).await { Ok(Ok(ns)) => { let anns = ns.metadata.annotations.unwrap_or_default(); tracing::info!( - namespace = %self.config.namespace, + namespace = %namespace, uid_range = ?anns.get(crate::config::ANNOTATION_SCC_UID_RANGE), sup_groups = ?anns.get(crate::config::ANNOTATION_SCC_SUPPLEMENTAL_GROUPS), "Resolved namespace annotations for sandbox identity" ); let uid = self.config.resolve_sandbox_uid(Some(&anns)); - // Explicit sandbox_gid config wins; SCC annotation only applies when not set. let baseline_gid = self.config.resolve_sandbox_gid(uid, None); let gid = self.config.sandbox_gid.map_or_else( || { @@ -637,7 +851,7 @@ impl KubernetesComputeDriver { } Ok(Err(e)) => { tracing::warn!( - namespace = %self.config.namespace, + namespace = %namespace, error = %e, "Failed to fetch namespace for SCC annotations, falling back to defaults" ); @@ -647,7 +861,7 @@ impl KubernetesComputeDriver { } Err(_) => { tracing::warn!( - namespace = %self.config.namespace, + namespace = %namespace, "Namespace fetch timed out, falling back to defaults" ); let uid = DEFAULT_SANDBOX_UID; @@ -672,7 +886,15 @@ impl KubernetesComputeDriver { let _ = self .validate_driver_config_for_sandbox(sandbox) .map_err(tonic::Status::invalid_argument)?; - validate_kube_resource_name_length(&sandbox.workspace, &sandbox.name)?; + match self.config.workspace_mode { + WorkspaceMode::Shared => { + validate_kube_resource_name_length(&sandbox.workspace, &sandbox.name)?; + } + WorkspaceMode::Managed | WorkspaceMode::Operator => { + validate_kubernetes_dns1123_label(&sandbox.name, "sandbox name") + .map_err(tonic::Status::invalid_argument)?; + } + } let gpu_requirements = sandbox .spec .as_ref() @@ -693,15 +915,14 @@ impl KubernetesComputeDriver { pub async fn get_sandbox(&self, sandbox_id: &str) -> Result, String> { info!( sandbox_id = %sandbox_id, - namespace = %self.config.namespace, + workspace_mode = %self.config.workspace_mode, "Fetching sandbox from Kubernetes" ); let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + .supported_sandbox_api_for_lookup(self.client.clone()) .await?; - let selector = - format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={sandbox_id}"); + let selector = self.sandbox_lookup_selector(sandbox_id); let lp = ListParams::default().labels(&selector); match tokio::time::timeout(KUBE_API_TIMEOUT, agent_sandbox_api.api.list(&lp)).await { Ok(Ok(list)) => list.items.into_iter().next().map_or_else( @@ -710,9 +931,12 @@ impl KubernetesComputeDriver { Ok(None) }, |obj| { - Ok(sandbox_from_object(&self.config.namespace, obj) - .ok() - .map(|(_, s)| s)) + let ns = obj + .metadata + .namespace + .clone() + .unwrap_or_else(|| self.config.namespace.clone()); + Ok(sandbox_from_object(&ns, obj).ok().map(|(_, s)| s)) }, ), Ok(Err(err)) => { @@ -739,18 +963,19 @@ impl KubernetesComputeDriver { pub async fn list_sandboxes(&self) -> Result, String> { info!( - namespace = %self.config.namespace, + workspace_mode = %self.config.workspace_mode, "Listing sandboxes from Kubernetes" ); let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + .supported_sandbox_api_for_lookup(self.client.clone()) .await?; + let selector = self.openshell_sandbox_selector(); match tokio::time::timeout( KUBE_API_TIMEOUT, agent_sandbox_api .api - .list(&ListParams::default().labels(&openshell_sandbox_label_selector())), + .list(&ListParams::default().labels(&selector)), ) .await { @@ -760,7 +985,12 @@ impl KubernetesComputeDriver { .into_iter() .filter_map(|obj| { let name = obj.metadata.name.clone().unwrap_or_default(); - match sandbox_from_object(&self.config.namespace, obj) { + let ns = obj + .metadata + .namespace + .clone() + .unwrap_or_else(|| self.config.namespace.clone()); + match sandbox_from_object(&ns, obj) { Ok((_, s)) => Some(s), Err(err) => { warn!(object_name = %name, error = %err, "skipping unrecognized Sandbox in list"); @@ -778,7 +1008,6 @@ impl KubernetesComputeDriver { } Ok(Err(err)) => { warn!( - namespace = %self.config.namespace, error = %err, "Failed to list sandboxes from Kubernetes" ); @@ -786,7 +1015,6 @@ impl KubernetesComputeDriver { } Err(_elapsed) => { warn!( - namespace = %self.config.namespace, timeout_secs = KUBE_API_TIMEOUT.as_secs(), "Timed out listing sandboxes from Kubernetes" ); @@ -813,21 +1041,32 @@ impl KubernetesComputeDriver { .map_err(KubernetesDriverError::InvalidArgument)?; let name = sandbox.name.as_str(); + let workspace = sandbox.workspace.as_str(); + + let target_namespace = match self.config.workspace_mode { + WorkspaceMode::Shared => self.config.namespace.clone(), + WorkspaceMode::Managed => self.ensure_namespace(workspace).await?, + WorkspaceMode::Operator => workspace.to_string(), + }; + info!( sandbox_id = %sandbox.id, sandbox_name = %name, - namespace = %self.config.namespace, + namespace = %target_namespace, + workspace = %workspace, + workspace_mode = %self.config.workspace_mode, "Creating sandbox in Kubernetes" ); let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + .supported_agent_sandbox_api(self.client.clone(), &target_namespace) .await .map_err(KubernetesDriverError::Message)?; // Resolve sandbox UID/GID from config or OpenShift SCC namespace annotations. - let (resolved_user_id, resolved_group_id, ns_annotations) = - self.resolve_sandbox_identity().await; + let (resolved_user_id, resolved_group_id, ns_annotations) = self + .resolve_sandbox_identity_in_namespace(&target_namespace) + .await; let params = SandboxPodParams { default_image: &self.config.default_image, @@ -866,11 +1105,8 @@ impl KubernetesComputeDriver { let data = sandbox_to_k8s_spec(sandbox.spec.as_ref(), ¶ms) .map_err(KubernetesDriverError::InvalidArgument)?; - let kube_name = kube_resource_name(&sandbox.workspace, name); + let kube_name = self.config.kube_resource_name(workspace, name); let mut obj = DynamicObject::new(&kube_name, &agent_sandbox_api.resource); - // Copy only the SCC-related annotations onto the Sandbox CR for - // traceability. Copying the full namespace annotation map exposes - // unrelated cluster metadata and can fail with oversized annotations. let mut annotations = sandbox_annotations(sandbox); for key in [ crate::config::ANNOTATION_SCC_UID_RANGE, @@ -882,7 +1118,7 @@ impl KubernetesComputeDriver { } obj.metadata = ObjectMeta { name: Some(kube_name), - namespace: Some(self.config.namespace.clone()), + namespace: Some(target_namespace), labels: Some(sandbox_labels(sandbox)), annotations: Some(annotations), ..Default::default() @@ -930,19 +1166,18 @@ impl KubernetesComputeDriver { pub async fn delete_sandbox(&self, sandbox_id: &str) -> Result { info!( sandbox_id = %sandbox_id, - namespace = %self.config.namespace, + workspace_mode = %self.config.workspace_mode, "Deleting sandbox from Kubernetes" ); - let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + let lookup_api = self + .supported_sandbox_api_for_lookup(self.client.clone()) .await?; - let selector = - format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={sandbox_id}"); + let selector = self.sandbox_lookup_selector(sandbox_id); let lp = ListParams::default().labels(&selector); - let (kube_name, preconditions) = match tokio::time::timeout( + let (kube_name, obj_namespace, workspace, preconditions) = match tokio::time::timeout( KUBE_API_TIMEOUT, - agent_sandbox_api.api.list(&lp), + lookup_api.api.list(&lp), ) .await { @@ -950,11 +1185,22 @@ impl KubernetesComputeDriver { if let Some(obj) = list.items.into_iter().next() { match obj.metadata.name { Some(name) => { + let ns = obj + .metadata + .namespace + .clone() + .unwrap_or_else(|| self.config.namespace.clone()); + let ws = obj + .metadata + .labels + .as_ref() + .and_then(|l| l.get(LABEL_SANDBOX_WORKSPACE).cloned()) + .unwrap_or_default(); let pc = Preconditions { uid: obj.metadata.uid, resource_version: obj.metadata.resource_version, }; - (name, pc) + (name, ns, ws, pc) } None => return Ok(false), } @@ -984,15 +1230,22 @@ impl KubernetesComputeDriver { } }; + let delete_api = self + .supported_agent_sandbox_api(self.client.clone(), &obj_namespace) + .await?; let dp = DeleteParams::default().preconditions(preconditions); - match tokio::time::timeout( - KUBE_API_TIMEOUT, - agent_sandbox_api.api.delete(&kube_name, &dp), - ) - .await - { + match tokio::time::timeout(KUBE_API_TIMEOUT, delete_api.api.delete(&kube_name, &dp)).await { Ok(Ok(_response)) => { - info!(sandbox_id = %sandbox_id, "Sandbox deleted from Kubernetes"); + info!(sandbox_id = %sandbox_id, namespace = %obj_namespace, "Sandbox deleted from Kubernetes"); + if self.config.workspace_mode == WorkspaceMode::Managed + && let Err(e) = self.delete_namespace_if_empty(&workspace).await + { + warn!( + workspace = %workspace, + error = %e, + "Failed to clean up empty managed namespace after sandbox deletion" + ); + } Ok(true) } Ok(Err(KubeError::Api(err))) if err.code == 404 || err.code == 409 => { @@ -1023,10 +1276,9 @@ impl KubernetesComputeDriver { pub async fn sandbox_exists(&self, sandbox_id: &str) -> Result { let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + .supported_sandbox_api_for_lookup(self.client.clone()) .await?; - let selector = - format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={sandbox_id}"); + let selector = self.sandbox_lookup_selector(sandbox_id); let lp = ListParams::default().labels(&selector); match tokio::time::timeout(KUBE_API_TIMEOUT, agent_sandbox_api.api.list(&lp)).await { Ok(Ok(list)) => Ok(!list.items.is_empty()), @@ -1041,9 +1293,17 @@ impl KubernetesComputeDriver { // Kept `async` to match the gRPC handler signature in `grpc.rs`, which awaits this method. #[allow(clippy::unused_async)] pub async fn watch_sandboxes(&self) -> Result { + if self.config.is_multi_namespace() { + self.watch_sandboxes_cluster_wide().await + } else { + self.watch_sandboxes_single_namespace().await + } + } + + async fn watch_sandboxes_single_namespace(&self) -> Result { let namespace = self.config.namespace.clone(); let agent_sandbox_api = self - .supported_agent_sandbox_api(self.watch_client.clone()) + .supported_agent_sandbox_api(self.watch_client.clone(), &self.config.namespace) .await?; let event_api: Api = Api::namespaced(self.watch_client.clone(), &namespace); let watcher_config = watcher::Config::default().labels(&openshell_sandbox_label_selector()); @@ -1151,6 +1411,85 @@ impl KubernetesComputeDriver { Ok(Box::pin(ReceiverStream::new(rx))) } + + async fn watch_sandboxes_cluster_wide(&self) -> Result { + let sandbox_api_version = self + .supported_sandbox_api_version(self.watch_client.clone()) + .await?; + let cluster_api = + Self::cluster_wide_sandbox_api(self.watch_client.clone(), sandbox_api_version); + let selector = self.openshell_sandbox_selector(); + let watcher_config = watcher::Config::default().labels(&selector); + let mut sandbox_stream = watcher::watcher(cluster_api.api, watcher_config).boxed(); + let (tx, rx) = mpsc::channel(256); + let default_namespace = self.config.namespace.clone(); + + tokio::spawn(async move { + loop { + tokio::select! { + result = sandbox_stream.try_next() => match result { + Ok(Some(Event::Applied(obj))) => { + let ns = obj.metadata.namespace.clone() + .unwrap_or_else(|| default_namespace.clone()); + if let Ok((_kube_name, sandbox)) = sandbox_from_object(&ns, obj) { + let event = WatchSandboxesEvent { + payload: Some(watch_sandboxes_event::Payload::Sandbox( + WatchSandboxesSandboxEvent { sandbox: Some(sandbox) } + )), + }; + if tx.send(Ok(event)).await.is_err() { + break; + } + } + } + Ok(Some(Event::Deleted(obj))) => { + if is_openshell_managed(&obj) + && let Ok(sandbox_id) = sandbox_id_from_object(&obj) + { + let event = WatchSandboxesEvent { + payload: Some(watch_sandboxes_event::Payload::Deleted( + WatchSandboxesDeletedEvent { sandbox_id } + )), + }; + if tx.send(Ok(event)).await.is_err() { + break; + } + } + } + Ok(Some(Event::Restarted(objs))) => { + for obj in objs { + let ns = obj.metadata.namespace.clone() + .unwrap_or_else(|| default_namespace.clone()); + if let Ok((_kube_name, sandbox)) = sandbox_from_object(&ns, obj) { + let event = WatchSandboxesEvent { + payload: Some(watch_sandboxes_event::Payload::Sandbox( + WatchSandboxesSandboxEvent { sandbox: Some(sandbox) } + )), + }; + if tx.send(Ok(event)).await.is_err() { + return; + } + } + } + } + Ok(None) => { + let _ = tx.send(Err(KubernetesDriverError::Message( + "sandbox watcher stream ended unexpectedly".to_string() + ))).await; + break; + } + Err(err) => { + let _ = tx.send(Err(KubernetesDriverError::Message(err.to_string()))).await; + break; + } + }, + () = tx.closed() => break, + } + } + }); + + Ok(Box::pin(ReceiverStream::new(rx))) + } } fn should_try_next_sandbox_api_version(err: &KubeError) -> bool { @@ -1169,10 +1508,6 @@ fn validate_gpu_request( Ok(()) } -fn kube_resource_name(workspace: &str, name: &str) -> String { - format!("{workspace}--{name}") -} - const MAX_KUBE_NAME_LEN: usize = 63; fn validate_kube_resource_name_length(workspace: &str, name: &str) -> Result<(), tonic::Status> { @@ -5849,22 +6184,6 @@ mod tests { assert!(validate_kubernetes_dns1123_label("dotted.name", "sandbox name").is_err()); } - #[test] - fn kube_resource_name_qualifies_with_workspace() { - assert_eq!(kube_resource_name("alpha", "work"), "alpha--work"); - assert_eq!( - kube_resource_name("default", "my-sandbox"), - "default--my-sandbox" - ); - } - - #[test] - fn kube_resource_name_different_workspaces_produce_different_names() { - let alpha = kube_resource_name("alpha", "work"); - let beta = kube_resource_name("beta", "work"); - assert_ne!(alpha, beta); - } - #[test] fn kube_resource_name_length_validation_accepts_short_names() { validate_kube_resource_name_length("default", "my-sandbox").unwrap(); @@ -5961,6 +6280,37 @@ mod tests { assert!(result.unwrap_err().contains("not managed by openshell")); } + #[test] + fn sandbox_from_object_uses_object_namespace_over_fallback() { + let obj = DynamicObject { + types: None, + metadata: ObjectMeta { + name: Some("work".to_string()), + namespace: Some("openshell-gw1-team-a".to_string()), + annotations: Some(BTreeMap::from([ + (LABEL_SANDBOX_ID.to_string(), "uuid-cross".to_string()), + (LABEL_SANDBOX_NAME.to_string(), "work".to_string()), + (LABEL_SANDBOX_WORKSPACE.to_string(), "team-a".to_string()), + ])), + labels: Some(BTreeMap::from([ + (LABEL_SANDBOX_ID.to_string(), "uuid-cross".to_string()), + (LABEL_SANDBOX_NAME.to_string(), "work".to_string()), + (LABEL_SANDBOX_WORKSPACE.to_string(), "team-a".to_string()), + ( + LABEL_MANAGED_BY.to_string(), + LABEL_MANAGED_BY_VALUE.to_string(), + ), + ])), + ..Default::default() + }, + data: serde_json::json!({}), + }; + + let (_, sandbox) = sandbox_from_object("openshell", obj).unwrap(); + assert_eq!(sandbox.namespace, "openshell-gw1-team-a"); + assert_eq!(sandbox.workspace, "team-a"); + } + #[test] fn sandbox_from_object_warns_on_managed_cr_missing_workspace() { let obj = DynamicObject { diff --git a/crates/openshell-driver-kubernetes/src/lib.rs b/crates/openshell-driver-kubernetes/src/lib.rs index 7c56c8de5b..d18a23a618 100644 --- a/crates/openshell-driver-kubernetes/src/lib.rs +++ b/crates/openshell-driver-kubernetes/src/lib.rs @@ -6,9 +6,10 @@ pub mod driver; pub mod grpc; pub use config::{ - AppArmorProfile, DEFAULT_PROXY_UID, DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, + AppArmorProfile, DEFAULT_GATEWAY_ID, DEFAULT_PROXY_UID, DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, DEFAULT_WORKSPACE_STORAGE_SIZE, KubernetesComputeConfig, KubernetesSidecarConfig, - SupervisorSideloadMethod, SupervisorTopology, + OperatorNamespaceAllowlist, SupervisorSideloadMethod, SupervisorTopology, WorkspaceMode, + managed_namespace_prefix, }; pub use driver::{KubernetesComputeDriver, KubernetesDriverError}; pub use grpc::ComputeDriverService; diff --git a/crates/openshell-driver-kubernetes/src/main.rs b/crates/openshell-driver-kubernetes/src/main.rs index b7d5514ac2..c5b7659406 100644 --- a/crates/openshell-driver-kubernetes/src/main.rs +++ b/crates/openshell-driver-kubernetes/src/main.rs @@ -10,9 +10,9 @@ use tracing_subscriber::EnvFilter; use openshell_core::VERSION; use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer; use openshell_driver_kubernetes::{ - AppArmorProfile, ComputeDriverService, DEFAULT_PROXY_UID, DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, - KubernetesComputeConfig, KubernetesComputeDriver, KubernetesSidecarConfig, - SupervisorSideloadMethod, SupervisorTopology, + AppArmorProfile, ComputeDriverService, DEFAULT_GATEWAY_ID, DEFAULT_PROXY_UID, + DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, KubernetesComputeConfig, KubernetesComputeDriver, + KubernetesSidecarConfig, SupervisorSideloadMethod, SupervisorTopology, WorkspaceMode, }; #[derive(Parser, Debug)] @@ -29,9 +29,25 @@ struct Args { #[arg(long, env = "OPENSHELL_LOG_LEVEL", default_value = "info")] log_level: String, + #[arg(long, env = "OPENSHELL_WORKSPACE_MODE", default_value = "shared")] + workspace_mode: WorkspaceMode, + + #[arg( + long, + env = "OPENSHELL_GATEWAY_ID", + default_value = DEFAULT_GATEWAY_ID + )] + gateway_id: String, + #[arg(long, env = "OPENSHELL_SANDBOX_NAMESPACE", default_value = "default")] sandbox_namespace: String, + #[arg(long, env = "OPENSHELL_OPERATOR_NAMESPACE_LABEL")] + operator_namespace_label: Option, + + #[arg(long, env = "OPENSHELL_OPERATOR_NAMESPACE_FILE")] + operator_namespace_file: Option, + #[arg( long, env = "OPENSHELL_K8S_SANDBOX_SERVICE_ACCOUNT", @@ -133,7 +149,11 @@ async fn main() -> Result<()> { .init(); let driver = KubernetesComputeDriver::new(KubernetesComputeConfig { + workspace_mode: args.workspace_mode, + gateway_id: args.gateway_id, namespace: args.sandbox_namespace, + operator_namespace_label: args.operator_namespace_label, + operator_namespace_file: args.operator_namespace_file, service_account_name: args.sandbox_service_account, default_image: args.sandbox_image.unwrap_or_default(), image_pull_policy: args.sandbox_image_pull_policy.unwrap_or_default(), diff --git a/crates/openshell-server/src/auth/k8s_sa.rs b/crates/openshell-server/src/auth/k8s_sa.rs index eed0e5f083..54f7ed9afa 100644 --- a/crates/openshell-server/src/auth/k8s_sa.rs +++ b/crates/openshell-server/src/auth/k8s_sa.rs @@ -26,7 +26,8 @@ use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta; use kube::Error as KubeError; use kube::api::{Api, ApiResource, PostParams}; use kube::core::{DynamicObject, gvk::GroupVersionKind}; -use std::sync::Arc; +use std::collections::BTreeSet; +use std::sync::{Arc, RwLock}; use tonic::Status; use tracing::{debug, info, warn}; @@ -135,8 +136,32 @@ impl Authenticator for K8sServiceAccountAuthenticator { } } +/// Validates the namespace extracted from an SA token username against the +/// expected set for the active workspace mode. +#[derive(Debug, Clone)] +pub enum NamespaceValidator { + /// Shared mode: accept only the single configured namespace. + Exact(String), + /// Managed mode: accept any namespace with the managed prefix + /// (`openshell-{gateway_id}-`). + Prefix(String), + /// Operator mode: accept namespaces in the dynamic allowlist. + Allowlist(Arc>>), +} + +impl NamespaceValidator { + pub fn accepts(&self, namespace: &str) -> bool { + match self { + Self::Exact(expected) => namespace == expected, + Self::Prefix(prefix) => namespace.starts_with(prefix.as_str()), + Self::Allowlist(set) => set.read().is_ok_and(|s| s.contains(namespace)), + } + } +} + #[derive(Debug)] struct TokenReviewIdentity { + namespace: String, pod_name: String, pod_uid: String, } @@ -151,59 +176,53 @@ struct SandboxOwnerReference { /// Resolver backed by the apiserver's `TokenReview` API and `kube::Client` /// for the per-pod annotation lookup. pub struct LiveK8sResolver { + client: kube::Client, token_reviews_api: Api, - pods_api: Api, - sandboxes_api_v1beta1: Api, - sandboxes_api_v1alpha1: Api, expected_audience: String, - sandbox_namespace: String, + namespace_validator: NamespaceValidator, expected_service_account: String, } impl LiveK8sResolver { pub fn new( client: kube::Client, - namespace: &str, + namespace_validator: NamespaceValidator, expected_audience: String, expected_service_account: String, ) -> Self { let token_reviews_api: Api = Api::all(client.clone()); - let pods_api: Api = Api::namespaced(client.clone(), namespace); - let sandbox_gvk_v1beta1 = - GroupVersionKind::gvk(SANDBOX_API_GROUP, SANDBOX_API_VERSION_V1BETA1, SANDBOX_KIND); - let sandbox_resource_v1beta1 = ApiResource::from_gvk(&sandbox_gvk_v1beta1); - let sandbox_gvk_v1alpha1 = GroupVersionKind::gvk( - SANDBOX_API_GROUP, - SANDBOX_API_VERSION_V1ALPHA1, - SANDBOX_KIND, - ); - let sandbox_resource_v1alpha1 = ApiResource::from_gvk(&sandbox_gvk_v1alpha1); - let sandboxes_api_v1beta1: Api = - Api::namespaced_with(client.clone(), namespace, &sandbox_resource_v1beta1); - let sandboxes_api_v1alpha1: Api = - Api::namespaced_with(client, namespace, &sandbox_resource_v1alpha1); Self { + client, token_reviews_api, - pods_api, - sandboxes_api_v1beta1, - sandboxes_api_v1alpha1, expected_audience, - sandbox_namespace: namespace.to_string(), + namespace_validator, expected_service_account, } } + fn pods_api(&self, namespace: &str) -> Api { + Api::namespaced(self.client.clone(), namespace) + } + + fn sandboxes_api(&self, namespace: &str, api_version: &str) -> Api { + let gvk = GroupVersionKind::gvk(SANDBOX_API_GROUP, api_version, SANDBOX_KIND); + let resource = ApiResource::from_gvk(&gvk); + Api::namespaced_with(self.client.clone(), namespace, &resource) + } + async fn get_sandbox_cr_for_owner( &self, + namespace: &str, owner: &SandboxOwnerReference, ) -> Result, KubeError> { - let apis = if owner.api_version == SANDBOX_API_VERSION_FULL_V1ALPHA1 { - [&self.sandboxes_api_v1alpha1, &self.sandboxes_api_v1beta1] + let versions = if owner.api_version == SANDBOX_API_VERSION_FULL_V1ALPHA1 { + [SANDBOX_API_VERSION_V1ALPHA1, SANDBOX_API_VERSION_V1BETA1] } else { - [&self.sandboxes_api_v1beta1, &self.sandboxes_api_v1alpha1] + [SANDBOX_API_VERSION_V1BETA1, SANDBOX_API_VERSION_V1ALPHA1] }; - for api in apis { + for version in versions { + let api = self.sandboxes_api(namespace, version); match api.get_opt(&owner.name).await { Ok(Some(sandbox_cr)) => return Ok(Some(sandbox_cr)), Ok(None) => {} @@ -242,7 +261,7 @@ impl K8sIdentityResolver for LiveK8sResolver { let Some(identity) = token_review_identity( &status, &self.expected_audience, - &self.sandbox_namespace, + &self.namespace_validator, &self.expected_service_account, )? else { @@ -252,34 +271,30 @@ impl K8sIdentityResolver for LiveK8sResolver { info!( pod_name = %identity.pod_name, pod_uid = %identity.pod_uid, + namespace = %identity.namespace, service_account = %self.expected_service_account, "validated K8s SA token via TokenReview" ); - // Look up the pod and read its sandbox-id annotation. - let pod = self - .pods_api - .get_opt(&identity.pod_name) - .await - .map_err(|e| { - warn!( - pod = %identity.pod_name, - error = %e, - "failed to fetch sandbox pod for annotation lookup" - ); - Status::internal(format!("pod GET failed: {e}")) - })?; + let pods_api = self.pods_api(&identity.namespace); + let pod = pods_api.get_opt(&identity.pod_name).await.map_err(|e| { + warn!( + pod = %identity.pod_name, + namespace = %identity.namespace, + error = %e, + "failed to fetch sandbox pod for annotation lookup" + ); + Status::internal(format!("pod GET failed: {e}")) + })?; let Some(pod) = pod else { warn!( pod = %identity.pod_name, - "sandbox pod referenced by SA token not found in this namespace" + namespace = %identity.namespace, + "sandbox pod referenced by SA token not found" ); return Err(Status::not_found("sandbox pod not found")); }; - // Defense-in-depth: confirm the pod UID matches the SA token's - // `kubernetes.io.pod.uid`. Prevents a replayed token from a - // recreated pod with the same name. let actual_uid = pod.metadata.uid.as_deref().unwrap_or_default(); if actual_uid != identity.pod_uid { warn!( @@ -294,16 +309,19 @@ impl K8sIdentityResolver for LiveK8sResolver { let sandbox_id = pod_sandbox_id(&pod)?; let owner = sandbox_owner_reference(&pod)?; - let sandbox_cr = self.get_sandbox_cr_for_owner(&owner).await.map_err(|e| { - warn!( - pod = %identity.pod_name, - sandbox_owner = %owner.name, - sandbox_owner_api_version = %owner.api_version, - error = %e, - "failed to fetch owning Sandbox CR for pod identity validation" - ); - Status::internal(format!("sandbox GET failed: {e}")) - })?; + let sandbox_cr = self + .get_sandbox_cr_for_owner(&identity.namespace, &owner) + .await + .map_err(|e| { + warn!( + pod = %identity.pod_name, + sandbox_owner = %owner.name, + sandbox_owner_api_version = %owner.api_version, + error = %e, + "failed to fetch owning Sandbox CR for pod identity validation" + ); + Status::internal(format!("sandbox GET failed: {e}")) + })?; let Some(sandbox_cr) = sandbox_cr else { warn!( pod = %identity.pod_name, @@ -327,7 +345,7 @@ impl K8sIdentityResolver for LiveK8sResolver { fn token_review_identity( status: &TokenReviewStatus, expected_audience: &str, - sandbox_namespace: &str, + namespace_validator: &NamespaceValidator, expected_service_account: &str, ) -> Result, Status> { if status.authenticated != Some(true) { @@ -356,13 +374,20 @@ fn token_review_identity( .username .as_deref() .ok_or_else(|| Status::permission_denied("TokenReview response missing username"))?; - let expected_username = - format!("system:serviceaccount:{sandbox_namespace}:{expected_service_account}"); - if username != expected_username { + + let (namespace, sa_name) = parse_sa_username(username).ok_or_else(|| { warn!( username = %username, - sandbox_namespace = %sandbox_namespace, - service_account = %expected_service_account, + "K8s TokenReview username is not a service account" + ); + Status::permission_denied("SA token username format not recognized") + })?; + + if sa_name != expected_service_account { + warn!( + username = %username, + service_account = %sa_name, + expected = %expected_service_account, "K8s TokenReview principal is not the configured sandbox service account" ); return Err(Status::permission_denied( @@ -370,9 +395,33 @@ fn token_review_identity( )); } + if !namespace_validator.accepts(&namespace) { + warn!( + username = %username, + namespace = %namespace, + "K8s TokenReview SA namespace not accepted by workspace mode validator" + ); + return Err(Status::permission_denied( + "SA token is not from an accepted sandbox namespace", + )); + } + let pod_name = user_extra_one(user, POD_NAME_EXTRA)?; let pod_uid = user_extra_one(user, POD_UID_EXTRA)?; - Ok(Some(TokenReviewIdentity { pod_name, pod_uid })) + Ok(Some(TokenReviewIdentity { + namespace, + pod_name, + pod_uid, + })) +} + +fn parse_sa_username(username: &str) -> Option<(String, String)> { + let rest = username.strip_prefix("system:serviceaccount:")?; + let (namespace, sa_name) = rest.split_once(':')?; + if namespace.is_empty() || sa_name.is_empty() { + return None; + } + Some((namespace.to_string(), sa_name.to_string())) } #[allow(clippy::result_large_err)] @@ -664,6 +713,10 @@ mod tests { cr } + fn exact_validator(ns: &str) -> NamespaceValidator { + NamespaceValidator::Exact(ns.to_string()) + } + #[test] fn token_review_identity_extracts_pod_binding() { let status = token_review_status( @@ -676,10 +729,12 @@ mod tests { ], ); - let identity = token_review_identity(&status, "openshell-gateway", "openshell", "default") + let validator = exact_validator("openshell"); + let identity = token_review_identity(&status, "openshell-gateway", &validator, "default") .unwrap() .expect("authenticated token should resolve"); + assert_eq!(identity.namespace, "openshell"); assert_eq!(identity.pod_name, "openshell-sandbox-a"); assert_eq!(identity.pod_uid, "uid-a"); } @@ -691,9 +746,10 @@ mod tests { error: Some("invalid audience".to_string()), ..Default::default() }; + let validator = exact_validator("openshell"); assert!( - token_review_identity(&status, "openshell-gateway", "openshell", "default") + token_review_identity(&status, "openshell-gateway", &validator, "default") .unwrap() .is_none() ); @@ -710,8 +766,9 @@ mod tests { (POD_UID_EXTRA, "uid-a"), ], ); + let validator = exact_validator("openshell"); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") + let err = token_review_identity(&status, "openshell-gateway", &validator, "default") .expect_err("wrong audience must fail closed"); assert_eq!(err.code(), tonic::Code::Unauthenticated); } @@ -727,8 +784,9 @@ mod tests { (POD_UID_EXTRA, "uid-a"), ], ); + let validator = exact_validator("openshell"); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") + let err = token_review_identity(&status, "openshell-gateway", &validator, "default") .expect_err("other namespace must be rejected"); assert_eq!(err.code(), tonic::Code::PermissionDenied); } @@ -744,8 +802,9 @@ mod tests { (POD_UID_EXTRA, "uid-a"), ], ); + let validator = exact_validator("openshell"); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") + let err = token_review_identity(&status, "openshell-gateway", &validator, "default") .expect_err("other service account must be rejected"); assert_eq!(err.code(), tonic::Code::PermissionDenied); } @@ -758,12 +817,71 @@ mod tests { "system:serviceaccount:openshell:default", vec![], ); + let validator = exact_validator("openshell"); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") + let err = token_review_identity(&status, "openshell-gateway", &validator, "default") .expect_err("non pod-bound tokens must be rejected"); assert_eq!(err.code(), tonic::Code::PermissionDenied); } + #[test] + fn namespace_validator_exact_accepts_matching() { + let v = NamespaceValidator::Exact("openshell".to_string()); + assert!(v.accepts("openshell")); + assert!(!v.accepts("other")); + } + + #[test] + fn namespace_validator_prefix_accepts_managed_namespaces() { + let v = NamespaceValidator::Prefix("openshell-gw1-".to_string()); + assert!(v.accepts("openshell-gw1-workspace-a")); + assert!(v.accepts("openshell-gw1-default")); + assert!(!v.accepts("openshell-gw2-workspace-a")); + assert!(!v.accepts("other")); + } + + #[test] + fn namespace_validator_allowlist_accepts_known_namespaces() { + let set = Arc::new(RwLock::new(BTreeSet::from([ + "ns-a".to_string(), + "ns-b".to_string(), + ]))); + let v = NamespaceValidator::Allowlist(set); + assert!(v.accepts("ns-a")); + assert!(v.accepts("ns-b")); + assert!(!v.accepts("ns-c")); + } + + #[test] + fn token_review_identity_prefix_validator_accepts_managed_namespace() { + let status = token_review_status( + true, + vec!["openshell-gateway"], + "system:serviceaccount:openshell-gw1-workspace-a:default", + vec![ + (POD_NAME_EXTRA, "openshell-sandbox-a"), + (POD_UID_EXTRA, "uid-a"), + ], + ); + let validator = NamespaceValidator::Prefix("openshell-gw1-".to_string()); + + let identity = token_review_identity(&status, "openshell-gateway", &validator, "default") + .unwrap() + .expect("managed namespace token should resolve"); + assert_eq!(identity.namespace, "openshell-gw1-workspace-a"); + } + + #[test] + fn parse_sa_username_extracts_namespace_and_sa() { + let (ns, sa) = parse_sa_username("system:serviceaccount:openshell:default").unwrap(); + assert_eq!(ns, "openshell"); + assert_eq!(sa, "default"); + + assert!(parse_sa_username("system:node:nodename").is_none()); + assert!(parse_sa_username("system:serviceaccount::default").is_none()); + assert!(parse_sa_username("system:serviceaccount:ns:").is_none()); + } + #[test] fn pod_sandbox_id_requires_annotation() { assert_eq!( diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 5cd06d3900..b669a457dc 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -454,13 +454,33 @@ pub(crate) async fn run_server( // namespace and service account used by the Kubernetes driver. let kubernetes_config = compute::driver_config::kubernetes_config_for_k8s_sa_bootstrap(config_file.as_ref())?; - let sandbox_namespace = kubernetes_config.namespace; - let sandbox_service_account = kubernetes_config.service_account_name; + let sandbox_namespace = kubernetes_config.namespace.clone(); + let sandbox_service_account = kubernetes_config.service_account_name.clone(); + let namespace_validator = match kubernetes_config.workspace_mode { + openshell_driver_kubernetes::WorkspaceMode::Shared => { + auth::k8s_sa::NamespaceValidator::Exact(kubernetes_config.namespace) + } + openshell_driver_kubernetes::WorkspaceMode::Managed => { + auth::k8s_sa::NamespaceValidator::Prefix( + openshell_driver_kubernetes::managed_namespace_prefix( + &kubernetes_config.gateway_id, + ), + ) + } + openshell_driver_kubernetes::WorkspaceMode::Operator => { + // The operator allowlist is populated at runtime by the label + // watcher and file watcher. An empty initial set is fail-closed + // until the watcher populates it. + auth::k8s_sa::NamespaceValidator::Allowlist(Arc::new(std::sync::RwLock::new( + std::collections::BTreeSet::new(), + ))) + } + }; match kube::Client::try_default().await { Ok(client) => { let resolver = Arc::new(auth::k8s_sa::LiveK8sResolver::new( client, - &sandbox_namespace, + namespace_validator, "openshell-gateway".to_string(), sandbox_service_account.clone(), )); diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 7096a8ca74..19e26562ed 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -225,6 +225,9 @@ add `ci/values-spire.yaml` to the OpenShell release values files. | server.dbUrl | string | `"sqlite:/var/openshell/openshell.db"` | Gateway database URL (used for the default SQLite backend). | | server.defaultRuntimeClassName | string | `""` | Default Kubernetes runtimeClassName for sandbox pods. Applied when a CreateSandbox request does not specify one. Empty (default) = omit the field, using the cluster's default RuntimeClass. Set to a RuntimeClass name (e.g. "kata-containers", "nvidia") to apply it to all sandboxes that don't explicitly override it. | | server.disableTls | bool | `false` | Disable TLS entirely - the server listens on plaintext HTTP. Set to true when a reverse proxy / tunnel terminates TLS at the edge. | +| server.drivers.kubernetes.operatorNamespaceFile | operator mode | `""` | Path to a drop-in JSON file mapping workspace names to namespace names. Hot-reloaded on change. | +| server.drivers.kubernetes.operatorNamespaceLabel | operator mode | `""` | K8s label selector for namespace discovery. The driver watches namespaces matching this label. | +| server.drivers.kubernetes.workspaceMode | string | `"shared"` | How workspaces map to Kubernetes namespaces. "shared" (default): all sandboxes in a single namespace. "managed": auto-creates per-workspace namespaces. "operator": uses pre-provisioned namespaces. | | server.enableLoopbackServiceHttp | bool | `true` | Enable plaintext HTTP routing for loopback sandbox service URLs on TLS-enabled gateways. | | server.enableUserNamespaces | bool | `false` | Enable Kubernetes user namespace isolation (hostUsers: false) for sandbox pods. Requires Kubernetes 1.33+ with user namespace support available (beta through 1.35, GA in 1.36+), plus a supporting container runtime and Linux 5.12+. When enabled, container UID 0 maps to an unprivileged host UID and capabilities become namespaced. | | server.externalDbSecret | string | `""` | Name of a pre-existing Opaque Secret containing a PostgreSQL connection URI (key: uri). When set, the gateway reads OPENSHELL_DB_URL from this Secret instead of using dbUrl. The Secret must contain a `uri` key, e.g. postgresql://user:pass@host:5432/dbname. | diff --git a/deploy/helm/openshell/templates/clusterrole.yaml b/deploy/helm/openshell/templates/clusterrole.yaml index 073c8835ec..2acfaa2dad 100644 --- a/deploy/helm/openshell/templates/clusterrole.yaml +++ b/deploy/helm/openshell/templates/clusterrole.yaml @@ -1,6 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: @@ -25,9 +26,60 @@ rules: - list - watch # Read namespace annotations for OpenShift SCC UID/GID range resolution. + # Managed/operator modes additionally need list+watch for cluster-wide + # namespace discovery. Managed mode needs create+delete for namespace + # lifecycle. - apiGroups: - "" resources: - namespaces verbs: - get + {{- if ne $workspaceMode "shared" }} + - list + - watch + {{- end }} + {{- if eq $workspaceMode "managed" }} + - create + - delete + {{- end }} + {{- if ne $workspaceMode "shared" }} + # Cluster-wide sandbox CRD access for managed/operator workspace modes. + - apiGroups: + - agents.x-k8s.io + resources: + - sandboxes + - sandboxes/status + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - "" + resources: + - events + verbs: + - get + - list + - watch + - apiGroups: + - "" + resources: + - pods + verbs: + - get + {{- end }} + {{- if eq $workspaceMode "managed" }} + # ServiceAccount creation in managed namespaces. + - apiGroups: + - "" + resources: + - serviceaccounts + verbs: + - create + - get + {{- end }} diff --git a/deploy/helm/openshell/templates/gateway-config.yaml b/deploy/helm/openshell/templates/gateway-config.yaml index e22b5e7485..454affd0d3 100644 --- a/deploy/helm/openshell/templates/gateway-config.yaml +++ b/deploy/helm/openshell/templates/gateway-config.yaml @@ -127,8 +127,16 @@ data: {{- end }} [openshell.drivers.kubernetes] + workspace_mode = {{ .Values.server.drivers.kubernetes.workspaceMode | default "shared" | quote }} + gateway_id = {{ .Values.server.sandboxJwt.gatewayId | default (include "openshell.fullname" .) | quote }} grpc_endpoint = {{ include "openshell.grpcEndpoint" . | quote }} service_account_name = {{ include "openshell.sandboxServiceAccountName" . | quote }} + {{- if .Values.server.drivers.kubernetes.operatorNamespaceLabel }} + operator_namespace_label = {{ .Values.server.drivers.kubernetes.operatorNamespaceLabel | quote }} + {{- end }} + {{- if .Values.server.drivers.kubernetes.operatorNamespaceFile }} + operator_namespace_file = {{ .Values.server.drivers.kubernetes.operatorNamespaceFile | quote }} + {{- end }} supervisor_sideload_method = {{ include "openshell.supervisorSideloadMethod" . | quote }} topology = {{ .Values.supervisor.topology | default "combined" | quote }} sa_token_ttl_secs = {{ .Values.server.sandboxJwt.k8sSaTokenTtlSecs | default 3600 }} @@ -178,6 +186,8 @@ data: [openshell.credential_drivers.kubernetes-secrets] namespace = {{ include "openshell.credentialKubernetesSecretsNamespace" . | quote }} allow_reference_namespace = {{ .Values.server.credentialDrivers.kubernetesSecrets.allowReferenceNamespace }} + workspace_mode = {{ .Values.server.drivers.kubernetes.workspaceMode | default "shared" | quote }} + gateway_id = {{ .Values.server.sandboxJwt.gatewayId | default (include "openshell.fullname" .) | quote }} {{- end }} {{- if .Values.server.credentialDrivers.vault.enabled }} diff --git a/deploy/helm/openshell/templates/role.yaml b/deploy/helm/openshell/templates/role.yaml index 5ecc4428ad..4ccc5e3d96 100644 --- a/deploy/helm/openshell/templates/role.yaml +++ b/deploy/helm/openshell/templates/role.yaml @@ -1,6 +1,8 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} +{{- if eq $workspaceMode "shared" }} apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: @@ -42,3 +44,4 @@ rules: - pods verbs: - get +{{- end }} diff --git a/deploy/helm/openshell/templates/rolebinding.yaml b/deploy/helm/openshell/templates/rolebinding.yaml index e5233f753c..9bf7c73fab 100644 --- a/deploy/helm/openshell/templates/rolebinding.yaml +++ b/deploy/helm/openshell/templates/rolebinding.yaml @@ -1,6 +1,8 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 +{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} +{{- if eq $workspaceMode "shared" }} apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: @@ -16,3 +18,4 @@ subjects: - kind: ServiceAccount name: {{ include "openshell.serviceAccountName" . }} namespace: {{ .Release.Namespace }} +{{- end }} diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index 39205df1bf..ab13c55632 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -223,6 +223,20 @@ server: # the field, "RuntimeDefault" to force the runtime default profile, or # "Localhost/profile-name" for an operator-managed localhost profile. appArmorProfile: "Unconfined" + # Kubernetes compute driver settings. + drivers: + kubernetes: + # -- How workspaces map to Kubernetes namespaces. + # "shared" (default): all sandboxes in a single namespace. + # "managed": auto-creates per-workspace namespaces. + # "operator": uses pre-provisioned namespaces. + workspaceMode: "shared" + # -- (operator mode) K8s label selector for namespace discovery. + # The driver watches namespaces matching this label. + operatorNamespaceLabel: "" + # -- (operator mode) Path to a drop-in JSON file mapping workspace + # names to namespace names. Hot-reloaded on change. + operatorNamespaceFile: "" # -- Disable TLS entirely - the server listens on plaintext HTTP. # Set to true when a reverse proxy / tunnel terminates TLS at the edge. disableTls: false diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 2cd10b8a0b..b201b6f198 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -413,6 +413,14 @@ key_path = "/etc/openshell-tls/server/tls.key" client_ca_path = "/etc/openshell-tls/client-ca/ca.crt" [openshell.drivers.kubernetes] +# Workspace isolation mode. "shared" renders all sandboxes into a single +# namespace. "managed" auto-creates a K8s namespace per workspace +# (openshell-{gateway_id}-{workspace}). "operator" maps each workspace to a +# pre-provisioned namespace discovered via label selector or drop-in file. +workspace_mode = "shared" +# Gateway identity used in managed-mode namespace naming. Defaults to the +# gateway JWT gateway_id. Must be a DNS-1123 label. +# gateway_id = "openshell" namespace = "agents" service_account_name = "openshell-sandbox" default_image = "ghcr.io/nvidia/openshell/sandbox:latest" @@ -455,6 +463,13 @@ provider_spiffe_workload_api_socket_path = "/spiffe-workload-api/spire-agent.soc # back to 1000 on non-OpenShift clusters. # sandbox_uid = 1500 # sandbox_gid = 1500 +# Operator-mode namespace discovery. At least one must be set when +# workspace_mode = "operator". Both can be combined. +# operator_namespace_label discovers namespaces matching a K8s label selector. +# operator_namespace_label = "openshell.ai/workspace=true" +# operator_namespace_file reads allowed namespaces from a JSON/YAML file +# (hot-reloaded on change, e.g. via ConfigMap volume mount). +# operator_namespace_file = "/etc/openshell/workspace-namespaces.json" [openshell.drivers.kubernetes.sidecar] # UID used by relaxed long-running network sidecars. Strict process/binary-aware From 485c65f491999c28baca6922d941550deb12f1de Mon Sep 17 00:00:00 2001 From: Derek Carr Date: Sat, 8 Aug 2026 13:32:15 -0400 Subject: [PATCH 2/4] test(k8s): add e2e tests for workspace namespace modes Add end-to-end tests for managed and operator workspace modes introduced in RFC 0011 Phase 3. The managed mode tests verify namespace creation with correct labels, ServiceAccount provisioning, sandbox CR placement, and namespace survival with remaining sandboxes. The operator mode tests verify rejection of unlabeled and nonexistent namespaces. The positive operator path (sandbox in labeled namespace) is known to fail due to an RBAC gap and will be addressed separately. Also fixes Helm 4 compatibility: move SPDX license headers inside conditional guards in 8 chart templates to prevent empty comment-only documents, and fix a trailing whitespace trimmer in clusterrole.yaml that concatenated the license header with apiVersion. Adds cleanup sweep in with-kube-gateway.sh to remove managed and operator namespaces before Helm uninstall, and mise tasks for running each mode independently. Signed-off-by: Derek Carr --- .../ci/values-workspace-managed.yaml | 9 + .../ci/values-workspace-operator.yaml | 10 + .../openshell/templates/cert-manager-pki.yaml | 3 +- .../helm/openshell/templates/clusterrole.yaml | 2 +- .../templates/credential-secrets-role.yaml | 3 +- .../credential-secrets-rolebinding.yaml | 3 +- .../helm/openshell/templates/deployment.yaml | 4 +- deploy/helm/openshell/templates/gateway.yaml | 3 +- .../helm/openshell/templates/grpcroute.yaml | 3 +- deploy/helm/openshell/templates/role.yaml | 5 +- .../helm/openshell/templates/rolebinding.yaml | 5 +- e2e/rust/Cargo.toml | 12 + e2e/rust/tests/workspace_namespace_managed.rs | 301 ++++++++++++++++++ .../tests/workspace_namespace_operator.rs | 263 +++++++++++++++ e2e/with-kube-gateway.sh | 16 + tasks/test.toml | 10 + 16 files changed, 633 insertions(+), 19 deletions(-) create mode 100644 deploy/helm/openshell/ci/values-workspace-managed.yaml create mode 100644 deploy/helm/openshell/ci/values-workspace-operator.yaml create mode 100644 e2e/rust/tests/workspace_namespace_managed.rs create mode 100644 e2e/rust/tests/workspace_namespace_operator.rs diff --git a/deploy/helm/openshell/ci/values-workspace-managed.yaml b/deploy/helm/openshell/ci/values-workspace-managed.yaml new file mode 100644 index 0000000000..9b8911fbe7 --- /dev/null +++ b/deploy/helm/openshell/ci/values-workspace-managed.yaml @@ -0,0 +1,9 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# E2E overlay: deploy the gateway in managed workspace mode. +# Sandbox namespaces are auto-created as openshell-{gateway_id}-{workspace}. +server: + drivers: + kubernetes: + workspaceMode: "managed" diff --git a/deploy/helm/openshell/ci/values-workspace-operator.yaml b/deploy/helm/openshell/ci/values-workspace-operator.yaml new file mode 100644 index 0000000000..8d895e4e98 --- /dev/null +++ b/deploy/helm/openshell/ci/values-workspace-operator.yaml @@ -0,0 +1,10 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# E2E overlay: deploy the gateway in operator workspace mode. +# Namespaces must be pre-provisioned and labeled before sandbox creation. +server: + drivers: + kubernetes: + workspaceMode: "operator" + operatorNamespaceLabel: "openshell.ai/e2e-operator-workspace=true" diff --git a/deploy/helm/openshell/templates/cert-manager-pki.yaml b/deploy/helm/openshell/templates/cert-manager-pki.yaml index fdd702a305..5f9e5f36f1 100644 --- a/deploy/helm/openshell/templates/cert-manager-pki.yaml +++ b/deploy/helm/openshell/templates/cert-manager-pki.yaml @@ -1,7 +1,6 @@ +{{- if .Values.certManager.enabled }} # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 - -{{- if .Values.certManager.enabled }} apiVersion: cert-manager.io/v1 kind: Issuer metadata: diff --git a/deploy/helm/openshell/templates/clusterrole.yaml b/deploy/helm/openshell/templates/clusterrole.yaml index 2acfaa2dad..66102ab751 100644 --- a/deploy/helm/openshell/templates/clusterrole.yaml +++ b/deploy/helm/openshell/templates/clusterrole.yaml @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 -{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} +{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" }} apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: diff --git a/deploy/helm/openshell/templates/credential-secrets-role.yaml b/deploy/helm/openshell/templates/credential-secrets-role.yaml index 72f0528cb2..f6187c9acb 100644 --- a/deploy/helm/openshell/templates/credential-secrets-role.yaml +++ b/deploy/helm/openshell/templates/credential-secrets-role.yaml @@ -1,7 +1,6 @@ +{{- if and .Values.server.credentialDrivers.kubernetesSecrets.enabled .Values.server.credentialDrivers.kubernetesSecrets.rbac.create }} # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 - -{{- if and .Values.server.credentialDrivers.kubernetesSecrets.enabled .Values.server.credentialDrivers.kubernetesSecrets.rbac.create }} apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: diff --git a/deploy/helm/openshell/templates/credential-secrets-rolebinding.yaml b/deploy/helm/openshell/templates/credential-secrets-rolebinding.yaml index 4274fa6e1a..3a9ee0bddc 100644 --- a/deploy/helm/openshell/templates/credential-secrets-rolebinding.yaml +++ b/deploy/helm/openshell/templates/credential-secrets-rolebinding.yaml @@ -1,7 +1,6 @@ +{{- if and .Values.server.credentialDrivers.kubernetesSecrets.enabled .Values.server.credentialDrivers.kubernetesSecrets.rbac.create }} # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 - -{{- if and .Values.server.credentialDrivers.kubernetesSecrets.enabled .Values.server.credentialDrivers.kubernetesSecrets.rbac.create }} apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: diff --git a/deploy/helm/openshell/templates/deployment.yaml b/deploy/helm/openshell/templates/deployment.yaml index e937979370..f94900b136 100644 --- a/deploy/helm/openshell/templates/deployment.yaml +++ b/deploy/helm/openshell/templates/deployment.yaml @@ -1,7 +1,7 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 {{- include "openshell.validateValues" . }} {{- if eq (include "openshell.workloadKind" .) "deployment" }} +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 apiVersion: apps/v1 kind: Deployment metadata: diff --git a/deploy/helm/openshell/templates/gateway.yaml b/deploy/helm/openshell/templates/gateway.yaml index f431ffbbd1..2b78595053 100644 --- a/deploy/helm/openshell/templates/gateway.yaml +++ b/deploy/helm/openshell/templates/gateway.yaml @@ -1,7 +1,6 @@ +{{- if and .Values.grpcRoute.enabled .Values.grpcRoute.gateway.create }} # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 - -{{- if and .Values.grpcRoute.enabled .Values.grpcRoute.gateway.create }} apiVersion: gateway.networking.k8s.io/v1 kind: Gateway metadata: diff --git a/deploy/helm/openshell/templates/grpcroute.yaml b/deploy/helm/openshell/templates/grpcroute.yaml index 8fde5458cd..362067fda3 100644 --- a/deploy/helm/openshell/templates/grpcroute.yaml +++ b/deploy/helm/openshell/templates/grpcroute.yaml @@ -1,7 +1,6 @@ +{{- if .Values.grpcRoute.enabled }} # SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 - -{{- if .Values.grpcRoute.enabled }} apiVersion: gateway.networking.k8s.io/v1 kind: GRPCRoute metadata: diff --git a/deploy/helm/openshell/templates/role.yaml b/deploy/helm/openshell/templates/role.yaml index 4ccc5e3d96..af80989072 100644 --- a/deploy/helm/openshell/templates/role.yaml +++ b/deploy/helm/openshell/templates/role.yaml @@ -1,8 +1,7 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - {{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} {{- if eq $workspaceMode "shared" }} +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 apiVersion: rbac.authorization.k8s.io/v1 kind: Role metadata: diff --git a/deploy/helm/openshell/templates/rolebinding.yaml b/deploy/helm/openshell/templates/rolebinding.yaml index 9bf7c73fab..381473a58b 100644 --- a/deploy/helm/openshell/templates/rolebinding.yaml +++ b/deploy/helm/openshell/templates/rolebinding.yaml @@ -1,8 +1,7 @@ -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - {{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} {{- if eq $workspaceMode "shared" }} +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index 3353f07af7..a65d452110 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -29,6 +29,8 @@ e2e-gpu = ["e2e"] e2e-docker-gpu = ["e2e-docker", "e2e-gpu"] e2e-kubernetes = ["e2e"] e2e-kubernetes-credential-drivers = ["e2e-kubernetes"] +e2e-kubernetes-workspace-managed = ["e2e-kubernetes"] +e2e-kubernetes-workspace-operator = ["e2e-kubernetes"] e2e-podman = ["e2e", "e2e-host-gateway", "e2e-local-container-driver"] e2e-podman-gpu = ["e2e-podman", "e2e-gpu"] e2e-oidc-pkce = [] @@ -134,6 +136,16 @@ name = "proxy_egress_pipeline" path = "tests/proxy_egress_pipeline.rs" required-features = ["e2e-host-gateway"] +[[test]] +name = "workspace_namespace_managed" +path = "tests/workspace_namespace_managed.rs" +required-features = ["e2e-kubernetes-workspace-managed"] + +[[test]] +name = "workspace_namespace_operator" +path = "tests/workspace_namespace_operator.rs" +required-features = ["e2e-kubernetes-workspace-operator"] + [[test]] name = "gpu" path = "tests/gpu.rs" diff --git a/e2e/rust/tests/workspace_namespace_managed.rs b/e2e/rust/tests/workspace_namespace_managed.rs new file mode 100644 index 0000000000..4bfc793a23 --- /dev/null +++ b/e2e/rust/tests/workspace_namespace_managed.rs @@ -0,0 +1,301 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-kubernetes-workspace-managed")] + +//! E2E tests for managed workspace mode. +//! +//! The gateway is deployed with `workspace_mode = "managed"`, which +//! auto-creates a K8s namespace per workspace (`openshell-{gateway_id}-{ws}`) +//! and deletes it when the last sandbox is removed. +//! +//! Namespace cleanup after sandbox deletion is best-effort and depends on +//! controller finalization timing. These tests focus on verifiable behavior: +//! namespace creation, labels, ServiceAccount provisioning, and sandbox CR +//! placement in the correct namespace. + +use std::process::Stdio; +use std::time::Duration; + +use openshell_e2e::harness::binary::{openshell_bin, openshell_cmd}; +use openshell_e2e::harness::output::strip_ansi; + +fn kube_context() -> String { + std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE") + .expect("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE must be set") +} + +async fn kubectl(args: &[&str]) -> (bool, String) { + let context = kube_context(); + let output = tokio::process::Command::new("kubectl") + .arg("--context") + .arg(&context) + .args(args) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .await + .expect("failed to spawn kubectl"); + + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + (output.status.success(), combined) +} + +fn managed_namespace(workspace: &str) -> String { + format!("openshell-openshell-{workspace}") +} + +async fn run_cli(args: &[&str]) -> (bool, String) { + let mut cmd = openshell_cmd(); + cmd.args(args).stdout(Stdio::piped()).stderr(Stdio::piped()); + let output = cmd.output().await.expect("failed to spawn openshell"); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + (output.status.success(), strip_ansi(&combined)) +} + +fn unique_workspace(prefix: &str) -> String { + let ts = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_millis() + % 100_000; + format!("{prefix}-{ts}") +} + +struct ManagedCleanup { + workspace: String, + sandboxes: Vec, +} + +impl Drop for ManagedCleanup { + fn drop(&mut self) { + let bin = openshell_bin(); + for sb in &self.sandboxes { + let _ = std::process::Command::new(&bin) + .args(["sandbox", "delete", sb, "--workspace", &self.workspace]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + } + let _ = std::process::Command::new(&bin) + .args(["workspace", "delete", &self.workspace]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + let context = std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE").unwrap_or_default(); + if !context.is_empty() { + let ns = managed_namespace(&self.workspace); + let _ = std::process::Command::new("kubectl") + .args([ + "--context", + &context, + "delete", + "namespace", + &ns, + "--ignore-not-found", + "--wait=false", + ]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + } + } +} + +#[tokio::test] +async fn managed_creates_namespace_with_labels() { + let ws = unique_workspace("mgd"); + let ns = managed_namespace(&ws); + let _cleanup = ManagedCleanup { + workspace: ws.clone(), + sandboxes: vec!["mgd-sb".into()], + }; + + let (ok, out) = run_cli(&["workspace", "create", "--name", &ws]).await; + assert!(ok, "workspace create failed: {out}"); + + // Create a sandbox — this triggers namespace creation. + let (ok, out) = run_cli(&[ + "sandbox", + "create", + "--workspace", + &ws, + "--name", + "mgd-sb", + "--", + "echo", + "managed-ok", + ]) + .await; + assert!(ok, "sandbox create failed: {out}"); + assert!( + out.contains("managed-ok"), + "sandbox output missing expected string: {out}" + ); + + // Verify the managed namespace was created. + let (ok, out) = kubectl(&["get", "namespace", &ns]).await; + assert!(ok, "managed namespace {ns} should exist: {out}"); + + // Verify labels on the namespace. + let (ok, label_out) = + kubectl(&["get", "namespace", &ns, "-o", "jsonpath={.metadata.labels}"]).await; + assert!(ok, "failed to read namespace labels: {label_out}"); + assert!( + label_out.contains("openshell.ai/managed-by"), + "namespace missing managed-by label: {label_out}" + ); + assert!( + label_out.contains("openshell.ai/gateway-id"), + "namespace missing gateway-id label: {label_out}" + ); + + // Verify the ServiceAccount was created in the managed namespace. + let (ok, _) = kubectl(&["get", "serviceaccount", "openshell-sandbox", "-n", &ns]).await; + assert!(ok, "ServiceAccount openshell-sandbox should exist in {ns}"); + + // Verify sandbox CR is in the managed namespace (not the gateway namespace). + let (ok, out) = kubectl(&[ + "get", + "sandbox.agents.x-k8s.io", + "-n", + &ns, + "-o", + "name", + ]) + .await; + assert!(ok, "sandbox CR should exist in namespace {ns}: {out}"); + assert!( + out.contains("mgd-sb"), + "sandbox CR name mismatch: {out}" + ); +} + +#[tokio::test] +async fn managed_namespace_survives_with_remaining_sandboxes() { + let ws = unique_workspace("mgd2"); + let ns = managed_namespace(&ws); + let _cleanup = ManagedCleanup { + workspace: ws.clone(), + sandboxes: vec!["sb-a".into(), "sb-b".into()], + }; + + let (ok, out) = run_cli(&["workspace", "create", "--name", &ws]).await; + assert!(ok, "workspace create failed: {out}"); + + // Create two sandboxes. + let (ok, out) = run_cli(&[ + "sandbox", "create", "--workspace", &ws, "--name", "sb-a", "--", "echo", "a", + ]) + .await; + assert!(ok, "sandbox sb-a create failed: {out}"); + + let (ok, out) = run_cli(&[ + "sandbox", "create", "--workspace", &ws, "--name", "sb-b", "--", "echo", "b", + ]) + .await; + assert!(ok, "sandbox sb-b create failed: {out}"); + + // Delete first sandbox — namespace should survive because sb-b still exists. + let (ok, out) = run_cli(&["sandbox", "delete", "sb-a", "--workspace", &ws]).await; + assert!(ok, "sandbox sb-a delete failed: {out}"); + + // Brief wait, then verify the namespace still exists. + tokio::time::sleep(Duration::from_secs(3)).await; + + let (ok, _) = kubectl(&["get", "namespace", &ns]).await; + assert!(ok, "managed namespace {ns} should still exist with sb-b"); + + // Verify sb-b's CR is still in the managed namespace. + let (ok, out) = kubectl(&[ + "get", + "sandbox.agents.x-k8s.io", + "-n", + &ns, + "-o", + "name", + ]) + .await; + assert!(ok, "sandbox CRs should still exist in {ns}: {out}"); + assert!( + out.contains("sb-b"), + "sb-b CR should still be present: {out}" + ); +} + +#[tokio::test] +async fn managed_isolates_workspaces_into_separate_namespaces() { + let ws_a = unique_workspace("iso-a"); + let ws_b = unique_workspace("iso-b"); + let ns_a = managed_namespace(&ws_a); + let ns_b = managed_namespace(&ws_b); + let _cleanup_a = ManagedCleanup { + workspace: ws_a.clone(), + sandboxes: vec!["sb-iso-a".into()], + }; + let _cleanup_b = ManagedCleanup { + workspace: ws_b.clone(), + sandboxes: vec!["sb-iso-b".into()], + }; + + // Create two workspaces with sandboxes. + let (ok, out) = run_cli(&["workspace", "create", "--name", &ws_a]).await; + assert!(ok, "workspace A create failed: {out}"); + let (ok, out) = run_cli(&["workspace", "create", "--name", &ws_b]).await; + assert!(ok, "workspace B create failed: {out}"); + + let (ok, out) = run_cli(&[ + "sandbox", "create", "--workspace", &ws_a, "--name", "sb-iso-a", "--", "echo", "a", + ]) + .await; + assert!(ok, "sandbox A create failed: {out}"); + + let (ok, out) = run_cli(&[ + "sandbox", "create", "--workspace", &ws_b, "--name", "sb-iso-b", "--", "echo", "b", + ]) + .await; + assert!(ok, "sandbox B create failed: {out}"); + + // Verify each workspace has its own namespace. + assert_ne!(ns_a, ns_b, "namespaces should differ"); + + let (ok, _) = kubectl(&["get", "namespace", &ns_a]).await; + assert!(ok, "namespace {ns_a} should exist"); + let (ok, _) = kubectl(&["get", "namespace", &ns_b]).await; + assert!(ok, "namespace {ns_b} should exist"); + + // Verify sandbox CRs are in the correct namespaces (no cross-contamination). + let (ok, out) = kubectl(&[ + "get", + "sandbox.agents.x-k8s.io", + "-n", + &ns_a, + "-o", + "name", + ]) + .await; + assert!(ok, "failed to list CRs in {ns_a}: {out}"); + assert!(out.contains("sb-iso-a"), "sb-iso-a should be in {ns_a}"); + assert!(!out.contains("sb-iso-b"), "sb-iso-b should NOT be in {ns_a}"); + + let (ok, out) = kubectl(&[ + "get", + "sandbox.agents.x-k8s.io", + "-n", + &ns_b, + "-o", + "name", + ]) + .await; + assert!(ok, "failed to list CRs in {ns_b}: {out}"); + assert!(out.contains("sb-iso-b"), "sb-iso-b should be in {ns_b}"); + assert!(!out.contains("sb-iso-a"), "sb-iso-a should NOT be in {ns_b}"); +} diff --git a/e2e/rust/tests/workspace_namespace_operator.rs b/e2e/rust/tests/workspace_namespace_operator.rs new file mode 100644 index 0000000000..172414e100 --- /dev/null +++ b/e2e/rust/tests/workspace_namespace_operator.rs @@ -0,0 +1,263 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-kubernetes-workspace-operator")] + +//! E2E tests for operator workspace mode. +//! +//! The gateway is deployed with `workspace_mode = "operator"` and +//! `operator_namespace_label = "openshell.ai/e2e-operator-workspace=true"`. +//! Namespaces must be pre-provisioned and labeled before sandbox creation. +//! The gateway discovers valid namespaces via the label selector. + +use std::process::Stdio; +use std::time::Duration; + +use openshell_e2e::harness::binary::{openshell_bin, openshell_cmd}; +use openshell_e2e::harness::output::strip_ansi; + +const OPERATOR_LABEL: &str = "openshell.ai/e2e-operator-workspace=true"; +const SA_NAME: &str = "openshell-sandbox"; + +fn kube_context() -> String { + std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE") + .expect("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE must be set") +} + +async fn kubectl(args: &[&str]) -> (bool, String) { + let context = kube_context(); + let output = tokio::process::Command::new("kubectl") + .arg("--context") + .arg(&context) + .args(args) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output() + .await + .expect("failed to spawn kubectl"); + + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + (output.status.success(), combined) +} + +async fn run_cli(args: &[&str]) -> (bool, String) { + let mut cmd = openshell_cmd(); + cmd.args(args).stdout(Stdio::piped()).stderr(Stdio::piped()); + let output = cmd.output().await.expect("failed to spawn openshell"); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + (output.status.success(), strip_ansi(&combined)) +} + +fn unique_namespace(prefix: &str) -> String { + let ts = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_millis() + % 100_000; + format!("{prefix}-{ts}") +} + +async fn provision_operator_namespace(name: &str) { + let (ok, out) = kubectl(&["create", "namespace", name]).await; + assert!(ok, "failed to create namespace {name}: {out}"); + + let (ok, out) = kubectl(&["label", "namespace", name, OPERATOR_LABEL]).await; + assert!(ok, "failed to label namespace {name}: {out}"); + + let (ok, out) = kubectl(&["create", "serviceaccount", SA_NAME, "-n", name]).await; + assert!(ok, "failed to create SA in {name}: {out}"); +} + +async fn delete_namespace(name: &str) { + let _ = kubectl(&[ + "delete", + "namespace", + name, + "--ignore-not-found", + "--wait=false", + ]) + .await; +} + +struct OperatorCleanup { + workspace: String, + namespace: String, + sandboxes: Vec, +} + +impl Drop for OperatorCleanup { + fn drop(&mut self) { + let bin = openshell_bin(); + for sb in &self.sandboxes { + let _ = std::process::Command::new(&bin) + .args(["sandbox", "delete", sb, "--workspace", &self.workspace]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + } + let _ = std::process::Command::new(&bin) + .args(["workspace", "delete", &self.workspace]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + let context = std::env::var("OPENSHELL_E2E_KUBE_CONTEXT_ACTIVE").unwrap_or_default(); + if !context.is_empty() { + let _ = std::process::Command::new("kubectl") + .args([ + "--context", + &context, + "delete", + "namespace", + &self.namespace, + "--ignore-not-found", + "--wait=false", + ]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); + } + } +} + +#[tokio::test] +async fn operator_sandbox_in_labeled_namespace() { + let ns = unique_namespace("op"); + let _cleanup = OperatorCleanup { + workspace: ns.clone(), + namespace: ns.clone(), + sandboxes: vec!["op-sb".into()], + }; + + // Pre-provision the namespace with the operator label and ServiceAccount. + provision_operator_namespace(&ns).await; + + // Wait for the gateway's namespace watcher to discover it. + tokio::time::sleep(Duration::from_secs(5)).await; + + // Create a workspace matching the namespace name (operator mode: 1:1 mapping). + let (ok, out) = run_cli(&["workspace", "create", "--name", &ns]).await; + assert!(ok, "workspace create failed: {out}"); + + // Create a sandbox in the workspace. + let (ok, out) = run_cli(&[ + "sandbox", + "create", + "--workspace", + &ns, + "--name", + "op-sb", + "--", + "echo", + "operator-ok", + ]) + .await; + assert!(ok, "sandbox create failed: {out}"); + assert!( + out.contains("operator-ok"), + "sandbox output missing expected string: {out}" + ); + + // Verify the sandbox CR lives in the pre-provisioned namespace. + let (ok, out) = kubectl(&["get", "sandbox.agents.x-k8s.io", "-n", &ns, "-o", "name"]).await; + assert!(ok, "sandbox CR should exist in namespace {ns}: {out}"); + assert!( + out.contains("op-sb"), + "sandbox CR name should be bare 'op-sb', got: {out}" + ); + + // Clean up. + let (ok, out) = run_cli(&["sandbox", "delete", "op-sb", "--workspace", &ns]).await; + assert!(ok, "sandbox delete failed: {out}"); + + let (ok, out) = run_cli(&["workspace", "delete", &ns]).await; + assert!(ok, "workspace delete failed: {out}"); + + delete_namespace(&ns).await; +} + +#[tokio::test] +async fn operator_rejects_unlabeled_namespace() { + let ns = unique_namespace("opun"); + let _cleanup = OperatorCleanup { + workspace: ns.clone(), + namespace: ns.clone(), + sandboxes: vec![], + }; + + // Create namespace WITHOUT the operator label. + let (ok, out) = kubectl(&["create", "namespace", &ns]).await; + assert!(ok, "failed to create namespace: {out}"); + + // Create the ServiceAccount (not the label — that's the point). + let (ok, _) = kubectl(&["create", "serviceaccount", SA_NAME, "-n", &ns]).await; + assert!(ok, "failed to create SA"); + + // Create workspace. + let (ok, out) = run_cli(&["workspace", "create", "--name", &ns]).await; + assert!(ok, "workspace create failed: {out}"); + + // Attempt sandbox creation — should fail because namespace is not in the allowlist. + let (ok, out) = run_cli(&[ + "sandbox", + "create", + "--workspace", + &ns, + "--name", + "should-fail", + "--", + "echo", + "nope", + ]) + .await; + assert!( + !ok, + "sandbox create should fail for unlabeled namespace, but succeeded: {out}" + ); + + // Clean up. + let _ = run_cli(&["workspace", "delete", &ns]).await; + delete_namespace(&ns).await; +} + +#[tokio::test] +async fn operator_rejects_nonexistent_namespace() { + let ns = unique_namespace("opne"); + let _cleanup = OperatorCleanup { + workspace: ns.clone(), + namespace: ns.clone(), + sandboxes: vec![], + }; + + // Create workspace with no matching namespace at all. + let (ok, out) = run_cli(&["workspace", "create", "--name", &ns]).await; + assert!(ok, "workspace create failed: {out}"); + + // Attempt sandbox creation — should fail. + let (ok, out) = run_cli(&[ + "sandbox", + "create", + "--workspace", + &ns, + "--name", + "should-fail", + "--", + "echo", + "nope", + ]) + .await; + assert!( + !ok, + "sandbox create should fail for nonexistent namespace, but succeeded: {out}" + ); + + // Clean up. + let _ = run_cli(&["workspace", "delete", &ns]).await; +} diff --git a/e2e/with-kube-gateway.sh b/e2e/with-kube-gateway.sh index cde230daaf..b8a7621a12 100755 --- a/e2e/with-kube-gateway.sh +++ b/e2e/with-kube-gateway.sh @@ -270,6 +270,22 @@ cleanup() { cleanup_vault_fixture fi + # Sweep managed-mode and operator-mode workspace namespaces before + # uninstalling the Helm release (ClusterRole still needed for deletion). + if command -v kubectl >/dev/null 2>&1 && [ -n "${KUBE_CONTEXT}" ]; then + for label in "openshell.ai/managed-by=openshell" \ + "openshell.ai/e2e-operator-workspace=true"; do + ns_list="$(kctl get namespaces -l "${label}" -o name 2>/dev/null || true)" + if [ -n "${ns_list}" ]; then + echo "Cleaning up namespaces with label ${label}..." + echo "${ns_list}" | while read -r ns_ref; do + kctl delete "${ns_ref}" --wait=false --ignore-not-found \ + 2>/dev/null || true + done + fi + done + fi + if [ "${HELM_INSTALLED}" = "1" ] && [ -n "${KUBE_CONTEXT}" ] && [ -n "${NAMESPACE}" ]; then if command -v helm >/dev/null 2>&1; then helmctl uninstall "${RELEASE_NAME}" --namespace "${NAMESPACE}" --wait \ diff --git a/tasks/test.toml b/tasks/test.toml index ed0d17d7af..df409c27d8 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -156,6 +156,16 @@ description = "Run Kubernetes e2e for provider credential storage backed by Kube env = { OPENSHELL_E2E_CREDENTIAL_DRIVERS = "1", OPENSHELL_E2E_KUBE_TEST = "credential_drivers", OPENSHELL_E2E_KUBERNETES_FEATURES = "e2e,e2e-kubernetes,e2e-kubernetes-credential-drivers" } run = "e2e/rust/e2e-kubernetes.sh" +["e2e:kubernetes:workspace-managed"] +description = "Run Kubernetes e2e with managed workspace mode (auto-created per-workspace namespaces)" +env = { OPENSHELL_E2E_KUBE_EXTRA_VALUES = "deploy/helm/openshell/ci/values-workspace-managed.yaml", OPENSHELL_E2E_KUBE_TEST = "workspace_namespace_managed", OPENSHELL_E2E_KUBERNETES_FEATURES = "e2e,e2e-kubernetes,e2e-kubernetes-workspace-managed" } +run = "e2e/rust/e2e-kubernetes.sh" + +["e2e:kubernetes:workspace-operator"] +description = "Run Kubernetes e2e with operator workspace mode (pre-provisioned per-workspace namespaces)" +env = { OPENSHELL_E2E_KUBE_EXTRA_VALUES = "deploy/helm/openshell/ci/values-workspace-operator.yaml", OPENSHELL_E2E_KUBE_TEST = "workspace_namespace_operator", OPENSHELL_E2E_KUBERNETES_FEATURES = "e2e,e2e-kubernetes,e2e-kubernetes-workspace-operator" } +run = "e2e/rust/e2e-kubernetes.sh" + ["e2e:vm"] description = "Start openshell-gateway with the VM compute driver and run VM e2e tests" run = "e2e/rust/e2e-vm.sh" From 628a4b7f4ff51f1fa223fca3b1b7c1b74cb522e9 Mon Sep 17 00:00:00 2001 From: Derek Carr Date: Sat, 8 Aug 2026 15:04:54 -0400 Subject: [PATCH 3/4] feat(k8s): add operator namespace label watcher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spawn a background kube::runtime::watcher in the K8s driver that watches namespaces matching the configured label selector and populates the OperatorNamespaceAllowlist at runtime. The driver owns the allowlist and exposes its Arc so the server can share the same set with the SA token authenticator. create_sandbox now gates pod creation on the allowlist in operator mode — workspaces whose namespace is not yet labeled are rejected at resource render time rather than silently proceeding. Workspace lifecycle itself is unaffected; only sandbox (resource) creation is gated. Signed-off-by: Derek Carr --- .../openshell-driver-kubernetes/src/driver.rs | 112 +++++++++++++++++- crates/openshell-server/src/compute/mod.rs | 17 ++- crates/openshell-server/src/lib.rs | 42 ++++--- e2e/rust/tests/workspace_namespace_managed.rs | 97 +++++++-------- 4 files changed, 200 insertions(+), 68 deletions(-) diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 53f8443d28..543d2c884a 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -6,8 +6,8 @@ use super::AppArmorProfile; use crate::config::{ DEFAULT_PROXY_UID, DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, DEFAULT_SANDBOX_UID, - DEFAULT_WORKSPACE_STORAGE_SIZE, KubernetesComputeConfig, SupervisorSideloadMethod, - SupervisorTopology, WorkspaceMode, managed_namespace, + DEFAULT_WORKSPACE_STORAGE_SIZE, KubernetesComputeConfig, OperatorNamespaceAllowlist, + SupervisorSideloadMethod, SupervisorTopology, WorkspaceMode, managed_namespace, }; use futures::{Stream, StreamExt, TryStreamExt}; use k8s_openapi::api::core::v1::{ @@ -438,6 +438,7 @@ pub struct KubernetesComputeDriver { watch_client: Client, sandbox_api_version: Arc>, config: KubernetesComputeConfig, + operator_allowlist: Option, } impl std::fmt::Debug for KubernetesComputeDriver { @@ -485,11 +486,26 @@ impl KubernetesComputeDriver { let watch_client = Client::try_from(watch_kube_config).map_err(KubernetesDriverError::from_kube)?; + let operator_allowlist = if matches!(config.workspace_mode, WorkspaceMode::Operator) { + config.operator_namespace_label.as_ref().map(|label| { + let allowlist = OperatorNamespaceAllowlist::new(); + spawn_namespace_label_watcher( + watch_client.clone(), + label.clone(), + allowlist.clone(), + ); + allowlist + }) + } else { + None + }; + Ok(Self { client, watch_client, sandbox_api_version: Arc::new(OnceCell::new()), config, + operator_allowlist, }) } @@ -501,6 +517,10 @@ impl KubernetesComputeDriver { )) } + pub fn operator_allowlist(&self) -> Option<&OperatorNamespaceAllowlist> { + self.operator_allowlist.as_ref() + } + pub fn default_image(&self) -> &str { &self.config.default_image } @@ -1046,7 +1066,16 @@ impl KubernetesComputeDriver { let target_namespace = match self.config.workspace_mode { WorkspaceMode::Shared => self.config.namespace.clone(), WorkspaceMode::Managed => self.ensure_namespace(workspace).await?, - WorkspaceMode::Operator => workspace.to_string(), + WorkspaceMode::Operator => { + if let Some(ref allowlist) = self.operator_allowlist + && !allowlist.contains(workspace) + { + return Err(KubernetesDriverError::InvalidArgument(format!( + "workspace '{workspace}' is not in the operator namespace allowlist" + ))); + } + workspace.to_string() + } }; info!( @@ -3479,6 +3508,83 @@ fn condition_from_value(value: &serde_json::Value) -> Option { }) } +fn spawn_namespace_label_watcher( + client: Client, + label_selector: String, + allowlist: OperatorNamespaceAllowlist, +) { + let ns_api: Api = Api::all(client); + let watcher_config = watcher::Config::default().labels(&label_selector); + + tokio::spawn(async move { + loop { + let mut stream = watcher::watcher(ns_api.clone(), watcher_config.clone()).boxed(); + + loop { + match stream.try_next().await { + Ok(Some(Event::Applied(ns))) => { + if let Some(name) = ns.metadata.name.as_deref() { + let inner = allowlist.shared(); + let mut guard = inner.write().expect("allowlist lock poisoned"); + if guard.insert(name.to_string()) { + let count = guard.len(); + drop(guard); + info!( + namespace = name, + total = count, + "operator namespace added to allowlist" + ); + } + } + } + Ok(Some(Event::Deleted(ns))) => { + if let Some(name) = ns.metadata.name.as_deref() { + let inner = allowlist.shared(); + let mut guard = inner.write().expect("allowlist lock poisoned"); + if guard.remove(name) { + let count = guard.len(); + drop(guard); + info!( + namespace = name, + total = count, + "operator namespace removed from allowlist" + ); + } + } + } + Ok(Some(Event::Restarted(namespaces))) => { + let names: std::collections::BTreeSet = namespaces + .into_iter() + .filter_map(|ns| ns.metadata.name) + .collect(); + let count = names.len(); + allowlist.replace(names); + info!( + total = count, + "operator namespace allowlist replaced from full relist" + ); + } + Ok(None) => { + warn!("operator namespace watcher stream ended unexpectedly"); + break; + } + Err(err) => { + warn!(error = %err, "operator namespace watcher stream error"); + break; + } + } + } + + tokio::time::sleep(Duration::from_secs(2)).await; + } + }); + + info!( + label_selector = %label_selector, + "operator namespace label watcher started" + ); +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index a1c33e49ff..36fb2aff7d 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -45,6 +45,7 @@ use openshell_core::{ObjectLabels, ObjectWorkspace}; use openshell_driver_docker::DockerComputeDriver; use openshell_driver_kubernetes::{ ComputeDriverService as KubernetesDriverService, KubernetesComputeDriver, + OperatorNamespaceAllowlist, }; use openshell_driver_podman::{ComputeDriverService as PodmanDriverService, PodmanComputeDriver}; use prost::Message; @@ -749,12 +750,21 @@ impl ComputeRuntime { sandbox_watch_bus: SandboxWatchBus, tracing_log_bus: TracingLogBus, supervisor_sessions: Arc, - ) -> Result { + ) -> Result< + ( + Self, + Option>>>, + ), + ComputeError, + > { let driver = KubernetesComputeDriver::new(config) .await .map_err(|err| ComputeError::Message(err.to_string()))?; + let operator_allowlist_arc = driver + .operator_allowlist() + .map(OperatorNamespaceAllowlist::shared); let driver: SharedComputeDriver = Arc::new(KubernetesDriverService::new(driver)); - Self::from_driver( + let runtime = Self::from_driver( ComputeDriverKind::Kubernetes.as_str().to_string(), driver, None, @@ -766,7 +776,8 @@ impl ComputeRuntime { tracing_log_bus, supervisor_sessions, ) - .await + .await?; + Ok((runtime, operator_allowlist_arc)) } pub(crate) async fn new_remote_driver( diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index b669a457dc..a0c686ae42 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -349,7 +349,7 @@ pub(crate) async fn run_server( gateway_tls_enabled: config.tls.is_some(), endpoint_overrides: &config.compute_driver_endpoints, }; - let compute = build_compute_runtime( + let (compute, operator_allowlist) = build_compute_runtime( &config, driver_startup, store.clone(), @@ -468,12 +468,12 @@ pub(crate) async fn run_server( ) } openshell_driver_kubernetes::WorkspaceMode::Operator => { - // The operator allowlist is populated at runtime by the label - // watcher and file watcher. An empty initial set is fail-closed - // until the watcher populates it. - auth::k8s_sa::NamespaceValidator::Allowlist(Arc::new(std::sync::RwLock::new( - std::collections::BTreeSet::new(), - ))) + // Share the driver's allowlist Arc so the SA authenticator and + // the driver's namespace label watcher use the same set. + let allowlist = operator_allowlist.clone().unwrap_or_else(|| { + Arc::new(std::sync::RwLock::new(std::collections::BTreeSet::new())) + }); + auth::k8s_sa::NamespaceValidator::Allowlist(allowlist) } }; match kube::Client::try_default().await { @@ -860,6 +860,8 @@ async fn terminate_signal() { // Internal wiring helper: each argument is a distinct piece of runtime state // that must be passed through, so the count is justified. #[allow(clippy::too_many_arguments)] +type OperatorAllowlistArc = Option>>>; + async fn build_compute_runtime( config: &Config, driver_startup: compute::driver_config::DriverStartupContext<'_>, @@ -868,16 +870,16 @@ async fn build_compute_runtime( sandbox_watch_bus: SandboxWatchBus, tracing_log_bus: TracingLogBus, supervisor_sessions: Arc, -) -> Result { +) -> Result<(ComputeRuntime, OperatorAllowlistArc)> { let driver = configured_compute_driver(config, driver_startup)?; info!(driver = %driver.name(), "Using compute driver"); - let runtime = match driver { + let (runtime, operator_allowlist) = match driver { ConfiguredComputeDriver::Builtin(ComputeDriverKind::Kubernetes) => { warn_if_kubernetes_sandbox_jwt_expiry_disabled(config); let k8s_config = compute::driver_config::kubernetes_config_from_context(driver_startup)?; - ComputeRuntime::new_kubernetes( + let (rt, allowlist) = ComputeRuntime::new_kubernetes( k8s_config, store, sandbox_index, @@ -886,10 +888,12 @@ async fn build_compute_runtime( supervisor_sessions.clone(), ) .await + .map_err(|e| Error::execution(format!("failed to create compute runtime: {e}")))?; + (rt, allowlist) } ConfiguredComputeDriver::Builtin(ComputeDriverKind::Docker) => { let docker_config = compute::driver_config::docker_config_from_context(driver_startup)?; - ComputeRuntime::new_docker( + let rt = ComputeRuntime::new_docker( config.clone(), docker_config, store, @@ -899,10 +903,12 @@ async fn build_compute_runtime( supervisor_sessions, ) .await + .map_err(|e| Error::execution(format!("failed to create compute runtime: {e}")))?; + (rt, None) } ConfiguredComputeDriver::Builtin(ComputeDriverKind::Podman) => { let podman_config = compute::driver_config::podman_config_from_context(driver_startup)?; - ComputeRuntime::new_podman( + let rt = ComputeRuntime::new_podman( podman_config, store, sandbox_index, @@ -911,6 +917,8 @@ async fn build_compute_runtime( supervisor_sessions, ) .await + .map_err(|e| Error::execution(format!("failed to create compute runtime: {e}")))?; + (rt, None) } ConfiguredComputeDriver::Builtin(ComputeDriverKind::Vm) => { let vm_config = compute::driver_config::vm_config_from_context(driver_startup)?; @@ -918,7 +926,7 @@ async fn build_compute_runtime( .file .and_then(|file| file.openshell.gateway.otlp.as_ref()); let endpoint = compute::vm::spawn(config, &vm_config, otlp_config).await?; - ComputeRuntime::new_remote_driver( + let rt = ComputeRuntime::new_remote_driver( endpoint, store, sandbox_index, @@ -927,6 +935,8 @@ async fn build_compute_runtime( supervisor_sessions, ) .await + .map_err(|e| Error::execution(format!("failed to create compute runtime: {e}")))?; + (rt, None) } ConfiguredComputeDriver::Remote { name } => { let remote_config = @@ -939,7 +949,7 @@ async fn build_compute_runtime( let endpoint = compute::connect_remote_compute_driver(name, &remote_config.socket_path) .await .map_err(|e| Error::execution(format!("failed to create compute runtime: {e}")))?; - ComputeRuntime::new_remote_driver( + let rt = ComputeRuntime::new_remote_driver( endpoint, store, sandbox_index, @@ -948,10 +958,12 @@ async fn build_compute_runtime( supervisor_sessions, ) .await + .map_err(|e| Error::execution(format!("failed to create compute runtime: {e}")))?; + (rt, None) } }; - runtime.map_err(|e| Error::execution(format!("failed to create compute runtime: {e}"))) + Ok((runtime, operator_allowlist)) } #[derive(Debug, Clone)] diff --git a/e2e/rust/tests/workspace_namespace_managed.rs b/e2e/rust/tests/workspace_namespace_managed.rs index 4bfc793a23..f18827478b 100644 --- a/e2e/rust/tests/workspace_namespace_managed.rs +++ b/e2e/rust/tests/workspace_namespace_managed.rs @@ -163,20 +163,9 @@ async fn managed_creates_namespace_with_labels() { assert!(ok, "ServiceAccount openshell-sandbox should exist in {ns}"); // Verify sandbox CR is in the managed namespace (not the gateway namespace). - let (ok, out) = kubectl(&[ - "get", - "sandbox.agents.x-k8s.io", - "-n", - &ns, - "-o", - "name", - ]) - .await; + let (ok, out) = kubectl(&["get", "sandbox.agents.x-k8s.io", "-n", &ns, "-o", "name"]).await; assert!(ok, "sandbox CR should exist in namespace {ns}: {out}"); - assert!( - out.contains("mgd-sb"), - "sandbox CR name mismatch: {out}" - ); + assert!(out.contains("mgd-sb"), "sandbox CR name mismatch: {out}"); } #[tokio::test] @@ -193,13 +182,29 @@ async fn managed_namespace_survives_with_remaining_sandboxes() { // Create two sandboxes. let (ok, out) = run_cli(&[ - "sandbox", "create", "--workspace", &ws, "--name", "sb-a", "--", "echo", "a", + "sandbox", + "create", + "--workspace", + &ws, + "--name", + "sb-a", + "--", + "echo", + "a", ]) .await; assert!(ok, "sandbox sb-a create failed: {out}"); let (ok, out) = run_cli(&[ - "sandbox", "create", "--workspace", &ws, "--name", "sb-b", "--", "echo", "b", + "sandbox", + "create", + "--workspace", + &ws, + "--name", + "sb-b", + "--", + "echo", + "b", ]) .await; assert!(ok, "sandbox sb-b create failed: {out}"); @@ -215,15 +220,7 @@ async fn managed_namespace_survives_with_remaining_sandboxes() { assert!(ok, "managed namespace {ns} should still exist with sb-b"); // Verify sb-b's CR is still in the managed namespace. - let (ok, out) = kubectl(&[ - "get", - "sandbox.agents.x-k8s.io", - "-n", - &ns, - "-o", - "name", - ]) - .await; + let (ok, out) = kubectl(&["get", "sandbox.agents.x-k8s.io", "-n", &ns, "-o", "name"]).await; assert!(ok, "sandbox CRs should still exist in {ns}: {out}"); assert!( out.contains("sb-b"), @@ -253,13 +250,29 @@ async fn managed_isolates_workspaces_into_separate_namespaces() { assert!(ok, "workspace B create failed: {out}"); let (ok, out) = run_cli(&[ - "sandbox", "create", "--workspace", &ws_a, "--name", "sb-iso-a", "--", "echo", "a", + "sandbox", + "create", + "--workspace", + &ws_a, + "--name", + "sb-iso-a", + "--", + "echo", + "a", ]) .await; assert!(ok, "sandbox A create failed: {out}"); let (ok, out) = run_cli(&[ - "sandbox", "create", "--workspace", &ws_b, "--name", "sb-iso-b", "--", "echo", "b", + "sandbox", + "create", + "--workspace", + &ws_b, + "--name", + "sb-iso-b", + "--", + "echo", + "b", ]) .await; assert!(ok, "sandbox B create failed: {out}"); @@ -273,29 +286,19 @@ async fn managed_isolates_workspaces_into_separate_namespaces() { assert!(ok, "namespace {ns_b} should exist"); // Verify sandbox CRs are in the correct namespaces (no cross-contamination). - let (ok, out) = kubectl(&[ - "get", - "sandbox.agents.x-k8s.io", - "-n", - &ns_a, - "-o", - "name", - ]) - .await; + let (ok, out) = kubectl(&["get", "sandbox.agents.x-k8s.io", "-n", &ns_a, "-o", "name"]).await; assert!(ok, "failed to list CRs in {ns_a}: {out}"); assert!(out.contains("sb-iso-a"), "sb-iso-a should be in {ns_a}"); - assert!(!out.contains("sb-iso-b"), "sb-iso-b should NOT be in {ns_a}"); - - let (ok, out) = kubectl(&[ - "get", - "sandbox.agents.x-k8s.io", - "-n", - &ns_b, - "-o", - "name", - ]) - .await; + assert!( + !out.contains("sb-iso-b"), + "sb-iso-b should NOT be in {ns_a}" + ); + + let (ok, out) = kubectl(&["get", "sandbox.agents.x-k8s.io", "-n", &ns_b, "-o", "name"]).await; assert!(ok, "failed to list CRs in {ns_b}: {out}"); assert!(out.contains("sb-iso-b"), "sb-iso-b should be in {ns_b}"); - assert!(!out.contains("sb-iso-a"), "sb-iso-a should NOT be in {ns_b}"); + assert!( + !out.contains("sb-iso-a"), + "sb-iso-a should NOT be in {ns_b}" + ); } From 6a067bbb29ef5fdde7d395fd248deef8b9d2ee69 Mon Sep 17 00:00:00 2001 From: Derek Carr Date: Sat, 8 Aug 2026 15:59:29 -0400 Subject: [PATCH 4/4] fix(k8s): harden operator mode and address review findings Close the fail-open gap in operator mode when only operator_namespace_file is configured: the allowlist is now created unconditionally in operator mode (fail-closed from startup). Implement the namespace file watcher using the notify crate, following the TLS hot-reload pattern (parent-directory watch, 1s debounce, ConfigMap symlink-swap safe). The file format is a JSON array of namespace name strings. Additional fixes from the 10-reviewer audit: - Change allowlist rejection from InvalidArgument to FailedPrecondition so callers know the request may succeed later once the namespace is provisioned. - NamespaceValidator::Allowlist now holds the OperatorNamespaceAllowlist newtype instead of a raw Arc>, eliminating silent denial on RwLock poison. - Verify LABEL_MANAGED_BY and LABEL_GATEWAY_ID ownership before deleting a managed namespace. - Replace fixed 5s sleep in operator e2e test with a 30s poll loop. - Add Helm validation for workspaceMode values. - Fix Helm README type column and description for operator fields. - Add insert/remove methods to OperatorNamespaceAllowlist; label watcher now uses them instead of reaching through shared(). - Reject configs with both operator_namespace_label and operator_namespace_file set. Signed-off-by: Derek Carr --- Cargo.lock | 1 + crates/openshell-driver-kubernetes/Cargo.toml | 1 + .../openshell-driver-kubernetes/src/config.rs | 28 ++- .../openshell-driver-kubernetes/src/driver.rs | 201 +++++++++++++++--- crates/openshell-server/src/auth/k8s_sa.rs | 14 +- crates/openshell-server/src/compute/mod.rs | 12 +- crates/openshell-server/src/lib.rs | 8 +- deploy/helm/openshell/README.md | 4 +- deploy/helm/openshell/templates/_helpers.tpl | 4 + deploy/helm/openshell/values.yaml | 6 +- .../tests/workspace_namespace_operator.rs | 47 ++-- 11 files changed, 245 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index acf5fff2c7..8861233b9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3785,6 +3785,7 @@ dependencies = [ "kube", "kube-runtime", "miette", + "notify", "openshell-core", "openshell-policy", "prost", diff --git a/crates/openshell-driver-kubernetes/Cargo.toml b/crates/openshell-driver-kubernetes/Cargo.toml index 2c02f864ab..9be2b1c76b 100644 --- a/crates/openshell-driver-kubernetes/Cargo.toml +++ b/crates/openshell-driver-kubernetes/Cargo.toml @@ -34,6 +34,7 @@ tracing = { workspace = true } tracing-subscriber = { workspace = true } thiserror = { workspace = true } miette = { workspace = true } +notify = "8" [dev-dependencies] temp-env = "0.3" diff --git a/crates/openshell-driver-kubernetes/src/config.rs b/crates/openshell-driver-kubernetes/src/config.rs index 21d3aea851..8849dde43e 100644 --- a/crates/openshell-driver-kubernetes/src/config.rs +++ b/crates/openshell-driver-kubernetes/src/config.rs @@ -293,8 +293,8 @@ pub struct KubernetesComputeConfig { /// this label and builds the allowlist dynamically. #[serde(default, skip_serializing_if = "Option::is_none")] pub operator_namespace_label: Option, - /// Path to a drop-in JSON file mapping workspace names to namespace names. - /// Hot-reloaded on change. Delivered via `ConfigMap` volume mount. + /// Path to a JSON file containing an array of namespace names allowed in + /// operator mode. Hot-reloaded on change. Delivered via `ConfigMap` volume mount. #[serde(default, skip_serializing_if = "Option::is_none")] pub operator_namespace_file: Option, /// Kubernetes `ServiceAccount` assigned to sandbox pods and accepted by @@ -623,10 +623,16 @@ impl KubernetesComputeConfig { WorkspaceMode::Operator => { if self.operator_namespace_label.is_none() && self.operator_namespace_file.is_none() { - return Err("operator workspace mode requires at least one of \ + return Err("operator workspace mode requires exactly one of \ operator_namespace_label or operator_namespace_file" .into()); } + if self.operator_namespace_label.is_some() && self.operator_namespace_file.is_some() + { + return Err("operator workspace mode requires exactly one of \ + operator_namespace_label or operator_namespace_file, not both" + .into()); + } if let Some(ref label) = self.operator_namespace_label && label.is_empty() { @@ -738,6 +744,22 @@ impl OperatorNamespaceAllowlist { .contains(namespace) } + /// Insert a namespace into the allowlist. Returns `true` if it was new. + pub fn insert(&self, name: String) -> bool { + self.inner + .write() + .expect("allowlist lock poisoned") + .insert(name) + } + + /// Remove a namespace from the allowlist. Returns `true` if it was present. + pub fn remove(&self, name: &str) -> bool { + self.inner + .write() + .expect("allowlist lock poisoned") + .remove(name) + } + /// Return a clone of the inner `Arc` for sharing with background tasks. #[must_use] pub fn shared(&self) -> Arc>> { diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 543d2c884a..1f492139ec 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -41,7 +41,7 @@ use openshell_core::proto::compute::v1::{ use openshell_core::proto_struct::{struct_to_json_object, value_to_json}; use serde::Deserialize; use std::collections::{BTreeMap, HashSet}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::pin::Pin; use std::sync::Arc; use std::time::Duration; @@ -487,15 +487,21 @@ impl KubernetesComputeDriver { Client::try_from(watch_kube_config).map_err(KubernetesDriverError::from_kube)?; let operator_allowlist = if matches!(config.workspace_mode, WorkspaceMode::Operator) { - config.operator_namespace_label.as_ref().map(|label| { - let allowlist = OperatorNamespaceAllowlist::new(); + let allowlist = OperatorNamespaceAllowlist::new(); + + if let Some(ref label) = config.operator_namespace_label { spawn_namespace_label_watcher( watch_client.clone(), label.clone(), allowlist.clone(), ); - allowlist - }) + } + + if let Some(ref path) = config.operator_namespace_file { + spawn_namespace_file_watcher(path.into(), allowlist.clone()); + } + + Some(allowlist) } else { None }; @@ -673,6 +679,36 @@ impl KubernetesComputeDriver { } let ns_api: Api = Api::all(self.client.clone()); + + let ns = match tokio::time::timeout(KUBE_API_TIMEOUT, ns_api.get(&ns_name)).await { + Ok(Ok(ns)) => ns, + Ok(Err(KubeError::Api(api))) if api.code == 404 => { + debug!(namespace = %ns_name, "managed namespace already deleted"); + return Ok(()); + } + Ok(Err(e)) => return Err(KubernetesDriverError::from_kube(e)), + Err(_) => { + return Err(KubernetesDriverError::Message(format!( + "timeout getting namespace {ns_name}" + ))); + } + }; + + let labels = ns.metadata.labels.as_ref(); + let is_owned = labels + .and_then(|l| l.get(LABEL_MANAGED_BY)) + .is_some_and(|v| v == LABEL_MANAGED_BY_VALUE) + && labels + .and_then(|l| l.get(LABEL_GATEWAY_ID)) + .is_some_and(|v| v == &self.config.gateway_id); + if !is_owned { + debug!( + namespace = %ns_name, + "namespace not owned by this gateway, skipping delete" + ); + return Ok(()); + } + match tokio::time::timeout( KUBE_API_TIMEOUT, ns_api.delete(&ns_name, &DeleteParams::default()), @@ -1070,7 +1106,7 @@ impl KubernetesComputeDriver { if let Some(ref allowlist) = self.operator_allowlist && !allowlist.contains(workspace) { - return Err(KubernetesDriverError::InvalidArgument(format!( + return Err(KubernetesDriverError::Precondition(format!( "workspace '{workspace}' is not in the operator namespace allowlist" ))); } @@ -3523,33 +3559,20 @@ fn spawn_namespace_label_watcher( loop { match stream.try_next().await { Ok(Some(Event::Applied(ns))) => { - if let Some(name) = ns.metadata.name.as_deref() { - let inner = allowlist.shared(); - let mut guard = inner.write().expect("allowlist lock poisoned"); - if guard.insert(name.to_string()) { - let count = guard.len(); - drop(guard); - info!( - namespace = name, - total = count, - "operator namespace added to allowlist" - ); - } + if let Some(name) = ns.metadata.name.as_deref() + && allowlist.insert(name.to_string()) + { + info!(namespace = name, "operator namespace added to allowlist"); } } Ok(Some(Event::Deleted(ns))) => { - if let Some(name) = ns.metadata.name.as_deref() { - let inner = allowlist.shared(); - let mut guard = inner.write().expect("allowlist lock poisoned"); - if guard.remove(name) { - let count = guard.len(); - drop(guard); - info!( - namespace = name, - total = count, - "operator namespace removed from allowlist" - ); - } + if let Some(name) = ns.metadata.name.as_deref() + && allowlist.remove(name) + { + info!( + namespace = name, + "operator namespace removed from allowlist" + ); } } Ok(Some(Event::Restarted(namespaces))) => { @@ -3581,10 +3604,126 @@ fn spawn_namespace_label_watcher( info!( label_selector = %label_selector, - "operator namespace label watcher started" + "operator namespace label watcher spawned" ); } +fn load_namespace_file(path: &Path) -> Result, String> { + let contents = std::fs::read_to_string(path) + .map_err(|e| format!("failed to read {}: {e}", path.display()))?; + let names: Vec = serde_json::from_str(&contents) + .map_err(|e| format!("failed to parse {}: {e}", path.display()))?; + Ok(names.into_iter().collect()) +} + +fn spawn_namespace_file_watcher(path: PathBuf, allowlist: OperatorNamespaceAllowlist) { + match load_namespace_file(&path) { + Ok(names) => { + let count = names.len(); + allowlist.replace(names); + info!( + path = %path.display(), + total = count, + "operator namespace allowlist loaded from file" + ); + } + Err(err) => { + warn!( + error = %err, + "failed to load initial operator namespace file, allowlist empty" + ); + } + } + + let watch_dir = path + .parent() + .unwrap_or_else(|| Path::new(".")) + .to_path_buf(); + let debounce = Duration::from_secs(1); + + tokio::spawn(async move { + let (tx, mut rx) = mpsc::unbounded_channel(); + + let mut watcher = + match notify::recommended_watcher(move |res: Result| { + if let Ok(event) = res + && matches!( + event.kind, + notify::EventKind::Modify(_) | notify::EventKind::Create(_) + ) + { + let _ = tx.send(()); + } + }) { + Ok(w) => w, + Err(e) => { + warn!( + error = %e, + "failed to start operator namespace file watcher, hot-reload disabled" + ); + return; + } + }; + + if let Err(e) = notify::Watcher::watch( + &mut watcher, + &watch_dir, + notify::RecursiveMode::NonRecursive, + ) { + warn!( + error = %e, + dir = %watch_dir.display(), + "failed to watch operator namespace file directory, hot-reload disabled" + ); + return; + } + + info!( + path = %path.display(), + "operator namespace file watcher started" + ); + + loop { + let got_event = rx.recv().await.is_some(); + if !got_event { + warn!("operator namespace file watcher disconnected"); + break; + } + + loop { + tokio::select! { + () = tokio::time::sleep(debounce) => { + match load_namespace_file(&path) { + Ok(names) => { + let count = names.len(); + allowlist.replace(names); + info!( + total = count, + "operator namespace allowlist reloaded from file" + ); + } + Err(err) => { + warn!( + error = %err, + "failed to reload operator namespace file, keeping existing allowlist" + ); + } + } + break; + } + r = rx.recv() => { + if r.is_some() { + continue; + } + warn!("operator namespace file watcher disconnected"); + return; + } + } + } + } + }); +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/openshell-server/src/auth/k8s_sa.rs b/crates/openshell-server/src/auth/k8s_sa.rs index 54f7ed9afa..32cb2e119c 100644 --- a/crates/openshell-server/src/auth/k8s_sa.rs +++ b/crates/openshell-server/src/auth/k8s_sa.rs @@ -26,8 +26,8 @@ use k8s_openapi::apimachinery::pkg::apis::meta::v1::ObjectMeta; use kube::Error as KubeError; use kube::api::{Api, ApiResource, PostParams}; use kube::core::{DynamicObject, gvk::GroupVersionKind}; -use std::collections::BTreeSet; -use std::sync::{Arc, RwLock}; +use openshell_driver_kubernetes::OperatorNamespaceAllowlist; +use std::sync::Arc; use tonic::Status; use tracing::{debug, info, warn}; @@ -146,7 +146,7 @@ pub enum NamespaceValidator { /// (`openshell-{gateway_id}-`). Prefix(String), /// Operator mode: accept namespaces in the dynamic allowlist. - Allowlist(Arc>>), + Allowlist(OperatorNamespaceAllowlist), } impl NamespaceValidator { @@ -154,7 +154,7 @@ impl NamespaceValidator { match self { Self::Exact(expected) => namespace == expected, Self::Prefix(prefix) => namespace.starts_with(prefix.as_str()), - Self::Allowlist(set) => set.read().is_ok_and(|s| s.contains(namespace)), + Self::Allowlist(al) => al.contains(namespace), } } } @@ -842,11 +842,11 @@ mod tests { #[test] fn namespace_validator_allowlist_accepts_known_namespaces() { - let set = Arc::new(RwLock::new(BTreeSet::from([ + let al = OperatorNamespaceAllowlist::from_set(std::collections::BTreeSet::from([ "ns-a".to_string(), "ns-b".to_string(), - ]))); - let v = NamespaceValidator::Allowlist(set); + ])); + let v = NamespaceValidator::Allowlist(al); assert!(v.accepts("ns-a")); assert!(v.accepts("ns-b")); assert!(!v.accepts("ns-c")); diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 36fb2aff7d..071309cf25 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -750,19 +750,11 @@ impl ComputeRuntime { sandbox_watch_bus: SandboxWatchBus, tracing_log_bus: TracingLogBus, supervisor_sessions: Arc, - ) -> Result< - ( - Self, - Option>>>, - ), - ComputeError, - > { + ) -> Result<(Self, Option), ComputeError> { let driver = KubernetesComputeDriver::new(config) .await .map_err(|err| ComputeError::Message(err.to_string()))?; - let operator_allowlist_arc = driver - .operator_allowlist() - .map(OperatorNamespaceAllowlist::shared); + let operator_allowlist_arc = driver.operator_allowlist().cloned(); let driver: SharedComputeDriver = Arc::new(KubernetesDriverService::new(driver)); let runtime = Self::from_driver( ComputeDriverKind::Kubernetes.as_str().to_string(), diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index a0c686ae42..96a17c7315 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -468,11 +468,7 @@ pub(crate) async fn run_server( ) } openshell_driver_kubernetes::WorkspaceMode::Operator => { - // Share the driver's allowlist Arc so the SA authenticator and - // the driver's namespace label watcher use the same set. - let allowlist = operator_allowlist.clone().unwrap_or_else(|| { - Arc::new(std::sync::RwLock::new(std::collections::BTreeSet::new())) - }); + let allowlist = operator_allowlist.clone().unwrap_or_default(); auth::k8s_sa::NamespaceValidator::Allowlist(allowlist) } }; @@ -860,7 +856,7 @@ async fn terminate_signal() { // Internal wiring helper: each argument is a distinct piece of runtime state // that must be passed through, so the count is justified. #[allow(clippy::too_many_arguments)] -type OperatorAllowlistArc = Option>>>; +type OperatorAllowlistArc = Option; async fn build_compute_runtime( config: &Config, diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 19e26562ed..a7f6dfda15 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -225,8 +225,8 @@ add `ci/values-spire.yaml` to the OpenShell release values files. | server.dbUrl | string | `"sqlite:/var/openshell/openshell.db"` | Gateway database URL (used for the default SQLite backend). | | server.defaultRuntimeClassName | string | `""` | Default Kubernetes runtimeClassName for sandbox pods. Applied when a CreateSandbox request does not specify one. Empty (default) = omit the field, using the cluster's default RuntimeClass. Set to a RuntimeClass name (e.g. "kata-containers", "nvidia") to apply it to all sandboxes that don't explicitly override it. | | server.disableTls | bool | `false` | Disable TLS entirely - the server listens on plaintext HTTP. Set to true when a reverse proxy / tunnel terminates TLS at the edge. | -| server.drivers.kubernetes.operatorNamespaceFile | operator mode | `""` | Path to a drop-in JSON file mapping workspace names to namespace names. Hot-reloaded on change. | -| server.drivers.kubernetes.operatorNamespaceLabel | operator mode | `""` | K8s label selector for namespace discovery. The driver watches namespaces matching this label. | +| server.drivers.kubernetes.operatorNamespaceFile | string | `""` | Path to a JSON file containing an array of namespace names allowed in operator mode. Hot-reloaded on change. | +| server.drivers.kubernetes.operatorNamespaceLabel | string | `""` | K8s label selector for namespace discovery in operator mode. The driver watches namespaces matching this label. | | server.drivers.kubernetes.workspaceMode | string | `"shared"` | How workspaces map to Kubernetes namespaces. "shared" (default): all sandboxes in a single namespace. "managed": auto-creates per-workspace namespaces. "operator": uses pre-provisioned namespaces. | | server.enableLoopbackServiceHttp | bool | `true` | Enable plaintext HTTP routing for loopback sandbox service URLs on TLS-enabled gateways. | | server.enableUserNamespaces | bool | `false` | Enable Kubernetes user namespace isolation (hostUsers: false) for sandbox pods. Requires Kubernetes 1.33+ with user namespace support available (beta through 1.35, GA in 1.36+), plus a supporting container runtime and Linux 5.12+. When enabled, container UID 0 maps to an unprivileged host UID and capabilities become namespaced. | diff --git a/deploy/helm/openshell/templates/_helpers.tpl b/deploy/helm/openshell/templates/_helpers.tpl index 3764fa6d7a..548418abc6 100644 --- a/deploy/helm/openshell/templates/_helpers.tpl +++ b/deploy/helm/openshell/templates/_helpers.tpl @@ -247,6 +247,10 @@ Validate chart values that Helm would otherwise accept silently. {{- if and (eq $workloadKind "statefulset") (gt $replicaCount 1) (not (get $workload "allowMultiReplicaStatefulSet" | default false)) -}} {{- fail "replicaCount > 1 with workload.kind=statefulset requires workload.allowMultiReplicaStatefulSet=true; use workload.kind=deployment for external database-backed multi-replica gateways." -}} {{- end -}} +{{- $workspaceMode := .Values.server.drivers.kubernetes.workspaceMode | default "shared" -}} +{{- if not (has $workspaceMode (list "shared" "managed" "operator")) -}} +{{- fail "server.drivers.kubernetes.workspaceMode must be one of: shared, managed, operator." -}} +{{- end -}} {{- $credentialDrivers := list -}} {{- if .Values.server.credentialDrivers.kubernetesSecrets.enabled -}} {{- $credentialDrivers = append $credentialDrivers "kubernetes-secrets" -}} diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index ab13c55632..81ec18c036 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -231,11 +231,11 @@ server: # "managed": auto-creates per-workspace namespaces. # "operator": uses pre-provisioned namespaces. workspaceMode: "shared" - # -- (operator mode) K8s label selector for namespace discovery. + # -- K8s label selector for namespace discovery in operator mode. # The driver watches namespaces matching this label. operatorNamespaceLabel: "" - # -- (operator mode) Path to a drop-in JSON file mapping workspace - # names to namespace names. Hot-reloaded on change. + # -- Path to a JSON file containing an array of namespace names + # allowed in operator mode. Hot-reloaded on change. operatorNamespaceFile: "" # -- Disable TLS entirely - the server listens on plaintext HTTP. # Set to true when a reverse proxy / tunnel terminates TLS at the edge. diff --git a/e2e/rust/tests/workspace_namespace_operator.rs b/e2e/rust/tests/workspace_namespace_operator.rs index 172414e100..a382de446c 100644 --- a/e2e/rust/tests/workspace_namespace_operator.rs +++ b/e2e/rust/tests/workspace_namespace_operator.rs @@ -139,30 +139,39 @@ async fn operator_sandbox_in_labeled_namespace() { // Pre-provision the namespace with the operator label and ServiceAccount. provision_operator_namespace(&ns).await; - // Wait for the gateway's namespace watcher to discover it. - tokio::time::sleep(Duration::from_secs(5)).await; - // Create a workspace matching the namespace name (operator mode: 1:1 mapping). let (ok, out) = run_cli(&["workspace", "create", "--name", &ns]).await; assert!(ok, "workspace create failed: {out}"); - // Create a sandbox in the workspace. - let (ok, out) = run_cli(&[ - "sandbox", - "create", - "--workspace", - &ns, - "--name", - "op-sb", - "--", - "echo", - "operator-ok", - ]) - .await; - assert!(ok, "sandbox create failed: {out}"); + // Poll until the gateway's namespace watcher discovers the labeled namespace + // and sandbox creation succeeds (up to 30s). + let mut sandbox_out = String::new(); + let deadline = tokio::time::Instant::now() + Duration::from_secs(30); + loop { + let (ok, out) = run_cli(&[ + "sandbox", + "create", + "--workspace", + &ns, + "--name", + "op-sb", + "--", + "echo", + "operator-ok", + ]) + .await; + if ok { + sandbox_out = out; + break; + } + if tokio::time::Instant::now() >= deadline { + panic!("sandbox create did not succeed within 30s: {out}"); + } + tokio::time::sleep(Duration::from_secs(2)).await; + } assert!( - out.contains("operator-ok"), - "sandbox output missing expected string: {out}" + sandbox_out.contains("operator-ok"), + "sandbox output missing expected string: {sandbox_out}" ); // Verify the sandbox CR lives in the pre-provisioned namespace.