v0.32.0: self-serve decommission endpoint + intent-aware re-assert (B2a)

POST /disks/decommission mirrors eject (withGuest, user-data role gate) — no operator signature, non-destructive (never formats): sets IntentDecommissioned, prunes the GuestBindStore entry, unmounts. ReassertGuestBinds is now intent-aware (skip non-enrolled) so a decommissioned-but-present drive never auto-rebinds on agent restart — the load-bearing F9-reconnect fix. GuestBindStore.Remove added. Operator-signed DecommissionExecutor + classify untouched. Non-hollow tests incl. the intent-aware reassert companion (mutation-proven to fail on intent-blind code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-14 19:55:17 +02:00
parent 8e6d00a57f
commit f43697c881
6 changed files with 397 additions and 49 deletions
@@ -0,0 +1,222 @@
+package localapi
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"net/http"
+	"path/filepath"
+	"sync"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
+	"gitea.dooplex.hu/admin/felhom-agent/internal/storage"
+)
+
+// fakeIntent implements the extended IntentRecorder (SetEnrolled/SetEjected/SetDecommissioned + Get).
+type fakeIntent struct {
+	mu sync.Mutex
+	m  map[string]storage.DriveIntent
+}
+
+func newFakeIntent() *fakeIntent { return &fakeIntent{m: map[string]storage.DriveIntent{}} }
+func (f *fakeIntent) set(id string, v storage.DriveIntent) {
+	f.mu.Lock()
+	f.m[id] = v
+	f.mu.Unlock()
+}
+func (f *fakeIntent) SetEnrolled(id string) error { f.set(id, storage.IntentEnrolled); return nil }
+func (f *fakeIntent) SetEjected(id string) error  { f.set(id, storage.IntentEjected); return nil }
+func (f *fakeIntent) SetDecommissioned(id string) error {
+	f.set(id, storage.IntentDecommissioned)
+	return nil
+}
+func (f *fakeIntent) Get(id string) storage.DriveIntent {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.m[id]
+}
+
+// decommServer wires a Server with Disks + Intent + GuestBinds + GuestAttach so both the endpoint and
+// the intent-aware re-assert can be exercised. Returns the *Server (use .Handler() for HTTP tests).
+func decommServer(t *testing.T, d *fakeDiskOps, sv StorageView, gl GuestLister, intent IntentRecorder, gb *GuestBindStore, ga GuestAttacher, mounts map[int]map[string]string) *Server {
+	t.Helper()
+	srv, err := NewServer(Options{
+		ListenAddr:  "127.0.0.1:0",
+		Guests:      &fakeGuestsCfg{mounts: mounts},
+		Backups:     &fakeBackups{},
+		Store:       &fakeStore{},
+		Storage:     sv,
+		Tokens:      staticTokens{"A": 8200, "B": 9300},
+		Disks:       d,
+		DiskGate:    &fakeGate{},
+		Guests2:     gl,
+		Intent:      intent,
+		GuestBinds:  gb,
+		GuestAttach: ga,
+		HostReader:  sysOnSDA(),
+		Logger:      slog.New(slog.NewTextHandler(io.Discard, nil)),
+	})
+	if err != nil {
+		t.Fatalf("new server: %v", err)
+	}
+	srv.baseCtx = context.Background()
+	return srv
+}
+
+// userDataAndProtected is a storage view with a user-data USB, a system dir, and a backup PBS mount.
+func userDataAndProtected() fakeStorage {
+	return fakeStorage{targets: []hub.StorageTarget{
+		{Name: "bulk", Type: hub.StorageTypeUSB, BackingDevice: "/dev/sdb1", MountPath: "/mnt/bulk", DurableID: "uuid:usb-1"},
+		{Name: "local", Type: "local", MountPath: "/var/lib/vz"},
+		{Name: "felhom-pbs", Type: hub.StorageTypePBS, MountPath: "/mnt/pbs"},
+	}}
+}
+
+// TestDecommission_RoleGated: like eject, a system/backup mount is refused 403 with NO unmount; only a
+// user-data mount decommissions. (Mirrors TestEject_RoleGated — the gate is the security control.)
+func TestDecommission_RoleGated(t *testing.T) {
+	for _, where := range []string{"/var/lib/vz", "/mnt/pbs", "/mnt/unknown"} {
+		d := &fakeDiskOps{}
+		srv := decommServer(t, d, userDataAndProtected(), fakeGuestList{}, newFakeIntent(), tempBindStore(t), &fakeGuestAttacher{}, nil)
+		w := do(t, srv.Handler(), "POST", "/disks/decommission", "A", `{"where":"`+where+`"}`)
+		if w.Code != http.StatusForbidden {
+			t.Fatalf("decommission %s: got %d want 403 (%s)", where, w.Code, w.Body.String())
+		}
+		d.mu.Lock()
+		if len(d.unmountCalls) != 0 {
+			t.Fatalf("Unmount called on protected mount %s — role-gate bypassed", where)
+		}
+		d.mu.Unlock()
+	}
+}
+
+// TestDecommission_Effects: a user-data decommission sets IntentDecommissioned, removes the
+// GuestBindStore entry, and unmounts — assert ALL THREE (not just no-error). NEVER formats.
+func TestDecommission_Effects(t *testing.T) {
+	d := &fakeDiskOps{}
+	intent := newFakeIntent()
+	intent.SetEnrolled("uuid:usb-1") // currently enrolled
+	gb := tempBindStore(t)
+	_ = gb.Record(8200, "uuid:usb-1")
+
+	srv := decommServer(t, d, userDataAndProtected(), fakeGuestList{}, intent, gb, &fakeGuestAttacher{}, nil)
+	w := do(t, srv.Handler(), "POST", "/disks/decommission", "A", `{"where":"/mnt/bulk"}`)
+	if w.Code != http.StatusOK {
+		t.Fatalf("decommission user-data: got %d want 200 (%s)", w.Code, w.Body.String())
+	}
+	// 1) intent recorded as decommissioned
+	if got := intent.Get("uuid:usb-1"); got != storage.IntentDecommissioned {
+		t.Errorf("intent = %q, want decommissioned", got)
+	}
+	// 2) guest-bind record pruned
+	if ids := gb.Guests()[8200]; len(ids) != 0 {
+		t.Errorf("guest-bind not removed: %v", ids)
+	}
+	// 3) unmounted, never formatted
+	d.mu.Lock()
+	defer d.mu.Unlock()
+	if len(d.unmountCalls) != 1 || d.unmountCalls[0] != "/mnt/bulk" {
+		t.Errorf("Unmount calls = %v, want [/mnt/bulk]", d.unmountCalls)
+	}
+	if len(d.formatCalls) != 0 {
+		t.Errorf("decommission must NEVER format; formatCalls = %v", d.formatCalls)
+	}
+}
+
+// TestReassertGuestBinds_SkipsDecommissioned is the load-bearing F9-reconnect invariant: a
+// decommissioned-but-present drive still recorded in the bind store must NOT auto-rebind on agent
+// restart. Companion: with intent=enrolled the SAME setup DOES rebind — proving the intent gate is
+// what blocks it (the pre-fix intent-blind code would rebind both → this test FAILS on it).
+func TestReassertGuestBinds_SkipsDecommissioned(t *testing.T) {
+	// decommissioned → must NOT rebind even though present + recorded.
+	gbD := tempBindStore(t)
+	_ = gbD.Record(8200, "uuid:usb-1")
+	intentD := newFakeIntent()
+	intentD.SetDecommissioned("uuid:usb-1")
+	gaD := &fakeGuestAttacher{}
+	srvD := decommServer(t, &fakeDiskOps{}, usbPresent(), fakeGuestList{}, intentD, gbD, gaD, map[int]map[string]string{
+		8200: {"mp0": "local-lvm:8,mp=/var/lib/docker"}, // bind missing → would re-add if not gated
+	})
+	srvD.ReassertGuestBinds(context.Background())
+	if gaD.count() != 0 {
+		t.Fatalf("decommissioned drive was re-bound (%d AttachBind) — intent gate missing", gaD.count())
+	}
+
+	// companion: enrolled → DOES rebind (same present drive + missing bind).
+	gbE := tempBindStore(t)
+	_ = gbE.Record(8200, "uuid:usb-1")
+	intentE := newFakeIntent()
+	intentE.SetEnrolled("uuid:usb-1")
+	gaE := &fakeGuestAttacher{}
+	srvE := decommServer(t, &fakeDiskOps{}, usbPresent(), fakeGuestList{}, intentE, gbE, gaE, map[int]map[string]string{
+		8200: {"mp0": "local-lvm:8,mp=/var/lib/docker"},
+	})
+	srvE.ReassertGuestBinds(context.Background())
+	if gaE.count() != 1 {
+		t.Fatalf("enrolled drive should rebind (got %d AttachBind) — gate too aggressive", gaE.count())
+	}
+}
+
+// TestReCommission_ReRecords: after a decommission prunes the bind, re-enrolling (SetEnrolled +
+// Record) restores it so the re-assert rebinds again. Proves Remove doesn't break re-enroll.
+func TestReCommission_ReRecords(t *testing.T) {
+	gb := tempBindStore(t)
+	intent := newFakeIntent()
+
+	// decommission removed the record + set decommissioned intent.
+	intent.SetDecommissioned("uuid:usb-1")
+	_ = gb.Remove(8200, "uuid:usb-1") // no-op (absent) — idempotent
+
+	// re-enroll (what handleDiskGuestAttach does): set enrolled + record.
+	intent.SetEnrolled("uuid:usb-1")
+	if err := gb.Record(8200, "uuid:usb-1"); err != nil {
+		t.Fatal(err)
+	}
+	ga := &fakeGuestAttacher{}
+	srv := decommServer(t, &fakeDiskOps{}, usbPresent(), fakeGuestList{}, intent, gb, ga, map[int]map[string]string{
+		8200: {"mp0": "local-lvm:8,mp=/var/lib/docker"},
+	})
+	srv.ReassertGuestBinds(context.Background())
+	if ga.count() != 1 {
+		t.Fatalf("re-commissioned drive should rebind (got %d)", ga.count())
+	}
+}
+
+// TestGuestBindStore_Remove covers idempotency (absent = no-op), present removal, empty-key drop, and
+// persistence across reopen.
+func TestGuestBindStore_Remove(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "guest-binds.json")
+	gb, err := OpenGuestBindStore(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// absent vmid + absent id → no-op, no error
+	if err := gb.Remove(8200, "uuid:nope"); err != nil {
+		t.Fatalf("Remove absent: %v", err)
+	}
+	_ = gb.Record(8200, "uuid:a")
+	_ = gb.Record(8200, "uuid:b")
+	// remove a present id → keeps the other
+	if err := gb.Remove(8200, "uuid:a"); err != nil {
+		t.Fatal(err)
+	}
+	if ids := gb.Guests()[8200]; len(ids) != 1 || ids[0] != "uuid:b" {
+		t.Fatalf("after remove uuid:a, vmid 8200 = %v want [uuid:b]", ids)
+	}
+	// remove the last id → vmid key dropped
+	if err := gb.Remove(8200, "uuid:b"); err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := gb.Guests()[8200]; ok {
+		t.Errorf("empty vmid key should be dropped")
+	}
+	// persistence: reopen and confirm empty
+	re, err := OpenGuestBindStore(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(re.Guests()) != 0 {
+		t.Errorf("store should be empty after reopen, got %v", re.Guests())
+	}
+}