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