From 0b9403bbb2f3e3b50a29f02a43372f33da170bf1 Mon Sep 17 00:00:00 2001 From: Gizzmo Date: Wed, 23 Sep 2026 11:33:40 +0100 Subject: [PATCH] fix(lxc): handle partial mount options safely --- internal/schema/lxc.go | 15 ++- internal/schema/lxc_wire_regressions_test.go | 124 +++++++++++++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/internal/schema/lxc.go b/internal/schema/lxc.go index 29440c8..8bf7c45 100644 --- a/internal/schema/lxc.go +++ b/internal/schema/lxc.go @@ -993,22 +993,35 @@ func lxcMountLiveRewrite(curWire string, m LXCMount) (string, bool) { want *bool tok string def bool - }{{o.ReadOnly, "ro=", false}, {o.Backup, "backup=", true}, {o.ACL, "acl=", false}, {o.Quota, "quota=", false}, {o.Shared, "shared=", false}} { + }{ + {o.ReadOnly, "ro=", false}, + {o.Backup, "backup=", false}, + {o.ACL, "acl=", false}, + {o.Quota, "quota=", false}, + {o.Shared, "shared=", false}, + } { + if c.want == nil { + continue + } + lv, present := lxcRawBoolToken(curWire, c.tok) eff := c.def if present { eff = lv } + if eff != *c.want { need = true } } + if o.MountOptions != "" { if lv := lxcRawToken(curWire, "mountoptions="); lv != "" && lv != o.MountOptions { need = true } } } + if !need { return "", false } diff --git a/internal/schema/lxc_wire_regressions_test.go b/internal/schema/lxc_wire_regressions_test.go index b5dc272..aff4c46 100644 --- a/internal/schema/lxc_wire_regressions_test.go +++ b/internal/schema/lxc_wire_regressions_test.go @@ -37,6 +37,7 @@ package schema_test // shape Go would emit. import ( + "strings" "testing" "github.com/Kelcode-Dev/proxops/internal/schema" @@ -264,6 +265,129 @@ func TestLXCDrift_ConsoleIsConvergeable(t *testing.T) { } } +// TestLXCDrift_MountPartialOptionsDoNotPanic pins tri-state mount-option +// semantics: nil option fields are unowned and must be ignored rather than +// dereferenced during drift comparison. +func TestLXCDrift_MountPartialOptionsDoNotPanic(t *testing.T) { + lxc := schema.NewLXC() + if err := schema.YAMLTo("apiVersion: "+schema.APIVersion+"\n"+ + "kind: LXC\n"+ + "metadata:\n"+ + " name: x\n"+ + "spec:\n"+ + " node: pve01\n"+ + " vmid: 100\n"+ + " memory: 1GiB\n"+ + " cpu: {cores: 1}\n"+ + " template: t\n"+ + " root: {storage: local-lvm, size: 4GiB}\n"+ + " networks:\n"+ + " - bridge: vmbr0\n"+ + " mount-points:\n"+ + " - storage: local-lvm\n"+ + " size: 1GiB\n"+ + " mount-point: /mnt/data\n"+ + " slot: mp0\n"+ + " options:\n"+ + " read-only: true\n", lxc); err != nil { + t.Fatalf("YAMLTo: %v", err) + } + + live := map[string]any{ + "cores": "1", + "memory": "1024", + "hostname": "x", + "rootfs": "local-lvm:vm-100-disk-0,size=4G", + "net0": "name=net0,bridge=vmbr0", + "mp0": "local-lvm:vm-100-disk-1,mp=/mnt/data,ro=1,size=1G", + "tags": "proxops", + } + + upd, _, changed := lxc.Drift(live) + if changed { + t.Fatalf("matched partial mount options reported drift: upd=%v", upd) + } + if _, ok := upd["mp0"]; ok { + t.Fatalf("matched partial mount options emitted mp0 rewrite: %v", upd["mp0"]) + } +} + +// TestLXCDrift_MountBackupAbsentMeansFalse pins PVE's mpN backup default: +// an absent backup= token means the mount is excluded from backup. Desired +// backup=false therefore converges without a rewrite, while backup=true +// must emit backup=1. +func TestLXCDrift_MountBackupAbsentMeansFalse(t *testing.T) { + newLXC := func(t *testing.T, backup string) *schema.LXC { + t.Helper() + + lxc := schema.NewLXC() + if err := schema.YAMLTo("apiVersion: "+schema.APIVersion+"\n"+ + "kind: LXC\n"+ + "metadata:\n"+ + " name: x\n"+ + "spec:\n"+ + " node: pve01\n"+ + " vmid: 100\n"+ + " memory: 1GiB\n"+ + " cpu: {cores: 1}\n"+ + " template: t\n"+ + " root: {storage: local-lvm, size: 4GiB}\n"+ + " networks:\n"+ + " - bridge: vmbr0\n"+ + " mount-points:\n"+ + " - storage: local-lvm\n"+ + " size: 1GiB\n"+ + " mount-point: /mnt/data\n"+ + " slot: mp0\n"+ + " options:\n"+ + " backup: "+backup+"\n", lxc); err != nil { + t.Fatalf("YAMLTo: %v", err) + } + return lxc + } + + live := func() map[string]any { + return map[string]any{ + "cores": "1", + "memory": "1024", + "hostname": "x", + "rootfs": "local-lvm:vm-100-disk-0,size=4G", + "net0": "name=net0,bridge=vmbr0", + "mp0": "local-lvm:vm-100-disk-1,mp=/mnt/data,size=1G", + "tags": "proxops", + } + } + + t.Run("false matches absent live token", func(t *testing.T) { + lxc := newLXC(t, "false") + + upd, _, changed := lxc.Drift(live()) + if changed { + t.Fatalf("backup=false + live backup absent reported drift: upd=%v", upd) + } + if _, ok := upd["mp0"]; ok { + t.Fatalf("backup=false + live backup absent emitted mp0 rewrite: %v", upd["mp0"]) + } + }) + + t.Run("true rewrites absent live token", func(t *testing.T) { + lxc := newLXC(t, "true") + + upd, _, changed := lxc.Drift(live()) + if !changed { + t.Fatal("backup=true + live backup absent reported no drift") + } + + got, ok := upd["mp0"].(string) + if !ok { + t.Fatalf("backup=true + live backup absent: mp0 update missing or not string: %v", upd) + } + if !strings.Contains(got, "backup=1") { + t.Fatalf("backup=true + live backup absent: mp0=%q, want backup=1", got) + } + }) +} + func contains(s, sub string) bool { for i := 0; i+len(sub) <= len(s); i++ { if s[i:i+len(sub)] == sub {