diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index 9cf03917..d6f0f4cf 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -1,4 +1,4 @@ -// Copyright 2014 CNI authors +// Copyright 2014-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. @@ -18,6 +18,7 @@ package skel import ( "fmt" + "io" "io/ioutil" "log" "os" @@ -36,11 +37,15 @@ type CmdArgs struct { StdinData []byte } +type dispatcher struct { + Getenv func(string) string + Stdin io.Reader + Stderr io.Writer +} + type reqForCmdEntry map[string]bool -// PluginMain is the "main" for a plugin. It accepts -// two callback functions for add and del commands. -func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { +func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) { var cmd, contID, netns, ifName, args, path string vars := []struct { @@ -100,20 +105,22 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { argsMissing := false for _, v := range vars { - *v.val = os.Getenv(v.name) - if v.reqForCmd[cmd] && *v.val == "" { - log.Printf("%v env variable missing", v.name) - argsMissing = true + *v.val = t.Getenv(v.name) + if *v.val == "" { + if v.reqForCmd[cmd] || v.name == "CNI_COMMAND" { + fmt.Fprintf(t.Stderr, "%v env variable missing\n", v.name) + argsMissing = true + } } } if argsMissing { - dieMsg("required env variables missing") + return "", nil, fmt.Errorf("required env variables missing") } - stdinData, err := ioutil.ReadAll(os.Stdin) + stdinData, err := ioutil.ReadAll(t.Stdin) if err != nil { - dieMsg("error reading from stdin: %v", err) + return "", nil, fmt.Errorf("error reading from stdin: %v", err) } cmdArgs := &CmdArgs{ @@ -124,6 +131,21 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { Path: path, StdinData: stdinData, } + return cmd, cmdArgs, nil +} + +func createTypedError(f string, args ...interface{}) *types.Error { + return &types.Error{ + Code: 100, + Msg: fmt.Sprintf(f, args...), + } +} + +func (t *dispatcher) pluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) *types.Error { + cmd, cmdArgs, err := t.getCmdArgsFromEnv() + if err != nil { + return createTypedError(err.Error()) + } switch cmd { case "ADD": @@ -133,24 +155,32 @@ func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { err = cmdDel(cmdArgs) default: - dieMsg("unknown CNI_COMMAND: %v", cmd) + return createTypedError("unknown CNI_COMMAND: %v", cmd) } if err != nil { if e, ok := err.(*types.Error); ok { // don't wrap Error in Error - dieErr(e) + return e } - dieMsg(err.Error()) + return createTypedError(err.Error()) } + return nil } -func dieMsg(f string, args ...interface{}) { - e := &types.Error{ - Code: 100, - Msg: fmt.Sprintf(f, args...), +// PluginMain is the "main" for a plugin. It accepts +// two callback functions for add and del commands. +func PluginMain(cmdAdd, cmdDel func(_ *CmdArgs) error) { + caller := dispatcher{ + Getenv: os.Getenv, + Stdin: os.Stdin, + Stderr: os.Stderr, + } + + err := caller.pluginMain(cmdAdd, cmdDel) + if err != nil { + dieErr(err) } - dieErr(e) } func dieErr(e *types.Error) { diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index a52e0145..39df2716 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -15,70 +15,238 @@ package skel import ( - "os" + "bytes" + "errors" + "io" + "strings" + "github.com/containernetworking/cni/pkg/types" + + "github.com/containernetworking/cni/pkg/testutils" . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" ) -var _ = Describe("Skel", func() { +type fakeCmd struct { + CallCount int + Returns struct { + Error error + } + Received struct { + CmdArgs *CmdArgs + } +} + +func (c *fakeCmd) Func(args *CmdArgs) error { + c.CallCount++ + c.Received.CmdArgs = args + return c.Returns.Error +} + +var _ = Describe("dispatching to the correct callback", func() { var ( - fNoop = func(_ *CmdArgs) error { return nil } - // fErr = func(_ *CmdArgs) error { return errors.New("dummy") } - envVars = []struct { - name string - val string - }{ - {"CNI_CONTAINERID", "dummy"}, - {"CNI_NETNS", "dummy"}, - {"CNI_IFNAME", "dummy"}, - {"CNI_ARGS", "dummy"}, - {"CNI_PATH", "dummy"}, - } + environment map[string]string + stdin io.Reader + stderr *bytes.Buffer + cmdAdd, cmdDel *fakeCmd + dispatch *dispatcher + expectedCmdArgs *CmdArgs ) - It("Must be possible to set the env vars", func() { - for _, v := range envVars { - err := os.Setenv(v.name, v.val) - Expect(err).NotTo(HaveOccurred()) + BeforeEach(func() { + environment = map[string]string{ + "CNI_COMMAND": "ADD", + "CNI_CONTAINERID": "some-container-id", + "CNI_NETNS": "/some/netns/path", + "CNI_IFNAME": "eth0", + "CNI_ARGS": "some;extra;args", + "CNI_PATH": "/some/cni/path", + } + stdin = strings.NewReader(`{ "some": "config" }`) + stderr = &bytes.Buffer{} + dispatch = &dispatcher{ + Getenv: func(key string) string { return environment[key] }, + Stdin: stdin, + Stderr: stderr, + } + cmdAdd = &fakeCmd{} + cmdDel = &fakeCmd{} + expectedCmdArgs = &CmdArgs{ + ContainerID: "some-container-id", + Netns: "/some/netns/path", + IfName: "eth0", + Args: "some;extra;args", + Path: "/some/cni/path", + StdinData: []byte(`{ "some": "config" }`), } }) - Context("When dummy environment variables are passed", func() { + var envVarChecker = func(envVar string, isRequired bool) { + delete(environment, envVar) - It("should not fail with ADD and noop callback", func() { - err := os.Setenv("CNI_COMMAND", "ADD") + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + if isRequired { + Expect(err).To(Equal(&types.Error{ + Code: 100, + Msg: "required env variables missing", + })) + Expect(stderr.String()).To(ContainSubstring(envVar + " env variable missing\n")) + } else { Expect(err).NotTo(HaveOccurred()) - PluginMain(fNoop, nil) + } + } + + Context("when the CNI_COMMAND is ADD", func() { + It("extracts env vars and stdin data and calls cmdAdd", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).NotTo(HaveOccurred()) + Expect(cmdAdd.CallCount).To(Equal(1)) + Expect(cmdDel.CallCount).To(Equal(0)) + Expect(cmdAdd.Received.CmdArgs).To(Equal(expectedCmdArgs)) }) - // TODO: figure out howto mock printing and os.Exit() - // It("should fail with ADD and error callback", func() { - // err := os.Setenv("CNI_COMMAND", "ADD") - // Expect(err).NotTo(HaveOccurred()) - // PluginMain(fErr, nil) - // }) + It("does not call cmdDel", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) - It("should not fail with DEL and noop callback", func() { - err := os.Setenv("CNI_COMMAND", "DEL") Expect(err).NotTo(HaveOccurred()) - PluginMain(nil, fNoop) + Expect(cmdDel.CallCount).To(Equal(0)) }) - // TODO: figure out howto mock printing and os.Exit() - // It("should fail with DEL and error callback", func() { - // err := os.Setenv("CNI_COMMAND", "DEL") - // Expect(err).NotTo(HaveOccurred()) - // PluginMain(fErr, nil) - // }) + DescribeTable("required / optional env vars", envVarChecker, + Entry("command", "CNI_COMMAND", true), + Entry("container id", "CNI_CONTAINER_ID", false), + Entry("net ns", "CNI_NETNS", true), + Entry("if name", "CNI_IFNAME", true), + Entry("args", "CNI_ARGS", false), + Entry("path", "CNI_PATH", true), + ) - It("should not fail with DEL and no NETNS and noop callback", func() { - err := os.Setenv("CNI_COMMAND", "DEL") - Expect(err).NotTo(HaveOccurred()) - err = os.Unsetenv("CNI_NETNS") - Expect(err).NotTo(HaveOccurred()) - PluginMain(nil, fNoop) + Context("when multiple required env vars are missing", func() { + BeforeEach(func() { + delete(environment, "CNI_NETNS") + delete(environment, "CNI_IFNAME") + delete(environment, "CNI_PATH") + }) + + It("reports that all of them are missing, not just the first", func() { + Expect(dispatch.pluginMain(cmdAdd.Func, cmdDel.Func)).NotTo(Succeed()) + log := stderr.String() + Expect(log).To(ContainSubstring("CNI_NETNS env variable missing\n")) + Expect(log).To(ContainSubstring("CNI_IFNAME env variable missing\n")) + Expect(log).To(ContainSubstring("CNI_PATH env variable missing\n")) + + }) + }) + }) + + Context("when the CNI_COMMAND is DEL", func() { + BeforeEach(func() { + environment["CNI_COMMAND"] = "DEL" }) + It("calls cmdDel with the env vars and stdin data", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).NotTo(HaveOccurred()) + Expect(cmdDel.CallCount).To(Equal(1)) + Expect(cmdDel.Received.CmdArgs).To(Equal(expectedCmdArgs)) + }) + + It("does not call cmdAdd", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).NotTo(HaveOccurred()) + Expect(cmdAdd.CallCount).To(Equal(0)) + }) + + DescribeTable("required / optional env vars", envVarChecker, + Entry("command", "CNI_COMMAND", true), + Entry("container id", "CNI_CONTAINER_ID", false), + Entry("net ns", "CNI_NETNS", false), + Entry("if name", "CNI_IFNAME", true), + Entry("args", "CNI_ARGS", false), + Entry("path", "CNI_PATH", true), + ) + }) + + Context("when the CNI_COMMAND is unrecognized", func() { + BeforeEach(func() { + environment["CNI_COMMAND"] = "NOPE" + }) + + It("does not call any cmd callback", func() { + dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(cmdAdd.CallCount).To(Equal(0)) + Expect(cmdDel.CallCount).To(Equal(0)) + }) + + It("returns an error", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).To(Equal(&types.Error{ + Code: 100, + Msg: "unknown CNI_COMMAND: NOPE", + })) + }) + }) + + Context("when stdin cannot be read", func() { + BeforeEach(func() { + dispatch.Stdin = &testutils.BadReader{} + }) + + It("does not call any cmd callback", func() { + dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(cmdAdd.CallCount).To(Equal(0)) + Expect(cmdDel.CallCount).To(Equal(0)) + }) + + It("wraps and returns the error", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).To(Equal(&types.Error{ + Code: 100, + Msg: "error reading from stdin: banana", + })) + }) + }) + + Context("when the callback returns an error", func() { + Context("when it is a typed Error", func() { + BeforeEach(func() { + cmdAdd.Returns.Error = &types.Error{ + Code: 1234, + Msg: "insufficient something", + } + }) + + It("returns the error as-is", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).To(Equal(&types.Error{ + Code: 1234, + Msg: "insufficient something", + })) + }) + }) + + Context("when it is an unknown error", func() { + BeforeEach(func() { + cmdAdd.Returns.Error = errors.New("potato") + }) + + It("wraps and returns the error", func() { + err := dispatch.pluginMain(cmdAdd.Func, cmdDel.Func) + + Expect(err).To(Equal(&types.Error{ + Code: 100, + Msg: "potato", + })) + }) + }) }) }) diff --git a/pkg/testutils/bad_reader.go b/pkg/testutils/bad_reader.go new file mode 100644 index 00000000..ca06c5e9 --- /dev/null +++ b/pkg/testutils/bad_reader.go @@ -0,0 +1,32 @@ +// 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 testutils + +import "errors" + +type BadReader struct { + Error error +} + +func (r *BadReader) Read(buffer []byte) (int, error) { + if r.Error != nil { + return 0, r.Error + } + return 0, errors.New("banana") +} + +func (r *BadReader) Close() error { + return nil +}