Plugins: StaticFS should implement FSRemover (#110706)

make staticfs implement fs removal interface
This commit is contained in:
Will Browne 2025-09-09 15:33:05 +01:00 committed by GitHub
parent 3558a1c627
commit 05a6e8503e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 406 additions and 1 deletions

View File

@ -0,0 +1,102 @@
package commands
import (
"flag"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v2"
"github.com/grafana/grafana/pkg/cmd/grafana-cli/utils"
)
func TestRemoveCommand_StaticFS_FailsWithImmutableError(t *testing.T) {
t.Run("removeCommand fails with immutable error for plugins using StaticFS", func(t *testing.T) {
tmpDir := t.TempDir()
pluginID := "test-plugin"
pluginDir := filepath.Join(tmpDir, pluginID)
err := os.MkdirAll(pluginDir, 0750)
require.NoError(t, err)
pluginJSON := `{
"id": "test-plugin",
"name": "Test Plugin",
"type": "datasource",
"info": {
"version": "1.0.0"
}
}`
err = os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0644)
require.NoError(t, err)
cmdLine := createCliContextWithArgs(t, []string{pluginID}, "pluginsDir", tmpDir)
require.NotNil(t, cmdLine)
// Verify plugin directory exists before attempting removal
_, err = os.Stat(pluginDir)
require.NoError(t, err, "Plugin directory should exist before removal attempt")
err = removeCommand(cmdLine)
require.NoError(t, err)
// Verify plugin directory has been removed
_, err = os.Stat(pluginDir)
require.ErrorIs(t, err, os.ErrNotExist)
})
}
func TestRemoveCommand_PluginNotFound(t *testing.T) {
t.Run("removeCommand should handle missing plugin gracefully", func(t *testing.T) {
tmpDir := t.TempDir()
cmdLine := createCliContextWithArgs(t, []string{"non-existent-plugin"}, "pluginsDir", tmpDir)
require.NotNil(t, cmdLine)
err := removeCommand(cmdLine)
require.NoError(t, err)
})
}
func TestRemoveCommand_MissingPluginParameter(t *testing.T) {
t.Run("removeCommand should error when no plugin ID is provided", func(t *testing.T) {
cmdLine := createCliContextWithArgs(t, []string{})
require.NotNil(t, cmdLine)
err := removeCommand(cmdLine)
require.Error(t, err)
require.Contains(t, err.Error(), "missing plugin parameter")
})
}
// createCliContextWithArgs creates a CLI context with the specified arguments and optional flag key-value pairs.
// Usage: createCliContextWithArgs(t, []string{"plugin-id"}, "pluginsDir", "/path/to/plugins", "flag2", "value2")
func createCliContextWithArgs(t *testing.T, args []string, flagPairs ...string) *utils.ContextCommandLine {
if len(flagPairs)%2 != 0 {
t.Fatalf("flagPairs must be provided in key-value pairs, got %d arguments", len(flagPairs))
}
app := &cli.App{
Name: "grafana",
}
flagSet := flag.NewFlagSet("test", 0)
// Add flags from the key-value pairs
for i := 0; i < len(flagPairs); i += 2 {
key := flagPairs[i]
value := flagPairs[i+1]
flagSet.String(key, "", "")
err := flagSet.Set(key, value)
require.NoError(t, err, "Failed to set flag %s=%s", key, value)
}
err := flagSet.Parse(args)
require.NoError(t, err)
ctx := cli.NewContext(app, flagSet, nil)
return &utils.ContextCommandLine{
Context: ctx,
}
}

View File

