Repository: containernetworking/cni
Pull request #1039 (2023-10-31 07:05:53)

Add Marshal function in Result/NetConf to omit empty value
This change fix to avoid empty DNS/IPAM field in NetConf/Result if DNS is empty.

s1061123: This will oversede #1035 and #1007 
coveralls: 
[![Coverage Status](https://coveralls.io/builds/63673662/badge)](https://coveralls.io/builds/63673662)

coverage: 69.97% (-1.6%) from 71.573% when pulling **b4cc41038a6ff519330b6111e42878740db8a85e on s1061123:json-remove-dns** into **5968d61a1e226ea5af3d89d81a0aca1dc54448a1 on containernetworking:main**.


Patch:
diff --git a/pkg/types/100/types.go b/pkg/types/100/types.go
index 6c13856..d6d069f 100644
--- a/pkg/types/100/types.go
+++ b/pkg/types/100/types.go
@@ -95,6 +95,29 @@ 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) {
+	// use type alias to escape recursion for json.Marshal() to MarshalJSON()
+	type fixObjType = Result
+
+	bytes, err := json.Marshal(fixObjType(*r)) //nolint:all
+	if err != nil {
+		return nil, err
+	}
+
+	fixupObj := make(map[string]interface{})
+	if err := json.Unmarshal(bytes, &fixupObj); err != nil {
+		return nil, err
+	}
+
+	if r.DNS.IsEmpty() {
+		delete(fixupObj, "dns")
+	}
+
+	return json.Marshal(fixupObj)
+}
+
 // 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
index 6e6c7ac..58b5794 100644
--- a/pkg/types/types.go
+++ b/pkg/types/types.go
@@ -64,16 +64,44 @@ 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) {
+	// use type alias to escape recursion for json.Marshal() to MarshalJSON()
+	type fixObjType = NetConf
+
+	bytes, err := json.Marshal(fixObjType(*n)) //nolint:all
+	if err != nil {
+		return nil, err
+	}
+
+	fixupObj := make(map[string]interface{})
+	if err := json.Unmarshal(bytes, &fixupObj); err != nil {
+		return nil, err
+	}
+
+	if n.DNS.IsEmpty() {
+		delete(fixupObj, "dns")
+	}
+
+	return json.Marshal(fixupObj)
+}
+
 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 +144,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


Test patch:
diff --git a/libcni/api_test.go b/libcni/api_test.go
index 1b441e2..26852f3 100644
--- 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()