From b34f2b8207d7ed69eb637b5516a14779d52603ff Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 27 Aug 2026 15:02:50 -0400 Subject: [PATCH 1/2] Allow Daytona snapshots from Docker images --- docs/public/changelog/2026-08-27.mdx | 21 +++ docs/public/docs.json | 1 + docs/public/execution/environments.mdx | 6 +- docs/public/execution/run-configuration.mdx | 4 +- docs/public/integrations/daytona.mdx | 22 ++- .../fabro-server/src/server/handler/runs.rs | 3 +- lib/components/fabro-sandbox/src/config.rs | 1 + .../fabro-sandbox/src/daytona/mod.rs | 169 ++++++++++++++---- .../fabro-sandbox/src/from_environment.rs | 71 +++++--- .../tests/it/daytona_integration.rs | 1 + .../fabro-config/src/resolve/environment.rs | 8 +- .../fabro-config/src/tests/resolve_run.rs | 39 +++- 12 files changed, 263 insertions(+), 83 deletions(-) create mode 100644 docs/public/changelog/2026-08-27.mdx diff --git a/docs/public/changelog/2026-08-27.mdx b/docs/public/changelog/2026-08-27.mdx new file mode 100644 index 000000000..2a858eba3 --- /dev/null +++ b/docs/public/changelog/2026-08-27.mdx @@ -0,0 +1,21 @@ +--- +title: "Docker images for Daytona snapshots" +date: "2026-08-27" +--- + +Daytona environments now accept `image.docker`. Fabro creates or reuses a +Daytona snapshot from that image, so an existing image no longer needs a +wrapper Dockerfile. `image.dockerfile` remains available for custom builds. + +Set one image source per Daytona environment: + +```toml +[environments.cloud] +provider = "daytona" + +[environments.cloud.image] +docker = "python:3.11-slim" +``` + +Fabro rejects a Daytona environment that sets both `image.docker` and +`image.dockerfile`. diff --git a/docs/public/docs.json b/docs/public/docs.json index 4a02c15a2..f76a3ec34 100644 --- a/docs/public/docs.json +++ b/docs/public/docs.json @@ -309,6 +309,7 @@ "group": "August 2026", "icon": "clock-rotate-left", "pages": [ + "changelog/2026-08-27", "changelog/2026-08-26", "changelog/2026-08-25", "changelog/2026-08-23", diff --git a/docs/public/execution/environments.mdx b/docs/public/execution/environments.mdx index d592b2a76..8630723b5 100644 --- a/docs/public/execution/environments.mdx +++ b/docs/public/execution/environments.mdx @@ -212,7 +212,7 @@ Install seeds a `default` environment into SQLite. It is a normal persisted envi | Environment field | Local | Docker | Daytona | |---|---|---|---| -| `image.docker` | Ignored | Docker image | Error | +| `image.docker` | Ignored | Docker image | Snapshot base image; Fabro computes the snapshot name | | `image.dockerfile` | Ignored | Warning; ignored | Snapshot Dockerfile; Fabro computes the snapshot name | | `resources.cpu` | Warning; ignored | `cpu_quota = cpu * 100000` | Snapshot CPU | | `resources.memory` | Warning; ignored | Container memory limit | Snapshot memory | @@ -276,14 +276,14 @@ The image must provide `/bin/bash`; Fabro evaluates every sandbox command with i ## Daytona -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. +Daytona runs tools in a cloud sandbox. Set either `image.docker` to use an existing Docker image or `image.dockerfile` to build a custom image. Fabro computes a deterministic internal snapshot name from the selected image source, resource hints, a single-tenant scope, and the Daytona API key. If neither field is set, Fabro uses Daytona's built-in `daytona-medium` snapshot. ```toml title="workflow.toml" [environments.cloud] provider = "daytona" [environments.cloud.image] -dockerfile = { path = "Dockerfile" } +docker = "python:3.11-slim" [environments.cloud.resources] cpu = 4 diff --git a/docs/public/execution/run-configuration.mdx b/docs/public/execution/run-configuration.mdx index 0d99b89bc..1c848b28e 100644 --- a/docs/public/execution/run-configuration.mdx +++ b/docs/public/execution/run-configuration.mdx @@ -320,8 +320,8 @@ memory = "8GB" |---|---| | `run.environment.id` | Environment slug to select. Defaults to `default`. | | `environments..provider` | Required provider: `local`, `docker`, or `daytona`. | -| `image.docker` | Docker image. Daytona rejects this field. | -| `image.dockerfile` | Inline Dockerfile or `{ path = "Dockerfile" }`; Daytona uses it to create or reuse an internally named snapshot. | +| `image.docker` | Docker image. Docker runs it directly; Daytona uses it to create or reuse an internally named snapshot. | +| `image.dockerfile` | Inline Dockerfile or `{ path = "Dockerfile" }`; Daytona uses it to create or reuse an internally named snapshot. Do not set it with `image.docker`. | | `resources.cpu` / `memory` / `disk` | Best-effort resource hints. Unsupported provider fields warn and continue. | | `network.mode` | `allow_all`, `block`, or `cidr_allow_list`. Local cannot enforce blocked/CIDR networking; Docker cannot enforce CIDR allow-lists. | | `network.allow` | CIDRs for `cidr_allow_list`; entries are validated as CIDRs. | diff --git a/docs/public/integrations/daytona.mdx b/docs/public/integrations/daytona.mdx index 9f512039f..4a7b7a266 100644 --- a/docs/public/integrations/daytona.mdx +++ b/docs/public/integrations/daytona.mdx @@ -71,8 +71,9 @@ overwritten. [environments.cloud.image] -dockerfile = "FROM rust:1.85-slim-bookworm\nRUN apt-get update && apt-get install -y git ripgrep" -# Or keep the Dockerfile next to this TOML file: +docker = "rust:1.85-slim-bookworm" +# Or replace docker with an inline or path-based Dockerfile: +# dockerfile = "FROM rust:1.85-slim-bookworm\nRUN apt-get update && apt-get install -y git ripgrep" # dockerfile = { path = "./Dockerfile" } [environments.cloud.resources] @@ -110,7 +111,7 @@ Snapshots let you pre-build an environment image so each run starts with depende ```toml title="run.toml" [environments.cloud.image] -dockerfile = "FROM node:20-slim\nRUN apt-get update && apt-get install -y git" +docker = "node:20-slim" [environments.cloud.resources] cpu = 4 @@ -118,10 +119,19 @@ memory = 8 disk = 20 ``` -When a run starts with `image.dockerfile`, Fabro computes an internal snapshot name and looks up that snapshot in Daytona. If it doesn't exist, Fabro creates it automatically and polls until it reaches `Active` state (up to 10 minutes). `dockerfile` can be inline content or `{ path = "..." }`; paths are resolved relative to the TOML file that declares them and are bundled into run manifests. If the snapshot already exists, it's reused immediately. +Set either `image.docker` or `image.dockerfile`. `image.docker` can name any image that Daytona can pull, so a Dockerfile is not required. Use `image.dockerfile` when the image needs extra packages or other build steps: + +```toml title="run.toml" +[environments.cloud.image] +dockerfile = "FROM node:20-slim\nRUN apt-get update && apt-get install -y git" +``` + +Fabro computes an internal snapshot name and looks up that snapshot in Daytona. If it does not exist, Fabro creates it automatically and polls until it reaches `Active` state for up to 30 minutes. A Dockerfile can be inline content or `{ path = "..." }`; paths are resolved relative to the TOML file that declares them and are bundled into run manifests. If the snapshot already exists, Fabro reuses it immediately. + +The exact `image.docker` value is part of the snapshot identity. Prefer a digest such as `registry.example.com/team/image@sha256:...` when the image must be reproducible. If a mutable tag moves without its text changing, Fabro continues to reuse the existing snapshot. -If no Dockerfile is configured, sandboxes are created from the `daytona-medium` snapshot which includes standard dev tools (git, etc.). To force a new custom snapshot, change the Dockerfile text, for example by adding a comment. +If neither image source is configured, sandboxes are created from the `daytona-medium` snapshot, which includes standard dev tools such as Git. To force a new Dockerfile snapshot, change the Dockerfile text, for example by adding a comment. ## Private repositories @@ -227,7 +237,7 @@ If doctor reports missing scopes, regenerate the Daytona key with `write:snapsho ### Custom snapshot did not roll -Custom Daytona snapshot names are computed from the Dockerfile, resource hints, tenant scope, and Daytona API key. To force a new custom snapshot, change `image.dockerfile` text under the selected `[environments..image]`. +Custom Daytona snapshot names are computed from the image reference or Dockerfile, resource hints, tenant scope, and Daytona API key. For `image.docker`, use an immutable digest and update it when the image changes. For `image.dockerfile`, change the Dockerfile text under the selected `[environments..image]`. ### "Timed out waiting for snapshot to become active" diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index f9bf551a4..ad16f8ea8 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -1087,9 +1087,8 @@ async fn validate_intent_environment( let provider = run_manifest::effective_sandbox_provider(&settings.run); let image = &settings.run.environment.image; let image_incompatible = match provider { - SandboxProviderKind::Local => false, SandboxProviderKind::Docker => image.docker.is_none() && image.dockerfile.is_some(), - SandboxProviderKind::Daytona => image.docker.is_some(), + SandboxProviderKind::Local | SandboxProviderKind::Daytona => false, }; let (target_incompatible, detail) = match target { RunTarget::Git(_) => ( diff --git a/lib/components/fabro-sandbox/src/config.rs b/lib/components/fabro-sandbox/src/config.rs index 208278d18..d6cd1b179 100644 --- a/lib/components/fabro-sandbox/src/config.rs +++ b/lib/components/fabro-sandbox/src/config.rs @@ -122,5 +122,6 @@ pub struct DaytonaSnapshotSettings { pub cpu: Option, pub memory: Option, pub disk: Option, + pub image: Option, pub dockerfile: Option, } diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index b3cbea53d..7a9e4efd4 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -44,7 +44,7 @@ use crate::{ /// Remediation shown when a Daytona sandbox has no usable Bash. const DAYTONA_BASH_REMEDIATION: &str = "Daytona sandboxes require /bin/bash for every command, with no `sh` fallback. Use the \ - built-in Daytona snapshot, or a custom snapshot whose Dockerfile installs bash."; + built-in Daytona snapshot, or a custom snapshot whose image provides bash."; /// Remediation shown when the session transport reaches Bash but never /// completes. @@ -162,6 +162,18 @@ pub mod snapshot_identity { entrypoint: Option<&'static str>, } + #[derive(Serialize)] + struct ImageSnapshotManifest<'a> { + identity_version: u8, + provider: &'static str, + tenant: &'static str, + image: &'a str, + cpu: Option, + memory_gb: Option, + disk_gb: Option, + entrypoint: Option<&'static str>, + } + pub fn snapshot_name(api_key: &str, config: &DaytonaSnapshotConfig) -> crate::Result { let manifest = canonical_manifest(config)?; let mut mac = HmacSha256::new_from_slice(api_key.as_bytes()) @@ -174,16 +186,36 @@ pub mod snapshot_identity { } fn canonical_manifest(config: &DaytonaSnapshotConfig) -> crate::Result> { - let dockerfile = match &config.dockerfile { - Some(DockerfileSource::Inline(text)) => text.as_str(), - Some(DockerfileSource::Path { .. }) => { + let dockerfile = match (&config.image, &config.dockerfile) { + (Some(image), None) => { + return serde_json::to_vec(&ImageSnapshotManifest { + identity_version: IDENTITY_VERSION, + provider: PROVIDER, + tenant: TENANT, + image, + cpu: config.cpu, + memory_gb: config.memory, + disk_gb: config.disk, + entrypoint: None, + }) + .map_err(|err| { + crate::Error::context("Failed to serialize Daytona snapshot identity", err) + }); + } + (Some(_), Some(_)) => { return Err(crate::Error::message( - "Daytona snapshot dockerfile path should have been resolved to inline content before sandbox creation", + "Daytona custom snapshots accept either image.docker or image.dockerfile, not both", )); } - None => { + (None, None) => { return Err(crate::Error::message( - "Daytona custom snapshots require image.dockerfile", + "Daytona custom snapshots require image.docker or image.dockerfile", + )); + } + (None, Some(DockerfileSource::Inline(text))) => text.as_str(), + (None, Some(DockerfileSource::Path { .. })) => { + return Err(crate::Error::message( + "Daytona snapshot dockerfile path should have been resolved to inline content before sandbox creation", )); } }; @@ -204,6 +236,45 @@ pub mod snapshot_identity { } } +fn create_snapshot_params( + name: &str, + config: &DaytonaSnapshotConfig, +) -> crate::Result { + let image = match (&config.image, &config.dockerfile) { + (Some(image), None) => daytona_sdk::ImageSource::Name(image.clone()), + (None, Some(DockerfileSource::Inline(dockerfile))) => { + daytona_sdk::ImageSource::Custom(daytona_sdk::DockerImage::from_dockerfile(dockerfile)) + } + (None, Some(DockerfileSource::Path { .. })) => { + return Err(crate::Error::message(format!( + "Snapshot '{name}': dockerfile path should have been resolved to inline content before sandbox creation" + ))); + } + (Some(_), Some(_)) => { + return Err(crate::Error::message(format!( + "Snapshot '{name}': image.docker and image.dockerfile cannot both be configured" + ))); + } + (None, None) => { + return Err(crate::Error::message(format!( + "Snapshot '{name}' does not exist and no image or dockerfile was provided to create it" + ))); + } + }; + + Ok(daytona_sdk::CreateSnapshotParams { + name: name.to_string(), + image, + resources: Some(daytona_sdk::Resources { + cpu: config.cpu, + memory: config.memory, + disk: config.disk, + ..Default::default() + }), + entrypoint: None, + }) +} + #[derive(Debug)] pub struct DaytonaKeyCheck { pub key_name: String, @@ -1101,9 +1172,9 @@ impl DaytonaSandbox { /// Ensure the named snapshot exists and is active. /// - /// If the snapshot doesn't exist and a dockerfile is provided, creates it - /// and polls until it reaches `Active` state. Returns an error if the - /// snapshot is in a terminal failure state. + /// If the snapshot doesn't exist and an image source is provided, creates + /// it and polls until it reaches `Active` state. Returns an error if + /// the snapshot is in a terminal failure state. async fn ensure_snapshot( &self, name: &str, @@ -1131,37 +1202,11 @@ impl DaytonaSandbox { } } Err(daytona_sdk::DaytonaError::NotFound { .. }) => { - let dockerfile = match &snap_cfg.dockerfile { - Some(DockerfileSource::Inline(s)) => s.as_str(), - Some(DockerfileSource::Path { .. }) => { - return Err(crate::Error::message(format!( - "Snapshot '{name}': dockerfile path should have been resolved to inline content before sandbox creation" - ))); - } - None => { - return Err(crate::Error::message(format!( - "Snapshot '{name}' does not exist and no dockerfile provided to create it" - ))); - } - }; - self.emit(SandboxEvent::SnapshotCreating { name: name.to_string(), }); - let params = daytona_sdk::CreateSnapshotParams { - name: name.to_string(), - image: daytona_sdk::ImageSource::Custom( - daytona_sdk::DockerImage::from_dockerfile(dockerfile), - ), - resources: Some(daytona_sdk::Resources { - cpu: snap_cfg.cpu, - memory: snap_cfg.memory, - disk: snap_cfg.disk, - ..Default::default() - }), - entrypoint: None, - }; + let params = create_snapshot_params(name, snap_cfg)?; self.client.snapshot.create(¶ms).await.map_err(|e| { crate::Error::context(format!("Failed to create snapshot '{name}'"), e) })?; @@ -1470,7 +1515,7 @@ impl Sandbox for DaytonaSandbox { .config .snapshot .as_ref() - .filter(|snapshot| snapshot.dockerfile.is_some()) + .filter(|snapshot| snapshot.image.is_some() || snapshot.dockerfile.is_some()) { let api_key = self.api_key.as_deref().ok_or_else(|| { self.fail_init( @@ -3653,6 +3698,7 @@ mod tests { cpu: Some(2), memory: Some(4), disk: Some(10), + image: None, dockerfile: Some(DockerfileSource::Inline( "FROM ubuntu:24.04\nRUN apt-get update".to_string(), )), @@ -3663,6 +3709,7 @@ mod tests { let rotated_key = snapshot_identity::snapshot_name("dtn_rotated", &config).unwrap(); assert_eq!(first, second); + assert_eq!(first, "fabro-e607185f-c7ab-88c9-bf9d-d70addba9298"); assert_ne!(first, rotated_key); let uuid = first .strip_prefix("fabro-") @@ -3678,6 +3725,7 @@ mod tests { cpu: Some(2), memory: Some(4), disk: Some(10), + image: None, dockerfile: Some(DockerfileSource::Inline("FROM ubuntu:24.04".to_string())), }; let base_name = snapshot_identity::snapshot_name("dtn_secret", &base).unwrap(); @@ -3715,6 +3763,7 @@ mod tests { cpu: None, memory: None, disk: None, + image: None, dockerfile: Some(DockerfileSource::Inline( "FROM private.example.com/secret-image\nRUN echo raw-secret".to_string(), )), @@ -3728,6 +3777,49 @@ mod tests { assert!(!name.contains("dtn_super_secret_key")); } + #[test] + fn computed_snapshot_identity_changes_for_image_reference() { + let config = DaytonaSnapshotConfig { + cpu: Some(2), + memory: Some(4), + disk: Some(10), + image: Some("ubuntu:24.04".to_string()), + dockerfile: None, + }; + let first = snapshot_identity::snapshot_name("dtn_secret", &config).unwrap(); + let changed = snapshot_identity::snapshot_name("dtn_secret", &DaytonaSnapshotConfig { + image: Some("ubuntu:24.10".to_string()), + ..config + }) + .unwrap(); + + assert_ne!(first, changed); + assert!(!first.contains("ubuntu")); + } + + #[test] + fn snapshot_creation_uses_named_image_source() { + let config = DaytonaSnapshotConfig { + cpu: Some(2), + memory: Some(4), + disk: Some(10), + image: Some("ubuntu:24.04".to_string()), + dockerfile: None, + }; + + let params = create_snapshot_params("fabro-test", &config).unwrap(); + + assert_eq!(params.name, "fabro-test"); + assert!(matches!( + params.image, + daytona_sdk::ImageSource::Name(ref image) if image == "ubuntu:24.04" + )); + let resources = params.resources.expect("resources should be configured"); + assert_eq!(resources.cpu, Some(2)); + assert_eq!(resources.memory, Some(4)); + assert_eq!(resources.disk, Some(10)); + } + #[tokio::test] async fn ensure_snapshot_uses_computed_snapshot_name_for_daytona_api_calls() { let api_key = "dtn_secret"; @@ -3735,6 +3827,7 @@ mod tests { cpu: Some(2), memory: Some(4), disk: Some(10), + image: None, dockerfile: Some(DockerfileSource::Inline("FROM ubuntu:24.04".to_string())), }; let computed_name = snapshot_identity::snapshot_name(api_key, &snapshot).unwrap(); diff --git a/lib/components/fabro-sandbox/src/from_environment.rs b/lib/components/fabro-sandbox/src/from_environment.rs index 825fca580..b6178e4eb 100644 --- a/lib/components/fabro-sandbox/src/from_environment.rs +++ b/lib/components/fabro-sandbox/src/from_environment.rs @@ -28,42 +28,48 @@ pub fn daytona_config_from_environment( settings: &RunEnvironmentSettings, clone: &RunCloneSettings, ) -> DaytonaConfig { + let dockerfile = settings + .image + .dockerfile + .as_ref() + .map(|dockerfile| match dockerfile { + ResolvedDockerfileSource::Inline(text) => SandboxDockerfileSource::Inline(text.clone()), + ResolvedDockerfileSource::Path { path } => { + SandboxDockerfileSource::Path { path: path.clone() } + } + }); + let snapshot = (settings.image.docker.is_some() || dockerfile.is_some()).then(|| { + DaytonaSnapshotSettings { + cpu: settings.resources.cpu, + memory: settings + .resources + .memory + .map(|size| size_to_gb_i32(size.as_bytes())), + disk: settings + .resources + .disk + .map(|size| size_to_gb_i32(size.as_bytes())), + image: settings.image.docker.clone(), + dockerfile, + } + }); + DaytonaConfig { auto_stop_interval: settings .lifecycle .auto_stop .map(|duration| duration_to_minutes_i32(duration.as_std())), - labels: (!settings.labels.is_empty()).then(|| settings.labels.clone()), - snapshot: settings.image.dockerfile.as_ref().map(|dockerfile| { - DaytonaSnapshotSettings { - cpu: settings.resources.cpu, - memory: settings - .resources - .memory - .map(|size| size_to_gb_i32(size.as_bytes())), - disk: settings - .resources - .disk - .map(|size| size_to_gb_i32(size.as_bytes())), - dockerfile: Some(match dockerfile { - ResolvedDockerfileSource::Inline(text) => { - SandboxDockerfileSource::Inline(text.clone()) - } - ResolvedDockerfileSource::Path { path } => { - SandboxDockerfileSource::Path { path: path.clone() } - } - }), - } - }), - network: Some(match settings.network.mode { + labels: (!settings.labels.is_empty()).then(|| settings.labels.clone()), + snapshot, + network: Some(match settings.network.mode { EnvironmentNetworkMode::Block => DaytonaNetwork::Block, EnvironmentNetworkMode::AllowAll => DaytonaNetwork::AllowAll, EnvironmentNetworkMode::CidrAllowList => { DaytonaNetwork::AllowList(settings.network.allow.clone()) } }), - clone_depth: clone.depth_limit(), - skip_clone: !clone.enabled, + clone_depth: clone.depth_limit(), + skip_clone: !clone.enabled, } } @@ -243,4 +249,19 @@ mod tests { ); assert!(!missing.exists()); } + + #[cfg(feature = "daytona")] + #[test] + fn daytona_config_maps_docker_image_to_snapshot() { + let mut settings = run_environment(EnvironmentProvider::Daytona); + settings.image.docker = Some("ubuntu:24.04".to_string()); + settings.resources.cpu = Some(2); + + let config = daytona_config_from_environment(&settings, &RunCloneSettings::default()); + let snapshot = config.snapshot.expect("image should configure a snapshot"); + + assert_eq!(snapshot.image.as_deref(), Some("ubuntu:24.04")); + assert!(snapshot.dockerfile.is_none()); + assert_eq!(snapshot.cpu, Some(2)); + } } diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index d183e74c0..0fd36836b 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -406,6 +406,7 @@ async fn daytona_snapshot_sandbox() { cpu: Some(2), memory: Some(4), disk: Some(10), + image: None, dockerfile: Some(fabro_sandbox::daytona::DockerfileSource::Inline( "FROM ubuntu:22.04\nRUN apt-get update && apt-get install -y ripgrep".to_string(), )), diff --git a/lib/foundation/fabro-config/src/resolve/environment.rs b/lib/foundation/fabro-config/src/resolve/environment.rs index 15ccb4bd2..cf13eade3 100644 --- a/lib/foundation/fabro-config/src/resolve/environment.rs +++ b/lib/foundation/fabro-config/src/resolve/environment.rs @@ -206,10 +206,14 @@ fn validate_daytona_image_settings( path: &str, errors: &mut Vec, ) { - if environment.provider == EnvironmentProvider::Daytona && environment.image.docker.is_some() { + if environment.provider == EnvironmentProvider::Daytona + && environment.image.docker.is_some() + && environment.image.dockerfile.is_some() + { errors.push(ResolveError::Invalid { path: format!("{path}.image"), - reason: "daytona environments do not support image.docker; use image.dockerfile for custom snapshots".to_string(), + reason: "daytona environments accept either image.docker or image.dockerfile, not both" + .to_string(), }); } } diff --git a/lib/foundation/fabro-config/src/tests/resolve_run.rs b/lib/foundation/fabro-config/src/tests/resolve_run.rs index bc6fe8bc4..b8c5a9823 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_run.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_run.rs @@ -658,8 +658,8 @@ dockerfile = { path = "Dockerfile" } } #[test] -fn daytona_image_docker_errors() { - let err = workflow_settings_from_toml_with_catalog( +fn daytona_image_docker_resolves() { + let settings = workflow_settings_from_toml_with_catalog( r#" _version = 1 @@ -674,12 +674,41 @@ provider = "daytona" docker = "ubuntu:24.04" "#, ) - .expect_err("daytona should reject docker image selection"); + .expect("daytona should accept docker image selection") + .run; + + assert_eq!(settings.environment.provider, EnvironmentProvider::Daytona); + assert_eq!( + settings.environment.image.docker.as_deref(), + Some("ubuntu:24.04") + ); + assert!(settings.environment.image.dockerfile.is_none()); +} + +#[test] +fn daytona_rejects_docker_image_and_dockerfile_together() { + let err = workflow_settings_from_toml_with_catalog( + r#" +_version = 1 + +[run.environment] +id = "cloud" +"#, + r#" +[environments.cloud] +provider = "daytona" + +[environments.cloud.image] +docker = "ubuntu:24.04" +dockerfile = "FROM ubuntu:24.04" +"#, + ) + .expect_err("daytona should reject two snapshot sources"); let message = err.to_string(); assert!( - message.contains("image.docker") && message.contains("daytona"), - "expected daytona image.docker diagnostic, got: {message}" + message.contains("image.docker") && message.contains("image.dockerfile"), + "expected mutually exclusive image diagnostic, got: {message}" ); } From 5ebf3ebd35851cebdab4898b74fd0cf25028e868 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 28 Aug 2026 16:19:56 -0400 Subject: [PATCH 2/2] Model the Daytona snapshot source as an enum `DaytonaSnapshotSettings` carried two independent `Option`s (`image` and `dockerfile`) that every consumer had to re-validate. Replace them with a single `source: DaytonaSnapshotSource { Image, Dockerfile }` so the both-set and neither-set states are unrepresentable at the sandbox layer. This removes four unreachable error arms in `canonical_manifest` and `create_snapshot_params`, the `.filter(...)` guard in `initialize`, and the presence guard in `daytona_config_from_environment`. The mutual-exclusion rule now lives only in fabro-config, which owns the `image.docker` / `image.dockerfile` keys the old messages named. Merge `ImageSnapshotManifest` into `SnapshotManifest` via a flattened `SourceManifest` enum. The dockerfile case serializes to the same bytes as before, so existing snapshot names are unchanged; the pinned identity test still passes. Pin the image-case identity as well so a future manifest change cannot silently orphan image snapshots. Fold `validate_daytona_image_settings` into the existing Daytona arm of `validate_provider_capabilities`; both callers already invoke it right after `resolve_environment_fields`, so error order is unchanged. Co-Authored-By: Claude Fable 5 --- lib/components/fabro-sandbox/src/config.rs | 18 +- .../fabro-sandbox/src/daytona/mod.rs | 164 +++++++----------- .../fabro-sandbox/src/from_environment.rs | 57 +++--- .../tests/it/daytona_integration.rs | 16 +- .../fabro-config/src/resolve/environment.rs | 29 ++-- 5 files changed, 124 insertions(+), 160 deletions(-) diff --git a/lib/components/fabro-sandbox/src/config.rs b/lib/components/fabro-sandbox/src/config.rs index d6cd1b179..ec946a537 100644 --- a/lib/components/fabro-sandbox/src/config.rs +++ b/lib/components/fabro-sandbox/src/config.rs @@ -117,11 +117,19 @@ pub enum DockerfileSource { Path { path: String }, } +/// Where a custom Daytona snapshot is built from. +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +pub enum DaytonaSnapshotSource { + /// A pullable image reference such as `ubuntu:24.04`. + Image(String), + /// A Dockerfile that Daytona builds into the snapshot. + Dockerfile(DockerfileSource), +} + #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct DaytonaSnapshotSettings { - pub cpu: Option, - pub memory: Option, - pub disk: Option, - pub image: Option, - pub dockerfile: Option, + pub cpu: Option, + pub memory: Option, + pub disk: Option, + pub source: DaytonaSnapshotSource, } diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index 7a9e4efd4..66173286f 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -133,7 +133,7 @@ pub const REQUIRED_DAYTONA_PERMISSIONS: &[Permissions] = &[ pub use crate::config::{ DaytonaNetwork, DaytonaSettings as DaytonaConfig, - DaytonaSnapshotSettings as DaytonaSnapshotConfig, DockerfileSource, + DaytonaSnapshotSettings as DaytonaSnapshotConfig, DaytonaSnapshotSource, DockerfileSource, }; pub mod snapshot_identity { @@ -142,7 +142,7 @@ pub mod snapshot_identity { use sha2::{Digest, Sha256}; use uuid::Uuid; - use super::{DaytonaSnapshotConfig, DockerfileSource}; + use super::{DaytonaSnapshotConfig, DaytonaSnapshotSource, DockerfileSource}; const IDENTITY_VERSION: u8 = 1; const PROVIDER: &str = "daytona"; @@ -150,27 +150,27 @@ pub mod snapshot_identity { type HmacSha256 = Hmac; + /// The snapshot source as it appears in the identity manifest. Each + /// variant flattens into a single `"": ""` entry. #[derive(Serialize)] - struct SnapshotManifest<'a> { - identity_version: u8, - provider: &'static str, - tenant: &'static str, - dockerfile_sha256: &'a str, - cpu: Option, - memory_gb: Option, - disk_gb: Option, - entrypoint: Option<&'static str>, + #[serde(rename_all = "snake_case")] + enum SourceManifest<'a> { + DockerfileSha256(String), + Image(&'a str), } #[derive(Serialize)] - struct ImageSnapshotManifest<'a> { + struct SnapshotManifest<'a> { identity_version: u8, provider: &'static str, tenant: &'static str, - image: &'a str, + #[serde(flatten)] + source: SourceManifest<'a>, cpu: Option, memory_gb: Option, disk_gb: Option, + /// Nothing sets an entrypoint yet. The field stays because removing + /// it would rename every existing snapshot under `IDENTITY_VERSION` 1. entrypoint: Option<&'static str>, } @@ -186,49 +186,26 @@ pub mod snapshot_identity { } fn canonical_manifest(config: &DaytonaSnapshotConfig) -> crate::Result> { - let dockerfile = match (&config.image, &config.dockerfile) { - (Some(image), None) => { - return serde_json::to_vec(&ImageSnapshotManifest { - identity_version: IDENTITY_VERSION, - provider: PROVIDER, - tenant: TENANT, - image, - cpu: config.cpu, - memory_gb: config.memory, - disk_gb: config.disk, - entrypoint: None, - }) - .map_err(|err| { - crate::Error::context("Failed to serialize Daytona snapshot identity", err) - }); + let source = match &config.source { + DaytonaSnapshotSource::Image(image) => SourceManifest::Image(image), + DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline(text)) => { + SourceManifest::DockerfileSha256(hex::encode(Sha256::digest(text.as_bytes()))) } - (Some(_), Some(_)) => { - return Err(crate::Error::message( - "Daytona custom snapshots accept either image.docker or image.dockerfile, not both", - )); - } - (None, None) => { - return Err(crate::Error::message( - "Daytona custom snapshots require image.docker or image.dockerfile", - )); - } - (None, Some(DockerfileSource::Inline(text))) => text.as_str(), - (None, Some(DockerfileSource::Path { .. })) => { + DaytonaSnapshotSource::Dockerfile(DockerfileSource::Path { .. }) => { return Err(crate::Error::message( "Daytona snapshot dockerfile path should have been resolved to inline content before sandbox creation", )); } }; - let dockerfile_sha256 = hex::encode(Sha256::digest(dockerfile.as_bytes())); let manifest = SnapshotManifest { - identity_version: IDENTITY_VERSION, - provider: PROVIDER, - tenant: TENANT, - dockerfile_sha256: &dockerfile_sha256, - cpu: config.cpu, - memory_gb: config.memory, - disk_gb: config.disk, - entrypoint: None, + identity_version: IDENTITY_VERSION, + provider: PROVIDER, + tenant: TENANT, + source, + cpu: config.cpu, + memory_gb: config.memory, + disk_gb: config.disk, + entrypoint: None, }; serde_json::to_vec(&manifest).map_err(|err| { crate::Error::context("Failed to serialize Daytona snapshot identity", err) @@ -240,26 +217,16 @@ fn create_snapshot_params( name: &str, config: &DaytonaSnapshotConfig, ) -> crate::Result { - let image = match (&config.image, &config.dockerfile) { - (Some(image), None) => daytona_sdk::ImageSource::Name(image.clone()), - (None, Some(DockerfileSource::Inline(dockerfile))) => { + let image = match &config.source { + DaytonaSnapshotSource::Image(image) => daytona_sdk::ImageSource::Name(image.clone()), + DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline(dockerfile)) => { daytona_sdk::ImageSource::Custom(daytona_sdk::DockerImage::from_dockerfile(dockerfile)) } - (None, Some(DockerfileSource::Path { .. })) => { + DaytonaSnapshotSource::Dockerfile(DockerfileSource::Path { .. }) => { return Err(crate::Error::message(format!( "Snapshot '{name}': dockerfile path should have been resolved to inline content before sandbox creation" ))); } - (Some(_), Some(_)) => { - return Err(crate::Error::message(format!( - "Snapshot '{name}': image.docker and image.dockerfile cannot both be configured" - ))); - } - (None, None) => { - return Err(crate::Error::message(format!( - "Snapshot '{name}' does not exist and no image or dockerfile was provided to create it" - ))); - } }; Ok(daytona_sdk::CreateSnapshotParams { @@ -1511,12 +1478,7 @@ impl Sandbox for DaytonaSandbox { }); let init_start = Instant::now(); - let params = if let Some(snap_cfg) = self - .config - .snapshot - .as_ref() - .filter(|snapshot| snapshot.image.is_some() || snapshot.dockerfile.is_some()) - { + let params = if let Some(snap_cfg) = self.config.snapshot.as_ref() { let api_key = self.api_key.as_deref().ok_or_else(|| { self.fail_init( init_start, @@ -3695,11 +3657,10 @@ mod tests { #[test] fn computed_snapshot_identity_is_deterministic_and_keyed() { let config = DaytonaSnapshotConfig { - cpu: Some(2), - memory: Some(4), - disk: Some(10), - image: None, - dockerfile: Some(DockerfileSource::Inline( + cpu: Some(2), + memory: Some(4), + disk: Some(10), + source: DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline( "FROM ubuntu:24.04\nRUN apt-get update".to_string(), )), }; @@ -3722,17 +3683,18 @@ mod tests { #[test] fn computed_snapshot_identity_changes_for_generation_inputs() { let base = DaytonaSnapshotConfig { - cpu: Some(2), - memory: Some(4), - disk: Some(10), - image: None, - dockerfile: Some(DockerfileSource::Inline("FROM ubuntu:24.04".to_string())), + cpu: Some(2), + memory: Some(4), + disk: Some(10), + source: DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline( + "FROM ubuntu:24.04".to_string(), + )), }; let base_name = snapshot_identity::snapshot_name("dtn_secret", &base).unwrap(); let cases = [ DaytonaSnapshotConfig { - dockerfile: Some(DockerfileSource::Inline( + source: DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline( "FROM ubuntu:24.04\n# roll cache".to_string(), )), ..base.clone() @@ -3760,11 +3722,10 @@ mod tests { #[test] fn computed_snapshot_identity_excludes_raw_dockerfile_and_key_material() { let config = DaytonaSnapshotConfig { - cpu: None, - memory: None, - disk: None, - image: None, - dockerfile: Some(DockerfileSource::Inline( + cpu: None, + memory: None, + disk: None, + source: DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline( "FROM private.example.com/secret-image\nRUN echo raw-secret".to_string(), )), }; @@ -3780,31 +3741,29 @@ mod tests { #[test] fn computed_snapshot_identity_changes_for_image_reference() { let config = DaytonaSnapshotConfig { - cpu: Some(2), - memory: Some(4), - disk: Some(10), - image: Some("ubuntu:24.04".to_string()), - dockerfile: None, + cpu: Some(2), + memory: Some(4), + disk: Some(10), + source: DaytonaSnapshotSource::Image("ubuntu:24.04".to_string()), }; let first = snapshot_identity::snapshot_name("dtn_secret", &config).unwrap(); let changed = snapshot_identity::snapshot_name("dtn_secret", &DaytonaSnapshotConfig { - image: Some("ubuntu:24.10".to_string()), + source: DaytonaSnapshotSource::Image("ubuntu:24.10".to_string()), ..config }) .unwrap(); + assert_eq!(first, "fabro-5d23a023-d7ff-8d68-b3ca-e6286f4211d9"); assert_ne!(first, changed); - assert!(!first.contains("ubuntu")); } #[test] fn snapshot_creation_uses_named_image_source() { let config = DaytonaSnapshotConfig { - cpu: Some(2), - memory: Some(4), - disk: Some(10), - image: Some("ubuntu:24.04".to_string()), - dockerfile: None, + cpu: Some(2), + memory: Some(4), + disk: Some(10), + source: DaytonaSnapshotSource::Image("ubuntu:24.04".to_string()), }; let params = create_snapshot_params("fabro-test", &config).unwrap(); @@ -3824,11 +3783,12 @@ mod tests { async fn ensure_snapshot_uses_computed_snapshot_name_for_daytona_api_calls() { let api_key = "dtn_secret"; let snapshot = DaytonaSnapshotConfig { - cpu: Some(2), - memory: Some(4), - disk: Some(10), - image: None, - dockerfile: Some(DockerfileSource::Inline("FROM ubuntu:24.04".to_string())), + cpu: Some(2), + memory: Some(4), + disk: Some(10), + source: DaytonaSnapshotSource::Dockerfile(DockerfileSource::Inline( + "FROM ubuntu:24.04".to_string(), + )), }; let computed_name = snapshot_identity::snapshot_name(api_key, &snapshot).unwrap(); let server = MockServer::start_async().await; diff --git a/lib/components/fabro-sandbox/src/from_environment.rs b/lib/components/fabro-sandbox/src/from_environment.rs index b6178e4eb..1689ddc83 100644 --- a/lib/components/fabro-sandbox/src/from_environment.rs +++ b/lib/components/fabro-sandbox/src/from_environment.rs @@ -15,7 +15,8 @@ use fabro_types::settings::run::{ #[cfg(feature = "daytona")] use crate::config::{ - DaytonaNetwork, DaytonaSnapshotSettings, DockerfileSource as SandboxDockerfileSource, + DaytonaNetwork, DaytonaSnapshotSettings, DaytonaSnapshotSource, + DockerfileSource as SandboxDockerfileSource, }; #[cfg(feature = "daytona")] use crate::daytona::DaytonaConfig; @@ -28,30 +29,30 @@ pub fn daytona_config_from_environment( settings: &RunEnvironmentSettings, clone: &RunCloneSettings, ) -> DaytonaConfig { - let dockerfile = settings - .image - .dockerfile - .as_ref() - .map(|dockerfile| match dockerfile { - ResolvedDockerfileSource::Inline(text) => SandboxDockerfileSource::Inline(text.clone()), - ResolvedDockerfileSource::Path { path } => { - SandboxDockerfileSource::Path { path: path.clone() } - } - }); - let snapshot = (settings.image.docker.is_some() || dockerfile.is_some()).then(|| { - DaytonaSnapshotSettings { - cpu: settings.resources.cpu, - memory: settings - .resources - .memory - .map(|size| size_to_gb_i32(size.as_bytes())), - disk: settings - .resources - .disk - .map(|size| size_to_gb_i32(size.as_bytes())), - image: settings.image.docker.clone(), - dockerfile, - } + // fabro-config rejects Daytona environments that set both image.docker + // and image.dockerfile. If both still arrive here, the image wins, which + // matches how the Docker provider treats the pair. + let source = match (&settings.image.docker, &settings.image.dockerfile) { + (Some(image), _) => Some(DaytonaSnapshotSource::Image(image.clone())), + (None, Some(ResolvedDockerfileSource::Inline(text))) => Some( + DaytonaSnapshotSource::Dockerfile(SandboxDockerfileSource::Inline(text.clone())), + ), + (None, Some(ResolvedDockerfileSource::Path { path })) => Some( + DaytonaSnapshotSource::Dockerfile(SandboxDockerfileSource::Path { path: path.clone() }), + ), + (None, None) => None, + }; + let snapshot = source.map(|source| DaytonaSnapshotSettings { + cpu: settings.resources.cpu, + memory: settings + .resources + .memory + .map(|size| size_to_gb_i32(size.as_bytes())), + disk: settings + .resources + .disk + .map(|size| size_to_gb_i32(size.as_bytes())), + source, }); DaytonaConfig { @@ -260,8 +261,10 @@ mod tests { let config = daytona_config_from_environment(&settings, &RunCloneSettings::default()); let snapshot = config.snapshot.expect("image should configure a snapshot"); - assert_eq!(snapshot.image.as_deref(), Some("ubuntu:24.04")); - assert!(snapshot.dockerfile.is_none()); + assert_eq!( + snapshot.source, + DaytonaSnapshotSource::Image("ubuntu:24.04".to_string()) + ); assert_eq!(snapshot.cpu, Some(2)); } } diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index 0fd36836b..407500c9d 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -403,13 +403,15 @@ async fn daytona_snapshot_sandbox() { let config = DaytonaConfig { auto_stop_interval: Some(60), snapshot: Some(DaytonaSnapshotConfig { - cpu: Some(2), - memory: Some(4), - disk: Some(10), - image: None, - dockerfile: Some(fabro_sandbox::daytona::DockerfileSource::Inline( - "FROM ubuntu:22.04\nRUN apt-get update && apt-get install -y ripgrep".to_string(), - )), + cpu: Some(2), + memory: Some(4), + disk: Some(10), + source: fabro_sandbox::daytona::DaytonaSnapshotSource::Dockerfile( + fabro_sandbox::daytona::DockerfileSource::Inline( + "FROM ubuntu:22.04\nRUN apt-get update && apt-get install -y ripgrep" + .to_string(), + ), + ), }), ..DaytonaConfig::default() }; diff --git a/lib/foundation/fabro-config/src/resolve/environment.rs b/lib/foundation/fabro-config/src/resolve/environment.rs index cf13eade3..3f84ee421 100644 --- a/lib/foundation/fabro-config/src/resolve/environment.rs +++ b/lib/foundation/fabro-config/src/resolve/environment.rs @@ -81,7 +81,6 @@ fn resolve_environment_fields( labels: layer.labels.clone().into_inner(), env: layer.env.clone().into_inner(), }; - validate_daytona_image_settings(&environment, path, errors); environment } @@ -201,23 +200,6 @@ fn dockerfile_source(dockerfile: &EnvironmentDockerfileLayer) -> DockerfileSourc } } -fn validate_daytona_image_settings( - environment: &EnvironmentSettings, - path: &str, - errors: &mut Vec, -) { - if environment.provider == EnvironmentProvider::Daytona - && environment.image.docker.is_some() - && environment.image.dockerfile.is_some() - { - errors.push(ResolveError::Invalid { - path: format!("{path}.image"), - reason: "daytona environments accept either image.docker or image.dockerfile, not both" - .to_string(), - }); - } -} - fn validate_provider_capabilities( environment: &EnvironmentSettings, path: &str, @@ -246,6 +228,15 @@ fn validate_provider_capabilities( }); } } - EnvironmentProvider::Daytona => {} + EnvironmentProvider::Daytona => { + if environment.image.docker.is_some() && environment.image.dockerfile.is_some() { + errors.push(ResolveError::Invalid { + path: format!("{path}.image"), + reason: "daytona environments accept either image.docker or image.dockerfile, \ + not both" + .to_string(), + }); + } + } } }