diff --git a/src/internal/cfg/cfg.go b/src/internal/cfg/cfg.go index 90d1720..480a53a 100644 --- a/src/internal/cfg/cfg.go +++ b/src/internal/cfg/cfg.go @@ -199,7 +199,9 @@ type SnapshotsCfg struct { // Limit caps how many snapshots a user may keep per container. The default // of 1 gives a light safety net; a snapshot is disk-only (no memory) and // lives under the container in Incus, so it is deleted together with the - // container on reinstall/delete. 0 or negative is treated as 1. + // container on reinstall/delete. 0 disables snapshots: users cannot create + // new ones, but existing snapshots are left untouched. Negative is treated + // as 0 (the config validator rejects negatives anyway). Limit int `yaml:"limit"` } diff --git a/src/internal/cfg/registry.go b/src/internal/cfg/registry.go index 55a3567..00882e2 100644 --- a/src/internal/cfg/registry.go +++ b/src/internal/cfg/registry.go @@ -346,7 +346,7 @@ var Fields = []Field{ return nil }}, {"snapshots.limit", KindOperator, ApplyRestart, - "max snapshots a user may keep per container (0 = default 1)", + "max snapshots a user may keep per container (0 = disable snapshots)", "1", getStr(func(c *Config) string { return strconv.Itoa(c.Snapshots.Limit) }), func(c *Config, v string) error { diff --git a/src/internal/mgr/mgr.go b/src/internal/mgr/mgr.go index 716edbf..e7bc0d1 100644 --- a/src/internal/mgr/mgr.go +++ b/src/internal/mgr/mgr.go @@ -875,10 +875,13 @@ var validSnapName = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,62}$`) // API. Exported for tests and the panel. func ValidSnapName(v string) bool { return validSnapName.MatchString(v) } -// SnapshotLimit returns the configured per-container snapshot cap. +// SnapshotLimit returns the configured per-container snapshot cap. 0 (or a +// negative value, which the config validator forbids anyway) means snapshots +// are disabled: users cannot create new ones, but existing snapshots are left +// untouched. Any positive value is the hard cap. func (m *Manager) SnapshotLimit() int { - if m.cfg.Snapshots.Limit < 1 { - return 1 + if m.cfg.Snapshots.Limit < 0 { + return 0 } return m.cfg.Snapshots.Limit } @@ -903,12 +906,16 @@ func (m *Manager) SnapshotCreate(name string) error { if err != nil { return err } + limit := m.SnapshotLimit() + if limit == 0 { + return errors.New("snapshots are disabled") + } snaps, err := m.lx.SnapshotList(u.Name) if err != nil { return err } - if len(snaps) >= m.SnapshotLimit() { - return fmt.Errorf("snapshot limit reached (%d)", m.SnapshotLimit()) + if len(snaps) >= limit { + return fmt.Errorf("snapshot limit reached (%d)", limit) } return m.lx.SnapshotCreate(u.Name, snapName()) } diff --git a/src/internal/mgr/snapshot_test.go b/src/internal/mgr/snapshot_test.go index 154f716..d18d3be 100644 --- a/src/internal/mgr/snapshot_test.go +++ b/src/internal/mgr/snapshot_test.go @@ -1,8 +1,12 @@ package mgr import ( + "path/filepath" "strings" "testing" + + "vpsmgr/internal/cfg" + "vpsmgr/internal/db" ) func TestValidSnapName(t *testing.T) { @@ -45,3 +49,25 @@ func TestSnapNameFormat(t *testing.T) { t.Error("snapName() returned the same value twice; concurrent creates would collide") } } + +// TestSnapshotCreateDisabled verifies that a snapshot cap of 0 refuses creation +// outright (before any Incus call), while leaving existing snapshots untouched. +func TestSnapshotCreateDisabled(t *testing.T) { + c := cfg.Default() + d, err := db.Open(filepath.Join(t.TempDir(), "test.db")) + if err != nil { + t.Fatal(err) + } + defer d.Close() + if _, err := d.CreateUser("alice", "x", "10.115.0.2", 1, 30001, 10000, 1, 1024, 10); err != nil { + t.Fatal(err) + } + m := New(c, d) + // A dead socket means any real Incus call would fail; with the cap disabled + // we must refuse before reaching Incus at all. + c.Incus.Socket = "/nonexistent/vpsmgr-test.sock" + c.Snapshots.Limit = 0 + if err := m.SnapshotCreate("alice"); err == nil || !strings.Contains(err.Error(), "disabled") { + t.Fatalf("SnapshotCreate with limit 0 = %v, want 'snapshots are disabled'", err) + } +} diff --git a/src/internal/panel/panel_test.go b/src/internal/panel/panel_test.go index cfb51d3..631efa7 100644 --- a/src/internal/panel/panel_test.go +++ b/src/internal/panel/panel_test.go @@ -920,16 +920,27 @@ func TestSnapshotRoutesRegistered(t *testing.T) { } } -// TestSnapshotLimitClamped checks the config-driven cap is clamped to >= 1. -func TestSnapshotLimitClamped(t *testing.T) { +// TestSnapshotLimitSemantics checks the config-driven snapshot cap: default is +// 1, a positive value is the cap, and 0 (or negative) disables snapshots. +func TestSnapshotLimitSemantics(t *testing.T) { srv, _ := newTestServer(t) if got := srv.mgr.SnapshotLimit(); got != 1 { t.Fatalf("SnapshotLimit() = %d, want 1 (default)", got) } - // A zero/negative config value is treated as 1. + // A positive value is the hard cap. + srv.cfg.Snapshots.Limit = 5 + if got := srv.mgr.SnapshotLimit(); got != 5 { + t.Fatalf("SnapshotLimit() with 5 = %d, want 5", got) + } + // 0 disables snapshots (no new snapshots, existing ones untouched). srv.cfg.Snapshots.Limit = 0 - if got := srv.mgr.SnapshotLimit(); got != 1 { - t.Fatalf("SnapshotLimit() with 0 = %d, want 1", got) + if got := srv.mgr.SnapshotLimit(); got != 0 { + t.Fatalf("SnapshotLimit() with 0 = %d, want 0 (disabled)", got) + } + // A negative value (shouldn't pass config validation) is treated as disabled. + srv.cfg.Snapshots.Limit = -1 + if got := srv.mgr.SnapshotLimit(); got != 0 { + t.Fatalf("SnapshotLimit() with -1 = %d, want 0 (disabled)", got) } }