Merge pull request #296 from rosenhouse/plugins-require-versioned-config

Plugins validate cniVersion of NetConf

README.md

@@ -70,6 +70,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",
@@ -86,6 +87,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

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{

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

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"))
 			})
 		})
-
 	})
 })

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{}
 	})
 

pkg/skel/skel.go

@@ -43,6 +43,9 @@ type dispatcher struct {
 	Stdin  io.Reader
 	Stdout io.Writer
 	Stderr io.Writer
+
+	ConfVersionDecoder version.ConfigDecoder
+	VersionReconciler  version.Reconciler
 }
 
 type reqForCmdEntry map[string]bool
@@ -143,6 +146,22 @@ func createTypedError(f string, args ...interface{}) *types.Error {
 	}
 }
 
+func (t *dispatcher) checkVersionAndCall(cmdArgs *CmdArgs, pluginVersionInfo version.PluginInfo, toCall func(*CmdArgs) error) error {
+	configVersion, err := t.ConfVersionDecoder.Decode(cmdArgs.StdinData)
+	if err != nil {
+		return err
+	}
+	verErr := t.VersionReconciler.Check(configVersion, pluginVersionInfo)
+	if verErr != nil {
+		return &types.Error{
+			Code:    types.ErrIncompatibleCNIVersion,
+			Msg:     "incompatible CNI versions",
+			Details: verErr.Details(),
+		}
+	}
+	return toCall(cmdArgs)
+}
+
 func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionInfo version.PluginInfo) *types.Error {
 	cmd, cmdArgs, err := t.getCmdArgsFromEnv()
 	if err != nil {
@@ -151,14 +170,11 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionIn
 
 	switch cmd {
 	case "ADD":
-		err = cmdAdd(cmdArgs)
+		err = t.checkVersionAndCall(cmdArgs, versionInfo, cmdAdd)
-
 	case "DEL":
-		err = cmdDel(cmdArgs)
+		err = t.checkVersionAndCall(cmdArgs, versionInfo, cmdDel)
-
 	case "VERSION":
 		err = versionInfo.Encode(t.Stdout)
-
 	default:
 		return createTypedError("unknown CNI_COMMAND: %v", cmd)
 	}

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,46 @@ 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" }`)
+			})
+
+			Context("when the plugin supports version 0.1.0", func() {
+				BeforeEach(func() {
+					versionInfo = version.PluginSupports("0.1.0")
+					expectedCmdArgs.StdinData = []byte(`{ "some": "config" }`)
+				})
+
+				It("infers the config is 0.1.0 and calls the cmdAdd callback", func() {
+					err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo)
+					Expect(err).NotTo(HaveOccurred())
+
+					Expect(cmdAdd.CallCount).To(Equal(1))
+					Expect(cmdAdd.Received.CmdArgs).To(Equal(expectedCmdArgs))
+				})
+			})
+
+			Context("when the plugin does not support 0.1.0", func() {
+				BeforeEach(func() {
+					versionInfo = version.PluginSupports("4.3.2")
+				})
+
+				It("immediately returns a useful error", func() {
+					err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo)
+					Expect(err.Code).To(Equal(types.ErrIncompatibleCNIVersion)) // see https://github.com/containernetworking/cni/blob/master/SPEC.md#well-known-error-codes
+					Expect(err.Msg).To(Equal("incompatible CNI versions"))
+					Expect(err.Details).To(Equal(`config is "0.1.0", plugin supports ["4.3.2"]`))
+				})
+
+				It("does not call either callback", func() {
+					dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo)
+					Expect(cmdAdd.CallCount).To(Equal(0))
+					Expect(cmdDel.CallCount).To(Equal(0))
+				})
+			})
+		})
 	})
 
 	Context("when the CNI_COMMAND is DEL", func() {
@@ -207,6 +247,22 @@ var _ = Describe("dispatching to the correct callback", func() {
 			Entry("args", "CNI_ARGS", false),
 			Entry("path", "CNI_PATH", false),
 		)
+
+		Context("when the stdin is empty", func() {
+			BeforeEach(func() {
+				dispatch.Stdin = strings.NewReader("")
+			})
+
+			It("succeeds without error", func() {
+				err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo)
+
+				Expect(err).NotTo(HaveOccurred())
+				Expect(stdout).To(MatchJSON(`{
+					"cniVersion": "0.2.0",
+					"supportedVersions": ["9.8.7"]
+			}`))
+			})
+		})
 	})
 
 	Context("when the CNI_COMMAND is unrecognized", func() {

pkg/types/types.go

@@ -112,6 +112,14 @@ type Route struct {
 	GW  net.IP
 }
 
+// Well known error codes
+// see https://github.com/containernetworking/cni/blob/master/SPEC.md#well-known-error-codes
+const (
+	ErrUnknown                uint = iota // 0
+	ErrIncompatibleCNIVersion             // 1
+	ErrUnsupportedField                   // 2
+)
+
 type Error struct {
 	Code    uint   `json:"code"`
 	Msg     string `json:"msg"`

pkg/version/conf.go

@@ -0,0 +1,37 @@
+// Copyright 2016 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package version
+
+import (
+	"encoding/json"
+	"fmt"
+)
+
+// ConfigDecoder can decode the CNI version available in network config data
+type ConfigDecoder struct{}
+
+func (*ConfigDecoder) Decode(jsonBytes []byte) (string, error) {
+	var conf struct {
+		CNIVersion string `json:"cniVersion"`
+	}
+	err := json.Unmarshal(jsonBytes, &conf)
+	if err != nil {
+		return "", fmt.Errorf("decoding version from network config: %s", err)
+	}
+	if conf.CNIVersion == "" {
+		return "0.1.0", nil
+	}
+	return conf.CNIVersion, nil
+}

pkg/version/conf_test.go

@@ -0,0 +1,69 @@
+// Copyright 2016 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package version_test
+
+import (
+	"github.com/containernetworking/cni/pkg/version"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Decoding the version of network config", func() {
+	var (
+		decoder     *version.ConfigDecoder
+		configBytes []byte
+	)
+
+	BeforeEach(func() {
+		decoder = &version.ConfigDecoder{}
+		configBytes = []byte(`{ "cniVersion": "4.3.2" }`)
+	})
+
+	Context("when the version is explict", func() {
+		It("returns the version", func() {
+			version, err := decoder.Decode(configBytes)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(version).To(Equal("4.3.2"))
+		})
+	})
+
+	Context("when the version is not present in the config", func() {
+		BeforeEach(func() {
+			configBytes = []byte(`{ "not-a-version-field": "foo" }`)
+		})
+
+		It("assumes the config is version 0.1.0", func() {
+			version, err := decoder.Decode(configBytes)
+			Expect(err).NotTo(HaveOccurred())
+
+			Expect(version).To(Equal("0.1.0"))
+		})
+	})
+
+	Context("when the config data is malformed", func() {
+		BeforeEach(func() {
+			configBytes = []byte(`{{{`)
+		})
+
+		It("returns a useful error", func() {
+			_, err := decoder.Decode(configBytes)
+			Expect(err).To(MatchError(HavePrefix(
+				"decoding version from network config: invalid character",
+			)))
+		})
+	})
+})

pkg/version/plugin.go

@@ -56,9 +56,10 @@ 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) {
+func (*PluginDecoder) Decode(jsonBytes []byte) (PluginInfo, error) {
 	var info pluginInfo
 	err := json.Unmarshal(jsonBytes, &info)
 	if err != nil {

pkg/version/reconcile.go

@@ -0,0 +1,47 @@
+// Copyright 2016 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package version
+
+import "fmt"
+
+type ErrorIncompatible struct {
+	Config string
+	Plugin []string
+}
+
+func (e *ErrorIncompatible) Details() string {
+	return fmt.Sprintf("config is %q, plugin supports %q", e.Config, e.Plugin)
+}
+
+func (e *ErrorIncompatible) Error() string {
+	return fmt.Sprintf("incompatible CNI versions: %s", e.Details())
+}
+
+type Reconciler struct{}
+
+func (*Reconciler) Check(configVersion string, pluginInfo PluginInfo) *ErrorIncompatible {
+	pluginVersions := pluginInfo.SupportedVersions()
+
+	for _, pluginVersion := range pluginVersions {
+		if configVersion == pluginVersion {
+			return nil
+		}
+	}
+
+	return &ErrorIncompatible{
+		Config: configVersion,
+		Plugin: pluginVersions,
+	}
+}

pkg/version/reconcile_test.go

@@ -0,0 +1,51 @@
+// Copyright 2016 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package version_test
+
+import (
+	"github.com/containernetworking/cni/pkg/version"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Reconcile versions of net config with versions supported by plugins", func() {
+	var (
+		reconciler *version.Reconciler
+		pluginInfo version.PluginInfo
+	)
+
+	BeforeEach(func() {
+		reconciler = &version.Reconciler{}
+		pluginInfo = version.PluginSupports("1.2.3", "4.3.2")
+	})
+
+	It("succeeds if the config version is supported by the plugin", func() {
+		err := reconciler.Check("4.3.2", pluginInfo)
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	Context("when the config version is not supported by the plugin", func() {
+		It("returns a helpful error", func() {
+			err := reconciler.Check("0.1.0", pluginInfo)
+
+			Expect(err).To(Equal(&version.ErrorIncompatible{
+				Config: "0.1.0",
+				Plugin: []string{"1.2.3", "4.3.2"},
+			}))
+
+			Expect(err.Error()).To(Equal(`incompatible CNI versions: config is "0.1.0", plugin supports ["1.2.3" "4.3.2"]`))
+		})
+	})
+})

plugins/main/bridge/bridge_test.go

@@ -51,8 +51,9 @@ var _ = Describe("bridge Operations", func() {
 
 		conf := &NetConf{
 			NetConf: types.NetConf{
-				Name: "testConfig",
+				CNIVersion: "0.2.0",
-				Type: "bridge",
+				Name:       "testConfig",
+				Type:       "bridge",
 			},
 			BrName: IFNAME,
 			IsGW:   false,
@@ -95,8 +96,9 @@ var _ = Describe("bridge Operations", func() {
 
 			conf := &NetConf{
 				NetConf: types.NetConf{
-					Name: "testConfig",
+					CNIVersion: "0.2.0",
-					Type: "bridge",
+					Name:       "testConfig",
+					Type:       "bridge",
 				},
 				BrName: IFNAME,
 				IsGW:   false,
@@ -126,6 +128,7 @@ var _ = Describe("bridge Operations", func() {
 		Expect(err).NotTo(HaveOccurred())
 
 		conf := fmt.Sprintf(`{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "bridge",
     "bridge": "%s",
@@ -253,15 +256,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",
+				CNIVersion: "0.2.0",
-				Type: "bridge",
+				Name:       "testConfig",
+				Type:       "bridge",
 			},
 			BrName: IFNAME,
 			IsGW:   true,
@@ -320,5 +323,4 @@ var _ = Describe("bridge Operations", func() {
 		})
 		Expect(err).NotTo(HaveOccurred())
 	})
-
 })

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",
+				CNIVersion: "0.2.0",
-				Type: "ipvlan",
+				Name:       "testConfig",
+				Type:       "ipvlan",
 			},
 			Master: MASTER_NAME,
 			Mode:   "l2",
@@ -101,6 +102,7 @@ var _ = Describe("ipvlan Operations", func() {
 		const IFNAME = "ipvl0"
 
 		conf := fmt.Sprintf(`{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "ipvlan",
     "master": "%s",

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() {

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",
+				CNIVersion: "0.2.0",
-				Type: "macvlan",
+				Name:       "testConfig",
+				Type:       "macvlan",
 			},
 			Master: MASTER_NAME,
 			Mode:   "bridge",
@@ -101,6 +102,7 @@ var _ = Describe("macvlan Operations", func() {
 		const IFNAME = "macvl0"
 
 		conf := fmt.Sprintf(`{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "macvlan",
     "master": "%s",

plugins/main/ptp/ptp_test.go

@@ -43,6 +43,7 @@ var _ = Describe("ptp Operations", func() {
 		const IFNAME = "ptp0"
 
 		conf := `{
+    "cniVersion": "0.2.0",
     "name": "mynet",
     "type": "ptp",
     "ipMasq": true,

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"}`),
 		}
 	})