From 21bf013a8eb601d98bb52d413fee3ea9eb899b75 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Wed, 21 Aug 2024 16:11:55 +0200 Subject: [PATCH] Add support for synchronous plugin installation (#92129) --- pkg/api/frontendsettings.go | 2 +- pkg/api/plugins.go | 4 +- pkg/api/plugins_test.go | 2 +- .../plugininstaller/service.go | 53 ++++++++++-- .../plugininstaller/service_test.go | 86 +++++++++++++++---- pkg/setting/setting.go | 3 +- pkg/setting/setting_plugins.go | 5 +- 7 files changed, 125 insertions(+), 30 deletions(-) diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index c8e1170454c..9bf8c35ef75 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -274,7 +274,7 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro PluginAdminExternalManageEnabled: hs.Cfg.PluginAdminEnabled && hs.Cfg.PluginAdminExternalManageEnabled, PluginCatalogHiddenPlugins: hs.Cfg.PluginCatalogHiddenPlugins, PluginCatalogManagedPlugins: hs.managedPluginsService.ManagedPlugins(c.Req.Context()), - PluginCatalogPreinstalledPlugins: hs.Cfg.InstallPlugins, + PluginCatalogPreinstalledPlugins: hs.Cfg.PreinstallPlugins, ExpressionsEnabled: hs.Cfg.ExpressionsEnabled, AwsAllowedAuthProviders: hs.Cfg.AWSAllowedAuthProviders, AwsAssumeRoleEnabled: hs.Cfg.AWSAssumeRoleEnabled, diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index 6b25d977ff1..0bbb49b7b47 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -458,7 +458,7 @@ 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.InstallPlugins { + 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) } @@ -502,7 +502,7 @@ func (hs *HTTPServer) UninstallPlugin(c *contextmodel.ReqContext) response.Respo return response.Error(http.StatusNotFound, "Plugin not installed", nil) } - for _, preinstalled := range hs.Cfg.InstallPlugins { + for _, preinstalled := range hs.Cfg.PreinstallPlugins { if preinstalled.ID == pluginID { return response.Error(http.StatusConflict, "Cannot uninstall a pre-installed plugin", nil) } diff --git a/pkg/api/plugins_test.go b/pkg/api/plugins_test.go index d1d44affb75..fd1ad0df80f 100644 --- a/pkg/api/plugins_test.go +++ b/pkg/api/plugins_test.go @@ -95,7 +95,7 @@ func Test_PluginsInstallAndUninstall(t *testing.T) { hs.Cfg.PluginAdminEnabled = tc.pluginAdminEnabled hs.Cfg.PluginAdminExternalManageEnabled = tc.pluginAdminExternalManageEnabled hs.Cfg.RBAC.SingleOrganization = tc.singleOrganization - hs.Cfg.InstallPlugins = []setting.InstallPlugin{{ID: "grafana-preinstalled-datasource", Version: "1.0.0"}} + hs.Cfg.PreinstallPlugins = []setting.InstallPlugin{{ID: "grafana-preinstalled-datasource", Version: "1.0.0"}} hs.orgService = &orgtest.FakeOrgService{ExpectedOrg: &org.Org{}} hs.accesscontrolService = &actest.FakeService{} diff --git a/pkg/services/pluginsintegration/plugininstaller/service.go b/pkg/services/pluginsintegration/plugininstaller/service.go index 76f993f2b28..a19d082fac7 100644 --- a/pkg/services/pluginsintegration/plugininstaller/service.go +++ b/pkg/services/pluginsintegration/plugininstaller/service.go @@ -3,8 +3,10 @@ package plugininstaller import ( "context" "errors" + "fmt" "runtime" + "cuelang.org/go/pkg/time" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -18,29 +20,57 @@ type Service struct { log log.Logger pluginInstaller plugins.Installer pluginStore pluginstore.Store + failOnErr bool } -func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, pluginStore pluginstore.Store, pluginInstaller plugins.Installer) *Service { +func ProvideService(cfg *setting.Cfg, features featuremgmt.FeatureToggles, pluginStore pluginstore.Store, pluginInstaller plugins.Installer) (*Service, error) { s := &Service{ features: features, log: log.New("plugin.backgroundinstaller"), cfg: cfg, pluginInstaller: pluginInstaller, pluginStore: pluginStore, + failOnErr: !cfg.PreinstallPluginsAsync, // Fail on error if preinstall is synchronous } - return s + if !cfg.PreinstallPluginsAsync { + // Block initialization process until plugins are installed + err := s.installPluginsWithTimeout() + if err != nil { + return nil, err + } + } + return s, nil } // IsDisabled disables background installation of plugins. func (s *Service) IsDisabled() bool { return !s.features.IsEnabled(context.Background(), featuremgmt.FlagBackgroundPluginInstaller) || - len(s.cfg.InstallPlugins) == 0 + len(s.cfg.PreinstallPlugins) == 0 || + !s.cfg.PreinstallPluginsAsync } -func (s *Service) Run(ctx context.Context) error { +func (s *Service) installPluginsWithTimeout() error { + // Installation process does not timeout by default nor reuses the context + // passed to the request so we need to handle the timeout here. + // We could make this timeout configurable in the future. + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + done := make(chan struct{ err error }) + go func() { + done <- struct{ err error }{err: s.installPlugins(ctx)} + }() + select { + case <-ctx.Done(): + return fmt.Errorf("failed to install plugins: %w", ctx.Err()) + case d := <-done: + return d.err + } +} + +func (s *Service) installPlugins(ctx context.Context) error { compatOpts := plugins.NewCompatOpts(s.cfg.BuildVersion, runtime.GOOS, runtime.GOARCH) - for _, installPlugin := range s.cfg.InstallPlugins { + for _, installPlugin := range s.cfg.PreinstallPlugins { // Check if the plugin is already installed p, exists := s.pluginStore.Plugin(ctx, installPlugin.ID) if exists { @@ -59,6 +89,10 @@ func (s *Service) Run(ctx context.Context) error { s.log.Debug("Plugin already installed", "pluginId", installPlugin.ID, "version", installPlugin.Version) continue } + if s.failOnErr { + // Halt execution in the synchronous scenario + return fmt.Errorf("failed to install plugin %s@%s: %w", installPlugin.ID, installPlugin.Version, err) + } s.log.Error("Failed to install plugin", "pluginId", installPlugin.ID, "version", installPlugin.Version, "error", err) continue } @@ -67,3 +101,12 @@ func (s *Service) Run(ctx context.Context) error { return nil } + +func (s *Service) Run(ctx context.Context) error { + err := s.installPlugins(ctx) + if err != nil { + // Unexpected error, asynchronous installation should not return errors + s.log.Error("Failed to install plugins", "error", err) + } + return nil +} diff --git a/pkg/services/pluginsintegration/plugininstaller/service_test.go b/pkg/services/pluginsintegration/plugininstaller/service_test.go index 19cc3386c6b..59feffe22de 100644 --- a/pkg/services/pluginsintegration/plugininstaller/service_test.go +++ b/pkg/services/pluginsintegration/plugininstaller/service_test.go @@ -16,14 +16,16 @@ import ( // Test if the service is disabled func TestService_IsDisabled(t *testing.T) { // Create a new service - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPluginsAsync: true, }, featuremgmt.WithFeatures(featuremgmt.FlagBackgroundPluginInstaller), pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), &fakes.FakePluginInstaller{}, ) + require.NoError(t, err) // Check if the service is disabled if s.IsDisabled() { @@ -34,9 +36,9 @@ func TestService_IsDisabled(t *testing.T) { func TestService_Run(t *testing.T) { t.Run("Installs a plugin", func(t *testing.T) { installed := false - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, }, featuremgmt.WithFeatures(), pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), @@ -47,17 +49,19 @@ func TestService_Run(t *testing.T) { }, }, ) + require.NoError(t, err) - err := s.Run(context.Background()) + err = s.Run(context.Background()) require.NoError(t, err) require.True(t, installed) }) t.Run("Install a plugin with version", func(t *testing.T) { installed := false - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin", Version: "1.0.0"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin", Version: "1.0.0"}}, + PreinstallPluginsAsync: true, }, featuremgmt.WithFeatures(), pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), @@ -70,8 +74,9 @@ func TestService_Run(t *testing.T) { }, }, ) + require.NoError(t, err) - err := s.Run(context.Background()) + err = s.Run(context.Background()) require.NoError(t, err) require.True(t, installed) }) @@ -84,9 +89,10 @@ func TestService_Run(t *testing.T) { }, }) require.NoError(t, err) - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPluginsAsync: true, }, featuremgmt.WithFeatures(), pluginstore.New(preg, &fakes.FakeLoader{}), @@ -97,6 +103,7 @@ func TestService_Run(t *testing.T) { }, }, ) + require.NoError(t, err) err = s.Run(context.Background()) require.NoError(t, err) @@ -114,9 +121,10 @@ func TestService_Run(t *testing.T) { }, }) require.NoError(t, err) - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin", Version: "2.0.0"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin", Version: "2.0.0"}}, + PreinstallPluginsAsync: true, }, featuremgmt.WithFeatures(), pluginstore.New(preg, &fakes.FakeLoader{}), @@ -127,6 +135,7 @@ func TestService_Run(t *testing.T) { }, }, ) + require.NoError(t, err) err = s.Run(context.Background()) require.NoError(t, err) @@ -135,9 +144,10 @@ func TestService_Run(t *testing.T) { t.Run("Install multiple plugins", func(t *testing.T) { installed := 0 - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin1"}, {ID: "myplugin2"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin1"}, {ID: "myplugin2"}}, + PreinstallPluginsAsync: true, }, featuremgmt.WithFeatures(), pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), @@ -148,17 +158,19 @@ func TestService_Run(t *testing.T) { }, }, ) + require.NoError(t, err) - err := s.Run(context.Background()) + err = s.Run(context.Background()) require.NoError(t, err) require.Equal(t, 2, installed) }) t.Run("Fails to install a plugin but install the rest", func(t *testing.T) { installed := 0 - s := ProvideService( + s, err := ProvideService( &setting.Cfg{ - InstallPlugins: []setting.InstallPlugin{{ID: "myplugin1"}, {ID: "myplugin2"}}, + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin1"}, {ID: "myplugin2"}}, + PreinstallPluginsAsync: true, }, featuremgmt.WithFeatures(), pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), @@ -172,8 +184,46 @@ func TestService_Run(t *testing.T) { }, }, ) - err := s.Run(context.Background()) + require.NoError(t, err) + err = s.Run(context.Background()) require.NoError(t, err) require.Equal(t, 1, installed) }) + + t.Run("Install a blocking plugin", func(t *testing.T) { + installed := false + _, err := ProvideService( + &setting.Cfg{ + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPluginsAsync: false, + }, + featuremgmt.WithFeatures(), + pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), + &fakes.FakePluginInstaller{ + AddFunc: func(ctx context.Context, pluginID string, version string, opts plugins.CompatOpts) error { + installed = true + return nil + }, + }, + ) + require.NoError(t, err) + require.True(t, installed) + }) + + t.Run("Fails to install a blocking plugin", func(t *testing.T) { + _, err := ProvideService( + &setting.Cfg{ + PreinstallPlugins: []setting.InstallPlugin{{ID: "myplugin"}}, + PreinstallPluginsAsync: false, + }, + featuremgmt.WithFeatures(), + pluginstore.New(registry.NewInMemory(), &fakes.FakeLoader{}), + &fakes.FakePluginInstaller{ + AddFunc: func(ctx context.Context, pluginID string, version string, opts plugins.CompatOpts) error { + return plugins.NotFoundError{} + }, + }, + ) + require.ErrorAs(t, err, &plugins.NotFoundError{}) + }) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 22f34b43f92..04612d220ac 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -197,7 +197,8 @@ type Cfg struct { HideAngularDeprecation []string PluginInstallToken string ForwardHostEnvVars []string - InstallPlugins []InstallPlugin + PreinstallPlugins []InstallPlugin + PreinstallPluginsAsync bool PluginsCDNURLTemplate string PluginLogBackendRequests bool diff --git a/pkg/setting/setting_plugins.go b/pkg/setting/setting_plugins.go index d6cf55172d5..54f315eef02 100644 --- a/pkg/setting/setting_plugins.go +++ b/pkg/setting/setting_plugins.go @@ -42,7 +42,7 @@ func (cfg *Cfg) readPluginSettings(iniFile *ini.File) error { disablePreinstall := pluginsSection.Key("disable_preinstall").MustBool(false) if !disablePreinstall { rawInstallPlugins := util.SplitString(pluginsSection.Key("preinstall").MustString("")) - cfg.InstallPlugins = make([]InstallPlugin, len(rawInstallPlugins)) + cfg.PreinstallPlugins = make([]InstallPlugin, len(rawInstallPlugins)) for i, plugin := range rawInstallPlugins { parts := strings.Split(plugin, "@") id := parts[0] @@ -50,8 +50,9 @@ func (cfg *Cfg) readPluginSettings(iniFile *ini.File) error { if len(parts) == 2 { v = parts[1] } - cfg.InstallPlugins[i] = InstallPlugin{id, v} + cfg.PreinstallPlugins[i] = InstallPlugin{id, v} } + cfg.PreinstallPluginsAsync = pluginsSection.Key("preinstall_async").MustBool(true) } cfg.PluginCatalogURL = pluginsSection.Key("plugin_catalog_url").MustString("https://grafana.com/grafana/plugins/")