@ -0,0 +1,150 @@
package commands
import (
"encoding/json"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v2"
"github.com/grafana/grafana/pkg/cmd/grafana-cli/models"
)
func TestUpgradeCommand(t *testing.T) {
t.Run("Plugin is removed even if upgrade fails", func(t *testing.T) {
tmpDir := t.TempDir()
pluginID := "test-upgrade-plugin"
pluginDir := filepath.Join(tmpDir, pluginID)
err := os.MkdirAll(pluginDir, 0750)
require.NoError(t, err)
pluginJSON := `{
"id": "test-upgrade-plugin",
"name": "Test Upgrade Plugin",
"type": "datasource",
"info": {
"version": "1.0.0"
}
}`
err = os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0644)
require.NoError(t, err)
// Create a mock HTTP server that returns plugin info with a newer version
mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Handle plugin info request
if r.URL.Path == "/repo/"+pluginID {
plugin := models.Plugin{
ID: pluginID,
Versions: []models.Version{
{
Version: "2.0.0", // Newer than the local version (1.0.0)
},
},
}
w.Header().Set("Content-Type", "application/json")
err = json.NewEncoder(w).Encode(plugin)
require.NoError(t, err)
return
}
// For any other request (like installation), return 500 to cause the upgrade to fail
// after the removal attempt, which is what we want to test
w.WriteHeader(http.StatusInternalServerError)
_, err = w.Write([]byte("Server error"))
require.NoError(t, err)
}))
defer mockServer.Close()
// Use our test implementation that properly implements GcomToken()
cmdLine := newTestCommandLine([]string{pluginID}, tmpDir, mockServer.URL)
// Verify plugin directory exists before attempting upgrade
_, err = os.Stat(pluginDir)
require.NoError(t, err)
err = upgradeCommand(cmdLine)
require.Error(t, err)
require.Contains(t, err.Error(), "API returned invalid status: 500 Internal Server Error")
// Verify plugin directory was removed during the removal step
_, err = os.Stat(pluginDir)
require.True(t, os.IsNotExist(err))
})
}
func TestUpgradeCommand_PluginNotFound(t *testing.T) {
t.Run("upgradeCommand should handle missing plugin gracefully", func(t *testing.T) {
tmpDir := t.TempDir()
cmdLine := createCliContextWithArgs(t, []string{"non-existent-plugin"}, "pluginsDir", tmpDir)
require.NotNil(t, cmdLine)
err := upgradeCommand(cmdLine)
require.Error(t, err)
// Should fail trying to find the local plugin
require.Contains(t, err.Error(), "could not find plugin non-existent-plugin")
})
}
func TestUpgradeCommand_MissingPluginParameter(t *testing.T) {
t.Run("upgradeCommand should error when no plugin ID is provided", func(t *testing.T) {
cmdLine := createCliContextWithArgs(t, []string{})
require.NotNil(t, cmdLine)
err := upgradeCommand(cmdLine)
require.Error(t, err)
require.Contains(t, err.Error(), "please specify plugin to update")
})
}
// Simple args implementation
type simpleArgs []string
func (a simpleArgs) First() string {
if len(a) > 0 {
return a[0]
}
return ""
}
func (a simpleArgs) Get(int) string { return "" }
func (a simpleArgs) Tail() []string { return nil }
func (a simpleArgs) Len() int { return len(a) }
func (a simpleArgs) Present() bool { return len(a) > 0 }
func (a simpleArgs) Slice() []string { return []string(a) }
// Base struct with default implementations for unused CommandLine methods
type baseCommandLine struct{}
func (b baseCommandLine) ShowHelp() error { return nil }
func (b baseCommandLine) ShowVersion() {}
func (b baseCommandLine) Application() *cli.App { return nil }
func (b baseCommandLine) Int(_ string) int { return 0 }
func (b baseCommandLine) String(_ string) string { return "" }
func (b baseCommandLine) StringSlice(_ string) []string { return nil }
func (b baseCommandLine) FlagNames() []string { return nil }
func (b baseCommandLine) Generic(_ string) any { return nil }
func (b baseCommandLine) Bool(_ string) bool { return false }
func (b baseCommandLine) PluginURL() string { return "" }
func (b baseCommandLine) GcomToken() string { return "" }
// Test implementation - only implements what we actually need
type testCommandLine struct {
baseCommandLine // Embedded struct provides default implementations
args simpleArgs
pluginDir string
repoURL string
}
func newTestCommandLine(args []string, pluginDir, repoURL string) *testCommandLine {
return &testCommandLine{args: simpleArgs(args), pluginDir: pluginDir, repoURL: repoURL}
}
// Only implement the methods actually used by upgradeCommand
func (t *testCommandLine) Args() cli.Args { return t.args }
func (t *testCommandLine) PluginDirectory() string { return t.pluginDir }
func (t *testCommandLine) PluginRepoURL() string { return t.repoURL }

View File

@ -236,6 +236,15 @@ func (f StaticFS) Files() ([]string, error) {
return files, nil
}
func (f StaticFS) Remove() error {
if remover, ok := f.FS.(FSRemover); ok {
if err := remover.Remove(); err != nil {
return err
}
}
return nil
}
// LocalFile implements a fs.File for accessing the local filesystem.
type LocalFile struct {
f *os.File

View File

@ -270,12 +270,27 @@ func TestStaticFS(t *testing.T) {
require.Equal(t, []string{allowedFn, deniedFn}, files)
})
t.Run("staticfs filters underelying fs's files", func(t *testing.T) {
t.Run("staticfs filters underlying fs's files", func(t *testing.T) {
files, err := staticFS.Files()
require.NoError(t, err)
require.Equal(t, []string{allowedFn}, files)
})
})
t.Run("FSRemover interface implementation verification", func(t *testing.T) {
tmpDir := t.TempDir()
lfs := NewLocalFS(tmpDir)
var localFSInterface FS = lfs
_, isRemover := localFSInterface.(FSRemover)
require.True(t, isRemover)
sfs, err := NewStaticFS(localFS)
require.NoError(t, err)
var staticFSInterface FS = sfs
_, isRemover = staticFSInterface.(FSRemover)
require.True(t, isRemover)
})
}
// TestFSTwoDotsInFileName ensures that LocalFS and StaticFS allow two dots in file names.

