diff --git a/lib/apps/fabro-server/src/serve.rs b/lib/apps/fabro-server/src/serve.rs index 0c1c49aaf..dd174630e 100644 --- a/lib/apps/fabro-server/src/serve.rs +++ b/lib/apps/fabro-server/src/serve.rs @@ -799,7 +799,7 @@ where github_api_base_url: None, active_config_path, http_client: None, - sandbox_provider_registry: None, + sandbox_inventory: None, shutdown: shutdown.clone(), #[cfg(test)] worker_control_bus: None, diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index b5da5d57d..ae7d158d6 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -65,10 +65,7 @@ use fabro_redact::redact_jsonl_line; use fabro_sandbox::details::sandbox_details; use fabro_sandbox::driver::{DaytonaCredentials, ProviderAccess, ProviderConnectOptions}; use fabro_sandbox::reconnect::reconnect_for_run; -use fabro_sandbox::{ - DriverInventoryProvider, LocalSandboxProvider, SandboxProvider, SandboxProviderRegistry, - daytona, -}; +use fabro_sandbox::{SandboxInventory, daytona}; use fabro_slack::client::{PostedMessage as SlackPostedMessage, SlackClient}; use fabro_slack::config::{ SlackCredentialResolution, @@ -1137,7 +1134,7 @@ pub struct AppState { pub(crate) github_api_base_url: String, active_config_path: PathBuf, http_client: Option, - sandbox_provider_registry: SandboxProviderRegistry, + sandbox_inventory: SandboxInventory, shutdown: CancellationToken, shutting_down: AtomicBool, registry_factory_override: Option>, @@ -1280,7 +1277,7 @@ pub(crate) struct AppStateConfig { pub(crate) github_api_base_url: Option, pub(crate) active_config_path: PathBuf, pub(crate) http_client: Option, - pub(crate) sandbox_provider_registry: Option, + pub(crate) sandbox_inventory: Option, pub(crate) shutdown: CancellationToken, #[cfg(test)] pub(crate) worker_control_bus: Option>, @@ -1538,8 +1535,8 @@ impl AppState { &self.session_runtimes } - pub(crate) fn sandbox_provider_registry(&self) -> &SandboxProviderRegistry { - &self.sandbox_provider_registry + pub(crate) fn sandbox_inventory(&self) -> &SandboxInventory { + &self.sandbox_inventory } pub(crate) fn server_secret(&self, name: &str) -> Option { @@ -2336,26 +2333,26 @@ fn worker_token_keys_from_server_secrets( .map_err(|err| jwt_auth::session_secret_key_error(&err)) } -fn build_sandbox_provider_registry( +fn build_sandbox_inventory( server_settings: &ServerSettings, daytona_api_key: Option, env_lookup: &EnvLookup, http_client: Option, -) -> SandboxProviderRegistry { +) -> SandboxInventory { let provider_settings = &server_settings.server.sandbox.providers; - let mut providers: Vec> = Vec::new(); + let mut inventory = SandboxInventory::empty(); if provider_settings.is_enabled(&SandboxProviderKind::LOCAL) { - providers.push(Arc::new(LocalSandboxProvider)); + inventory = inventory.with_host_directories(SandboxProviderKind::LOCAL); } if let Some(docker) = provider_settings.get(&SandboxProviderKind::DOCKER) { if docker.enabled { - providers.push(Arc::new(DriverInventoryProvider::lazy( + inventory = inventory.with_lazy( SandboxProviderKind::DOCKER, docker.clone(), ProviderConnectOptions::default(), - ))); + ); } } @@ -2369,18 +2366,18 @@ fn build_sandbox_provider_registry( target: None, http_client, }; - providers.push(Arc::new(DriverInventoryProvider::lazy( + inventory = inventory.with_lazy( SandboxProviderKind::DAYTONA, daytona.clone(), ProviderConnectOptions { host_registry_root: None, daytona: Some(credentials), }, - ))); + ); } } - SandboxProviderRegistry::new(providers) + inventory } pub(crate) fn automation_dir_for_active_config(active_config_path: &std::path::Path) -> PathBuf { @@ -2433,7 +2430,7 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result anyhow::Result anyhow::Result>, _auth: RequiredRunManagementActor, ) -> Json { - Json(state.sandbox_provider_registry().list_managed().await) + Json(state.sandbox_inventory().list_managed().await) } async fn retrieve_sandbox( @@ -30,7 +30,7 @@ async fn retrieve_sandbox( _auth: RequiredRunManagementActor, ) -> Result, ApiError> { state - .sandbox_provider_registry() + .sandbox_inventory() .get_managed_by_native_id(&id) .await .map(Json) @@ -79,23 +79,49 @@ fn provider_list(providers: &[SandboxProviderKind]) -> String { mod tests { use axum::body::{Body, to_bytes}; use axum::http::{Request, StatusCode}; - use fabro_sandbox::SandboxProviderRegistry; - use fabro_sandbox::test_support::{ - FakeGet, FakeList, FakeSandboxProvider, fake_registry, fake_sandbox_info, - }; + use fabro_sandbox::SandboxInventory; + use fabro_sandbox::driver::{ConnectedProvider, ProviderConnectOptions}; + use fabro_sandbox::test_support::{managed_scripted_sandbox, scripted_inventory_provider}; use fabro_types::SandboxProviderKind; + use fabro_types::settings::server::{SandboxPluginSettings, ServerSandboxProviderSettings}; use serde_json::{Value, json}; use tower::ServiceExt; use crate::test_support::{TestAppStateBuilder, build_test_router}; - fn app_with_registry(registry: SandboxProviderRegistry) -> axum::Router { + fn app_with_inventory(inventory: SandboxInventory) -> axum::Router { let state = TestAppStateBuilder::new() - .sandbox_provider_registry(registry) + .sandbox_inventory(inventory) .build(); build_test_router(state) } + /// A connected provider of `kind` holding fabro-managed sandboxes `ids`. + fn provider(kind: SandboxProviderKind, ids: &[&str]) -> ConnectedProvider { + scripted_inventory_provider( + kind, + ids.iter().map(|id| managed_scripted_sandbox(id)).collect(), + ) + } + + /// A plugin kind whose executable does not exist, so every lookup fails + /// to connect. + fn with_unreachable_plugin(inventory: SandboxInventory, name: &str) -> SandboxInventory { + let settings = ServerSandboxProviderSettings { + enabled: true, + plugin: Some(SandboxPluginSettings { + path: Some(format!("/nonexistent/fabro-sandbox-{name}")), + dev: true, + ..SandboxPluginSettings::default() + }), + }; + inventory.with_lazy( + SandboxProviderKind::try_new(name).expect("valid kind"), + settings, + ProviderConnectOptions::default(), + ) + } + fn req_get(uri: &str) -> Request { Request::builder() .method("GET") @@ -113,12 +139,10 @@ mod tests { #[tokio::test] async fn list_returns_provider_backed_data_without_run_projection_state() { - let docker = fake_sandbox_info(SandboxProviderKind::DOCKER, "docker-native-id"); - let app = app_with_registry(fake_registry(vec![FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(vec![docker]), - FakeGet::Missing, - )])); + let app = app_with_inventory( + SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &["docker-native-id"])), + ); let response = app.oneshot(req_get("/api/v1/sandboxes")).await.unwrap(); @@ -126,24 +150,17 @@ mod tests { let body = body_json(response).await; assert_eq!(body["data"][0]["id"], "docker-native-id"); assert_eq!(body["data"][0]["provider"], "docker"); + assert_eq!(body["data"][0]["state"], "running"); assert_eq!(body["meta"]["provider_errors"], json!([])); } #[tokio::test] async fn retrieve_searches_all_configured_providers() { - let daytona = fake_sandbox_info(SandboxProviderKind::DAYTONA, "native-id"); - let app = app_with_registry(fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Found(Box::new(daytona)), - ), - ])); + let app = app_with_inventory( + SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &[])) + .with_connected(provider(SandboxProviderKind::DAYTONA, &["native-id"])), + ); let response = app .oneshot(req_get("/api/v1/sandboxes/native-id")) @@ -158,18 +175,11 @@ mod tests { #[tokio::test] async fn no_matching_sandbox_returns_404() { - let app = app_with_registry(fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - ])); + let app = app_with_inventory( + SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &[])) + .with_connected(provider(SandboxProviderKind::DAYTONA, &[])), + ); let response = app .oneshot(req_get("/api/v1/sandboxes/missing")) @@ -181,24 +191,11 @@ mod tests { #[tokio::test] async fn duplicate_native_ids_return_409() { - let app = app_with_registry(fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Found(Box::new(fake_sandbox_info( - SandboxProviderKind::DOCKER, - "same-id", - ))), - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Found(Box::new(fake_sandbox_info( - SandboxProviderKind::DAYTONA, - "same-id", - ))), - ), - ])); + let app = app_with_inventory( + SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &["same-id"])) + .with_connected(provider(SandboxProviderKind::DAYTONA, &["same-id"])), + ); let response = app .oneshot(req_get("/api/v1/sandboxes/same-id")) @@ -217,18 +214,10 @@ mod tests { #[tokio::test] async fn provider_lookup_uncertainty_returns_502() { - let app = app_with_registry(fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Err("daytona unavailable"), - ), - ])); + let app = app_with_inventory(with_unreachable_plugin( + SandboxInventory::empty().with_connected(provider(SandboxProviderKind::DOCKER, &[])), + "e2b", + )); let response = app .oneshot(req_get("/api/v1/sandboxes/maybe-missing")) @@ -241,7 +230,7 @@ mod tests { body["errors"][0]["detail"] .as_str() .unwrap_or_default() - .contains("daytona unavailable") + .contains("e2b: Failed to connect to the e2b provider") ); } } diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 9ecf1c98c..1f8cee03e 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -2107,7 +2107,7 @@ fn slack_app_state_with_settings_and_secret_sources( github_api_base_url: None, active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"), http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), - sandbox_provider_registry: None, + sandbox_inventory: None, shutdown: tokio_util::sync::CancellationToken::new(), worker_control_bus: None, worker_runtime: None, @@ -2268,7 +2268,7 @@ fn slack_service_respects_disabled_server_config_even_with_vault_tokens() { github_api_base_url: None, active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"), http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), - sandbox_provider_registry: None, + sandbox_inventory: None, shutdown: tokio_util::sync::CancellationToken::new(), worker_control_bus: None, worker_runtime: None, @@ -2622,7 +2622,7 @@ methods = ["dev-token"] github_api_base_url: None, active_config_path: tempfile::tempdir().unwrap().path().join("settings.toml"), http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), - sandbox_provider_registry: None, + sandbox_inventory: None, shutdown: tokio_util::sync::CancellationToken::new(), worker_control_bus: None, worker_runtime: None, @@ -8440,7 +8440,7 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings( github_api_base_url, active_config_path, http_client: Some(fabro_http::test_http_client().expect("test HTTP client should build")), - sandbox_provider_registry: None, + sandbox_inventory: None, shutdown: tokio_util::sync::CancellationToken::new(), worker_control_bus: None, worker_runtime: None, diff --git a/lib/apps/fabro-server/src/test_support.rs b/lib/apps/fabro-server/src/test_support.rs index 8522206c1..0f6b0e2d5 100644 --- a/lib/apps/fabro-server/src/test_support.rs +++ b/lib/apps/fabro-server/src/test_support.rs @@ -19,7 +19,7 @@ use fabro_config::{LlmLayer, RunLayer, ServerSettingsBuilder, Storage, envfile}; use fabro_db::DbPool; use fabro_interview::Interviewer; use fabro_llm::lithos_catalog::Catalog; -use fabro_sandbox::SandboxProviderRegistry; +use fabro_sandbox::SandboxInventory; use fabro_static::EnvVars; use fabro_store::{ArtifactStore, Database, test_support as store_test_support}; use fabro_types::settings::ServerAuthMethod; @@ -90,7 +90,7 @@ pub struct TestAppStateBuilder { manifest_run_defaults: RunLayer, max_concurrent_runs: usize, registry_factory_override: Option>, - sandbox_provider_registry: Option, + sandbox_inventory: Option, store_bundle: Option<(Arc, ArtifactStore)>, vault_path: Option, vault_entries: Vec<(String, String)>, @@ -112,7 +112,7 @@ impl Default for TestAppStateBuilder { manifest_run_defaults: RunLayer::default(), max_concurrent_runs: 5, registry_factory_override: None, - sandbox_provider_registry: None, + sandbox_inventory: None, store_bundle: None, vault_path: None, vault_entries: Vec::new(), @@ -160,11 +160,8 @@ impl TestAppStateBuilder { self } - pub fn sandbox_provider_registry( - mut self, - sandbox_provider_registry: SandboxProviderRegistry, - ) -> Self { - self.sandbox_provider_registry = Some(sandbox_provider_registry); + pub fn sandbox_inventory(mut self, sandbox_inventory: SandboxInventory) -> Self { + self.sandbox_inventory = Some(sandbox_inventory); self } @@ -312,7 +309,7 @@ impl TestAppStateBuilder { http_client: Some( fabro_http::test_http_client().expect("test HTTP client should build"), ), - sandbox_provider_registry: self.sandbox_provider_registry, + sandbox_inventory: self.sandbox_inventory, shutdown: CancellationToken::new(), #[cfg(test)] worker_control_bus: None, diff --git a/lib/components/fabro-sandbox/src/lib.rs b/lib/components/fabro-sandbox/src/lib.rs index ed5cbc540..e5524c132 100644 --- a/lib/components/fabro-sandbox/src/lib.rs +++ b/lib/components/fabro-sandbox/src/lib.rs @@ -52,10 +52,7 @@ pub use options::{ SandboxOptions, local_working_directory_from_environment, options_from_environment, unresolved_env, }; -pub use provider::driver::DriverInventoryProvider; -pub use provider::{ - LocalSandboxProvider, SandboxLookupError, SandboxProvider, SandboxProviderRegistry, -}; +pub use provider::{SandboxInventory, SandboxLookupError}; pub use provider_sandbox::{attach_provider_sandbox, provider_sandbox}; pub use push_credentials::RefreshErrorKind; pub use reconnect::{ diff --git a/lib/components/fabro-sandbox/src/provider.rs b/lib/components/fabro-sandbox/src/provider.rs index d32e5b4f1..6991fd34f 100644 --- a/lib/components/fabro-sandbox/src/provider.rs +++ b/lib/components/fabro-sandbox/src/provider.rs @@ -1,47 +1,116 @@ -pub mod driver; +//! Fabro's inventory of the sandboxes it manages, across the providers a +//! server has configured. +//! +//! Every entry is a sandbox-driver provider narrowed by fabro's ownership +//! labels, so a listing shows only the sandboxes fabro created and an +//! attach to anything else is refused. A provider connects on first use: +//! the inventory is assembled synchronously at startup, and a provider that +//! is down surfaces as a lookup error rather than a startup failure. The +//! `local` kind has an entry too, so a caller can ask whether the kind is +//! ready, but its sandboxes are directories the run record names and there +//! is nothing to list. use std::sync::Arc; -use async_trait::async_trait; +use fabro_types::settings::server::ServerSandboxProviderSettings; use fabro_types::{ SandboxInfo, SandboxListMeta, SandboxListResponse, SandboxProviderKind, SandboxProviderLookupError, }; use fabro_util::error::collect_chain; use futures::future::join_all; +use sandbox_driver::{ + Error as DriverError, OwnedProvider, SandboxFilter, SandboxId, + SandboxProvider as DriverProvider, SandboxState, +}; +use tokio::sync::OnceCell; -#[async_trait] -pub trait SandboxProvider: Send + Sync { - fn kind(&self) -> SandboxProviderKind; - - async fn list(&self) -> crate::Result>; - async fn get(&self, id: &str) -> crate::Result>; - async fn delete(&self, id: &str) -> crate::Result<()>; -} +use crate::driver::{ConnectedProvider, ProviderConnectOptions, connect_provider}; +use crate::{details, managed_labels}; +/// The sandboxes fabro manages, by provider. #[derive(Clone, Default)] -pub struct SandboxProviderRegistry { - providers: Vec>, +pub struct SandboxInventory { + entries: Vec>, } -impl SandboxProviderRegistry { - pub fn new(providers: Vec>) -> Self { - Self { providers } - } +struct InventoryEntry { + kind: SandboxProviderKind, + connection: Connection, +} +enum Connection { + /// Sandboxes on this host are directories the run record names; + /// there is nothing to list. + HostDirectories, + Connected(Arc), + /// Connected through [`connect_provider`] on first use. + Lazy(Box), +} + +struct LazyConnection { + settings: ServerSandboxProviderSettings, + options: ProviderConnectOptions, + provider: OnceCell>, +} + +impl SandboxInventory { + #[must_use] pub fn empty() -> Self { Self::default() } - pub fn providers(&self) -> &[Arc] { - &self.providers + /// A kind whose sandboxes are directories on this host: ready to run, + /// nothing to list. + #[must_use] + pub fn with_host_directories(self, kind: SandboxProviderKind) -> Self { + self.with_entry(kind, Connection::HostDirectories) + } + + /// A provider already connected, tagged with the kind fabro persists + /// for it. + #[must_use] + pub fn with_connected(self, connected: ConnectedProvider) -> Self { + self.with_entry( + connected.kind, + Connection::Connected(owned(connected.provider)), + ) + } + + /// A provider connected through [`connect_provider`] on first use. + #[must_use] + pub fn with_lazy( + self, + kind: SandboxProviderKind, + settings: ServerSandboxProviderSettings, + options: ProviderConnectOptions, + ) -> Self { + self.with_entry( + kind, + Connection::Lazy(Box::new(LazyConnection { + settings, + options, + provider: OnceCell::new(), + })), + ) + } + + fn with_entry(mut self, kind: SandboxProviderKind, connection: Connection) -> Self { + self.entries + .push(Arc::new(InventoryEntry { kind, connection })); + self + } + + /// The provider kinds this inventory covers. + pub fn kinds(&self) -> impl Iterator { + self.entries.iter().map(|entry| &entry.kind) } pub async fn list_managed(&self) -> SandboxListResponse { let results = join_all( - self.providers + self.entries .iter() - .map(|provider| async move { (provider.kind(), provider.list().await) }), + .map(|entry| async move { (&entry.kind, entry.list().await) }), ) .await; @@ -50,7 +119,7 @@ impl SandboxProviderRegistry { for (kind, result) in results { match result { Ok(mut sandboxes) => data.append(&mut sandboxes), - Err(err) => provider_errors.push(provider_error(kind, &err)), + Err(err) => provider_errors.push(provider_error(kind.clone(), &err)), } } @@ -65,9 +134,9 @@ impl SandboxProviderRegistry { id: &str, ) -> Result { let results = join_all( - self.providers + self.entries .iter() - .map(|provider| async move { (provider.kind(), provider.get(id).await) }), + .map(|entry| async move { (&entry.kind, entry.get(id).await) }), ) .await; @@ -77,7 +146,7 @@ impl SandboxProviderRegistry { match result { Ok(Some(sandbox)) => matches.push(sandbox), Ok(None) => {} - Err(err) => provider_errors.push(provider_error(kind, &err)), + Err(err) => provider_errors.push(provider_error(kind.clone(), &err)), } } @@ -101,6 +170,88 @@ impl SandboxProviderRegistry { } } +impl InventoryEntry { + /// The provider narrowed to fabro's sandboxes, connected on first use; + /// `None` when the kind has nothing to list. + async fn provider(&self) -> crate::Result>> { + match &self.connection { + Connection::HostDirectories => Ok(None), + Connection::Connected(provider) => Ok(Some(provider)), + Connection::Lazy(lazy) => lazy + .provider + .get_or_try_init(|| async { + connect_provider(&self.kind, &lazy.settings, &lazy.options) + .await + .map(|connected| owned(connected.provider)) + .map_err(|error| { + crate::Error::context( + format!("Failed to connect to the {} provider", self.kind), + error, + ) + }) + }) + .await + .map(Some), + } + } + + async fn list(&self) -> crate::Result> { + let Some(provider) = self.provider().await? else { + return Ok(Vec::new()); + }; + let statuses = provider + .list(&SandboxFilter::default()) + .await + .map_err(|error| { + crate::Error::context(format!("Failed to list {} sandboxes", self.kind), error) + })?; + Ok(statuses + .iter() + .map(|status| details::info_from_status(&self.kind, status)) + .collect()) + } + + async fn get(&self, id: &str) -> crate::Result> { + let Some(provider) = self.provider().await? else { + return Ok(None); + }; + // An id the driver cannot even name is not one of ours. + let Ok(sandbox_id) = SandboxId::try_new(id) else { + return Ok(None); + }; + let handle = match provider.attach(&sandbox_id, None).await { + Ok(handle) => handle, + // Unknown to the provider, or not fabro's: neither is in the + // inventory. + Err(DriverError::NotFound { .. } | DriverError::NotOwned { .. }) => return Ok(None), + Err(error) => { + return Err(crate::Error::context( + format!("Failed to look up {} sandbox '{id}'", self.kind), + error, + )); + } + }; + let status = handle.describe().await.map_err(|error| { + crate::Error::context( + format!("Failed to describe {} sandbox '{id}'", self.kind), + error, + ) + })?; + if status.state == SandboxState::Deleted { + return Ok(None); + } + Ok(Some(details::info_from_status(&self.kind, &status))) + } +} + +/// The provider narrowed to fabro's sandboxes. +fn owned(provider: Arc) -> Arc { + Arc::new(OwnedProvider::new( + provider, + managed_labels::ownership(None), + )) +} + #[derive(Debug, thiserror::Error)] pub enum SandboxLookupError { #[error("sandbox '{id}' was not found by any configured provider")] @@ -117,28 +268,6 @@ pub enum SandboxLookupError { }, } -#[derive(Debug, Clone, Copy, Default)] -pub struct LocalSandboxProvider; - -#[async_trait] -impl SandboxProvider for LocalSandboxProvider { - fn kind(&self) -> SandboxProviderKind { - SandboxProviderKind::LOCAL - } - - async fn list(&self) -> crate::Result> { - Ok(Vec::new()) - } - - async fn get(&self, _id: &str) -> crate::Result> { - Ok(None) - } - - async fn delete(&self, _id: &str) -> crate::Result<()> { - Ok(()) - } -} - fn provider_error( provider: SandboxProviderKind, err: &(dyn std::error::Error + 'static), @@ -151,170 +280,177 @@ fn provider_error( #[cfg(test)] mod tests { + use fabro_types::settings::server::SandboxPluginSettings; + use sandbox_driver::SandboxState; + use super::*; use crate::test_support::{ - FakeGet, FakeList, FakeSandboxProvider, fake_registry, fake_sandbox_info, + ScriptedSandbox, managed_scripted_sandbox, scripted_inventory_provider, }; + fn kind(name: &str) -> SandboxProviderKind { + SandboxProviderKind::try_new(name).expect("valid kind") + } + + fn provider(kind: SandboxProviderKind, ids: &[&str]) -> ConnectedProvider { + scripted_inventory_provider( + kind, + ids.iter().map(|id| managed_scripted_sandbox(id)).collect(), + ) + } + + /// A plugin kind whose executable does not exist, so every lookup fails + /// to connect. + fn unreachable_plugin(inventory: SandboxInventory, name: &str) -> SandboxInventory { + let settings = ServerSandboxProviderSettings { + enabled: true, + plugin: Some(SandboxPluginSettings { + path: Some(format!("/nonexistent/fabro-sandbox-{name}")), + dev: true, + ..SandboxPluginSettings::default() + }), + }; + inventory.with_lazy(kind(name), settings, ProviderConnectOptions::default()) + } + #[tokio::test] - async fn list_returns_aggregate_data_from_successful_providers() { - let docker = fake_sandbox_info(SandboxProviderKind::DOCKER, "docker-1"); - let daytona = fake_sandbox_info(SandboxProviderKind::DAYTONA, "daytona-1"); - let registry = fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(vec![docker.clone()]), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(vec![daytona.clone()]), - FakeGet::Missing, - ), + async fn list_aggregates_fabro_owned_sandboxes_across_providers() { + let foreign = Arc::new( + ScriptedSandbox::with_id_and_working_dir("someone-elses", "/work") + .state(SandboxState::Running), + ); + let docker = scripted_inventory_provider(SandboxProviderKind::DOCKER, vec![ + managed_scripted_sandbox("docker-1"), + foreign, ]); + let inventory = SandboxInventory::empty() + .with_host_directories(SandboxProviderKind::LOCAL) + .with_connected(docker) + .with_connected(provider(SandboxProviderKind::DAYTONA, &["daytona-1"])); - let response = registry.list_managed().await; + let response = inventory.list_managed().await; - assert_eq!(response.data, vec![docker, daytona]); + let mut ids: Vec<_> = response.data.iter().map(|s| s.id.as_str()).collect(); + ids.sort_unstable(); + assert_eq!(ids, ["daytona-1", "docker-1"]); assert!(response.meta.provider_errors.is_empty()); - } - - #[tokio::test] - async fn list_includes_provider_error_metadata_when_one_provider_fails() { - let docker = fake_sandbox_info(SandboxProviderKind::DOCKER, "docker-1"); - let registry = fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(vec![docker.clone()]), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Err("daytona unavailable"), - FakeGet::Missing, - ), - ]); - - let response = registry.list_managed().await; - - assert_eq!(response.data, vec![docker]); - assert_eq!(response.meta.provider_errors, vec![ - SandboxProviderLookupError { - provider: SandboxProviderKind::DAYTONA, - message: "daytona unavailable".to_string(), - } + let kinds: Vec<_> = inventory.kinds().cloned().collect(); + assert_eq!(kinds, [ + SandboxProviderKind::LOCAL, + SandboxProviderKind::DOCKER, + SandboxProviderKind::DAYTONA ]); } #[tokio::test] - async fn get_returns_one_matching_sandbox() { - let docker = fake_sandbox_info(SandboxProviderKind::DOCKER, "same-id"); - let registry = fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Found(Box::new(docker.clone())), - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - ]); + async fn list_reports_a_provider_that_cannot_connect_beside_the_others() { + let inventory = unreachable_plugin( + SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &["docker-1"])), + "e2b", + ); - assert_eq!( - registry.get_managed_by_native_id("same-id").await.unwrap(), - docker + let response = inventory.list_managed().await; + + assert_eq!(response.data.len(), 1); + assert_eq!(response.meta.provider_errors.len(), 1); + assert_eq!(response.meta.provider_errors[0].provider, kind("e2b")); + assert!( + response.meta.provider_errors[0] + .message + .contains("Failed to connect to the e2b provider"), + "{}", + response.meta.provider_errors[0].message ); } #[tokio::test] - async fn get_returns_not_found_when_all_providers_miss() { - let registry = fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - ]); + async fn get_finds_one_sandbox_by_native_id() { + let inventory = SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &[])) + .with_connected(provider(SandboxProviderKind::DAYTONA, &["native-id"])); - let err = registry + let sandbox = inventory + .get_managed_by_native_id("native-id") + .await + .expect("one provider matches"); + + assert_eq!(sandbox.id, "native-id"); + assert_eq!(sandbox.provider, SandboxProviderKind::DAYTONA); + } + + #[tokio::test] + async fn get_reports_not_found_when_every_provider_misses() { + let inventory = SandboxInventory::empty() + .with_host_directories(SandboxProviderKind::LOCAL) + .with_connected(provider(SandboxProviderKind::DOCKER, &[])); + + let error = inventory .get_managed_by_native_id("missing") .await - .unwrap_err(); + .expect_err("nothing matches"); - assert!(matches!(err, SandboxLookupError::NotFound { id } if id == "missing")); + assert!(matches!(error, SandboxLookupError::NotFound { id } if id == "missing")); } #[tokio::test] - async fn get_returns_conflict_when_two_providers_match() { - let registry = fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Found(Box::new(fake_sandbox_info( - SandboxProviderKind::DOCKER, - "same-id", - ))), - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Found(Box::new(fake_sandbox_info( - SandboxProviderKind::DAYTONA, - "same-id", - ))), - ), - ]); + async fn get_reports_a_conflict_when_two_providers_match() { + let inventory = SandboxInventory::empty() + .with_connected(provider(SandboxProviderKind::DOCKER, &["same-id"])) + .with_connected(provider(SandboxProviderKind::DAYTONA, &["same-id"])); - let err = registry + let error = inventory .get_managed_by_native_id("same-id") .await - .unwrap_err(); + .expect_err("two providers match"); - assert!(matches!( - err, - SandboxLookupError::Conflict { id, providers } - if id == "same-id" - && providers == vec![SandboxProviderKind::DOCKER, SandboxProviderKind::DAYTONA] - )); + let SandboxLookupError::Conflict { providers, .. } = error else { + panic!("expected a conflict, got {error:?}"); + }; + assert_eq!(providers, [ + SandboxProviderKind::DOCKER, + SandboxProviderKind::DAYTONA + ]); } #[tokio::test] - async fn get_returns_provider_unavailable_when_no_match_and_one_provider_fails() { - let registry = fake_registry(vec![ - FakeSandboxProvider::new( - SandboxProviderKind::DOCKER, - FakeList::Ok(Vec::new()), - FakeGet::Missing, - ), - FakeSandboxProvider::new( - SandboxProviderKind::DAYTONA, - FakeList::Ok(Vec::new()), - FakeGet::Err("daytona unavailable"), - ), - ]); + async fn get_is_unavailable_when_no_match_and_a_provider_failed() { + let inventory = unreachable_plugin( + SandboxInventory::empty().with_connected(provider(SandboxProviderKind::DOCKER, &[])), + "e2b", + ); - let err = registry + let error = inventory .get_managed_by_native_id("maybe-missing") .await - .unwrap_err(); + .expect_err("the failed provider may have held it"); - assert!(matches!( - err, - SandboxLookupError::ProviderUnavailable { - id, - provider_errors - } if id == "maybe-missing" - && provider_errors == vec![SandboxProviderLookupError { - provider: SandboxProviderKind::DAYTONA, - message: "daytona unavailable".to_string(), - }] + let SandboxLookupError::ProviderUnavailable { + provider_errors, .. + } = error + else { + panic!("expected provider unavailable, got {error:?}"); + }; + assert_eq!(provider_errors.len(), 1); + assert_eq!(provider_errors[0].provider, kind("e2b")); + } + + #[tokio::test] + async fn get_ignores_a_sandbox_without_the_managed_label() { + let foreign = Arc::new( + ScriptedSandbox::with_id_and_working_dir("foreign", "/work") + .state(SandboxState::Running), + ); + let inventory = SandboxInventory::empty().with_connected(scripted_inventory_provider( + SandboxProviderKind::DOCKER, + vec![foreign], )); + + let error = inventory + .get_managed_by_native_id("foreign") + .await + .expect_err("a foreign sandbox is not in the inventory"); + + assert!(matches!(error, SandboxLookupError::NotFound { .. })); } } diff --git a/lib/components/fabro-sandbox/src/provider/driver.rs b/lib/components/fabro-sandbox/src/provider/driver.rs deleted file mode 100644 index 069469b77..000000000 --- a/lib/components/fabro-sandbox/src/provider/driver.rs +++ /dev/null @@ -1,234 +0,0 @@ -//! Fabro-managed inventory over a sandbox-driver provider. -//! -//! Lists and looks up the sandboxes fabro created, identified by fabro's -//! own `sh.fabro.managed` label. The driver marks every sandbox it creates -//! with its own label too, but that covers every application on the same -//! daemon or account; the provider is connected through the driver's -//! ownership scope, which lists only fabro's sandboxes and refuses to -//! attach to or delete any other. - -use std::sync::Arc; - -use async_trait::async_trait; -use fabro_types::settings::server::ServerSandboxProviderSettings; -use fabro_types::{SandboxInfo, SandboxProviderKind}; -use sandbox_driver::{ - Error as DriverError, OwnedProvider, SandboxFilter, SandboxId, - SandboxProvider as DriverProvider, -}; -use tokio::sync::OnceCell; - -use super::SandboxProvider; -use crate::driver::{ConnectedProvider, ProviderConnectOptions, connect_provider}; -use crate::{details, managed_labels}; - -/// How the driver provider behind the inventory is obtained. -enum Connection { - Connected(Arc), - /// Connected on first use, so a registry can be assembled synchronously - /// and a provider that is down surfaces as a lookup error rather than a - /// startup failure. - Lazy(Box), -} - -struct LazyConnection { - settings: ServerSandboxProviderSettings, - options: ProviderConnectOptions, - provider: OnceCell>, -} - -pub struct DriverInventoryProvider { - kind: SandboxProviderKind, - connection: Connection, -} - -impl DriverInventoryProvider { - #[must_use] - pub fn new(connected: ConnectedProvider) -> Self { - Self { - kind: connected.kind, - connection: Connection::Connected(owned(connected.provider)), - } - } - - /// An inventory over a provider connected through - /// [`connect_provider`] on first use. - #[must_use] - pub fn lazy( - kind: SandboxProviderKind, - settings: ServerSandboxProviderSettings, - options: ProviderConnectOptions, - ) -> Self { - Self { - kind, - connection: Connection::Lazy(Box::new(LazyConnection { - settings, - options, - provider: OnceCell::new(), - })), - } - } - - async fn provider(&self) -> crate::Result<&Arc> { - match &self.connection { - Connection::Connected(provider) => Ok(provider), - Connection::Lazy(lazy) => { - lazy.provider - .get_or_try_init(|| async { - connect_provider(&self.kind, &lazy.settings, &lazy.options) - .await - .map(|connected| owned(connected.provider)) - .map_err(|error| { - crate::Error::context( - format!("Failed to connect to the {} provider", self.kind), - error, - ) - }) - }) - .await - } - } - } - - async fn describe_managed( - &self, - id: &str, - ) -> crate::Result> { - // An id the driver cannot even name is not one of ours. - let Ok(sandbox_id) = SandboxId::try_new(id) else { - return Ok(None); - }; - let handle = match self.provider().await?.attach(&sandbox_id, None).await { - Ok(handle) => handle, - // Unknown to the provider, or not fabro's: neither is in the - // inventory. - Err(DriverError::NotFound { .. } | DriverError::NotOwned { .. }) => return Ok(None), - Err(error) => { - return Err(crate::Error::context( - format!("Failed to look up {} sandbox '{id}'", self.kind), - error, - )); - } - }; - let status = handle.describe().await.map_err(|error| { - crate::Error::context( - format!("Failed to describe {} sandbox '{id}'", self.kind), - error, - ) - })?; - if status.state == sandbox_driver::SandboxState::Deleted { - return Ok(None); - } - Ok(Some(status)) - } -} - -/// The provider narrowed to fabro's sandboxes. -fn owned(provider: Arc) -> Arc { - Arc::new(OwnedProvider::new( - provider, - managed_labels::ownership(None), - )) -} - -#[async_trait] -impl SandboxProvider for DriverInventoryProvider { - fn kind(&self) -> SandboxProviderKind { - self.kind.clone() - } - - async fn list(&self) -> crate::Result> { - let statuses = self - .provider() - .await? - .list(&SandboxFilter::default()) - .await - .map_err(|error| { - crate::Error::context(format!("Failed to list {} sandboxes", self.kind), error) - })?; - Ok(statuses - .iter() - .map(|status| details::info_from_status(&self.kind, status)) - .collect()) - } - - async fn get(&self, id: &str) -> crate::Result> { - Ok(self - .describe_managed(id) - .await? - .map(|status| details::info_from_status(&self.kind, &status))) - } - - async fn delete(&self, id: &str) -> crate::Result<()> { - // Missing or already deleted is an idempotent success; the scope - // refuses a sandbox that is not fabro's, which must never be - // deleted here. - let Ok(sandbox_id) = SandboxId::try_new(id) else { - return Ok(()); - }; - match self.provider().await?.delete(&sandbox_id, None).await { - Ok(()) => Ok(()), - Err(DriverError::NotOwned { .. }) => Err(crate::Error::message(format!( - "Refusing to delete {} sandbox '{id}' because it is missing label {}={}", - self.kind, - managed_labels::MANAGED_LABEL, - managed_labels::MANAGED_LABEL_VALUE - ))), - Err(error) => Err(crate::Error::context( - format!("Failed to delete {} sandbox '{id}'", self.kind), - error, - )), - } - } -} - -#[cfg(test)] -mod tests { - use sandbox_driver::{SandboxSource, SandboxSpec}; - use sandbox_driver_host::HostProvider; - - use super::*; - - fn inventory() -> (DriverInventoryProvider, Arc) { - let host = Arc::new(HostProvider::new()); - let provider = DriverInventoryProvider::new(ConnectedProvider { - kind: SandboxProviderKind::try_new("host").unwrap(), - provider: host.clone(), - }); - (provider, host) - } - - #[tokio::test] - async fn lists_and_deletes_only_fabro_managed_sandboxes() { - let (inventory, host) = inventory(); - let ours = host - .create( - &SandboxSpec::new(SandboxSource::HostDirectory) - .label(managed_labels::MANAGED_LABEL, "true"), - None, - ) - .await - .unwrap(); - let theirs = host - .create(&SandboxSpec::new(SandboxSource::HostDirectory), None) - .await - .unwrap(); - - let listed = inventory.list().await.unwrap(); - assert_eq!(listed.len(), 1); - assert_eq!(listed[0].id, ours.id().as_str()); - assert_eq!(listed[0].provider.as_str(), "host"); - assert!(inventory.get(ours.id().as_str()).await.unwrap().is_some()); - assert!(inventory.get(theirs.id().as_str()).await.unwrap().is_none()); - - let refused = inventory.delete(theirs.id().as_str()).await.unwrap_err(); - assert!( - refused.to_string().contains("Refusing to delete"), - "{refused}" - ); - inventory.delete(ours.id().as_str()).await.unwrap(); - assert!(inventory.get(ours.id().as_str()).await.unwrap().is_none()); - inventory.delete(ours.id().as_str()).await.unwrap(); - inventory.delete("never-existed").await.unwrap(); - } -} diff --git a/lib/components/fabro-sandbox/src/test_support.rs b/lib/components/fabro-sandbox/src/test_support.rs index 734aa9711..b2a389169 100644 --- a/lib/components/fabro-sandbox/src/test_support.rs +++ b/lib/components/fabro-sandbox/src/test_support.rs @@ -15,10 +15,14 @@ use fabro_types::SandboxProviderKind; use sandbox_driver::{ ExecResult, GrepMatch, PlatformInfo, SandboxState, StderrTail, Termination, WalkedFile, }; -pub use sandbox_driver_testing::{ScriptedExec, ScriptedSandbox, ScriptedStdioProcess}; +pub use sandbox_driver_testing::{ + ScriptedExec, ScriptedProvider, ScriptedSandbox, ScriptedStdioProcess, +}; use tokio::io::DuplexStream; +use crate::driver::ConnectedProvider; use crate::driver_sandbox::RunSandbox; +use crate::managed_labels::{MANAGED_LABEL, MANAGED_LABEL_VALUE}; use crate::sandbox::SandboxFile; /// A driver [`ExecResult`] with the given streams, for scripting a mock @@ -418,98 +422,32 @@ impl MockStdioProcess { } } -// --- FakeSandboxProvider --- +// --- Inventory doubles --- -pub use fake_provider::{FakeGet, FakeList, FakeSandboxProvider, fake_registry, fake_sandbox_info}; +/// A running scripted sandbox carrying fabro's managed label, so an owned +/// inventory lists it and attaches to it. +#[must_use] +pub fn managed_scripted_sandbox(id: &str) -> Arc { + Arc::new( + ScriptedSandbox::with_id_and_working_dir(id, "/work") + .state(SandboxState::Running) + .label(MANAGED_LABEL, MANAGED_LABEL_VALUE), + ) +} -mod fake_provider { - use std::collections::BTreeMap; - use std::sync::Arc; - - use async_trait::async_trait; - use fabro_types::{ - SandboxInfo, SandboxNetwork, SandboxProviderKind, SandboxResources, SandboxState, - SandboxTimestamps, - }; - - use crate::provider::{SandboxProvider, SandboxProviderRegistry}; - - #[derive(Clone)] - pub enum FakeList { - Ok(Vec), - Err(&'static str), +/// A connected inventory provider of `kind` holding `sandboxes`, over the +/// driver's scripted provider. +#[must_use] +pub fn scripted_inventory_provider( + kind: SandboxProviderKind, + sandboxes: Vec>, +) -> ConnectedProvider { + let provider = ScriptedProvider::new(kind.as_str()); + for sandbox in sandboxes { + provider.register(sandbox); } - - #[derive(Clone)] - pub enum FakeGet { - Found(Box), - Missing, - Err(&'static str), - } - - pub struct FakeSandboxProvider { - kind: SandboxProviderKind, - list: FakeList, - get: FakeGet, - } - - impl FakeSandboxProvider { - pub fn new(kind: SandboxProviderKind, list: FakeList, get: FakeGet) -> Self { - Self { kind, list, get } - } - } - - #[async_trait] - impl SandboxProvider for FakeSandboxProvider { - fn kind(&self) -> SandboxProviderKind { - self.kind.clone() - } - - async fn list(&self) -> crate::Result> { - match &self.list { - FakeList::Ok(sandboxes) => Ok(sandboxes.clone()), - FakeList::Err(message) => Err(crate::Error::message(*message)), - } - } - - async fn get(&self, _id: &str) -> crate::Result> { - match &self.get { - FakeGet::Found(sandbox) => Ok(Some((**sandbox).clone())), - FakeGet::Missing => Ok(None), - FakeGet::Err(message) => Err(crate::Error::message(*message)), - } - } - - async fn delete(&self, _id: &str) -> crate::Result<()> { - Ok(()) - } - } - - pub fn fake_registry(providers: Vec) -> SandboxProviderRegistry { - SandboxProviderRegistry::new( - providers - .into_iter() - .map(|provider| Arc::new(provider) as Arc) - .collect(), - ) - } - - pub fn fake_sandbox_info(provider: SandboxProviderKind, id: &str) -> SandboxInfo { - SandboxInfo { - provider, - id: id.to_string(), - display_name: None, - state: SandboxState::Running, - native_state: None, - image: None, - snapshot: None, - region: None, - web_url: None, - working_directory: None, - resources: SandboxResources::default(), - network: SandboxNetwork::unknown(), - labels: BTreeMap::new(), - timestamps: SandboxTimestamps::default(), - } + ConnectedProvider { + kind, + provider: Arc::new(provider), } }