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

* 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>

pkg/api/datasource/validation.go

@@ -21,7 +21,7 @@ type URLValidationError struct {
 
 
 // Error returns the error message.
 // Error returns the error message.
 func (e URLValidationError) Error() string {
 func (e URLValidationError) Error() string {
-	return fmt.Sprintf("Validation of data source URL %q failed: %s", e.URL, e.Err.Error())
+	return fmt.Sprintf("validation of data source URL %q failed: %s", e.URL, e.Err.Error())
 }
 }
 
 
 // Unwrap returns the wrapped error.
 // Unwrap returns the wrapped error.

pkg/api/pluginproxy/ds_proxy_test.go

@@ -585,7 +585,7 @@ func TestNewDataSourceProxy_InvalidURL(t *testing.T) {
 	plugin := plugins.DataSourcePlugin{}
 	plugin := plugins.DataSourcePlugin{}
 	_, err := NewDataSourceProxy(&ds, &plugin, &ctx, "api/method", &cfg)
 	_, err := NewDataSourceProxy(&ds, &plugin, &ctx, "api/method", &cfg)
 	require.Error(t, err)
 	require.Error(t, err)
-	assert.True(t, strings.HasPrefix(err.Error(), `Validation of data source URL "://host/root" failed`))
+	assert.True(t, strings.HasPrefix(err.Error(), `validation of data source URL "://host/root" failed`))
 }
 }
 
 
 func TestNewDataSourceProxy_ProtocolLessURL(t *testing.T) {
 func TestNewDataSourceProxy_ProtocolLessURL(t *testing.T) {

pkg/models/dashboards.go

@@ -139,7 +139,7 @@ type UpdatePluginDashboardError struct {
 }
 }
 
 
 func (d UpdatePluginDashboardError) Error() string {
 func (d UpdatePluginDashboardError) Error() string {
-	return "Dashboard belong to plugin"
+	return "Dashboard belongs to plugin"
 }
 }
 
 
 const (
 const (

pkg/plugins/models.go

@@ -35,11 +35,25 @@ const (
 )
 )
 
 
 type PluginNotFoundError struct {
 type PluginNotFoundError struct {
-	PluginId string
+	PluginID string
 }
 }
 
 
 func (e PluginNotFoundError) Error() 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.
 // PluginLoader can load a plugin.
@@ -77,8 +91,8 @@ type PluginBase struct {
 }
 }
 
 
 func (pb *PluginBase) registerPlugin(base *PluginBase) error {
 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) {
 	if !strings.HasPrefix(base.PluginDir, setting.StaticRootPath) {

pkg/plugins/plugins.go

@@ -270,6 +270,12 @@ func (pm *PluginManager) scan(pluginDir string, requireSigned bool) error {
 
 
 		// Load the full plugin, and add it to manager
 		// Load the full plugin, and add it to manager
 		if err := loader.Load(jsonParser, plugin, scanner.backendPluginManager); err != nil {
 		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
 			return err
 		}
 		}
 
 

pkg/plugins/plugins_test.go

@@ -2,6 +2,7 @@ package plugins
 
 
 import (
 import (
 	"context"
 	"context"
+	"errors"
 	"fmt"
 	"fmt"
 	"path/filepath"
 	"path/filepath"
 	"testing"
 	"testing"
@@ -163,6 +164,23 @@ func TestPluginManager_Init(t *testing.T) {
 		require.Empty(t, pm.scanningErrors)
 		require.Empty(t, pm.scanningErrors)
 		assert.Equal(t, []string{"gel"}, fm.registeredPlugins)
 		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) {
 func TestPluginManager_IsBackendOnlyPlugin(t *testing.T) {

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

+{
+  "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

+{
+  "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"
+  }
+}