From b74b5e77e42f0224c784ba09a5756ef0eafab35f Mon Sep 17 00:00:00 2001 From: damocles Date: Fri, 6 Nov 2026 17:40:05 +0100 Subject: [PATCH] hive-c0re: priv_client builds its requests in testable functions and tests them --- hive-c0re/src/priv_client.rs | 270 ++++++++++++++++++++++++++++++----- 1 file changed, 236 insertions(+), 34 deletions(-) diff --git a/hive-c0re/src/priv_client.rs b/hive-c0re/src/priv_client.rs index 431a55b9..962a879c 100644 --- a/hive-c0re/src/priv_client.rs +++ b/hive-c0re/src/priv_client.rs @@ -252,15 +252,15 @@ pub async fn update_container_streaming( toplevel: &str, on_line: impl FnMut(PrivStream, &str), ) -> Result<()> { - ok(call_streaming( - &PrivRequest::UpdateContainer { - name: name.to_owned(), - stream: true, - toplevel: toplevel.to_owned(), - }, - on_line, - ) - .await?) + ok(call_streaming(&update_container_request(name, toplevel), on_line).await?) +} + +fn update_container_request(name: &str, toplevel: &str) -> PrivRequest { + PrivRequest::UpdateContainer { + name: name.to_owned(), + stream: true, + toplevel: toplevel.to_owned(), + } } /// Streaming variant: forward stdout/stderr lines to `on_line` as they @@ -270,15 +270,15 @@ pub async fn create_container_streaming( toplevel: &str, on_line: impl FnMut(PrivStream, &str), ) -> Result<()> { - ok(call_streaming( - &PrivRequest::CreateContainer { - name: name.to_owned(), - stream: true, - toplevel: toplevel.to_owned(), - }, - on_line, - ) - .await?) + ok(call_streaming(&create_container_request(name, toplevel), on_line).await?) +} + +fn create_container_request(name: &str, toplevel: &str) -> PrivRequest { + PrivRequest::CreateContainer { + name: name.to_owned(), + stream: true, + toplevel: toplevel.to_owned(), + } } /// See [`PrivRequest::RealiseClosure`]. A path hive-priv could not download @@ -323,7 +323,28 @@ pub async fn write_nspawn_flags( private_users: Option, nix_builds: Option, ) -> Result<()> { - ok(call(&PrivRequest::WriteNspawnFlags { + ok(call(&write_nspawn_flags_request( + container, + binds, + isolation, + load_credentials, + hive_facts, + private_users, + nix_builds, + )) + .await?) +} + +fn write_nspawn_flags_request( + container: &str, + binds: &[BindMount], + isolation: NetworkIsolation, + load_credentials: &[CredentialMount], + hive_facts: HiveFacts, + private_users: Option, + nix_builds: Option, +) -> PrivRequest { + PrivRequest::WriteNspawnFlags { container: container.to_owned(), binds: binds.to_vec(), isolation, @@ -331,8 +352,7 @@ pub async fn write_nspawn_flags( hive_facts: Box::new(hive_facts), private_users, nix_builds, - }) - .await?) + } } /// See [`PrivRequest::EnsureAgentSocketDir`]. @@ -351,14 +371,26 @@ pub async fn write_resource_limits( cpu_weight: Option, io_weight: Option, ) -> Result<()> { - ok(call(&PrivRequest::WriteResourceLimits { + ok(call(&resource_limits_request( + container, memory_max, cpu_quota, cpu_weight, io_weight, + )) + .await?) +} + +fn resource_limits_request( + container: &str, + memory_max: &str, + cpu_quota: &str, + cpu_weight: Option, + io_weight: Option, +) -> PrivRequest { + PrivRequest::WriteResourceLimits { container: container.to_owned(), memory_max: memory_max.to_owned(), cpu_quota: cpu_quota.to_owned(), cpu_weight, io_weight, - }) - .await?) + } } pub async fn remove_service_dropin(container: &str) -> Result<()> { @@ -567,18 +599,30 @@ pub async fn send_agent_snapshot_to_file( parent_snapshot_name: Option<&str>, dest_file_name: &str, ) -> Result { - let (stdout, _) = check( - call(&PrivRequest::SendAgentSnapshotToFile { - agent_name: agent_name.to_owned(), - snapshot_name: snapshot_name.to_owned(), - parent_snapshot_name: parent_snapshot_name.map(str::to_owned), - dest_file_name: dest_file_name.to_owned(), - }) - .await?, - )?; + let req = send_snapshot_request( + agent_name, + snapshot_name, + parent_snapshot_name, + dest_file_name, + ); + let (stdout, _) = check(call(&req).await?)?; Ok(stdout) } +fn send_snapshot_request( + agent_name: &str, + snapshot_name: &str, + parent_snapshot_name: Option<&str>, + dest_file_name: &str, +) -> PrivRequest { + PrivRequest::SendAgentSnapshotToFile { + agent_name: agent_name.to_owned(), + snapshot_name: snapshot_name.to_owned(), + parent_snapshot_name: parent_snapshot_name.map(str::to_owned), + dest_file_name: dest_file_name.to_owned(), + } +} + /// Parse `(referenced, exclusive)` bytes from `btrfs qgroup show -f --raw` /// output (a qgroup row is ` …`). /// @@ -620,9 +664,167 @@ fn ok(resp: PrivResponse) -> Result<()> { #[cfg(test)] mod tests { - use super::within; + use super::{ + BindMount, CredentialMount, HiveFacts, NetworkIsolation, NixBuilds, PrivRequest, + PrivResponse, check, create_container_request, parse_qgroup_usage, resource_limits_request, + send_snapshot_request, update_container_request, within, write_nspawn_flags_request, + }; + use serde_json::{Value, json}; use std::time::Duration; + fn wire(req: &PrivRequest) -> Value { + serde_json::to_value(req).expect("a request serialises") + } + + #[test] + fn create_and_update_keep_the_name_and_toplevel_apart_and_stream() { + assert_eq!( + wire(&create_container_request("alice", "/nix/store/x")), + json!({"op": "create_container", "name": "alice", "stream": true, "toplevel": "/nix/store/x"}), + ); + assert_eq!( + wire(&update_container_request("alice", "/nix/store/x")), + json!({"op": "update_container", "name": "alice", "stream": true, "toplevel": "/nix/store/x"}), + ); + } + + #[test] + fn resource_limits_put_each_value_in_its_own_field() { + let got = wire(&resource_limits_request( + "alice", + "4G", + "200%", + Some(50), + None, + )); + assert_eq!(got["op"], "write_resource_limits"); + assert_eq!(got["container"], "alice"); + assert_eq!(got["memory_max"], "4G"); + assert_eq!(got["cpu_quota"], "200%"); + assert_eq!(got["cpu_weight"], 50); + assert_eq!(got["io_weight"], Value::Null); + } + + #[test] + fn nspawn_flags_carry_every_argument_to_its_field() { + let binds = [BindMount { + host_path: "/host/a".into(), + container_path: "/ctr/a".into(), + read_only: true, + }]; + let creds = [CredentialMount { + name: "otel-headers".into(), + host_path: "/run/secrets/otel".into(), + }]; + let isolation = NetworkIsolation { + bridge: "hive0".into(), + gateway_ip: "10.42.0.1".into(), + }; + let facts = HiveFacts { + name: Some("pr1ma".into()), + ..HiveFacts::default() + }; + let got = wire(&write_nspawn_flags_request( + "alice", + &binds, + isolation.clone(), + &creds, + facts.clone(), + Some(true), + Some(NixBuilds::Disabled), + )); + assert_eq!(got["nix_builds"], "none"); + let unset = wire(&write_nspawn_flags_request( + "alice", + &binds, + isolation, + &creds, + facts, + Some(true), + None, + )); + assert_eq!(unset["nix_builds"], Value::Null); + assert_eq!(got["op"], "write_nspawn_flags"); + assert_eq!(got["container"], "alice"); + assert_eq!( + got["binds"], + json!([{"host_path": "/host/a", "container_path": "/ctr/a", "read_only": true}]) + ); + assert_eq!( + got["isolation"], + json!({"bridge": "hive0", "gateway_ip": "10.42.0.1"}) + ); + assert_eq!( + got["load_credentials"], + json!([{"name": "otel-headers", "host_path": "/run/secrets/otel"}]) + ); + assert_eq!(got["hive_facts"]["name"], "pr1ma"); + assert_eq!(got["private_users"], true); + } + + #[test] + fn a_snapshot_send_names_the_parent_only_when_given() { + let full = wire(&send_snapshot_request("alice", "snap-2", None, "out.btrfs")); + assert_eq!(full["agent_name"], "alice"); + assert_eq!(full["snapshot_name"], "snap-2"); + assert_eq!(full["dest_file_name"], "out.btrfs"); + assert_eq!(full["parent_snapshot_name"], Value::Null); + + let incremental = wire(&send_snapshot_request( + "alice", + "snap-2", + Some("snap-1"), + "out.btrfs", + )); + assert_eq!(incremental["parent_snapshot_name"], "snap-1"); + assert_eq!(incremental["snapshot_name"], "snap-2"); + } + + #[test] + fn the_level_zero_qgroup_row_gives_the_usage() { + let out = "qgroupid rfer excl\n\ + -------- ---- ----\n\ + 1/100 999 888\n\ + 0/257 4096 2048\n"; + assert_eq!(parse_qgroup_usage(out).expect("a 0/ row"), (4096, 2048)); + } + + #[test] + fn output_with_no_level_zero_row_is_an_error() { + let err = parse_qgroup_usage("qgroupid rfer excl\n1/100 1 2\n").expect_err("no 0/ row"); + assert!(err.to_string().contains("no level-0 qgroup"), "{err}"); + // A 0/ row whose columns are not numbers is skipped, not misread. + parse_qgroup_usage("0/257 x y\n").expect_err("non-numeric columns"); + } + + #[test] + fn a_refused_response_surfaces_the_error_text() { + let refused = PrivResponse { + ok: false, + stdout: String::new(), + stderr: String::new(), + error: Some("bad path".into()), + }; + assert_eq!(check(refused).expect_err("refused").to_string(), "bad path"); + + let silent = PrivResponse { + ok: false, + stdout: String::new(), + stderr: String::new(), + error: None, + }; + let err = check(silent).expect_err("refused"); + assert_eq!(err.to_string(), "hive-priv returned error"); + + let fine = PrivResponse { + ok: true, + stdout: "out".into(), + stderr: "err".into(), + error: None, + }; + assert_eq!(check(fine).expect("ok"), ("out".into(), "err".into())); + } + #[tokio::test] async fn a_silent_hive_priv_times_out_as_an_error() { let err = within(