omitempty

cmd/kubelet/app/options/options_test.go

@@ -120,7 +120,12 @@ func asArgs(fn, defaultFn func(*pflag.FlagSet)) []string {
 	defaultFn(defaults)
 	var args []string
 	fs.VisitAll(func(flag *pflag.Flag) {
+		// if the flag implements utilflag.OmitEmpty and the value is Empty, then just omit it from the command line
+		if omit, ok := flag.Value.(utilflag.OmitEmpty); ok && omit.Empty() {
+			return
+		}
 		s := flag.Value.String()
+		// if the flag has the same value as the default, we can omit it without changing the meaning of the command line
 		var defaultValue string
 		if defaultFlag := defaults.Lookup(flag.Name); defaultFlag != nil {
 			defaultValue = defaultFlag.Value.String()
@@ -128,6 +133,7 @@ func asArgs(fn, defaultFn func(*pflag.FlagSet)) []string {
 				return
 			}
 		}
+		// if the flag is a string slice, each element is specified with an independent flag invocation
 		if values, err := fs.GetStringSlice(flag.Name); err == nil {
 			for _, s := range values {
 				args = append(args, fmt.Sprintf("--%s=%s", flag.Name, s))

staging/src/k8s.io/apiserver/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",
     ],

staging/src/k8s.io/apiserver/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
+}

staging/src/k8s.io/apiserver/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)
+			}
+		})
+	}
+}

staging/src/k8s.io/apiserver/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
+}

staging/src/k8s.io/apiserver/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)
+			}
+		})
+	}
+}

staging/src/k8s.io/apiserver/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
+}

staging/src/k8s.io/apiserver/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)
+			}
+		})
+	}
+}

staging/src/k8s.io/apiserver/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
+}