From 8a09e8fa08a40389997f26c069c3d595e4077633 Mon Sep 17 00:00:00 2001 From: Scott Werner Date: Fri, 25 Sep 2026 11:29:29 -0400 Subject: [PATCH] refactor: tidy the in-process sandbox provider wiring Load the Daytona key for fork and prune through one AppState method instead of two copied vault reads, and pass the sandbox configuration into runtime_spec rather than building it and overwriting it. The worker reuses the CLI's process_env_var lookup. Share one Docker availability check and the backend-requirement variable through fabro-test, drop the built-in plugin path and pin constants nothing reads any more, and let enabled_plugins() exclude the bundled kinds itself. Refresh the comments and the spawn_env test that still described built-in plugins. Co-Authored-By: Claude Opus 5.5 --- .../src/commands/run/petri_worker.rs | 13 ++----- lib/apps/fabro-cli/src/main.rs | 4 +-- lib/apps/fabro-cli/tests/it/scenario/petri.rs | 4 +-- .../tests/it/scenario/petri_docker.rs | 28 +++------------ .../fabro-cli/tests/it/workflow/docker.rs | 9 ++--- lib/apps/fabro-server/src/run_manifest.rs | 14 ++++++-- lib/apps/fabro-server/src/sandbox_access.rs | 2 +- lib/apps/fabro-server/src/server.rs | 32 +++++++++++------ .../src/server/handler/lineage.rs | 14 ++------ .../fabro-server/src/server/petri_runs.rs | 22 ++++++++---- lib/apps/fabro-server/src/spawn_env.rs | 27 ++++---------- .../fabro-server/tests/it/scenario/petri.rs | 28 ++------------- lib/components/fabro-petri/tests/hooks.rs | 23 +----------- lib/foundation/fabro-static/src/env_vars.rs | 27 +++----------- lib/foundation/fabro-test/src/lib.rs | 36 ++++++++++++++++--- .../fabro-types/src/settings/server.rs | 5 +-- 16 files changed, 115 insertions(+), 173 deletions(-) diff --git a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs index 80a1cef29..0b35ef58d 100644 --- a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs +++ b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs @@ -618,9 +618,9 @@ async fn runtime_spec( .read() .await .get(EnvVars::DAYTONA_API_KEY) - .map(|key| DaytonaCredentials::from_api_key(key.to_owned(), provider_env)); + .map(|key| DaytonaCredentials::from_api_key(key.to_owned(), crate::process_env_var)); Ok(RuntimeSpec { - sandbox: SandboxProviderConfig::from_lookup(daytona, provider_env), + sandbox: SandboxProviderConfig::from_lookup(daytona, crate::process_env_var), settings_toml: None, mcp_catalog_toml: None, model_client, @@ -629,12 +629,3 @@ async fn runtime_spec( run_tools, }) } - -/// Non-secret provider selection inherited from the server. -#[expect( - clippy::disallowed_methods, - reason = "worker boundary snapshots inherited provider selection" -)] -fn provider_env(name: &str) -> Option { - std::env::var(name).ok() -} diff --git a/lib/apps/fabro-cli/src/main.rs b/lib/apps/fabro-cli/src/main.rs index d4c13101e..83deb5c3e 100644 --- a/lib/apps/fabro-cli/src/main.rs +++ b/lib/apps/fabro-cli/src/main.rs @@ -182,9 +182,9 @@ impl miette::Diagnostic for CliDiagnostic { #[expect( clippy::disallowed_methods, - reason = "CLI main reads documented process-env controls before telemetry and worker dispatch." + reason = "CLI main reads documented process-env controls before telemetry and worker dispatch, and the worker snapshots inherited sandbox provider selection." )] -fn process_env_var(name: &str) -> Option { +pub(crate) fn process_env_var(name: &str) -> Option { std::env::var(name).ok() } diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri.rs b/lib/apps/fabro-cli/tests/it/scenario/petri.rs index 72abc5bb6..5324bc76a 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri.rs @@ -1752,10 +1752,10 @@ async fn built_in_host_runs_and_prunes_without_plugins() { let server = RunningServer::start_with_env("", &[], &[ (EnvVars::PATH, path), ( - EnvVars::PETRI_SANDBOX_HOST_PLUGIN, + "PETRI_SANDBOX_HOST_PLUGIN", "/nonexistent/sandbox-driver-host", ), - (EnvVars::PETRI_SANDBOX_HOST_SHA256, "invalid-pin"), + ("PETRI_SANDBOX_HOST_SHA256", "invalid-pin"), (EnvVars::PETRI_SANDBOX_PLUGIN_DEV, "0"), ]) .await; diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs b/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs index 1c32b0a01..91db4d090 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri_docker.rs @@ -17,7 +17,6 @@ clippy::disallowed_methods, reason = "these scenarios inspect backend availability and drive the Docker daemon with its CLI" )] -#![expect(clippy::print_stderr, reason = "a skipped test says why on its stderr")] use std::env; use std::path::{Path, PathBuf}; @@ -37,29 +36,10 @@ use super::petri::{ use crate::support::TEST_DEV_TOKEN; /// The server-side environment the runs select. -const REQUIRE_ENV: &str = "FABRO_REQUIRE_SANDBOX_BACKENDS"; const ENVIRONMENT: &str = "docker"; /// The twin's model, for the Ask Fabro session. const MODEL: &str = "gpt-5.4"; -/// A reachable Docker daemon. CI requires the backend instead of skipping. -fn docker_available() -> bool { - let daemon = Command::new("docker") - .args(["version", "--format", "{{.Server.Version}}"]) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .is_ok_and(|status| status.success()); - if !daemon { - assert!( - env::var_os(REQUIRE_ENV).is_none(), - "{REQUIRE_ENV} is set, but no Docker daemon answers" - ); - eprintln!("skipping: no Docker daemon answers"); - } - daemon -} - /// A server with a Docker environment beside the default local one. async fn docker_server() -> RunningServer { docker_server_with("", &[]).await @@ -274,7 +254,7 @@ fn restore_actions(server: &RunningServer, run_id: &str) -> Vec { /// nothing of the workspace is on the host. #[tokio::test(flavor = "multi_thread")] async fn a_docker_run_publishes_every_stages_checkpoint_from_the_container() { - if !docker_available() { + if !fabro_test::docker_available() { return; } let context = test_context!(); @@ -305,7 +285,7 @@ async fn a_docker_run_publishes_every_stages_checkpoint_from_the_container() { /// the second stage sees the first stage's files and nothing else. #[tokio::test(flavor = "multi_thread")] async fn a_retained_container_whose_workspace_drifted_is_reset_on_restart() { - if !docker_available() { + if !fabro_test::docker_available() { return; } let context = test_context!(); @@ -350,7 +330,7 @@ async fn a_retained_container_whose_workspace_drifted_is_reset_on_restart() { /// repository, and the second stage sees the first stage's files. #[tokio::test(flavor = "multi_thread")] async fn a_lost_container_is_replaced_and_its_workspace_restored_from_the_snapshot() { - if !docker_available() { + if !fabro_test::docker_available() { return; } let context = test_context!(); @@ -443,7 +423,7 @@ async fn question_inputs(twin: &fabro_test::TwinOpenAi, namespace: &str) -> Vec< /// follow-up request carries the file's content back as the tool's answer. #[tokio::test(flavor = "multi_thread")] async fn an_ask_fabro_turn_reads_a_file_inside_the_runs_container() { - if !docker_available() { + if !fabro_test::docker_available() { return; } let context = test_context!(); diff --git a/lib/apps/fabro-cli/tests/it/workflow/docker.rs b/lib/apps/fabro-cli/tests/it/workflow/docker.rs index 53151d4a9..f6028f9ca 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/docker.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/docker.rs @@ -17,14 +17,11 @@ use std::path::Path; use std::process::{Command, Stdio}; -use fabro_test::{TestContext, expect_reqwest_status}; +use fabro_test::{REQUIRE_SANDBOX_BACKENDS, TestContext, expect_reqwest_status}; use serde_json::json; use crate::cmd::support::server_endpoint; -/// Set in CI so a missing daemon or image fails the test instead of -/// skipping it. -const REQUIRE_ENV: &str = "FABRO_REQUIRE_SANDBOX_BACKENDS"; const DOCKER_IMAGE: &str = "buildpack-deps:noble"; /// The environment id the scenario selects with `--environment`. pub(crate) const ENVIRONMENT: &str = "docker"; @@ -33,11 +30,11 @@ pub(crate) const ENVIRONMENT: &str = "docker"; /// [`DOCKER_IMAGE`]. Returns the environment id, or `None` when the /// prerequisites are missing and the test should skip. pub(crate) fn configure(context: &mut TestContext) -> Option<&'static str> { - let required = std::env::var_os(REQUIRE_ENV).is_some(); + let required = std::env::var_os(REQUIRE_SANDBOX_BACKENDS).is_some(); if !docker_image_available() { assert!( !required, - "{REQUIRE_ENV} is set but no Docker daemon with {DOCKER_IMAGE} is available" + "{REQUIRE_SANDBOX_BACKENDS} is set but no Docker daemon with {DOCKER_IMAGE} is available" ); eprintln!("skipping: no Docker daemon with {DOCKER_IMAGE}"); return None; diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 347a569df..d1b4ad105 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -233,7 +233,12 @@ pub(crate) async fn check_prepared_manifest( None, ); let dry_run = prepared.settings.run.execution.mode == RunMode::DryRun; - let runtime = petri_runs::runtime_spec(state, ready_providers, dry_run); + let runtime = petri_runs::runtime_spec( + state, + ready_providers, + dry_run, + state.sandbox_provider_config(None), + ); let has_ready_provider = !ready_providers.is_empty(); let prepared = prepared.clone(); task::spawn_blocking(move || { @@ -1530,7 +1535,12 @@ mod tests { None, None, ); - let runtime = crate::server::petri_runs::runtime_spec(state, ready_providers, false); + let runtime = crate::server::petri_runs::runtime_spec( + state, + ready_providers, + false, + state.sandbox_provider_config(None), + ); validate_prepared_manifest( prepared, &HashMap::new(), diff --git a/lib/apps/fabro-server/src/sandbox_access.rs b/lib/apps/fabro-server/src/sandbox_access.rs index 3a0f7359d..9c07adcaf 100644 --- a/lib/apps/fabro-server/src/sandbox_access.rs +++ b/lib/apps/fabro-server/src/sandbox_access.rs @@ -144,7 +144,7 @@ pub(crate) enum ConnectError { /// a run's host sandbox is reached through the run's own registry by /// [`attach_run_sandbox`]. /// `docker` connects to the daemon the process environment names, the -/// same variables Petri hands its Docker plugin, without requiring the +/// same variables Fabro forwards to its worker, without requiring the /// daemon to answer: `health` reports an unreachable daemon so preflight /// and the doctor see the cause. `daytona` needs the vault key. Any other /// kind launches the plugin executable its settings name and supervises diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 29704fe62..c1aea9c4a 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -1558,6 +1558,25 @@ impl AppState { ) } + /// [`Self::sandbox_provider_config`] with the Daytona key read from the + /// vault, for server-side fork and prune; a secret store failure is a + /// 500. + pub(crate) async fn load_sandbox_provider_config( + &self, + ) -> Result { + let daytona_api_key = self + .vault_secret(EnvVars::DAYTONA_API_KEY) + .await + .map_err(|err| { + error!(error = ?err, "Loading sandbox credentials failed"); + ApiError::new( + StatusCode::INTERNAL_SERVER_ERROR, + "secret store operation failed", + ) + })?; + Ok(self.sandbox_provider_config(daytona_api_key)) + } + /// Everything a reconnect needs to reach a run's provider: the server's /// provider settings and the Daytona credentials from the vault (`None` /// when no key is stored). @@ -2888,18 +2907,9 @@ async fn delete_run_sandbox_resource( .run_scratch(&id) .root() .join("petri"); - let daytona_api_key = state - .vault_secret(EnvVars::DAYTONA_API_KEY) - .await - .map_err(|err| { - error!(error = ?err, "Loading sandbox credentials failed"); - ApiError::new( - StatusCode::INTERNAL_SERVER_ERROR, - "secret store operation failed", - ) - })?; + let sandbox = state.load_sandbox_provider_config().await?; let report = prune::prune(PruneRequest { - sandbox: state.sandbox_provider_config(daytona_api_key), + sandbox, run_id: id.to_string(), run_dir, store: state.petri_runs.shared_store(), diff --git a/lib/apps/fabro-server/src/server/handler/lineage.rs b/lib/apps/fabro-server/src/server/handler/lineage.rs index 508ba35d4..bd4ad38ac 100644 --- a/lib/apps/fabro-server/src/server/handler/lineage.rs +++ b/lib/apps/fabro-server/src/server/handler/lineage.rs @@ -26,7 +26,6 @@ use fabro_petri::SqliteRunStore; use fabro_petri::fork::{self as petri_fork, ForkError, ForkRequest}; use fabro_petri::petri::RunStore; use fabro_petri::platform_records::SqlitePlatformRecords; -use fabro_static::EnvVars; use fabro_store::{PlatformRecordKind, RunProjection}; use fabro_types::{FailureReason, Principal, RunId}; use fabro_util::error as error_util; @@ -282,16 +281,7 @@ async fn fork_at( .await .map_err(|err| fork_error(&err))?; - let daytona_api_key = state - .vault_secret(EnvVars::DAYTONA_API_KEY) - .await - .map_err(|err| { - error!(error = ?err, "Loading sandbox credentials failed"); - ApiError::new( - StatusCode::INTERNAL_SERVER_ERROR, - "secret store operation failed", - ) - })?; + let sandbox = state.load_sandbox_provider_config().await?; let new_run_id = RunId::new(); let storage = Storage::new(state.server_storage_dir()); let source_run_dir = storage.run_scratch(&id).root().to_path_buf(); @@ -310,7 +300,7 @@ async fn fork_at( .map_err(workflow_operation_error)?; let seeded = petri_fork::fork(ForkRequest { - sandbox: state.sandbox_provider_config(daytona_api_key), + sandbox, source: id, fork: new_run_id, source_run_dir: source_run_dir.join("petri"), diff --git a/lib/apps/fabro-server/src/server/petri_runs.rs b/lib/apps/fabro-server/src/server/petri_runs.rs index c25461f12..db8954e86 100644 --- a/lib/apps/fabro-server/src/server/petri_runs.rs +++ b/lib/apps/fabro-server/src/server/petri_runs.rs @@ -46,6 +46,7 @@ use fabro_petri::hooks::HooksSpec; use fabro_petri::interview::{Approval, FabroInterviewer}; use fabro_petri::petri::{Access, Digest, LogId, Record, RunKey, RunLogs, RunStore, StoreError}; use fabro_petri::platform_records::SqlitePlatformRecords; +use fabro_petri::providers::SandboxProviderConfig; use fabro_petri::recovery::{self, Recovery, RecoveryRequest}; use fabro_petri::runtime::{self, RuntimeSpec}; use fabro_petri::secrets::VaultSecrets; @@ -73,11 +74,12 @@ use crate::run_compiler::{AdmittedRun, PreparedRun, RunCompilerError}; /// The runtime Petri gets, at create and at execution: the server's run /// defaults and environment catalog as the settings layer, the MCP /// catalog, the model client over the server's catalog and credentials for -/// the eligible providers, and the run mode. +/// the eligible providers, the sandbox providers, and the run mode. pub(crate) fn runtime_spec( state: &AppState, eligible: &[ProviderId], dry_run: bool, + sandbox: SandboxProviderConfig, ) -> RuntimeSpec { let settings_toml = settings_layer_toml(state); let mcp_catalog_toml = mcp_catalog_toml(&state.mcp_server_store().catalog_settings()); @@ -95,7 +97,7 @@ pub(crate) fn runtime_spec( } }; RuntimeSpec { - sandbox: state.sandbox_provider_config(None), + sandbox, settings_toml, mcp_catalog_toml, model_client, @@ -276,7 +278,12 @@ pub(crate) async fn admit( settings, prepared.vars(), launch, - runtime_spec(state, eligible, dry_run), + runtime_spec( + state, + eligible, + dry_run, + state.sandbox_provider_config(None), + ), false, ) .map_err(RunCompilerError::Workflow)?; @@ -458,9 +465,12 @@ pub(crate) async fn execute(state: Arc, run_id: RunId) { ))), &run_state.spec.settings.run, ); - let mut runtime = runtime_spec(&state, &eligible, dry_run); - runtime.sandbox = - state.sandbox_provider_config(vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_owned)); + let runtime = runtime_spec( + &state, + &eligible, + dry_run, + state.sandbox_provider_config(vault.get(EnvVars::DAYTONA_API_KEY).map(str::to_owned)), + ); let request = RunRequest { run_id: run_id.to_string(), run_dir: run_dir.join("petri"), diff --git a/lib/apps/fabro-server/src/spawn_env.rs b/lib/apps/fabro-server/src/spawn_env.rs index 99627919f..cf7c2669d 100644 --- a/lib/apps/fabro-server/src/spawn_env.rs +++ b/lib/apps/fabro-server/src/spawn_env.rs @@ -115,9 +115,6 @@ pub(crate) fn sandbox_plugin_env( let mut env = Vec::new(); let mut dev = false; for (kind, plugin) in providers.enabled_plugins() { - if kind.bundled().is_some() { - continue; - } let upper = kind.as_str().to_ascii_uppercase().replace('-', "_"); if let Some(path) = &plugin.path { env.push((format!("PETRI_SANDBOX_{upper}_PLUGIN"), path.clone())); @@ -403,7 +400,7 @@ mod tests { /// while built-in paths and pins and disabled third-party plugins never /// cross. #[tokio::test] - async fn configured_plugins_reach_the_worker_and_win_over_ambient_variables() { + async fn third_party_plugins_reach_the_worker_and_built_ins_never_do() { let mut providers = ServerSandboxProvidersSettings::default(); providers.entries.extend([ provider("e2b", true, SandboxPluginSettings { @@ -422,23 +419,13 @@ mod tests { ..SandboxPluginSettings::default() }), ]); - let env = HashMap::from([ - ("PATH".to_string(), "/bin".to_string()), - ( - "PETRI_SANDBOX_HOST_PLUGIN".to_string(), - "/ambient/sandbox-driver-host".to_string(), - ), - ( - "PETRI_SANDBOX_DOCKER_PLUGIN".to_string(), - "/ambient/sandbox-driver-docker".to_string(), - ), - ( - "PETRI_SANDBOX_DAYTONA_PLUGIN".to_string(), - "/ambient/sandbox-driver-daytona".to_string(), - ), - ]); - let mut env = env; + let mut env = HashMap::from([("PATH".to_string(), "/bin".to_string())]); for kind in ["HOST", "DOCKER", "DAYTONA"] { + let lower = kind.to_ascii_lowercase(); + env.insert( + format!("PETRI_SANDBOX_{kind}_PLUGIN"), + format!("/ambient/sandbox-driver-{lower}"), + ); env.insert( format!("PETRI_SANDBOX_{kind}_SHA256"), "invalid-pin".to_string(), diff --git a/lib/apps/fabro-server/tests/it/scenario/petri.rs b/lib/apps/fabro-server/tests/it/scenario/petri.rs index 360860aff..dd5a68422 100644 --- a/lib/apps/fabro-server/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-server/tests/it/scenario/petri.rs @@ -13,10 +13,8 @@ clippy::disallowed_methods, reason = "the tests inspect backend availability through the process environment" )] -#![expect(clippy::print_stderr, reason = "a skipped test says why on its stderr")] use std::collections::BTreeMap; -use std::env; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::sync::Arc; @@ -43,8 +41,6 @@ use crate::helpers::{ test_settings, wait_for_run_status, }; -const REQUIRE_ENV: &str = "FABRO_REQUIRE_SANDBOX_BACKENDS"; - const OPENAI_MODEL: &str = "gpt-5.4"; /// A command-only workflow: one script stage between start and exit. @@ -103,24 +99,6 @@ const PARALLEL_DOT: &str = r#"digraph Parallel { pub(super) const PLAIN_SETTINGS: &str = "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n"; -/// A reachable Docker daemon. CI requires the backend instead of skipping. -fn docker_available() -> bool { - let daemon = Command::new("docker") - .args(["version", "--format", "{{.Server.Version}}"]) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .status() - .is_ok_and(|status| status.success()); - if !daemon { - assert!( - env::var_os(REQUIRE_ENV).is_none(), - "{REQUIRE_ENV} is set, but no Docker daemon answers" - ); - eprintln!("skipping: no Docker daemon answers"); - } - daemon -} - /// Register a version whose entrypoint is `workflow.fabro`, with the given /// files beside it. pub(super) async fn register_version(app: &axum::Router, files: &[(&str, &str)]) -> String { @@ -892,7 +870,7 @@ async fn a_delete_right_after_the_run_reads_ended_is_accepted() { /// attaches to it on the daemon. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn a_runs_projection_carries_its_docker_sandbox_instance() { - if !docker_available() { + if !fabro_test::docker_available() { return; } let settings = settings_from_toml("_version = 1\n\n[run.environment]\nid = \"docker\"\n"); @@ -1009,7 +987,7 @@ async fn a_runs_projection_carries_its_docker_sandbox_instance() { /// run label. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn the_server_attaches_to_the_container_petri_created() { - if !docker_available() { + if !fabro_test::docker_available() { return; } let twin = twin_openai().await; @@ -1368,7 +1346,7 @@ async fn a_bundle_naming_a_catalog_environment_runs_on_docker_with_its_image() { graph["params"]["fabro.launch"] ); - if !docker_available() { + if !fabro_test::docker_available() { return; } start_run(&app, &run_id).await; diff --git a/lib/components/fabro-petri/tests/hooks.rs b/lib/components/fabro-petri/tests/hooks.rs index ceadd6ffc..2ad46a608 100644 --- a/lib/components/fabro-petri/tests/hooks.rs +++ b/lib/components/fabro-petri/tests/hooks.rs @@ -11,7 +11,6 @@ clippy::disallowed_methods, reason = "the tests inspect backend availability and read the workspace's history with git" )] -#![expect(clippy::print_stderr, reason = "a skipped test says why on its stderr")] use std::collections::BTreeMap; use std::env; @@ -44,8 +43,6 @@ use tokio_util::sync::CancellationToken; mod support; -const REQUIRE_ENV: &str = "FABRO_REQUIRE_SANDBOX_BACKENDS"; - /// A command-only bundle: the stage lines go between `start` and `exit`, /// the edge lines after them. fn workflow(stages: &str, edges: &str) -> String { @@ -81,24 +78,6 @@ fn admit(workflow: &str, settings: &str) -> AdmittedGraphs { } } -/// A reachable Docker daemon. CI requires the backend instead of skipping. -fn docker_available() -> bool { - let daemon = std::process::Command::new("docker") - .args(["version", "--format", "{{.Server.Version}}"]) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .is_ok_and(|status| status.success()); - if !daemon { - assert!( - env::var_os(REQUIRE_ENV).is_none(), - "{REQUIRE_ENV} is set, but no Docker daemon answers" - ); - eprintln!("skipping: no Docker daemon answers"); - } - daemon -} - /// One run's pieces: the store, its platform records, where it ran. struct Harness { run_id: RunId, @@ -900,7 +879,7 @@ async fn parallel_branches_checkpoint_the_shared_workspace_in_turn() { /// its ref, with the platform records naming the same commits. #[tokio::test] async fn a_docker_run_commits_inside_the_container_and_publishes_every_checkpoint() { - if !docker_available() { + if !fabro_test::docker_available() { return; } assert_sandbox_run_publishes_every_checkpoint(SandboxProviderKind::DOCKER).await; diff --git a/lib/foundation/fabro-static/src/env_vars.rs b/lib/foundation/fabro-static/src/env_vars.rs index 4cd3d9f32..53a45f158 100644 --- a/lib/foundation/fabro-static/src/env_vars.rs +++ b/lib/foundation/fabro-static/src/env_vars.rs @@ -49,29 +49,16 @@ impl EnvVars { pub const FABRO_WEB_URL: &'static str = "FABRO_WEB_URL"; pub const FABRO_WORKER_TOKEN: &'static str = "FABRO_WORKER_TOKEN"; - // Petri's sandbox-driver plugins: where each provider's plugin executable - // is, its checksum override, dev mode for unpinned plugins, and how a - // remote Docker daemon's containers reach this machine. A run's worker - // resolves the plugins, so these cross into the worker process. - pub const PETRI_SANDBOX_HOST_PLUGIN: &'static str = "PETRI_SANDBOX_HOST_PLUGIN"; - pub const PETRI_SANDBOX_HOST_SHA256: &'static str = "PETRI_SANDBOX_HOST_SHA256"; - pub const PETRI_SANDBOX_DOCKER_PLUGIN: &'static str = "PETRI_SANDBOX_DOCKER_PLUGIN"; - pub const PETRI_SANDBOX_DOCKER_SHA256: &'static str = "PETRI_SANDBOX_DOCKER_SHA256"; - pub const PETRI_SANDBOX_DAYTONA_PLUGIN: &'static str = "PETRI_SANDBOX_DAYTONA_PLUGIN"; - pub const PETRI_SANDBOX_DAYTONA_SHA256: &'static str = "PETRI_SANDBOX_DAYTONA_SHA256"; + // Petri's sandbox settings: dev mode for unpinned third-party plugins, + // how a remote Docker daemon's containers reach this machine, and the + // action-host image. These cross into a run's worker process. pub const PETRI_SANDBOX_PLUGIN_DEV: &'static str = "PETRI_SANDBOX_PLUGIN_DEV"; pub const PETRI_SANDBOX_DOCKER_HOST_ADDRESS: &'static str = "PETRI_SANDBOX_DOCKER_HOST_ADDRESS"; pub const PETRI_SANDBOX_ACTION_HOST_IMAGE: &'static str = "PETRI_SANDBOX_ACTION_HOST_IMAGE"; - /// Every Petri plugin variable, in one list for the process boundaries + /// Every Petri sandbox variable, in one list for the process boundaries /// that forward them. pub const PETRI_SANDBOX_PLUGIN_VARS: &'static [&'static str] = &[ - Self::PETRI_SANDBOX_HOST_PLUGIN, - Self::PETRI_SANDBOX_HOST_SHA256, - Self::PETRI_SANDBOX_DOCKER_PLUGIN, - Self::PETRI_SANDBOX_DOCKER_SHA256, - Self::PETRI_SANDBOX_DAYTONA_PLUGIN, - Self::PETRI_SANDBOX_DAYTONA_SHA256, Self::PETRI_SANDBOX_PLUGIN_DEV, Self::PETRI_SANDBOX_DOCKER_HOST_ADDRESS, Self::PETRI_SANDBOX_ACTION_HOST_IMAGE, @@ -257,12 +244,6 @@ mod tests { EnvVars::FABRO_VERBOSE, EnvVars::FABRO_WEB_URL, EnvVars::FABRO_WORKER_TOKEN, - EnvVars::PETRI_SANDBOX_HOST_PLUGIN, - EnvVars::PETRI_SANDBOX_HOST_SHA256, - EnvVars::PETRI_SANDBOX_DOCKER_PLUGIN, - EnvVars::PETRI_SANDBOX_DOCKER_SHA256, - EnvVars::PETRI_SANDBOX_DAYTONA_PLUGIN, - EnvVars::PETRI_SANDBOX_DAYTONA_SHA256, EnvVars::PETRI_SANDBOX_PLUGIN_DEV, EnvVars::PETRI_SANDBOX_DOCKER_HOST_ADDRESS, EnvVars::PETRI_SANDBOX_ACTION_HOST_IMAGE, diff --git a/lib/foundation/fabro-test/src/lib.rs b/lib/foundation/fabro-test/src/lib.rs index 96f901de4..e192452c5 100644 --- a/lib/foundation/fabro-test/src/lib.rs +++ b/lib/foundation/fabro-test/src/lib.rs @@ -155,6 +155,34 @@ pub fn require_env(name: &str) -> Option { } } +/// Set in CI so a missing sandbox backend (a Docker daemon or image) fails +/// the test instead of skipping it. +pub const REQUIRE_SANDBOX_BACKENDS: &str = "FABRO_REQUIRE_SANDBOX_BACKENDS"; + +/// A reachable Docker daemon. When [`REQUIRE_SANDBOX_BACKENDS`] is set, a +/// missing daemon fails the test instead of skipping it. +#[must_use] +#[allow( + clippy::print_stderr, + reason = "Skip notices go to stderr so stdout stays assertable." +)] +pub fn docker_available() -> bool { + let daemon = std::process::Command::new("docker") + .args(["version", "--format", "{{.Server.Version}}"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|status| status.success()); + if !daemon { + assert!( + std::env::var_os(REQUIRE_SANDBOX_BACKENDS).is_none(), + "{REQUIRE_SANDBOX_BACKENDS} is set, but no Docker daemon answers" + ); + eprintln!("skipping: no Docker daemon answers"); + } + daemon +} + /// Apply baseline environment isolation to a `Command` that spawns the /// `fabro` binary (or a helper that will act like it). /// @@ -228,10 +256,10 @@ fn apply_test_isolation_with_lookup( if let Some(path) = lookup(EnvVars::PATH) { cmd.env(EnvVars::PATH, path); } - // Petri resolves its sandbox-driver plugins from these, in the server a - // test starts and in the workers that server launches; a developer's - // plugin override reaches them like `PATH` does, and so does the Docker - // daemon selection the Docker plugin needs. + // Petri reads its sandbox settings from these, in the server a test + // starts and in the workers that server launches; a developer's + // override reaches them like `PATH` does, and so does the Docker daemon + // selection the Docker provider needs. for name in EnvVars::PETRI_SANDBOX_PLUGIN_VARS .iter() .chain(EnvVars::DOCKER_VARS) diff --git a/lib/foundation/fabro-types/src/settings/server.rs b/lib/foundation/fabro-types/src/settings/server.rs index 4f4279ded..09466052b 100644 --- a/lib/foundation/fabro-types/src/settings/server.rs +++ b/lib/foundation/fabro-types/src/settings/server.rs @@ -147,12 +147,13 @@ impl ServerSandboxProvidersSettings { .map(|(kind, _)| kind) } - /// Enabled kinds that are served by a plugin executable. + /// Enabled third-party kinds that are served by a plugin executable. + /// Bundled kinds run in process, so plugin settings on them are ignored. pub fn enabled_plugins( &self, ) -> impl Iterator { self.entries.iter().filter_map(|(kind, entry)| { - (entry.enabled) + (entry.enabled && kind.bundled().is_none()) .then_some(entry.plugin.as_ref()) .flatten() .map(|plugin| (kind, plugin))