fix(clawdie): harden FreeBSD installer plan (Sam & Codex)
Use the clawdie service user in the generated FreeBSD rc.d script, chown state directories after the user is created, and reject unknown existing ZFS pools before rendering/applying a plan. Update the FreeBSD validation handoff to cover these checks.\n\nFreeBSD checks: cargo fmt --check; ./scripts/check-format.sh; git diff --check; cargo test -p clawdie -- --nocapture; cargo clippy -p clawdie --all-targets -- -D warnings; cargo build -p clawdie --release; target/release/clawdie discover; target/release/clawdie plan; target/release/clawdie apply --pool zroot (dry-run); target/release/clawdie plan --pool does-not-exist (expected error).
This commit is contained in:
parent
2cdefc00a0
commit
4abc2c5294
5 changed files with 111 additions and 27 deletions
|
|
@ -37,9 +37,10 @@ ZFS layout under the pool:
|
|||
<pool>/clawdie/log -> /var/log/clawdie
|
||||
```
|
||||
|
||||
Plain-dirs fallback creates the same mountpoints as ordinary directories owned by
|
||||
the `clawdie` user. Either way, a `clawdie` service user and the rc.d script
|
||||
(FreeBSD) or systemd unit (Linux) are installed and enabled to run
|
||||
Plain-dirs fallback creates the same mountpoints as ordinary directories. Either
|
||||
way, a `clawdie` service user is created, the state directories are owned by
|
||||
`clawdie:clawdie`, and the rc.d script (FreeBSD, via `daemon -u clawdie`) or
|
||||
systemd unit (Linux, `User=clawdie`) is installed and enabled to run
|
||||
`/usr/local/bin/colibri-daemon`.
|
||||
|
||||
## Safety
|
||||
|
|
|
|||
|
|
@ -76,6 +76,31 @@ fn pick_pool(explicit: Option<String>) -> Result<Option<String>, String> {
|
|||
}
|
||||
}
|
||||
|
||||
fn validate_existing_pool(pool: &str, pools: &[zfs::Pool]) -> Result<(), String> {
|
||||
if pools.iter().any(|p| p.name == pool) {
|
||||
return Ok(());
|
||||
}
|
||||
let available = if pools.is_empty() {
|
||||
"none".to_string()
|
||||
} else {
|
||||
pools
|
||||
.iter()
|
||||
.map(|p| p.name.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
};
|
||||
Err(format!(
|
||||
"ZFS pool `{pool}` not found; available pools: {available}"
|
||||
))
|
||||
}
|
||||
|
||||
fn validate_storage(storage: &Storage) -> Result<(), String> {
|
||||
if let Storage::ZfsExisting { pool } = storage {
|
||||
validate_existing_pool(pool, &zfs::list_pools())?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Refuse to create a pool on a disk that isn't clearly empty, unless --force.
|
||||
fn validate_create_device(device: &str, force: bool) -> Result<(), String> {
|
||||
if force {
|
||||
|
|
@ -151,6 +176,7 @@ fn run() -> Result<(), String> {
|
|||
pick_pool(pool)?,
|
||||
create_pool,
|
||||
)?;
|
||||
validate_storage(&storage)?;
|
||||
let (pf, svc, steps) = deploy_steps(&storage);
|
||||
print!("{}", plan::render(pf.as_ref(), &storage, &svc, &steps));
|
||||
offer_zfs_if_plain(&storage);
|
||||
|
|
@ -168,6 +194,7 @@ fn run() -> Result<(), String> {
|
|||
pick_pool(pool)?,
|
||||
create_pool,
|
||||
)?;
|
||||
validate_storage(&storage)?;
|
||||
if let Storage::ZfsCreate { device, .. } = &storage {
|
||||
validate_create_device(device, force)?;
|
||||
}
|
||||
|
|
@ -215,3 +242,30 @@ fn main() -> ExitCode {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn validate_existing_pool_accepts_known_pool() {
|
||||
let pools = vec![zfs::Pool {
|
||||
name: "zroot".to_string(),
|
||||
size: "100G".to_string(),
|
||||
health: "ONLINE".to_string(),
|
||||
}];
|
||||
assert!(validate_existing_pool("zroot", &pools).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_existing_pool_rejects_unknown_pool() {
|
||||
let pools = vec![zfs::Pool {
|
||||
name: "zroot".to_string(),
|
||||
size: "100G".to_string(),
|
||||
health: "ONLINE".to_string(),
|
||||
}];
|
||||
let err = validate_existing_pool("missing", &pools).unwrap_err();
|
||||
assert!(err.contains("missing"));
|
||||
assert!(err.contains("zroot"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -82,7 +82,6 @@ fn zfs_dataset_steps(pool: &str, svc: &ServiceSpec) -> Vec<Step> {
|
|||
}
|
||||
|
||||
fn plain_dir_steps(svc: &ServiceSpec) -> Vec<Step> {
|
||||
let owner = format!("{}:{}", svc.user, svc.user);
|
||||
vec![
|
||||
Step::run(
|
||||
format!("create {}", svc.data_dir),
|
||||
|
|
@ -92,16 +91,6 @@ fn plain_dir_steps(svc: &ServiceSpec) -> Vec<Step> {
|
|||
"create /var/log/clawdie",
|
||||
&["mkdir", "-p", "/var/log/clawdie"],
|
||||
),
|
||||
Step::run(
|
||||
"own clawdie directories",
|
||||
&[
|
||||
"chown",
|
||||
"-R",
|
||||
owner.as_str(),
|
||||
svc.data_dir.as_str(),
|
||||
"/var/log/clawdie",
|
||||
],
|
||||
),
|
||||
]
|
||||
}
|
||||
|
||||
|
|
@ -232,11 +221,11 @@ mod tests {
|
|||
#[test]
|
||||
fn plan_shapes() {
|
||||
let svc = ServiceSpec::default();
|
||||
// plain dirs: 3 dir steps + 4 linux service steps
|
||||
// plain dirs: 2 dir steps + 5 linux service steps
|
||||
let s = build(&Linux, &Storage::PlainDirs, &svc);
|
||||
assert_eq!(s.len(), 7);
|
||||
assert!(matches!(&s[0].action, Action::Run(a) if a[0] == "mkdir"));
|
||||
// create pool: 1 zpool + 3 datasets + 3 freebsd service steps
|
||||
// create pool: 1 zpool + 3 dataset steps + 4 freebsd service steps
|
||||
let c = build(
|
||||
&FreeBsd,
|
||||
&Storage::ZfsCreate {
|
||||
|
|
@ -248,7 +237,7 @@ mod tests {
|
|||
assert!(
|
||||
matches!(&c[0].action, Action::Run(a) if a[0] == "zpool" && a.contains(&"/dev/sdb".to_string()))
|
||||
);
|
||||
assert_eq!(c.len(), 7);
|
||||
assert_eq!(c.len(), 8);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -100,10 +100,12 @@ impl Platform for FreeBsd {
|
|||
fn install_service_steps(&self, s: &ServiceSpec) -> Vec<Step> {
|
||||
let rc_path = format!("/usr/local/etc/rc.d/{}", s.name);
|
||||
let rc = format!(
|
||||
"#!/bin/sh\n# PROVIDE: {name}\n# REQUIRE: LOGIN\n# KEYWORD: shutdown\n. /etc/rc.subr\nname=\"{name}\"\nrcvar=\"{name}_enable\"\ncommand=\"/usr/sbin/daemon\"\ncommand_args=\"-f -o /var/log/clawdie/daemon.log {exec}\"\nload_rc_config $name\n: ${{{name}_enable:=NO}}\nrun_rc_command \"$1\"\n",
|
||||
"#!/bin/sh\n# PROVIDE: {name}\n# REQUIRE: LOGIN\n# KEYWORD: shutdown\n. /etc/rc.subr\nname=\"{name}\"\nrcvar=\"{name}_enable\"\ncommand=\"/usr/sbin/daemon\"\ncommand_args=\"-f -u {user} -o /var/log/clawdie/daemon.log {exec}\"\nload_rc_config $name\n: ${{{name}_enable:=NO}}\nrun_rc_command \"$1\"\n",
|
||||
name = s.name,
|
||||
user = s.user,
|
||||
exec = s.exec,
|
||||
);
|
||||
let owner = format!("{}:{}", s.user, s.user);
|
||||
vec![
|
||||
Step::run(
|
||||
format!("create service user {}", s.user),
|
||||
|
|
@ -119,6 +121,16 @@ impl Platform for FreeBsd {
|
|||
"Clawdie host service",
|
||||
],
|
||||
),
|
||||
Step::run(
|
||||
"own clawdie state directories",
|
||||
&[
|
||||
"chown",
|
||||
"-R",
|
||||
owner.as_str(),
|
||||
s.data_dir.as_str(),
|
||||
"/var/log/clawdie",
|
||||
],
|
||||
),
|
||||
Step::write(format!("install rc.d script {rc_path}"), rc_path, 0o555, rc),
|
||||
Step::run(
|
||||
format!("enable {} service", s.name),
|
||||
|
|
@ -141,6 +153,7 @@ impl Platform for Linux {
|
|||
user = s.user,
|
||||
exec = s.exec,
|
||||
);
|
||||
let owner = format!("{}:{}", s.user, s.user);
|
||||
vec![
|
||||
Step::run(
|
||||
format!("create service user {}", s.user),
|
||||
|
|
@ -154,6 +167,16 @@ impl Platform for Linux {
|
|||
&s.user,
|
||||
],
|
||||
),
|
||||
Step::run(
|
||||
"own clawdie state directories",
|
||||
&[
|
||||
"chown",
|
||||
"-R",
|
||||
owner.as_str(),
|
||||
s.data_dir.as_str(),
|
||||
"/var/log/clawdie",
|
||||
],
|
||||
),
|
||||
Step::write(
|
||||
format!("install systemd unit {unit_path}"),
|
||||
unit_path,
|
||||
|
|
@ -185,10 +208,22 @@ mod tests {
|
|||
fn freebsd_steps_shape() {
|
||||
let steps = FreeBsd.install_service_steps(&ServiceSpec::default());
|
||||
assert_eq!(FreeBsd.service_kind(), "rc.d");
|
||||
// user create, rc.d write, sysrc enable
|
||||
assert_eq!(steps.len(), 3);
|
||||
assert!(matches!(steps[1].action, Action::WriteFile { .. }));
|
||||
if let Action::Run(argv) = &steps[2].action {
|
||||
// user create, ownership, rc.d write, sysrc enable
|
||||
assert_eq!(steps.len(), 4);
|
||||
if let Action::Run(argv) = &steps[1].action {
|
||||
assert_eq!(argv[0], "chown");
|
||||
assert!(argv.contains(&"clawdie:clawdie".to_string()));
|
||||
} else {
|
||||
panic!("expected chown run step");
|
||||
}
|
||||
assert!(matches!(steps[2].action, Action::WriteFile { .. }));
|
||||
if let Action::WriteFile { contents, .. } = &steps[2].action {
|
||||
assert!(contents.contains("daemon\""));
|
||||
assert!(contents.contains("-u clawdie"));
|
||||
} else {
|
||||
panic!("expected rc.d write step");
|
||||
}
|
||||
if let Action::Run(argv) = &steps[3].action {
|
||||
assert_eq!(argv[0], "sysrc");
|
||||
assert_eq!(argv[1], "clawdie_enable=YES");
|
||||
} else {
|
||||
|
|
@ -200,8 +235,13 @@ mod tests {
|
|||
fn linux_steps_shape() {
|
||||
let steps = Linux.install_service_steps(&ServiceSpec::default());
|
||||
assert_eq!(Linux.service_kind(), "systemd");
|
||||
assert_eq!(steps.len(), 4);
|
||||
if let Action::WriteFile { path, .. } = &steps[1].action {
|
||||
assert_eq!(steps.len(), 5);
|
||||
if let Action::Run(argv) = &steps[1].action {
|
||||
assert_eq!(argv[0], "chown");
|
||||
} else {
|
||||
panic!("expected chown run step");
|
||||
}
|
||||
if let Action::WriteFile { path, .. } = &steps[2].action {
|
||||
assert_eq!(path, "/etc/systemd/system/clawdie.service");
|
||||
} else {
|
||||
panic!("expected unit write step");
|
||||
|
|
|
|||
|
|
@ -26,7 +26,7 @@ clawdie plan # expect: resolves to the single pool (or pass --pool), ZFS
|
|||
Verify:
|
||||
|
||||
- `discover` lists the real pools (parsed from `zpool list -H`) and any clawdie datasets.
|
||||
- On FreeBSD `plan` always resolves to **ZFS** (never plain-dirs); with no pool it errors clearly.
|
||||
- On FreeBSD `plan` always resolves to **ZFS** (never plain-dirs); with no pool or an unknown explicit pool it errors clearly.
|
||||
- Expected gap: the `disks:` section uses `lsblk` (Linux-only), so it is **empty on FreeBSD** — that is correct. `--create-pool` is a Linux convenience; FreeBSD already has ZFS.
|
||||
|
||||
## 3. Provisioning — DESTRUCTIVE, scratch host / VM / test pool only
|
||||
|
|
@ -39,9 +39,9 @@ clawdie apply --pool <testpool> --yes # provisions
|
|||
Validate after `--yes`:
|
||||
|
||||
1. **Datasets** (`zfs list`): `<testpool>/clawdie` (canmount=off), `…/db` → `/var/db/clawdie`, `…/log` → `/var/log/clawdie`.
|
||||
2. **Service user** (`pw usershow clawdie`): nologin, home `/var/db/clawdie`.
|
||||
2. **Service user + ownership** (`pw usershow clawdie`, `ls -ld /var/db/clawdie /var/log/clawdie`): nologin, home `/var/db/clawdie`, state directories owned by `clawdie:clawdie`.
|
||||
3. **rc.d service**: `/usr/local/etc/rc.d/clawdie` (mode 0555) and `clawdie_enable=YES` (`sysrc`). Check `service clawdie enabled` / `rcvar`.
|
||||
- The script execs `/usr/local/bin/colibri-daemon`; if that binary isn't staged, `service clawdie start` will fail on the missing exec — expected. Confirm the script + enable are correct regardless.
|
||||
- The script execs `/usr/local/bin/colibri-daemon` through `/usr/sbin/daemon -u clawdie`; if that binary isn't staged, `service clawdie start` will fail on the missing exec — expected. Confirm the script + enable are correct regardless.
|
||||
4. **Teardown** to reset:
|
||||
```sh
|
||||
service clawdie stop 2>/dev/null; sysrc -x clawdie_enable
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue