diff --git a/pkg/plugins/manager/loader/finder/local_test.go b/pkg/plugins/manager/loader/finder/local_test.go index d7da23e0e15..2fe9140ec42 100644 --- a/pkg/plugins/manager/loader/finder/local_test.go +++ b/pkg/plugins/manager/loader/finder/local_test.go @@ -354,27 +354,7 @@ func TestFinder_getAbsPluginJSONPaths(t *testing.T) { require.Empty(t, paths) }) - t.Run("should forward if the dist folder should be evaluated", func(t *testing.T) { - origWalk := walk - walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop, followDistFolder bool, walkFn util.WalkFunc) error { - if followDistFolder { - return walkFn(path, nil, errors.New("unexpected followDistFolder")) - } - return walkFn(path, nil, filepath.SkipDir) - } - t.Cleanup(func() { - walk = origWalk - }) - - finder := NewLocalFinder(false, featuremgmt.WithFeatures()) - paths, err := finder.getAbsPluginJSONPaths("test", false) - require.ErrorIs(t, err, filepath.SkipDir) - require.Empty(t, paths) - }) -} - -func TestFinder_getAbsPluginJSONPaths_PluginClass(t *testing.T) { - t.Run("When a dist folder exists as a direct child of the plugins path, it will always be resolved", func(t *testing.T) { + t.Run("The followDistFolder state controls whether certain folders are followed", func(t *testing.T) { dir, err := filepath.Abs("../../testdata/pluginRootWithDist") require.NoError(t, err) @@ -384,20 +364,17 @@ func TestFinder_getAbsPluginJSONPaths_PluginClass(t *testing.T) { expected []string }{ { - name: "When followDistFolder is enabled, a nested dist folder will also be resolved", + name: "When followDistFolder is enabled, only the nested dist folder will be followed", followDist: true, expected: []string{ - filepath.Join(dir, "datasource/plugin.json"), filepath.Join(dir, "dist/plugin.json"), - filepath.Join(dir, "panel/dist/plugin.json"), }, }, { - name: "When followDistFolder is disabled, a nested dist folder will not be resolved", + name: "When followDistFolder is disabled, no dist folders will be followed", followDist: false, expected: []string{ filepath.Join(dir, "datasource/plugin.json"), - filepath.Join(dir, "dist/plugin.json"), filepath.Join(dir, "panel/src/plugin.json"), }, }, diff --git a/pkg/plugins/manager/sources/sources.go b/pkg/plugins/manager/sources/sources.go index 4bf907cba2f..01de0145f30 100644 --- a/pkg/plugins/manager/sources/sources.go +++ b/pkg/plugins/manager/sources/sources.go @@ -2,52 +2,82 @@ package sources import ( "context" + "os" "path/filepath" + "slices" "github.com/grafana/grafana/pkg/plugins" - "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/setting" ) type Service struct { - gCfg *setting.Cfg - cfg *config.Cfg - log log.Logger + cfg *setting.Cfg + log log.Logger } -func ProvideService(gCfg *setting.Cfg, cfg *config.Cfg) *Service { +func ProvideService(cfg *setting.Cfg) *Service { return &Service{ - gCfg: gCfg, - cfg: cfg, - log: log.New("plugin.sources"), + cfg: cfg, + log: log.New("plugin.sources"), } } func (s *Service) List(_ context.Context) []plugins.PluginSource { - return []plugins.PluginSource{ - NewLocalSource(plugins.ClassCore, corePluginPaths(s.gCfg.StaticRootPath)), - NewLocalSource(plugins.ClassBundled, []string{s.gCfg.BundledPluginsPath}), - NewLocalSource(plugins.ClassExternal, append([]string{s.cfg.PluginsPath}, pluginFSPaths(s.cfg.PluginSettings)...)), + r := []plugins.PluginSource{ + NewLocalSource(plugins.ClassCore, corePluginPaths(s.cfg.StaticRootPath)), + NewLocalSource(plugins.ClassBundled, []string{s.cfg.BundledPluginsPath}), } + r = append(r, s.externalPluginSources()...) + r = append(r, s.pluginSettingSources()...) + return r } -// corePluginPaths provides a list of the Core plugin file system paths -func corePluginPaths(staticRootPath string) []string { - datasourcePaths := filepath.Join(staticRootPath, "app/plugins/datasource") - panelsPath := filepath.Join(staticRootPath, "app/plugins/panel") - return []string{datasourcePaths, panelsPath} +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) + if err != nil { + s.log.Error("Failed to open plugins path", "path", pluginsPath, "error", err) + return sources + } + + var pluginDirs []string + for _, dir := range d { + if dir.IsDir() { + pluginDirs = append(pluginDirs, filepath.Join(pluginsPath, dir.Name())) + } + } + slices.Sort(pluginDirs) + + for _, dir := range pluginDirs { + sources = append(sources, NewLocalSource(plugins.ClassExternal, []string{dir})) + } + return sources } -// pluginSettingPaths provides plugin file system paths defined in cfg.PluginSettings -func pluginFSPaths(ps map[string]map[string]string) []string { - var pluginSettingDirs []string - for _, s := range ps { - path, exists := s["path"] +func (s *Service) pluginSettingSources() []plugins.PluginSource { + var sources []plugins.PluginSource + for _, ps := range s.cfg.PluginSettings { + path, exists := ps["path"] if !exists || path == "" { continue } - pluginSettingDirs = append(pluginSettingDirs, path) + sources = append(sources, NewLocalSource(plugins.ClassExternal, []string{path})) } - return pluginSettingDirs + return sources +} + +// corePluginPaths provides a list of the Core plugin file system paths +func corePluginPaths(staticRootPath string) []string { + datasourcePaths := filepath.Join(staticRootPath, "app/plugins/datasource") + panelsPath := filepath.Join(staticRootPath, "app/plugins/panel") + return []string{datasourcePaths, panelsPath} } diff --git a/pkg/plugins/manager/sources/sources_test.go b/pkg/plugins/manager/sources/sources_test.go index 9bffb3e7245..433bc25d665 100644 --- a/pkg/plugins/manager/sources/sources_test.go +++ b/pkg/plugins/manager/sources/sources_test.go @@ -8,20 +8,21 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/plugins" - "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/setting" ) func TestSources_List(t *testing.T) { t.Run("Plugin sources are populated by default and listed in specific order", func(t *testing.T) { + testdata, err := filepath.Abs("../testdata") + require.NoError(t, err) + cfg := &setting.Cfg{ - BundledPluginsPath: "path1", - } - pCfg := &config.Cfg{ - PluginsPath: "path2", + StaticRootPath: testdata, + PluginsPath: filepath.Join(testdata, "pluginRootWithDist"), + BundledPluginsPath: filepath.Join(testdata, "unsigned-panel"), PluginSettings: setting.PluginSettings{ "foo": map[string]string{ - "path": "path3", + "path": filepath.Join(testdata, "test-app"), }, "bar": map[string]string{ "url": "https://grafana.plugin", @@ -29,15 +30,18 @@ func TestSources_List(t *testing.T) { }, } - s := ProvideService(cfg, pCfg) + s := ProvideService(cfg) srcs := s.List(context.Background()) ctx := context.Background() - require.Len(t, srcs, 3) + require.Len(t, srcs, 6) require.Equal(t, srcs[0].PluginClass(ctx), plugins.ClassCore) - require.Equal(t, srcs[0].PluginURIs(ctx), []string{filepath.Join("app", "plugins", "datasource"), filepath.Join("app", "plugins", "panel")}) + require.Equal(t, srcs[0].PluginURIs(ctx), []string{ + filepath.Join(testdata, "app", "plugins", "datasource"), + filepath.Join(testdata, "app", "plugins", "panel"), + }) sig, exists := srcs[0].DefaultSignature(ctx) require.True(t, exists) require.Equal(t, plugins.SignatureStatusInternal, sig.Status) @@ -45,15 +49,33 @@ func TestSources_List(t *testing.T) { require.Equal(t, "", sig.SigningOrg) require.Equal(t, srcs[1].PluginClass(ctx), plugins.ClassBundled) - require.Equal(t, srcs[1].PluginURIs(ctx), []string{"path1"}) + require.Equal(t, srcs[1].PluginURIs(ctx), []string{filepath.Join(testdata, "unsigned-panel")}) sig, exists = srcs[1].DefaultSignature(ctx) require.False(t, exists) require.Equal(t, plugins.Signature{}, sig) require.Equal(t, srcs[2].PluginClass(ctx), plugins.ClassExternal) - require.Equal(t, srcs[2].PluginURIs(ctx), []string{"path2", "path3"}) + require.Equal(t, srcs[2].PluginURIs(ctx), []string{ + filepath.Join(testdata, "pluginRootWithDist", "datasource"), + }) sig, exists = srcs[2].DefaultSignature(ctx) require.False(t, exists) require.Equal(t, plugins.Signature{}, sig) + + require.Equal(t, srcs[3].PluginClass(ctx), plugins.ClassExternal) + require.Equal(t, srcs[3].PluginURIs(ctx), []string{ + filepath.Join(testdata, "pluginRootWithDist", "dist"), + }) + sig, exists = srcs[3].DefaultSignature(ctx) + require.False(t, exists) + require.Equal(t, plugins.Signature{}, sig) + + require.Equal(t, srcs[4].PluginClass(ctx), plugins.ClassExternal) + require.Equal(t, srcs[4].PluginURIs(ctx), []string{ + filepath.Join(testdata, "pluginRootWithDist", "panel"), + }) + sig, exists = srcs[4].DefaultSignature(ctx) + require.False(t, exists) + require.Equal(t, plugins.Signature{}, sig) }) } diff --git a/pkg/services/pluginsintegration/plugins_integration_test.go b/pkg/services/pluginsintegration/plugins_integration_test.go index 773e1b7703e..cbe33ec0df4 100644 --- a/pkg/services/pluginsintegration/plugins_integration_test.go +++ b/pkg/services/pluginsintegration/plugins_integration_test.go @@ -51,30 +51,20 @@ func TestIntegrationPluginManager(t *testing.T) { bundledPluginsPath, err := filepath.Abs("../../../plugins-bundled/internal") require.NoError(t, err) - // We use the raw config here as it forms the basis for the setting.Provider implementation - // The plugin manager also relies directly on the setting.Cfg struct to provide Grafana specific - // properties such as the loading paths - raw, err := ini.Load([]byte(` - app_mode = production - - [plugin.test-app] - path=../../plugins/manager/testdata/test-app - - [plugin.test-panel] - not=included - `), - ) - require.NoError(t, err) - features := featuremgmt.WithFeatures() cfg := &setting.Cfg{ - Raw: raw, + Raw: ini.Empty(), StaticRootPath: staticRootPath, BundledPluginsPath: bundledPluginsPath, Azure: &azsettings.AzureSettings{}, - - // nolint:staticcheck - IsFeatureToggleEnabled: features.IsEnabledGlobally, + PluginSettings: map[string]map[string]string{ + "test-app": { + "path": "../../plugins/manager/testdata/test-app", + }, + "test-panel": { + "not": "included", + }, + }, } tracer := tracing.InitializeTracerForTest() diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index 5c63d56c1c6..600cdc976e9 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/pkg/services/pluginsintegration/test_helper.go @@ -66,7 +66,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core Terminator: term, }) - ps, err := pluginstore.ProvideService(reg, sources.ProvideService(cfg, pCfg), l) + ps, err := pluginstore.ProvideService(reg, sources.ProvideService(cfg), l) require.NoError(t, err) return &IntegrationTestCtx{ diff --git a/pkg/util/filepath.go b/pkg/util/filepath.go index 5d642f7b13c..c438e942faf 100644 --- a/pkg/util/filepath.go +++ b/pkg/util/filepath.go @@ -27,7 +27,7 @@ func newWalker(rootDir string) *walker { // it calls the walkFn passed. // // It is similar to filepath.Walk, except that it supports symbolic links and -// can detect infinite loops while following sym links. +// can detect infinite loops while following symlinks. // It solves the issue where your WalkFunc needs a path relative to the symbolic link // (resolving links within walkfunc loses the path to the symbolic link for each traversal). func Walk(path string, followSymlinks bool, detectSymlinkInfiniteLoop bool, followDistFolder bool, walkFn WalkFunc) error { @@ -112,20 +112,22 @@ func (w *walker) walk(path string, info os.FileInfo, resolvedPath string, symlin subFiles = append(subFiles, subFile{path: path2, resolvedPath: resolvedPath2, fileInfo: fileInfo}) } - // If we have found a dist directory in a subdirectory (IE not at root path), and followDistFolder is true, - // then we want to follow only the dist directory and ignore all other subdirectories. - atRootDir := w.rootDir == path - if followDistFolder && w.containsDistFolder(subFiles) && !atRootDir { - return w.walk(filepath.Join(path, "dist"), info, filepath.Join(resolvedPath, "dist"), symlinkPathsFollowed, - followDistFolder, walkFn) + if followDistFolder && w.containsDistFolder(subFiles) { + err := w.walk( + filepath.Join(path, "dist"), + info, + filepath.Join(resolvedPath, "dist"), + symlinkPathsFollowed, + followDistFolder, + walkFn) + if err != nil { + return err + } } else { - // Follow all subdirectories, with special handling for dist directories. for _, p := range subFiles { - // We only want to skip a dist directory if it is not in the root directory, and followDistFolder is false. - if p.isDistDir() && !atRootDir && !followDistFolder { + if p.isDistDir() && !followDistFolder { continue } - err = w.walk(p.path, p.fileInfo, p.resolvedPath, symlinkPathsFollowed, followDistFolder, walkFn) if err != nil { return err