types: fix marshalling of omitted "interfaces" key in IPConfig JSON
authorDan Williams <dcbw@redhat.com>
Thu, 15 Jun 2017 03:16:30 +0000 (22:16 -0500)
committerDan Williams <dcbw@redhat.com>
Wed, 21 Jun 2017 14:56:12 +0000 (09:56 -0500)
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
pkg/types/current/types_test.go

index b5715fe..caac92b 100644 (file)
@@ -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"`
 }
index afc6867..f2b1066 100644 (file)
@@ -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 := &current.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 := &current.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"
 }`))
        })
 })