diff --git a/SPEC.md b/SPEC.md index 6bf57597..e6b031b0 100644 --- a/SPEC.md +++ b/SPEC.md @@ -758,4 +758,8 @@ Error codes 1-99 must not be used other than as specified here. - `1` - Incompatible CNI version - `2` - Unsupported field in network configuration. The error message must contain the key and value of the unsupported field. - `3` - Container unknown or does not exist. This error implies the runtime does not need to perform any container network cleanup (for example, calling the `DEL` action on the container). +- `4` - Invalid necessary environment variables, like CNI_COMMAND, CNI_CONTAINERID, etc. The error message must contain the names of invalid variables. +- `5` - I/O failure. For example, failed to read network config bytes from stdin. +- `6` - Failed to decode content. For example, failed to unmarshal network config from bytes or failed to decode version info from string. +- `7` - Invalid network config. If some validations on network configs do not pass, this error will be raised. - `11` - Try again later. If the plugin detects some transient condition that should clear up, it can use this code to notify the runtime it should re-try the operation later. diff --git a/pkg/skel/skel.go b/pkg/skel/skel.go index af56b8a1..f8be8080 100644 --- a/pkg/skel/skel.go +++ b/pkg/skel/skel.go @@ -62,7 +62,7 @@ func (e missingEnvError) Error() string { return e.msg } -func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) { +func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, *types.Error) { var cmd, contID, netns, ifName, args, path string vars := []struct { @@ -138,7 +138,7 @@ func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) { if len(argsMissing) > 0 { joined := strings.Join(argsMissing, ",") - return "", nil, missingEnvError{fmt.Sprintf("required env variables [%s] missing", joined)} + return "", nil, types.NewError(types.ErrInvalidEnvironmentVariables, fmt.Sprintf("required env variables [%s] missing", joined), "") } if cmd == "VERSION" { @@ -147,7 +147,7 @@ func (t *dispatcher) getCmdArgsFromEnv() (string, *CmdArgs, error) { stdinData, err := ioutil.ReadAll(t.Stdin) if err != nil { - return "", nil, fmt.Errorf("error reading from stdin: %v", err) + return "", nil, types.NewError(types.ErrIOFailure, fmt.Sprintf("error reading from stdin: %v", err), "") } cmdArgs := &CmdArgs{ @@ -168,32 +168,36 @@ func createTypedError(f string, args ...interface{}) *types.Error { } } -func (t *dispatcher) checkVersionAndCall(cmdArgs *CmdArgs, pluginVersionInfo version.PluginInfo, toCall func(*CmdArgs) error) error { +func (t *dispatcher) checkVersionAndCall(cmdArgs *CmdArgs, pluginVersionInfo version.PluginInfo, toCall func(*CmdArgs) error) *types.Error { configVersion, err := t.ConfVersionDecoder.Decode(cmdArgs.StdinData) if err != nil { - return err + return types.NewError(types.ErrDecodingFailure, err.Error(), "") } verErr := t.VersionReconciler.Check(configVersion, pluginVersionInfo) if verErr != nil { - return &types.Error{ - Code: types.ErrIncompatibleCNIVersion, - Msg: "incompatible CNI versions", - Details: verErr.Details(), + return types.NewError(types.ErrIncompatibleCNIVersion, "incompatible CNI versions", verErr.Details()) + } + + if err = toCall(cmdArgs); err != nil { + if e, ok := err.(*types.Error); ok { + // don't wrap Error in Error + return e } + return types.NewError(types.ErrInternal, err.Error(), "") } - return toCall(cmdArgs) + return nil } -func validateConfig(jsonBytes []byte) error { +func validateConfig(jsonBytes []byte) *types.Error { var conf struct { Name string `json:"name"` } if err := json.Unmarshal(jsonBytes, &conf); err != nil { - return fmt.Errorf("error reading network config: %s", err) + return types.NewError(types.ErrDecodingFailure, fmt.Sprintf("error unmarshall network config: %v", err), "") } if conf.Name == "" { - return fmt.Errorf("missing network name") + return types.NewError(types.ErrInvalidNetworkConfig, "missing network name", "") } return nil } @@ -202,17 +206,17 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error, cmd, cmdArgs, err := t.getCmdArgsFromEnv() if err != nil { // Print the about string to stderr when no command is set - if _, ok := err.(missingEnvError); ok && t.Getenv("CNI_COMMAND") == "" && about != "" { - fmt.Fprintln(t.Stderr, about) + if err.Code == types.ErrInvalidEnvironmentVariables && t.Getenv("CNI_COMMAND") == "" && about != "" { + _, _ = fmt.Fprintln(t.Stderr, about) return nil } - return createTypedError(err.Error()) + return err } if cmd != "VERSION" { err = validateConfig(cmdArgs.StdinData) if err != nil { - return createTypedError(err.Error()) + return err } } @@ -222,45 +226,37 @@ func (t *dispatcher) pluginMain(cmdAdd, cmdCheck, cmdDel func(_ *CmdArgs) error, case "CHECK": configVersion, err := t.ConfVersionDecoder.Decode(cmdArgs.StdinData) if err != nil { - return createTypedError(err.Error()) + return types.NewError(types.ErrDecodingFailure, err.Error(), "") } if gtet, err := version.GreaterThanOrEqualTo(configVersion, "0.4.0"); err != nil { - return createTypedError(err.Error()) + return types.NewError(types.ErrDecodingFailure, err.Error(), "") } else if !gtet { - return &types.Error{ - Code: types.ErrIncompatibleCNIVersion, - Msg: "config version does not allow CHECK", - } + return types.NewError(types.ErrIncompatibleCNIVersion, "config version does not allow CHECK", "") } for _, pluginVersion := range versionInfo.SupportedVersions() { gtet, err := version.GreaterThanOrEqualTo(pluginVersion, configVersion) if err != nil { - return createTypedError(err.Error()) + return types.NewError(types.ErrDecodingFailure, err.Error(), "") } else if gtet { if err := t.checkVersionAndCall(cmdArgs, versionInfo, cmdCheck); err != nil { - return createTypedError(err.Error()) + return err } return nil } } - return &types.Error{ - Code: types.ErrIncompatibleCNIVersion, - Msg: "plugin version does not allow CHECK", - } + return types.NewError(types.ErrIncompatibleCNIVersion, "plugin version does not allow CHECK", "") case "DEL": err = t.checkVersionAndCall(cmdArgs, versionInfo, cmdDel) case "VERSION": - err = versionInfo.Encode(t.Stdout) + if err := versionInfo.Encode(t.Stdout); err != nil { + return types.NewError(types.ErrIOFailure, err.Error(), "") + } default: - return createTypedError("unknown CNI_COMMAND: %v", cmd) + return types.NewError(types.ErrInvalidEnvironmentVariables, fmt.Sprintf("unknown CNI_COMMAND: %v", cmd), "") } if err != nil { - if e, ok := err.(*types.Error); ok { - // don't wrap Error in Error - return e - } - return createTypedError(err.Error()) + return err } return nil } diff --git a/pkg/skel/skel_test.go b/pkg/skel/skel_test.go index 95e1b309..d1d95c25 100644 --- a/pkg/skel/skel_test.go +++ b/pkg/skel/skel_test.go @@ -95,7 +95,7 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") if isRequired { Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrInvalidEnvironmentVariables, Msg: "required env variables [" + envVar + "] missing", })) } else { @@ -143,7 +143,7 @@ var _ = Describe("dispatching to the correct callback", func() { Expect(err).To(HaveOccurred()) Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrInvalidEnvironmentVariables, Msg: "required env variables [CNI_NETNS,CNI_IFNAME,CNI_PATH] missing", })) }) @@ -236,7 +236,7 @@ var _ = Describe("dispatching to the correct callback", func() { Expect(err).To(HaveOccurred()) Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrInvalidEnvironmentVariables, Msg: "required env variables [CNI_NETNS,CNI_IFNAME,CNI_PATH] missing", })) }) @@ -269,10 +269,10 @@ var _ = Describe("dispatching to the correct callback", func() { Context("when the config has a bad version", func() { It("immediately returns a useful error", func() { - dispatch.Stdin = strings.NewReader(`{ "cniVersion": "adsfsadf", "some": "config" }`) + dispatch.Stdin = strings.NewReader(`{ "cniVersion": "adsfsadf", "some": "config", "name": "test" }`) versionInfo = version.PluginSupports("0.1.0", "0.2.0", "0.3.0") err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err.Code).To(Equal(uint(100))) + Expect(err.Code).To(Equal(uint(types.ErrDecodingFailure))) Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdCheck.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) @@ -281,10 +281,10 @@ var _ = Describe("dispatching to the correct callback", func() { Context("when the plugin has a bad version", func() { It("immediately returns a useful error", func() { - dispatch.Stdin = strings.NewReader(`{ "cniVersion": "0.4.0", "some": "config" }`) + dispatch.Stdin = strings.NewReader(`{ "cniVersion": "0.4.0", "some": "config", "name": "test" }`) versionInfo = version.PluginSupports("0.1.0", "0.2.0", "adsfasdf") err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") - Expect(err.Code).To(Equal(uint(100))) + Expect(err.Code).To(Equal(uint(types.ErrDecodingFailure))) Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdCheck.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) @@ -385,7 +385,7 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrInvalidEnvironmentVariables, Msg: "unknown CNI_COMMAND: NOPE", })) }) @@ -417,7 +417,7 @@ var _ = Describe("dispatching to the correct callback", func() { Expect(cmdAdd.CallCount).To(Equal(0)) Expect(cmdDel.CallCount).To(Equal(0)) Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrInvalidEnvironmentVariables, Msg: "required env variables [CNI_COMMAND] missing", })) }) @@ -439,7 +439,7 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrIOFailure, Msg: "error reading from stdin: banana", })) }) @@ -473,7 +473,7 @@ var _ = Describe("dispatching to the correct callback", func() { err := dispatch.pluginMain(cmdAdd.Func, cmdCheck.Func, cmdDel.Func, versionInfo, "") Expect(err).To(Equal(&types.Error{ - Code: 100, + Code: types.ErrInternal, Msg: "potato", })) }) diff --git a/pkg/types/types.go b/pkg/types/types.go index 09be6ee5..3e185c1c 100644 --- a/pkg/types/types.go +++ b/pkg/types/types.go @@ -133,9 +133,16 @@ func (r *Route) String() string { // 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 + ErrUnknown uint = iota // 0 + ErrIncompatibleCNIVersion // 1 + ErrUnsupportedField // 2 + ErrUnknownContainer // 3 + ErrInvalidEnvironmentVariables // 4 + ErrIOFailure // 5 + ErrDecodingFailure // 6 + ErrInvalidNetworkConfig // 7 + ErrTryAgainLater uint = 11 + ErrInternal uint = 999 ) type Error struct { @@ -144,6 +151,14 @@ type Error struct { Details string `json:"details,omitempty"` } +func NewError(code uint, msg, details string) *Error { + return &Error{ + Code: code, + Msg: msg, + Details: details, + } +} + func (e *Error) Error() string { details := "" if e.Details != "" { diff --git a/pkg/types/types_test.go b/pkg/types/types_test.go index b9e152bf..894cdb7c 100644 --- a/pkg/types/types_test.go +++ b/pkg/types/types_test.go @@ -137,5 +137,10 @@ var _ = Describe("Types", func() { }) }) }) + + It("NewError method", func() { + err := types.NewError(1234, "some message", "some details") + Expect(err).To(Equal(example)) + }) }) }) diff --git a/plugins/test/noop/noop_test.go b/plugins/test/noop/noop_test.go index 683d99f4..e0272739 100644 --- a/plugins/test/noop/noop_test.go +++ b/plugins/test/noop/noop_test.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/containernetworking/cni/pkg/skel" + "github.com/containernetworking/cni/pkg/types" "github.com/containernetworking/cni/pkg/version" noop_debug "github.com/containernetworking/cni/plugins/test/noop/debug" . "github.com/onsi/ginkgo" @@ -219,7 +220,7 @@ var _ = Describe("No-op plugin", func() { session, err := gexec.Start(cmd, GinkgoWriter, GinkgoWriter) Expect(err).NotTo(HaveOccurred()) Eventually(session).Should(gexec.Exit(1)) - Expect(session.Out.Contents()).To(MatchJSON(`{ "code": 100, "msg": "banana" }`)) + Expect(session.Out.Contents()).To(MatchJSON(fmt.Sprintf(`{ "code": %d, "msg": "banana" }`, types.ErrInternal))) }) })