Files
DriverVault/API Server/internal/plugins/manager_persist_test.go
T
tajniak81andClaude Opus 5 c173ca3653 Plugins: a save that fails should say so, not vanish on redeploy
Reported symptom: every plugin comes back disabled after redeploying the
image, having been enabled before it. The persistence design was already
right - each compose file mounts api_data:/data and points PLUGINS_FILE at
/data/plugins.json - so the fault was that a failed write to that file was
invisible. Three defects, each confirmed with a test before being fixed:

A failed write was reported as success. Upsert set rec.Enabled before it
persisted, and the handler folded the resulting error into the same
200-with-warning used for "saved, but the connector failed to start". The
panel reloaded, read the in-memory record and showed the plugin enabled;
only a restart revealed that nothing had reached the disk. A save that
fails now rolls back in memory and returns 500, so the panel row shows the
error instead of "Saved".

A corrupt state file silently wiped the rest. Load returned an error,
main.go logged it and carried on with an empty record set, so the next
toggle overwrote plugins.json and took every other plugin's config with
it. An unreadable file is now moved aside to plugins.json.corrupt, and
persistLocked writes through a temp file + rename so an interrupted write
cannot produce that corrupt file in the first place.

A state file holding "null" panicked the server with "assignment to entry
in nil map" on the next save, and a null entry nil-dereferenced in Load.
Both now decode to "nothing configured".

Two changes make the next such failure loud rather than silent.
StartPlugins probes writability at boot and warns that plugin changes will
not survive a restart. And the API Server image gains the root entrypoint
the AIO image already had - chown /data, then drop to app via su-exec -
because a host bind mount (API_DATA=/srv/...) or a volume created before
/data existed arrives root-owned, and the unprivileged process cannot
write to it.

Not addressed here: a deployment that never reuses the named volume
(docker compose down -v, a renamed compose project, an anonymous volume
from a bare docker run) loses the file whatever the code does. The new
boot warning tells the two apart - writable but empty means the volume is
the problem, not permissions.

go build, go vet and go test ./... all pass. The Dockerfile change is
reviewed but not built: there is no Docker CLI on this machine, so the
su-exec privilege drop follows standard Alpine practice rather than an
observed run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-21 16:17:47 +02:00

210 lines
6.9 KiB
Go

