From b96eecee1644224674119782e4c810deb24a2ba2 Mon Sep 17 00:00:00 2001
From: Michael Taufen <mtaufen@google.com>
Date: Thu, 16 Nov 2017 14:47:39 -0800
Subject: [PATCH] omitempty

Kubernetes-commit: 617b49858fad83a72ce072356bb2808a259943c5
---
 pkg/util/flag/BUILD                           |  1 +
 .../langle_separated_map_string_string.go     | 15 ++++------
 ...langle_separated_map_string_string_test.go | 29 ++++++++++++++-----
 pkg/util/flag/map_string_bool.go              | 15 ++++------
 pkg/util/flag/map_string_bool_test.go         | 29 ++++++++++++++-----
 pkg/util/flag/map_string_string.go            | 15 ++++------
 pkg/util/flag/map_string_string_test.go       | 29 ++++++++++++++-----
 pkg/util/flag/omitempty.go                    | 24 +++++++++++++++
 8 files changed, 106 insertions(+), 51 deletions(-)
 create mode 100644 pkg/util/flag/omitempty.go

diff --git a/pkg/util/flag/BUILD b/pkg/util/flag/BUILD
index 9cf2715c6..57ab4058d 100644
--- a/pkg/util/flag/BUILD
+++ b/pkg/util/flag/BUILD
@@ -30,6 +30,7 @@ go_library(
         "map_string_bool.go",
         "map_string_string.go",
         "namedcertkey_flag.go",
+        "omitempty.go",
         "string_flag.go",
         "tristate.go",
     ],
