From 788b9afda316c379a72c4ea8f38d7ba494de80b1 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Mon, 12 Feb 2024 12:47:49 +0100 Subject: [PATCH] Plugins: Make it possible to support multiple plugin versions (#82116) * first pass * use version in more places * add comment * update installer * fix wire * fix tests * tidy * simplify changes * fix in mem * remove unused step * fix step dupe logic for child plugins + add tests --- pkg/api/fakes.go | 2 +- pkg/api/plugins.go | 23 +++-- pkg/api/plugins_test.go | 30 +++--- pkg/plugins/ifaces.go | 4 +- pkg/plugins/manager/client/client.go | 18 ++-- pkg/plugins/manager/fakes/fakes.go | 10 +- pkg/plugins/manager/filestore/fs.go | 4 +- pkg/plugins/manager/installer.go | 12 +-- pkg/plugins/manager/installer_test.go | 12 +-- .../manager/pipeline/discovery/steps.go | 40 -------- .../manager/pipeline/termination/steps.go | 2 +- pkg/plugins/manager/registry/ifaces.go | 6 +- pkg/plugins/manager/registry/in_memory.go | 5 +- .../manager/registry/in_memory_test.go | 66 ++++++++++--- .../clientmiddleware/metrics_middleware.go | 8 +- .../dashboards/filestore.go | 6 +- .../dashboards/filestore_test.go | 2 +- .../pluginsintegration/loader/loader_test.go | 2 +- .../pluginsintegration/pipeline/pipeline.go | 2 +- .../pluginsintegration/pipeline/steps.go | 53 ++++++++++ .../pluginsintegration/pipeline/steps_test.go | 97 +++++++++++++++++++ .../plugins_integration_test.go | 13 ++- .../pluginsintegration/pluginstore/store.go | 3 +- .../pluginsintegration/renderer/renderer.go | 3 +- 24 files changed, 292 insertions(+), 131 deletions(-) diff --git a/pkg/api/fakes.go b/pkg/api/fakes.go index 209a27f0304..28b1a747443 100644 --- a/pkg/api/fakes.go +++ b/pkg/api/fakes.go @@ -30,7 +30,7 @@ func (pm *fakePluginInstaller) Add(_ context.Context, pluginID, version string, return nil } -func (pm *fakePluginInstaller) Remove(_ context.Context, pluginID string) error { +func (pm *fakePluginInstaller) Remove(_ context.Context, pluginID, _ string) error { delete(pm.plugins, pluginID) return nil } diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index f8cbf7ac66e..c8e9d1b48a5 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -274,7 +274,12 @@ func (hs *HTTPServer) GetPluginMarkdown(c *contextmodel.ReqContext) response.Res pluginID := web.Params(c.Req)[":pluginId"] name := web.Params(c.Req)[":name"] - content, err := hs.pluginMarkdown(c.Req.Context(), pluginID, name) + p, exists := hs.pluginStore.Plugin(c.Req.Context(), pluginID) + if !exists { + return response.Error(http.StatusNotFound, "Plugin not installed", nil) + } + + content, err := hs.pluginMarkdown(c.Req.Context(), pluginID, p.Info.Version, name) if err != nil { var notFound plugins.NotFoundError if errors.As(err, ¬Found) { @@ -286,7 +291,7 @@ func (hs *HTTPServer) GetPluginMarkdown(c *contextmodel.ReqContext) response.Res // fallback try readme if len(content) == 0 { - content, err = hs.pluginMarkdown(c.Req.Context(), pluginID, "readme") + content, err = hs.pluginMarkdown(c.Req.Context(), pluginID, p.Info.Version, "readme") if err != nil { if errors.Is(err, plugins.ErrFileNotExist) { return response.Error(http.StatusNotFound, plugins.ErrFileNotExist.Error(), nil) @@ -354,7 +359,7 @@ func (hs *HTTPServer) getPluginAssets(c *contextmodel.ReqContext) { // serveLocalPluginAsset returns the content of a plugin asset file from the local filesystem to the http client. func (hs *HTTPServer) serveLocalPluginAsset(c *contextmodel.ReqContext, plugin pluginstore.Plugin, assetPath string) { - f, err := hs.pluginFileStore.File(c.Req.Context(), plugin.ID, assetPath) + f, err := hs.pluginFileStore.File(c.Req.Context(), plugin.ID, plugin.Info.Version, assetPath) if err != nil { if errors.Is(err, plugins.ErrFileNotExist) { c.JsonApiErr(404, "Plugin file not found", nil) @@ -476,8 +481,12 @@ func (hs *HTTPServer) InstallPlugin(c *contextmodel.ReqContext) response.Respons func (hs *HTTPServer) UninstallPlugin(c *contextmodel.ReqContext) response.Response { pluginID := web.Params(c.Req)[":pluginId"] + plugin, exists := hs.pluginStore.Plugin(c.Req.Context(), pluginID) + if !exists { + return response.Error(http.StatusNotFound, "Plugin not installed", nil) + } - err := hs.pluginInstaller.Remove(c.Req.Context(), pluginID) + err := hs.pluginInstaller.Remove(c.Req.Context(), pluginID, plugin.Info.Version) if err != nil { if errors.Is(err, plugins.ErrPluginNotInstalled) { return response.Error(http.StatusNotFound, "Plugin not installed", err) @@ -494,19 +503,19 @@ func translatePluginRequestErrorToAPIError(err error) response.Response { return response.ErrOrFallback(http.StatusInternalServerError, "Plugin request failed", err) } -func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginID string, name string) ([]byte, error) { +func (hs *HTTPServer) pluginMarkdown(ctx context.Context, pluginID, pluginVersion, name string) ([]byte, error) { file, err := mdFilepath(strings.ToUpper(name)) if err != nil { return make([]byte, 0), err } - md, err := hs.pluginFileStore.File(ctx, pluginID, file) + md, err := hs.pluginFileStore.File(ctx, pluginID, pluginVersion, file) if err != nil { if errors.Is(err, plugins.ErrPluginNotInstalled) { return make([]byte, 0), plugins.NotFoundError{PluginID: pluginID} } - md, err = hs.pluginFileStore.File(ctx, pluginID, strings.ToLower(file)) + md, err = hs.pluginFileStore.File(ctx, pluginID, pluginVersion, strings.ToLower(file)) if err != nil { return make([]byte, 0), nil } diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index 29eefb94762..96fb1078a14 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -49,6 +49,7 @@ func Test_PluginsInstallAndUninstall(t *testing.T) { canInstall := []ac.Permission{{Action: pluginaccesscontrol.ActionInstall}} cannotInstall := []ac.Permission{{Action: "plugins:cannotinstall"}} + pluginID := "grafana-test-datasource" localOrg := int64(1) globalOrg := int64(ac.GlobalOrgID) @@ -88,11 +89,17 @@ func Test_PluginsInstallAndUninstall(t *testing.T) { hs.pluginInstaller = NewFakePluginInstaller() hs.pluginFileStore = &fakes.FakePluginFileStore{} + hs.pluginStore = pluginstore.NewFakePluginStore(pluginstore.Plugin{ + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }) }) t.Run(testName("Install", tc), func(t *testing.T) { input := strings.NewReader(`{"version": "1.0.2"}`) - req := webtest.RequestWithSignedInUser(server.NewPostRequest("/api/plugins/test/install", input), userWithPermissions(tc.permissionOrg, tc.permissions)) + endpoint := fmt.Sprintf("/api/plugins/%s/install", pluginID) + req := webtest.RequestWithSignedInUser(server.NewPostRequest(endpoint, input), userWithPermissions(tc.permissionOrg, tc.permissions)) res, err := server.SendJSON(req) require.NoError(t, err) require.Equal(t, tc.expectedCode, res.StatusCode) @@ -101,7 +108,8 @@ func Test_PluginsInstallAndUninstall(t *testing.T) { t.Run(testName("Uninstall", tc), func(t *testing.T) { input := strings.NewReader("{ }") - req := webtest.RequestWithSignedInUser(server.NewPostRequest("/api/plugins/test/uninstall", input), userWithPermissions(tc.permissionOrg, tc.permissions)) + endpoint := fmt.Sprintf("/api/plugins/%s/uninstall", pluginID) + req := webtest.RequestWithSignedInUser(server.NewPostRequest(endpoint, input), userWithPermissions(tc.permissionOrg, tc.permissions)) res, err := server.SendJSON(req) require.NoError(t, err) require.Equal(t, tc.expectedCode, res.StatusCode) @@ -401,14 +409,14 @@ func TestMakePluginResourceRequestContentTypeEmpty(t *testing.T) { func TestPluginMarkdown(t *testing.T) { t.Run("Plugin not installed returns error", func(t *testing.T) { pluginFileStore := &fakes.FakePluginFileStore{ - FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) { + FileFunc: func(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) { return nil, plugins.ErrPluginNotInstalled }, } hs := HTTPServer{pluginFileStore: pluginFileStore} pluginID := "test-datasource" - md, err := hs.pluginMarkdown(context.Background(), pluginID, "test") + md, err := hs.pluginMarkdown(context.Background(), pluginID, "", "test") require.ErrorAs(t, err, &plugins.NotFoundError{PluginID: pluginID}) require.Equal(t, []byte{}, md) }) @@ -416,7 +424,7 @@ func TestPluginMarkdown(t *testing.T) { t.Run("File fetch will be retried using different casing if error occurs", func(t *testing.T) { var requestedFiles []string pluginFileStore := &fakes.FakePluginFileStore{ - FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) { + FileFunc: func(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) { requestedFiles = append(requestedFiles, filename) return nil, errors.New("some error") }, @@ -424,7 +432,7 @@ func TestPluginMarkdown(t *testing.T) { hs := HTTPServer{pluginFileStore: pluginFileStore} - md, err := hs.pluginMarkdown(context.Background(), "", "reAdMe") + md, err := hs.pluginMarkdown(context.Background(), "", "", "reAdMe") require.NoError(t, err) require.Equal(t, []byte{}, md) require.Equal(t, []string{"README.md", "readme.md"}, requestedFiles) @@ -453,7 +461,7 @@ func TestPluginMarkdown(t *testing.T) { data := []byte{123} var requestedFiles []string pluginFileStore := &fakes.FakePluginFileStore{ - FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) { + FileFunc: func(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) { requestedFiles = append(requestedFiles, filename) return &plugins.File{Content: data}, nil }, @@ -461,7 +469,7 @@ func TestPluginMarkdown(t *testing.T) { hs := HTTPServer{pluginFileStore: pluginFileStore} - md, err := hs.pluginMarkdown(context.Background(), "test-datasource", tc.filePath) + md, err := hs.pluginMarkdown(context.Background(), "test-datasource", "", tc.filePath) require.NoError(t, err) require.Equal(t, data, md) require.Equal(t, tc.expected, requestedFiles) @@ -471,7 +479,7 @@ func TestPluginMarkdown(t *testing.T) { t.Run("Non markdown file request returns an error", func(t *testing.T) { hs := HTTPServer{pluginFileStore: &fakes.FakePluginFileStore{}} - md, err := hs.pluginMarkdown(context.Background(), "", "test.json") + md, err := hs.pluginMarkdown(context.Background(), "", "", "test.json") require.ErrorIs(t, err, ErrUnexpectedFileExtension) require.Equal(t, []byte{}, md) }) @@ -480,14 +488,14 @@ func TestPluginMarkdown(t *testing.T) { data := []byte{1, 2, 3} pluginFileStore := &fakes.FakePluginFileStore{ - FileFunc: func(ctx context.Context, pluginID, filename string) (*plugins.File, error) { + FileFunc: func(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) { return &plugins.File{Content: data}, nil }, } hs := HTTPServer{pluginFileStore: pluginFileStore} - md, err := hs.pluginMarkdown(context.Background(), "", "someFile") + md, err := hs.pluginMarkdown(context.Background(), "", "", "someFile") require.NoError(t, err) require.Equal(t, data, md) }) diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index 00fe2bc7f74..aa3dcd0180b 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -14,7 +14,7 @@ type Installer interface { // Add adds a new plugin. Add(ctx context.Context, pluginID, version string, opts CompatOpts) error // Remove removes an existing plugin. - Remove(ctx context.Context, pluginID string) error + Remove(ctx context.Context, pluginID, version string) error } type PluginSource interface { @@ -25,7 +25,7 @@ type PluginSource interface { type FileStore interface { // File retrieves a plugin file. - File(ctx context.Context, pluginID, filename string) (*File, error) + File(ctx context.Context, pluginID, pluginVersion, filename string) (*File, error) } type File struct { diff --git a/pkg/plugins/manager/client/client.go b/pkg/plugins/manager/client/client.go index 9914b615f2e..3565d97ef9f 100644 --- a/pkg/plugins/manager/client/client.go +++ b/pkg/plugins/manager/client/client.go @@ -44,7 +44,7 @@ func (s *Service) QueryData(ctx context.Context, req *backend.QueryDataRequest) return nil, errNilRequest } - p, exists := s.plugin(ctx, req.PluginContext.PluginID) + p, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return nil, plugins.ErrPluginNotRegistered } @@ -87,7 +87,7 @@ func (s *Service) CallResource(ctx context.Context, req *backend.CallResourceReq return errNilSender } - p, exists := s.plugin(ctx, req.PluginContext.PluginID) + p, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return plugins.ErrPluginNotRegistered } @@ -130,7 +130,7 @@ func (s *Service) CollectMetrics(ctx context.Context, req *backend.CollectMetric return nil, errNilRequest } - p, exists := s.plugin(ctx, req.PluginContext.PluginID) + p, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return nil, plugins.ErrPluginNotRegistered } @@ -152,7 +152,7 @@ func (s *Service) CheckHealth(ctx context.Context, req *backend.CheckHealthReque return nil, errNilRequest } - p, exists := s.plugin(ctx, req.PluginContext.PluginID) + p, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return nil, plugins.ErrPluginNotRegistered } @@ -182,7 +182,7 @@ func (s *Service) SubscribeStream(ctx context.Context, req *backend.SubscribeStr return nil, errNilRequest } - plugin, exists := s.plugin(ctx, req.PluginContext.PluginID) + plugin, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return nil, plugins.ErrPluginNotRegistered } @@ -195,7 +195,7 @@ func (s *Service) PublishStream(ctx context.Context, req *backend.PublishStreamR return nil, errNilRequest } - plugin, exists := s.plugin(ctx, req.PluginContext.PluginID) + plugin, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return nil, plugins.ErrPluginNotRegistered } @@ -212,7 +212,7 @@ func (s *Service) RunStream(ctx context.Context, req *backend.RunStreamRequest, return errNilSender } - plugin, exists := s.plugin(ctx, req.PluginContext.PluginID) + plugin, exists := s.plugin(ctx, req.PluginContext.PluginID, req.PluginContext.PluginVersion) if !exists { return plugins.ErrPluginNotRegistered } @@ -221,8 +221,8 @@ func (s *Service) RunStream(ctx context.Context, req *backend.RunStreamRequest, } // plugin finds a plugin with `pluginID` from the registry that is not decommissioned -func (s *Service) plugin(ctx context.Context, pluginID string) (*plugins.Plugin, bool) { - p, exists := s.pluginRegistry.Plugin(ctx, pluginID) +func (s *Service) plugin(ctx context.Context, pluginID, pluginVersion string) (*plugins.Plugin, bool) { + p, exists := s.pluginRegistry.Plugin(ctx, pluginID, pluginVersion) if !exists { return nil, false } diff --git a/pkg/plugins/manager/fakes/fakes.go b/pkg/plugins/manager/fakes/fakes.go index e460cef4ef9..51f40db5838 100644 --- a/pkg/plugins/manager/fakes/fakes.go +++ b/pkg/plugins/manager/fakes/fakes.go @@ -176,7 +176,7 @@ func NewFakePluginRegistry() *FakePluginRegistry { } } -func (f *FakePluginRegistry) Plugin(_ context.Context, id string) (*plugins.Plugin, bool) { +func (f *FakePluginRegistry) Plugin(_ context.Context, id, _ string) (*plugins.Plugin, bool) { p, exists := f.Store[id] return p, exists } @@ -195,7 +195,7 @@ func (f *FakePluginRegistry) Add(_ context.Context, p *plugins.Plugin) error { return nil } -func (f *FakePluginRegistry) Remove(_ context.Context, id string) error { +func (f *FakePluginRegistry) Remove(_ context.Context, id, _ string) error { delete(f.Store, id) return nil } @@ -423,12 +423,12 @@ func (s *FakePluginSource) DefaultSignature(ctx context.Context) (plugins.Signat } type FakePluginFileStore struct { - FileFunc func(ctx context.Context, pluginID, filename string) (*plugins.File, error) + FileFunc func(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) } -func (f *FakePluginFileStore) File(ctx context.Context, pluginID, filename string) (*plugins.File, error) { +func (f *FakePluginFileStore) File(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) { if f.FileFunc != nil { - return f.FileFunc(ctx, pluginID, filename) + return f.FileFunc(ctx, pluginID, pluginVersion, filename) } return nil, nil } diff --git a/pkg/plugins/manager/filestore/fs.go b/pkg/plugins/manager/filestore/fs.go index 6d133d7bc6d..155880eb570 100644 --- a/pkg/plugins/manager/filestore/fs.go +++ b/pkg/plugins/manager/filestore/fs.go @@ -21,8 +21,8 @@ func ProvideService(pluginRegistry registry.Service) *Service { } } -func (s *Service) File(ctx context.Context, pluginID, filename string) (*plugins.File, error) { - if p, exists := s.pluginRegistry.Plugin(ctx, pluginID); exists { +func (s *Service) File(ctx context.Context, pluginID, pluginVersion, filename string) (*plugins.File, error) { + if p, exists := s.pluginRegistry.Plugin(ctx, pluginID, pluginVersion); exists { f, err := p.File(filename) if err != nil { return nil, err diff --git a/pkg/plugins/manager/installer.go b/pkg/plugins/manager/installer.go index e4f26dc54c8..4463220a885 100644 --- a/pkg/plugins/manager/installer.go +++ b/pkg/plugins/manager/installer.go @@ -55,7 +55,7 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt } var pluginArchive *repo.PluginArchive - if plugin, exists := m.plugin(ctx, pluginID); exists { + if plugin, exists := m.plugin(ctx, pluginID, version); exists { if plugin.IsCorePlugin() || plugin.IsBundledPlugin() { return plugins.ErrInstallCorePlugin } @@ -84,7 +84,7 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt } // remove existing installation of plugin - err = m.Remove(ctx, plugin.ID) + err = m.Remove(ctx, plugin.ID, plugin.Info.Version) if err != nil { return err } @@ -139,8 +139,8 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt return nil } -func (m *PluginInstaller) Remove(ctx context.Context, pluginID string) error { - plugin, exists := m.plugin(ctx, pluginID) +func (m *PluginInstaller) Remove(ctx context.Context, pluginID, version string) error { + plugin, exists := m.plugin(ctx, pluginID, version) if !exists { return plugins.ErrPluginNotInstalled } @@ -168,8 +168,8 @@ func (m *PluginInstaller) Remove(ctx context.Context, pluginID string) error { } // plugin finds a plugin with `pluginID` from the store -func (m *PluginInstaller) plugin(ctx context.Context, pluginID string) (*plugins.Plugin, bool) { - p, exists := m.pluginRegistry.Plugin(ctx, pluginID) +func (m *PluginInstaller) plugin(ctx context.Context, pluginID, pluginVersion string) (*plugins.Plugin, bool) { + p, exists := m.pluginRegistry.Plugin(ctx, pluginID, pluginVersion) if !exists { return nil, false } diff --git a/pkg/plugins/manager/installer_test.go b/pkg/plugins/manager/installer_test.go index 43465cd23ff..7bfadece3af 100644 --- a/pkg/plugins/manager/installer_test.go +++ b/pkg/plugins/manager/installer_test.go @@ -23,6 +23,8 @@ func TestPluginManager_Add_Remove(t *testing.T) { const ( pluginID, v1 = "test-panel", "1.0.0" zipNameV1 = "test-panel-1.0.0.zip" + v2 = "2.0.0" + zipNameV2 = "test-panel-2.0.0.zip" ) // mock a plugin to be returned automatically by the plugin loader @@ -83,10 +85,6 @@ func TestPluginManager_Add_Remove(t *testing.T) { }) t.Run("Update plugin to different version", func(t *testing.T) { - const ( - v2 = "2.0.0" - zipNameV2 = "test-panel-2.0.0.zip" - ) // mock a plugin to be returned automatically by the plugin loader pluginV2 := createPlugin(t, pluginID, plugins.ClassExternal, true, true, func(plugin *plugins.Plugin) { plugin.Info.Version = v2 @@ -138,7 +136,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { }, } - err = inst.Remove(context.Background(), pluginID) + err = inst.Remove(context.Background(), pluginID, v2) require.NoError(t, err) require.Equal(t, []string{pluginID}, unloadedPlugins) @@ -146,7 +144,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { t.Run("Won't remove if not exists", func(t *testing.T) { inst.pluginRegistry = fakes.NewFakePluginRegistry() - err = inst.Remove(context.Background(), pluginID) + err = inst.Remove(context.Background(), pluginID, v2) require.Equal(t, plugins.ErrPluginNotInstalled, err) }) }) @@ -179,7 +177,7 @@ func TestPluginManager_Add_Remove(t *testing.T) { require.Equal(t, plugins.ErrInstallCorePlugin, err) t.Run(fmt.Sprintf("Can't uninstall %s plugin", tc.class), func(t *testing.T) { - err = pm.Remove(context.Background(), p.ID) + err = pm.Remove(context.Background(), p.ID, p.Info.Version) require.Equal(t, plugins.ErrUninstallCorePlugin, err) }) } diff --git a/pkg/plugins/manager/pipeline/discovery/steps.go b/pkg/plugins/manager/pipeline/discovery/steps.go index 179eca4c12a..bff2c41856d 100644 --- a/pkg/plugins/manager/pipeline/discovery/steps.go +++ b/pkg/plugins/manager/pipeline/discovery/steps.go @@ -6,9 +6,7 @@ import ( "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/loader/finder" - "github.com/grafana/grafana/pkg/plugins/manager/registry" ) // DefaultFindFunc is the default function used for the Find step of the Discovery stage. It will scan the local @@ -17,44 +15,6 @@ func DefaultFindFunc(cfg *config.Cfg) FindFunc { return finder.NewLocalFinder(cfg.DevMode, cfg.Features).Find } -// DuplicatePluginValidation is a filter step that will filter out any plugins that are already registered with the -// registry. This includes both the primary plugin and any child plugins, which are matched using the plugin ID field. -type DuplicatePluginValidation struct { - registry registry.Service - log log.Logger -} - -// NewDuplicatePluginFilterStep returns a new DuplicatePluginValidation. -func NewDuplicatePluginFilterStep(registry registry.Service) *DuplicatePluginValidation { - return &DuplicatePluginValidation{ - registry: registry, - log: log.New("plugins.dedupe"), - } -} - -// Filter will filter out any plugins that are already registered with the registry. -func (d *DuplicatePluginValidation) Filter(ctx context.Context, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { - res := make([]*plugins.FoundBundle, 0, len(bundles)) - for _, b := range bundles { - _, exists := d.registry.Plugin(ctx, b.Primary.JSONData.ID) - if exists { - d.log.Warn("Skipping loading of plugin as it's a duplicate", "pluginId", b.Primary.JSONData.ID) - continue - } - - for _, child := range b.Children { - _, exists = d.registry.Plugin(ctx, child.JSONData.ID) - if exists { - d.log.Warn("Skipping loading of child plugin as it's a duplicate", "pluginId", child.JSONData.ID) - continue - } - } - res = append(res, b) - } - - return res, nil -} - // PermittedPluginTypesFilter is a filter step that will filter out any plugins that are not of a permitted type. type PermittedPluginTypesFilter struct { permittedTypes []plugins.Type diff --git a/pkg/plugins/manager/pipeline/termination/steps.go b/pkg/plugins/manager/pipeline/termination/steps.go index e2fdcc3bed8..0d5e2144d40 100644 --- a/pkg/plugins/manager/pipeline/termination/steps.go +++ b/pkg/plugins/manager/pipeline/termination/steps.go @@ -54,7 +54,7 @@ func newDeregister(pluginRegistry registry.Service) *Deregister { // Deregister removes a plugin from the plugin registry. func (d *Deregister) Deregister(ctx context.Context, p *plugins.Plugin) error { - if err := d.pluginRegistry.Remove(ctx, p.ID); err != nil { + if err := d.pluginRegistry.Remove(ctx, p.ID, p.Info.Version); err != nil { return err } d.log.Debug("Plugin unregistered", "pluginId", p.ID) diff --git a/pkg/plugins/manager/registry/ifaces.go b/pkg/plugins/manager/registry/ifaces.go index 94c7696eac8..680dcdab5bf 100644 --- a/pkg/plugins/manager/registry/ifaces.go +++ b/pkg/plugins/manager/registry/ifaces.go @@ -8,12 +8,12 @@ import ( // Service is responsible for the internal storing and retrieval of plugins. type Service interface { - // Plugin finds a plugin by its ID. - Plugin(ctx context.Context, id string) (*plugins.Plugin, bool) + // Plugin finds a plugin by its ID and version. + Plugin(ctx context.Context, id, version string) (*plugins.Plugin, bool) // Plugins returns all plugins. Plugins(ctx context.Context) []*plugins.Plugin // Add adds the provided plugin to the registry. Add(ctx context.Context, plugin *plugins.Plugin) error // Remove deletes the requested plugin from the registry. - Remove(ctx context.Context, id string) error + Remove(ctx context.Context, id, version string) error } diff --git a/pkg/plugins/manager/registry/in_memory.go b/pkg/plugins/manager/registry/in_memory.go index 8b533b985d8..af3652e5d6c 100644 --- a/pkg/plugins/manager/registry/in_memory.go +++ b/pkg/plugins/manager/registry/in_memory.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/plugins" ) +// InMemory is a registry that only allows a single version of a plugin to be registered at a time. type InMemory struct { store map[string]*plugins.Plugin alias map[string]*plugins.Plugin @@ -25,7 +26,7 @@ func NewInMemory() *InMemory { } } -func (i *InMemory) Plugin(_ context.Context, pluginID string) (*plugins.Plugin, bool) { +func (i *InMemory) Plugin(_ context.Context, pluginID, _ string) (*plugins.Plugin, bool) { return i.plugin(pluginID) } @@ -56,7 +57,7 @@ func (i *InMemory) Add(_ context.Context, p *plugins.Plugin) error { return nil } -func (i *InMemory) Remove(_ context.Context, pluginID string) error { +func (i *InMemory) Remove(_ context.Context, pluginID, _ string) error { p, ok := i.plugin(pluginID) if !ok { return fmt.Errorf("plugin %s is not registered", pluginID) diff --git a/pkg/plugins/manager/registry/in_memory_test.go b/pkg/plugins/manager/registry/in_memory_test.go index 25d3f42342a..31d6a21a2df 100644 --- a/pkg/plugins/manager/registry/in_memory_test.go +++ b/pkg/plugins/manager/registry/in_memory_test.go @@ -11,29 +11,41 @@ import ( "github.com/grafana/grafana/pkg/plugins" ) -const pluginID = "test-ds" +const ( + pluginID = "test-ds" + v1 = "1.0.0" + v2 = "2.0.0" +) func TestInMemory(t *testing.T) { t.Run("Test mix of registry operations", func(t *testing.T) { i := NewInMemory() ctx := context.Background() - p, exists := i.Plugin(ctx, pluginID) + p, exists := i.Plugin(ctx, pluginID, v1) require.False(t, exists) require.Nil(t, p) - err := i.Remove(ctx, pluginID) + err := i.Remove(ctx, pluginID, v1) require.EqualError(t, err, fmt.Errorf("plugin %s is not registered", pluginID).Error()) + pv1 := &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID, Info: plugins.Info{Version: v1}}} + err = i.Add(ctx, pv1) + require.NoError(t, err) + + pv2 := &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID, Info: plugins.Info{Version: v2}}} + err = i.Add(ctx, pv2) + require.Errorf(t, err, fmt.Sprintf("plugin %s is already registered", pluginID)) + + existingP, exists := i.Plugin(ctx, pluginID, v1) + require.True(t, exists) + require.Equal(t, pv1, existingP) + p = &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}} err = i.Add(ctx, p) - require.NoError(t, err) + require.Errorf(t, err, fmt.Sprintf("plugin %s is already registered", pluginID)) - existingP, exists := i.Plugin(ctx, pluginID) - require.True(t, exists) - require.Equal(t, p, existingP) - - err = i.Remove(ctx, pluginID) + err = i.Remove(ctx, pluginID, v1) require.NoError(t, err) existingPlugins := i.Plugins(ctx) @@ -87,6 +99,28 @@ func TestInMemory_Add(t *testing.T) { }, err: fmt.Errorf("plugin %s is already registered", pluginID), }, + { + name: "Cannot add a plugin to the registry even if it has a different version", + mocks: mocks{ + store: map[string]*plugins.Plugin{ + pluginID: { + JSONData: plugins.JSONData{ + ID: pluginID, + Info: plugins.Info{Version: v1}, + }, + }, + }, + }, + args: args{ + p: &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: pluginID, + Info: plugins.Info{Version: v2}, + }, + }, + }, + err: fmt.Errorf("plugin %s is already registered", pluginID), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -151,7 +185,7 @@ func TestInMemory_Plugin(t *testing.T) { i := &InMemory{ store: tt.mocks.store, } - p, exists := i.Plugin(context.Background(), tt.args.pluginID) + p, exists := i.Plugin(context.Background(), tt.args.pluginID, "") if exists != tt.exists { t.Errorf("Plugin() got1 = %v, expected %v", exists, tt.exists) } @@ -265,7 +299,7 @@ func TestInMemory_Remove(t *testing.T) { i := &InMemory{ store: tt.mocks.store, } - err := i.Remove(context.Background(), tt.args.pluginID) + err := i.Remove(context.Background(), tt.args.pluginID, "") require.Equal(t, tt.err, err) }) } @@ -280,7 +314,7 @@ func TestAliasSupport(t *testing.T) { pluginIdOld := "plugin-old" pluginIdOld2 := "plugin-old2" - p, exists := i.Plugin(ctx, pluginIdNew) + p, exists := i.Plugin(ctx, pluginIdNew, "") require.False(t, exists) require.Nil(t, p) @@ -294,17 +328,17 @@ func TestAliasSupport(t *testing.T) { require.NoError(t, err) // Can lookup by the new ID - found, exists := i.Plugin(ctx, pluginIdNew) + found, exists := i.Plugin(ctx, pluginIdNew, "") require.True(t, exists) require.Equal(t, pluginNew, found) // Can lookup by the old ID - found, exists = i.Plugin(ctx, pluginIdOld) + found, exists = i.Plugin(ctx, pluginIdOld, "") require.True(t, exists) require.Equal(t, pluginNew, found) // Can lookup by the other old ID - found, exists = i.Plugin(ctx, pluginIdOld2) + found, exists = i.Plugin(ctx, pluginIdOld2, "") require.True(t, exists) require.Equal(t, pluginNew, found) @@ -313,7 +347,7 @@ func TestAliasSupport(t *testing.T) { ID: pluginIdOld, }} require.NoError(t, i.Add(ctx, pluginOld)) - found, exists = i.Plugin(ctx, pluginIdOld) + found, exists = i.Plugin(ctx, pluginIdOld, "") require.True(t, exists) require.Equal(t, pluginOld, found) }) diff --git a/pkg/services/pluginsintegration/clientmiddleware/metrics_middleware.go b/pkg/services/pluginsintegration/clientmiddleware/metrics_middleware.go index 329117f5c87..3ea7f566118 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/metrics_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/metrics_middleware.go @@ -89,8 +89,8 @@ func NewMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry r } // pluginTarget returns the value for the "target" Prometheus label for the given plugin ID. -func (m *MetricsMiddleware) pluginTarget(ctx context.Context, pluginID string) (string, error) { - p, exists := m.pluginRegistry.Plugin(ctx, pluginID) +func (m *MetricsMiddleware) pluginTarget(ctx context.Context, pluginID, pluginVersion string) (string, error) { + p, exists := m.pluginRegistry.Plugin(ctx, pluginID, pluginVersion) if !exists { return "", plugins.ErrPluginNotRegistered } @@ -99,7 +99,7 @@ func (m *MetricsMiddleware) pluginTarget(ctx context.Context, pluginID string) ( // instrumentPluginRequestSize tracks the size of the given request in the m.pluginRequestSize metric. func (m *MetricsMiddleware) instrumentPluginRequestSize(ctx context.Context, pluginCtx backend.PluginContext, endpoint string, requestSize float64) error { - target, err := m.pluginTarget(ctx, pluginCtx.PluginID) + target, err := m.pluginTarget(ctx, pluginCtx.PluginID, pluginCtx.PluginVersion) if err != nil { return err } @@ -109,7 +109,7 @@ func (m *MetricsMiddleware) instrumentPluginRequestSize(ctx context.Context, plu // instrumentPluginRequest increments the m.pluginRequestCounter metric and tracks the duration of the given request. func (m *MetricsMiddleware) instrumentPluginRequest(ctx context.Context, pluginCtx backend.PluginContext, endpoint string, fn func(context.Context) (requestStatus, error)) error { - target, err := m.pluginTarget(ctx, pluginCtx.PluginID) + target, err := m.pluginTarget(ctx, pluginCtx.PluginID, pluginCtx.PluginVersion) if err != nil { return err } diff --git a/pkg/services/pluginsintegration/dashboards/filestore.go b/pkg/services/pluginsintegration/dashboards/filestore.go index 988650e2a42..e854e6e1340 100644 --- a/pkg/services/pluginsintegration/dashboards/filestore.go +++ b/pkg/services/pluginsintegration/dashboards/filestore.go @@ -24,8 +24,8 @@ func ProvideFileStoreManager(pluginStore pluginstore.Store, pluginFileStore plug } } -var openDashboardFile = func(ctx context.Context, pluginFileStore plugins.FileStore, pluginID, name string) (*plugins.File, error) { - f, err := pluginFileStore.File(ctx, pluginID, name) +var openDashboardFile = func(ctx context.Context, pluginFileStore plugins.FileStore, pluginID, pluginVersion, name string) (*plugins.File, error) { + f, err := pluginFileStore.File(ctx, pluginID, pluginVersion, name) if err != nil { return &plugins.File{}, err } @@ -93,7 +93,7 @@ func (m *FileStoreManager) GetPluginDashboardFileContents(ctx context.Context, a return nil, err } - file, err := openDashboardFile(ctx, m.pluginFileStore, plugin.ID, cleanPath) + file, err := openDashboardFile(ctx, m.pluginFileStore, plugin.ID, plugin.Info.Version, cleanPath) if err != nil { return nil, err } diff --git a/pkg/services/pluginsintegration/dashboards/filestore_test.go b/pkg/services/pluginsintegration/dashboards/filestore_test.go index 9981e768d02..8f8a76252d4 100644 --- a/pkg/services/pluginsintegration/dashboards/filestore_test.go +++ b/pkg/services/pluginsintegration/dashboards/filestore_test.go @@ -132,7 +132,7 @@ func TestDashboardFileStore(t *testing.T) { Data: []byte("dash2"), }, } - openDashboardFile = func(ctx context.Context, pluginFiles plugins.FileStore, pluginID, name string) (*plugins.File, error) { + openDashboardFile = func(ctx context.Context, pluginFiles plugins.FileStore, pluginID, _, name string) (*plugins.File, error) { f, err := mapFs.Open(name) require.NoError(t, err) diff --git a/pkg/services/pluginsintegration/loader/loader_test.go b/pkg/services/pluginsintegration/loader/loader_test.go index 42799286cc1..c2b605716d8 100644 --- a/pkg/services/pluginsintegration/loader/loader_test.go +++ b/pkg/services/pluginsintegration/loader/loader_test.go @@ -1666,7 +1666,7 @@ func verifyState(t *testing.T, ps []*plugins.Plugin, reg registry.Service, t.Helper() for _, p := range ps { - regP, exists := reg.Plugin(context.Background(), p.ID) + regP, exists := reg.Plugin(context.Background(), p.ID, p.Info.Version) require.True(t, exists) if !cmp.Equal(p, regP, compareOpts...) { t.Fatalf("Result mismatch (-want +got):\n%s", cmp.Diff(p, regP, compareOpts...)) diff --git a/pkg/services/pluginsintegration/pipeline/pipeline.go b/pkg/services/pluginsintegration/pipeline/pipeline.go index 1c2c9598e61..6ed3cbe622c 100644 --- a/pkg/services/pluginsintegration/pipeline/pipeline.go +++ b/pkg/services/pluginsintegration/pipeline/pipeline.go @@ -29,7 +29,7 @@ func ProvideDiscoveryStage(cfg *config.Cfg, pf finder.Finder, pr registry.Servic plugins.TypeDataSource, plugins.TypeApp, plugins.TypePanel, plugins.TypeSecretsManager, }), func(ctx context.Context, _ plugins.Class, b []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { - return discovery.NewDuplicatePluginFilterStep(pr).Filter(ctx, b) + return NewDuplicatePluginIDFilterStep(pr).Filter(ctx, b) }, func(_ context.Context, _ plugins.Class, b []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { return NewDisablePluginsStep(cfg).Filter(b) diff --git a/pkg/services/pluginsintegration/pipeline/steps.go b/pkg/services/pluginsintegration/pipeline/steps.go index c1a013223c1..2685a634c6a 100644 --- a/pkg/services/pluginsintegration/pipeline/steps.go +++ b/pkg/services/pluginsintegration/pipeline/steps.go @@ -3,6 +3,7 @@ package pipeline import ( "context" "errors" + "slices" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/plugins" @@ -11,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/log" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/initialization" "github.com/grafana/grafana/pkg/plugins/manager/pipeline/validation" + "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/manager/signature" "github.com/grafana/grafana/pkg/plugins/plugindef" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -195,3 +197,54 @@ func (c *AsExternal) Filter(cl plugins.Class, bundles []*plugins.FoundBundle) ([ } return bundles, nil } + +// DuplicatePluginIDValidation is a filter step that will filter out any plugins that are already registered with the same +// plugin ID. This includes both the primary plugin and child plugins, which are matched using the plugin.json plugin +// ID field. +type DuplicatePluginIDValidation struct { + registry registry.Service + log log.Logger +} + +// NewDuplicatePluginIDFilterStep returns a new DuplicatePluginIDValidation. +func NewDuplicatePluginIDFilterStep(registry registry.Service) *DuplicatePluginIDValidation { + return &DuplicatePluginIDValidation{ + registry: registry, + log: log.New("plugins.dedupe"), + } +} + +// Filter will filter out any plugins that have already been registered under the same plugin ID. +func (d *DuplicatePluginIDValidation) Filter(ctx context.Context, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { + res := make([]*plugins.FoundBundle, 0, len(bundles)) + + var matchesPluginIDFunc = func(fp plugins.FoundPlugin) func(p *plugins.Plugin) bool { + return func(p *plugins.Plugin) bool { + return p.ID == fp.JSONData.ID + } + } + + for _, b := range bundles { + ps := d.registry.Plugins(ctx) + + if slices.ContainsFunc(ps, matchesPluginIDFunc(b.Primary)) { + d.log.Warn("Skipping loading of plugin as it's a duplicate", "pluginId", b.Primary.JSONData.ID) + continue + } + + var nonDupeChildren []*plugins.FoundPlugin + for _, child := range b.Children { + if slices.ContainsFunc(ps, matchesPluginIDFunc(*child)) { + d.log.Warn("Skipping loading of child plugin as it's a duplicate", "pluginId", child.JSONData.ID) + continue + } + nonDupeChildren = append(nonDupeChildren, child) + } + res = append(res, &plugins.FoundBundle{ + Primary: b.Primary, + Children: nonDupeChildren, + }) + } + + return res, nil +} diff --git a/pkg/services/pluginsintegration/pipeline/steps_test.go b/pkg/services/pluginsintegration/pipeline/steps_test.go index edad51c8916..e3431d5ced8 100644 --- a/pkg/services/pluginsintegration/pipeline/steps_test.go +++ b/pkg/services/pluginsintegration/pipeline/steps_test.go @@ -1,12 +1,14 @@ package pipeline import ( + "context" "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/registry" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" ) @@ -82,3 +84,98 @@ func TestAsExternal(t *testing.T) { require.Equal(t, filtered[0].Primary.JSONData.ID, "plugin2") }) } + +func TestDuplicatePluginIDValidation(t *testing.T) { + tcs := []struct { + name string + registeredPlugins []string + in []*plugins.FoundBundle + out []*plugins.FoundBundle + }{ + { + name: "should filter out a plugin if it already exists in the plugin registry", + registeredPlugins: []string{"foobar-datasource"}, + in: []*plugins.FoundBundle{ + { + Primary: plugins.FoundPlugin{ + JSONData: plugins.JSONData{ + ID: "foobar-datasource", + }, + }, + }, + }, + out: []*plugins.FoundBundle{}, + }, + { + name: "should not filter out a plugin if it doesn't exist in the plugin registry", + registeredPlugins: []string{"foobar-datasource"}, + in: []*plugins.FoundBundle{ + { + Primary: plugins.FoundPlugin{ + JSONData: plugins.JSONData{ + ID: "test-datasource", + }, + }, + }, + }, + out: []*plugins.FoundBundle{ + { + Primary: plugins.FoundPlugin{ + JSONData: plugins.JSONData{ + ID: "test-datasource", + }, + }, + }, + }, + }, + { + name: "should filter out child plugins if they are already registered", + registeredPlugins: []string{"foobar-datasource"}, + in: []*plugins.FoundBundle{ + { + Primary: plugins.FoundPlugin{ + JSONData: plugins.JSONData{ + ID: "test-datasource", + }, + }, + Children: []*plugins.FoundPlugin{ + { + JSONData: plugins.JSONData{ + ID: "foobar-datasource", + }, + }, + }, + }, + }, + out: []*plugins.FoundBundle{ + { + Primary: plugins.FoundPlugin{ + JSONData: plugins.JSONData{ + ID: "test-datasource", + }, + }, + }, + }, + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + r := registry.NewInMemory() + s := NewDuplicatePluginIDFilterStep(r) + + ctx := context.Background() + for _, pluginID := range tc.registeredPlugins { + err := r.Add(ctx, &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }) + require.NoError(t, err) + } + + res, err := s.Filter(ctx, tc.in) + require.NoError(t, err) + require.Equal(t, tc.out, res) + }) + } +} diff --git a/pkg/services/pluginsintegration/plugins_integration_test.go b/pkg/services/pluginsintegration/plugins_integration_test.go index 46f450878a5..778d0c13014 100644 --- a/pkg/services/pluginsintegration/plugins_integration_test.go +++ b/pkg/services/pluginsintegration/plugins_integration_test.go @@ -17,7 +17,6 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" - "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/searchV2" @@ -100,7 +99,7 @@ func TestIntegrationPluginManager(t *testing.T) { ctx := context.Background() verifyCorePluginCatalogue(t, ctx, testCtx.PluginStore) verifyBundledPlugins(t, ctx, testCtx.PluginStore) - verifyPluginStaticRoutes(t, ctx, testCtx.PluginStore, testCtx.PluginRegistry) + verifyPluginStaticRoutes(t, ctx, testCtx.PluginStore, testCtx.PluginStore) verifyBackendProcesses(t, testCtx.PluginRegistry.Plugins(ctx)) verifyPluginQuery(t, ctx, testCtx.PluginClient) } @@ -253,7 +252,7 @@ func verifyBundledPlugins(t *testing.T, ctx context.Context, ps *pluginstore.Ser } } -func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, rr plugins.StaticRouteResolver, reg registry.Service) { +func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, rr plugins.StaticRouteResolver, ps *pluginstore.Service) { routes := make(map[string]*plugins.StaticRoute) for _, route := range rr.Routes(ctx) { routes[route.PluginID] = route @@ -261,13 +260,13 @@ func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, rr plugins.Stat require.Len(t, routes, 2) - inputPlugin, _ := reg.Plugin(ctx, "input") + inputPlugin, _ := ps.Plugin(ctx, "input") require.NotNil(t, routes["input"]) - require.Equal(t, routes["input"].Directory, inputPlugin.FS.Base()) + require.Equal(t, routes["input"].Directory, inputPlugin.Base()) - testAppPlugin, _ := reg.Plugin(ctx, "test-app") + testAppPlugin, _ := ps.Plugin(ctx, "test-app") require.Contains(t, routes, "test-app") - require.Equal(t, routes["test-app"].Directory, testAppPlugin.FS.Base()) + require.Equal(t, routes["test-app"].Directory, testAppPlugin.Base()) } func verifyBackendProcesses(t *testing.T, ps []*plugins.Plugin) { diff --git a/pkg/services/pluginsintegration/pluginstore/store.go b/pkg/services/pluginsintegration/pluginstore/store.go index 164876d187b..e4d188572d2 100644 --- a/pkg/services/pluginsintegration/pluginstore/store.go +++ b/pkg/services/pluginsintegration/pluginstore/store.go @@ -18,6 +18,7 @@ var _ Store = (*Service)(nil) // Store is the publicly accessible storage for plugins. type Store interface { // Plugin finds a plugin by its ID. + // Note: version is not required since Grafana only supports single versions of a plugin. Plugin(ctx context.Context, pluginID string) (Plugin, bool) // Plugins returns plugins by their requested type. Plugins(ctx context.Context, pluginTypes ...plugins.Type) []Plugin @@ -104,7 +105,7 @@ func (s *Service) SecretsManager(ctx context.Context) *plugins.Plugin { // plugin finds a plugin with `pluginID` from the registry that is not decommissioned func (s *Service) plugin(ctx context.Context, pluginID string) (*plugins.Plugin, bool) { - p, exists := s.pluginRegistry.Plugin(ctx, pluginID) + p, exists := s.pluginRegistry.Plugin(ctx, pluginID, "") // version is not required since Grafana only supports single versions of a plugin if !exists { return nil, false } diff --git a/pkg/services/pluginsintegration/renderer/renderer.go b/pkg/services/pluginsintegration/renderer/renderer.go index 940a6652534..fda142b5391 100644 --- a/pkg/services/pluginsintegration/renderer/renderer.go +++ b/pkg/services/pluginsintegration/renderer/renderer.go @@ -19,6 +19,7 @@ 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/pluginsintegration/pipeline" "github.com/grafana/grafana/pkg/services/rendering" ) @@ -106,7 +107,7 @@ func createLoader(cfg *config.Cfg, pr registry.Service, l plugins.Licensing) (lo FindFilterFuncs: []discovery.FindFilterFunc{ discovery.NewPermittedPluginTypesFilterStep([]plugins.Type{plugins.TypeRenderer}), func(ctx context.Context, class plugins.Class, bundles []*plugins.FoundBundle) ([]*plugins.FoundBundle, error) { - return discovery.NewDuplicatePluginFilterStep(pr).Filter(ctx, bundles) + return pipeline.NewDuplicatePluginIDFilterStep(pr).Filter(ctx, bundles) }, }, })