From 3d37f969e71d2a56a6dbe52e511a33b25dd05673 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Thu, 19 Jun 2025 10:28:23 +0100 Subject: [PATCH] Plugins: Move discovery logic to plugin sources (#106911) * move finder behaviour to source * tidy * undo go.mod changes * fix comment * tidy unsafe local source --- pkg/cmd/grafana-cli/services/services.go | 5 +- pkg/plugins/ifaces.go | 5 +- pkg/plugins/manager/fakes/fakes.go | 10 +- pkg/plugins/manager/installer.go | 15 +- pkg/plugins/manager/installer_test.go | 34 +- pkg/plugins/manager/loader/finder/ifaces.go | 11 - pkg/plugins/manager/loader/finder/local.go | 222 -------- .../manager/loader/finder/local_test.go | 473 ------------------ pkg/plugins/manager/loader/loader_test.go | 9 - .../manager/pipeline/discovery/discovery.go | 53 +- .../manager/pipeline/discovery/steps.go | 12 +- .../manager/sources/source_local_disk.go | 231 ++++++++- .../manager/sources/source_local_disk_test.go | 61 ++- pkg/plugins/manager/sources/sources.go | 32 +- pkg/plugins/manager/sources/sources_test.go | 68 ++- .../pluginsintegration/loader/loader_test.go | 59 +-- .../pluginsintegration/pipeline/pipeline.go | 6 +- .../pluginsintegration/pluginsintegration.go | 3 - .../pluginstore/store_test.go | 19 +- .../plugintest/plugins_test.go | 28 +- .../pluginsintegration/renderer/renderer.go | 7 +- .../renderer/renderer_test.go | 22 +- .../pluginsintegration/test_helper.go | 7 +- 23 files changed, 475 insertions(+), 917 deletions(-) delete mode 100644 pkg/plugins/manager/loader/finder/ifaces.go delete mode 100644 pkg/plugins/manager/loader/finder/local.go delete mode 100644 pkg/plugins/manager/loader/finder/local_test.go diff --git a/pkg/cmd/grafana-cli/services/services.go b/pkg/cmd/grafana-cli/services/services.go index 89cfa072c2a..d3dd0c239bc 100644 --- a/pkg/cmd/grafana-cli/services/services.go +++ b/pkg/cmd/grafana-cli/services/services.go @@ -13,7 +13,6 @@ import ( "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" "github.com/grafana/grafana/pkg/cmd/grafana-cli/models" "github.com/grafana/grafana/pkg/plugins" - "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/sources" ) @@ -77,9 +76,7 @@ func GetLocalPlugin(pluginDir, pluginID string) (plugins.FoundPlugin, error) { } func GetLocalPlugins(pluginDir string) []*plugins.FoundBundle { - f := finder.NewLocalFinder(true) - - res, err := f.Find(context.Background(), sources.NewLocalSource(plugins.ClassExternal, []string{pluginDir})) + res, err := sources.NewLocalSource(plugins.ClassExternal, []string{pluginDir}).Discover(context.Background()) if err != nil { logger.Error("Could not get local plugins", err) return make([]*plugins.FoundBundle, 0) diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index fd227184de5..315855cb9d3 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -18,9 +18,12 @@ type Installer interface { } type PluginSource interface { + // PluginClass is the associated Class of plugin for this source PluginClass(ctx context.Context) Class - PluginURIs(ctx context.Context) []string + // DefaultSignature is the (optional) default signature information for this source DefaultSignature(ctx context.Context, pluginID string) (Signature, bool) + // Discover finds and returns plugin bundles from this source + Discover(ctx context.Context) ([]*FoundBundle, error) } type FileStore interface { diff --git a/pkg/plugins/manager/fakes/fakes.go b/pkg/plugins/manager/fakes/fakes.go index 0c192b12817..b677fba8d78 100644 --- a/pkg/plugins/manager/fakes/fakes.go +++ b/pkg/plugins/manager/fakes/fakes.go @@ -480,7 +480,7 @@ func (s *FakeSourceRegistry) List(ctx context.Context) []plugins.PluginSource { type FakePluginSource struct { PluginClassFunc func(ctx context.Context) plugins.Class - PluginURIsFunc func(ctx context.Context) []string + DiscoverFunc func(ctx context.Context) ([]*plugins.FoundBundle, error) DefaultSignatureFunc func(ctx context.Context) (plugins.Signature, bool) } @@ -491,11 +491,11 @@ func (s *FakePluginSource) PluginClass(ctx context.Context) plugins.Class { return "" } -func (s *FakePluginSource) PluginURIs(ctx context.Context) []string { - if s.PluginURIsFunc != nil { - return s.PluginURIsFunc(ctx) +func (s *FakePluginSource) Discover(ctx context.Context) ([]*plugins.FoundBundle, error) { + if s.DiscoverFunc != nil { + return s.DiscoverFunc(ctx) } - return []string{} + return []*plugins.FoundBundle{}, nil } func (s *FakePluginSource) DefaultSignature(ctx context.Context, _ string) (plugins.Signature, bool) { diff --git a/pkg/plugins/manager/installer.go b/pkg/plugins/manager/installer.go index 48953f912e1..b0e96515807 100644 --- a/pkg/plugins/manager/installer.go +++ b/pkg/plugins/manager/installer.go @@ -25,19 +25,21 @@ type PluginInstaller struct { pluginStorageDirFunc storage.DirNameGeneratorFunc pluginRegistry registry.Service pluginLoader loader.Service - installing sync.Map - log log.Logger - serviceRegistry auth.ExternalServiceRegistry + cfg *config.PluginManagementCfg + + installing sync.Map + log log.Logger + serviceRegistry auth.ExternalServiceRegistry } func ProvideInstaller(cfg *config.PluginManagementCfg, pluginRegistry registry.Service, pluginLoader loader.Service, pluginRepo repo.Service, serviceRegistry auth.ExternalServiceRegistry) *PluginInstaller { - return New(pluginRegistry, pluginLoader, pluginRepo, + return New(cfg, pluginRegistry, pluginLoader, pluginRepo, storage.FileSystem(log.NewPrettyLogger("installer.fs"), cfg.PluginsPath), storage.SimpleDirNameGeneratorFunc, serviceRegistry) } -func New(pluginRegistry registry.Service, pluginLoader loader.Service, pluginRepo repo.Service, - pluginStorage storage.ZipExtractor, pluginStorageDirFunc storage.DirNameGeneratorFunc, +func New(cfg *config.PluginManagementCfg, pluginRegistry registry.Service, pluginLoader loader.Service, + pluginRepo repo.Service, pluginStorage storage.ZipExtractor, pluginStorageDirFunc storage.DirNameGeneratorFunc, serviceRegistry auth.ExternalServiceRegistry) *PluginInstaller { return &PluginInstaller{ pluginLoader: pluginLoader, @@ -45,6 +47,7 @@ func New(pluginRegistry registry.Service, pluginLoader loader.Service, pluginRep pluginRepo: pluginRepo, pluginStorage: pluginStorage, pluginStorageDirFunc: pluginStorageDirFunc, + cfg: cfg, installing: sync.Map{}, log: log.New("plugin.installer"), serviceRegistry: serviceRegistry, diff --git a/pkg/plugins/manager/installer_test.go b/pkg/plugins/manager/installer_test.go index ae2025445e1..1ca2f67735e 100644 --- a/pkg/plugins/manager/installer_test.go +++ b/pkg/plugins/manager/installer_test.go @@ -11,8 +11,10 @@ import ( "github.com/stretchr/testify/require" "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/plugins/manager/fakes" + "github.com/grafana/grafana/pkg/plugins/manager/sources" "github.com/grafana/grafana/pkg/plugins/repo" "github.com/grafana/grafana/pkg/plugins/storage" ) @@ -36,11 +38,8 @@ func TestPluginManager_Add_Remove(t *testing.T) { FileHeader: zip.FileHeader{Name: zipNameV1}, }}}} - var loadedPaths []string loader := &fakes.FakeLoader{ LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { - loadedPaths = append(loadedPaths, src.PluginURIs(ctx)...) - require.Equal(t, []string{zipNameV1}, src.PluginURIs(ctx)) return []*plugins.Plugin{pluginV1}, nil }, UnloadFunc: func(_ context.Context, p *plugins.Plugin) (*plugins.Plugin, error) { @@ -70,7 +69,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, } - inst := New(fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + inst := New(&config.PluginManagementCfg{}, fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) err := inst.Add(context.Background(), pluginID, v1, testCompatOpts()) require.NoError(t, err) @@ -98,7 +97,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, nil }, } - inst := New(fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + inst := New(&config.PluginManagementCfg{}, fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) err := inst.Add(context.Background(), pluginID, v1, plugins.NewAddOpts(v1, runtime.GOOS, runtime.GOARCH, url)) require.NoError(t, err) }) @@ -114,7 +113,6 @@ func TestPluginManager_Add_Remove(t *testing.T) { }}}} loader.LoadFunc = func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { require.Equal(t, plugins.ClassExternal, src.PluginClass(ctx)) - require.Equal(t, []string{zipNameV2}, src.PluginURIs(ctx)) return []*plugins.Plugin{pluginV2}, nil } pluginRepo.GetPluginArchiveInfoFunc = func(_ context.Context, _, _ string, _ repo.CompatOpts) (*repo.PluginArchiveInfo, error) { @@ -154,7 +152,6 @@ func TestPluginManager_Add_Remove(t *testing.T) { }}}} loader.LoadFunc = func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { require.Equal(t, plugins.ClassExternal, src.PluginClass(ctx)) - require.Equal(t, []string{zipNameV2}, src.PluginURIs(ctx)) return []*plugins.Plugin{pluginV2}, nil } pluginRepo.GetPluginArchiveInfoFunc = func(_ context.Context, _, _ string, _ repo.CompatOpts) (*repo.PluginArchiveInfo, error) { @@ -223,7 +220,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, } - pm := New(reg, &fakes.FakeLoader{}, &fakes.FakePluginRepo{}, &fakes.FakePluginStorage{}, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + pm := New(&config.PluginManagementCfg{}, reg, &fakes.FakeLoader{}, &fakes.FakePluginRepo{}, &fakes.FakePluginStorage{}, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) err := pm.Add(context.Background(), p.ID, "3.2.0", testCompatOpts()) require.ErrorIs(t, err, plugins.ErrInstallCorePlugin) @@ -246,7 +243,10 @@ func TestPluginManager_Add_Remove(t *testing.T) { var loadedPaths []string loader := &fakes.FakeLoader{ LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { - loadedPaths = append(loadedPaths, src.PluginURIs(ctx)...) + // Check if this is a LocalSource and get its paths + if localSrc, ok := src.(*sources.LocalSource); ok { + loadedPaths = append(loadedPaths, localSrc.Paths()...) + } return []*plugins.Plugin{}, nil }, } @@ -285,7 +285,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, } - inst := New(fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + inst := New(&config.PluginManagementCfg{}, fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) err := inst.Add(context.Background(), p3, "", testCompatOpts()) require.NoError(t, err) require.Equal(t, []string{p1Zip, p2Zip, p3Zip}, loadedPaths) @@ -300,7 +300,10 @@ func TestPluginManager_Add_Remove(t *testing.T) { var loadedPaths []string loader := &fakes.FakeLoader{ LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { - loadedPaths = append(loadedPaths, src.PluginURIs(ctx)...) + // Check if this is a LocalSource and get its paths + if localSrc, ok := src.(*sources.LocalSource); ok { + loadedPaths = append(loadedPaths, localSrc.Paths()...) + } return []*plugins.Plugin{}, nil }, } @@ -334,7 +337,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, } - inst := New(fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + inst := New(&config.PluginManagementCfg{}, fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) err := inst.Add(context.Background(), p1, "", testCompatOpts()) require.NoError(t, err) require.Equal(t, []string{p2Zip, p1Zip}, loadedPaths) @@ -351,7 +354,10 @@ func TestPluginManager_Add_Remove(t *testing.T) { var loadedPaths []string loader := &fakes.FakeLoader{ LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { - loadedPaths = append(loadedPaths, src.PluginURIs(ctx)...) + // Check if this is a LocalSource and get its paths + if localSrc, ok := src.(*sources.LocalSource); ok { + loadedPaths = append(loadedPaths, localSrc.Paths()...) + } return []*plugins.Plugin{}, nil }, } @@ -379,7 +385,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, } - inst := New(reg, loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + inst := New(&config.PluginManagementCfg{}, reg, loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) err := inst.Add(context.Background(), testPluginID, "", testCompatOpts()) require.NoError(t, err) require.Equal(t, []string{"test-plugin.zip"}, loadedPaths) diff --git a/pkg/plugins/manager/loader/finder/ifaces.go b/pkg/plugins/manager/loader/finder/ifaces.go deleted file mode 100644 index 288c907a532..00000000000 --- a/pkg/plugins/manager/loader/finder/ifaces.go +++ /dev/null @@ -1,11 +0,0 @@ -package finder - -import ( - "context" - - "github.com/grafana/grafana/pkg/plugins" -) - -type Finder interface { - Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) -} diff --git a/pkg/plugins/manager/loader/finder/local.go b/pkg/plugins/manager/loader/finder/local.go deleted file mode 100644 index 29f3bacfed1..00000000000 --- a/pkg/plugins/manager/loader/finder/local.go +++ /dev/null @@ -1,222 +0,0 @@ -package finder - -import ( - "context" - "errors" - "fmt" - "io" - "os" - "path/filepath" - "strings" - - "github.com/grafana/grafana/pkg/infra/fs" - "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/util" -) - -var walk = util.Walk - -var ( - ErrInvalidPluginJSONFilePath = errors.New("invalid plugin.json filepath was provided") -) - -type Local struct { - log log.Logger - production bool -} - -func NewLocalFinder(devMode bool) *Local { - return &Local{ - production: !devMode, - log: log.New("local.finder"), - } -} - -func ProvideLocalFinder(cfg *config.PluginManagementCfg) *Local { - return NewLocalFinder(cfg.DevMode) -} - -func (l *Local) Find(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) { - if len(src.PluginURIs(ctx)) == 0 { - return []*plugins.FoundBundle{}, nil - } - - pluginURIs := src.PluginURIs(ctx) - pluginJSONPaths := make([]string, 0, len(pluginURIs)) - for _, path := range pluginURIs { - exists, err := fs.Exists(path) - if err != nil { - l.log.Warn("Skipping finding plugins as an error occurred", "path", path, "error", err) - continue - } - if !exists { - l.log.Warn("Skipping finding plugins as directory does not exist", "path", path) - continue - } - - paths, err := l.getAbsPluginJSONPaths(path) - if err != nil { - return nil, err - } - pluginJSONPaths = append(pluginJSONPaths, paths...) - } - - // load plugin.json files and map directory to JSON data - foundPlugins := make(map[string]plugins.JSONData) - for _, pluginJSONPath := range pluginJSONPaths { - plugin, err := l.readPluginJSON(pluginJSONPath) - if err != nil { - l.log.Warn("Skipping plugin loading as its plugin.json could not be read", "path", pluginJSONPath, "error", err) - continue - } - - pluginJSONAbsPath, err := filepath.Abs(pluginJSONPath) - if err != nil { - l.log.Warn("Skipping plugin loading as absolute plugin.json path could not be calculated", "pluginId", plugin.ID, "error", err) - continue - } - - foundPlugins[filepath.Dir(pluginJSONAbsPath)] = plugin - } - - res := make(map[string]*plugins.FoundBundle) - for pluginDir, data := range foundPlugins { - var pluginFs plugins.FS - pluginFs = plugins.NewLocalFS(pluginDir) - if l.production { - // In prod, tighten up security by allowing access only to the files present up to this point. - // Any new file "sneaked in" won't be allowed and will acts as if the file did not exist. - var err error - pluginFs, err = plugins.NewStaticFS(pluginFs) - if err != nil { - return nil, err - } - } - res[pluginDir] = &plugins.FoundBundle{ - Primary: plugins.FoundPlugin{ - JSONData: data, - FS: pluginFs, - }, - } - } - - // Track child plugins and add them to their parent. - childPlugins := make(map[string]struct{}) - for dir, p := range res { - // Check if this plugin is the parent of another plugin. - for dir2, p2 := range res { - if dir == dir2 { - continue - } - - relPath, err := filepath.Rel(dir, dir2) - if err != nil { - l.log.Error("Cannot calculate relative path. Skipping", "pluginId", p2.Primary.JSONData.ID, "err", err) - continue - } - if !strings.Contains(relPath, "..") { - child := p2.Primary - l.log.Debug("Adding child", "parent", p.Primary.JSONData.ID, "child", child.JSONData.ID, "relPath", relPath) - p.Children = append(p.Children, &child) - childPlugins[dir2] = struct{}{} - } - } - } - - // Remove child plugins from the result (they are already tracked via their parent). - result := make([]*plugins.FoundBundle, 0, len(res)) - for k := range res { - if _, ok := childPlugins[k]; !ok { - result = append(result, res[k]) - } - } - - return result, nil -} - -func (l *Local) readPluginJSON(pluginJSONPath string) (plugins.JSONData, error) { - reader, err := l.readFile(pluginJSONPath) - defer func() { - if reader == nil { - return - } - if err = reader.Close(); err != nil { - l.log.Warn("Failed to close plugin JSON file", "path", pluginJSONPath, "error", err) - } - }() - if err != nil { - l.log.Warn("Skipping plugin loading as its plugin.json could not be read", "path", pluginJSONPath, "error", err) - return plugins.JSONData{}, err - } - plugin, err := plugins.ReadPluginJSON(reader) - if err != nil { - l.log.Warn("Skipping plugin loading as its plugin.json could not be read", "path", pluginJSONPath, "error", err) - return plugins.JSONData{}, err - } - - return plugin, nil -} - -func (l *Local) getAbsPluginJSONPaths(path string) ([]string, error) { - var pluginJSONPaths []string - - var err error - path, err = filepath.Abs(path) - if err != nil { - return []string{}, err - } - - if err = walk(path, true, true, - func(currentPath string, fi os.FileInfo, err error) error { - if err != nil { - if errors.Is(err, os.ErrNotExist) { - l.log.Error("Couldn't scan directory since it doesn't exist", "pluginDir", path, "error", err) - return nil - } - if errors.Is(err, os.ErrPermission) { - l.log.Error("Couldn't scan directory due to lack of permissions", "pluginDir", path, "error", err) - return nil - } - - return fmt.Errorf("filepath.Walk reported an error for %q: %w", currentPath, err) - } - - if fi.Name() == "node_modules" { - return util.ErrWalkSkipDir - } - - if fi.IsDir() { - return nil - } - - if fi.Name() != "plugin.json" { - return nil - } - - pluginJSONPaths = append(pluginJSONPaths, currentPath) - return nil - }); err != nil { - return []string{}, err - } - - return pluginJSONPaths, nil -} - -func (l *Local) readFile(pluginJSONPath string) (io.ReadCloser, error) { - l.log.Debug("Loading plugin", "path", pluginJSONPath) - - if !strings.EqualFold(filepath.Ext(pluginJSONPath), ".json") { - return nil, ErrInvalidPluginJSONFilePath - } - - absPluginJSONPath, err := filepath.Abs(pluginJSONPath) - if err != nil { - return nil, err - } - - // Wrapping in filepath.Clean to properly handle - // gosec G304 Potential file inclusion via variable rule. - return os.Open(filepath.Clean(absPluginJSONPath)) -} diff --git a/pkg/plugins/manager/loader/finder/local_test.go b/pkg/plugins/manager/loader/finder/local_test.go deleted file mode 100644 index 72a08d9844d..00000000000 --- a/pkg/plugins/manager/loader/finder/local_test.go +++ /dev/null @@ -1,473 +0,0 @@ -package finder - -import ( - "context" - "errors" - "os" - "path/filepath" - "sort" - "testing" - - "github.com/google/go-cmp/cmp" - "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/plugins" - "github.com/grafana/grafana/pkg/plugins/manager/fakes" - "github.com/grafana/grafana/pkg/util" -) - -func TestFinder_Find(t *testing.T) { - testData, err := filepath.Abs("../../testdata") - if err != nil { - require.NoError(t, err) - } - - testCases := []struct { - name string - pluginDirs []string - pluginClass plugins.Class - expectedBundles []*plugins.FoundBundle - err error - }{ - { - name: "Dir with single plugin", - pluginDirs: []string{filepath.Join(testData, "valid-v2-signature")}, - expectedBundles: []*plugins.FoundBundle{ - { - Primary: plugins.FoundPlugin{ - JSONData: plugins.JSONData{ - ID: "test-datasource", - Type: plugins.TypeDataSource, - Name: "Test", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Will Browne", - URL: "https://willbrowne.com", - }, - Description: "Test", - Version: "1.0.0", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - State: plugins.ReleaseStateAlpha, - Backend: true, - Executable: "test", - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "valid-v2-signature/plugin")), - }, - }, - }, - }, - { - name: "Dir with nested plugins", - pluginDirs: []string{"../../testdata/duplicate-plugins"}, - expectedBundles: []*plugins.FoundBundle{ - { - Primary: plugins.FoundPlugin{ - JSONData: plugins.JSONData{ - ID: "test-app", - Type: plugins.TypeDataSource, - Name: "Parent", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Grafana Labs", - URL: "http://grafana.com", - }, - Description: "Parent plugin", - Version: "1.0.0", - Updated: "2020-10-20", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "duplicate-plugins/nested")), - }, - Children: []*plugins.FoundPlugin{ - { - JSONData: plugins.JSONData{ - ID: "test-app", - Type: plugins.TypeDataSource, - Name: "Child", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Grafana Labs", - URL: "http://grafana.com", - }, - Description: "Child plugin", - Version: "1.0.0", - Updated: "2020-10-20", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "duplicate-plugins/nested/nested")), - }, - }, - }, - }, - }, - { - name: "Dir with single plugin which has symbolic link root directory", - pluginDirs: []string{"../../testdata/symbolic-plugin-dirs"}, - expectedBundles: []*plugins.FoundBundle{ - { - Primary: plugins.FoundPlugin{ - JSONData: plugins.JSONData{ - ID: "test-app", - Type: plugins.TypeApp, - Name: "Test App", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Test Inc.", - URL: "http://test.com", - }, - Description: "Official Grafana Test App & Dashboard bundle", - Version: "1.0.0", - Links: []plugins.InfoLink{ - {Name: "Project site", URL: "http://project.com"}, - {Name: "License & Terms", URL: "http://license.com"}, - }, - Updated: "2015-02-10", - Logos: plugins.Logos{ - Small: "img/logo_small.png", - Large: "img/logo_large.png", - }, - Screenshots: []plugins.Screenshots{ - {Name: "img1", Path: "img/screenshot1.png"}, - {Name: "img2", Path: "img/screenshot2.png"}, - }, - Keywords: []string{"test"}, - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "3.x.x", - Plugins: []plugins.Dependency{ - {ID: "graphite", Type: "datasource", Name: "Graphite"}, - {ID: "graph", Type: "panel", Name: "Graph"}, - }, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Includes: []*plugins.Includes{ - { - Name: "Nginx Connections", - Path: "dashboards/connections.json", - Type: "dashboard", - Role: "Viewer", - Action: "plugins.app:access", - }, - { - Name: "Nginx Memory", - Path: "dashboards/memory.json", - Type: "dashboard", - Role: "Viewer", - Action: "plugins.app:access", - }, - {Name: "Nginx Panel", Type: "panel", Role: "Viewer", Action: "plugins.app:access"}, - {Name: "Nginx Datasource", Type: "datasource", Role: "Viewer", Action: "plugins.app:access"}, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "includes-symlinks")), - }, - }, - }, - }, - { - name: "Multiple plugin dirs", - pluginDirs: []string{"../../testdata/duplicate-plugins", "../../testdata/invalid-v1-signature"}, - expectedBundles: []*plugins.FoundBundle{ - { - Primary: plugins.FoundPlugin{ - JSONData: plugins.JSONData{ - ID: "test-app", - Type: plugins.TypeDataSource, - Name: "Parent", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Grafana Labs", - URL: "http://grafana.com", - }, - Description: "Parent plugin", - Version: "1.0.0", - Updated: "2020-10-20", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "duplicate-plugins/nested")), - }, - Children: []*plugins.FoundPlugin{ - { - JSONData: plugins.JSONData{ - ID: "test-app", - Type: plugins.TypeDataSource, - Name: "Child", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Grafana Labs", - URL: "http://grafana.com", - }, - Description: "Child plugin", - Version: "1.0.0", - Updated: "2020-10-20", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "duplicate-plugins/nested/nested")), - }, - }, - }, - { - Primary: plugins.FoundPlugin{ - JSONData: plugins.JSONData{ - ID: "test-datasource", - Type: plugins.TypeDataSource, - Name: "Test", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Grafana Labs", - URL: "https://grafana.com", - }, - Description: "Test", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - State: plugins.ReleaseStateAlpha, - Backend: true, - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "invalid-v1-signature/plugin")), - }, - }, - }, - }, - { - name: "Plugin with dist folder (core class)", - pluginDirs: []string{filepath.Join(testData, "plugin-with-dist")}, - pluginClass: plugins.ClassCore, - expectedBundles: []*plugins.FoundBundle{ - { - Primary: plugins.FoundPlugin{ - JSONData: plugins.JSONData{ - ID: "test-datasource", - Type: plugins.TypeDataSource, - Name: "Test", - Info: plugins.Info{ - Author: plugins.InfoLink{ - Name: "Will Browne", - URL: "https://willbrowne.com", - }, - Description: "Test", - Version: "1.0.0", - }, - Dependencies: plugins.Dependencies{ - GrafanaVersion: "*", - Plugins: []plugins.Dependency{}, - Extensions: plugins.ExtensionsDependencies{ - ExposedComponents: []string{}, - }, - }, - Extensions: plugins.Extensions{ - AddedLinks: []plugins.AddedLink{}, - AddedComponents: []plugins.AddedComponent{}, - AddedFunctions: []plugins.AddedFunction{}, - - ExposedComponents: []plugins.ExposedComponent{}, - ExtensionPoints: []plugins.ExtensionPoint{}, - }, - State: plugins.ReleaseStateAlpha, - Backend: true, - Executable: "test", - }, - FS: mustNewStaticFSForTests(t, filepath.Join(testData, "plugin-with-dist/plugin/dist")), - }, - }, - }, - }, - } - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - f := NewLocalFinder(false) - pluginBundles, err := f.Find(context.Background(), &fakes.FakePluginSource{ - PluginClassFunc: func(ctx context.Context) plugins.Class { - return tc.pluginClass - }, - PluginURIsFunc: func(ctx context.Context) []string { - return tc.pluginDirs - }, - }) - if (err != nil) && !errors.Is(err, tc.err) { - t.Errorf("Find() error = %v, expected error %v", err, tc.err) - return - } - - // to ensure we can compare with expected - sort.SliceStable(pluginBundles, func(i, j int) bool { - return pluginBundles[i].Primary.JSONData.ID < pluginBundles[j].Primary.JSONData.ID - }) - - if !cmp.Equal(pluginBundles, tc.expectedBundles, fsComparer) { - t.Fatalf("Result mismatch (-want +got):\n%s", cmp.Diff(pluginBundles, tc.expectedBundles, fsComparer)) - } - }) - } -} - -func TestFinder_getAbsPluginJSONPaths(t *testing.T) { - t.Run("When scanning a folder that doesn't exists shouldn't return an error", func(t *testing.T) { - origWalk := walk - walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error { - return walkFn(path, nil, os.ErrNotExist) - } - t.Cleanup(func() { - walk = origWalk - }) - - finder := NewLocalFinder(false) - paths, err := finder.getAbsPluginJSONPaths("test") - require.NoError(t, err) - require.Empty(t, paths) - }) - - t.Run("When scanning a folder that lacks permission shouldn't return an error", func(t *testing.T) { - origWalk := walk - walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error { - return walkFn(path, nil, os.ErrPermission) - } - t.Cleanup(func() { - walk = origWalk - }) - - finder := NewLocalFinder(false) - paths, err := finder.getAbsPluginJSONPaths("test") - require.NoError(t, err) - require.Empty(t, paths) - }) - - t.Run("When scanning a folder that returns a non-handled error should return that error", func(t *testing.T) { - origWalk := walk - walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error { - return walkFn(path, nil, errors.New("random error")) - } - t.Cleanup(func() { - walk = origWalk - }) - - finder := NewLocalFinder(false) - paths, err := finder.getAbsPluginJSONPaths("test") - require.Error(t, err) - require.Empty(t, paths) - }) -} - -var fsComparer = cmp.Comparer(func(fs1 plugins.FS, fs2 plugins.FS) bool { - fs1Files, err := fs1.Files() - if err != nil { - panic(err) - } - fs2Files, err := fs2.Files() - if err != nil { - panic(err) - } - - sort.SliceStable(fs1Files, func(i, j int) bool { - return fs1Files[i] < fs1Files[j] - }) - - sort.SliceStable(fs2Files, func(i, j int) bool { - return fs2Files[i] < fs2Files[j] - }) - - return cmp.Equal(fs1Files, fs2Files) && fs1.Base() == fs2.Base() -}) - -func mustNewStaticFSForTests(t *testing.T, dir string) plugins.FS { - sfs, err := plugins.NewStaticFS(plugins.NewLocalFS(dir)) - require.NoError(t, err) - return sfs -} diff --git a/pkg/plugins/manager/loader/loader_test.go b/pkg/plugins/manager/loader/loader_test.go index d1b6855f5a4..c51b200074c 100644 --- a/pkg/plugins/manager/loader/loader_test.go +++ b/pkg/plugins/manager/loader/loader_test.go @@ -451,9 +451,6 @@ func TestLoader_Load(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{"http://example.com"} - }, DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) { return plugins.Signature{}, false }, @@ -509,9 +506,6 @@ func TestLoader_Load(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{"http://example.com"} - }, DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) { return plugins.Signature{}, false }, @@ -571,9 +565,6 @@ func TestLoader_Load(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{"http://example.com"} - }, DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) { return plugins.Signature{}, false }, diff --git a/pkg/plugins/manager/pipeline/discovery/discovery.go b/pkg/plugins/manager/pipeline/discovery/discovery.go index f05f2582e23..e624fbb3fcc 100644 --- a/pkg/plugins/manager/pipeline/discovery/discovery.go +++ b/pkg/plugins/manager/pipeline/discovery/discovery.go @@ -13,62 +13,57 @@ type Discoverer interface { Discover(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) } -// FindFunc is the function used for the Find step of the Discovery stage. -type FindFunc func(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) - -// FindFilterFunc is the function used for the Filter step of the Discovery stage. -type FindFilterFunc func(ctx context.Context, class plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) +// FilterFunc is the function used for the Filter step of the Discovery stage. +type FilterFunc func(ctx context.Context, class plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) // Discovery implements the Discoverer interface. // // The Discovery stage is made up of the following steps (in order): -// - Find: Find plugins (from disk, remote, etc.) +// - Discover: Each source discovers its own plugins // - Filter: Filter the results based on some criteria. // -// The Find step is implemented by the FindFunc type. -// -// The Filter step is implemented by the FindFilterFunc type. +// The Filter step is implemented by the FilterFunc type. type Discovery struct { - findStep FindFunc - findFilterSteps []FindFilterFunc - log log.Logger + filterSteps []FilterFunc + log log.Logger } type Opts struct { - FindFunc FindFunc - FindFilterFuncs []FindFilterFunc + FilterFuncs []FilterFunc } // New returns a new Discovery stage. -func New(cfg *config.PluginManagementCfg, opts Opts) *Discovery { - if opts.FindFunc == nil { - opts.FindFunc = DefaultFindFunc(cfg) - } - - if opts.FindFilterFuncs == nil { - opts.FindFilterFuncs = []FindFilterFunc{} // no filters by default +func New(_ *config.PluginManagementCfg, opts Opts) *Discovery { + if opts.FilterFuncs == nil { + opts.FilterFuncs = []FilterFunc{} // no filters by default } return &Discovery{ - findStep: opts.FindFunc, - findFilterSteps: opts.FindFilterFuncs, - log: log.New("plugins.discovery"), + filterSteps: opts.FilterFuncs, + log: log.New("plugins.discovery"), } } -// Discover will execute the Find and Filter steps of the Discovery stage. +// Discover will execute the Filter step of the Discovery stage. func (d *Discovery) Discover(ctx context.Context, src plugins.PluginSource) ([]*plugins.FoundBundle, error) { - discoveredPlugins, err := d.findStep(ctx, src) + // Use the source's own Discover method + found, err := src.Discover(ctx) if err != nil { + d.log.Warn("Discovery source failed", "class", src.PluginClass(ctx), "error", err) return nil, err } - for _, filter := range d.findFilterSteps { - discoveredPlugins, err = filter(ctx, src.PluginClass(ctx), discoveredPlugins) + d.log.Debug("Found plugins", "class", src.PluginClass(ctx), "count", len(found)) + + // Apply filtering steps + result := found + for _, filter := range d.filterSteps { + result, err = filter(ctx, src.PluginClass(ctx), result) if err != nil { return nil, err } } - return discoveredPlugins, nil + d.log.Debug("Discovery complete", "class", src.PluginClass(ctx), "found", len(found), "filtered", len(result)) + return result, nil } diff --git a/pkg/plugins/manager/pipeline/discovery/steps.go b/pkg/plugins/manager/pipeline/discovery/steps.go index 6c5a1d07437..fbeae9f305c 100644 --- a/pkg/plugins/manager/pipeline/discovery/steps.go +++ b/pkg/plugins/manager/pipeline/discovery/steps.go @@ -5,24 +5,16 @@ import ( "slices" "github.com/grafana/grafana/pkg/plugins" - "github.com/grafana/grafana/pkg/plugins/config" - "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" ) -// DefaultFindFunc is the default function used for the Find step of the Discovery stage. It will scan the local -// filesystem for plugins. -func DefaultFindFunc(cfg *config.PluginManagementCfg) FindFunc { - return finder.NewLocalFinder(cfg.DevMode).Find -} - // PermittedPluginTypesFilter is a filter step that will filter out any plugins that are not of a permitted type. type PermittedPluginTypesFilter struct { permittedTypes []plugins.Type } -// NewPermittedPluginTypesFilterStep returns a new FindFilterFunc for filtering out any plugins that are not of a +// NewPermittedPluginTypesFilterStep returns a new FilterFunc for filtering out any plugins that are not of a // permitted type. This includes both the primary plugin and any child plugins. -func NewPermittedPluginTypesFilterStep(permittedTypes []plugins.Type) FindFilterFunc { +func NewPermittedPluginTypesFilterStep(permittedTypes []plugins.Type) FilterFunc { f := &PermittedPluginTypesFilter{ permittedTypes: permittedTypes, } diff --git a/pkg/plugins/manager/sources/source_local_disk.go b/pkg/plugins/manager/sources/source_local_disk.go index fc20b45499f..0ec55afbe0b 100644 --- a/pkg/plugins/manager/sources/source_local_disk.go +++ b/pkg/plugins/manager/sources/source_local_disk.go @@ -3,22 +3,51 @@ package sources import ( "context" "errors" + "fmt" + "io" "os" "path/filepath" "slices" + "strings" + "github.com/grafana/grafana/pkg/infra/fs" "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/util" +) + +var walk = util.Walk + +var ( + ErrInvalidPluginJSONFilePath = errors.New("invalid plugin.json filepath was provided") ) type LocalSource struct { - paths []string - class plugins.Class + paths []string + class plugins.Class + strictMode bool // If true, tracks files via a StaticFS + log log.Logger } +// NewLocalSource represents a plugin with a fixed set of files. func NewLocalSource(class plugins.Class, paths []string) *LocalSource { return &LocalSource{ - class: class, - paths: paths, + paths: paths, + class: class, + strictMode: true, + log: log.New("local.source"), + } +} + +// NewUnsafeLocalSource represents a plugin that has an unbounded set of files. This useful when running in +// dev mode whilst developing a plugin. +func NewUnsafeLocalSource(class plugins.Class, paths []string) *LocalSource { + return &LocalSource{ + paths: paths, + class: class, + strictMode: false, + log: log.New("local.source"), } } @@ -26,7 +55,9 @@ func (s *LocalSource) PluginClass(_ context.Context) plugins.Class { return s.class } -func (s *LocalSource) PluginURIs(_ context.Context) []string { +// Paths returns the file system paths that this source will search for plugins. +// This method is primarily intended for testing purposes. +func (s *LocalSource) Paths() []string { return s.paths } @@ -41,7 +72,189 @@ func (s *LocalSource) DefaultSignature(_ context.Context, _ string) (plugins.Sig } } -func DirAsLocalSources(pluginsPath string, class plugins.Class) ([]*LocalSource, error) { +func (s *LocalSource) Discover(_ context.Context) ([]*plugins.FoundBundle, error) { + if len(s.paths) == 0 { + return []*plugins.FoundBundle{}, nil + } + + pluginJSONPaths := make([]string, 0, len(s.paths)) + for _, path := range s.paths { + exists, err := fs.Exists(path) + if err != nil { + s.log.Warn("Skipping finding plugins as an error occurred", "path", path, "error", err) + continue + } + if !exists { + s.log.Warn("Skipping finding plugins as directory does not exist", "path", path) + continue + } + + paths, err := s.getAbsPluginJSONPaths(path) + if err != nil { + return nil, err + } + pluginJSONPaths = append(pluginJSONPaths, paths...) + } + + // load plugin.json files and map directory to JSON data + foundPlugins := make(map[string]plugins.JSONData) + for _, pluginJSONPath := range pluginJSONPaths { + plugin, err := s.readPluginJSON(pluginJSONPath) + if err != nil { + s.log.Warn("Skipping plugin loading as its plugin.json could not be read", "path", pluginJSONPath, "error", err) + continue + } + + pluginJSONAbsPath, err := filepath.Abs(pluginJSONPath) + if err != nil { + s.log.Warn("Skipping plugin loading as absolute plugin.json path could not be calculated", "pluginId", plugin.ID, "error", err) + continue + } + + foundPlugins[filepath.Dir(pluginJSONAbsPath)] = plugin + } + + res := make(map[string]*plugins.FoundBundle) + for pluginDir, data := range foundPlugins { + var pluginFs plugins.FS + pluginFs = plugins.NewLocalFS(pluginDir) + if s.strictMode { + // Tighten up security by allowing access only to the files present up to this point. + // Any new file "sneaked in" won't be allowed and will act as if the file does not exist. + var err error + pluginFs, err = plugins.NewStaticFS(pluginFs) + if err != nil { + return nil, err + } + } + res[pluginDir] = &plugins.FoundBundle{ + Primary: plugins.FoundPlugin{ + JSONData: data, + FS: pluginFs, + }, + } + } + + // Track child plugins and add them to their parent. + childPlugins := make(map[string]struct{}) + for dir, p := range res { + // Check if this plugin is the parent of another plugin. + for dir2, p2 := range res { + if dir == dir2 { + continue + } + + relPath, err := filepath.Rel(dir, dir2) + if err != nil { + s.log.Error("Cannot calculate relative path. Skipping", "pluginId", p2.Primary.JSONData.ID, "err", err) + continue + } + if !strings.Contains(relPath, "..") { + child := p2.Primary + s.log.Debug("Adding child", "parent", p.Primary.JSONData.ID, "child", child.JSONData.ID, "relPath", relPath) + p.Children = append(p.Children, &child) + childPlugins[dir2] = struct{}{} + } + } + } + + // Remove child plugins from the result (they are already tracked via their parent). + result := make([]*plugins.FoundBundle, 0, len(res)) + for k := range res { + if _, ok := childPlugins[k]; !ok { + result = append(result, res[k]) + } + } + + return result, nil +} + +func (s *LocalSource) readPluginJSON(pluginJSONPath string) (plugins.JSONData, error) { + reader, err := s.readFile(pluginJSONPath) + defer func() { + if reader == nil { + return + } + if err = reader.Close(); err != nil { + s.log.Warn("Failed to close plugin JSON file", "path", pluginJSONPath, "error", err) + } + }() + if err != nil { + s.log.Warn("Skipping plugin loading as its plugin.json could not be read", "path", pluginJSONPath, "error", err) + return plugins.JSONData{}, err + } + plugin, err := plugins.ReadPluginJSON(reader) + if err != nil { + s.log.Warn("Skipping plugin loading as its plugin.json could not be read", "path", pluginJSONPath, "error", err) + return plugins.JSONData{}, err + } + + return plugin, nil +} + +func (s *LocalSource) getAbsPluginJSONPaths(path string) ([]string, error) { + var pluginJSONPaths []string + + var err error + path, err = filepath.Abs(path) + if err != nil { + return []string{}, err + } + + if err = walk(path, true, true, + func(currentPath string, fi os.FileInfo, err error) error { + if err != nil { + if errors.Is(err, os.ErrNotExist) { + s.log.Error("Couldn't scan directory since it doesn't exist", "pluginDir", path, "error", err) + return nil + } + if errors.Is(err, os.ErrPermission) { + s.log.Error("Couldn't scan directory due to lack of permissions", "pluginDir", path, "error", err) + return nil + } + + return fmt.Errorf("filepath.Walk reported an error for %q: %w", currentPath, err) + } + + if fi.Name() == "node_modules" { + return util.ErrWalkSkipDir + } + + if fi.IsDir() { + return nil + } + + if fi.Name() != "plugin.json" { + return nil + } + + pluginJSONPaths = append(pluginJSONPaths, currentPath) + return nil + }); err != nil { + return []string{}, err + } + + return pluginJSONPaths, nil +} + +func (s *LocalSource) readFile(pluginJSONPath string) (io.ReadCloser, error) { + s.log.Debug("Loading plugin", "path", pluginJSONPath) + + if !strings.EqualFold(filepath.Ext(pluginJSONPath), ".json") { + return nil, ErrInvalidPluginJSONFilePath + } + + absPluginJSONPath, err := filepath.Abs(pluginJSONPath) + if err != nil { + return nil, err + } + + // Wrapping in filepath.Clean to properly handle + // gosec G304 Potential file inclusion via variable rule. + return os.Open(filepath.Clean(absPluginJSONPath)) +} + +func DirAsLocalSources(cfg *config.PluginManagementCfg, pluginsPath string, class plugins.Class) ([]*LocalSource, error) { if pluginsPath == "" { return []*LocalSource{}, errors.New("plugins path not configured") } @@ -64,7 +277,11 @@ func DirAsLocalSources(pluginsPath string, class plugins.Class) ([]*LocalSource, sources := make([]*LocalSource, len(pluginDirs)) for i, dir := range pluginDirs { - sources[i] = NewLocalSource(class, []string{dir}) + if cfg.DevMode { + sources[i] = NewUnsafeLocalSource(class, []string{dir}) + } else { + sources[i] = 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 index 0a903c4c4cc..5ec0855ad13 100644 --- a/pkg/plugins/manager/sources/source_local_disk_test.go +++ b/pkg/plugins/manager/sources/source_local_disk_test.go @@ -5,17 +5,23 @@ import ( "path/filepath" "testing" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/config" ) +var compareOpts = []cmp.Option{cmpopts.IgnoreFields(LocalSource{}, "log"), cmp.AllowUnexported(LocalSource{})} + func TestDirAsLocalSources(t *testing.T) { testdataDir := "../testdata" tests := []struct { name string pluginsPath string + cfg *config.PluginManagementCfg expected []*LocalSource err error }{ @@ -28,44 +34,79 @@ func TestDirAsLocalSources(t *testing.T) { { name: "Directory with subdirectories", pluginsPath: filepath.Join(testdataDir, "pluginRootWithDist"), + cfg: &config.PluginManagementCfg{}, expected: []*LocalSource{ { - paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "datasource")}, - class: plugins.ClassExternal, + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "datasource")}, + strictMode: true, + class: plugins.ClassExternal, }, { - paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "dist")}, - class: plugins.ClassExternal, + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "dist")}, + strictMode: true, + class: plugins.ClassExternal, }, { - paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "panel")}, - class: plugins.ClassExternal, + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "panel")}, + strictMode: true, + class: plugins.ClassExternal, + }, + }, + }, + { + name: "Dev mode disables strict mode for source", + cfg: &config.PluginManagementCfg{ + DevMode: true, + }, + pluginsPath: filepath.Join(testdataDir, "pluginRootWithDist"), + expected: []*LocalSource{ + { + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "datasource")}, + class: plugins.ClassExternal, + strictMode: false, + }, + { + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "dist")}, + class: plugins.ClassExternal, + strictMode: false, + }, + { + paths: []string{filepath.Join(testdataDir, "pluginRootWithDist", "panel")}, + class: plugins.ClassExternal, + strictMode: false, }, }, }, { name: "Directory with no subdirectories", + cfg: &config.PluginManagementCfg{}, pluginsPath: filepath.Join(testdataDir, "pluginRootWithDist", "datasource"), expected: []*LocalSource{}, }, { name: "Directory with a symlink to a directory", pluginsPath: filepath.Join(testdataDir, "symbolic-plugin-dirs"), + cfg: &config.PluginManagementCfg{}, expected: []*LocalSource{ { - paths: []string{filepath.Join(testdataDir, "symbolic-plugin-dirs", "plugin")}, - class: plugins.ClassExternal, + paths: []string{filepath.Join(testdataDir, "symbolic-plugin-dirs", "plugin")}, + class: plugins.ClassExternal, + strictMode: true, }, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := DirAsLocalSources(tt.pluginsPath, plugins.ClassExternal) + got, err := DirAsLocalSources(tt.cfg, tt.pluginsPath, plugins.ClassExternal) if tt.err != nil { require.Errorf(t, err, tt.err.Error()) + return + } + require.NoError(t, err) + if !cmp.Equal(got, tt.expected, compareOpts...) { + t.Fatalf("Result mismatch (-want +got):\n%s", cmp.Diff(got, tt.expected, compareOpts...)) } - require.Equal(t, tt.expected, got) }) } } diff --git a/pkg/plugins/manager/sources/sources.go b/pkg/plugins/manager/sources/sources.go index 767d0aedeb5..addeede1dd7 100644 --- a/pkg/plugins/manager/sources/sources.go +++ b/pkg/plugins/manager/sources/sources.go @@ -5,25 +5,32 @@ import ( "path/filepath" "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 { - cfg *setting.Cfg + cfg *config.PluginManagementCfg + staticRootPath string + log log.Logger } -func ProvideService(cfg *setting.Cfg) *Service { +func ProvideService(cfg *setting.Cfg, pCcfg *config.PluginManagementCfg) *Service { return &Service{ - cfg: cfg, - log: log.New("plugin.sources"), + cfg: pCcfg, + staticRootPath: cfg.StaticRootPath, + log: log.New("plugin.sources"), } } func (s *Service) List(_ context.Context) []plugins.PluginSource { r := []plugins.PluginSource{ - NewLocalSource(plugins.ClassCore, corePluginPaths(s.cfg.StaticRootPath)), + NewLocalSource( + plugins.ClassCore, + s.corePluginPaths(), + ), } r = append(r, s.externalPluginSources()...) r = append(r, s.pluginSettingSources()...) @@ -31,7 +38,7 @@ func (s *Service) List(_ context.Context) []plugins.PluginSource { } func (s *Service) externalPluginSources() []plugins.PluginSource { - localSrcs, err := DirAsLocalSources(s.cfg.PluginsPath, plugins.ClassExternal) + localSrcs, err := DirAsLocalSources(s.cfg, s.cfg.PluginsPath, plugins.ClassExternal) if err != nil { s.log.Error("Failed to load external plugins", "error", err) return []plugins.PluginSource{} @@ -52,16 +59,19 @@ func (s *Service) pluginSettingSources() []plugins.PluginSource { if !exists || path == "" { continue } - - sources = append(sources, NewLocalSource(plugins.ClassExternal, []string{path})) + if s.cfg.DevMode { + sources = append(sources, NewUnsafeLocalSource(plugins.ClassExternal, []string{path})) + } else { + sources = append(sources, NewLocalSource(plugins.ClassExternal, []string{path})) + } } 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") +func (s *Service) corePluginPaths() []string { + datasourcePaths := filepath.Join(s.staticRootPath, "app", "plugins", "datasource") + panelsPath := filepath.Join(s.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 898aedff819..f56fbffa52b 100644 --- a/pkg/plugins/manager/sources/sources_test.go +++ b/pkg/plugins/manager/sources/sources_test.go @@ -8,6 +8,7 @@ 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" ) @@ -18,7 +19,10 @@ func TestSources_List(t *testing.T) { cfg := &setting.Cfg{ StaticRootPath: testdata, - PluginsPath: filepath.Join(testdata, "pluginRootWithDist"), + } + + pCfg := &config.PluginManagementCfg{ + PluginsPath: filepath.Join(testdata, "pluginRootWithDist"), PluginSettings: setting.PluginSettings{ "foo": map[string]string{ "path": filepath.Join(testdata, "test-app"), @@ -29,7 +33,7 @@ func TestSources_List(t *testing.T) { }, } - s := ProvideService(cfg) + s := ProvideService(cfg, pCfg) srcs := s.List(context.Background()) ctx := context.Background() @@ -37,10 +41,14 @@ func TestSources_List(t *testing.T) { require.Len(t, srcs, 5) require.Equal(t, srcs[0].PluginClass(ctx), plugins.ClassCore) - require.Equal(t, srcs[0].PluginURIs(ctx), []string{ - filepath.Join(testdata, "app", "plugins", "datasource"), - filepath.Join(testdata, "app", "plugins", "panel"), - }) + if localSrc, ok := srcs[0].(*LocalSource); ok { + require.Equal(t, localSrc.Paths(), []string{ + filepath.Join(testdata, "app", "plugins", "datasource"), + filepath.Join(testdata, "app", "plugins", "panel"), + }) + } else { + t.Fatalf("Expected LocalSource, got %T", srcs[0]) + } sig, exists := srcs[0].DefaultSignature(ctx, "") require.True(t, exists) require.Equal(t, plugins.SignatureStatusInternal, sig.Status) @@ -48,25 +56,37 @@ func TestSources_List(t *testing.T) { require.Equal(t, "", sig.SigningOrg) require.Equal(t, srcs[1].PluginClass(ctx), plugins.ClassExternal) - require.Equal(t, srcs[1].PluginURIs(ctx), []string{ - filepath.Join(testdata, "pluginRootWithDist", "datasource"), - }) + if localSrc, ok := srcs[1].(*LocalSource); ok { + require.Equal(t, localSrc.Paths(), []string{ + filepath.Join(testdata, "pluginRootWithDist", "datasource"), + }) + } else { + t.Fatalf("Expected LocalSource, got %T", srcs[1]) + } 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{ - filepath.Join(testdata, "pluginRootWithDist", "dist"), - }) + if localSrc, ok := srcs[2].(*LocalSource); ok { + require.Equal(t, localSrc.Paths(), []string{ + filepath.Join(testdata, "pluginRootWithDist", "dist"), + }) + } else { + t.Fatalf("Expected LocalSource, got %T", srcs[2]) + } 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", "panel"), - }) + if localSrc, ok := srcs[3].(*LocalSource); ok { + require.Equal(t, localSrc.Paths(), []string{ + filepath.Join(testdata, "pluginRootWithDist", "panel"), + }) + } else { + t.Fatalf("Expected LocalSource, got %T", srcs[3]) + } sig, exists = srcs[3].DefaultSignature(ctx, "") require.False(t, exists) require.Equal(t, plugins.Signature{}, sig) @@ -78,19 +98,25 @@ func TestSources_List(t *testing.T) { cfg := &setting.Cfg{ StaticRootPath: testdata, - PluginsPath: filepath.Join(testdata, "symbolic-plugin-dirs"), } - s := ProvideService(cfg) + + pCfg := &config.PluginManagementCfg{ + PluginsPath: filepath.Join(testdata, "symbolic-plugin-dirs"), + } + + s := ProvideService(cfg, pCfg) ctx := context.Background() srcs := s.List(ctx) uris := map[plugins.Class]map[string]struct{}{} - for _, s := range srcs { - class := s.PluginClass(ctx) + for _, src := range srcs { + class := src.PluginClass(ctx) if _, exists := uris[class]; !exists { uris[class] = map[string]struct{}{} } - for _, uri := range s.PluginURIs(ctx) { - uris[class][uri] = struct{}{} + if localSrc, ok := src.(*LocalSource); ok { + for _, path := range localSrc.Paths() { + uris[class][path] = struct{}{} + } } } diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index ceb9bcba269..bca3a42339b 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -21,7 +21,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/fakes" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" - "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/process" "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/manager/signature" @@ -566,9 +565,7 @@ func TestLoader_Load_ExternalRegistration(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return pluginPaths - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, pluginPaths).Discover, DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) { return plugins.Signature{}, false }, @@ -650,9 +647,7 @@ func TestLoader_Load_CustomSource(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return pluginPaths - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, pluginPaths).Discover, DefaultSignatureFunc: func(ctx context.Context) (plugins.Signature, bool) { return plugins.Signature{ Status: plugins.SignatureStatusValid, @@ -754,9 +749,7 @@ func TestLoader_Load_MultiplePlugins(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return tt.pluginPaths - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, tt.pluginPaths).Discover, }) require.NoError(t, err) sort.SliceStable(got, func(i, j int) bool { @@ -866,9 +859,7 @@ func TestLoader_Load_RBACReady(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return tt.pluginPaths - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, tt.pluginPaths).Discover, }) require.NoError(t, err) @@ -933,9 +924,7 @@ func TestLoader_Load_Signature_RootURL(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "valid-v2-pvt-signature-root-url-uri")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "valid-v2-pvt-signature-root-url-uri")}).Discover, }) require.NoError(t, err) @@ -1021,9 +1010,7 @@ func TestLoader_Load_DuplicatePlugins(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "test-app"), filepath.Join(testDataDir(t), "test-app")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "test-app"), filepath.Join(testDataDir(t), "test-app")}).Discover, }) require.NoError(t, err) @@ -1122,9 +1109,7 @@ func TestLoader_Load_SkipUninitializedPlugins(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{pluginDir1, pluginDir2} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{pluginDir1, pluginDir2}).Discover, }) require.NoError(t, err) @@ -1163,9 +1148,7 @@ func TestLoader_AngularClass(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return tc.class }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "valid-v2-signature")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "valid-v2-signature")}).Discover, } // if angularDetected = true, it means that the detection has run l := newLoaderWithOpts(t, &config.PluginManagementCfg{}, loaderDepOpts{ @@ -1188,9 +1171,7 @@ func TestLoader_Load_Angular(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "valid-v2-signature")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "valid-v2-signature")}).Discover, } for _, cfgTc := range []struct { name string @@ -1238,9 +1219,7 @@ func TestLoader_HideAngularDeprecation(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "valid-v2-signature")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "valid-v2-signature")}).Discover, } for _, tc := range []struct { name string @@ -1369,9 +1348,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "nested-plugins")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "nested-plugins")}).Discover, }) require.NoError(t, err) @@ -1392,9 +1369,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "nested-plugins")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "nested-plugins")}).Discover, }) require.NoError(t, err) @@ -1571,9 +1546,7 @@ func TestLoader_Load_NestedPlugins(t *testing.T) { PluginClassFunc: func(ctx context.Context) plugins.Class { return plugins.ClassExternal }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{filepath.Join(testDataDir(t), "app-with-child")} - }, + DiscoverFunc: sources.NewLocalSource(plugins.ClassExternal, []string{filepath.Join(testDataDir(t), "app-with-child")}).Discover, }) require.NoError(t, err) @@ -1605,8 +1578,7 @@ func newLoader(t *testing.T, cfg *config.PluginManagementCfg, reg registry.Servi terminate, err := pipeline.ProvideTerminationStage(cfg, reg, proc) require.NoError(t, err) - return ProvideService(cfg, pipeline.ProvideDiscoveryStage(cfg, - finder.NewLocalFinder(false), reg), + return ProvideService(cfg, pipeline.ProvideDiscoveryStage(cfg, reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), pipeline.ProvideInitializationStage(cfg, reg, backendFactory, proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), @@ -1637,8 +1609,7 @@ func newLoaderWithOpts(t *testing.T, cfg *config.PluginManagementCfg, opts loade backendFactoryProvider = fakes.NewFakeBackendProcessProvider() } - return ProvideService(cfg, pipeline.ProvideDiscoveryStage(cfg, - finder.NewLocalFinder(false), reg), + return ProvideService(cfg, pipeline.ProvideDiscoveryStage(cfg, reg), pipeline.ProvideBootstrapStage(cfg, signature.DefaultCalculator(cfg), assets), pipeline.ProvideValidationStage(cfg, signature.NewValidator(signature.NewUnsignedAuthorizer(cfg)), angularInspector), pipeline.ProvideInitializationStage(cfg, reg, backendFactoryProvider, proc, authServiceRegistry, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), fakes.NewFakePluginEnvProvider(), tracing.InitializeTracerForTest()), diff --git a/pkg/services/pluginsintegration/pipeline/pipeline.go b/pkg/services/pluginsintegration/pipeline/pipeline.go index c678f1f2bc0..2ffba23f4b5 100644 --- a/pkg/services/pluginsintegration/pipeline/pipeline.go +++ b/pkg/services/pluginsintegration/pipeline/pipeline.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/envvars" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" - "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization" @@ -22,10 +21,9 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" ) -func ProvideDiscoveryStage(cfg *config.PluginManagementCfg, pf finder.Finder, pr registry.Service) *discovery.Discovery { +func ProvideDiscoveryStage(cfg *config.PluginManagementCfg, pr registry.Service) *discovery.Discovery { return discovery.New(cfg, discovery.Opts{ - FindFunc: pf.Find, - FindFilterFuncs: []discovery.FindFilterFunc{ + FilterFuncs: []discovery.FilterFunc{ discovery.NewPermittedPluginTypesFilterStep([]plugins.Type{ plugins.TypeDataSource, plugins.TypeApp, plugins.TypePanel, }), diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index ecb4d32ed94..2bc2415b074 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -18,7 +18,6 @@ import ( pluginLoader "github.com/grafana/grafana/pkg/plugins/manager/loader" pAngularInspector "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" - "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization" @@ -144,8 +143,6 @@ var WireExtensionSet = wire.NewSet( wire.Bind(new(plugins.BackendFactoryProvider), new(*provider.Service)), signature.ProvideOSSAuthorizer, wire.Bind(new(plugins.PluginLoaderAuthorizer), new(*signature.UnsignedPluginAuthorizer)), - finder.ProvideLocalFinder, - wire.Bind(new(finder.Finder), new(*finder.Local)), ProvideClientWithMiddlewares, wire.Bind(new(plugins.Client), new(*backend.MiddlewareHandler)), managedplugins.NewNoop, diff --git a/pkg/services/pluginsintegration/pluginstore/store_test.go b/pkg/services/pluginsintegration/pluginstore/store_test.go index cead243e33e..b0c0b408cf9 100644 --- a/pkg/services/pluginsintegration/pluginstore/store_test.go +++ b/pkg/services/pluginsintegration/pluginstore/store_test.go @@ -15,10 +15,10 @@ import ( func TestStore_ProvideService(t *testing.T) { t.Run("Plugin sources are added in order", func(t *testing.T) { - var addedPaths []string + var loadedSrcs []plugins.Class l := &fakes.FakeLoader{ LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { - addedPaths = append(addedPaths, src.PluginURIs(ctx)...) + loadedSrcs = append(loadedSrcs, src.PluginClass(ctx)) return nil, nil }, } @@ -27,18 +27,17 @@ func TestStore_ProvideService(t *testing.T) { return []plugins.PluginSource{ &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { - return "foobar" - }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{"path1"} + return "1" }, }, &fakes.FakePluginSource{ PluginClassFunc: func(ctx context.Context) plugins.Class { - return plugins.ClassExternal + return "2" }, - PluginURIsFunc: func(ctx context.Context) []string { - return []string{"path2", "path3"} + }, + &fakes.FakePluginSource{ + PluginClassFunc: func(ctx context.Context) plugins.Class { + return "3" }, }, } @@ -46,7 +45,7 @@ func TestStore_ProvideService(t *testing.T) { _, err := ProvideService(fakes.NewFakePluginRegistry(), srcs, l) require.NoError(t, err) - require.Equal(t, []string{"path1", "path2", "path3"}, addedPaths) + require.Equal(t, []plugins.Class{"1", "2", "3"}, loadedSrcs) }) } diff --git a/pkg/services/pluginsintegration/plugintest/plugins_test.go b/pkg/services/pluginsintegration/plugintest/plugins_test.go index 9b5b32244de..3516f298b81 100644 --- a/pkg/services/pluginsintegration/plugintest/plugins_test.go +++ b/pkg/services/pluginsintegration/plugintest/plugins_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "gopkg.in/ini.v1" - "github.com/grafana/grafana-azure-sdk-go/v2/azsettings" "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" @@ -130,21 +129,26 @@ func TestIntegrationPluginManager(t *testing.T) { staticRootPath, err := filepath.Abs("../../../../public/") 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: ini.Empty(), + Raw: raw, StaticRootPath: staticRootPath, - Azure: &azsettings.AzureSettings{}, - PluginSettings: map[string]map[string]string{ - "test-app": { - "path": "../../../plugins/manager/testdata/test-app", - }, - "test-panel": { - "not": "included", - }, - }, } - tracer := tracing.InitializeTracerForTest() hcp := httpclient.NewProvider() diff --git a/pkg/services/pluginsintegration/renderer/renderer.go b/pkg/services/pluginsintegration/renderer/renderer.go index 3bb20377fe6..c061693879b 100644 --- a/pkg/services/pluginsintegration/renderer/renderer.go +++ b/pkg/services/pluginsintegration/renderer/renderer.go @@ -4,6 +4,8 @@ import ( "context" "errors" + "go.opentelemetry.io/otel/trace" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" @@ -23,7 +25,6 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pipeline" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" "github.com/grafana/grafana/pkg/services/rendering" - "go.opentelemetry.io/otel/trace" ) func ProvideService(cfg *config.PluginManagementCfg, pluginEnvProvider envvars.Provider, @@ -84,7 +85,7 @@ func (m *Manager) Renderer(ctx context.Context) (rendering.Plugin, bool) { return m.renderer, true } - srcs, err := sources.DirAsLocalSources(m.cfg.PluginsPath, plugins.ClassExternal) + srcs, err := sources.DirAsLocalSources(m.cfg, m.cfg.PluginsPath, plugins.ClassExternal) if err != nil { m.log.Error("Failed to get renderer plugin sources", "error", err) return nil, false @@ -109,7 +110,7 @@ func (m *Manager) Renderer(ctx context.Context) (rendering.Plugin, bool) { func createLoader(cfg *config.PluginManagementCfg, pluginEnvProvider envvars.Provider, pr registry.Service, tracer trace.Tracer) (loader.Service, error) { d := discovery.New(cfg, discovery.Opts{ - FindFilterFuncs: []discovery.FindFilterFunc{ + FilterFuncs: []discovery.FilterFunc{ discovery.NewPermittedPluginTypesFilterStep([]plugins.Type{plugins.TypeRenderer}), func(ctx context.Context, class plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { return pipeline.NewDuplicatePluginIDFilterStep(pr).Filter(ctx, bundles) diff --git a/pkg/services/pluginsintegration/renderer/renderer_test.go b/pkg/services/pluginsintegration/renderer/renderer_test.go index 755622d2e52..2cf85ddad22 100644 --- a/pkg/services/pluginsintegration/renderer/renderer_test.go +++ b/pkg/services/pluginsintegration/renderer/renderer_test.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/config" "github.com/grafana/grafana/pkg/plugins/manager/fakes" + "github.com/grafana/grafana/pkg/plugins/manager/sources" ) func TestRenderer(t *testing.T) { @@ -22,8 +23,14 @@ func TestRenderer(t *testing.T) { 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)) + + if localSrc, ok := src.(*sources.LocalSource); ok { + paths := localSrc.Paths() + require.Len(t, paths, 1) + require.True(t, strings.HasPrefix(paths[0], testdataDir)) + } else { + t.Fatalf("Expected LocalSource, got %T", src) + } numLoaded++ return []*plugins.Plugin{}, nil @@ -55,9 +62,16 @@ func TestRenderer(t *testing.T) { 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 + + if localSrc, ok := src.(*sources.LocalSource); ok { + paths := localSrc.Paths() + if strings.HasPrefix(paths[0], filepath.Join(testdataDir, "renderer")) { + return []*plugins.Plugin{p}, nil + } + } else { + t.Fatalf("Expected LocalSource, got %T", src) } + return []*plugins.Plugin{}, nil }, UnloadFunc: func(_ context.Context, _ *plugins.Plugin) (*plugins.Plugin, error) { diff --git a/pkg/services/pluginsintegration/test_helper.go b/pkg/services/pluginsintegration/test_helper.go index 92ce0dab164..e7c432f9189 100644 --- a/pkg/services/pluginsintegration/test_helper.go +++ b/pkg/services/pluginsintegration/test_helper.go @@ -16,7 +16,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/loader" "github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector" "github.com/grafana/grafana/pkg/plugins/manager/loader/assetpath" - "github.com/grafana/grafana/pkg/plugins/manager/loader/finder" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/bootstrap" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/discovery" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization" @@ -51,7 +50,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core angularInspector := angularinspector.NewStaticInspector() proc := process.ProvideService() - disc := pipeline.ProvideDiscoveryStage(pCfg, finder.NewLocalFinder(true), reg) + disc := pipeline.ProvideDiscoveryStage(pCfg, reg) boot := pipeline.ProvideBootstrapStage(pCfg, signature.ProvideService(pCfg, statickey.New()), assetpath.ProvideService(pCfg, cdn)) valid := pipeline.ProvideValidationStage(pCfg, signature.NewValidator(signature.NewUnsignedAuthorizer(pCfg)), angularInspector) init := pipeline.ProvideInitializationStage(pCfg, reg, provider.ProvideService(coreRegistry), proc, &fakes.FakeAuthService{}, fakes.NewFakeRoleRegistry(), fakes.NewFakeActionSetRegistry(), nil, tracing.InitializeTracerForTest()) @@ -66,7 +65,7 @@ func CreateIntegrationTestCtx(t *testing.T, cfg *setting.Cfg, coreRegistry *core Terminator: term, }) - ps, err := pluginstore.ProvideService(reg, sources.ProvideService(cfg), l) + ps, err := pluginstore.ProvideService(reg, sources.ProvideService(cfg, pCfg), l) require.NoError(t, err) return &IntegrationTestCtx{ @@ -86,7 +85,7 @@ type LoaderOpts struct { func CreateTestLoader(t *testing.T, cfg *pluginsCfg.PluginManagementCfg, opts LoaderOpts) *loader.Loader { if opts.Discoverer == nil { - opts.Discoverer = pipeline.ProvideDiscoveryStage(cfg, finder.NewLocalFinder(cfg.DevMode), registry.ProvideService()) + opts.Discoverer = pipeline.ProvideDiscoveryStage(cfg, registry.ProvideService()) } if opts.Bootstrapper == nil {