From 30ab80517f4b8eee5e8f2cf111ecb0df37edd23a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 14 Jun 2017 22:16:30 -0500 Subject: [PATCH] types: fix marshalling of omitted "interfaces" key in IPConfig JSON Plugins that don't have knowledge of interfaces, like host-local or other IPAM plugins, should not set the 'interfaces' key of their returned "Result" JSON. This should then not be translated into an interface index of 0, which it was due to the int marshaling and omitempty. Instead, ensure that an omitted 'interface' in JSON ends up being nil in the IPConfig structure, and that a nil ensures that no 'interfaces' key is present in the JSON. Yes, this means that plugins must call the 'current.Int(x)' method when setting the Interfaces member. Oh well. Fixes: https://github.com/containernetworking/cni/issues/404 --- pkg/types/current/types.go | 24 +++++++++------- pkg/types/current/types_test.go | 49 +++++++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/pkg/types/current/types.go b/pkg/types/current/types.go index b5715fe..caac92b 100644 --- a/pkg/types/current/types.go +++ b/pkg/types/current/types.go @@ -70,10 +70,9 @@ func convertFrom020(result types.Result) (*Result, error) { if oldResult.IP4 != nil { newResult.IPs = append(newResult.IPs, &IPConfig{ - Version: "4", - Interface: -1, - Address: oldResult.IP4.IP, - Gateway: oldResult.IP4.Gateway, + Version: "4", + Address: oldResult.IP4.IP, + Gateway: oldResult.IP4.Gateway, }) for _, route := range oldResult.IP4.Routes { gw := route.GW @@ -89,10 +88,9 @@ func convertFrom020(result types.Result) (*Result, error) { if oldResult.IP6 != nil { newResult.IPs = append(newResult.IPs, &IPConfig{ - Version: "6", - Interface: -1, - Address: oldResult.IP6.IP, - Gateway: oldResult.IP6.Gateway, + Version: "6", + Address: oldResult.IP6.IP, + Gateway: oldResult.IP6.Gateway, }) for _, route := range oldResult.IP6.Routes { gw := route.GW @@ -249,12 +247,18 @@ func (i *Interface) String() string { return fmt.Sprintf("%+v", *i) } +// Int returns a pointer to the int value passed in. Used to +// set the IPConfig.Interface field. +func Int(v int) *int { + return &v +} + // IPConfig contains values necessary to configure an IP address on an interface type IPConfig struct { // IP version, either "4" or "6" Version string // Index into Result structs Interfaces list - Interface int + Interface *int Address net.IPNet Gateway net.IP } @@ -266,7 +270,7 @@ func (i *IPConfig) String() string { // JSON (un)marshallable types type ipConfig struct { Version string `json:"version"` - Interface int `json:"interface,omitempty"` + Interface *int `json:"interface,omitempty"` Address types.IPNet `json:"address"` Gateway net.IP `json:"gateway,omitempty"` } diff --git a/pkg/types/current/types_test.go b/pkg/types/current/types_test.go index afc6867..f2b1066 100644 --- a/pkg/types/current/types_test.go +++ b/pkg/types/current/types_test.go @@ -15,6 +15,7 @@ package current_test import ( + "encoding/json" "io/ioutil" "net" "os" @@ -58,13 +59,13 @@ func testResult() *current.Result { IPs: []*current.IPConfig{ { Version: "4", - Interface: 0, + Interface: current.Int(0), Address: *ipv4, Gateway: net.ParseIP("1.2.3.1"), }, { Version: "6", - Interface: 0, + Interface: current.Int(0), Address: *ipv6, Gateway: net.ParseIP("abcd:1234:ffff::1"), }, @@ -86,8 +87,6 @@ var _ = Describe("Current types operations", func() { It("correctly encodes a 0.3.x Result", func() { res := testResult() - Expect(res.String()).To(Equal("Interfaces:[{Name:eth0 Mac:00:11:22:33:44:55 Sandbox:/proc/3553/ns/net}], IP:[{Version:4 Interface:0 Address:{IP:1.2.3.30 Mask:ffffff00} Gateway:1.2.3.1} {Version:6 Interface:0 Address:{IP:abcd:1234:ffff::cdde Mask:ffffffffffffffff0000000000000000} Gateway:abcd:1234:ffff::1}], Routes:[{Dst:{IP:15.5.6.0 Mask:ffffff00} GW:15.5.6.8} {Dst:{IP:1111:dddd:: Mask:ffffffffffffffffffff000000000000} GW:1111:dddd::aaaa}], DNS:{Nameservers:[1.2.3.4 1::cafe] Domain:acompany.com Search:[somedomain.com otherdomain.net] Options:[foo bar]}")) - // Redirect stdout to capture JSON result oldStdout := os.Stdout r, w, err := os.Pipe() @@ -103,7 +102,7 @@ var _ = Describe("Current types operations", func() { os.Stdout = oldStdout Expect(err).NotTo(HaveOccurred()) - Expect(string(out)).To(Equal(`{ + Expect(string(out)).To(MatchJSON(`{ "cniVersion": "0.3.1", "interfaces": [ { @@ -115,11 +114,13 @@ var _ = Describe("Current types operations", func() { "ips": [ { "version": "4", + "interface": 0, "address": "1.2.3.30/24", "gateway": "1.2.3.1" }, { "version": "6", + "interface": 0, "address": "abcd:1234:ffff::cdde/64", "gateway": "abcd:1234:ffff::1" } @@ -173,7 +174,7 @@ var _ = Describe("Current types operations", func() { os.Stdout = oldStdout Expect(err).NotTo(HaveOccurred()) - Expect(string(out)).To(Equal(`{ + Expect(string(out)).To(MatchJSON(`{ "cniVersion": "0.2.0", "ip4": { "ip": "1.2.3.30/24", @@ -210,6 +211,42 @@ var _ = Describe("Current types operations", func() { "bar" ] } +}`)) + }) + + It("correctly marshals interface index 0", func() { + ipc := ¤t.IPConfig{ + Version: "4", + Interface: current.Int(0), + Address: net.IPNet{ + IP: net.ParseIP("10.1.2.3"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + } + + json, err := json.Marshal(ipc) + Expect(err).NotTo(HaveOccurred()) + Expect(json).To(MatchJSON(`{ + "version": "4", + "interface": 0, + "address": "10.1.2.3/24" +}`)) + }) + + It("correctly marshals a missing interface index", func() { + ipc := ¤t.IPConfig{ + Version: "4", + Address: net.IPNet{ + IP: net.ParseIP("10.1.2.3"), + Mask: net.IPv4Mask(255, 255, 255, 0), + }, + } + + json, err := json.Marshal(ipc) + Expect(err).NotTo(HaveOccurred()) + Expect(json).To(MatchJSON(`{ + "version": "4", + "address": "10.1.2.3/24" }`)) }) }) -- 2.44.0