Plugins: drop the plugins.json migration, and the volume it needed
The project has no public installs, so there is nothing to migrate from. MigrateLegacyFile, the file-backed Store it read through, PLUGINS_FILE and the legacy path threaded through the Server all go. What is left is one store, PocketBase, and a plugins package that touches no filesystem at all. That was the last thing keeping api_data alive, so the volume goes too. All four compose files now declare exactly one volume, pb_data, and the standalone API Server compose declares none - it talks to an external PocketBase and has nothing of its own to keep. Backing up the stack is backing up one path again. Both images get simpler for it. The API Server image loses VOLUME /data and the su-exec entrypoint that existed only to fix a mounted volume's ownership, so it goes back to a plain USER app; its working directory is now /app and holds nothing. The AIO image loses its second volume and chowns only /pb/pb_data. One consequence worth stating plainly, because it is a small regression rather than a no-op. The panel's Settings -> PocketBase and Settings -> Web App screens write .env in the working directory, which is now ephemeral. In the multi-container stack that changes nothing: compose sets all five of those keys as container environment, and loadDotEnv only applies a key that is not already set, so the file could never win a restart there anyway. In the AIO image it did win for POCKETBASE_ADMIN_EMAIL/_PASSWORD, which are not in that container's environment - so a service account fixed from the panel now lasts only until the container is recreated. Both READMEs say so. Moving those two screens into the app_settings singleton would close it properly; the PocketBase URL and credentials cannot follow, since they are how the database is reached in the first place. go build, go vet and go test ./... pass; the compose files parse and each resolves to a single pb_data volume. Not verified: no Docker CLI here, so neither image was built. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
660af5736a
commit
ee4ac441be
@@ -260,9 +260,6 @@ it runs as its own process/container, an external plugin is also the
|
||||
and no write is accepted. The server never treats an unreachable database as
|
||||
"no plugins configured", so an outage cannot quietly erase the settings; it
|
||||
retries in the background until the read succeeds.
|
||||
- **Upgrading from a pre-PocketBase install**: an existing `plugins.json` (see
|
||||
`PLUGINS_FILE`) is imported into the database on the first boot that finds no
|
||||
settings there, and renamed to `plugins.json.migrated`.
|
||||
- **If the `app_settings` collection is missing** — an upgrade on a stack that
|
||||
runs with `PB_BOOTSTRAP=false`, so the on-boot schema pass never created it —
|
||||
the server creates that one collection itself and reads again. A missing
|
||||
|
||||
@@ -70,7 +70,7 @@ func TestDescriptor(t *testing.T) {
|
||||
|
||||
func TestRegistered(t *testing.T) {
|
||||
var found bool
|
||||
for _, v := range plugins.NewManager(plugins.NewMemoryStore()).List() {
|
||||
for _, v := range plugins.NewManager(plugins.NewMemoryStore(nil)).List() {
|
||||
if v.Name == "anker-solix" {
|
||||
found = true
|
||||
}
|
||||
|
||||
@@ -44,7 +44,7 @@ func TestDescriptor(t *testing.T) {
|
||||
func TestRegistered(t *testing.T) {
|
||||
// The plugin must self-register via init() so the manager can construct it.
|
||||
var found bool
|
||||
for _, v := range plugins.NewManager(plugins.NewMemoryStore()).List() {
|
||||
for _, v := range plugins.NewManager(plugins.NewMemoryStore(nil)).List() {
|
||||
if v.Name == "toyota" {
|
||||
found = true
|
||||
}
|
||||
|
||||
@@ -4,9 +4,6 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -194,144 +191,3 @@ func TestExternalPluginRoundTrip(t *testing.T) {
|
||||
t.Fatal("removed external plugin came back")
|
||||
}
|
||||
}
|
||||
|
||||
// --- file store -------------------------------------------------------------
|
||||
|
||||
// The file store writes via temp file + rename, leaving nothing behind.
|
||||
func TestFileStoreLeavesNoTempFiles(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "plugins.json")
|
||||
m := loadedManager(t, NewFileStore(path))
|
||||
|
||||
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())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// An unwritable directory is a persist failure, not a silent success.
|
||||
func TestFileStoreUnwritableIsAPersistFailure(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "missing-dir", "plugins.json")
|
||||
m := loadedManager(t, NewFileStore(path))
|
||||
|
||||
if _, err := m.Upsert(context.Background(), "test-a", true, nil); !IsPersist(err) {
|
||||
t.Fatalf("expected persist failure, got %v", err)
|
||||
}
|
||||
if v, _ := m.Get("test-a"); v.Enabled {
|
||||
t.Fatal("plugin reported enabled although the save never landed")
|
||||
}
|
||||
}
|
||||
|
||||
// --- legacy import ----------------------------------------------------------
|
||||
|
||||
func TestMigrateLegacyFileImportsOnceThenRenames(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "plugins.json")
|
||||
legacy := `{"test-a":{"enabled":true,"config":{"user":"me"}}}`
|
||||
if err := os.WriteFile(path, []byte(legacy), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
store := &memoryStore{}
|
||||
ctx := context.Background()
|
||||
|
||||
migrated, err := MigrateLegacyFile(ctx, store, path)
|
||||
if err != nil || !migrated {
|
||||
t.Fatalf("first import: migrated=%v err=%v", migrated, err)
|
||||
}
|
||||
if _, err := os.Stat(path); !os.IsNotExist(err) {
|
||||
t.Fatal("the legacy file should have been renamed away")
|
||||
}
|
||||
if _, err := os.Stat(path + ".migrated"); err != nil {
|
||||
t.Fatalf("renamed file missing: %v", err)
|
||||
}
|
||||
|
||||
m := loadedManager(t, store)
|
||||
if v, _ := m.Get("test-a"); !v.Enabled || v.Config["user"] != "me" {
|
||||
t.Fatalf("imported settings wrong: enabled=%v config=%v", v.Enabled, v.Config)
|
||||
}
|
||||
|
||||
// A second pass must not re-import (and there is nothing left to import).
|
||||
if migrated, err := MigrateLegacyFile(ctx, store, path); migrated || err != nil {
|
||||
t.Fatalf("second import: migrated=%v err=%v", migrated, err)
|
||||
}
|
||||
}
|
||||
|
||||
// A store that already holds settings must never be seeded from a stale file.
|
||||
func TestMigrateLegacyFileSkipsWhenStoreHasSettings(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "plugins.json")
|
||||
if err := os.WriteFile(path, []byte(`{"test-a":{"enabled":true}}`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
store := seeded(`{"test-b":{"enabled":true}}`)
|
||||
|
||||
migrated, err := MigrateLegacyFile(context.Background(), store, path)
|
||||
if err != nil || migrated {
|
||||
t.Fatalf("migrated=%v err=%v — a populated store must not be overwritten", migrated, err)
|
||||
}
|
||||
if _, err := os.Stat(path); err != nil {
|
||||
t.Fatal("the file should be left alone when nothing was imported")
|
||||
}
|
||||
}
|
||||
|
||||
// An unreadable store must not trigger an import either: seeding on a failed
|
||||
// read would overwrite live settings with a stale file.
|
||||
func TestMigrateLegacyFileSkipsWhenStoreUnreachable(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "plugins.json")
|
||||
if err := os.WriteFile(path, []byte(`{"test-a":{"enabled":true}}`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
store := &memoryStore{failLoad: errNotReady}
|
||||
|
||||
if migrated, err := MigrateLegacyFile(context.Background(), store, path); migrated || err == nil {
|
||||
t.Fatalf("migrated=%v err=%v — must refuse while the store is unreachable", migrated, err)
|
||||
}
|
||||
}
|
||||
|
||||
// A corrupt legacy file is reported and left on disk, not written into the DB.
|
||||
func TestMigrateLegacyFileRejectsCorruptFile(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "plugins.json")
|
||||
if err := os.WriteFile(path, []byte(`{"test-a":{"enabled":true}}junk`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
store := &memoryStore{}
|
||||
|
||||
migrated, err := MigrateLegacyFile(context.Background(), store, path)
|
||||
if migrated || err == nil {
|
||||
t.Fatalf("migrated=%v err=%v — a corrupt file must not be imported", migrated, err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "not imported") {
|
||||
t.Fatalf("error should say nothing was imported, got: %v", err)
|
||||
}
|
||||
if store.data != nil {
|
||||
t.Fatal("nothing should have been written")
|
||||
}
|
||||
if _, statErr := os.Stat(path); statErr != nil {
|
||||
t.Fatal("the corrupt file should be left on disk for inspection")
|
||||
}
|
||||
}
|
||||
|
||||
// A fresh install has no file and no settings; that is not an error.
|
||||
func TestMigrateLegacyFileNoFile(t *testing.T) {
|
||||
store := &memoryStore{}
|
||||
path := filepath.Join(t.TempDir(), "plugins.json")
|
||||
if migrated, err := MigrateLegacyFile(context.Background(), store, path); migrated || err != nil {
|
||||
t.Fatalf("migrated=%v err=%v", migrated, err)
|
||||
}
|
||||
if migrated, err := MigrateLegacyFile(context.Background(), store, ""); migrated || err != nil {
|
||||
t.Fatalf("empty path: migrated=%v err=%v", migrated, err)
|
||||
}
|
||||
}
|
||||
|
||||
// writeTestFile is a small helper shared with store_pb_test.go.
|
||||
func writeTestFile(path, content string) error {
|
||||
return os.WriteFile(path, []byte(content), 0o600)
|
||||
}
|
||||
|
||||
@@ -8,9 +8,10 @@
|
||||
// - "external" — a remote service registered at runtime (no rebuild) that speaks
|
||||
// a small JSON contract over HTTP. See external.go.
|
||||
//
|
||||
// Enable-state and per-plugin config (including secrets) are persisted to a local
|
||||
// plugins.json by the Manager, mirroring how the PocketBase connection persists to
|
||||
// .env. See doc.go for the deliberately-deferred extension points.
|
||||
// Enable-state and per-plugin config (including secrets) are persisted by the
|
||||
// Manager to PocketBase — the app_settings singleton, in the same pluginSettings
|
||||
// field the org and user layers of the cascade use. Nothing is kept on disk. See
|
||||
// doc.go for the deliberately-deferred extension points.
|
||||
package plugins
|
||||
|
||||
import (
|
||||
|
||||
@@ -6,8 +6,6 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
|
||||
"drivervault/apiserver/internal/pb"
|
||||
@@ -15,14 +13,11 @@ import (
|
||||
|
||||
// Store is where a Manager keeps plugin enable-state and global (L1) config.
|
||||
//
|
||||
// It deals in the raw JSON document rather than in records, so the same Manager
|
||||
// logic works whichever backing it has: PocketBase in production, a file for the
|
||||
// one-time import of the legacy plugins.json, memory in tests.
|
||||
// It deals in the raw JSON document rather than in records, so the Manager does
|
||||
// not care where that document lives: PocketBase in production, memory in tests.
|
||||
type Store interface {
|
||||
// Load returns the stored document. found is false when the store holds no
|
||||
// document at all — a fresh install — which callers must tell apart from a
|
||||
// document that exists and is empty, because only the former may be seeded
|
||||
// from a legacy file.
|
||||
// document yet — a fresh install, nothing configured.
|
||||
//
|
||||
// A returned error means the store could not be reached or read. It must
|
||||
// NOT be taken as "no settings": the Manager stays unloaded and refuses to
|
||||
@@ -53,7 +48,8 @@ const globalSettingsKey = "global"
|
||||
// --- PocketBase-backed store (production) -----------------------------------
|
||||
|
||||
// pbStore keeps the document in a single record of the app_settings collection,
|
||||
// identified by key="global". This is the production store.
|
||||
// identified by key="global". This is the only store the server itself uses:
|
||||
// nothing about the plugin layer touches the filesystem.
|
||||
type pbStore struct {
|
||||
client *pb.Client
|
||||
collection string
|
||||
@@ -185,62 +181,6 @@ func isNotFound(err error) bool {
|
||||
return errors.As(err, &apiErr) && apiErr.Status == 404
|
||||
}
|
||||
|
||||
// --- File-backed store (legacy import, and tests) ---------------------------
|
||||
|
||||
// fileStore keeps the document in a local JSON file. This was the only store
|
||||
// before the global layer moved into PocketBase; it survives as the source of
|
||||
// the one-time import, and as a convenient store for tests.
|
||||
type fileStore struct{ path string }
|
||||
|
||||
// NewFileStore returns a Store backed by the JSON file at path.
|
||||
func NewFileStore(path string) Store { return &fileStore{path: path} }
|
||||
|
||||
func (s *fileStore) Describe() string { return s.path }
|
||||
|
||||
func (s *fileStore) Load(ctx context.Context) ([]byte, bool, error) {
|
||||
data, err := os.ReadFile(s.path)
|
||||
switch {
|
||||
case err == nil:
|
||||
return data, true, nil
|
||||
case errors.Is(err, os.ErrNotExist):
|
||||
return nil, false, nil
|
||||
default:
|
||||
return nil, false, fmt.Errorf("%w: %v", errNotReady, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Save writes through a temporary file and renames it into place, so an
|
||||
// interrupted write cannot leave a half-written document behind.
|
||||
func (s *fileStore) Save(ctx context.Context, data []byte) error {
|
||||
tmp, err := os.CreateTemp(filepath.Dir(s.path), ".plugins-*.json")
|
||||
if err != nil {
|
||||
return fmt.Errorf("%w: %v", errPersist, err)
|
||||
}
|
||||
tmpName := tmp.Name()
|
||||
defer func() {
|
||||
if tmpName != "" {
|
||||
_ = os.Remove(tmpName)
|
||||
}
|
||||
}()
|
||||
|
||||
if _, err := tmp.Write(data); err != nil {
|
||||
_ = tmp.Close()
|
||||
return fmt.Errorf("%w: %v", errPersist, err)
|
||||
}
|
||||
if err := tmp.Sync(); err != nil {
|
||||
_ = tmp.Close()
|
||||
return fmt.Errorf("%w: %v", errPersist, err)
|
||||
}
|
||||
if err := tmp.Close(); err != nil {
|
||||
return fmt.Errorf("%w: %v", errPersist, err)
|
||||
}
|
||||
if err := os.Rename(tmpName, s.path); err != nil {
|
||||
return fmt.Errorf("%w: %v", errPersist, err)
|
||||
}
|
||||
tmpName = ""
|
||||
return nil
|
||||
}
|
||||
|
||||
// --- In-memory store (tests) ------------------------------------------------
|
||||
|
||||
// memoryStore holds the document in memory. failLoad/failSave let a test drive
|
||||
@@ -253,8 +193,11 @@ type memoryStore struct {
|
||||
failSave error
|
||||
}
|
||||
|
||||
// NewMemoryStore returns a Store that keeps the document in memory.
|
||||
func NewMemoryStore() Store { return &memoryStore{} }
|
||||
// NewMemoryStore returns a Store that keeps the document in memory, seeded with
|
||||
// document (nil for an empty store). Intended for tests.
|
||||
func NewMemoryStore(document []byte) Store {
|
||||
return &memoryStore{data: document, found: document != nil}
|
||||
}
|
||||
|
||||
func (s *memoryStore) Describe() string { return "memory" }
|
||||
|
||||
@@ -277,60 +220,3 @@ func (s *memoryStore) Save(ctx context.Context, data []byte) error {
|
||||
s.found = true
|
||||
return nil
|
||||
}
|
||||
|
||||
// --- Legacy import ----------------------------------------------------------
|
||||
|
||||
// MigrateLegacyFile seeds store from the pre-PocketBase plugins.json at path,
|
||||
// and is a no-op unless every condition holds: the store has no document at all,
|
||||
// the file exists, and it parses. On success the file is renamed to
|
||||
// <path>.migrated so a later boot cannot import it a second time or let the two
|
||||
// copies drift apart.
|
||||
//
|
||||
// It reports whether an import happened. A store that could not be read returns
|
||||
// an error and imports nothing — seeding on a failed read would overwrite live
|
||||
// settings with a stale file.
|
||||
func MigrateLegacyFile(ctx context.Context, store Store, path string) (bool, error) {
|
||||
if path == "" {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
_, found, err := store.Load(ctx)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if found {
|
||||
return false, nil // already living in the store; the file is history
|
||||
}
|
||||
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return false, nil // fresh install, nothing to import
|
||||
}
|
||||
return false, err
|
||||
}
|
||||
|
||||
// Only import something we can actually parse, so a corrupt file is left on
|
||||
// disk for inspection rather than written into the database.
|
||||
recs := map[string]*record{}
|
||||
if err := decodeRecords(data, recs); err != nil {
|
||||
return false, fmt.Errorf("legacy %s not imported: %v", path, err)
|
||||
}
|
||||
if len(recs) == 0 {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
encoded, err := encodeRecords(recs)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if err := store.Save(ctx, encoded); err != nil {
|
||||
return false, err
|
||||
}
|
||||
if err := os.Rename(path, path+".migrated"); err != nil {
|
||||
// The settings are safely in the store; failing to rename only risks a
|
||||
// confusing leftover file, so report it without undoing the import.
|
||||
return true, fmt.Errorf("imported %s but could not rename it: %v", path, err)
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
|
||||
@@ -220,35 +220,3 @@ func TestPBStoreOutageIsNotAMissingCollection(t *testing.T) {
|
||||
t.Fatalf("an outage must not be reported as a missing collection: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// The legacy file is imported into PocketBase exactly once.
|
||||
func TestPBStoreLegacyImport(t *testing.T) {
|
||||
f := &fakePB{}
|
||||
store := newPBStore(t, f)
|
||||
path := t.TempDir() + "/plugins.json"
|
||||
if err := writeTestFile(path, `{"test-a":{"enabled":true,"config":{"user":"me"}}}`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
migrated, err := MigrateLegacyFile(context.Background(), store, path)
|
||||
if err != nil || !migrated {
|
||||
t.Fatalf("import: migrated=%v err=%v", migrated, err)
|
||||
}
|
||||
|
||||
m := loadedManager(t, store)
|
||||
if v, _ := m.Get("test-a"); !v.Enabled || v.Config["user"] != "me" {
|
||||
t.Fatalf("imported settings wrong: enabled=%v config=%v", v.Enabled, v.Config)
|
||||
}
|
||||
|
||||
// Second boot: the store now has a document, so nothing is re-imported.
|
||||
migrated, err = MigrateLegacyFile(context.Background(), store, path)
|
||||
if err != nil || migrated {
|
||||
t.Fatalf("second import: migrated=%v err=%v", migrated, err)
|
||||
}
|
||||
f.mu.Lock()
|
||||
creates := f.creates
|
||||
f.mu.Unlock()
|
||||
if creates != 1 {
|
||||
t.Fatalf("expected one create, got %d", creates)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user