diff --git a/pkg/util/flag/langle_separated_map_string_string.go b/pkg/util/flag/langle_separated_map_string_string.go
index c2b950e77..bf8dbfb9b 100644
--- a/pkg/util/flag/langle_separated_map_string_string.go
+++ b/pkg/util/flag/langle_separated_map_string_string.go
@@ -38,10 +38,6 @@ func NewLangleSeparatedMapStringString(m *map[string]string) *LangleSeparatedMap
 
 // String implements github.com/spf13/pflag.Value
 func (m *LangleSeparatedMapStringString) String() string {
-	if *m.Map == nil {
-		return "nil"
-	}
-
 	pairs := []string{}
 	for k, v := range *m.Map {
 		pairs = append(pairs, fmt.Sprintf("%s<%s", k, v))
@@ -55,12 +51,6 @@ func (m *LangleSeparatedMapStringString) Set(value string) error {
 	if m.Map == nil {
 		return fmt.Errorf("no target (nil pointer to map[string]string)")
 	}
-	// allow explicit nil map
-	if value == "nil" {
-		*m.Map = nil
-		m.initialized = true
-		return nil
-	}
 	if !m.initialized || *m.Map == nil {
 		// clear default values, or allocate if no existing map
 		*m.Map = make(map[string]string)
@@ -85,3 +75,8 @@ func (m *LangleSeparatedMapStringString) Set(value string) error {
 func (*LangleSeparatedMapStringString) Type() string {
 	return "mapStringString"
 }
+
+// Empty implements OmitEmpty
+func (m *LangleSeparatedMapStringString) Empty() bool {
+	return len(*m.Map) == 0
+}
diff --git a/pkg/util/flag/langle_separated_map_string_string_test.go b/pkg/util/flag/langle_separated_map_string_string_test.go
index fd2ca25a5..80c67c32b 100644
--- a/pkg/util/flag/langle_separated_map_string_string_test.go
+++ b/pkg/util/flag/langle_separated_map_string_string_test.go
@@ -28,7 +28,7 @@ func TestStringLangleSeparatedMapStringString(t *testing.T) {
 		m      *LangleSeparatedMapStringString
 		expect string
 	}{
-		{"nil", NewLangleSeparatedMapStringString(&nilMap), "nil"},
+		{"nil", NewLangleSeparatedMapStringString(&nilMap), ""},
 		{"empty", NewLangleSeparatedMapStringString(&map[string]string{}), ""},
 		{"one key", NewLangleSeparatedMapStringString(&map[string]string{"one": "foo"}), "one<foo"},
 		{"two keys", NewLangleSeparatedMapStringString(&map[string]string{"one": "foo", "two": "bar"}), "one<foo,two<bar"},
@@ -59,12 +59,6 @@ func TestSetLangleSeparatedMapStringString(t *testing.T) {
 				initialized: true,
 				Map:         &map[string]string{},
 			}, ""},
-		{"explicitly nil", []string{"nil"},
-			NewLangleSeparatedMapStringString(&map[string]string{"default": ""}),
-			&LangleSeparatedMapStringString{
-				initialized: true,
-				Map:         &nilMap,
-			}, ""},
 		// make sure we still allocate for "initialized" maps where Map was initially set to a nil map
 		{"allocates map if currently nil", []string{""},
 			&LangleSeparatedMapStringString{initialized: true, Map: &nilMap},
@@ -142,3 +136,24 @@ func TestSetLangleSeparatedMapStringString(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyLangleSeparatedMapStringString(t *testing.T) {
+	var nilMap map[string]string
+	cases := []struct {
+		desc   string
+		val    *LangleSeparatedMapStringString
+		expect bool
+	}{
+		{"nil", NewLangleSeparatedMapStringString(&nilMap), true},
+		{"empty", NewLangleSeparatedMapStringString(&map[string]string{}), true},
+		{"populated", NewLangleSeparatedMapStringString(&map[string]string{"foo": ""}), false},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			result := c.val.Empty()
+			if result != c.expect {
+				t.Fatalf("expect %t but got %t", c.expect, result)
+			}
+		})
+	}
+}
diff --git a/pkg/util/flag/map_string_bool.go b/pkg/util/flag/map_string_bool.go
index 73410d9c9..46ed43836 100644
--- a/pkg/util/flag/map_string_bool.go
+++ b/pkg/util/flag/map_string_bool.go
@@ -39,10 +39,6 @@ func NewMapStringBool(m *map[string]bool) *MapStringBool {
 
 // String implements github.com/spf13/pflag.Value
 func (m *MapStringBool) String() string {
-	if *m.Map == nil {
-		return "nil"
-	}
-
 	pairs := []string{}
 	for k, v := range *m.Map {
 		pairs = append(pairs, fmt.Sprintf("%s=%t", k, v))
@@ -56,12 +52,6 @@ func (m *MapStringBool) Set(value string) error {
 	if m.Map == nil {
 		return fmt.Errorf("no target (nil pointer to map[string]bool)")
 	}
-	// allow explicit nil map
-	if value == "nil" {
-		*m.Map = nil
-		m.initialized = true
-		return nil
-	}
 	if !m.initialized || *m.Map == nil {
 		// clear default values, or allocate if no existing map
 		*m.Map = make(map[string]bool)
@@ -90,3 +80,8 @@ func (m *MapStringBool) Set(value string) error {
 func (*MapStringBool) Type() string {
 	return "mapStringBool"
 }
+
+// Empty implements OmitEmpty
+func (m *MapStringBool) Empty() bool {
+	return len(*m.Map) == 0
+}
diff --git a/pkg/util/flag/map_string_bool_test.go b/pkg/util/flag/map_string_bool_test.go
index 80e0ce597..f23b240a1 100644
--- a/pkg/util/flag/map_string_bool_test.go
+++ b/pkg/util/flag/map_string_bool_test.go
@@ -28,7 +28,7 @@ func TestStringMapStringBool(t *testing.T) {
 		m      *MapStringBool
 		expect string
 	}{
-		{"nil", NewMapStringBool(&nilMap), "nil"},
+		{"nil", NewMapStringBool(&nilMap), ""},
 		{"empty", NewMapStringBool(&map[string]bool{}), ""},
 		{"one key", NewMapStringBool(&map[string]bool{"one": true}), "one=true"},
 		{"two keys", NewMapStringBool(&map[string]bool{"one": true, "two": false}), "one=true,two=false"},
@@ -59,12 +59,6 @@ func TestSetMapStringBool(t *testing.T) {
 				initialized: true,
 				Map:         &map[string]bool{},
 			}, ""},
-		{"explicitly nil", []string{"nil"},
-			NewMapStringBool(&map[string]bool{"default": true}),
-			&MapStringBool{
-				initialized: true,
-				Map:         &nilMap,
-			}, ""},
 		// make sure we still allocate for "initialized" maps where Map was initially set to a nil map
 		{"allocates map if currently nil", []string{""},
 			&MapStringBool{initialized: true, Map: &nilMap},
@@ -146,3 +140,24 @@ func TestSetMapStringBool(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyMapStringBool(t *testing.T) {
+	var nilMap map[string]bool
+	cases := []struct {
+		desc   string
+		val    *MapStringBool
+		expect bool
+	}{
+		{"nil", NewMapStringBool(&nilMap), true},
+		{"empty", NewMapStringBool(&map[string]bool{}), true},
+		{"populated", NewMapStringBool(&map[string]bool{"foo": true}), false},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			result := c.val.Empty()
+			if result != c.expect {
+				t.Fatalf("expect %t but got %t", c.expect, result)
+			}
+		})
+	}
+}
diff --git a/pkg/util/flag/map_string_string.go b/pkg/util/flag/map_string_string.go
index 85d36f8a9..48468a8dd 100644
--- a/pkg/util/flag/map_string_string.go
+++ b/pkg/util/flag/map_string_string.go
@@ -38,10 +38,6 @@ func NewMapStringString(m *map[string]string) *MapStringString {
 
 // String implements github.com/spf13/pflag.Value
 func (m *MapStringString) String() string {
-	if *m.Map == nil {
-		return "nil"
-	}
-
 	pairs := []string{}
 	for k, v := range *m.Map {
 		pairs = append(pairs, fmt.Sprintf("%s=%s", k, v))
@@ -55,12 +51,6 @@ func (m *MapStringString) Set(value string) error {
 	if m.Map == nil {
 		return fmt.Errorf("no target (nil pointer to map[string]string)")
 	}
-	// allow explicit nil map
-	if value == "nil" {
-		*m.Map = nil
-		m.initialized = true
-		return nil
-	}
 	if !m.initialized || *m.Map == nil {
 		// clear default values, or allocate if no existing map
 		*m.Map = make(map[string]string)
@@ -85,3 +75,8 @@ func (m *MapStringString) Set(value string) error {
 func (*MapStringString) Type() string {
 	return "mapStringString"
 }
+
+// Empty implements OmitEmpty
+func (m *MapStringString) Empty() bool {
+	return len(*m.Map) == 0
+}
diff --git a/pkg/util/flag/map_string_string_test.go b/pkg/util/flag/map_string_string_test.go
index b71777a8a..8feb62524 100644
--- a/pkg/util/flag/map_string_string_test.go
+++ b/pkg/util/flag/map_string_string_test.go
@@ -28,7 +28,7 @@ func TestStringMapStringString(t *testing.T) {
 		m      *MapStringString
 		expect string
 	}{
-		{"nil", NewMapStringString(&nilMap), "nil"},
+		{"nil", NewMapStringString(&nilMap), ""},
 		{"empty", NewMapStringString(&map[string]string{}), ""},
 		{"one key", NewMapStringString(&map[string]string{"one": "foo"}), "one=foo"},
 		{"two keys", NewMapStringString(&map[string]string{"one": "foo", "two": "bar"}), "one=foo,two=bar"},
@@ -59,12 +59,6 @@ func TestSetMapStringString(t *testing.T) {
 				initialized: true,
 				Map:         &map[string]string{},
 			}, ""},
-		{"explicitly nil", []string{"nil"},
-			NewMapStringString(&map[string]string{"default": ""}),
-			&MapStringString{
-				initialized: true,
-				Map:         &nilMap,
-			}, ""},
 		// make sure we still allocate for "initialized" maps where Map was initially set to a nil map
 		{"allocates map if currently nil", []string{""},
 			&MapStringString{initialized: true, Map: &nilMap},
@@ -142,3 +136,24 @@ func TestSetMapStringString(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyMapStringString(t *testing.T) {
+	var nilMap map[string]string
+	cases := []struct {
+		desc   string
+		val    *MapStringString
+		expect bool
+	}{
+		{"nil", NewMapStringString(&nilMap), true},
+		{"empty", NewMapStringString(&map[string]string{}), true},
+		{"populated", NewMapStringString(&map[string]string{"foo": ""}), false},
+	}
+	for _, c := range cases {
+		t.Run(c.desc, func(t *testing.T) {
+			result := c.val.Empty()
+			if result != c.expect {
+				t.Fatalf("expect %t but got %t", c.expect, result)
+			}
+		})
+	}
+}
diff --git a/pkg/util/flag/omitempty.go b/pkg/util/flag/omitempty.go
new file mode 100644
index 000000000..c354754ea
--- /dev/null
+++ b/pkg/util/flag/omitempty.go
@@ -0,0 +1,24 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package flag
+
+// OmitEmpty is an interface for flags to report whether their underlying value
+// is "empty." If a flag implements OmitEmpty and returns true for a call to Empty(),
+// it is assumed that flag may be omitted from the command line.
+type OmitEmpty interface {
+	Empty() bool
+}