From d1276f1ed4bf8974117e430b9c4f1adc836edf7a Mon Sep 17 00:00:00 2001
From: Tomofumi Hayashi <tohayash@redhat.com>
Date: Tue, 31 Oct 2023 16:03:09 +0900
Subject: [PATCH] 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 <tohayash@redhat.com>
---
 libcni/api_test.go     |  8 +++-----
 pkg/types/100/types.go | 23 +++++++++++++++++++++++
 pkg/types/types.go     | 38 +++++++++++++++++++++++++++++++++++++-
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/libcni/api_test.go b/libcni/api_test.go
index 1b441e24..26852f39 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()
diff --git a/pkg/types/100/types.go b/pkg/types/100/types.go
index 6c138568..d6d069f5 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 6e6c7ac7..58b57947 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