Skip to content

Commit

Permalink
invoke: if Result CNIVersion is empty use netconf CNIVersion
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
dcbw committed May 25, 2022
1 parent 940e662 commit 55fe94e
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 1 deletion.
45 changes: 44 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 (
"context"
"encoding/json"
"fmt"
"os"

Expand All @@ -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:
Expand Down Expand Up @@ -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 {
Expand Down
32 changes: 32 additions & 0 deletions pkg/invoke/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 55fe94e

Please sign in to comment.