diff --git a/src/commands/rotate.rs b/src/commands/rotate.rs index 132a43ac..eefc7d99 100644 --- a/src/commands/rotate.rs +++ b/src/commands/rotate.rs @@ -128,14 +128,28 @@ pub(super) struct RotateContext { pub(super) state_file: PathBuf, /// The `docker` executable every spawn in this rotation runs. /// - /// Set once in [`run_rotate`] and only read afterwards, so a test - /// that builds a context can point the whole tree at a fake without - /// touching `PATH`. + /// Set once in [`run_rotate_with_exec`] and only read afterwards, so + /// a test that builds a context — or drives that entry point — can + /// point the whole tree at a fake without touching `PATH`. pub(super) docker: PathBuf, } -#[allow(clippy::too_many_lines)] pub(crate) async fn run_rotate(args: &RotateArgs, messages: &Messages) -> Result { + run_rotate_with_exec(args, Path::new(DOCKER_BIN), messages).await +} + +/// [`run_rotate`] with the `docker` executable supplied by the caller. +/// +/// Mirrors the `_with_exec` pairs in [`crate::commands::infra`]: the +/// wrapper above is this function with [`DOCKER_BIN`], and a test +/// drives the whole entry point — state-file handling and strategy +/// normalisation included — against a fake it names by path. +#[allow(clippy::too_many_lines)] +async fn run_rotate_with_exec( + args: &RotateArgs, + docker: &Path, + messages: &Messages, +) -> Result { let state_path = args .state_file .clone() @@ -173,7 +187,7 @@ pub(crate) async fn run_rotate(args: &RotateArgs, messages: &Messages) -> Result paths, state_dir, state_file: state_path, - docker: PathBuf::from(DOCKER_BIN), + docker: docker.to_path_buf(), }; // InfraCert operates on local files and Docker only — it must not @@ -282,7 +296,6 @@ pub(super) mod test_support { static ENV_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); pub(in crate::commands) const TEST_DOCKER_ARGS_ENV: &str = "BOOTROOT_TEST_DOCKER_ARGS"; - pub(super) const TEST_DOCKER_EXIT_ENV: &str = "BOOTROOT_TEST_DOCKER_EXIT"; pub(in crate::commands) struct ScopedEnvVar { key: &'static str, @@ -347,4 +360,310 @@ exit 0 } env::join_paths(paths).expect("PATH components should be valid") } + + /// Writes a fake `docker` at `path` that appends one record per + /// invocation to `args_log` and reads nothing from its environment. + /// + /// The log path is baked into the script text as it is written, so a + /// test handing this executable to production through the `docker` + /// seam gets its argv back without setting a single variable on this + /// process — which is the point, since the test does not construct + /// the `Command` that runs the fake. + pub(super) fn write_self_contained_fake_docker(path: &Path, args_log: &Path) { + write_self_contained_fake_docker_exiting(path, args_log, 0); + } + + /// [`write_self_contained_fake_docker`] whose fake exits `exit_code` + /// after logging, so a test can steer the failure path of a docker + /// call it does not spawn itself. + /// + /// Each invocation appends its argument count and then exactly that + /// many arguments, every field NUL-terminated: `docker restart c` + /// appends `2\0restart\0c\0` and `docker a '' b` appends + /// `3\0a\0\0b\0`. The record is framed by its count rather than + /// delimited by a byte, so an empty argument stays an empty field + /// and two invocations can never merge into one. + /// [`decode_fake_docker_log`] reads it back. + pub(super) fn write_self_contained_fake_docker_exiting( + path: &Path, + args_log: &Path, + exit_code: u8, + ) { + use std::os::unix::ffi::OsStrExt; + use std::os::unix::fs::PermissionsExt; + + // The script is assembled as bytes, not as a `String`: a Unix + // path is an arbitrary NUL-free byte sequence, and rendering + // `args_log` through `Display` would replace any byte that is + // not valid UTF-8, pointing the fake at a different path that + // nothing would ever create. + let mut script = b"#!/bin/sh\nset -eu\nprintf '%s\\0' \"$#\" \"$@\" >> ".to_vec(); + script.extend_from_slice(&shell_single_quote(args_log.as_os_str().as_bytes())); + script.extend_from_slice(format!("\nexit {exit_code}\n").as_bytes()); + fs::write(path, script).expect("fake docker script should be written"); + fs::set_permissions(path, fs::Permissions::from_mode(0o700)) + .expect("fake docker script should be executable"); + } + + /// Quotes `value` as a single POSIX shell word, byte for byte. + /// + /// A tempdir path may legally contain `'`, which ends the quoted + /// word; the usual `'\''` dance closes, escapes and reopens it. + /// Every other byte is copied through unchanged, so a path that is + /// not UTF-8 reaches the script intact. + fn shell_single_quote(value: &[u8]) -> Vec { + let mut quoted = vec![b'\'']; + for byte in value { + if *byte == b'\'' { + quoted.extend_from_slice(br"'\''"); + } else { + quoted.push(*byte); + } + } + quoted.push(b'\''); + quoted + } + + /// Decodes a log written by [`write_self_contained_fake_docker`], + /// returning one argument vector per invocation in call order. + /// + /// # Panics + /// + /// Panics if the log is unreadable or is not the framing the fake + /// writes — a missing count, or a record the file ends inside. + pub(super) fn decode_fake_docker_log(args_log: &Path) -> Vec> { + let bytes = fs::read(args_log).expect("fake docker log should be readable"); + if bytes.is_empty() { + return Vec::new(); + } + // Every field is NUL-terminated, so dropping the final + // terminator leaves the fields themselves — including an empty + // argument, which a terminator-less split would swallow. + let body = bytes + .strip_suffix(&[0]) + .expect("the fake terminates every field it writes"); + let mut fields = body.split(|byte| *byte == 0); + let mut invocations = Vec::new(); + while let Some(count_field) = fields.next() { + let count: usize = std::str::from_utf8(count_field) + .expect("argument count must be UTF-8") + .parse() + .expect("argument count must be a decimal number"); + let argv: Vec = fields + .by_ref() + .take(count) + .map(|field| String::from_utf8_lossy(field).into_owned()) + .collect(); + assert_eq!(argv.len(), count, "the log ends inside a record"); + invocations.push(argv); + } + invocations + } +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::path::Path; + use std::process::Command; + + use tempfile::tempdir; + + use super::test_support::{ + decode_fake_docker_log, write_self_contained_fake_docker, + write_self_contained_fake_docker_exiting, + }; + + /// Runs the fake once with `args`, the way production would. + fn run_fake(fake: &Path, args: &[&str]) { + let status = Command::new(fake) + .args(args) + .status() + .expect("the fake docker must be spawnable"); + assert!(status.success(), "the fake docker must exit 0"); + } + + /// A space-joined encoding cannot tell one argument holding a space + /// from two arguments, which is exactly what the `--user :` + /// assertion in `stepca_password` rests on. + #[test] + fn the_fake_docker_log_keeps_argument_boundaries() { + let dir = tempdir().expect("tempdir"); + let split_log = dir.path().join("split.log"); + let joined_log = dir.path().join("joined.log"); + let split = dir.path().join("split-docker"); + let joined = dir.path().join("joined-docker"); + write_self_contained_fake_docker(&split, &split_log); + write_self_contained_fake_docker(&joined, &joined_log); + + run_fake(&split, &["--user", "1000:1000"]); + run_fake(&joined, &["--user 1000:1000"]); + + assert_eq!( + decode_fake_docker_log(&split_log), + [["--user", "1000:1000"]] + ); + assert_eq!(decode_fake_docker_log(&joined_log), [["--user 1000:1000"]]); + assert_ne!( + decode_fake_docker_log(&split_log), + decode_fake_docker_log(&joined_log) + ); + } + + /// A multi-call flow must decode as its own invocations, in order, + /// rather than as the last call or one merged record. + #[test] + fn the_fake_docker_log_keeps_every_invocation_in_order() { + let dir = tempdir().expect("tempdir"); + let args_log = dir.path().join("docker_args.log"); + let fake = dir.path().join("fake-docker"); + write_self_contained_fake_docker(&fake, &args_log); + + run_fake(&fake, &["run", "first"]); + run_fake(&fake, &["kill", "-s", "SIGHUP", "c"]); + run_fake(&fake, &["restart", "c"]); + + assert_eq!( + decode_fake_docker_log(&args_log), + vec![ + vec!["run", "first"], + vec!["kill", "-s", "SIGHUP", "c"], + vec!["restart", "c"], + ] + ); + } + + /// The pair a doubled-NUL record terminator collapses: `["a", "", + /// "b"]` once and `["a"]` then `["b"]` encode to the same bytes + /// under that scheme, and must not under this one. + #[test] + fn the_fake_docker_log_frames_empty_arguments() { + let dir = tempdir().expect("tempdir"); + let single_log = dir.path().join("single.log"); + let pair_log = dir.path().join("pair.log"); + let single = dir.path().join("single-docker"); + let pair = dir.path().join("pair-docker"); + write_self_contained_fake_docker(&single, &single_log); + write_self_contained_fake_docker(&pair, &pair_log); + + run_fake(&single, &["a", "", "b"]); + run_fake(&pair, &["a"]); + run_fake(&pair, &["b"]); + + assert_eq!(decode_fake_docker_log(&single_log), [["a", "", "b"]]); + assert_eq!(decode_fake_docker_log(&pair_log), [["a"], ["b"]]); + assert_ne!( + decode_fake_docker_log(&single_log), + decode_fake_docker_log(&pair_log) + ); + } + + /// An argument-less invocation is the degenerate record, `0\0`, and + /// the count is the only thing that separates it from the next one: + /// it contributes no fields of its own, so a decoder that scanned + /// for a boundary instead of counting would swallow the record after + /// it. + #[test] + fn the_fake_docker_log_frames_an_argument_less_invocation() { + let dir = tempdir().expect("tempdir"); + let args_log = dir.path().join("docker_args.log"); + let fake = dir.path().join("fake-docker"); + write_self_contained_fake_docker(&fake, &args_log); + + run_fake(&fake, &[]); + run_fake(&fake, &["restart", "c"]); + run_fake(&fake, &[]); + + assert_eq!( + decode_fake_docker_log(&args_log), + vec![vec![], vec!["restart".to_string(), "c".to_string()], vec![]] + ); + } + + /// `tempfile::tempdir()` can legitimately hand back a path holding + /// an apostrophe, so the helper quotes rather than rejects one. + #[test] + fn the_fake_docker_handles_a_quoted_log_path() { + let dir = tempdir().expect("tempdir"); + let awkward = dir.path().join("it's a dir"); + fs::create_dir(&awkward).expect("create awkward dir"); + let args_log = awkward.join("docker args.log"); + let fake = dir.path().join("fake-docker"); + write_self_contained_fake_docker(&fake, &args_log); + + run_fake(&fake, &["restart", "c"]); + + assert_eq!(decode_fake_docker_log(&args_log), [["restart", "c"]]); + } + + /// The redirect the fake is written with must hold the log path's + /// own bytes. Rendering the path through `Display` instead replaces + /// every byte that is not valid UTF-8 with `U+FFFD`, which silently + /// aims the fake at a path nothing creates. + /// + /// This asserts on the script text rather than on running it, so it + /// holds on filesystems that would refuse to create the name. + #[test] + fn the_fake_docker_script_embeds_the_log_path_verbatim() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let dir = tempdir().expect("tempdir"); + let args_log = dir.path().join(OsStr::from_bytes(b"non\xffutf8.log")); + let fake = dir.path().join("fake-docker"); + write_self_contained_fake_docker(&fake, &args_log); + + let script = fs::read(&fake).expect("the fake docker script should be readable"); + let expected = args_log.as_os_str().as_bytes(); + assert!( + script + .windows(expected.len()) + .any(|window| window == expected), + "the script must redirect to the log path's own bytes" + ); + } + + /// A Unix path is an arbitrary NUL-free byte sequence, so a tempdir + /// rooted below one that is not UTF-8 — which `TMPDIR` can be — must + /// still yield a fake that logs where the test reads. + /// + /// The name is only creatable where the filesystem takes it: APFS + /// and other UTF-8-enforcing filesystems reject it with `EILSEQ`, + /// and on those the property is unobservable rather than broken. + #[test] + fn the_fake_docker_handles_a_non_utf8_log_path() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let dir = tempdir().expect("tempdir"); + let awkward = dir.path().join(OsStr::from_bytes(b"non\xffutf8")); + if fs::create_dir(&awkward).is_err() { + return; + } + let args_log = awkward.join("docker_args.log"); + let fake = dir.path().join("fake-docker"); + write_self_contained_fake_docker(&fake, &args_log); + + run_fake(&fake, &["restart", "c"]); + + assert_eq!(decode_fake_docker_log(&args_log), [["restart", "c"]]); + } + + /// The baked-in exit code is what replaces the environment variable + /// the shared `PATH` fake reads. + #[test] + fn the_fake_docker_reports_the_baked_in_exit_code() { + let dir = tempdir().expect("tempdir"); + let args_log = dir.path().join("docker_args.log"); + let fake = dir.path().join("fake-docker"); + write_self_contained_fake_docker_exiting(&fake, &args_log, 7); + + let status = Command::new(&fake) + .arg("run") + .status() + .expect("the fake docker must be spawnable"); + + assert_eq!(status.code(), Some(7)); + assert_eq!(decode_fake_docker_log(&args_log), [["run"]]); + } } diff --git a/src/commands/rotate/approle.rs b/src/commands/rotate/approle.rs index bfb0b63f..0538eef9 100644 --- a/src/commands/rotate/approle.rs +++ b/src/commands/rotate/approle.rs @@ -790,8 +790,7 @@ mod tests { use wiremock::{Mock, MockServer, ResponseTemplate}; use super::super::test_support::{ - ScopedEnvVar, TEST_DOCKER_ARGS_ENV, env_lock, path_with_prepend, test_messages, - write_fake_docker_script, + decode_fake_docker_log, test_messages, write_self_contained_fake_docker, }; use super::*; use crate::commands::compose_project::DOCKER_BIN; @@ -802,7 +801,11 @@ mod tests { /// Builds a rotate context whose compose directory records a /// non-default identity, so every by-name docker call these tests /// observe has to be `insight-*` rather than `bootroot-*`. - fn make_ctx(dir: &std::path::Path) -> RotateContext { + /// + /// `docker` is the executable every spawn in the rotation runs, so a + /// test that wants the argv — or wants the real `docker` kept out of + /// the run — points this at a fake instead of touching `PATH`. + fn make_ctx(dir: &std::path::Path, docker: &Path) -> RotateContext { fs::write(dir.join(".env"), "BOOTROOT_INSTANCE=insight\n").expect("write .env"); RotateContext { openbao_url: String::new(), @@ -827,7 +830,7 @@ mod tests { paths: super::super::StatePaths::new(dir.join("secrets")), state_dir: dir.to_path_buf(), state_file: dir.join("state.json"), - docker: PathBuf::from(DOCKER_BIN), + docker: docker.to_path_buf(), } } @@ -905,19 +908,12 @@ mod tests { }))) } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn rotate_infra_writes_secret_id_restarts_agent_and_verifies_login() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(APPROLE_BOOTROOT_STEPCA) @@ -926,7 +922,7 @@ mod tests { .await; mount_login_mock().expect(1).mount(&server).await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); let stepca_dir = ctx .paths .secrets_dir() @@ -967,24 +963,18 @@ mod tests { & 0o777; assert_eq!(mode, 0o600); } - let logged = fs::read_to_string(&args_log).expect("read docker args"); - let args: Vec<&str> = logged.lines().collect(); - assert_eq!(args, vec!["restart", "insight-openbao-agent-stepca"]); + assert_eq!( + decode_fake_docker_log(&args_log), + [["restart", "insight-openbao-agent-stepca"]] + ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn rotate_infra_backfills_missing_role_id_file() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; Mock::given(method("GET")) @@ -1002,7 +992,7 @@ mod tests { .await; mount_login_mock().mount(&server).await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); client.set_token("scoped-token".to_string()); let messages = test_messages(); @@ -1026,24 +1016,18 @@ mod tests { let role_id = fs::read_to_string(responder_dir.join(OPENBAO_AGENT_ROLE_ID_NAME)) .expect("role_id backfilled"); assert_eq!(role_id, "responder-role-id"); - let logged = fs::read_to_string(&args_log).expect("read docker args"); - let args: Vec<&str> = logged.lines().collect(); - assert_eq!(args, vec!["restart", "insight-openbao-agent-responder"]); + assert_eq!( + decode_fake_docker_log(&args_log), + [["restart", "insight-openbao-agent-responder"]] + ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn rotate_infra_permission_denied_hints_at_infra_credential() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; Mock::given(method("POST")) @@ -1056,7 +1040,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); let stepca_dir = ctx .paths .secrets_dir() @@ -1109,7 +1093,7 @@ mod tests { let server = MockServer::start().await; mount_provisioning_mocks(&server).await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); client.set_token("root-token".to_string()); let messages = test_messages(); @@ -1174,7 +1158,7 @@ mod tests { let server = MockServer::start().await; mount_provisioning_mocks(&server).await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); let label = AppRoleLabel::InfraRotate.to_string(); ctx.state .approles @@ -1423,7 +1407,7 @@ mod tests { #[tokio::test] async fn rotate_all_services_empty_registry_is_noop_success() { let dir = tempdir().expect("tempdir"); - let ctx = make_ctx(dir.path()); + let ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); // No OpenBao requests may happen; an unroutable URL makes any // accidental call fail loudly. let mut client = OpenBaoClient::new("http://127.0.0.1:1").expect("client"); @@ -1434,19 +1418,12 @@ mod tests { .expect("an empty service registry must be a no-op success"); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn rotate_all_services_rotates_local_and_remote_targets() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(&service_role_name("alpha")) @@ -1467,7 +1444,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); ctx.state.services.insert( "beta".to_string(), @@ -1495,19 +1472,12 @@ mod tests { ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn rotate_all_services_continues_after_per_target_failure() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; Mock::given(method("POST")) @@ -1527,7 +1497,7 @@ mod tests { .await; mount_login_mock().expect(1).mount(&server).await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); insert_local_service(&mut ctx, dir.path(), "beta"); @@ -1620,19 +1590,12 @@ mod tests { .respond_with(response) } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn self_mint_replaces_credential_file_after_successful_run() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(&service_role_name("alpha")) @@ -1652,7 +1615,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); let credential_path = dir.path().join("rotate-cred").join("secret_id"); fs::create_dir_all(credential_path.parent().expect("parent")).expect("create cred dir"); @@ -1699,19 +1662,12 @@ mod tests { ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn self_mint_skipped_with_warning_when_auth_not_file_based() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(&service_role_name("alpha")) @@ -1726,7 +1682,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); client.set_token("runtime-rotate-token".to_string()); @@ -1753,19 +1709,12 @@ mod tests { ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn no_self_mint_under_root_auth() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(&service_role_name("alpha")) @@ -1778,7 +1727,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); client.set_token("root-token".to_string()); @@ -1826,7 +1775,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); insert_local_service(&mut ctx, dir.path(), "alpha"); let credential_path = dir.path().join("rotate-cred").join("secret_id"); fs::create_dir_all(credential_path.parent().expect("parent")).expect("create cred dir"); @@ -1872,19 +1821,12 @@ mod tests { // fails its login verification must not replace the working file — // the old secret_id stays valid until TTL, so the next run // self-heals. - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn self_mint_verify_failure_keeps_old_credential_file() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(&service_role_name("alpha")) @@ -1904,7 +1846,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); let credential_path = dir.path().join("rotate-cred").join("secret_id"); fs::create_dir_all(credential_path.parent().expect("parent")).expect("create cred dir"); @@ -1951,19 +1893,12 @@ mod tests { ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn self_mint_applies_recorded_cidr_binding() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(&service_role_name("alpha")) @@ -1994,7 +1929,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); insert_local_service(&mut ctx, dir.path(), "alpha"); ctx.state.rotate_bound_cidrs.insert( AppRoleLabel::RuntimeRotate.to_string(), @@ -2030,19 +1965,12 @@ mod tests { ); } - // The env-var lock must be held across the `.await` to prevent - // parallel tests from seeing a corrupted PATH. - #[allow(clippy::await_holding_lock)] #[tokio::test] async fn infra_invocation_self_mints_the_infra_rotate_credential() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake_docker = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake_docker, &args_log); let server = MockServer::start().await; mount_secret_id_mock(APPROLE_BOOTROOT_STEPCA) @@ -2062,7 +1990,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), &fake_docker); let stepca_dir = ctx .paths .secrets_dir() @@ -2113,7 +2041,7 @@ mod tests { #[tokio::test] async fn rotate_bound_cidrs_rejected_without_root_auth() { let dir = tempdir().expect("tempdir"); - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); // No OpenBao requests may happen; an unroutable URL makes any // accidental call fail loudly. let mut client = OpenBaoClient::new("http://127.0.0.1:1").expect("client"); @@ -2179,7 +2107,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); client.set_token("root-token".to_string()); let messages = test_messages(); @@ -2249,7 +2177,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); let label = AppRoleLabel::InfraRotate.to_string(); ctx.state .approles @@ -2316,7 +2244,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); let label = AppRoleLabel::InfraRotate.to_string(); ctx.state .approles @@ -2395,7 +2323,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); ctx.state.rotate_secret_id_ttl = Some("48h".to_string()); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); client.set_token("root-token".to_string()); @@ -2408,7 +2336,7 @@ mod tests { #[tokio::test] async fn clear_rotate_bound_cidrs_rejected_without_root_auth() { let dir = tempdir().expect("tempdir"); - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); // No OpenBao requests may happen; an unroutable URL makes any // accidental call fail loudly. let mut client = OpenBaoClient::new("http://127.0.0.1:1").expect("client"); @@ -2445,7 +2373,7 @@ mod tests { .mount(&server) .await; - let mut ctx = make_ctx(dir.path()); + let mut ctx = make_ctx(dir.path(), Path::new(DOCKER_BIN)); insert_local_service(&mut ctx, dir.path(), "alpha"); let mut client = OpenBaoClient::new(&server.uri()).expect("client"); diff --git a/src/commands/rotate/helpers.rs b/src/commands/rotate/helpers.rs index 4239be49..4a872ccc 100644 --- a/src/commands/rotate/helpers.rs +++ b/src/commands/rotate/helpers.rs @@ -204,23 +204,18 @@ mod tests { use tempfile::tempdir; use super::super::test_support::{ - ScopedEnvVar, TEST_DOCKER_ARGS_ENV, env_lock, path_with_prepend, test_messages, - write_fake_docker_script, + decode_fake_docker_log, test_messages, write_self_contained_fake_docker, }; use super::*; use crate::commands::compose_project::DEFAULT_INSTANCE_NAME; - /// Runs `restart_openbao_agent` against a fake `docker` on `PATH` - /// and returns the argument vector it was given. - fn restart_args(instance: &str, container: BootrootContainer) -> Vec { + /// Runs `restart_openbao_agent` against a fake `docker` handed in + /// through the executable seam and returns the invocations it saw. + fn restart_invocations(instance: &str, container: BootrootContainer) -> Vec> { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - std::fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake, &args_log); std::fs::write( dir.path().join(".env"), @@ -230,16 +225,12 @@ mod tests { restart_openbao_agent( &dir.path().join("docker-compose.yml"), container, - Path::new("docker"), + &fake, &test_messages(), ) .expect("restarting the sidecar must succeed"); - std::fs::read_to_string(&args_log) - .expect("read docker args") - .lines() - .map(str::to_string) - .collect() + decode_fake_docker_log(&args_log) } /// The sidecars bypass Compose's project scoping, so `rotate db` and @@ -250,8 +241,8 @@ mod tests { #[test] fn restarting_the_stepca_agent_names_the_recorded_instance() { assert_eq!( - restart_args("insight", BootrootContainer::OpenBaoAgentStepCa), - vec!["restart", "insight-openbao-agent-stepca"] + restart_invocations("insight", BootrootContainer::OpenBaoAgentStepCa), + [["restart", "insight-openbao-agent-stepca"]] ); } @@ -260,8 +251,8 @@ mod tests { #[test] fn restarting_the_responder_agent_names_the_recorded_instance() { assert_eq!( - restart_args("insight", BootrootContainer::OpenBaoAgentResponder), - vec!["restart", "insight-openbao-agent-responder"] + restart_invocations("insight", BootrootContainer::OpenBaoAgentResponder), + [["restart", "insight-openbao-agent-responder"]] ); } @@ -271,18 +262,14 @@ mod tests { #[test] fn restarting_a_sidecar_without_a_recorded_identity_keeps_the_default_name() { let dir = tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - std::fs::create_dir(&bin_dir).expect("create bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); + let fake = dir.path().join("fake-docker"); let args_log = dir.path().join("docker_args.log"); - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log.as_os_str()); + write_self_contained_fake_docker(&fake, &args_log); restart_openbao_agent( &dir.path().join("docker-compose.yml"), BootrootContainer::OpenBaoAgentStepCa, - Path::new("docker"), + &fake, &test_messages(), ) .expect("restarting the sidecar must succeed"); @@ -291,10 +278,9 @@ mod tests { // bare default-instance sidecar name in `src/` is what // `container_name`'s single-declaration guard forbids. let expected = BootrootContainer::OpenBaoAgentStepCa.name(DEFAULT_INSTANCE_NAME); - let logged = std::fs::read_to_string(&args_log).expect("read docker args"); assert_eq!( - logged.lines().collect::>(), - vec!["restart", expected.as_str()] + decode_fake_docker_log(&args_log), + [["restart", expected.as_str()]] ); } diff --git a/src/commands/rotate/infra_cert.rs b/src/commands/rotate/infra_cert.rs index 5e4cc70b..c47c4313 100644 --- a/src/commands/rotate/infra_cert.rs +++ b/src/commands/rotate/infra_cert.rs @@ -440,8 +440,7 @@ mod tests { use std::fs; use super::super::test_support::{ - ScopedEnvVar, TEST_DOCKER_ARGS_ENV, env_lock, path_with_prepend, test_messages, - write_fake_docker_script, + decode_fake_docker_log, test_messages, write_self_contained_fake_docker, }; use super::*; use crate::cli::args::{ @@ -455,41 +454,12 @@ mod tests { OPENBAO_INFRA_CERT_KEY, OPENBAO_TLS_CERT_PATH, OPENBAO_TLS_DEFAULT_RENEW_BEFORE, OPENBAO_TLS_KEY_PATH, }; - use crate::commands::rotate::run_rotate; + use crate::commands::rotate::{run_rotate, run_rotate_with_exec}; use crate::state::StateFile; /// The `OpenBao` container name a default install renders. const DEFAULT_OPENBAO_CONTAINER: &str = "bootroot-openbao"; - /// Writes a fake `docker` that appends one line per invocation to - /// `args_log` and reads nothing from its environment. - /// - /// The log path is baked into the script text as it is written, so a - /// test handing this executable through the docker seam gets its - /// argv back without setting a single variable on this process. - /// `init::steps::test_support` carries a twin for the `init` tree. - /// The one module both trees can reach, `rotate::test_support`, - /// holds the `PATH`-based fake every unconverted test still depends - /// on and stays untouched until those conversions land; the `init` - /// twin is scoped to its own tree and is not visible here. The - /// duplication is what that costs, and it goes away when the - /// conversions are free to rework the shared harness. - fn write_self_contained_fake_docker(path: &Path, args_log: &Path) { - use std::os::unix::fs::PermissionsExt; - - let log = args_log.display().to_string(); - assert!( - !log.contains('\''), - "the log path is interpolated into a single-quoted shell word" - ); - let script = format!( - "#!/bin/sh\nset -eu\n{{ printf '%s ' \"$@\"; printf '\\n'; }} >> '{log}'\nexit 0\n" - ); - fs::write(path, script).expect("fake docker script should be written"); - fs::set_permissions(path, fs::Permissions::from_mode(0o700)) - .expect("fake docker script should be executable"); - } - fn make_openbao_infra_entry(compose_dir: &std::path::Path) -> InfraCertEntry { InfraCertEntry { cert_path: compose_dir.join(OPENBAO_TLS_CERT_PATH), @@ -585,10 +555,9 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let messages = test_messages(); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("bin dir"); - let docker_path = bin_dir.join("docker"); - write_fake_docker_script(&docker_path); + let fake_docker = dir.path().join("fake-docker"); + let args_log = dir.path().join("docker_args.log"); + write_self_contained_fake_docker(&fake_docker, &args_log); let compose_dir = dir.path().join("compose"); let secrets_dir = dir.path().join("secrets"); @@ -665,13 +634,7 @@ mod tests { show_secrets: false, }; - let args_log = dir.path().join("docker_args.log"); - - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _log = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, &args_log); - - rt.block_on(run_rotate(&args, &messages)) + rt.block_on(run_rotate_with_exec(&args, &fake_docker, &messages)) .expect("run_rotate(InfraCert) must succeed and verify the swap"); // The updated state must have been written back to the @@ -701,16 +664,22 @@ mod tests { "state must not be written to hardcoded state.json" ); - // The fake docker log is truncated on every call, so its final - // contents are the last docker invocation — the reload signal. - let log = fs::read_to_string(&args_log).unwrap_or_default(); + // The fake keeps every invocation, so the reload signal is + // pinned as one whole argv rather than as bytes anywhere in the + // log, and the restart check now covers the entire run. + let invocations = decode_fake_docker_log(&args_log); assert!( - log.contains("kill") && log.contains("SIGHUP"), - "final docker call must be kill -s SIGHUP, got: {log}" + invocations.contains(&vec![ + "kill".to_string(), + "-s".to_string(), + "SIGHUP".to_string(), + DEFAULT_OPENBAO_CONTAINER.to_string(), + ]), + "the reload must be kill -s SIGHUP against the openbao container, got: {invocations:?}" ); assert!( - !log.contains("restart"), - "openbao entry must never trigger docker restart, got: {log}" + !invocations.iter().flatten().any(|arg| arg == "restart"), + "openbao entry must never trigger docker restart, got: {invocations:?}" ); } @@ -723,10 +692,9 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let messages = test_messages(); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("bin dir"); - let docker_path = bin_dir.join("docker"); - write_fake_docker_script(&docker_path); + let fake_docker = dir.path().join("fake-docker"); + let args_log = dir.path().join("docker_args.log"); + write_self_contained_fake_docker(&fake_docker, &args_log); let compose_dir = dir.path().join("compose"); let secrets_dir = dir.path().join("secrets"); @@ -793,15 +761,9 @@ mod tests { show_secrets: false, }; - let args_log = dir.path().join("docker_args.log"); - - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _log = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, &args_log); - let rt = tokio::runtime::Runtime::new().expect("tokio runtime"); let err = rt - .block_on(run_rotate(&args, &messages)) + .block_on(run_rotate_with_exec(&args, &fake_docker, &messages)) .expect_err("unreachable probe target must fail the command"); let chain = format!("{err:#}"); assert!( @@ -815,10 +777,10 @@ mod tests { // The reload signal (`kill`) must have run before the probe, so // the container was never restarted. - let log = fs::read_to_string(&args_log).unwrap_or_default(); + let invocations = decode_fake_docker_log(&args_log); assert!( - !log.contains("restart"), - "openbao entry must never trigger docker restart, got: {log}" + !invocations.iter().flatten().any(|arg| arg == "restart"), + "openbao entry must never trigger docker restart, got: {invocations:?}" ); } @@ -852,10 +814,9 @@ mod tests { let dir = tempfile::tempdir().expect("tempdir"); let messages = test_messages(); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("bin dir"); - let docker_path = bin_dir.join("docker"); - write_fake_docker_script(&docker_path); + let fake_docker = dir.path().join("fake-docker"); + let args_log = dir.path().join("docker_args.log"); + write_self_contained_fake_docker(&fake_docker, &args_log); let compose_dir = dir.path().join("compose"); let secrets_dir = dir.path().join("secrets"); @@ -920,14 +881,8 @@ mod tests { show_secrets: false, }; - let args_log = dir.path().join("docker_args.log"); - - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _log = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, &args_log); - let rt = tokio::runtime::Runtime::new().expect("tokio runtime"); - rt.block_on(run_rotate(&args, &messages)) + rt.block_on(run_rotate_with_exec(&args, &fake_docker, &messages)) .expect("run_rotate(InfraCert) must succeed for http01 admin entry"); let reloaded = StateFile::load(&state_file).expect("state file must be readable"); @@ -942,10 +897,15 @@ mod tests { // Verify the fake docker received a `kill -s SIGHUP` command // (the ContainerSignal reload strategy). - let log = fs::read_to_string(&args_log).unwrap_or_default(); + let invocations = decode_fake_docker_log(&args_log); assert!( - log.contains("kill") && log.contains("SIGHUP"), - "docker must have been called with kill -s SIGHUP, got: {log}" + invocations.contains(&vec![ + "kill".to_string(), + "-s".to_string(), + "SIGHUP".to_string(), + RESPONDER_SERVICE_NAME.to_string(), + ]), + "docker must have been called with kill -s SIGHUP, got: {invocations:?}" ); // The HTTP-01 admin entry gets no post-reload verification, so no // openbao_url is consulted and no probe runs. @@ -995,43 +955,14 @@ mod tests { ); } - /// The signal has to reach the instance's own responder container, - /// not a co-located install's. - #[test] - fn container_signal_addresses_the_named_container() { - let dir = tempfile::tempdir().expect("tempdir"); - let bin_dir = dir.path().join("bin"); - fs::create_dir(&bin_dir).expect("bin dir"); - write_fake_docker_script(&bin_dir.join("docker")); - let args_log = dir.path().join("docker_args.log"); - - let _lock = env_lock(); - let _path = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _log = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, &args_log); - - execute_reload_strategy( - &ReloadStrategy::ContainerSignal { - container_name: "insight-http01".to_string(), - signal: "SIGHUP".to_string(), - }, - Path::new("docker"), - ) - .expect("signalling must succeed against the fake docker"); - - let logged = fs::read_to_string(&args_log).expect("read docker args"); - let logged_args: Vec<&str> = logged.lines().collect(); - assert_eq!(logged_args, vec!["kill", "-s", "SIGHUP", "insight-http01"]); - } - /// The seam at the signal spawn: `execute_reload_strategy` kills the /// container with whichever program its caller named, which in the - /// flow is `ctx.docker`. + /// flow is `ctx.docker`. The signal therefore has to reach the + /// instance's own responder container, not a co-located install's. /// /// The test mutates nothing process-global — no `PATH` edit, no /// variable set, no lock — because the fake carries its own - /// argv-log path in its script text. That is the property the seam - /// exists to make possible, and the model the conversion of this - /// file's remaining `PATH` fakes follows. + /// argv-log path in its script text. #[test] fn container_signal_runs_the_supplied_executable() { let dir = tempfile::tempdir().expect("tempdir"); @@ -1048,10 +979,9 @@ mod tests { ) .expect("signalling must succeed against the supplied executable"); - let logged = fs::read_to_string(&args_log).expect("read docker args"); assert_eq!( - logged.lines().collect::>(), - vec!["kill -s SIGHUP insight-http01 "], + decode_fake_docker_log(&args_log), + [["kill", "-s", "SIGHUP", "insight-http01"]], "the supplied executable must have received the unchanged argv" ); } diff --git a/src/commands/rotate/stepca_password.rs b/src/commands/rotate/stepca_password.rs index b9711fc7..bbe7b861 100644 --- a/src/commands/rotate/stepca_password.rs +++ b/src/commands/rotate/stepca_password.rs @@ -168,22 +168,18 @@ mod tests { use tempfile::tempdir; - use super::super::test_support::*; + use super::super::test_support::{ + decode_fake_docker_log, test_messages, write_self_contained_fake_docker, + write_self_contained_fake_docker_exiting, + }; use super::*; #[test] fn change_stepca_passphrase_invokes_docker_with_force_and_expected_paths() { - let _lock = env_lock(); let temp = tempdir().expect("tempdir"); - let bin_dir = temp.path().join("bin"); - fs::create_dir_all(&bin_dir).expect("bin dir"); - let docker_path = bin_dir.join("docker"); - write_fake_docker_script(&docker_path); - + let docker_path = temp.path().join("fake-docker"); let args_log_path = temp.path().join("docker-args.log"); - let _path_guard = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args_guard = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log_path.as_os_str()); - let _exit_guard = ScopedEnvVar::set(TEST_DOCKER_EXIT_ENV, "0"); + write_self_contained_fake_docker(&docker_path, &args_log_path); let secrets_dir = temp.path().join("secrets"); fs::create_dir_all(secrets_dir.join("secrets")).expect("create secrets key dir"); @@ -199,13 +195,12 @@ mod tests { ¤t_password, &new_password, &key_path, - Path::new("docker"), + &docker_path, &test_messages(), ) .expect("change passphrase should succeed"); - let logged_args = fs::read_to_string(&args_log_path).expect("read logged args"); - let args: Vec<&str> = logged_args.lines().collect(); + let invocations = decode_fake_docker_log(&args_log_path); let mount_root = fs::canonicalize(&secrets_dir).expect("canonicalize secrets dir"); let expected_mount = format!("{}:/home/step", mount_root.display()); // The container must run as the secrets-directory owner, not root, @@ -230,7 +225,7 @@ mod tests { "/home/step/password.txt.new", "-f", ]; - assert_eq!(args, expected); + assert_eq!(invocations, vec![expected]); } #[test] @@ -259,17 +254,10 @@ mod tests { #[test] fn change_stepca_passphrase_surfaces_docker_failure_status() { - let _lock = env_lock(); let temp = tempdir().expect("tempdir"); - let bin_dir = temp.path().join("bin"); - fs::create_dir_all(&bin_dir).expect("bin dir"); - let docker_path = bin_dir.join("docker"); - write_fake_docker_script(&docker_path); - + let docker_path = temp.path().join("fake-docker"); let args_log_path = temp.path().join("docker-args.log"); - let _path_guard = ScopedEnvVar::set("PATH", path_with_prepend(&bin_dir)); - let _args_guard = ScopedEnvVar::set(TEST_DOCKER_ARGS_ENV, args_log_path.as_os_str()); - let _exit_guard = ScopedEnvVar::set(TEST_DOCKER_EXIT_ENV, "7"); + write_self_contained_fake_docker_exiting(&docker_path, &args_log_path, 7); let secrets_dir = temp.path().join("secrets"); fs::create_dir_all(secrets_dir.join("secrets")).expect("create secrets key dir"); @@ -285,7 +273,7 @@ mod tests { ¤t_password, &new_password, &key_path, - Path::new("docker"), + &docker_path, &test_messages(), ) .expect_err("docker failure should bubble up");