View File

@ -5,6 +5,8 @@ import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"runtime"
"testing"
@ -422,3 +424,100 @@ func createPlugin(t *testing.T, pluginID string, class plugins.Class, managed, b
func testCompatOpts() plugins.AddOpts {
return plugins.NewAddOpts("10.0.0", runtime.GOOS, runtime.GOARCH, "")
}
func TestPluginInstaller_Removal(t *testing.T) {
tmpDir := t.TempDir()
t.Run("LocalFS plugin removal succeeds via installer.Remove", func(t *testing.T) {
pluginDir := filepath.Join(tmpDir, "localfs-plugin")
err := os.MkdirAll(pluginDir, 0750)
require.NoError(t, err)
pluginJSON := `{
"id": "localfs-plugin",
"name": "LocalFS Plugin",
"type": "datasource",
"info": {
"version": "1.0.0"
}
}`
err = os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0644)
require.NoError(t, err)
localFS := plugins.NewLocalFS(pluginDir)
pluginV1 := createPlugin(t, "localfs-plugin", plugins.ClassExternal, true, true, func(plugin *plugins.Plugin) {
plugin.Info.Version = "1.0.0"
plugin.FS = localFS
})
registry := &fakes.FakePluginRegistry{
Store: map[string]*plugins.Plugin{
"localfs-plugin": pluginV1,
},
}
loader := &fakes.FakeLoader{
UnloadFunc: func(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, error) {
return p, nil
},
}
_, err = os.Stat(pluginDir)
require.NoError(t, err)
inst := New(&config.PluginManagementCfg{}, registry, loader, &fakes.FakePluginRepo{}, &fakes.FakePluginStorage{}, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{})
err = inst.Remove(context.Background(), "localfs-plugin", "1.0.0")
require.NoError(t, err)
_, err = os.Stat(pluginDir)
require.True(t, os.IsNotExist(err))
})
t.Run("StaticFS plugin removal is skipped via installer.Remove", func(t *testing.T) {
pluginDir := filepath.Join(tmpDir, "staticfs-plugin")
err := os.MkdirAll(pluginDir, 0750)
require.NoError(t, err)
pluginJSON := `{
"id": "staticfs-plugin",
"name": "StaticFS Plugin",
"type": "datasource",
"info": {
"version": "1.0.0"
}
}`
err = os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0644)
require.NoError(t, err)
localFS := plugins.NewLocalFS(pluginDir)
staticFS, err := plugins.NewStaticFS(localFS)
require.NoError(t, err)
pluginV1 := createPlugin(t, "staticfs-plugin", plugins.ClassExternal, true, true, func(plugin *plugins.Plugin) {
plugin.Info.Version = "1.0.0"
plugin.FS = staticFS
})
registry := &fakes.FakePluginRegistry{
Store: map[string]*plugins.Plugin{
"staticfs-plugin": pluginV1,
},
}
loader := &fakes.FakeLoader{
UnloadFunc: func(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, error) {
return p, nil
},
}
_, err = os.Stat(pluginDir)
require.NoError(t, err)
inst := New(&config.PluginManagementCfg{}, registry, loader, &fakes.FakePluginRepo{}, &fakes.FakePluginStorage{}, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{})
err = inst.Remove(context.Background(), "staticfs-plugin", "1.0.0")
require.NoError(t, err)
_, err = os.Stat(pluginDir)
require.ErrorIs(t, err, os.ErrNotExist)
})
}

View File

@ -2,6 +2,7 @@ package sources
import (
"errors"
"os"
"path/filepath"
"testing"
@ -110,3 +111,32 @@ func TestDirAsLocalSources(t *testing.T) {
})
}
}
func TestLocalSource(t *testing.T) {
t.Run("NewLocalSource should always return plugins with StaticFS", func(t *testing.T) {
tmpDir := t.TempDir()
pluginID := "test-plugin"
pluginDir := filepath.Join(tmpDir, pluginID)
err := os.MkdirAll(pluginDir, 0750)
require.NoError(t, err)
pluginJSON := `{
"id": "test-plugin",
"name": "Test Plugin",
"type": "datasource",
"info": {
"version": "1.0.0"
}
}`
err = os.WriteFile(filepath.Join(pluginDir, "plugin.json"), []byte(pluginJSON), 0644)
require.NoError(t, err)
bundles, err := NewLocalSource(plugins.ClassExternal, []string{pluginDir}).Discover(t.Context())
require.NoError(t, err)
require.Len(t, bundles, 1, "Should discover exactly one plugin")
require.Equal(t, pluginID, bundles[0].Primary.JSONData.ID)
_, canRemove := bundles[0].Primary.FS.(plugins.FSRemover)
require.True(t, canRemove)
})
}