Skip to content

Commit

Permalink
skel: Plugins require a cniVersion in the NetConf
Browse files Browse the repository at this point in the history
  • Loading branch information
rosenhouse committed Sep 19, 2016
1 parent 5603239 commit fd150a4
Show file tree
Hide file tree
Showing 14 changed files with 76 additions and 26 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion libcni/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 3 additions & 1 deletion pkg/invoke/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package invoke

import (
"encoding/json"
"fmt"
"os"

"github.com/containernetworking/cni/pkg/types"
Expand Down Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions pkg/invoke/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package invoke_test

import (
"encoding/json"
"errors"

"github.com/containernetworking/cni/pkg/invoke"
Expand Down Expand Up @@ -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"}
})
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -146,6 +148,5 @@ var _ = Describe("Executing a plugin, unit tests", func() {
Expect(env).To(ContainElement("CNI_PATH=dummy"))
})
})

})
})
2 changes: 1 addition & 1 deletion pkg/invoke/raw_exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
})

Expand Down
17 changes: 17 additions & 0 deletions pkg/skel/skel.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package skel

import (
"encoding/json"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 21 additions & 5 deletions pkg/skel/skel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package skel
import (
"bytes"
"errors"
"io"
"strings"

"github.com/containernetworking/cni/pkg/types"
Expand Down Expand Up @@ -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
Expand All @@ -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,
}
Expand All @@ -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),
}
})

Expand Down Expand Up @@ -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() {
Expand Down
1 change: 1 addition & 0 deletions pkg/version/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
19 changes: 11 additions & 8 deletions plugins/main/bridge/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -320,5 +324,4 @@ var _ = Describe("bridge Operations", func() {
})
Expect(err).NotTo(HaveOccurred())
})

})
7 changes: 5 additions & 2 deletions plugins/main/ipvlan/ipvlan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
}
Expand Down
2 changes: 1 addition & 1 deletion plugins/main/loopback/loopback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
7 changes: 5 additions & 2 deletions plugins/main/macvlan/macvlan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
}
Expand Down
2 changes: 2 additions & 0 deletions plugins/main/ptp/ptp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/test/noop/noop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}`),
}
})

Expand Down

0 comments on commit fd150a4

Please sign in to comment.