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
-1 in the IPConfig structure, and that a -1 ensures that no 'interfaces'
key is present in the JSON.

Yes, this means that you must set Interface:-1 in IPConfig in your
plugin, but if you weren't already doing that and your plugin didn't
deal with interface indexes, you already had a problem.

Fixes: #404

libcni/api_test.go

@@ -300,7 +300,8 @@ var _ = Describe("Invoking plugins", func() {
 					CNIVersion: current.ImplementedSpecVersion,
 					IPs: []*current.IPConfig{
 						{
-							Version: "4",
+							Version:   "4",
+							Interface: -1,
 							Address: net.IPNet{
 								IP:   net.ParseIP("10.1.2.3"),
 								Mask: net.IPv4Mask(255, 255, 255, 0),
@@ -484,7 +485,8 @@ var _ = Describe("Invoking plugins", func() {
 					// IP4 added by first plugin
 					IPs: []*current.IPConfig{
 						{
-							Version: "4",
+							Version:   "4",
+							Interface: -1,
 							Address: net.IPNet{
 								IP:   net.ParseIP("10.1.2.3"),
 								Mask: net.IPv4Mask(255, 255, 255, 0),

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
@@ -253,7 +251,7 @@ func (i *Interface) String() string {
 type IPConfig struct {
 	// IP version, either "4" or "6"
 	Version string
-	// Index into Result structs Interfaces list
+	// Index into Result structs Interfaces list; -1 if no interface is given
 	Interface int
 	Address   net.IPNet
 	Gateway   net.IP
@@ -266,17 +264,19 @@ 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"`
 }
 
 func (c *IPConfig) MarshalJSON() ([]byte, error) {
 	ipc := ipConfig{
-		Version:   c.Version,
-		Interface: c.Interface,
-		Address:   types.IPNet(c.Address),
-		Gateway:   c.Gateway,
+		Version: c.Version,
+		Address: types.IPNet(c.Address),
+		Gateway: c.Gateway,
+	}
+	if c.Interface >= 0 {
+		ipc.Interface = &c.Interface
 	}
 
 	return json.Marshal(ipc)
@@ -289,7 +289,10 @@ func (c *IPConfig) UnmarshalJSON(data []byte) error {
 	}
 
 	c.Version = ipc.Version
-	c.Interface = ipc.Interface
+	c.Interface = -1
+	if ipc.Interface != nil {
+		c.Interface = *ipc.Interface
+	}
 	c.Address = net.IPNet(ipc.Address)
 	c.Gateway = ipc.Gateway
 	return nil

pkg/types/current/types_test.go

@@ -15,6 +15,7 @@
 package current_test
 
 import (
+	"encoding/json"
 	"io/ioutil"
 	"net"
 	"os"
@@ -103,7 +104,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 +116,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 +176,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 +213,43 @@ var _ = Describe("Current types operations", func() {
             "bar"
         ]
     }
+}`))
+	})
+
+	It("correctly marshals interface index 0", func() {
+		ipc := &current.IPConfig{
+			Version:   "4",
+			Interface: 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",
+			Interface: -1,
+			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"
 }`))
 	})
 })