Merge pull request #896 from dcbw/empty-result-version-regression

invoke: if Result CNIVersion is empty use netconf CNIVersion

Author: mccv1r0

pkg/invoke/exec.go

@@ -16,6 +16,7 @@ package invoke
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 
@@ -33,6 +34,43 @@ type Exec interface {
 	Decode(jsonBytes []byte) (version.PluginInfo, error)
 }
 
+// Plugin must return result in same version as specified in netconf; but
+// for backwards compatibility reasons if the result version is empty use
+// config version (rather than technically correct 0.1.0).
+// https://github.com/containernetworking/cni/issues/895
+func fixupResultVersion(netconf, result []byte) (string, []byte, error) {
+	versionDecoder := &version.ConfigDecoder{}
+	confVersion, err := versionDecoder.Decode(netconf)
+	if err != nil {
+		return "", nil, err
+	}
+
+	var rawResult map[string]interface{}
+	if err := json.Unmarshal(result, &rawResult); err != nil {
+		return "", nil, fmt.Errorf("failed to unmarshal raw result: %w", err)
+	}
+
+	// Manually decode Result version; we need to know whether its cniVersion
+	// is empty, while built-in decoders (correctly) substitute 0.1.0 for an
+	// empty version per the CNI spec.
+	if resultVerRaw, ok := rawResult["cniVersion"]; ok {
+		resultVer, ok := resultVerRaw.(string)
+		if ok && resultVer != "" {
+			return resultVer, result, nil
+		}
+	}
+
+	// If the cniVersion is not present or empty, assume the result is
+	// the same CNI spec version as the config
+	rawResult["cniVersion"] = confVersion
+	newBytes, err := json.Marshal(rawResult)
+	if err != nil {
+		return "", nil, fmt.Errorf("failed to remarshal fixed result: %w", err)
+	}
+
+	return confVersion, newBytes, nil
+}
+
 // For example, a testcase could pass an instance of the following fakeExec
 // object to ExecPluginWithResult() to verify the incoming stdin and environment
 // and provide a tailored response:
@@ -84,7 +122,12 @@ func ExecPluginWithResult(ctx context.Context, pluginPath string, netconf []byte
 		return nil, err
 	}
 
-	return create.CreateFromBytes(stdoutBytes)
+	resultVersion, fixedBytes, err := fixupResultVersion(netconf, stdoutBytes)
+	if err != nil {
+		return nil, err
+	}
+
+	return create.Create(resultVersion, fixedBytes)
 }
 
 func ExecPluginWithoutResult(ctx context.Context, pluginPath string, netconf []byte, args CNIArgs, exec Exec) error {

pkg/invoke/exec_test.go

@@ -96,6 +96,38 @@ var _ = Describe("Executing a plugin, unit tests", func() {
 			_, err := invoke.ExecPluginWithResult(ctx, pluginPath, netconf, cniargs, nil)
 			Expect(err).To(HaveOccurred())
 		})
+
+		It("assumes config version if result version is missing", func() {
+			rawExec.ExecPluginCall.Returns.ResultBytes = []byte(`{ "ips": [ { "version": "4", "address": "1.2.3.4/24" } ] }`)
+			r, err := invoke.ExecPluginWithResult(ctx, pluginPath, netconf, cniargs, pluginExec)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(r.Version()).To(Equal("0.3.1"))
+
+			result, err := current.GetResult(r)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(result.IPs)).To(Equal(1))
+			Expect(result.IPs[0].Address.IP.String()).To(Equal("1.2.3.4"))
+		})
+
+		It("assumes config version if result version is empty", func() {
+			rawExec.ExecPluginCall.Returns.ResultBytes = []byte(`{ "cniVersion": "", "ips": [ { "version": "4", "address": "1.2.3.4/24" } ] }`)
+			r, err := invoke.ExecPluginWithResult(ctx, pluginPath, netconf, cniargs, pluginExec)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(r.Version()).To(Equal("0.3.1"))
+
+			result, err := current.GetResult(r)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(result.IPs)).To(Equal(1))
+			Expect(result.IPs[0].Address.IP.String()).To(Equal("1.2.3.4"))
+		})
+
+		It("assumes 0.1.0 if config and result version are empty", func() {
+			netconf = []byte(`{ "some": "stdin" }`)
+			rawExec.ExecPluginCall.Returns.ResultBytes = []byte(`{ "some": "version-info" }`)
+			r, err := invoke.ExecPluginWithResult(ctx, pluginPath, netconf, cniargs, pluginExec)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(r.Version()).To(Equal("0.1.0"))
+		})
 	})
 
 	Describe("without returning a result", func() {