From fd150a4c9798bcfcdc3e44da2c88332e059ec47a Mon Sep 17 00:00:00 2001
From: Gabe Rosenhouse <rosenhouse@gmail.com>
Date: Tue, 6 Sep 2016 20:19:26 -0400
Subject: [PATCH] skel: Plugins require a cniVersion in the NetConf

---
 README.md                              |  2 ++
 libcni/api_test.go                     |  2 +-
 pkg/invoke/exec.go                     |  4 +++-
 pkg/invoke/exec_test.go                |  7 ++++---
 pkg/invoke/raw_exec_test.go            |  2 +-
 pkg/skel/skel.go                       | 17 +++++++++++++++++
 pkg/skel/skel_test.go                  | 26 +++++++++++++++++++++-----
 pkg/version/plugin.go                  |  1 +
 plugins/main/bridge/bridge_test.go     | 19 +++++++++++--------
 plugins/main/ipvlan/ipvlan_test.go     |  7 +++++--
 plugins/main/loopback/loopback_test.go |  2 +-
 plugins/main/macvlan/macvlan_test.go   |  7 +++++--
 plugins/main/ptp/ptp_test.go           |  2 ++
 plugins/test/noop/noop_test.go         |  4 ++--
 14 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/README.md b/README.md
index 56f06094..c37a3065 100644
--- a/README.md
+++ b/README.md
@@ -69,6 +69,7 @@ Start out by creating a netconf file to describe a network:
 $ mkdir -p /etc/cni/net.d
 $ cat >/etc/cni/net.d/10-mynet.conf <<EOF
 {
+	"cniVersion": "0.2.0",
 	"name": "mynet",
 	"type": "bridge",
 	"bridge": "cni0",
@@ -85,6 +86,7 @@ $ cat >/etc/cni/net.d/10-mynet.conf <<EOF
 EOF
 $ cat >/etc/cni/net.d/99-loopback.conf <<EOF
 {
+	"cniVersion": "0.2.0",
 	"type": "loopback"
 }
 EOF
diff --git a/libcni/api_test.go b/libcni/api_test.go
index 6ac88e89..9aeb52a4 100644
--- a/libcni/api_test.go
+++ b/libcni/api_test.go
@@ -53,7 +53,7 @@ var _ = Describe("Invoking the plugin", func() {
 		Expect(debug.WriteDebug(debugFilePath)).To(Succeed())
 
 		cniBinPath = filepath.Dir(pathToPlugin)
-		pluginConfig = `{ "type": "noop", "some-key": "some-value" }`
+		pluginConfig = `{ "type": "noop", "some-key": "some-value", "cniVersion": "0.2.0" }`
 		cniConfig = libcni.CNIConfig{Path: []string{cniBinPath}}
 		netConfig = &libcni.NetworkConfig{
 			Network: &types.NetConf{
diff --git a/pkg/invoke/exec.go b/pkg/invoke/exec.go
index 7eb06156..167d38f5 100644
--- a/pkg/invoke/exec.go
+++ b/pkg/invoke/exec.go
@@ -16,6 +16,7 @@ package invoke
 
 import (
 	"encoding/json"
+	"fmt"
 	"os"
 
 	"github.com/containernetworking/cni/pkg/types"
@@ -77,7 +78,8 @@ func (e *PluginExec) GetVersionInfo(pluginPath string) (version.PluginInfo, erro
 		IfName: "dummy",
 		Path:   "dummy",
 	}
-	stdoutBytes, err := e.RawExec.ExecPlugin(pluginPath, nil, args.AsEnv())
+	stdin := []byte(fmt.Sprintf(`{"cniVersion":%q}`, version.Current()))
+	stdoutBytes, err := e.RawExec.ExecPlugin(pluginPath, stdin, args.AsEnv())
 	if err != nil {
 		if err.Error() == "unknown CNI_COMMAND: VERSION" {
 			return version.PluginSupports("0.1.0"), nil
diff --git a/pkg/invoke/exec_test.go b/pkg/invoke/exec_test.go
index 94007d7c..2b9c9bf9 100644
--- a/pkg/invoke/exec_test.go
+++ b/pkg/invoke/exec_test.go
@@ -15,6 +15,7 @@
 package invoke_test
 
 import (
+	"encoding/json"
 	"errors"
 
 	"github.com/containernetworking/cni/pkg/invoke"
@@ -48,7 +49,7 @@ var _ = Describe("Executing a plugin, unit tests", func() {
 			VersionDecoder: versionDecoder,
 		}
 		pluginPath = "/some/plugin/path"
-		netconf = []byte(`{ "some": "stdin" }`)
+		netconf = []byte(`{ "some": "stdin", "cniVersion": "0.2.0" }`)
 		cniargs = &fakes.CNIArgs{}
 		cniargs.AsEnvCall.Returns.Env = []string{"SOME=ENV"}
 	})
@@ -105,8 +106,9 @@ var _ = Describe("Executing a plugin, unit tests", func() {
 		It("execs the plugin with the command VERSION", func() {
 			pluginExec.GetVersionInfo(pluginPath)
 			Expect(rawExec.ExecPluginCall.Received.PluginPath).To(Equal(pluginPath))
-			Expect(rawExec.ExecPluginCall.Received.StdinData).To(BeNil())
 			Expect(rawExec.ExecPluginCall.Received.Environ).To(ContainElement("CNI_COMMAND=VERSION"))
+			expectedStdin, _ := json.Marshal(map[string]string{"cniVersion": version.Current()})
+			Expect(rawExec.ExecPluginCall.Received.StdinData).To(MatchJSON(expectedStdin))
 		})
 
 		It("decodes and returns the version info", func() {
@@ -146,6 +148,5 @@ var _ = Describe("Executing a plugin, unit tests", func() {
 				Expect(env).To(ContainElement("CNI_PATH=dummy"))
 			})
 		})
-
 	})
 })
diff --git a/pkg/invoke/raw_exec_test.go b/pkg/invoke/raw_exec_test.go
index 7df60a11..b0ca9607 100644
--- a/pkg/invoke/raw_exec_test.go
+++ b/pkg/invoke/raw_exec_test.go
@@ -58,7 +58,7 @@ var _ = Describe("RawExec", func() {
 			"CNI_PATH=/some/bin/path",
 			"CNI_IFNAME=some-eth0",
 		}
-		stdin = []byte(`{"some":"stdin-json"}`)
+		stdin = []byte(`{"some":"stdin-json", "cniVersion": "0.2.0"}`)
 		execer = &invoke.RawExec{}
 	})
 
diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go
index de64d7dd..180ce24e 100644
--- a/pkg/skel/skel.go
+++ b/pkg/skel/skel.go
@@ -17,6 +17,7 @@
 package skel
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -143,12 +144,28 @@ func createTypedError(f string, args ...interface{}) *types.Error {
 	}
 }
 
+func (t *dispatcher) validateVersion(stdinData []byte) error {
+	var netconf types.NetConf
+	if err := json.Unmarshal(stdinData, &netconf); err != nil {
+		return err
+	}
+	if netconf.CNIVersion == "" {
+		return fmt.Errorf("missing required config cniVersion")
+	}
+
+	return nil
+}
+
 func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionInfo version.PluginInfo) *types.Error {
 	cmd, cmdArgs, err := t.getCmdArgsFromEnv()
 	if err != nil {
 		return createTypedError(err.Error())
 	}
 
+	if err = t.validateVersion(cmdArgs.StdinData); err != nil {
+		return createTypedError(err.Error())
+	}
+
 	switch cmd {
 	case "ADD":
 		err = cmdAdd(cmdArgs)
diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go
index 1cc533bd..7ae25d35 100644
--- a/pkg/skel/skel_test.go
+++ b/pkg/skel/skel_test.go
@@ -17,7 +17,6 @@ package skel
 import (
 	"bytes"
 	"errors"
-	"io"
 	"strings"
 
 	"github.com/containernetworking/cni/pkg/types"
@@ -48,7 +47,7 @@ func (c *fakeCmd) Func(args *CmdArgs) error {
 var _ = Describe("dispatching to the correct callback", func() {
 	var (
 		environment     map[string]string
-		stdin           io.Reader
+		stdinData       string
 		stdout, stderr  *bytes.Buffer
 		cmdAdd, cmdDel  *fakeCmd
 		dispatch        *dispatcher
@@ -65,13 +64,14 @@ var _ = Describe("dispatching to the correct callback", func() {
 			"CNI_ARGS":        "some;extra;args",
 			"CNI_PATH":        "/some/cni/path",
 		}
-		stdin = strings.NewReader(`{ "some": "config" }`)
+
+		stdinData = `{ "some": "config", "cniVersion": "9.8.7" }`
 		stdout = &bytes.Buffer{}
 		stderr = &bytes.Buffer{}
 		versionInfo = version.PluginSupports("9.8.7")
 		dispatch = &dispatcher{
 			Getenv: func(key string) string { return environment[key] },
-			Stdin:  stdin,
+			Stdin:  strings.NewReader(stdinData),
 			Stdout: stdout,
 			Stderr: stderr,
 		}
@@ -83,7 +83,7 @@ var _ = Describe("dispatching to the correct callback", func() {
 			IfName:      "eth0",
 			Args:        "some;extra;args",
 			Path:        "/some/cni/path",
-			StdinData:   []byte(`{ "some": "config" }`),
+			StdinData:   []byte(stdinData),
 		}
 	})
 
@@ -144,6 +144,22 @@ var _ = Describe("dispatching to the correct callback", func() {
 
 			})
 		})
+
+		Context("when the stdin data is missing the required cniVersion config", func() {
+			BeforeEach(func() {
+				dispatch.Stdin = strings.NewReader(`{ "some": "config" }`)
+			})
+
+			It("immediately returns a useful error", func() {
+				err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo)
+				Expect(err).To(MatchError("missing required config cniVersion"))
+			})
+
+			It("does not call either callback", func() {
+				Expect(cmdAdd.CallCount).To(Equal(0))
+				Expect(cmdDel.CallCount).To(Equal(0))
+			})
+		})
 	})
 
 	Context("when the CNI_COMMAND is DEL", func() {
diff --git a/pkg/version/plugin.go b/pkg/version/plugin.go
index 9bd7dc83..3a42fe25 100644
--- a/pkg/version/plugin.go
+++ b/pkg/version/plugin.go
@@ -56,6 +56,7 @@ func PluginSupports(supportedVersions ...string) PluginInfo {
 	}
 }
 
+// PluginDecoder can decode the response returned by a plugin's VERSION command
 type PluginDecoder struct{}
 
 func (_ *PluginDecoder) Decode(jsonBytes []byte) (PluginInfo, error) {
diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go
index c9af9153..3688fee2 100644
--- a/plugins/main/bridge/bridge_test.go
+++ b/plugins/main/bridge/bridge_test.go
@@ -51,8 +51,9 @@ var _ = Describe("bridge Operations", func() {
 
 		conf := &NetConf{
 			NetConf: types.NetConf{
-				Name: "testConfig",
-				Type: "bridge",
+				CNIVersion: "0.2.0",
+				Name:       "testConfig",
+				Type:       "bridge",
 			},
 			BrName: IFNAME,
 			IsGW:   false,
@@ -95,8 +96,9 @@ var _ = Describe("bridge Operations", func() {
 
 			conf := &NetConf{
 				NetConf: types.NetConf{
-					Name: "testConfig",
-					Type: "bridge",
+					CNIVersion: "0.2.0",
+					Name:       "testConfig",
+					Type:       "bridge",
 				},
 				BrName: IFNAME,
 				IsGW:   false,
@@ -126,12 +128,14 @@ var _ = Describe("bridge Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		conf := fmt.Sprintf(`{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "bridge",
     "bridge": "%s",
     "isDefaultGateway": true,
     "ipMasq": false,
     "ipam": {
+        "cniVersion": "0.2.0",
         "type": "host-local",
         "subnet": "%s"
     }
@@ -253,15 +257,15 @@ var _ = Describe("bridge Operations", func() {
 	})
 
 	It("ensure bridge address", func() {
-
 		const IFNAME = "bridge0"
 		const EXPECTED_IP = "10.0.0.0/8"
 		const CHANGED_EXPECTED_IP = "10.1.2.3/16"
 
 		conf := &NetConf{
 			NetConf: types.NetConf{
-				Name: "testConfig",
-				Type: "bridge",
+				CNIVersion: "0.2.0",
+				Name:       "testConfig",
+				Type:       "bridge",
 			},
 			BrName: IFNAME,
 			IsGW:   true,
@@ -320,5 +324,4 @@ var _ = Describe("bridge Operations", func() {
 		})
 		Expect(err).NotTo(HaveOccurred())
 	})
-
 })
diff --git a/plugins/main/ipvlan/ipvlan_test.go b/plugins/main/ipvlan/ipvlan_test.go
index e4352ee8..e38aa056 100644
--- a/plugins/main/ipvlan/ipvlan_test.go
+++ b/plugins/main/ipvlan/ipvlan_test.go
@@ -63,8 +63,9 @@ var _ = Describe("ipvlan Operations", func() {
 	It("creates an ipvlan link in a non-default namespace", func() {
 		conf := &NetConf{
 			NetConf: types.NetConf{
-				Name: "testConfig",
-				Type: "ipvlan",
+				CNIVersion: "0.2.0",
+				Name:       "testConfig",
+				Type:       "ipvlan",
 			},
 			Master: MASTER_NAME,
 			Mode:   "l2",
@@ -101,10 +102,12 @@ var _ = Describe("ipvlan Operations", func() {
 		const IFNAME = "ipvl0"
 
 		conf := fmt.Sprintf(`{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "ipvlan",
     "master": "%s",
     "ipam": {
+        "cniVersion": "0.2.0",
         "type": "host-local",
         "subnet": "10.1.2.0/24"
     }
diff --git a/plugins/main/loopback/loopback_test.go b/plugins/main/loopback/loopback_test.go
index 65b11e0f..f71595b0 100644
--- a/plugins/main/loopback/loopback_test.go
+++ b/plugins/main/loopback/loopback_test.go
@@ -49,7 +49,7 @@ var _ = Describe("Loopback", func() {
 			fmt.Sprintf("CNI_ARGS=%s", "none"),
 			fmt.Sprintf("CNI_PATH=%s", "/some/test/path"),
 		}
-		command.Stdin = strings.NewReader("this doesn't matter")
+		command.Stdin = strings.NewReader(`{ "cniVersion": "0.1.0" }`)
 	})
 
 	AfterEach(func() {
diff --git a/plugins/main/macvlan/macvlan_test.go b/plugins/main/macvlan/macvlan_test.go
index a0a14863..6feb3427 100644
--- a/plugins/main/macvlan/macvlan_test.go
+++ b/plugins/main/macvlan/macvlan_test.go
@@ -64,8 +64,9 @@ var _ = Describe("macvlan Operations", func() {
 	It("creates an macvlan link in a non-default namespace", func() {
 		conf := &NetConf{
 			NetConf: types.NetConf{
-				Name: "testConfig",
-				Type: "macvlan",
+				CNIVersion: "0.2.0",
+				Name:       "testConfig",
+				Type:       "macvlan",
 			},
 			Master: MASTER_NAME,
 			Mode:   "bridge",
@@ -101,10 +102,12 @@ var _ = Describe("macvlan Operations", func() {
 		const IFNAME = "macvl0"
 
 		conf := fmt.Sprintf(`{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "macvlan",
     "master": "%s",
     "ipam": {
+        "cniVersion": "0.2.0",
         "type": "host-local",
         "subnet": "10.1.2.0/24"
     }
diff --git a/plugins/main/ptp/ptp_test.go b/plugins/main/ptp/ptp_test.go
index 0b7411e0..2ebaaf5a 100644
--- a/plugins/main/ptp/ptp_test.go
+++ b/plugins/main/ptp/ptp_test.go
@@ -43,11 +43,13 @@ var _ = Describe("ptp Operations", func() {
 		const IFNAME = "ptp0"
 
 		conf := `{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "ptp",
     "ipMasq": true,
     "mtu": 5000,
     "ipam": {
+        "cniVersion": "0.2.0",
         "type": "host-local",
         "subnet": "10.1.2.0/24"
     }
diff --git a/plugins/test/noop/noop_test.go b/plugins/test/noop/noop_test.go
index cb44817f..40a37414 100644
--- a/plugins/test/noop/noop_test.go
+++ b/plugins/test/noop/noop_test.go
@@ -60,14 +60,14 @@ var _ = Describe("No-op plugin", func() {
 			"CNI_IFNAME=some-eth0",
 			"CNI_PATH=/some/bin/path",
 		}
-		cmd.Stdin = strings.NewReader(`{"some":"stdin-json"}`)
+		cmd.Stdin = strings.NewReader(`{"some":"stdin-json", "cniVersion": "0.2.0"}`)
 		expectedCmdArgs = skel.CmdArgs{
 			ContainerID: "some-container-id",
 			Netns:       "/some/netns/path",
 			IfName:      "some-eth0",
 			Args:        "DEBUG=" + debugFileName,
 			Path:        "/some/bin/path",
-			StdinData:   []byte(`{"some":"stdin-json"}`),
+			StdinData:   []byte(`{"some":"stdin-json", "cniVersion": "0.2.0"}`),
 		}
 	})