From b0e74cf73784178f1129d6e617a9f534ae841916 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Mon, 27 Jan 2025 16:39:46 +0100 Subject: [PATCH] Grafana Advisor: Plugin checks (#99502) --- .../pkg/app/checkregistry/checkregistry.go | 19 +- .../pkg/app/checks/plugincheck/check.go | 108 +++++++++++ .../pkg/app/checks/plugincheck/check_test.go | 181 ++++++++++++++++++ pkg/api/http_server.go | 5 +- pkg/api/plugins.go | 12 +- pkg/api/plugins_test.go | 2 + .../plugininstaller/checker.go | 36 ++++ .../plugininstaller/checker_test.go | 36 ++++ .../pluginsintegration/pluginsintegration.go | 2 + 9 files changed, 391 insertions(+), 10 deletions(-) create mode 100644 apps/advisor/pkg/app/checks/plugincheck/check.go create mode 100644 apps/advisor/pkg/app/checks/plugincheck/check_test.go create mode 100644 pkg/services/pluginsintegration/plugininstaller/checker.go create mode 100644 pkg/services/pluginsintegration/plugininstaller/checker_test.go diff --git a/apps/advisor/pkg/app/checkregistry/checkregistry.go b/apps/advisor/pkg/app/checkregistry/checkregistry.go index 0c375f035c5..e1bd27387d0 100644 --- a/apps/advisor/pkg/app/checkregistry/checkregistry.go +++ b/apps/advisor/pkg/app/checkregistry/checkregistry.go @@ -3,9 +3,13 @@ package checkregistry import ( "github.com/grafana/grafana/apps/advisor/pkg/app/checks" "github.com/grafana/grafana/apps/advisor/pkg/app/checks/datasourcecheck" + "github.com/grafana/grafana/apps/advisor/pkg/app/checks/plugincheck" "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/repo" "github.com/grafana/grafana/pkg/registry/apis/datasource" "github.com/grafana/grafana/pkg/services/datasources" + "github.com/grafana/grafana/pkg/services/pluginsintegration/managedplugins" + "github.com/grafana/grafana/pkg/services/pluginsintegration/plugininstaller" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" ) @@ -18,15 +22,22 @@ type Service struct { pluginStore pluginstore.Store pluginContextProvider datasource.PluginContextWrapper pluginClient plugins.Client + pluginRepo repo.Service + pluginPreinstall plugininstaller.Preinstall + managedPlugins managedplugins.Manager } func ProvideService(datasourceSvc datasources.DataSourceService, pluginStore pluginstore.Store, - pluginContextProvider datasource.PluginContextWrapper, pluginClient plugins.Client) *Service { + pluginContextProvider datasource.PluginContextWrapper, pluginClient plugins.Client, + pluginRepo repo.Service, pluginPreinstall plugininstaller.Preinstall, managedPlugins managedplugins.Manager) *Service { return &Service{ datasourceSvc: datasourceSvc, pluginStore: pluginStore, pluginContextProvider: pluginContextProvider, pluginClient: pluginClient, + pluginRepo: pluginRepo, + pluginPreinstall: pluginPreinstall, + managedPlugins: managedPlugins, } } @@ -38,5 +49,11 @@ func (s *Service) Checks() []checks.Check { s.pluginContextProvider, s.pluginClient, ), + plugincheck.New( + s.pluginStore, + s.pluginRepo, + s.pluginPreinstall, + s.managedPlugins, + ), } } diff --git a/apps/advisor/pkg/app/checks/plugincheck/check.go b/apps/advisor/pkg/app/checks/plugincheck/check.go new file mode 100644 index 00000000000..2d0f10f7a72 --- /dev/null +++ b/apps/advisor/pkg/app/checks/plugincheck/check.go @@ -0,0 +1,108 @@ +package plugincheck + +import ( + "context" + "fmt" + sysruntime "runtime" + + "github.com/Masterminds/semver/v3" + advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" + "github.com/grafana/grafana/apps/advisor/pkg/app/checks" + "github.com/grafana/grafana/pkg/cmd/grafana-cli/services" + "github.com/grafana/grafana/pkg/plugins/repo" + "github.com/grafana/grafana/pkg/services/pluginsintegration/managedplugins" + "github.com/grafana/grafana/pkg/services/pluginsintegration/plugininstaller" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" +) + +func New( + pluginStore pluginstore.Store, + pluginRepo repo.Service, + pluginPreinstall plugininstaller.Preinstall, + managedPlugins managedplugins.Manager, +) checks.Check { + return &check{ + PluginStore: pluginStore, + PluginRepo: pluginRepo, + PluginPreinstall: pluginPreinstall, + ManagedPlugins: managedPlugins, + } +} + +type check struct { + PluginStore pluginstore.Store + PluginRepo repo.Service + PluginPreinstall plugininstaller.Preinstall + ManagedPlugins managedplugins.Manager +} + +func (c *check) Type() string { + return "plugin" +} + +func (c *check) Run(ctx context.Context, _ *advisor.CheckSpec) (*advisor.CheckV0alpha1StatusReport, error) { + ps := c.PluginStore.Plugins(ctx) + + errs := []advisor.CheckV0alpha1StatusReportErrors{} + for _, p := range ps { + // Skip if it's a core plugin + if p.IsCorePlugin() { + continue + } + + // Check if plugin is deprecated + i, err := c.PluginRepo.PluginInfo(ctx, p.ID) + if err != nil { + continue + } + if i.Status == "deprecated" { + errs = append(errs, advisor.CheckV0alpha1StatusReportErrors{ + Severity: advisor.CheckStatusSeverityHigh, + Reason: fmt.Sprintf("Plugin deprecated: %s", p.ID), + Action: "Look for alternatives", + }) + } + + // Check if plugin has a newer version, only if it's not managed or pinned + if c.isManaged(ctx, p.ID) || c.PluginPreinstall.IsPinned(p.ID) { + continue + } + compatOpts := repo.NewCompatOpts(services.GrafanaVersion, sysruntime.GOOS, sysruntime.GOARCH) + info, err := c.PluginRepo.GetPluginArchiveInfo(ctx, p.ID, "", compatOpts) + if err != nil { + continue + } + if hasUpdate(p, info) { + errs = append(errs, advisor.CheckV0alpha1StatusReportErrors{ + Severity: advisor.CheckStatusSeverityLow, + Reason: fmt.Sprintf("New version available: %s", p.ID), + Action: "Update plugin", + }) + } + } + + return &advisor.CheckV0alpha1StatusReport{ + Count: int64(len(ps)), + Errors: errs, + }, nil +} + +func hasUpdate(current pluginstore.Plugin, latest *repo.PluginArchiveInfo) bool { + // If both versions are semver-valid, compare them + v1, err1 := semver.NewVersion(current.Info.Version) + v2, err2 := semver.NewVersion(latest.Version) + if err1 == nil && err2 == nil { + return v1.LessThan(v2) + } + // In other case, assume that a different latest version will always be newer + return current.Info.Version != latest.Version +} + +func (c *check) isManaged(ctx context.Context, pluginID string) bool { + for _, managedPlugin := range c.ManagedPlugins.ManagedPlugins(ctx) { + if managedPlugin == pluginID { + return true + } + } + return false +} diff --git a/apps/advisor/pkg/app/checks/plugincheck/check_test.go b/apps/advisor/pkg/app/checks/plugincheck/check_test.go new file mode 100644 index 00000000000..a831f41b67f --- /dev/null +++ b/apps/advisor/pkg/app/checks/plugincheck/check_test.go @@ -0,0 +1,181 @@ +package plugincheck + +import ( + "context" + "testing" + + advisor "github.com/grafana/grafana/apps/advisor/pkg/apis/advisor/v0alpha1" + "github.com/grafana/grafana/pkg/plugins" + "github.com/grafana/grafana/pkg/plugins/repo" + "github.com/grafana/grafana/pkg/services/pluginsintegration/managedplugins" + "github.com/grafana/grafana/pkg/services/pluginsintegration/plugininstaller" + "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" + "github.com/stretchr/testify/assert" +) + +func TestRun(t *testing.T) { + tests := []struct { + name string + plugins []pluginstore.Plugin + pluginInfo map[string]*repo.PluginInfo + pluginArchives map[string]*repo.PluginArchiveInfo + pluginPreinstalled []string + pluginManaged []string + expectedErrors []advisor.CheckV0alpha1StatusReportErrors + }{ + { + name: "No plugins", + plugins: []pluginstore.Plugin{}, + expectedErrors: []advisor.CheckV0alpha1StatusReportErrors{}, + }, + { + name: "Deprecated plugin", + plugins: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "plugin1", Info: plugins.Info{Version: "1.0.0"}}}, + }, + pluginInfo: map[string]*repo.PluginInfo{ + "plugin1": {Status: "deprecated"}, + }, + pluginArchives: map[string]*repo.PluginArchiveInfo{ + "plugin1": {Version: "1.0.0"}, + }, + expectedErrors: []advisor.CheckV0alpha1StatusReportErrors{ + { + Severity: advisor.CheckStatusSeverityHigh, + Reason: "Plugin deprecated: plugin1", + Action: "Look for alternatives", + }, + }, + }, + { + name: "Plugin with update", + plugins: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "plugin2", Info: plugins.Info{Version: "1.0.0"}}}, + }, + pluginInfo: map[string]*repo.PluginInfo{ + "plugin2": {Status: "active"}, + }, + pluginArchives: map[string]*repo.PluginArchiveInfo{ + "plugin2": {Version: "1.1.0"}, + }, + expectedErrors: []advisor.CheckV0alpha1StatusReportErrors{ + { + Severity: advisor.CheckStatusSeverityLow, + Reason: "New version available: plugin2", + Action: "Update plugin", + }, + }, + }, + { + name: "Plugin with update (non semver)", + plugins: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "plugin2", Info: plugins.Info{Version: "alpha"}}}, + }, + pluginInfo: map[string]*repo.PluginInfo{ + "plugin2": {Status: "active"}, + }, + pluginArchives: map[string]*repo.PluginArchiveInfo{ + "plugin2": {Version: "beta"}, + }, + expectedErrors: []advisor.CheckV0alpha1StatusReportErrors{ + { + Severity: advisor.CheckStatusSeverityLow, + Reason: "New version available: plugin2", + Action: "Update plugin", + }, + }, + }, + { + name: "Plugin pinned", + plugins: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "plugin3", Info: plugins.Info{Version: "1.0.0"}}}, + }, + pluginInfo: map[string]*repo.PluginInfo{ + "plugin3": {Status: "active"}, + }, + pluginArchives: map[string]*repo.PluginArchiveInfo{ + "plugin3": {Version: "1.1.0"}, + }, + pluginPreinstalled: []string{"plugin3"}, + expectedErrors: []advisor.CheckV0alpha1StatusReportErrors{}, + }, + { + name: "Managed plugin", + plugins: []pluginstore.Plugin{ + {JSONData: plugins.JSONData{ID: "plugin4", Info: plugins.Info{Version: "1.0.0"}}}, + }, + pluginInfo: map[string]*repo.PluginInfo{ + "plugin4": {Status: "active"}, + }, + pluginArchives: map[string]*repo.PluginArchiveInfo{ + "plugin4": {Version: "1.1.0"}, + }, + pluginManaged: []string{"plugin4"}, + expectedErrors: []advisor.CheckV0alpha1StatusReportErrors{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pluginStore := &mockPluginStore{plugins: tt.plugins} + pluginRepo := &mockPluginRepo{ + pluginInfo: tt.pluginInfo, + pluginArchiveInfo: tt.pluginArchives, + } + pluginPreinstall := &mockPluginPreinstall{pinned: tt.pluginPreinstalled} + managedPlugins := &mockManagedPlugins{managed: tt.pluginManaged} + check := New(pluginStore, pluginRepo, pluginPreinstall, managedPlugins) + + report, err := check.Run(context.Background(), nil) + assert.NoError(t, err) + assert.Equal(t, int64(len(tt.plugins)), report.Count) + assert.Equal(t, tt.expectedErrors, report.Errors) + }) + } +} + +type mockPluginStore struct { + pluginstore.Store + plugins []pluginstore.Plugin +} + +func (m *mockPluginStore) Plugins(ctx context.Context, t ...plugins.Type) []pluginstore.Plugin { + return m.plugins +} + +type mockPluginRepo struct { + repo.Service + pluginInfo map[string]*repo.PluginInfo + pluginArchiveInfo map[string]*repo.PluginArchiveInfo +} + +func (m *mockPluginRepo) PluginInfo(ctx context.Context, id string) (*repo.PluginInfo, error) { + return m.pluginInfo[id], nil +} + +func (m *mockPluginRepo) GetPluginArchiveInfo(ctx context.Context, id, version string, opts repo.CompatOpts) (*repo.PluginArchiveInfo, error) { + return m.pluginArchiveInfo[id], nil +} + +type mockPluginPreinstall struct { + plugininstaller.Preinstall + pinned []string +} + +func (m *mockPluginPreinstall) IsPinned(pluginID string) bool { + for _, p := range m.pinned { + if p == pluginID { + return true + } + } + return false +} + +type mockManagedPlugins struct { + managedplugins.Manager + managed []string +} + +func (m *mockManagedPlugins) ManagedPlugins(ctx context.Context) []string { + return m.managed +} diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 8b553284221..5d2308e7540 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -82,6 +82,7 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/managedplugins" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginassets" "github.com/grafana/grafana/pkg/services/pluginsintegration/plugincontext" + "github.com/grafana/grafana/pkg/services/pluginsintegration/plugininstaller" pluginSettings "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" pref "github.com/grafana/grafana/pkg/services/preference" @@ -149,6 +150,7 @@ type HTTPServer struct { pluginStaticRouteResolver plugins.StaticRouteResolver pluginErrorResolver plugins.ErrorResolver pluginAssets *pluginassets.Service + pluginPreinstall plugininstaller.Preinstall SearchService search.Service ShortURLService shorturls.Service QueryHistoryService queryhistory.Service @@ -270,7 +272,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi annotationRepo annotations.Repository, tagService tag.Service, searchv2HTTPService searchV2.SearchHTTPService, oauthTokenService oauthtoken.OAuthTokenService, statsService stats.Service, authnService authn.Service, pluginsCDNService *pluginscdn.Service, promGatherer prometheus.Gatherer, starApi *starApi.API, promRegister prometheus.Registerer, clientConfigProvider grafanaapiserver.DirectRestConfigProvider, anonService anonymous.Service, - userVerifier user.Verifier, + userVerifier user.Verifier, pluginPreinstall plugininstaller.Preinstall, ) (*HTTPServer, error) { web.Env = cfg.Env m := web.New() @@ -295,6 +297,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi pluginFileStore: pluginFileStore, grafanaUpdateChecker: grafanaUpdateChecker, pluginsUpdateChecker: pluginsUpdateChecker, + pluginPreinstall: pluginPreinstall, SettingsProvider: settingsProvider, DataSourceCache: dataSourceCache, AuthTokenService: userTokenService, diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index b608165a8e0..4fa2a45c9f0 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -466,10 +466,8 @@ func (hs *HTTPServer) InstallPlugin(c *contextmodel.ReqContext) response.Respons hs.log.Info("Plugin install/update requested", "pluginId", pluginID, "user", c.Login) - for _, preinstalled := range hs.Cfg.PreinstallPlugins { - if preinstalled.ID == pluginID && preinstalled.Version != "" { - return response.Error(http.StatusConflict, "Cannot update a pinned pre-installed plugin", nil) - } + for hs.pluginPreinstall.IsPinned(pluginID) { + return response.Error(http.StatusConflict, "Cannot update a pinned pre-installed plugin", nil) } compatOpts := plugins.NewAddOpts(hs.Cfg.BuildVersion, runtime.GOOS, runtime.GOARCH, "") @@ -511,10 +509,8 @@ func (hs *HTTPServer) UninstallPlugin(c *contextmodel.ReqContext) response.Respo return response.Error(http.StatusNotFound, "Plugin not installed", nil) } - for _, preinstalled := range hs.Cfg.PreinstallPlugins { - if preinstalled.ID == pluginID { - return response.Error(http.StatusConflict, "Cannot uninstall a pre-installed plugin", nil) - } + for hs.pluginPreinstall.IsPreinstalled(pluginID) { + return response.Error(http.StatusConflict, "Cannot uninstall a pre-installed plugin", nil) } err := hs.pluginInstaller.Remove(c.Req.Context(), pluginID, plugin.Info.Version) diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index 5dd1ab5aa67..56d0d526db0 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -45,6 +45,7 @@ import ( "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginaccesscontrol" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginassets" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginerrs" + "github.com/grafana/grafana/pkg/services/pluginsintegration/plugininstaller" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginstore" "github.com/grafana/grafana/pkg/services/updatechecker" @@ -111,6 +112,7 @@ func Test_PluginsInstallAndUninstall(t *testing.T) { }, }) hs.managedPluginsService = managedplugins.NewNoop() + hs.pluginPreinstall = plugininstaller.ProvidePreinstall(hs.Cfg) expectedIdentity := &authn.Identity{ OrgID: tc.permissionOrg, diff --git a/pkg/services/pluginsintegration/plugininstaller/checker.go b/pkg/services/pluginsintegration/plugininstaller/checker.go new file mode 100644 index 00000000000..58cfde3571a --- /dev/null +++ b/pkg/services/pluginsintegration/plugininstaller/checker.go @@ -0,0 +1,36 @@ +package plugininstaller + +import "github.com/grafana/grafana/pkg/setting" + +type Preinstall interface { + IsPreinstalled(pluginID string) bool + IsPinned(pluginID string) bool +} + +func ProvidePreinstall( + cfg *setting.Cfg, +) *PreinstallImpl { + plugins := make(map[string]*setting.InstallPlugin) + for _, p := range cfg.PreinstallPlugins { + plugins[p.ID] = &p + } + return &PreinstallImpl{ + plugins: plugins, + } +} + +type PreinstallImpl struct { + plugins map[string]*setting.InstallPlugin +} + +func (c *PreinstallImpl) IsPreinstalled(pluginID string) bool { + _, ok := c.plugins[pluginID] + return ok +} + +func (c *PreinstallImpl) IsPinned(pluginID string) bool { + if p, ok := c.plugins[pluginID]; ok { + return p.Version != "" + } + return false +} diff --git a/pkg/services/pluginsintegration/plugininstaller/checker_test.go b/pkg/services/pluginsintegration/plugininstaller/checker_test.go new file mode 100644 index 00000000000..b099ec8138b --- /dev/null +++ b/pkg/services/pluginsintegration/plugininstaller/checker_test.go @@ -0,0 +1,36 @@ +package plugininstaller + +import ( + "testing" + + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" +) + +func TestIsPreinstalled(t *testing.T) { + cfg := &setting.Cfg{ + PreinstallPlugins: []setting.InstallPlugin{ + {ID: "plugin1"}, + {ID: "plugin2"}, + }, + } + preinstall := ProvidePreinstall(cfg) + + assert.True(t, preinstall.IsPreinstalled("plugin1")) + assert.True(t, preinstall.IsPreinstalled("plugin2")) + assert.False(t, preinstall.IsPreinstalled("plugin3")) +} + +func TestIsPinned(t *testing.T) { + cfg := &setting.Cfg{ + PreinstallPlugins: []setting.InstallPlugin{ + {ID: "plugin1", Version: "1.0.0"}, + {ID: "plugin2"}, + }, + } + preinstall := ProvidePreinstall(cfg) + + assert.True(t, preinstall.IsPinned("plugin1")) + assert.False(t, preinstall.IsPinned("plugin2")) + assert.False(t, preinstall.IsPinned("plugin3")) +} diff --git a/pkg/services/pluginsintegration/pluginsintegration.go b/pkg/services/pluginsintegration/pluginsintegration.go index 1851106b945..22929782cf8 100644 --- a/pkg/services/pluginsintegration/pluginsintegration.go +++ b/pkg/services/pluginsintegration/pluginsintegration.go @@ -129,6 +129,8 @@ var WireSet = wire.NewSet( wire.Bind(new(plugincontext.BasePluginContextProvider), new(*plugincontext.BaseProvider)), plugininstaller.ProvideService, pluginassets.ProvideService, + plugininstaller.ProvidePreinstall, + wire.Bind(new(plugininstaller.Preinstall), new(*plugininstaller.PreinstallImpl)), ) // WireExtensionSet provides a wire.ProviderSet of plugin providers that can be