From fd150a4c9798bcfcdc3e44da2c88332e059ec47a Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Tue, 6 Sep 2016 20:19:26 -0400 Subject: [PATCH 1/5] 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 </etc/cni/net.d/10-mynet.conf </etc/cni/net.d/99-loopback.conf < Date: Sun, 18 Sep 2016 21:56:17 -0700 Subject: [PATCH 2/5] versioning: add basic version decode for network config --- pkg/version/conf.go | 37 +++++++++++++++++++++ pkg/version/conf_test.go | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 pkg/version/conf.go create mode 100644 pkg/version/conf_test.go diff --git a/pkg/version/conf.go b/pkg/version/conf.go new file mode 100644 index 00000000..3cca58bb --- /dev/null +++ b/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 +} diff --git a/pkg/version/conf_test.go b/pkg/version/conf_test.go new file mode 100644 index 00000000..881c57ad --- /dev/null +++ b/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", + ))) + }) + }) +}) From fba37620e0f418178e23cbae5c3c0cf3e74aec4b Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Mon, 19 Sep 2016 13:00:49 -0700 Subject: [PATCH 3/5] versioning: plugins require version match with config infer version 0.1.0 when config is missing an explicit "cniVersion" field --- pkg/skel/skel.go | 33 +++++++++++----------- pkg/skel/skel_test.go | 52 +++++++++++++++++++++++++++++++---- pkg/types/types.go | 8 ++++++ pkg/version/plugin.go | 2 +- pkg/version/reconcile.go | 47 +++++++++++++++++++++++++++++++ pkg/version/reconcile_test.go | 51 ++++++++++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 24 deletions(-) create mode 100644 pkg/version/reconcile.go create mode 100644 pkg/version/reconcile_test.go diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index 180ce24e..8fc9636e 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -17,7 +17,6 @@ package skel import ( - "encoding/json" "fmt" "io" "io/ioutil" @@ -44,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 @@ -144,16 +146,20 @@ 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 { +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 } - if netconf.CNIVersion == "" { - return fmt.Errorf("missing required config cniVersion") + verErr := t.VersionReconciler.Check(configVersion, pluginVersionInfo) + if verErr != nil { + return &types.Error{ + Code: types.ErrIncompatibleCNIVersion, + Msg: "incompatible CNI versions", + Details: verErr.Details(), + } } - - return nil + return toCall(cmdArgs) } func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionInfo version.PluginInfo) *types.Error { @@ -162,20 +168,13 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error, versionIn return createTypedError(err.Error()) } - if err = t.validateVersion(cmdArgs.StdinData); err != nil { - return createTypedError(err.Error()) - } - 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) } diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 7ae25d35..570f0272 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -150,14 +150,38 @@ var _ = Describe("dispatching to the correct callback", 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")) + 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)) + }) }) - It("does not call either callback", func() { - Expect(cmdAdd.CallCount).To(Equal(0)) - Expect(cmdDel.CallCount).To(Equal(0)) + 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(uint(1))) // 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)) + }) }) }) }) @@ -223,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() { diff --git a/pkg/types/types.go b/pkg/types/types.go index ba02580a..17caa49b 100644 --- a/pkg/types/types.go +++ b/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"` diff --git a/pkg/version/plugin.go b/pkg/version/plugin.go index 3a42fe25..dc937b54 100644 --- a/pkg/version/plugin.go +++ b/pkg/version/plugin.go @@ -59,7 +59,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) { +func (*PluginDecoder) Decode(jsonBytes []byte) (PluginInfo, error) { var info pluginInfo err := json.Unmarshal(jsonBytes, &info) if err != nil { diff --git a/pkg/version/reconcile.go b/pkg/version/reconcile.go new file mode 100644 index 00000000..f61ef653 --- /dev/null +++ b/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, + } +} diff --git a/pkg/version/reconcile_test.go b/pkg/version/reconcile_test.go new file mode 100644 index 00000000..19a9e23f --- /dev/null +++ b/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"]`)) + }) + }) +}) From 0135e2751ec9c42bb692f93632db8ec033cd4662 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Mon, 19 Sep 2016 13:10:27 -0700 Subject: [PATCH 4/5] versioning: ipam config is not versioned --- plugins/main/bridge/bridge_test.go | 1 - plugins/main/ipvlan/ipvlan_test.go | 1 - plugins/main/macvlan/macvlan_test.go | 1 - plugins/main/ptp/ptp_test.go | 1 - 4 files changed, 4 deletions(-) diff --git a/plugins/main/bridge/bridge_test.go b/plugins/main/bridge/bridge_test.go index 3688fee2..3840aa91 100644 --- a/plugins/main/bridge/bridge_test.go +++ b/plugins/main/bridge/bridge_test.go @@ -135,7 +135,6 @@ var _ = Describe("bridge Operations", func() { "isDefaultGateway": true, "ipMasq": false, "ipam": { - "cniVersion": "0.2.0", "type": "host-local", "subnet": "%s" } diff --git a/plugins/main/ipvlan/ipvlan_test.go b/plugins/main/ipvlan/ipvlan_test.go index e38aa056..4e09ceba 100644 --- a/plugins/main/ipvlan/ipvlan_test.go +++ b/plugins/main/ipvlan/ipvlan_test.go @@ -107,7 +107,6 @@ var _ = Describe("ipvlan Operations", func() { "type": "ipvlan", "master": "%s", "ipam": { - "cniVersion": "0.2.0", "type": "host-local", "subnet": "10.1.2.0/24" } diff --git a/plugins/main/macvlan/macvlan_test.go b/plugins/main/macvlan/macvlan_test.go index 6feb3427..9594a59b 100644 --- a/plugins/main/macvlan/macvlan_test.go +++ b/plugins/main/macvlan/macvlan_test.go @@ -107,7 +107,6 @@ var _ = Describe("macvlan Operations", func() { "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 2ebaaf5a..5b85670d 100644 --- a/plugins/main/ptp/ptp_test.go +++ b/plugins/main/ptp/ptp_test.go @@ -49,7 +49,6 @@ var _ = Describe("ptp Operations", func() { "ipMasq": true, "mtu": 5000, "ipam": { - "cniVersion": "0.2.0", "type": "host-local", "subnet": "10.1.2.0/24" } From 5b696f3307fcffdb620e366120f3c0b77360b5f5 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Mon, 19 Sep 2016 13:14:02 -0700 Subject: [PATCH 5/5] skel: use named constant for Incompatible CNI Version error code --- pkg/skel/skel_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 570f0272..cd668c91 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -172,7 +172,7 @@ var _ = Describe("dispatching to the correct callback", func() { It("immediately returns a useful error", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func, versionInfo) - Expect(err.Code).To(Equal(uint(1))) // see https://github.com/containernetworking/cni/blob/master/SPEC.md#well-known-error-codes + 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"]`)) })