Plugins: Support > 1 levels of plugin dependencies (#90174)

* do it

* prevent loops

* change to sync.Map

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"),
+		SkipTLSVerify: o.insecure,
-		BaseURL:       c.PluginRepoURL(),
+		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)
+		services.Logger.Infof("Fetching %s dependency %s...", pluginID, dep.ID)
-		d, err := repository.GetPluginArchive(ctx, dep.ID, dep.Version, compatOpts)
+		return doInstallPlugin(ctx, dep.ID, dep.Version, pluginInstallOpts{
-		if err != nil {
+			insecure:  o.insecure,
-			return fmt.Errorf("%v: %w", fmt.Sprintf("failed to download plugin %s from repository", dep.ID), err)
+			repoURL:   o.repoURL,
-		}
+			pluginDir: o.pluginDir,
-
+		}, installing)
-		_, err = pluginFs.Extract(ctx, dep.ID, storage.SimpleDirNameGeneratorFunc, d.File)
-		if err != nil {
-			return err
-		}
 	}
 	return nil
 }

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
 		}

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()
 		}

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
+	return extractedArchive, nil
-	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
 }
 
 func (m *PluginInstaller) Remove(ctx context.Context, pluginID, version string) error {

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())
+	if p.Backend {
 		p.RegisterClient(&fakes.FakePluginClient{
 			ID:      pluginID,
 			Managed: managed,
 			Log:     p.Logger(),
 		})
+	}
 
 	for _, cb := range cbs {
 		cb(p)