From 4084b53f917c0b2a3c0710401a3b96bdc5baf6b0 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Wed, 21 Oct 2020 12:39:41 +0200 Subject: [PATCH] plugins: Don't exit on duplicate plugin (#28390) * plugins: Don't exit on duplicate plugin Signed-off-by: Arve Knudsen * Add missing files Signed-off-by: Arve Knudsen * Fix test Signed-off-by: Arve Knudsen --- pkg/api/datasource/validation.go | 2 +- pkg/api/pluginproxy/ds_proxy_test.go | 2 +- pkg/models/dashboards.go | 2 +- pkg/plugins/models.go | 22 +++++++++++++++---- pkg/plugins/plugins.go | 6 +++++ pkg/plugins/plugins_test.go | 18 +++++++++++++++ .../nested/nested/plugin.json | 14 ++++++++++++ .../duplicate-plugins/nested/plugin.json | 14 ++++++++++++ 8 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 pkg/plugins/testdata/duplicate-plugins/nested/nested/plugin.json create mode 100644 pkg/plugins/testdata/duplicate-plugins/nested/plugin.json diff --git a/pkg/api/datasource/validation.go b/pkg/api/datasource/validation.go index 469069344f0..c2e95649bc8 100644 --- a/pkg/api/datasource/validation.go +++ b/pkg/api/datasource/validation.go @@ -21,7 +21,7 @@ type URLValidationError struct { // Error returns the error message. 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. diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index 6e138e49270..d2c77aa5e11 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -585,7 +585,7 @@ func TestNewDataSourceProxy_InvalidURL(t *testing.T) { plugin := plugins.DataSourcePlugin{} _, err := NewDataSourceProxy(&ds, &plugin, &ctx, "api/method", &cfg) 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) { diff --git a/pkg/models/dashboards.go b/pkg/models/dashboards.go index b133cf231df..30afd0b900d 100644 --- a/pkg/models/dashboards.go +++ b/pkg/models/dashboards.go @@ -139,7 +139,7 @@ type UpdatePluginDashboardError struct { } func (d UpdatePluginDashboardError) Error() string { - return "Dashboard belong to plugin" + return "Dashboard belongs to plugin" } const ( diff --git a/pkg/plugins/models.go b/pkg/plugins/models.go index ce8057bd0a8..3a0d8056ff7 100644 --- a/pkg/plugins/models.go +++ b/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) { diff --git a/pkg/plugins/plugins.go b/pkg/plugins/plugins.go index bd98a585a17..f6f6f846f3c 100644 --- a/pkg/plugins/plugins.go +++ b/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 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 } diff --git a/pkg/plugins/plugins_test.go b/pkg/plugins/plugins_test.go index 38a33cc9e39..79b3a569f81 100644 --- a/pkg/plugins/plugins_test.go +++ b/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) { diff --git a/pkg/plugins/testdata/duplicate-plugins/nested/nested/plugin.json b/pkg/plugins/testdata/duplicate-plugins/nested/nested/plugin.json new file mode 100644 index 00000000000..07aa333e698 --- /dev/null +++ b/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" + } +} diff --git a/pkg/plugins/testdata/duplicate-plugins/nested/plugin.json b/pkg/plugins/testdata/duplicate-plugins/nested/plugin.json new file mode 100644 index 00000000000..bd604df1754 --- /dev/null +++ b/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" + } +}