diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go index c6b85bbb5f4..91b2175664e 100644 --- a/pkg/plugins/manager/loader/loader.go +++ b/pkg/plugins/manager/loader/loader.go @@ -75,6 +75,10 @@ func (l *Loader) instrumentLoad(ctx context.Context, src plugins.PluginSource) f return func(logger log.Logger, start time.Time) func([]*plugins.Plugin) { return func(plugins []*plugins.Plugin) { + if len(plugins) == 0 { + logger.Debug("Plugin source loaded, though no plugins were found") + return + } names := make([]string, len(plugins)) for i, p := range plugins { names[i] = p.ID diff --git a/pkg/plugins/manager/sources/source_local_disk.go b/pkg/plugins/manager/sources/source_local_disk.go index 07ab84d41c2..5f6ab1c3dd1 100644 --- a/pkg/plugins/manager/sources/source_local_disk.go +++ b/pkg/plugins/manager/sources/source_local_disk.go @@ -2,6 +2,10 @@ package sources import ( "context" + "errors" + "os" + "path/filepath" + "slices" "github.com/grafana/grafana/pkg/plugins" ) @@ -36,3 +40,31 @@ func (s *LocalSource) DefaultSignature(_ context.Context) (plugins.Signature, bo return plugins.Signature{}, false } } + +func DirAsLocalSources(pluginsPath string, class plugins.Class) ([]*LocalSource, error) { + if pluginsPath == "" { + return []*LocalSource{}, errors.New("plugins path not configured") + } + + // It's safe to ignore gosec warning G304 since the variable part of the file path comes from a configuration + // variable. + // nolint:gosec + d, err := os.ReadDir(pluginsPath) + if err != nil { + return []*LocalSource{}, errors.New("failed to open plugins path") + } + + var pluginDirs []string + for _, dir := range d { + if dir.IsDir() || dir.Type()&os.ModeSymlink == os.ModeSymlink { + pluginDirs = append(pluginDirs, filepath.Join(pluginsPath, dir.Name())) + } + } + slices.Sort(pluginDirs) + + var sources []*LocalSource + for _, dir := range pluginDirs { + sources = append(sources, NewLocalSource(class, []string{dir})) + } + return sources, nil +} diff --git a/pkg/plugins/manager/sources/source_local_disk_test.go b/pkg/plugins/manager/sources/source_local_disk_test.go new file mode 100644 index 00000000000..e380b5d5388 --- /dev/null +++ b/pkg/plugins/manager/sources/source_local_disk_test.go @@ -0,0 +1,71 @@ +package sources + +import ( + "errors" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/plugins" +) + +func TestDirAsLocalSources(t *testing.T) { + testdataDir := "../testdata" + + tests := []struct { + name string + pluginsPath string + expected []*LocalSource + err error + }{ + { + name: "Empty path returns an error", + pluginsPath: "", + expected: []*LocalSource{}, + err: errors.New("plugins path not configured"), + }, + { + name: "Directory with subdirectories", + pluginsPath: filepath.Join(testdataDir, "pluginRootWithDist"), + expected: []*LocalSource{ + { + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "datasource")}, + class: plugins.ClassExternal, + }, + { + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "dist")}, + class: plugins.ClassExternal, + }, + { + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "panel")}, + class: plugins.ClassExternal, + }, + }, + }, + { + name: "Directory with no subdirectories", + pluginsPath: filepath.Join(testdataDir, "pluginRootWithDist", "datasource"), + expected: nil, + }, + { + name: "Directory with a symlink to a directory", + pluginsPath: filepath.Join(testdataDir, "symbolic-plugin-dirs"), + expected: []*LocalSource{ + { + paths: []string{filepath.Join(testdataDir, "symbolic-plugin-dirs", "plugin")}, + class: plugins.ClassExternal, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := DirAsLocalSources(tt.pluginsPath, plugins.ClassExternal) + if tt.err != nil { + require.Errorf(t, err, tt.err.Error()) + } + require.Equal(t, tt.expected, got) + }) + } +} diff --git a/pkg/plugins/manager/sources/sources.go b/pkg/plugins/manager/sources/sources.go index 2693bcd74b6..a3bfe1d4112 100644 --- a/pkg/plugins/manager/sources/sources.go +++ b/pkg/plugins/manager/sources/sources.go @@ -2,9 +2,7 @@ package sources import ( "context" - "os" "path/filepath" - "slices" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/log" @@ -34,33 +32,17 @@ func (s *Service) List(_ context.Context) []plugins.PluginSource { } func (s *Service) externalPluginSources() []plugins.PluginSource { - var sources []plugins.PluginSource - if s.cfg.PluginsPath == "" { - return sources - } - - pluginsPath := s.cfg.PluginsPath - // It's safe to ignore gosec warning G304 since the variable part of the file path comes from a configuration - // variable. - // nolint:gosec - d, err := os.ReadDir(pluginsPath) + localSrcs, err := DirAsLocalSources(s.cfg.PluginsPath, plugins.ClassExternal) if err != nil { - s.log.Error("Failed to open plugins path", "path", pluginsPath, "error", err) - return sources + s.log.Error("Failed to load external plugins", "error", err) + return []plugins.PluginSource{} } - var pluginDirs []string - for _, dir := range d { - if dir.IsDir() || dir.Type()&os.ModeSymlink == os.ModeSymlink { - pluginDirs = append(pluginDirs, filepath.Join(pluginsPath, dir.Name())) - } + var srcs []plugins.PluginSource + for _, src := range localSrcs { + srcs = append(srcs, src) } - slices.Sort(pluginDirs) - - for _, dir := range pluginDirs { - sources = append(sources, NewLocalSource(plugins.ClassExternal, []string{dir})) - } - return sources + return srcs } func (s *Service) pluginSettingSources() []plugins.PluginSource { diff --git a/pkg/services/pluginsintegration/renderer/renderer.go b/pkg/services/pluginsintegration/renderer/renderer.go index 102c5da7adf..940a6652534 100644 --- a/pkg/services/pluginsintegration/renderer/renderer.go +++ b/pkg/services/pluginsintegration/renderer/renderer.go @@ -19,22 +19,16 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/manager/signature" "github.com/grafana/grafana/pkg/plugins/manager/sources" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/rendering" ) -func ProvideService(cfg *config.Cfg, registry registry.Service, licensing plugins.Licensing, - features featuremgmt.FeatureToggles) (*Manager, error) { - l, err := createLoader(cfg, registry, licensing, features) +func ProvideService(cfg *config.Cfg, registry registry.Service, licensing plugins.Licensing) (*Manager, error) { + l, err := createLoader(cfg, registry, licensing) if err != nil { return nil, err } - return &Manager{ - cfg: cfg, - loader: l, - log: log.New("plugins.renderer"), - }, nil + return NewManager(cfg, l), nil } type Manager struct { @@ -45,6 +39,14 @@ type Manager struct { renderer *Plugin } +func NewManager(cfg *config.Cfg, loader loader.Service) *Manager { + return &Manager{ + cfg: cfg, + loader: loader, + log: log.New("renderer.manager"), + } +} + type Plugin struct { plugin *plugins.Plugin @@ -77,22 +79,29 @@ func (m *Manager) Renderer(ctx context.Context) (rendering.Plugin, bool) { return m.renderer, true } - ps, err := m.loader.Load(ctx, sources.NewLocalSource(plugins.ClassExternal, []string{m.cfg.PluginsPath})) + srcs, err := sources.DirAsLocalSources(m.cfg.PluginsPath, plugins.ClassExternal) if err != nil { - m.log.Error("Failed to load renderer plugin", "error", err) + m.log.Error("Failed to get renderer plugin sources", "error", err) return nil, false } - if len(ps) >= 1 { - m.renderer = &Plugin{plugin: ps[0]} - return m.renderer, true + for _, src := range srcs { + ps, err := m.loader.Load(ctx, src) + if err != nil { + m.log.Error("Failed to load renderer plugin", "error", err) + return nil, false + } + + if len(ps) >= 1 { + m.renderer = &Plugin{plugin: ps[0]} + return m.renderer, true + } } return nil, false } -func createLoader(cfg *config.Cfg, pr registry.Service, l plugins.Licensing, - features featuremgmt.FeatureToggles) (loader.Service, error) { +func createLoader(cfg *config.Cfg, pr registry.Service, l plugins.Licensing) (loader.Service, error) { d := discovery.New(cfg, discovery.Opts{ FindFilterFuncs: []discovery.FindFilterFunc{ discovery.NewPermittedPluginTypesFilterStep([]plugins.Type{plugins.TypeRenderer}), diff --git a/pkg/services/pluginsintegration/renderer/renderer_test.go b/pkg/services/pluginsintegration/renderer/renderer_test.go new file mode 100644 index 00000000000..a2c8230bb79 --- /dev/null +++ b/pkg/services/pluginsintegration/renderer/renderer_test.go @@ -0,0 +1,82 @@ +package renderer + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/config" + "github.com/grafana/grafana/pkg/plugins/manager/fakes" +) + +func TestRenderer(t *testing.T) { + t.Run("Test Renderer will treat directories under plugins path as individual sources", func(t *testing.T) { + testdataDir := filepath.Join("testdata", "plugins") + + numLoaded := 0 + numUnloaded := 0 + loader := &fakes.FakeLoader{ + LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { + require.True(t, src.PluginClass(ctx) == plugins.ClassExternal) + require.Len(t, src.PluginURIs(ctx), 1) + require.True(t, strings.HasPrefix(src.PluginURIs(ctx)[0], testdataDir)) + + numLoaded++ + return []*plugins.Plugin{}, nil + }, + UnloadFunc: func(_ context.Context, _ *plugins.Plugin) (*plugins.Plugin, error) { + numUnloaded++ + return nil, nil + }, + } + cfg := &config.Cfg{ + PluginsPath: filepath.Join(testdataDir), + } + + m := NewManager(cfg, loader) + + r, exists := m.Renderer(context.Background()) + require.False(t, exists) + require.Equal(t, 4, numLoaded) + require.Equal(t, 0, numUnloaded) + require.Nil(t, r) + }) + + t.Run("Test Renderer load all directories until a plugin is returned", func(t *testing.T) { + testdataDir := filepath.Join("testdata", "plugins") + + numLoaded := 0 + numUnloaded := 0 + p := &plugins.Plugin{ + JSONData: plugins.JSONData{ID: "test"}, + } + loader := &fakes.FakeLoader{ + LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { + numLoaded++ + if strings.HasPrefix(src.PluginURIs(ctx)[0], filepath.Join(testdataDir, "renderer")) { + return []*plugins.Plugin{p}, nil + } + return []*plugins.Plugin{}, nil + }, + UnloadFunc: func(_ context.Context, _ *plugins.Plugin) (*plugins.Plugin, error) { + numUnloaded++ + return nil, nil + }, + } + cfg := &config.Cfg{ + PluginsPath: filepath.Join(testdataDir), + } + + m := NewManager(cfg, loader) + + r, exists := m.Renderer(context.Background()) + require.True(t, exists) + require.Equal(t, 3, numLoaded) + require.Equal(t, 0, numUnloaded) + require.NotNil(t, r) + }) +} diff --git a/pkg/services/pluginsintegration/renderer/testdata/plugins/app/plugin.json b/pkg/services/pluginsintegration/renderer/testdata/plugins/app/plugin.json new file mode 100644 index 00000000000..14dfb7655af --- /dev/null +++ b/pkg/services/pluginsintegration/renderer/testdata/plugins/app/plugin.json @@ -0,0 +1,3 @@ +{ + "type": "app" +} diff --git a/pkg/services/pluginsintegration/renderer/testdata/plugins/datasource/plugin.json b/pkg/services/pluginsintegration/renderer/testdata/plugins/datasource/plugin.json new file mode 100644 index 00000000000..5c57cba8286 --- /dev/null +++ b/pkg/services/pluginsintegration/renderer/testdata/plugins/datasource/plugin.json @@ -0,0 +1,3 @@ +{ + "type": "datasource" +} diff --git a/pkg/services/pluginsintegration/renderer/testdata/plugins/renderer/plugin.json b/pkg/services/pluginsintegration/renderer/testdata/plugins/renderer/plugin.json new file mode 100644 index 00000000000..6d423963a4b --- /dev/null +++ b/pkg/services/pluginsintegration/renderer/testdata/plugins/renderer/plugin.json @@ -0,0 +1,3 @@ +{ + "type": "renderer" +} diff --git a/pkg/services/pluginsintegration/renderer/testdata/plugins/secrets-manager/plugin.json b/pkg/services/pluginsintegration/renderer/testdata/plugins/secrets-manager/plugin.json new file mode 100644 index 00000000000..635a3ff47b9 --- /dev/null +++ b/pkg/services/pluginsintegration/renderer/testdata/plugins/secrets-manager/plugin.json @@ -0,0 +1,3 @@ +{ + "type": "secretsmanager" +}