Add Marshal function in Result/NetConf to omit empty value

This change fix to avoid empty DNS field in NetConf/Result if DNS is empty. Signed-off-by: Tomofumi Hayashi <[email protected]>
containernetworking · Nov 1, 2023 · b4cc410 · b4cc410
1 parent 5968d61
commit b4cc410
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 6 deletions.
diff --git a/libcni/api_test.go b/libcni/api_test.go
@@ -486,8 +486,7 @@ var _ = Describe("Invoking plugins", func() {
 				Expect(err).NotTo(HaveOccurred())
 				cachedJson := `{
 					"cniVersion": "` + version.Current() + `",
-					"ips": [{"address": "10.1.2.3/24"}],
-					"dns": {}
+					"ips": [{"address": "10.1.2.3/24"}]
 				}`
 				err = os.WriteFile(cacheFile, []byte(cachedJson), 0o600)
 				Expect(err).NotTo(HaveOccurred())
@@ -676,8 +675,7 @@ var _ = Describe("Invoking plugins", func() {
 				Expect(err).NotTo(HaveOccurred())
 				cachedJson := `{
 					"cniVersion": "` + version.Current() + `",
-					"ips": [{"address": "10.1.2.3/24"}],
-					"dns": {}
+					"ips": [{"address": "10.1.2.3/24"}]
 				}`
 				err = os.WriteFile(cacheFile, []byte(cachedJson), 0o600)
 				Expect(err).NotTo(HaveOccurred())
@@ -971,7 +969,7 @@ var _ = Describe("Invoking plugins", func() {
 				"otherCapability": capabilityArgs["otherCapability"],
 			}
 
-			ipResult = fmt.Sprintf(`{"cniVersion": "%s", "dns":{},"ips":[{"address": "10.1.2.3/24"}]}`, version.Current())
+			ipResult = fmt.Sprintf(`{"cniVersion": "%s", "ips":[{"address": "10.1.2.3/24"}]}`, version.Current())
 			netConfigList, plugins = makePluginList(version.Current(), ipResult, rcMap)
 
 			ctx = context.TODO()

diff --git a/pkg/types/100/types.go b/pkg/types/100/types.go
@@ -15,6 +15,7 @@
 package types100
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -95,6 +96,61 @@ type Result struct {
 	DNS        types.DNS      `json:"dns,omitempty"`
 }
 
+// Note: DNS should be omit if DNS is empty but default Marshal function
+// will output empty structure hence need to write a Marshal function
+func (r *Result) MarshalJSON() ([]byte, error) {
+	var buf bytes.Buffer
+	buf.WriteString("{")
+	fieldSeparator := ""
+
+	if r.CNIVersion != "" {
+		buf.WriteString(fmt.Sprintf("\"cniVersion\":%q", r.CNIVersion))
+		fieldSeparator = ","
+	}
+
+	if len(r.Interfaces) != 0 {
+		buf.WriteString(fmt.Sprintf("%s\"interfaces\":", fieldSeparator))
+		b, err := json.Marshal(r.Interfaces)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+		fieldSeparator = ","
+	}
+
+	if len(r.IPs) != 0 {
+		buf.WriteString(fmt.Sprintf("%s\"ips\":", fieldSeparator))
+		b, err := json.Marshal(r.IPs)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+		fieldSeparator = ","
+	}
+
+	if len(r.Routes) != 0 {
+		buf.WriteString(fmt.Sprintf("%s\"routes\":", fieldSeparator))
+		b, err := json.Marshal(r.Routes)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+		fieldSeparator = ","
+	}
+
+	if !r.DNS.IsEmpty() {
+		buf.WriteString(fmt.Sprintf("%s\"dns\":", fieldSeparator))
+		b, err := json.Marshal(r.DNS)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+	}
+	buf.WriteString("}")
+
+	return buf.Bytes(), nil
+}
+
 // convertFrom100 does nothing except set the version; the types are the same
 func convertFrom100(from types.Result, toVersion string) (types.Result, error) {
 	fromResult := from.(*Result)

diff --git a/pkg/types/types.go b/pkg/types/types.go
@@ -15,6 +15,7 @@
 package types
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -64,16 +65,86 @@ type NetConf struct {
 	Type         string          `json:"type,omitempty"`
 	Capabilities map[string]bool `json:"capabilities,omitempty"`
 	IPAM         IPAM            `json:"ipam,omitempty"`
-	DNS          DNS             `json:"dns"`
+	DNS          DNS             `json:"dns,omitempty"`
 
 	RawPrevResult map[string]interface{} `json:"prevResult,omitempty"`
 	PrevResult    Result                 `json:"-"`
 }
 
+// Note: DNS should be omit if DNS is empty but default Marshal function
+// will output empty structure hence need to write a Marshal function
+func (n *NetConf) MarshalJSON() ([]byte, error) {
+	var buf bytes.Buffer
+	buf.WriteString("{")
+	fieldSeparator := ""
+
+	if n.CNIVersion != "" {
+		buf.WriteString(fmt.Sprintf("\"cniVersion\":%q", n.CNIVersion))
+		fieldSeparator = ","
+	}
+
+	if n.Name != "" {
+		buf.WriteString(fmt.Sprintf("%s\"name\":%q", fieldSeparator, n.Name))
+		fieldSeparator = ","
+	}
+
+	if n.Type != "" {
+		buf.WriteString(fmt.Sprintf("%s\"type\":%q", fieldSeparator, n.Type))
+		fieldSeparator = ","
+	}
+
+	if len(n.Capabilities) != 0 {
+		buf.WriteString(fmt.Sprintf("%s\"capabilities\":", fieldSeparator))
+		b, err := json.Marshal(n.Capabilities)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+		fieldSeparator = ","
+	}
+
+	if !n.IPAM.IsEmpty() {
+		buf.WriteString(fmt.Sprintf("%s\"ipam\":", fieldSeparator))
+		b, err := json.Marshal(n.IPAM)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+		fieldSeparator = ","
+	}
+
+	if !n.DNS.IsEmpty() {
+		buf.WriteString(fmt.Sprintf("%s\"dns\":", fieldSeparator))
+		b, err := json.Marshal(n.DNS)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+		fieldSeparator = ","
+	}
+
+	if len(n.RawPrevResult) != 0 {
+		buf.WriteString(fmt.Sprintf("%s\"prevResult\":", fieldSeparator))
+		b, err := json.Marshal(n.RawPrevResult)
+		if err != nil {
+			return nil, err
+		}
+		buf.Write(b)
+	}
+	buf.WriteString("}")
+
+	return buf.Bytes(), nil
+}
+
 type IPAM struct {
 	Type string `json:"type,omitempty"`
 }
 
+// IsEmpty returns true if IPAM structure has no value, otherwise return false
+func (i *IPAM) IsEmpty() bool {
+	return i.Type == ""
+}
+
 // NetConfList describes an ordered list of networks.
 type NetConfList struct {
 	CNIVersion string `json:"cniVersion,omitempty"`
@@ -116,6 +187,14 @@ type DNS struct {
 	Options     []string `json:"options,omitempty"`
 }
 
+// IsEmpty returns true if DNS structure has no value, otherwise return false
+func (d *DNS) IsEmpty() bool {
+	if len(d.Nameservers) == 0 && d.Domain == "" && len(d.Search) == 0 && len(d.Options) == 0 {
+		return true
+	}
+	return false
+}
+
 func (d *DNS) Copy() *DNS {
 	if d == nil {
 		return nil