diff --git a/Cargo.lock b/Cargo.lock index bde521116..b61186339 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7007,7 +7007,7 @@ dependencies = [ [[package]] name = "sandbox-driver" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "async-trait", "globset", @@ -7023,7 +7023,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "anyhow", "async-trait", @@ -7048,7 +7048,7 @@ dependencies = [ [[package]] name = "sandbox-driver-daytona-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "sandbox-driver-docker-config", "serde", @@ -7058,7 +7058,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "anyhow", "async-trait", @@ -7079,7 +7079,7 @@ dependencies = [ [[package]] name = "sandbox-driver-docker-config" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "serde", "serde_json", @@ -7088,7 +7088,7 @@ dependencies = [ [[package]] name = "sandbox-driver-host" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "anyhow", "async-trait", @@ -7106,7 +7106,7 @@ dependencies = [ [[package]] name = "sandbox-driver-protocol" version = "0.1.0" -source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d#d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" +source = "git+https://github.com/lithoscomputer/sandbox-driver?rev=1c30e72062d20df802d491f07b428318391498f1#1c30e72062d20df802d491f07b428318391498f1" dependencies = [ "async-trait", "base64", diff --git a/Cargo.toml b/Cargo.toml index fd2118feb..a36df5526 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,14 +102,14 @@ futures-util = "0.3" # git failures, stop grace, snapshot ensure, ownership scope, testing doubles), to # move to main on merge. The CI plugin job installs the driver executables at the # same rev, read from this file. -sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } -sandbox-driver-testing = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "d12e0aba10ddc4438eea5e9cc0f27caf8ad22f9d" } +sandbox-driver = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-protocol = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-host = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-docker = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-docker-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-daytona = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-daytona-config = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } +sandbox-driver-testing = { git = "https://github.com/lithoscomputer/sandbox-driver", rev = "1c30e72062d20df802d491f07b428318391498f1" } sentry = { version = "0.35", default-features = false, features = ["backtrace", "contexts", "ureq", "rustls"] } fork = "0.2" exec = "0.3" diff --git a/lib/components/fabro-sandbox/src/clone.rs b/lib/components/fabro-sandbox/src/clone.rs index c81653157..358baf2d4 100644 --- a/lib/components/fabro-sandbox/src/clone.rs +++ b/lib/components/fabro-sandbox/src/clone.rs @@ -18,13 +18,13 @@ use fabro_types::SandboxProviderKind; use sandbox_driver::{Git as _, GitCloneOptions, GitCredentials, Sandbox as DriverHandle}; use tokio::time; +use crate::ExecResult; use crate::clone_source::{self, GitHubRepoLayout, PinnedRevision}; use crate::exec::SandboxExec; use crate::git_retry::{self, CredentialContext, GitRetryReason, RetryPlan}; use crate::push_credentials::PushCredentialState; use crate::redact::redact_auth_url; use crate::sandbox::shell_quote; -use crate::{ExecResult, ExecStreamingRequest}; /// Whole-clone budget, shared by every network and local step. pub(crate) const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(5); @@ -107,91 +107,69 @@ pub(crate) async fn clone_github_repo( let deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; let has_app = credentials.source().is_some(); - match PinnedRevision::from_selectors(plan.tag.as_deref(), plan.commit_sha.as_deref()) { - Some(PinnedRevision::Tag(tag)) => { - // `decide_clone` already requires a branch for a pin; the branch - // names the checkout the run works on. - let branch = plan - .branch - .as_deref() - .filter(|branch| !branch.trim().is_empty()) - .ok_or_else(|| { - crate::Error::message("Tag checkout requires a repository branch") - })?; - clone_pinned_tag( - kind, - exec, - &layout, - plan, - &tag, - branch, - auth_url.as_ref(), - credential_context, - deadline, - has_app, - ) - .await?; - } - pin => { - let git = handle.git().ok_or_else(|| { - crate::Error::message(format!( - "sandbox provider `{kind}` does not support git operations" - )) - })?; - let mut options = GitCloneOptions::default(); - options.branch = plan - .branch - .clone() - .filter(|branch| !branch.trim().is_empty()); - options.commit = plan.commit_sha.clone(); - options.depth = plan.depth; - options.credentials = resolved_token - .as_ref() - .map(|token| GitCredentials::new("x-access-token", token.token.expose())); - let retry_plan = RetryPlan::clone_default(Some(deadline)); - let target = layout.primary_repo_path.clone(); - git_retry::retry_git_operation( - kind.clone(), - "clone", - &retry_plan, - |_attempt| { - let options = options.clone(); - let target = target.clone(); - let origin_url = plan.origin_url.clone(); - let git = &git; - async move { - git.clone_repo(&origin_url, &target, &options) - .await - .map_err(|error| CloneFailure { - retry_reason: git_retry::classify_driver_failure( - &error, - credential_context, - ), - error: clone_failure_error( - crate::Error::driver_error(error), - CloneStep::Network, - has_app, - ), - }) - } - }, - |failure: &CloneFailure| failure.retry_reason, - ) - .await - .map_err(|failure| failure.error)?; - if let Some(pin) = pin { - let head = run_local_step( - exec, - &clone_source::exact_head_revision_command(&layout.primary_repo_path), - "git rev-parse HEAD (pinned checkout)", - deadline, - auth_url.as_ref(), - has_app, - ) - .await?; - pin.verify_head(&head.stdout)?; + let git = handle.git().ok_or_else(|| { + crate::Error::message(format!( + "sandbox provider `{kind}` does not support git operations" + )) + })?; + // `decide_clone` already requires a branch for a pin; the branch names + // the checkout the run works on, and the driver attaches it to the + // pinned commit or tag. + let mut options = GitCloneOptions::default(); + options.branch = plan + .branch + .clone() + .filter(|branch| !branch.trim().is_empty()); + options.commit = plan.commit_sha.clone(); + options.tag = plan.tag.clone().filter(|_| plan.commit_sha.is_none()); + options.depth = plan.depth; + options.credentials = resolved_token + .as_ref() + .map(|token| GitCredentials::new("x-access-token", token.token.expose())); + let retry_plan = RetryPlan::clone_default(Some(deadline)); + let target = layout.primary_repo_path.clone(); + git_retry::retry_git_operation( + kind.clone(), + "clone", + &retry_plan, + |_attempt| { + let options = options.clone(); + let target = target.clone(); + let origin_url = plan.origin_url.clone(); + let git = &git; + async move { + git.clone_repo(&origin_url, &target, &options) + .await + .map_err(|error| CloneFailure { + retry_reason: git_retry::classify_driver_failure( + &error, + credential_context, + ), + error: clone_failure_error( + crate::Error::driver_error(error), + CloneStep::Network, + has_app, + ), + }) } - } + }, + |failure: &CloneFailure| failure.retry_reason, + ) + .await + .map_err(|failure| failure.error)?; + if let Some(pin) = + PinnedRevision::from_selectors(plan.tag.as_deref(), plan.commit_sha.as_deref()) + { + let head = run_local_step( + exec, + &clone_source::exact_head_revision_command(&layout.primary_repo_path), + "git rev-parse HEAD (pinned checkout)", + deadline, + auth_url.as_ref(), + has_app, + ) + .await?; + pin.verify_head(&head.stdout)?; } run_local_step( @@ -210,114 +188,6 @@ pub(crate) async fn clone_github_repo( Ok(CloneOutcome { layout }) } -/// Fabro's exact tag checkout: init, fetch the fully qualified tag ref at -/// the same depth a branch clone gets, attach the admitted branch to the -/// fetched commit, and verify HEAD. Every step runs through `Exec`. -#[expect( - clippy::too_many_arguments, - reason = "the pinned path threads clone inputs, credentials, and the shared deadline" -)] -async fn clone_pinned_tag( - kind: &SandboxProviderKind, - exec: &SandboxExec<'_>, - layout: &GitHubRepoLayout, - plan: &GitHubClone, - tag: &str, - branch: &str, - auth_url: Option<&DisplaySafeUrl>, - credential_context: CredentialContext, - deadline: time::Instant, - has_app: bool, -) -> crate::Result<()> { - let clone_url = auth_url.map_or(plan.origin_url.as_str(), |url| url.as_raw_url().as_str()); - let init = clone_source::exact_repository_init_command(clone_url, &layout.primary_repo_path); - run_local_step( - exec, - &init, - "initialize pinned repository checkout", - deadline, - auth_url, - has_app, - ) - .await?; - - let pin = PinnedRevision::Tag(tag.to_string()); - let fetch = clone_source::pinned_fetch_command( - &layout.primary_repo_path, - "origin", - &pin.fetch_refspec(), - plan.depth.map(|depth| depth as usize), - ); - let retry_plan = RetryPlan::clone_default(Some(deadline)); - git_retry::retry_git_operation( - kind.clone(), - "fetch", - &retry_plan, - |_attempt| async { - let remaining = deadline.saturating_duration_since(time::Instant::now()); - if remaining.is_zero() { - return Err(CloneFailure { - error: crate::Error::message( - "git fetch pinned revision deadline expired before retry", - ), - retry_reason: None, - }); - } - let result = exec - .run_streaming(ExecStreamingRequest { - timeout_ms: Some(millis(remaining)), - working_dir: Some("/"), - ..ExecStreamingRequest::new(&fetch) - }) - .await - .map_err(|error| CloneFailure { - error: crate::Error::context( - "git fetch pinned revision transport failed", - error, - ), - retry_reason: None, - })? - .result; - if result.is_success() { - return Ok(()); - } - let retry_reason = - git_retry::classify_output(&result.stderr, &result.stdout, credential_context) - .retry_reason(); - Err(CloneFailure { - error: clone_failure_error( - result.into_exec_error_with_redactor("git fetch pinned revision", |output| { - redact_auth_url(output, auth_url) - }), - CloneStep::Network, - has_app, - ), - retry_reason, - }) - }, - |failure: &CloneFailure| failure.retry_reason, - ) - .await - .map_err(|failure| failure.error)?; - - let checkout = clone_source::exact_checkout_verify_command( - &layout.primary_repo_path, - branch, - clone_source::FETCH_HEAD_COMMIT, - ); - let head = run_local_step( - exec, - &checkout, - "git checkout pinned revision", - deadline, - auth_url, - has_app, - ) - .await?; - pin.verify_head(&head.stdout)?; - Ok(()) -} - async fn verify_git_available(exec: &SandboxExec<'_>) -> crate::Result<()> { let result = exec .run("git --version", Some(STEP_TIMEOUT), Some("/"), None, None) @@ -424,7 +294,3 @@ async fn embed_origin_credentials( } } } - -fn millis(duration: Duration) -> u64 { - u64::try_from(duration.as_millis()).unwrap_or(u64::MAX) -} diff --git a/lib/components/fabro-sandbox/src/clone_source.rs b/lib/components/fabro-sandbox/src/clone_source.rs index a8ea6b7d4..2487ab22e 100644 --- a/lib/components/fabro-sandbox/src/clone_source.rs +++ b/lib/components/fabro-sandbox/src/clone_source.rs @@ -73,15 +73,6 @@ pub(crate) fn repo_symlink_command(layout: &GitHubRepoLayout) -> String { ) } -pub(crate) fn exact_repository_init_command(clone_url: &str, checkout_path: &str) -> String { - format!( - "{git} init -- {path} && git -C {path} remote add origin {origin}", - git = sandbox::GIT, - path = sandbox::shell_quote(checkout_path), - origin = sandbox::shell_quote(clone_url), - ) -} - /// A revision the checkout is pinned to instead of the branch's current HEAD. /// /// The working branch names the checkout the run works on; it never constrains @@ -92,8 +83,8 @@ pub(crate) enum PinnedRevision { /// An exact commit SHA, already normalized by /// [`normalize_exact_commit_sha`]. Commit(String), - /// A bare tag name, fetched as `refs/tags/` so a same-named branch is - /// never consulted. + /// A bare tag name; the driver fetches it as `refs/tags/` so a + /// same-named branch is never consulted. Tag(String), } @@ -116,14 +107,6 @@ impl PinnedRevision { } } - /// The refspec handed to `git fetch`. - pub(crate) fn fetch_refspec(&self) -> String { - match self { - Self::Commit(sha) => sha.clone(), - Self::Tag(tag) => tag_ref(tag), - } - } - /// The commit HEAD must resolve to after checkout, when one is known. pub(crate) fn expected_sha(&self) -> Option<&str> { match self { @@ -148,57 +131,6 @@ impl PinnedRevision { } } -/// Fully-qualified ref for a bare tag name. -pub(crate) fn tag_ref(tag: &str) -> String { - format!("refs/tags/{tag}") -} - -/// Fetch a single pinned refspec with the same history depth a branch clone -/// gets, so both paths can reach the same number of parent commits. -/// -/// The fetch names the revision directly rather than the branch, and -/// `--no-tags` keeps unrelated tags from being pulled alongside it. -pub(crate) fn pinned_fetch_command( - checkout_path: &str, - fetch_source: &str, - refspec: &str, - depth: Option, -) -> String { - let depth_arg = depth_argument(depth); - format!( - "{git} -C {} fetch{depth_arg} --no-tags {} -- {}", - sandbox::shell_quote(checkout_path), - sandbox::shell_quote(fetch_source), - sandbox::shell_quote(refspec), - git = sandbox::GIT, - ) -} - -/// Leading-space ` --depth N` fragment for a Git command, or empty when -/// `depth` is `None` to fetch full history. -pub(crate) fn depth_argument(depth: Option) -> String { - depth.map_or_else(String::new, |depth| format!(" --depth {depth}")) -} - -/// Point the admitted branch at `revision` and attach HEAD to it. -/// -/// The checkout attaches to a real branch instead of detaching so callers that -/// read the current branch back out of the workspace still see the admitted -/// branch name. -pub(crate) fn exact_branch_checkout_command( - checkout_path: &str, - branch: &str, - revision: &str, -) -> String { - format!( - "{git} -C {path} checkout -B {branch} {revision}", - path = sandbox::shell_quote(checkout_path), - branch = sandbox::shell_quote(branch), - revision = sandbox::shell_quote(revision), - git = sandbox::GIT, - ) -} - /// Print the current HEAD commit and nothing else, for /// [`PinnedRevision::verify_head`]. pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { @@ -209,25 +141,6 @@ pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { ) } -/// Check out the admitted branch and print the resulting HEAD in one shell -/// command; stdout is the `rev-parse HEAD` output for -/// [`PinnedRevision::verify_head`]. -pub(crate) fn exact_checkout_verify_command( - checkout_path: &str, - branch: &str, - revision: &str, -) -> String { - format!( - "{} && {}", - exact_branch_checkout_command(checkout_path, branch, revision), - exact_head_revision_command(checkout_path), - ) -} - -/// The peeled commit behind whatever `git fetch` just wrote to `FETCH_HEAD`; -/// a commit peels to itself, an annotated tag to the commit it points at. -pub(crate) const FETCH_HEAD_COMMIT: &str = "FETCH_HEAD^{commit}"; - /// Validate that a `rev-parse HEAD` output is a single commit ID and return it /// normalized. pub(crate) fn verify_resolved_head(output: &str) -> crate::Result { @@ -351,62 +264,8 @@ pub(crate) fn repo_cloned_for_record( #[cfg(test)] mod tests { - use std::fs; - use std::path::Path; - use std::process::{Command, Output}; - use super::*; - fn isolated_command(command: &mut Command) -> Output { - command - .env("GIT_CONFIG_NOSYSTEM", "1") - .env("GIT_CONFIG_GLOBAL", "/dev/null") - .env("GIT_TERMINAL_PROMPT", "0") - .env("GIT_AUTHOR_NAME", "Fabro Test") - .env("GIT_AUTHOR_EMAIL", "fabro-test@example.com") - .env("GIT_COMMITTER_NAME", "Fabro Test") - .env("GIT_COMMITTER_EMAIL", "fabro-test@example.com") - .output() - .expect("test command should start") - } - - #[expect( - clippy::disallowed_methods, - reason = "hermetic Git proof intentionally runs the local git executable synchronously" - )] - fn run_git(cwd: &Path, args: &[&str]) -> String { - let output = isolated_command(Command::new("git").current_dir(cwd).args(args)); - assert!( - output.status.success(), - "git {args:?} failed: {}", - String::from_utf8_lossy(&output.stderr) - ); - String::from_utf8(output.stdout).expect("git output should be UTF-8") - } - - fn run_shell(cwd: &Path, command: &str) -> String { - let output = run_shell_output(cwd, command); - assert!( - output.status.success(), - "command failed: {}", - String::from_utf8_lossy(&output.stderr) - ); - String::from_utf8(output.stdout).expect("command output should be UTF-8") - } - - #[expect( - clippy::disallowed_methods, - reason = "hermetic command-builder proof intentionally runs local Bash synchronously" - )] - fn run_shell_output(cwd: &Path, command: &str) -> Output { - isolated_command(Command::new("/bin/bash").current_dir(cwd).args([ - "--noprofile", - "--norc", - "-c", - command, - ])) - } - #[test] fn skip_clone_overrides_present_origin() { assert_eq!( @@ -479,24 +338,9 @@ mod tests { let sha = "0123456789abcdef0123456789abcdef01234567"; assert_eq!(PinnedRevision::from_selectors(None, None), None); let tag = PinnedRevision::from_selectors(Some("release/v1"), None).unwrap(); - assert_eq!(tag.fetch_refspec(), "refs/tags/release/v1"); assert_eq!(tag.expected_sha(), None); let commit = PinnedRevision::from_selectors(Some("release/v1"), Some(sha)).unwrap(); - assert_eq!(commit.fetch_refspec(), sha); assert_eq!(commit.expected_sha(), Some(sha)); - assert_eq!( - pinned_fetch_command( - "/repos/acme/widgets", - "origin", - &tag.fetch_refspec(), - Some(10) - ), - "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets fetch --depth 10 --no-tags origin -- refs/tags/release/v1" - ); - assert_eq!( - exact_checkout_verify_command("/repos/acme/widgets", "release", FETCH_HEAD_COMMIT), - "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets checkout -B release FETCH_HEAD'^{commit}' && git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets rev-parse HEAD" - ); } #[test] @@ -624,49 +468,6 @@ mod tests { assert!(empty_tag.to_string().contains("non-empty tag")); } - #[test] - fn docker_exact_checkout_commands_quote_inputs() { - let sha = "0123456789abcdef0123456789abcdef01234567"; - let init = exact_repository_init_command( - "https://token@example.com/acme/widgets.git?x=a b", - "/repos/acme's widgets", - ); - let fetch = pinned_fetch_command( - "/repos/acme's widgets", - "https://token@example.com/acme/widgets.git?x=a b", - sha, - Some(10), - ); - let checkout = - exact_checkout_verify_command("/repos/acme's widgets", "feature/a b", "FETCH_HEAD"); - - assert_eq!( - init, - "git -c maintenance.auto=0 -c gc.auto=0 init -- \"/repos/acme's widgets\" && git -C \"/repos/acme's widgets\" remote add origin 'https://token@example.com/acme/widgets.git?x=a b'" - ); - assert_eq!( - fetch, - "git -c maintenance.auto=0 -c gc.auto=0 -C \"/repos/acme's widgets\" fetch --depth 10 --no-tags 'https://token@example.com/acme/widgets.git?x=a b' -- 0123456789abcdef0123456789abcdef01234567" - ); - assert_eq!( - checkout, - "git -c maintenance.auto=0 -c gc.auto=0 -C \"/repos/acme's widgets\" checkout -B 'feature/a b' FETCH_HEAD && git -c maintenance.auto=0 -c gc.auto=0 -C \"/repos/acme's widgets\" rev-parse HEAD" - ); - } - - #[test] - fn pinned_fetch_omits_depth_for_full_history() { - assert_eq!( - pinned_fetch_command( - "/repos/acme/widgets", - "origin", - "0123456789abcdef0123456789abcdef01234567", - None, - ), - "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets fetch --no-tags origin -- 0123456789abcdef0123456789abcdef01234567" - ); - } - #[test] fn exact_checkout_verification_rejects_invalid_or_mismatched_head() { let expected = "0123456789abcdef0123456789abcdef01234567"; @@ -686,161 +487,6 @@ mod tests { assert!(mismatched.to_string().contains("did not match")); } - #[test] - #[expect( - clippy::disallowed_methods, - reason = "hermetic Git proof uses isolated synchronous temp-repository I/O" - )] - fn exact_checkout_fetches_admitted_commit_after_branch_and_tag_advance() { - let temp = tempfile::tempdir().expect("tempdir"); - let remote = temp.path().join("remote.git"); - let source = temp.path().join("source"); - let checkout = temp.path().join("exact checkout"); - fs::create_dir(&source).expect("source directory"); - - run_git(temp.path(), &[ - "init", - "--bare", - remote.to_str().expect("UTF-8 remote path"), - ]); - run_git(&source, &["init"]); - fs::write(source.join("revision.txt"), "A\n").expect("write commit A"); - run_git(&source, &["add", "revision.txt"]); - run_git(&source, &["commit", "-m", "commit A"]); - run_git(&source, &["branch", "-M", "main"]); - run_git(&source, &[ - "remote", - "add", - "origin", - remote.to_str().expect("UTF-8 remote path"), - ]); - run_git(&source, &["push", "-u", "origin", "main"]); - let admitted_sha = run_git(&source, &["rev-parse", "HEAD"]).trim().to_string(); - run_git(&source, &["tag", "release"]); - run_git(&source, &["push", "origin", "refs/tags/release"]); - - fs::write(source.join("revision.txt"), "B\n").expect("write commit B"); - run_git(&source, &["commit", "-am", "commit B"]); - run_git(&source, &["push", "origin", "main"]); - run_git(&source, &["tag", "-f", "release"]); - run_git(&source, &["push", "--force", "origin", "refs/tags/release"]); - let advanced_sha = run_git(&source, &["rev-parse", "HEAD"]).trim().to_string(); - assert_ne!(admitted_sha, advanced_sha); - - let remote_path = remote.to_str().expect("UTF-8 remote path"); - let checkout_path = checkout.to_str().expect("UTF-8 checkout path"); - run_shell( - temp.path(), - &exact_repository_init_command(remote_path, checkout_path), - ); - run_shell( - temp.path(), - &pinned_fetch_command(checkout_path, remote_path, &admitted_sha, Some(10)), - ); - let checked_out_sha = run_shell( - temp.path(), - &exact_checkout_verify_command(checkout_path, "main", FETCH_HEAD_COMMIT), - ); - - assert_eq!(checked_out_sha.trim(), admitted_sha); - assert_eq!( - fs::read_to_string(checkout.join("revision.txt")).expect("checked-out contents"), - "A\n" - ); - assert_eq!( - run_git(&checkout, &["symbolic-ref", "HEAD"]).trim(), - "refs/heads/main", - "HEAD should stay attached to the admitted branch" - ); - assert_eq!( - run_git(&checkout, &["rev-parse", "--abbrev-ref", "HEAD"]).trim(), - "main" - ); - assert_eq!( - run_git(temp.path(), &[ - "--git-dir", - remote_path, - "rev-parse", - "refs/heads/main", - ],) - .trim(), - advanced_sha - ); - } - - #[test] - #[expect( - clippy::disallowed_methods, - reason = "hermetic Git proof uses isolated synchronous temp-repository I/O" - )] - fn tag_checkout_peels_lightweight_and_annotated_tags_without_branch_fallback() { - let temp = tempfile::tempdir().expect("tempdir"); - let remote = temp.path().join("remote.git"); - let source = temp.path().join("source"); - fs::create_dir(&source).expect("source directory"); - - run_git(temp.path(), &[ - "init", - "--bare", - remote.to_str().expect("UTF-8 remote path"), - ]); - run_git(&source, &["init"]); - fs::write(source.join("revision.txt"), "release\n").expect("write release commit"); - run_git(&source, &["add", "revision.txt"]); - run_git(&source, &["commit", "-m", "release"]); - run_git(&source, &["branch", "-M", "main"]); - let release_sha = run_git(&source, &["rev-parse", "HEAD"]).trim().to_string(); - run_git(&source, &["tag", "lightweight"]); - run_git(&source, &["tag", "-a", "annotated", "-m", "annotated"]); - run_git(&source, &[ - "remote", - "add", - "origin", - remote.to_str().expect("UTF-8 remote path"), - ]); - run_git(&source, &["push", "origin", "main", "--tags"]); - - let remote_path = remote.to_str().expect("UTF-8 remote path"); - for tag in ["lightweight", "annotated"] { - let checkout = temp.path().join(format!("checkout-{tag}")); - let checkout_path = checkout.to_str().expect("UTF-8 checkout path"); - run_shell( - temp.path(), - &exact_repository_init_command(remote_path, checkout_path), - ); - run_shell( - temp.path(), - &pinned_fetch_command(checkout_path, "origin", &tag_ref(tag), Some(10)), - ); - let head = run_shell( - temp.path(), - &exact_checkout_verify_command(checkout_path, "release-work", FETCH_HEAD_COMMIT), - ); - - assert_eq!(verify_resolved_head(&head).unwrap(), release_sha); - assert_eq!( - run_git(&checkout, &["symbolic-ref", "HEAD"]).trim(), - "refs/heads/release-work" - ); - } - - let missing = temp.path().join("missing"); - let missing_path = missing.to_str().expect("UTF-8 checkout path"); - run_shell( - temp.path(), - &exact_repository_init_command(remote_path, missing_path), - ); - let output = run_shell_output( - temp.path(), - &pinned_fetch_command(missing_path, "origin", &tag_ref("main"), Some(10)), - ); - assert!( - !output.status.success(), - "a branch must not satisfy a tag fetch" - ); - assert!(!missing.join("revision.txt").exists()); - } - #[test] fn github_layout_maps_ssh_origin_to_repos_checkout_and_workspace_link() { let layout = github_repo_layout( diff --git a/lib/components/fabro-sandbox/src/daytona.rs b/lib/components/fabro-sandbox/src/daytona.rs index 2637ec330..b9ea3cf0c 100644 --- a/lib/components/fabro-sandbox/src/daytona.rs +++ b/lib/components/fabro-sandbox/src/daytona.rs @@ -17,8 +17,7 @@ use fabro_types::settings::server::ServerSandboxProviderSettings; use fabro_types::{RunId, SandboxProviderKind}; use sandbox_driver::{ HealthStatus, LifecycleTimers, Resources, SandboxProvider, SandboxSource, - SandboxSpec as DriverSpec, SnapshotFilter, SnapshotId, SnapshotProvider, SnapshotSource, - SnapshotSpec, SnapshotState, + SandboxSpec as DriverSpec, SnapshotId, SnapshotSource, SnapshotSpec, }; use tokio::time; @@ -333,57 +332,21 @@ async fn ensure_snapshot( provider: &dyn SandboxProvider, api_key: &str, inputs: &SnapshotInputs<'_>, - emit: &(dyn Fn(SandboxEvent) + Send + Sync), ) -> crate::Result<(SnapshotId, String)> { let name = snapshot_identity::snapshot_name(api_key, inputs)?; let snapshots = provider.snapshots().ok_or_else(|| { crate::Error::message("The Daytona provider does not expose snapshot management") })?; - let mut filter = SnapshotFilter::default(); - filter.name = Some(name.clone()); - let existing = snapshots - .list(&filter) + let id = snapshots + .ensure( + &snapshot_spec(&name, inputs), + DAYTONA_SNAPSHOT_ACTIVE_TIMEOUT, + None, + ) .await .map_err(|error| { - crate::Error::context(format!("Failed to look up snapshot '{name}'"), error) - })? - .into_iter() - .find(|status| status.name.as_deref() == Some(name.as_str())); - let id = if let Some(status) = existing { - match status.state { - SnapshotState::Active => return Ok((status.id, name)), - SnapshotState::Error => { - return Err(crate::Error::message(format!( - "Snapshot '{name}' is in an error state: {}", - status.error_reason.unwrap_or_default() - ))); - } - SnapshotState::Inactive => { - emit(SandboxEvent::SnapshotCreating { name: name.clone() }); - snapshots - .activate(&status.id, None) - .await - .map_err(|error| { - crate::Error::context( - format!("Failed to activate snapshot '{name}'"), - error, - ) - })?; - status.id - } - _ => { - emit(SandboxEvent::SnapshotCreating { name: name.clone() }); - status.id - } - } - } else { - emit(SandboxEvent::SnapshotCreating { name: name.clone() }); - let spec = snapshot_spec(&name, inputs); - snapshots.create(&spec, None).await.map_err(|error| { - crate::Error::context(format!("Failed to create snapshot '{name}'"), error) - })? - }; - wait_for_active_snapshot(snapshots, &id, &name).await?; + crate::Error::context(format!("Failed to ensure snapshot '{name}'"), error) + })?; Ok((id, name)) } @@ -409,37 +372,6 @@ fn snapshot_spec(name: &str, inputs: &SnapshotInputs<'_>) -> SnapshotSpec { SnapshotSpec::new(source).name(name).resources(resources) } -/// Polls a snapshot until it is active, with exponential back-off, or fails -/// when it errors or the budget runs out. -async fn wait_for_active_snapshot( - snapshots: &dyn SnapshotProvider, - id: &SnapshotId, - name: &str, -) -> crate::Result<()> { - let mut delay = Duration::from_secs(2); - let max_delay = Duration::from_secs(30); - let deadline = time::Instant::now() + DAYTONA_SNAPSHOT_ACTIVE_TIMEOUT; - while time::Instant::now() < deadline { - time::sleep(delay).await; - let status = snapshots.get(id).await.map_err(|error| { - crate::Error::context(format!("Failed to poll snapshot '{name}'"), error) - })?; - match status.state { - SnapshotState::Active => return Ok(()), - SnapshotState::Error | SnapshotState::Deleting => { - return Err(crate::Error::message(format!( - "Snapshot '{name}' failed: {}", - status.error_reason.unwrap_or_default() - ))); - } - _ => delay = (delay * 2).min(max_delay), - } - } - Err(crate::Error::message(format!( - "Timed out waiting for snapshot '{name}' to become active" - ))) -} - /// Prepares a Daytona create: the snapshot first, then the spec naming it. pub(crate) struct DaytonaCreatePlan { provider: Arc, @@ -477,8 +409,11 @@ impl CreatePlan for DaytonaCreatePlan { let (snapshot_id, snapshot_name) = match snapshot_inputs(&self.options) { Some(inputs) => { let started = time::Instant::now(); - let result = - ensure_snapshot(self.provider.as_ref(), &self.api_key, &inputs, emit).await; + // The driver finds, activates, builds, or waits for the + // snapshot as needed; fabro reports the step around it. + let name = snapshot_identity::snapshot_name(&self.api_key, &inputs)?; + emit(SandboxEvent::SnapshotCreating { name }); + let result = ensure_snapshot(self.provider.as_ref(), &self.api_key, &inputs).await; match result { Ok((id, name)) => { emit(SandboxEvent::SnapshotReady { @@ -594,14 +529,9 @@ mod tests { Some("fabro-01HY0000000000000000000000") ); assert_eq!(spec.working_directory.as_deref(), Some(WORKING_DIRECTORY)); - assert_eq!( - spec.labels.get("sh.fabro.managed").map(String::as_str), - Some("true") - ); - assert_eq!( - spec.labels.get("sh.fabro.run_id").map(String::as_str), - Some("01HY0000000000000000000000") - ); + // Fabro's ownership labels are stamped by the scope the provider is + // connected through, not by the spec. + assert!(!spec.labels.contains_key("sh.fabro.managed")); assert_eq!( spec.labels.get("team").map(String::as_str), Some("platform") @@ -634,7 +564,6 @@ mod tests { ); assert!(matches!(explicit.network, NetworkPolicy::Block)); assert!(explicit.name.is_none()); - assert!(!explicit.labels.contains_key("sh.fabro.run_id")); let options = SandboxOptions { auto_stop: Some(Duration::ZERO), diff --git a/lib/components/fabro-sandbox/src/docker.rs b/lib/components/fabro-sandbox/src/docker.rs index 45e634d3a..87cb66f57 100644 --- a/lib/components/fabro-sandbox/src/docker.rs +++ b/lib/components/fabro-sandbox/src/docker.rs @@ -107,13 +107,9 @@ mod tests { Some("fabro-run-01HY0000000000000000000000") ); assert_eq!(spec.working_directory.as_deref(), Some(WORKING_DIRECTORY)); - assert_eq!( - spec.labels.get("sh.fabro.managed").map(String::as_str), - Some("true") - ); - assert_eq!( - spec.labels.get("sh.fabro.run_id").map(String::as_str), - Some("01HY0000000000000000000000") + assert!( + !spec.labels.contains_key("sh.fabro.managed"), + "ownership labels come from the scope the provider is connected through" ); assert_eq!(spec.env.get("FOO").map(String::as_str), Some("bar")); assert_eq!(spec.resources.cpu_cores, Some(2)); @@ -132,6 +128,5 @@ mod tests { SandboxSource::Image { reference } if reference == DEFAULT_IMAGE )); assert!(spec.name.is_none()); - assert!(!spec.labels.contains_key("sh.fabro.run_id")); } } diff --git a/lib/components/fabro-sandbox/src/exec.rs b/lib/components/fabro-sandbox/src/exec.rs index 688aecd2c..1aad8a396 100644 --- a/lib/components/fabro-sandbox/src/exec.rs +++ b/lib/components/fabro-sandbox/src/exec.rs @@ -1,15 +1,15 @@ //! Fabro's command execution policy over the sandbox-driver [`Exec`] facet. //! -//! The driver sends signals; fabro decides when. A command runs as Bash -//! source under `bash -c` with `BASH_ENV` blanked, and ends in one of three -//! ways fabro controls: +//! A command runs as Bash source under `bash -c` with `BASH_ENV` blanked, +//! and ends in one of three ways: //! -//! - **timeout**: fabro's own timer fires, the process group gets `TERM`, and -//! after [`SandboxExec::stop_grace`] it gets `KILL`. The result reports -//! [`CommandTermination::TimedOut`]. The driver's hard timeout is disabled so -//! the graceful ladder always runs first. -//! - **cancellation**: the caller's [`CancellationToken`] runs the same ladder -//! and reports [`CommandTermination::Cancelled`]. +//! - **timeout**: the spec's timeout fires and the provider runs the stop +//! ladder fabro asks for — `TERM`, then `KILL` after +//! [`SandboxExec::stop_grace`]. The result reports +//! [`CommandTermination::TimedOut`]. +//! - **cancellation**: the caller's [`CancellationToken`] is the `term` stop; +//! the provider escalates to `KILL` after the same grace. The result reports +//! [`CommandTermination::Cancelled`]. //! - **exit**: the process ended on its own. //! //! Output is drained regardless of the retention cap, redacted only when a @@ -19,9 +19,7 @@ //! matching what the Host provider already does for inherited variables. use std::collections::HashMap; -use std::future; -use std::pin::pin; -use std::sync::{Arc, Mutex, PoisonError}; +use std::sync::Arc; use std::time::{Duration, Instant}; use async_trait::async_trait; @@ -31,7 +29,6 @@ use sandbox_driver::{ BASH_ENV_VAR, CaptureStats, Exec, ExecControls, ExecSpec, OutputStream, SpawnSpec, StdioProcessHandle as DriverStdioProcessHandle, Termination, TransportError, }; -use tokio::time; use tokio_util::sync::CancellationToken; use crate::sandbox::{ @@ -117,7 +114,8 @@ impl<'a> SandboxExec<'a> { self } - /// Time between `TERM` and `KILL` when a command is stopped. + /// Time between `TERM` and `KILL` when a command is stopped; the + /// provider runs the ladder. #[must_use] pub fn with_stop_grace(mut self, stop_grace: Duration) -> Self { self.stop_grace = stop_grace; @@ -173,7 +171,12 @@ impl<'a> SandboxExec<'a> { } = request; let started = Instant::now(); - let mut spec = ExecSpec::bash(command).no_timeout(); + let mut spec = ExecSpec::bash(command) + .no_timeout() + .stop_grace(self.stop_grace); + if let Some(timeout_ms) = timeout_ms { + spec = spec.timeout(Duration::from_millis(timeout_ms)); + } if let Some(dir) = working_dir.or(self.working_dir.as_deref()) { spec = spec.working_dir(dir); } @@ -184,10 +187,11 @@ impl<'a> SandboxExec<'a> { spec = spec.stdin(bytes); } - let ladder = StopLadder::new(self.stop_grace); + // The caller's cancellation is the `term` stop; the provider runs + // the grace and the `kill` itself. let controls = ExecControls { - term: Some(ladder.term.clone()), - kill: Some(ladder.kill.clone()), + term: cancel_token, + kill: None, stdin: None, sink: output_callback.map(adapt_output_callback), retained_output_limit: Some( @@ -195,17 +199,9 @@ impl<'a> SandboxExec<'a> { ), }; - let mut escalation = - pin!(ladder.drive(timeout_ms.map(Duration::from_millis), cancel_token)); - let mut running = pin!(self.exec.run_streaming(&spec, controls)); - let streaming = loop { - tokio::select! { - result = &mut running => break result?, - () = &mut escalation => {} - } - }; + let streaming = self.exec.run_streaming(&spec, controls).await?; - let termination = map_termination(streaming.result.termination, ladder.cause()); + let termination = map_termination(streaming.result.termination); let duration_ms = elapsed_ms(started); Ok(ExecStreamingResult { result: ExecResult { @@ -280,76 +276,15 @@ impl<'a> SandboxExec<'a> { } } -/// Why fabro stopped a command, recorded when the ladder starts. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum StopCause { - TimedOut, - Cancelled, -} - -/// The TERM, grace, KILL escalation. Fabro fires `term`, waits `grace`, -/// then fires `kill`; the provider only delivers the signals. -struct StopLadder { - term: CancellationToken, - kill: CancellationToken, - grace: Duration, - cause: Mutex>, -} - -impl StopLadder { - fn new(grace: Duration) -> Self { - Self { - term: CancellationToken::new(), - kill: CancellationToken::new(), - grace, - cause: Mutex::new(None), - } - } - - fn cause(&self) -> Option { - *self.cause.lock().unwrap_or_else(PoisonError::into_inner) - } - - /// Waits for the timeout or the caller's cancellation, runs the ladder, - /// then never resolves so it can sit in a `select!` beside the command. - async fn drive(&self, timeout: Option, cancel_token: Option) { - let cause = tokio::select! { - () = sleep_or_never(timeout) => StopCause::TimedOut, - () = cancelled_or_never(cancel_token.as_ref()) => StopCause::Cancelled, - }; - *self.cause.lock().unwrap_or_else(PoisonError::into_inner) = Some(cause); - self.term.cancel(); - time::sleep(self.grace).await; - self.kill.cancel(); - future::pending::<()>().await; - } -} - -async fn sleep_or_never(timeout: Option) { - match timeout { - Some(timeout) => time::sleep(timeout).await, - None => future::pending().await, - } -} - -async fn cancelled_or_never(token: Option<&CancellationToken>) { - match token { - Some(token) => token.cancelled().await, - None => future::pending().await, - } -} - -/// The driver reports which signal ended the command; fabro reports why it -/// sent it. A stop the driver saw without fabro asking for one (a foreign -/// `kill`, a provider-side abort) reads as cancelled: the command did not -/// finish and fabro did not time it out. -fn map_termination(termination: Termination, cause: Option) -> CommandTermination { +/// The driver says how the command ended; fabro's vocabulary has two stops. +/// A timeout is the provider's deadline (the ladder ran for it); a +/// cancelled or killed command was stopped by the caller's token, by a +/// foreign `kill`, or by a provider-side abort — it did not finish and no +/// deadline passed. +fn map_termination(termination: Termination) -> CommandTermination { match termination { Termination::TimedOut => CommandTermination::TimedOut, - Termination::Cancelled | Termination::Killed => match cause { - Some(StopCause::TimedOut) => CommandTermination::TimedOut, - Some(StopCause::Cancelled) | None => CommandTermination::Cancelled, - }, + Termination::Cancelled | Termination::Killed => CommandTermination::Cancelled, // `Exited`, or a provider that could not tell how the command ended. // Nothing asserts success here: `exit_code` is whatever was observed // and `is_success` still requires `Some(0)`. @@ -410,7 +345,7 @@ impl StdioProcessControl for DriverStdioControl { async fn wait(&self) -> crate::Result { let (termination, exit_code) = self.handle.wait().await; - let termination = map_termination(termination, None); + let termination = map_termination(termination); Ok(StdioProcessTermination { termination, exit_code: exit_code_for(termination, exit_code), @@ -420,12 +355,12 @@ impl StdioProcessControl for DriverStdioControl { #[cfg(test)] mod tests { - use std::sync::Arc; + use std::sync::{Arc, Mutex}; use sandbox_driver::{SandboxProvider as _, SandboxSource, SandboxSpec}; use sandbox_driver_host::HostProvider; - use tokio::fs; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; + use tokio::{fs, time}; use super::*; @@ -740,21 +675,21 @@ mod tests { } #[test] - fn termination_mapping_reports_fabro_intent_over_driver_signal() { + fn termination_mapping_reads_the_drivers_verdict() { assert_eq!( - map_termination(Termination::Killed, Some(StopCause::TimedOut)), + map_termination(Termination::TimedOut), CommandTermination::TimedOut ); assert_eq!( - map_termination(Termination::Cancelled, Some(StopCause::Cancelled)), + map_termination(Termination::Cancelled), CommandTermination::Cancelled ); assert_eq!( - map_termination(Termination::Killed, None), + map_termination(Termination::Killed), CommandTermination::Cancelled ); assert_eq!( - map_termination(Termination::Exited, Some(StopCause::TimedOut)), + map_termination(Termination::Exited), CommandTermination::Exited ); } diff --git a/lib/components/fabro-sandbox/src/git_retry.rs b/lib/components/fabro-sandbox/src/git_retry.rs index 288f5c9fe..fd8047de6 100644 --- a/lib/components/fabro-sandbox/src/git_retry.rs +++ b/lib/components/fabro-sandbox/src/git_retry.rs @@ -13,6 +13,9 @@ //! Retries reuse the same token on purpose. Replication of a given token only //! makes progress, so each attempt strictly improves the odds, while //! re-minting would restart the replication clock. +//! +//! The driver classifies what a failure was ([`GitFailureKind`]); this module +//! decides what the class means for the credentials in hand. use std::future::Future; use std::time::Duration; @@ -24,6 +27,7 @@ use fabro_github::token_source::{REFRESH_MARGIN, TokenProvenance}; use fabro_types::SandboxProviderKind; pub use fabro_types::run_event::GitPushRetryReason as GitRetryReason; use fabro_util::backoff::BackoffPolicy; +use sandbox_driver::GitFailureKind; use tokio::time; /// How long after its mint a token is presumed to still be replicating to @@ -82,95 +86,43 @@ impl CredentialContext { } } -/// Message fragments that mean the operation failed on infrastructure. -/// -/// These are safe to retry whether or not the operation was authenticated. -const TRANSIENT_HINTS: &[&str] = &[ - "could not resolve host", - "temporary failure in name resolution", - "connection refused", - "connection reset", - "connection timed out", - "timed out", - "network is unreachable", - "no route to host", - "tls handshake", - "early eof", - "rpc failed", - "unexpected disconnect", - "the remote end hung up unexpectedly", - "index-pack failed", - "service unavailable", - "gateway timeout", - "too many requests", - "rate limit", -]; - -/// Message fragments GitHub uses when a token is not yet visible. -/// -/// Only meaningful when the operation carried credentials. The same lag -/// surfaces as 404 or as an auth failure depending on which endpoint answers -/// first. -const TOKEN_REPLICATION_HINTS: &[&str] = &[ - "repository not found", - "authentication failed", - "invalid username or password", - "bad credentials", - // git CLI over HTTP. - "the requested url returned error: 401", - "the requested url returned error: 403", - "the requested url returned error: 404", - // libgit2 (the run-metadata writer pushes through git2). - "unexpected http status code: 401", - "unexpected http status code: 403", - "unexpected http status code: 404", -]; - /// Whether a failure message has the 404/auth-failure shape GitHub produces /// for both token-replication lag and a drifted or missing embedded token. pub(crate) fn matches_auth_failure_hints(message: &str) -> bool { - let lower = message.to_ascii_lowercase(); - TOKEN_REPLICATION_HINTS - .iter() - .any(|hint| lower.contains(hint)) + GitFailureKind::from_message(message) == GitFailureKind::AuthRejected } pub(crate) fn output_matches_auth_failure_hints(stderr: &str, stdout: &str) -> bool { - matches_auth_failure_hints(stderr) || matches_auth_failure_hints(stdout) + GitFailureKind::from_output(stderr.as_bytes(), stdout.as_bytes()) + == GitFailureKind::AuthRejected } -/// Classify a failed git operation by its rendered message. +/// What a classified git failure means for retrying with these credentials. /// -/// `cred` gates the reading of 404/auth-failure messages: a fresh App token -/// retries as replication lag, a mature one as transient infrastructure, and -/// a static credential (or none) fails fast because waiting cannot make it -/// valid. -pub(crate) fn classify_message(message: &str, cred: CredentialContext) -> GitMessageClass { - let lower = message.to_ascii_lowercase(); - - if TRANSIENT_HINTS.iter().any(|hint| lower.contains(hint)) { - return GitMessageClass::Retry(GitRetryReason::TransientInfra); - } - if TOKEN_REPLICATION_HINTS - .iter() - .any(|hint| lower.contains(hint)) - { - return match cred { +/// The driver reads the failure; fabro decides. A remote that could not +/// be reached is retried whatever the credential. A rejected credential +/// is retried only while a just-minted App token may still be replicating +/// (`FreshApp`), retried as a service blip for a mature App token, and +/// fails fast for a static credential or none, because waiting cannot make +/// those valid. Every other class is permanent. +pub(crate) fn decide(kind: GitFailureKind, cred: CredentialContext) -> GitMessageClass { + match kind { + GitFailureKind::RemoteUnavailable => GitMessageClass::Retry(GitRetryReason::TransientInfra), + GitFailureKind::AuthRejected => match cred { CredentialContext::FreshApp => GitMessageClass::Retry(GitRetryReason::TokenReplication), CredentialContext::MatureApp => GitMessageClass::Retry(GitRetryReason::TransientInfra), CredentialContext::Static | CredentialContext::None => GitMessageClass::Permanent, - }; + }, + GitFailureKind::AccessDenied + | GitFailureKind::RefNotFound + | GitFailureKind::TargetExists => GitMessageClass::Permanent, + _ => GitMessageClass::Unknown, } - let permanent = lower.contains("could not read username") - || lower.contains("terminal prompts disabled") - || lower.contains("permission denied") - || (lower.contains("permission to") && lower.contains("denied")) - || (lower.contains("destination path") && lower.contains("already exists")) - || (lower.contains("remote branch") && lower.contains("not found")); - if permanent { - return GitMessageClass::Permanent; - } - GitMessageClass::Unknown +} + +/// Classify a failed git operation by its rendered message. +pub(crate) fn classify_message(message: &str, cred: CredentialContext) -> GitMessageClass { + decide(GitFailureKind::from_message(message), cred) } pub(crate) fn classify_output( @@ -178,12 +130,10 @@ pub(crate) fn classify_output( stdout: &str, cred: CredentialContext, ) -> GitMessageClass { - let by_stderr = classify_message(stderr, cred); - if by_stderr == GitMessageClass::Unknown { - classify_message(stdout, cred) - } else { - by_stderr - } + decide( + GitFailureKind::from_output(stderr.as_bytes(), stdout.as_bytes()), + cred, + ) } /// Classify a rendered git failure message, returning the retry reason when @@ -196,33 +146,17 @@ pub fn classify_failure(message: &str, cred: CredentialContext) -> Option Option { match error { - sandbox_driver::Error::Exec(failure) => classify_output( - &String::from_utf8_lossy(failure.stderr()), - &String::from_utf8_lossy(failure.stdout()), - cred, - ) - .retry_reason(), - sandbox_driver::Error::Provider(provider) => { - match classify_message(&provider.message, cred) { - GitMessageClass::Retry(reason) => Some(reason), - GitMessageClass::Permanent => None, - GitMessageClass::Unknown => { - provider.retryable.then_some(GitRetryReason::TransientInfra) - } - } - } + sandbox_driver::Error::Git(failure) => decide(failure.kind(), cred).retry_reason(), sandbox_driver::Error::RateLimited { .. } | sandbox_driver::Error::Overloaded { .. } => { Some(GitRetryReason::TransientInfra) } diff --git a/lib/components/fabro-sandbox/src/managed_labels.rs b/lib/components/fabro-sandbox/src/managed_labels.rs index f0ecc71fd..81e81586f 100644 --- a/lib/components/fabro-sandbox/src/managed_labels.rs +++ b/lib/components/fabro-sandbox/src/managed_labels.rs @@ -1,63 +1,32 @@ -use std::collections::{BTreeMap, HashMap}; +//! The labels that mark a sandbox as fabro's. +//! +//! Providers share a daemon or an organization with every other +//! application, so a persisted id is trusted only when the sandbox behind +//! it still carries fabro's labels. The driver's ownership scope stamps them +//! on every sandbox fabro creates, narrows every listing to them, and +//! refuses to attach to or delete a sandbox without them; this module only +//! says which labels those are. -use fabro_types::{RunId, SandboxProviderKind}; +use fabro_types::RunId; +use sandbox_driver::Ownership; pub(crate) const MANAGED_LABEL: &str = "sh.fabro.managed"; pub(crate) const MANAGED_LABEL_VALUE: &str = "true"; pub(crate) const RUN_ID_LABEL: &str = "sh.fabro.run_id"; -/// True when the provided label map carries the Fabro managed sentinel. -pub(crate) fn is_managed(labels: &BTreeMap) -> bool { - labels.get(MANAGED_LABEL).map(String::as_str) == Some(MANAGED_LABEL_VALUE) -} - -/// Refuses a sandbox fabro did not create, or one created for another run. -/// -/// Providers share a daemon or an organization with every other -/// application, so a persisted id is trusted only when the sandbox behind -/// it still carries fabro's labels. -pub(crate) fn verify_managed( - kind: &SandboxProviderKind, - sandbox_id: &str, - labels: &BTreeMap, - run_id: Option<&RunId>, -) -> crate::Result<()> { - if !is_managed(labels) { - return Err(crate::Error::message(format!( - "Refusing to operate on {kind} sandbox '{sandbox_id}' because it is missing label {MANAGED_LABEL}={MANAGED_LABEL_VALUE}" - ))); - } - if let Some(run_id) = run_id { - let actual = labels.get(RUN_ID_LABEL).map(String::as_str); - let expected = run_id.to_string(); - if actual != Some(expected.as_str()) { - return Err(crate::Error::message(format!( - "Refusing to operate on {kind} sandbox '{sandbox_id}' because label {RUN_ID_LABEL}={actual:?} does not match run {run_id}" - ))); - } - } - Ok(()) -} - -pub(crate) fn merge_for_run( - user_labels: Option<&HashMap>, - run_id: Option<&RunId>, -) -> HashMap { - let mut labels = user_labels.cloned().unwrap_or_default(); - insert_for_run(&mut labels, run_id); - labels -} - -fn insert_for_run(labels: &mut HashMap, run_id: Option<&RunId>) { - labels.insert(MANAGED_LABEL.to_string(), MANAGED_LABEL_VALUE.to_string()); - if let Some(run_id) = run_id { - labels.insert(RUN_ID_LABEL.to_string(), run_id.to_string()); +/// Fabro's ownership of a sandbox: everything fabro manages, narrowed to +/// one run when `run_id` is known. +pub(crate) fn ownership(run_id: Option<&RunId>) -> Ownership { + let ownership = Ownership::label(MANAGED_LABEL, MANAGED_LABEL_VALUE); + match run_id { + Some(run_id) => ownership.and_label(RUN_ID_LABEL, run_id.to_string()), + None => ownership, } } #[cfg(test)] mod tests { - use std::collections::HashMap; + use std::collections::BTreeMap; use fabro_types::RunId; @@ -77,47 +46,28 @@ mod tests { } #[test] - fn managed_labels_include_run_id_when_present() { + fn ownership_requires_fabro_and_the_run_when_known() { let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); - let labels = merge_for_run(None, Some(&run_id)); + let mut labels = BTreeMap::new(); + assert!(!ownership(None).owns(&labels)); + labels.insert(MANAGED_LABEL.to_string(), "true".to_string()); + assert!(ownership(None).owns(&labels)); + assert!(!ownership(Some(&run_id)).owns(&labels)); + labels.insert(RUN_ID_LABEL.to_string(), run_id.to_string()); + assert!(ownership(Some(&run_id)).owns(&labels)); - assert_eq!(labels.get(MANAGED_LABEL).map(String::as_str), Some("true")); - assert_eq!( - labels.get(RUN_ID_LABEL).map(String::as_str), - Some("01HY0000000000000000000000") - ); - assert!(is_managed(&labels.clone().into_iter().collect())); - } - - #[test] - fn managed_labels_override_reserved_user_labels() { - let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); - let user_labels = HashMap::from([ + // Stamping overrides whatever a caller put under the reserved keys. + let mut given = BTreeMap::from([ ("team".to_string(), "platform".to_string()), (MANAGED_LABEL.to_string(), "false".to_string()), (RUN_ID_LABEL.to_string(), "wrong".to_string()), ]); - - let labels = merge_for_run(Some(&user_labels), Some(&run_id)); - - assert_eq!(labels.get("team").map(String::as_str), Some("platform")); - assert_eq!(labels.get(MANAGED_LABEL).map(String::as_str), Some("true")); + ownership(Some(&run_id)).stamp(&mut given); + assert_eq!(given.get("team").map(String::as_str), Some("platform")); + assert_eq!(given.get(MANAGED_LABEL).map(String::as_str), Some("true")); assert_eq!( - labels.get(RUN_ID_LABEL).map(String::as_str), + given.get(RUN_ID_LABEL).map(String::as_str), Some("01HY0000000000000000000000") ); } - - #[test] - fn verify_managed_requires_fabro_ownership_and_matching_run() { - let run_id: RunId = "01HY0000000000000000000000".parse().unwrap(); - let kind = SandboxProviderKind::DOCKER; - let mut labels = BTreeMap::new(); - assert!(verify_managed(&kind, "c1", &labels, None).is_err()); - labels.insert(MANAGED_LABEL.to_string(), "true".to_string()); - assert!(verify_managed(&kind, "c1", &labels, None).is_ok()); - assert!(verify_managed(&kind, "c1", &labels, Some(&run_id)).is_err()); - labels.insert(RUN_ID_LABEL.to_string(), run_id.to_string()); - assert!(verify_managed(&kind, "c1", &labels, Some(&run_id)).is_ok()); - } } diff --git a/lib/components/fabro-sandbox/src/options.rs b/lib/components/fabro-sandbox/src/options.rs index b23848a31..f192e534a 100644 --- a/lib/components/fabro-sandbox/src/options.rs +++ b/lib/components/fabro-sandbox/src/options.rs @@ -19,8 +19,6 @@ use sandbox_driver::{ Capabilities, NetworkPolicy, Resources, SandboxSource, SandboxSpec as DriverSpec, }; -use crate::managed_labels; - /// What an environment asks of a sandbox, provider-neutral. #[derive(Clone, Debug, Default)] pub struct SandboxOptions { @@ -159,9 +157,9 @@ pub fn local_working_directory_from_environment( /// The driver spec every provider starts from: the environment's source /// (an image, a Dockerfile, or a managed directory when it names -/// neither), the run's name and labels, the variables, resources, and -/// network policy. A bundled provider's overlay adjusts what its backend -/// needs. +/// neither), the run's name, the environment's labels, variables, +/// resources, and network policy. A bundled provider's overlay adjusts +/// what its backend needs, and the ownership scope adds fabro's labels. pub(crate) fn base_spec(options: &SandboxOptions, run_id: Option<&RunId>) -> DriverSpec { let source = match (&options.image, &options.dockerfile) { (Some(reference), _) => SandboxSource::Image { @@ -178,17 +176,9 @@ pub(crate) fn base_spec(options: &SandboxOptions, run_id: Option<&RunId>) -> Dri if let Some(run_id) = run_id { spec = spec.name(run_name(run_id)); } - let user_labels: std::collections::HashMap = options - .labels - .iter() - .map(|(key, value)| (key.clone(), value.clone())) - .collect(); - let mut labels: Vec<(String, String)> = - managed_labels::merge_for_run(Some(&user_labels), run_id) - .into_iter() - .collect(); - labels.sort(); - for (key, value) in labels { + // The environment's labels; fabro's ownership labels are stamped by the + // ownership scope the provider is connected through. + for (key, value) in &options.labels { spec = spec.label(key, value); } for (key, value) in &options.env { @@ -279,13 +269,9 @@ mod tests { spec.labels.get("team").map(String::as_str), Some("platform") ); - assert_eq!( - spec.labels.get("sh.fabro.managed").map(String::as_str), - Some("true") - ); - assert_eq!( - spec.labels.get("sh.fabro.run_id").map(String::as_str), - Some("01HY0000000000000000000000") + assert!( + !spec.labels.contains_key("sh.fabro.managed"), + "ownership labels come from the scope, not the environment" ); assert!(matches!(spec.network, NetworkPolicy::AllowAll)); } @@ -319,7 +305,6 @@ mod tests { assert_eq!(spec.resources.memory_mb, Some(3815)); assert!(matches!(spec.network, NetworkPolicy::Block)); assert!(spec.name.is_none()); - assert!(!spec.labels.contains_key("sh.fabro.run_id")); } #[test] diff --git a/lib/components/fabro-sandbox/src/provider/driver.rs b/lib/components/fabro-sandbox/src/provider/driver.rs index d7aba5332..069469b77 100644 --- a/lib/components/fabro-sandbox/src/provider/driver.rs +++ b/lib/components/fabro-sandbox/src/provider/driver.rs @@ -3,21 +3,24 @@ //! Lists and looks up the sandboxes fabro created, identified by fabro's //! own `sh.fabro.managed` label. The driver marks every sandbox it creates //! with its own label too, but that covers every application on the same -//! daemon or account; fabro filters on its label and refuses to delete a -//! sandbox that does not carry it. +//! daemon or account; the provider is connected through the driver's +//! ownership scope, which lists only fabro's sandboxes and refuses to +//! attach to or delete any other. use std::sync::Arc; use async_trait::async_trait; use fabro_types::settings::server::ServerSandboxProviderSettings; use fabro_types::{SandboxInfo, SandboxProviderKind}; -use sandbox_driver::{SandboxFilter, SandboxId, SandboxProvider as DriverProvider}; +use sandbox_driver::{ + Error as DriverError, OwnedProvider, SandboxFilter, SandboxId, + SandboxProvider as DriverProvider, +}; use tokio::sync::OnceCell; use super::SandboxProvider; -use crate::details; use crate::driver::{ConnectedProvider, ProviderConnectOptions, connect_provider}; -use crate::managed_labels::{self, MANAGED_LABEL, MANAGED_LABEL_VALUE}; +use crate::{details, managed_labels}; /// How the driver provider behind the inventory is obtained. enum Connection { @@ -44,7 +47,7 @@ impl DriverInventoryProvider { pub fn new(connected: ConnectedProvider) -> Self { Self { kind: connected.kind, - connection: Connection::Connected(connected.provider), + connection: Connection::Connected(owned(connected.provider)), } } @@ -74,7 +77,7 @@ impl DriverInventoryProvider { .get_or_try_init(|| async { connect_provider(&self.kind, &lazy.settings, &lazy.options) .await - .map(|connected| connected.provider) + .map(|connected| owned(connected.provider)) .map_err(|error| { crate::Error::context( format!("Failed to connect to the {} provider", self.kind), @@ -87,14 +90,6 @@ impl DriverInventoryProvider { } } - fn managed_filter() -> SandboxFilter { - let mut filter = SandboxFilter::default(); - filter - .labels - .insert(MANAGED_LABEL.to_string(), MANAGED_LABEL_VALUE.to_string()); - filter - } - async fn describe_managed( &self, id: &str, @@ -105,7 +100,9 @@ impl DriverInventoryProvider { }; let handle = match self.provider().await?.attach(&sandbox_id, None).await { Ok(handle) => handle, - Err(sandbox_driver::Error::NotFound { .. }) => return Ok(None), + // Unknown to the provider, or not fabro's: neither is in the + // inventory. + Err(DriverError::NotFound { .. } | DriverError::NotOwned { .. }) => return Ok(None), Err(error) => { return Err(crate::Error::context( format!("Failed to look up {} sandbox '{id}'", self.kind), @@ -119,15 +116,21 @@ impl DriverInventoryProvider { error, ) })?; - if status.state == sandbox_driver::SandboxState::Deleted - || !managed_labels::is_managed(&status.labels) - { + if status.state == sandbox_driver::SandboxState::Deleted { return Ok(None); } Ok(Some(status)) } } +/// The provider narrowed to fabro's sandboxes. +fn owned(provider: Arc) -> Arc { + Arc::new(OwnedProvider::new( + provider, + managed_labels::ownership(None), + )) +} + #[async_trait] impl SandboxProvider for DriverInventoryProvider { fn kind(&self) -> SandboxProviderKind { @@ -138,16 +141,13 @@ impl SandboxProvider for DriverInventoryProvider { let statuses = self .provider() .await? - .list(&Self::managed_filter()) + .list(&SandboxFilter::default()) .await .map_err(|error| { crate::Error::context(format!("Failed to list {} sandboxes", self.kind), error) })?; Ok(statuses .iter() - // The filter is a request; a provider that cannot filter on - // labels returns everything, so the label is checked again. - .filter(|status| managed_labels::is_managed(&status.labels)) .map(|status| details::info_from_status(&self.kind, status)) .collect()) } @@ -160,33 +160,25 @@ impl SandboxProvider for DriverInventoryProvider { } async fn delete(&self, id: &str) -> crate::Result<()> { - let Some(status) = self.describe_managed(id).await? else { - // Missing, already deleted, or not fabro's: the first two are - // idempotent successes and the third must never be deleted here, - // so distinguish them for the caller. - if let Ok(sandbox_id) = SandboxId::try_new(id) { - if let Ok(handle) = self.provider().await?.attach(&sandbox_id, None).await { - let status = handle.describe().await?; - if status.state != sandbox_driver::SandboxState::Deleted { - return Err(crate::Error::message(format!( - "Refusing to delete {} sandbox '{id}' because it is missing label {MANAGED_LABEL}={MANAGED_LABEL_VALUE}", - self.kind - ))); - } - } - } + // Missing or already deleted is an idempotent success; the scope + // refuses a sandbox that is not fabro's, which must never be + // deleted here. + let Ok(sandbox_id) = SandboxId::try_new(id) else { return Ok(()); }; - self.provider() - .await? - .delete(&status.id, None) - .await - .map_err(|error| { - crate::Error::context( - format!("Failed to delete {} sandbox '{id}'", self.kind), - error, - ) - }) + match self.provider().await?.delete(&sandbox_id, None).await { + Ok(()) => Ok(()), + Err(DriverError::NotOwned { .. }) => Err(crate::Error::message(format!( + "Refusing to delete {} sandbox '{id}' because it is missing label {}={}", + self.kind, + managed_labels::MANAGED_LABEL, + managed_labels::MANAGED_LABEL_VALUE + ))), + Err(error) => Err(crate::Error::context( + format!("Failed to delete {} sandbox '{id}'", self.kind), + error, + )), + } } } @@ -211,7 +203,8 @@ mod tests { let (inventory, host) = inventory(); let ours = host .create( - &SandboxSpec::new(SandboxSource::HostDirectory).label(MANAGED_LABEL, "true"), + &SandboxSpec::new(SandboxSource::HostDirectory) + .label(managed_labels::MANAGED_LABEL, "true"), None, ) .await diff --git a/lib/components/fabro-sandbox/src/provider_sandbox.rs b/lib/components/fabro-sandbox/src/provider_sandbox.rs index 44a66ca82..e1b5c008e 100644 --- a/lib/components/fabro-sandbox/src/provider_sandbox.rs +++ b/lib/components/fabro-sandbox/src/provider_sandbox.rs @@ -13,7 +13,7 @@ use std::sync::Arc; use fabro_github::GitHubCredentials; use fabro_types::{BundledProvider, RunId, SandboxProviderKind}; -use sandbox_driver::{SandboxId, SandboxProvider}; +use sandbox_driver::{OwnedProvider, SandboxId, SandboxProvider}; use crate::driver::{ProviderAccess, connect_provider}; use crate::driver_sandbox::{DriverSandbox, LayoutSource, RepoWorkspace}; @@ -49,7 +49,7 @@ pub async fn provider_sandbox( options.clone_depth, github_app, )?; - let provider = connect(&kind, access).await?; + let provider = connect(&kind, access, run_id.as_ref()).await?; let base = options::base_spec(&options, run_id.as_ref()); Ok(match kind.bundled() { Some(BundledProvider::Docker) => { @@ -88,7 +88,8 @@ pub async fn provider_sandbox( /// The sandbox must carry fabro's managed label and, when a run id is /// known, the matching run label: the provider shares its backend with /// every other application, and fabro never operates on a sandbox it did -/// not create. +/// not create. The ownership scope the provider is connected through +/// refuses anything else. pub async fn attach_provider_sandbox( kind: SandboxProviderKind, access: &ProviderAccess, @@ -98,7 +99,7 @@ pub async fn attach_provider_sandbox( clone_origin_url: Option, run_id: Option, ) -> crate::Result { - let provider = connect(&kind, access).await?; + let provider = connect(&kind, access, run_id.as_ref()).await?; let id = SandboxId::try_new(sandbox_id) .map_err(|error| crate::Error::context(format!("Invalid {kind} sandbox id"), error))?; let handle = provider.attach(&id, None).await.map_err(|error| { @@ -108,7 +109,6 @@ pub async fn attach_provider_sandbox( ) })?; let status = handle.describe().await?; - managed_labels::verify_managed(&kind, sandbox_id, &status.labels, run_id.as_ref())?; let workspace = RepoWorkspace::attached( layout_source(&kind), repo_cloned, @@ -151,14 +151,18 @@ pub(crate) fn layout_source(kind: &SandboxProviderKind) -> LayoutSource { pub(crate) async fn connect_bundled_docker( access: &ProviderAccess, ) -> crate::Result> { - connect(&SandboxProviderKind::DOCKER, access).await + connect(&SandboxProviderKind::DOCKER, access, None).await } const MISSING_DAYTONA_CREDENTIALS: &str = "Daytona sandboxes require DAYTONA_API_KEY in the vault; run `fabro secret set DAYTONA_API_KEY`"; +/// The provider for `kind`, scoped to the sandboxes fabro owns — narrowed to +/// one run when `run_id` is known — so creates carry fabro's labels and +/// attaches to anything else are refused. async fn connect( kind: &SandboxProviderKind, access: &ProviderAccess, + run_id: Option<&RunId>, ) -> crate::Result> { if kind.bundled() == Some(BundledProvider::Daytona) && access.daytona.is_none() { return Err(crate::Error::message(MISSING_DAYTONA_CREDENTIALS)); @@ -168,10 +172,13 @@ async fn connect( "sandbox provider `{kind}` is not configured; add [server.sandbox.providers.{kind}] to settings.toml" )) })?; - connect_provider(kind, &settings, &access.connect_options()) + let connected = connect_provider(kind, &settings, &access.connect_options()) .await - .map(|connected| connected.provider) .map_err(|error| { crate::Error::context(format!("Failed to connect to the {kind} provider"), error) - }) + })?; + Ok(Arc::new(OwnedProvider::new( + connected.provider, + managed_labels::ownership(run_id), + ))) } diff --git a/lib/components/fabro-workflow/tests/it/cp_integration.rs b/lib/components/fabro-workflow/tests/it/cp_integration.rs index 4087efb8d..d0acf5ca7 100644 --- a/lib/components/fabro-workflow/tests/it/cp_integration.rs +++ b/lib/components/fabro-workflow/tests/it/cp_integration.rs @@ -14,11 +14,10 @@ reason = "This integration test stages sandbox fixtures with sync std::fs." )] -use fabro_sandbox::ProviderAccess; use fabro_sandbox::reconnect::reconnect; +use fabro_sandbox::{ProviderAccess, Sandbox as _, SandboxOptions, provider_sandbox}; use fabro_types::{RunSandboxInstance, RunSandboxRuntime, SandboxProviderKind}; -const DOCKER_MANAGED_LABEL: &str = "sh.fabro.managed"; const DOCKER_CP_IMAGE: &str = "buildpack-deps:noble"; // --------------------------------------------------------------------------- @@ -176,38 +175,41 @@ impl Drop for DockerCpContainer { } } -fn docker_cp_container() -> DockerCpContainer { +/// A container the driver created, so a reconnect by id finds it: the +/// driver attaches only to containers carrying its own label, the way +/// fabro's ownership scope attaches only to those carrying fabro's. +async fn docker_cp_container() -> DockerCpContainer { if let Ok(id) = std::env::var("FABRO_DOCKER_CP_CONTAINER") { return DockerCpContainer { id, cleanup: false }; } ensure_docker_image(DOCKER_CP_IMAGE); - let output = std::process::Command::new("docker") - .args([ - "run", - "-d", - "--label", - &format!("{DOCKER_MANAGED_LABEL}=true"), - "--workdir", - "/workspace", - DOCKER_CP_IMAGE, - "sh", - "-c", - "mkdir -p /workspace && sleep 300", - ]) - .output() - .expect("docker run should execute"); + let sandbox = provider_sandbox( + SandboxProviderKind::DOCKER, + &ProviderAccess::default(), + SandboxOptions { + image: Some(DOCKER_CP_IMAGE.to_string()), + skip_clone: true, + ..SandboxOptions::default() + }, + None, + None, + None, + None, + None, + None, + ) + .await + .expect("docker sandbox should construct"); + sandbox + .initialize() + .await + .expect("docker sandbox should initialize"); + let id = sandbox.sandbox_info(); assert!( - output.status.success(), - "docker run failed\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) + !id.is_empty(), + "the docker sandbox should have a container id" ); - let id = String::from_utf8(output.stdout) - .expect("docker run stdout should be UTF-8") - .trim() - .to_string(); - assert!(!id.is_empty(), "docker run should return a container id"); DockerCpContainer { id, cleanup: true } } @@ -235,7 +237,7 @@ fn ensure_docker_image(image: &str) { #[tokio::test] #[ignore] // requires Docker daemon async fn docker_cp_upload_download_round_trip() { - let container = docker_cp_container(); + let container = docker_cp_container().await; let scratch = tempfile::tempdir().unwrap(); let record = docker_record(&container.id); @@ -266,7 +268,7 @@ async fn docker_cp_upload_download_round_trip() { #[tokio::test] #[ignore] // requires Docker daemon async fn docker_cp_binary_round_trip() { - let container = docker_cp_container(); + let container = docker_cp_container().await; let scratch = tempfile::tempdir().unwrap(); let record = docker_record(&container.id); @@ -295,7 +297,7 @@ async fn docker_cp_binary_round_trip() { #[tokio::test] #[ignore] // requires Docker daemon async fn docker_cp_creates_parent_dirs() { - let container = docker_cp_container(); + let container = docker_cp_container().await; let scratch = tempfile::tempdir().unwrap(); let record = docker_record(&container.id);