From 343d6f8a0ccce3ff01ac0a2a0f92b06094841664 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Tue, 9 Jul 2024 15:46:30 +0100 Subject: [PATCH] Plugins: Support > 1 levels of plugin dependencies (#90174) * do it * prevent loops * change to sync.Map --- .../grafana-cli/commands/install_command.go | 60 +++++++--- .../commands/upgrade_all_command.go | 2 +- .../grafana-cli/commands/upgrade_command.go | 2 +- pkg/plugins/manager/installer.go | 80 +++++++------ pkg/plugins/manager/installer_test.go | 109 +++++++++++++++++- 5 files changed, 195 insertions(+), 58 deletions(-) diff --git a/pkg/cmd/grafana-cli/commands/install_command.go b/pkg/cmd/grafana-cli/commands/install_command.go index c15bd99fd30..ed82430edbe 100644 --- a/pkg/cmd/grafana-cli/commands/install_command.go +++ b/pkg/cmd/grafana-cli/commands/install_command.go @@ -75,27 +75,57 @@ func installCommand(c utils.CommandLine) error { pluginID := c.Args().First() version := c.Args().Get(1) - err := installPlugin(context.Background(), pluginID, version, c) + err := installPlugin(context.Background(), pluginID, version, newInstallPluginOpts(c)) if err == nil { logRestartNotice() } return err } +type pluginInstallOpts struct { + insecure bool + repoURL string + pluginURL string + pluginDir string +} + +func newInstallPluginOpts(c utils.CommandLine) pluginInstallOpts { + return pluginInstallOpts{ + insecure: c.Bool("insecure"), + repoURL: c.PluginRepoURL(), + pluginURL: c.PluginURL(), + pluginDir: c.PluginDirectory(), + } +} + // installPlugin downloads the plugin code as a zip file from the Grafana.com API // and then extracts the zip into the plugin's directory. -func installPlugin(ctx context.Context, pluginID, version string, c utils.CommandLine) error { +func installPlugin(ctx context.Context, pluginID, version string, o pluginInstallOpts) error { + return doInstallPlugin(ctx, pluginID, version, o, map[string]bool{}) +} + +// doInstallPlugin is a recursive function that installs a plugin and its dependencies. +// installing is a map that keeps track of which plugins are currently being installed to avoid infinite loops. +func doInstallPlugin(ctx context.Context, pluginID, version string, o pluginInstallOpts, installing map[string]bool) error { + if installing[pluginID] { + return nil + } + installing[pluginID] = true + defer func() { + installing[pluginID] = false + }() + // If a version is specified, check if it is already installed if version != "" { - if services.PluginVersionInstalled(pluginID, version, c.PluginDirectory()) { + if services.PluginVersionInstalled(pluginID, version, o.pluginDir) { services.Logger.Successf("Plugin %s v%s already installed.", pluginID, version) return nil } } repository := repo.NewManager(repo.ManagerCfg{ - SkipTLSVerify: c.Bool("insecure"), - BaseURL: c.PluginRepoURL(), + SkipTLSVerify: o.insecure, + BaseURL: o.repoURL, Logger: services.Logger, }) @@ -103,7 +133,7 @@ func installPlugin(ctx context.Context, pluginID, version string, c utils.Comman var archive *repo.PluginArchive var err error - pluginZipURL := c.PluginURL() + pluginZipURL := o.pluginURL if pluginZipURL != "" { if archive, err = repository.GetPluginArchiveByURL(ctx, pluginZipURL, compatOpts); err != nil { return err @@ -114,23 +144,19 @@ func installPlugin(ctx context.Context, pluginID, version string, c utils.Comman } } - pluginFs := storage.FileSystem(services.Logger, c.PluginDirectory()) + pluginFs := storage.FileSystem(services.Logger, o.pluginDir) extractedArchive, err := pluginFs.Extract(ctx, pluginID, storage.SimpleDirNameGeneratorFunc, archive.File) if err != nil { return err } for _, dep := range extractedArchive.Dependencies { - services.Logger.Infof("Fetching %s dependency...", dep.ID) - d, err := repository.GetPluginArchive(ctx, dep.ID, dep.Version, compatOpts) - if err != nil { - return fmt.Errorf("%v: %w", fmt.Sprintf("failed to download plugin %s from repository", dep.ID), err) - } - - _, err = pluginFs.Extract(ctx, dep.ID, storage.SimpleDirNameGeneratorFunc, d.File) - if err != nil { - return err - } + services.Logger.Infof("Fetching %s dependency %s...", pluginID, dep.ID) + return doInstallPlugin(ctx, dep.ID, dep.Version, pluginInstallOpts{ + insecure: o.insecure, + repoURL: o.repoURL, + pluginDir: o.pluginDir, + }, installing) } return nil } diff --git a/pkg/cmd/grafana-cli/commands/upgrade_all_command.go b/pkg/cmd/grafana-cli/commands/upgrade_all_command.go index 74b3e65f038..87381dcd650 100644 --- a/pkg/cmd/grafana-cli/commands/upgrade_all_command.go +++ b/pkg/cmd/grafana-cli/commands/upgrade_all_command.go @@ -63,7 +63,7 @@ func upgradeAllCommand(c utils.CommandLine) error { return err } - err = installPlugin(ctx, p.JSONData.ID, "", c) + err = installPlugin(ctx, p.JSONData.ID, "", newInstallPluginOpts(c)) if err != nil { return err } diff --git a/pkg/cmd/grafana-cli/commands/upgrade_command.go b/pkg/cmd/grafana-cli/commands/upgrade_command.go index a0a3ba4500e..0832c5818b5 100644 --- a/pkg/cmd/grafana-cli/commands/upgrade_command.go +++ b/pkg/cmd/grafana-cli/commands/upgrade_command.go @@ -35,7 +35,7 @@ func upgradeCommand(c utils.CommandLine) error { return fmt.Errorf("failed to remove plugin '%s': %w", pluginID, err) } - err = installPlugin(ctx, pluginID, "", c) + err = installPlugin(ctx, pluginID, "", newInstallPluginOpts(c)) if err == nil { logRestartNotice() } diff --git a/pkg/plugins/manager/installer.go b/pkg/plugins/manager/installer.go index 02aafee6733..b0be555af16 100644 --- a/pkg/plugins/manager/installer.go +++ b/pkg/plugins/manager/installer.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "sync" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/plugins/auth" @@ -24,6 +25,7 @@ type PluginInstaller struct { pluginStorageDirFunc storage.DirNameGeneratorFunc pluginRegistry registry.Service pluginLoader loader.Service + installing sync.Map log log.Logger serviceRegistry auth.ExternalServiceRegistry } @@ -43,6 +45,7 @@ func New(pluginRegistry registry.Service, pluginLoader loader.Service, pluginRep pluginRepo: pluginRepo, pluginStorage: pluginStorage, pluginStorageDirFunc: pluginStorageDirFunc, + installing: sync.Map{}, log: log.New("plugin.installer"), serviceRegistry: serviceRegistry, } @@ -54,14 +57,46 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt return err } + if ok, _ := m.installing.Load(pluginID); ok != nil { + return nil + } + m.installing.Store(pluginID, true) + defer func() { + m.installing.Delete(pluginID) + }() + + archive, err := m.install(ctx, pluginID, version, compatOpts) + if err != nil { + return err + } + + for _, dep := range archive.Dependencies { + m.log.Info(fmt.Sprintf("Fetching %s dependency %s...", pluginID, dep.ID)) + + err = m.Add(ctx, dep.ID, dep.Version, opts) + if err != nil { + return fmt.Errorf("%v: %w", fmt.Sprintf("failed to download plugin %s from repository", dep.ID), err) + } + } + + _, err = m.pluginLoader.Load(ctx, sources.NewLocalSource(plugins.ClassExternal, []string{archive.Path})) + if err != nil { + m.log.Error("Could not load plugins", "path", archive.Path, "error", err) + return err + } + + return nil +} + +func (m *PluginInstaller) install(ctx context.Context, pluginID, version string, compatOpts repo.CompatOpts) (*storage.ExtractedPluginArchive, error) { var pluginArchive *repo.PluginArchive if plugin, exists := m.plugin(ctx, pluginID, version); exists { if plugin.IsCorePlugin() || plugin.IsBundledPlugin() { - return plugins.ErrInstallCorePlugin + return nil, plugins.ErrInstallCorePlugin } if plugin.Info.Version == version { - return plugins.DuplicateError{ + return nil, plugins.DuplicateError{ PluginID: plugin.ID, } } @@ -69,74 +104,51 @@ func (m *PluginInstaller) Add(ctx context.Context, pluginID, version string, opt // get plugin update information to confirm if target update is possible pluginArchiveInfo, err := m.pluginRepo.GetPluginArchiveInfo(ctx, pluginID, version, compatOpts) if err != nil { - return err + return nil, err } // if existing plugin version is the same as the target update version if pluginArchiveInfo.Version == plugin.Info.Version { - return plugins.DuplicateError{ + return nil, plugins.DuplicateError{ PluginID: plugin.ID, } } if pluginArchiveInfo.URL == "" && pluginArchiveInfo.Version == "" { - return fmt.Errorf("could not determine update options for %s", pluginID) + return nil, fmt.Errorf("could not determine update options for %s", pluginID) } // remove existing installation of plugin err = m.Remove(ctx, plugin.ID, plugin.Info.Version) if err != nil { - return err + return nil, err } if pluginArchiveInfo.URL != "" { pluginArchive, err = m.pluginRepo.GetPluginArchiveByURL(ctx, pluginArchiveInfo.URL, compatOpts) if err != nil { - return err + return nil, err } } else { pluginArchive, err = m.pluginRepo.GetPluginArchive(ctx, pluginID, pluginArchiveInfo.Version, compatOpts) if err != nil { - return err + return nil, err } } } else { var err error pluginArchive, err = m.pluginRepo.GetPluginArchive(ctx, pluginID, version, compatOpts) if err != nil { - return err + return nil, err } } extractedArchive, err := m.pluginStorage.Extract(ctx, pluginID, m.pluginStorageDirFunc, pluginArchive.File) if err != nil { - return err + return nil, err } - // download dependency plugins - pathsToScan := []string{extractedArchive.Path} - for _, dep := range extractedArchive.Dependencies { - m.log.Info(fmt.Sprintf("Fetching %s dependencies...", dep.ID)) - d, err := m.pluginRepo.GetPluginArchive(ctx, dep.ID, dep.Version, compatOpts) - if err != nil { - return fmt.Errorf("%v: %w", fmt.Sprintf("failed to download plugin %s from repository", dep.ID), err) - } - - depArchive, err := m.pluginStorage.Extract(ctx, dep.ID, m.pluginStorageDirFunc, d.File) - if err != nil { - return err - } - - pathsToScan = append(pathsToScan, depArchive.Path) - } - - _, err = m.pluginLoader.Load(ctx, sources.NewLocalSource(plugins.ClassExternal, pathsToScan)) - if err != nil { - m.log.Error("Could not load plugins", "paths", pathsToScan, "error", err) - return err - } - - return nil + return extractedArchive, nil } func (m *PluginInstaller) Remove(ctx context.Context, pluginID, version string) error { diff --git a/pkg/plugins/manager/installer_test.go b/pkg/plugins/manager/installer_test.go index 7bfadece3af..f90c3ba11c3 100644 --- a/pkg/plugins/manager/installer_test.go +++ b/pkg/plugins/manager/installer_test.go @@ -182,6 +182,103 @@ func TestPluginManager_Add_Remove(t *testing.T) { }) } }) + + t.Run("Can install multiple dependency levels", func(t *testing.T) { + const ( + p1, p1Zip = "foo-panel", "foo-panel.zip" + p2, p2Zip = "foo-datasource", "foo-datasource.zip" + p3, p3Zip = "foo-app", "foo-app.zip" + ) + + var loadedPaths []string + loader := &fakes.FakeLoader{ + LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { + loadedPaths = append(loadedPaths, src.PluginURIs(ctx)...) + return []*plugins.Plugin{}, nil + }, + } + + pluginRepo := &fakes.FakePluginRepo{ + GetPluginArchiveFunc: func(_ context.Context, id, version string, _ repo.CompatOpts) (*repo.PluginArchive, error) { + return &repo.PluginArchive{File: &zip.ReadCloser{Reader: zip.Reader{File: []*zip.File{{ + FileHeader: zip.FileHeader{Name: fmt.Sprintf("%s.zip", id)}, + }}}}}, nil + }, + } + + fs := &fakes.FakePluginStorage{ + ExtractFunc: func(_ context.Context, id string, _ storage.DirNameGeneratorFunc, z *zip.ReadCloser) (*storage.ExtractedPluginArchive, error) { + switch id { + case p1: + return &storage.ExtractedPluginArchive{Path: p1Zip}, nil + case p2: + return &storage.ExtractedPluginArchive{ + Dependencies: []*storage.Dependency{{ID: p1}}, + Path: p2Zip, + }, nil + case p3: + return &storage.ExtractedPluginArchive{ + Dependencies: []*storage.Dependency{{ID: p2}}, + Path: p3Zip, + }, nil + default: + return nil, fmt.Errorf("unknown plugin %s", id) + } + }, + } + + inst := New(fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + err := inst.Add(context.Background(), p3, "", testCompatOpts()) + require.NoError(t, err) + require.Equal(t, []string{p1Zip, p2Zip, p3Zip}, loadedPaths) + }) + + t.Run("Livelock prevented when two plugins depend on each other", func(t *testing.T) { + const ( + p1, p1Zip = "foo-panel", "foo-panel.zip" + p2, p2Zip = "foo-datasource", "foo-datasource.zip" + ) + + var loadedPaths []string + loader := &fakes.FakeLoader{ + LoadFunc: func(ctx context.Context, src plugins.PluginSource) ([]*plugins.Plugin, error) { + loadedPaths = append(loadedPaths, src.PluginURIs(ctx)...) + return []*plugins.Plugin{}, nil + }, + } + + pluginRepo := &fakes.FakePluginRepo{ + GetPluginArchiveFunc: func(_ context.Context, id, version string, _ repo.CompatOpts) (*repo.PluginArchive, error) { + return &repo.PluginArchive{File: &zip.ReadCloser{Reader: zip.Reader{File: []*zip.File{{ + FileHeader: zip.FileHeader{Name: fmt.Sprintf("%s.zip", id)}, + }}}}}, nil + }, + } + + fs := &fakes.FakePluginStorage{ + ExtractFunc: func(_ context.Context, id string, _ storage.DirNameGeneratorFunc, z *zip.ReadCloser) (*storage.ExtractedPluginArchive, error) { + switch id { + case p1: + return &storage.ExtractedPluginArchive{ + Dependencies: []*storage.Dependency{{ID: p2}}, + Path: p1Zip, + }, nil + case p2: + return &storage.ExtractedPluginArchive{ + Dependencies: []*storage.Dependency{{ID: p1}}, + Path: p2Zip, + }, nil + default: + return nil, fmt.Errorf("unknown plugin %s", id) + } + }, + } + + inst := New(fakes.NewFakePluginRegistry(), loader, pluginRepo, fs, storage.SimpleDirNameGeneratorFunc, &fakes.FakeAuthService{}) + err := inst.Add(context.Background(), p1, "", testCompatOpts()) + require.NoError(t, err) + require.Equal(t, []string{p2Zip, p1Zip}, loadedPaths) + }) } func createPlugin(t *testing.T, pluginID string, class plugins.Class, managed, backend bool, cbs ...func(*plugins.Plugin)) *plugins.Plugin { @@ -196,11 +293,13 @@ func createPlugin(t *testing.T, pluginID string, class plugins.Class, managed, b }, } p.SetLogger(log.NewTestLogger()) - p.RegisterClient(&fakes.FakePluginClient{ - ID: pluginID, - Managed: managed, - Log: p.Logger(), - }) + if p.Backend { + p.RegisterClient(&fakes.FakePluginClient{ + ID: pluginID, + Managed: managed, + Log: p.Logger(), + }) + } for _, cb := range cbs { cb(p)