driver-vault/DriverVault
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Plugins: the global layer moves into the database, beside the other two

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
tajniak81
2026-08-21 16:52:47 +02:00
co-authored by Claude Opus 5
parent d2803bbd93
commit 9bd5c523c4
28 changed files with 1161 additions and 299 deletions
Download Patch File Download Diff File Expand all files Collapse all files
API Server/internal/plugins/manager.go
+109 -104
View File
@@ -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) }