hive-c0re: priv_client builds its requests in testable functions and tests them

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(
+    ok(call_streaming(&update_container_request(name, toplevel), on_line).await?)
-        &PrivRequest::UpdateContainer {
+}
-            name: name.to_owned(),
+
-            stream: true,
+fn update_container_request(name: &str, toplevel: &str) -> PrivRequest {
-            toplevel: toplevel.to_owned(),
+    PrivRequest::UpdateContainer {
-        },
+        name: name.to_owned(),
-        on_line,
+        stream: true,
-    )
+        toplevel: toplevel.to_owned(),
-    .await?)
+    }
 }
 
 /// 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(
+    ok(call_streaming(&create_container_request(name, toplevel), on_line).await?)
-        &PrivRequest::CreateContainer {
+}
-            name: name.to_owned(),
+
-            stream: true,
+fn create_container_request(name: &str, toplevel: &str) -> PrivRequest {
-            toplevel: toplevel.to_owned(),
+    PrivRequest::CreateContainer {
-        },
+        name: name.to_owned(),
-        on_line,
+        stream: true,
-    )
+        toplevel: toplevel.to_owned(),
-    .await?)
+    }
 }
 
 /// See [`PrivRequest::RealiseClosure`]. A path hive-priv could not download
@@ -323,7 +323,28 @@ pub async fn write_nspawn_flags(
     private_users: Option<bool>,
     nix_builds: Option<NixBuilds>,
 ) -> 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<bool>,
+    nix_builds: Option<NixBuilds>,
+) -> 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<u32>,
     io_weight: Option<u32>,
 ) -> 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<u32>,
+    io_weight: Option<u32>,
+) -> 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<String> {
-    let (stdout, _) = check(
+    let req = send_snapshot_request(
-        call(&PrivRequest::SendAgentSnapshotToFile {
+        agent_name,
-            agent_name: agent_name.to_owned(),
+        snapshot_name,
-            snapshot_name: snapshot_name.to_owned(),
+        parent_snapshot_name,
-            parent_snapshot_name: parent_snapshot_name.map(str::to_owned),
+        dest_file_name,
-            dest_file_name: dest_file_name.to_owned(),
+    );
-        })
+    let (stdout, _) = check(call(&req).await?)?;
-        .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 `<id-with-slash> <rfer> <excl> …`).
 ///
@@ -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(