invoke: if Result CNIVersion is empty use netconf CNIVersion

The CNI Spec requires plugins to return a CNIVersion in the Result
that is the same as the CNIVersion of the incoming CNI config.

Empty CNIVersions are specified to map to 0.1.0 (since the first
CNI spec didn't have versioning) and libcni handles that automatically.

Unfortunately some versions of Weave don't do that and depend on
a libcni <= 0.8 quirk that used the netconf version if the result
version was empty. This is technically a libcni regression, though
the plugin itself is violating the specification.

Thus for backwards compatibility assume that if the Result
CNIVersion is empty, the netconf CNIVersion should be used as
the Result version.

Fixes: #895

Signed-off-by: Dan Williams <[email protected]>

Author: dcbw

Diff for: 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 {

Diff for: 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() {