From bf4c74f93f6ade8888dc972a6670651c4913f561 Mon Sep 17 00:00:00 2001 From: Abhinav Chordia Date: Sun, 11 Oct 2026 05:22:51 +0000 Subject: [PATCH] sbx-mgr: characterize stop, delete and create core RPC paths (tests only) Pin the RPC-layer behavior of Stop, Delete and Create in service/grpc_core.rs so the planned lifecycle consolidation has a safety net: - Create, from a forwarded peer and for every boot mode: owner authorization and every network-policy authority (Local refused, unspecified and unknown values invalid, absent policy and Cluster accepted). - Create, Stop and Delete refuse missing credentials and unlisted peers without persisting anything. - Stop selects only a live runtime and Delete any row. An empty template filter means none. Absent sandboxes succeed with no ids. - A non-blocking stop or delete returns once admitted, with a durable cleanup job. x-space-skip-cluster-sync skips only the admission-time dirty ref; completion still marks the sandbox dirty. - A template-manager peer may clean up only template-owned sandboxes. Shutdown refuses cleanup. - Auto-lifecycle delete guards: a template filter or non-canonical id is refused, and a stale policy aborts with the CAS-lost metadata. - Pool-clone disposal through Delete: refused while publication owns the clone and, after quarantine, for template-manager or auto-lifecycle callers or while cleanup is busy; an operator delete re-arms it. Template lookup connects to the fixed template-manager socket, so every create request carries an auto-lifecycle policy with an unknown authority, the last check before lookup. Each case asserts code and message, so removing the refusal a case pins stops at that barrier with a refusal the case does not expect. Auto-lifecycle authority cases have no later barrier and are left until template resolution can be injected. The two earlier forwarded-peer create tests are folded into the new table; their auto-lifecycle case is among those deferred. Stop and delete reach node cleanup only through the cleanup executor's existing test dispatch, which waits on test-granted permits. Services are built from explicit options: a fixed node id, absent sockets, and runtime liveness reading the test-owned run directory. They run in a child process with the feature-flag provider environment cleared. Admitted stops and ordinary deletes stat the fixed sidecar pid path of the fresh sandbox id. The absent-sandbox case stats the fixed sandbox cache path of a fresh id. Every wait on cleanup, including correct runs and shutdown, is bounded by 30 seconds. No production code changes. --- .../mgr/server/src/service/grpc_core/tests.rs | 1122 ++++++++++++++++- 1 file changed, 1070 insertions(+), 52 deletions(-) diff --git a/crates/libs/sbx/mgr/server/src/service/grpc_core/tests.rs b/crates/libs/sbx/mgr/server/src/service/grpc_core/tests.rs index df0b5b79fb..b3b7760dd3 100644 --- a/crates/libs/sbx/mgr/server/src/service/grpc_core/tests.rs +++ b/crates/libs/sbx/mgr/server/src/service/grpc_core/tests.rs @@ -1,70 +1,1088 @@ use super::*; +use crate::service::tests::service_tests::insert_test_sandbox; +use pplx_space_common_core::host::linux::identities::users; +use pplx_space_common_svc::PeerIdentity; use pplx_space_template_mgr_client::proto::TemplateFullInfo; -#[tokio::test] -async fn create_rejects_local_network_policy_from_forwarded_service_peer() { - use pplx_space_common_core::host::linux::identities::users; - use pplx_space_common_svc::PeerIdentity; - use pplx_space_net_common::{NetworkPolicyV1, proto::PolicyAuthority}; +fn peer_request(message: T, peer: Option) -> Request { + let mut request = Request::new(message); + if let Some(peer) = peer { + request.extensions_mut().insert(peer); + } + request +} - for boot_mode in [BootMode::Cold, BootMode::Pool] { - let service = crate::service::SandboxManagerService::new(crate::db::test_conn()); - let sandbox_id = SandboxId::new().to_string(); - let mut request = tonic::Request::new(CreateRequest { - template: "seed".into(), - owner: "root@local".into(), - runtime: pplx_space_common_core::types::RUNTIME_FIRECRACKER_ALIAS.into(), - boot_mode: boot_mode as i32, - sandbox_id: Some(sandbox_id.clone()), - network_policy: Some((&NetworkPolicyV1::allow_all()).into()), - network_policy_authority: PolicyAuthority::Local as i32, - ..Default::default() - }); - request.extensions_mut().insert(PeerIdentity::new( - users::SPACE_NODEAPI.uid, - users::SPACE_NODEAPI.primary_group.gid, - None, - )); +fn root_peer() -> PeerIdentity { + PeerIdentity::new(0, 0, None) +} - let error = service - .create(request) - .await - .expect_err("forwarded create must not persist a local network policy"); - assert_eq!(error.code(), tonic::Code::PermissionDenied, "{boot_mode:?}"); - assert!(!crate::db::sandbox_exists(&service.db_conn().unwrap(), &sandbox_id).unwrap()); - } +fn service_peer(user: &pplx_space_common_core::host::linux::identities::LinuxUser) -> PeerIdentity { + PeerIdentity::new(user.uid, user.primary_group.gid, None) } -#[tokio::test] -async fn create_rejects_local_auto_lifecycle_from_forwarded_service_peer() { - use pplx_space_common_core::host::linux::identities::users; - use pplx_space_common_svc::PeerIdentity; - use pplx_space_net_common::proto::PolicyAuthority; - - let service = crate::service::SandboxManagerService::new(crate::db::test_conn()); - let sandbox_id = SandboxId::new().to_string(); - let mut request = tonic::Request::new(CreateRequest { +/// Run the test in a child process without the feature-flag provider and +/// route-fence environment, so service construction cannot start a provider. +/// Call before constructing any service; return when false. +fn enter_isolated_test_process() -> bool { + crate::service::tests::enter_route_test_process(&[ + ("EPPO_SDK_KEY", None), + ("SPACE_FLAGD_ENABLED", None), + ]) +} + +/// A service built from explicit options. `Default` options read the node +/// identity from a fixed host file and probe runtimes under the fixed run +/// directory; here the node identity is fixed, runtime liveness reads +/// `scratch`, and the host-control and credential-manager sockets are absent +/// paths under `scratch`. +fn isolated_service( + conn: rusqlite::Connection, + scratch: &std::path::Path, + node_role: Option, +) -> SandboxManagerService { + let run_dir = scratch.to_owned(); + SandboxManagerService::new_with_options( + conn, + SandboxManagerServiceOptions { + host_control_socket_path: scratch.join("absent-host-control.sock"), + credential_manager_socket_path: scratch.join("absent-credential-manager.sock"), + sandbox_runtime_live: Arc::new(move |id| { + crate::service::cache_cleanup::is_sandbox_running_in(&run_dir, id) + }), + shutdown_token: tokio_util::sync::CancellationToken::new(), + task_tracker: tokio_util::task::TaskTracker::new(), + local_node_id: Some(pplx_space_common_core::types::NodeId::from_uuid( + uuid::Uuid::from_u128(0x019f_8000_0000_7000_8000_0000_0000_00fe), + )), + node_role, + node_managed_by: None, + route_ed25519_verification_bundle: None, + }, + ) +} + +/// An auto-lifecycle authority no proto value names. Create parses it last +/// before template lookup, which would contact the fixed template-manager +/// socket. +const BARRIER_AUTHORITY: i32 = 98; +/// The barrier's refusal; no case expects this message for its own refusal. +const BARRIER_REFUSAL: &str = "policy authority proto value 98 is unknown"; + +/// A create refused by the auto-lifecycle authority parse, the last check +/// before template lookup. Every create case starts from it, so a regression +/// that removes the refusal a case pins still stops here, with a refusal that +/// case does not expect. +fn create_refused_before_template_lookup() -> CreateRequest { + CreateRequest { template: "seed".into(), owner: "root@local".into(), runtime: pplx_space_common_core::types::RUNTIME_FIRECRACKER_ALIAS.into(), boot_mode: BootMode::Cold as i32, - sandbox_id: Some(sandbox_id.clone()), + sandbox_id: Some(SandboxId::new().to_string()), auto_lifecycle: Some(Default::default()), - auto_lifecycle_authority: PolicyAuthority::Local as i32, + auto_lifecycle_authority: BARRIER_AUTHORITY, ..Default::default() - }); - request.extensions_mut().insert(PeerIdentity::new( - users::SPACE_NODEAPI.uid, - users::SPACE_NODEAPI.primary_group.gid, - None, - )); + } +} + +fn every_boot_mode() -> [BootMode; 4] { + let modes = [ + BootMode::Unspecified, + BootMode::Cold, + BootMode::BaseSnapshot, + BootMode::Pool, + ]; + // A new variant fails to compile here until it joins the sweep. + for mode in modes { + match mode { + BootMode::Unspecified | BootMode::Cold | BootMode::BaseSnapshot | BootMode::Pool => {} + } + } + modes +} - let error = service - .create(request) +#[tokio::test] +async fn forwarded_create_refuses_owner_and_network_policy_authority_before_template_lookup() { + use pplx_space_net_common::{NetworkPolicyV1, proto::PolicyAuthority}; + + if !enter_isolated_test_process() { + return; + } + struct Case { + name: &'static str, + owner: &'static str, + network_policy_authority: Option, + code: tonic::Code, + message: &'static str, + } + // Every declared network-policy authority plus an unknown value. + let cases = [ + Case { + name: "template-manager owner", + owner: "tmplmgr@local", + network_policy_authority: None, + code: tonic::Code::PermissionDenied, + message: "tmplmgr@local", + }, + Case { + name: "local network policy", + owner: "root@local", + network_policy_authority: Some(PolicyAuthority::Local as i32), + code: tonic::Code::PermissionDenied, + message: "authority=local", + }, + Case { + name: "unspecified network policy authority", + owner: "root@local", + network_policy_authority: Some(PolicyAuthority::Unspecified as i32), + code: tonic::Code::InvalidArgument, + message: "policy authority proto value is unspecified", + }, + Case { + name: "unknown network policy authority", + owner: "root@local", + network_policy_authority: Some(99), + code: tonic::Code::InvalidArgument, + message: "policy authority proto value 99 is unknown", + }, + // Accepted from a forwarded peer: only the barrier refuses these. + Case { + name: "no network policy", + owner: "root@local", + network_policy_authority: None, + code: tonic::Code::InvalidArgument, + message: BARRIER_REFUSAL, + }, + Case { + name: "cluster network policy", + owner: "root@local", + network_policy_authority: Some(PolicyAuthority::Cluster as i32), + code: tonic::Code::InvalidArgument, + message: BARRIER_REFUSAL, + }, + ]; + for boot_mode in every_boot_mode() { + for case in &cases { + let context = format!("{} ({boot_mode:?})", case.name); + let scratch = tempfile::tempdir().unwrap(); + let service = isolated_service(crate::db::test_conn(), scratch.path(), None); + let request = CreateRequest { + owner: case.owner.into(), + boot_mode: boot_mode as i32, + network_policy: case + .network_policy_authority + .map(|_| (&NetworkPolicyV1::allow_all()).into()), + network_policy_authority: case.network_policy_authority.unwrap_or_default(), + ..create_refused_before_template_lookup() + }; + let sandbox_id = request.sandbox_id.clone().unwrap(); + + let error = service + .create(peer_request( + request, + Some(service_peer(&users::SPACE_NODEAPI)), + )) + .await + .expect_err(case.name); + assert_eq!(error.code(), case.code, "{context}: {error}"); + assert!(error.message().contains(case.message), "{context}: {error}"); + assert!( + !crate::db::sandbox_exists(&service.db_conn().unwrap(), &sandbox_id).unwrap(), + "{context}" + ); + } + } +} + +/// Bounds every wait on cleanup, including correct runs: blocking RPCs and +/// job waits rely on the executor's wake-up, whose fallback is a one-second +/// tick, so the bound leaves room for a loaded CI runner. A regression that +/// blocks on held node cleanup fails here instead of hanging. +const CLEANUP_WAIT: Duration = Duration::from_secs(30); + +async fn within_cleanup_wait(context: &str, future: F) -> F::Output { + tokio::time::timeout(CLEANUP_WAIT, future) .await - .expect_err("forwarded create must not persist a local auto-lifecycle policy"); - assert_eq!(error.code(), tonic::Code::PermissionDenied); - assert!(!crate::db::sandbox_exists(&service.db_conn().unwrap(), &sandbox_id).unwrap()); + .unwrap_or_else(|_| panic!("{context}: waited longer than {CLEANUP_WAIT:?}")) +} + +/// Stop and delete reach node cleanup only through the executor's test +/// dispatch, which waits for a permit the test grants. The runtime liveness +/// probe reads a test-owned run directory. Every admitted stop or ordinary +/// delete still removes watchdog monitoring, which stats the fixed sidecar +/// pid path of the fresh sandbox id; nothing exists there and nothing is +/// probed. +struct CleanupFixture { + service: SandboxManagerService, + run_dir: tempfile::TempDir, + dispatch_permits: Arc, +} + +impl CleanupFixture { + fn new(conn: rusqlite::Connection, dispatch_permits: usize) -> Self { + Self::with_node_role(conn, dispatch_permits, None) + } + + fn with_node_role( + conn: rusqlite::Connection, + dispatch_permits: usize, + node_role: Option, + ) -> Self { + let run_dir = tempfile::tempdir().unwrap(); + let service = isolated_service(conn, run_dir.path(), node_role); + *service.cleanup_executor.run_dir.lock().unwrap() = Some(run_dir.path().to_owned()); + let dispatch_permits = Arc::new(tokio::sync::Semaphore::new(dispatch_permits)); + let permits = Arc::clone(&dispatch_permits); + *service.cleanup_executor.dispatch.lock().unwrap() = Some(Arc::new(move |_, _| { + let permits = Arc::clone(&permits); + Box::pin(async move { + match permits.acquire().await { + Ok(permit) => { + permit.forget(); + Ok(()) + } + Err(_) => Err(Status::unavailable("test node cleanup closed")), + } + }) + })); + Self { + service, + run_dir, + dispatch_permits, + } + } + + /// Make the liveness probe report a runtime for `id`. + fn mark_running(&self, id: &str) { + let sandbox_dir = self.run_dir.path().join(id); + std::fs::create_dir(&sandbox_dir).unwrap(); + std::os::unix::fs::symlink(self.run_dir.path(), sandbox_dir.join("fc.pid")).unwrap(); + } + + fn state(&self, id: &str) -> Option { + crate::db::get_sandbox(&self.service.db_conn().unwrap(), id) + .unwrap() + .map(|row| row.state) + } + + fn active_cleanup_job(&self, id: &str) -> Option { + pplx_space_common_jobs::db::find_active_job( + &self.service.db_conn().unwrap(), + crate::db::cleanup_jobs::KIND, + id, + ) + .unwrap() + } + + fn dirty_ids(&self) -> Vec { + crate::db::list_node_dirty_refs(&self.service.db_conn().unwrap(), 100) + .unwrap() + .into_iter() + .map(|row| row.element_value) + .collect() + } + + /// The sandbox kept `state` and gained no cleanup work or publication. + fn assert_untouched(&self, id: &str, state: SandboxState, context: &str) { + assert_eq!(self.state(id), Some(state), "{context}"); + assert!( + crate::db::cleanup_episode(&self.service.db_conn().unwrap(), id) + .unwrap() + .is_none(), + "{context}" + ); + assert!(self.active_cleanup_job(id).is_none(), "{context}"); + assert!(self.dirty_ids().is_empty(), "{context}"); + } + + async fn shutdown(self) { + self.dispatch_permits.close(); + self.service.shutdown_token.cancel(); + self.service.task_tracker.close(); + within_cleanup_wait("shutdown", self.service.task_tracker.wait()).await; + } +} + +#[derive(Clone, Copy, Debug)] +enum CleanupRpc { + Stop, + Delete, +} + +impl CleanupRpc { + async fn call( + self, + service: &SandboxManagerService, + peer: Option, + sandbox_id: &str, + template_filter: Option<&str>, + blocking: bool, + ) -> Result, Status> { + let template_filter = template_filter.map(str::to_owned); + let call = async { + match self { + Self::Stop => service + .stop(peer_request( + StopRequest { + sandbox_id: sandbox_id.to_owned(), + template_filter, + blocking, + immediate_kill: false, + }, + peer, + )) + .await + .map(|response| response.into_inner().stopped_ids), + Self::Delete => service + .delete(peer_request( + DeleteRequest { + sandbox_id: sandbox_id.to_owned(), + template_filter, + blocking, + }, + peer, + )) + .await + .map(|response| response.into_inner().deleted_ids), + } + }; + within_cleanup_wait(&format!("{self:?}"), call).await + } + + /// Where a completed cleanup leaves the row. + fn completed_state(self) -> Option { + match self { + Self::Stop => Some(SandboxState::Stopped), + Self::Delete => None, + } + } + + /// Where an admitted, not yet completed cleanup leaves the row. + fn admitted_state(self) -> SandboxState { + match self { + Self::Stop => SandboxState::Stopping, + Self::Delete => SandboxState::Deleting, + } + } +} + +fn insert_owned_sandbox(conn: &rusqlite::Connection, id: &str, owner: &str) { + let sandbox_id = SandboxId::from_str(id).unwrap(); + crate::db::insert_sandbox( + conn, + crate::db::InsertSandboxParams { + id, + uuid8: &SandboxRuntimeLabel::from_sandbox_id(sandbox_id), + state: SandboxState::Started, + template: "seed", + template_hash: Some("hash"), + user: &SandboxOwner::parse(owner.to_owned()).unwrap(), + runtime: "fc", + snapshots_enabled: false, + rootfs_size_mb: None, + disk_io_limits: Default::default(), + pool_template: None, + volume_encryption: false, + process_env: &BTreeMap::new(), + auto_lifecycle: &AutoLifecyclePolicy::disabled(), + network_policy_override: None, + network_policy_generation: 0, + network_policy_authority: crate::db::PolicyAuthority::Cluster, + auto_lifecycle_authority: crate::db::PolicyAuthority::Cluster, + }, + ) + .unwrap(); +} + +#[tokio::test] +async fn core_rpcs_refuse_unauthenticated_and_unlisted_peers_without_mutation() { + if !enter_isolated_test_process() { + return; + } + // Create's owner check refuses node-ctl as well, so its refusal is pinned + // by message to the peer check. + for (peer, code, create_message) in [ + ( + None, + tonic::Code::Unauthenticated, + "UDS peer credentials missing", + ), + ( + Some(service_peer(&users::SPACE_NODECTL)), + tonic::Code::PermissionDenied, + "not authorized for Create", + ), + ] { + let conn = crate::db::test_conn(); + let id = SandboxId::new().to_string(); + insert_test_sandbox(&conn, &id, SandboxState::Started); + // Grant dispatch so a regression that admits the peer completes + // instead of waiting for node cleanup. + let fixture = CleanupFixture::new(conn, 4); + fixture.mark_running(&id); + let create = create_refused_before_template_lookup(); + let created_id = create.sandbox_id.clone().unwrap(); + + let create_error = fixture + .service + .create(peer_request(create, peer.clone())) + .await + .expect_err("create"); + assert_eq!(create_error.code(), code, "create: {create_error}"); + assert!( + create_error.message().contains(create_message), + "create: {create_error}" + ); + for rpc in [CleanupRpc::Stop, CleanupRpc::Delete] { + let error = rpc + .call(&fixture.service, peer.clone(), &id, None, true) + .await + .expect_err("unauthorized cleanup"); + assert_eq!(error.code(), code, "{rpc:?}"); + } + + assert!( + !crate::db::sandbox_exists(&fixture.service.db_conn().unwrap(), &created_id).unwrap() + ); + fixture.assert_untouched(&id, SandboxState::Started, &format!("{code:?}")); + fixture.shutdown().await; + } +} + +#[tokio::test] +async fn stop_selects_only_a_live_runtime_and_both_treat_an_empty_template_filter_as_none() { + if !enter_isolated_test_process() { + return; + } + struct Case { + name: &'static str, + row: bool, + running: bool, + template_filter: Option<&'static str>, + stop_selects: bool, + delete_selects: bool, + } + let cases = [ + Case { + name: "no template filter", + row: true, + running: true, + template_filter: None, + stop_selects: true, + delete_selects: true, + }, + Case { + name: "empty template filter", + row: true, + running: true, + template_filter: Some(""), + stop_selects: true, + delete_selects: true, + }, + Case { + name: "other template", + row: true, + running: true, + template_filter: Some("other"), + stop_selects: false, + delete_selects: false, + }, + Case { + name: "no live runtime", + row: true, + running: false, + template_filter: None, + stop_selects: false, + delete_selects: true, + }, + // Absent sandboxes are idempotent successes. The lookup stats the + // fixed sandbox cache path of a fresh UUID, which does not exist. + Case { + name: "absent sandbox", + row: false, + running: false, + template_filter: None, + stop_selects: false, + delete_selects: false, + }, + ]; + for case in cases { + for rpc in [CleanupRpc::Stop, CleanupRpc::Delete] { + let context = format!("{} ({rpc:?})", case.name); + let conn = crate::db::test_conn(); + let id = SandboxId::new().to_string(); + if case.row { + insert_test_sandbox(&conn, &id, SandboxState::Started); + } + let fixture = CleanupFixture::new(conn, 4); + if case.running { + fixture.mark_running(&id); + } + let selected = match rpc { + CleanupRpc::Stop => case.stop_selects, + CleanupRpc::Delete => case.delete_selects, + }; + + let ids = rpc + .call( + &fixture.service, + Some(root_peer()), + &id, + case.template_filter, + true, + ) + .await + .unwrap(); + + if selected { + assert_eq!(ids, vec![id.clone()], "{context}"); + assert_eq!(fixture.state(&id), rpc.completed_state(), "{context}"); + assert!(fixture.dirty_ids().contains(&id), "{context}"); + } else { + assert!(ids.is_empty(), "{context}"); + if case.row { + fixture.assert_untouched(&id, SandboxState::Started, &context); + } else { + assert_eq!(fixture.state(&id), None, "{context}"); + assert!(fixture.dirty_ids().is_empty(), "{context}"); + } + } + fixture.shutdown().await; + } + } +} + +#[tokio::test] +async fn nonblocking_cleanup_returns_once_admitted_and_honors_skip_node_publish() { + if !enter_isolated_test_process() { + return; + } + for rpc in [CleanupRpc::Stop, CleanupRpc::Delete] { + for skip_node_publish in [false, true] { + let context = format!("{rpc:?} skip_node_publish={skip_node_publish}"); + let conn = crate::db::test_conn(); + let id = SandboxId::new().to_string(); + insert_test_sandbox(&conn, &id, SandboxState::Started); + // No dispatch permit: node cleanup cannot finish before the RPC + // returns. + let fixture = CleanupFixture::new(conn, 0); + fixture.mark_running(&id); + let mut request = peer_request((), Some(root_peer())); + if skip_node_publish { + request.metadata_mut().insert( + "x-space-skip-cluster-sync", + tonic::metadata::MetadataValue::from_static("true"), + ); + } + let (metadata, extensions, ()) = request.into_parts(); + let call = async { + match rpc { + CleanupRpc::Stop => { + fixture + .service + .stop(Request::from_parts( + metadata, + extensions, + StopRequest { + sandbox_id: id.clone(), + template_filter: None, + blocking: false, + immediate_kill: false, + }, + )) + .await + .unwrap() + .into_inner() + .stopped_ids + } + CleanupRpc::Delete => { + fixture + .service + .delete(Request::from_parts( + metadata, + extensions, + DeleteRequest { + sandbox_id: id.clone(), + template_filter: None, + blocking: false, + }, + )) + .await + .unwrap() + .into_inner() + .deleted_ids + } + } + }; + // A non-blocking RPC must return while node cleanup is still held. + let ids = within_cleanup_wait(&context, call).await; + + assert_eq!(ids, vec![id.clone()], "{context}"); + assert_eq!(fixture.state(&id), Some(rpc.admitted_state()), "{context}"); + let job = fixture + .active_cleanup_job(&id) + .unwrap_or_else(|| panic!("{context}: admitted cleanup is durable")); + assert_eq!( + fixture.dirty_ids().contains(&id), + !skip_node_publish, + "{context}" + ); + + fixture.dispatch_permits.add_permits(1); + within_cleanup_wait(&context, fixture.service.wait_cleanup_job(job)) + .await + .unwrap(); + assert_eq!(fixture.state(&id), rpc.completed_state(), "{context}"); + // The header defers the dirty ref rather than suppressing it: + // completion marks the sandbox dirty just after the job settles. + within_cleanup_wait(&context, async { + while !fixture.dirty_ids().contains(&id) { + tokio::task::yield_now().await; + } + }) + .await; + fixture.shutdown().await; + } + } +} + +#[tokio::test] +async fn template_manager_cleanup_requires_a_template_owned_sandbox() { + if !enter_isolated_test_process() { + return; + } + let template_manager_owner = SandboxOwnerKind::TemplateManager + .reserved_owner_label() + .unwrap(); + for (owner, allowed) in [("user", false), (template_manager_owner, true)] { + for rpc in [CleanupRpc::Stop, CleanupRpc::Delete] { + let context = format!("{owner} ({rpc:?})"); + let conn = crate::db::test_conn(); + let id = SandboxId::new().to_string(); + insert_owned_sandbox(&conn, &id, owner); + let fixture = CleanupFixture::new(conn, 4); + fixture.mark_running(&id); + + let result = rpc + .call( + &fixture.service, + Some(service_peer(&users::SPACE_TMPLMGR)), + &id, + None, + true, + ) + .await; + + if allowed { + assert_eq!(result.unwrap(), vec![id.clone()], "{context}"); + assert_eq!(fixture.state(&id), rpc.completed_state(), "{context}"); + } else { + assert_eq!( + result.unwrap_err().code(), + tonic::Code::PermissionDenied, + "{context}" + ); + fixture.assert_untouched(&id, SandboxState::Started, &context); + } + fixture.shutdown().await; + } + } +} + +#[tokio::test] +async fn cleanup_rpcs_refuse_after_shutdown_without_mutation() { + if !enter_isolated_test_process() { + return; + } + for rpc in [CleanupRpc::Stop, CleanupRpc::Delete] { + let conn = crate::db::test_conn(); + let id = SandboxId::new().to_string(); + insert_test_sandbox(&conn, &id, SandboxState::Started); + let fixture = CleanupFixture::new(conn, 4); + fixture.mark_running(&id); + fixture.service.shutdown_token.cancel(); + + let error = rpc + .call(&fixture.service, Some(root_peer()), &id, None, false) + .await + .expect_err("shutdown closes cleanup admission"); + + assert_eq!(error.code(), tonic::Code::Unavailable, "{rpc:?}"); + fixture.assert_untouched(&id, SandboxState::Started, &format!("{rpc:?}")); + fixture.shutdown().await; + } +} + +fn auto_lifecycle_expectation( + policy: &AutoLifecyclePolicy, +) -> crate::auto_lifecycle_delete_receipts::AutoLifecycleDeleteExpectation { + crate::auto_lifecycle_delete_receipts::AutoLifecycleDeleteExpectation { + policy_fingerprint: + pplx_space_common_svc::types::AutoLifecyclePolicyFingerprint::for_policy(policy), + authority: crate::db::PolicyAuthority::Cluster, + } +} + +fn access_idle_delete_policy(at: u64) -> AutoLifecyclePolicy { + use pplx_space_common_core::types::{ + AutoLifecycleConditionSet, AutoLifecycleConditionSetId, AutoLifecycleStage, + }; + AutoLifecyclePolicy { + wake: Default::default(), + condition_sets: BTreeMap::from([( + AutoLifecycleConditionSetId::AccessIdle, + AutoLifecycleConditionSet::access_idle(), + )]), + stages: vec![AutoLifecycleStage { + condition_set: AutoLifecycleConditionSetId::AccessIdle, + at, + action: AutoLifecycleIdleAction::Delete, + }], + } + .normalized() +} + +#[tokio::test] +async fn auto_lifecycle_delete_requires_one_canonical_id_and_the_expected_policy() { + use crate::service::tests::service_tests::insert_test_sandbox_with_policy; + + if !enter_isolated_test_process() { + return; + } + + let policy = access_idle_delete_policy(900); + let stale = access_idle_delete_policy(1800); + struct Case<'a> { + name: &'static str, + uppercase_id: bool, + template_filter: Option<&'static str>, + expected_policy: &'a AutoLifecyclePolicy, + refusal: Option, + } + for case in [ + Case { + name: "template filter", + uppercase_id: false, + template_filter: Some("seed"), + expected_policy: &policy, + refusal: Some(tonic::Code::Internal), + }, + Case { + name: "non-canonical id", + uppercase_id: true, + template_filter: None, + expected_policy: &policy, + refusal: Some(tonic::Code::Internal), + }, + Case { + name: "stale policy", + uppercase_id: false, + template_filter: None, + expected_policy: &stale, + refusal: Some(tonic::Code::Aborted), + }, + Case { + name: "current policy", + uppercase_id: false, + template_filter: None, + expected_policy: &policy, + refusal: None, + }, + ] { + let conn = crate::db::test_conn(); + let sandbox_id = SandboxId::new(); + let id = sandbox_id.to_string(); + insert_test_sandbox_with_policy(&conn, &id, SandboxState::Started, &policy); + // Auto-lifecycle delete is admitted only for a cluster-fenced route. + crate::db::update_space_api_route_identity( + &conn, + sandbox_id, + &pplx_space_common_core::types::SpaceApiRouteIdentity { + route_generation: Some( + pplx_space_common_core::types::RouteGeneration::new(7).unwrap(), + ), + current_node_id: Some(pplx_space_common_core::types::NodeId::new()), + execution_generation: Some(ExecutionGeneration::new(7).unwrap()), + activation_attempt_id: Some( + pplx_space_common_core::types::SandboxActivationAttemptId::new(), + ), + substrate_kind: Some(NodeManagedBy::Kubernetes), + pod_uid: Some(pplx_space_common_core::types::PodUid::parse("pod-uid-7").unwrap()), + ..Default::default() + }, + crate::auto_lifecycle_delete_receipts::RouteIdentityProvenance::ClusterFenced, + ) + .unwrap(); + let fixture = CleanupFixture::with_node_role(conn, 4, Some(NodeRole::Worker)); + fixture.mark_running(&id); + let selector = if case.uppercase_id { + id.to_uppercase() + } else { + id.clone() + }; + + let delete = fixture.service.delete_with_auto_lifecycle_expectation( + peer_request( + DeleteRequest { + sandbox_id: selector, + template_filter: case.template_filter.map(str::to_owned), + blocking: true, + }, + Some(root_peer()), + ), + Some(auto_lifecycle_expectation(case.expected_policy)), + ); + let result = within_cleanup_wait(case.name, delete).await; + + match case.refusal { + Some(code) => { + let error = result.unwrap_err(); + assert_eq!(error.code(), code, "{}", case.name); + // The reconciler retries a lost policy comparison by this key. + assert_eq!( + error.metadata().contains_key( + crate::service::cleanup::AUTO_LIFECYCLE_DELETE_CAS_LOST_METADATA + ), + code == tonic::Code::Aborted, + "{}", + case.name + ); + fixture.assert_untouched(&id, SandboxState::Started, case.name); + assert_eq!( + crate::auto_lifecycle_delete_receipts::db::pending_receipt_for_sandbox( + &fixture.service.db_conn().unwrap(), + sandbox_id, + ) + .unwrap(), + None, + "{}", + case.name + ); + } + None => { + assert_eq!( + result.unwrap().into_inner().deleted_ids, + vec![id.clone()], + "{}", + case.name + ); + assert_eq!(fixture.state(&id), None, "{}", case.name); + } + } + fixture.shutdown().await; + } +} + +/// Insert a pool clone whose publication still owns an armed disposal +/// episode, as `PoolWaitClone` leaves it before publication completes. +fn insert_armed_pool_clone( + conn: &rusqlite::Connection, + owner: &str, +) -> (SandboxId, SandboxCleanupEpisodeId) { + let id = SandboxId::new(); + let cleanup_id = SandboxCleanupEpisodeId::new(); + crate::db::insert_sandbox_with_cleanup_episode_and_route_evidence( + conn, + crate::db::InsertSandboxParams { + id: &id.to_string(), + uuid8: &SandboxRuntimeLabel::from_sandbox_id(id), + state: SandboxState::Creating, + template: "seed", + template_hash: Some("sha256:test"), + user: &SandboxOwner::parse(owner.to_owned()).unwrap(), + runtime: RuntimeKind::Firecracker.as_str(), + snapshots_enabled: false, + rootfs_size_mb: None, + disk_io_limits: Default::default(), + pool_template: None, + volume_encryption: false, + process_env: &BTreeMap::new(), + auto_lifecycle: &AutoLifecyclePolicy::disabled(), + network_policy_override: None, + network_policy_generation: 0, + network_policy_authority: crate::db::PolicyAuthority::Cluster, + auto_lifecycle_authority: crate::db::PolicyAuthority::Cluster, + }, + crate::db::CleanupEpisodeSpec { + id: cleanup_id, + completion: CleanupCompletion::DeletePoolClone, + keep_filesystem: false, + entered_from_state: None, + activation_state: SandboxState::Creating, + execution_generation: None, + activation_attempt_id: None, + }, + None, + None, + ) + .unwrap(); + (id, cleanup_id) +} + +#[tokio::test] +async fn delete_of_a_pool_clone_disposal_requires_an_operator_after_quarantine() { + if !enter_isolated_test_process() { + return; + } + let template_manager_owner = SandboxOwnerKind::TemplateManager + .reserved_owner_label() + .unwrap(); + struct Case { + name: &'static str, + owner: &'static str, + quarantined: bool, + template_manager_peer: bool, + auto_lifecycle: bool, + hold_cleanup_fence: bool, + blocking: bool, + refusal: Option, + } + for case in [ + Case { + name: "publication still owns the clone", + owner: "deploy", + quarantined: false, + template_manager_peer: false, + auto_lifecycle: false, + hold_cleanup_fence: false, + blocking: false, + refusal: Some(tonic::Code::FailedPrecondition), + }, + Case { + name: "template manager after quarantine", + owner: template_manager_owner, + quarantined: true, + template_manager_peer: true, + auto_lifecycle: false, + hold_cleanup_fence: false, + blocking: false, + refusal: Some(tonic::Code::FailedPrecondition), + }, + Case { + name: "auto-lifecycle after quarantine", + owner: "deploy", + quarantined: true, + template_manager_peer: false, + auto_lifecycle: true, + hold_cleanup_fence: false, + blocking: false, + refusal: Some(tonic::Code::FailedPrecondition), + }, + Case { + name: "operator while cleanup is busy", + owner: "deploy", + quarantined: true, + template_manager_peer: false, + auto_lifecycle: false, + hold_cleanup_fence: true, + blocking: false, + refusal: Some(tonic::Code::Unavailable), + }, + Case { + name: "operator after quarantine", + owner: "deploy", + quarantined: true, + template_manager_peer: false, + auto_lifecycle: false, + hold_cleanup_fence: false, + blocking: false, + refusal: None, + }, + Case { + name: "blocking operator after quarantine", + owner: "deploy", + quarantined: true, + template_manager_peer: false, + auto_lifecycle: false, + hold_cleanup_fence: false, + blocking: true, + refusal: None, + }, + ] { + let conn = crate::db::test_conn(); + let (sandbox_id, cleanup_id) = insert_armed_pool_clone(&conn, case.owner); + let id = sandbox_id.to_string(); + if case.quarantined { + assert!( + crate::db::mark_quarantined_from_state( + &conn, + &id, + SandboxState::Creating, + pplx_space_common_core::types::SandboxQuarantineSource::SbxManager, + SandboxQuarantineReasonCode::CreateInterrupted, + "operator hold during publication", + pplx_space_common_core::types::SandboxQuarantineNextAction::RequiresOperatorAttention, + ) + .unwrap() + ); + } + let held_state = if case.quarantined { + SandboxState::Quarantined + } else { + SandboxState::Creating + }; + // Without a dispatch permit an admitted non-blocking disposal stays + // observable. + let fixture = CleanupFixture::new(conn, if case.blocking { 1 } else { 0 }); + let fence = case.hold_cleanup_fence.then(|| { + let CleanupLockTryResult::Acquired(fence) = fixture.service.try_cleanup_fence(&id) + else { + panic!("unused cleanup fence"); + }; + fence + }); + let peer = if case.template_manager_peer { + service_peer(&users::SPACE_TMPLMGR) + } else { + root_peer() + }; + + let delete = fixture.service.delete_with_auto_lifecycle_expectation( + peer_request( + DeleteRequest { + sandbox_id: id.clone(), + template_filter: None, + blocking: case.blocking, + }, + Some(peer), + ), + case.auto_lifecycle + .then(|| auto_lifecycle_expectation(&AutoLifecyclePolicy::disabled())), + ); + // A non-blocking delete must return while node cleanup is still held. + let result = within_cleanup_wait(case.name, delete).await; + drop(fence); + + let episode = crate::db::cleanup_episode(&fixture.service.db_conn().unwrap(), &id).unwrap(); + match case.refusal { + Some(code) => { + let episode = episode.unwrap(); + assert_eq!(episode.id, cleanup_id, "{}", case.name); + assert_eq!(result.unwrap_err().code(), code, "{}", case.name); + assert_eq!(fixture.state(&id), Some(held_state), "{}", case.name); + assert_eq!(episode.requested_at, None, "{}", case.name); + assert!(fixture.active_cleanup_job(&id).is_none(), "{}", case.name); + } + None => { + assert_eq!( + result.unwrap().into_inner().deleted_ids, + vec![id.clone()], + "{}", + case.name + ); + if case.blocking { + assert_eq!(fixture.state(&id), None, "{}", case.name); + assert!(episode.is_none(), "{}", case.name); + } else { + let episode = episode.unwrap(); + assert_eq!(episode.id, cleanup_id, "{}", case.name); + assert!(episode.requested_at.is_some(), "{}", case.name); + assert!(fixture.active_cleanup_job(&id).is_some(), "{}", case.name); + } + } + } + fixture.shutdown().await; + } } #[test]