mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-09 17:18:45 +00:00
feat(cli): add a 'navidrome plugin' CLI for managing and inspecting plugins (#5682)
Some checks are pending
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (Windows) (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Some checks are pending
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (Windows) (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
* feat(plugins): export ReadPackageManifest and ValidatePackage for off-disk inspection * test(plugins): cover ValidatePackage cross-field validation branch * feat(cmd): add 'plugin list' command * refactor(cmd): drop premature pluginManager interface until first use * feat(cmd): add 'plugin enable' and 'plugin disable' commands * feat(cmd): add 'plugin edit' for config and permission updates * test(cmd): cover plugin edit aborting on config validation failure * feat(cmd): add 'plugin info' and 'plugin validate' (installed id or .ndp file) * test(cmd): unit-test formatManifestInfo nil-guard and json branches * feat(cmd): add 'plugin rescan' command * feat(cmd): enrich plugin info text output and re-validate config on validate * fix(cmd): omit zero-value timestamps in plugin info text output * fix(cmd): give plugin list and info separate format flag vars A shared package-global bound to both commands' --format flag caused the later init() registration (info, default "text") to clobber the earlier one (list, default "table"), so 'plugin list' with no -f failed with 'invalid output format "text"'. Split into pluginListFormat / pluginInfoFormat and add a regression test on the registered defaults. * fix(plugins): address code-review findings on plugin CLI - edit: read-modify-write merge so flipping one permission flag no longer wipes unspecified fields (matches the native API); reject non-JSON --users/--libraries values. - info: return an error on unknown --format (was silently text); include sha256 in off-disk JSON output; align the text columns. - move declaredPermissions onto plugins.Permissions.DeclaredNames() as the single source of truth; drop ValidatePackage's redundant Validate call. - readConfigFile: pass ctx to log.Fatal; copy flag globals before taking their address; trim comments that restated the code. * refactor(plugins): export ReadManifest/ComputeFileSHA256, drop thin CLI wrappers Remove the ReadPackageManifest/ComputeFileSHA256/ValidatePackage wrappers in favor of exporting the underlying readManifest->ReadManifest and computeFileSHA256->ComputeFileSHA256 directly. ValidatePackage was identical to ReadPackageManifest (readManifest already validates via ParseManifest), so the CLI's validate path now calls ReadManifest too. * test(plugins): consolidate ReadManifest specs and move ComputeFileSHA256 tests Merge the two ReadManifest Describe blocks into one, and move the ComputeFileSHA256 tests out of package_test.go into a new manager_sync_test.go (where the function now lives), deduping the overlapping hash specs. * test(plugins): use createTestPackage everywhere, drop writeNDP helper The schema-invalid and cross-field ReadManifest specs now build packages with createTestPackage (typed Manifest) instead of the raw-JSON writeNDP helper, so there is a single package-building helper. Also trims the gratuitous 1MB wasm buffer in the read-only spec to a few bytes. * test(plugins): drop duplicate missing-manifest spec from ReadManifest The openPackage block already covers the missing-manifest.json error path (shared zip-walk code); ReadManifest's identical copy added no distinct coverage. * test(plugins): fix misleading ReadManifest spec name and drop unused wasm bytes The spec was named 'should read only the manifest without loading wasm' but ReadManifest returns only (*Manifest, error) — it cannot assert whether wasm was loaded. Rename to what it actually verifies (manifest parses from a package that also contains a wasm entry) and pass nil wasm bytes, since the contents are never observed. * fix(cmd): address PR review feedback on plugin CLI - edit --users/--libraries now accept both comma-separated and JSON-array input (CSV is converted to the JSON the manager stores); help text documents both. - Permissions.DeclaredNames() derives names by reflecting over the generated struct's json tags instead of a hand-maintained list, so new permission types are picked up automatically. - isPackagePath checks only the .ndp suffix, so a mistyped package path yields a precise 'no such file' error instead of falling back to a misleading 'Plugin not found'. - Restore a ReadManifest test for the missing-manifest.json error path (it has its own branch, separate from openPackage). * docs(cmd): trim verbose comments to the why * fix(cmd): setting an explicit users/libraries list clears the all-* flag Per Codex review: with allUsers/allLibraries true, 'plugin edit --users <list>' preserved the all-* flag so the allow-list was silently ignored, and the mutually-exclusive flags meant it couldn't be corrected in one command. An explicit list now implies all-*=false.
This commit is contained in:
parent
a1412ef1c2
commit
5a3ac80a8a
11 changed files with 1046 additions and 43 deletions
|
|
@ -382,7 +382,7 @@ func (m *Manager) ValidatePluginConfig(ctx context.Context, id, configJSON strin
|
|||
return fmt.Errorf("getting plugin from DB: %w", err)
|
||||
}
|
||||
|
||||
manifest, err := readManifest(plugin.Path)
|
||||
manifest, err := ReadManifest(plugin.Path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("reading manifest: %w", err)
|
||||
}
|
||||
|
|
@ -460,7 +460,7 @@ func (m *Manager) updatePluginSettings(ctx context.Context, id string, updateFn
|
|||
shouldDisable := false
|
||||
disableReason := ""
|
||||
if wasEnabled {
|
||||
manifest, err := readManifest(plugin.Path)
|
||||
manifest, err := ReadManifest(plugin.Path)
|
||||
if err == nil && manifest.Permissions != nil {
|
||||
if manifest.Permissions.Users != nil && !hasValidUsersConfig(plugin.Users, plugin.AllUsers) {
|
||||
shouldDisable = true
|
||||
|
|
@ -591,7 +591,7 @@ func (m *Manager) UnloadDisabledPlugins(ctx context.Context) {
|
|||
// before a plugin can be enabled. Returns an error if any gate condition fails.
|
||||
func (m *Manager) checkPermissionGates(p *model.Plugin) error {
|
||||
// Parse manifest to check permissions
|
||||
manifest, err := readManifest(p.Path)
|
||||
manifest, err := ReadManifest(p.Path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("reading manifest: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -155,12 +155,12 @@ func (m *Manager) extractManifest(ndpPath string) (*PluginMetadata, error) {
|
|||
return nil, fmt.Errorf("manager is stopped")
|
||||
}
|
||||
|
||||
manifest, err := readManifest(ndpPath)
|
||||
manifest, err := ReadManifest(ndpPath)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
sha256Hash, err := computeFileSHA256(ndpPath)
|
||||
sha256Hash, err := ComputeFileSHA256(ndpPath)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("computing hash: %w", err)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,9 +36,9 @@ func marshalManifest(m *Manifest) string {
|
|||
return string(b)
|
||||
}
|
||||
|
||||
// computeFileSHA256 computes the SHA-256 hash of a file without loading it into memory.
|
||||
// ComputeFileSHA256 computes the SHA-256 hash of a file without loading it into memory.
|
||||
// This is used for quick change detection before full plugin compilation.
|
||||
func computeFileSHA256(path string) (string, error) {
|
||||
func ComputeFileSHA256(path string) (string, error) {
|
||||
f, err := os.Open(path)
|
||||
if err != nil {
|
||||
return "", err
|
||||
|
|
@ -165,7 +165,7 @@ func (m *Manager) syncPlugins(ctx context.Context, folder string) error {
|
|||
dbPlugin, exists := pluginsInDB[name]
|
||||
|
||||
// Compute SHA256 first (lightweight operation) to check if plugin changed
|
||||
sha256Hash, err := computeFileSHA256(path)
|
||||
sha256Hash, err := ComputeFileSHA256(path)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Failed to compute SHA256 for plugin", "plugin", name, "path", path, err)
|
||||
continue
|
||||
|
|
|
|||
30
plugins/manager_sync_test.go
Normal file
30
plugins/manager_sync_test.go
Normal file
|
|
@ -0,0 +1,30 @@
|
|||
package plugins
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("ComputeFileSHA256", func() {
|
||||
It("returns a consistent 64-char lowercase hex hash for the same file", func() {
|
||||
dir := GinkgoT().TempDir()
|
||||
ndpPath := filepath.Join(dir, "test.ndp")
|
||||
err := createTestPackage(ndpPath, &Manifest{Name: "S", Author: "a", Version: "1.0.0"}, []byte{0x00, 0x61, 0x73, 0x6d})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
hash1, err := ComputeFileSHA256(ndpPath)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
hash2, err := ComputeFileSHA256(ndpPath)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
Expect(hash1).To(Equal(hash2))
|
||||
Expect(hash1).To(MatchRegexp(`^[0-9a-f]{64}$`))
|
||||
})
|
||||
|
||||
It("returns an error for a non-existent path", func() {
|
||||
_, err := ComputeFileSHA256(filepath.Join(GinkgoT().TempDir(), "does-not-exist.ndp"))
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
})
|
||||
|
|
@ -158,7 +158,7 @@ func (m *Manager) processPluginEvent(pluginName string) {
|
|||
switch action {
|
||||
case actionUpdate:
|
||||
// File changed - check SHA256 first, then extract manifest if needed
|
||||
sha256Hash, err := computeFileSHA256(ndpPath)
|
||||
sha256Hash, err := ComputeFileSHA256(ndpPath)
|
||||
if err != nil {
|
||||
log.Error(m.ctx, "Failed to compute SHA256 for changed plugin", "plugin", pluginName, err)
|
||||
return
|
||||
|
|
|
|||
|
|
@ -3,10 +3,37 @@ package plugins
|
|||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/santhosh-tekuri/jsonschema/v6"
|
||||
)
|
||||
|
||||
// DeclaredNames returns the sorted names of the non-nil permission fields. It
|
||||
// reflects over the generated json tags so new permission types are picked up
|
||||
// automatically rather than via a hand-maintained list.
|
||||
func (p *Permissions) DeclaredNames() []string {
|
||||
if p == nil {
|
||||
return nil
|
||||
}
|
||||
var names []string
|
||||
v := reflect.ValueOf(*p)
|
||||
t := v.Type()
|
||||
for i := 0; i < t.NumField(); i++ {
|
||||
f := v.Field(i)
|
||||
if f.Kind() != reflect.Pointer || f.IsNil() {
|
||||
continue
|
||||
}
|
||||
tag := t.Field(i).Tag.Get("json")
|
||||
if name, _, _ := strings.Cut(tag, ","); name != "" && name != "-" {
|
||||
names = append(names, name)
|
||||
}
|
||||
}
|
||||
sort.Strings(names)
|
||||
return names
|
||||
}
|
||||
|
||||
//go:generate go tool go-jsonschema -p plugins --struct-name-from-title -o manifest_gen.go manifest-schema.json
|
||||
|
||||
// ParseManifest unmarshals manifest JSON and performs cross-field validation.
|
||||
|
|
|
|||
|
|
@ -464,3 +464,27 @@ var _ = Describe("Manifest", func() {
|
|||
})
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("Permissions.DeclaredNames", func() {
|
||||
It("returns nil for a nil receiver", func() {
|
||||
var p *Permissions
|
||||
Expect(p.DeclaredNames()).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("returns declared names sorted", func() {
|
||||
p := &Permissions{
|
||||
Subsonicapi: &SubsonicAPIPermission{},
|
||||
Users: &UsersPermission{},
|
||||
}
|
||||
Expect(p.DeclaredNames()).To(Equal([]string{"subsonicapi", "users"}))
|
||||
})
|
||||
|
||||
It("returns all declared names sorted regardless of field order", func() {
|
||||
p := &Permissions{
|
||||
Http: &HTTPPermission{},
|
||||
Artwork: &ArtworkPermission{},
|
||||
Cache: &CachePermission{},
|
||||
}
|
||||
Expect(p.DeclaredNames()).To(Equal([]string{"artwork", "cache", "http"}))
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -72,9 +72,10 @@ func openPackage(ndpPath string) (*ndpPackage, error) {
|
|||
}, nil
|
||||
}
|
||||
|
||||
// readManifest reads only the manifest from an .ndp file without loading the wasm bytes.
|
||||
// This is useful for quick plugin discovery.
|
||||
func readManifest(ndpPath string) (*Manifest, error) {
|
||||
// ReadManifest reads and validates the manifest from a .ndp file without loading
|
||||
// the wasm bytes (it runs ParseManifest, so JSON-schema and cross-field
|
||||
// validation are applied). Useful for quick plugin discovery and validation.
|
||||
func ReadManifest(ndpPath string) (*Manifest, error) {
|
||||
// Open the zip archive
|
||||
zr, err := zip.OpenReader(ndpPath)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
|
@ -132,8 +132,8 @@ var _ = Describe("ndpPackage", func() {
|
|||
})
|
||||
})
|
||||
|
||||
Describe("readManifest", func() {
|
||||
It("should read only the manifest without loading wasm", func() {
|
||||
Describe("ReadManifest", func() {
|
||||
It("parses the manifest from a package that also contains wasm", func() {
|
||||
ndpPath := filepath.Join(tmpDir, "test.ndp")
|
||||
manifest := &Manifest{
|
||||
Name: "Test Plugin",
|
||||
|
|
@ -141,57 +141,60 @@ var _ = Describe("ndpPackage", func() {
|
|||
Version: "1.0.0",
|
||||
Description: new("A test plugin"),
|
||||
}
|
||||
wasmBytes := make([]byte, 1024*1024) // 1MB of zeros
|
||||
|
||||
err := createTestPackage(ndpPath, manifest, wasmBytes)
|
||||
err := createTestPackage(ndpPath, manifest, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
m, err := readManifest(ndpPath)
|
||||
m, err := ReadManifest(ndpPath)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(m.Name).To(Equal("Test Plugin"))
|
||||
Expect(*m.Description).To(Equal("A test plugin"))
|
||||
})
|
||||
|
||||
It("should return error for missing manifest", func() {
|
||||
ndpPath := filepath.Join(tmpDir, "no-manifest.ndp")
|
||||
It("returns an error for a non-existent file", func() {
|
||||
_, err := ReadManifest(filepath.Join(tmpDir, "does-not-exist.ndp"))
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
|
||||
It("returns a specific error for a package missing manifest.json", func() {
|
||||
ndpPath := filepath.Join(tmpDir, "no-manifest.ndp")
|
||||
f, err := os.Create(ndpPath)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
defer f.Close()
|
||||
|
||||
zw := newTestZipWriter(f)
|
||||
err = zw.addFile("plugin.wasm", []byte{0x00})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
err = zw.close()
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(zw.addFile("plugin.wasm", []byte{0x00})).To(Succeed())
|
||||
Expect(zw.close()).To(Succeed())
|
||||
|
||||
_, err = readManifest(ndpPath)
|
||||
_, err = ReadManifest(ndpPath)
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("missing manifest.json"))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("ComputePackageSHA256", func() {
|
||||
It("should compute consistent hash for same file", func() {
|
||||
ndpPath := filepath.Join(tmpDir, "test.ndp")
|
||||
It("fails for a package with a schema-invalid manifest", func() {
|
||||
ndp := filepath.Join(tmpDir, "bad.ndp")
|
||||
// empty required fields violate the manifest JSON schema
|
||||
err := createTestPackage(ndp, &Manifest{}, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
_, err = ReadManifest(ndp)
|
||||
Expect(err).To(HaveOccurred())
|
||||
})
|
||||
|
||||
It("enforces cross-field validation", func() {
|
||||
ndp := filepath.Join(tmpDir, "crossfield.ndp")
|
||||
// subsonicapi permission without users: violates cross-field rule
|
||||
manifest := &Manifest{
|
||||
Name: "Test Plugin",
|
||||
Author: "Test Author",
|
||||
Version: "1.0.0",
|
||||
Name: "X",
|
||||
Author: "me",
|
||||
Version: "1.0.0",
|
||||
Permissions: &Permissions{Subsonicapi: &SubsonicAPIPermission{}},
|
||||
}
|
||||
wasmBytes := []byte{0x00, 0x61, 0x73, 0x6d}
|
||||
|
||||
err := createTestPackage(ndpPath, manifest, wasmBytes)
|
||||
err := createTestPackage(ndp, manifest, nil)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
hash1, err := computeFileSHA256(ndpPath)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
hash2, err := computeFileSHA256(ndpPath)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
Expect(hash1).To(Equal(hash2))
|
||||
Expect(hash1).To(HaveLen(64)) // SHA-256 produces 64 hex characters
|
||||
_, err = ReadManifest(ndp)
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(err.Error()).To(ContainSubstring("subsonicapi"))
|
||||
Expect(err.Error()).To(ContainSubstring("users"))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue