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 20, 2023 · 4ebc03d · 4ebc03d
1 parent 93878e6
commit 4ebc03d
Show file tree

Hide file tree

Showing 3 changed files with 63 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
@@ -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
@@ -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