From 8d49b2ce65eb72bf9f63f9928bcaf5603beafa28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20Hu=C3=9F?= Date: Thu, 16 Jul 2026 12:55:56 +0200 Subject: [PATCH] feat(workspace): gRPC handlers, compute, and driver workspace threading Part 2/3 of workspace review split (PR 2243 mirror). gRPC: workspace-scoped sandbox/provider/service/policy handlers, auth enforcement points. Compute: workspace-aware SandboxIndex, lease scoping. Drivers: Docker/Podman/Kubernetes workspace-qualified container names and labels. --- crates/openshell-driver-docker/src/lib.rs | 37 +- crates/openshell-driver-docker/src/tests.rs | 10 +- .../openshell-driver-kubernetes/src/driver.rs | 372 +++- .../openshell-driver-kubernetes/src/grpc.rs | 17 +- .../openshell-driver-podman/src/container.rs | 43 +- crates/openshell-driver-podman/src/driver.rs | 220 ++- crates/openshell-driver-podman/src/grpc.rs | 80 +- crates/openshell-driver-podman/src/watcher.rs | 27 +- .../src/auth/sandbox_methods.rs | 4 +- crates/openshell-server/src/compute/lease.rs | 3 + crates/openshell-server/src/compute/mod.rs | 126 +- crates/openshell-server/src/grpc/auth_rpc.rs | 2 +- crates/openshell-server/src/grpc/mod.rs | 129 +- crates/openshell-server/src/grpc/policy.rs | 787 ++++++--- crates/openshell-server/src/grpc/provider.rs | 1530 ++++++++++++++--- crates/openshell-server/src/grpc/sandbox.rs | 395 ++++- crates/openshell-server/src/grpc/service.rs | 301 +++- .../openshell-server/src/provider_refresh.rs | 135 +- crates/openshell-server/src/ssh_sessions.rs | 4 +- .../src/supervisor_session.rs | 2 +- crates/openshell-server/tests/common/mod.rs | 49 + .../tests/supervisor_relay_integration.rs | 45 + 22 files changed, 3357 insertions(+), 961 deletions(-) diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 06d575a1e1..90f24f51f7 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -25,7 +25,8 @@ use openshell_core::config::{ 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_NAMESPACE, SUPERVISOR_IMAGE_BINARY_PATH, supervisor_image_should_refresh, + LABEL_SANDBOX_NAMESPACE, LABEL_SANDBOX_WORKSPACE, SUPERVISOR_IMAGE_BINARY_PATH, + supervisor_image_should_refresh, }; use openshell_core::gpu::{ CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements, @@ -1535,6 +1536,7 @@ fn pending_sandbox_snapshot( conditions: vec![condition], deleting, }), + workspace: sandbox.workspace.clone(), } } @@ -2292,6 +2294,10 @@ fn build_container_create_body_with_gpu_devices( ); labels.insert(LABEL_SANDBOX_ID.to_string(), sandbox.id.clone()); labels.insert(LABEL_SANDBOX_NAME.to_string(), sandbox.name.clone()); + labels.insert( + LABEL_SANDBOX_WORKSPACE.to_string(), + sandbox.workspace.clone(), + ); // The list/get/find paths filter by `config.sandbox_namespace`, so use // the same value here. `DriverSandbox.namespace` is unset on the request // path (the gateway elides it), and using it would produce containers @@ -2723,6 +2729,10 @@ fn sandbox_from_container_summary( .get(LABEL_SANDBOX_NAMESPACE) .cloned() .unwrap_or_default(); + let workspace = labels + .get(LABEL_SANDBOX_WORKSPACE) + .cloned() + .unwrap_or_default(); let supervisor_connected = readiness.is_supervisor_connected(&id); Some(DriverSandbox { @@ -2735,6 +2745,7 @@ fn sandbox_from_container_summary( &name, supervisor_connected, )), + workspace, }) } @@ -2908,24 +2919,26 @@ const CONTAINER_NAME_PREFIX: &str = "openshell-"; fn container_name_for_sandbox(sandbox: &DriverSandbox) -> String { let id_suffix = sanitize_docker_name(&sandbox.id); + let workspace = sanitize_docker_name(&sandbox.workspace); let name = sanitize_docker_name(&sandbox.name); + + // Format: openshell-{workspace}--{name}-{id} + // The workspace and id are never truncated — they ensure uniqueness. + // Only the sandbox name portion is truncated when the total exceeds + // MAX_CONTAINER_NAME_LEN. + if name.is_empty() { - let mut base = format!("{CONTAINER_NAME_PREFIX}{id_suffix}"); - // The prefix is always < MAX_CONTAINER_NAME_LEN. Truncate the id - // suffix only if the sandbox id itself is pathologically long. + let mut base = format!("{CONTAINER_NAME_PREFIX}{workspace}---{id_suffix}"); if base.len() > MAX_CONTAINER_NAME_LEN { base.truncate(MAX_CONTAINER_NAME_LEN); } - return base; + return trim_container_name_tail(base); } - // Reserve space for the prefix and the `-` tail so the id - // suffix — which is what makes the name unique between sandboxes that - // share a human-readable prefix — is never truncated away. - let reserved = CONTAINER_NAME_PREFIX.len() + 1 + id_suffix.len(); + // Reserve space for fixed parts: prefix + workspace + "--" + "-" + id + let reserved = CONTAINER_NAME_PREFIX.len() + workspace.len() + 2 + 1 + id_suffix.len(); if reserved >= MAX_CONTAINER_NAME_LEN { - // Pathological sandbox id. Fall back to `` and truncate. - let mut base = format!("{CONTAINER_NAME_PREFIX}{id_suffix}"); + let mut base = format!("{CONTAINER_NAME_PREFIX}{workspace}---{id_suffix}"); base.truncate(MAX_CONTAINER_NAME_LEN); return trim_container_name_tail(base); } @@ -2936,7 +2949,7 @@ fn container_name_for_sandbox(sandbox: &DriverSandbox) -> String { } else { name }; - format!("{CONTAINER_NAME_PREFIX}{truncated_name}-{id_suffix}") + format!("{CONTAINER_NAME_PREFIX}{workspace}--{truncated_name}-{id_suffix}") } /// Docker container names may not end with `-`, `.`, or `_`. Truncation can diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 594308bce5..14525e7dc0 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -46,6 +46,7 @@ fn test_sandbox() -> DriverSandbox { sandbox_token: String::new(), }), status: None, + workspace: String::new(), } } @@ -1980,6 +1981,7 @@ fn container_name_preserves_id_suffix_for_long_names() { namespace: "default".to_string(), spec: None, status: None, + workspace: "default".to_string(), }; let second = DriverSandbox { id: "sbx-second-0987654321".to_string(), @@ -2005,15 +2007,19 @@ fn container_name_preserves_id_suffix_for_long_names() { } #[test] -fn container_name_empty_sandbox_name_uses_id_only() { +fn container_name_empty_sandbox_name_uses_workspace_and_id() { let sandbox = DriverSandbox { id: "sbx-abc".to_string(), name: String::new(), namespace: "default".to_string(), spec: None, status: None, + workspace: "default".to_string(), }; - assert_eq!(container_name_for_sandbox(&sandbox), "openshell-sbx-abc",); + assert_eq!( + container_name_for_sandbox(&sandbox), + "openshell-default---sbx-abc", + ); } #[test] diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index 5315d30409..5288da9e67 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -20,7 +20,8 @@ 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, SUPERVISOR_IMAGE_BINARY_PATH, + LABEL_MANAGED_BY, LABEL_MANAGED_BY_VALUE, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, + LABEL_SANDBOX_WORKSPACE, SUPERVISOR_IMAGE_BINARY_PATH, }; use openshell_core::gpu::{driver_gpu_requirements, effective_driver_gpu_count}; use openshell_core::progress::{ @@ -667,6 +668,7 @@ 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)?; let gpu_requirements = sandbox .spec .as_ref() @@ -684,9 +686,9 @@ impl KubernetesComputeDriver { Ok(()) } - pub async fn get_sandbox(&self, name: &str) -> Result, String> { + pub async fn get_sandbox(&self, sandbox_id: &str) -> Result, String> { info!( - sandbox_name = %name, + sandbox_id = %sandbox_id, namespace = %self.config.namespace, "Fetching sandbox from Kubernetes" ); @@ -694,15 +696,19 @@ impl KubernetesComputeDriver { let agent_sandbox_api = self .supported_agent_sandbox_api(self.client.clone()) .await?; - match tokio::time::timeout(KUBE_API_TIMEOUT, agent_sandbox_api.api.get(name)).await { - Ok(Ok(obj)) => sandbox_from_object(&self.config.namespace, obj).map(Some), - Ok(Err(KubeError::Api(err))) if err.code == 404 => { - debug!(sandbox_name = %name, "Sandbox not found in Kubernetes"); - Ok(None) - } + let selector = format!("{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| sandbox_from_object(&self.config.namespace, obj).map(|(_, s)| Some(s)), + ), Ok(Err(err)) => { warn!( - sandbox_name = %name, + sandbox_id = %sandbox_id, error = %err, "Failed to fetch sandbox from Kubernetes" ); @@ -710,7 +716,7 @@ impl KubernetesComputeDriver { } Err(_elapsed) => { warn!( - sandbox_name = %name, + sandbox_id = %sandbox_id, timeout_secs = KUBE_API_TIMEOUT.as_secs(), "Timed out fetching sandbox from Kubernetes" ); @@ -738,10 +744,10 @@ impl KubernetesComputeDriver { .await { Ok(Ok(list)) => { - let mut sandboxes = list + let mut sandboxes: Vec = list .items .into_iter() - .map(|obj| sandbox_from_object(&self.config.namespace, obj)) + .map(|obj| sandbox_from_object(&self.config.namespace, obj).map(|(_, s)| s)) .collect::, _>>()?; sandboxes.sort_by(|left, right| { left.name @@ -832,32 +838,25 @@ impl KubernetesComputeDriver { }; validate_sidecar_proxy_identity(¶ms)?; - let data = sandbox_to_k8s_spec(sandbox.spec.as_ref(), ¶ms) - .map_err(KubernetesDriverError::InvalidArgument)?; - let mut obj = DynamicObject::new(name, &agent_sandbox_api.resource); + let kube_name = kube_resource_name(&sandbox.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 scc_annotations: BTreeMap = [ + let mut annotations = sandbox_annotations(sandbox); + for key in [ crate::config::ANNOTATION_SCC_UID_RANGE, crate::config::ANNOTATION_SCC_SUPPLEMENTAL_GROUPS, - ] - .iter() - .filter_map(|key| { - ns_annotations - .get(*key) - .map(|v| ((*key).to_string(), v.clone())) - }) - .collect(); + ] { + if let Some(v) = ns_annotations.get(key) { + annotations.insert(key.to_string(), v.clone()); + } + } obj.metadata = ObjectMeta { - name: Some(name.to_string()), + name: Some(kube_name), namespace: Some(self.config.namespace.clone()), labels: Some(sandbox_labels(sandbox)), - annotations: if scc_annotations.is_empty() { - None - } else { - Some(scc_annotations) - }, + annotations: Some(annotations), ..Default::default() }; @@ -900,9 +899,9 @@ impl KubernetesComputeDriver { } } - pub async fn delete_sandbox(&self, name: &str) -> Result { + pub async fn delete_sandbox(&self, sandbox_id: &str) -> Result { info!( - sandbox_name = %name, + sandbox_id = %sandbox_id, namespace = %self.config.namespace, "Deleting sandbox from Kubernetes" ); @@ -910,23 +909,65 @@ impl KubernetesComputeDriver { let agent_sandbox_api = self .supported_agent_sandbox_api(self.client.clone()) .await?; + let selector = format!("{LABEL_SANDBOX_ID}={sandbox_id}"); + let lp = ListParams::default().labels(&selector); + let kube_name = 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) => name, + 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() + )); + } + }; + match tokio::time::timeout( KUBE_API_TIMEOUT, - agent_sandbox_api.api.delete(name, &DeleteParams::default()), + agent_sandbox_api + .api + .delete(&kube_name, &DeleteParams::default()), ) .await { Ok(Ok(_response)) => { - info!(sandbox_name = %name, "Sandbox deleted from Kubernetes"); + info!(sandbox_id = %sandbox_id, "Sandbox deleted from Kubernetes"); Ok(true) } Ok(Err(KubeError::Api(err))) if err.code == 404 => { - debug!(sandbox_name = %name, "Sandbox not found in Kubernetes (already deleted)"); + debug!(sandbox_id = %sandbox_id, "Sandbox not found in Kubernetes (already deleted)"); Ok(false) } Ok(Err(err)) => { warn!( - sandbox_name = %name, + sandbox_id = %sandbox_id, error = %err, "Failed to delete sandbox from Kubernetes" ); @@ -934,7 +975,7 @@ impl KubernetesComputeDriver { } Err(_elapsed) => { warn!( - sandbox_name = %name, + sandbox_id = %sandbox_id, timeout_secs = KUBE_API_TIMEOUT.as_secs(), "Timed out deleting sandbox from Kubernetes" ); @@ -946,13 +987,14 @@ impl KubernetesComputeDriver { } } - pub async fn sandbox_exists(&self, name: &str) -> Result { + pub async fn sandbox_exists(&self, sandbox_id: &str) -> Result { let agent_sandbox_api = self .supported_agent_sandbox_api(self.client.clone()) .await?; - match tokio::time::timeout(KUBE_API_TIMEOUT, agent_sandbox_api.api.get(name)).await { - Ok(Ok(_)) => Ok(true), - Ok(Err(KubeError::Api(err))) if err.code == 404 => Ok(false), + let selector = format!("{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", @@ -983,8 +1025,8 @@ impl KubernetesComputeDriver { result = sandbox_stream.try_next() => match result { Ok(Some(Event::Applied(obj))) => { match sandbox_from_object(&namespace, obj) { - Ok(sandbox) => { - update_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &sandbox); + Ok((kube_name, sandbox)) => { + 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) } @@ -1024,8 +1066,8 @@ impl KubernetesComputeDriver { Ok(Some(Event::Restarted(objs))) => { for obj in objs { match sandbox_from_object(&namespace, obj) { - Ok(sandbox) => { - update_indexes(&mut sandbox_name_to_id, &mut agent_pod_to_id, &sandbox); + Ok((kube_name, sandbox)) => { + 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) } @@ -1110,9 +1152,31 @@ 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> { + let combined = workspace.len() + 2 + name.len(); // "--" separator + if combined > MAX_KUBE_NAME_LEN { + return Err(tonic::Status::invalid_argument(format!( + "combined Kubernetes resource name '{workspace}--{name}' is {combined} characters, \ + exceeding the DNS-1123 limit of {MAX_KUBE_NAME_LEN}" + ))); + } + Ok(()) +} + fn sandbox_labels(sandbox: &Sandbox) -> BTreeMap { let mut labels = BTreeMap::new(); labels.insert(LABEL_SANDBOX_ID.to_string(), sandbox.id.clone()); + labels.insert(LABEL_SANDBOX_NAME.to_string(), sandbox.name.clone()); + labels.insert( + LABEL_SANDBOX_WORKSPACE.to_string(), + sandbox.workspace.clone(), + ); labels.insert( LABEL_MANAGED_BY.to_string(), LABEL_MANAGED_BY_VALUE.to_string(), @@ -1120,24 +1184,48 @@ fn sandbox_labels(sandbox: &Sandbox) -> BTreeMap { labels } +fn sandbox_annotations(sandbox: &Sandbox) -> BTreeMap { + let mut annotations = BTreeMap::new(); + annotations.insert(LABEL_SANDBOX_ID.to_string(), sandbox.id.clone()); + annotations.insert(LABEL_SANDBOX_NAME.to_string(), sandbox.name.clone()); + annotations.insert( + LABEL_SANDBOX_WORKSPACE.to_string(), + sandbox.workspace.clone(), + ); + annotations +} + fn sandbox_id_from_object(obj: &DynamicObject) -> Result { + if let Some(annotations) = obj.metadata.annotations.as_ref() + && let Some(id) = annotations.get(LABEL_SANDBOX_ID) + { + return Ok(id.clone()); + } if let Some(labels) = obj.metadata.labels.as_ref() && let Some(id) = labels.get(LABEL_SANDBOX_ID) { return Ok(id.clone()); } - - let name = obj.metadata.name.clone().unwrap_or_default(); - if let Some(id) = name.strip_prefix("sandbox-") { - return Ok(id.to_string()); - } - Err("sandbox id not found on object".to_string()) } -fn sandbox_from_object(namespace: &str, obj: DynamicObject) -> Result { +fn annotation_or_label(obj: &DynamicObject, key: &str) -> Option { + obj.metadata + .annotations + .as_ref() + .and_then(|a| a.get(key)) + .or_else(|| obj.metadata.labels.as_ref().and_then(|l| l.get(key))) + .cloned() +} + +/// Returns `(kube_resource_name, DriverSandbox)`. +fn sandbox_from_object(namespace: &str, obj: DynamicObject) -> Result<(String, Sandbox), String> { let id = sandbox_id_from_object(&obj)?; - let name = obj.metadata.name.clone().unwrap_or_default(); + let kube_name = obj.metadata.name.clone().unwrap_or_default(); + let name = + annotation_or_label(&obj, LABEL_SANDBOX_NAME).ok_or("sandbox name not found on object")?; + let workspace = annotation_or_label(&obj, LABEL_SANDBOX_WORKSPACE) + .ok_or("sandbox workspace not found on object")?; let namespace = obj .metadata .namespace @@ -1145,22 +1233,27 @@ fn sandbox_from_object(namespace: &str, obj: DynamicObject) -> Result, agent_pod_to_id: &mut std::collections::HashMap, + kube_name: &str, sandbox: &Sandbox, ) { - if !sandbox.name.is_empty() { - sandbox_name_to_id.insert(sandbox.name.clone(), sandbox.id.clone()); + if !kube_name.is_empty() { + sandbox_name_to_id.insert(kube_name.to_string(), sandbox.id.clone()); } if let Some(status) = sandbox.status.as_ref() && !status.instance_id.is_empty() @@ -5625,4 +5718,157 @@ mod tests { let storage = &vct[0]["spec"]["resources"]["requests"]["storage"]; assert_eq!(storage, DEFAULT_WORKSPACE_STORAGE_SIZE); } + + #[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(); + } + + #[test] + fn kube_resource_name_length_validation_rejects_oversized_names() { + let long_ws = "a".repeat(40); + let long_name = "b".repeat(25); + let err = validate_kube_resource_name_length(&long_ws, &long_name).unwrap_err(); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + assert!(err.message().contains("67")); + } + + #[test] + fn sandbox_from_object_reads_annotations() { + let obj = DynamicObject { + types: None, + metadata: ObjectMeta { + name: Some("alpha--work".to_string()), + namespace: Some("default".to_string()), + annotations: Some(BTreeMap::from([ + (LABEL_SANDBOX_ID.to_string(), "uuid-123".to_string()), + (LABEL_SANDBOX_NAME.to_string(), "work".to_string()), + (LABEL_SANDBOX_WORKSPACE.to_string(), "alpha".to_string()), + ])), + labels: Some(BTreeMap::from([ + (LABEL_SANDBOX_ID.to_string(), "uuid-123".to_string()), + (LABEL_SANDBOX_NAME.to_string(), "work".to_string()), + (LABEL_SANDBOX_WORKSPACE.to_string(), "alpha".to_string()), + ( + LABEL_MANAGED_BY.to_string(), + LABEL_MANAGED_BY_VALUE.to_string(), + ), + ])), + ..Default::default() + }, + data: serde_json::json!({}), + }; + + let (kube_name, sandbox) = sandbox_from_object("default", obj).unwrap(); + assert_eq!(kube_name, "alpha--work"); + assert_eq!(sandbox.name, "work"); + assert_eq!(sandbox.workspace, "alpha"); + assert_eq!(sandbox.id, "uuid-123"); + } + + #[test] + fn sandbox_from_object_falls_back_to_labels() { + let obj = DynamicObject { + types: None, + metadata: ObjectMeta { + name: Some("alpha--work".to_string()), + namespace: Some("default".to_string()), + annotations: None, + labels: Some(BTreeMap::from([ + (LABEL_SANDBOX_ID.to_string(), "uuid-456".to_string()), + (LABEL_SANDBOX_NAME.to_string(), "work".to_string()), + (LABEL_SANDBOX_WORKSPACE.to_string(), "alpha".to_string()), + ])), + ..Default::default() + }, + data: serde_json::json!({}), + }; + + let (_, sandbox) = sandbox_from_object("default", obj).unwrap(); + assert_eq!(sandbox.name, "work"); + assert_eq!(sandbox.workspace, "alpha"); + assert_eq!(sandbox.id, "uuid-456"); + } + + #[test] + fn sandbox_from_object_errors_without_workspace() { + let obj = DynamicObject { + types: None, + metadata: ObjectMeta { + name: Some("work".to_string()), + namespace: Some("default".to_string()), + labels: Some(BTreeMap::from([ + (LABEL_SANDBOX_ID.to_string(), "uuid-789".to_string()), + (LABEL_SANDBOX_NAME.to_string(), "work".to_string()), + ])), + ..Default::default() + }, + data: serde_json::json!({}), + }; + + let result = sandbox_from_object("default", obj); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("workspace")); + } + + #[test] + fn sandbox_labels_includes_workspace_and_name() { + let sandbox = Sandbox { + id: "uuid-1".to_string(), + name: "work".to_string(), + workspace: "alpha".to_string(), + ..Default::default() + }; + let labels = sandbox_labels(&sandbox); + assert_eq!(labels.get(LABEL_SANDBOX_ID).unwrap(), "uuid-1"); + assert_eq!(labels.get(LABEL_SANDBOX_NAME).unwrap(), "work"); + assert_eq!(labels.get(LABEL_SANDBOX_WORKSPACE).unwrap(), "alpha"); + assert_eq!( + labels.get(LABEL_MANAGED_BY).unwrap(), + LABEL_MANAGED_BY_VALUE + ); + } + + #[test] + fn sandbox_annotations_stores_authoritative_values() { + let sandbox = Sandbox { + id: "uuid-2".to_string(), + name: "dev".to_string(), + workspace: "beta".to_string(), + ..Default::default() + }; + let annotations = sandbox_annotations(&sandbox); + assert_eq!(annotations.get(LABEL_SANDBOX_ID).unwrap(), "uuid-2"); + assert_eq!(annotations.get(LABEL_SANDBOX_NAME).unwrap(), "dev"); + assert_eq!(annotations.get(LABEL_SANDBOX_WORKSPACE).unwrap(), "beta"); + } + + #[test] + fn sandbox_id_from_object_errors_without_label() { + let obj = DynamicObject { + types: None, + metadata: ObjectMeta { + name: Some("some-name".to_string()), + ..Default::default() + }, + data: serde_json::json!({}), + }; + assert!(sandbox_id_from_object(&obj).is_err()); + } } diff --git a/crates/openshell-driver-kubernetes/src/grpc.rs b/crates/openshell-driver-kubernetes/src/grpc.rs index eced74f57b..fccfa9464b 100644 --- a/crates/openshell-driver-kubernetes/src/grpc.rs +++ b/crates/openshell-driver-kubernetes/src/grpc.rs @@ -57,23 +57,17 @@ impl ComputeDriver for ComputeDriverService { request: Request, ) -> Result, Status> { let request = request.into_inner(); - if request.sandbox_name.is_empty() { - return Err(Status::invalid_argument("sandbox_name is required")); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); } let sandbox = self .driver - .get_sandbox(&request.sandbox_name) + .get_sandbox(&request.sandbox_id) .await .map_err(Status::internal)? .ok_or_else(|| Status::not_found("sandbox not found"))?; - if !request.sandbox_id.is_empty() && request.sandbox_id != sandbox.id { - return Err(Status::failed_precondition( - "sandbox_id did not match the fetched sandbox", - )); - } - Ok(Response::new(GetSandboxResponse { sandbox: Some(sandbox), })) @@ -120,9 +114,12 @@ impl ComputeDriver for ComputeDriverService { request: Request, ) -> Result, Status> { let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } let deleted = self .driver - .delete_sandbox(&request.sandbox_name) + .delete_sandbox(&request.sandbox_id) .await .map_err(Status::internal)?; Ok(Response::new(DeleteSandboxResponse { deleted })) diff --git a/crates/openshell-driver-podman/src/container.rs b/crates/openshell-driver-podman/src/container.rs index e4a0b79189..e926f549d9 100644 --- a/crates/openshell-driver-podman/src/container.rs +++ b/crates/openshell-driver-podman/src/container.rs @@ -36,19 +36,17 @@ fn is_selinux_enabled() -> bool { false } -/// Label key for the sandbox ID. -pub const LABEL_SANDBOX_ID: &str = "openshell.sandbox-id"; -/// Label key for the sandbox name. -pub const LABEL_SANDBOX_NAME: &str = "openshell.sandbox-name"; -/// Label key for the sandbox namespace. -pub const LABEL_SANDBOX_NAMESPACE: &str = "openshell.sandbox-namespace"; +pub use openshell_core::driver_utils::{ + LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, LABEL_SANDBOX_NAMESPACE, LABEL_SANDBOX_WORKSPACE, +}; + /// Label applied to all managed containers. pub const LABEL_MANAGED: &str = "openshell.managed"; /// Label filter string for list/event queries. pub const LABEL_MANAGED_FILTER: &str = "openshell.managed=true"; /// Container name prefix to avoid collisions with user containers. -const CONTAINER_PREFIX: &str = "openshell-sandbox-"; +const CONTAINER_PREFIX: &str = "openshell-"; /// Volume name prefix. const VOLUME_PREFIX: &str = "openshell-sandbox-"; @@ -144,10 +142,12 @@ fn default_true() -> bool { true } -/// Build a Podman container name from the sandbox name. +/// Build a Podman container name from the sandbox workspace, name, and ID. +/// +/// Format: `openshell-{workspace}--{name}-{id}` #[must_use] -pub fn container_name(sandbox_name: &str) -> String { - format!("{CONTAINER_PREFIX}{sandbox_name}") +pub fn container_name(workspace: &str, name: &str, id: &str) -> String { + format!("{CONTAINER_PREFIX}{workspace}--{name}-{id}") } /// Build the workspace volume name from the sandbox ID. @@ -464,6 +464,7 @@ fn build_labels(sandbox: &DriverSandbox) -> BTreeMap { labels.insert(LABEL_SANDBOX_ID.into(), sandbox.id.clone()); labels.insert(LABEL_SANDBOX_NAME.into(), sandbox.name.clone()); labels.insert(LABEL_SANDBOX_NAMESPACE.into(), sandbox.namespace.clone()); + labels.insert(LABEL_SANDBOX_WORKSPACE.into(), sandbox.workspace.clone()); labels.insert(LABEL_MANAGED.into(), "true".into()); labels @@ -832,7 +833,7 @@ pub fn build_container_spec_with_token_and_gpu_devices( gpu_device_ids: Option<&[String]>, ) -> Result { let image = resolve_image(sandbox, config); - let name = container_name(&sandbox.name); + let name = container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); let vol = volume_name(&sandbox.id); let env = build_env(sandbox, config, image); @@ -1276,8 +1277,11 @@ mod tests { } #[test] - fn container_name_is_prefixed() { - assert_eq!(container_name("my-sandbox"), "openshell-sandbox-my-sandbox"); + fn container_name_is_workspace_qualified() { + assert_eq!( + container_name("default", "my-sandbox", "abc-123"), + "openshell-default--my-sandbox-abc-123" + ); } #[test] @@ -1682,13 +1686,13 @@ mod tests { let mut sandbox = test_sandbox("real-id", "real-name"); sandbox.namespace = "real-namespace".to_string(); let mut label_overrides = std::collections::HashMap::new(); - label_overrides.insert("openshell.sandbox-id".to_string(), "spoofed-id".to_string()); + label_overrides.insert("openshell.ai/sandbox-id".to_string(), "spoofed-id".to_string()); label_overrides.insert( - "openshell.sandbox-name".to_string(), + "openshell.ai/sandbox-name".to_string(), "spoofed-name".to_string(), ); label_overrides.insert( - "openshell.sandbox-namespace".to_string(), + "openshell.ai/sandbox-namespace".to_string(), "spoofed-namespace".to_string(), ); sandbox.spec = Some(DriverSandboxSpec { @@ -1706,20 +1710,20 @@ mod tests { .as_object() .expect("labels should be an object"); assert_eq!( - labels.get("openshell.sandbox-id").and_then(|v| v.as_str()), + labels.get("openshell.ai/sandbox-id").and_then(|v| v.as_str()), Some("real-id"), "openshell.sandbox-id must not be overridden by template labels" ); assert_eq!( labels - .get("openshell.sandbox-name") + .get("openshell.ai/sandbox-name") .and_then(|v| v.as_str()), Some("real-name"), "openshell.sandbox-name must not be overridden by template labels" ); assert_eq!( labels - .get("openshell.sandbox-namespace") + .get("openshell.ai/sandbox-namespace") .and_then(|v| v.as_str()), Some("real-namespace"), "openshell.sandbox-namespace must not be overridden by template labels" @@ -1772,6 +1776,7 @@ mod tests { namespace: String::new(), spec: None, status: None, + workspace: String::new(), } } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index cf639fa265..b606443c98 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -22,7 +22,7 @@ use openshell_core::proto::compute::v1::{ use std::path::Path; use std::sync::Arc; use std::time::Duration; -use tracing::{info, warn}; +use tracing::{debug, info, warn}; impl From for ComputeDriverError { fn from(value: PodmanApiError) -> Self { @@ -62,12 +62,12 @@ struct ValidatedPodmanSandbox<'a> { gpu_requirements: Option<&'a GpuResourceRequirements>, } -/// Construct and validate a container name from a sandbox name. +/// Construct and validate a container name from a sandbox. /// -/// Combines the prefix with the sandbox name and validates the result -/// against Podman's naming rules before any resources are created. -fn validated_container_name(sandbox_name: &str) -> Result { - let name = container::container_name(sandbox_name); +/// Combines the prefix with workspace, name, and ID, then validates the +/// result against Podman's naming rules before any resources are created. +fn validated_container_name(sandbox: &DriverSandbox) -> Result { + let name = container::container_name(&sandbox.workspace, &sandbox.name, &sandbox.id); crate::client::validate_name(&name) .map_err(|e| ComputeDriverError::Precondition(e.to_string()))?; Ok(name) @@ -451,7 +451,7 @@ impl PodmanComputeDriver { // Validate the composed container name early, before creating any // resources (volume), so we don't leave orphans when the name is // invalid. - let name = validated_container_name(&sandbox.name)?; + let name = validated_container_name(sandbox)?; let validated = self.validated_sandbox_create(sandbox).await?; let vol_name = container::volume_name(&sandbox.id); @@ -593,10 +593,29 @@ impl PodmanComputeDriver { Ok(()) } + /// Find the Podman container name for a sandbox by its ID using label lookup. + async fn find_container_name_by_id( + &self, + sandbox_id: &str, + ) -> Result, ComputeDriverError> { + let filter = format!("{LABEL_SANDBOX_ID}={sandbox_id}"); + let entries = self + .client + .list_containers(&filter) + .await + .map_err(ComputeDriverError::from)?; + Ok(entries.first().and_then(|e| e.names.first().cloned())) + } + /// Stop a sandbox container without deleting it. - pub async fn stop_sandbox(&self, sandbox_name: &str) -> Result<(), ComputeDriverError> { - let name = validated_container_name(sandbox_name)?; - info!(sandbox_name = %sandbox_name, container = %name, "Stopping sandbox container"); + pub async fn stop_sandbox(&self, sandbox_id: &str) -> Result<(), ComputeDriverError> { + let name = self + .find_container_name_by_id(sandbox_id) + .await? + .ok_or_else(|| { + ComputeDriverError::Precondition("sandbox container not found".into()) + })?; + info!(sandbox_id = %sandbox_id, container = %name, "Stopping sandbox container"); self.client .stop_container(&name, self.config.stop_timeout_secs) @@ -605,52 +624,24 @@ impl PodmanComputeDriver { } /// Delete a sandbox container and its workspace volume. - pub async fn delete_sandbox( - &self, - sandbox_id: &str, - sandbox_name: &str, - ) -> Result { + pub async fn delete_sandbox(&self, sandbox_id: &str) -> Result { if sandbox_id.is_empty() { return Err(ComputeDriverError::Precondition( "sandbox id is required".into(), )); } - let name = validated_container_name(sandbox_name)?; - info!( - sandbox_id = %sandbox_id, - sandbox_name = %sandbox_name, - container = %name, - "Deleting sandbox container" - ); - // Use the request's stable sandbox ID as the source of truth for - // cleanup. Inspect is only used as a best-effort cross-check so - // cleanup still works if the container is already gone or mislabeled. - match self.client.inspect_container(&name).await { - Ok(inspect) => match inspect.config.labels.get(LABEL_SANDBOX_ID) { - Some(label_id) if label_id != sandbox_id => { - warn!( - sandbox_id = %sandbox_id, - sandbox_name = %sandbox_name, - container = %name, - label_sandbox_id = %label_id, - "Container label sandbox ID did not match delete request; cleaning up using request sandbox_id" - ); - } - None => { - warn!( - sandbox_id = %sandbox_id, - sandbox_name = %sandbox_name, - container = %name, - "Container missing '{}' label; cleaning up using request sandbox_id", - LABEL_SANDBOX_ID, - ); - } - Some(_) => {} - }, - Err(PodmanApiError::NotFound(_)) => {} - Err(e) => return Err(ComputeDriverError::from(e)), - } + let Some(name) = self.find_container_name_by_id(sandbox_id).await? else { + debug!(sandbox_id = %sandbox_id, "Sandbox container not found (already deleted)"); + let vol = container::volume_name(sandbox_id); + if let Err(e) = self.client.remove_volume(&vol).await { + warn!(sandbox_id = %sandbox_id, volume = %vol, error = %e, "Failed to remove workspace volume"); + } + cleanup_sandbox_token_secret(&self.client, &container::token_secret_name(sandbox_id)) + .await; + return Ok(false); + }; + info!(sandbox_id = %sandbox_id, container = %name, "Deleting sandbox container"); // Stop (best-effort). let _ = self @@ -659,7 +650,7 @@ impl PodmanComputeDriver { .await; // Remove container. If NotFound, the container was removed between - // inspect and here (TOCTOU race); proceed with volume cleanup + // list and here (TOCTOU race); proceed with volume cleanup // since the workspace volume is idempotent to remove. let container_existed = match self.client.remove_container(&name).await { Ok(()) => true, @@ -672,7 +663,6 @@ impl PodmanComputeDriver { if let Err(e) = self.client.remove_volume(&vol).await { warn!( sandbox_id = %sandbox_id, - sandbox_name = %sandbox_name, volume = %vol, error = %e, "Failed to remove workspace volume" @@ -684,25 +674,40 @@ impl PodmanComputeDriver { } /// Check whether a sandbox container exists. - pub async fn sandbox_exists(&self, sandbox_name: &str) -> Result { - let name = container::container_name(sandbox_name); - match self.client.inspect_container(&name).await { - Ok(_) => Ok(true), - Err(PodmanApiError::NotFound(_)) => Ok(false), - Err(e) => Err(ComputeDriverError::from(e)), - } + pub async fn sandbox_exists(&self, sandbox_id: &str) -> Result { + let filter = format!("{LABEL_SANDBOX_ID}={sandbox_id}"); + let entries = self + .client + .list_containers(&filter) + .await + .map_err(ComputeDriverError::from)?; + Ok(!entries.is_empty()) } - /// Fetch a single sandbox by name. + /// Fetch a single sandbox by ID. pub async fn get_sandbox( &self, - sandbox_name: &str, + sandbox_id: &str, ) -> Result, ComputeDriverError> { - let name = container::container_name(sandbox_name); - match self.client.inspect_container(&name).await { - Ok(inspect) => Ok(driver_sandbox_from_inspect(&inspect)), - Err(PodmanApiError::NotFound(_)) => Ok(None), - Err(e) => Err(ComputeDriverError::from(e)), + let filter = format!("{LABEL_SANDBOX_ID}={sandbox_id}"); + let entries = self + .client + .list_containers(&filter) + .await + .map_err(ComputeDriverError::from)?; + let Some(entry) = entries.first() else { + return Ok(None); + }; + if entry.state == "running" { + Ok(self + .client + .inspect_container(&entry.id) + .await + .ok() + .and_then(|inspect| driver_sandbox_from_inspect(&inspect)) + .or_else(|| driver_sandbox_from_list_entry(entry))) + } else { + Ok(driver_sandbox_from_list_entry(entry)) } } @@ -1312,6 +1317,7 @@ mod tests { ..Default::default() }), status: None, + workspace: String::new(), } } @@ -1470,24 +1476,22 @@ mod tests { } #[tokio::test] - async fn delete_sandbox_cleans_up_with_request_id_when_container_is_already_gone() { + async fn delete_sandbox_cleans_up_volume_when_container_is_already_gone() { let sandbox_id = "sandbox-123"; - let sandbox_name = "demo"; - let container_name = container::container_name(sandbox_name); let volume_name = container::volume_name(sandbox_id); let (socket_path, request_log, handle) = spawn_podman_stub( "delete-not-found", vec![ - StubResponse::new(StatusCode::NOT_FOUND, r#"{"message":"gone"}"#), - StubResponse::new(StatusCode::NOT_FOUND, r#"{"message":"gone"}"#), - StubResponse::new(StatusCode::NOT_FOUND, r#"{"message":"gone"}"#), + // list_containers returns empty (container already gone) + StubResponse::new(StatusCode::OK, "[]"), + // remove_volume StubResponse::new(StatusCode::NO_CONTENT, ""), ], ); let driver = test_driver(socket_path.clone()); let deleted = driver - .delete_sandbox(sandbox_id, sandbox_name) + .delete_sandbox(sandbox_id) .await .expect("delete should succeed"); @@ -1497,67 +1501,48 @@ mod tests { .lock() .expect("request log lock should not be poisoned") .clone(); + assert!(requests[0].contains("/libpod/containers/json")); assert_eq!( - requests, - vec![ - format!( - "GET {}", - api_path(&format!("/libpod/containers/{container_name}/json")) - ), - format!( - "POST {}", - api_path(&format!( - "/libpod/containers/{container_name}/stop?timeout=10" - )) - ), - format!( - "DELETE {}", - api_path(&format!( - "/libpod/containers/{container_name}?force=true&v=true" - )) - ), - format!( - "DELETE {}", - api_path(&format!("/libpod/volumes/{volume_name}")) - ), - ] + requests[1], + format!( + "DELETE {}", + api_path(&format!("/libpod/volumes/{volume_name}")) + ) ); let _ = fs::remove_file(socket_path); } #[tokio::test] - async fn delete_sandbox_uses_request_id_when_container_label_disagrees() { + async fn delete_sandbox_finds_container_by_label_and_removes() { let sandbox_id = "sandbox-request-id"; - let sandbox_name = "demo"; - let container_name = container::container_name(sandbox_name); + let container_name = "openshell-default--demo-sandbox-request-id"; let volume_name = container::volume_name(sandbox_id); - let inspect_body = serde_json::json!({ + let list_body = serde_json::json!([{ "Id": "container-id", - "Name": format!("/{container_name}"), - "State": { - "Status": "running", - "Running": true - }, - "Config": { - "Labels": { - LABEL_SANDBOX_ID: "sandbox-label-id" - } + "Names": [container_name], + "State": "running", + "Labels": { + LABEL_SANDBOX_ID: sandbox_id } - }) + }]) .to_string(); let (socket_path, request_log, handle) = spawn_podman_stub( - "delete-mismatch", + "delete-label-lookup", vec![ - StubResponse::new(StatusCode::OK, inspect_body), + // list_containers by label + StubResponse::new(StatusCode::OK, list_body), + // stop_container StubResponse::new(StatusCode::NO_CONTENT, ""), + // remove_container StubResponse::new(StatusCode::NO_CONTENT, ""), + // remove_volume StubResponse::new(StatusCode::NO_CONTENT, ""), ], ); let driver = test_driver(socket_path.clone()); let deleted = driver - .delete_sandbox(sandbox_id, sandbox_name) + .delete_sandbox(sandbox_id) .await .expect("delete should succeed"); @@ -1567,12 +1552,15 @@ mod tests { .lock() .expect("request log lock should not be poisoned") .clone(); + assert!(requests[0].contains("/libpod/containers/json")); + assert!(requests[1].contains(&format!("/libpod/containers/{container_name}/stop"))); + assert!(requests[2].contains(&format!("/libpod/containers/{container_name}"))); assert_eq!( - requests[3..], - [format!( + requests[3], + format!( "DELETE {}", api_path(&format!("/libpod/volumes/{volume_name}")) - )] + ) ); let _ = fs::remove_file(socket_path); } diff --git a/crates/openshell-driver-podman/src/grpc.rs b/crates/openshell-driver-podman/src/grpc.rs index 4840ee2810..1f7cce89a4 100644 --- a/crates/openshell-driver-podman/src/grpc.rs +++ b/crates/openshell-driver-podman/src/grpc.rs @@ -60,23 +60,17 @@ impl ComputeDriver for ComputeDriverService { request: Request, ) -> Result, Status> { let request = request.into_inner(); - if request.sandbox_name.is_empty() { - return Err(Status::invalid_argument("sandbox_name is required")); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); } let sandbox = self .driver - .get_sandbox(&request.sandbox_name) + .get_sandbox(&request.sandbox_id) .await .map_err(Status::from)? .ok_or_else(|| Status::not_found("sandbox not found"))?; - if !request.sandbox_id.is_empty() && request.sandbox_id != sandbox.id { - return Err(Status::failed_precondition( - "sandbox_id did not match the fetched sandbox", - )); - } - Ok(Response::new(GetSandboxResponse { sandbox: Some(sandbox), })) @@ -110,11 +104,11 @@ impl ComputeDriver for ComputeDriverService { request: Request, ) -> Result, Status> { let request = request.into_inner(); - if request.sandbox_name.is_empty() { - return Err(Status::invalid_argument("sandbox_name is required")); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); } self.driver - .stop_sandbox(&request.sandbox_name) + .stop_sandbox(&request.sandbox_id) .await .map_err(Status::from)?; Ok(Response::new(StopSandboxResponse {})) @@ -128,12 +122,9 @@ impl ComputeDriver for ComputeDriverService { if request.sandbox_id.is_empty() { return Err(Status::invalid_argument("sandbox_id is required")); } - if request.sandbox_name.is_empty() { - return Err(Status::invalid_argument("sandbox_name is required")); - } let deleted = self .driver - .delete_sandbox(&request.sandbox_id, &request.sandbox_name) + .delete_sandbox(&request.sandbox_id) .await .map_err(Status::from)?; Ok(Response::new(DeleteSandboxResponse { deleted })) @@ -190,24 +181,6 @@ mod tests { format!("/v5.0.0{path}") } - #[tokio::test] - async fn delete_sandbox_rejects_missing_sandbox_name() { - let service = test_service(unique_socket_path("missing-name")); - - let err = ComputeDriver::delete_sandbox( - &service, - Request::new(DeleteSandboxRequest { - sandbox_id: "sandbox-123".to_string(), - sandbox_name: String::new(), - }), - ) - .await - .expect_err("missing sandbox_name should fail"); - - assert_eq!(err.code(), tonic::Code::InvalidArgument); - assert_eq!(err.message(), "sandbox_name is required"); - } - #[tokio::test] async fn delete_sandbox_rejects_missing_sandbox_id() { let service = test_service(unique_socket_path("missing-id")); @@ -229,15 +202,13 @@ mod tests { #[tokio::test] async fn delete_sandbox_forwards_request_sandbox_id_to_driver_cleanup() { let sandbox_id = "sandbox-abc"; - let sandbox_name = "demo"; - let container_name = container::container_name(sandbox_name); let volume_name = container::volume_name(sandbox_id); let (socket_path, request_log, handle) = spawn_podman_stub( "forward-id", vec![ - StubResponse::new(StatusCode::NOT_FOUND, r#"{"message":"gone"}"#), - StubResponse::new(StatusCode::NOT_FOUND, r#"{"message":"gone"}"#), - StubResponse::new(StatusCode::NOT_FOUND, r#"{"message":"gone"}"#), + // list_containers returns empty (container already gone) + StubResponse::new(StatusCode::OK, "[]"), + // remove_volume StubResponse::new(StatusCode::NO_CONTENT, ""), ], ); @@ -247,7 +218,7 @@ mod tests { &service, Request::new(DeleteSandboxRequest { sandbox_id: sandbox_id.to_string(), - sandbox_name: sandbox_name.to_string(), + sandbox_name: "demo".to_string(), }), ) .await @@ -263,30 +234,13 @@ mod tests { .lock() .expect("request log lock should not be poisoned") .clone(); + assert!(requests[0].contains("/libpod/containers/json")); assert_eq!( - requests, - vec![ - format!( - "GET {}", - api_path(&format!("/libpod/containers/{container_name}/json")) - ), - format!( - "POST {}", - api_path(&format!( - "/libpod/containers/{container_name}/stop?timeout=10" - )) - ), - format!( - "DELETE {}", - api_path(&format!( - "/libpod/containers/{container_name}?force=true&v=true" - )) - ), - format!( - "DELETE {}", - api_path(&format!("/libpod/volumes/{volume_name}")) - ), - ] + requests[1], + format!( + "DELETE {}", + api_path(&format!("/libpod/volumes/{volume_name}")) + ) ); let _ = std::fs::remove_file(socket_path); } diff --git a/crates/openshell-driver-podman/src/watcher.rs b/crates/openshell-driver-podman/src/watcher.rs index 54606ea442..6babdaee04 100644 --- a/crates/openshell-driver-podman/src/watcher.rs +++ b/crates/openshell-driver-podman/src/watcher.rs @@ -7,7 +7,9 @@ use crate::client::{ ContainerInspect, ContainerListEntry, ContainerState, HealthState, PodmanApiError, PodmanClient, PodmanEvent, }; -use crate::container::{LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, short_id}; +use crate::container::{ + LABEL_MANAGED_FILTER, LABEL_SANDBOX_ID, LABEL_SANDBOX_NAME, LABEL_SANDBOX_WORKSPACE, short_id, +}; use futures::Stream; use openshell_core::ComputeDriverError; use openshell_core::proto::compute::v1::{ @@ -215,9 +217,16 @@ async fn map_podman_event( .get(LABEL_SANDBOX_NAME) .cloned() .unwrap_or_default(); + let workspace = event + .actor + .attributes + .get(LABEL_SANDBOX_WORKSPACE) + .cloned() + .unwrap_or_default(); Some(sandbox_event(build_driver_sandbox( sandbox_id.clone(), sandbox_name, + workspace, String::new(), short_id(container_id), DriverCondition { @@ -247,6 +256,7 @@ async fn map_podman_event( fn build_driver_sandbox( sandbox_id: String, sandbox_name: String, + workspace: String, instance_name: String, instance_id: String, condition: DriverCondition, @@ -265,6 +275,7 @@ fn build_driver_sandbox( conditions: vec![condition], deleting, }), + workspace, } } @@ -277,6 +288,12 @@ pub fn driver_sandbox_from_inspect(inspect: &ContainerInspect) -> Option Option Option ( @@ -316,6 +339,7 @@ pub fn driver_sandbox_from_list_entry(entry: &ContainerListEntry) -> Option Result { + pub async fn delete_sandbox(&self, workspace: &str, name: &str) -> Result { let _guard = self.sync_lock.lock().await; // Resolve sandbox ID from name let sandbox = self .store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))?; @@ -718,6 +720,7 @@ impl ComputeRuntime { Sandbox::object_type(), &id, &name, + sandbox.object_workspace(), &sandbox.encode_to_vec(), labels_json.as_deref(), WriteCondition::MatchResourceVersion(expected_resource_version), @@ -774,7 +777,7 @@ impl ComputeRuntime { let records = self .store - .list(Sandbox::object_type(), 1000, 0) + .list_by_type(Sandbox::object_type(), 1000, 0) .await .map_err(|e| e.to_string())?; @@ -1074,7 +1077,7 @@ impl ComputeRuntime { let records = self .store - .list(Sandbox::object_type(), 500, 0) + .list_by_type(Sandbox::object_type(), 500, 0) .await .map_err(|e| e.to_string())?; @@ -1172,6 +1175,7 @@ impl ComputeRuntime { if supervisor_promoted { ensure_supervisor_ready_status(&mut status, &sandbox_name); } + let workspace = incoming.workspace.clone(); let mut sandbox = Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: incoming.id.clone(), @@ -1179,7 +1183,7 @@ impl ComputeRuntime { created_at_ms: now_ms, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: std::collections::HashMap::new(), + workspace, }), spec: None, status, @@ -1191,6 +1195,7 @@ impl ComputeRuntime { Sandbox::object_type(), &incoming.id, sandbox.object_name(), + sandbox.object_workspace(), &sandbox.encode_to_vec(), None, WriteCondition::MustCreate, @@ -1392,11 +1397,12 @@ impl ComputeRuntime { } async fn cleanup_sandbox_owned_records(&self, sandbox: &Sandbox) { - self.cleanup_sandbox_ssh_sessions(sandbox.object_id()).await; + self.cleanup_sandbox_ssh_sessions(sandbox.object_id(), sandbox.object_workspace()) + .await; if let Err(e) = self .store - .delete_by_name(SANDBOX_SETTINGS_OBJECT_TYPE, sandbox.object_name()) + .delete_by_name(SANDBOX_SETTINGS_OBJECT_TYPE, sandbox.object_workspace(), sandbox.object_name()) .await { warn!( @@ -1408,8 +1414,12 @@ impl ComputeRuntime { } } - async fn cleanup_sandbox_ssh_sessions(&self, sandbox_id: &str) { - if let Ok(records) = self.store.list(SshSession::object_type(), 1000, 0).await { + async fn cleanup_sandbox_ssh_sessions(&self, sandbox_id: &str, workspace: &str) { + if let Ok(records) = self + .store + .list(SshSession::object_type(), workspace, 1000, 0) + .await + { for record in records { if let Ok(session) = SshSession::decode(record.payload.as_slice()) && session.sandbox_id == sandbox_id @@ -1582,6 +1592,7 @@ fn driver_sandbox_from_public( .map(|spec| driver_sandbox_spec_from_public(spec, driver_name)) .transpose()?, status: sandbox.status.as_ref().map(driver_status_from_public), + workspace: sandbox.object_workspace().to_string(), }) } @@ -2415,7 +2426,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), ..Default::default() }; @@ -2431,7 +2442,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), sandbox_id: sandbox_id.to_string(), token: format!("token-{id}"), @@ -2845,6 +2856,7 @@ mod tests { }], deleting: false, }), + workspace: "default".to_string(), }) .await .unwrap(); @@ -2887,6 +2899,7 @@ mod tests { namespace: "default".to_string(), spec: None, status: None, + workspace: "default".to_string(), }) .await .unwrap(); @@ -2935,6 +2948,7 @@ mod tests { "Starting", "VM is starting", ))), + workspace: "default".to_string(), }) .await .unwrap(); @@ -3054,6 +3068,7 @@ mod tests { }], deleting: false, }), + workspace: "default".to_string(), }], current_sandboxes: vec![DriverSandbox { id: "sb-1".to_string(), @@ -3074,6 +3089,7 @@ mod tests { }], deleting: false, }), + workspace: "default".to_string(), }], })) .await; @@ -3122,6 +3138,7 @@ mod tests { "DependenciesNotReady", "Pod exists with phase: Pending; Service Exists", ))), + workspace: "default".to_string(), }], current_sandboxes: vec![DriverSandbox { id: "sb-1".to_string(), @@ -3135,6 +3152,7 @@ mod tests { message: "Pod is Ready".to_string(), last_transition_time: String::new(), })), + workspace: "default".to_string(), }], })) .await; @@ -3176,6 +3194,7 @@ mod tests { }], deleting: false, }), + workspace: "default".to_string(), }], ..Default::default() })) @@ -3214,6 +3233,7 @@ mod tests { SANDBOX_SETTINGS_OBJECT_TYPE, "settings-sb-1", sandbox.object_name(), + sandbox.object_workspace(), br#"{"revision":1,"settings":{}}"#, None, ) @@ -3246,7 +3266,7 @@ mod tests { assert!( runtime .store - .get_by_name(SANDBOX_SETTINGS_OBJECT_TYPE, sandbox.object_name()) + .get_by_name(SANDBOX_SETTINGS_OBJECT_TYPE, sandbox.object_workspace(), sandbox.object_name()) .await .unwrap() .is_none() @@ -3521,7 +3541,12 @@ mod tests { runtime.validate_sandbox_create(&sandbox).await.unwrap(); runtime.create_sandbox(sandbox, None).await.unwrap(); - assert!(runtime.delete_sandbox("uds-sandbox").await.unwrap()); + assert!( + runtime + .delete_sandbox("default", "uds-sandbox") + .await + .unwrap() + ); let calls = driver.calls(); assert_eq!(calls.len(), 4, "unexpected calls: {calls:?}"); @@ -3577,7 +3602,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }); let created = runtime.create_sandbox(sandbox, None).await.unwrap(); @@ -3653,7 +3678,7 @@ mod tests { .expect("should have one successful creation"); let retrieved = runtime .store - .get_message_by_name::("test-concurrent") + .get_message_by_name::("default", "test-concurrent") .await .unwrap(); assert!( @@ -3666,4 +3691,75 @@ mod tests { "retrieved sandbox should match created sandbox" ); } + + #[test] + fn driver_sandbox_from_public_populates_workspace() { + let sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "sb-1".to_string(), + name: "work".to_string(), + workspace: "alpha".to_string(), + ..Default::default() + }), + ..Default::default() + }; + let driver_sb = driver_sandbox_from_public(&sandbox, "kubernetes").unwrap(); + assert_eq!(driver_sb.workspace, "alpha"); + assert_eq!(driver_sb.name, "work"); + assert_eq!(driver_sb.id, "sb-1"); + } + + #[tokio::test] + async fn apply_sandbox_update_uses_workspace_from_driver() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-w1".to_string(), + name: "work".to_string(), + namespace: "default".to_string(), + spec: None, + status: Some(make_driver_status(make_driver_condition( + "DependenciesNotReady", + "Provisioning", + ))), + workspace: "team-ml".to_string(), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-w1") + .await + .unwrap() + .unwrap(); + assert_eq!(stored.object_workspace(), "team-ml"); + } + + #[tokio::test] + async fn apply_sandbox_update_preserves_workspace() { + let runtime = test_runtime(Arc::new(TestDriver::default())).await; + runtime + .apply_sandbox_update(DriverSandbox { + id: "sb-w2".to_string(), + name: "work".to_string(), + namespace: "default".to_string(), + spec: None, + status: Some(make_driver_status(make_driver_condition( + "DependenciesNotReady", + "Provisioning", + ))), + workspace: "alpha".to_string(), + }) + .await + .unwrap(); + + let stored = runtime + .store + .get_message::("sb-w2") + .await + .unwrap() + .unwrap(); + assert_eq!(stored.object_workspace(), "alpha"); + } } diff --git a/crates/openshell-server/src/grpc/auth_rpc.rs b/crates/openshell-server/src/grpc/auth_rpc.rs index 944b9b3ed7..cd8edf63a0 100644 --- a/crates/openshell-server/src/grpc/auth_rpc.rs +++ b/crates/openshell-server/src/grpc/auth_rpc.rs @@ -200,7 +200,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::default(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 36f3e64cbb..51dd33b316 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -9,41 +9,45 @@ pub mod provider; mod sandbox; mod service; mod validation; +pub mod workspace; use openshell_core::proto::{ - ApproveAllDraftChunksRequest, ApproveAllDraftChunksResponse, ApproveDraftChunkRequest, - ApproveDraftChunkResponse, AttachSandboxProviderRequest, AttachSandboxProviderResponse, - ClearDraftChunksRequest, ClearDraftChunksResponse, ComputeDriverCapabilities, - ComputeDriverInfo, ConfigureProviderRefreshRequest, ConfigureProviderRefreshResponse, + AddWorkspaceMemberRequest, AddWorkspaceMemberResponse, ApproveAllDraftChunksRequest, + ApproveAllDraftChunksResponse, ApproveDraftChunkRequest, ApproveDraftChunkResponse, + AttachSandboxProviderRequest, AttachSandboxProviderResponse, ClearDraftChunksRequest, + ClearDraftChunksResponse, ConfigureProviderRefreshRequest, ConfigureProviderRefreshResponse, CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, - DeleteProviderProfileRequest, DeleteProviderProfileResponse, DeleteProviderRefreshRequest, - DeleteProviderRefreshResponse, DeleteProviderRequest, DeleteProviderResponse, - DeleteSandboxRequest, DeleteSandboxResponse, DeleteServiceRequest, DeleteServiceResponse, + CreateWorkspaceRequest, CreateWorkspaceResponse, DeleteProviderProfileRequest, + DeleteProviderProfileResponse, DeleteProviderRefreshRequest, DeleteProviderRefreshResponse, + DeleteProviderRequest, DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, + DeleteServiceRequest, DeleteServiceResponse, DeleteWorkspaceRequest, DeleteWorkspaceResponse, DetachSandboxProviderRequest, DetachSandboxProviderResponse, EditDraftChunkRequest, EditDraftChunkResponse, ExecSandboxEvent, ExecSandboxInput, ExecSandboxRequest, ExposeServiceRequest, GatewayMessage, GetDraftHistoryRequest, GetDraftHistoryResponse, GetDraftPolicyRequest, GetDraftPolicyResponse, GetGatewayConfigRequest, - GetGatewayConfigResponse, GetGatewayInfoRequest, GetGatewayInfoResponse, - GetProviderProfileRequest, GetProviderRefreshStatusRequest, GetProviderRefreshStatusResponse, - GetProviderRequest, GetSandboxConfigRequest, GetSandboxConfigResponse, GetSandboxLogsRequest, - GetSandboxLogsResponse, GetSandboxPolicyStatusRequest, GetSandboxPolicyStatusResponse, + GetGatewayConfigResponse, GetProviderProfileRequest, GetProviderRefreshStatusRequest, + GetProviderRefreshStatusResponse, GetProviderRequest, GetSandboxConfigRequest, + GetSandboxConfigResponse, GetSandboxLogsRequest, GetSandboxLogsResponse, + GetSandboxPolicyStatusRequest, GetSandboxPolicyStatusResponse, GetSandboxProviderEnvironmentRequest, GetSandboxProviderEnvironmentResponse, GetSandboxRequest, - GetServiceRequest, HealthRequest, HealthResponse, ImportProviderProfilesRequest, - ImportProviderProfilesResponse, IssueSandboxTokenRequest, IssueSandboxTokenResponse, - LintProviderProfilesRequest, LintProviderProfilesResponse, ListProviderProfilesRequest, - ListProviderProfilesResponse, ListProvidersRequest, ListProvidersResponse, - ListSandboxPoliciesRequest, ListSandboxPoliciesResponse, ListSandboxProvidersRequest, - ListSandboxProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, ListServicesRequest, - ListServicesResponse, ProviderProfileResponse, ProviderResponse, PushSandboxLogsRequest, - PushSandboxLogsResponse, RefreshSandboxTokenRequest, RefreshSandboxTokenResponse, - RejectDraftChunkRequest, RejectDraftChunkResponse, RelayFrame, ReportPolicyStatusRequest, - ReportPolicyStatusResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, - RotateProviderCredentialRequest, RotateProviderCredentialResponse, SandboxResponse, - SandboxStreamEvent, ServiceEndpointResponse, ServiceStatus, SubmitPolicyAnalysisRequest, - SubmitPolicyAnalysisResponse, SupervisorMessage, TcpForwardFrame, UndoDraftChunkRequest, - UndoDraftChunkResponse, UpdateConfigRequest, UpdateConfigResponse, - UpdateProviderProfilesRequest, UpdateProviderProfilesResponse, UpdateProviderRequest, - WatchSandboxRequest, open_shell_server::OpenShell, + GetServiceRequest, GetWorkspaceRequest, GetWorkspaceResponse, HealthRequest, HealthResponse, + ImportProviderProfilesRequest, ImportProviderProfilesResponse, IssueSandboxTokenRequest, + IssueSandboxTokenResponse, LintProviderProfilesRequest, LintProviderProfilesResponse, + ListProviderProfilesRequest, ListProviderProfilesResponse, ListProvidersRequest, + ListProvidersResponse, ListSandboxPoliciesRequest, ListSandboxPoliciesResponse, + ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, + ListSandboxesResponse, ListServicesRequest, ListServicesResponse, ListWorkspaceMembersRequest, + ListWorkspaceMembersResponse, ListWorkspacesRequest, ListWorkspacesResponse, + ProviderProfileResponse, ProviderResponse, PushSandboxLogsRequest, PushSandboxLogsResponse, + RefreshSandboxTokenRequest, RefreshSandboxTokenResponse, RejectDraftChunkRequest, + RejectDraftChunkResponse, RelayFrame, RemoveWorkspaceMemberRequest, + RemoveWorkspaceMemberResponse, ReportPolicyStatusRequest, ReportPolicyStatusResponse, + RevokeSshSessionRequest, RevokeSshSessionResponse, RotateProviderCredentialRequest, + RotateProviderCredentialResponse, SandboxResponse, SandboxStreamEvent, ServiceEndpointResponse, + ServiceStatus, SubmitPolicyAnalysisRequest, SubmitPolicyAnalysisResponse, SupervisorMessage, + TcpForwardFrame, UndoDraftChunkRequest, UndoDraftChunkResponse, UpdateConfigRequest, + UpdateConfigResponse, UpdateProviderProfilesRequest, UpdateProviderProfilesResponse, + UpdateProviderRequest, WatchSandboxRequest, open_shell_server::OpenShell, }; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; @@ -245,6 +249,9 @@ impl OpenShell for OpenShellService { type WatchSandboxStream = ReceiverStream>; + // TODO(phase2): data-plane RPCs do not carry a workspace field. Add + // workspace verification to confirm the sandbox belongs to the caller's + // workspace before proxying. #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn watch_sandbox( &self, @@ -261,6 +268,8 @@ impl OpenShell for OpenShellService { sandbox::handle_get_sandbox(&self.state, request).await } + // TODO(phase2): all_workspaces flag is currently accessible to any + // authenticated user. Restrict to Platform Admin role in Phase 2. #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn list_sandboxes( &self, @@ -305,6 +314,7 @@ impl OpenShell for OpenShellService { type ExecSandboxStream = ReceiverStream>; + // TODO(phase2): no workspace field — see watch_sandbox comment. #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn exec_sandbox( &self, @@ -316,6 +326,7 @@ impl OpenShell for OpenShellService { type ForwardTcpStream = Pin> + Send + 'static>>; + // TODO(phase2): no workspace field — see watch_sandbox comment. #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn forward_tcp( &self, @@ -336,6 +347,7 @@ impl OpenShell for OpenShellService { // --- SSH sessions --- + // TODO(phase2): no workspace field — see watch_sandbox comment. #[rpc_auth(auth = "bearer", scope = "sandbox:write", role = "user")] async fn create_ssh_session( &self, @@ -360,6 +372,8 @@ impl OpenShell for OpenShellService { service::handle_get_service(&self.state, request).await } + // TODO(phase2): all_workspaces flag is currently accessible to any + // authenticated user. Restrict to Platform Admin role in Phase 2. #[rpc_auth(auth = "bearer", scope = "sandbox:read", role = "user")] async fn list_services( &self, @@ -402,6 +416,8 @@ impl OpenShell for OpenShellService { provider::handle_get_provider(&self.state, request).await } + // TODO(phase2): all_workspaces flag is currently accessible to any + // authenticated user. Restrict to Platform Admin role in Phase 2. #[rpc_auth(auth = "bearer", scope = "provider:read", role = "user")] async fn list_providers( &self, @@ -698,6 +714,64 @@ impl OpenShell for OpenShellService { crate::supervisor_session::handle_relay_stream(&self.state.supervisor_sessions, request) .await } + + // --- Workspace management --- + + #[rpc_auth(auth = "bearer", scope = "workspace:write", role = "admin")] + async fn create_workspace( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_create_workspace(&self.state, request).await + } + + #[rpc_auth(auth = "bearer", scope = "workspace:read", role = "user")] + async fn get_workspace( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_get_workspace(&self.state, request).await + } + + #[rpc_auth(auth = "bearer", scope = "workspace:read", role = "user")] + async fn list_workspaces( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_list_workspaces(&self.state, request).await + } + + #[rpc_auth(auth = "bearer", scope = "workspace:write", role = "admin")] + async fn delete_workspace( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_delete_workspace(&self.state, request).await + } + + #[rpc_auth(auth = "bearer", scope = "workspace:write", role = "admin")] + async fn add_workspace_member( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_add_workspace_member(&self.state, request).await + } + + #[rpc_auth(auth = "bearer", scope = "workspace:write", role = "admin")] + async fn remove_workspace_member( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_remove_workspace_member(&self.state, request).await + } + + #[rpc_auth(auth = "bearer", scope = "workspace:read", role = "user")] + async fn list_workspace_members( + &self, + request: Request, + ) -> Result, Status> { + workspace::handle_list_workspace_members(&self.state, request).await + } } // --------------------------------------------------------------------------- @@ -725,6 +799,7 @@ pub mod test_support { .await .unwrap(), ); + crate::ensure_default_workspace(&store).await.unwrap(); let compute = new_test_runtime(store.clone()).await; Arc::new(ServerState::new( Config::new(None).with_database_url("sqlite::memory:?cache=shared"), diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 6fccf1eade..2127fa6294 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -12,11 +12,10 @@ use crate::ServerState; use crate::auth::principal::Principal; -use crate::persistence::{DraftChunkRecord, ObjectId, ObjectName, ObjectType, PolicyRecord, Store}; -use crate::policy_store::{AtomicPolicyRevisionWrite, PolicyStoreExt}; -use crate::provider_profile_sources::EffectiveProviderProfileCatalog; -#[cfg(test)] -use crate::provider_profile_sources::ProviderProfileSources; +use crate::persistence::{ + DraftChunkRecord, ObjectId, ObjectName, ObjectType, ObjectWorkspace, PolicyRecord, Store, +}; +use crate::policy_store::PolicyStoreExt; use openshell_core::net::is_internal_ip; use openshell_core::proto::policy_merge_operation; use openshell_core::proto::setting_value; @@ -522,14 +521,14 @@ fn run_prover_findings( /// with the merged policy the prover validates against. async fn build_credential_set_for_sandbox_with_catalog( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, provider_names: &[String], ) -> Result { let mut credentials = Vec::new(); for name in provider_names { let Some(provider) = store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("failed to fetch provider '{name}': {e}")))? else { @@ -538,16 +537,32 @@ async fn build_credential_set_for_sandbox_with_catalog( }; let provider_type = provider.r#type.trim(); - let profile_id = normalize_provider_type(provider_type).unwrap_or(provider_type); - let Some(profile) = - super::provider::get_provider_type_profile_with_catalog(catalog, profile_id) - else { - warn!( - provider_name = %name, + let profile = if let Some(canonical_type) = normalize_provider_type(provider_type) { + let Some(profile) = get_default_profile(canonical_type) else { + warn!( + provider_name = %name, + provider_type, + "legacy provider type has no profile; skipping credential entry" + ); + continue; + }; + profile.clone() + } else { + let Some(profile) = super::provider::get_provider_type_profile( + store, + &provider.profile_workspace, provider_type, - "provider type has no profile; skipping credential entry" - ); - continue; + ) + .await? + else { + warn!( + provider_name = %name, + provider_type, + "provider type has no profile; skipping credential entry" + ); + continue; + }; + profile }; let target_hosts: Vec = profile @@ -881,6 +896,7 @@ async fn self_reject_mechanistic_if_already_covered( /// manual. async fn resolve_proposal_approval_mode( store: &Store, + workspace: &str, sandbox_name: &str, ) -> Result<(bool, &'static str), Status> { let global = load_global_settings(store).await?; @@ -890,7 +906,7 @@ async fn resolve_proposal_approval_mode( return Ok((value == "auto", "gateway")); } - let sandbox = load_sandbox_settings(store, sandbox_name).await?; + let sandbox = load_sandbox_settings(store, workspace, sandbox_name).await?; if let Some(StoredSettingValue::String(value)) = sandbox.settings.get(settings::PROPOSAL_APPROVAL_MODE_KEY) { @@ -903,6 +919,7 @@ async fn resolve_proposal_approval_mode( async fn auto_approve_chunk( state: &Arc, sandbox_id: &str, + workspace: &str, sandbox_name: &str, chunk_id: &str, source: &str, @@ -930,7 +947,8 @@ async fn auto_approve_chunk( return Ok(()); } - let (version, hash) = merge_chunk_into_policy(state.store.as_ref(), sandbox_id, &chunk).await?; + let (version, hash) = + merge_chunk_into_policy(state.store.as_ref(), sandbox_id, workspace, &chunk).await?; let chunk_summary = summarize_draft_chunk_rule(&chunk)?; let now_ms = current_time_ms(); @@ -979,7 +997,7 @@ async fn auto_approve_chunk( // agent-authored proposal validation slice. async fn current_effective_policy_for_sandbox( state: &ServerState, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, sandbox: &Sandbox, sandbox_id: &str, ) -> Result { @@ -1016,12 +1034,9 @@ async fn current_effective_policy_for_sandbox( .as_ref() .map(|spec| spec.providers.clone()) .unwrap_or_default(); - let provider_layers = profile_provider_policy_layers_with_catalog( - state.store.as_ref(), - catalog, - &provider_names, - ) - .await?; + let provider_layers = + profile_provider_policy_layers(state.store.as_ref(), workspace, &provider_names) + .await?; if !provider_layers.is_empty() { policy = compose_effective_policy(&policy, &provider_layers); } @@ -1167,11 +1182,12 @@ async fn persist_existing_policy_projection( async fn resolve_sandbox_by_name_for_principal( store: &Store, + workspace: &str, principal: &Principal, name: &str, ) -> Result { let sandbox = store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))?; @@ -1218,6 +1234,7 @@ pub(super) async fn handle_get_sandbox_config( .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; + let workspace = sandbox.object_workspace().to_string(); let sandbox_provider_names = sandbox .spec .as_ref() @@ -1271,7 +1288,7 @@ pub(super) async fn handle_get_sandbox_config( if let Err(e) = state .store - .put_policy_revision(&policy_id, &sandbox_id, 1, &payload, &hash) + .put_policy_revision(&policy_id, &sandbox_id, &workspace, 1, &payload, &hash) .await { warn!( @@ -1303,7 +1320,7 @@ pub(super) async fn handle_get_sandbox_config( let global_settings = load_global_settings(state.store.as_ref()).await?; let sandbox_settings = - load_sandbox_settings(state.store.as_ref(), sandbox.object_name()).await?; + load_sandbox_settings(state.store.as_ref(), &workspace, sandbox.object_name()).await?; let providers_v2_enabled = bool_setting_enabled(&global_settings, settings::PROVIDERS_V2_ENABLED_KEY)?; @@ -1329,9 +1346,9 @@ pub(super) async fn handle_get_sandbox_config( && !matches!(policy_source, PolicySource::Global) && let Some(source_policy) = policy.as_ref() { - let provider_layers = profile_provider_policy_layers_with_catalog( + let provider_layers = profile_provider_policy_layers( state.store.as_ref(), - &provider_profile_catalog, + &workspace, &sandbox_provider_names, ) .await?; @@ -1344,12 +1361,9 @@ pub(super) async fn handle_get_sandbox_config( let settings = merge_effective_settings(&global_settings, &sandbox_settings)?; let config_revision = compute_config_revision(policy.as_ref(), &settings, policy_source); - let provider_env_revision = compute_provider_env_revision_with_catalog( - state.store.as_ref(), - &provider_profile_catalog, - &sandbox_provider_names, - ) - .await?; + let provider_env_revision = + compute_provider_env_revision(state.store.as_ref(), &workspace, &sandbox_provider_names) + .await?; Ok(Response::new(GetSandboxConfigResponse { policy, @@ -1360,12 +1374,14 @@ pub(super) async fn handle_get_sandbox_config( policy_source: policy_source.into(), global_policy_version, provider_env_revision, + workspace, })) } #[cfg(test)] async fn compute_provider_env_revision( store: &Store, + workspace: &str, provider_names: &[String], ) -> Result { let catalog = ProviderProfileSources::with_default_sources() @@ -1385,7 +1401,7 @@ pub(super) async fn compute_provider_env_revision_with_catalog( for provider_name in provider_names { hasher.update(provider_name.as_bytes()); match store - .get_by_name(Provider::object_type(), provider_name) + .get_by_name(Provider::object_type(), workspace, provider_name) .await .map_err(|e| { Status::internal(format!("fetch provider '{provider_name}' failed: {e}")) @@ -1398,7 +1414,13 @@ pub(super) async fn compute_provider_env_revision_with_catalog( Status::internal(format!("decode provider '{provider_name}' failed: {e}")) })?; hasher.update(provider.r#type.as_bytes()); - hash_provider_profile_revision(catalog, &provider.r#type, &mut hasher); + hash_provider_profile_revision( + store, + &provider.profile_workspace, + &provider.r#type, + &mut hasher, + ) + .await?; let mut credential_keys: Vec<_> = provider.credentials.keys().collect(); credential_keys.sort(); @@ -1424,18 +1446,47 @@ pub(super) async fn compute_provider_env_revision_with_catalog( )?)) } -fn hash_provider_profile_revision( - catalog: &EffectiveProviderProfileCatalog, +async fn hash_provider_profile_revision( + store: &Store, + profile_workspace: &str, provider_type: &str, hasher: &mut Sha256, -) { - let profile_id = normalize_provider_type(provider_type).unwrap_or(provider_type); - catalog.hash_profile_revision(profile_id, hasher); +) -> Result<(), Status> { + if let Some(profile) = get_default_profile(provider_type) { + hasher.update(b"builtin-profile"); + hasher.update(profile.to_proto().encode_to_vec()); + return Ok(()); + } + + hasher.update(b"custom-profile"); + match store + .get_by_name( + openshell_core::proto::StoredProviderProfile::object_type(), + profile_workspace, + provider_type, + ) + .await + .map_err(|e| { + Status::internal(format!( + "fetch provider profile '{provider_type}' failed: {e}" + )) + })? { + Some(record) => { + hasher.update(record.id.as_bytes()); + hasher.update(record.updated_at_ms.to_le_bytes()); + hasher.update(record.payload.as_slice()); + } + None => { + hasher.update(b"missing"); + } + } + Ok(()) } #[cfg(test)] async fn profile_provider_policy_layers( store: &Store, + workspace: &str, provider_names: &[String], ) -> Result, Status> { let catalog = ProviderProfileSources::with_default_sources() @@ -1453,22 +1504,38 @@ async fn profile_provider_policy_layers_with_catalog( for name in provider_names { let provider = store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("failed to fetch provider '{name}': {e}")))? .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; let provider_type = provider.r#type.trim(); - let profile_id = normalize_provider_type(provider_type).unwrap_or(provider_type); - let Some(profile) = - super::provider::get_provider_type_profile_with_catalog(catalog, profile_id) - else { - warn!( - provider_name = %name, + let profile = if let Some(canonical_type) = normalize_provider_type(provider_type) { + let Some(profile) = get_default_profile(canonical_type) else { + warn!( + provider_name = %name, + provider_type, + "legacy provider type has no profile; skipping provider policy layer" + ); + continue; + }; + profile.clone() + } else { + let Some(profile) = super::provider::get_provider_type_profile( + store, + &provider.profile_workspace, provider_type, - "provider type has no profile; skipping provider policy layer" - ); - continue; + ) + .await? + else { + warn!( + provider_name = %name, + provider_type, + "provider type has no profile; skipping provider policy layer" + ); + continue; + }; + profile }; let rule_name = openshell_policy::provider_rule_name(provider.object_name()); @@ -1517,25 +1584,18 @@ pub(super) async fn handle_get_sandbox_provider_environment( .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; + let workspace = sandbox.object_workspace().to_string(); let spec = sandbox .spec .ok_or_else(|| Status::internal("sandbox has no spec"))?; let provider_names = spec.providers; - let provider_profile_catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; - let provider_env_revision = compute_provider_env_revision_with_catalog( - state.store.as_ref(), - &provider_profile_catalog, - &provider_names, - ) - .await?; - let provider_environment = super::provider::resolve_provider_environment_with_catalog( + let provider_env_revision = + compute_provider_env_revision(state.store.as_ref(), &workspace, &provider_names).await?; + let provider_environment = super::provider::resolve_provider_environment( state.store.as_ref(), - &provider_profile_catalog, + &workspace, &provider_names, ) .await?; @@ -1583,11 +1643,13 @@ async fn handle_update_config_inner( sandbox_caller: bool, ) -> Result, Status> { let req = request.into_inner(); - validate_annotations(&req.annotations, "annotations")?; + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if sandbox_caller { validate_sandbox_caller_update(&req)?; resolve_sandbox_by_name_for_principal( state.store.as_ref(), + &workspace, principal .as_ref() .expect("sandbox_caller implies principal"), @@ -1683,6 +1745,7 @@ async fn handle_update_config_inner( .put_policy_revision( &policy_id, GLOBAL_POLICY_SANDBOX_ID, + "", next_version, &payload, &hash, @@ -1789,7 +1852,7 @@ async fn handle_update_config_inner( // Resolve sandbox by name. let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -1816,12 +1879,14 @@ async fn handle_update_config_inner( } let mut sandbox_settings = - load_sandbox_settings(state.store.as_ref(), sandbox.object_name()).await?; + load_sandbox_settings(state.store.as_ref(), &workspace, sandbox.object_name()) + .await?; let removed = sandbox_settings.settings.remove(key).is_some(); if removed { sandbox_settings.revision = sandbox_settings.revision.wrapping_add(1); save_sandbox_settings( state.store.as_ref(), + &workspace, sandbox.object_name(), &sandbox_settings, ) @@ -1859,12 +1924,13 @@ async fn handle_update_config_inner( let stored = proto_setting_to_stored(key, setting)?; let mut sandbox_settings = - load_sandbox_settings(state.store.as_ref(), sandbox.object_name()).await?; + load_sandbox_settings(state.store.as_ref(), &workspace, sandbox.object_name()).await?; let changed = upsert_setting_value(&mut sandbox_settings.settings, key, stored); if changed { sandbox_settings.revision = sandbox_settings.revision.wrapping_add(1); save_sandbox_settings( state.store.as_ref(), + &workspace, sandbox.object_name(), &sandbox_settings, ) @@ -1911,6 +1977,7 @@ async fn handle_update_config_inner( let (version, hash, updated_sandbox) = apply_merge_operations_with_retry( state.store.as_ref(), &sandbox_id, + &workspace, spec.policy.as_ref(), &merge_ops, Some(&atomic_context), @@ -2105,6 +2172,47 @@ async fn handle_update_config_inner( ); } + let latest = state + .store + .get_latest_policy(&sandbox_id) + .await + .map_err(|e| Status::internal(format!("fetch latest policy failed: {e}")))?; + + let payload = new_policy.encode_to_vec(); + let hash = deterministic_policy_hash(&new_policy); + + if let Some(ref current) = latest + && current.policy_hash == hash + { + return Ok(Response::new(UpdateConfigResponse { + version: u32::try_from(current.version).unwrap_or(0), + policy_hash: hash, + settings_revision: 0, + deleted: false, + })); + } + + let next_version = latest.map_or(1, |r| r.version + 1); + let policy_id = uuid::Uuid::new_v4().to_string(); + + state + .store + .put_policy_revision( + &policy_id, + &sandbox_id, + &workspace, + next_version, + &payload, + &hash, + ) + .await + .map_err(|e| Status::internal(format!("persist policy revision failed: {e}")))?; + + let _ = state + .store + .supersede_older_policies(&sandbox_id, next_version) + .await; + state.sandbox_watch_bus.notify(&sandbox_id); info!( @@ -2133,6 +2241,11 @@ pub(super) async fn handle_get_sandbox_policy_status( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = if req.global { + String::new() + } else { + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await? + }; let (policy_id, active_version) = if req.global { (GLOBAL_POLICY_SANDBOX_ID.to_string(), 0_u32) @@ -2142,7 +2255,7 @@ pub(super) async fn handle_get_sandbox_policy_status( } let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -2184,6 +2297,11 @@ pub(super) async fn handle_list_sandbox_policies( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = if req.global { + String::new() + } else { + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await? + }; let policy_id = if req.global { GLOBAL_POLICY_SANDBOX_ID.to_string() @@ -2193,7 +2311,7 @@ pub(super) async fn handle_list_sandbox_policies( } let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -2305,6 +2423,10 @@ pub(super) async fn handle_get_sandbox_logs( request: Request, ) -> Result, Status> { let req = request.into_inner(); + // TODO(phase2): workspace is resolved but not used for authorization. + // Verify the sandbox belongs to this workspace before returning logs. + let _workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.sandbox_id.is_empty() { return Err(Status::invalid_argument("sandbox_id is required")); } @@ -2419,12 +2541,19 @@ pub(super) async fn handle_submit_policy_analysis( .cloned() .ok_or_else(|| Status::unauthenticated("missing principal"))?; let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } - let sandbox = - resolve_sandbox_by_name_for_principal(state.store.as_ref(), &principal, &req.name).await?; + let sandbox = resolve_sandbox_by_name_for_principal( + state.store.as_ref(), + &workspace, + &principal, + &req.name, + ) + .await?; let sandbox_id = sandbox.object_id().to_string(); for summary in &req.network_activity_summaries { state @@ -2451,17 +2580,8 @@ pub(super) async fn handle_submit_policy_analysis( // case for the common single-chunk submission shape. If real workloads // surface a problem with batches that interact across chunks, the right // fix is to recompute baseline after each successful auto-approve. - let provider_profile_catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; - let current_policy = current_effective_policy_for_sandbox( - state, - &provider_profile_catalog, - &sandbox, - &sandbox_id, - ) - .await?; + let current_policy = + current_effective_policy_for_sandbox(state, &workspace, &sandbox, &sandbox_id).await?; // Auto-approval is an opt-in behavior, sourced from the settings model // (sandbox or gateway scope) so it can be flipped on a running sandbox @@ -2469,7 +2589,8 @@ pub(super) async fn handle_submit_policy_analysis( // exact "auto") preserves OpenShell's default-deny posture: every // proposal lands in `pending` for a human reviewer. let (auto_approve_enabled, resolved_from) = - resolve_proposal_approval_mode(state.store.as_ref(), sandbox.object_name()).await?; + resolve_proposal_approval_mode(state.store.as_ref(), &workspace, sandbox.object_name()) + .await?; // The credential set is stable across all chunks in this batch, so build // it once. v1 captures presence only — no scope modeling — so the prover @@ -2480,9 +2601,9 @@ pub(super) async fn handle_submit_policy_analysis( .as_ref() .map(|spec| spec.providers.clone()) .unwrap_or_default(); - let credential_set = build_credential_set_for_sandbox_with_catalog( + let credential_set = build_credential_set_for_sandbox( state.store.as_ref(), - &provider_profile_catalog, + &workspace, &provider_names_for_creds, ) .await?; @@ -2598,7 +2719,7 @@ pub(super) async fn handle_submit_policy_analysis( .then(|| crate::policy_store::observation_dedup_key(&record)); let effective_id = state .store - .put_draft_chunk(&record, dedup_key.as_deref()) + .put_draft_chunk(&record, dedup_key.as_deref(), &workspace) .await .map_err(|e| Status::internal(format!("persist draft chunk failed: {e}")))?; accepted += 1; @@ -2652,6 +2773,7 @@ pub(super) async fn handle_submit_policy_analysis( && let Err(err) = auto_approve_chunk( state, &sandbox_id, + &workspace, sandbox.object_name(), &effective_id, &req.analysis_mode, @@ -2699,12 +2821,19 @@ pub(super) async fn handle_get_draft_policy( .cloned() .ok_or_else(|| Status::unauthenticated("missing principal"))?; let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } - let sandbox = - resolve_sandbox_by_name_for_principal(state.store.as_ref(), &principal, &req.name).await?; + let sandbox = resolve_sandbox_by_name_for_principal( + state.store.as_ref(), + &workspace, + &principal, + &req.name, + ) + .await?; let sandbox_id = sandbox.object_id().to_string(); let status_filter = if req.status_filter.is_empty() { @@ -2763,6 +2892,8 @@ async fn handle_approve_draft_chunk_inner( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } @@ -2774,7 +2905,7 @@ async fn handle_approve_draft_chunk_inner( let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -2807,7 +2938,7 @@ async fn handle_approve_draft_chunk_inner( ); let (version, hash) = - merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, &chunk).await?; + merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, &workspace, &chunk).await?; let chunk_summary = summarize_draft_chunk_rule(&chunk)?; let now_ms = current_time_ms(); @@ -2863,6 +2994,8 @@ async fn handle_reject_draft_chunk_inner( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } @@ -2872,7 +3005,7 @@ async fn handle_reject_draft_chunk_inner( let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -2908,7 +3041,8 @@ async fn handle_reject_draft_chunk_inner( if was_approved { require_no_global_policy(state).await?; - let (version, hash) = remove_chunk_from_policy(state, &sandbox_id, &chunk).await?; + let (version, hash) = + remove_chunk_from_policy(state, &sandbox_id, &workspace, &chunk).await?; emit_gateway_policy_audit_log( &sandbox_id, sandbox.object_name(), @@ -2960,6 +3094,8 @@ async fn handle_approve_all_draft_chunks_inner( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } @@ -2968,7 +3104,7 @@ async fn handle_approve_all_draft_chunks_inner( let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -3019,7 +3155,7 @@ async fn handle_approve_all_draft_chunks_inner( ); let (version, hash) = - merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, chunk).await?; + merge_chunk_into_policy(state.store.as_ref(), &sandbox_id, &workspace, chunk).await?; last_version = version; last_hash = hash; let chunk_summary = summarize_draft_chunk_rule(chunk)?; @@ -3081,6 +3217,8 @@ pub(super) async fn handle_edit_draft_chunk( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } @@ -3093,7 +3231,7 @@ pub(super) async fn handle_edit_draft_chunk( let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -3145,6 +3283,8 @@ async fn handle_undo_draft_chunk_inner( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } @@ -3154,7 +3294,7 @@ async fn handle_undo_draft_chunk_inner( let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -3184,7 +3324,7 @@ async fn handle_undo_draft_chunk_inner( "UndoDraftChunk: removing rule from active policy" ); - let (version, hash) = remove_chunk_from_policy(state, &sandbox_id, &chunk).await?; + let (version, hash) = remove_chunk_from_policy(state, &sandbox_id, &workspace, &chunk).await?; // Clear any prior rejection_reason on the way back to "pending" so an // agent reading the chunk via policy.local cannot see a stale guidance @@ -3230,13 +3370,15 @@ pub(super) async fn handle_clear_draft_chunks( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -3266,13 +3408,15 @@ pub(super) async fn handle_get_draft_history( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } let sandbox = state .store - .get_message_by_name::(&req.name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -3742,6 +3886,7 @@ struct AtomicPolicyWriteContext<'a> { async fn apply_merge_operations_with_retry( store: &Store, sandbox_id: &str, + workspace: &str, baseline_policy: Option<&ProtoSandboxPolicy>, operations: &[PolicyMergeOp], atomic_context: Option<&AtomicPolicyWriteContext<'_>>, @@ -3783,35 +3928,21 @@ async fn apply_merge_operations_with_retry( let next_version = latest.as_ref().map_or(1, |record| record.version + 1); let policy_id = uuid::Uuid::new_v4().to_string(); - let write_result = if let Some(context) = atomic_context { - store - .put_policy_revision_atomic(&AtomicPolicyRevisionWrite { - id: policy_id, - sandbox_id: sandbox_id.to_string(), - version: next_version, - policy_payload: payload, - policy_hash: hash.clone(), - provenance: context.provenance.clone(), - expected_resource_version: context.expected_resource_version, - annotations: context.annotations.clone(), - backfill_policy: None, - }) - .await - .map(Some) - } else { - store - .put_policy_revision(&policy_id, sandbox_id, next_version, &payload, &hash) - .await - .map(|()| None) - }; - - match write_result { - Ok(updated_sandbox) => { - if atomic_context.is_none() { - let _ = store - .supersede_older_policies(sandbox_id, next_version) - .await; - } + match store + .put_policy_revision( + &policy_id, + sandbox_id, + workspace, + next_version, + &payload, + &hash, + ) + .await + { + Ok(()) => { + let _ = store + .supersede_older_policies(sandbox_id, next_version) + .await; if attempt > 1 { info!( @@ -3852,6 +3983,7 @@ async fn apply_merge_operations_with_retry( pub(super) async fn merge_chunk_into_policy( store: &Store, sandbox_id: &str, + workspace: &str, chunk: &DraftChunkRecord, ) -> Result<(i64, String), Status> { let rule = NetworkPolicyRule::decode(chunk.proposed_rule.as_slice()) @@ -3861,19 +3993,19 @@ pub(super) async fn merge_chunk_into_policy( rule, }]; validate_merge_operations_for_server(&operations)?; - apply_merge_operations_with_retry(store, sandbox_id, None, &operations, None) - .await - .map(|(version, hash, _)| (version, hash)) + apply_merge_operations_with_retry(store, sandbox_id, workspace, None, &operations).await } async fn remove_chunk_from_policy( state: &ServerState, sandbox_id: &str, + workspace: &str, chunk: &DraftChunkRecord, ) -> Result<(i64, String), Status> { apply_merge_operations_with_retry( state.store.as_ref(), sandbox_id, + workspace, None, &[PolicyMergeOp::RemoveBinary { rule_name: chunk.rule_name.clone(), @@ -3978,7 +4110,7 @@ fn upsert_setting_value( } pub(super) async fn load_global_settings(store: &Store) -> Result { - load_settings_record(store, GLOBAL_SETTINGS_OBJECT_TYPE, GLOBAL_SETTINGS_NAME).await + load_settings_record(store, GLOBAL_SETTINGS_OBJECT_TYPE, "", GLOBAL_SETTINGS_NAME).await } pub(super) async fn save_global_settings( @@ -3988,6 +4120,7 @@ pub(super) async fn save_global_settings( save_settings_record( store, GLOBAL_SETTINGS_OBJECT_TYPE, + "", GLOBAL_SETTINGS_NAME, settings, ) @@ -3996,32 +4129,41 @@ pub(super) async fn save_global_settings( pub(super) async fn load_sandbox_settings( store: &Store, + workspace: &str, sandbox_name: &str, ) -> Result { - load_settings_record(store, SANDBOX_SETTINGS_OBJECT_TYPE, sandbox_name).await + load_settings_record(store, SANDBOX_SETTINGS_OBJECT_TYPE, workspace, sandbox_name).await } pub(super) async fn save_sandbox_settings( store: &Store, + workspace: &str, sandbox_name: &str, settings: &StoredSettings, ) -> Result<(), Status> { - save_settings_record(store, SANDBOX_SETTINGS_OBJECT_TYPE, sandbox_name, settings).await + save_settings_record( + store, + SANDBOX_SETTINGS_OBJECT_TYPE, + workspace, + sandbox_name, + settings, + ) + .await } async fn load_settings_record( store: &Store, object_type: &str, + workspace: &str, name: &str, ) -> Result { let record = store - .get_by_name(object_type, name) + .get_by_name(object_type, workspace, name) .await .map_err(|e| Status::internal(format!("fetch settings failed: {e}")))?; if let Some(record) = record { let mut settings = serde_json::from_slice::(&record.payload) .map_err(|e| Status::internal(format!("decode settings payload failed: {e}")))?; - // Populate resource_version from database record for CAS settings.resource_version = record.resource_version; Ok(settings) } else { @@ -4032,6 +4174,7 @@ async fn load_settings_record( async fn save_settings_record( store: &Store, object_type: &str, + workspace: &str, name: &str, settings: &StoredSettings, ) -> Result<(), Status> { @@ -4041,13 +4184,10 @@ async fn save_settings_record( .map_err(|e| Status::internal(format!("encode settings payload failed: {e}")))?; let (id, condition) = if settings.resource_version == 0 { - // Create new settings (resource_version 0 means never persisted) (uuid::Uuid::new_v4().to_string(), WriteCondition::MustCreate) } else { - // Update existing with CAS on the version from when it was loaded - // Fetch the record to get the stable ID let existing = store - .get_by_name(object_type, name) + .get_by_name(object_type, workspace, name) .await .map_err(|e| Status::internal(format!("fetch settings for CAS failed: {e}")))? .ok_or_else(|| Status::not_found("settings disappeared since load"))?; @@ -4058,9 +4198,8 @@ async fn save_settings_record( ) }; - // Single-attempt CAS write store - .put_if(object_type, &id, name, &payload, None, condition) + .put_if(object_type, &id, name, workspace, &payload, None, condition) .await .map_err(|e| match e { crate::persistence::PersistenceError::Conflict { .. } => { @@ -4328,7 +4467,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -4362,7 +4501,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -4395,7 +4534,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -4431,7 +4570,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -4446,6 +4585,7 @@ mod tests { Request::new(GetDraftPolicyRequest { name: "sandbox-b".to_string(), status_filter: String::new(), + workspace: "default".to_string(), }), "sb-a", ); @@ -4497,6 +4637,7 @@ mod tests { Request::new(GetDraftPolicyRequest { name: "missing-sandbox".to_string(), status_filter: String::new(), + workspace: "default".to_string(), }), "sb-a", ); @@ -4519,7 +4660,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -4600,7 +4741,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -4627,13 +4768,14 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: provider_type.to_string(), credentials: std::iter::once(("GITHUB_TOKEN".to_string(), "ghp-test".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), } } @@ -4671,7 +4813,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(policy), @@ -4917,9 +5059,10 @@ mod tests { .await .unwrap(); - let layers = profile_provider_policy_layers(&store, &["custom-provider".to_string()]) - .await - .unwrap(); + let layers = + profile_provider_policy_layers(&store, "default", &["custom-provider".to_string()]) + .await + .unwrap(); assert!(layers.is_empty()); } @@ -4939,7 +5082,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), profile: Some(openshell_core::proto::ProviderProfile { id: "generic".to_string(), @@ -4962,9 +5105,10 @@ mod tests { .await .unwrap(); - let layers = profile_provider_policy_layers(&store, &["custom-provider".to_string()]) - .await - .unwrap(); + let layers = + profile_provider_policy_layers(&store, "default", &["custom-provider".to_string()]) + .await + .unwrap(); assert_eq!(layers.len(), 1); assert_eq!(layers[0].rule.endpoints[0].host, "backdoor.example"); @@ -4986,7 +5130,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), profile: Some(openshell_core::proto::ProviderProfile { id: "custom-api".to_string(), @@ -5023,9 +5167,10 @@ mod tests { .await .unwrap(); - let layers = profile_provider_policy_layers(&store, &["work-custom".to_string()]) - .await - .unwrap(); + let layers = + profile_provider_policy_layers(&store, "default", &["work-custom".to_string()]) + .await + .unwrap(); assert_eq!(layers.len(), 1); assert_eq!(layers[0].rule_name, "_provider_work_custom"); @@ -5053,7 +5198,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), profile: Some(openshell_core::proto::ProviderProfile { id: "custom-api".to_string(), @@ -5076,9 +5221,10 @@ mod tests { .await .unwrap(); - let layers = profile_provider_policy_layers(&store, &["work-custom".to_string()]) - .await - .unwrap(); + let layers = + profile_provider_policy_layers(&store, "default", &["work-custom".to_string()]) + .await + .unwrap(); assert_eq!(layers.len(), 1); assert_eq!(layers[0].rule.endpoints[0].host, "api.custom.example"); @@ -5092,9 +5238,10 @@ mod tests { .await .unwrap(); - let layers = profile_provider_policy_layers(&store, &["work-github".to_string()]) - .await - .unwrap(); + let layers = + profile_provider_policy_layers(&store, "default", &["work-github".to_string()]) + .await + .unwrap(); assert_eq!(layers.len(), 1); assert_eq!(layers[0].rule_name, "_provider_work_github"); @@ -5321,7 +5468,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), profile: Some(ProviderProfile { id: "custom-policy".to_string(), @@ -5374,7 +5521,7 @@ mod tests { let mut updated_profile = stored_profile("api.after.example").profile.unwrap(); updated_profile.resource_version = state .store - .get_message_by_name::("custom-policy") + .get_message_by_name::("default", "custom-policy") .await .unwrap() .unwrap() @@ -5391,6 +5538,7 @@ mod tests { }), expected_resource_version: 0, id: "custom-policy".to_string(), + workspace: "default".to_string(), })), ) .await @@ -5415,7 +5563,7 @@ mod tests { let persisted_provider: Provider = state .store - .get_message_by_name("work-custom") + .get_message_by_name::("default", "work-custom") .await .unwrap() .unwrap(); @@ -5607,7 +5755,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), profile: Some(ProviderProfile { id: "custom-token".to_string(), @@ -5651,10 +5799,13 @@ mod tests { .await .unwrap(); - let first = - compute_provider_env_revision(state.store.as_ref(), &["work-custom-token".to_string()]) - .await - .unwrap(); + let first = compute_provider_env_revision( + state.store.as_ref(), + "default", + &["work-custom-token".to_string()], + ) + .await + .unwrap(); tokio::time::sleep(Duration::from_millis(2)).await; let mut rotated_profile = token_grant_profile("https://auth.example.com/rotated-token") @@ -5662,7 +5813,7 @@ mod tests { .unwrap(); rotated_profile.resource_version = state .store - .get_message_by_name::("custom-token") + .get_message_by_name::("default", "custom-token") .await .unwrap() .unwrap() @@ -5679,15 +5830,19 @@ mod tests { }), expected_resource_version: 0, id: "custom-token".to_string(), + workspace: "default".to_string(), })), ) .await .unwrap(); - let second = - compute_provider_env_revision(state.store.as_ref(), &["work-custom-token".to_string()]) - .await - .unwrap(); + let second = compute_provider_env_revision( + state.store.as_ref(), + "default", + &["work-custom-token".to_string()], + ) + .await + .unwrap(); assert_ne!( first, second, @@ -5745,6 +5900,7 @@ mod tests { sandbox_name: "attach-lifecycle".to_string(), provider_name: "work-github".to_string(), expected_resource_version: 0, + workspace: "default".to_string(), })), ) .await @@ -5781,6 +5937,7 @@ mod tests { sandbox_name: "attach-lifecycle".to_string(), provider_name: "work-github".to_string(), expected_resource_version: 0, + workspace: "default".to_string(), }), ) .await @@ -5866,6 +6023,7 @@ mod tests { discovery: None, }), }], + workspace: "default".to_string(), }), ) .await @@ -5908,6 +6066,7 @@ mod tests { sandbox_name: "custom-attach-lifecycle".to_string(), provider_name: "work-custom".to_string(), expected_resource_version: 0, + workspace: "default".to_string(), })), ) .await @@ -5947,6 +6106,7 @@ mod tests { sandbox_name: "custom-attach-lifecycle".to_string(), provider_name: "work-custom".to_string(), expected_resource_version: 0, + workspace: "default".to_string(), }), ) .await @@ -6011,7 +6171,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(sandbox_policy), @@ -6101,7 +6261,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -6154,7 +6314,7 @@ mod tests { /// settings model, mirroring what `openshell settings set /// proposal_approval_mode ` would do at runtime. async fn seed_sandbox_approval_mode(state: &Arc, sandbox_name: &str, mode: &str) { - let mut settings = load_sandbox_settings(state.store.as_ref(), sandbox_name) + let mut settings = load_sandbox_settings(state.store.as_ref(), "default", sandbox_name) .await .unwrap(); settings.settings.insert( @@ -6162,7 +6322,7 @@ mod tests { StoredSettingValue::String(mode.to_string()), ); settings.revision = settings.revision.wrapping_add(1); - save_sandbox_settings(state.store.as_ref(), sandbox_name, &settings) + save_sandbox_settings(state.store.as_ref(), "default", sandbox_name, &settings) .await .unwrap(); } @@ -6206,7 +6366,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -6263,6 +6423,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name.clone(), status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6281,6 +6442,7 @@ mod tests { Request::new(ApproveDraftChunkRequest { name: sandbox_name.clone(), chunk_id: chunk_id.clone(), + workspace: "default".to_string(), }), ) .await @@ -6293,6 +6455,7 @@ mod tests { &state, Request::new(GetDraftHistoryRequest { name: sandbox_name.clone(), + workspace: "default".to_string(), }), ) .await @@ -6310,6 +6473,7 @@ mod tests { limit: 10, offset: 0, global: false, + workspace: "default".to_string(), }), ) .await @@ -6323,6 +6487,7 @@ mod tests { Request::new(UndoDraftChunkRequest { name: sandbox_name.clone(), chunk_id: chunk_id.clone(), + workspace: "default".to_string(), }), ) .await @@ -6336,6 +6501,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name.clone(), status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6348,6 +6514,7 @@ mod tests { &state, Request::new(GetDraftHistoryRequest { name: sandbox_name.clone(), + workspace: "default".to_string(), }), ) .await @@ -6363,6 +6530,7 @@ mod tests { limit: 10, offset: 0, global: false, + workspace: "default".to_string(), }), ) .await @@ -6376,6 +6544,7 @@ mod tests { &state, Request::new(ClearDraftChunksRequest { name: sandbox_name.clone(), + workspace: "default".to_string(), }), ) .await @@ -6388,6 +6557,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name.clone(), status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6397,7 +6567,10 @@ mod tests { let history_after_clear = handle_get_draft_history( &state, - Request::new(GetDraftHistoryRequest { name: sandbox_name }), + Request::new(GetDraftHistoryRequest { + name: sandbox_name, + workspace: "default".to_string(), + }), ) .await .unwrap() @@ -6422,7 +6595,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -6471,6 +6644,7 @@ mod tests { name: sandbox_name.clone(), chunk_id: chunk_id.clone(), reason: guidance.to_string(), + workspace: "default".to_string(), }), ) .await @@ -6481,6 +6655,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6519,7 +6694,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -6585,6 +6760,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6635,7 +6811,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -6694,6 +6870,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name.clone(), status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6769,6 +6946,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6839,7 +7017,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -6896,6 +7074,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -6937,7 +7116,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -6997,6 +7176,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7043,7 +7223,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7098,6 +7278,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7137,7 +7318,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7192,6 +7373,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7223,7 +7405,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7277,6 +7459,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7311,7 +7494,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7366,6 +7549,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7399,7 +7583,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7458,6 +7642,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7492,7 +7677,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7606,7 +7791,7 @@ mod tests { }; state .store - .put_draft_chunk(&chunk, None) + .put_draft_chunk(&chunk, None, "default") .await .expect("draft chunk should persist"); @@ -7615,6 +7800,7 @@ mod tests { with_user(Request::new(ApproveDraftChunkRequest { name: sandbox_name.to_string(), chunk_id: chunk.id.clone(), + workspace: "default".to_string(), })), ) .await @@ -7667,7 +7853,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7721,6 +7907,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7764,7 +7951,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7818,6 +8005,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7850,7 +8038,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -7904,6 +8092,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -7945,7 +8134,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), profile: Some(ProviderProfile { id: "custom-api".to_string(), @@ -7985,7 +8174,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -8048,6 +8237,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -8108,7 +8298,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: Some(SandboxPolicy { @@ -8216,6 +8406,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -8295,7 +8486,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -8364,6 +8555,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name.clone(), status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -8385,6 +8577,7 @@ mod tests { name: sandbox_name, chunk_id: second.accepted_chunk_ids[0].clone(), reason: "redraft test".to_string(), + workspace: "default".to_string(), }), ) .await @@ -8410,7 +8603,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -8466,6 +8659,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -8775,7 +8969,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -8822,6 +9016,7 @@ mod tests { name: sandbox_name.clone(), chunk_id: chunk_id.clone(), reason: "scope too broad".to_string(), + workspace: "default".to_string(), }), ) .await @@ -8832,6 +9027,7 @@ mod tests { Request::new(ApproveDraftChunkRequest { name: sandbox_name.clone(), chunk_id: chunk_id.clone(), + workspace: "default".to_string(), }), ) .await @@ -8842,6 +9038,7 @@ mod tests { Request::new(UndoDraftChunkRequest { name: sandbox_name.clone(), chunk_id: chunk_id.clone(), + workspace: "default".to_string(), }), ) .await @@ -8852,6 +9049,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_name, status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -8890,7 +9088,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -8907,7 +9105,7 @@ mod tests { created_at_ms: 1_000_001, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -8958,6 +9156,7 @@ mod tests { with_user(Request::new(GetDraftPolicyRequest { name: sandbox_a.object_name().to_string(), status_filter: String::new(), + workspace: "default".to_string(), })), ) .await @@ -8971,6 +9170,7 @@ mod tests { Request::new(ApproveDraftChunkRequest { name: other_name.clone(), chunk_id: chunk_id.clone(), + workspace: "default".to_string(), }), ) .await @@ -8983,6 +9183,7 @@ mod tests { name: other_name.clone(), chunk_id: chunk_id.clone(), reason: "wrong sandbox".to_string(), + workspace: "default".to_string(), }), ) .await @@ -8995,6 +9196,7 @@ mod tests { name: other_name.clone(), chunk_id: chunk_id.clone(), proposed_rule: Some(proposed_rule.clone()), + workspace: "default".to_string(), }), ) .await @@ -9006,6 +9208,7 @@ mod tests { Request::new(ApproveDraftChunkRequest { name: sandbox_a.object_name().to_string(), chunk_id: chunk_id.clone(), + workspace: "default".to_string(), }), ) .await @@ -9016,6 +9219,7 @@ mod tests { Request::new(UndoDraftChunkRequest { name: other_name, chunk_id, + workspace: "default".to_string(), }), ) .await @@ -9234,7 +9438,7 @@ mod tests { rejection_reason: String::new(), }; - let (version, _) = merge_chunk_into_policy(&store, &chunk.sandbox_id, &chunk) + let (version, _) = merge_chunk_into_policy(&store, &chunk.sandbox_id, "default", &chunk) .await .unwrap(); @@ -9288,6 +9492,7 @@ mod tests { .put_policy_revision( "p-seed", sandbox_id, + "default", 1, &initial_policy.encode_to_vec(), "seed-hash", @@ -9330,7 +9535,7 @@ mod tests { rejection_reason: String::new(), }; - let (version, _) = merge_chunk_into_policy(&store, sandbox_id, &chunk) + let (version, _) = merge_chunk_into_policy(&store, sandbox_id, "default", &chunk) .await .unwrap(); assert_eq!(version, 2); @@ -9389,6 +9594,7 @@ mod tests { .put_policy_revision( "p-seed", sandbox_id, + "default", 1, &initial_policy.encode_to_vec(), "seed-hash", @@ -9431,7 +9637,7 @@ mod tests { rejection_reason: String::new(), }; - let (version, _) = merge_chunk_into_policy(&store, sandbox_id, &chunk) + let (version, _) = merge_chunk_into_policy(&store, sandbox_id, "default", &chunk) .await .unwrap(); assert_eq!(version, 2); @@ -9476,6 +9682,7 @@ mod tests { .put_policy_revision( "p-seed", sandbox_id, + "default", 1, &initial_policy.encode_to_vec(), "seed-hash", @@ -9511,8 +9718,8 @@ mod tests { }]; let (left, right) = tokio::join!( - apply_merge_operations_with_retry(&store, sandbox_id, None, &add_allow, None), - apply_merge_operations_with_retry(&store, sandbox_id, None, &add_deny, None), + apply_merge_operations_with_retry(&store, sandbox_id, "default", None, &add_allow), + apply_merge_operations_with_retry(&store, sandbox_id, "default", None, &add_deny), ); let mut versions = vec![left.unwrap().0, right.unwrap().0]; @@ -10182,7 +10389,9 @@ mod tests { #[tokio::test] async fn sandbox_settings_load_returns_default_when_empty() { let store = test_store().await; - let settings = load_sandbox_settings(&store, "nonexistent").await.unwrap(); + let settings = load_sandbox_settings(&store, "default", "nonexistent") + .await + .unwrap(); assert!(settings.settings.is_empty()); assert_eq!(settings.revision, 0); } @@ -10226,11 +10435,13 @@ mod tests { StoredSettingValue::String("auto".to_string()), ); settings.revision = 3; - save_sandbox_settings(&store, sandbox_name, &settings) + save_sandbox_settings(&store, "default", sandbox_name, &settings) .await .unwrap(); - let loaded = load_sandbox_settings(&store, sandbox_name).await.unwrap(); + let loaded = load_sandbox_settings(&store, "default", sandbox_name) + .await + .unwrap(); assert_eq!(loaded.revision, 3); assert_eq!( loaded.settings.get(settings::PROPOSAL_APPROVAL_MODE_KEY), @@ -10395,7 +10606,9 @@ mod tests { assert!(!loaded.settings.contains_key("log_level")); let sandbox_name = "test-sandbox"; - let mut sandbox_settings = load_sandbox_settings(&store, sandbox_name).await.unwrap(); + let mut sandbox_settings = load_sandbox_settings(&store, "default", sandbox_name) + .await + .unwrap(); let changed = upsert_setting_value( &mut sandbox_settings.settings, "log_level", @@ -10403,17 +10616,115 @@ mod tests { ); assert!(changed); sandbox_settings.revision = sandbox_settings.revision.wrapping_add(1); - save_sandbox_settings(&store, sandbox_name, &sandbox_settings) + save_sandbox_settings(&store, "default", sandbox_name, &sandbox_settings) .await .unwrap(); - let reloaded = load_sandbox_settings(&store, sandbox_name).await.unwrap(); + let reloaded = load_sandbox_settings(&store, "default", sandbox_name) + .await + .unwrap(); assert_eq!( reloaded.settings.get("log_level"), Some(&StoredSettingValue::String("debug".to_string())), ); } + #[tokio::test] + async fn sandbox_settings_are_workspace_isolated() { + let store = test_store().await; + let sandbox_name = "work"; + + let mut alpha_settings = StoredSettings::default(); + alpha_settings.settings.insert( + settings::PROPOSAL_APPROVAL_MODE_KEY.to_string(), + StoredSettingValue::String("auto".to_string()), + ); + alpha_settings.revision = 1; + save_sandbox_settings(&store, "alpha", sandbox_name, &alpha_settings) + .await + .unwrap(); + + let mut beta_settings = StoredSettings::default(); + beta_settings.settings.insert( + settings::PROPOSAL_APPROVAL_MODE_KEY.to_string(), + StoredSettingValue::String("manual".to_string()), + ); + beta_settings.revision = 1; + save_sandbox_settings(&store, "beta", sandbox_name, &beta_settings) + .await + .unwrap(); + + let alpha_loaded = load_sandbox_settings(&store, "alpha", sandbox_name) + .await + .unwrap(); + let beta_loaded = load_sandbox_settings(&store, "beta", sandbox_name) + .await + .unwrap(); + + assert_eq!( + alpha_loaded + .settings + .get(settings::PROPOSAL_APPROVAL_MODE_KEY), + Some(&StoredSettingValue::String("auto".to_string())), + ); + assert_eq!( + beta_loaded + .settings + .get(settings::PROPOSAL_APPROVAL_MODE_KEY), + Some(&StoredSettingValue::String("manual".to_string())), + ); + } + + #[tokio::test] + async fn get_sandbox_config_returns_workspace() { + use openshell_core::proto::{SandboxPhase, SandboxSpec}; + let state = test_server_state().await; + + for (ws, sb_id, sb_name) in [("alpha", "sb-alpha", "work"), ("beta", "sb-beta", "work")] { + let mut sandbox = Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: sb_id.to_string(), + name: sb_name.to_string(), + created_at_ms: 1_000_000, + labels: HashMap::new(), + resource_version: 0, + workspace: ws.to_string(), + }), + spec: Some(SandboxSpec { + policy: None, + ..Default::default() + }), + ..Default::default() + }; + sandbox.set_phase(SandboxPhase::Provisioning as i32); + state.store.put_message(&sandbox).await.unwrap(); + } + + let alpha_req = with_sandbox( + Request::new(GetSandboxConfigRequest { + sandbox_id: "sb-alpha".to_string(), + }), + "sb-alpha", + ); + let alpha_resp = handle_get_sandbox_config(&state, alpha_req) + .await + .unwrap() + .into_inner(); + assert_eq!(alpha_resp.workspace, "alpha"); + + let beta_req = with_sandbox( + Request::new(GetSandboxConfigRequest { + sandbox_id: "sb-beta".to_string(), + }), + "sb-beta", + ); + let beta_resp = handle_get_sandbox_config(&state, beta_req) + .await + .unwrap() + .into_inner(); + assert_eq!(beta_resp.workspace, "beta"); + } + #[test] fn validate_registered_setting_key_rejects_policy() { let err = validate_registered_setting_key("policy").unwrap_err(); @@ -10517,7 +10828,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, // No policy yet - will be backfilled @@ -10532,7 +10843,7 @@ mod tests { // Fetch the sandbox to get its current resource_version let current = state .store - .get_message_by_name::("test-sandbox") + .get_message_by_name::("default", "test-sandbox") .await .unwrap() .unwrap(); @@ -10552,7 +10863,7 @@ mod tests { global: false, merge_operations: vec![], expected_resource_version: current_version, - annotations: HashMap::new(), + workspace: "default".to_string(), }), ) .await @@ -10565,7 +10876,7 @@ mod tests { // Verify the resource_version incremented and policy was backfilled let updated_sandbox = state .store - .get_message_by_name::("test-sandbox") + .get_message_by_name::("default", "test-sandbox") .await .unwrap() .unwrap(); @@ -11180,7 +11491,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -11194,7 +11505,7 @@ mod tests { let current = state .store - .get_message_by_name::("sync-strip") + .get_message_by_name::("default", "sync-strip") .await .unwrap() .unwrap(); @@ -11234,7 +11545,7 @@ mod tests { let updated_sandbox = state .store - .get_message_by_name::("sync-strip") + .get_message_by_name::("default", "sync-strip") .await .unwrap() .unwrap(); @@ -11284,7 +11595,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -11299,7 +11610,7 @@ mod tests { // Get current version let current = state .store - .get_message_by_name::("test-sandbox") + .get_message_by_name::("default", "test-sandbox") .await .unwrap() .unwrap(); @@ -11319,7 +11630,7 @@ mod tests { global: false, merge_operations: vec![], expected_resource_version: 99, // stale version - annotations: HashMap::new(), + workspace: "default".to_string(), }), ) .await @@ -11337,7 +11648,7 @@ mod tests { // Verify the sandbox was not modified (policy still None) let unchanged = state .store - .get_message_by_name::("test-sandbox") + .get_message_by_name::("default", "test-sandbox") .await .unwrap() .unwrap(); @@ -11376,7 +11687,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { policy: None, @@ -11391,7 +11702,7 @@ mod tests { // All three clients fetch the sandbox and see the same version let initial = state .store - .get_message_by_name::("test-sandbox") + .get_message_by_name::("default", "test-sandbox") .await .unwrap() .unwrap(); @@ -11415,7 +11726,7 @@ mod tests { global: false, merge_operations: vec![], expected_resource_version: initial_version, - annotations: HashMap::new(), + workspace: "default".to_string(), }), ) .await @@ -11448,7 +11759,7 @@ mod tests { // Final sandbox should have resource_version = initial_version + 1 and policy backfilled let final_sandbox = state .store - .get_message_by_name::("test-sandbox") + .get_message_by_name::("default", "test-sandbox") .await .unwrap() .unwrap(); diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index aa4b3ab195..51b95811c9 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -70,17 +70,7 @@ impl ProviderEnvironment { #[cfg(test)] pub(super) async fn create_provider_record( store: &Store, - provider: Provider, -) -> Result { - let catalog = ProviderProfileSources::with_default_sources() - .snapshot_catalog(store) - .await?; - create_provider_record_with_catalog(store, &catalog, provider).await -} - -pub(super) async fn create_provider_record_with_catalog( - store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, mut provider: Provider, ) -> Result { use crate::persistence::{ObjectName, current_time_ms}; @@ -94,7 +84,7 @@ pub(super) async fn create_provider_record_with_catalog( created_at_ms: now_ms, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: std::collections::HashMap::new(), + workspace: workspace.to_string(), }); } @@ -114,8 +104,18 @@ pub(super) async fn create_provider_record_with_catalog( if provider.r#type.trim().is_empty() { return Err(Status::invalid_argument("provider.type is required")); } + if !provider.profile_workspace.is_empty() && provider.profile_workspace != workspace { + return Err(Status::invalid_argument( + "profile_workspace must be empty (global) or match the provider workspace", + )); + } + let profile_ws = if provider.profile_workspace.is_empty() { + "" + } else { + workspace + }; if provider.credentials.is_empty() - && !provider_type_allows_empty_credentials(catalog, &provider.r#type) + && !provider_type_allows_empty_credentials(store, profile_ws, &provider.r#type).await? { return Err(Status::invalid_argument( "provider.credentials must not be empty", @@ -138,6 +138,7 @@ pub(super) async fn create_provider_record_with_catalog( Provider::object_type(), &provider_id, provider.object_name(), + workspace, &provider.encode_to_vec(), None, WriteCondition::MustCreate, @@ -161,13 +162,17 @@ pub(super) async fn create_provider_record_with_catalog( Ok(redact_provider_credentials(provider)) } -pub(super) async fn get_provider_record(store: &Store, name: &str) -> Result { +pub(super) async fn get_provider_record( + store: &Store, + workspace: &str, + name: &str, +) -> Result { if name.is_empty() { return Err(Status::invalid_argument("name is required")); } store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::not_found("provider not found")) @@ -176,11 +181,12 @@ pub(super) async fn get_provider_record(store: &Store, name: &str) -> Result Result, Status> { let providers: Vec = store - .list_messages(limit, offset) + .list_messages(workspace, limit, offset) .await .map_err(|e| Status::internal(format!("list providers failed: {e}")))?; @@ -193,6 +199,7 @@ pub(super) async fn list_provider_records( #[cfg(test)] pub(super) async fn update_provider_record( store: &Store, + workspace: &str, provider: Provider, ) -> Result { let catalog = ProviderProfileSources::with_default_sources() @@ -217,7 +224,7 @@ pub(super) async fn update_provider_record_with_catalog( // Resolve provider ID from name for CAS update let existing = store - .get_message_by_name::(provider.object_name()) + .get_message_by_name::(workspace, provider.object_name()) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))?; @@ -233,6 +240,13 @@ pub(super) async fn update_provider_record_with_catalog( "provider type cannot be changed; delete and recreate the provider", )); } + if !provider.profile_workspace.is_empty() + && provider.profile_workspace != existing.profile_workspace + { + return Err(Status::invalid_argument( + "profile_workspace cannot be changed; delete and recreate the provider", + )); + } let current_version = existing.metadata.as_ref().map_or(0, |m| m.resource_version); @@ -259,8 +273,7 @@ pub(super) async fn update_provider_record_with_catalog( // #1347. super::validation::validate_object_metadata(candidate.metadata.as_ref(), "provider")?; validate_provider_mutable_fields(&candidate)?; - validate_provider_update_against_attached_sandboxes_with_catalog(store, catalog, &candidate) - .await?; + validate_provider_update_against_attached_sandboxes(store, workspace, &candidate).await?; // Serialize labels for storage let labels_map = candidate.object_labels(); @@ -282,6 +295,7 @@ pub(super) async fn update_provider_record_with_catalog( Provider::object_type(), candidate.object_id(), candidate.object_name(), + workspace, &candidate.encode_to_vec(), labels_json.as_deref(), WriteCondition::MatchResourceVersion(cas_version), @@ -311,20 +325,24 @@ pub(super) async fn update_provider_record_with_catalog( Ok(redact_provider_credentials(candidate)) } -pub(super) async fn delete_provider_record(store: &Store, name: &str) -> Result { +pub(super) async fn delete_provider_record( + store: &Store, + workspace: &str, + name: &str, +) -> Result { if name.is_empty() { return Err(Status::invalid_argument("name is required")); } let Some(provider) = store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? else { return Ok(false); }; - let blocking_sandboxes = sandboxes_using_provider(store, name).await?; + let blocking_sandboxes = sandboxes_using_provider(store, workspace, name).await?; if !blocking_sandboxes.is_empty() { return Err(Status::failed_precondition(format!( "provider '{name}' is attached to sandbox(es): {}", @@ -336,7 +354,7 @@ pub(super) async fn delete_provider_record(store: &Store, name: &str) -> Result< .await?; store - .delete_by_name(Provider::object_type(), name) + .delete_by_name(Provider::object_type(), workspace, name) .await .map_err(|e| Status::internal(format!("delete provider failed: {e}"))) } @@ -346,7 +364,7 @@ pub(super) async fn delete_provider_record(store: &Store, name: &str) -> Result< /// value in the output, `None` skips it. /// /// This is the shared pagination kernel used by all sandbox-scan helpers. -async fn scan_sandboxes(store: &Store, mut f: F) -> Result, Status> +async fn scan_sandboxes(store: &Store, workspace: &str, mut f: F) -> Result, Status> where F: FnMut(Sandbox) -> Option, { @@ -354,7 +372,7 @@ where let mut offset = 0u32; loop { let records = store - .list(Sandbox::object_type(), 1000, offset) + .list(Sandbox::object_type(), workspace, 1000, offset) .await .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))?; if records.is_empty() { @@ -379,10 +397,11 @@ where async fn sandboxes_using_provider( store: &Store, + workspace: &str, provider_name: &str, ) -> Result, Status> { let provider_name = provider_name.to_string(); - let mut names = scan_sandboxes(store, |sandbox| { + let mut names = scan_sandboxes(store, workspace, |sandbox| { let spec = sandbox.spec.as_ref()?; if spec.providers.iter().any(|n| n == &provider_name) { Some(sandbox.object_name().to_string()) @@ -398,10 +417,11 @@ async fn sandboxes_using_provider( async fn sandboxes_using_provider_records( store: &Store, + workspace: &str, provider_name: &str, ) -> Result, Status> { let provider_name = provider_name.to_string(); - scan_sandboxes(store, |sandbox| { + scan_sandboxes(store, workspace, |sandbox| { let spec = sandbox.spec.as_ref()?; if spec.providers.iter().any(|n| n == &provider_name) { Some(sandbox) @@ -464,6 +484,7 @@ fn merge_i64_map( #[cfg(test)] pub(super) async fn resolve_provider_environment( store: &Store, + workspace: &str, provider_names: &[String], ) -> Result { let catalog = ProviderProfileSources::with_default_sources() @@ -484,13 +505,13 @@ pub(super) async fn resolve_provider_environment_with_catalog( let mut env = std::collections::HashMap::new(); let mut expires = std::collections::HashMap::new(); let now_ms = crate::persistence::current_time_ms(); - validate_provider_environment_keys_unique_at(store, catalog, provider_names, None, now_ms) + validate_provider_environment_keys_unique_at(store, workspace, provider_names, None, now_ms) .await?; let registry = openshell_providers::ProviderRegistry::new(); for name in provider_names { let provider = store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("failed to fetch provider '{name}': {e}")))? .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; @@ -538,12 +559,7 @@ pub(super) async fn resolve_provider_environment_with_catalog( Ok(ProviderEnvironment { environment: env, credential_expires_at_ms: expires, - dynamic_credentials: resolve_dynamic_credentials_with_catalog( - store, - catalog, - provider_names, - ) - .await?, + dynamic_credentials: resolve_dynamic_credentials(store, workspace, provider_names).await?, }) } @@ -554,7 +570,7 @@ pub(super) async fn resolve_provider_environment_with_catalog( /// host, port, endpoint path, and provider credential identity. pub(super) async fn resolve_dynamic_credentials_with_catalog( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, provider_names: &[String], ) -> Result, Status> { if provider_names.is_empty() { @@ -565,7 +581,7 @@ pub(super) async fn resolve_dynamic_credentials_with_catalog( for provider_name in provider_names { let provider = store - .get_message_by_name::(provider_name) + .get_message_by_name::(workspace, provider_name) .await .map_err(|e| { Status::internal(format!("failed to fetch provider '{provider_name}': {e}")) @@ -576,7 +592,9 @@ pub(super) async fn resolve_dynamic_credentials_with_catalog( let profile_id = normalize_provider_type(&provider.r#type).unwrap_or(provider.r#type.as_str()); - let Some(profile) = get_provider_type_profile_with_catalog(catalog, profile_id) else { + let Some(profile) = + get_provider_type_profile(store, &provider.profile_workspace, profile_id).await? + else { continue; }; @@ -865,6 +883,7 @@ fn endpoint_path_matches(pattern: &str, path: &str) -> bool { pub async fn validate_provider_environment_keys_unique( store: &Store, + workspace: &str, provider_names: &[String], ) -> Result<(), Status> { let catalog = ProviderProfileSources::with_default_sources() @@ -880,7 +899,7 @@ pub async fn validate_provider_environment_keys_unique_with_catalog( ) -> Result<(), Status> { validate_provider_environment_keys_unique_at( store, - catalog, + workspace, provider_names, None, crate::persistence::current_time_ms(), @@ -890,7 +909,7 @@ pub async fn validate_provider_environment_keys_unique_with_catalog( pub async fn validate_provider_credential_key_available_for_attached_sandboxes_with_catalog( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, provider: &Provider, credential_key: &str, ) -> Result<(), Status> { @@ -900,12 +919,12 @@ pub async fn validate_provider_credential_key_available_for_attached_sandboxes_w .entry(credential_key.to_string()) .or_insert_with(|| "pending".to_string()); candidate.credential_expires_at_ms.remove(credential_key); - validate_provider_update_against_attached_sandboxes_with_catalog(store, catalog, &candidate) - .await + validate_provider_update_against_attached_sandboxes(store, workspace, &candidate).await } pub async fn validate_provider_update_against_attached_sandboxes( store: &Store, + workspace: &str, provider: &Provider, ) -> Result<(), Status> { let catalog = ProviderProfileSources::with_default_sources() @@ -921,14 +940,14 @@ pub async fn validate_provider_update_against_attached_sandboxes_with_catalog( provider: &Provider, ) -> Result<(), Status> { let provider_name = provider.object_name().to_string(); - for sandbox in sandboxes_using_provider_records(store, &provider_name).await? { + for sandbox in sandboxes_using_provider_records(store, workspace, &provider_name).await? { let sandbox_name = sandbox.object_name().to_string(); let Some(spec) = sandbox.spec.as_ref() else { continue; }; validate_provider_environment_keys_unique_at( store, - catalog, + workspace, &spec.providers, Some(provider), crate::persistence::current_time_ms(), @@ -946,7 +965,7 @@ pub async fn validate_provider_update_against_attached_sandboxes_with_catalog( async fn validate_provider_environment_keys_unique_at( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, provider_names: &[String], candidate_provider: Option<&Provider>, now_ms: i64, @@ -957,7 +976,7 @@ async fn validate_provider_environment_keys_unique_at( let provider = match candidate_provider { Some(candidate) if candidate.object_name() == name.as_str() => candidate.clone(), _ => store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("failed to fetch provider '{name}': {e}")))? .ok_or_else(|| { @@ -976,9 +995,8 @@ async fn validate_provider_environment_keys_unique_at( seen.insert(key, provider_name.clone()); } } - dynamic_bindings.extend(dynamic_token_grant_bindings_for_provider_with_catalog( - catalog, &provider, - )); + dynamic_bindings + .extend(dynamic_token_grant_bindings_for_provider(store, &provider).await?); } validate_dynamic_token_grant_bindings_unambiguous(&dynamic_bindings)?; Ok(()) @@ -994,14 +1012,16 @@ struct DynamicTokenGrantBinding { score: u32, } -fn dynamic_token_grant_bindings_for_provider_with_catalog( - catalog: &EffectiveProviderProfileCatalog, +async fn dynamic_token_grant_bindings_for_provider( + store: &Store, provider: &Provider, ) -> Vec { let provider_name = provider.object_name().to_string(); let profile_id = normalize_provider_type(&provider.r#type).unwrap_or(provider.r#type.as_str()); - let Some(profile) = get_provider_type_profile_with_catalog(catalog, profile_id) else { - return Vec::new(); + let Some(profile) = + get_provider_type_profile(store, &provider.profile_workspace, profile_id).await? + else { + return Ok(Vec::new()); }; dynamic_token_grant_bindings_for_profile(&provider_name, &profile.to_proto()) } @@ -1228,7 +1248,9 @@ pub(super) async fn handle_create_provider( request: Request, ) -> Result, Status> { let req = request.into_inner(); - let Some(provider) = req.provider else { + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; + let Some(mut provider) = req.provider else { emit_provider_lifecycle( "custom", LifecycleOperation::Create, @@ -1236,13 +1258,11 @@ pub(super) async fn handle_create_provider( ); return Err(Status::invalid_argument("provider is required")); }; + if let Some(metadata) = provider.metadata.as_mut() { + metadata.workspace.clone_from(&workspace); + } let provider_type = provider.r#type.clone(); - let catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; - let result = - create_provider_record_with_catalog(state.store.as_ref(), &catalog, provider).await; + let result = create_provider_record(state.store.as_ref(), &workspace, provider).await; match result { Ok(provider) => { emit_provider_lifecycle( @@ -1269,8 +1289,10 @@ pub(super) async fn handle_get_provider( state: &Arc, request: Request, ) -> Result, Status> { - let name = request.into_inner().name; - let provider = get_provider_record(state.store.as_ref(), &name).await?; + let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; + let provider = get_provider_record(state.store.as_ref(), &workspace, &req.name).await?; Ok(Response::new(ProviderResponse { provider: Some(provider), @@ -1282,8 +1304,25 @@ pub(super) async fn handle_list_providers( request: Request, ) -> Result, Status> { let request = request.into_inner(); + if request.all_workspaces && !request.workspace.is_empty() { + return Err(Status::invalid_argument( + "all_workspaces and workspace are mutually exclusive", + )); + } let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE); - let providers = list_provider_records(state.store.as_ref(), limit, request.offset).await?; + + let providers = if request.all_workspaces { + let all: Vec = state + .store + .list_all_messages(limit, request.offset) + .await + .map_err(|e| Status::internal(format!("list providers failed: {e}")))?; + all.into_iter().map(redact_provider_credentials).collect() + } else { + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; + list_provider_records(state.store.as_ref(), &workspace, limit, request.offset).await? + }; Ok(Response::new(ListProvidersResponse { providers })) } @@ -1293,14 +1332,14 @@ pub(super) async fn handle_list_provider_profiles( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_profile_workspace(state.store.as_ref(), &request.workspace) + .await?; let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE) as usize; let offset = request.offset as usize; - let catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; - let profiles = catalog - .list_profiles() + let mut profiles = merged_provider_profiles(state.store.as_ref(), &workspace).await?; + profiles.sort_by(|left, right| left.id.cmp(&right.id)); + let profiles = profiles .into_iter() .skip(offset) .take(limit) @@ -1313,15 +1352,15 @@ pub(super) async fn handle_get_provider_profile( state: &Arc, request: Request, ) -> Result, Status> { - let id = request.into_inner().id; + let req = request.into_inner(); + let workspace = + super::workspace::resolve_profile_workspace(state.store.as_ref(), &req.workspace).await?; + let id = req.id; let id = normalize_profile_id_request(&id)?; - let catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; - let profile = catalog - .get_profile(&id) - .ok_or_else(|| Status::not_found("provider profile not found"))?; + let profile = get_provider_type_profile(state.store.as_ref(), &workspace, &id) + .await? + .ok_or_else(|| Status::not_found("provider profile not found"))? + .to_proto(); Ok(Response::new(ProviderProfileResponse { profile: Some(profile), @@ -1333,21 +1372,20 @@ pub(super) async fn handle_import_provider_profiles( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_profile_workspace(state.store.as_ref(), &request.workspace) + .await?; let (profiles, mut diagnostics) = profiles_from_import_items(&request.profiles); add_empty_profile_set_diagnostic(&profiles, &mut diagnostics); let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; - let catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; diagnostics - .extend(profile_conflict_diagnostics(state.store.as_ref(), &catalog, &profiles).await?); + .extend(profile_conflict_diagnostics(state.store.as_ref(), &workspace, &profiles).await?); diagnostics.extend(validate_profile_set(&profiles)); if !has_errors(&diagnostics) { diagnostics.extend( profile_attached_sandbox_diagnostics( state.store.as_ref(), - &catalog, + &workspace, &profiles, "import", ) @@ -1365,13 +1403,14 @@ pub(super) async fn handle_import_provider_profiles( let mut imported = Vec::with_capacity(profiles.len()); for (_, profile) in profiles { - let mut stored = stored_provider_profile(profile.to_proto()); + let mut stored = stored_provider_profile_for_workspace(profile.to_proto(), &workspace); let result = state .store .put_if( StoredProviderProfile::object_type(), stored.object_id(), stored.object_name(), + &workspace, &stored.encode_to_vec(), None, WriteCondition::MustCreate, @@ -1400,6 +1439,9 @@ pub(super) async fn handle_update_provider_profiles( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_profile_workspace(state.store.as_ref(), &request.workspace) + .await?; let items = request.profile.into_iter().collect::>(); let (profiles, mut diagnostics) = profiles_from_import_items(&items); add_empty_profile_set_diagnostic(&profiles, &mut diagnostics); @@ -1410,7 +1452,7 @@ pub(super) async fn handle_update_provider_profiles( .snapshot_catalog(state.store.as_ref()) .await?; diagnostics.extend( - profile_update_target_diagnostics(state.store.as_ref(), &catalog, &profiles, &target_id) + profile_update_target_diagnostics(state.store.as_ref(), &workspace, &profiles, &target_id) .await?, ); diagnostics.extend(validate_profile_set(&profiles)); @@ -1436,7 +1478,7 @@ pub(super) async fn handle_update_provider_profiles( diagnostics.extend( profile_attached_sandbox_diagnostics( state.store.as_ref(), - &catalog, + &workspace, &profiles, "update", ) @@ -1459,7 +1501,7 @@ pub(super) async fn handle_update_provider_profiles( .ok_or_else(|| Status::internal("validated provider profile update is missing"))?; let mut stored = state .store - .get_message_by_name::(&target_id) + .get_message_by_name::(&workspace, &target_id) .await .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? .ok_or_else(|| Status::not_found("provider profile not found"))?; @@ -1488,6 +1530,7 @@ pub(super) async fn handle_update_provider_profiles( StoredProviderProfile::object_type(), stored.object_id(), stored.object_name(), + &workspace, &stored.encode_to_vec(), labels_json.as_deref(), WriteCondition::MatchResourceVersion(expected_resource_version), @@ -1512,14 +1555,13 @@ pub(super) async fn handle_lint_provider_profiles( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_profile_workspace(state.store.as_ref(), &request.workspace) + .await?; let (profiles, mut diagnostics) = profiles_from_import_items(&request.profiles); add_empty_profile_set_diagnostic(&profiles, &mut diagnostics); - let catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; diagnostics - .extend(profile_conflict_diagnostics(state.store.as_ref(), &catalog, &profiles).await?); + .extend(profile_conflict_diagnostics(state.store.as_ref(), &workspace, &profiles).await?); diagnostics.extend(validate_profile_set(&profiles)); let valid = !has_errors(&diagnostics); @@ -1533,7 +1575,10 @@ pub(super) async fn handle_delete_provider_profile( state: &Arc, request: Request, ) -> Result, Status> { - let id = request.into_inner().id; + let req = request.into_inner(); + let workspace = + super::workspace::resolve_profile_workspace(state.store.as_ref(), &req.workspace).await?; + let id = req.id; let id = normalize_profile_id_request(&id)?; let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; let catalog = state @@ -1548,14 +1593,14 @@ pub(super) async fn handle_delete_provider_profile( let existing = state .store - .get_message_by_name::(&id) + .get_message_by_name::(&workspace, &id) .await .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))?; if existing.is_none() { return Err(Status::not_found("provider profile not found")); } - let blocking_sandboxes = sandboxes_using_profile(state.store.as_ref(), &id).await?; + let blocking_sandboxes = sandboxes_using_profile(state.store.as_ref(), &workspace, &id).await?; if !blocking_sandboxes.is_empty() { return Err(Status::failed_precondition(format!( "provider profile '{id}' is in use by sandboxes: {}", @@ -1565,27 +1610,51 @@ pub(super) async fn handle_delete_provider_profile( let deleted = state .store - .delete_by_name(StoredProviderProfile::object_type(), &id) + .delete_by_name(StoredProviderProfile::object_type(), &workspace, &id) .await .map_err(|e| Status::internal(format!("delete provider profile failed: {e}")))?; Ok(Response::new(DeleteProviderProfileResponse { deleted })) } -pub(super) fn get_provider_type_profile_with_catalog( - catalog: &EffectiveProviderProfileCatalog, +pub(super) async fn get_provider_type_profile( + store: &Store, + workspace: &str, id: &str, -) -> Option { - catalog.get_type_profile(id) +) -> Result, Status> { + let Some(id) = normalize_profile_id(id) else { + return Ok(None); + }; + if let Some(profile) = get_default_profile(&id) { + return Ok(Some(profile.clone())); + } + let profile = store + .get_message_by_name::(workspace, &id) + .await + .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? + .and_then(|stored| { + let resource_version = stored_profile_resource_version(&stored); + stored.profile.map(|profile| { + ProviderTypeProfile::from_proto(&profile_response_payload( + profile, + resource_version, + )) + }) + }); + Ok(profile) } -fn provider_refresh_defaults( - catalog: &EffectiveProviderProfileCatalog, +async fn provider_refresh_defaults( + store: &Store, provider: &Provider, credential_key: &str, -) -> Option { - let profile = get_provider_type_profile_with_catalog(catalog, &provider.r#type)?; - profile +) -> Result, Status> { + let Some(profile) = + get_provider_type_profile(store, &provider.profile_workspace, &provider.r#type).await? + else { + return Ok(None); + }; + Ok(profile .credentials .iter() .find(|credential| { @@ -1623,14 +1692,48 @@ fn validate_refresh_material( Ok(()) } -fn provider_type_allows_empty_credentials( - catalog: &EffectiveProviderProfileCatalog, +async fn provider_type_allows_empty_credentials( + store: &Store, + workspace: &str, provider_type: &str, -) -> bool { - let Some(profile) = get_provider_type_profile_with_catalog(catalog, provider_type) else { - return false; +) -> Result { + let Some(profile) = get_provider_type_profile(store, workspace, provider_type).await? else { + return Ok(false); }; - profile.allows_empty_provider_credentials() + Ok(profile.allows_empty_provider_credentials()) +} + +async fn merged_provider_profiles( + store: &Store, + workspace: &str, +) -> Result, Status> { + let mut profiles = default_profiles().to_vec(); + profiles.extend( + custom_provider_profiles(store, workspace) + .await? + .into_iter() + .filter_map(|stored| { + let resource_version = stored_profile_resource_version(&stored); + stored.profile.map(|profile| { + ProviderTypeProfile::from_proto(&profile_response_payload( + profile, + resource_version, + )) + }) + }), + ); + Ok(profiles) +} + +async fn custom_provider_profiles( + store: &Store, + workspace: &str, +) -> Result, Status> { + let profiles: Vec = store + .list_messages(workspace, 10_000, 0) + .await + .map_err(|e| Status::internal(format!("list provider profiles failed: {e}")))?; + Ok(profiles) } fn normalize_profile_id_request(id: &str) -> Result { @@ -1684,7 +1787,7 @@ fn add_empty_profile_set_diagnostic( async fn profile_conflict_diagnostics( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, profiles: &[(String, ProviderTypeProfile)], ) -> Result, Status> { let mut diagnostics = Vec::new(); @@ -1705,7 +1808,7 @@ async fn profile_conflict_diagnostics( continue; } if store - .get_message_by_name::(&id) + .get_message_by_name::(workspace, &id) .await .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? .is_some() @@ -1724,7 +1827,7 @@ async fn profile_conflict_diagnostics( async fn profile_update_target_diagnostics( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, profiles: &[(String, ProviderTypeProfile)], target_id: &str, ) -> Result, Status> { @@ -1758,7 +1861,7 @@ async fn profile_update_target_diagnostics( return Ok(diagnostics); } if store - .get_message_by_name::(target_id) + .get_message_by_name::(workspace, target_id) .await .map_err(|e| Status::internal(format!("fetch provider profile failed: {e}")))? .is_none() @@ -1792,7 +1895,7 @@ async fn profile_update_target_diagnostics( async fn profile_attached_sandbox_diagnostics( store: &Store, - catalog: &EffectiveProviderProfileCatalog, + workspace: &str, profiles: &[(String, ProviderTypeProfile)], operation: &str, ) -> Result, Status> { @@ -1808,7 +1911,7 @@ async fn profile_attached_sandbox_diagnostics( return Ok(Vec::new()); } - let sandboxes = scan_sandboxes(store, |sandbox| { + let sandboxes = scan_sandboxes(store, workspace, |sandbox| { sandbox .spec .as_ref() @@ -1825,7 +1928,7 @@ async fn profile_attached_sandbox_diagnostics( for provider_name in &spec.providers { let Some(provider) = store - .get_message_by_name::(provider_name) + .get_message_by_name::(workspace, provider_name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? else { @@ -1843,9 +1946,9 @@ async fn profile_attached_sandbox_diagnostics( imported_profiles_used.push(used); } } else { - bindings.extend(dynamic_token_grant_bindings_for_provider_with_catalog( - catalog, &provider, - )); + bindings.extend( + dynamic_token_grant_bindings_for_provider(store, &provider).await?, + ); } } @@ -1871,6 +1974,51 @@ async fn profile_attached_sandbox_diagnostics( Ok(diagnostics) } +fn stored_provider_profile_for_workspace( + profile: ProviderProfile, + workspace: &str, +) -> StoredProviderProfile { + use crate::persistence::current_time_ms; + let now_ms = current_time_ms(); + let profile = profile_storage_payload(profile); + StoredProviderProfile { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: uuid::Uuid::new_v4().to_string(), + name: profile.id.clone(), + created_at_ms: now_ms, + labels: std::collections::HashMap::new(), + resource_version: 0, + workspace: workspace.to_string(), + }), + profile: Some(profile), + } +} + +#[cfg(test)] +fn stored_provider_profile(profile: ProviderProfile) -> StoredProviderProfile { + stored_provider_profile_for_workspace(profile, "default") +} + +fn profile_storage_payload(mut profile: ProviderProfile) -> ProviderProfile { + profile.resource_version = 0; + profile +} + +fn profile_response_payload( + mut profile: ProviderProfile, + resource_version: u64, +) -> ProviderProfile { + profile.resource_version = resource_version; + profile +} + +fn stored_profile_resource_version(stored: &StoredProviderProfile) -> u64 { + stored + .metadata + .as_ref() + .map_or(0, |metadata| metadata.resource_version) +} + fn proto_diagnostic(diagnostic: ProfileValidationDiagnostic) -> ProviderProfileDiagnostic { ProviderProfileDiagnostic { source: diagnostic.source, @@ -1887,10 +2035,14 @@ fn has_errors(diagnostics: &[ProfileValidationDiagnostic]) -> bool { .any(|diagnostic| diagnostic.severity == "error") } -async fn sandboxes_using_profile(store: &Store, profile_id: &str) -> Result, Status> { +async fn sandboxes_using_profile( + store: &Store, + workspace: &str, + profile_id: &str, +) -> Result, Status> { // Collect all sandboxes that reference at least one provider — pagination // is handled by `scan_sandboxes`; the async provider lookup happens below. - let candidates = scan_sandboxes(store, |sandbox| { + let candidates = scan_sandboxes(store, workspace, |sandbox| { let has_providers = sandbox .spec .as_ref() @@ -1904,7 +2056,7 @@ async fn sandboxes_using_profile(store: &Store, profile_id: &str) -> Result(provider_name) + .get_message_by_name::(workspace, provider_name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? else { @@ -1926,6 +2078,8 @@ pub(super) async fn handle_update_provider( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; let Some(mut provider) = req.provider else { emit_provider_lifecycle( "custom", @@ -1938,12 +2092,7 @@ pub(super) async fn handle_update_provider( provider .credential_expires_at_ms .extend(req.credential_expires_at_ms); - let catalog = state - .provider_profile_sources - .snapshot_catalog(state.store.as_ref()) - .await?; - let result = - update_provider_record_with_catalog(state.store.as_ref(), &catalog, provider).await; + let result = update_provider_record(state.store.as_ref(), &workspace, provider).await; match result { Ok(provider) => { emit_provider_lifecycle( @@ -1971,12 +2120,14 @@ pub(super) async fn handle_get_provider_refresh_status( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; if request.provider.trim().is_empty() { return Err(Status::invalid_argument("provider is required")); } let provider = state .store - .get_message_by_name::(&request.provider) + .get_message_by_name::(&workspace, &request.provider) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::not_found("provider not found"))?; @@ -1990,6 +2141,7 @@ pub(super) async fn handle_get_provider_refresh_status( } else { crate::provider_refresh::get_refresh_state( state.store.as_ref(), + &workspace, provider.object_id(), request.credential_key.trim(), ) @@ -2011,6 +2163,8 @@ pub(super) async fn handle_configure_provider_refresh( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; let provider_name = request.provider.trim(); let credential_key = request.credential_key.trim(); if provider_name.is_empty() { @@ -2093,7 +2247,7 @@ pub(super) async fn handle_configure_provider_refresh( let provider = state .store - .get_message_by_name::(provider_name) + .get_message_by_name::(&workspace, provider_name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::not_found("provider not found"))?; @@ -2103,12 +2257,14 @@ pub(super) async fn handle_configure_provider_refresh( .await?; validate_provider_credential_key_available_for_attached_sandboxes_with_catalog( state.store.as_ref(), - &catalog, + &workspace, &provider, credential_key, ) .await?; - let refresh_defaults = provider_refresh_defaults(&catalog, &provider, credential_key); + let refresh_defaults = + provider_refresh_defaults(state.store.as_ref(), &provider, credential_key) + .await?; validate_refresh_material(&request.material, refresh_defaults.as_ref())?; let material_scopes = crate::provider_refresh::material_scopes(&request.material); let token_url = refresh_defaults @@ -2151,6 +2307,7 @@ pub(super) async fn handle_configure_provider_refresh( } let existing_refresh_state = crate::provider_refresh::get_refresh_state( state.store.as_ref(), + &workspace, provider.object_id(), credential_key, ) @@ -2163,6 +2320,7 @@ pub(super) async fn handle_configure_provider_refresh( }); let mut state_record = crate::provider_refresh::new_refresh_state( &provider, + &workspace, credential_key, crate::provider_refresh::NewRefreshStateConfig { strategy, @@ -2189,7 +2347,7 @@ pub(super) async fn handle_configure_provider_refresh( created_at_ms: 0, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: std::collections::HashMap::new(), + workspace: String::new(), }), r#type: String::new(), credentials: std::collections::HashMap::new(), @@ -2198,8 +2356,9 @@ pub(super) async fn handle_configure_provider_refresh( credential_key.to_string(), expires_at_ms, )]), + profile_workspace: String::new(), }; - update_provider_record_with_catalog(state.store.as_ref(), &catalog, updated).await?; + update_provider_record(state.store.as_ref(), &workspace, updated).await?; } Ok(Response::new(ConfigureProviderRefreshResponse { @@ -2214,6 +2373,8 @@ pub(super) async fn handle_rotate_provider_credential( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; let provider_name = request.provider.trim(); let credential_key = request.credential_key.trim(); if provider_name.is_empty() { @@ -2224,6 +2385,7 @@ pub(super) async fn handle_rotate_provider_credential( } let refresh_state = crate::provider_refresh::refresh_provider_credential( state.store.as_ref(), + &workspace, provider_name, credential_key, ) @@ -2241,6 +2403,8 @@ pub(super) async fn handle_delete_provider_refresh( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; let provider_name = request.provider.trim(); let credential_key = request.credential_key.trim(); if provider_name.is_empty() { @@ -2251,18 +2415,20 @@ pub(super) async fn handle_delete_provider_refresh( } let provider = state .store - .get_message_by_name::(provider_name) + .get_message_by_name::(&workspace, provider_name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::not_found("provider not found"))?; let existing_refresh_state = crate::provider_refresh::get_refresh_state( state.store.as_ref(), + &workspace, provider.object_id(), credential_key, ) .await?; let deleted_refresh_state = crate::provider_refresh::delete_refresh_state( state.store.as_ref(), + &workspace, provider.object_id(), credential_key, ) @@ -2289,7 +2455,7 @@ pub(super) async fn handle_delete_provider_refresh( created_at_ms: 0, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: std::collections::HashMap::new(), + workspace: String::new(), }), r#type: String::new(), credentials: std::collections::HashMap::new(), @@ -2298,8 +2464,9 @@ pub(super) async fn handle_delete_provider_refresh( credential_key.to_string(), 0, )]), + profile_workspace: String::new(), }; - update_provider_record_with_catalog(state.store.as_ref(), &catalog, updated).await?; + update_provider_record(state.store.as_ref(), &workspace, updated).await?; } Ok(Response::new(DeleteProviderRefreshResponse { @@ -2311,9 +2478,12 @@ pub(super) async fn handle_delete_provider( state: &Arc, request: Request, ) -> Result, Status> { - let name = request.into_inner().name; - let provider_profile = provider_profile_for_name(state.store.as_ref(), &name).await; - let result = delete_provider_record(state.store.as_ref(), &name).await; + let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; + let name = req.name; + let provider_profile = provider_profile_for_name(state.store.as_ref(), &workspace, &name).await; + let result = delete_provider_record(state.store.as_ref(), &workspace, &name).await; match result { Ok(deleted) => { let outcome = TelemetryOutcome::from_success(deleted); @@ -2352,9 +2522,13 @@ fn emit_provider_profile_lifecycle( openshell_core::telemetry::emit_provider_lifecycle(operation, outcome, provider_profile); } -async fn provider_profile_for_name(store: &Store, name: &str) -> Option { +async fn provider_profile_for_name( + store: &Store, + workspace: &str, + name: &str, +) -> Option { store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .ok() .flatten() @@ -2389,13 +2563,13 @@ mod tests { use crate::grpc::{MAX_MAP_KEY_LEN, MAX_PROVIDER_TYPE_LEN}; use crate::persistence::test_store; use openshell_core::proto::{ - DeleteProviderProfileRequest, GetProviderProfileRequest, ImportProviderProfilesRequest, - L7Allow, L7Rule, LintProviderProfilesRequest, ListProviderProfilesRequest, NetworkBinary, - NetworkEndpoint, ProviderCredentialRefresh, ProviderCredentialRefreshMaterial, - ProviderCredentialTokenGrant, ProviderCredentialTokenGrantAudienceOverride, - ProviderProfile, ProviderProfileCategory, ProviderProfileCredential, - ProviderProfileImportItem, Sandbox, SandboxSpec, StoredProviderProfile, - UpdateProviderProfilesRequest, + CreateWorkspaceRequest, DeleteProviderProfileRequest, GetProviderProfileRequest, + ImportProviderProfilesRequest, L7Allow, L7Rule, LintProviderProfilesRequest, + ListProviderProfilesRequest, NetworkBinary, NetworkEndpoint, ProviderCredentialRefresh, + ProviderCredentialRefreshMaterial, ProviderCredentialTokenGrant, + ProviderCredentialTokenGrantAudienceOverride, ProviderProfile, ProviderProfileCategory, + ProviderProfileCredential, ProviderProfileImportItem, Sandbox, SandboxSpec, + StoredProviderProfile, UpdateProviderProfilesRequest, }; use openshell_core::{ObjectId, ObjectName}; use std::collections::HashMap; @@ -2556,6 +2730,7 @@ mod tests { profile: Some(profile), source: format!("{id}.yaml"), }], + workspace: "default".to_string(), }), ) .await @@ -2569,6 +2744,7 @@ mod tests { ) -> Provider { create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -2576,12 +2752,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: provider_type.to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -2599,6 +2776,7 @@ mod tests { let err = validate_provider_environment_keys_unique( store, + "default", &["provider-a".to_string(), "provider-b".to_string()], ) .await @@ -2627,6 +2805,7 @@ mod tests { validate_provider_environment_keys_unique( store, + "default", &["provider-default".to_string(), "provider-admin".to_string()], ) .await @@ -2642,6 +2821,7 @@ mod tests { create_empty_token_grant_provider(store, "provider-existing", "grant-existing").await; create_provider_record( store, + "default", provider_with_values("provider-candidate", "grant-new"), ) .await @@ -2654,7 +2834,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec![ @@ -2684,6 +2864,7 @@ mod tests { profile: Some(profile), source: "grant-new.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -2711,6 +2892,7 @@ mod tests { profile: Some(custom_profile("guarded-import")), source: "guarded-import.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -2743,7 +2925,7 @@ mod tests { state.store.put_message(&stored).await.unwrap(); let before: StoredProviderProfile = state .store - .get_message_by_name("custom-api") + .get_message_by_name("default", "custom-api") .await .unwrap() .unwrap(); @@ -2765,6 +2947,7 @@ mod tests { }), expected_resource_version: 0, id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -2779,7 +2962,7 @@ mod tests { ); let after: StoredProviderProfile = state .store - .get_message_by_name("custom-api") + .get_message_by_name("default", "custom-api") .await .unwrap() .unwrap(); @@ -2809,6 +2992,7 @@ mod tests { }), expected_resource_version: 0, id: "github".to_string(), + workspace: "default".to_string(), }), ) .await @@ -2830,6 +3014,7 @@ mod tests { }), expected_resource_version: 0, id: "missing-custom".to_string(), + workspace: "default".to_string(), }), ) .await @@ -2861,6 +3046,7 @@ mod tests { }), expected_resource_version: 0, id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -2883,6 +3069,7 @@ mod tests { }), expected_resource_version: 0, id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -2910,7 +3097,7 @@ mod tests { .unwrap(); let profile_a_version = state .store - .get_message_by_name::("profile-a") + .get_message_by_name::("default", "profile-a") .await .unwrap() .unwrap() @@ -2931,6 +3118,7 @@ mod tests { }), expected_resource_version: 0, id: "profile-a".to_string(), + workspace: "default".to_string(), }), ) .await @@ -2946,7 +3134,7 @@ mod tests { })); let stored_b: StoredProviderProfile = state .store - .get_message_by_name("profile-b") + .get_message_by_name("default", "profile-b") .await .unwrap() .unwrap(); @@ -2970,7 +3158,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec![ @@ -2986,7 +3174,7 @@ mod tests { let mut profile = custom_profile("grant-updated"); profile.resource_version = store - .get_message_by_name::("grant-updated") + .get_message_by_name::("default", "grant-updated") .await .unwrap() .unwrap() @@ -3011,6 +3199,7 @@ mod tests { }), expected_resource_version: 0, id: "grant-updated".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3033,7 +3222,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: provider_type.to_string(), credentials: [ @@ -3049,6 +3238,7 @@ mod tests { .into_iter() .collect(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), } } @@ -3124,6 +3314,7 @@ mod tests { profile: Some(profile), source: format!("{id}.yaml"), }], + workspace: "default".to_string(), }), ) .await @@ -3183,6 +3374,7 @@ mod tests { Request::new(ListProviderProfilesRequest { limit: 100, offset: 0, + workspace: "default".to_string(), }), ) .await @@ -3229,6 +3421,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: "github".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3246,6 +3439,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: "generic".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3263,6 +3457,7 @@ mod tests { profile: Some(custom_profile("custom-api")), source: "custom-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -3277,6 +3472,7 @@ mod tests { Request::new(ListProviderProfilesRequest { limit: 100, offset: 0, + workspace: "default".to_string(), }), ) .await @@ -3293,6 +3489,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3313,6 +3510,7 @@ mod tests { profile: Some(custom_profile("github")), source: "github.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -3340,6 +3538,7 @@ mod tests { profile: Some(custom_profile("custom-llm")), source: "custom-llm.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -3353,6 +3552,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: "custom-llm".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3383,6 +3583,7 @@ mod tests { source: "case.yaml".to_string(), }, ], + workspace: "default".to_string(), }), ) .await @@ -3410,6 +3611,7 @@ mod tests { profile: Some(custom_profile("alex-api")), source: "alex-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -3419,6 +3621,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: " Alex-API ".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3432,6 +3635,7 @@ mod tests { &state, Request::new(DeleteProviderProfileRequest { id: " Alex-API ".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3460,6 +3664,7 @@ mod tests { source: "bulk-two.yaml".to_string(), }, ], + workspace: "default".to_string(), }), ) .await @@ -3477,7 +3682,10 @@ mod tests { for id in ["bulk-one", "bulk-two"] { let missing = handle_get_provider_profile( &state, - Request::new(GetProviderProfileRequest { id: id.to_string() }), + Request::new(GetProviderProfileRequest { + id: id.to_string(), + workspace: "default".to_string(), + }), ) .await .unwrap_err(); @@ -3526,6 +3734,7 @@ mod tests { }), source: "advanced-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -3538,6 +3747,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: "advanced-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3581,6 +3791,7 @@ mod tests { source: "lint-two.yaml".to_string(), }, ], + workspace: "default".to_string(), }), ) .await @@ -3597,7 +3808,10 @@ mod tests { for id in ["lint-one", "lint-two"] { let missing = handle_get_provider_profile( &state, - Request::new(GetProviderProfileRequest { id: id.to_string() }), + Request::new(GetProviderProfileRequest { + id: id.to_string(), + workspace: "default".to_string(), + }), ) .await .unwrap_err(); @@ -3605,6 +3819,77 @@ mod tests { } } + #[tokio::test] + async fn lint_provider_profiles_checks_conflicts_in_request_workspace() { + let state = test_server_state().await; + + handle_import_provider_profiles( + &state, + Request::new(ImportProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile("scoped-lint")), + source: "scoped-lint.yaml".to_string(), + }], + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + + let conflict = handle_lint_provider_profiles( + &state, + Request::new(LintProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile("scoped-lint")), + source: "scoped-lint.yaml".to_string(), + }], + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert!( + !conflict.valid, + "lint should detect conflict with existing profile in the same workspace" + ); + assert!(conflict.diagnostics.iter().any(|d| { + d.profile_id == "scoped-lint" && d.message.contains("already exists") + })); + + crate::grpc::workspace::handle_create_workspace( + &state, + Request::new(CreateWorkspaceRequest { + name: "alpha".to_string(), + labels: HashMap::new(), + }), + ) + .await + .unwrap(); + + let no_conflict = handle_lint_provider_profiles( + &state, + Request::new(LintProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile("scoped-lint")), + source: "scoped-lint.yaml".to_string(), + }], + workspace: "alpha".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert!( + no_conflict + .diagnostics + .iter() + .all(|d| d.profile_id != "scoped-lint" + || !d.message.contains("already exists")), + "lint against different workspace should not see profile from default" + ); + } + #[tokio::test] async fn delete_provider_profile_rejects_builtin_and_in_use_custom_profiles() { let state = test_server_state().await; @@ -3615,6 +3900,7 @@ mod tests { profile: Some(custom_profile("custom-api")), source: "custom-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -3624,6 +3910,7 @@ mod tests { &state, Request::new(DeleteProviderProfileRequest { id: "github".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3632,6 +3919,7 @@ mod tests { create_provider_record( state.store.as_ref(), + "default", provider_with_values("custom-provider", "custom-api"), ) .await @@ -3645,7 +3933,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["custom-provider".to_string()], @@ -3660,6 +3948,7 @@ mod tests { &state, Request::new(DeleteProviderProfileRequest { id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3674,6 +3963,7 @@ mod tests { import_test_graph_refresh_profile(&state).await; create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -3681,7 +3971,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(), credentials: std::iter::once(( @@ -3691,6 +3981,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -3710,6 +4001,7 @@ mod tests { ]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: Some(expires_at_ms), + workspace: "default".to_string(), }), ) .await @@ -3724,6 +4016,7 @@ mod tests { Request::new(GetProviderRefreshStatusRequest { provider: "msgraph".to_string(), credential_key: "MS_GRAPH_ACCESS_TOKEN".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3734,7 +4027,7 @@ mod tests { let provider = state .store - .get_message_by_name::("msgraph") + .get_message_by_name::("default", "msgraph") .await .unwrap() .expect("provider"); @@ -3750,6 +4043,7 @@ mod tests { Request::new(DeleteProviderRefreshRequest { provider: "msgraph".to_string(), credential_key: "MS_GRAPH_ACCESS_TOKEN".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3762,6 +4056,7 @@ mod tests { Request::new(GetProviderRefreshStatusRequest { provider: "msgraph".to_string(), credential_key: "MS_GRAPH_ACCESS_TOKEN".to_string(), + workspace: "default".to_string(), }), ) .await @@ -3771,7 +4066,7 @@ mod tests { let provider_after_delete = state .store - .get_message_by_name::("msgraph") + .get_message_by_name::("default", "msgraph") .await .unwrap() .expect("provider"); @@ -3920,6 +4215,7 @@ mod tests { let state = test_server_state().await; create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -3927,7 +4223,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-vertex-ai".to_string(), credentials: std::iter::once(( @@ -3937,6 +4233,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -3960,6 +4257,7 @@ mod tests { ]), secret_material_keys: vec!["private_key".to_string()], expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -3984,6 +4282,7 @@ mod tests { import_test_graph_refresh_profile(&state).await; create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -3991,7 +4290,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(), credentials: std::iter::once(( @@ -4001,6 +4300,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4020,6 +4320,7 @@ mod tests { ]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: Some(refresh_expires_at_ms), + workspace: "default".to_string(), }), ) .await @@ -4028,6 +4329,7 @@ mod tests { let manual_expires_at_ms = refresh_expires_at_ms + 60_000; update_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4035,7 +4337,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: HashMap::new(), @@ -4044,6 +4346,7 @@ mod tests { "MS_GRAPH_ACCESS_TOKEN".to_string(), manual_expires_at_ms, )]), + profile_workspace: "default".to_string(), }, ) .await @@ -4054,6 +4357,7 @@ mod tests { Request::new(DeleteProviderRefreshRequest { provider: "msgraph".to_string(), credential_key: "MS_GRAPH_ACCESS_TOKEN".to_string(), + workspace: "default".to_string(), }), ) .await @@ -4063,7 +4367,7 @@ mod tests { let provider_after_delete = state .store - .get_message_by_name::("msgraph") + .get_message_by_name::("default", "msgraph") .await .unwrap() .expect("provider"); @@ -4081,6 +4385,7 @@ mod tests { import_test_graph_refresh_profile(&state).await; create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4088,7 +4393,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(), credentials: std::iter::once(( @@ -4098,12 +4403,14 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4111,13 +4418,14 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(), credentials: std::iter::once(("OTHER_TOKEN".to_string(), "other".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4131,7 +4439,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["existing-graph".to_string(), "refreshing-graph".to_string()], @@ -4155,6 +4463,7 @@ mod tests { ]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -4176,6 +4485,7 @@ mod tests { for name in ["first-graph", "second-graph"] { create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4183,12 +4493,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4203,7 +4514,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["first-graph".to_string(), "second-graph".to_string()], @@ -4227,6 +4538,7 @@ mod tests { ]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -4245,6 +4557,7 @@ mod tests { ]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -4263,6 +4576,7 @@ mod tests { import_test_graph_refresh_profile(&state).await; create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4270,7 +4584,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: TEST_GRAPH_PROVIDER_TYPE.to_string(), credentials: std::iter::once(( @@ -4280,6 +4594,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4302,6 +4617,7 @@ mod tests { ]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -4318,6 +4634,7 @@ mod tests { material: HashMap::from([("tenant_id".to_string(), "tenant".to_string())]), secret_material_keys: vec!["client_secret".to_string()], expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -4331,6 +4648,7 @@ mod tests { let state = test_server_state().await; create_provider_record( state.store.as_ref(), + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4338,7 +4656,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "outlook".to_string(), credentials: std::iter::once(( @@ -4348,6 +4666,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4366,6 +4685,7 @@ mod tests { material: HashMap::new(), secret_material_keys: Vec::new(), expires_at_ms: None, + workspace: "default".to_string(), }), ) .await @@ -4394,6 +4714,7 @@ mod tests { profile: Some(custom_profile("custom-api")), source: "custom-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await @@ -4403,6 +4724,7 @@ mod tests { &state, Request::new(DeleteProviderProfileRequest { id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -4414,6 +4736,7 @@ mod tests { &state, Request::new(GetProviderProfileRequest { id: "custom-api".to_string(), + workspace: "default".to_string(), }), ) .await @@ -4437,6 +4760,7 @@ mod tests { &task_state, Request::new(DeleteProviderProfileRequest { id: "guarded-delete".to_string(), + workspace: "default".to_string(), }), ) .await @@ -4463,7 +4787,7 @@ mod tests { let store = test_store().await; let created = provider_with_values("gitlab-local", "gitlab"); - let persisted = create_provider_record(&store, created.clone()) + let persisted = create_provider_record(&store, "default", created.clone()) .await .unwrap(); assert_eq!(persisted.object_name(), "gitlab-local"); @@ -4471,18 +4795,25 @@ mod tests { assert!(!persisted.object_id().is_empty()); let provider_id = persisted.object_id().to_string(); - let duplicate_err = create_provider_record(&store, created).await.unwrap_err(); + let duplicate_err = create_provider_record(&store, "default", created) + .await + .unwrap_err(); assert_eq!(duplicate_err.code(), Code::AlreadyExists); - let loaded = get_provider_record(&store, "gitlab-local").await.unwrap(); + let loaded = get_provider_record(&store, "default", "gitlab-local") + .await + .unwrap(); assert_eq!(loaded.object_id(), provider_id); - let listed = list_provider_records(&store, 100, 0).await.unwrap(); + let listed = list_provider_records(&store, "default", 100, 0) + .await + .unwrap(); assert_eq!(listed.len(), 1); assert_eq!(listed[0].object_name(), "gitlab-local"); let updated = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4490,7 +4821,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "gitlab".to_string(), credentials: std::iter::once(( @@ -4501,6 +4832,7 @@ mod tests { config: std::iter::once(("endpoint".to_string(), "https://gitlab.com".to_string())) .collect(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4517,7 +4849,7 @@ mod tests { Some(&"REDACTED".to_string()), ); let stored: Provider = store - .get_message_by_name("gitlab-local") + .get_message_by_name("default", "gitlab-local") .await .unwrap() .unwrap(); @@ -4535,17 +4867,17 @@ mod tests { ); assert_eq!(updated.config.get("region"), Some(&"us-west".to_string())); - let deleted = delete_provider_record(&store, "gitlab-local") + let deleted = delete_provider_record(&store, "default", "gitlab-local") .await .unwrap(); assert!(deleted); - let deleted_again = delete_provider_record(&store, "gitlab-local") + let deleted_again = delete_provider_record(&store, "default", "gitlab-local") .await .unwrap(); assert!(!deleted_again); - let missing = get_provider_record(&store, "gitlab-local") + let missing = get_provider_record(&store, "default", "gitlab-local") .await .unwrap_err(); assert_eq!(missing.code(), Code::NotFound); @@ -4557,6 +4889,7 @@ mod tests { let provider = create_provider_record( &store, + "default", Provider { credential_expires_at_ms: HashMap::from([("API_TOKEN".to_string(), 123_456)]), ..provider_with_values("gitlab-local", "gitlab") @@ -4566,6 +4899,7 @@ mod tests { .unwrap(); let refresh_state = crate::provider_refresh::new_refresh_state( &provider, + "default", "API_TOKEN", crate::provider_refresh::NewRefreshStateConfig { strategy: ProviderCredentialRefreshStrategy::External, @@ -4586,7 +4920,7 @@ mod tests { .await .unwrap(); - let deleted = delete_provider_record(&store, "gitlab-local") + let deleted = delete_provider_record(&store, "default", "gitlab-local") .await .unwrap(); assert!(deleted); @@ -4602,9 +4936,13 @@ mod tests { async fn delete_provider_rejects_attached_provider() { let store = test_store().await; - create_provider_record(&store, provider_with_values("gitlab-local", "gitlab")) - .await - .unwrap(); + create_provider_record( + &store, + "default", + provider_with_values("gitlab-local", "gitlab"), + ) + .await + .unwrap(); store .put_message(&Sandbox { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { @@ -4613,7 +4951,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["gitlab-local".to_string()], @@ -4624,7 +4962,7 @@ mod tests { .await .unwrap(); - let err = delete_provider_record(&store, "gitlab-local") + let err = delete_provider_record(&store, "default", "gitlab-local") .await .unwrap_err(); assert_eq!(err.code(), Code::FailedPrecondition); @@ -4641,7 +4979,9 @@ mod tests { // Create provider and verify resource_version: 1 in response let created = provider_with_values("test-provider", "openai"); - let persisted = create_provider_record(&store, created).await.unwrap(); + let persisted = create_provider_record(&store, "default", created) + .await + .unwrap(); assert_eq!( persisted.metadata.as_ref().unwrap().resource_version, 1, @@ -4651,6 +4991,7 @@ mod tests { // Update provider and verify resource_version: 2 in response let updated = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4658,7 +4999,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "openai".to_string(), credentials: std::iter::once(( @@ -4668,6 +5009,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4681,6 +5023,7 @@ mod tests { // Update again and verify resource_version: 3 let updated_again = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4688,7 +5031,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "openai".to_string(), credentials: std::iter::once(( @@ -4698,6 +5041,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4716,6 +5060,7 @@ mod tests { let create_missing_type = create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4723,12 +5068,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4737,6 +5083,7 @@ mod tests { let create_missing_credentials = create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4744,12 +5091,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "gitlab".to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4807,12 +5155,14 @@ mod tests { }), source: "delegated-refresh-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await .unwrap(); let delegated_refresh_bootstrap_provider = create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4820,12 +5170,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "delegated-refresh-api".to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4844,12 +5195,14 @@ mod tests { profile: Some(mixed_required_profile), source: "mixed-required-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await .unwrap(); let mixed_required_empty = create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4857,12 +5210,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "mixed-required-api".to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4881,12 +5235,14 @@ mod tests { profile: Some(optional_static_profile), source: "optional-static-api.yaml".to_string(), }], + workspace: "default".to_string(), }), ) .await .unwrap(); let optional_static_empty = create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4894,12 +5250,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "optional-static-api".to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4908,6 +5265,7 @@ mod tests { let vertex_empty = create_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4915,26 +5273,30 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-vertex-ai".to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); assert!(vertex_empty.credentials.is_empty()); - let get_err = get_provider_record(store, "").await.unwrap_err(); + let get_err = get_provider_record(store, "default", "").await.unwrap_err(); assert_eq!(get_err.code(), Code::InvalidArgument); - let delete_err = delete_provider_record(store, "").await.unwrap_err(); + let delete_err = delete_provider_record(store, "default", "") + .await + .unwrap_err(); assert_eq!(delete_err.code(), Code::InvalidArgument); let update_missing_err = update_provider_record( store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4942,12 +5304,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4960,10 +5323,13 @@ mod tests { let store = test_store().await; let created = provider_with_values("noop-test", "nvidia"); - let persisted = create_provider_record(&store, created).await.unwrap(); + let persisted = create_provider_record(&store, "default", created) + .await + .unwrap(); let updated = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -4971,12 +5337,13 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -4996,7 +5363,7 @@ mod tests { ); assert_eq!(updated.config.get("region"), Some(&"us-west".to_string())); let stored: Provider = store - .get_message_by_name("noop-test") + .get_message_by_name("default", "noop-test") .await .unwrap() .unwrap(); @@ -5008,10 +5375,13 @@ mod tests { let store = test_store().await; let created = provider_with_values("delete-key-test", "openai"); - create_provider_record(&store, created).await.unwrap(); + create_provider_record(&store, "default", created) + .await + .unwrap(); let updated = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5019,12 +5389,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: std::iter::once(("SECONDARY".to_string(), String::new())).collect(), config: std::iter::once(("region".to_string(), String::new())).collect(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5043,7 +5414,7 @@ mod tests { ); assert!(!updated.config.contains_key("region")); let stored: Provider = store - .get_message_by_name("delete-key-test") + .get_message_by_name("default", "delete-key-test") .await .unwrap() .unwrap(); @@ -5060,10 +5431,13 @@ mod tests { let store = test_store().await; let created = provider_with_values("type-preserve-test", "anthropic"); - create_provider_record(&store, created).await.unwrap(); + create_provider_record(&store, "default", created) + .await + .unwrap(); let updated = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5071,12 +5445,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5090,10 +5465,13 @@ mod tests { let store = test_store().await; let created = provider_with_values("type-change-test", "nvidia"); - create_provider_record(&store, created).await.unwrap(); + create_provider_record(&store, "default", created) + .await + .unwrap(); let err = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5101,12 +5479,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "openai".to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5121,11 +5500,14 @@ mod tests { let store = test_store().await; let created = provider_with_values("validate-merge-test", "gitlab"); - create_provider_record(&store, created).await.unwrap(); + create_provider_record(&store, "default", created) + .await + .unwrap(); let oversized_key = "K".repeat(MAX_MAP_KEY_LEN + 1); let err = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5133,12 +5515,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: std::iter::once((oversized_key, "value".to_string())).collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5162,17 +5545,19 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: oversized_type.clone(), credentials: std::iter::once(("API_TOKEN".to_string(), "old".to_string())).collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }; store.put_message(&legacy).await.unwrap(); let updated = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5180,13 +5565,14 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: std::iter::once(("API_TOKEN".to_string(), "new".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5198,7 +5584,9 @@ mod tests { #[tokio::test] async fn resolve_provider_env_empty_list_returns_empty() { let store = test_store().await; - let result = resolve_provider_environment(&store, &[]).await.unwrap(); + let result = resolve_provider_environment(&store, "default", &[]) + .await + .unwrap(); assert!(result.is_empty()); } @@ -5212,7 +5600,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "claude".to_string(), credentials: [ @@ -5227,10 +5615,13 @@ mod tests { )) .collect(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }; - create_provider_record(&store, provider).await.unwrap(); + create_provider_record(&store, "default", provider) + .await + .unwrap(); - let result = resolve_provider_environment(&store, &["claude-local".to_string()]) + let result = resolve_provider_environment(&store, "default", &["claude-local".to_string()]) .await .unwrap(); assert_eq!(result.get("ANTHROPIC_API_KEY"), Some(&"sk-abc".to_string())); @@ -5243,14 +5634,16 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", provider_with_values("static-provider", "unprofiled-static-api"), ) .await .unwrap(); - let result = resolve_provider_environment(&store, &["static-provider".to_string()]) - .await - .unwrap(); + let result = + resolve_provider_environment(&store, "default", &["static-provider".to_string()]) + .await + .unwrap(); assert_eq!(result.get("API_TOKEN"), Some(&"token-123".to_string())); assert!(result.dynamic_credentials.is_empty()); @@ -5267,7 +5660,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "test".to_string(), credentials: [ @@ -5283,12 +5676,16 @@ mod tests { ] .into_iter() .collect(), + profile_workspace: "default".to_string(), }; - create_provider_record(&store, provider).await.unwrap(); - - let result = resolve_provider_environment(&store, &["expiring-provider".to_string()]) + create_provider_record(&store, "default", provider) .await .unwrap(); + + let result = + resolve_provider_environment(&store, "default", &["expiring-provider".to_string()]) + .await + .unwrap(); assert_eq!(result.get("FRESH_TOKEN"), Some(&"fresh".to_string())); assert!(!result.contains_key("STALE_TOKEN")); assert_eq!( @@ -5300,7 +5697,7 @@ mod tests { #[tokio::test] async fn resolve_provider_env_unknown_name_returns_error() { let store = test_store().await; - let err = resolve_provider_environment(&store, &["nonexistent".to_string()]) + let err = resolve_provider_environment(&store, "default", &["nonexistent".to_string()]) .await .unwrap_err(); assert_eq!(err.code(), Code::FailedPrecondition); @@ -5317,7 +5714,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "test".to_string(), credentials: [ @@ -5329,12 +5726,16 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }; - create_provider_record(&store, provider).await.unwrap(); - - let result = resolve_provider_environment(&store, &["test-provider".to_string()]) + create_provider_record(&store, "default", provider) .await .unwrap(); + + let result = + resolve_provider_environment(&store, "default", &["test-provider".to_string()]) + .await + .unwrap(); assert_eq!(result.get("VALID_KEY"), Some(&"value".to_string())); assert!(!result.contains_key("nested.api_key")); assert!(!result.contains_key("bad-key")); @@ -5345,6 +5746,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5352,7 +5754,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "claude".to_string(), credentials: std::iter::once(( @@ -5362,12 +5764,14 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5375,13 +5779,14 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "gitlab".to_string(), credentials: std::iter::once(("GITLAB_TOKEN".to_string(), "glpat-xyz".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5389,6 +5794,7 @@ mod tests { let result = resolve_provider_environment( &store, + "default", &["claude-local".to_string(), "gitlab-local".to_string()], ) .await @@ -5402,6 +5808,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5409,19 +5816,21 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "claude".to_string(), credentials: std::iter::once(("SHARED_KEY".to_string(), "first-value".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5429,7 +5838,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "gitlab".to_string(), credentials: std::iter::once(( @@ -5439,6 +5848,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5446,6 +5856,7 @@ mod tests { let err = resolve_provider_environment( &store, + "default", &["provider-a".to_string(), "provider-b".to_string()], ) .await @@ -5461,6 +5872,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5468,7 +5880,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-vertex-ai".to_string(), credentials: std::iter::once(( @@ -5486,12 +5898,13 @@ mod tests { .into_iter() .collect(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-local".to_string()]) + let result = resolve_provider_environment(&store, "default", &["vertex-local".to_string()]) .await .unwrap(); @@ -5536,6 +5949,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5543,7 +5957,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-vertex-ai".to_string(), credentials: [ @@ -5560,14 +5974,16 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-bootstrap".to_string()]) - .await - .unwrap(); + let result = + resolve_provider_environment(&store, "default", &["vertex-bootstrap".to_string()]) + .await + .unwrap(); assert!(!result.contains_key("GOOGLE_SERVICE_ACCOUNT_KEY")); assert_eq!( @@ -5581,6 +5997,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5588,7 +6005,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-vertex-ai".to_string(), credentials: std::iter::once(( @@ -5598,14 +6015,16 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-no-config".to_string()]) - .await - .unwrap(); + let result = + resolve_provider_environment(&store, "default", &["vertex-no-config".to_string()]) + .await + .unwrap(); // Static flags still present. assert!(!result.contains_key("CLAUDE_CODE_USE_VERTEX")); @@ -5630,6 +6049,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5637,7 +6057,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-vertex-ai".to_string(), credentials: [ @@ -5657,14 +6077,16 @@ mod tests { .into_iter() .collect(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); - let result = resolve_provider_environment(&store, &["vertex-collision".to_string()]) - .await - .unwrap(); + let result = + resolve_provider_environment(&store, "default", &["vertex-collision".to_string()]) + .await + .unwrap(); // Credential value wins over the injected static value. assert_eq!( @@ -5678,6 +6100,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5685,19 +6108,20 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "openai".to_string(), credentials: std::iter::once(("OPENAI_API_KEY".to_string(), "sk-test".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); - let result = resolve_provider_environment(&store, &["openai-local".to_string()]) + let result = resolve_provider_environment(&store, "default", &["openai-local".to_string()]) .await .unwrap(); @@ -5717,6 +6141,7 @@ mod tests { let store = test_store().await; create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5724,7 +6149,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "outlook".to_string(), credentials: std::iter::once(( @@ -5734,12 +6159,14 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await .unwrap(); create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5747,7 +6174,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-drive".to_string(), credentials: std::iter::once(( @@ -5757,6 +6184,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5768,7 +6196,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["provider-a".to_string(), "provider-b".to_string()], @@ -5780,6 +6208,7 @@ mod tests { let err = update_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5787,7 +6216,7 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), credentials: std::iter::once(( @@ -5797,6 +6226,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5815,6 +6245,7 @@ mod tests { create_provider_record( &store, + "default", Provider { metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { id: String::new(), @@ -5822,7 +6253,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "claude".to_string(), credentials: std::iter::once(( @@ -5832,6 +6263,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }, ) .await @@ -5844,7 +6276,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["my-claude".to_string()], @@ -5861,7 +6293,7 @@ mod tests { .unwrap() .unwrap(); let spec = loaded.spec.unwrap(); - let env = resolve_provider_environment(&store, &spec.providers) + let env = resolve_provider_environment(&store, "default", &spec.providers) .await .unwrap(); @@ -5881,7 +6313,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec::default()), status: None, @@ -5895,7 +6327,7 @@ mod tests { .unwrap() .unwrap(); let spec = loaded.spec.unwrap(); - let env = resolve_provider_environment(&store, &spec.providers) + let env = resolve_provider_environment(&store, "default", &spec.providers) .await .unwrap(); @@ -5917,7 +6349,7 @@ mod tests { // Create a valid provider let provider = provider_with_values("test-validate-provider", "test-type"); - let created = create_provider_record(&store, provider.clone()) + let created = create_provider_record(&store, "default", provider.clone()) .await .unwrap(); @@ -5929,12 +6361,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: String::new(), // Empty type is ignored in update credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }; // Attempt to update with an oversized credential key (exceeds MAX_MAP_KEY_LEN) @@ -5943,7 +6376,7 @@ mod tests { "oversized-key-value".to_string(), ); - let result = update_provider_record(&store, update_req).await; + let result = update_provider_record(&store, "default", update_req).await; // Update should fail with InvalidArgument due to oversized key assert!(result.is_err(), "update with invalid data should fail"); @@ -5961,7 +6394,7 @@ mod tests { // Verify database still contains the ORIGINAL valid provider (not the invalid one) let stored = store - .get_message_by_name::("test-validate-provider") + .get_message_by_name::("default", "test-validate-provider") .await .unwrap() .expect("provider should still exist"); @@ -5993,11 +6426,17 @@ mod tests { // Spawn two concurrent creation attempts for the same provider let store1 = store.clone(); let provider1 = provider.clone(); - let handle1 = tokio::spawn(async move { create_provider_record(&store1, provider1).await }); + let handle1 = + tokio::spawn( + async move { create_provider_record(&store1, "default", provider1).await }, + ); let store2 = store.clone(); let provider2 = provider.clone(); - let handle2 = tokio::spawn(async move { create_provider_record(&store2, provider2).await }); + let handle2 = + tokio::spawn( + async move { create_provider_record(&store2, "default", provider2).await }, + ); // Wait for both to complete let result1 = handle1.await.unwrap(); @@ -6029,7 +6468,7 @@ mod tests { .find_map(Result::ok) .expect("should have one successful creation"); let retrieved = store - .get_message_by_name::("test-concurrent-provider") + .get_message_by_name::("default", "test-concurrent-provider") .await .unwrap(); assert!( @@ -6056,6 +6495,7 @@ mod tests { &state, Request::new(CreateProviderRequest { provider: Some(provider.clone()), + workspace: "default".to_string(), }), ) .await @@ -6064,7 +6504,7 @@ mod tests { // Fetch the provider to get its current resource_version let current = state .store - .get_message_by_name::("test-provider") + .get_message_by_name::("default", "test-provider") .await .unwrap() .unwrap(); @@ -6083,6 +6523,7 @@ mod tests { Request::new(UpdateProviderRequest { provider: Some(updated_provider.clone()), credential_expires_at_ms: HashMap::new(), + workspace: "default".to_string(), }), ) .await @@ -6124,6 +6565,7 @@ mod tests { &state, Request::new(CreateProviderRequest { provider: Some(provider.clone()), + workspace: "default".to_string(), }), ) .await @@ -6132,7 +6574,7 @@ mod tests { // Fetch the current state let current = state .store - .get_message_by_name::("test-provider") + .get_message_by_name::("default", "test-provider") .await .unwrap() .unwrap(); @@ -6151,6 +6593,7 @@ mod tests { Request::new(UpdateProviderRequest { provider: Some(stale_provider), credential_expires_at_ms: HashMap::new(), + workspace: "default".to_string(), }), ) .await @@ -6167,7 +6610,7 @@ mod tests { // Verify the provider was not modified let unchanged = state .store - .get_message_by_name::("test-provider") + .get_message_by_name::("default", "test-provider") .await .unwrap() .unwrap(); @@ -6191,6 +6634,7 @@ mod tests { &state, Request::new(CreateProviderRequest { provider: Some(provider.clone()), + workspace: "default".to_string(), }), ) .await @@ -6199,7 +6643,7 @@ mod tests { // All three clients fetch the provider and see the same version let initial = state .store - .get_message_by_name::("test-provider") + .get_message_by_name::("default", "test-provider") .await .unwrap() .unwrap(); @@ -6221,6 +6665,7 @@ mod tests { Request::new(UpdateProviderRequest { provider: Some(updated), credential_expires_at_ms: HashMap::new(), + workspace: "default".to_string(), }), ) .await @@ -6253,7 +6698,7 @@ mod tests { // Final provider should have exactly 1 new credential key and resource_version = initial_version + 1 let final_provider = state .store - .get_message_by_name::("test-provider") + .get_message_by_name::("default", "test-provider") .await .unwrap() .unwrap(); @@ -6277,12 +6722,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "google-cloud".to_string(), credentials: HashMap::new(), config, credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), } } @@ -6380,12 +6826,13 @@ mod tests { created_at_ms: 0, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: "github".to_string(), credentials: HashMap::new(), config: HashMap::from([("project_id".to_string(), "should-be-ignored".to_string())]), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), }; let mut env = HashMap::new(); openshell_providers::ProviderRegistry::new().inject_env(&provider, &mut env); @@ -6394,4 +6841,533 @@ mod tests { "non-GCP provider should not inject any env vars" ); } + + #[tokio::test] + async fn provider_crud_is_workspace_isolated() { + use openshell_core::proto::{ + CreateProviderRequest, CreateWorkspaceRequest, DeleteProviderRequest, + GetProviderRequest, ListProvidersRequest, + }; + + let state = test_server_state().await; + + // Create a second workspace "beta". + crate::grpc::workspace::handle_create_workspace( + &state, + Request::new(CreateWorkspaceRequest { + name: "beta".to_string(), + labels: HashMap::new(), + }), + ) + .await + .unwrap(); + + // Create same-named provider in each workspace via handlers. + let make_provider = || Provider { + metadata: None, + r#type: "custom".to_string(), + credentials: HashMap::from([("TOKEN".to_string(), "secret".to_string())]), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: String::new(), + }; + + let created_default = handle_create_provider( + &state, + Request::new(CreateProviderRequest { + provider: Some({ + let mut p = make_provider(); + p.metadata = Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "shared-name".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: String::new(), + }); + p + }), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + let default_id = created_default + .provider + .as_ref() + .unwrap() + .object_id() + .to_string(); + + let created_beta = handle_create_provider( + &state, + Request::new(CreateProviderRequest { + provider: Some({ + let mut p = make_provider(); + p.metadata = Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "shared-name".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: String::new(), + }); + p + }), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + let beta_id = created_beta + .provider + .as_ref() + .unwrap() + .object_id() + .to_string(); + + assert_ne!(default_id, beta_id); + + // Get in each workspace returns the correct provider. + let got = handle_get_provider( + &state, + Request::new(GetProviderRequest { + name: "shared-name".to_string(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.provider.as_ref().unwrap().object_id(), default_id); + + let got = handle_get_provider( + &state, + Request::new(GetProviderRequest { + name: "shared-name".to_string(), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.provider.as_ref().unwrap().object_id(), beta_id); + + // List is workspace-scoped. + let listed = handle_list_providers( + &state, + Request::new(ListProvidersRequest { + limit: 100, + offset: 0, + workspace: "default".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.providers.len(), 1); + assert_eq!(listed.providers[0].object_id(), default_id); + + let listed = handle_list_providers( + &state, + Request::new(ListProvidersRequest { + limit: 100, + offset: 0, + workspace: "beta".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.providers.len(), 1); + assert_eq!(listed.providers[0].object_id(), beta_id); + + // Delete in "default" does not affect "beta". + let deleted = handle_delete_provider( + &state, + Request::new(DeleteProviderRequest { + name: "shared-name".to_string(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert!(deleted.deleted); + + let listed = handle_list_providers( + &state, + Request::new(ListProvidersRequest { + limit: 100, + offset: 0, + workspace: "default".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert!(listed.providers.is_empty()); + + let got = handle_get_provider( + &state, + Request::new(GetProviderRequest { + name: "shared-name".to_string(), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.provider.as_ref().unwrap().object_id(), beta_id); + + // all_workspaces returns providers from all workspaces. + // Re-create the "default" provider. + handle_create_provider( + &state, + Request::new(CreateProviderRequest { + provider: Some({ + let mut p = make_provider(); + p.metadata = Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "provider-d".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: String::new(), + }); + p + }), + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + + let listed = handle_list_providers( + &state, + Request::new(ListProvidersRequest { + limit: 100, + offset: 0, + workspace: String::new(), + all_workspaces: true, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.providers.len(), 2); + + // all_workspaces with non-empty workspace is rejected. + let err = handle_list_providers( + &state, + Request::new(ListProvidersRequest { + limit: 100, + offset: 0, + workspace: "default".to_string(), + all_workspaces: true, + }), + ) + .await + .unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + } + + #[tokio::test] + async fn create_provider_rejects_cross_workspace_profile_workspace() { + let store = test_store().await; + let provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "cross-ws".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: "default".to_string(), + }), + r#type: "claude".to_string(), + credentials: HashMap::from([("ANTHROPIC_API_KEY".to_string(), "sk-123".to_string())]), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: "other-workspace".to_string(), + }; + let err = create_provider_record(&store, "default", provider) + .await + .unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("profile_workspace")); + } + + #[tokio::test] + async fn create_provider_accepts_global_profile_workspace() { + let store = test_store().await; + let provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "global-profile".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: "default".to_string(), + }), + r#type: "claude".to_string(), + credentials: HashMap::from([("ANTHROPIC_API_KEY".to_string(), "sk-123".to_string())]), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: String::new(), + }; + let created = create_provider_record(&store, "default", provider) + .await + .unwrap(); + assert!(created.profile_workspace.is_empty()); + } + + #[tokio::test] + async fn create_provider_accepts_same_workspace_profile_workspace() { + let store = test_store().await; + let provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "same-ws-profile".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: "default".to_string(), + }), + r#type: "claude".to_string(), + credentials: HashMap::from([("ANTHROPIC_API_KEY".to_string(), "sk-123".to_string())]), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), + }; + let created = create_provider_record(&store, "default", provider) + .await + .unwrap(); + assert_eq!(created.profile_workspace, "default"); + } + + #[tokio::test] + async fn update_provider_rejects_profile_workspace_change() { + let store = test_store().await; + let provider = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "immutable-pw".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: "default".to_string(), + }), + r#type: "claude".to_string(), + credentials: HashMap::from([("ANTHROPIC_API_KEY".to_string(), "sk-123".to_string())]), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), + }; + create_provider_record(&store, "default", provider) + .await + .unwrap(); + + let update = Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "immutable-pw".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: "default".to_string(), + }), + r#type: String::new(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: "other".to_string(), + }; + let err = update_provider_record(&store, "default", update) + .await + .unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("profile_workspace")); + } + + #[tokio::test] + async fn provider_with_global_profile_resolves_platform_scoped_profile() { + use crate::persistence::{ObjectName, WriteCondition}; + use prost::Message; + + let store = test_store().await; + + let stored = stored_provider_profile_for_workspace(custom_profile("global-custom"), ""); + store + .put_if( + StoredProviderProfile::object_type(), + stored.object_id(), + stored.object_name(), + "", + &stored.encode_to_vec(), + None, + WriteCondition::MustCreate, + ) + .await + .unwrap(); + + let profile = get_provider_type_profile(&store, "", "global-custom") + .await + .unwrap(); + assert!(profile.is_some(), "global profile should be found at workspace ''"); + + let miss = get_provider_type_profile(&store, "default", "global-custom") + .await + .unwrap(); + assert!(miss.is_none(), "global profile should NOT be found at workspace 'default'"); + } + + #[tokio::test] + async fn provider_with_workspace_profile_resolves_workspace_scoped_profile() { + let state = test_server_state().await; + + handle_import_provider_profiles( + &state, + Request::new(ImportProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile("ws-custom")), + source: "ws-custom.yaml".to_string(), + }], + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + + create_provider_record( + state.store.as_ref(), + "default", + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "uses-ws".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + workspace: "default".to_string(), + }), + r#type: "ws-custom".to_string(), + credentials: HashMap::from([("TOKEN".to_string(), "val".to_string())]), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), + }, + ) + .await + .unwrap(); + + let profile = get_provider_type_profile(state.store.as_ref(), "default", "ws-custom") + .await + .unwrap(); + assert!(profile.is_some()); + } + + #[tokio::test] + async fn import_provider_profile_platform_and_workspace_scopes_are_isolated() { + let state = test_server_state().await; + + let import = |id: &str, workspace: &str| { + let state = state.clone(); + let id = id.to_string(); + let workspace = workspace.to_string(); + async move { + handle_import_provider_profiles( + &state, + Request::new(ImportProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + profile: Some(custom_profile(&id)), + source: format!("{id}.yaml"), + }], + workspace, + }), + ) + .await + .unwrap() + .into_inner() + } + }; + + let resp = import("e2e-platform", "").await; + assert!(resp.imported); + + let resp = import("e2e-workspace", "default").await; + assert!(resp.imported); + + let list = |workspace: &str| { + let state = state.clone(); + let workspace = workspace.to_string(); + async move { + handle_list_provider_profiles( + &state, + Request::new(ListProviderProfilesRequest { + limit: 200, + offset: 0, + workspace, + }), + ) + .await + .unwrap() + .into_inner() + } + }; + + let platform_profiles = list("").await; + assert!( + platform_profiles + .profiles + .iter() + .any(|p| p.id == "e2e-platform"), + "platform-scoped profile should appear in platform list" + ); + assert!( + !platform_profiles + .profiles + .iter() + .any(|p| p.id == "e2e-workspace"), + "workspace-scoped profile should NOT appear in platform list" + ); + + let workspace_profiles = list("default").await; + assert!( + workspace_profiles + .profiles + .iter() + .any(|p| p.id == "e2e-workspace"), + "workspace-scoped profile should appear in workspace list" + ); + assert!( + !workspace_profiles + .profiles + .iter() + .any(|p| p.id == "e2e-platform"), + "platform-scoped profile should NOT appear in workspace list" + ); + + let delete = |id: &str, workspace: &str| { + let state = state.clone(); + let id = id.to_string(); + let workspace = workspace.to_string(); + async move { + handle_delete_provider_profile( + &state, + Request::new(DeleteProviderProfileRequest { id, workspace }), + ) + .await + .unwrap() + .into_inner() + } + }; + + assert!(delete("e2e-platform", "").await.deleted); + assert!(delete("e2e-workspace", "default").await.deleted); + } } diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 8e2f6d25c5..47936ace5d 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -27,7 +27,7 @@ use openshell_core::telemetry::{ LifecycleOperation, LifecycleResource, SandboxTemplateSource, TelemetryComputeDriver, TelemetryOutcome, }; -use openshell_core::{ObjectId, ObjectName}; +use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; use prost::Message; use std::net::IpAddr; use std::pin::Pin; @@ -135,6 +135,9 @@ async fn handle_create_sandbox_inner( } crate::grpc::validation::validate_annotations(&request.annotations, "annotations")?; + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; + let _sandbox_sync_guard = if spec.providers.is_empty() { None } else { @@ -145,12 +148,13 @@ async fn handle_create_sandbox_inner( for name in &spec.providers { state .store - .get_message_by_name::(name) + .get_message_by_name::(&workspace, name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; } - validate_provider_environment_keys_unique(state.store.as_ref(), &spec.providers).await?; + validate_provider_environment_keys_unique(state.store.as_ref(), &workspace, &spec.providers) + .await?; // Ensure the template always carries the resolved image. let mut spec = spec; @@ -183,7 +187,7 @@ async fn handle_create_sandbox_inner( created_at_ms: now_ms, labels: request.labels.clone(), resource_version: 0, - annotations: request.annotations.clone(), + workspace, }), spec: Some(spec), status: None, @@ -238,14 +242,16 @@ pub(super) async fn handle_get_sandbox( state: &Arc, request: Request, ) -> Result, Status> { - let name = request.into_inner().name; - if name.is_empty() { + let req = request.into_inner(); + if req.name.is_empty() { return Err(Status::invalid_argument("name is required")); } + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; let sandbox = state .store - .get_message_by_name::(&name) + .get_message_by_name::(&workspace, &req.name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))?; @@ -260,21 +266,56 @@ pub(super) async fn handle_list_sandboxes( request: Request, ) -> Result, Status> { let request = request.into_inner(); + if request.all_workspaces && !request.workspace.is_empty() { + return Err(Status::invalid_argument( + "all_workspaces and workspace are mutually exclusive", + )); + } let limit = clamp_limit(request.limit, 100, MAX_PAGE_SIZE); - let sandboxes: Vec = if request.label_selector.is_empty() { - state - .store - .list_messages(limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? + let sandboxes: Vec = if request.all_workspaces { + if request.label_selector.is_empty() { + state + .store + .list_all_messages(limit, request.offset) + .await + .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? + } else { + crate::grpc::validation::validate_label_selector(&request.label_selector)?; + state + .store + .list_all_messages_with_selector( + &request.label_selector, + limit, + request.offset, + ) + .await + .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? + } } else { - crate::grpc::validation::validate_label_selector(&request.label_selector)?; - state - .store - .list_messages_with_selector(&request.label_selector, limit, request.offset) - .await - .map_err(|e| Status::internal(format!("list sandboxes with selector failed: {e}")))? + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; + if request.label_selector.is_empty() { + state + .store + .list_messages(&workspace, limit, request.offset) + .await + .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))? + } else { + crate::grpc::validation::validate_label_selector(&request.label_selector)?; + state + .store + .list_messages_with_selector( + &workspace, + &request.label_selector, + limit, + request.offset, + ) + .await + .map_err(|e| { + Status::internal(format!("list sandboxes with selector failed: {e}")) + })? + } }; Ok(Response::new(ListSandboxesResponse { sandboxes })) @@ -284,8 +325,11 @@ pub(super) async fn handle_list_sandbox_providers( state: &Arc, request: Request, ) -> Result, Status> { - let sandbox = sandbox_by_name(state, &request.into_inner().sandbox_name).await?; - let providers = providers_for_sandbox(state, &sandbox).await?; + let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; + let sandbox = sandbox_by_name(state, &workspace, &req.sandbox_name).await?; + let providers = providers_for_sandbox(state, &sandbox, &workspace).await?; Ok(Response::new(ListSandboxProvidersResponse { providers })) } @@ -294,6 +338,8 @@ pub(super) async fn handle_attach_sandbox_provider( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; if request.provider_name.is_empty() { return Err(Status::invalid_argument("provider_name is required")); } @@ -308,7 +354,7 @@ pub(super) async fn handle_attach_sandbox_provider( ))); } - get_provider_record(state.store.as_ref(), &request.provider_name) + get_provider_record(state.store.as_ref(), &workspace, &request.provider_name) .await .map_err(|err| { if err.code() == tonic::Code::NotFound { @@ -322,7 +368,7 @@ pub(super) async fn handle_attach_sandbox_provider( })?; let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; - let sandbox = sandbox_by_name(state, &request.sandbox_name).await?; + let sandbox = sandbox_by_name(state, &workspace, &request.sandbox_name).await?; let sandbox_id = sandbox .metadata .as_ref() @@ -358,8 +404,12 @@ pub(super) async fn handle_attach_sandbox_provider( candidate_spec.providers.push(request.provider_name.clone()); } validate_sandbox_spec(&request.sandbox_name, &candidate_spec)?; - validate_provider_environment_keys_unique(state.store.as_ref(), &candidate_spec.providers) - .await?; + validate_provider_environment_keys_unique( + state.store.as_ref(), + &workspace, + &candidate_spec.providers, + ) + .await?; let provider_name = request.provider_name.clone(); let attached = Arc::new(AtomicBool::new(false)); @@ -408,6 +458,8 @@ pub(super) async fn handle_detach_sandbox_provider( request: Request, ) -> Result, Status> { let request = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &request.workspace).await?; if request.provider_name.is_empty() { return Err(Status::invalid_argument("provider_name is required")); } @@ -422,7 +474,7 @@ pub(super) async fn handle_detach_sandbox_provider( } let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; - let sandbox = sandbox_by_name(state, &request.sandbox_name).await?; + let sandbox = sandbox_by_name(state, &workspace, &request.sandbox_name).await?; let sandbox_id = sandbox .metadata .as_ref() @@ -499,19 +551,22 @@ async fn handle_delete_sandbox_inner( state: &Arc, request: Request, ) -> Result, Status> { - let name = request.into_inner().name; + let req = request.into_inner(); + let name = req.name; if name.is_empty() { return Err(Status::invalid_argument("name is required")); } + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; let sandbox_id = state .store - .get_message_by_name::(&name) + .get_message_by_name::(&workspace, &name) .await .ok() .flatten() .map(|sandbox| sandbox.object_id().to_string()); - let deleted = state.compute.delete_sandbox(&name).await?; + let deleted = state.compute.delete_sandbox(&workspace, &name).await?; if deleted && let Some(sandbox_id) = sandbox_id { state.telemetry.end_sandbox_session(&sandbox_id); } @@ -519,14 +574,18 @@ async fn handle_delete_sandbox_inner( Ok(Response::new(DeleteSandboxResponse { deleted })) } -async fn sandbox_by_name(state: &Arc, name: &str) -> Result { +async fn sandbox_by_name( + state: &Arc, + workspace: &str, + name: &str, +) -> Result { if name.is_empty() { return Err(Status::invalid_argument("sandbox_name is required")); } state .store - .get_message_by_name::(name) + .get_message_by_name::(workspace, name) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found")) @@ -535,6 +594,7 @@ async fn sandbox_by_name(state: &Arc, name: &str) -> Result, sandbox: &Sandbox, + workspace: &str, ) -> Result, Status> { let provider_names = sandbox .spec @@ -544,7 +604,7 @@ async fn providers_for_sandbox( let mut providers = Vec::with_capacity(provider_names.len()); for name in provider_names { - let provider = get_provider_record(state.store.as_ref(), name) + let provider = get_provider_record(state.store.as_ref(), workspace, name) .await .map_err(|err| { if err.code() == tonic::Code::NotFound { @@ -1363,7 +1423,7 @@ pub(super) async fn handle_create_ssh_session( created_at_ms: now_ms, labels: std::collections::HashMap::new(), resource_version: 0, - annotations: std::collections::HashMap::new(), + workspace: sandbox.object_workspace().to_string(), }), sandbox_id: req.sandbox_id.clone(), token: token.clone(), @@ -1381,6 +1441,7 @@ pub(super) async fn handle_create_ssh_session( SshSession::object_type(), &token, session.object_name(), + session.object_workspace(), &session.encode_to_vec(), None, WriteCondition::MustCreate, @@ -1439,6 +1500,7 @@ pub(super) async fn handle_revoke_ssh_session( SshSession::object_type(), session.object_id(), session.object_name(), + session.object_workspace(), &session.encode_to_vec(), None, WriteCondition::MatchResourceVersion(resource_version), @@ -2309,13 +2371,14 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: provider_type.to_string(), credentials: std::iter::once((credential_key.to_string(), "secret".to_string())) .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), } } @@ -2327,7 +2390,7 @@ mod tests { created_at_ms: 1_000_000, labels: std::iter::once(("team".to_string(), "agents".to_string())).collect(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(openshell_core::proto::SandboxSpec { log_level: "debug".to_string(), @@ -2362,6 +2425,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "work-github".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2371,7 +2435,7 @@ mod tests { assert!(response.attached); let sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -2405,6 +2469,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "work-github".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2414,7 +2479,7 @@ mod tests { assert!(!response.attached); let providers = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap() @@ -2446,6 +2511,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "work-github".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2455,7 +2521,7 @@ mod tests { assert!(response.detached); let providers = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap() @@ -2470,6 +2536,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "work-github".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2496,6 +2563,7 @@ mod tests { &state, Request::new(ListSandboxProvidersRequest { sandbox_name: "work".to_string(), + workspace: String::new(), }), ) .await @@ -2525,6 +2593,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "missing".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2698,7 +2767,7 @@ mod tests { ..Default::default() }), labels: HashMap::new(), - annotations: HashMap::new(), + workspace: String::new(), }), ) .await @@ -2731,7 +2800,7 @@ mod tests { ..Default::default() }), labels: HashMap::new(), - annotations: HashMap::new(), + workspace: String::new(), }), ) .await @@ -2830,7 +2899,7 @@ mod tests { ..Default::default() }), labels: HashMap::new(), - annotations: HashMap::new(), + workspace: String::new(), }), ) .await @@ -2880,6 +2949,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "provider-b".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2926,6 +2996,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "provider-31".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2935,7 +3006,7 @@ mod tests { assert!(response.attached); let providers = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap() @@ -2980,6 +3051,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "provider-32".to_string(), expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -2991,7 +3063,7 @@ mod tests { // Verify sandbox was not modified let providers = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap() @@ -3026,6 +3098,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: long_name, expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -3052,6 +3125,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: long_name, expected_resource_version: 0, + workspace: String::new(), }), ) .await @@ -3203,7 +3277,7 @@ mod tests { // Fetch the sandbox to get its current resource_version let sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3216,6 +3290,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "github".to_string(), expected_resource_version: current_version, + workspace: String::new(), }), ) .await @@ -3227,7 +3302,7 @@ mod tests { // Verify the resource_version incremented let updated_sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3254,7 +3329,7 @@ mod tests { // Get current version let sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3267,6 +3342,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "github".to_string(), expected_resource_version: 99, + workspace: String::new(), }), ) .await @@ -3284,7 +3360,7 @@ mod tests { // Verify the sandbox was not modified let unchanged_sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3316,7 +3392,7 @@ mod tests { // Fetch the sandbox to get its current resource_version let sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3329,6 +3405,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "github".to_string(), expected_resource_version: current_version, + workspace: String::new(), }), ) .await @@ -3340,7 +3417,7 @@ mod tests { // Verify the resource_version incremented let updated_sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3367,7 +3444,7 @@ mod tests { // Get current version let sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3380,6 +3457,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: "github".to_string(), expected_resource_version: 99, + workspace: String::new(), }), ) .await @@ -3397,7 +3475,7 @@ mod tests { // Verify the sandbox was not modified let unchanged_sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3440,7 +3518,7 @@ mod tests { // All three clients fetch the sandbox and see version 1 let initial_version = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap() @@ -3460,6 +3538,7 @@ mod tests { sandbox_name: "work".to_string(), provider_name: format!("provider-{i}"), expected_resource_version: initial_version, + workspace: String::new(), }), ) .await @@ -3496,7 +3575,7 @@ mod tests { // Final sandbox should have exactly 1 provider and resource_version = initial_version + 1 let final_sandbox = state .store - .get_message_by_name::("work") + .get_message_by_name::("default", "work") .await .unwrap() .unwrap(); @@ -3506,4 +3585,214 @@ mod tests { initial_version + 1 ); } + + #[tokio::test] + async fn sandbox_crud_is_workspace_isolated() { + use crate::persistence::ObjectType; + use openshell_core::proto::{ + CreateWorkspaceRequest, GetSandboxRequest, ListSandboxesRequest, + }; + + let state = test_server_state().await; + + // Create a second workspace "beta". + crate::grpc::workspace::handle_create_workspace( + &state, + Request::new(CreateWorkspaceRequest { + name: "beta".to_string(), + labels: HashMap::new(), + }), + ) + .await + .unwrap(); + + // Seed a sandbox named "shared-name" in each workspace. + let mut sbx_default = test_sandbox("shared-name", Vec::new()); + sbx_default.metadata.as_mut().unwrap().id = "sbx-default-id".to_string(); + sbx_default.metadata.as_mut().unwrap().workspace = "default".to_string(); + state.store.put_message(&sbx_default).await.unwrap(); + + let mut sbx_beta = test_sandbox("shared-name", Vec::new()); + sbx_beta.metadata.as_mut().unwrap().id = "sbx-beta-id".to_string(); + sbx_beta.metadata.as_mut().unwrap().workspace = "beta".to_string(); + state.store.put_message(&sbx_beta).await.unwrap(); + + // Get in "default" returns the default sandbox. + let got = handle_get_sandbox( + &state, + Request::new(GetSandboxRequest { + name: "shared-name".to_string(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.sandbox.as_ref().unwrap().object_id(), "sbx-default-id"); + + // Get in "beta" returns the beta sandbox. + let got = handle_get_sandbox( + &state, + Request::new(GetSandboxRequest { + name: "shared-name".to_string(), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.sandbox.as_ref().unwrap().object_id(), "sbx-beta-id"); + + // List in "default" returns 1 sandbox. + let listed = handle_list_sandboxes( + &state, + Request::new(ListSandboxesRequest { + limit: 100, + offset: 0, + label_selector: String::new(), + workspace: "default".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.sandboxes.len(), 1); + assert_eq!(listed.sandboxes[0].object_id(), "sbx-default-id",); + + // List in "beta" returns 1 sandbox. + let listed = handle_list_sandboxes( + &state, + Request::new(ListSandboxesRequest { + limit: 100, + offset: 0, + label_selector: String::new(), + workspace: "beta".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.sandboxes.len(), 1); + assert_eq!(listed.sandboxes[0].object_id(), "sbx-beta-id"); + + // Delete in "default" (via store) does not affect "beta". + state + .store + .delete_by_name(Sandbox::object_type(), "default", "shared-name") + .await + .unwrap(); + + // "default" now has 0 sandboxes. + let listed = handle_list_sandboxes( + &state, + Request::new(ListSandboxesRequest { + limit: 100, + offset: 0, + label_selector: String::new(), + workspace: "default".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert!(listed.sandboxes.is_empty()); + + // "beta" still has its sandbox. + let got = handle_get_sandbox( + &state, + Request::new(GetSandboxRequest { + name: "shared-name".to_string(), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.sandbox.as_ref().unwrap().object_id(), "sbx-beta-id"); + + // all_workspaces returns sandboxes from all workspaces. + // Re-create the "default" sandbox so both workspaces have one. + state + .store + .put( + Sandbox::object_type(), + "sbx-default-2", + "sandbox-d", + "default", + &Sandbox::default().encode_to_vec(), + None, + ) + .await + .unwrap(); + let listed = handle_list_sandboxes( + &state, + Request::new(ListSandboxesRequest { + limit: 100, + offset: 0, + label_selector: String::new(), + workspace: String::new(), + all_workspaces: true, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.sandboxes.len(), 2); + + // all_workspaces with non-empty workspace is rejected. + let err = handle_list_sandboxes( + &state, + Request::new(ListSandboxesRequest { + limit: 100, + offset: 0, + label_selector: String::new(), + workspace: "default".to_string(), + all_workspaces: true, + }), + ) + .await + .unwrap_err(); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + } + + #[tokio::test] + async fn revoke_ssh_session_preserves_workspace() { + let state = test_server_state().await; + state + .store + .put_message(&test_sandbox("ws-test", Vec::new())) + .await + .unwrap(); + + let response = handle_create_ssh_session( + &state, + Request::new(CreateSshSessionRequest { + sandbox_id: "sandbox-ws-test".to_string(), + }), + ) + .await + .unwrap(); + let token = response.into_inner().token; + + handle_revoke_ssh_session( + &state, + Request::new(RevokeSshSessionRequest { + token: token.clone(), + }), + ) + .await + .unwrap(); + + let session: SshSession = state + .store + .get_message::(&token) + .await + .unwrap() + .expect("session should still exist after revocation"); + assert!(session.revoked); + assert_eq!(session.object_workspace(), "default"); + } } diff --git a/crates/openshell-server/src/grpc/service.rs b/crates/openshell-server/src/grpc/service.rs index 01d8dbfe8e..31e3389b4b 100644 --- a/crates/openshell-server/src/grpc/service.rs +++ b/crates/openshell-server/src/grpc/service.rs @@ -4,12 +4,12 @@ use std::collections::HashMap; use std::sync::Arc; -use openshell_core::ObjectId; use openshell_core::proto::datamodel::v1::ObjectMeta; use openshell_core::proto::{ DeleteServiceRequest, DeleteServiceResponse, ExposeServiceRequest, GetServiceRequest, ListServicesRequest, ListServicesResponse, Sandbox, ServiceEndpoint, ServiceEndpointResponse, }; +use openshell_core::{ObjectId, ObjectWorkspace}; use prost::Message as _; use tonic::{Request, Response, Status}; use uuid::Uuid; @@ -26,6 +26,8 @@ pub(super) async fn handle_expose_service( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; validate_endpoint_name("sandbox", &req.sandbox, MAX_SANDBOX_NAME_LEN)?; validate_optional_endpoint_name("service", &req.service, MAX_SERVICE_NAME_LEN)?; if req.target_port == 0 || req.target_port > u32::from(u16::MAX) { @@ -34,7 +36,7 @@ pub(super) async fn handle_expose_service( let sandbox = state .store - .get_message_by_name::(&req.sandbox) + .get_message_by_name::(&workspace, &req.sandbox) .await .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? .ok_or_else(|| Status::not_found("sandbox not found"))?; @@ -45,7 +47,7 @@ pub(super) async fn handle_expose_service( // Fetch existing endpoint to determine create vs. update path let existing = state .store - .get_message_by_name::(&key) + .get_message_by_name::(&workspace, &key) .await .map_err(|e| Status::internal(format!("fetch endpoint failed: {e}")))?; @@ -87,7 +89,7 @@ pub(super) async fn handle_expose_service( created_at_ms, labels: HashMap::from([("sandbox".to_string(), req.sandbox.clone())]), resource_version: 0, - annotations: HashMap::new(), + workspace: workspace.clone(), }), sandbox_id: sandbox.object_id().to_string(), sandbox_name: req.sandbox.clone(), @@ -103,6 +105,7 @@ pub(super) async fn handle_expose_service( ServiceEndpoint::object_type(), &id, &key, + &workspace, &endpoint.encode_to_vec(), Some(&labels_json), condition, @@ -115,7 +118,7 @@ pub(super) async fn handle_expose_service( meta.resource_version = result.resource_version; } - let url = service_routing::endpoint_url(&state.config, &req.sandbox, &req.service) + let url = service_routing::endpoint_url(&state.config, &workspace, &req.sandbox, &req.service) .unwrap_or_default(); service_routing::emit_service_endpoint_config_event(&endpoint, &url, created); @@ -130,10 +133,12 @@ pub(super) async fn handle_get_service( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; validate_endpoint_name("sandbox", &req.sandbox, MAX_SANDBOX_NAME_LEN)?; validate_optional_endpoint_name("service", &req.service, MAX_SERVICE_NAME_LEN)?; - let endpoint = get_service_endpoint(state, &req.sandbox, &req.service) + let endpoint = get_service_endpoint(state, &workspace, &req.sandbox, &req.service) .await? .ok_or_else(|| Status::not_found("service endpoint not found"))?; @@ -145,18 +150,42 @@ pub(super) async fn handle_list_services( request: Request, ) -> Result, Status> { let req = request.into_inner(); + if req.all_workspaces && !req.workspace.is_empty() { + return Err(Status::invalid_argument( + "all_workspaces and workspace are mutually exclusive", + )); + } if !req.sandbox.is_empty() { validate_endpoint_name("sandbox", &req.sandbox, MAX_SANDBOX_NAME_LEN)?; } let limit = super::clamp_limit(req.limit, 100, super::MAX_PAGE_SIZE); - let endpoints: Vec = if req.sandbox.is_empty() { - state.store.list_messages(limit, req.offset).await + let endpoints: Vec = if req.all_workspaces { + if !req.sandbox.is_empty() { + return Err(Status::invalid_argument( + "sandbox filter is not supported with all_workspaces", + )); + } + state.store.list_all_messages(limit, req.offset).await } else { - state - .store - .list_messages_with_selector(&format!("sandbox={}", req.sandbox), limit, req.offset) - .await + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; + if req.sandbox.is_empty() { + state + .store + .list_messages(&workspace, limit, req.offset) + .await + } else { + state + .store + .list_messages_with_selector( + &workspace, + &format!("sandbox={}", req.sandbox), + limit, + req.offset, + ) + .await + } } .map_err(|e| Status::internal(format!("list endpoints failed: {e}")))?; @@ -173,10 +202,12 @@ pub(super) async fn handle_delete_service( request: Request, ) -> Result, Status> { let req = request.into_inner(); + let workspace = + super::workspace::resolve_workspace(state.store.as_ref(), &req.workspace).await?; validate_endpoint_name("sandbox", &req.sandbox, MAX_SANDBOX_NAME_LEN)?; validate_optional_endpoint_name("service", &req.service, MAX_SERVICE_NAME_LEN)?; - let endpoint = get_service_endpoint(state, &req.sandbox, &req.service).await?; + let endpoint = get_service_endpoint(state, &workspace, &req.sandbox, &req.service).await?; let Some(endpoint) = endpoint else { return Ok(Response::new(DeleteServiceResponse { deleted: false })); }; @@ -184,7 +215,7 @@ pub(super) async fn handle_delete_service( let key = service_routing::endpoint_key(&req.sandbox, &req.service); let deleted = state .store - .delete_by_name(ServiceEndpoint::object_type(), &key) + .delete_by_name(ServiceEndpoint::object_type(), &workspace, &key) .await .map_err(|e| Status::internal(format!("delete endpoint failed: {e}")))?; @@ -197,13 +228,14 @@ pub(super) async fn handle_delete_service( async fn get_service_endpoint( state: &Arc, + workspace: &str, sandbox: &str, service: &str, ) -> Result, Status> { let key = service_routing::endpoint_key(sandbox, service); state .store - .get_message_by_name::(&key) + .get_message_by_name::(workspace, &key) .await .map_err(|e| Status::internal(format!("fetch endpoint failed: {e}"))) } @@ -212,8 +244,10 @@ fn service_endpoint_response( state: &Arc, endpoint: ServiceEndpoint, ) -> ServiceEndpointResponse { + let workspace = endpoint.object_workspace(); let url = service_routing::endpoint_url( &state.config, + workspace, &endpoint.sandbox_name, &endpoint.service_name, ) @@ -287,7 +321,7 @@ mod tests { created_at_ms: 1_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(openshell_core::proto::SandboxSpec::default()), ..Default::default() @@ -333,6 +367,7 @@ mod tests { service: "web".to_string(), target_port: 8080, domain: true, + workspace: "default".to_string(), }), ) .await @@ -346,6 +381,8 @@ mod tests { sandbox: "my-sandbox".to_string(), limit: 0, offset: 0, + workspace: "default".to_string(), + all_workspaces: false, }), ) .await @@ -362,6 +399,7 @@ mod tests { Request::new(GetServiceRequest { sandbox: "my-sandbox".to_string(), service: "web".to_string(), + workspace: "default".to_string(), }), ) .await @@ -374,6 +412,7 @@ mod tests { Request::new(DeleteServiceRequest { sandbox: "my-sandbox".to_string(), service: "web".to_string(), + workspace: "default".to_string(), }), ) .await @@ -386,6 +425,7 @@ mod tests { Request::new(GetServiceRequest { sandbox: "my-sandbox".to_string(), service: "web".to_string(), + workspace: "default".to_string(), }), ) .await @@ -398,6 +438,8 @@ mod tests { sandbox: "my-sandbox".to_string(), limit: 0, offset: 0, + workspace: "default".to_string(), + all_workspaces: false, }), ) .await @@ -421,6 +463,7 @@ mod tests { service: "web".to_string(), target_port: 8080, domain: true, + workspace: "default".to_string(), }), ) .await @@ -435,6 +478,7 @@ mod tests { service: "web".to_string(), target_port: 9090, domain: true, + workspace: "default".to_string(), }), ) .await @@ -459,6 +503,8 @@ mod tests { sandbox: "my-sandbox".to_string(), limit: 0, offset: 0, + workspace: "default".to_string(), + all_workspaces: false, }), ) .await @@ -480,6 +526,7 @@ mod tests { service: "web".to_string(), target_port: 7070, domain: true, + workspace: "default".to_string(), }), ) .await @@ -495,6 +542,7 @@ mod tests { service: "web".to_string(), target_port: 8080, domain: true, + workspace: "default".to_string(), }), ) .await @@ -509,6 +557,7 @@ mod tests { service: "web".to_string(), target_port: 9090, domain: true, + workspace: "default".to_string(), }), ) .await @@ -531,6 +580,7 @@ mod tests { Request::new(GetServiceRequest { sandbox: "my-sandbox".to_string(), service: "web".to_string(), + workspace: "default".to_string(), }), ) .await @@ -543,4 +593,223 @@ mod tests { ); assert_ne!(port, 7070, "port should not be the original value"); } + + #[tokio::test] + async fn service_crud_is_workspace_isolated() { + use openshell_core::proto::{ + CreateWorkspaceRequest, DeleteServiceRequest, GetServiceRequest, ListServicesRequest, + }; + + let state = test_server_state().await; + + // Create a second workspace "beta". + crate::grpc::workspace::handle_create_workspace( + &state, + Request::new(CreateWorkspaceRequest { + name: "beta".to_string(), + labels: HashMap::new(), + }), + ) + .await + .unwrap(); + + // Seed a sandbox named "my-sandbox" in each workspace. + seed_sandbox(&state, "my-sandbox").await; + + let mut sbx_beta = Sandbox { + metadata: Some(ObjectMeta { + id: "sandbox-my-sandbox-beta".to_string(), + name: "my-sandbox".to_string(), + created_at_ms: 1_000, + labels: HashMap::new(), + resource_version: 0, + workspace: "beta".to_string(), + }), + spec: Some(openshell_core::proto::SandboxSpec::default()), + ..Default::default() + }; + sbx_beta.set_phase(SandboxPhase::Ready as i32); + state.store.put_message(&sbx_beta).await.unwrap(); + + // Expose same service name on the same sandbox name in each workspace. + handle_expose_service( + &state, + Request::new(ExposeServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "web".to_string(), + target_port: 8080, + domain: true, + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + + handle_expose_service( + &state, + Request::new(ExposeServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "web".to_string(), + target_port: 9090, + domain: true, + workspace: "beta".to_string(), + }), + ) + .await + .unwrap(); + + // Get in "default" returns port 8080. + let got = handle_get_service( + &state, + Request::new(GetServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "web".to_string(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.endpoint.as_ref().unwrap().target_port, 8080); + + // Get in "beta" returns port 9090. + let got = handle_get_service( + &state, + Request::new(GetServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "web".to_string(), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.endpoint.as_ref().unwrap().target_port, 9090); + + // List in each workspace returns 1 service. + let listed = handle_list_services( + &state, + Request::new(ListServicesRequest { + sandbox: "my-sandbox".to_string(), + limit: 100, + offset: 0, + workspace: "default".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.services.len(), 1); + assert_eq!( + listed.services[0].endpoint.as_ref().unwrap().target_port, + 8080 + ); + + let listed = handle_list_services( + &state, + Request::new(ListServicesRequest { + sandbox: "my-sandbox".to_string(), + limit: 100, + offset: 0, + workspace: "beta".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.services.len(), 1); + assert_eq!( + listed.services[0].endpoint.as_ref().unwrap().target_port, + 9090 + ); + + // Delete in "default" does not affect "beta". + let deleted = handle_delete_service( + &state, + Request::new(DeleteServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "web".to_string(), + workspace: "default".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert!(deleted.deleted); + + let listed = handle_list_services( + &state, + Request::new(ListServicesRequest { + sandbox: "my-sandbox".to_string(), + limit: 100, + offset: 0, + workspace: "default".to_string(), + all_workspaces: false, + }), + ) + .await + .unwrap() + .into_inner(); + assert!(listed.services.is_empty()); + + let got = handle_get_service( + &state, + Request::new(GetServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "web".to_string(), + workspace: "beta".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(got.endpoint.as_ref().unwrap().target_port, 9090); + + // all_workspaces returns services from all workspaces. + // Re-create the "default" service. + handle_expose_service( + &state, + Request::new(ExposeServiceRequest { + sandbox: "my-sandbox".to_string(), + service: "api".to_string(), + target_port: 3000, + domain: true, + workspace: "default".to_string(), + }), + ) + .await + .unwrap(); + + let listed = handle_list_services( + &state, + Request::new(ListServicesRequest { + sandbox: String::new(), + limit: 100, + offset: 0, + workspace: String::new(), + all_workspaces: true, + }), + ) + .await + .unwrap() + .into_inner(); + assert_eq!(listed.services.len(), 2); + + // all_workspaces with non-empty workspace is rejected. + let err = handle_list_services( + &state, + Request::new(ListServicesRequest { + sandbox: String::new(), + limit: 100, + offset: 0, + workspace: "default".to_string(), + all_workspaces: true, + }), + ) + .await + .unwrap_err(); + assert_eq!(err.code(), tonic::Code::InvalidArgument); + } } diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index b604a3ef33..f543795db8 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -6,6 +6,7 @@ #![allow(clippy::result_large_err)] use crate::persistence::{ObjectType, Store, current_time_ms}; +use openshell_core::ObjectWorkspace; use openshell_core::proto::{ Provider, ProviderCredentialRefreshStatus, ProviderCredentialRefreshStrategy, StoredProviderCredentialRefreshState, @@ -79,7 +80,7 @@ pub async fn list_all_refresh_states( let mut offset = 0; loop { let records = store - .list( + .list_by_type( StoredProviderCredentialRefreshState::object_type(), REFRESH_WORKER_PAGE_SIZE, offset, @@ -108,24 +109,30 @@ pub async fn list_all_refresh_states( pub async fn get_refresh_state( store: &Store, + workspace: &str, provider_id: &str, credential_key: &str, ) -> Result, Status> { let name = refresh_state_name(provider_id, credential_key); store - .get_message_by_name::(&name) + .get_message_by_name::(workspace, &name) .await .map_err(|e| Status::internal(format!("fetch provider refresh state failed: {e}"))) } pub async fn delete_refresh_state( store: &Store, + workspace: &str, provider_id: &str, credential_key: &str, ) -> Result { let name = refresh_state_name(provider_id, credential_key); store - .delete_by_name(StoredProviderCredentialRefreshState::object_type(), &name) + .delete_by_name( + StoredProviderCredentialRefreshState::object_type(), + workspace, + &name, + ) .await .map_err(|e| Status::internal(format!("delete provider refresh state failed: {e}"))) } @@ -136,10 +143,11 @@ pub async fn delete_refresh_states_for_provider( ) -> Result { let states = list_refresh_states_for_provider(store, provider_id).await?; let mut deleted = 0; - for state in states { + for state in &states { if store .delete_by_name( StoredProviderCredentialRefreshState::object_type(), + state.object_workspace(), state.object_name(), ) .await @@ -181,6 +189,7 @@ pub struct NewRefreshStateConfig { #[allow(clippy::unnecessary_wraps)] pub fn new_refresh_state( provider: &Provider, + workspace: &str, credential_key: &str, config: NewRefreshStateConfig, ) -> Result { @@ -200,7 +209,7 @@ pub fn new_refresh_state( created_at_ms: now_ms, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: workspace.to_string(), }), provider_id, provider_name, @@ -295,15 +304,17 @@ pub fn is_gateway_mintable_strategy(strategy: ProviderCredentialRefreshStrategy) pub async fn refresh_provider_credential( store: &Store, + workspace: &str, provider_name: &str, credential_key: &str, ) -> Result { let provider = store - .get_message_by_name::(provider_name) + .get_message_by_name::(workspace, provider_name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::not_found("provider not found"))?; - let Some(mut state) = get_refresh_state(store, provider.object_id(), credential_key).await? + let Some(mut state) = + get_refresh_state(store, workspace, provider.object_id(), credential_key).await? else { return Err(Status::not_found("provider refresh state not found")); }; @@ -322,7 +333,7 @@ pub async fn refresh_provider_credential( Ok(minted) => { let now_ms = current_time_ms(); if let Err(err) = - apply_minted_credential(store, &provider, credential_key, &minted).await + apply_minted_credential(store, workspace, &provider, credential_key, &minted).await { state.status = "error".to_string(); state.last_error = err.message().to_string(); @@ -400,6 +411,7 @@ pub async fn refresh_provider_credential( async fn apply_minted_credential( store: &Store, + workspace: &str, provider: &Provider, credential_key: &str, minted: &MintedCredential, @@ -415,8 +427,10 @@ async fn apply_minted_credential( } else { updated.credential_expires_at_ms.remove(credential_key); } - crate::grpc::provider::validate_provider_update_against_attached_sandboxes(store, &updated) - .await?; + crate::grpc::provider::validate_provider_update_against_attached_sandboxes( + store, workspace, &updated, + ) + .await?; store .update_message_cas::(provider.object_id(), 0, |current| { current @@ -753,8 +767,13 @@ async fn run_refresh_worker_tick(store: &Store) -> Result<(), Status> { status = %state.status, "refreshing provider credential" ); - if let Err(err) = - refresh_provider_credential(store, &state.provider_name, &state.credential_key).await + if let Err(err) = refresh_provider_credential( + store, + state.object_workspace(), + &state.provider_name, + &state.credential_key, + ) + .await { warn!( provider = %state.provider_name, @@ -853,6 +872,7 @@ mod tests { let before_refresh_ms = crate::persistence::current_time_ms(); let state = new_refresh_state( &provider, + "default", "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials, @@ -871,9 +891,10 @@ mod tests { .unwrap(); put_refresh_state(&store, &state).await.unwrap(); - let refreshed = refresh_provider_credential(&store, "my-graph", "MS_GRAPH_ACCESS_TOKEN") - .await - .unwrap(); + let refreshed = + refresh_provider_credential(&store, "default", "my-graph", "MS_GRAPH_ACCESS_TOKEN") + .await + .unwrap(); assert_eq!(refreshed.status, "refreshed"); assert!(refreshed.expires_at_ms > 0); assert!(refreshed.next_refresh_at_ms > 0); @@ -881,7 +902,7 @@ mod tests { assert!(refreshed.last_error.is_empty()); let stored = store - .get_message_by_name::("my-graph") + .get_message_by_name::("default", "my-graph") .await .unwrap() .unwrap(); @@ -925,7 +946,7 @@ mod tests { created_at_ms: 1, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), spec: Some(SandboxSpec { providers: vec!["existing-graph".to_string(), "refreshing-graph".to_string()], @@ -937,6 +958,7 @@ mod tests { .unwrap(); let state = new_refresh_state( &provider_b, + "default", "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { strategy: ProviderCredentialRefreshStrategy::Oauth2ClientCredentials, @@ -955,21 +977,30 @@ mod tests { .unwrap(); put_refresh_state(&store, &state).await.unwrap(); - let err = refresh_provider_credential(&store, "refreshing-graph", "MS_GRAPH_ACCESS_TOKEN") - .await - .unwrap_err(); + let err = refresh_provider_credential( + &store, + "default", + "refreshing-graph", + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap_err(); assert_eq!(err.code(), tonic::Code::FailedPrecondition); assert!(err.message().contains("MS_GRAPH_ACCESS_TOKEN")); - let stored_state = - get_refresh_state(&store, provider_b.object_id(), "MS_GRAPH_ACCESS_TOKEN") - .await - .unwrap() - .unwrap(); + let stored_state = get_refresh_state( + &store, + "default", + provider_b.object_id(), + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap() + .unwrap(); assert_eq!(stored_state.status, "error"); assert!(stored_state.last_error.contains("MS_GRAPH_ACCESS_TOKEN")); let stored_provider = store - .get_message_by_name::("refreshing-graph") + .get_message_by_name::("default", "refreshing-graph") .await .unwrap() .unwrap(); @@ -1005,6 +1036,7 @@ mod tests { store.put_message(&provider).await.unwrap(); let state = new_refresh_state( &provider, + "default", "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { strategy: ProviderCredentialRefreshStrategy::Oauth2RefreshToken, @@ -1023,15 +1055,19 @@ mod tests { .unwrap(); put_refresh_state(&store, &state).await.unwrap(); - let refreshed = - refresh_provider_credential(&store, "my-delegated-graph", "MS_GRAPH_ACCESS_TOKEN") - .await - .unwrap(); + let refreshed = refresh_provider_credential( + &store, + "default", + "my-delegated-graph", + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap(); assert_eq!(refreshed.status, "refreshed"); assert!(refreshed.expires_at_ms > 0); let stored_provider = store - .get_message_by_name::("my-delegated-graph") + .get_message_by_name::("default", "my-delegated-graph") .await .unwrap() .unwrap(); @@ -1046,10 +1082,15 @@ mod tests { Some(&refreshed.expires_at_ms) ); - let stored_state = get_refresh_state(&store, provider.object_id(), "MS_GRAPH_ACCESS_TOKEN") - .await - .unwrap() - .unwrap(); + let stored_state = get_refresh_state( + &store, + "default", + provider.object_id(), + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap() + .unwrap(); assert_eq!( stored_state.material.get("refresh_token"), Some(&"rotated-refresh-token".to_string()) @@ -1084,6 +1125,7 @@ mod tests { store.put_message(&provider).await.unwrap(); let state = new_refresh_state( &provider, + "default", "GOOGLE_DRIVE_ACCESS_TOKEN", NewRefreshStateConfig { strategy: ProviderCredentialRefreshStrategy::GoogleServiceAccountJwt, @@ -1106,14 +1148,14 @@ mod tests { put_refresh_state(&store, &state).await.unwrap(); let refreshed = - refresh_provider_credential(&store, "my-drive", "GOOGLE_DRIVE_ACCESS_TOKEN") + refresh_provider_credential(&store, "default", "my-drive", "GOOGLE_DRIVE_ACCESS_TOKEN") .await .unwrap(); assert_eq!(refreshed.status, "refreshed"); assert!(refreshed.expires_at_ms > 0); let stored = store - .get_message_by_name::("my-drive") + .get_message_by_name::("default", "my-drive") .await .unwrap() .unwrap(); @@ -1130,6 +1172,7 @@ mod tests { store.put_message(&provider).await.unwrap(); let state = new_refresh_state( &provider, + "default", "MS_GRAPH_ACCESS_TOKEN", NewRefreshStateConfig { strategy: ProviderCredentialRefreshStrategy::External, @@ -1147,15 +1190,20 @@ mod tests { run_refresh_worker_tick(&store).await.unwrap(); - let stored_state = get_refresh_state(&store, provider.object_id(), "MS_GRAPH_ACCESS_TOKEN") - .await - .unwrap() - .unwrap(); + let stored_state = get_refresh_state( + &store, + "default", + provider.object_id(), + "MS_GRAPH_ACCESS_TOKEN", + ) + .await + .unwrap() + .unwrap(); assert_ne!(stored_state.status, "error"); assert!(stored_state.last_error.is_empty()); let stored_provider = store - .get_message_by_name::("my-external") + .get_message_by_name::("default", "my-external") .await .unwrap() .unwrap(); @@ -1174,12 +1222,13 @@ mod tests { created_at_ms: 1, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), r#type: provider_type.to_string(), credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + profile_workspace: "default".to_string(), } } diff --git a/crates/openshell-server/src/ssh_sessions.rs b/crates/openshell-server/src/ssh_sessions.rs index 4c6589b9f7..8c168eccfe 100644 --- a/crates/openshell-server/src/ssh_sessions.rs +++ b/crates/openshell-server/src/ssh_sessions.rs @@ -37,7 +37,7 @@ async fn reap_expired_sessions(store: &Store) -> Result<(), String> { let now_ms = now_ms(); let records = store - .list(SshSession::object_type(), 1000, 0) + .list_by_type(SshSession::object_type(), 1000, 0) .await .map_err(|e| e.to_string())?; @@ -86,7 +86,7 @@ mod tests { created_at_ms: 1000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), sandbox_id: sandbox_id.to_string(), token: id.to_string(), diff --git a/crates/openshell-server/src/supervisor_session.rs b/crates/openshell-server/src/supervisor_session.rs index 16ddbc0d47..03fd032d3a 100644 --- a/crates/openshell-server/src/supervisor_session.rs +++ b/crates/openshell-server/src/supervisor_session.rs @@ -841,7 +841,7 @@ mod tests { created_at_ms: 1_000_000, labels: HashMap::new(), resource_version: 0, - annotations: HashMap::new(), + workspace: "default".to_string(), }), ..Default::default() } diff --git a/crates/openshell-server/tests/common/mod.rs b/crates/openshell-server/tests/common/mod.rs index a5bfa1057e..93beeacf15 100644 --- a/crates/openshell-server/tests/common/mod.rs +++ b/crates/openshell-server/tests/common/mod.rs @@ -480,6 +480,55 @@ impl OpenShell for TestOpenShell { ) -> Result, Status> { Err(Status::unimplemented("not implemented in test")) } + + async fn create_workspace( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + + async fn get_workspace( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + + async fn list_workspaces( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + + async fn delete_workspace( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + + async fn add_workspace_member( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + + async fn remove_workspace_member( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } + + async fn list_workspace_members( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("not implemented in test")) + } } // --------------------------------------------------------------------------- diff --git a/crates/openshell-server/tests/supervisor_relay_integration.rs b/crates/openshell-server/tests/supervisor_relay_integration.rs index 98c5d99f55..721baacd88 100644 --- a/crates/openshell-server/tests/supervisor_relay_integration.rs +++ b/crates/openshell-server/tests/supervisor_relay_integration.rs @@ -419,6 +419,51 @@ impl OpenShell for RelayGateway { ) -> Result, Status> { Err(Status::unimplemented("unused")) } + async fn create_workspace( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + async fn get_workspace( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + async fn list_workspaces( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + async fn delete_workspace( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + + async fn add_workspace_member( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + + async fn remove_workspace_member( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + + async fn list_workspace_members( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } } // ---------------------------------------------------------------------------