From 5d2b0c9a5405dc9b4ac503e2e7fb37cae4c8cfa9 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 6 Nov 2026 10:03:18 +0100 Subject: [PATCH] swarm-builder: an agent option whose eval failed is left out of the reply, not read as its default --- docs/agent-lifecycle/persistence.md | 8 +- docs/swarm/builder.md | 24 ++++ docs/trust-boundary/security.md | 9 +- hive-c0re/src/attic/closure.rs | 4 +- hive-c0re/src/attic/probe.rs | 4 +- hive-c0re/src/job_queue/exec/tests.rs | 4 +- hive-c0re/src/lifecycle/host_config.rs | 67 ++++++---- swarm-builder/src/main.rs | 34 +++-- swarm-builder/src/nix_build.rs | 178 ++++++++++++++++--------- swarm-builder/src/push.rs | 7 +- swarm-controller/src/cache_miss.rs | 16 +-- swarm-controller/src/meta_build.rs | 22 ++- swarm-wire/src/agent_closures.rs | 60 +++++++-- swarm-wire/src/builder.rs | 61 ++++++--- 14 files changed, 340 insertions(+), 158 deletions(-) diff --git a/docs/agent-lifecycle/persistence.md b/docs/agent-lifecycle/persistence.md index 337d921d..66d71e9f 100644 --- a/docs/agent-lifecycle/persistence.md +++ b/docs/agent-lifecycle/persistence.md @@ -476,9 +476,11 @@ first start afterwards has hive-priv, as root, chown the agent's `state`, builder reads `config.services.hyperhive.agent.user.privateUsers` from the config it evaluates in its sandbox and puts it in the agent's closure record (`private_users`). hive-c0re reads the record when it writes the container's -flags and sends the value to hive-priv with each start. A record or builder -from before the field reads false; a record hive-c0re can't read, or none, -leaves the agent as it is. +flags and sends the value to hive-priv with each start. An agent whose module +doesn't declare the option reads false. When the builder can't read the +option (its eval fails or times out), the record carries no flag, and so does a +record or builder from before the field. A record without a flag, a record +hive-c0re can't read, or none, leaves the agent as it is. - **On disk the data belongs to one host uid:gid**, `services.hyperhive.deploy.hive-controller.userns.dataOwner` diff --git a/docs/swarm/builder.md b/docs/swarm/builder.md index d690aef0..18ca2094 100644 --- a/docs/swarm/builder.md +++ b/docs/swarm/builder.md @@ -282,6 +282,30 @@ The record's `cached` field says whether the builder pushed the closure to [the binary cache](#binary-cache). Each hive reads its own agents' keys to [report whether it can pull them and download them in a rebuild](#what-hives-do-with-the-cache). The closure stays unrooted in the builder host's store. +The built reply and the record also carry two options of the agent's evaluated +config, `private_users` (`services.hyperhive.agent.user.privateUsers`) and +`nix_builds` (`services.hyperhive.agent.nixBuilds`). The builder reads each with +a `nix eval` in its sandbox, and an eval has three outcomes: + +- **A value:** the field holds it. +- **The module doesn't declare the option** (`nix` says the flake doesn't + provide the attribute): the field holds the option's default, off for + `private_users` and `host` for `nix_builds`. +- **Any other failure, or a timeout:** the value is unknown, and the reply and + the record leave the field out. hive-c0re then sends nothing for that option, + and hive-priv keeps what the container's last write chose, as it does for an + agent with no record. + +Mixed versions: + +| builder | controller and hive-c0re | what happens | +| ------- | ------------------------ | -------------------------------------------------------------------------------------------------------------- | +| new | new | an unread option leaves the field out, and the last write stands | +| old | new | the old builder always sends a value, which the record carries | +| new | old | the old side reads a missing field as the default, so an unread option behaves as it did before this behaviour | + +A record or reply from before the fields reads as unknown. + The controller gives the `builder` forge account read on `internal/meta` when it updates the flake, and again before each build, since the repo is private. A grant the forge refuses fails the build with no verdict. diff --git a/docs/trust-boundary/security.md b/docs/trust-boundary/security.md index 69f28780..12fa1bfc 100644 --- a/docs/trust-boundary/security.md +++ b/docs/trust-boundary/security.md @@ -211,12 +211,15 @@ the option from the evaluated config into the agent's closure record flags and sends the mode to hive-priv, and the flags apply at the container's next start. -- A record or builder from before the field reads `host`. +- An agent whose module doesn't declare the option reads `host`. When the builder + can't read the option (its eval fails or times out), the record carries no + mode, and so does a record or builder from before the field. hive-c0re then + sends no mode, and hive-priv keeps what the container's last write chose. - A mode this build doesn't know (a record written by a newer swarm) reads as `none`, never `host`: an agent that asked for a mode its hive can't give gets no builds, not builds outside its limits. -- A record hive-c0re can't read, or none, sends no mode, and hive-priv keeps - what the container's last write chose. +- A record hive-c0re can't read, or none, sends no mode either, and hive-priv + keeps what the container's last write chose. - A hive-c0re or hive-priv from before the option ignores it, so the agent keeps host builds until you upgrade both. diff --git a/hive-c0re/src/attic/closure.rs b/hive-c0re/src/attic/closure.rs index b39a574e..c82ef923 100644 --- a/hive-c0re/src/attic/closure.rs +++ b/hive-c0re/src/attic/closure.rs @@ -173,8 +173,8 @@ mod tests { system: "x86_64-linux".to_owned(), built_unix: 1, cached, - private_users: false, - nix_builds: hive_types::NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(hive_types::NixBuilds::Host), } } diff --git a/hive-c0re/src/attic/probe.rs b/hive-c0re/src/attic/probe.rs index 66c2c1a6..871d8dd5 100644 --- a/hive-c0re/src/attic/probe.rs +++ b/hive-c0re/src/attic/probe.rs @@ -245,8 +245,8 @@ mod tests { system: "x86_64-linux".to_owned(), built_unix: 1, cached: true, - private_users: false, - nix_builds: hive_types::NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(hive_types::NixBuilds::Host), } } diff --git a/hive-c0re/src/job_queue/exec/tests.rs b/hive-c0re/src/job_queue/exec/tests.rs index ccc6106d..0d7592d2 100644 --- a/hive-c0re/src/job_queue/exec/tests.rs +++ b/hive-c0re/src/job_queue/exec/tests.rs @@ -274,8 +274,8 @@ fn closure(cached: bool) -> AgentClosure { system: "x86_64-linux".to_owned(), built_unix: 1, cached, - private_users: false, - nix_builds: hive_types::NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(hive_types::NixBuilds::Host), } } diff --git a/hive-c0re/src/lifecycle/host_config.rs b/hive-c0re/src/lifecycle/host_config.rs index ef19adcc..ba6bf775 100644 --- a/hive-c0re/src/lifecycle/host_config.rs +++ b/hive-c0re/src/lifecycle/host_config.rs @@ -518,13 +518,14 @@ fn socket_bind(agent_name: &str, socket_dir: &Path, private_users: Option) } /// What hive-priv is told about the agent's user namespace: the record's flag, -/// or `None` (leave the agent as it is) when there is no record or it cannot be -/// read, so a swarm outage never turns a namespaced agent off or on. +/// or `None` (leave the agent as it is) when there is no record, it cannot be +/// read, or it carries no flag because the builder could not read the option, +/// so neither a swarm outage nor a failed eval turns a namespaced agent off or on. fn private_users_of( record: &Result>, ) -> Option { match record { - Ok(record) => record.as_ref().map(|r| r.private_users), + Ok(record) => record.as_ref().and_then(|r| r.private_users), Err(e) => { tracing::warn!(error = %format!("{e:#}"), "closure record unreadable; the agent's user namespace is left as it is"); None @@ -533,13 +534,15 @@ fn private_users_of( } /// What hive-priv is told about the agent's nix builds: the record's mode, or -/// `None` (leave the agent as the last write set it) when there is no record or -/// it cannot be read, so a swarm outage never turns an agent's host builds on. +/// `None` (leave the agent as the last write set it) when there is no record, +/// it cannot be read, or it carries no mode because the builder could not read +/// the option, so neither a swarm outage nor a failed eval turns an agent's +/// host builds on. fn nix_builds_of( record: &Result>, ) -> Option { match record { - Ok(record) => record.as_ref().map(|r| r.nix_builds), + Ok(record) => record.as_ref().and_then(|r| r.nix_builds), Err(e) => { tracing::warn!(error = %format!("{e:#}"), "closure record unreadable; the agent's nix builds are left as they are"); None @@ -579,13 +582,13 @@ mod tests { use crate::coordinator::HiveEnv; use hive_types::NixBuilds; - fn record_with(private_users: bool) -> swarm_wire::agent_closures::AgentClosure { - record_with_builds(private_users, NixBuilds::Host) + fn record_with(private_users: Option) -> swarm_wire::agent_closures::AgentClosure { + record_with_builds(private_users, Some(NixBuilds::Host)) } fn record_with_builds( - private_users: bool, - nix_builds: NixBuilds, + private_users: Option, + nix_builds: Option, ) -> swarm_wire::agent_closures::AgentClosure { swarm_wire::agent_closures::AgentClosure { meta_rev: "a".repeat(40), @@ -613,48 +616,58 @@ mod tests { assert!(!b.read_only); } - /// The flag in the record is what hive-priv is told, both ways. No record - /// and an unreadable one say nothing, so a swarm outage changes no agent's - /// namespace; a record from before the field reads false. + /// The flag in the record is what hive-priv is told, both ways. No record, + /// an unreadable one, and a record whose builder could not read the option + /// (or that predates the field) say nothing, so neither a swarm outage nor + /// a failed eval changes an agent's namespace. #[test] fn the_records_flag_is_what_the_start_path_passes() { - assert_eq!( - super::private_users_of(&Ok(Some(record_with(true)))), - Some(true) - ); - assert_eq!( - super::private_users_of(&Ok(Some(record_with(false)))), - Some(false) - ); + for flag in [true, false] { + assert_eq!( + super::private_users_of(&Ok(Some(record_with(Some(flag))))), + Some(flag) + ); + } assert_eq!(super::private_users_of(&Ok(None)), None); assert_eq!(super::private_users_of(&Err(anyhow::anyhow!("down"))), None); + assert_eq!( + super::private_users_of(&Ok(Some(record_with(None)))), + None, + "an unread flag keeps the last write" + ); let old = serde_json::json!({ "meta_rev": "a", "store_path": "/nix/store/x", "system": "s", "built_unix": 1, }); let old = serde_json::from_value(old).expect("decodes"); - assert_eq!(super::private_users_of(&Ok(Some(old))), Some(false)); + assert_eq!(super::private_users_of(&Ok(Some(old))), None); } - /// The record's mode is what hive-priv is told, both ways. No record and an - /// unreadable one say nothing, so a swarm outage changes no agent; a record - /// from before the field reads as host builds. + /// The record's mode is what hive-priv is told, both ways. No record, an + /// unreadable one, and a record whose builder could not read the option (or + /// that predates the field) say nothing, so neither a swarm outage nor a + /// failed eval changes an agent. #[test] fn the_records_nix_builds_is_what_the_start_path_passes() { for mode in [NixBuilds::Host, NixBuilds::Disabled] { assert_eq!( - super::nix_builds_of(&Ok(Some(record_with_builds(false, mode)))), + super::nix_builds_of(&Ok(Some(record_with_builds(None, Some(mode))))), Some(mode) ); } assert_eq!(super::nix_builds_of(&Ok(None)), None); assert_eq!(super::nix_builds_of(&Err(anyhow::anyhow!("down"))), None); + assert_eq!( + super::nix_builds_of(&Ok(Some(record_with_builds(None, None)))), + None, + "an unread mode keeps the last write" + ); let old = serde_json::json!({ "meta_rev": "a", "store_path": "/nix/store/x", "system": "s", "built_unix": 1, }); let old = serde_json::from_value(old).expect("decodes"); - assert_eq!(super::nix_builds_of(&Ok(Some(old))), Some(NixBuilds::Host)); + assert_eq!(super::nix_builds_of(&Ok(Some(old))), None); // A mode a newer swarm wrote, which this build does not know, decodes // and builds nothing. let newer = serde_json::json!({ diff --git a/swarm-builder/src/main.rs b/swarm-builder/src/main.rs index 660f4dad..0d69268a 100644 --- a/swarm-builder/src/main.rs +++ b/swarm-builder/src/main.rs @@ -862,8 +862,8 @@ mod tests { out_path: "/nix/store/x".to_owned(), system: "s".to_owned(), cached: false, - private_users: false, - nix_builds: hive_types::NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(hive_types::NixBuilds::Host), } }, ) @@ -885,8 +885,8 @@ mod tests { out_path: "/nix/store/x".to_owned(), system: "s".to_owned(), cached: false, - private_users: false, - nix_builds: hive_types::NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(hive_types::NixBuilds::Host), } }, ) @@ -1454,7 +1454,9 @@ mod tests { #[tokio::test] async fn a_meta_build_is_answered_with_the_path_the_system_and_the_private_users_flag() { - let fake = meta_nix("echo true"); + let fake = meta_nix( + "for a in \"$@\"; do last=$a; done; case \"$last\" in *nixBuilds) echo '\"host\"';; *) echo true;; esac", + ); let (env, mut sent) = env(); let request = msg(Some("_INBOX.5"), &meta_json("atlas", REV)); serve_meta(fake.builder(), env, vec![request]).await; @@ -1490,12 +1492,17 @@ mod tests { ); } + /// An option the module does not declare is false; one whose eval failed, or + /// printed no bool, is left out of the reply, for the controller to record as + /// unknown. `echo false` is the control that the field is read at all. #[tokio::test] - async fn private_users_is_false_unless_nix_eval_printed_true() { - for eval_says in [ - "echo false", - "echo null", - "echo 'error: no such option' >&2; exit 1", + async fn private_users_is_left_out_unless_nix_eval_printed_a_bool_or_the_option_is_missing() { + let missing = "echo \"error: flake 'f' does not provide attribute 'a'\" >&2; exit 1"; + for (eval_says, want) in [ + ("echo false", Some(false)), + (missing, Some(false)), + ("echo null", None), + ("echo 'error: no such option' >&2; exit 1", None), ] { let fake = meta_nix(eval_says); let (env, mut sent) = env(); @@ -1503,7 +1510,12 @@ mod tests { serve_meta(fake.builder(), env, vec![request]).await; let (_, body) = next(&mut sent).await; assert_eq!(body["state"], "built", "{eval_says}"); - assert_eq!(body["private_users"], false, "{eval_says}"); + assert_eq!( + body.get("private_users") + .and_then(serde_json::Value::as_bool), + want, + "{eval_says}: {body}" + ); } } diff --git a/swarm-builder/src/nix_build.rs b/swarm-builder/src/nix_build.rs index 9b5d1f31..6ed4e70e 100644 --- a/swarm-builder/src/nix_build.rs +++ b/swarm-builder/src/nix_build.rs @@ -186,58 +186,50 @@ impl Builder { } /// `nix eval --json` of `option` in the agent's evaluated config, run in this - /// sandbox against the flake the build just evaluated. `None` when the eval - /// fails or times out, and logs why along with what the caller falls back to. - async fn eval_option( - &self, - job: &BuildFromMeta, - token: &str, - option: &str, - fallback: &str, - ) -> Option> { + /// sandbox against the flake the build just evaluated. Logs why when the + /// answer is not a value. + async fn eval_option(&self, job: &BuildFromMeta, token: &str, option: &str) -> Eval { let mut cmd = self.nix_command(token); cmd.args(EVAL_FLAGS) .arg(format!("{}.{option}", self.meta_config(job))); match run_raw(cmd, self.deadline).await { - Ok((status, out, _)) if status.success() => Some(out), + Ok((status, out, _)) if status.success() => Eval::Value(out), + Ok((_, _, err)) if attribute_missing(&String::from_utf8_lossy(&err)) => { + tracing::info!(agent = %job.agent, "{option} is not declared by the agent's module; its default applies"); + Eval::Missing + } Ok((status, _, err)) => { - tracing::info!(agent = %job.agent, %status, error = %tail(&String::from_utf8_lossy(&err), status), "{option} not read; {fallback}"); - None + tracing::info!(agent = %job.agent, %status, error = %tail(&String::from_utf8_lossy(&err), status), "{option} not read; the reply carries no value for it"); + Eval::Unknown } Err(why) => { - tracing::warn!(agent = %job.agent, %why, "{option} not read; {fallback}"); - None + tracing::warn!(agent = %job.agent, %why, "{option} not read; the reply carries no value for it"); + Eval::Unknown } } } /// Whether the agent's config sets `services.hyperhive.agent.user.privateUsers`. - /// Anything but a clean `true` is false: an agent whose module predates the - /// option, a failed eval and a timeout all keep the agent out of a user - /// namespace. - async fn private_users(&self, job: &BuildFromMeta, token: &str) -> bool { - self.eval_option( - job, - token, - PRIVATE_USERS_OPTION, - "the agent stays out of a user namespace", - ) - .await - .is_some_and(|out| parse_flag(&out)) + /// An agent whose module predates the option is out of a user namespace; + /// a failed eval, a timeout and an output that is no bool are `None`. + async fn private_users(&self, job: &BuildFromMeta, token: &str) -> Option { + match self.eval_option(job, token, PRIVATE_USERS_OPTION).await { + Eval::Value(out) => parse_flag(&out), + Eval::Missing => Some(false), + Eval::Unknown => None, + } } /// The agent's `services.hyperhive.agent.nixBuilds`. An agent whose module - /// predates the option, a failed eval and a timeout all read as - /// [`NixBuilds::Host`], which is what every agent did before the option. - async fn nix_builds(&self, job: &BuildFromMeta, token: &str) -> NixBuilds { - self.eval_option( - job, - token, - NIX_BUILDS_OPTION, - "the agent builds through the host daemon", - ) - .await - .map_or_else(NixBuilds::default, |out| parse_nix_builds(&out)) + /// predates the option reads as [`NixBuilds::Host`], which is what every + /// agent did before the option; a failed eval, a timeout and an output that + /// is no string are `None`. + async fn nix_builds(&self, job: &BuildFromMeta, token: &str) -> Option { + match self.eval_option(job, token, NIX_BUILDS_OPTION).await { + Eval::Value(out) => parse_nix_builds(&out), + Eval::Missing => Some(NixBuilds::default()), + Eval::Unknown => None, + } } /// The config commit as a flake reference, fetched through the PR's head @@ -281,15 +273,35 @@ const PRIVATE_USERS_OPTION: &str = "services.hyperhive.agent.user.privateUsers"; /// The agent option holding where its nix builds run. const NIX_BUILDS_OPTION: &str = "services.hyperhive.agent.nixBuilds"; -/// `nix eval --json` of a bool: true only for exactly `true`. -fn parse_flag(out: &[u8]) -> bool { - out.trim_ascii() == b"true" +/// What `nix eval --json` of an agent option came to. +enum Eval { + /// It printed a value. + Value(Vec), + /// The config has no such attribute: the agent's module predates the option. + Missing, + /// It failed another way or timed out, so the value is not known. + Unknown, } -/// `nix eval --json` of a [`NixBuilds`]: the spelling, or the default when the +/// Whether `stderr` is `nix` saying the attribute path is not in the flake's +/// outputs, which for an option path means the module does not declare it. +fn attribute_missing(stderr: &str) -> bool { + stderr.contains("does not provide attribute") +} + +/// `nix eval --json` of a bool: the bool, or `None` for anything else. +fn parse_flag(out: &[u8]) -> Option { + match out.trim_ascii() { + b"true" => Some(true), + b"false" => Some(false), + _ => None, + } +} + +/// `nix eval --json` of a [`NixBuilds`]: the spelling, or `None` when the /// output is not a string. -fn parse_nix_builds(out: &[u8]) -> NixBuilds { - serde_json::from_slice(out).unwrap_or_default() +fn parse_nix_builds(out: &[u8]) -> Option { + serde_json::from_slice(out).ok() } /// `nix flake lock` of the flake in the working directory. The flake's config @@ -577,17 +589,27 @@ mod tests { ); } - /// The reply carries what the evaluated config says about the option, and - /// anything but a clean `true` reads as false. The `true` arm is the - /// control for the others. + /// The shell for a `nix eval` of an option the module does not declare, as + /// `nix` 2.34 words it. + const ATTRIBUTE_MISSING: &str = "echo \"error: flake 'git+https://forge.example/internal/meta.git' does not provide attribute 'packages.x86_64-linux.nixosConfigurations.atlas.config.services.hyperhive.agent.user.privateUsers'\" >&2; exit 1"; + + /// The reply carries what the evaluated config says about the option. An + /// agent whose module predates the option is false; an eval that fails + /// another way, or prints no bool, leaves the field out. The `true` arm is + /// the control for the others. #[tokio::test] async fn a_meta_build_reports_the_agents_private_users_option() { for (i, (answer, want)) in [ - ("echo true", true), - ("echo false", false), - ("echo null", false), - ("echo true; exit 1", false), - ("echo error: attribute missing >&2; exit 1", false), + ("echo true", Some(true)), + ("echo false", Some(false)), + (ATTRIBUTE_MISSING, Some(false)), + ("echo null", None), + ("echo true; exit 1", None), + ("echo error: attribute missing >&2; exit 1", None), + ( + "echo 'error: infinite recursion encountered' >&2; exit 1", + None, + ), ] .into_iter() .enumerate() @@ -606,17 +628,19 @@ mod tests { } /// The reply carries the option's spelling. `host` is the control for - /// `none`; a mode this build does not know reads as `none`, and anything - /// that is no spelling at all (a failed eval, an older module) as `host`. + /// `none`; a mode this build does not know reads as `none`, an agent whose + /// module predates the option as `host`, and a failed eval, or output that + /// is no spelling at all, leaves the field out. #[tokio::test] async fn a_meta_build_reports_the_agents_nix_builds_option() { for (i, (answer, want)) in [ - ("echo '\"host\"'", NixBuilds::Host), - ("echo '\"none\"'", NixBuilds::Disabled), - ("echo '\"container\"'", NixBuilds::Disabled), - ("echo null", NixBuilds::Host), - ("echo '\"none\"'; exit 1", NixBuilds::Host), - ("echo error: attribute missing >&2; exit 1", NixBuilds::Host), + ("echo '\"host\"'", Some(NixBuilds::Host)), + ("echo '\"none\"'", Some(NixBuilds::Disabled)), + ("echo '\"container\"'", Some(NixBuilds::Disabled)), + (ATTRIBUTE_MISSING, Some(NixBuilds::Host)), + ("echo null", None), + ("echo '\"none\"'; exit 1", None), + ("echo error: attribute missing >&2; exit 1", None), ] .into_iter() .enumerate() @@ -636,6 +660,33 @@ mod tests { } } + /// An eval that does not finish in time is unknown for both options, not + /// their defaults. The build itself finishes, which is the control. + #[tokio::test] + async fn an_option_eval_that_times_out_leaves_both_options_out_of_the_reply() { + let nix = fake_nix( + "eval-timeout", + "if [ \"$1\" = eval ]; then exec sleep 30; else echo /nix/store/x; fi\n", + ); + let b = Builder { + nix, + deadline: Duration::from_secs(1), + ..builder() + }; + let reply = b.build_from_meta(&meta_job(), "t").await; + assert!( + matches!( + &reply, + FromMetaReply::Built { + private_users: None, + nix_builds: None, + .. + } + ), + "{reply:?}" + ); + } + #[test] fn the_option_is_read_from_the_evaluated_config_not_the_toplevel() { assert_eq!( @@ -647,8 +698,11 @@ mod tests { ?rev=0123456789abcdef0123456789abcdef01234567\ #nixosConfigurations.atlas.config.services.hyperhive.agent.user.privateUsers" ); - assert!(parse_flag(b"true\n") && !parse_flag(b"false\n") && !parse_flag(b"")); + assert_eq!(parse_flag(b"true\n"), Some(true)); + assert_eq!(parse_flag(b"false\n"), Some(false)); + assert_eq!(parse_flag(b""), None); } + #[test] fn a_meta_job_that_would_bend_the_reference_is_refused() { for bad in [ @@ -685,8 +739,8 @@ mod tests { out_path: format!("{} {}", BUILD_FLAGS.join(" "), b.meta_attr(&meta_job())), system: "x86_64-linux".to_owned(), cached: false, - private_users: false, - nix_builds: NixBuilds::Host, + private_users: None, + nix_builds: None, } ); } diff --git a/swarm-builder/src/push.rs b/swarm-builder/src/push.rs index a0d0c2dd..1c95f1ea 100644 --- a/swarm-builder/src/push.rs +++ b/swarm-builder/src/push.rs @@ -266,8 +266,8 @@ mod tests { out_path: OUT.to_owned(), system: "x86_64-linux".to_owned(), cached: false, - private_users: false, - nix_builds: NixBuilds::Disabled, + private_users: None, + nix_builds: Some(NixBuilds::Disabled), } } @@ -304,7 +304,8 @@ mod tests { matches!( reply, FromMetaReply::Built { - nix_builds: NixBuilds::Disabled, + private_users: None, + nix_builds: Some(NixBuilds::Disabled), .. } ), diff --git a/swarm-controller/src/cache_miss.rs b/swarm-controller/src/cache_miss.rs index 89191cf0..ce824ccc 100644 --- a/swarm-controller/src/cache_miss.rs +++ b/swarm-controller/src/cache_miss.rs @@ -564,8 +564,8 @@ mod tests { system: "x86_64-linux".to_owned(), built_unix: 7, cached, - private_users: false, - nix_builds: NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(NixBuilds::Host), } } @@ -652,8 +652,8 @@ mod tests { out_path: out_path.to_owned(), system: "x86_64-linux".to_owned(), cached, - private_users: false, - nix_builds: NixBuilds::Host, + private_users: Some(false), + nix_builds: Some(NixBuilds::Host), } } @@ -916,15 +916,15 @@ mod tests { out_path, system, cached, - private_users: true, - nix_builds: NixBuilds::Disabled, + private_users: Some(true), + nix_builds: Some(NixBuilds::Disabled), }; let world = Fake::missing(&["iris"]).answering("iris", answer); pass_at(&world, &mut Backoff::default(), Instant::now()).await; let writes = world.writes(); assert_eq!(writes.len(), 1); - assert!(writes[0].1.private_users); - assert_eq!(writes[0].1.nix_builds, NixBuilds::Disabled); + assert_eq!(writes[0].1.private_users, Some(true)); + assert_eq!(writes[0].1.nix_builds, Some(NixBuilds::Disabled)); } #[tokio::test] async fn another_path_leaves_a_record_that_moved_on() { diff --git a/swarm-controller/src/meta_build.rs b/swarm-controller/src/meta_build.rs index cd3de123..a27e5e6f 100644 --- a/swarm-controller/src/meta_build.rs +++ b/swarm-controller/src/meta_build.rs @@ -492,10 +492,14 @@ mod tests { } fn built_cached(cached: bool) -> Self { - Self::built_with(cached, false, NixBuilds::Host) + Self::built_with(cached, Some(false), Some(NixBuilds::Host)) } - fn built_with(cached: bool, private_users: bool, nix_builds: NixBuilds) -> Self { + fn built_with( + cached: bool, + private_users: Option, + nix_builds: Option, + ) -> Self { Self::answering(Ok(FromMetaReply::Built { out_path: "/nix/store/x-toplevel".to_owned(), system: "x86_64-linux".to_owned(), @@ -629,11 +633,15 @@ mod tests { } } - /// The record says what the builder said about the agent's user namespace, - /// both ways; a builder that said nothing leaves it off. + /// The record says what the builder said about the agent's user namespace + /// and nix builds, both ways; an option the builder left unread stays unread. #[tokio::test] async fn the_record_carries_the_builders_user_namespace_and_nix_builds_answers() { - for (private_users, nix_builds) in [(true, NixBuilds::Disabled), (false, NixBuilds::Host)] { + for (private_users, nix_builds) in [ + (Some(true), Some(NixBuilds::Disabled)), + (Some(false), Some(NixBuilds::Host)), + (None, None), + ] { let queue = FakeQueue::built_with(true, private_users, nix_builds); let outcome = run_with(&FakeHead::reading(Ok(Some(REV))), &queue).await; assert_eq!(outcome, Outcome::Done); @@ -927,8 +935,8 @@ mod tests { system: "x86_64-linux".to_owned(), built_unix: 1, cached: false, - private_users: true, - nix_builds: NixBuilds::Disabled, + private_users: Some(true), + nix_builds: Some(NixBuilds::Disabled), }; client.record("atlas", &closure).await.expect("written"); closure.store_path = "/nix/store/b".to_owned(); diff --git a/swarm-wire/src/agent_closures.rs b/swarm-wire/src/agent_closures.rs index 305b109a..6fed790d 100644 --- a/swarm-wire/src/agent_closures.rs +++ b/swarm-wire/src/agent_closures.rs @@ -40,15 +40,15 @@ pub struct AgentClosure { pub cached: bool, /// Whether the agent's evaluated config sets /// `services.hyperhive.agent.user.privateUsers`: the agent runs in its own - /// user namespace. A record written before the field existed reads as - /// false, which keeps the agent out of one. - #[serde(default)] - pub private_users: bool, + /// user namespace. `None` when the builder could not read it, and for a + /// record written before the field existed: the record says nothing, and + /// the reader leaves the agent as its last start set it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub private_users: Option, /// The agent's `services.hyperhive.agent.nixBuilds`: where its nix builds - /// run. A record written before the field existed reads as - /// [`NixBuilds::Host`], which is what every agent did before it. - #[serde(default)] - pub nix_builds: NixBuilds, + /// run. `None` as for `private_users`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub nix_builds: Option, } #[cfg(test)] @@ -69,8 +69,8 @@ mod tests { system: "x86_64-linux".to_owned(), built_unix: 7, cached: true, - private_users: true, - nix_builds: NixBuilds::Disabled, + private_users: Some(true), + nix_builds: Some(NixBuilds::Disabled), }; let wire = serde_json::json!({ "meta_rev": "abc", @@ -89,7 +89,7 @@ mod tests { } #[test] - fn a_record_without_the_fields_reads_as_not_cached_not_namespaced_and_host_builds() { + fn a_record_without_the_fields_reads_as_not_cached_and_says_nothing_of_the_options() { let old = serde_json::json!({ "meta_rev": "abc", "store_path": "/nix/store/x", @@ -98,7 +98,41 @@ mod tests { }); let record = serde_json::from_value::(old).expect("decodes"); assert!(!record.cached); - assert!(!record.private_users); - assert_eq!(record.nix_builds, NixBuilds::Host); + assert_eq!(record.private_users, None); + assert_eq!(record.nix_builds, None); + } + + /// An option the builder could not read is left out of the record, not + /// written as `null`: a reader that predates the option still decodes it. + #[test] + fn an_unread_option_is_left_out_of_the_record() { + // The shape a reader before the option had: a bool that defaults to false. + #[derive(serde::Deserialize)] + struct Before { + #[serde(default)] + private_users: bool, + } + let record = AgentClosure { + meta_rev: "abc".to_owned(), + store_path: "/nix/store/x".to_owned(), + system: "x86_64-linux".to_owned(), + built_unix: 7, + cached: true, + private_users: None, + nix_builds: Some(NixBuilds::Host), + }; + let wire = serde_json::to_value(&record).expect("encodes"); + let fields = wire.as_object().expect("an object"); + assert!(!fields.contains_key("private_users"), "{wire}"); + assert_eq!(fields["nix_builds"], "host"); + assert!( + !serde_json::from_value::(wire.clone()) + .expect("decodes") + .private_users + ); + assert_eq!( + serde_json::from_value::(wire).expect("decodes"), + record + ); } } diff --git a/swarm-wire/src/builder.rs b/swarm-wire/src/builder.rs index c9041697..0c2c7acc 100644 --- a/swarm-wire/src/builder.rs +++ b/swarm-wire/src/builder.rs @@ -151,15 +151,15 @@ pub enum FromMetaReply { #[serde(default)] cached: bool, /// Whether the agent's evaluated config sets - /// `services.hyperhive.agent.user.privateUsers`. A builder that - /// predates the field answers without it, which reads as false. - #[serde(default)] - private_users: bool, - /// The agent's `services.hyperhive.agent.nixBuilds`. A builder that - /// predates the field answers without it, which reads as - /// [`NixBuilds::Host`]. - #[serde(default)] - nix_builds: NixBuilds, + /// `services.hyperhive.agent.user.privateUsers`. Absent (`None`) when + /// the builder could not read it: its eval failed or timed out. A + /// builder that predates the field answers without it too. + #[serde(default, skip_serializing_if = "Option::is_none")] + private_users: Option, + /// The agent's `services.hyperhive.agent.nixBuilds`. Absent (`None`) + /// when the builder could not read it, as for `private_users`. + #[serde(default, skip_serializing_if = "Option::is_none")] + nix_builds: Option, }, /// The build did not succeed: the last lines `nix` printed, or why it /// stopped (it timed out, or could not start). The controller holds the @@ -273,8 +273,8 @@ mod tests { out_path: "/nix/store/x".to_owned(), system: "x86_64-linux".to_owned(), cached: true, - private_users: true, - nix_builds: NixBuilds::Disabled, + private_users: Some(true), + nix_builds: Some(NixBuilds::Disabled), }, json!({ "state": "built", @@ -307,8 +307,7 @@ mod tests { } #[test] - fn a_built_reply_from_a_builder_without_the_fields_reads_as_not_cached_not_namespaced_and_host_builds() - { + fn a_built_reply_from_a_builder_without_the_fields_reads_as_not_cached_and_unknown() { let old = json!({"state": "built", "out_path": "/nix/store/x", "system": "s"}); assert_eq!( serde_json::from_value::(old).expect("decodes"), @@ -316,12 +315,44 @@ mod tests { out_path: "/nix/store/x".to_owned(), system: "s".to_owned(), cached: false, - private_users: false, - nix_builds: NixBuilds::Host, + private_users: None, + nix_builds: None, } ); } + /// A value the builder could not read is left out of the reply, not sent as + /// `null`: a controller that predates the option still decodes it. + #[test] + fn an_unread_option_is_left_out_of_the_built_reply() { + // The shape a reader before the option had: a bool that defaults to false. + #[derive(serde::Deserialize)] + struct Before { + #[serde(default)] + private_users: bool, + } + let reply = FromMetaReply::Built { + out_path: "/nix/store/x".to_owned(), + system: "s".to_owned(), + cached: false, + private_users: None, + nix_builds: Some(NixBuilds::Host), + }; + let wire = serde_json::to_value(&reply).expect("encodes"); + let fields = wire.as_object().expect("an object"); + assert!(!fields.contains_key("private_users"), "{wire}"); + assert_eq!(fields["nix_builds"], "host"); + assert!( + !serde_json::from_value::(wire.clone()) + .expect("decodes") + .private_users + ); + assert_eq!( + serde_json::from_value::(wire).expect("decodes"), + reply + ); + } + #[test] fn the_subjects_are_pinned() { assert_eq!(SUBJECT, "$SWARM.builder.build");