From 333de57999b45dbc60e4d7d5fba9e1667ffc091c Mon Sep 17 00:00:00 2001
From: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Date: Mon, 31 Jan 2022 14:07:10 +0100
Subject: [PATCH] Plugins: Fix scanning plugins when permission for directories
 is lacking (#44587)

Fixes so that errors (directory not exists, no permission) when scanning plugins are logged as
errors rather than with debug level. In addition, before the scanning would halt in case of
referenced errors, but with these changes the scanning will continue. If any other error
than the referenced error happens the scanning for specific directory would halt and return
the error, e.g. stop Grafana from starting.

Fixes #43012
---
 pkg/plugins/manager/loader/finder/finder.go   | 22 ++++---
 .../manager/loader/finder/finder_test.go      | 64 ++++++++++++++++++-
 pkg/plugins/manager/loader/loader.go          |  2 +-
 3 files changed, 75 insertions(+), 13 deletions(-)

diff --git a/pkg/plugins/manager/loader/finder/finder.go b/pkg/plugins/manager/loader/finder/finder.go
index 5893b8d20e2..2838749adb2 100644
--- a/pkg/plugins/manager/loader/finder/finder.go
+++ b/pkg/plugins/manager/loader/finder/finder.go
@@ -11,6 +11,8 @@ import (
 	"github.com/grafana/grafana/pkg/util"
 )
 
+var walk = util.Walk
+
 type Finder struct {
 	log log.Logger
 }
@@ -51,9 +53,18 @@ func (f *Finder) getAbsPluginJSONPaths(path string) ([]string, error) {
 		return []string{}, err
 	}
 
-	if err := util.Walk(path, true, true,
+	if err := walk(path, true, true,
 		func(currentPath string, fi os.FileInfo, err error) error {
 			if err != nil {
+				if errors.Is(err, os.ErrNotExist) {
+					f.log.Error("Couldn't scan directory since it doesn't exist", "pluginDir", path, "err", err)
+					return nil
+				}
+				if errors.Is(err, os.ErrPermission) {
+					f.log.Error("Couldn't scan directory due to lack of permissions", "pluginDir", path, "err", err)
+					return nil
+				}
+
 				return fmt.Errorf("filepath.Walk reported an error for %q: %w", currentPath, err)
 			}
 
@@ -72,15 +83,6 @@ func (f *Finder) getAbsPluginJSONPaths(path string) ([]string, error) {
 			pluginJSONPaths = append(pluginJSONPaths, currentPath)
 			return nil
 		}); err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			f.log.Debug("Couldn't scan directory since it doesn't exist", "pluginDir", path, "err", err)
-			return []string{}, nil
-		}
-		if errors.Is(err, os.ErrPermission) {
-			f.log.Debug("Couldn't scan directory due to lack of permissions", "pluginDir", path, "err", err)
-			return []string{}, nil
-		}
-
 		return []string{}, err
 	}
 
diff --git a/pkg/plugins/manager/loader/finder/finder_test.go b/pkg/plugins/manager/loader/finder/finder_test.go
index 94782b05c3e..093b2fc1bc4 100644
--- a/pkg/plugins/manager/loader/finder/finder_test.go
+++ b/pkg/plugins/manager/loader/finder/finder_test.go
@@ -2,12 +2,16 @@ package finder
 
 import (
 	"errors"
+	"fmt"
+	"os"
 	"strings"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
-
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/util"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestFinder_Find(t *testing.T) {
@@ -65,3 +69,59 @@ func TestFinder_Find(t *testing.T) {
 		})
 	}
 }
+
+func TestFinder_getAbsPluginJSONPaths(t *testing.T) {
+	t.Run("When scanning a folder that doesn't exists shouldn't return an error", func(t *testing.T) {
+		origWalk := walk
+		walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error {
+			return walkFn(path, nil, os.ErrNotExist)
+		}
+		t.Cleanup(func() {
+			walk = origWalk
+		})
+
+		finder := &Finder{
+			log: log.New(),
+		}
+
+		paths, err := finder.getAbsPluginJSONPaths("test")
+		require.NoError(t, err)
+		require.Empty(t, paths)
+	})
+
+	t.Run("When scanning a folder that lacks permission shouldn't return an error", func(t *testing.T) {
+		origWalk := walk
+		walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error {
+			return walkFn(path, nil, os.ErrPermission)
+		}
+		t.Cleanup(func() {
+			walk = origWalk
+		})
+
+		finder := &Finder{
+			log: log.New(),
+		}
+
+		paths, err := finder.getAbsPluginJSONPaths("test")
+		require.NoError(t, err)
+		require.Empty(t, paths)
+	})
+
+	t.Run("When scanning a folder that returns a non-handled error should return that error", func(t *testing.T) {
+		origWalk := walk
+		walk = func(path string, followSymlinks, detectSymlinkInfiniteLoop bool, walkFn util.WalkFunc) error {
+			return walkFn(path, nil, fmt.Errorf("random error"))
+		}
+		t.Cleanup(func() {
+			walk = origWalk
+		})
+
+		finder := &Finder{
+			log: log.New(),
+		}
+
+		paths, err := finder.getAbsPluginJSONPaths("test")
+		require.Error(t, err)
+		require.Empty(t, paths)
+	})
+}
diff --git a/pkg/plugins/manager/loader/loader.go b/pkg/plugins/manager/loader/loader.go
index 41f470fa758..e438d9a5cbc 100644
--- a/pkg/plugins/manager/loader/loader.go
+++ b/pkg/plugins/manager/loader/loader.go
@@ -62,7 +62,7 @@ func New(cfg *plugins.Cfg, license models.Licensing, authorizer plugins.PluginLo
 func (l *Loader) Load(ctx context.Context, class plugins.Class, paths []string, ignore map[string]struct{}) ([]*plugins.Plugin, error) {
 	pluginJSONPaths, err := l.pluginFinder.Find(paths)
 	if err != nil {
-		l.log.Error("plugin finder encountered an error", "err", err)
+		return nil, err
 	}
 
 	return l.loadPlugins(ctx, class, pluginJSONPaths, ignore)