skel: Plugins require a cniVersion in the NetConf

diff --git a/README.md b/README.md
index 56f0609..c37a306 100644 (file)
--- 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 6ac88e8..9aeb52a 100644 (file)
--- 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 7eb0615..167d38f 100644 (file)
--- 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 94007d7..2b9c9bf 100644 (file)
--- 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 7df60a1..b0ca960 100644 (file)
--- 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 de64d7d..180ce24 100644 (file)
--- 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 1cc533b..7ae25d3 100644 (file)
--- 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 9bd7dc8..3a42fe2 100644 (file)
--- 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 c9af915..3688fee 100644 (file)
--- 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 e4352ee..e38aa05 100644 (file)
--- 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 65b11e0..f71595b 100644 (file)
--- 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 a0a1486..6feb342 100644 (file)
--- 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 0b7411e..2ebaaf5 100644 (file)
--- 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 cb44817..40a3741 100644 (file)
--- 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"}`),
                }
        })