plugins: Don't exit on duplicate plugin (#28390) (#28430)

* plugins: Don't exit on duplicate plugin

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Add missing files

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>

* Fix test

Signed-off-by: Arve Knudsen <arve.knudsen@gmail.com>
(cherry picked from commit 4084b53f91)

Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>

pkg/plugins/models.go

@@ -35,11 +35,25 @@ const (
 )
 
 type PluginNotFoundError struct {
-	PluginId string
+	PluginID string
 }
 
 func (e PluginNotFoundError) Error() string {
-	return fmt.Sprintf("Plugin with id %s not found", e.PluginId)
+	return fmt.Sprintf("plugin with ID %q not found", e.PluginID)
+}
+
+type duplicatePluginError struct {
+	Plugin         *PluginBase
+	ExistingPlugin *PluginBase
+}
+
+func (e duplicatePluginError) Error() string {
+	return fmt.Sprintf("plugin with ID %q already loaded from %q", e.Plugin.Id, e.ExistingPlugin.PluginDir)
+}
+
+func (e duplicatePluginError) Is(err error) bool {
+	_, ok := err.(duplicatePluginError)
+	return ok
 }
 
 // PluginLoader can load a plugin.
@@ -77,8 +91,8 @@ type PluginBase struct {
 }
 
 func (pb *PluginBase) registerPlugin(base *PluginBase) error {
-	if _, exists := Plugins[pb.Id]; exists {
-		return fmt.Errorf("Plugin with ID %q already exists", pb.Id)
+	if p, exists := Plugins[pb.Id]; exists {
+		return duplicatePluginError{Plugin: pb, ExistingPlugin: p}
 	}
 
 	if !strings.HasPrefix(base.PluginDir, setting.StaticRootPath) {

pkg/plugins/plugins.go

@@ -271,6 +271,12 @@ func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error {
 
 		// Load the full plugin, and add it to manager
 		if err := loader.Load(jsonParser, plugin, scanner.backendPluginManager); err != nil {
+			if errors.Is(err, duplicatePluginError{}) {
+				pm.log.Warn("Plugin is duplicate", "error", err)
+				scanner.errors = append(scanner.errors, err)
+				continue
+			}
+
 			return err
 		}
 

pkg/plugins/plugins_test.go

@@ -2,6 +2,7 @@ package plugins
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"path/filepath"
 	"testing"
@@ -163,6 +164,23 @@ func TestPluginManager_Init(t *testing.T) {
 		require.Empty(t, pm.scanningErrors)
 		assert.Equal(t, []string{"gel"}, fm.registeredPlugins)
 	})
+
+	t.Run("With nested plugin duplicating parent", func(t *testing.T) {
+		origPluginsPath := setting.PluginsPath
+		t.Cleanup(func() {
+			setting.PluginsPath = origPluginsPath
+		})
+		setting.PluginsPath = "testdata/duplicate-plugins"
+
+		pm := &PluginManager{
+			Cfg: &setting.Cfg{},
+		}
+		err := pm.Init()
+		require.NoError(t, err)
+
+		assert.Len(t, pm.scanningErrors, 1)
+		assert.True(t, errors.Is(pm.scanningErrors[0], duplicatePluginError{}))
+	})
 }
 
 func TestPluginManager_IsBackendOnlyPlugin(t *testing.T) {

pkg/plugins/testdata/duplicate-plugins/nested/nested/plugin.json

@@ -0,0 +1,14 @@
+{
+  "type": "datasource",
+  "name": "Child",
+  "id": "test-app",
+  "info": {
+    "description": "Child plugin",
+    "author": {
+      "name": "Grafana Labs",
+      "url": "http://grafana.com"
+    },
+    "version": "1.0.0",
+    "updated": "2020-10-20"
+  }
+}

pkg/plugins/testdata/duplicate-plugins/nested/plugin.json

@@ -0,0 +1,14 @@
+{
+  "type": "datasource",
+  "name": "Parent",
+  "id": "test-app",
+  "info": {
+    "description": "Parent plugin",
+    "author": {
+      "name": "Grafana Labs",
+      "url": "http://grafana.com"
+    },
+    "version": "1.0.0",
+    "updated": "2020-10-20"
+  }
+}