From ccd26fe69d36bd7ce576447ccc78687c62f2a0c8 Mon Sep 17 00:00:00 2001
From: Krzesimir Nowak <krzesimir@kinvolk.io>
Date: Tue, 12 Jul 2016 13:00:22 +0200
Subject: [PATCH] Ban the possibility of changing plugin's ID

Changing plugin's ID only complicates control handling in plugins so
let's ban it.
---
 probe/plugins/registry.go               |  3 +++
 probe/plugins/registry_internal_test.go | 21 +++++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/probe/plugins/registry.go b/probe/plugins/registry.go
index 80612a08d..7dfbf2d1b 100644
--- a/probe/plugins/registry.go
+++ b/probe/plugins/registry.go
@@ -257,6 +257,9 @@ func (p *Plugin) Report() (result report.Report, err error) {
 
 	key := result.Plugins.Keys()[0]
 	spec, _ := result.Plugins.Lookup(key)
+	if spec.ID != p.PluginSpec.ID {
+		return result, fmt.Errorf("plugin must not change its id (is %q, should be %q)", spec.ID, p.PluginSpec.ID)
+	}
 	p.PluginSpec = spec
 
 	foundReporter := false
diff --git a/probe/plugins/registry_internal_test.go b/probe/plugins/registry_internal_test.go
index 6f60cc82b..64dbeed1e 100644
--- a/probe/plugins/registry_internal_test.go
+++ b/probe/plugins/registry_internal_test.go
@@ -316,10 +316,17 @@ func TestRegistryUpdatesPluginsWhenTheyChange(t *testing.T) {
 	checkLoadedPluginIDs(t, r.ForEach, []string{"testPlugin"})
 
 	// Update the plugin. Just change what the handler will respond with.
-	resp = `{"Plugins":[{"id":"updatedPlugin","label":"updatedPlugin","interfaces":["reporter"]}]}`
+	resp = `{"Plugins":[{"id":"testPlugin","label":"updatedPlugin","interfaces":["reporter"]}]}`
 
 	r.Report()
-	checkLoadedPluginIDs(t, r.ForEach, []string{"updatedPlugin"})
+	checkLoadedPlugins(t, r.ForEach, []xfer.PluginSpec{
+		{
+			ID:         "testPlugin",
+			Label:      "updatedPlugin",
+			Interfaces: []string{"reporter"},
+			Status:     "ok",
+		},
+	})
 }
 
 func TestRegistryReturnsPluginsByInterface(t *testing.T) {
@@ -413,6 +420,11 @@ func TestRegistryRejectsErroneousPluginResponses(t *testing.T) {
 			Name:    "nonJSONResponseBody",
 			Handler: stringHandler(http.StatusOK, `notJSON`),
 		}.file(),
+		mockPlugin{
+			t:       t,
+			Name:    "changedId",
+			Handler: stringHandler(http.StatusOK, `{"Plugins":[{"id":"differentId","label":"changedId","interfaces":["reporter"]}]}`),
+		}.file(),
 	)
 	defer restore(t)
 
@@ -425,6 +437,11 @@ func TestRegistryRejectsErroneousPluginResponses(t *testing.T) {
 
 	r.Report()
 	checkLoadedPlugins(t, r.ForEach, []xfer.PluginSpec{
+		{
+			ID:     "changedId",
+			Label:  "changedId",
+			Status: `error: plugin must not change its id (is "differentId", should be "changedId")`,
+		},
 		{
 			ID:         "noInterface",
 			Label:      "noInterface",