Files
felhom-controller/controller/internal/web/attach_sources_test.go
T
admin b762a37097
gates / gates (push) Successful in 17s
R-280: attach list from mounted-but-unregistered filesystems; two-clicks promise made conditional
After a reinstall the data drive could not be re-attached through any dashboard
route: both candidate lists came from the agent's unclaimed-disk scan, and the
rebuilt box's drives are claimed. The restore page said it was two clicks while
pointing at an empty picker.

The attach list now also carries the controller's own mounted-but-unregistered
filesystems. initialize is untouched, so the format wizard's system/backup
protection is unchanged. The 'two clicks' sentence is conditional on the picker
being non-empty, and says something true and actionable when it is not.
2026-08-10 13:41:32 +02:00

253 lines
11 KiB
Go

package web
import (
"strings"
"testing"
"gitea.dooplex.hu/admin/felhom-controller/internal/agentapi"
)
// R-280 — THE DRIVE CAN BE RE-ATTACHED AFTER A REINSTALL.
//
// Measured on the rebuilt demo-hp (2026-08-09): the restore page diagnosed the situation correctly
// and then sent the customer to a picker that was empty. `GET /disks/candidates` answered
// `{"initialize":[],"attach":[]}` because BOTH lists come from the agent's unclaimed-DISK scan, and
// the box's NVMe is claimed (it is the felhom-backup target). Getting past it needed an internal path
// no customer could produce.
//
// These tests pin the SOURCE of the attach list, and the mount table below is the real one from that
// box — /mnt/sys_drive is the guest data volume the escape hatch had to register by hand.
// demoHPMounts is guest 9201's actual mount table on the rebuilt demo-hp, trimmed to the rows that
// matter. Keeping the real shape means the fixture cannot quietly diverge from the box.
const demoHPMounts = `proc /proc proc rw,relatime 0 0
/dev/mapper/pve-vm--9201--disk--0 /mnt ext4 rw,relatime,stripe=16 0 0
/dev/mapper/pve-root /mnt/felhom-drives ext4 rw,relatime,errors=remount-ro 0 0
/dev/mapper/pve-vm--9201--disk--1 /mnt/sys_drive ext4 rw,relatime,stripe=16 0 0
tmpfs /dev/shm tmpfs rw,nosuid,nodev 0 0
overlay /var/lib/docker/overlay2/x/merged overlay rw,relatime 0 0
`
// ── SCENARIO A — a mounted, unregistered filesystem on a CLAIMED disk is offered ────────────────
//
// RED-PROOF (the one that matters): revert the attach list to the unclaimed-disk scan — i.e. make
// mountedUnregisteredStores return nil, or drop the `resp.Attach = append(...)` line in
// agentDiskCandidatesHandler. This fails with `attach candidates: 0`, which IS yesterday's wall:
// an empty picker under a sentence promising two clicks.
func TestMountedUnregisteredStores_OffersTheGuestDataVolume(t *testing.T) {
got := mountedUnregisteredStores(demoHPMounts, map[string]bool{})
if len(got) != 1 {
t.Fatalf("attach candidates: %d, want 1 (/mnt/sys_drive) — got %+v", len(got), got)
}
if got[0].Path != "/mnt/sys_drive" {
t.Errorf("offered %q, want /mnt/sys_drive — the drive the rebuilt box could not re-attach", got[0].Path)
}
if got[0].FSType != "ext4" {
t.Errorf("fstype %q, want ext4", got[0].FSType)
}
if got[0].Device != "/dev/mapper/pve-vm--9201--disk--1" {
t.Errorf("device %q — the backing device is shown to the customer and must be the real one", got[0].Device)
}
}
// The disk is CLAIMED — that is the whole point. The claim filter is a property of the agent's scan,
// and this source deliberately does not consult it, because attaching erases nothing. This pins that
// the mount table alone decides, so re-introducing a claim check here would fail.
func TestMountedUnregisteredStores_ClaimedDiskIsStillOffered(t *testing.T) {
// pve-vm--9201--disk--1 is LVM on the OS disk: claimed by every definition the agent uses.
got := mountedUnregisteredStores(demoHPMounts, map[string]bool{})
if len(got) == 0 {
t.Fatal("a claimed-but-mounted filesystem was filtered out — attaching is non-destructive, " +
"and this filter is exactly what made the picker empty on the rebuilt box")
}
}
// ── SCENARIO D — a normal box with a registered store is unchanged ──────────────────────────────
// RED-PROOF: drop the `registered[where]` exclusion and this fails with the already-registered store
// offered for attaching a second time.
func TestMountedUnregisteredStores_RegisteredStoreIsNotOffered(t *testing.T) {
got := mountedUnregisteredStores(demoHPMounts, map[string]bool{"/mnt/sys_drive": true})
if len(got) != 0 {
t.Errorf("a healthy box offered %+v — its store is registered, so the picker must be empty "+
"and the page byte-identical to before this change", got)
}
}
// ── The exclusions, each with the reason it exists ──────────────────────────────────────────────
func TestMountedUnregisteredStores_Exclusions(t *testing.T) {
cases := []struct {
name, table, why string
}{
{
"guest rootfs at /mnt",
"/dev/mapper/pve-vm--9201--disk--0 /mnt ext4 rw 0 0\n",
"/mnt is the guest rootfs mount, not a drive — offering it would register the box's own root",
},
{
"the managed parent itself",
"/dev/mapper/pve-root /mnt/felhom-drives ext4 rw 0 0\n",
"the intermediary-model parent holds drives; it is not one",
},
{
"tmpfs",
"tmpfs /mnt/scratch tmpfs rw 0 0\n",
"a RAM filesystem would silently lose the customer's data on reboot",
},
{
"overlay",
"overlay /mnt/ovl overlay rw 0 0\n",
"not a real block device",
},
{
"outside /mnt",
"/dev/sdb1 /srv/data ext4 rw 0 0\n",
"the storage convention is /mnt/<name>; registering outside it is the manual-add path",
},
{
"unsupported fs",
"/dev/sdb1 /mnt/win ntfs rw 0 0\n",
"ntfs is data-bearing but not one the stack hands to apps",
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
if got := mountedUnregisteredStores(c.table, map[string]bool{}); len(got) != 0 {
t.Errorf("offered %+v — %s", got, c.why)
}
})
}
}
// FAIL-SAFE: an unreadable mount table yields nothing, never everything. Paired with the caller's
// non-empty assertion, "we could not look" renders as "we cannot offer this", never as a promise.
func TestMountedUnregisteredStores_UnreadableTableOffersNothing(t *testing.T) {
if got := mountedUnregisteredStores("", map[string]bool{}); len(got) != 0 {
t.Errorf("an unreadable mount table produced %+v — it must produce nothing", got)
}
}
// A mountpoint listed twice (bind / re-mount) must appear once, or the picker shows a duplicate.
func TestMountedUnregisteredStores_DeduplicatesMountpoints(t *testing.T) {
table := "/dev/sdb1 /mnt/data ext4 rw 0 0\n/dev/sdb1 /mnt/data ext4 rw,remount 0 0\n"
if got := mountedUnregisteredStores(table, map[string]bool{}); len(got) != 1 {
t.Errorf("got %d entries, want 1 — a re-mount must not double the picker row", len(got))
}
}
// ── SCENARIO C — nothing attachable, said plainly ───────────────────────────────────────────────
// RED-PROOF: remove the `{{if .HasAttachDestination}}` conditional from backups_restore.html and this
// fails on the first assertion — the „két kattintás" promise returns over an empty picker, which is
// the exact sentence that cost the rehearsal its time.
func TestRestorePage_NothingAttachable_DoesNotPromiseTwoClicks(t *testing.T) {
d := restoreData()
d["NoRestoreDestination"] = true
d["HasAttachDestination"] = false
html := renderBackupPage(t, "backups_restore", d)
if strings.Contains(html, "két kattintás") {
t.Error("R-280: the page still promises „két kattintás" +
"\" while there is nothing to click — it is zero clicks, and the customer cannot get past it")
}
if !strings.Contains(html, "Csatolható meghajtót viszont most nem látunk") {
t.Error("R-280: the page does not say plainly that there is nothing to attach")
}
if !strings.Contains(html, "üzemeltető") {
t.Error("R-280: a refusal with no route is the R-252 defect again — it must name what to do instead")
}
}
// ── SCENARIO A, rendered — the promise is kept only when it is true ─────────────────────────────
func TestRestorePage_Attachable_KeepsTheTwoClicksRoute(t *testing.T) {
d := restoreData()
d["NoRestoreDestination"] = true
d["HasAttachDestination"] = true
html := renderBackupPage(t, "backups_restore", d)
if !strings.Contains(html, "két kattintás") {
t.Error("with a real destination the original instruction must survive — this change narrows " +
"a false promise, it does not remove a true one")
}
if !strings.Contains(html, `href="/storage"`) {
t.Error("the instruction no longer routes to the picker")
}
}
// ── SCENARIO B — `initialize` is left exactly as it was ─────────────────────────────────────────
//
// The format wizard hides system and backup drives, and it says so on the page. That protection is a
// property of the agent's unclaimed scan, and the mounted-store source must never reach it.
//
// RED-PROOF: switch the initialize list to the new source too — in mergeAttachCandidates, add
// `resp.Initialize = append(resp.Initialize, mountedStoreCandidates(stores)...)`. This fails with the
// guest data volume offered for FORMATTING, which is the protection breaking in the open.
func TestMergeAttachCandidates_InitializeIsUntouched(t *testing.T) {
agentSaid := agentapi.CandidatesResult{
VMID: 9201,
Initialize: []agentapi.DiskCandidate{{Device: "/dev/sdd", FSType: ""}},
Attach: []agentapi.DiskCandidate{{Device: "/dev/sdd", MountSource: "/dev/sdd1", FSType: "ext4"}},
}
stores := []mountedStore{{Path: "/mnt/sys_drive", Device: "/dev/mapper/pve-vm--9201--disk--1", FSType: "ext4"}}
got := mergeAttachCandidates(agentSaid, stores)
// initialize: byte-for-byte the agent's list.
if len(got.Initialize) != 1 || got.Initialize[0].Device != "/dev/sdd" {
t.Fatalf("initialize was modified: %+v — the format wizard's system/backup protection lives "+
"in the agent's unclaimed scan, and widening it is how a customer is offered their own "+
"data drive to format", got.Initialize)
}
for _, c := range got.Initialize {
if c.AlreadyMounted {
t.Errorf("a mounted store reached the FORMAT list: %+v", c)
}
}
// attach: the agent's entry survives (the fresh-USB case) AND the mounted store is added.
if len(got.Attach) != 2 {
t.Fatalf("attach has %d entries, want 2 (agent's + the mounted store) — got %+v", len(got.Attach), got.Attach)
}
var foundMounted bool
for _, c := range got.Attach {
if c.MountSource == "/mnt/sys_drive" {
foundMounted = true
if !c.AlreadyMounted {
t.Error("the mounted store is not flagged already_mounted — the wizard would send it " +
"down the device-attach path and try to mount an in-guest path as a raw device")
}
}
}
if !foundMounted {
t.Error("the mounted store did not reach attach — this is the rebuilt box's empty picker")
}
if got.Attach[0].Device != "/dev/sdd" {
t.Error("the agent's own attach entry was dropped — a fresh external drive with a filesystem " +
"on it is exactly what this wizard was built for, and the mount table cannot report it")
}
}
// A bind mount republishes a filesystem under a second path. A bind of the guest ROOTFS under
// /mnt/<name> is indistinguishable from a data drive by path alone — and registering it would put
// app data on the root filesystem.
//
// RED-PROOF: drop the `rootDevices[dev]` exclusion and this fails with /mnt/rootcopy offered.
func TestMountedUnregisteredStores_RootfsAliasIsNotOffered(t *testing.T) {
table := demoHPMounts + "/dev/mapper/pve-vm--9201--disk--0 /mnt/rootcopy ext4 rw 0 0\n"
got := mountedUnregisteredStores(table, map[string]bool{})
for _, m := range got {
if m.Path == "/mnt/rootcopy" {
t.Error("a bind of the guest rootfs was offered as an attachable store — registering it " +
"would store the customer's app data on the box's own root filesystem")
}
}
// The real drive on a DIFFERENT device must survive the new exclusion.
if len(got) != 1 || got[0].Path != "/mnt/sys_drive" {
t.Errorf("the genuine data volume was lost to the rootfs guard: %+v", got)
}
}