diff --git a/Cargo.lock b/Cargo.lock index f5473abc4..5b3704880 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2536,16 +2536,22 @@ dependencies = [ name = "fabro-environment" version = "0.281.0-nightly.0" dependencies = [ + "anyhow", + "chrono", "fabro-config", + "fabro-db", "fabro-types", "hex", "serde", + "serde_json", "sha2 0.10.9", + "sqlx", "tempfile", "thiserror 2.0.18", "tokio", "toml 0.8.23", "toml_edit", + "tracing", ] [[package]] @@ -2624,6 +2630,8 @@ dependencies = [ "anyhow", "base64", "fabro-config", + "fabro-db", + "fabro-environment", "fabro-static", "fabro-types", "fabro-util", @@ -3088,11 +3096,12 @@ dependencies = [ name = "fabro-test" version = "0.281.0-nightly.0" dependencies = [ + "anyhow", "assert_cmd", "axum", "fabro-config", - "fabro-environment", "fabro-http", + "fabro-install", "fabro-proc", "fabro-static", "fabro-types", diff --git a/docs/public/execution/environments.mdx b/docs/public/execution/environments.mdx index 44c1a8bb1..f88d9bdd4 100644 --- a/docs/public/execution/environments.mdx +++ b/docs/public/execution/environments.mdx @@ -11,7 +11,7 @@ Fabro separates **environments** from **sandboxes**: Older pre-v1.0 config files that still use `[run.sandbox]` are temporarily auto-migrated when Fabro loads them from disk. Fabro writes a sibling `*.legacy-sandbox-migration.bak` file, rewrites the config to `[run.environment]` plus an environment definition, and then continues startup. -Similarly, `[environments.*]` tables in the server's active `settings.toml` are auto-migrated on startup: each entry is extracted into a sibling `environments/.toml` file, with a `.settings-environments-migration.bak` backup written first. +Similarly, `[environments.*]` tables in the server's active `settings.toml` are auto-migrated on startup. Existing sibling `environments/*.toml` files are then treated as a legacy import source: Fabro imports missing environment IDs into SQLite once and renames the directory to `environments.imported-.bak`. These compatibility rewrites only handle direct field mappings. Unsupported legacy fields fail with a migration message that lists the keys to edit manually. The rewrite paths will be removed before v1.0. @@ -23,45 +23,76 @@ Runs select environments by slug: id = "fabro-dev" ``` -Environments are server-managed. The server keeps one TOML file per environment in an `environments/` directory next to its `settings.toml`, seeded on first startup with built-in `default`, `local`, `docker`, and `daytona` environments. Manage them by editing those files or through the `/api/v1/environments` REST API. Workflow and project TOML can additionally define `[environments.]` catalog entries that merge with the server catalog through the normal settings precedence. The built-in default is `default`, a Docker environment using `buildpack-deps:noble`. +Server-managed environments are stored in the server SQLite database. Manage them through the web UI or the `/api/v1/environments` REST API. Install seeds `default` as an ordinary persisted environment; users can replace or delete it. `local` is reserved, synthetic, and unpersisted: it appears only when the local sandbox provider is enabled, and it cannot be created, replaced, or deleted through the environments API. Workflow and project TOML can additionally define `[environments.]` catalog entries that merge with the server catalog through the normal settings precedence. -## Defining environments +## Defining Server Environments -Each server-managed environment is a file whose name is its slug: +Create server-managed environments through the REST API. Stored environments use inline Dockerfile content; local Dockerfile paths are rejected by the API because the server cannot safely resolve client-side paths. -```toml title="environments/fabro-dev.toml" -provider = "daytona" # local | docker | daytona - -[image] -dockerfile = { path = "Dockerfile" } - -[resources] -cpu = 8 -memory = "16GB" -disk = "20GB" - -[network] -mode = "cidr_allow_list" # allow_all | block | cidr_allow_list -allow = ["10.0.0.0/8"] - -[lifecycle] -preserve = false -stop_on_terminal = true -auto_stop = "30m" - -[labels] -repo = "fabro-sh/fabro" - -[env] -NODE_ENV = "development" +```json title="POST /api/v1/environments" +{ + "id": "fabro-dev", + "provider": "daytona", + "cwd": null, + "image": { + "docker": null, + "dockerfile": { + "type": "inline", + "value": "FROM buildpack-deps:noble\n" + } + }, + "resources": { + "cpu": 8, + "memory": "16GB", + "disk": "20GB" + }, + "network": { + "mode": "cidr_allow_list", + "allow": ["10.0.0.0/8"] + }, + "lifecycle": { + "preserve": false, + "stop_on_terminal": true, + "auto_stop": "30m" + }, + "labels": { + "repo": "fabro-sh/fabro" + }, + "env": { + "NODE_ENV": "development" + } +} ``` -Server-managed local environments can also set `cwd`, an optional runtime +Server-managed local-provider environments can also set `cwd`, an optional runtime command working directory: -```toml title="environments/host.toml" -provider = "local" -cwd = "/srv/fabro/workspaces/team-a" +```json title="POST /api/v1/environments" +{ + "id": "host", + "provider": "local", + "cwd": "/srv/fabro/workspaces/team-a", + "image": { + "docker": null, + "dockerfile": null + }, + "resources": { + "cpu": null, + "memory": null, + "disk": null + }, + "network": { + "mode": "allow_all", + "allow": [] + }, + "lifecycle": { + "preserve": false, + "stop_on_terminal": true, + "auto_stop": null + }, + "labels": {}, + "env": {} +} ``` `cwd` is owned by the server environment and is only honored by the `local` @@ -69,7 +100,7 @@ provider. It is not a replacement for `run.working_dir`. Docker and Daytona ignore `cwd` and report a preflight warning because those clone-based providers own their workspace layout. Workflow, project, user, and direct-run `[environments.]` catalogs cannot set `cwd`; configure it in the -server-managed environment file or through the environments API. +server-managed environment through the environments API. The same fields nest under `[environments.]` when defined in workflow or project TOML instead: @@ -131,25 +162,39 @@ fabro server start --environment default `--preserve-sandbox` still controls the concrete runtime instance lifecycle for a run. Runtime commands such as `fabro sandbox ssh` keep the word "sandbox" because they operate on an already-created runtime instance. -## Built-in environments +## Seeded Environments -The server seeds four built-in environments on first startup: `default`, `local`, `docker`, and `daytona`. Missing files are re-seeded, so editing a seeded file customizes it while deleting it restores the built-in definition on the next restart. The seeded default: +Install seeds a `default` environment into SQLite. It is a normal persisted environment, so deleting it removes the default run target until you recreate it. The standard Docker default is: -```toml title="environments/default.toml" -provider = "docker" - -[image] -docker = "buildpack-deps:noble" - -[resources] -cpu = 2 -memory = "4GB" - -[lifecycle] -preserve = false -stop_on_terminal = true +```json title="GET /api/v1/environments/default" +{ + "id": "default", + "provider": "docker", + "image": { + "docker": "buildpack-deps:noble", + "dockerfile": null + }, + "resources": { + "cpu": 2, + "memory": "4GB", + "disk": null + }, + "network": { + "mode": "allow_all", + "allow": [] + }, + "lifecycle": { + "preserve": false, + "stop_on_terminal": true, + "auto_stop": null + }, + "labels": {}, + "env": {} +} ``` +`local` is not stored in SQLite. It is synthesized at runtime when the local sandbox provider is enabled. + ## Provider mappings | Environment field | Local | Docker | Daytona | @@ -171,10 +216,7 @@ stop_on_terminal = true `local` runs tools directly in the resolved working directory. It offers no filesystem or network isolation, so use it only for trusted workflows. -```toml title="environments/host.toml" -provider = "local" -cwd = "/srv/fabro/workspaces/team-a" -``` +Create a server-managed local-provider environment through the environments API when you need a host `cwd`. When `cwd` is set, local runs execute commands from that absolute server-side path. When it is unset, Fabro keeps same-host compatibility by using the @@ -188,17 +230,18 @@ Fabro hard-errors if a local environment asks for blocked or CIDR-restricted net Docker runs tools inside a container created from `image.docker`. Docker is the built-in default provider. -```toml title="environments/ci.toml" +```toml title="workflow.toml" +[environments.ci] provider = "docker" -[image] +[environments.ci.image] docker = "buildpack-deps:noble" -[resources] +[environments.ci.resources] cpu = 2 memory = "4GB" -[network] +[environments.ci.network] mode = "block" ``` @@ -208,21 +251,22 @@ Docker and Daytona are clone-based providers. When a run has a GitHub origin, Fa Daytona runs tools in a cloud sandbox. Without `image.dockerfile`, Fabro uses Daytona's built-in `daytona-medium` snapshot. With `image.dockerfile`, Fabro computes a deterministic internal snapshot name from the Dockerfile, resource hints, a single-tenant scope, and the Daytona API key. -```toml title="environments/cloud.toml" +```toml title="workflow.toml" +[environments.cloud] provider = "daytona" -[image] +[environments.cloud.image] dockerfile = { path = "Dockerfile" } -[resources] +[environments.cloud.resources] cpu = 4 memory = "8GB" disk = "20GB" -[lifecycle] +[environments.cloud.lifecycle] auto_stop = "30m" -[network] +[environments.cloud.network] mode = "cidr_allow_list" allow = ["208.80.154.232/32", "10.0.0.0/8"] ``` diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 4c40e3dad..d5c2145e4 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -31,15 +31,14 @@ use fabro_install::{ GITHUB_APP_VAULT_KEYS, GITHUB_INSTALL_SECRET_KEYS, InstallListenConfig, InstallPersistencePlan, PendingDevTokenWrite, PendingSettingsWrite, VaultSecretWrite, merge_server_settings as merge_server_settings_impl, prepare_dev_token_write_for_install, - restore_optional_file, rollback_dev_token_write, write_github_app_settings, - write_token_settings, + restore_optional_file, rollback_dev_token_write, seed_environments_in_storage, + write_github_app_settings, write_token_settings, }; use fabro_model::catalog::CatalogProvider; use fabro_model::{Catalog, CredentialRef, ProviderId}; use fabro_server::serve; use fabro_store::ArtifactStore; use fabro_types::ServerSettings; -use fabro_types::settings::run::EnvironmentProvider; use fabro_types::settings::server::ServerAuthMethod; use fabro_types::settings::validate_public_url_with_label; use fabro_util::printer::Printer; @@ -2014,16 +2013,11 @@ async fn run_install_inner(args: &InstallArgs, ctx: &CommandContext) -> Result<( ) .await?; - // Seed the default environment next to the settings file. The server never - // seeds on startup, so install is the only place the default is written; - // existing files are preserved, so re-running install never clobbers edits. - let environment_dir = config_path - .parent() - .unwrap_or_else(|| Path::new(".")) - .join("environments"); - if let Err(err) = - fabro_environment::seed_default_environment(&environment_dir, EnvironmentProvider::Docker) - { + // Seed the default environment in SQLite. The server never seeds on + // startup, so install is the only place the default is written; existing + // rows are preserved, so re-running install never clobbers edits. + let environment_seed_result = seed_environments_in_storage(&storage_dir).await; + if let Err(err) = environment_seed_result { fabro_util::printerr!( printer, " {} Failed to seed default environment: {err}", diff --git a/lib/crates/fabro-db/migrations/2026063002_environments.sql b/lib/crates/fabro-db/migrations/2026063002_environments.sql new file mode 100644 index 000000000..5219d6509 --- /dev/null +++ b/lib/crates/fabro-db/migrations/2026063002_environments.sql @@ -0,0 +1,31 @@ +CREATE TABLE environments ( + id TEXT PRIMARY KEY NOT NULL, + revision TEXT NOT NULL, + provider TEXT NOT NULL, + cwd TEXT, + image_docker TEXT, + image_dockerfile_inline TEXT, + resources_cpu INTEGER, + resources_memory TEXT, + resources_disk TEXT, + network_mode TEXT NOT NULL, + network_allow_json TEXT NOT NULL DEFAULT '[]', + lifecycle_preserve INTEGER NOT NULL, + lifecycle_stop_on_terminal INTEGER NOT NULL, + lifecycle_auto_stop TEXT, + labels_json TEXT NOT NULL DEFAULT '{}', + env_json TEXT NOT NULL DEFAULT '{}', + CHECK (length(id) BETWEEN 1 AND 63), + CHECK (substr(id, 1, 1) GLOB '[a-z0-9]'), + CHECK (id NOT GLOB '*[^a-z0-9-]*'), + CHECK (id <> 'local'), + CHECK (length(revision) = 64), + CHECK (revision NOT GLOB '*[^0-9a-f]*'), + CHECK (provider IN ('local', 'docker', 'daytona')), + CHECK (network_mode IN ('allow_all', 'block', 'cidr_allow_list')), + CHECK (lifecycle_preserve IN (0, 1)), + CHECK (lifecycle_stop_on_terminal IN (0, 1)), + CHECK (json_valid(network_allow_json)), + CHECK (json_valid(labels_json)), + CHECK (json_valid(env_json)) +); diff --git a/lib/crates/fabro-db/tests/sqlite.rs b/lib/crates/fabro-db/tests/sqlite.rs index e37e05fdf..a030dafa7 100644 --- a/lib/crates/fabro-db/tests/sqlite.rs +++ b/lib/crates/fabro-db/tests/sqlite.rs @@ -18,6 +18,13 @@ async fn connect_creates_parent_directory_and_migrate_is_idempotent() -> anyhow: .await?; assert_eq!(variable_table_count, 1); + let environments_table_count: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'environments'", + ) + .fetch_one(database.pool()) + .await?; + assert_eq!(environments_table_count, 1); + let legacy_import_table_count: i64 = sqlx::query_scalar( "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'legacy_imports'", ) @@ -34,6 +41,61 @@ async fn connect_creates_parent_directory_and_migrate_is_idempotent() -> anyhow: Ok(()) } +#[tokio::test] +async fn environments_schema_rejects_invalid_rows() -> anyhow::Result<()> { + let dir = tempfile::tempdir()?; + let database = fabro_db::Database::connect(dir.path().join("fabro.sqlite3")).await?; + database.migrate().await?; + + insert_minimal_environment(database.pool(), "valid", "docker", "allow_all").await?; + + for (id, provider, network_mode) in [ + ("Bad", "docker", "allow_all"), + ("local", "docker", "allow_all"), + ("bad-provider", "bogus", "allow_all"), + ("bad-network", "docker", "bogus"), + ] { + let result = insert_minimal_environment(database.pool(), id, provider, network_mode).await; + assert!( + result.is_err(), + "environment row should be rejected: id={id}, provider={provider}, network_mode={network_mode}" + ); + } + + Ok(()) +} + +async fn insert_minimal_environment( + pool: &fabro_db::DbPool, + id: &str, + provider: &str, + network_mode: &str, +) -> Result<(), sqlx::Error> { + sqlx::query( + r" + INSERT INTO environments ( + id, + revision, + provider, + network_mode, + lifecycle_preserve, + lifecycle_stop_on_terminal + ) + VALUES (?, ?, ?, ?, ?, ?) + ", + ) + .bind(id) + .bind("a".repeat(64)) + .bind(provider) + .bind(network_mode) + .bind(false) + .bind(true) + .execute(pool) + .await?; + + Ok(()) +} + #[tokio::test] async fn variables_schema_enforces_env_style_names() -> anyhow::Result<()> { let dir = tempfile::tempdir()?; diff --git a/lib/crates/fabro-environment/Cargo.toml b/lib/crates/fabro-environment/Cargo.toml index 638538df8..ab06a442f 100644 --- a/lib/crates/fabro-environment/Cargo.toml +++ b/lib/crates/fabro-environment/Cargo.toml @@ -13,16 +13,22 @@ doctest = false workspace = true [dependencies] +chrono.workspace = true +fabro-db = { path = "../fabro-db" } fabro-config = { path = "../fabro-config" } fabro-types = { path = "../fabro-types" } hex.workspace = true serde.workspace = true +serde_json.workspace = true sha2.workspace = true +sqlx.workspace = true thiserror.workspace = true tokio.workspace = true toml.workspace = true toml_edit.workspace = true +tracing.workspace = true [dev-dependencies] +anyhow.workspace = true tempfile = "3" tokio = { workspace = true, features = ["macros", "test-util"] } diff --git a/lib/crates/fabro-environment/src/error.rs b/lib/crates/fabro-environment/src/error.rs index 9adfbcecd..838898a59 100644 --- a/lib/crates/fabro-environment/src/error.rs +++ b/lib/crates/fabro-environment/src/error.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use toml::de::Error as TomlDeError; use toml::ser::Error as TomlSerError; -use crate::{EnvironmentId, EnvironmentRevision}; +use crate::{EnvironmentId, EnvironmentRevision, EnvironmentRevisionParseError}; #[derive(Debug, thiserror::Error)] pub enum EnvironmentValidationError { @@ -17,6 +17,10 @@ pub enum EnvironmentValidationError { #[source] source: std::io::Error, }, + #[error( + "Dockerfile path sources are not supported for stored environments; use inline Dockerfile content" + )] + DockerfilePathUnsupported, } #[derive(Debug, thiserror::Error)] @@ -46,6 +50,12 @@ pub enum EnvironmentStoreError { #[source] source: TomlDeError, }, + #[error("invalid persisted environment revision for {id}")] + InvalidRevision { + id: EnvironmentId, + #[source] + source: EnvironmentRevisionParseError, + }, #[error("environment TOML at {path:?} is not UTF-8")] InvalidUtf8 { path: PathBuf, @@ -63,6 +73,25 @@ pub enum EnvironmentStoreError { #[source] source: std::io::Error, }, + #[error("failed to encode environment JSON for {field}")] + JsonEncode { + field: &'static str, + #[source] + source: serde_json::Error, + }, + #[error("failed to decode environment JSON for {field}")] + JsonDecode { + field: &'static str, + #[source] + source: serde_json::Error, + }, + #[error("database error")] + Db { + #[from] + source: sqlx::Error, + }, + #[error("environment row count {count} exceeds SQLite integer range")] + RowCountOverflow { count: usize }, } impl EnvironmentStoreError { @@ -96,8 +125,11 @@ impl EnvironmentStoreError { Self::Reserved { .. } => "reserved", Self::Validation { .. } => "validation", Self::InvalidFilename { .. } => "invalid_filename", - Self::Parse { .. } | Self::InvalidUtf8 { .. } => "parse", + Self::Parse { .. } | Self::InvalidUtf8 { .. } | Self::InvalidRevision { .. } => "parse", Self::Serialize { .. } => "serialize", + Self::JsonEncode { .. } | Self::JsonDecode { .. } => "json", + Self::Db { .. } => "db", + Self::RowCountOverflow { .. } => "row_count_overflow", Self::Io { .. } => "io", } } diff --git a/lib/crates/fabro-environment/src/lib.rs b/lib/crates/fabro-environment/src/lib.rs index 330ea1eda..bd6029bd8 100644 --- a/lib/crates/fabro-environment/src/lib.rs +++ b/lib/crates/fabro-environment/src/lib.rs @@ -7,5 +7,6 @@ pub use error::{EnvironmentStoreError, EnvironmentValidationError}; pub use id::{EnvironmentId, EnvironmentRevision, EnvironmentRevisionParseError}; pub use model::{Environment, EnvironmentDraft}; pub use store::{ - EnvironmentStore, seed_default_environment, seed_environments, seeded_catalog_layer, + EnvironmentStore, ImportReport, import_legacy_directory_once, seed_default_environment, + seed_environments, seeded_catalog_layer, }; diff --git a/lib/crates/fabro-environment/src/model.rs b/lib/crates/fabro-environment/src/model.rs index 5f468938a..9ab67cb51 100644 --- a/lib/crates/fabro-environment/src/model.rs +++ b/lib/crates/fabro-environment/src/model.rs @@ -28,16 +28,27 @@ pub struct Environment { } impl Environment { - pub(crate) fn from_persisted_path( + pub(crate) async fn from_legacy_path( id: EnvironmentId, bytes: &[u8], path: &Path, ) -> Result { - let revision = EnvironmentRevision::from_bytes(bytes); let mut persisted = parse_persisted(bytes, path)?; let base_dir = path.parent().unwrap_or_else(|| Path::new(".")); - inline_layer_dockerfile_paths(&mut persisted, base_dir)?; + inline_layer_dockerfile_paths(&mut persisted, base_dir).await?; let settings = resolve_environment(&persisted)?; + Self::from_settings(id, &settings) + } + + pub(crate) fn from_settings( + id: EnvironmentId, + settings: &EnvironmentSettings, + ) -> Result { + reject_dockerfile_paths(settings)?; + let persisted = environment_settings_to_layer(settings); + let settings = resolve_environment(&persisted)?; + let bytes = canonical_bytes(&persisted).into_bytes(); + let revision = EnvironmentRevision::from_bytes(&bytes); Ok(Self { id, revision, @@ -45,24 +56,17 @@ impl Environment { }) } - pub(crate) async fn from_settings( + pub(crate) fn from_row( id: EnvironmentId, - settings: EnvironmentSettings, - dockerfile_base_dir: &Path, - ) -> Result<(Self, Vec), EnvironmentStoreError> { - let settings = inline_dense_dockerfile(settings, dockerfile_base_dir).await?; - let persisted = environment_settings_to_layer(&settings); - let settings = resolve_environment(&persisted)?; - let bytes = canonical_bytes(&persisted).into_bytes(); - let revision = EnvironmentRevision::from_bytes(&bytes); - Ok(( - Self { - id, - revision, - settings, - }, - bytes, - )) + revision: EnvironmentRevision, + layer: &EnvironmentLayer, + ) -> Result { + let settings = resolve_environment(layer)?; + Ok(Self { + id, + revision, + settings, + }) } /// Builds an in-memory environment from settings without touching the @@ -137,11 +141,7 @@ fn resolve_environment( }) } -#[expect( - clippy::disallowed_methods, - reason = "Dockerfile inlining runs during synchronous startup load before request handling." -)] -fn inline_layer_dockerfile_paths( +async fn inline_layer_dockerfile_paths( layer: &mut EnvironmentLayer, base_dir: &Path, ) -> Result<(), EnvironmentValidationError> { @@ -152,7 +152,7 @@ fn inline_layer_dockerfile_paths( return Ok(()); }; let path = base_dir.join(path); - let content = std::fs::read_to_string(&path).map_err(|source| { + let content = fs::read_to_string(&path).await.map_err(|source| { EnvironmentValidationError::DockerfileRead { path: path.clone(), source, @@ -162,22 +162,16 @@ fn inline_layer_dockerfile_paths( Ok(()) } -async fn inline_dense_dockerfile( - mut settings: EnvironmentSettings, - base_dir: &Path, -) -> Result { - let Some(DockerfileSource::Path { path }) = settings.image.dockerfile.as_ref() else { - return Ok(settings); - }; - let path = base_dir.join(path); - let content = fs::read_to_string(&path).await.map_err(|source| { - EnvironmentValidationError::DockerfileRead { - path: path.clone(), - source, - } - })?; - settings.image.dockerfile = Some(DockerfileSource::Inline(content)); - Ok(settings) +fn reject_dockerfile_paths( + settings: &EnvironmentSettings, +) -> Result<(), EnvironmentValidationError> { + if matches!( + settings.image.dockerfile, + Some(DockerfileSource::Path { .. }) + ) { + return Err(EnvironmentValidationError::DockerfilePathUnsupported); + } + Ok(()) } fn environment_settings_to_layer(settings: &EnvironmentSettings) -> EnvironmentLayer { diff --git a/lib/crates/fabro-environment/src/store.rs b/lib/crates/fabro-environment/src/store.rs index 4e1ce9afe..8d465d459 100644 --- a/lib/crates/fabro-environment/src/store.rs +++ b/lib/crates/fabro-environment/src/store.rs @@ -1,29 +1,38 @@ -use std::collections::HashMap; -use std::io::ErrorKind; +use std::collections::{BTreeMap, HashMap, HashSet}; +use std::ffi::OsString; use std::path::{Path, PathBuf}; +use std::str::FromStr; use std::sync::Arc; -use std::time::{SystemTime, UNIX_EPOCH}; -use fabro_config::{EnvironmentLayer, MergeMap}; -use fabro_types::settings::run::{EnvironmentProvider, EnvironmentSettings}; +use chrono::{DateTime, Utc}; +use fabro_config::{ + EnvironmentDockerfileLayer, EnvironmentImageLayer, EnvironmentLayer, EnvironmentLifecycleLayer, + EnvironmentNetworkLayer, EnvironmentResourcesLayer, MergeMap, StickyMap, +}; +use fabro_db::DbPool; +use fabro_types::settings::run::{DockerfileSource, EnvironmentProvider, EnvironmentSettings}; +use fabro_types::settings::{Duration, InterpString, Size}; +use serde::de::DeserializeOwned; +use sqlx::Row as _; +use sqlx::sqlite::SqliteRow; use tokio::fs; -use tokio::io::AsyncWriteExt as _; use tokio::sync::Mutex; +use tracing::info; use crate::{ Environment, EnvironmentDraft, EnvironmentId, EnvironmentRevision, EnvironmentStoreError, + EnvironmentValidationError, }; -/// Built-in default environment written to disk by the installer (see -/// [`seed_default_environment`]). The server itself never seeds: a Fabro -/// instance that has not been installed has no managed environments, and a run -/// that selects an absent environment fails explicitly. `local` is -/// intentionally absent: it is a reserved, in-memory environment (see -/// [`RESERVED_LOCAL_ID`]). +/// Built-in default environment seeded by install/test setup. The server itself +/// never seeds during normal startup: an uninstalled instance has no persisted +/// managed environments, and a run that selects an absent environment fails +/// explicitly. `local` is intentionally absent from SQLite because it is a +/// reserved, in-memory environment. const DEFAULT_ENVIRONMENT_ID: &str = "default"; /// `local` is a reserved environment: it is synthesized in memory only when the -/// local sandbox provider is enabled, is never persisted to disk, and cannot be +/// local sandbox provider is enabled, is never persisted, and cannot be /// created, replaced, or deleted through the store. const RESERVED_LOCAL_ID: &str = "local"; @@ -76,10 +85,18 @@ stop_on_terminal = true #[derive(Debug)] pub struct EnvironmentStore { - dir: PathBuf, - request_base_dir: PathBuf, - mutations: Mutex<()>, - state: std::sync::RwLock, + pool: DbPool, + mutations: Mutex<()>, + state: std::sync::RwLock, +} + +#[derive(Debug, Clone)] +pub struct ImportReport { + pub source_path: PathBuf, + pub backup_path: PathBuf, + pub imported_rows: i64, + pub skipped_rows: i64, + pub environment_ids: Vec, } #[derive(Debug, Clone)] @@ -97,7 +114,18 @@ impl CatalogState { } } - fn refresh_catalog(&mut self) { + fn insert(&mut self, environment: Environment) { + self.environments + .insert(environment.id.clone(), environment); + self.rebuild_catalog(); + } + + fn remove(&mut self, id: &EnvironmentId) { + self.environments.remove(id); + self.rebuild_catalog(); + } + + fn rebuild_catalog(&mut self) { self.catalog = Arc::new(build_catalog_layer(&self.environments)); } } @@ -125,29 +153,12 @@ fn build_catalog_layer( } impl EnvironmentStore { - /// Synchronously load all persisted environments. The synchronous file - /// access runs during server startup before request handling begins. - /// - /// The server never seeds the default environment; seeding is an - /// install-time action (see [`seed_default_environment`]). An uninstalled - /// instance therefore - /// has no managed environments on disk, and the reserved `local` - /// environment is the only entry present (when the local provider is - /// enabled). - pub fn load( - dir: impl Into, - local_enabled: bool, - ) -> Result { - let dir = dir.into(); - let mut environments = load_environments(&dir)?; - if local_enabled { - let local = synthetic_local_environment()?; - environments.insert(local.id.clone(), local); - } - let request_base_dir = dir.parent().unwrap_or_else(|| Path::new(".")).to_path_buf(); + /// Load all persisted environments from SQLite and build the synchronous + /// in-memory catalog cache used by request paths. + pub async fn load(pool: DbPool, local_enabled: bool) -> Result { + let environments = load_environments(&pool, local_enabled).await?; Ok(Self { - dir, - request_base_dir, + pool, mutations: Mutex::new(()), state: std::sync::RwLock::new(CatalogState::new(environments)), }) @@ -180,21 +191,15 @@ impl EnvironmentStore { if id.as_str() == RESERVED_LOCAL_ID { return Err(EnvironmentStoreError::Reserved { id }); } - let (environment, bytes) = - Environment::from_settings(id.clone(), settings, &self.request_base_dir).await?; + let environment = Environment::from_settings(id.clone(), &settings)?; + let _mutation = self.mutations.lock().await; - if self.read_state().environments.contains_key(&id) { + let mut transaction = self.pool.begin().await?; + if !insert_environment_ignoring_conflict(&mut transaction, &environment).await? { return Err(EnvironmentStoreError::AlreadyExists { id }); } - - let path = environment_path(&self.dir, &id); - write_new(&self.dir, &path, &bytes) - .await - .map_err(|err| create_error_for(id.clone(), err))?; - - let mut state = self.write_state(); - state.environments.insert(id, environment.clone()); - state.refresh_catalog(); + transaction.commit().await?; + self.write_state().insert(environment.clone()); Ok(environment) } @@ -207,15 +212,13 @@ impl EnvironmentStore { if id.as_str() == RESERVED_LOCAL_ID { return Err(EnvironmentStoreError::Reserved { id: id.clone() }); } - let (environment, bytes) = - Environment::from_settings(id.clone(), settings, &self.request_base_dir).await?; - let _mutation = self.mutations.lock().await; - check_revision(&self.read_state().environments, id, expected)?; + let environment = Environment::from_settings(id.clone(), &settings)?; - write_atomic(&self.dir, &environment_path(&self.dir, id), &bytes).await?; - let mut state = self.write_state(); - state.environments.insert(id.clone(), environment.clone()); - state.refresh_catalog(); + let _mutation = self.mutations.lock().await; + let mut transaction = self.pool.begin().await?; + update_environment(&mut transaction, &environment, expected).await?; + transaction.commit().await?; + self.write_state().insert(environment.clone()); Ok(environment) } @@ -232,15 +235,17 @@ impl EnvironmentStore { } let _mutation = self.mutations.lock().await; - check_revision(&self.read_state().environments, id, expected)?; - - let path = environment_path(&self.dir, id); - fs::remove_file(&path) - .await - .map_err(|err| EnvironmentStoreError::io(path, err))?; - let mut state = self.write_state(); - state.environments.remove(id); - state.refresh_catalog(); + let mut transaction = self.pool.begin().await?; + let result = sqlx::query("DELETE FROM environments WHERE id = ? AND revision = ?") + .bind(id.as_str()) + .bind(expected.as_str()) + .execute(&mut *transaction) + .await?; + if result.rows_affected() == 0 { + return Err(revision_mismatch_error(&mut transaction, id, expected).await?); + } + transaction.commit().await?; + self.write_state().remove(id); Ok(()) } @@ -249,112 +254,522 @@ impl EnvironmentStore { } } -fn check_revision( - environments: &HashMap, +async fn load_environments( + pool: &DbPool, + local_enabled: bool, +) -> Result, EnvironmentStoreError> { + let rows = sqlx::query( + r" + SELECT + id, + revision, + provider, + cwd, + image_docker, + image_dockerfile_inline, + resources_cpu, + resources_memory, + resources_disk, + network_mode, + network_allow_json, + lifecycle_preserve, + lifecycle_stop_on_terminal, + lifecycle_auto_stop, + labels_json, + env_json + FROM environments + ORDER BY id + ", + ) + .fetch_all(pool) + .await?; + + let mut environments = HashMap::new(); + for row in rows { + let environment = environment_from_row(&row)?; + environments.insert(environment.id.clone(), environment); + } + if local_enabled { + let local = synthetic_local_environment()?; + environments.insert(local.id.clone(), local); + } + Ok(environments) +} + +fn environment_from_row(row: &SqliteRow) -> Result { + let id_text = row.get::("id"); + let id = EnvironmentId::new(id_text)?; + let revision_text = row.get::("revision"); + let revision = EnvironmentRevision::from_str(&revision_text).map_err(|source| { + EnvironmentStoreError::InvalidRevision { + id: id.clone(), + source, + } + })?; + let network_allow_json = row.get::("network_allow_json"); + let labels_json = row.get::("labels_json"); + let env_json = row.get::("env_json"); + let layer = EnvironmentLayer { + provider: Some(row.get("provider")), + cwd: row.get("cwd"), + image: image_layer_from_row(row), + resources: resources_layer_from_row(row)?, + network: Some(EnvironmentNetworkLayer { + mode: Some(row.get("network_mode")), + allow: decode_json("network_allow_json", &network_allow_json)?, + }), + lifecycle: Some(EnvironmentLifecycleLayer { + preserve: Some(row.get("lifecycle_preserve")), + stop_on_terminal: Some(row.get("lifecycle_stop_on_terminal")), + auto_stop: parse_duration( + "lifecycle_auto_stop", + row.get("lifecycle_auto_stop"), + )?, + }), + labels: StickyMap::from(decode_json::>( + "labels_json", + &labels_json, + )?), + env: StickyMap::from(decode_env_json(&env_json)?), + }; + + Environment::from_row(id, revision, &layer) +} + +fn image_layer_from_row(row: &SqliteRow) -> Option { + let docker: Option = row.get("image_docker"); + let dockerfile_inline: Option = row.get("image_dockerfile_inline"); + if docker.is_none() && dockerfile_inline.is_none() { + return None; + } + Some(EnvironmentImageLayer { + docker, + dockerfile: dockerfile_inline.map(EnvironmentDockerfileLayer::Inline), + }) +} + +fn resources_layer_from_row( + row: &SqliteRow, +) -> Result, EnvironmentStoreError> { + let cpu: Option = row.get("resources_cpu"); + let memory = parse_size("resources_memory", row.get("resources_memory"))?; + let disk = parse_size("resources_disk", row.get("resources_disk"))?; + if cpu.is_none() && memory.is_none() && disk.is_none() { + return Ok(None); + } + Ok(Some(EnvironmentResourcesLayer { cpu, memory, disk })) +} + +fn parse_optional_field( + field: &'static str, + value: Option, +) -> Result, EnvironmentValidationError> +where + T: FromStr, + T::Err: std::fmt::Display, +{ + value + .map(|value| { + value + .parse() + .map_err(|err| EnvironmentValidationError::InvalidSettings { + errors: vec![format!("environment.{field}: {err}")], + }) + }) + .transpose() +} + +fn parse_size( + field: &'static str, + value: Option, +) -> Result, EnvironmentValidationError> { + parse_optional_field(field, value) +} + +fn parse_duration( + field: &'static str, + value: Option, +) -> Result, EnvironmentValidationError> { + parse_optional_field(field, value) +} + +async fn current_revision( + transaction: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + id: &EnvironmentId, +) -> Result, EnvironmentStoreError> { + let current: Option = + sqlx::query_scalar("SELECT revision FROM environments WHERE id = ?") + .bind(id.as_str()) + .fetch_optional(&mut **transaction) + .await?; + current + .map(|revision| { + EnvironmentRevision::from_str(&revision).map_err(|source| { + EnvironmentStoreError::InvalidRevision { + id: id.clone(), + source, + } + }) + }) + .transpose() +} + +async fn revision_mismatch_error( + transaction: &mut sqlx::Transaction<'_, sqlx::Sqlite>, id: &EnvironmentId, expected: &EnvironmentRevision, +) -> Result { + let Some(actual) = current_revision(transaction, id).await? else { + return Err(EnvironmentStoreError::NotFound { id: id.clone() }); + }; + Ok(EnvironmentStoreError::StaleRevision { + id: id.clone(), + expected: expected.clone(), + actual, + }) +} + +async fn insert_environment_ignoring_conflict( + transaction: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + environment: &Environment, +) -> Result { + let result = execute_environment_insert_sql( + transaction, + environment, + INSERT_ENVIRONMENT_IGNORE_CONFLICT_SQL, + ) + .await?; + Ok(result > 0) +} + +async fn execute_environment_insert_sql( + transaction: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + environment: &Environment, + sql: &'static str, +) -> Result { + let row = EnvironmentSqlRow::from_environment(environment)?; + let result = sqlx::query(sql) + .bind(row.id) + .bind(row.revision) + .bind(row.provider) + .bind(row.cwd) + .bind(row.image_docker) + .bind(row.image_dockerfile_inline) + .bind(row.resources_cpu) + .bind(row.resources_memory) + .bind(row.resources_disk) + .bind(row.network_mode) + .bind(row.network_allow_json) + .bind(row.lifecycle_preserve) + .bind(row.lifecycle_stop_on_terminal) + .bind(row.lifecycle_auto_stop) + .bind(row.labels_json) + .bind(row.env_json) + .execute(&mut **transaction) + .await?; + Ok(result.rows_affected()) +} + +async fn update_environment( + transaction: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + environment: &Environment, + expected: &EnvironmentRevision, ) -> Result<(), EnvironmentStoreError> { - let current = environments - .get(id) - .ok_or_else(|| EnvironmentStoreError::NotFound { id: id.clone() })?; - if ¤t.revision != expected { - return Err(EnvironmentStoreError::StaleRevision { - id: id.clone(), - expected: expected.clone(), - actual: current.revision.clone(), - }); + let row = EnvironmentSqlRow::from_environment(environment)?; + let result = sqlx::query(UPDATE_ENVIRONMENT_SQL) + .bind(row.revision) + .bind(row.provider) + .bind(row.cwd) + .bind(row.image_docker) + .bind(row.image_dockerfile_inline) + .bind(row.resources_cpu) + .bind(row.resources_memory) + .bind(row.resources_disk) + .bind(row.network_mode) + .bind(row.network_allow_json) + .bind(row.lifecycle_preserve) + .bind(row.lifecycle_stop_on_terminal) + .bind(row.lifecycle_auto_stop) + .bind(row.labels_json) + .bind(row.env_json) + .bind(row.id) + .bind(expected.as_str()) + .execute(&mut **transaction) + .await?; + if result.rows_affected() == 0 { + return Err(revision_mismatch_error(transaction, &environment.id, expected).await?); } Ok(()) } -/// Writes the built-in Docker default environment into `dir`, creating the -/// directory if needed. Existing files are left untouched, so this is -/// idempotent and never clobbers operator edits. Called by legacy installer -/// paths; the running server does not seed. -pub fn seed_environments(dir: &Path) -> Result<(), EnvironmentStoreError> { - seed_default_environment(dir, EnvironmentProvider::Docker) +const INSERT_ENVIRONMENT_IGNORE_CONFLICT_SQL: &str = r" +INSERT INTO environments ( + id, + revision, + provider, + cwd, + image_docker, + image_dockerfile_inline, + resources_cpu, + resources_memory, + resources_disk, + network_mode, + network_allow_json, + lifecycle_preserve, + lifecycle_stop_on_terminal, + lifecycle_auto_stop, + labels_json, + env_json +) +VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) +ON CONFLICT(id) DO NOTHING +"; + +const UPDATE_ENVIRONMENT_SQL: &str = r" +UPDATE environments SET + revision = ?, + provider = ?, + cwd = ?, + image_docker = ?, + image_dockerfile_inline = ?, + resources_cpu = ?, + resources_memory = ?, + resources_disk = ?, + network_mode = ?, + network_allow_json = ?, + lifecycle_preserve = ?, + lifecycle_stop_on_terminal = ?, + lifecycle_auto_stop = ?, + labels_json = ?, + env_json = ? +WHERE id = ? AND revision = ? +"; + +struct EnvironmentSqlRow { + id: String, + revision: String, + provider: String, + cwd: Option, + image_docker: Option, + image_dockerfile_inline: Option, + resources_cpu: Option, + resources_memory: Option, + resources_disk: Option, + network_mode: String, + network_allow_json: String, + lifecycle_preserve: bool, + lifecycle_stop_on_terminal: bool, + lifecycle_auto_stop: Option, + labels_json: String, + env_json: String, } -/// Writes the selected built-in `default` environment into `dir`, creating the -/// directory if needed. Existing files are left untouched, so this is -/// idempotent and never clobbers operator edits. Called by the installer; the -/// running server does not seed. -#[expect( - clippy::disallowed_methods, - clippy::disallowed_types, - reason = "Install-time environment seeding runs synchronously from the installer before the server starts." -)] -pub fn seed_default_environment( - dir: &Path, +impl EnvironmentSqlRow { + fn from_environment(environment: &Environment) -> Result { + let settings = &environment.settings; + let image_dockerfile_inline = match &settings.image.dockerfile { + Some(DockerfileSource::Inline(value)) => Some(value.clone()), + Some(DockerfileSource::Path { .. }) => { + return Err(EnvironmentValidationError::DockerfilePathUnsupported.into()); + } + None => None, + }; + Ok(Self { + id: environment.id.to_string(), + revision: environment.revision.to_string(), + provider: settings.provider.to_string(), + cwd: settings.cwd.clone(), + image_docker: settings.image.docker.clone(), + image_dockerfile_inline, + resources_cpu: settings.resources.cpu, + resources_memory: settings.resources.memory.map(|size| size.to_string()), + resources_disk: settings.resources.disk.map(|size| size.to_string()), + network_mode: settings.network.mode.to_string(), + network_allow_json: encode_json("network_allow_json", &settings.network.allow)?, + lifecycle_preserve: settings.lifecycle.preserve, + lifecycle_stop_on_terminal: settings.lifecycle.stop_on_terminal, + lifecycle_auto_stop: settings + .lifecycle + .auto_stop + .map(|duration| duration.to_string()), + labels_json: encode_string_map_json("labels_json", &settings.labels)?, + env_json: encode_env_json(&settings.env)?, + }) + } +} + +pub async fn seed_environments(pool: &DbPool) -> Result<(), EnvironmentStoreError> { + seed_default_environment(pool, EnvironmentProvider::Docker).await +} + +pub async fn seed_default_environment( + pool: &DbPool, provider: EnvironmentProvider, ) -> Result<(), EnvironmentStoreError> { - std::fs::create_dir_all(dir).map_err(|err| EnvironmentStoreError::io(dir, err))?; let content = match provider { EnvironmentProvider::Docker => DEFAULT_ENVIRONMENT_TOML, EnvironmentProvider::Daytona => DAYTONA_DEFAULT_ENVIRONMENT_TOML, EnvironmentProvider::Local => LOCAL_ENVIRONMENT_TOML, }; - let path = dir.join(format!("{DEFAULT_ENVIRONMENT_ID}.toml")); - match std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&path) - { - Ok(mut file) => { - use std::io::Write as _; - file.write_all(content.as_bytes()) - .map_err(|err| EnvironmentStoreError::io(&path, err))?; - file.sync_all() - .map_err(|err| EnvironmentStoreError::io(&path, err))?; - } - Err(err) if err.kind() == ErrorKind::AlreadyExists => {} - Err(err) => return Err(EnvironmentStoreError::io(path, err)), - } + let layer: EnvironmentLayer = toml::from_str(content).map_err(|source| { + EnvironmentStoreError::parse(PathBuf::from("built-in-default-environment.toml"), source) + })?; + let settings = + fabro_config::resolve_environment_layer(&layer, "environment").map_err(|errors| { + EnvironmentValidationError::InvalidSettings { + errors: errors.into_iter().map(|err| err.to_string()).collect(), + } + })?; + let environment = Environment::from_settings( + EnvironmentId::new(DEFAULT_ENVIRONMENT_ID).expect("default environment id is valid"), + &settings, + )?; + let mut transaction = pool.begin().await?; + insert_environment_ignoring_conflict(&mut transaction, &environment).await?; + transaction.commit().await?; + Ok(()) } -#[expect( - clippy::disallowed_methods, - reason = "Environment directory scan runs once at startup; std::fs avoids requiring a Tokio runtime for callers." -)] -fn load_environments( - dir: &Path, -) -> Result, EnvironmentStoreError> { - let entries = match std::fs::read_dir(dir) { - Ok(entries) => entries, - Err(err) if err.kind() == ErrorKind::NotFound => return Ok(HashMap::new()), - Err(err) => return Err(EnvironmentStoreError::io(dir, err)), +pub async fn import_legacy_directory_once( + pool: &DbPool, + source_dir: impl AsRef, +) -> Result, EnvironmentStoreError> { + let source_dir = source_dir.as_ref(); + let paths = legacy_environment_paths(source_dir).await?; + let Some(paths) = paths else { + return Ok(None); + }; + let existing_ids = existing_environment_ids(pool).await?; + let candidates = read_legacy_environment_directory(paths, &existing_ids).await?; + + let mut transaction = pool.begin().await?; + let mut imported_ids = Vec::new(); + let mut skipped_rows = candidates.skipped_rows; + for environment in &candidates.environments { + if !insert_environment_ignoring_conflict(&mut transaction, environment).await? { + skipped_rows += 1; + continue; + } + imported_ids.push(environment.id.to_string()); + } + transaction.commit().await?; + + let backup_path = rename_imported_legacy_directory(source_dir).await?; + let report = ImportReport { + source_path: source_dir.to_path_buf(), + backup_path, + imported_rows: row_count(imported_ids.len())?, + skipped_rows: row_count(skipped_rows)?, + environment_ids: imported_ids, }; - let mut environments = HashMap::new(); - for entry in entries { - let entry = entry.map_err(|err| EnvironmentStoreError::io(dir, err))?; + info!( + source_path = %source_dir.display(), + backup_path = %report.backup_path.display(), + imported_rows = report.imported_rows, + skipped_rows = report.skipped_rows, + environment_ids = ?report.environment_ids, + "imported legacy environments directory into sqlite" + ); + + Ok(Some(report)) +} + +struct LegacyCandidates { + environments: Vec, + skipped_rows: usize, +} + +struct LegacyEnvironmentPath { + id: EnvironmentId, + path: PathBuf, +} + +async fn legacy_environment_paths( + source_dir: &Path, +) -> Result>, EnvironmentStoreError> { + let mut entries = match fs::read_dir(source_dir).await { + Ok(entries) => entries, + Err(source) if source.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(source) => return Err(EnvironmentStoreError::io(source_dir, source)), + }; + + let mut paths = Vec::new(); + while let Some(entry) = entries + .next_entry() + .await + .map_err(|source| EnvironmentStoreError::io(source_dir, source))? + { let path = entry.path(); let file_type = entry .file_type() - .map_err(|err| EnvironmentStoreError::io(&path, err))?; - if !file_type.is_file() || !is_toml_file(&path) { - continue; + .await + .map_err(|source| EnvironmentStoreError::io(&path, source))?; + if file_type.is_file() && is_toml_file(&path) { + paths.push(LegacyEnvironmentPath { + id: id_from_path(&path)?, + path, + }); } - // `local` is reserved and synthesized in memory; never load a stale - // `local.toml` left behind by an earlier build that seeded it. - if id_from_path(&path).is_ok_and(|id| id.as_str() == RESERVED_LOCAL_ID) { - continue; - } - let environment = load_environment_file(&path)?; - environments.insert(environment.id.clone(), environment); } - Ok(environments) + paths.sort_by(|left, right| left.path.cmp(&right.path)); + Ok(Some(paths)) } -#[expect( - clippy::disallowed_methods, - reason = "Sync sibling of `load_environments`; only invoked from the synchronous startup load path." -)] -fn load_environment_file(path: &Path) -> Result { - let id = id_from_path(path)?; - let bytes = std::fs::read(path).map_err(|err| EnvironmentStoreError::io(path, err))?; - Environment::from_persisted_path(id, &bytes, path) +async fn read_legacy_environment_directory( + paths: Vec, + existing_ids: &HashSet, +) -> Result { + let mut environments = Vec::new(); + let mut skipped_rows = 0usize; + for LegacyEnvironmentPath { id, path } in paths { + if id.as_str() == RESERVED_LOCAL_ID || existing_ids.contains(&id) { + skipped_rows += 1; + continue; + } + let bytes = fs::read(&path) + .await + .map_err(|source| EnvironmentStoreError::io(&path, source))?; + let environment = Environment::from_legacy_path(id, &bytes, &path).await?; + environments.push(environment); + } + environments.sort_by(|left, right| left.id.cmp(&right.id)); + + Ok(LegacyCandidates { + environments, + skipped_rows, + }) +} + +async fn existing_environment_ids( + pool: &DbPool, +) -> Result, EnvironmentStoreError> { + let rows = sqlx::query_scalar::<_, String>("SELECT id FROM environments") + .fetch_all(pool) + .await?; + rows.into_iter() + .map(|id| EnvironmentId::new(id).map_err(EnvironmentStoreError::from)) + .collect() +} + +async fn rename_imported_legacy_directory( + source_dir: &Path, +) -> Result { + let backup_path = legacy_backup_path(source_dir, Utc::now()); + fs::rename(source_dir, &backup_path) + .await + .map_err(|source| EnvironmentStoreError::io(&backup_path, source))?; + Ok(backup_path) +} + +fn legacy_backup_path(source_dir: &Path, imported_at: DateTime) -> PathBuf { + let timestamp = imported_at.format("%Y%m%dT%H%M%S%fZ"); + let mut file_name = source_dir + .file_name() + .map_or_else(|| OsString::from("environments"), OsString::from); + file_name.push(format!(".imported-{timestamp}.bak")); + source_dir.with_file_name(file_name) } fn id_from_path(path: &Path) -> Result { @@ -377,577 +792,53 @@ fn is_toml_file(path: &Path) -> bool { .is_some_and(|extension| extension == "toml") } -async fn write_atomic(dir: &Path, path: &Path, bytes: &[u8]) -> Result<(), EnvironmentStoreError> { - fs::create_dir_all(dir) - .await - .map_err(|err| EnvironmentStoreError::io(dir, err))?; - let temp_path = temp_path_for(path); - let mut file = fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&temp_path) - .await - .map_err(|err| EnvironmentStoreError::io(&temp_path, err))?; - - if let Err(err) = file.write_all(bytes).await { - cleanup_temp(&temp_path).await; - return Err(EnvironmentStoreError::io(&temp_path, err)); - } - if let Err(err) = file.sync_all().await { - cleanup_temp(&temp_path).await; - return Err(EnvironmentStoreError::io(&temp_path, err)); - } - drop(file); - - if let Err(err) = fs::rename(&temp_path, path).await { - cleanup_temp(&temp_path).await; - return Err(EnvironmentStoreError::io(path, err)); - } - - Ok(()) +fn encode_json( + field: &'static str, + value: &T, +) -> Result { + serde_json::to_string(value) + .map_err(|source| EnvironmentStoreError::JsonEncode { field, source }) } -async fn write_new(dir: &Path, path: &Path, bytes: &[u8]) -> Result<(), EnvironmentStoreError> { - fs::create_dir_all(dir) - .await - .map_err(|err| EnvironmentStoreError::io(dir, err))?; - let mut file = fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(path) - .await - .map_err(|err| EnvironmentStoreError::io(path, err))?; - file.write_all(bytes) - .await - .map_err(|err| EnvironmentStoreError::io(path, err))?; - file.sync_all() - .await - .map_err(|err| EnvironmentStoreError::io(path, err))?; - Ok(()) +fn decode_json( + field: &'static str, + value: &str, +) -> Result { + serde_json::from_str(value) + .map_err(|source| EnvironmentStoreError::JsonDecode { field, source }) } -async fn cleanup_temp(path: &Path) { - let _ = fs::remove_file(path).await; +fn encode_string_map_json( + field: &'static str, + map: &HashMap, +) -> Result { + let ordered = map + .iter() + .map(|(key, value)| (key.as_str(), value.as_str())) + .collect::>(); + encode_json(field, &ordered) } -fn create_error_for(id: EnvironmentId, err: EnvironmentStoreError) -> EnvironmentStoreError { - match err { - EnvironmentStoreError::Io { source, .. } if source.kind() == ErrorKind::AlreadyExists => { - EnvironmentStoreError::AlreadyExists { id } - } - err => err, - } -} - -fn temp_path_for(path: &Path) -> PathBuf { - let parent = path.parent().unwrap_or_else(|| Path::new(".")); - let file_name = path - .file_name() - .and_then(|name| name.to_str()) - .unwrap_or("environment.toml"); - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_or(0, |duration| duration.as_nanos()); - parent.join(format!(".{file_name}.{}.{}.tmp", std::process::id(), now)) -} - -fn environment_path(dir: &Path, id: &EnvironmentId) -> PathBuf { - dir.join(format!("{id}.toml")) -} - -#[cfg(test)] #[expect( clippy::disallowed_methods, - reason = "Unit tests for sync startup helpers use sync std::fs to set up fixtures." + reason = "persisting InterpString source text; resolution happens at consumption time" )] -mod tests { - use std::collections::HashMap; - - use fabro_types::settings::InterpString; - use fabro_types::settings::run::{ - DockerfileSource, EnvironmentImageSettings, EnvironmentLifecycleSettings, - EnvironmentNetworkMode, EnvironmentNetworkSettings, EnvironmentProvider, - EnvironmentResourcesSettings, EnvironmentSettings, - }; - use tokio::fs; - - use crate::{ - EnvironmentDraft, EnvironmentId, EnvironmentRevision, EnvironmentStore, - EnvironmentStoreError, - }; - - fn settings(provider: EnvironmentProvider) -> EnvironmentSettings { - EnvironmentSettings { - provider, - cwd: None, - image: EnvironmentImageSettings::default(), - resources: EnvironmentResourcesSettings::default(), - network: EnvironmentNetworkSettings::default(), - lifecycle: EnvironmentLifecycleSettings::default(), - labels: HashMap::new(), - env: HashMap::new(), - } - } - - fn draft(id: &str, provider: EnvironmentProvider) -> EnvironmentDraft { - EnvironmentDraft { - id: EnvironmentId::new(id).unwrap(), - settings: settings(provider), - } - } - - #[test] - fn seeded_catalog_layer_contains_built_ins() { - let catalog = super::seeded_catalog_layer(); - let inner = catalog.into_inner(); - for id in ["default", "local"] { - assert!(inner.contains_key(id), "missing {id}"); - } - assert!(!inner.contains_key("docker")); - assert!(!inner.contains_key("daytona")); - } - - #[tokio::test] - async fn load_does_not_seed_built_ins() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - - // The server loads without seeding: an uninstalled instance has only the - // reserved in-memory `local` environment, and nothing is written to disk. - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - assert_eq!( - store - .list() - .iter() - .map(|environment| environment.id.as_str()) - .collect::>(), - vec!["local"] - ); - for id in ["default", "docker", "daytona"] { - assert!(!environment_dir.join(format!("{id}.toml")).exists()); - } - } - - #[tokio::test] - async fn seed_environments_writes_default_only_and_load_picks_it_up() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - - super::seed_environments(&environment_dir).unwrap(); - assert!(environment_dir.join("default.toml").exists()); - assert!(!environment_dir.join("docker.toml").exists()); - assert!(!environment_dir.join("daytona.toml").exists()); - // `local` is reserved and in-memory; it is never written to disk. - assert!(!environment_dir.join("local.toml").exists()); - - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - assert_eq!( - store - .list() - .iter() - .map(|environment| environment.id.as_str()) - .collect::>(), - vec!["default", "local"] - ); - } - - #[tokio::test] - async fn seed_environments_is_idempotent_and_preserves_edits() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - - super::seed_environments(&environment_dir).unwrap(); - // An operator edit to a seeded file must survive a re-seed. - fs::write( - environment_dir.join("default.toml"), - "provider = \"docker\"\n[resources]\ncpu = 7\n", - ) - .await - .unwrap(); - - super::seed_environments(&environment_dir).unwrap(); - - let store = EnvironmentStore::load(&environment_dir, false).unwrap(); - let default = store.get(&EnvironmentId::new("default").unwrap()).unwrap(); - assert_eq!(default.settings.resources.cpu, Some(7)); - } - - #[tokio::test] - async fn local_present_only_when_enabled() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - - let enabled = EnvironmentStore::load(&environment_dir, true).unwrap(); - assert!(enabled.get(&EnvironmentId::new("local").unwrap()).is_some()); - - let disabled = EnvironmentStore::load(&environment_dir, false).unwrap(); - assert!( - disabled - .get(&EnvironmentId::new("local").unwrap()) - .is_none() - ); - } - - #[tokio::test] - async fn on_disk_local_is_ignored_in_favor_of_synthetic() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - fs::create_dir_all(&environment_dir).await.unwrap(); - // A stale `local.toml` left by an earlier build that seeded it. - fs::write( - environment_dir.join("local.toml"), - "provider = \"local\"\n[resources]\ncpu = 99\n", - ) - .await - .unwrap(); - - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - let local = store.get(&EnvironmentId::new("local").unwrap()).unwrap(); - - // The synthetic local carries no resources; the stale file was ignored. - assert_eq!(local.settings.resources.cpu, None); - } - - #[tokio::test] - async fn local_mutations_are_reserved() { - let dir = tempfile::tempdir().unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - let local = EnvironmentId::new("local").unwrap(); - let revision = store.get(&local).unwrap().revision; - - let create_err = store - .create(draft("local", EnvironmentProvider::Local)) - .await - .unwrap_err(); - assert!(matches!(create_err, EnvironmentStoreError::Reserved { .. })); - - let replace_err = store - .replace(&local, &revision, settings(EnvironmentProvider::Local)) - .await - .unwrap_err(); - assert!(matches!( - replace_err, - EnvironmentStoreError::Reserved { .. } - )); - - let delete_err = store.delete(&local, &revision).await.unwrap_err(); - assert!(matches!(delete_err, EnvironmentStoreError::Reserved { .. })); - } - - #[tokio::test] - async fn listing_is_sorted() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - fs::create_dir_all(&environment_dir).await.unwrap(); - fs::write(environment_dir.join("z.toml"), r#"provider = "local""#) - .await - .unwrap(); - fs::write(environment_dir.join("a.toml"), r#"provider = "local""#) - .await - .unwrap(); - - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - - assert_eq!( - store - .list() - .iter() - .map(|environment| environment.id.as_str()) - .collect::>(), - vec!["a", "local", "z"] - ); - } - - #[test] - fn invalid_id_file_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - std::fs::create_dir_all(&environment_dir).unwrap(); - std::fs::write(environment_dir.join("Bad.toml"), r#"provider = "local""#).unwrap(); - - let err = EnvironmentStore::load(&environment_dir, true).unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::InvalidFilename { .. })); - } - - #[test] - fn invalid_provider_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - std::fs::create_dir_all(&environment_dir).unwrap(); - std::fs::write(environment_dir.join("bad.toml"), r#"provider = "bogus""#).unwrap(); - - let err = EnvironmentStore::load(&environment_dir, true).unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::Validation { .. })); - assert!(err.to_string().contains("unknown environment provider")); - } - - #[test] - fn invalid_network_mode_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - std::fs::create_dir_all(&environment_dir).unwrap(); - std::fs::write( - environment_dir.join("bad.toml"), - r#" -provider = "docker" - -[network] -mode = "cidr_allow_list" -"#, - ) - .unwrap(); - - let err = EnvironmentStore::load(&environment_dir, true).unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::Validation { .. })); - assert!( - err.to_string() - .contains("docker environments cannot enforce") - ); - } - - #[test] - fn missing_dockerfile_path_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - std::fs::create_dir_all(&environment_dir).unwrap(); - std::fs::write( - environment_dir.join("bad.toml"), - r#" -provider = "docker" - -[image.dockerfile] -path = "Dockerfile" -"#, - ) - .unwrap(); - - let err = EnvironmentStore::load(&environment_dir, true).unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::Validation { .. })); - assert!(err.to_string().contains("Dockerfile")); - } - - #[tokio::test] - async fn create_conflict_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - store - .create(draft("docker", EnvironmentProvider::Docker)) - .await - .unwrap(); - - let err = store - .create(draft("docker", EnvironmentProvider::Docker)) - .await - .unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::AlreadyExists { .. })); - } - - #[tokio::test] - async fn create_invalid_settings_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - let mut settings = settings(EnvironmentProvider::Local); - settings.network.mode = EnvironmentNetworkMode::Block; - - let err = store - .create(EnvironmentDraft { - id: EnvironmentId::new("invalid").unwrap(), - settings, - }) - .await - .unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::Validation { .. })); - assert!( - err.to_string() - .contains("local environments cannot enforce") - ); - } - - #[tokio::test] - async fn replace_stale_revision_is_rejected() { - let dir = tempfile::tempdir().unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - let current = store - .create(draft("docker", EnvironmentProvider::Docker)) - .await - .unwrap(); - let stale = EnvironmentRevision::from_bytes(b"stale"); - - let err = store - .replace(¤t.id, &stale, settings(EnvironmentProvider::Docker)) - .await - .unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::StaleRevision { .. })); - } - - #[tokio::test] - async fn default_is_deletable() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - super::seed_environments(&environment_dir).unwrap(); - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - let default = store.get(&EnvironmentId::new("default").unwrap()).unwrap(); - - // `default` is an ordinary environment: deleting it succeeds and removes - // the run fallback rather than being protected. - store.delete(&default.id, &default.revision).await.unwrap(); - - assert!(store.get(&default.id).is_none()); - assert!(!environment_dir.join("default.toml").exists()); - } - - #[tokio::test] - async fn delete_success_removes_file_and_memory_entry() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - let created = store - .create(draft("tmp", EnvironmentProvider::Local)) - .await - .unwrap(); - - store.delete(&created.id, &created.revision).await.unwrap(); - - assert!(store.get(&created.id).is_none()); - assert!(!environment_dir.join("tmp.toml").exists()); - } - - #[tokio::test] - async fn canonical_revision_changes_when_persisted_bytes_change() { - let dir = tempfile::tempdir().unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - let created = store - .create(draft("rev", EnvironmentProvider::Local)) - .await - .unwrap(); - let mut next = settings(EnvironmentProvider::Local); - next.env.insert( - "TOKEN".to_string(), - InterpString::parse("{{ env.TEST_TOKEN }}"), - ); - - let replaced = store - .replace(&created.id, &created.revision, next) - .await - .unwrap(); - - assert_ne!(created.revision, replaced.revision); - } - - #[tokio::test] - async fn create_persists_cwd_and_load_round_trips() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - let store = EnvironmentStore::load(&environment_dir, true).unwrap(); - let mut settings = settings(EnvironmentProvider::Local); - settings.cwd = Some("/srv/fabro/workspaces/team-a".to_string()); - - let created = store - .create(EnvironmentDraft { - id: EnvironmentId::new("host").unwrap(), - settings, - }) - .await - .unwrap(); - - assert_eq!( - created.settings.cwd.as_deref(), - Some("/srv/fabro/workspaces/team-a") - ); - let persisted = fs::read_to_string(environment_dir.join("host.toml")) - .await - .unwrap(); - assert!(persisted.contains("cwd = \"/srv/fabro/workspaces/team-a\"")); - - let loaded = EnvironmentStore::load(&environment_dir, true).unwrap(); - let host = loaded.get(&EnvironmentId::new("host").unwrap()).unwrap(); - assert_eq!( - host.settings.cwd.as_deref(), - Some("/srv/fabro/workspaces/team-a") - ); - } - - #[tokio::test] - async fn create_rejects_relative_cwd() { - let dir = tempfile::tempdir().unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - let mut settings = settings(EnvironmentProvider::Local); - settings.cwd = Some("relative/workspace".to_string()); - - let err = store - .create(EnvironmentDraft { - id: EnvironmentId::new("host").unwrap(), - settings, - }) - .await - .unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::Validation { .. })); - let message = err.to_string(); - assert!( - message.contains("environment.cwd") && message.contains("absolute path"), - "unexpected error: {message}" - ); - } - - #[test] - fn load_rejects_empty_cwd() { - let dir = tempfile::tempdir().unwrap(); - let environment_dir = dir.path().join("environments"); - std::fs::create_dir_all(&environment_dir).unwrap(); - std::fs::write( - environment_dir.join("bad.toml"), - r#" -provider = "local" -cwd = "" -"#, - ) - .unwrap(); - - let err = EnvironmentStore::load(&environment_dir, true).unwrap_err(); - - assert!(matches!(err, EnvironmentStoreError::Validation { .. })); - let message = err.to_string(); - assert!( - message.contains("environment.cwd") && message.contains("must not be empty"), - "unexpected error: {message}" - ); - } - - #[tokio::test] - async fn api_dockerfile_path_is_resolved_relative_to_settings_dir_and_persisted_inline() { - let dir = tempfile::tempdir().unwrap(); - fs::write(dir.path().join("Dockerfile"), "FROM alpine\n") - .await - .unwrap(); - let store = EnvironmentStore::load(dir.path().join("environments"), true).unwrap(); - let mut settings = settings(EnvironmentProvider::Docker); - settings.image.dockerfile = Some(DockerfileSource::Path { - path: "Dockerfile".to_string(), - }); - let draft = EnvironmentDraft { - id: EnvironmentId::new("with-dockerfile").unwrap(), - settings, - }; - - let created = store.create(draft).await.unwrap(); - let persisted = - fs::read_to_string(dir.path().join("environments").join("with-dockerfile.toml")) - .await - .unwrap(); - - assert_eq!( - created.settings.image.dockerfile, - Some(DockerfileSource::Inline("FROM alpine\n".to_string())) - ); - assert!(persisted.contains("FROM alpine")); - assert!(!persisted.contains("path =")); - } +fn encode_env_json(map: &HashMap) -> Result { + let ordered = map + .iter() + .map(|(key, value)| (key.as_str(), value.as_source())) + .collect::>(); + encode_json("env_json", &ordered) +} + +fn decode_env_json(value: &str) -> Result, EnvironmentStoreError> { + let decoded = decode_json::>("env_json", value)?; + Ok(decoded + .into_iter() + .map(|(key, value)| (key, InterpString::parse(&value))) + .collect()) +} + +fn row_count(count: usize) -> Result { + i64::try_from(count).map_err(|_| EnvironmentStoreError::RowCountOverflow { count }) } diff --git a/lib/crates/fabro-environment/tests/store.rs b/lib/crates/fabro-environment/tests/store.rs new file mode 100644 index 000000000..b63fd3425 --- /dev/null +++ b/lib/crates/fabro-environment/tests/store.rs @@ -0,0 +1,435 @@ +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +use fabro_environment::{ + EnvironmentDraft, EnvironmentId, EnvironmentStore, EnvironmentStoreError, + import_legacy_directory_once, seed_default_environment, seed_environments, +}; +use fabro_types::settings::InterpString; +use fabro_types::settings::run::{ + DockerfileSource, EnvironmentImageSettings, EnvironmentLifecycleSettings, + EnvironmentNetworkMode, EnvironmentNetworkSettings, EnvironmentProvider, + EnvironmentResourcesSettings, EnvironmentSettings, +}; +use tokio::fs; + +struct TestStore { + dir: tempfile::TempDir, + pool: fabro_db::DbPool, + store: EnvironmentStore, +} + +async fn test_store(local_enabled: bool) -> anyhow::Result { + let dir = tempfile::tempdir()?; + let database = fabro_db::Database::connect(dir.path().join("fabro.sqlite3")).await?; + database.migrate().await?; + let pool = database.clone_pool(); + let store = EnvironmentStore::load(pool.clone(), local_enabled).await?; + Ok(TestStore { dir, pool, store }) +} + +fn settings(provider: EnvironmentProvider) -> EnvironmentSettings { + EnvironmentSettings { + provider, + cwd: None, + image: EnvironmentImageSettings::default(), + resources: EnvironmentResourcesSettings::default(), + network: EnvironmentNetworkSettings::default(), + lifecycle: EnvironmentLifecycleSettings::default(), + labels: HashMap::new(), + env: HashMap::new(), + } +} + +fn draft(id: &str, provider: EnvironmentProvider) -> EnvironmentDraft { + EnvironmentDraft { + id: EnvironmentId::new(id).expect("test environment id should be valid"), + settings: settings(provider), + } +} + +#[tokio::test] +async fn new_store_lists_only_synthetic_local_when_enabled() -> anyhow::Result<()> { + let enabled = test_store(true).await?; + assert_eq!( + environment_ids(&enabled.store), + vec!["local"], + "local should be synthesized, not persisted" + ); + assert_eq!(sql_environment_count(&enabled.pool).await?, 0); + + let disabled = test_store(false).await?; + assert!(disabled.store.list().is_empty()); + + Ok(()) +} + +#[tokio::test] +async fn seed_default_is_idempotent_and_reopen_loads_sql_rows() -> anyhow::Result<()> { + let test = test_store(true).await?; + + seed_environments(&test.pool).await?; + seed_environments(&test.pool).await?; + + let reopened = EnvironmentStore::load(test.pool.clone(), true).await?; + assert_eq!(environment_ids(&reopened), vec!["default", "local"]); + assert_eq!(sql_environment_count(&test.pool).await?, 1); + + Ok(()) +} + +#[tokio::test] +async fn create_get_replace_delete_and_reload_round_trip_sql_rows() -> anyhow::Result<()> { + let test = test_store(true).await?; + let created = test + .store + .create(draft("custom", EnvironmentProvider::Docker)) + .await?; + + assert_eq!(created.id.as_str(), "custom"); + assert_eq!( + test.store + .get(&EnvironmentId::new("custom").expect("valid id")) + .expect("created environment should be cached") + .revision, + created.revision + ); + + let reopened = EnvironmentStore::load(test.pool.clone(), true).await?; + assert_eq!( + reopened + .get(&EnvironmentId::new("custom").expect("valid id")) + .expect("created environment should reload") + .revision, + created.revision + ); + + let mut replacement = settings(EnvironmentProvider::Local); + replacement.cwd = Some("/workspace/custom".to_string()); + replacement + .labels + .insert("tier".to_string(), "dev".to_string()); + let replaced = test + .store + .replace(&created.id, &created.revision, replacement) + .await?; + assert_ne!(replaced.revision, created.revision); + assert_eq!(replaced.settings.cwd.as_deref(), Some("/workspace/custom")); + + let stale = test + .store + .replace( + &created.id, + &created.revision, + settings(EnvironmentProvider::Docker), + ) + .await + .expect_err("stale revision should be rejected"); + assert!(matches!(stale, EnvironmentStoreError::StaleRevision { .. })); + + test.store.delete(&created.id, &replaced.revision).await?; + assert!(test.store.get(&created.id).is_none()); + assert!( + EnvironmentStore::load(test.pool.clone(), true) + .await? + .get(&created.id) + .is_none() + ); + + Ok(()) +} + +#[tokio::test] +async fn default_is_deletable() -> anyhow::Result<()> { + let test = test_store(true).await?; + seed_default_environment(&test.pool, EnvironmentProvider::Docker).await?; + let store = EnvironmentStore::load(test.pool.clone(), true).await?; + let default = store + .get(&EnvironmentId::new("default").expect("valid id")) + .expect("default should be seeded"); + + store.delete(&default.id, &default.revision).await?; + + assert!(store.get(&default.id).is_none()); + assert_eq!(sql_environment_count(&test.pool).await?, 0); + + Ok(()) +} + +#[tokio::test] +async fn maps_network_lifecycle_and_inline_dockerfile_round_trip() -> anyhow::Result<()> { + let test = test_store(true).await?; + let mut settings = settings(EnvironmentProvider::Daytona); + settings.image.dockerfile = Some(DockerfileSource::Inline("FROM alpine\n".to_string())); + settings.resources.cpu = Some(4); + settings.resources.memory = Some("8GB".parse()?); + settings.resources.disk = Some("20GB".parse()?); + settings.network.mode = EnvironmentNetworkMode::CidrAllowList; + settings.network.allow = vec!["10.0.0.0/8".to_string(), "192.168.0.0/16".to_string()]; + settings.lifecycle.preserve = true; + settings.lifecycle.stop_on_terminal = false; + settings.lifecycle.auto_stop = Some("30m".parse()?); + settings + .labels + .insert("team".to_string(), "platform".to_string()); + settings.env.insert( + "TOKEN".to_string(), + InterpString::parse("Bearer {{ secrets.API_TOKEN }}"), + ); + + let created = test + .store + .create(EnvironmentDraft { + id: EnvironmentId::new("rich").expect("valid id"), + settings, + }) + .await?; + let reloaded = EnvironmentStore::load(test.pool.clone(), true) + .await? + .get(&created.id) + .expect("rich environment should reload"); + + assert_eq!(reloaded.settings, created.settings); + + Ok(()) +} + +#[tokio::test] +async fn direct_create_rejects_dockerfile_path_without_reading_it() -> anyhow::Result<()> { + let test = test_store(true).await?; + let mut settings = settings(EnvironmentProvider::Docker); + settings.image.dockerfile = Some(DockerfileSource::Path { + path: test.dir.path().join("Dockerfile").display().to_string(), + }); + + let err = test + .store + .create(EnvironmentDraft { + id: EnvironmentId::new("path").expect("valid id"), + settings, + }) + .await + .expect_err("path Dockerfile should be rejected"); + + assert!(matches!(err, EnvironmentStoreError::Validation { .. })); + assert_eq!(sql_environment_count(&test.pool).await?, 0); + + Ok(()) +} + +#[tokio::test] +async fn legacy_import_missing_directory_is_noop() -> anyhow::Result<()> { + let test = test_store(true).await?; + let report = + import_legacy_directory_once(&test.pool, test.dir.path().join("environments")).await?; + + assert!(report.is_none()); + assert_eq!(sql_environment_count(&test.pool).await?, 0); + + Ok(()) +} + +#[tokio::test] +async fn legacy_import_imports_rows_renames_source_and_is_idempotent() -> anyhow::Result<()> { + let test = test_store(true).await?; + let environment_dir = test.dir.path().join("environments"); + fs::create_dir(&environment_dir).await?; + fs::write( + environment_dir.join("cloud.toml"), + r#" +provider = "docker" + +[resources] +cpu = 3 +"#, + ) + .await?; + fs::write( + environment_dir.join("local.toml"), + r#" +provider = "local" + +[resources] +cpu = 99 +"#, + ) + .await?; + + let report = import_legacy_directory_once(&test.pool, &environment_dir) + .await? + .expect("legacy directory should import"); + let second = import_legacy_directory_once(&test.pool, &environment_dir).await?; + + assert_eq!(report.imported_rows, 1); + assert_eq!(report.skipped_rows, 1); + assert_eq!(report.environment_ids, vec!["cloud"]); + assert!(second.is_none()); + assert!(!environment_dir.exists()); + assert!(report.backup_path.exists()); + + let store = EnvironmentStore::load(test.pool.clone(), true).await?; + assert_eq!(environment_ids(&store), vec!["cloud", "local"]); + assert_eq!( + store + .get(&EnvironmentId::new("cloud").expect("valid id")) + .expect("cloud should import") + .settings + .resources + .cpu, + Some(3) + ); + assert_eq!(sql_environment_count(&test.pool).await?, 1); + + Ok(()) +} + +#[tokio::test] +async fn legacy_import_keeps_existing_sql_row_and_inlines_dockerfile_path() -> anyhow::Result<()> { + let test = test_store(true).await?; + test.store + .create(draft("existing", EnvironmentProvider::Local)) + .await?; + let environment_dir = test.dir.path().join("environments"); + fs::create_dir(&environment_dir).await?; + fs::write(environment_dir.join("Dockerfile"), "FROM alpine\n").await?; + fs::write( + environment_dir.join("existing.toml"), + r#" +provider = "docker" + +[image.dockerfile] +path = "missing.Dockerfile" +"#, + ) + .await?; + fs::write( + environment_dir.join("with-dockerfile.toml"), + r#" +provider = "docker" + +[image.dockerfile] +path = "Dockerfile" +"#, + ) + .await?; + + let report = import_legacy_directory_once(&test.pool, &environment_dir) + .await? + .expect("legacy directory should import"); + + assert_eq!(report.imported_rows, 1); + assert_eq!(report.skipped_rows, 1); + assert_eq!(report.environment_ids, vec!["with-dockerfile"]); + + let store = EnvironmentStore::load(test.pool.clone(), true).await?; + assert_eq!( + store + .get(&EnvironmentId::new("existing").expect("valid id")) + .expect("existing row should win") + .settings + .provider, + EnvironmentProvider::Local + ); + assert_eq!( + store + .get(&EnvironmentId::new("with-dockerfile").expect("valid id")) + .expect("dockerfile row should import") + .settings + .image + .dockerfile, + Some(DockerfileSource::Inline("FROM alpine\n".to_string())) + ); + + Ok(()) +} + +#[tokio::test] +async fn legacy_import_invalid_input_leaves_source_directory_in_place() -> anyhow::Result<()> { + assert_invalid_legacy_import_leaves_source_directory( + "invalid filename", + "Bad.toml", + r#"provider = "local""#, + "invalid_filename", + ) + .await?; + assert_invalid_legacy_import_leaves_source_directory( + "invalid toml", + "broken.toml", + "provider = [", + "parse", + ) + .await?; + assert_invalid_legacy_import_leaves_source_directory( + "invalid settings", + "invalid-settings.toml", + r#"provider = "bogus""#, + "validation", + ) + .await?; + + Ok(()) +} + +async fn assert_invalid_legacy_import_leaves_source_directory( + case: &str, + file_name: &str, + content: &str, + expected_kind: &str, +) -> anyhow::Result<()> { + let test = test_store(true).await?; + let environment_dir = test.dir.path().join("environments"); + fs::create_dir(&environment_dir).await?; + fs::write(environment_dir.join(file_name), content).await?; + + let Err(err) = import_legacy_directory_once(&test.pool, &environment_dir).await else { + panic!("{case} should fail import"); + }; + + assert_eq!(err.kind(), expected_kind, "{case} error kind"); + assert!(environment_dir.exists(), "{case} source dir should remain"); + assert!( + legacy_backups(test.dir.path()).await?.is_empty(), + "{case} should not create a backup" + ); + assert_eq!( + sql_environment_count(&test.pool).await?, + 0, + "{case} should not import rows" + ); + + Ok(()) +} + +fn environment_ids(store: &EnvironmentStore) -> Vec { + store + .list() + .into_iter() + .map(|environment| environment.id.to_string()) + .collect() +} + +async fn sql_environment_count(pool: &fabro_db::DbPool) -> anyhow::Result { + Ok(sqlx::query_scalar("SELECT COUNT(*) FROM environments") + .fetch_one(pool) + .await?) +} + +async fn legacy_backups(dir: &Path) -> anyhow::Result> { + let mut entries = fs::read_dir(dir).await?; + let mut backups = Vec::new(); + while let Some(entry) = entries.next_entry().await? { + let path = entry.path(); + let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else { + continue; + }; + if file_name.starts_with("environments.imported-") + && path + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("bak")) + { + backups.push(path); + } + } + backups.sort(); + Ok(backups) +} diff --git a/lib/crates/fabro-install/Cargo.toml b/lib/crates/fabro-install/Cargo.toml index 608e5ef6e..c7be60f2e 100644 --- a/lib/crates/fabro-install/Cargo.toml +++ b/lib/crates/fabro-install/Cargo.toml @@ -15,6 +15,8 @@ base64.workspace = true ring = "0.17" toml.workspace = true fabro-config = { path = "../fabro-config" } +fabro-db = { path = "../fabro-db" } +fabro-environment.workspace = true fabro-static.workspace = true fabro-types = { path = "../fabro-types" } fabro-util = { path = "../fabro-util" } diff --git a/lib/crates/fabro-install/src/lib.rs b/lib/crates/fabro-install/src/lib.rs index 83a59a2ad..d26a0cf0e 100644 --- a/lib/crates/fabro-install/src/lib.rs +++ b/lib/crates/fabro-install/src/lib.rs @@ -8,6 +8,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; use fabro_config::{Storage, envfile}; use fabro_static::EnvVars; +use fabro_types::settings::run::EnvironmentProvider; use fabro_util::dev_token; use fabro_vault::{SecretType as VaultSecretType, Vault}; @@ -141,6 +142,25 @@ pub fn default_web_url() -> String { "http://127.0.0.1:32276".to_string() } +pub async fn seed_environments_in_storage(storage_dir: &Path) -> Result<()> { + seed_default_environment_in_storage(storage_dir, EnvironmentProvider::Docker).await +} + +pub async fn seed_default_environment_in_storage( + storage_dir: &Path, + provider: EnvironmentProvider, +) -> Result<()> { + let database = open_migrated_database(storage_dir).await?; + fabro_environment::seed_default_environment(database.pool(), provider).await?; + Ok(()) +} + +async fn open_migrated_database(storage_dir: &Path) -> Result { + let database = fabro_db::Database::connect(Storage::new(storage_dir).sqlite_path()).await?; + database.migrate().await?; + Ok(database) +} + pub fn prepare_dev_token_write_for_install(path: &Path) -> Result { if let Some(token) = dev_token::read_dev_token_for_install(path)? { return Ok(PreparedInstallDevToken { token, write: None }); diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index 91ed5bd6e..aae91785a 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -20,8 +20,9 @@ use fabro_install::{ GITHUB_APP_VAULT_KEYS, GITHUB_INSTALL_SECRET_KEYS, InstallListenConfig, InstallPersistencePlan, InstallSandboxSelection, OBJECT_STORE_ACCESS_KEY_ID_ENV, OBJECT_STORE_SECRET_ACCESS_KEY_ENV, PendingSettingsWrite, VaultSecretWrite, merge_server_settings, - prepare_dev_token_write_for_install, write_github_app_settings, write_object_store_settings, - write_sandbox_settings, write_token_settings, + prepare_dev_token_write_for_install, seed_default_environment_in_storage, + write_github_app_settings, write_object_store_settings, write_sandbox_settings, + write_token_settings, }; use fabro_llm::client::Client as LlmClient; use fabro_llm::generate::{GenerateParams, generate}; @@ -749,15 +750,6 @@ fn install_listen_config(bind: &Bind) -> InstallListenConfig { } } -/// The environments directory sits next to the active settings file, matching -/// the server's own `environment_dir_for_active_config` derivation. -fn install_environment_dir(config_path: &Path) -> PathBuf { - config_path - .parent() - .unwrap_or_else(|| Path::new(".")) - .join("environments") -} - async fn health() -> Response { Json(serde_json::json!({ "status": "ok", @@ -1751,14 +1743,15 @@ async fn post_install_finish( .into_response(); } - // Seed the default environment next to the settings file. The server does - // not seed on startup, so install is the only place the default is written; - // existing files are preserved, so re-running install never clobbers edits. - let environment_dir = install_environment_dir(state.config_path.as_ref()); - if let Err(err) = fabro_environment::seed_default_environment( - &environment_dir, + // Seed the default environment in SQLite. The server does not seed on + // startup, so install is the only place the default is written; existing + // rows are preserved, so re-running install never clobbers edits. + if let Err(err) = seed_default_environment_in_storage( + state.storage_dir.as_ref(), sandbox.to_environment_provider(), - ) { + ) + .await + { warn!(error = %err, "failed to seed default environment after install"); } diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index e7bd92275..d6d33f99c 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -691,6 +691,18 @@ where variables_path.display() ) })?; + let legacy_environment_dir = active_config_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("environments"); + fabro_environment::import_legacy_directory_once(database.pool(), &legacy_environment_dir) + .await + .with_context(|| { + format!( + "importing legacy environments directory {}", + legacy_environment_dir.display() + ) + })?; let db_pool = database.clone_pool(); let max_concurrent_runs = resolved_server_settings.scheduler.max_concurrent_runs; // In `--watch-web` mode the build watcher will populate `dist/` shortly diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 37d67bb9d..254dc9658 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -125,6 +125,7 @@ use tempfile::NamedTempFile; use tokio::fs; use tokio::io::{AsyncBufReadExt, AsyncRead, AsyncWriteExt, BufReader}; use tokio::process::Command; +use tokio::runtime::Builder as TokioRuntimeBuilder; use tokio::sync::broadcast::error::RecvError; use tokio::sync::{ Mutex as AsyncMutex, Notify, OwnedMutexGuard, RwLock as AsyncRwLock, Semaphore, broadcast, @@ -2292,13 +2293,6 @@ fn automation_dir_for_active_config(active_config_path: &std::path::Path) -> Pat .join("automations") } -fn environment_dir_for_active_config(active_config_path: &std::path::Path) -> PathBuf { - active_config_path - .parent() - .unwrap_or_else(|| std::path::Path::new(".")) - .join("environments") -} - fn mcp_server_dir_for_active_config(active_config_path: &std::path::Path) -> PathBuf { active_config_path .parent() @@ -2306,6 +2300,28 @@ fn mcp_server_dir_for_active_config(active_config_path: &std::path::Path) -> Pat .join("mcps") } +#[expect( + clippy::disallowed_methods, + reason = "synchronous app-state assembly may run inside an async runtime; a short-lived OS \ + thread avoids nested Tokio runtimes" +)] +fn load_environment_store_blocking( + pool: DbPool, + local_enabled: bool, +) -> anyhow::Result { + std::thread::spawn(move || { + let runtime = TokioRuntimeBuilder::new_current_thread() + .enable_all() + .build() + .context("build environment store runtime")?; + runtime + .block_on(EnvironmentStore::load(pool, local_enabled)) + .map_err(anyhow::Error::new) + }) + .join() + .expect("environment store load thread should not panic") +} + pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result> { let AppStateConfig { resolved_settings, @@ -2337,7 +2353,6 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result anyhow::Result for ApiError { Self::new(StatusCode::UNPROCESSABLE_ENTITY, source.to_string()) } EnvironmentStoreError::InvalidFilename { .. } + | EnvironmentStoreError::InvalidRevision { .. } | EnvironmentStoreError::Parse { .. } | EnvironmentStoreError::InvalidUtf8 { .. } | EnvironmentStoreError::Serialize { .. } + | EnvironmentStoreError::JsonEncode { .. } + | EnvironmentStoreError::JsonDecode { .. } + | EnvironmentStoreError::Db { .. } + | EnvironmentStoreError::RowCountOverflow { .. } | EnvironmentStoreError::Io { .. } => Self::new( StatusCode::INTERNAL_SERVER_ERROR, "environment store operation failed", diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 7220bc0d6..214bff225 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -83,6 +83,40 @@ fn manifest_run_defaults_from_toml(source: &str) -> fabro_config::RunLayer { .unwrap_or_default() } +fn test_environment_store( + default_provider: Option, + local_enabled: bool, +) -> (tempfile::TempDir, EnvironmentStore) { + let temp = tempfile::tempdir().expect("environment store tempdir should be created"); + let db_path = temp.path().join("fabro.sqlite3"); + let pool = std::thread::spawn(move || { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("environment store setup runtime should build"); + runtime.block_on(async move { + let database = fabro_db::Database::connect(db_path) + .await + .expect("test environment database should connect"); + database + .migrate() + .await + .expect("test environment database should migrate"); + if let Some(provider) = default_provider { + fabro_environment::seed_default_environment(database.pool(), provider) + .await + .expect("test default environment should seed"); + } + database.clone_pool() + }) + }) + .join() + .expect("environment store setup thread should not panic"); + let store = load_environment_store_blocking(pool, local_enabled) + .expect("test environment store should load"); + (temp, store) +} + fn server_settings_from_toml(source: &str) -> ServerSettings { ServerSettingsBuilder::from_toml(source).expect("server settings should resolve") } @@ -1251,12 +1285,8 @@ id = "missing" #[test] fn system_sandbox_provider_uses_manifest_defaults() { - let temp = tempfile::tempdir().unwrap(); - let environment_dir = temp.path().join("environments"); - fabro_environment::seed_default_environment(&environment_dir, EnvironmentProvider::Daytona) - .expect("seed built-in environments"); - let environment_store = - EnvironmentStore::load(&environment_dir, true).expect("environment store should load"); + let (temp, environment_store) = + test_environment_store(Some(EnvironmentProvider::Daytona), true); let mcp_server_store = McpServerStore::load(temp.path().join("mcps")).expect("mcp server store should load"); let source = r#" @@ -1276,9 +1306,7 @@ id = "default" #[test] fn system_sandbox_provider_defaults_when_manifest_run_settings_do_not_resolve() { - let temp = tempfile::tempdir().unwrap(); - let environment_store = EnvironmentStore::load(temp.path().join("environments"), true) - .expect("environment store should load"); + let (temp, environment_store) = test_environment_store(None, true); let mcp_server_store = McpServerStore::load(temp.path().join("mcps")).expect("mcp server store should load"); let source = r#" @@ -6012,11 +6040,6 @@ fn create_github_token_app_state_with_env_lookup_and_llm_catalog_settings( let vault_path = test_secret_store_path(); let server_env_path = vault_path.with_file_name("server.env"); let active_config_path = vault_path.with_file_name("settings.toml"); - let environment_dir = active_config_path - .parent() - .unwrap_or_else(|| std::path::Path::new(".")) - .join("environments"); - fabro_environment::seed_environments(&environment_dir).expect("test environments should seed"); let db_pool = test_db_pool_for_vault_path(&vault_path).expect("test db pool should build"); let config = AppStateConfig { resolved_settings: resolved_runtime_settings_for_tests( diff --git a/lib/crates/fabro-server/src/test_support.rs b/lib/crates/fabro-server/src/test_support.rs index a448ee90f..e7ad3722b 100644 --- a/lib/crates/fabro-server/src/test_support.rs +++ b/lib/crates/fabro-server/src/test_support.rs @@ -13,7 +13,7 @@ use axum::middleware::Next; use axum::response::Response; use axum::{Router, middleware}; use chrono::Duration as ChronoDuration; -use fabro_config::{RunLayer, ServerSettingsBuilder, envfile}; +use fabro_config::{RunLayer, ServerSettingsBuilder, Storage, envfile}; use fabro_db::DbPool; use fabro_interview::Interviewer; use fabro_model::catalog::{LlmCatalogSettings, ProviderCatalogSettings}; @@ -21,6 +21,7 @@ use fabro_sandbox::SandboxProviderRegistry; use fabro_static::EnvVars; use fabro_store::{ArtifactStore, Database}; use fabro_types::settings::ServerAuthMethod; +use fabro_types::settings::run::EnvironmentProvider; use fabro_types::{AuthMethod, IdpIdentity, ServerSettings}; use fabro_vault::{SecretType, Vault}; use fabro_workflow::handler::HandlerRegistry; @@ -63,43 +64,45 @@ methods = ["dev-token"] #[must_use] pub struct TestAppStateBuilder { - server_settings: ServerSettings, - manifest_run_defaults: RunLayer, - max_concurrent_runs: usize, - registry_factory_override: Option>, - sandbox_provider_registry: Option, - store_bundle: Option<(Arc, ArtifactStore)>, - vault_path: Option, - vault_entries: Vec<(String, String)>, - server_env_path: Option, - active_config_path: Option, - server_secret_env: HashMap, - env_lookup: EnvLookup, - llm_catalog_settings: LlmCatalogSettings, - automation_materializer: Option>, + server_settings: ServerSettings, + manifest_run_defaults: RunLayer, + max_concurrent_runs: usize, + registry_factory_override: Option>, + sandbox_provider_registry: Option, + store_bundle: Option<(Arc, ArtifactStore)>, + vault_path: Option, + vault_entries: Vec<(String, String)>, + server_env_path: Option, + active_config_path: Option, + server_secret_env: HashMap, + default_environment_provider: Option, + env_lookup: EnvLookup, + llm_catalog_settings: LlmCatalogSettings, + automation_materializer: Option>, #[cfg(test)] - worker_runtime: Option>, + worker_runtime: Option>, } impl Default for TestAppStateBuilder { fn default() -> Self { Self { - server_settings: default_test_server_settings(), - manifest_run_defaults: RunLayer::default(), - max_concurrent_runs: 5, - registry_factory_override: None, - sandbox_provider_registry: None, - store_bundle: None, - vault_path: None, - vault_entries: Vec::new(), - server_env_path: None, - active_config_path: None, - server_secret_env: HashMap::new(), - env_lookup: default_env_lookup(), - llm_catalog_settings: LlmCatalogSettings::default(), - automation_materializer: None, + server_settings: default_test_server_settings(), + manifest_run_defaults: RunLayer::default(), + max_concurrent_runs: 5, + registry_factory_override: None, + sandbox_provider_registry: None, + store_bundle: None, + vault_path: None, + vault_entries: Vec::new(), + server_env_path: None, + active_config_path: None, + server_secret_env: HashMap::new(), + default_environment_provider: Some(EnvironmentProvider::Docker), + env_lookup: default_env_lookup(), + llm_catalog_settings: LlmCatalogSettings::default(), + automation_materializer: None, #[cfg(test)] - worker_runtime: None, + worker_runtime: None, } } } @@ -186,6 +189,11 @@ impl TestAppStateBuilder { self } + pub fn default_environment_provider(mut self, provider: Option) -> Self { + self.default_environment_provider = provider; + self + } + pub fn store_bundle(mut self, store: Arc, artifact_store: ArtifactStore) -> Self { self.store_bundle = Some((store, artifact_store)); self @@ -240,16 +248,10 @@ impl TestAppStateBuilder { let active_config_path = self .active_config_path .unwrap_or_else(|| vault_path.with_file_name("settings.toml")); - // Production seeds environments at install time, not on startup. Tests - // exercise an installed instance, so seed the built-ins next to the - // settings file before `build_app_state` loads them. - let environment_dir = active_config_path - .parent() - .unwrap_or_else(|| std::path::Path::new(".")) - .join("environments"); - fabro_environment::seed_environments(&environment_dir) - .expect("test environments should seed"); - let db_pool = test_db_pool_for_vault_path(&vault_path)?; + let db_pool = test_db_pool_for_vault_path_with_default_environment( + &vault_path, + self.default_environment_provider, + )?; build_app_state(AppStateConfig { resolved_settings: resolved_runtime_settings_for_tests( self.server_settings, @@ -496,8 +498,22 @@ pub fn test_store_bundle() -> (Arc, ArtifactStore) { (store, artifact_store) } +#[cfg(test)] pub(crate) fn test_db_pool_for_vault_path(vault_path: &Path) -> anyhow::Result { - test_db_pool(sqlite_path_for_vault_path(vault_path)) + test_db_pool_for_vault_path_with_default_environment( + vault_path, + Some(EnvironmentProvider::Docker), + ) +} + +pub(crate) fn test_db_pool_for_vault_path_with_default_environment( + vault_path: &Path, + default_environment_provider: Option, +) -> anyhow::Result { + test_db_pool( + sqlite_path_for_vault_path(vault_path), + default_environment_provider, + ) } fn sqlite_path_for_vault_path(vault_path: &Path) -> PathBuf { @@ -508,11 +524,24 @@ fn sqlite_path_for_vault_path(vault_path: &Path) -> PathBuf { .join("fabro.sqlite3") } +pub async fn test_environment_from_storage_dir( + storage_dir: &Path, + id: &str, +) -> anyhow::Result> { + let database = fabro_db::Database::connect(Storage::new(storage_dir).sqlite_path()).await?; + let store = fabro_environment::EnvironmentStore::load(database.clone_pool(), false).await?; + let id = fabro_environment::EnvironmentId::new(id)?; + Ok(store.get(&id)) +} + #[expect( clippy::disallowed_methods, reason = "sync test builders may be called inside async tests; a short-lived OS thread avoids nested Tokio runtimes" )] -fn test_db_pool(path: PathBuf) -> anyhow::Result { +fn test_db_pool( + path: PathBuf, + default_environment_provider: Option, +) -> anyhow::Result { std::thread::spawn(move || { let runtime = TokioRuntimeBuilder::new_current_thread() .enable_all() @@ -520,6 +549,9 @@ fn test_db_pool(path: PathBuf) -> anyhow::Result { runtime.block_on(async move { let database = fabro_db::Database::connect(&path).await?; database.migrate().await?; + if let Some(provider) = default_environment_provider { + fabro_environment::seed_default_environment(database.pool(), provider).await?; + } Ok(database.clone_pool()) }) }) diff --git a/lib/crates/fabro-server/tests/it/api/environments.rs b/lib/crates/fabro-server/tests/it/api/environments.rs index a10beeaa3..c32ed6774 100644 --- a/lib/crates/fabro-server/tests/it/api/environments.rs +++ b/lib/crates/fabro-server/tests/it/api/environments.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use axum::body::Body; use axum::http::{Method, Request, StatusCode, header}; @@ -6,6 +6,7 @@ use fabro_config::{RunEnvironmentLayer, RunLayer}; use fabro_server::server::build_router; use fabro_server::test_support::{ TestAppStateBuilder, build_test_router, default_test_server_settings, test_auth_mode, + test_environment_from_storage_dir, }; use serde_json::{Value, json}; use tower::ServiceExt; @@ -48,8 +49,10 @@ fn environment_app() -> (axum::Router, tempfile::TempDir, PathBuf) { let temp_dir = tempfile::tempdir().expect("environment test tempdir should be created"); let active_config_path = temp_dir.path().join("settings.toml"); let environment_dir = temp_dir.path().join("environments"); + let vault_path = temp_dir.path().join("secrets.json"); let state = TestAppStateBuilder::new() .active_config_path(active_config_path) + .vault_path(vault_path) .build(); (build_test_router(state), temp_dir, environment_dir) } @@ -59,6 +62,7 @@ fn environment_app_with_default_environment( ) -> (axum::Router, tempfile::TempDir) { let temp_dir = tempfile::tempdir().expect("environment test tempdir should be created"); let active_config_path = temp_dir.path().join("settings.toml"); + let vault_path = temp_dir.path().join("secrets.json"); let manifest_run_defaults = RunLayer { environment: Some(RunEnvironmentLayer { id: Some(environment_id.to_string()), @@ -69,6 +73,7 @@ fn environment_app_with_default_environment( let state = TestAppStateBuilder::new() .runtime_settings(default_test_server_settings(), manifest_run_defaults) .active_config_path(active_config_path) + .vault_path(vault_path) .build(); (build_test_router(state), temp_dir) } @@ -133,11 +138,13 @@ fn revision_from(body: &Value) -> &str { .expect("environment response should include a revision") } -async fn persisted_environment_toml(environment_dir: &Path, id: &str) -> toml::Value { - let persisted = tokio::fs::read_to_string(environment_dir.join(format!("{id}.toml"))) +async fn persisted_environment( + temp_dir: &tempfile::TempDir, + id: &str, +) -> Option { + test_environment_from_storage_dir(temp_dir.path(), id) .await - .expect("persisted environment TOML should be readable"); - toml::from_str(&persisted).expect("persisted environment TOML should parse") + .expect("environment store should load from test storage") } async fn system_info(app: &axum::Router) -> Value { @@ -174,8 +181,8 @@ async fn list_environments_returns_seeded_catalog_sorted_by_id() { } #[tokio::test] -async fn create_environment_persists_sibling_toml_and_is_visible() { - let (app, _temp_dir, environment_dir) = environment_app(); +async fn create_environment_persists_to_sqlite_and_is_visible() { + let (app, temp_dir, environment_dir) = environment_app(); let mut body = environment_body("custom-env", "docker"); body["cwd"] = json!("/workspace/custom"); @@ -184,7 +191,7 @@ async fn create_environment_persists_sibling_toml_and_is_visible() { assert_eq!(created["id"], "custom-env"); assert_eq!(created["provider"], "docker"); assert_eq!(created["cwd"], "/workspace/custom"); - assert!(environment_dir.join("custom-env.toml").exists()); + assert!(!environment_dir.join("custom-env.toml").exists()); let retrieved = app .clone() @@ -214,17 +221,11 @@ async fn create_environment_persists_sibling_toml_and_is_visible() { .any(|environment| environment["id"] == "custom-env") ); - let persisted = persisted_environment_toml(&environment_dir, "custom-env").await; - assert_eq!( - persisted.get("provider").and_then(toml::Value::as_str), - Some("docker") - ); - assert_eq!( - persisted.get("cwd").and_then(toml::Value::as_str), - Some("/workspace/custom") - ); - assert!(persisted.get("id").is_none()); - assert!(persisted.get("revision").is_none()); + let persisted = persisted_environment(&temp_dir, "custom-env") + .await + .expect("custom environment should persist to SQLite"); + assert_eq!(persisted.settings.provider.to_string(), "docker"); + assert_eq!(persisted.settings.cwd.as_deref(), Some("/workspace/custom")); } #[tokio::test] @@ -256,8 +257,8 @@ async fn get_environment_returns_current_etag() { } #[tokio::test] -async fn replace_environment_updates_file_and_returns_new_etag() { - let (app, _temp_dir, environment_dir) = environment_app(); +async fn replace_environment_updates_sqlite_and_returns_new_etag() { + let (app, temp_dir, environment_dir) = environment_app(); let created = create_environment(&app, "replace-env", "docker").await; let revision = revision_from(&created); let mut replacement = environment_settings("local"); @@ -293,19 +294,15 @@ async fn replace_environment_updates_file_and_returns_new_etag() { assert_eq!(body["cwd"], "/srv/fabro/local"); assert_ne!(body["revision"], revision); assert_eq!(etag, format!("\"{}\"", revision_from(&body))); - let persisted = persisted_environment_toml(&environment_dir, "replace-env").await; + assert!(!environment_dir.join("replace-env.toml").exists()); + let persisted = persisted_environment(&temp_dir, "replace-env") + .await + .expect("replacement should persist to SQLite"); assert_eq!( - persisted - .get("labels") - .and_then(toml::Value::as_table) - .and_then(|labels| labels.get("tier")) - .and_then(toml::Value::as_str), + persisted.settings.labels.get("tier").map(String::as_str), Some("dev") ); - assert_eq!( - persisted.get("cwd").and_then(toml::Value::as_str), - Some("/srv/fabro/local") - ); + assert_eq!(persisted.settings.cwd.as_deref(), Some("/srv/fabro/local")); } #[tokio::test] @@ -540,7 +537,7 @@ async fn invalid_environment_settings_return_unprocessable_entity() { #[tokio::test] async fn relative_environment_cwd_over_rest_returns_unprocessable_entity() { - let (app, _temp_dir, environment_dir) = environment_app(); + let (app, temp_dir, environment_dir) = environment_app(); let mut body = environment_body("relative-cwd", "local"); body["cwd"] = json!("relative/workspace"); @@ -556,6 +553,11 @@ async fn relative_environment_cwd_over_rest_returns_unprocessable_entity() { .await; assert!(!environment_dir.join("relative-cwd.toml").exists()); + assert!( + persisted_environment(&temp_dir, "relative-cwd") + .await + .is_none() + ); let message = serde_json::to_string(&error).expect("error should serialize"); assert!( message.contains("environment.cwd") && message.contains("absolute path"), @@ -592,6 +594,7 @@ async fn dockerfile_path_over_rest_is_rejected_without_persisting_or_exposing_co .await; assert!(!environment_dir.join("path-env.toml").exists()); + assert!(persisted_environment(&temp_dir, "path-env").await.is_none()); assert!( !serde_json::to_string(&error) .expect("error body should serialize") @@ -613,7 +616,7 @@ async fn dockerfile_path_over_rest_is_rejected_without_persisting_or_exposing_co #[tokio::test] async fn delete_environment_removes_non_default_and_default_is_deletable() { - let (app, _temp_dir, environment_dir) = environment_app(); + let (app, temp_dir, environment_dir) = environment_app(); let created = create_environment(&app, "delete-env", "local").await; let revision = revision_from(&created); @@ -635,6 +638,11 @@ async fn delete_environment_removes_non_default_and_default_is_deletable() { .await; assert!(!environment_dir.join("delete-env.toml").exists()); + assert!( + persisted_environment(&temp_dir, "delete-env") + .await + .is_none() + ); let missing = app .clone() .oneshot(empty_request(Method::GET, "/environments/delete-env")) @@ -673,6 +681,7 @@ async fn delete_environment_removes_non_default_and_default_is_deletable() { .await; assert!(!environment_dir.join("default.toml").exists()); + assert!(persisted_environment(&temp_dir, "default").await.is_none()); let missing_default = app .oneshot(empty_request(Method::GET, "/environments/default")) .await diff --git a/lib/crates/fabro-server/tests/it/api/install.rs b/lib/crates/fabro-server/tests/it/api/install.rs index 7b54b13dc..f7945fa38 100644 --- a/lib/crates/fabro-server/tests/it/api/install.rs +++ b/lib/crates/fabro-server/tests/it/api/install.rs @@ -17,6 +17,7 @@ use fabro_model::ProviderId; use fabro_server::install::{ InstallAppState, InstallFinishHook, InstallFinishInfo, build_install_router, }; +use fabro_server::test_support::test_environment_from_storage_dir; use fabro_util::Home; use fabro_vault::Vault; use httpmock::Method::GET; @@ -54,6 +55,22 @@ fn assert_sandbox_provider_policy( assert_eq!(resolved.daytona.enabled, daytona_enabled); } +async fn seeded_default_environment( + temp_dir: &tempfile::TempDir, +) -> fabro_environment::Environment { + test_environment_from_storage_dir(temp_dir.path(), "default") + .await + .expect("install test environment store should load") + .expect("default environment should be seeded") +} + +fn assert_no_legacy_environment_dir(temp_dir: &tempfile::TempDir) { + assert!( + !temp_dir.path().join("environments").exists(), + "install should not write legacy environments/*.toml files" + ); +} + async fn mock_daytona_auth_probe(server: &MockServer) -> httpmock::Mock<'_> { server .mock_async(|when, then| { @@ -935,17 +952,13 @@ async fn token_install_finish_persists_settings_env_and_vault() { "settings.toml should not contain environment catalog entries" ); assert_sandbox_provider_policy(&settings, true, true, false); - let environment_dir = temp_dir.path().join("environments"); - let mut environment_files = std::fs::read_dir(&environment_dir) - .unwrap() - .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) - .collect::>(); - environment_files.sort(); - assert_eq!(environment_files, vec!["default.toml"]); - let default_environment = - std::fs::read_to_string(environment_dir.join("default.toml")).unwrap(); - assert!(default_environment.contains("provider = \"docker\"")); - assert!(default_environment.contains("docker = \"buildpack-deps:noble\"")); + assert_no_legacy_environment_dir(&temp_dir); + let default_environment = seeded_default_environment(&temp_dir).await; + assert_eq!(default_environment.settings.provider.to_string(), "docker"); + assert_eq!( + default_environment.settings.image.docker.as_deref(), + Some("buildpack-deps:noble") + ); let resolved = ServerSettingsBuilder::from_toml(&settings) .expect("settings should resolve") .server; @@ -2588,9 +2601,9 @@ async fn sandbox_switching_from_daytona_to_docker_drops_saved_key() { settings.contains("[run.environment]"), "settings.toml should select the default environment" ); - let default_environment = - std::fs::read_to_string(temp_dir.path().join("environments/default.toml")).unwrap(); - assert!(default_environment.contains("provider = \"docker\"")); + assert_no_legacy_environment_dir(&temp_dir); + let default_environment = seeded_default_environment(&temp_dir).await; + assert_eq!(default_environment.settings.provider.to_string(), "docker"); let vault = Vault::load(Storage::new(temp_dir.path()).secrets_path()).unwrap(); assert_eq!(vault.get("DAYTONA_API_KEY"), None); } @@ -2708,17 +2721,14 @@ async fn daytona_install_finish_writes_settings_and_vault_secret() { "settings.toml should not contain environment catalog entries" ); assert_sandbox_provider_policy(&settings, true, false, true); - let environment_dir = temp_dir.path().join("environments"); - let mut environment_files = std::fs::read_dir(&environment_dir) - .unwrap() - .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) - .collect::>(); - environment_files.sort(); - assert_eq!(environment_files, vec!["default.toml"]); - let default_environment = - std::fs::read_to_string(environment_dir.join("default.toml")).unwrap(); - assert!(default_environment.contains("provider = \"daytona\"")); - assert!(default_environment.contains("buildpack-deps:noble")); + assert_no_legacy_environment_dir(&temp_dir); + let default_environment = seeded_default_environment(&temp_dir).await; + assert_eq!(default_environment.settings.provider.to_string(), "daytona"); + assert!(matches!( + default_environment.settings.image.dockerfile.as_ref(), + Some(fabro_types::settings::run::DockerfileSource::Inline(content)) + if content.contains("buildpack-deps:noble") + )); let vault = Vault::load(Storage::new(temp_dir.path()).secrets_path()).unwrap(); assert_eq!(vault.get("DAYTONA_API_KEY"), Some(api_key)); diff --git a/lib/crates/fabro-server/tests/it/api/runs.rs b/lib/crates/fabro-server/tests/it/api/runs.rs index 73d0bc68a..9e28d86ae 100644 --- a/lib/crates/fabro-server/tests/it/api/runs.rs +++ b/lib/crates/fabro-server/tests/it/api/runs.rs @@ -73,13 +73,11 @@ enabled = false fn daytona_disabled_app() -> (axum::Router, tempfile::TempDir) { let temp_dir = tempfile::tempdir().expect("daytona disabled test tempdir should be created"); let active_config_path = temp_dir.path().join("settings.toml"); - let environment_dir = temp_dir.path().join("environments"); - fabro_environment::seed_default_environment(&environment_dir, EnvironmentProvider::Daytona) - .expect("daytona default environment should seed"); let settings = daytona_disabled_settings(); let state = fabro_server::test_support::TestAppStateBuilder::new() .runtime_settings(settings.server_settings, settings.manifest_run_defaults) .active_config_path(active_config_path) + .default_environment_provider(Some(EnvironmentProvider::Daytona)) .build(); ( fabro_server::test_support::build_test_router(state), diff --git a/lib/crates/fabro-test/Cargo.toml b/lib/crates/fabro-test/Cargo.toml index fde950442..eebfcf09c 100644 --- a/lib/crates/fabro-test/Cargo.toml +++ b/lib/crates/fabro-test/Cargo.toml @@ -13,10 +13,11 @@ doctest = false workspace = true [dependencies] +anyhow.workspace = true assert_cmd = "2" axum = { workspace = true } fabro-config = { path = "../fabro-config" } -fabro-environment.workspace = true +fabro-install = { path = "../fabro-install" } fabro-proc = { path = "../fabro-proc" } fabro-static.workspace = true fabro-types = { path = "../fabro-types" } diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 21c6d6a05..bfa267320 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -19,6 +19,7 @@ pub use fabro_static::EnvVars; use fabro_types::RunId; use regex::Regex; use serde_json::{Map, Value, json}; +use tokio::runtime::Builder as TokioRuntimeBuilder; use toml::Value as TomlValue; use toml::map::Map as TomlMap; @@ -642,7 +643,11 @@ fn settings_storage_dir(settings_path: &Path) -> Option { return None; } let value = toml::from_str::(&content).ok()?; - value + value.as_table().and_then(server_storage_root_from_table) +} + +fn server_storage_root_from_table(table: &TomlMap) -> Option { + table .get("server") .and_then(toml::Value::as_table) .and_then(|server| server.get("storage")) @@ -652,17 +657,35 @@ fn settings_storage_dir(settings_path: &Path) -> Option { .map(PathBuf::from) } +fn storage_dir_for_environment_seed( + table: &TomlMap, + fallback: &Path, +) -> PathBuf { + server_storage_root_from_table(table) + .filter(|path| path.is_absolute()) + .unwrap_or_else(|| fallback.to_path_buf()) +} + fn home_settings_path(home_dir: &Path) -> PathBuf { home_dir.join(".fabro/settings.toml") } -fn seed_settings_environments(settings_path: &Path) { - let environment_dir = settings_path - .parent() - .unwrap_or_else(|| Path::new(".")) - .join("environments"); - fabro_environment::seed_environments(&environment_dir) - .unwrap_or_else(|err| panic!("failed to seed {}: {err}", environment_dir.display())); +fn seed_storage_environments(storage_dir: &Path) { + let storage_dir = storage_dir.to_path_buf(); + let display_path = Storage::new(&storage_dir).sqlite_path(); + std::thread::spawn(move || { + let runtime = TokioRuntimeBuilder::new_current_thread() + .enable_all() + .build() + .expect("environment seed runtime should build"); + runtime + .block_on( + async move { fabro_install::seed_environments_in_storage(&storage_dir).await }, + ) + .unwrap_or_else(|err| panic!("failed to seed {}: {err}", display_path.display())); + }) + .join() + .expect("environment seed thread should not panic"); } fn write_settings_file(path: &Path, storage_dir: &Path, rest: &str) { @@ -675,7 +698,7 @@ fn write_settings_file(path: &Path, storage_dir: &Path, rest: &str) { ), ) .unwrap_or_else(|err| panic!("failed to write {}: {err}", path.display())); - seed_settings_environments(path); + seed_storage_environments(storage_dir); } fn write_test_server_dev_token(storage_dir: &Path) { @@ -711,7 +734,6 @@ fn write_settings_table(path: &Path, table: &TomlMap) { } std::fs::write(path, contents) .unwrap_or_else(|err| panic!("failed to write {}: {err}", path.display())); - seed_settings_environments(path); } fn server_target_from_table(table: &TomlMap) -> Option { @@ -816,11 +838,14 @@ fn sync_home_settings( ensure_parent_dir(settings_path); std::fs::write(settings_path, contents) .unwrap_or_else(|err| panic!("failed to write {}: {err}", settings_path.display())); - seed_settings_environments(settings_path); + let seed_storage_dir = storage_dir_for_environment_seed(&table, storage_dir); + seed_storage_environments(&seed_storage_dir); return; } write_settings_table(settings_path, &table); + let seed_storage_dir = storage_dir_for_environment_seed(&table, storage_dir); + seed_storage_environments(&seed_storage_dir); } fn has_explicit_server_auth_methods(table: &TomlMap) -> bool { @@ -870,7 +895,8 @@ fn ensure_home_server_auth_methods( }; if has_explicit_server_auth_methods(&table) { - seed_settings_environments(settings_path); + let seed_storage_dir = storage_dir_for_environment_seed(&table, storage_dir); + seed_storage_environments(&seed_storage_dir); return; } @@ -883,13 +909,7 @@ fn ensure_home_server_auth_methods( } fn has_explicit_storage_root(table: &TomlMap) -> bool { - table - .get("server") - .and_then(TomlValue::as_table) - .and_then(|server| server.get("storage")) - .and_then(TomlValue::as_table) - .and_then(|storage| storage.get("root")) - .is_some() + server_storage_root_from_table(table).is_some() } fn set_server_storage_root(table: &mut TomlMap, storage_dir: &Path) { @@ -969,7 +989,7 @@ fn ensure_server_running(fabro_bin: &Path, server: &ServerPaths, config_path: &P ensure_parent_dir(config_path); std::fs::create_dir_all(&server.storage_dir) .unwrap_or_else(|err| panic!("failed to create {}: {err}", server.storage_dir.display())); - seed_settings_environments(config_path); + seed_storage_environments(&server.storage_dir); write_test_server_dev_token(&server.storage_dir); ServerDaemon::remove(&server_runtime_directory(server)); let _ = std::fs::remove_file(&server.socket_path);