From 9bd5c523c4597bd3c80568545602195f0ecbffd8 Mon Sep 17 00:00:00 2001 From: tajniak81 <13187254+tajniak81@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:52:47 +0200 Subject: [PATCH] Plugins: the global layer moves into the database, beside the other two The integration cascade stored its top layer differently from the two below it: org (L2) and user (L3) plugin config lived in PocketBase, in a pluginSettings field, while the global (L1) layer sat in a plugins.json next to the binary. That split was accretion rather than design - the file was the whole store in the v1 MVP, and the per-tenant layers were later built on PocketBase and layered on top of it instead of replacing it. It also cost something real. plugins.json was a second state store with different durability from pb_data: its own volume, its own ownership, its own backup. Losing pb_data is unmissable; losing api_data was silent, which is how "every plugin comes back disabled after a redeploy" happened. L1 now lives in the app_settings collection - one record keyed "global", holding its settings in a pluginSettings field, the same mechanism and the same field name the layers below use. The documents still differ in shape, because only L1 carries enable state and the registration of external plugins, but the storage is no longer a special case. The Manager grows a Store seam (PocketBase in production, file for the import, memory for tests) and, more importantly, a loaded gate. Settings in a database mean the store can be unreachable at boot - a cold stack, or a service account still to be set from the panel. That must not read as "no plugins configured", or the first save would write emptiness over real settings. So until a read succeeds the Manager stays unloaded, every mutation is refused, /api/admin/plugins* answers 503, and a background retry backs off to two minutes. The same gate covers a document that will not parse: it is never replaced by one built from an empty map, which is a stronger guarantee than the .corrupt backup it replaces. Writing to a store also revealed a hole in the previous fix. Classifying a save failure as errPersist was left to each Store, and a store that returned a plain error would fall through to the "saved, but the plugin failed to start" branch and be reported as a 200 - the same silent-success bug through a different door. The Manager now classifies, whatever the Store returns; a test pins it. Upgrades are automatic: on the first boot that finds no settings in the database, an existing plugins.json is imported and renamed to plugins.json.migrated. The import is refused if the store is merely unreachable, or if the file does not parse, so a stale or broken file can never overwrite live settings. /data is still needed - the panel rewrites .env there when it retargets PocketBase - but plugin settings no longer depend on it. 21 tests in internal/plugins cover both stores, including the production path against a fake PocketBase: create-then-update of the singleton, round-trip across a restart, an outage that leaves settings intact, a missing collection reading as not-ready rather than empty, and the import running exactly once. go build, go vet and go test ./... pass. Schema changes are mirrored into scripts/setup-pocketbase.mjs as that file requires. Not verified: no Docker CLI here, so no image was built and the bootstrap of app_settings against a real PocketBase is untested outside the fake. Co-Authored-By: Claude Opus 5 --- API Server/.env.example | 8 +- API Server/Dockerfile | 19 +- API Server/README.md | 17 +- API Server/cmd/server/main.go | 8 +- API Server/docker-compose.yml | 3 +- API Server/internal/api/control_e2e_test.go | 7 +- API Server/internal/api/plugins.go | 37 +- API Server/internal/api/server.go | 108 ++++-- .../internal/api/vehicleproviders_test.go | 8 +- API Server/internal/bootstrap/schema.go | 14 + API Server/internal/config/config.go | 5 +- API Server/internal/plugins/README.md | 22 +- .../builtin/ankersolix/ankersolix_test.go | 2 +- .../plugins/builtin/toyota/toyota_test.go | 2 +- API Server/internal/plugins/manager.go | 213 ++++++----- .../internal/plugins/manager_persist_test.go | 360 ++++++++++++------ API Server/internal/plugins/store.go | 329 ++++++++++++++++ API Server/internal/plugins/store_pb_test.go | 236 ++++++++++++ API Server/scripts/setup-pocketbase.mjs | 16 +- Docker-AIO/.env.prod.example | 3 +- Docker-AIO/Dockerfile | 7 +- Docker-AIO/README.md | 2 +- Docker-AIO/docker-compose.prod.yml | 2 +- Docker-AIO/docker-compose.yml | 2 +- Docker/.env.prod.example | 3 +- Docker/README.md | 16 +- Docker/docker-compose.prod.yml | 5 +- Docker/docker-compose.yml | 6 +- 28 files changed, 1161 insertions(+), 299 deletions(-) create mode 100644 API Server/internal/plugins/store.go create mode 100644 API Server/internal/plugins/store_pb_test.go diff --git a/API Server/.env.example b/API Server/.env.example index 7547949..b894e36 100644 --- a/API Server/.env.example +++ b/API Server/.env.example @@ -30,9 +30,11 @@ WEBAPP_URL=http://localhost:8090 # PocketBase auth collection holding app users (default: users). AUTH_USERS_COLLECTION=users -# Local JSON store for plugin enable-state + config (default: plugins.json). -# Relative paths resolve against the working directory, so in Docker this points -# at the mounted volume (see the Dockerfile) rather than the image layer. +# Pre-PocketBase JSON store for the global plugin layer (default: plugins.json). +# Those settings now live in the database, in the app_settings singleton, next to +# the per-org and per-user layers. This path is only read once — to import an +# existing file on the first boot after the upgrade — and is renamed to +# plugins.json.migrated afterwards. A fresh install can leave it unset. PLUGINS_FILE=plugins.json # --- EV charging control (Anker Solix, OCPP) --------------------------------- diff --git a/API Server/Dockerfile b/API Server/Dockerfile index affd3ec..26b281d 100644 --- a/API Server/Dockerfile +++ b/API Server/Dockerfile @@ -31,11 +31,12 @@ RUN addgroup -S app && adduser -S -G app app COPY --from=build /out/api-server /usr/local/bin/api-server -# The server writes two files relative to its working directory: plugins.json -# (plugin enable-state + config) and .env, which the panel rewrites when a -# superadmin retargets the PocketBase connection. Both must therefore live on a -# writable, persistent path — hence /data, owned by the unprivileged user and -# declared as a volume. A fresh named volume inherits this ownership. +# The server writes .env relative to its working directory — the panel rewrites +# it when a superadmin retargets the PocketBase connection — and reads a legacy +# plugins.json from there once, to import it into the database. So the working +# directory must be writable and persistent: hence /data, owned by the +# unprivileged user and declared as a volume. A fresh named volume inherits this +# ownership. (Plugin settings themselves live in PocketBase, not here.) RUN mkdir -p /data && chown app:app /data WORKDIR /data VOLUME /data @@ -44,9 +45,9 @@ VOLUME /data # a host bind mount (API_DATA=/srv/... in docker-compose.prod.yml) arrives owned # by root, and so does a volume created by an image from before /data existed, # when the server ran with a root-owned working directory. In both cases the -# unprivileged process cannot write plugins.json — which shows up as plugins -# that enable fine in the panel and come back disabled after the next redeploy. -# So the entrypoint starts as root purely to fix ownership, then drops to app. +# unprivileged process cannot write .env, so retargeting PocketBase from the +# panel silently fails to stick across a restart. The entrypoint therefore starts +# as root purely to fix ownership, then drops to app. RUN cat > /entrypoint.sh <<'ENTRY' #!/bin/sh set -e @@ -66,6 +67,8 @@ RUN chmod +x /entrypoint.sh # Config comes entirely from environment variables (see .env.example). # POCKETBASE_ADMIN_EMAIL / _PASSWORD are optional at startup: without them the # server still runs and a superadmin can configure the connection from the panel. +# PLUGINS_FILE is only the one-time import path for a pre-PocketBase install; +# the settings themselves live in the database. ENV API_ADDR=:8080 \ PLUGINS_FILE=/data/plugins.json EXPOSE 8080 diff --git a/API Server/README.md b/API Server/README.md index ed587fc..03a0e5f 100644 --- a/API Server/README.md +++ b/API Server/README.md @@ -284,8 +284,9 @@ and then rebuild the Go binary, since `dist` is embedded. An extension system for integrating third-party services, managed by a superadmin. Two kinds share one contract: **built-in** (Go, compiled in) and -**external** (any HTTP service, registered at runtime, **no rebuild**). State -persists to `plugins.json`. +**external** (any HTTP service, registered at runtime, **no rebuild**). Enable +state and global config persist to PocketBase, in the `app_settings` singleton — +the same place the per-org and per-user layers of the cascade live. See **[`internal/plugins/README.md`](internal/plugins/README.md)** for the full guide. Two built-in connectors ship today — **Toyota Connected** (`toyota`, @@ -317,7 +318,7 @@ Copy `.env.example` to `.env` and fill in. Summary: | `CORS_ALLOW_ORIGINS` | `*` | comma-separated browser origins | | `WEBAPP_URL` | `http://localhost:8090` | probed by `/api/status` | | `AUTH_USERS_COLLECTION` | `users` | PocketBase auth collection | -| `PLUGINS_FILE` | `plugins.json` | plugin state store | +| `PLUGINS_FILE` | `plugins.json` | legacy plugin store, imported once then renamed | | `OCPP_REQUIRE_TLS` | `true` | reject chargers that did not connect over TLS | | `OCPP_PUBLIC_URL` | — | canonical `ws(s)://` base to point chargers at | | `PB_BOOTSTRAP` | `true` | run the on-boot schema create/reconcile | @@ -326,10 +327,12 @@ Copy `.env.example` to `.env` and fill in. Summary: `PB_URL`, `PB_ADMIN_EMAIL`, `PB_ADMIN_PASSWORD`, `PORT` and `CORS_ORIGINS` are still honoured for older deployments; the modern names win when both are set. -Two paths are resolved **relative to the working directory**: `PLUGINS_FILE` and -the `.env` the panel rewrites when a superadmin retargets PocketBase. In Docker -the working directory is `/data`, a volume, so both survive a container -recreate — see [`Dockerfile`](Dockerfile) and [`../Docker`](../Docker). +Two paths are resolved **relative to the working directory**: the `.env` the +panel rewrites when a superadmin retargets PocketBase, and `PLUGINS_FILE` (read +once, to import a pre-PocketBase plugin store). In Docker the working directory +is `/data`, a volume, so the `.env` survives a container recreate — see +[`Dockerfile`](Dockerfile) and [`../Docker`](../Docker). Plugin settings no +longer depend on that volume: they live in the database with everything else. `CORS_ALLOW_ORIGINS` only matters for **browser** clients (the web app). Native mobile apps are not subject to CORS. diff --git a/API Server/cmd/server/main.go b/API Server/cmd/server/main.go index 8c18eec..dc3299e 100644 --- a/API Server/cmd/server/main.go +++ b/API Server/cmd/server/main.go @@ -67,9 +67,11 @@ func main() { srv := api.New(cfg, client) - if err := srv.StartPlugins(); err != nil { - log.Printf("plugins: load failed: %v", err) - } + // Not fatal: the plugin settings live in PocketBase, which may not be + // reachable yet on a cold stack or before a service account is configured. + // StartPlugins logs the reason and retries in the background; the plugin + // endpoints answer 503 until the read succeeds. + _ = srv.StartPlugins() httpServer := &http.Server{ Addr: cfg.Addr, diff --git a/API Server/docker-compose.yml b/API Server/docker-compose.yml index bac9218..455bfa0 100644 --- a/API Server/docker-compose.yml +++ b/API Server/docker-compose.yml @@ -37,7 +37,8 @@ services: # on the host rather than in this compose file. - "host.docker.internal:host-gateway" volumes: - # Holds plugins.json and the .env the panel writes back — see Dockerfile. + # Holds the .env the panel writes back when a superadmin retargets + # PocketBase — see Dockerfile. Plugin settings live in the database. - api_data:/data healthcheck: test: ["CMD-SHELL", "wget -qO- http://127.0.0.1:8080/healthz || exit 1"] diff --git a/API Server/internal/api/control_e2e_test.go b/API Server/internal/api/control_e2e_test.go index 0dbd821..5173254 100644 --- a/API Server/internal/api/control_e2e_test.go +++ b/API Server/internal/api/control_e2e_test.go @@ -16,6 +16,7 @@ import ( "drivervault/apiserver/internal/config" "drivervault/apiserver/internal/ocpp" "drivervault/apiserver/internal/pb" + "drivervault/apiserver/internal/plugins" ) // This test exercises the two PocketBase-touching control paths end-to-end @@ -131,7 +132,11 @@ func TestControlStepUpAndAuditE2E(t *testing.T) { PluginsFile: pluginsFile, OCPPRequireTLS: false, // httptest is plaintext; TLS enforcement covered elsewhere }, pb.New(pbSrv.URL, "admin@test.local", "pw")) - if err := s.plugins.Load(); err != nil { + // The global layer normally lives in PocketBase; point it at the file above + // so this test does not have to stand up an app_settings collection too. + s.pluginStore = plugins.NewFileStore(pluginsFile) + s.plugins = plugins.NewManager(s.pluginStore) + if err := s.plugins.Load(context.Background()); err != nil { t.Fatalf("load plugins: %v", err) } // Seed the control-token index (mirrors what generating a token does). diff --git a/API Server/internal/api/plugins.go b/API Server/internal/api/plugins.go index 19e005d..4a70ebf 100644 --- a/API Server/internal/api/plugins.go +++ b/API Server/internal/api/plugins.go @@ -8,13 +8,32 @@ import ( "drivervault/apiserver/internal/plugins" ) +// pluginsReady guards every plugin endpoint. Until the settings have been read +// from PocketBase the server knows of no plugins — reporting that as an empty or +// all-disabled list would be a lie the panel could then save back over the real +// settings, so the endpoints answer 503 instead. +func (s *Server) pluginsReady(w http.ResponseWriter) bool { + if s.plugins.Ready() { + return true + } + writeError(w, http.StatusServiceUnavailable, + "plugin settings are not loaded yet — the database is unreachable; retrying") + return false +} + // GET /api/admin/plugins — every known plugin (registry ∪ persisted), secrets masked. func (s *Server) handleListPlugins(w http.ResponseWriter, r *http.Request) { + if !s.pluginsReady(w) { + return + } writeJSON(w, http.StatusOK, map[string]any{"plugins": s.plugins.List()}) } // GET /api/admin/plugins/{name} — one plugin's view. func (s *Server) handleGetPlugin(w http.ResponseWriter, r *http.Request) { + if !s.pluginsReady(w) { + return + } v, ok := s.plugins.Get(r.PathValue("name")) if !ok { writeError(w, http.StatusNotFound, "unknown plugin") @@ -26,6 +45,9 @@ func (s *Server) handleGetPlugin(w http.ResponseWriter, r *http.Request) { // PUT /api/admin/plugins/{name} — enable/disable + merge config. Body: // {enabled?, config?}. A secret left at the mask keeps its stored value. func (s *Server) handleUpdatePlugin(w http.ResponseWriter, r *http.Request) { + if !s.pluginsReady(w) { + return + } name := r.PathValue("name") current, ok := s.plugins.Get(name) if !ok { @@ -51,8 +73,10 @@ func (s *Server) handleUpdatePlugin(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, map[string]any{"plugin": v}) case plugins.IsUnknown(err): writeError(w, http.StatusNotFound, "unknown plugin") + case plugins.IsNotReady(err): + writeError(w, http.StatusServiceUnavailable, err.Error()) case plugins.IsPersist(err): - // The change never reached plugins.json and has been rolled back. + // The change never reached the store and has been rolled back. // Reporting this as a 200-with-warning is what let a plugin look // enabled in the panel and come back disabled after a redeploy. writeError(w, http.StatusInternalServerError, err.Error()) @@ -65,6 +89,9 @@ func (s *Server) handleUpdatePlugin(w http.ResponseWriter, r *http.Request) { // POST /api/admin/plugins — register an external (remote HTTP) plugin. Body: // {name, baseURL, provider?}. This is the "add a plugin without a rebuild" path. func (s *Server) handleRegisterPlugin(w http.ResponseWriter, r *http.Request) { + if !s.pluginsReady(w) { + return + } var body struct { Name string `json:"name"` BaseURL string `json:"baseURL"` @@ -79,7 +106,7 @@ func (s *Server) handleRegisterPlugin(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "name and baseURL are required") return } - if err := s.plugins.RegisterExternal(body.Name, body.BaseURL, body.Provider); err != nil { + if err := s.plugins.RegisterExternal(r.Context(), body.Name, body.BaseURL, body.Provider); err != nil { if plugins.IsPersist(err) { writeError(w, http.StatusInternalServerError, err.Error()) return @@ -94,6 +121,9 @@ func (s *Server) handleRegisterPlugin(w http.ResponseWriter, r *http.Request) { // DELETE /api/admin/plugins/{name} — remove an external plugin (builtins can // only be disabled). func (s *Server) handleDeletePlugin(w http.ResponseWriter, r *http.Request) { + if !s.pluginsReady(w) { + return + } if err := s.plugins.Remove(r.Context(), r.PathValue("name")); err != nil { if plugins.IsPersist(err) { writeError(w, http.StatusInternalServerError, err.Error()) @@ -108,6 +138,9 @@ func (s *Server) handleDeletePlugin(w http.ResponseWriter, r *http.Request) { // POST /api/admin/plugins/{name}/health — run a health check now. Works on // disabled plugins too, so a config can be verified before enabling it. func (s *Server) handlePluginHealth(w http.ResponseWriter, r *http.Request) { + if !s.pluginsReady(w) { + return + } h, err := s.plugins.HealthCheck(r.Context(), r.PathValue("name")) if err != nil { if plugins.IsUnknown(err) { diff --git a/API Server/internal/api/server.go b/API Server/internal/api/server.go index 77b0574..4567112 100644 --- a/API Server/internal/api/server.go +++ b/API Server/internal/api/server.go @@ -148,6 +148,7 @@ const ( colDocuments = "car_documents" colReminders = "reminders" colControlAudit = "control_audit" + colAppSettings = "app_settings" ) // Server wires together the HTTP handlers and their dependencies. @@ -157,6 +158,13 @@ type Server struct { pb *pb.Client plugins *plugins.Manager + // pluginStore is the Manager's backing store, kept here so the one-time + // import of a legacy plugins.json can address it directly. legacyPlugins is + // the path that import reads; pluginsStop ends the background load retry. + pluginStore plugins.Store + legacyPlugins string + pluginsStop context.CancelFunc + // ocpp is the OCPP 1.6J Central System that Anker Solix chargers connect to // when their owner picks a control mode of own/proxy (see internal/ocpp and // integrations_ankersolix_control.go). Nil-safe: control endpoints report a @@ -168,41 +176,95 @@ type Server struct { // New constructs a Server around an already-built PocketBase client. func New(cfg config.Config, client *pb.Client) *Server { + // The global (L1) plugin layer lives in PocketBase alongside the org (L2) + // and user (L3) layers, rather than in a file beside the binary. + store := plugins.NewPocketBaseStore(client, colAppSettings) return &Server{ - cfg: cfg, - pb: client, - plugins: plugins.NewManager(cfg.PluginsFile), - ocpp: ocpp.NewCSMS(func(f string, a ...any) { log.Printf("ocpp: "+f, a...) }), - control: newControlIndex(), - ctlRL: newRateLimiter(30, time.Minute), // 30 control commands / min / charger + cfg: cfg, + pb: client, + pluginStore: store, + legacyPlugins: cfg.PluginsFile, + plugins: plugins.NewManager(store), + ocpp: ocpp.NewCSMS(func(f string, a ...any) { log.Printf("ocpp: "+f, a...) }), + control: newControlIndex(), + ctlRL: newRateLimiter(30, time.Minute), // 30 control commands / min / charger } } -// StartPlugins loads persisted plugin state and initialises enabled plugins. It -// also warms the OCPP control-token index from PocketBase (its source of truth), -// so the first charger to reconnect after a restart resolves immediately instead -// of triggering a lazy rebuild mid-handshake. The warm-up is best-effort and -// non-blocking; if PocketBase is not yet configured it no-ops and the lazy path -// rebuilds on first connect. +// StartPlugins reads the plugin settings from PocketBase and initialises every +// enabled plugin. It also warms the OCPP control-token index from PocketBase (its +// source of truth), so the first charger to reconnect after a restart resolves +// immediately instead of triggering a lazy rebuild mid-handshake. The warm-up is +// best-effort and non-blocking; if PocketBase is not yet configured it no-ops and +// the lazy path rebuilds on first connect. +// +// The settings now live in the database, so at boot the database may not be +// reachable yet — a cold stack, or a service account still to be configured from +// the panel. That is not fatal and, crucially, not treated as "no plugins +// configured": the Manager stays unloaded, the admin endpoints answer 503, and a +// background retry keeps trying until the read succeeds. Nothing is written +// until something has been read, so an outage cannot erase the settings. func (s *Server) StartPlugins() error { - // Surface an unwritable state directory at boot. Without this the first - // symptom is a superadmin enabling plugins, seeing them work, and finding - // them all disabled after the next redeploy — because every save failed. - if err := s.plugins.CheckWritable(); err != nil { - log.Printf("WARNING: %v", err) - log.Printf("WARNING: plugin changes will NOT survive a restart — make the directory holding PLUGINS_FILE writable by the container user") - } - err := s.plugins.Load() + ctx, cancel := context.WithCancel(context.Background()) + s.pluginsStop = cancel + go func() { - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - s.ensureControlIndex(ctx) + warmCtx, warmCancel := context.WithTimeout(context.Background(), 15*time.Second) + defer warmCancel() + s.ensureControlIndex(warmCtx) }() + + err := s.loadPlugins(ctx) + if err != nil { + log.Printf("plugins: settings unavailable, retrying in the background (%v)", err) + go s.retryLoadPlugins(ctx) + } return err } +// loadPlugins imports a pre-PocketBase plugins.json if one is still lying around +// and the database holds no settings yet, then reads the settings. +func (s *Server) loadPlugins(ctx context.Context) error { + switch migrated, err := plugins.MigrateLegacyFile(ctx, s.pluginStore, s.legacyPlugins); { + case err != nil: + // Not fatal: the read below reports the real problem if there is one. + log.Printf("plugins: legacy import skipped: %v", err) + case migrated: + log.Printf("plugins: imported %s into PocketBase; renamed it to %s.migrated", + s.legacyPlugins, s.legacyPlugins) + } + return s.plugins.Load(ctx) +} + +// retryLoadPlugins keeps reading until the settings load or the server stops. +// The backoff caps at two minutes, so a long outage costs at most one log line +// every two minutes rather than a tight spin. +func (s *Server) retryLoadPlugins(ctx context.Context) { + const maxBackoff = 2 * time.Minute + backoff := 5 * time.Second + for { + select { + case <-ctx.Done(): + return + case <-time.After(backoff): + } + if err := s.loadPlugins(ctx); err == nil { + log.Println("plugins: settings loaded") + return + } else { + log.Printf("plugins: still unavailable, retrying in %s (%v)", backoff, err) + } + if backoff < maxBackoff { + backoff *= 2 + } + } +} + // Stop releases server-held resources (plugin instances and OCPP sessions). func (s *Server) Stop(ctx context.Context) { + if s.pluginsStop != nil { + s.pluginsStop() + } s.ocpp.Shutdown(ctx) s.plugins.Shutdown(ctx) } diff --git a/API Server/internal/api/vehicleproviders_test.go b/API Server/internal/api/vehicleproviders_test.go index 639e1fd..db8e894 100644 --- a/API Server/internal/api/vehicleproviders_test.go +++ b/API Server/internal/api/vehicleproviders_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "io" "net/http" @@ -13,6 +14,7 @@ import ( "drivervault/apiserver/internal/config" "drivervault/apiserver/internal/pb" + "drivervault/apiserver/internal/plugins" ) // The vehicle-provider layer deliberately searches payloads by key name instead @@ -460,7 +462,11 @@ func newProviderTestServer(t *testing.T, fake *fakeProviderPB) *httptest.Server } s := New(config.Config{UsersCollection: "users", PluginsFile: pluginsFile}, pb.New(pbSrv.URL, "admin@test.local", "pw")) - if err := s.plugins.Load(); err != nil { + // The global layer normally lives in PocketBase; point it at the file above + // so this test does not have to stand up an app_settings collection too. + s.pluginStore = plugins.NewFileStore(pluginsFile) + s.plugins = plugins.NewManager(s.pluginStore) + if err := s.plugins.Load(context.Background()); err != nil { t.Fatalf("load plugins: %v", err) } srv := httptest.NewServer(s.Handler()) diff --git a/API Server/internal/bootstrap/schema.go b/API Server/internal/bootstrap/schema.go index 2c63425..0e79741 100644 --- a/API Server/internal/bootstrap/schema.go +++ b/API Server/internal/bootstrap/schema.go @@ -173,6 +173,16 @@ var collectionsSchema = map[string][]fieldDef{ fJSON("params", 10000), fAutodate("created", true, false), }, + // Server-wide settings as a single record, keyed "global". Today it holds + // pluginSettings: the top (L1) layer of the integration cascade — every + // plugin's enable state, its global config, and the registration of any + // external HTTP plugin. The org (L2) and user (L3) layers keep their own + // plugin config in a field of the same name below, so the global layer is + // stored the way they are instead of in a file beside the binary. + "app_settings": { + fText("key", true), + fJSON("pluginSettings", 200000), + }, // Tenants that users belong to. "organizations": { fText("name", true), @@ -213,6 +223,7 @@ var collectionsSchema = map[string][]fieldDef{ // createOrder is the dependency order for creating missing collections. // "users" is PocketBase's built-in auth collection and is never created here. var createOrder = []string{ + "app_settings", "organizations", "cars", "service_records", @@ -230,6 +241,7 @@ var createOrder = []string{ // reconcileOrder additionally includes "users" so its custom fields (role, // organization, preferences) are added to the built-in collection. var reconcileOrder = []string{ + "app_settings", "organizations", "users", "cars", @@ -247,6 +259,8 @@ var reconcileOrder = []string{ // indexes are extra SQL indexes applied at collection-create time. var indexes = map[string][]string{ + // One settings record per key, so the global singleton cannot be duplicated. + "app_settings": {"CREATE UNIQUE INDEX `idx_app_settings_key` ON `app_settings` (`key`)"}, "organizations": {"CREATE UNIQUE INDEX `idx_organizations_name` ON `organizations` (`name`)"}, "fuel_entries": {"CREATE INDEX `idx_fuel_entries_car_km` ON `fuel_entries` (`car`, `km`)"}, "charging_sessions": {"CREATE INDEX `idx_charging_sessions_car_km` ON `charging_sessions` (`car`, `km`)"}, diff --git a/API Server/internal/config/config.go b/API Server/internal/config/config.go index ba0cc47..b06f958 100644 --- a/API Server/internal/config/config.go +++ b/API Server/internal/config/config.go @@ -19,7 +19,10 @@ type Config struct { // UsersCollection is the PocketBase auth collection holding app users. UsersCollection string - // PluginsFile is the local JSON store for plugin enable-state + config. + // PluginsFile is the pre-PocketBase JSON store for the global plugin layer. + // Those settings now live in the database (the app_settings singleton), so + // this path is read once — to import an existing file on the first boot + // after the upgrade — and renamed to *.migrated afterwards. PluginsFile string // Superuser service account. Every privileged flow (user/organization diff --git a/API Server/internal/plugins/README.md b/API Server/internal/plugins/README.md index c06cb47..5e7422a 100644 --- a/API Server/internal/plugins/README.md +++ b/API Server/internal/plugins/README.md @@ -10,9 +10,9 @@ two kinds: | **external** | any HTTP service | registering a URL at runtime | **no** | third-party / less-trusted / independently deployed | Both implement the same behaviour; the server treats them identically. Enable -state and per-plugin config persist to `plugins.json` and load on boot. Every -plugin is managed by a **superadmin** from the panel (`/`) or the -`/api/admin/plugins*` API. +state and per-plugin config persist to PocketBase and load on boot. Every plugin +is managed by a **superadmin** from the panel (`/`) or the `/api/admin/plugins*` +API. --- @@ -251,8 +251,18 @@ it runs as its own process/container, an external plugin is also the ## Lifecycle, config & secrets -- **Enable/disable** and **config** persist to `plugins.json` (gitignored; override - the path with `PLUGINS_FILE`). Enabling calls `Init`; disabling calls `Shutdown`. +- **Enable/disable** and **config** persist to PocketBase: the `app_settings` + record keyed `global`, in its `pluginSettings` field. That is the top (L1) + layer of the integration cascade, stored the same way the org (L2) and user + (L3) layers are. Enabling calls `Init`; disabling calls `Shutdown`. +- **Before the settings have been read** — a cold database, or a service account + still to be configured — every `/api/admin/plugins*` endpoint answers **503** + 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`. - **Secrets** (`Secret: true` fields) are returned masked. On save, a field still equal to the mask keeps its stored value; send a new value to change it, or an empty string to clear it. @@ -285,7 +295,7 @@ All endpoints require a superadmin bearer token (`Authorization: ` from 2. `GET /api/admin/plugins` → confirm your descriptor, config fields, capabilities. 3. `PUT /api/admin/plugins/{name} {"enabled":true, "config":{…}}` → enable with config. 4. `POST /api/admin/plugins/{name}/health` → confirm the live probe classifies correctly. -5. Restart the server → confirm state reloads from `plugins.json`. +5. Restart the server → confirm state reloads from PocketBase. A Go unit test can exercise a built-in directly: diff --git a/API Server/internal/plugins/builtin/ankersolix/ankersolix_test.go b/API Server/internal/plugins/builtin/ankersolix/ankersolix_test.go index 59bc582..c726ef1 100644 --- a/API Server/internal/plugins/builtin/ankersolix/ankersolix_test.go +++ b/API Server/internal/plugins/builtin/ankersolix/ankersolix_test.go @@ -70,7 +70,7 @@ func TestDescriptor(t *testing.T) { func TestRegistered(t *testing.T) { var found bool - for _, v := range plugins.NewManager(t.TempDir() + "/plugins.json").List() { + for _, v := range plugins.NewManager(plugins.NewMemoryStore()).List() { if v.Name == "anker-solix" { found = true } diff --git a/API Server/internal/plugins/builtin/toyota/toyota_test.go b/API Server/internal/plugins/builtin/toyota/toyota_test.go index 3c2fd7b..0e2037b 100644 --- a/API Server/internal/plugins/builtin/toyota/toyota_test.go +++ b/API Server/internal/plugins/builtin/toyota/toyota_test.go @@ -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(t.TempDir()+"/plugins.json").List() { + for _, v := range plugins.NewManager(plugins.NewMemoryStore()).List() { if v.Name == "toyota" { found = true } diff --git a/API Server/internal/plugins/manager.go b/API Server/internal/plugins/manager.go index 49e6331..28b4c04 100644 --- a/API Server/internal/plugins/manager.go +++ b/API Server/internal/plugins/manager.go @@ -8,8 +8,6 @@ import ( "fmt" "log" "net/http" - "os" - "path/filepath" "sort" "strings" "sync" @@ -41,8 +39,9 @@ type View struct { // Manager owns the plugin registry, persisted state, and live instances. type Manager struct { - path string + store Store mu sync.Mutex + loaded bool // settings have been read; until then saves are refused factories map[string]Factory records map[string]*record live map[string]Plugin @@ -50,10 +49,12 @@ type Manager struct { client *http.Client } -// NewManager builds a Manager backed by the JSON state file at path. -func NewManager(path string) *Manager { +// NewManager builds a Manager over the given Store. It starts unloaded: call +// Load before serving, and keep calling it until it succeeds if the store is +// not reachable yet (see Ready). +func NewManager(store Store) *Manager { return &Manager{ - path: path, + store: store, factories: builtinFactories(), records: map[string]*record{}, live: map[string]Plugin{}, @@ -62,25 +63,47 @@ func NewManager(path string) *Manager { } } -// Load reads the state file and initialises every enabled plugin. A missing file -// is fine (no plugins configured yet). -func (m *Manager) Load() error { +// Ready reports whether the settings have been read from the store. While it is +// false the Manager knows of no configured plugins and refuses every mutation, +// so an unreachable store cannot cause settings to be overwritten or lost. +func (m *Manager) Ready() bool { + m.mu.Lock() + defer m.mu.Unlock() + return m.loaded +} + +// Load reads the settings from the store and initialises every enabled plugin. +// An absent document is fine (nothing configured yet); an unreachable store is +// not, and leaves the Manager unloaded so a caller can retry. +func (m *Manager) Load(ctx context.Context) error { + // Read outside the lock: the production store is a network call. + data, found, err := m.store.Load(ctx) + if err != nil { + return classify(err, errNotReady) + } + + recs := map[string]*record{} + if found { + if err := decodeRecords(data, recs); err != nil { + // Stay unloaded on purpose. A document we cannot parse must never be + // replaced by one built from an empty map — that would turn a read + // problem into permanent data loss. Saves refuse until it is fixed. + return fmt.Errorf("plugin settings in %s are unreadable: %w", m.store.Describe(), err) + } + } + m.mu.Lock() defer m.mu.Unlock() - data, err := os.ReadFile(m.path) - switch { - case err == nil: - if err := m.decodeLocked(data); err != nil { - return err - } - case errors.Is(err, os.ErrNotExist): - // No state file yet — first boot, nothing configured. - default: - return err + // Load is called again by the boot retry, so make it idempotent: the + // previous generation of instances must not be left running beside the new. + for name, p := range m.live { + _ = p.Shutdown(ctx) + delete(m.live, name) } + m.records = recs + m.loaded = true - ctx := context.Background() for name, rec := range m.records { if !rec.Enabled { continue @@ -99,56 +122,38 @@ func (m *Manager) Load() error { return nil } -// decodeLocked parses the state file into m.records. It is deliberately strict -// about two shapes that would otherwise take the server down or quietly destroy -// state: +// decodeRecords parses a settings document into into. It is deliberately strict +// about two shapes that would otherwise take the server down: // -// - An empty file, or a literal "null", decodes to a nil map. Assigning that -// to m.records makes the next save panic with "assignment to entry in nil -// map", so both are treated as "nothing configured" instead. -// - A null entry ({"toyota": null}) leaves a nil *record that the enable loop -// in Load would dereference. Those entries are dropped. -// -// Content that does not parse at all is moved aside rather than left in place: -// the server carries on with no plugins configured, and the next save would -// otherwise overwrite the unreadable file and take every setting in it along. -func (m *Manager) decodeLocked(data []byte) error { +// - An empty document, or a literal "null", decodes to a nil map. Assigning +// that straight to m.records made the next save panic with "assignment to +// entry in nil map"; both now mean "nothing configured". +// - A null entry ({"toyota": null}) leaves a nil *record that Load's enable +// loop would dereference. Those entries are dropped. +func decodeRecords(data []byte, into map[string]*record) error { if len(bytes.TrimSpace(data)) == 0 { return nil } var recs map[string]*record if err := json.Unmarshal(data, &recs); err != nil { - backup := m.path + ".corrupt" - if renameErr := os.Rename(m.path, backup); renameErr != nil { - return fmt.Errorf("plugin state file %s is unreadable (%v) and could not be set aside: %v", m.path, err, renameErr) - } - return fmt.Errorf("plugin state file %s was unreadable (%v); moved it to %s and started with no plugins configured", m.path, err, backup) + return err } for name, rec := range recs { if rec == nil { - delete(recs, name) + continue } - } - if recs != nil { - m.records = recs + into[name] = rec } return nil } -// CheckWritable reports whether the state file can actually be written, by -// creating and removing a temporary file beside it. Worth calling at startup: -// an unwritable state directory (a root-owned bind mount under an unprivileged -// process, or a volume left over from an image that ran as root) otherwise -// stays invisible until a restart brings every plugin back disabled. -func (m *Manager) CheckWritable() error { - f, err := os.CreateTemp(filepath.Dir(m.path), ".plugins-writecheck-*") +// encodeRecords renders the settings document written to the store. +func encodeRecords(recs map[string]*record) ([]byte, error) { + data, err := json.MarshalIndent(recs, "", " ") if err != nil { - return fmt.Errorf("%w: %v", errPersist, err) + return nil, fmt.Errorf("%w: %v", errPersist, err) } - name := f.Name() - _ = f.Close() - _ = os.Remove(name) - return nil + return append(data, '\n'), nil } // construct builds a plugin instance from a builtin factory or an external record. @@ -236,6 +241,11 @@ func (m *Manager) Get(name string) (View, bool) { func (m *Manager) Upsert(ctx context.Context, name string, enabled bool, incoming map[string]string) (View, error) { m.mu.Lock() + if !m.loaded { + m.mu.Unlock() + return View{}, errNotReady + } + _, isBuiltin := m.factories[name] rec := m.records[name] if !isBuiltin && (rec == nil || rec.Kind != KindExternal) { @@ -282,11 +292,11 @@ func (m *Manager) Upsert(ctx context.Context, name string, enabled bool, incomin prevEnabled, prevConfig := rec.Enabled, rec.Config rec.Enabled = enabled rec.Config = merged - if err := m.persistLocked(); err != nil { - // Roll back, so the panel shows what is actually on disk. Keeping the - // change in memory is what made an unwritable state file look like a - // successful save — right up until the next restart brought it back - // disabled. + if err := m.saveLocked(ctx); err != nil { + // Roll back, so the panel shows what the store actually holds. Keeping + // the change in memory is what made a failed save look like a + // successful one — right up until the next restart brought every + // plugin back disabled. if isNew { delete(m.records, name) } else { @@ -320,7 +330,7 @@ func (m *Manager) Upsert(ctx context.Context, name string, enabled bool, incomin // RegisterExternal adds a new external (remote HTTP) plugin at runtime — the // "add a plugin without a rebuild" path. It starts disabled. -func (m *Manager) RegisterExternal(name, baseURL, provider string) error { +func (m *Manager) RegisterExternal(ctx context.Context, name, baseURL, provider string) error { name = strings.TrimSpace(name) baseURL = strings.TrimRight(strings.TrimSpace(baseURL), "/") if name == "" || baseURL == "" { @@ -332,6 +342,9 @@ func (m *Manager) RegisterExternal(name, baseURL, provider string) error { m.mu.Lock() defer m.mu.Unlock() + if !m.loaded { + return errNotReady + } if _, dup := m.factories[name]; dup { return errors.New("a builtin plugin already uses that name") } @@ -339,7 +352,7 @@ func (m *Manager) RegisterExternal(name, baseURL, provider string) error { return errors.New("a plugin with that name already exists") } m.records[name] = &record{Kind: KindExternal, BaseURL: baseURL, Provider: provider} - if err := m.persistLocked(); err != nil { + if err := m.saveLocked(ctx); err != nil { delete(m.records, name) return err } @@ -350,6 +363,9 @@ func (m *Manager) RegisterExternal(name, baseURL, provider string) error { func (m *Manager) Remove(ctx context.Context, name string) error { m.mu.Lock() defer m.mu.Unlock() + if !m.loaded { + return errNotReady + } rec := m.records[name] if rec == nil || rec.Kind != KindExternal { return errors.New("only external plugins can be removed") @@ -357,7 +373,7 @@ func (m *Manager) Remove(ctx context.Context, name string) error { // Persist before tearing the instance down, so a failed write leaves a // still-registered plugin still running rather than a half-removed one. delete(m.records, name) - if err := m.persistLocked(); err != nil { + if err := m.saveLocked(ctx); err != nil { m.records[name] = rec return err } @@ -529,59 +545,48 @@ func (m *Manager) Shutdown(ctx context.Context) { } } -// persistLocked writes the state file. Caller must hold m.mu. +// saveLocked writes the current records to the store. Caller must hold m.mu. // -// The write goes to a temporary file in the same directory, is flushed, and is -// then renamed over the target. A truncating write in place can be interrupted -// (crash, container stop, full disk) and leave a half-written plugins.json that -// fails to parse on the next boot — which surfaces as every plugin coming back -// disabled. Every failure is wrapped in errPersist so callers can tell "your -// change was not saved" apart from "saved, but the plugin failed to start". -func (m *Manager) persistLocked() error { - data, err := json.MarshalIndent(m.records, "", " ") +// Every failure is classified as errPersist here rather than relying on the +// Store to have done it. That distinction drives the HTTP status: an +// unclassified save failure would fall through to the "saved, but the plugin +// failed to start" branch and be reported as a 200, which is precisely how a +// failed save used to masquerade as a successful one. +func (m *Manager) saveLocked(ctx context.Context) error { + data, err := encodeRecords(m.records) if err != nil { - return fmt.Errorf("%w: %v", errPersist, err) + return err } - data = append(data, '\n') - - tmp, err := os.CreateTemp(filepath.Dir(m.path), ".plugins-*.json") - if err != nil { - return fmt.Errorf("%w: %v", errPersist, err) + if err := m.store.Save(ctx, data); err != nil { + return classify(err, errPersist) } - 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, m.path); err != nil { - return fmt.Errorf("%w: %v", errPersist, err) - } - tmpName = "" // renamed into place; nothing left to clean up return nil } +// classify tags err with sentinel unless it already carries it, so callers can +// branch on IsPersist/IsNotReady whatever a Store returned. +func classify(err error, sentinel error) error { + if errors.Is(err, sentinel) { + return err + } + return fmt.Errorf("%w: %v", sentinel, err) +} + var ( - errUnknown = errors.New("unknown plugin") - errPersist = errors.New("plugin state could not be saved") + errUnknown = errors.New("unknown plugin") + errPersist = errors.New("plugin settings could not be saved") + errNotReady = errors.New("plugin settings are not loaded yet") ) // IsUnknown reports whether err came from addressing a plugin that doesn't exist. func IsUnknown(err error) bool { return errors.Is(err, errUnknown) } -// IsPersist reports whether err means the change never reached the state file. -// Such a change has been rolled back in memory: it must be reported as a -// failure, or the caller sees a save that silently vanishes on the next restart. +// IsPersist reports whether err means the change never reached the store. Such a +// change has been rolled back in memory: it must be reported as a failure, or +// the caller sees a save that silently vanishes on the next restart. func IsPersist(err error) bool { return errors.Is(err, errPersist) } + +// IsNotReady reports whether err means the settings have not been read yet — +// the store was unreachable at boot and is still being retried. Callers should +// answer 503 rather than present the plugin list as empty. +func IsNotReady(err error) bool { return errors.Is(err, errNotReady) } diff --git a/API Server/internal/plugins/manager_persist_test.go b/API Server/internal/plugins/manager_persist_test.go index 3f59993..1fd2c3e 100644 --- a/API Server/internal/plugins/manager_persist_test.go +++ b/API Server/internal/plugins/manager_persist_test.go @@ -3,6 +3,7 @@ package plugins import ( "context" "encoding/json" + "errors" "os" "path/filepath" "strings" @@ -28,155 +29,180 @@ func init() { } } -// 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) - } +// seeded returns a memory store already holding doc. +func seeded(doc string) *memoryStore { + return &memoryStore{data: []byte(doc), found: true} +} + +// loadedManager returns a Manager that has successfully read doc. +func loadedManager(t *testing.T, store Store) *Manager { + t.Helper() + m := NewManager(store) + if err := m.Load(context.Background()); err != nil { + t.Fatalf("Load: %v", err) + } + if !m.Ready() { + t.Fatal("manager should be ready after a successful load") + } + return m +} + +// A document holding "null" (or nothing) used to decode to a nil map, which +// made the next save panic with "assignment to entry in nil map". +func TestLoadNullDocumentDoesNotPanic(t *testing.T) { + for _, doc := range []string{"null", "", " \n", "{}"} { + m := loadedManager(t, seeded(doc)) if _, err := m.Upsert(context.Background(), "test-a", true, nil); err != nil { - t.Fatalf("Upsert after %q state file: %v", content, err) + t.Fatalf("Upsert after %q document: %v", doc, err) } if v, _ := m.Get("test-a"); !v.Enabled { - t.Fatalf("plugin not enabled after save (state file was %q)", content) + t.Fatalf("plugin not enabled after save (document was %q)", doc) } } } // 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) - } + m := loadedManager(t, seeded(`{"test-a":null,"test-b":{"enabled":true}}`)) if v, _ := m.Get("test-b"); !v.Enabled { t.Fatal("test-b should still be enabled") } + if v, _ := m.Get("test-a"); v.Enabled { + t.Fatal("test-a should not 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) +// An unreachable store must leave the Manager unloaded and refusing to write, +// so an outage cannot overwrite settings that were never read. +func TestUnreachableStoreRefusesEveryMutation(t *testing.T) { + store := &memoryStore{failLoad: errNotReady} + m := NewManager(store) + + if err := m.Load(context.Background()); !IsNotReady(err) { + t.Fatalf("expected a not-ready error, got %v", err) + } + if m.Ready() { + t.Fatal("manager must not report ready after a failed load") } - _, err := m.Upsert(context.Background(), "test-a", true, map[string]string{"k": "v"}) + ctx := context.Background() + if _, err := m.Upsert(ctx, "test-a", true, nil); !IsNotReady(err) { + t.Fatalf("Upsert should refuse while unloaded, got %v", err) + } + if err := m.RegisterExternal(ctx, "ext", "http://example.test", ""); !IsNotReady(err) { + t.Fatalf("RegisterExternal should refuse while unloaded, got %v", err) + } + if err := m.Remove(ctx, "test-a"); !IsNotReady(err) { + t.Fatalf("Remove should refuse while unloaded, got %v", err) + } + if store.data != nil { + t.Fatal("nothing may be written to a store that was never read") + } +} + +// A document that does not parse must not be replaced by an empty one — the +// read problem must stay a read problem rather than becoming data loss. +func TestUnreadableDocumentIsNeverOverwritten(t *testing.T) { + original := `{"test-a":{"enabled":true,"config":{"user":"me"}}}garbage` + store := seeded(original) + m := NewManager(store) + + err := m.Load(context.Background()) if err == nil { - t.Fatal("expected an error when the state file cannot be written") + t.Fatal("Load should report an unreadable document") + } + if m.Ready() { + t.Fatal("manager must not report ready after an unreadable document") + } + if _, err := m.Upsert(context.Background(), "test-a", true, nil); !IsNotReady(err) { + t.Fatalf("Upsert should refuse, got %v", err) + } + if string(store.data) != original { + t.Fatalf("the unreadable document was modified: %s", store.data) + } +} + +// A failed save must roll back, so callers never see a change that vanishes. +func TestUpsertRollsBackWhenSaveFails(t *testing.T) { + store := seeded(`{"test-a":{"enabled":true,"config":{"user":"first"}}}`) + m := loadedManager(t, store) + + store.failSave = errors.New("database is gone") + + _, err := m.Upsert(context.Background(), "test-a", false, map[string]string{"user": "second"}) + if err == nil { + t.Fatal("expected an error when the store 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) - } +// A newly created record must not survive a save that failed. +func TestUpsertRollsBackNewRecord(t *testing.T) { + store := seeded(`{}`) + m := loadedManager(t, store) + store.failSave = errors.New("database is gone") - m := NewManager(path) - err := m.Load() - if err == nil { - t.Fatal("Load should report an unreadable state file") + if _, err := m.Upsert(context.Background(), "test-a", true, nil); !IsPersist(err) { + t.Fatalf("expected persist failure, got %v", err) } - 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") + if v, _ := m.Get("test-a"); v.Enabled { + t.Fatal("a record from a failed save is still present") } } -// Enable/disable state must survive a restart — the whole point of the file. +// Enable/disable state must survive a restart — the whole point of the store. func TestStateSurvivesReload(t *testing.T) { - path := filepath.Join(t.TempDir(), "plugins.json") - - m := NewManager(path) - if err := m.Load(); err != nil { - t.Fatal(err) - } + store := seeded(`{}`) + m := loadedManager(t, store) 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) - } + // Restart against the same store. + m2 := loadedManager(t, store) 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) +// External plugins are registered and removed through the same store. +func TestExternalPluginRoundTrip(t *testing.T) { + store := seeded(`{}`) + m := loadedManager(t, store) + ctx := context.Background() + + if err := m.RegisterExternal(ctx, "ext", "example.test/api", "ACME"); err != nil { + t.Fatalf("RegisterExternal: %v", err) } + m2 := loadedManager(t, store) + if v, ok := m2.Get("ext"); !ok || v.BaseURL != "http://example.test/api" { + t.Fatalf("external plugin did not persist: ok=%v baseURL=%q", ok, v.BaseURL) + } + + if err := m2.Remove(ctx, "ext"); err != nil { + t.Fatalf("Remove: %v", err) + } + m3 := loadedManager(t, store) + if _, ok := m3.Get("ext"); ok { + 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) @@ -193,17 +219,119 @@ func TestPersistLeavesNoTempFiles(t *testing.T) { } } -// 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) +// 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) } - 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) + 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) +} diff --git a/API Server/internal/plugins/store.go b/API Server/internal/plugins/store.go new file mode 100644 index 0000000..b826549 --- /dev/null +++ b/API Server/internal/plugins/store.go @@ -0,0 +1,329 @@ +package plugins + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/url" + "os" + "path/filepath" + "sync" + + "drivervault/apiserver/internal/pb" +) + +// 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. +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. + // + // 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 + // save, so a transient outage cannot overwrite settings it never read. + Load(ctx context.Context) (data []byte, found bool, err error) + + // Save replaces the stored document. + Save(ctx context.Context, data []byte) error + + // Describe names the store for log and error messages. + Describe() string +} + +// PluginSettingsField is the record field the document lives in. The org (L2) +// and user (L3) layers of the cascade keep their plugin config in a field of +// this name too (see internal/api/integrations.go), so the global layer is +// stored the same way they are rather than being a special case. +// +// The documents are not identical in shape: L2/L3 hold per-plugin user config +// only, while L1 additionally carries the enable state and the registration of +// external plugins. Same mechanism and same field, different payload. +const PluginSettingsField = "pluginSettings" + +// globalSettingsKey identifies the singleton record holding the global layer. +// It is a fixed constant, never caller input. +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. +type pbStore struct { + client *pb.Client + collection string + + mu sync.Mutex + id string // cached id of the singleton record, resolved on first Load/Save +} + +// NewPocketBaseStore returns a Store backed by the singleton settings record in +// the given collection. +func NewPocketBaseStore(client *pb.Client, collection string) Store { + return &pbStore{client: client, collection: collection} +} + +func (s *pbStore) Describe() string { + return "PocketBase " + s.collection + " (key=" + globalSettingsKey + ")" +} + +// settingsRecord is the slice of the singleton record this store reads. +type settingsRecord struct { + ID string `json:"id"` + PluginSettings json.RawMessage `json:"pluginSettings"` +} + +func (s *pbStore) Load(ctx context.Context) ([]byte, bool, error) { + if !s.client.Configured() { + return nil, false, fmt.Errorf("%w: no PocketBase service account configured", errNotReady) + } + rec, found, err := s.find(ctx) + if err != nil { + return nil, false, err + } + if !found { + return nil, false, nil + } + s.mu.Lock() + s.id = rec.ID + s.mu.Unlock() + return rec.PluginSettings, true, nil +} + +// find resolves the singleton record. A missing record is not an error. +func (s *pbStore) find(ctx context.Context) (settingsRecord, bool, error) { + q := url.Values{} + q.Set("filter", "key='"+globalSettingsKey+"'") + q.Set("perPage", "1") + + res, err := s.client.List(ctx, s.collection, q) + if err != nil { + // A missing collection means bootstrap has not run yet — still "not + // ready" rather than "no settings", so nothing gets overwritten. + return settingsRecord{}, false, fmt.Errorf("%w: %v", errNotReady, err) + } + var items []settingsRecord + if err := json.Unmarshal(res.Items, &items); err != nil { + return settingsRecord{}, false, fmt.Errorf("%w: %v", errNotReady, err) + } + if len(items) == 0 { + return settingsRecord{}, false, nil + } + return items[0], true, nil +} + +func (s *pbStore) Save(ctx context.Context, data []byte) error { + if !s.client.Configured() { + return fmt.Errorf("%w: no PocketBase service account configured", errPersist) + } + + s.mu.Lock() + id := s.id + s.mu.Unlock() + + // Resolve the record id if this process has not seen it yet (or if the + // record was recreated behind our back). + if id == "" { + rec, found, err := s.find(ctx) + if err != nil { + return fmt.Errorf("%w: %v", errPersist, err) + } + if found { + id = rec.ID + } + } + + payload := map[string]any{ + "key": globalSettingsKey, + PluginSettingsField: json.RawMessage(data), + } + + if id != "" { + if err := s.client.Update(ctx, s.collection, id, payload, nil); err != nil { + if !isNotFound(err) { + return fmt.Errorf("%w: %v", errPersist, err) + } + // The record was deleted since we resolved it; fall through and + // recreate it rather than losing the save. + s.mu.Lock() + s.id = "" + s.mu.Unlock() + id = "" + } else { + s.mu.Lock() + s.id = id + s.mu.Unlock() + return nil + } + } + + var created settingsRecord + if err := s.client.Create(ctx, s.collection, payload, &created); err != nil { + return fmt.Errorf("%w: %v", errPersist, err) + } + s.mu.Lock() + s.id = created.ID + s.mu.Unlock() + return nil +} + +// isNotFound reports whether err is a PocketBase 404. +func isNotFound(err error) bool { + var apiErr *pb.APIError + 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 +// the not-ready and failed-save paths. +type memoryStore struct { + mu sync.Mutex + data []byte + found bool + failLoad error + failSave error +} + +// NewMemoryStore returns a Store that keeps the document in memory. +func NewMemoryStore() Store { return &memoryStore{} } + +func (s *memoryStore) Describe() string { return "memory" } + +func (s *memoryStore) Load(ctx context.Context) ([]byte, bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.failLoad != nil { + return nil, false, s.failLoad + } + return s.data, s.found, nil +} + +func (s *memoryStore) Save(ctx context.Context, data []byte) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.failSave != nil { + return s.failSave + } + s.data = append([]byte(nil), data...) + 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 +// .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 +} diff --git a/API Server/internal/plugins/store_pb_test.go b/API Server/internal/plugins/store_pb_test.go new file mode 100644 index 0000000..05bf2f2 --- /dev/null +++ b/API Server/internal/plugins/store_pb_test.go @@ -0,0 +1,236 @@ +package plugins + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "drivervault/apiserver/internal/pb" +) + +// fakePB is the slice of PocketBase the settings store touches: superuser auth, +// and list/create/update on one collection. It holds a single record so the +// singleton behaviour can be asserted directly. +type fakePB struct { + mu sync.Mutex + + recordID string // "" while no record exists + settings json.RawMessage // the stored pluginSettings value + listErr bool // make list calls fail (database unreachable) + creates int + updates int + missingCol bool // answer list with a 404, as if bootstrap had not run +} + +func (f *fakePB) server(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + + switch { + case strings.HasSuffix(r.URL.Path, "/auth-with-password"): + writeTestJSON(w, http.StatusOK, map[string]any{"token": "test-token"}) + + case r.Method == http.MethodGet && r.URL.Path == "/api/collections/app_settings/records": + if f.listErr { + http.Error(w, "connection refused", http.StatusBadGateway) + return + } + if f.missingCol { + http.Error(w, `{"message":"Missing collection context."}`, http.StatusNotFound) + return + } + items := []any{} + if f.recordID != "" { + items = append(items, map[string]any{ + "id": f.recordID, + "key": globalSettingsKey, + "pluginSettings": f.settings, + }) + } + writeTestJSON(w, http.StatusOK, map[string]any{ + "page": 1, "perPage": 1, "totalItems": len(items), "totalPages": 1, + "items": items, + }) + + case r.Method == http.MethodPost && r.URL.Path == "/api/collections/app_settings/records": + var body struct { + Key string `json:"key"` + PluginSettings json.RawMessage `json:"pluginSettings"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + if body.Key != globalSettingsKey { + t.Errorf("create used key %q, want %q", body.Key, globalSettingsKey) + } + f.creates++ + f.recordID = "rec1" + f.settings = body.PluginSettings + writeTestJSON(w, http.StatusOK, map[string]any{"id": f.recordID}) + + case r.Method == http.MethodPatch && + strings.HasPrefix(r.URL.Path, "/api/collections/app_settings/records/"): + id := strings.TrimPrefix(r.URL.Path, "/api/collections/app_settings/records/") + if id != f.recordID { + http.Error(w, `{"message":"not found"}`, http.StatusNotFound) + return + } + var body struct { + PluginSettings json.RawMessage `json:"pluginSettings"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + f.updates++ + f.settings = body.PluginSettings + writeTestJSON(w, http.StatusOK, map[string]any{"id": id}) + + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + http.Error(w, "unexpected", http.StatusInternalServerError) + } + })) + t.Cleanup(srv.Close) + return srv +} + +func writeTestJSON(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(v) +} + +func newPBStore(t *testing.T, f *fakePB) Store { + t.Helper() + srv := f.server(t) + return NewPocketBaseStore(pb.New(srv.URL, "admin@test.local", "pw"), "app_settings") +} + +// The first save creates the singleton; later saves update it in place. +func TestPBStoreCreatesThenUpdatesOneRecord(t *testing.T) { + f := &fakePB{} + m := loadedManager(t, newPBStore(t, f)) + ctx := context.Background() + + if _, err := m.Upsert(ctx, "test-a", true, map[string]string{"user": "me"}); err != nil { + t.Fatalf("first save: %v", err) + } + if _, err := m.Upsert(ctx, "test-b", true, nil); err != nil { + t.Fatalf("second save: %v", err) + } + + f.mu.Lock() + creates, updates := f.creates, f.updates + f.mu.Unlock() + if creates != 1 { + t.Fatalf("expected exactly one record to be created, got %d", creates) + } + if updates != 1 { + t.Fatalf("expected the second save to update, got %d updates", updates) + } +} + +// Settings written through PocketBase must come back on the next boot. +func TestPBStoreRoundTrip(t *testing.T) { + f := &fakePB{} + store := newPBStore(t, f) + m := loadedManager(t, store) + + if _, err := m.Upsert(context.Background(), "test-a", true, map[string]string{"user": "me"}); err != nil { + t.Fatal(err) + } + + // A fresh Manager on a fresh store, as after a restart. + m2 := loadedManager(t, newPBStore(t, f)) + v, ok := m2.Get("test-a") + if !ok || !v.Enabled || v.Config["user"] != "me" { + t.Fatalf("settings did not survive: ok=%v enabled=%v config=%v", ok, v.Enabled, v.Config) + } +} + +// A fresh install (no record yet) loads cleanly as "nothing configured". +func TestPBStoreEmptyIsNotAnError(t *testing.T) { + m := loadedManager(t, newPBStore(t, &fakePB{})) + for _, v := range m.List() { + if v.Enabled { + t.Fatalf("%s should not be enabled on a fresh install", v.Name) + } + } +} + +// An unreachable database must not read as "no plugins configured", and must +// not let anything be written over settings that were never read. +func TestPBStoreUnreachableStaysUnloaded(t *testing.T) { + f := &fakePB{recordID: "rec1", settings: json.RawMessage(`{"test-a":{"enabled":true}}`), listErr: true} + store := newPBStore(t, f) + + m := NewManager(store) + err := m.Load(context.Background()) + if !IsNotReady(err) { + t.Fatalf("expected not-ready, got %v", err) + } + if m.Ready() { + t.Fatal("manager must not be ready") + } + if _, err := m.Upsert(context.Background(), "test-a", false, nil); !IsNotReady(err) { + t.Fatalf("Upsert should refuse, got %v", err) + } + + // The database comes back; the real settings load and are intact. + f.mu.Lock() + f.listErr = false + f.mu.Unlock() + + if err := m.Load(context.Background()); err != nil { + t.Fatalf("reload after recovery: %v", err) + } + if v, _ := m.Get("test-a"); !v.Enabled { + t.Fatal("settings lost across the outage") + } +} + +// A missing collection (bootstrap has not run) is "not ready", never "empty" — +// otherwise the first save would write a fresh document over nothing. +func TestPBStoreMissingCollectionIsNotReady(t *testing.T) { + m := NewManager(newPBStore(t, &fakePB{missingCol: true})) + if err := m.Load(context.Background()); !IsNotReady(err) { + t.Fatalf("expected not-ready for a missing collection, got %v", err) + } + if m.Ready() { + t.Fatal("manager must not be ready without its collection") + } +} + +// 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) + } +} diff --git a/API Server/scripts/setup-pocketbase.mjs b/API Server/scripts/setup-pocketbase.mjs index d930046..2c3e30e 100644 --- a/API Server/scripts/setup-pocketbase.mjs +++ b/API Server/scripts/setup-pocketbase.mjs @@ -438,6 +438,15 @@ const DESIRED = { F.json("params", 10000), F.autodate("created", true, false), ], + // Server-wide settings as a single record, keyed "global". Today it holds + // pluginSettings: the top (L1) layer of the integration cascade — every + // plugin's enable state, its global config, and the registration of any + // external HTTP plugin. The org and user layers below keep their own plugin + // config in a field of the same name. See internal/plugins/store.go. + app_settings: [ + F.text("key", true), + F.json("pluginSettings", 200000), + ], // Tenants that users belong to. A superadmin spans all of them; an admin // manages only their own. organizations: [ @@ -492,6 +501,8 @@ const DESIRED = { // Extra SQL indexes, applied at collection-create time. Organization names are // unique so the API can rely on PocketBase rejecting a duplicate. const INDEXES = { + // One settings record per key, so the global singleton cannot be duplicated. + app_settings: ["CREATE UNIQUE INDEX `idx_app_settings_key` ON `app_settings` (`key`)"], organizations: ["CREATE UNIQUE INDEX `idx_organizations_name` ON `organizations` (`name`)"], // Every read of these is "…for this car", and the fuel history is walked in // odometer order to build its efficiency windows. @@ -525,6 +536,7 @@ async function main() { // before its relations; "users" already exists as PocketBase's built-in auth // collection, so it's never created here — only reconciled below). for (const name of [ + "app_settings", "organizations", "cars", "service_records", @@ -550,6 +562,7 @@ async function main() { // and select values — this is what grows users.role to include "superadmin" // and adds users.organization on an existing deployment). for (const name of [ + "app_settings", "organizations", "users", "cars", @@ -568,7 +581,8 @@ async function main() { } console.log( - "\nDone. Collections ready: organizations, users, cars, service_records,\n" + + "\nDone. Collections ready: app_settings, organizations, users, cars,\n" + + "service_records,\n" + "technical_checks, parts, car_shares, fuel_entries, charging_sessions,\n" + "maintenance_entries, car_documents, reminders, control_audit.", ); diff --git a/Docker-AIO/.env.prod.example b/Docker-AIO/.env.prod.example index 8e1e2e9..4ac8b43 100644 --- a/Docker-AIO/.env.prod.example +++ b/Docker-AIO/.env.prod.example @@ -44,7 +44,8 @@ API_PORT=8080 # Defaults are Docker-managed named volumes. Set either to an absolute host path # for a bind mount, e.g. PB_DATA=/srv/drivervault/pb_data. # PB_DATA — the PocketBase database and uploads. -# API_DATA — the API Server's plugins.json and the .env the panel writes back +# API_DATA — the .env the API Server panel writes back (plugin settings are in +# the database, under PB_DATA) # when a superadmin retargets the PocketBase connection. PB_DATA=pb_data API_DATA=api_data diff --git a/Docker-AIO/Dockerfile b/Docker-AIO/Dockerfile index 301bad8..103d5e5 100644 --- a/Docker-AIO/Dockerfile +++ b/Docker-AIO/Dockerfile @@ -163,8 +163,9 @@ stderr_logfile=/dev/stderr stderr_logfile_maxbytes=0 ; API Server: wait for PocketBase to be healthy, then start. It runs from /data -; because it writes plugins.json and the panel .env relative to its working -; directory, and /data is the volume that keeps them across container recreates. +; because it writes the panel .env relative to its working directory, and /data +; is the volume that keeps it across container recreates. (Plugin settings live +; in PocketBase, under /pb/pb_data.) [program:api-server] directory=/data user=app @@ -225,7 +226,7 @@ ENV API_ADDR=:8080 \ # base, or set OCPP_REQUIRE_TLS=false on a trusted network. # Pass them with `docker run -e ...`. -# pb_data holds the database; /data holds the API Server plugins.json and the +# pb_data holds the database — including the plugin settings; /data holds the # .env the panel rewrites when a superadmin retargets PocketBase. Both are # pre-created and owned by app so a fresh named volume inherits that ownership. RUN mkdir -p /pb/pb_data /data && chown -R app:app /pb /data diff --git a/Docker-AIO/README.md b/Docker-AIO/README.md index 7353a37..7e39384 100644 --- a/Docker-AIO/README.md +++ b/Docker-AIO/README.md @@ -70,7 +70,7 @@ Identical to the multi-container stack, and idempotent: | Volume | Holds | |---|---| | `/pb/pb_data` | the PocketBase SQLite database and uploaded files | -| `/data` | the API Server's `plugins.json`, and the `.env` the panel rewrites when a superadmin retargets the PocketBase connection | +| `/data` | the `.env` the API Server panel rewrites when a superadmin retargets the PocketBase connection (plugin settings live in `/pb/pb_data`, with everything else) | Both default to Docker-managed named volumes; set `PB_DATA` / `API_DATA` to absolute host paths in the prod file for bind mounts. diff --git a/Docker-AIO/docker-compose.prod.yml b/Docker-AIO/docker-compose.prod.yml index 4e6d6ce..8adc035 100644 --- a/Docker-AIO/docker-compose.prod.yml +++ b/Docker-AIO/docker-compose.prod.yml @@ -46,7 +46,7 @@ services: # Named volumes by default; set PB_DATA / API_DATA to host paths in .env # for bind mounts. - "${PB_DATA:-pb_data}:/pb/pb_data" - # plugins.json + the .env the panel writes back. + # The .env the panel writes back; plugin settings live in the database. - "${API_DATA:-api_data}:/data" healthcheck: # All three processes must answer. Declared here as well as in the image so diff --git a/Docker-AIO/docker-compose.yml b/Docker-AIO/docker-compose.yml index d74f7cb..af1bee9 100644 --- a/Docker-AIO/docker-compose.yml +++ b/Docker-AIO/docker-compose.yml @@ -47,7 +47,7 @@ services: - "${API_PORT:-8080}:8080" # API Server + panel (root /) + /ocpp/{serial} volumes: - pb_data:/pb/pb_data - # plugins.json + the .env the panel writes back. + # The .env the panel writes back; plugin settings live in the database. - api_data:/data healthcheck: # All three processes must answer. Declared here as well as in the image so diff --git a/Docker/.env.prod.example b/Docker/.env.prod.example index f4afce5..366e94e 100644 --- a/Docker/.env.prod.example +++ b/Docker/.env.prod.example @@ -58,7 +58,8 @@ API_BIND=127.0.0.1 # Defaults are Docker-managed named volumes. To store either on a host path # instead, set it to an absolute path, e.g. PB_DATA=/srv/drivervault/pb_data. # PB_DATA — the PocketBase database and uploads. -# API_DATA — the API Server's plugins.json and the .env the panel writes back +# API_DATA — the .env the API Server panel writes back (plugin settings are in +# the database, under PB_DATA) # when a superadmin retargets the PocketBase connection. PB_DATA=pb_data API_DATA=api_data diff --git a/Docker/README.md b/Docker/README.md index 2645985..a8b4c08 100644 --- a/Docker/README.md +++ b/Docker/README.md @@ -58,7 +58,7 @@ schema reconcile itself. | Volume | Holds | |---|---| | `pb_data` | the PocketBase SQLite database and uploaded files | -| `api_data` | the API Server's `plugins.json`, and the `.env` the panel rewrites when a superadmin retargets the PocketBase connection | +| `api_data` | the `.env` the API Server panel rewrites when a superadmin retargets the PocketBase connection (plugin settings live in `pb_data`, with everything else) | Both default to Docker-managed named volumes. In the prod file, set `PB_DATA` / `API_DATA` to absolute host paths for bind mounts instead. @@ -70,14 +70,16 @@ Both default to Docker-managed named volumes. In the prod file, set `PB_DATA` / > neither does a **named volume** left over from an image that ran as root. The > one case it cannot fix is a container forced to another user (`user:` in > compose, `docker run --user`), where the entrypoint has no privileges to -> `chown` with — prepare the host directory yourself there. -> -> If `/data` is still unwritable the server says so at boot, with -> `WARNING: plugin changes will NOT survive a restart`. That warning is worth -> watching for: the alternative symptom is plugins that enable normally in the -> panel and come back disabled after the next redeploy. PocketBase runs as +> `chown` with — prepare the host directory yourself there. PocketBase runs as > root, so `PB_DATA` is unaffected either way. +Plugin enable-state and global config used to live in a `plugins.json` on this +volume, which made them the one piece of configuration a lost volume could erase +without anyone noticing. They are now in PocketBase, backed up with `pb_data` +like everything else. An existing `plugins.json` is imported automatically on the +first boot after the upgrade and renamed to `plugins.json.migrated`; keep the +volume mounted for that boot. + ## Charger control (OCPP) Chargers in own/proxy mode dial in to `/ocpp/{serial}` **on the API Server diff --git a/Docker/docker-compose.prod.yml b/Docker/docker-compose.prod.yml index e45745e..b37fa1e 100644 --- a/Docker/docker-compose.prod.yml +++ b/Docker/docker-compose.prod.yml @@ -77,8 +77,9 @@ services: # /ocpp/{serial} endpoint chargers dial into — on the network. - "${API_BIND:-127.0.0.1}:${API_PORT:-8080}:8080" volumes: - # plugins.json + the .env the panel writes back — see the API Server - # Dockerfile. Without this, plugin state is lost on container recreate. + # The .env the panel writes back when a superadmin retargets PocketBase — + # see the API Server Dockerfile. Plugin settings live in the database, so + # they no longer depend on this volume. - "${API_DATA:-api_data}:/data" healthcheck: # Declared here rather than relying only on the image's HEALTHCHECK, so the diff --git a/Docker/docker-compose.yml b/Docker/docker-compose.yml index b0595e1..1087162 100644 --- a/Docker/docker-compose.yml +++ b/Docker/docker-compose.yml @@ -68,9 +68,9 @@ services: # dialling /ocpp/{serial} also arrive here. - "${API_PORT:-8080}:8080" volumes: - # plugins.json + the .env the panel writes back — see the API Server - # Dockerfile. Without this, plugin state is lost when the container is - # recreated. + # The .env the panel writes back when a superadmin retargets PocketBase — + # see the API Server Dockerfile. Plugin settings live in the database, so + # they no longer depend on this volume. - api_data:/data healthcheck: # Declared here rather than relying only on the image's HEALTHCHECK, so the