package plugins
import (
"context"
"encoding/json"
"os"
"path/filepath"
"strings"
"testing"
)
// testPlugin is a no-op plugin used to exercise the manager's persistence
// without reaching for a real connector (the builtins register themselves from
// a package that imports this one, so they are not available here).
type testPlugin struct{ name string }
func (t *testPlugin) Descriptor() Descriptor { return Descriptor{Name: t.name} }
func (t *testPlugin) Init(context.Context, map[string]string) error { return nil }
func (t *testPlugin) Shutdown(context.Context) error { return nil }
func (t *testPlugin) HealthCheck(context.Context) Health { return Health{Status: "ok"} }
func (t *testPlugin) Invoke(context.Context, string, json.RawMessage) (json.RawMessage, error) {
return nil, nil
}
func init() {
for _, n := range []string{"test-a", "test-b"} {
Register(n, func() Plugin { return &testPlugin{name: n} })
}
}
// A state file holding "null" (or nothing at all) used to decode to a nil map,
// which made the next save panic with "assignment to entry in nil map".
func TestLoadNullStateFileDoesNotPanic(t *testing.T) {
for _, content := range []string{"null", "", " \n"} {
path := filepath.Join(t.TempDir(), "plugins.json")
if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
t.Fatal(err)
}
m := NewManager(path)
if err := m.Load(); err != nil {
t.Fatalf("Load(%q): %v", content, err)
}
if _, err := m.Upsert(context.Background(), "test-a", true, nil); err != nil {
t.Fatalf("Upsert after %q state file: %v", content, err)
}
if v, _ := m.Get("test-a"); !v.Enabled {
t.Fatalf("plugin not enabled after save (state file was %q)", content)
}
}
}
// A null entry for a single plugin left a nil *record that Load dereferenced.
func TestLoadNullRecordIsDropped(t *testing.T) {
path := filepath.Join(t.TempDir(), "plugins.json")
if err := os.WriteFile(path, []byte(`{"test-a":null,"test-b":{"enabled":true}}`), 0o600); err != nil {
t.Fatal(err)
}
m := NewManager(path)
if err := m.Load(); err != nil {
t.Fatalf("Load: %v", err)
}
if v, _ := m.Get("test-b"); !v.Enabled {
t.Fatal("test-b should still be enabled")
}
}
// An unwritable state directory must fail loudly and leave the in-memory state
// matching the disk, rather than reporting success and reverting on restart.
func TestUpsertRollsBackWhenStateCannotBeSaved(t *testing.T) {
path := filepath.Join(t.TempDir(), "missing-dir", "plugins.json")
m := NewManager(path)
if err := m.Load(); err != nil {
t.Fatalf("Load: %v", err)
}
_, err := m.Upsert(context.Background(), "test-a", true, map[string]string{"k": "v"})
if err == nil {
t.Fatal("expected an error when the state file cannot be written")
}
if !IsPersist(err) {
t.Fatalf("error should be classified as a persist failure, got %v", err)
}
if v, _ := m.Get("test-a"); v.Enabled {
t.Fatal("plugin reported as enabled although the save never reached disk")
}
if _, statErr := os.Stat(path); statErr == nil {
t.Fatal("state file unexpectedly exists")
}
}
// A rolled-back save must not clobber the value that was already stored.
func TestUpsertRollbackKeepsPreviousConfig(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "plugins.json")
m := NewManager(path)
if err := m.Load(); err != nil {
t.Fatal(err)
}
if _, err := m.Upsert(context.Background(), "test-a", true, map[string]string{"user": "first"}); err != nil {
t.Fatal(err)
}
// Make the next write fail by pointing the manager at an unusable directory.
m.path = filepath.Join(dir, "missing-dir", "plugins.json")
if _, err := m.Upsert(context.Background(), "test-a", false, map[string]string{"user": "second"}); !IsPersist(err) {
t.Fatalf("expected persist failure, got %v", err)
}
v, _ := m.Get("test-a")
if !v.Enabled || v.Config["user"] != "first" {
t.Fatalf("failed save leaked into memory: enabled=%v user=%q", v.Enabled, v.Config["user"])
}
}
// A state file that does not parse must be preserved, not silently replaced by
// the next save — which used to take every other plugin's settings with it.
func TestCorruptStateFileIsPreservedNotOverwritten(t *testing.T) {
path := filepath.Join(t.TempDir(), "plugins.json")
good := `{"test-a":{"enabled":true,"config":{"user":"me"}},"test-b":{"enabled":true}}`
if err := os.WriteFile(path, []byte(good+"garbage"), 0o600); err != nil {
t.Fatal(err)
}
m := NewManager(path)
err := m.Load()
if err == nil {
t.Fatal("Load should report an unreadable state file")
}
if !strings.Contains(err.Error(), ".corrupt") {
t.Fatalf("error should name the backup it made, got: %v", err)
}
backup, readErr := os.ReadFile(path + ".corrupt")
if readErr != nil {
t.Fatalf("original state was not preserved: %v", readErr)
}
if !strings.Contains(string(backup), `"user":"me"`) {
t.Fatal("backup does not hold the original content")
}
// The server keeps running; a later save must not touch the backup.
if _, err := m.Upsert(context.Background(), "test-a", true, nil); err != nil {
t.Fatal(err)
}
if again, _ := os.ReadFile(path + ".corrupt"); string(again) != string(backup) {
t.Fatal("backup was modified by a later save")
}
}
// Enable/disable state must survive a restart — the whole point of the file.
func TestStateSurvivesReload(t *testing.T) {
path := filepath.Join(t.TempDir(), "plugins.json")
m := NewManager(path)
if err := m.Load(); err != nil {
t.Fatal(err)
}
if _, err := m.Upsert(context.Background(), "test-a", true, map[string]string{"user": "me"}); err != nil {
t.Fatal(err)
}
// Restart.
m2 := NewManager(path)
if err := m2.Load(); err != nil {
t.Fatalf("reload: %v", err)
}
v, ok := m2.Get("test-a")
if !ok || !v.Enabled || v.Config["user"] != "me" {
t.Fatalf("state lost across restart: ok=%v enabled=%v config=%v", ok, v.Enabled, v.Config)
}
}
// persistLocked writes via a temp file + rename; no strays may be left behind.
func TestPersistLeavesNoTempFiles(t *testing.T) {
dir := t.TempDir()
m := NewManager(filepath.Join(dir, "plugins.json"))
if err := m.Load(); err != nil {
t.Fatal(err)
}
for i := 0; i < 3; i++ {
if _, err := m.Upsert(context.Background(), "test-a", true, nil); err != nil {
t.Fatal(err)
}
}
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
for _, e := range entries {
if e.Name() != "plugins.json" {
t.Fatalf("unexpected leftover file: %s", e.Name())
}
}
}
// CheckWritable is the boot-time probe that makes an unwritable volume visible.
func TestCheckWritable(t *testing.T) {
dir := t.TempDir()
if err := NewManager(filepath.Join(dir, "plugins.json")).CheckWritable(); err != nil {
t.Fatalf("writable directory reported as unwritable: %v", err)
}
err := NewManager(filepath.Join(dir, "missing-dir", "plugins.json")).CheckWritable()
if err == nil {
t.Fatal("missing directory should not report as writable")
}
if !IsPersist(err) {
t.Fatalf("expected a persist-classified error, got %v", err)
}
}