mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
b34f2b8207
commit
5ebf3ebd35
5 changed files with 124 additions and 160 deletions
|
|
@ -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<i32>,
|
||||
pub memory: Option<i32>,
|
||||
pub disk: Option<i32>,
|
||||
pub image: Option<String>,
|
||||
pub dockerfile: Option<DockerfileSource>,
|
||||
pub cpu: Option<i32>,
|
||||
pub memory: Option<i32>,
|
||||
pub disk: Option<i32>,
|
||||
pub source: DaytonaSnapshotSource,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Sha256>;
|
||||
|
||||
/// The snapshot source as it appears in the identity manifest. Each
|
||||
/// variant flattens into a single `"<key>": "<value>"` entry.
|
||||
#[derive(Serialize)]
|
||||
struct SnapshotManifest<'a> {
|
||||
identity_version: u8,
|
||||
provider: &'static str,
|
||||
tenant: &'static str,
|
||||
dockerfile_sha256: &'a str,
|
||||
cpu: Option<i32>,
|
||||
memory_gb: Option<i32>,
|
||||
disk_gb: Option<i32>,
|
||||
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<i32>,
|
||||
memory_gb: Option<i32>,
|
||||
disk_gb: Option<i32>,
|
||||
/// 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<Vec<u8>> {
|
||||
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<daytona_sdk::CreateSnapshotParams> {
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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<ResolveError>,
|
||||
) {
|
||||
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(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue