smoke: cover the gaps — NAT, vm ports/restart/kill/prune, workspace variants, ssh-config

Audit of banger's advertised CLI surface vs. what smoke was exercising
turned up several gaps where a regression would have shipped silently.
New scenarios:

- NAT: asserts the per-VM POSTROUTING MASQUERADE rule is installed
  with --nat (scoped to the guest /32), idempotent across stop/start,
  and torn down on delete. End-to-end curl tests don't work here
  because the bridge IP and uplink IP both belong to the host — a
  guest reaching the uplink lands on host-local loopback whether
  MASQUERADE is set up or not — so the test pins the iptables rule
  itself. Skipped if passwordless `sudo iptables` isn't available.

- vm ports: sshd :22 must be visible with the <name>.vm endpoint
  (not localhost, not the raw guest IP — the daemon prefers the
  DNS record when one exists).

- vm restart: dedicated verb, not a stop+start alias. Asserts a
  fresh boot_id to prove the kernel actually recycled.

- vm kill --signal KILL: forceful termination path (distinct from
  `vm stop`'s graceful Ctrl-Alt-Del). Post-kill state must be
  'stopped' (not 'error') and the dm-snapshot must be cleaned up.

- vm prune -f: batch delete of non-running VMs while preserving any
  that are still running. Regression guard for the case where prune
  could wipe a live session.

- workspace prepare --readonly: mode bits on /root/repo/<file>
  must drop all write bits. Enforcement is advisory against a root
  guest, so the test asserts the bits, not EACCES.

- workspace prepare --mode full_copy: alternate transfer path
  (tarred into rootfs, no overlay) still lands the repo contents
  at /root/repo.

- workspace export --base-commit: guest-side commits captured in
  the patch when the pre-commit SHA is pinned. The feature's whole
  reason for existing; it had zero coverage. Includes a control
  assertion that the plain (no --base-commit) export does NOT see
  the committed file.

- ssh-config --install / --uninstall: HOME-isolated to a smoke
  tempdir so we don't touch the invoking user's ~/.ssh/config.
  Seeds a pre-existing config to catch any regression where
  install clobbers instead of appending. Asserts idempotency
  (second install doesn't duplicate the Include line) and clean
  round-trip (uninstall leaves the user's own content intact).

Coverage deltas from smoke (vs the last run):
  internal/hostnat          14.1% → 64.1%  (+50pp — NAT rule dance)
  internal/daemon/opstate   56.2% → 87.5%  (+31pp)
  internal/daemon           43.4% → 49.4%  (+6pp)
  internal/cli              36.1% → 40.4%  (+4pp)
  internal/daemon/workspace 64.1% → 67.5%  (+3pp)

Scenario count: 12 → 21.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Thales Maciel 2026-04-23 12:50:39 -03:00
parent b4afe13b2a
commit bafe816fc7
No known key found for this signature in database
GPG key ID: 33112E6833C34679

View file

@ -289,6 +289,296 @@ set -e
# shellcheck disable=SC2064 # shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT trap "rm -rf '$runtime_dir'" EXIT
# --- vm restart (dedicated verb) --------------------------------------
# `vm restart` is its own verb, not a stop+start composite at the API
# level — it must end up with a freshly booted guest. The assertion is
# a fresh boot ID: /proc/sys/kernel/random/boot_id changes on every
# kernel boot, so post-restart != pre-restart proves the kernel was
# actually recycled rather than the verb no-op'ing.
log 'vm restart: boot_id must change across the verb'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-restart >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-restart >/dev/null || die 'vm restart: create failed'
wait_for_ssh smoke-restart || die 'vm restart: initial ssh never came up'
boot_before="$("$BANGER" vm ssh smoke-restart -- cat /proc/sys/kernel/random/boot_id | tr -d '[:space:]')"
[[ -n "$boot_before" ]] || die 'vm restart: could not read initial boot_id'
"$BANGER" vm restart smoke-restart >/dev/null || die 'vm restart: verb failed'
wait_for_ssh smoke-restart || die 'vm restart: ssh did not come up after restart'
boot_after="$("$BANGER" vm ssh smoke-restart -- cat /proc/sys/kernel/random/boot_id | tr -d '[:space:]')"
[[ -n "$boot_after" ]] || die 'vm restart: could not read post-restart boot_id'
[[ "$boot_before" != "$boot_after" ]] \
|| die "vm restart: boot_id unchanged ($boot_before); verb didn't actually reboot the guest"
"$BANGER" vm delete smoke-restart >/dev/null || die 'vm restart: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- vm kill (--signal KILL, forceful path) ---------------------------
# `vm stop` takes the graceful Ctrl-Alt-Del route. `vm kill --signal
# KILL` is the explicit "the guest is wedged, drop it" path. It must
# (a) terminate firecracker, (b) leave the VM record in a stopped
# state (not 'error'), (c) tear down the dm-snapshot + loops so the
# next create/start doesn't trip over leftovers.
log 'vm kill --signal KILL: forceful terminate, state=stopped, no leaked dm device'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-kill >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-kill >/dev/null || die 'vm kill: create failed'
dm_name="$("$BANGER" vm show smoke-kill 2>/dev/null | awk -F'"' '/"dm_dev"|fc-rootfs-/ {for(i=1;i<=NF;i++) if($i~/^fc-rootfs-/) print $i}' | head -1 || true)"
"$BANGER" vm kill --signal KILL smoke-kill >/dev/null || die 'vm kill: verb failed'
show_out="$("$BANGER" vm show smoke-kill)" || die 'vm kill: show after kill failed'
grep -q '"state": "stopped"' <<<"$show_out" || die "vm kill: post-kill state not stopped: $show_out"
if [[ -n "$dm_name" ]]; then
if sudo -n dmsetup ls 2>/dev/null | awk '{print $1}' | grep -qx "$dm_name"; then
die "vm kill: dm device $dm_name still mapped (cleanup didn't run)"
fi
fi
"$BANGER" vm delete smoke-kill >/dev/null || die 'vm kill: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- vm prune (-f) ----------------------------------------------------
# Create two VMs: one running, one stopped. `vm prune -f` must delete
# the stopped one and leave the running one alone. Skip interactive
# confirmation with -f (smoke has no tty). Regression guard: a bug
# that deleted the running VM would wreck any session the user had.
log 'vm prune -f: removes stopped VMs, preserves running ones'
cleanup_prune() {
"$BANGER" vm delete smoke-prune-running >/dev/null 2>&1 || true
"$BANGER" vm delete smoke-prune-stopped >/dev/null 2>&1 || true
}
# shellcheck disable=SC2064
trap "cleanup_prune; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-prune-running >/dev/null || die 'vm prune: create running failed'
"$BANGER" vm create --name smoke-prune-stopped >/dev/null || die 'vm prune: create stopped failed'
"$BANGER" vm stop smoke-prune-stopped >/dev/null || die 'vm prune: stop the stopped one failed'
"$BANGER" vm prune -f >/dev/null || die 'vm prune: verb failed'
"$BANGER" vm show smoke-prune-running >/dev/null 2>&1 || die 'vm prune: running VM was deleted (regression!)'
if "$BANGER" vm show smoke-prune-stopped >/dev/null 2>&1; then
die 'vm prune: stopped VM survived prune'
fi
"$BANGER" vm delete smoke-prune-running >/dev/null || die 'vm prune: cleanup delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- vm ports ---------------------------------------------------------
# sshd binds :22 in every guest — it's the minimum promise of a VM.
# If `vm ports` can't see that, the host→guest port visibility pipe
# (vsock-agent on-demand query, daemon aggregation, CLI rendering) is
# broken. Endpoint shape is also asserted: daemon prefers the
# <name>.vm DNS record over the raw guest IP, so we grep for the
# name form.
log 'vm ports: sshd :22 visible from host, endpoint uses the VM DNS name'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-ports >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-ports >/dev/null || die 'vm ports: create failed'
wait_for_ssh smoke-ports || die 'vm ports: ssh did not come up'
ports_out="$("$BANGER" vm ports smoke-ports 2>&1)" \
|| die "vm ports: verb failed: $ports_out"
grep -q 'smoke-ports.vm:22' <<<"$ports_out" \
|| die "vm ports: expected 'smoke-ports.vm:22' in output; got: $ports_out"
grep -q 'sshd' <<<"$ports_out" \
|| die "vm ports: expected process 'sshd' in output; got: $ports_out"
"$BANGER" vm delete smoke-ports >/dev/null || die 'vm ports: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- workspace prepare --readonly -------------------------------------
# --readonly runs `chmod -R a-w` over the workspace. Root in the
# guest bypasses DAC anyway, so this is advisory rather than
# enforced — the point of the flag is tooling contract: "the
# mode bits SAY readonly". Assert that contract: the write bit
# must be cleared on the guest file after --readonly prepare, and
# set without it. A regression where the chmod silently no-op'd
# would leave the bits unchanged.
log 'workspace prepare --readonly: mode bits reflect the request'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-ro >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-ro >/dev/null || die 'workspace ro: create failed'
"$BANGER" vm workspace prepare smoke-ro "$repodir" --readonly >/dev/null \
|| die 'workspace ro: prepare --readonly failed'
# stat octal mode. a-w clears the 0222 write bits across u/g/o, so
# none of the write bits should be set on the file.
ro_mode="$("$BANGER" vm ssh smoke-ro -- stat -c '%a' /root/repo/smoke-file.txt | tr -d '[:space:]')"
[[ -n "$ro_mode" ]] || die 'workspace ro: could not read mode bits'
case "$ro_mode" in
*[2367])
die "workspace ro: file still has write bit set after --readonly (mode=$ro_mode)"
;;
esac
# Read must still succeed (--readonly means a-w, not a-r).
"$BANGER" vm ssh smoke-ro -- cat /root/repo/smoke-file.txt >/dev/null \
|| die 'workspace ro: read denied — --readonly dropped read perm too'
"$BANGER" vm delete smoke-ro >/dev/null || die 'workspace ro: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- workspace prepare --mode full_copy -------------------------------
# Default mode is shallow_overlay. full_copy copies the repo via a
# different transfer path (tar stream into the guest's rootfs with
# no overlay). Smoke asserts it still lands the content at the same
# guest path.
log 'workspace prepare --mode full_copy: alternate transfer path still delivers'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-fc >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-fc >/dev/null || die 'workspace fc: create failed'
"$BANGER" vm workspace prepare smoke-fc "$repodir" --mode full_copy >/dev/null \
|| die 'workspace fc: prepare --mode full_copy failed'
fc_out="$("$BANGER" vm ssh smoke-fc -- cat /root/repo/smoke-file.txt)" \
|| die 'workspace fc: guest read failed'
grep -q 'smoke-workspace-marker' <<<"$fc_out" \
|| die "workspace fc: marker missing in full_copy workspace: $fc_out"
"$BANGER" vm delete smoke-fc >/dev/null || die 'workspace fc: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- workspace export --base-commit (committed guest delta) -----------
# Without --base-commit, export diffs the worktree against HEAD — it
# misses commits the worker made inside the guest (because the guest
# HEAD advanced). With --base-commit pinned at the prepare-time SHA,
# those commits land in the patch. This is the happy path the feature
# was added for; zero coverage until now.
log 'workspace export --base-commit: guest-side commits captured in patch'
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-basecommit >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-basecommit >/dev/null || die 'export base: create failed'
"$BANGER" vm workspace prepare smoke-basecommit "$repodir" >/dev/null \
|| die 'export base: prepare failed'
# Capture the prepare-time HEAD from the guest directly (same SHA the
# daemon returns as HeadCommit in the RPC result).
base_sha="$("$BANGER" vm ssh smoke-basecommit -- sh -c 'cd /root/repo && git rev-parse HEAD' | tr -d '[:space:]')"
[[ "${#base_sha}" -eq 40 ]] || die "export base: bad base sha: $base_sha"
# Make a guest-side commit: new file + git add + git commit. Without
# --base-commit, this commit would be invisible to a HEAD-relative diff.
"$BANGER" vm ssh smoke-basecommit -- sh -c "cd /root/repo && git -c user.email=smoke@smoke -c user.name=smoke checkout -b smoke-branch >/dev/null 2>&1 && echo committed-marker > smoke-committed.txt && git add smoke-committed.txt && git -c user.email=smoke@smoke -c user.name=smoke commit -q -m 'guest side'" \
|| die 'export base: guest-side commit failed'
# Control: plain export (no --base-commit) must NOT see the committed file.
plain_patch="$runtime_dir/smoke-plain.diff"
"$BANGER" vm workspace export smoke-basecommit --output "$plain_patch" \
|| die 'export base: plain export failed'
if grep -q 'smoke-committed.txt' "$plain_patch"; then
die 'export base: plain export unexpectedly captured the guest-side commit'
fi
# With --base-commit pinned to the pre-commit SHA, the delta appears.
base_patch="$runtime_dir/smoke-base.diff"
"$BANGER" vm workspace export smoke-basecommit --base-commit "$base_sha" --output "$base_patch" \
|| die 'export base: --base-commit export failed'
[[ -s "$base_patch" ]] || die 'export base: patch file empty'
grep -q 'smoke-committed.txt' "$base_patch" \
|| die "export base: --base-commit patch missing committed marker (head: $(head -c 400 "$base_patch"))"
"$BANGER" vm delete smoke-basecommit >/dev/null || die 'export base: delete failed'
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- ssh-config install / uninstall (HOME-isolated) -------------------
# `banger ssh-config --install` edits ~/.ssh/config. Smoke runs under
# the invoking user, so we isolate by pointing HOME at the smoke XDG
# dir before the commands run (os.UserHomeDir respects $HOME on
# Linux). No daemon / VM involved — pure CLI + filesystem surface,
# exercising the install/status/uninstall code paths end-to-end.
log 'ssh-config --install / --uninstall: idempotent, survives round-trip'
fake_home="$BANGER_SMOKE_XDG_DIR/fake-home"
mkdir -p "$fake_home/.ssh"
# Seed a pre-existing ~/.ssh/config so install must APPEND, not
# replace. A bug that clobbered pre-existing content would nuke the
# user's real config on first run.
printf 'Host myserver\n HostName example.invalid\n' > "$fake_home/.ssh/config"
(
export HOME="$fake_home"
"$BANGER" ssh-config --install >/dev/null || die 'ssh-config: install failed'
grep -q '^Include ' "$fake_home/.ssh/config" \
|| die "ssh-config: install didn't add Include line to ~/.ssh/config"
grep -q '^Host myserver' "$fake_home/.ssh/config" \
|| die 'ssh-config: install clobbered pre-existing content (!!)'
# Second install must be idempotent (no duplicate Include lines).
"$BANGER" ssh-config --install >/dev/null || die 'ssh-config: second install failed'
include_count="$(grep -c '^Include .*banger' "$fake_home/.ssh/config")"
[[ "$include_count" == "1" ]] \
|| die "ssh-config: install not idempotent (Include appeared $include_count times)"
"$BANGER" ssh-config --uninstall >/dev/null || die 'ssh-config: uninstall failed'
if grep -q '^Include .*banger' "$fake_home/.ssh/config"; then
die 'ssh-config: uninstall left the Include line behind'
fi
grep -q '^Host myserver' "$fake_home/.ssh/config" \
|| die 'ssh-config: uninstall nuked user content (!!)'
)
# --- NAT rule installation (per-VM MASQUERADE) ------------------------
# `--nat` installs a per-VM iptables POSTROUTING MASQUERADE rule
# scoped to the guest's /32 (see natCapability). End-to-end curl
# tests don't work here because the bridge IP and the host's uplink
# IP both belong to the host — a guest reaching the uplink address
# lands on the host's local loopback whether MASQUERADE is set up
# or not. So assert the rule itself: NAT VM gets a POSTROUTING
# MASQUERADE, non-NAT VM does not. This catches the two most
# plausible regressions (rule never installed; rule not scoped to
# the right VM) without depending on an external reachable host.
log 'NAT: --nat installs a per-VM MASQUERADE rule; no --nat means no rule'
if ! sudo -n iptables -t nat -S POSTROUTING >/dev/null 2>&1; then
log 'NAT: skipping — passwordless sudo iptables unavailable'
else
# shellcheck disable=SC2064
trap "\"$BANGER\" vm delete smoke-nat >/dev/null 2>&1 || true; \"$BANGER\" vm delete smoke-nocnat >/dev/null 2>&1 || true; rm -rf '$runtime_dir'" EXIT
"$BANGER" vm create --name smoke-nat --nat >/dev/null || die 'NAT: create --nat failed'
"$BANGER" vm create --name smoke-nocnat >/dev/null || die 'NAT: control create failed'
nat_ip="$("$BANGER" vm show smoke-nat 2>/dev/null | awk -F'"' '/"guest_ip"/ {print $4}')"
ctl_ip="$("$BANGER" vm show smoke-nocnat 2>/dev/null | awk -F'"' '/"guest_ip"/ {print $4}')"
[[ -n "$nat_ip" && -n "$ctl_ip" ]] || die "NAT: couldn't read guest IPs (nat='$nat_ip', ctl='$ctl_ip')"
postrouting="$(sudo -n iptables -t nat -S POSTROUTING 2>/dev/null || true)"
grep -q -- "-s $nat_ip/32.*-j MASQUERADE" <<<"$postrouting" \
|| die "NAT: --nat VM has no POSTROUTING MASQUERADE rule for $nat_ip; got:"$'\n'"$postrouting"
if grep -q -- "-s $ctl_ip/32.*-j MASQUERADE" <<<"$postrouting"; then
die "NAT: control VM unexpectedly has a MASQUERADE rule for $ctl_ip"
fi
# Stop + start the --nat VM to exercise the install-is-idempotent
# path (capability runs again on each start; a buggy add-without-
# check would leave two identical rules behind).
"$BANGER" vm stop smoke-nat >/dev/null || die 'NAT: stop --nat VM failed'
"$BANGER" vm start smoke-nat >/dev/null || die 'NAT: restart --nat VM failed'
postrouting="$(sudo -n iptables -t nat -S POSTROUTING 2>/dev/null || true)"
rule_count="$(grep -c -- "-s $nat_ip/32.*-j MASQUERADE" <<<"$postrouting" || true)"
[[ "$rule_count" == "1" ]] \
|| die "NAT: MASQUERADE rule count for $nat_ip = $rule_count after restart, want 1"
# Delete must tear the rule down — regression guard against leaks.
"$BANGER" vm delete smoke-nat >/dev/null || die 'NAT: delete --nat VM failed'
"$BANGER" vm delete smoke-nocnat >/dev/null || die 'NAT: delete control VM failed'
postrouting="$(sudo -n iptables -t nat -S POSTROUTING 2>/dev/null || true)"
if grep -q -- "-s $nat_ip/32.*-j MASQUERADE" <<<"$postrouting"; then
die "NAT: delete left a MASQUERADE rule behind for $nat_ip"
fi
fi
# shellcheck disable=SC2064
trap "rm -rf '$runtime_dir'" EXIT
# --- invalid spec rejection + no artifact leak ------------------------ # --- invalid spec rejection + no artifact leak ------------------------
# Tests the negative-path create flow: a blatantly invalid VM spec # Tests the negative-path create flow: a blatantly invalid VM spec
# must fail before any VM row is persisted. The review cycle flagged # must fail before any VM row is persisted. The review cycle flagged