From c7ff17acedb1f62a6e3ba6bb9d1dd88dbe55b9b9 Mon Sep 17 00:00:00 2001 From: Dhiraj Bokde Date: Sun, 26 Jul 2026 20:59:04 -0700 Subject: [PATCH] feat(kubernetes): map workspaces to namespaces Signed-off-by: Dhiraj Bokde --- .../skills/debug-openshell-cluster/SKILL.md | 17 + architecture/compute-runtimes.md | 8 + crates/openshell-driver-kubernetes/README.md | 18 +- .../openshell-driver-kubernetes/src/config.rs | 133 ++++- .../openshell-driver-kubernetes/src/driver.rs | 542 +++++++++--------- .../openshell-driver-kubernetes/src/main.rs | 3 +- crates/openshell-server/src/auth/k8s_sa.rs | 211 ++++--- .../src/compute/driver_config.rs | 26 +- crates/openshell-server/src/lib.rs | 10 +- deploy/helm/openshell/README.md | 6 + deploy/helm/openshell/README.md.gotmpl | 5 + .../openshell/templates/gateway-config.yaml | 3 + .../openshell/tests/gateway_config_test.yaml | 18 + deploy/helm/openshell/values.yaml | 5 + docs/reference/gateway-config.mdx | 3 + docs/reference/sandbox-compute-drivers.mdx | 7 + 16 files changed, 679 insertions(+), 336 deletions(-) diff --git a/.agents/skills/debug-openshell-cluster/SKILL.md b/.agents/skills/debug-openshell-cluster/SKILL.md index 319031b1d1..1215f49384 100644 --- a/.agents/skills/debug-openshell-cluster/SKILL.md +++ b/.agents/skills/debug-openshell-cluster/SKILL.md @@ -355,6 +355,23 @@ helm -n openshell get values openshell | grep sandboxNamespace Then inspect sandbox resources in that namespace. +If `workspace_namespaces` is present in `gateway.toml`, inspect every mapped +namespace rather than only `sandboxNamespace`: + +```bash +kubectl -n openshell get configmap openshell-config \ + -o jsonpath='{.data.gateway\.toml}' | grep workspace_namespaces +kubectl -n get sandbox,pod,event +kubectl auth can-i list sandboxes.agents.x-k8s.io \ + --namespace \ + --as system:serviceaccount:openshell:openshell +``` + +An unmapped workspace is rejected before the driver creates a Sandbox CR. +Registration failures from an otherwise healthy mapped sandbox usually mean +the gateway RoleBinding, sandbox ServiceAccount name, or projected-token +bootstrap prerequisites differ between namespaces. + Check the configured sandbox service account when TokenReview bootstrap or sandbox registration fails. Helm creates a dedicated sandbox service account by default and writes it to `[openshell.drivers.kubernetes].service_account_name`; diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index 646b6320bd..3d07d667fc 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -142,6 +142,14 @@ through the driver configuration. The Helm chart defaults sandbox agents to `Unconfined` so runtime/default AppArmor profiles do not block supervisor network namespace setup on AppArmor-enabled nodes. +The Kubernetes driver can map logical workspaces to a bounded set of +pre-provisioned namespaces. Creation resolves the namespace from the workspace; +all other lifecycle operations reconcile across the distinct configured +namespace set. Kubernetes ServiceAccount bootstrap authentication uses the same +allowlist before reading a pod or its owning Sandbox CR. The driver never +creates or deletes namespaces, and a non-empty mapping rejects unmapped +workspaces rather than falling back to a shared namespace. + Resource requirements enter the driver layer through `SandboxSpec.resource_requirements`. This includes a set of GPU requirements, where a user can request a specific number of GPUs or the driver-specific default behaviour. For all in-tree drivers, this is equivalent to selecting a single GPU. diff --git a/crates/openshell-driver-kubernetes/README.md b/crates/openshell-driver-kubernetes/README.md index 1356e2d932..528dcd44d6 100644 --- a/crates/openshell-driver-kubernetes/README.md +++ b/crates/openshell-driver-kubernetes/README.md @@ -3,8 +3,10 @@ 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. By default it uses one configured namespace. Operators can +instead configure `workspace_namespaces` to map logical OpenShell workspaces to +pre-provisioned Kubernetes namespaces. It runs in-process with the gateway +server. ## Runtime Model @@ -26,6 +28,18 @@ by the gateway. Kubernetes API calls use explicit timeouts so gRPC handlers do not block indefinitely when the API server is slow or unavailable. +## Workspace Namespace Mapping + +An empty `workspace_namespaces` map preserves legacy behavior and routes every +sandbox to `namespace`. A non-empty map is an allowlist: sandbox creation fails +when its logical workspace is not mapped. Get, list, delete, existence checks, +Sandbox watches, Event watches, OpenShift SCC discovery, and ServiceAccount +bootstrap authentication operate across only the distinct mapped namespaces. + +The driver does not create namespaces or RBAC. Operators must install the +sandbox ServiceAccount, Secrets, Role, and gateway RoleBinding in every mapped +namespace. Mapping changes require a gateway restart. + ## Workspace Persistence Sandbox pods use a PVC-backed `/sandbox` workspace. An init container seeds the diff --git a/crates/openshell-driver-kubernetes/src/config.rs b/crates/openshell-driver-kubernetes/src/config.rs index 5311f56436..b12cc2ebf6 100644 --- a/crates/openshell-driver-kubernetes/src/config.rs +++ b/crates/openshell-driver-kubernetes/src/config.rs @@ -3,6 +3,7 @@ use openshell_core::config; use serde::{Deserialize, Deserializer, Serialize}; +use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; use std::str::FromStr; @@ -233,6 +234,11 @@ where #[serde(default, deny_unknown_fields)] pub struct KubernetesComputeConfig { pub namespace: String, + /// Optional mapping from logical `OpenShell` workspace names to + /// pre-provisioned Kubernetes namespaces. When empty, all sandboxes use + /// `namespace` for backward compatibility. When non-empty, sandbox + /// creation fails closed for an unmapped workspace. + pub workspace_namespaces: BTreeMap, /// Kubernetes `ServiceAccount` assigned to sandbox pods and accepted by /// the gateway's `TokenReview` bootstrap authenticator. pub service_account_name: String, @@ -333,6 +339,7 @@ impl Default for KubernetesComputeConfig { fn default() -> Self { Self { namespace: DEFAULT_K8S_NAMESPACE.to_string(), + workspace_namespaces: BTreeMap::new(), 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 @@ -364,6 +371,55 @@ impl Default for KubernetesComputeConfig { } impl KubernetesComputeConfig { + /// Resolve the Kubernetes namespace for a logical workspace. + /// + /// An empty mapping retains the legacy single-namespace behavior. Once a + /// mapping is configured, every workspace must be explicitly present. + pub fn namespace_for_workspace(&self, workspace: &str) -> Result<&str, String> { + if self.workspace_namespaces.is_empty() { + return Ok(&self.namespace); + } + self.workspace_namespaces + .get(workspace) + .map(String::as_str) + .ok_or_else(|| { + format!("workspace '{workspace}' is not mapped to a Kubernetes namespace") + }) + } + + /// Namespaces the driver is allowed to manage, in deterministic order. + #[must_use] + pub fn sandbox_namespaces(&self) -> Vec<&str> { + if self.workspace_namespaces.is_empty() { + return vec![self.namespace.as_str()]; + } + self.workspace_namespaces + .values() + .map(String::as_str) + .collect::>() + .into_iter() + .collect() + } + + /// Validate the legacy namespace and every configured workspace mapping. + pub fn validate_workspace_namespaces(&self) -> Result<(), String> { + validate_kubernetes_namespace_name(&self.namespace)?; + for (workspace, namespace) in &self.workspace_namespaces { + if workspace.trim().is_empty() { + return Err("workspace_namespaces keys must not be empty".to_string()); + } + if workspace.trim() != workspace { + return Err(format!( + "workspace_namespaces key '{workspace}' must not contain leading or trailing whitespace" + )); + } + validate_kubernetes_namespace_name(namespace).map_err(|err| { + format!("invalid namespace mapped from workspace '{workspace}': {err}") + })?; + } + Ok(()) + } + /// Clamp `sa_token_ttl_secs` into the `[MIN_SA_TOKEN_TTL_SECS, /// MAX_SA_TOKEN_TTL_SECS]` range used by the projected-volume spec. /// Invalid (≤0) values fall back to the default 3600. @@ -405,7 +461,7 @@ impl KubernetesComputeConfig { /// 3. Fallback defaults: UID=`1000`, GID=UID pub fn resolve_sandbox_uid( &self, - namespace_annotations: Option<&std::collections::BTreeMap>, + namespace_annotations: Option<&BTreeMap>, ) -> u32 { if let Some(uid) = self.sandbox_uid { return uid; @@ -423,7 +479,7 @@ impl KubernetesComputeConfig { pub fn resolve_sandbox_gid( &self, resolved_uid: u32, - _namespace_annotations: Option<&std::collections::BTreeMap>, + _namespace_annotations: Option<&BTreeMap>, ) -> u32 { self.sandbox_gid .or(self.sandbox_uid) @@ -475,6 +531,24 @@ impl KubernetesComputeConfig { } } +fn validate_kubernetes_namespace_name(namespace: &str) -> Result<(), String> { + let bytes = namespace.as_bytes(); + let valid = !bytes.is_empty() + && bytes.len() <= 63 + && bytes.first().is_some_and(u8::is_ascii_alphanumeric) + && bytes.last().is_some_and(u8::is_ascii_alphanumeric) + && bytes + .iter() + .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || *byte == b'-'); + if valid { + Ok(()) + } else { + Err(format!( + "Kubernetes namespace '{namespace}' must be a lowercase DNS-1123 label of at most 63 characters" + )) + } +} + fn validate_provider_spiffe_workload_api_socket_path_value( socket_path: &str, ) -> Result<(), String> { @@ -527,6 +601,61 @@ mod tests { assert!(cfg.workspace_storage_class.is_empty()); } + #[test] + fn empty_workspace_mapping_preserves_legacy_namespace() { + let cfg = KubernetesComputeConfig { + namespace: "shared".to_string(), + ..KubernetesComputeConfig::default() + }; + assert_eq!(cfg.namespace_for_workspace("default").unwrap(), "shared"); + assert_eq!(cfg.sandbox_namespaces(), vec!["shared"]); + } + + #[test] + fn workspace_mapping_resolves_and_fails_closed() { + let cfg = KubernetesComputeConfig { + workspace_namespaces: BTreeMap::from([ + ("team-a".to_string(), "app-a".to_string()), + ("team-b".to_string(), "app-b".to_string()), + ]), + ..KubernetesComputeConfig::default() + }; + cfg.validate_workspace_namespaces().unwrap(); + assert_eq!(cfg.namespace_for_workspace("team-a").unwrap(), "app-a"); + assert!( + cfg.namespace_for_workspace("unmapped") + .unwrap_err() + .contains("not mapped") + ); + assert_eq!(cfg.sandbox_namespaces(), vec!["app-a", "app-b"]); + } + + #[test] + fn workspace_mapping_deduplicates_shared_target_namespaces() { + let cfg = KubernetesComputeConfig { + workspace_namespaces: BTreeMap::from([ + ("team-a".to_string(), "shared-app".to_string()), + ("team-b".to_string(), "shared-app".to_string()), + ]), + ..KubernetesComputeConfig::default() + }; + assert_eq!(cfg.sandbox_namespaces(), vec!["shared-app"]); + } + + #[test] + fn workspace_mapping_rejects_invalid_namespaces_and_keys() { + for workspace_namespaces in [ + BTreeMap::from([(String::new(), "app-a".to_string())]), + BTreeMap::from([("team-a".to_string(), "Invalid_Namespace".to_string())]), + ] { + let cfg = KubernetesComputeConfig { + workspace_namespaces, + ..KubernetesComputeConfig::default() + }; + assert!(cfg.validate_workspace_namespaces().is_err()); + } + } + #[test] fn default_topology_is_combined() { let cfg = KubernetesComputeConfig::default(); diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 2f1ea72a32..7d5d572a60 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -442,6 +442,7 @@ impl std::fmt::Debug for KubernetesComputeDriver { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("KubernetesComputeDriver") .field("namespace", &self.config.namespace) + .field("workspace_namespaces", &self.config.workspace_namespaces) .field("default_image", &self.config.default_image) .field("grpc_endpoint", &self.config.grpc_endpoint) .finish() @@ -459,6 +460,9 @@ impl KubernetesComputeDriver { config .validate_proxy_uid() .map_err(KubernetesDriverError::Precondition)?; + config + .validate_workspace_namespaces() + .map_err(KubernetesDriverError::Precondition)?; let base_config = match kube::Config::incluster() { Ok(c) => c, Err(_) => kube::Config::infer() @@ -504,6 +508,10 @@ impl KubernetesComputeDriver { &self.config.namespace } + pub fn sandbox_namespaces(&self) -> Vec<&str> { + self.config.sandbox_namespaces() + } + pub fn ssh_socket_path(&self) -> &str { &self.config.ssh_socket_path } @@ -522,16 +530,28 @@ 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, &self.config.namespace, &resource); + let api = Api::namespaced_with(client, namespace, &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_version(&self, client: Client) -> Result<&'static str, String> { @@ -547,8 +567,15 @@ impl KubernetesComputeDriver { &self, client: Client, ) -> Result<&'static str, String> { + let namespace = self + .config + .sandbox_namespaces() + .into_iter() + .next() + .unwrap_or(self.config.namespace.as_str()); 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, namespace); match tokio::time::timeout( KUBE_API_TIMEOUT, agent_sandbox_api.api.list(&ListParams::default().limit(1)), @@ -557,7 +584,7 @@ impl KubernetesComputeDriver { { Ok(Ok(_)) => { debug!( - namespace = %self.config.namespace, + namespace = %namespace, sandbox_api_version = %sandbox_api_version, "Selected Agent Sandbox API version" ); @@ -565,7 +592,7 @@ impl KubernetesComputeDriver { } Ok(Err(err)) if should_try_next_sandbox_api_version(&err) => { debug!( - namespace = %self.config.namespace, + namespace = %namespace, sandbox_api_version = %sandbox_api_version, error = %err, "Sandbox API version is not available; trying next supported version" @@ -594,7 +621,10 @@ impl KubernetesComputeDriver { /// `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) { + async fn resolve_sandbox_identity( + &self, + namespace: &str, + ) -> (u32, u32, BTreeMap) { // Explicit config takes priority — skip namespace lookup entirely. if self.config.sandbox_uid.is_some() { let uid = self.config.resolve_sandbox_uid(None); @@ -606,13 +636,11 @@ impl KubernetesComputeDriver { // 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" @@ -637,7 +665,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 +675,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,6 +700,10 @@ impl KubernetesComputeDriver { let _ = self .validate_driver_config_for_sandbox(sandbox) .map_err(tonic::Status::invalid_argument)?; + let _ = self + .config + .namespace_for_workspace(&sandbox.workspace) + .map_err(tonic::Status::failed_precondition)?; validate_kube_resource_name_length(&sandbox.workspace, &sandbox.name)?; let gpu_requirements = sandbox .spec @@ -690,112 +722,116 @@ impl KubernetesComputeDriver { Ok(()) } + async fn sandbox_objects_by_id( + &self, + sandbox_id: &str, + ) -> Result, String> { + let sandbox_api_version = self + .supported_sandbox_api_version(self.client.clone()) + .await?; + let selector = + format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={sandbox_id}"); + let lp = ListParams::default().labels(&selector); + let mut matches = Vec::new(); + for namespace in self.config.sandbox_namespaces() { + let api = Self::agent_sandbox_api(self.client.clone(), sandbox_api_version, namespace); + match tokio::time::timeout(KUBE_API_TIMEOUT, api.api.list(&lp)).await { + Ok(Ok(list)) => { + matches.extend( + list.items + .into_iter() + .map(|obj| (namespace.to_string(), obj)), + ); + } + Ok(Err(err)) => { + return Err(format!( + "failed to list sandbox {sandbox_id} in namespace {namespace}: {err}" + )); + } + Err(_elapsed) => { + return Err(format!( + "timed out after {}s listing sandbox {sandbox_id} in namespace {namespace}", + KUBE_API_TIMEOUT.as_secs() + )); + } + } + } + Ok(matches) + } + pub async fn get_sandbox(&self, sandbox_id: &str) -> Result, String> { + let namespaces = self.config.sandbox_namespaces(); info!( sandbox_id = %sandbox_id, - namespace = %self.config.namespace, + namespaces = ?namespaces, "Fetching sandbox from Kubernetes" ); - let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) - .await?; - let selector = - format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={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( - || { - debug!(sandbox_id = %sandbox_id, "Sandbox not found in Kubernetes"); - Ok(None) - }, - |obj| { - Ok(sandbox_from_object(&self.config.namespace, obj) - .ok() - .map(|(_, s)| s)) - }, - ), - Ok(Err(err)) => { - warn!( - sandbox_id = %sandbox_id, - error = %err, - "Failed to fetch sandbox from Kubernetes" - ); - Err(err.to_string()) - } - Err(_elapsed) => { - warn!( - sandbox_id = %sandbox_id, - timeout_secs = KUBE_API_TIMEOUT.as_secs(), - "Timed out fetching sandbox from Kubernetes" - ); - Err(format!( - "timed out after {}s waiting for Kubernetes API", - KUBE_API_TIMEOUT.as_secs() - )) - } + let mut matches = self.sandbox_objects_by_id(sandbox_id).await?; + if matches.len() > 1 { + return Err(format!( + "sandbox id {sandbox_id} exists in multiple configured Kubernetes namespaces" + )); } + let Some((namespace, obj)) = matches.pop() else { + debug!(sandbox_id = %sandbox_id, "Sandbox not found in Kubernetes"); + return Ok(None); + }; + sandbox_from_object(&namespace, obj) + .map(|(_, sandbox)| Some(sandbox)) + .or_else(|err| { + warn!(sandbox_id = %sandbox_id, namespace = %namespace, error = %err, "Sandbox object is not recognized"); + Ok(None) + }) } pub async fn list_sandboxes(&self) -> Result, String> { + let namespaces = self.config.sandbox_namespaces(); info!( - namespace = %self.config.namespace, + namespaces = ?namespaces, "Listing sandboxes from Kubernetes" ); - let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + let sandbox_api_version = self + .supported_sandbox_api_version(self.client.clone()) .await?; - match tokio::time::timeout( - KUBE_API_TIMEOUT, - agent_sandbox_api - .api - .list(&ListParams::default().labels(&openshell_sandbox_label_selector())), - ) - .await - { - Ok(Ok(list)) => { - let mut sandboxes: Vec = list - .items - .into_iter() - .filter_map(|obj| { + let lp = ListParams::default().labels(&openshell_sandbox_label_selector()); + let mut sandboxes = Vec::new(); + for namespace in namespaces { + let api = Self::agent_sandbox_api(self.client.clone(), sandbox_api_version, namespace); + match tokio::time::timeout(KUBE_API_TIMEOUT, api.api.list(&lp)).await { + Ok(Ok(list)) => { + sandboxes.extend(list.items.into_iter().filter_map(|obj| { let name = obj.metadata.name.clone().unwrap_or_default(); - match sandbox_from_object(&self.config.namespace, obj) { + match sandbox_from_object(namespace, obj) { Ok((_, s)) => Some(s), Err(err) => { - warn!(object_name = %name, error = %err, "skipping unrecognized Sandbox in list"); + warn!(namespace = %namespace, object_name = %name, error = %err, "skipping unrecognized Sandbox in list"); None } } - }) - .collect(); - sandboxes.sort_by(|left, right| { - left.name - .cmp(&right.name) - .then_with(|| left.id.cmp(&right.id)) - }); - Ok(sandboxes) - } - Ok(Err(err)) => { - warn!( - namespace = %self.config.namespace, - error = %err, - "Failed to list sandboxes from Kubernetes" - ); - Err(err.to_string()) - } - Err(_elapsed) => { - warn!( - namespace = %self.config.namespace, - timeout_secs = KUBE_API_TIMEOUT.as_secs(), - "Timed out listing sandboxes from Kubernetes" - ); - Err(format!( - "timed out after {}s waiting for Kubernetes API", - KUBE_API_TIMEOUT.as_secs() - )) + })); + } + Ok(Err(err)) => { + return Err(format!( + "failed to list sandboxes in namespace {namespace}: {err}" + )); + } + Err(_elapsed) => { + return Err(format!( + "timed out after {}s listing sandboxes in namespace {namespace}", + KUBE_API_TIMEOUT.as_secs() + )); + } } } + sandboxes.sort_by(|left, right| { + left.workspace + .cmp(&right.workspace) + .then_with(|| left.name.cmp(&right.name)) + .then_with(|| left.id.cmp(&right.id)) + }); + Ok(sandboxes) } #[allow(clippy::similar_names)] @@ -813,21 +849,27 @@ impl KubernetesComputeDriver { .map_err(KubernetesDriverError::InvalidArgument)?; let name = sandbox.name.as_str(); + let namespace = self + .config + .namespace_for_workspace(&sandbox.workspace) + .map_err(KubernetesDriverError::Precondition)? + .to_string(); info!( sandbox_id = %sandbox.id, sandbox_name = %name, - namespace = %self.config.namespace, + workspace = %sandbox.workspace, + namespace = %namespace, "Creating sandbox in Kubernetes" ); let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) + .supported_agent_sandbox_api(self.client.clone(), &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; + self.resolve_sandbox_identity(&namespace).await; let params = SandboxPodParams { default_image: &self.config.default_image, @@ -882,7 +924,7 @@ impl KubernetesComputeDriver { } obj.metadata = ObjectMeta { name: Some(kube_name), - namespace: Some(self.config.namespace.clone()), + namespace: Some(namespace), labels: Some(sandbox_labels(sandbox)), annotations: Some(annotations), ..Default::default() @@ -928,61 +970,35 @@ impl KubernetesComputeDriver { } pub async fn delete_sandbox(&self, sandbox_id: &str) -> Result { + let namespaces = self.config.sandbox_namespaces(); info!( sandbox_id = %sandbox_id, - namespace = %self.config.namespace, + namespaces = ?namespaces, "Deleting sandbox from Kubernetes" ); - let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) - .await?; - let selector = - format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={sandbox_id}"); - let lp = ListParams::default().labels(&selector); - let (kube_name, preconditions) = match tokio::time::timeout( - KUBE_API_TIMEOUT, - agent_sandbox_api.api.list(&lp), - ) - .await - { - Ok(Ok(list)) => { - if let Some(obj) = list.items.into_iter().next() { - match obj.metadata.name { - Some(name) => { - let pc = Preconditions { - uid: obj.metadata.uid, - resource_version: obj.metadata.resource_version, - }; - (name, pc) - } - None => return Ok(false), - } - } else { - debug!(sandbox_id = %sandbox_id, "Sandbox not found in Kubernetes (already deleted)"); - return Ok(false); - } - } - Ok(Err(err)) => { - warn!( - sandbox_id = %sandbox_id, - error = %err, - "Failed to list sandbox for deletion from Kubernetes" - ); - return Err(err.to_string()); - } - Err(_elapsed) => { - warn!( - sandbox_id = %sandbox_id, - timeout_secs = KUBE_API_TIMEOUT.as_secs(), - "Timed out listing sandbox for deletion from Kubernetes" - ); - return Err(format!( - "timed out after {}s waiting for Kubernetes API", - KUBE_API_TIMEOUT.as_secs() - )); - } + let mut matches = self.sandbox_objects_by_id(sandbox_id).await?; + if matches.len() > 1 { + return Err(format!( + "refusing to delete sandbox id {sandbox_id}: it exists in multiple configured Kubernetes namespaces" + )); + } + let Some((namespace, obj)) = matches.pop() else { + debug!(sandbox_id = %sandbox_id, "Sandbox not found in Kubernetes (already deleted)"); + return Ok(false); + }; + let Some(kube_name) = obj.metadata.name else { + return Ok(false); + }; + let preconditions = Preconditions { + uid: obj.metadata.uid, + resource_version: obj.metadata.resource_version, }; + let sandbox_api_version = self + .supported_sandbox_api_version(self.client.clone()) + .await?; + let agent_sandbox_api = + Self::agent_sandbox_api(self.client.clone(), sandbox_api_version, &namespace); let dp = DeleteParams::default().preconditions(preconditions); match tokio::time::timeout( @@ -992,7 +1008,7 @@ impl KubernetesComputeDriver { .await { Ok(Ok(_response)) => { - info!(sandbox_id = %sandbox_id, "Sandbox deleted from Kubernetes"); + info!(sandbox_id = %sandbox_id, namespace = %namespace, "Sandbox deleted from Kubernetes"); Ok(true) } Ok(Err(KubeError::Api(err))) if err.code == 404 || err.code == 409 => { @@ -1022,43 +1038,81 @@ impl KubernetesComputeDriver { } pub async fn sandbox_exists(&self, sandbox_id: &str) -> Result { - let agent_sandbox_api = self - .supported_agent_sandbox_api(self.client.clone()) - .await?; - let selector = - format!("{LABEL_MANAGED_BY}={LABEL_MANAGED_BY_VALUE},{LABEL_SANDBOX_ID}={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()), - Ok(Err(err)) => Err(err.to_string()), - Err(_elapsed) => Err(format!( - "timed out after {}s waiting for Kubernetes API", - KUBE_API_TIMEOUT.as_secs() - )), + let matches = self.sandbox_objects_by_id(sandbox_id).await?; + if matches.len() > 1 { + return Err(format!( + "sandbox id {sandbox_id} exists in multiple configured Kubernetes namespaces" + )); } + Ok(!matches.is_empty()) } - // 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 { - let namespace = self.config.namespace.clone(); - let agent_sandbox_api = self - .supported_agent_sandbox_api(self.watch_client.clone()) + let sandbox_api_version = self + .supported_sandbox_api_version(self.watch_client.clone()) .await?; - let event_api: Api = Api::namespaced(self.watch_client.clone(), &namespace); - let watcher_config = watcher::Config::default().labels(&openshell_sandbox_label_selector()); - let mut sandbox_stream = watcher::watcher(agent_sandbox_api.api, watcher_config).boxed(); - let mut event_stream = watcher::watcher(event_api, watcher::Config::default()).boxed(); let (tx, rx) = mpsc::channel(256); + for namespace in self.config.sandbox_namespaces() { + let namespace = namespace.to_string(); + let agent_sandbox_api = + Self::agent_sandbox_api(self.watch_client.clone(), sandbox_api_version, &namespace); + let event_api: Api = + Api::namespaced(self.watch_client.clone(), &namespace); + spawn_namespace_watch(namespace, agent_sandbox_api, event_api, tx.clone()); + } + drop(tx); - tokio::spawn(async move { - let mut sandbox_name_to_id = std::collections::HashMap::::new(); - let mut agent_pod_to_id = std::collections::HashMap::::new(); + Ok(Box::pin(ReceiverStream::new(rx))) + } +} - loop { - tokio::select! { - result = sandbox_stream.try_next() => match result { - Ok(Some(Event::Applied(obj))) => { +fn spawn_namespace_watch( + namespace: String, + agent_sandbox_api: AgentSandboxApi, + event_api: Api, + tx: mpsc::Sender>, +) { + let watcher_config = watcher::Config::default().labels(&openshell_sandbox_label_selector()); + let mut sandbox_stream = watcher::watcher(agent_sandbox_api.api, watcher_config).boxed(); + let mut event_stream = watcher::watcher(event_api, watcher::Config::default()).boxed(); + + tokio::spawn(async move { + let mut sandbox_name_to_id = std::collections::HashMap::::new(); + let mut agent_pod_to_id = std::collections::HashMap::::new(); + + loop { + tokio::select! { + result = sandbox_stream.try_next() => match result { + Ok(Some(Event::Applied(obj))) => { + if let Ok((kube_name, sandbox)) = sandbox_from_object(&namespace, obj) { + update_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &kube_name, &sandbox); + 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) + { + remove_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &sandbox_id); + 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 { if let Ok((kube_name, sandbox)) = sandbox_from_object(&namespace, obj) { update_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &kube_name, &sandbox); let event = WatchSandboxesEvent { @@ -1067,90 +1121,62 @@ impl KubernetesComputeDriver { )), }; 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) - { - remove_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &sandbox_id); - 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 { - if let Ok((kube_name, sandbox)) = sandbox_from_object(&namespace, obj) { - update_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &kube_name, &sandbox); - let event = WatchSandboxesEvent { - payload: Some(watch_sandboxes_event::Payload::Sandbox( - WatchSandboxesSandboxEvent { sandbox: Some(sandbox) } - )), - }; - if tx.send(Ok(event)).await.is_err() { - return; - } + 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; - } - }, - result = event_stream.try_next() => match result { - Ok(Some(Event::Applied(obj))) => { - if let Some((sandbox_id, event)) = map_kube_event_to_platform( - &sandbox_name_to_id, - &agent_pod_to_id, - &obj, - ) { - let event = WatchSandboxesEvent { - payload: Some(watch_sandboxes_event::Payload::PlatformEvent( - WatchSandboxesPlatformEvent { sandbox_id, event: Some(event) } - )), - }; - if tx.send(Ok(event)).await.is_err() { - break; - } + } + Ok(None) => { + let _ = tx.send(Err(KubernetesDriverError::Message(format!( + "sandbox watcher stream for namespace {namespace} ended unexpectedly" + )))).await; + break; + } + Err(err) => { + let _ = tx.send(Err(KubernetesDriverError::Message(format!( + "sandbox watcher for namespace {namespace} failed: {err}" + )))).await; + break; + } + }, + result = event_stream.try_next() => match result { + Ok(Some(Event::Applied(obj))) => { + if let Some((sandbox_id, event)) = map_kube_event_to_platform( + &sandbox_name_to_id, + &agent_pod_to_id, + &obj, + ) { + let event = WatchSandboxesEvent { + payload: Some(watch_sandboxes_event::Payload::PlatformEvent( + WatchSandboxesPlatformEvent { sandbox_id, event: Some(event) } + )), + }; + if tx.send(Ok(event)).await.is_err() { + break; } } - Ok(Some(Event::Deleted(_))) => {} - Ok(Some(Event::Restarted(_))) => { - debug!(namespace = %namespace, "Kubernetes event watcher restarted"); - } - Ok(None) => { - let _ = tx.send(Err(KubernetesDriverError::Message( - "kubernetes event watcher stream ended".to_string() - ))).await; - break; - } - Err(err) => { - let _ = tx.send(Err(KubernetesDriverError::Message(err.to_string()))).await; - break; - } - }, - () = tx.closed() => break, - } + } + Ok(Some(Event::Deleted(_))) => {} + Ok(Some(Event::Restarted(_))) => { + debug!(namespace = %namespace, "Kubernetes event watcher restarted"); + } + Ok(None) => { + let _ = tx.send(Err(KubernetesDriverError::Message(format!( + "Kubernetes event watcher for namespace {namespace} ended" + )))).await; + break; + } + Err(err) => { + let _ = tx.send(Err(KubernetesDriverError::Message(format!( + "Kubernetes event watcher for namespace {namespace} failed: {err}" + )))).await; + break; + } + }, + () = tx.closed() => break, } - }); - - Ok(Box::pin(ReceiverStream::new(rx))) - } + } + }); } fn should_try_next_sandbox_api_version(err: &KubeError) -> bool { diff --git a/crates/openshell-driver-kubernetes/src/main.rs b/crates/openshell-driver-kubernetes/src/main.rs index b7d5514ac2..aec47ad78d 100644 --- a/crates/openshell-driver-kubernetes/src/main.rs +++ b/crates/openshell-driver-kubernetes/src/main.rs @@ -3,7 +3,7 @@ use clap::{ArgAction, Parser}; use miette::{IntoDiagnostic, Result}; -use std::net::SocketAddr; +use std::{collections::BTreeMap, net::SocketAddr}; use tracing::info; use tracing_subscriber::EnvFilter; @@ -134,6 +134,7 @@ async fn main() -> Result<()> { let driver = KubernetesComputeDriver::new(KubernetesComputeConfig { namespace: args.sandbox_namespace, + workspace_namespaces: BTreeMap::default(), 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..61c1d44b67 100644 --- a/crates/openshell-server/src/auth/k8s_sa.rs +++ b/crates/openshell-server/src/auth/k8s_sa.rs @@ -26,6 +26,7 @@ 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; use tonic::Status; use tracing::{debug, info, warn}; @@ -137,6 +138,7 @@ impl Authenticator for K8sServiceAccountAuthenticator { #[derive(Debug)] struct TokenReviewIdentity { + namespace: String, pod_name: String, pod_uid: String, } @@ -151,56 +153,52 @@ 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, + sandbox_namespaces: BTreeSet, expected_service_account: String, } impl LiveK8sResolver { pub fn new( client: kube::Client, - namespace: &str, + namespaces: impl IntoIterator, 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(), + sandbox_namespaces: namespaces.into_iter().collect(), expected_service_account, } } async fn get_sandbox_cr_for_owner( &self, + namespace: &str, owner: &SandboxOwnerReference, ) -> Result, KubeError> { + 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(self.client.clone(), namespace, &sandbox_resource_v1beta1); + let sandboxes_api_v1alpha1: Api = + Api::namespaced_with(self.client.clone(), namespace, &sandbox_resource_v1alpha1); let apis = if owner.api_version == SANDBOX_API_VERSION_FULL_V1ALPHA1 { - [&self.sandboxes_api_v1alpha1, &self.sandboxes_api_v1beta1] + [&sandboxes_api_v1alpha1, &sandboxes_api_v1beta1] } else { - [&self.sandboxes_api_v1beta1, &self.sandboxes_api_v1alpha1] + [&sandboxes_api_v1beta1, &sandboxes_api_v1alpha1] }; for api in apis { @@ -242,7 +240,7 @@ impl K8sIdentityResolver for LiveK8sResolver { let Some(identity) = token_review_identity( &status, &self.expected_audience, - &self.sandbox_namespace, + &self.sandbox_namespaces, &self.expected_service_account, )? else { @@ -252,23 +250,21 @@ 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: Api = Api::namespaced(self.client.clone(), &identity.namespace); + let pod = 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 Some(pod) = pod else { warn!( pod = %identity.pod_name, @@ -294,16 +290,20 @@ 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, + namespace = %identity.namespace, + 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 +327,7 @@ impl K8sIdentityResolver for LiveK8sResolver { fn token_review_identity( status: &TokenReviewStatus, expected_audience: &str, - sandbox_namespace: &str, + sandbox_namespaces: &BTreeSet, expected_service_account: &str, ) -> Result, Status> { if status.authenticated != Some(true) { @@ -356,12 +356,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 Some(service_account_identity) = username.strip_prefix("system:serviceaccount:") else { + return Err(Status::permission_denied( + "SA token is not from a Kubernetes service account", + )); + }; + let Some((namespace, service_account)) = service_account_identity.split_once(':') else { + return Err(Status::permission_denied( + "SA token has an invalid Kubernetes service account principal", + )); + }; + if service_account != expected_service_account || !sandbox_namespaces.contains(namespace) { warn!( username = %username, - sandbox_namespace = %sandbox_namespace, + sandbox_namespaces = ?sandbox_namespaces, service_account = %expected_service_account, "K8s TokenReview principal is not the configured sandbox service account" ); @@ -372,7 +380,11 @@ fn token_review_identity( 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: namespace.to_string(), + pod_name, + pod_uid, + })) } #[allow(clippy::result_large_err)] @@ -547,7 +559,14 @@ mod tests { use super::test_support::FakeResolver; use super::*; use k8s_openapi::apimachinery::pkg::apis::meta::v1::OwnerReference; - use std::collections::BTreeMap; + use std::collections::{BTreeMap, BTreeSet}; + + fn sandbox_namespaces(namespaces: &[&str]) -> BTreeSet { + namespaces + .iter() + .map(|namespace| (*namespace).to_string()) + .collect() + } fn bearer_headers(token: &str) -> http::HeaderMap { let mut h = http::HeaderMap::new(); @@ -676,10 +695,16 @@ mod tests { ], ); - let identity = token_review_identity(&status, "openshell-gateway", "openshell", "default") - .unwrap() - .expect("authenticated token should resolve"); + let identity = token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["openshell"]), + "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"); } @@ -693,9 +718,14 @@ mod tests { }; assert!( - token_review_identity(&status, "openshell-gateway", "openshell", "default") - .unwrap() - .is_none() + token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["openshell"]), + "default", + ) + .unwrap() + .is_none() ); } @@ -711,8 +741,13 @@ mod tests { ], ); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") - .expect_err("wrong audience must fail closed"); + let err = token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["openshell"]), + "default", + ) + .expect_err("wrong audience must fail closed"); assert_eq!(err.code(), tonic::Code::Unauthenticated); } @@ -728,8 +763,13 @@ mod tests { ], ); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") - .expect_err("other namespace must be rejected"); + let err = token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["openshell"]), + "default", + ) + .expect_err("other namespace must be rejected"); assert_eq!(err.code(), tonic::Code::PermissionDenied); } @@ -745,8 +785,13 @@ mod tests { ], ); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") - .expect_err("other service account must be rejected"); + let err = token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["openshell"]), + "default", + ) + .expect_err("other service account must be rejected"); assert_eq!(err.code(), tonic::Code::PermissionDenied); } @@ -759,11 +804,39 @@ mod tests { vec![], ); - let err = token_review_identity(&status, "openshell-gateway", "openshell", "default") - .expect_err("non pod-bound tokens must be rejected"); + let err = token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["openshell"]), + "default", + ) + .expect_err("non pod-bound tokens must be rejected"); assert_eq!(err.code(), tonic::Code::PermissionDenied); } + #[test] + fn token_review_identity_accepts_any_configured_workspace_namespace() { + let status = token_review_status( + true, + vec!["openshell-gateway"], + "system:serviceaccount:app-b:default", + vec![ + (POD_NAME_EXTRA, "openshell-sandbox-b"), + (POD_UID_EXTRA, "uid-b"), + ], + ); + + let identity = token_review_identity( + &status, + "openshell-gateway", + &sandbox_namespaces(&["app-a", "app-b"]), + "default", + ) + .unwrap() + .expect("configured namespace should authenticate"); + assert_eq!(identity.namespace, "app-b"); + } + #[test] fn pod_sandbox_id_requires_annotation() { assert_eq!( diff --git a/crates/openshell-server/src/compute/driver_config.rs b/crates/openshell-server/src/compute/driver_config.rs index f56d233f2f..c55432e8d8 100644 --- a/crates/openshell-server/src/compute/driver_config.rs +++ b/crates/openshell-server/src/compute/driver_config.rs @@ -67,7 +67,12 @@ pub fn kubernetes_config_for_k8s_sa_bootstrap( "K8s ServiceAccount bootstrap requires [openshell.drivers.kubernetes] when sandbox JWT issuing is enabled in-cluster", )); } - driver_config_from_file(Some(file), ComputeDriverKind::Kubernetes.as_str()) + let config: KubernetesComputeConfig = + driver_config_from_file(Some(file), ComputeDriverKind::Kubernetes.as_str())?; + config + .validate_workspace_namespaces() + .map_err(Error::config)?; + Ok(config) } /// Build the selected Podman config from TOML plus runtime defaults. @@ -294,6 +299,25 @@ service_account_name = "sandbox-sa" assert_eq!(cfg.service_account_name, "sandbox-sa"); } + #[test] + fn k8s_sa_bootstrap_uses_workspace_namespace_allowlist() { + let file: config_file::ConfigFile = toml::from_str( + r#" +[openshell.gateway] + +[openshell.drivers.kubernetes] +namespace = "legacy" +workspace_namespaces = { "team-a" = "app-a", "team-b" = "app-b" } +service_account_name = "sandbox-sa" +"#, + ) + .expect("valid config"); + + let cfg = kubernetes_config_for_k8s_sa_bootstrap(Some(&file)).unwrap(); + assert_eq!(cfg.namespace_for_workspace("team-a").unwrap(), "app-a"); + assert_eq!(cfg.sandbox_namespaces(), vec!["app-a", "app-b"]); + } + #[test] fn podman_config_reads_bind_mount_opt_in_from_driver_table() { let file: config_file::ConfigFile = toml::from_str( diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 5cd06d3900..78ba96ec30 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -454,20 +454,24 @@ 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_namespaces = kubernetes_config + .sandbox_namespaces() + .into_iter() + .map(str::to_string) + .collect::>(); let sandbox_service_account = kubernetes_config.service_account_name; match kube::Client::try_default().await { Ok(client) => { let resolver = Arc::new(auth::k8s_sa::LiveK8sResolver::new( client, - &sandbox_namespace, + sandbox_namespaces.clone(), "openshell-gateway".to_string(), sandbox_service_account.clone(), )); let authenticator = auth::k8s_sa::K8sServiceAccountAuthenticator::new(resolver); state.k8s_sa_authenticator = Some(Arc::new(authenticator)); info!( - namespace = %sandbox_namespace, + namespaces = ?sandbox_namespaces, service_account = %sandbox_service_account, "K8s ServiceAccount bootstrap authenticator enabled" ); diff --git a/deploy/helm/openshell/README.md b/deploy/helm/openshell/README.md index 7096a8ca74..6151a73460 100644 --- a/deploy/helm/openshell/README.md +++ b/deploy/helm/openshell/README.md @@ -9,6 +9,11 @@ Edit README.md.gotmpl and values.yaml, then run `mise run helm:docs`. This chart deploys the OpenShell gateway into a Kubernetes cluster. It is published as an OCI artifact to GHCR at `oci://ghcr.io/nvidia/openshell/helm-chart`. +To route logical workspaces to pre-provisioned namespaces, set +`server.workspaceNamespaces`. The gateway fails closed for unmapped workspaces +and requires its namespaced RBAC, sandbox ServiceAccount, and referenced +Secrets in every mapped namespace. + ## Prerequisites The Kubernetes Agent Sandbox CRDs and controller must be installed on the cluster before deploying OpenShell. Install them with: @@ -257,6 +262,7 @@ add `ci/values-spire.yaml` to the OpenShell release values files. | server.tls.clientCaSecretName | string | `"openshell-server-client-ca"` | K8s secret with ca.crt for client certificate verification (mTLS). Set to "" to disable mTLS and run HTTPS-only (use OIDC for auth instead). | | server.tls.clientTlsSecretName | string | `"openshell-client-tls"` | K8s secret mounted into sandbox pods for mTLS to the server. | | server.workspaceDefaultStorageSize | string | `""` | Default storage size for the workspace PVC in sandbox pods. Uses Kubernetes quantity syntax (e.g. "2Gi", "10Gi", "500Mi"). Empty = built-in default (2Gi). | +| server.workspaceNamespaces | object | `{}` | Optional map from logical OpenShell workspace names to pre-provisioned Kubernetes namespaces. When non-empty, sandbox creation fails for an unmapped workspace and lifecycle operations are limited to these namespaces. Empty preserves server.sandboxNamespace behavior. | | server.workspaceStorageClass | string | `""` | Kubernetes StorageClass for the workspace PVC in sandbox pods. Empty (default) = omit storageClassName, using the cluster's default StorageClass. Set this on clusters with no default StorageClass, otherwise the workspace PVC stays Pending and the sandbox never starts. | | service.healthPort | int | `8081` | Gateway health service port. | | service.metricsPort | int | `9090` | Gateway metrics service port. | diff --git a/deploy/helm/openshell/README.md.gotmpl b/deploy/helm/openshell/README.md.gotmpl index 0242d8118c..db3fb28d40 100644 --- a/deploy/helm/openshell/README.md.gotmpl +++ b/deploy/helm/openshell/README.md.gotmpl @@ -9,6 +9,11 @@ Edit README.md.gotmpl and values.yaml, then run `mise run helm:docs`. This chart deploys the OpenShell gateway into a Kubernetes cluster. It is published as an OCI artifact to GHCR at `oci://ghcr.io/nvidia/openshell/helm-chart`. +To route logical workspaces to pre-provisioned namespaces, set +`server.workspaceNamespaces`. The gateway fails closed for unmapped workspaces +and requires its namespaced RBAC, sandbox ServiceAccount, and referenced +Secrets in every mapped namespace. + ## Prerequisites The Kubernetes Agent Sandbox CRDs and controller must be installed on the cluster before deploying OpenShell. Install them with: diff --git a/deploy/helm/openshell/templates/gateway-config.yaml b/deploy/helm/openshell/templates/gateway-config.yaml index e22b5e7485..7fdc939e57 100644 --- a/deploy/helm/openshell/templates/gateway-config.yaml +++ b/deploy/helm/openshell/templates/gateway-config.yaml @@ -129,6 +129,9 @@ data: [openshell.drivers.kubernetes] grpc_endpoint = {{ include "openshell.grpcEndpoint" . | quote }} service_account_name = {{ include "openshell.sandboxServiceAccountName" . | quote }} + {{- if .Values.server.workspaceNamespaces }} + workspace_namespaces = { {{- range $index, $workspace := keys .Values.server.workspaceNamespaces | sortAlpha }}{{ if $index }}, {{ end }}{{ $workspace | quote }} = {{ index $.Values.server.workspaceNamespaces $workspace | quote }}{{- end }} } + {{- 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 }} diff --git a/deploy/helm/openshell/tests/gateway_config_test.yaml b/deploy/helm/openshell/tests/gateway_config_test.yaml index f98c321fee..6bddaf270a 100644 --- a/deploy/helm/openshell/tests/gateway_config_test.yaml +++ b/deploy/helm/openshell/tests/gateway_config_test.yaml @@ -83,6 +83,24 @@ tests: path: data["gateway.toml"] pattern: '(?ms)\[openshell\.drivers\.kubernetes\].*?service_account_name\s*=\s*"openshell-sandbox"' + - it: omits workspace namespace mappings by default + template: templates/gateway-config.yaml + asserts: + - notMatchRegex: + path: data["gateway.toml"] + pattern: 'workspace_namespaces\s*=' + + - it: renders workspace namespace mappings deterministically + template: templates/gateway-config.yaml + set: + server.workspaceNamespaces: + team-b: app-b + team-a: app-a + asserts: + - matchRegex: + path: data["gateway.toml"] + pattern: 'workspace_namespaces\s*=\s*\{\s*"team-a"\s*=\s*"app-a",\s*"team-b"\s*=\s*"app-b"\s*\}' + - it: renders combined supervisor topology by default under [openshell.drivers.kubernetes] template: templates/gateway-config.yaml asserts: diff --git a/deploy/helm/openshell/values.yaml b/deploy/helm/openshell/values.yaml index 39205df1bf..e3329f854f 100644 --- a/deploy/helm/openshell/values.yaml +++ b/deploy/helm/openshell/values.yaml @@ -167,6 +167,11 @@ server: # -- Namespace where sandbox pods are created. Defaults to the Helm release # namespace (.Release.Namespace) when left empty. sandboxNamespace: "" + # -- Optional map from logical OpenShell workspace names to pre-provisioned + # Kubernetes namespaces. When non-empty, sandbox creation fails for an + # unmapped workspace and lifecycle operations are limited to these + # namespaces. Empty preserves server.sandboxNamespace behavior. + workspaceNamespaces: {} # -- Gateway database URL (used for the default SQLite backend). dbUrl: "sqlite:/var/openshell/openshell.db" # -- Name of a pre-existing Opaque Secret containing a PostgreSQL diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 2cd10b8a0b..c7c32a8f5c 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -414,6 +414,9 @@ client_ca_path = "/etc/openshell-tls/client-ca/ca.crt" [openshell.drivers.kubernetes] namespace = "agents" +# Optional logical-workspace routing. When this map is non-empty, every +# workspace must be listed and `namespace` is used only as the legacy default. +workspace_namespaces = { "team-a" = "agents-a", "team-b" = "agents-b" } service_account_name = "openshell-sandbox" default_image = "ghcr.io/nvidia/openshell/sandbox:latest" image_pull_policy = "IfNotPresent" diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 2132f3360e..9ceaa5c019 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -312,6 +312,7 @@ For maintainer-level implementation details, refer to the [Kubernetes driver REA |---|---|---| | `compute_drivers = ["kubernetes"]` | Not applicable | Select the Kubernetes compute driver. | | `[openshell.drivers.kubernetes].namespace` | `server.sandboxNamespace` | Set the namespace for sandbox resources. The Helm chart defaults to the release namespace when left empty. | +| `workspace_namespaces` | `server.workspaceNamespaces` | Map logical workspace names to pre-provisioned Kubernetes namespaces. A non-empty map fails closed for unmapped workspaces and limits lifecycle operations and bootstrap authentication to the mapped namespace set. | | `service_account_name` | `sandboxServiceAccount.name` | Set the Kubernetes service account assigned to sandbox pods and accepted by the gateway TokenReview bootstrap path. The Helm chart creates a dedicated sandbox service account by default. | | `default_image` | `server.sandboxImage` | Set the default sandbox image. | | `image_pull_policy` | `server.sandboxImagePullPolicy` | Set the Kubernetes image pull policy for sandbox pods. | @@ -329,6 +330,12 @@ For maintainer-level implementation details, refer to the [Kubernetes driver REA | `workspace_storage_class` | `server.workspaceStorageClass` | Set the `StorageClass` for the workspace PVC. Empty (default) omits `storageClassName` and uses the cluster's default `StorageClass`. Set this on clusters with no default `StorageClass`, otherwise the workspace PVC stays `Pending` and the sandbox never starts. | | `sa_token_ttl_secs` | `server.sandboxJwt.k8sSaTokenTtlSecs` | Set the projected ServiceAccount token TTL used for the bootstrap token exchange. | +When `workspace_namespaces` is configured, the gateway opens one namespaced +Sandbox and Event watch per distinct mapped namespace. The gateway +ServiceAccount needs the same namespaced RoleBinding in every target namespace, +and the configured sandbox ServiceAccount and referenced Secrets must exist in +each namespace. Namespace creation and deletion remain operator-owned. + In `combined` topology, the agent container carries the Linux capabilities needed by the supervisor for network namespace setup, Landlock filesystem policy, process privilege changes, and network policy enforcement. In `sidecar`