diff --git a/pkg/plugins/backendplugin/errors.go b/pkg/plugins/backendplugin/errors.go index 9da8254788fae..9d9fcb70e4c19 100644 --- a/pkg/plugins/backendplugin/errors.go +++ b/pkg/plugins/backendplugin/errors.go @@ -3,7 +3,7 @@ package backendplugin import "errors" var ( - // ErrPluginNotRegistered error returned when plugin not registered. + // ErrPluginNotRegistered error returned when plugin is not registered. ErrPluginNotRegistered = errors.New("plugin not registered") // ErrHealthCheckFailed error returned when health check failed. ErrHealthCheckFailed = errors.New("health check failed") diff --git a/pkg/plugins/ifaces.go b/pkg/plugins/ifaces.go index c10283b0031b5..5d3aa9cb7f35e 100644 --- a/pkg/plugins/ifaces.go +++ b/pkg/plugins/ifaces.go @@ -21,22 +21,6 @@ type Store interface { Remove(ctx context.Context, pluginID string) error } -// Loader is responsible for loading plugins from the file system. -type Loader interface { - // Load will return a list of plugins found in the provided file system paths. - Load(ctx context.Context, class Class, paths []string, ignore map[string]struct{}) ([]*Plugin, error) -} - -// Installer is responsible for managing plugins (add / remove) on the file system. -type Installer interface { - // Install downloads the requested plugin in the provided file system location. - Install(ctx context.Context, pluginID, version, pluginsDir, pluginZipURL, pluginRepoURL string) error - // Uninstall removes the requested plugin from the provided file system location. - Uninstall(ctx context.Context, pluginDir string) error - // GetUpdateInfo provides update information for the requested plugin. - GetUpdateInfo(ctx context.Context, pluginID, version, pluginRepoURL string) (UpdateInfo, error) -} - type UpdateInfo struct { PluginZipURL string } diff --git a/pkg/plugins/manager/client.go b/pkg/plugins/manager/client.go index 5af54e1e5f6cc..cd1d853b5b012 100644 --- a/pkg/plugins/manager/client.go +++ b/pkg/plugins/manager/client.go @@ -12,7 +12,7 @@ import ( ) func (m *PluginManager) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { - plugin, exists := m.plugin(req.PluginContext.PluginID) + plugin, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return nil, backendplugin.ErrPluginNotRegistered } @@ -48,11 +48,10 @@ func (m *PluginManager) QueryData(ctx context.Context, req *backend.QueryDataReq } func (m *PluginManager) CallResource(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { - p, exists := m.plugin(req.PluginContext.PluginID) + p, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return backendplugin.ErrPluginNotRegistered } - err := instrumentation.InstrumentCallResourceRequest(p.PluginID(), func() error { if err := p.CallResource(ctx, req, sender); err != nil { return err @@ -68,7 +67,7 @@ func (m *PluginManager) CallResource(ctx context.Context, req *backend.CallResou } func (m *PluginManager) CollectMetrics(ctx context.Context, req *backend.CollectMetricsRequest) (*backend.CollectMetricsResult, error) { - p, exists := m.plugin(req.PluginContext.PluginID) + p, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return nil, backendplugin.ErrPluginNotRegistered } @@ -86,7 +85,7 @@ func (m *PluginManager) CollectMetrics(ctx context.Context, req *backend.Collect } func (m *PluginManager) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { - p, exists := m.plugin(req.PluginContext.PluginID) + p, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return nil, backendplugin.ErrPluginNotRegistered } @@ -113,7 +112,7 @@ func (m *PluginManager) CheckHealth(ctx context.Context, req *backend.CheckHealt } func (m *PluginManager) SubscribeStream(ctx context.Context, req *backend.SubscribeStreamRequest) (*backend.SubscribeStreamResponse, error) { - plugin, exists := m.plugin(req.PluginContext.PluginID) + plugin, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return nil, backendplugin.ErrPluginNotRegistered } @@ -122,7 +121,7 @@ func (m *PluginManager) SubscribeStream(ctx context.Context, req *backend.Subscr } func (m *PluginManager) PublishStream(ctx context.Context, req *backend.PublishStreamRequest) (*backend.PublishStreamResponse, error) { - plugin, exists := m.plugin(req.PluginContext.PluginID) + plugin, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return nil, backendplugin.ErrPluginNotRegistered } @@ -131,7 +130,7 @@ func (m *PluginManager) PublishStream(ctx context.Context, req *backend.PublishS } func (m *PluginManager) RunStream(ctx context.Context, req *backend.RunStreamRequest, sender *backend.StreamSender) error { - plugin, exists := m.plugin(req.PluginContext.PluginID) + plugin, exists := m.plugin(ctx, req.PluginContext.PluginID) if !exists { return backendplugin.ErrPluginNotRegistered } diff --git a/pkg/plugins/manager/dashboard_file_store_test.go b/pkg/plugins/manager/dashboard_file_store_test.go index 0c32a5bdee463..44b5036290b04 100644 --- a/pkg/plugins/manager/dashboard_file_store_test.go +++ b/pkg/plugins/manager/dashboard_file_store_test.go @@ -190,30 +190,32 @@ func setupPluginDashboardsForTest(t *testing.T) *PluginManager { t.Helper() return &PluginManager{ - store: map[string]*plugins.Plugin{ - "pluginWithoutDashboards": { - JSONData: plugins.JSONData{ - Includes: []*plugins.Includes{ - { - Type: "page", + pluginRegistry: &fakePluginRegistry{ + store: map[string]*plugins.Plugin{ + "pluginWithoutDashboards": { + JSONData: plugins.JSONData{ + Includes: []*plugins.Includes{ + { + Type: "page", + }, }, }, }, - }, - "pluginWithDashboards": { - PluginDir: "plugins/plugin-id", - JSONData: plugins.JSONData{ - Includes: []*plugins.Includes{ - { - Type: "page", - }, - { - Type: "dashboard", - Path: "dashboards/dash1.json", - }, - { - Type: "dashboard", - Path: "dashboards/dash2.json", + "pluginWithDashboards": { + PluginDir: "plugins/plugin-id", + JSONData: plugins.JSONData{ + Includes: []*plugins.Includes{ + { + Type: "page", + }, + { + Type: "dashboard", + Path: "dashboards/dash1.json", + }, + { + Type: "dashboard", + Path: "dashboards/dash2.json", + }, }, }, }, diff --git a/pkg/plugins/manager/installer/ifaces.go b/pkg/plugins/manager/installer/ifaces.go index 9de8c07adfc29..1de32b2aa459a 100644 --- a/pkg/plugins/manager/installer/ifaces.go +++ b/pkg/plugins/manager/installer/ifaces.go @@ -1,5 +1,21 @@ package installer +import ( + "context" + + "github.com/grafana/grafana/pkg/plugins" +) + +// Service is responsible for managing plugins (add / remove) on the file system. +type Service interface { + // Install downloads the requested plugin in the provided file system location. + Install(ctx context.Context, pluginID, version, pluginsDir, pluginZipURL, pluginRepoURL string) error + // Uninstall removes the requested plugin from the provided file system location. + Uninstall(ctx context.Context, pluginDir string) error + // GetUpdateInfo provides update information for the requested plugin. + GetUpdateInfo(ctx context.Context, pluginID, version, pluginRepoURL string) (plugins.UpdateInfo, error) +} + type Logger interface { Successf(format string, args ...interface{}) Failuref(format string, args ...interface{}) diff --git a/pkg/plugins/manager/installer/installer.go b/pkg/plugins/manager/installer/installer.go index af7f61ff349fb..1d5a9543d1d20 100644 --- a/pkg/plugins/manager/installer/installer.go +++ b/pkg/plugins/manager/installer/installer.go @@ -80,7 +80,7 @@ func (e ErrVersionNotFound) Error() string { return fmt.Sprintf("%s v%s either does not exist or is not supported on your system (%s)", e.PluginID, e.RequestedVersion, e.SystemInfo) } -func New(skipTLSVerify bool, grafanaVersion string, logger Logger) plugins.Installer { +func New(skipTLSVerify bool, grafanaVersion string, logger Logger) Service { return &Installer{ httpClient: makeHttpClient(skipTLSVerify, 10*time.Second), httpClientNoTimeout: makeHttpClient(skipTLSVerify, 0), diff --git a/pkg/plugins/manager/loader/ifaces.go b/pkg/plugins/manager/loader/ifaces.go new file mode 100644 index 0000000000000..02645adeb67bc --- /dev/null +++ b/pkg/plugins/manager/loader/ifaces.go @@ -0,0 +1,13 @@ +package loader + +import ( + "context" + + "github.com/grafana/grafana/pkg/plugins" +) + +// Service is responsible for loading plugins from the file system. +type Service interface { + // Load will return a list of plugins found in the provided file system paths. + Load(ctx context.Context, class plugins.Class, paths []string, ignore map[string]struct{}) ([]*plugins.Plugin, error) +} diff --git a/pkg/plugins/manager/manager.go b/pkg/plugins/manager/manager.go index 119fd500f30d0..db3f665c74a0f 100644 --- a/pkg/plugins/manager/manager.go +++ b/pkg/plugins/manager/manager.go @@ -3,7 +3,6 @@ package manager import ( "context" "errors" - "fmt" "path/filepath" "sync" "time" @@ -12,6 +11,8 @@ import ( "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin" "github.com/grafana/grafana/pkg/plugins/manager/installer" + "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/setting" ) @@ -26,9 +27,9 @@ var _ plugins.RendererManager = (*PluginManager)(nil) type PluginManager struct { cfg *plugins.Cfg - store map[string]*plugins.Plugin - pluginInstaller plugins.Installer - pluginLoader plugins.Loader + pluginRegistry registry.Service + pluginInstaller installer.Service + pluginLoader loader.Service pluginsMu sync.RWMutex pluginSources []PluginSource log log.Logger @@ -39,8 +40,8 @@ type PluginSource struct { Paths []string } -func ProvideService(grafanaCfg *setting.Cfg, pluginLoader plugins.Loader) (*PluginManager, error) { - pm := New(plugins.FromGrafanaCfg(grafanaCfg), []PluginSource{ +func ProvideService(grafanaCfg *setting.Cfg, pluginRegistry registry.Service, pluginLoader loader.Service) (*PluginManager, error) { + pm := New(plugins.FromGrafanaCfg(grafanaCfg), pluginRegistry, []PluginSource{ {Class: plugins.Core, Paths: corePluginPaths(grafanaCfg)}, {Class: plugins.Bundled, Paths: []string{grafanaCfg.BundledPluginsPath}}, {Class: plugins.External, Paths: append([]string{grafanaCfg.PluginsPath}, pluginSettingPaths(grafanaCfg)...)}, @@ -51,12 +52,12 @@ func ProvideService(grafanaCfg *setting.Cfg, pluginLoader plugins.Loader) (*Plug return pm, nil } -func New(cfg *plugins.Cfg, pluginSources []PluginSource, pluginLoader plugins.Loader) *PluginManager { +func New(cfg *plugins.Cfg, pluginRegistry registry.Service, pluginSources []PluginSource, pluginLoader loader.Service) *PluginManager { return &PluginManager{ cfg: cfg, pluginLoader: pluginLoader, pluginSources: pluginSources, - store: make(map[string]*plugins.Plugin), + pluginRegistry: pluginRegistry, log: log.New("plugin.manager"), pluginInstaller: installer.New(false, cfg.BuildVersion, newInstallerLogger("plugin.installer", true)), } @@ -91,7 +92,7 @@ func (m *PluginManager) loadPlugins(ctx context.Context, class plugins.Class, pa } } - loadedPlugins, err := m.pluginLoader.Load(ctx, class, pluginPaths, m.registeredPlugins()) + loadedPlugins, err := m.pluginLoader.Load(ctx, class, pluginPaths, m.registeredPlugins(ctx)) if err != nil { m.log.Error("Could not load plugins", "paths", pluginPaths, "err", err) return err @@ -107,7 +108,7 @@ func (m *PluginManager) loadPlugins(ctx context.Context, class plugins.Class, pa } func (m *PluginManager) Renderer() *plugins.Plugin { - for _, p := range m.plugins() { + for _, p := range m.availablePlugins(context.TODO()) { if p.IsRenderer() { return p } @@ -119,7 +120,7 @@ func (m *PluginManager) Renderer() *plugins.Plugin { func (m *PluginManager) Routes() []*plugins.StaticRoute { staticRoutes := make([]*plugins.StaticRoute, 0) - for _, p := range m.plugins() { + for _, p := range m.availablePlugins(context.TODO()) { if p.StaticRoute() != nil { staticRoutes = append(staticRoutes, p.StaticRoute()) } @@ -127,33 +128,16 @@ func (m *PluginManager) Routes() []*plugins.StaticRoute { return staticRoutes } -func (m *PluginManager) registerAndStart(ctx context.Context, plugin *plugins.Plugin) error { - err := m.register(plugin) - if err != nil { +func (m *PluginManager) registerAndStart(ctx context.Context, p *plugins.Plugin) error { + if err := m.pluginRegistry.Add(ctx, p); err != nil { return err } - if !m.isRegistered(plugin.ID) { - return fmt.Errorf("plugin %s is not registered", plugin.ID) - } - - return m.start(ctx, plugin) -} - -func (m *PluginManager) register(p *plugins.Plugin) error { - if m.isRegistered(p.ID) { - return fmt.Errorf("plugin %s is already registered", p.ID) - } - - m.pluginsMu.Lock() - m.store[p.ID] = p - m.pluginsMu.Unlock() - if !p.IsCorePlugin() { m.log.Info("Plugin registered", "pluginId", p.ID) } - return nil + return m.start(ctx, p) } func (m *PluginManager) unregisterAndStop(ctx context.Context, p *plugins.Plugin) error { @@ -169,7 +153,9 @@ func (m *PluginManager) unregisterAndStop(ctx context.Context, p *plugins.Plugin return err } - delete(m.store, p.ID) + if err := m.pluginRegistry.Remove(ctx, p.ID); err != nil { + return err + } m.log.Debug("Plugin unregistered", "pluginId", p.ID) return nil @@ -181,7 +167,7 @@ func (m *PluginManager) start(ctx context.Context, p *plugins.Plugin) error { return nil } - if !m.isRegistered(p.ID) { + if _, exists := m.pluginRegistry.Plugin(ctx, p.ID); !exists { return backendplugin.ErrPluginNotRegistered } @@ -245,7 +231,7 @@ func restartKilledProcess(ctx context.Context, p *plugins.Plugin) error { // shutdown stops all backend plugin processes func (m *PluginManager) shutdown(ctx context.Context) { var wg sync.WaitGroup - for _, p := range m.plugins() { + for _, p := range m.availablePlugins(ctx) { wg.Add(1) go func(p backendplugin.Plugin, ctx context.Context) { defer wg.Done() diff --git a/pkg/plugins/manager/manager_integration_test.go b/pkg/plugins/manager/manager_integration_test.go index f698ac2ba96cc..a4fc6f3335a57 100644 --- a/pkg/plugins/manager/manager_integration_test.go +++ b/pkg/plugins/manager/manager_integration_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" "github.com/grafana/grafana/pkg/plugins/backendplugin/provider" "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/manager/signature" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" @@ -92,16 +93,17 @@ func TestPluginManager_int_init(t *testing.T) { coreRegistry := coreplugin.ProvideCoreRegistry(am, cw, cm, es, grap, idb, lk, otsdb, pr, tmpo, td, pg, my, ms, graf) pmCfg := plugins.FromGrafanaCfg(cfg) - pm, err := ProvideService(cfg, loader.New(pmCfg, license, signature.NewUnsignedAuthorizer(pmCfg), + pm, err := ProvideService(cfg, registry.NewInMemory(), loader.New(pmCfg, license, signature.NewUnsignedAuthorizer(pmCfg), provider.ProvideService(coreRegistry))) require.NoError(t, err) - verifyCorePluginCatalogue(t, pm) - verifyBundledPlugins(t, pm) - verifyPluginStaticRoutes(t, pm) + ctx := context.Background() + verifyCorePluginCatalogue(t, ctx, pm) + verifyBundledPlugins(t, ctx, pm) + verifyPluginStaticRoutes(t, ctx, pm) } -func verifyCorePluginCatalogue(t *testing.T, pm *PluginManager) { +func verifyCorePluginCatalogue(t *testing.T, ctx context.Context, pm *PluginManager) { t.Helper() expPanels := map[string]struct{}{ @@ -167,44 +169,44 @@ func verifyCorePluginCatalogue(t *testing.T, pm *PluginManager) { "test-app": {}, } - panels := pm.Plugins(context.Background(), plugins.Panel) + panels := pm.Plugins(ctx, plugins.Panel) assert.Equal(t, len(expPanels), len(panels)) for _, p := range panels { - p, exists := pm.Plugin(context.Background(), p.ID) + p, exists := pm.Plugin(ctx, p.ID) require.NotEqual(t, plugins.PluginDTO{}, p) assert.True(t, exists) assert.Contains(t, expPanels, p.ID) - assert.Contains(t, pm.registeredPlugins(), p.ID) + assert.Contains(t, pm.registeredPlugins(ctx), p.ID) } - dataSources := pm.Plugins(context.Background(), plugins.DataSource) + dataSources := pm.Plugins(ctx, plugins.DataSource) assert.Equal(t, len(expDataSources), len(dataSources)) for _, ds := range dataSources { - p, exists := pm.Plugin(context.Background(), ds.ID) + p, exists := pm.Plugin(ctx, ds.ID) require.NotEqual(t, plugins.PluginDTO{}, p) assert.True(t, exists) assert.Contains(t, expDataSources, ds.ID) - assert.Contains(t, pm.registeredPlugins(), ds.ID) + assert.Contains(t, pm.registeredPlugins(ctx), ds.ID) } - apps := pm.Plugins(context.Background(), plugins.App) + apps := pm.Plugins(ctx, plugins.App) assert.Equal(t, len(expApps), len(apps)) for _, app := range apps { - p, exists := pm.Plugin(context.Background(), app.ID) + p, exists := pm.Plugin(ctx, app.ID) require.NotEqual(t, plugins.PluginDTO{}, p) assert.True(t, exists) assert.Contains(t, expApps, app.ID) - assert.Contains(t, pm.registeredPlugins(), app.ID) + assert.Contains(t, pm.registeredPlugins(ctx), app.ID) } - assert.Equal(t, len(expPanels)+len(expDataSources)+len(expApps), len(pm.Plugins(context.Background()))) + assert.Equal(t, len(expPanels)+len(expDataSources)+len(expApps), len(pm.Plugins(ctx))) } -func verifyBundledPlugins(t *testing.T, pm *PluginManager) { +func verifyBundledPlugins(t *testing.T, ctx context.Context, pm *PluginManager) { t.Helper() dsPlugins := make(map[string]struct{}) - for _, p := range pm.Plugins(context.Background(), plugins.DataSource) { + for _, p := range pm.Plugins(ctx, plugins.DataSource) { dsPlugins[p.ID] = struct{}{} } @@ -213,7 +215,7 @@ func verifyBundledPlugins(t *testing.T, pm *PluginManager) { pluginRoutes[r.PluginID] = r } - inputPlugin, exists := pm.Plugin(context.Background(), "input") + inputPlugin, exists := pm.Plugin(ctx, "input") require.NotEqual(t, plugins.PluginDTO{}, inputPlugin) assert.True(t, exists) assert.NotNil(t, dsPlugins["input"]) @@ -224,7 +226,7 @@ func verifyBundledPlugins(t *testing.T, pm *PluginManager) { } } -func verifyPluginStaticRoutes(t *testing.T, pm *PluginManager) { +func verifyPluginStaticRoutes(t *testing.T, ctx context.Context, pm *PluginManager) { routes := make(map[string]*plugins.StaticRoute) for _, route := range pm.Routes() { routes[route.PluginID] = route @@ -232,11 +234,11 @@ func verifyPluginStaticRoutes(t *testing.T, pm *PluginManager) { assert.Len(t, routes, 2) - inputPlugin, _ := pm.Plugin(context.Background(), "input") + inputPlugin, _ := pm.Plugin(ctx, "input") assert.NotNil(t, routes["input"]) assert.Equal(t, routes["input"].Directory, inputPlugin.PluginDir) - testAppPlugin, _ := pm.Plugin(context.Background(), "test-app") + testAppPlugin, _ := pm.Plugin(ctx, "test-app") assert.Contains(t, routes, "test-app") assert.Equal(t, routes["test-app"].Directory, testAppPlugin.PluginDir) } diff --git a/pkg/plugins/manager/manager_test.go b/pkg/plugins/manager/manager_test.go index 1e6421c406bee..22f1c3e233c43 100644 --- a/pkg/plugins/manager/manager_test.go +++ b/pkg/plugins/manager/manager_test.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/backendplugin" + "github.com/grafana/grafana/pkg/plugins/manager/registry" ) const ( @@ -25,7 +26,7 @@ const ( func TestPluginManager_Init(t *testing.T) { t.Run("Plugin sources are loaded in order", func(t *testing.T) { loader := &fakeLoader{} - pm := New(&plugins.Cfg{}, []PluginSource{ + pm := New(&plugins.Cfg{}, newFakePluginRegistry(), []PluginSource{ {Class: plugins.Bundled, Paths: []string{"path1"}}, {Class: plugins.Core, Paths: []string{"path2"}}, {Class: plugins.External, Paths: []string{"path3"}}, @@ -181,7 +182,7 @@ func TestPluginManager_Installer(t *testing.T) { t.Run("Won't install if already installed", func(t *testing.T) { err := pm.Add(context.Background(), testPluginID, "1.0.0") - assert.Equal(t, plugins.DuplicateError{ + require.Equal(t, plugins.DuplicateError{ PluginID: p.ID, ExistingPluginDir: p.PluginDir, }, err) @@ -302,8 +303,6 @@ func TestPluginManager_Installer(t *testing.T) { func TestPluginManager_registeredPlugins(t *testing.T) { t.Run("Decommissioned plugins are included in registeredPlugins", func(t *testing.T) { - pm := New(&plugins.Cfg{}, []PluginSource{}, &fakeLoader{}) - decommissionedPlugin, _ := createPlugin(t, testPluginID, "", plugins.Core, false, true, func(plugin *plugins.Plugin) { err := plugin.Decommission() @@ -312,12 +311,14 @@ func TestPluginManager_registeredPlugins(t *testing.T) { ) require.True(t, decommissionedPlugin.IsDecommissioned()) - pm.store = map[string]*plugins.Plugin{ - testPluginID: decommissionedPlugin, - "test-app": {}, - } + pm := New(&plugins.Cfg{}, &fakePluginRegistry{ + store: map[string]*plugins.Plugin{ + testPluginID: decommissionedPlugin, + "test-app": {}, + }, + }, []PluginSource{}, &fakeLoader{}) - rps := pm.registeredPlugins() + rps := pm.registeredPlugins(context.Background()) require.Equal(t, 2, len(rps)) require.NotNil(t, rps[testPluginID]) require.NotNil(t, rps["test-app"]) @@ -334,13 +335,13 @@ func TestPluginManager_lifecycle_managed(t *testing.T) { require.Equal(t, testPluginID, ctx.plugin.ID) require.Equal(t, 1, ctx.pluginClient.startCount) testPlugin, exists := ctx.manager.Plugin(context.Background(), testPluginID) - assert.True(t, exists) + require.True(t, exists) require.NotNil(t, testPlugin) t.Run("Should not be able to register an already registered plugin", func(t *testing.T) { err := ctx.manager.registerAndStart(context.Background(), ctx.plugin) - require.Equal(t, 1, ctx.pluginClient.startCount) require.Error(t, err) + require.Equal(t, 1, ctx.pluginClient.startCount) }) t.Run("When manager runs should start and stop plugin", func(t *testing.T) { @@ -481,7 +482,9 @@ func TestPluginManager_lifecycle_unmanaged(t *testing.T) { t.Run("Should be able to register plugin", func(t *testing.T) { err := ctx.manager.registerAndStart(context.Background(), ctx.plugin) require.NoError(t, err) - require.True(t, ctx.manager.isRegistered(testPluginID)) + p, exists := ctx.manager.Plugin(context.Background(), testPluginID) + require.True(t, exists) + require.NotNil(t, p) require.False(t, ctx.pluginClient.managed) t.Run("When manager runs should not start plugin", func(t *testing.T) { @@ -521,7 +524,7 @@ func TestPluginManager_lifecycle_unmanaged(t *testing.T) { func createManager(t *testing.T, cbs ...func(*PluginManager)) *PluginManager { t.Helper() - pm := New(&plugins.Cfg{}, nil, &fakeLoader{}) + pm := New(&plugins.Cfg{}, newFakePluginRegistry(), nil, &fakeLoader{}) for _, cb := range cbs { cb(pm) @@ -575,16 +578,13 @@ func newScenario(t *testing.T, managed bool, fn func(t *testing.T, ctx *managerS cfg := &plugins.Cfg{} cfg.AWSAllowedAuthProviders = []string{"keys", "credentials"} cfg.AWSAssumeRoleEnabled = true - cfg.Azure = &azsettings.AzureSettings{ ManagedIdentityEnabled: true, Cloud: "AzureCloud", ManagedIdentityClientId: "client-id", } - loader := &fakeLoader{} - manager := New(cfg, nil, loader) - manager.pluginLoader = loader + manager := New(cfg, registry.NewInMemory(), nil, &fakeLoader{}) ctx := &managerScenarioCtx{ manager: manager, } @@ -601,8 +601,6 @@ func verifyNoPluginErrors(t *testing.T, pm *PluginManager) { } type fakePluginInstaller struct { - plugins.Installer - installCount int uninstallCount int } @@ -758,3 +756,38 @@ func (s *fakeSender) Send(crr *backend.CallResourceResponse) error { return nil } + +type fakePluginRegistry struct { + store map[string]*plugins.Plugin +} + +func newFakePluginRegistry() *fakePluginRegistry { + return &fakePluginRegistry{ + store: make(map[string]*plugins.Plugin), + } +} + +func (f *fakePluginRegistry) Plugin(_ context.Context, id string) (*plugins.Plugin, bool) { + p, exists := f.store[id] + return p, exists +} + +func (f *fakePluginRegistry) Plugins(_ context.Context) []*plugins.Plugin { + var res []*plugins.Plugin + + for _, p := range f.store { + res = append(res, p) + } + + return res +} + +func (f *fakePluginRegistry) Add(_ context.Context, p *plugins.Plugin) error { + f.store[p.ID] = p + return nil +} + +func (f *fakePluginRegistry) Remove(_ context.Context, id string) error { + delete(f.store, id) + return nil +} diff --git a/pkg/plugins/manager/registry/ifaces.go b/pkg/plugins/manager/registry/ifaces.go new file mode 100644 index 0000000000000..5f81086c5fcb9 --- /dev/null +++ b/pkg/plugins/manager/registry/ifaces.go @@ -0,0 +1,19 @@ +package registry + +import ( + "context" + + "github.com/grafana/grafana/pkg/plugins" +) + +// Service is responsible for the storing and retrieval of plugins. +type Service interface { + // Plugin finds a plugin by its ID. + Plugin(ctx context.Context, id 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 +} diff --git a/pkg/plugins/manager/registry/in_memory.go b/pkg/plugins/manager/registry/in_memory.go new file mode 100644 index 0000000000000..b6bf6ab3c07ad --- /dev/null +++ b/pkg/plugins/manager/registry/in_memory.go @@ -0,0 +1,81 @@ +package registry + +import ( + "context" + "fmt" + "sync" + + "github.com/grafana/grafana/pkg/plugins" +) + +type InMemory struct { + store map[string]*plugins.Plugin + mu sync.RWMutex +} + +func ProvideService() *InMemory { + return NewInMemory() +} + +func NewInMemory() *InMemory { + return &InMemory{ + store: make(map[string]*plugins.Plugin), + } +} + +func (i *InMemory) Plugin(_ context.Context, pluginID string) (*plugins.Plugin, bool) { + return i.plugin(pluginID) +} + +func (i *InMemory) Plugins(_ context.Context) []*plugins.Plugin { + i.mu.RLock() + defer i.mu.RUnlock() + + res := make([]*plugins.Plugin, 0) + for _, p := range i.store { + res = append(res, p) + } + + return res +} + +func (i *InMemory) Add(_ context.Context, p *plugins.Plugin) error { + if i.isRegistered(p.ID) { + return fmt.Errorf("plugin %s is already registered", p.ID) + } + + i.mu.Lock() + i.store[p.ID] = p + i.mu.Unlock() + + return nil +} + +func (i *InMemory) Remove(_ context.Context, pluginID string) error { + if !i.isRegistered(pluginID) { + return fmt.Errorf("plugin %s is not registered", pluginID) + } + + i.mu.Lock() + delete(i.store, pluginID) + i.mu.Unlock() + + return nil +} + +func (i *InMemory) plugin(pluginID string) (*plugins.Plugin, bool) { + i.mu.RLock() + defer i.mu.RUnlock() + p, exists := i.store[pluginID] + + if !exists { + return nil, false + } + + return p, true +} + +func (i *InMemory) isRegistered(pluginID string) bool { + _, exists := i.plugin(pluginID) + return exists +} diff --git a/pkg/plugins/manager/registry/in_memory_test.go b/pkg/plugins/manager/registry/in_memory_test.go new file mode 100644 index 0000000000000..d2ac98f508835 --- /dev/null +++ b/pkg/plugins/manager/registry/in_memory_test.go @@ -0,0 +1,268 @@ +package registry + +import ( + "context" + "fmt" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/plugins" +) + +const pluginID = "test-ds" + +func TestInMemory(t *testing.T) { + t.Run("Test mix of registry operations", func(t *testing.T) { + i := &InMemory{ + store: map[string]*plugins.Plugin{}, + } + ctx := context.Background() + + p, exists := i.Plugin(ctx, pluginID) + require.False(t, exists) + require.Nil(t, p) + + err := i.Remove(ctx, pluginID) + require.EqualError(t, err, fmt.Errorf("plugin %s is not registered", pluginID).Error()) + + p = &plugins.Plugin{JSONData: plugins.JSONData{ID: pluginID}} + err = i.Add(ctx, p) + require.NoError(t, err) + + existingP, exists := i.Plugin(ctx, pluginID) + require.True(t, exists) + require.Equal(t, p, existingP) + + err = i.Remove(ctx, pluginID) + require.NoError(t, err) + + existingPlugins := i.Plugins(ctx) + require.Empty(t, existingPlugins) + }) +} + +func TestInMemory_Add(t *testing.T) { + type mocks struct { + store map[string]*plugins.Plugin + } + type args struct { + p *plugins.Plugin + } + var tests = []struct { + name string + mocks mocks + args args + err error + }{ + { + name: "Can add a new plugin to the registry", + mocks: mocks{ + store: map[string]*plugins.Plugin{}, + }, + args: args{ + p: &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + }, + }, + { + name: "Cannot add a plugin to the registry if it already exists", + mocks: mocks{ + store: map[string]*plugins.Plugin{ + pluginID: { + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + }, + }, + args: args{ + p: &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + }, + err: fmt.Errorf("plugin %s is already registered", pluginID), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + i := &InMemory{ + store: tt.mocks.store, + } + err := i.Add(context.Background(), tt.args.p) + require.Equal(t, tt.err, err) + }) + } +} + +func TestInMemory_Plugin(t *testing.T) { + type mocks struct { + store map[string]*plugins.Plugin + } + type args struct { + pluginID string + } + tests := []struct { + name string + mocks mocks + args args + expected *plugins.Plugin + exists bool + }{ + { + name: "Can retrieve a plugin", + mocks: mocks{ + store: map[string]*plugins.Plugin{ + pluginID: { + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + }, + }, + args: args{ + pluginID: pluginID, + }, + expected: &plugins.Plugin{ + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + exists: true, + }, + { + name: "Non-existing plugin", + mocks: mocks{ + store: map[string]*plugins.Plugin{}, + }, + args: args{ + pluginID: pluginID, + }, + expected: nil, + exists: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + i := &InMemory{ + store: tt.mocks.store, + } + p, exists := i.Plugin(context.Background(), tt.args.pluginID) + if exists != tt.exists { + t.Errorf("Plugin() got1 = %v, expected %v", exists, tt.exists) + } + require.Equal(t, tt.expected, p) + }) + } +} + +func TestInMemory_Plugins(t *testing.T) { + type mocks struct { + store map[string]*plugins.Plugin + } + tests := []struct { + name string + mocks mocks + expected []*plugins.Plugin + }{ + { + name: "Can retrieve a list of plugin", + mocks: mocks{ + store: map[string]*plugins.Plugin{ + pluginID: { + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + "test-panel": { + JSONData: plugins.JSONData{ + ID: "test-panel", + }, + }, + }, + }, + expected: []*plugins.Plugin{ + { + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + { + JSONData: plugins.JSONData{ + ID: "test-panel", + }, + }, + }, + }, + { + name: "No existing plugins", + mocks: mocks{ + store: map[string]*plugins.Plugin{}, + }, + expected: []*plugins.Plugin{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + i := &InMemory{ + store: tt.mocks.store, + } + result := i.Plugins(context.Background()) + require.Equal(t, tt.expected, result) + }) + } +} + +func TestInMemory_Remove(t *testing.T) { + type mocks struct { + store map[string]*plugins.Plugin + } + type args struct { + pluginID string + } + tests := []struct { + name string + mocks mocks + args args + err error + }{ + { + name: "Can remove a plugin", + mocks: mocks{ + store: map[string]*plugins.Plugin{ + pluginID: { + JSONData: plugins.JSONData{ + ID: pluginID, + }, + }, + }, + }, + args: args{ + pluginID: pluginID, + }, + }, { + name: "Cannot remove a plugin from the registry if it doesn't exist", + mocks: mocks{ + store: map[string]*plugins.Plugin{}, + }, + args: args{ + pluginID: pluginID, + }, + err: fmt.Errorf("plugin %s is not registered", pluginID), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + i := &InMemory{ + store: tt.mocks.store, + } + err := i.Remove(context.Background(), tt.args.pluginID) + require.Equal(t, tt.err, err) + }) + } +} diff --git a/pkg/plugins/manager/store.go b/pkg/plugins/manager/store.go index 487cc1c7f7adb..696866bcefb96 100644 --- a/pkg/plugins/manager/store.go +++ b/pkg/plugins/manager/store.go @@ -8,9 +8,8 @@ import ( "github.com/grafana/grafana/pkg/plugins" ) -func (m *PluginManager) Plugin(_ context.Context, pluginID string) (plugins.PluginDTO, bool) { - p, exists := m.plugin(pluginID) - +func (m *PluginManager) Plugin(ctx context.Context, pluginID string) (plugins.PluginDTO, bool) { + p, exists := m.plugin(ctx, pluginID) if !exists { return plugins.PluginDTO{}, false } @@ -18,7 +17,7 @@ func (m *PluginManager) Plugin(_ context.Context, pluginID string) (plugins.Plug return p.ToDTO(), true } -func (m *PluginManager) Plugins(_ context.Context, pluginTypes ...plugins.Type) []plugins.PluginDTO { +func (m *PluginManager) Plugins(ctx context.Context, pluginTypes ...plugins.Type) []plugins.PluginDTO { // if no types passed, assume all if len(pluginTypes) == 0 { pluginTypes = plugins.PluginTypes @@ -30,7 +29,7 @@ func (m *PluginManager) Plugins(_ context.Context, pluginTypes ...plugins.Type) } pluginsList := make([]plugins.PluginDTO, 0) - for _, p := range m.plugins() { + for _, p := range m.availablePlugins(ctx) { if _, exists := requestedTypes[p.Type]; exists { pluginsList = append(pluginsList, p.ToDTO()) } @@ -38,44 +37,31 @@ func (m *PluginManager) Plugins(_ context.Context, pluginTypes ...plugins.Type) return pluginsList } -func (m *PluginManager) plugin(pluginID string) (*plugins.Plugin, bool) { - m.pluginsMu.RLock() - defer m.pluginsMu.RUnlock() - p, exists := m.store[pluginID] - - if !exists || (p.IsDecommissioned()) { +// plugin finds a plugin with `pluginID` from the registry that is not decommissioned +func (m *PluginManager) plugin(ctx context.Context, pluginID string) (*plugins.Plugin, bool) { + p, exists := m.pluginRegistry.Plugin(ctx, pluginID) + if !exists || p.IsDecommissioned() { return nil, false } return p, true } -func (m *PluginManager) plugins() []*plugins.Plugin { - m.pluginsMu.RLock() - defer m.pluginsMu.RUnlock() - - res := make([]*plugins.Plugin, 0) - for _, p := range m.store { +// availablePlugins returns all non-decommissioned plugins from the registry +func (m *PluginManager) availablePlugins(ctx context.Context) []*plugins.Plugin { + var res []*plugins.Plugin + for _, p := range m.pluginRegistry.Plugins(ctx) { if !p.IsDecommissioned() { res = append(res, p) } } - return res } -func (m *PluginManager) isRegistered(pluginID string) bool { - p, exists := m.plugin(pluginID) - if !exists { - return false - } - - return !p.IsDecommissioned() -} - -func (m *PluginManager) registeredPlugins() map[string]struct{} { +// registeredPlugins returns all registered plugins from the registry +func (m *PluginManager) registeredPlugins(ctx context.Context) map[string]struct{} { pluginsByID := make(map[string]struct{}) - for _, p := range m.store { + for _, p := range m.pluginRegistry.Plugins(ctx) { pluginsByID[p.ID] = struct{}{} } @@ -85,7 +71,7 @@ func (m *PluginManager) registeredPlugins() map[string]struct{} { func (m *PluginManager) Add(ctx context.Context, pluginID, version string) error { var pluginZipURL string - if plugin, exists := m.plugin(pluginID); exists { + if plugin, exists := m.plugin(ctx, pluginID); exists { if !plugin.IsExternalPlugin() { return plugins.ErrInstallCorePlugin } @@ -126,7 +112,7 @@ func (m *PluginManager) Add(ctx context.Context, pluginID, version string) error } func (m *PluginManager) Remove(ctx context.Context, pluginID string) error { - plugin, exists := m.plugin(pluginID) + plugin, exists := m.plugin(ctx, pluginID) if !exists { return plugins.ErrPluginNotInstalled } @@ -141,11 +127,8 @@ func (m *PluginManager) Remove(ctx context.Context, pluginID string) error { return plugins.ErrUninstallOutsideOfPluginDir } - if m.isRegistered(pluginID) { - err := m.unregisterAndStop(ctx, plugin) - if err != nil { - return err - } + if err := m.unregisterAndStop(ctx, plugin); err != nil { + return err } return m.pluginInstaller.Uninstall(ctx, plugin.PluginDir) diff --git a/pkg/server/wire.go b/pkg/server/wire.go index d1ccb21a460fa..d996a3acd0e68 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -33,6 +33,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/backendplugin/coreplugin" "github.com/grafana/grafana/pkg/plugins/manager" "github.com/grafana/grafana/pkg/plugins/manager/loader" + "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/plugins/plugincontext" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" @@ -140,6 +141,8 @@ var wireBasicSet = wire.NewSet( updatechecker.ProvidePluginsService, uss.ProvideService, wire.Bind(new(usagestats.Service), new(*uss.UsageStats)), + registry.ProvideService, + wire.Bind(new(registry.Service), new(*registry.InMemory)), manager.ProvideService, wire.Bind(new(plugins.Client), new(*manager.PluginManager)), wire.Bind(new(plugins.Store), new(*manager.PluginManager)), @@ -148,7 +151,7 @@ var wireBasicSet = wire.NewSet( wire.Bind(new(plugins.RendererManager), new(*manager.PluginManager)), coreplugin.ProvideCoreRegistry, loader.ProvideService, - wire.Bind(new(plugins.Loader), new(*loader.Loader)), + wire.Bind(new(loader.Service), new(*loader.Loader)), wire.Bind(new(plugins.ErrorResolver), new(*loader.Loader)), cloudwatch.ProvideService, cloudmonitoring.ProvideService,