ColonSeparatedMultimapStringString: allow multiple Set invocations with default override

The first call to Set will clear the map before adding entries;
subsequent calls will simply append to the map.
This makes it possible to override default values with a command-line
option rather than appending to defaults,
while still allowing the distribution of key-value pairs across
multiple flag invocations.

For example: `--flag "a:hello" --flag "b:again" --flag "b:beautiful"
--flag "c:world"` results in `{"a": ["hello"], "b": ["again",
"beautiful"], "c": ["world"]}`

cmd/kubelet/app/options/options.go

@@ -371,7 +371,7 @@ func AddKubeletConfigFlags(fs *pflag.FlagSet, c *kubeletconfig.KubeletConfigurat
 	fs.DurationVar(&c.FileCheckFrequency.Duration, "file-check-frequency", c.FileCheckFrequency.Duration, "Duration between checking config files for new data")
 	fs.DurationVar(&c.HTTPCheckFrequency.Duration, "http-check-frequency", c.HTTPCheckFrequency.Duration, "Duration between checking http for new data")
 	fs.StringVar(&c.ManifestURL, "manifest-url", c.ManifestURL, "URL for accessing the container manifest")
-	fs.Var(flag.ColonSeparatedMultimapStringString(c.ManifestURLHeader), "manifest-url-header", "Comma-separated list of HTTP headers to use when accessing the manifest URL. Multiple headers with the same name will be added in the same order provided. For example: `a:hello,b:again,c:world,b:beautiful`")
+	fs.Var(flag.NewColonSeparatedMultimapStringString(&c.ManifestURLHeader), "manifest-url-header", "Comma-separated list of HTTP headers to use when accessing the manifest URL. Multiple headers with the same name will be added in the same order provided. This flag can be repeatedly invoked. For example: `--manifest-url-header 'a:hello,b:again,c:world' --manifest-url-header 'b:beautiful'`")
 	fs.BoolVar(&c.EnableServer, "enable-server", c.EnableServer, "Enable the Kubelet's server")
 	fs.Var(componentconfig.IPVar{Val: &c.Address}, "address", "The IP address for the Kubelet to serve on (set to 0.0.0.0 for all interfaces)")
 	fs.Int32Var(&c.Port, "port", c.Port, "The port for the Kubelet to serve on.")

pkg/kubelet/apis/kubeletconfig/v1alpha1/defaults.go

@@ -231,9 +231,6 @@ func SetDefaults_KubeletConfiguration(obj *KubeletConfiguration) {
 	if obj.EnforceNodeAllocatable == nil {
 		obj.EnforceNodeAllocatable = defaultNodeAllocatableEnforcement
 	}
-	if obj.ManifestURLHeader == nil {
-		obj.ManifestURLHeader = make(map[string][]string)
-	}
 }
 
 func boolVar(b bool) *bool {

staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string.go

@@ -28,13 +28,36 @@ import (
 // slice of strings associated with that key. Items in the list associated with a given
 // key will appear in the order provided.
 // For example: `a:hello,b:again,c:world,b:beautiful` results in `{"a": ["hello"], "b": ["again", "beautiful"], "c": ["world"]}`
-type ColonSeparatedMultimapStringString map[string][]string
+// The first call to Set will clear the map before adding entries; subsequent calls will simply append to the map.
+// This makes it possible to override default values with a command-line option rather than appending to defaults,
+// while still allowing the distribution of key-value pairs across multiple flag invocations.
+// For example: `--flag "a:hello" --flag "b:again" --flag "b:beautiful" --flag "c:world"` results in `{"a": ["hello"], "b": ["again", "beautiful"], "c": ["world"]}`
+type ColonSeparatedMultimapStringString struct {
+	Multimap    *map[string][]string
+	initialized bool // set to true after the first Set call
+}
+
+// NewColonSeparatedMultimapStringString takes a pointer to a map[string][]string and returns the
+// ColonSeparatedMultimapStringString flag parsing shim for that map.
+func NewColonSeparatedMultimapStringString(m *map[string][]string) *ColonSeparatedMultimapStringString {
+	return &ColonSeparatedMultimapStringString{Multimap: m}
+}
 
 // Set implements github.com/spf13/pflag.Value
-func (m ColonSeparatedMultimapStringString) Set(value string) error {
+func (m *ColonSeparatedMultimapStringString) Set(value string) error {
-	// clear old values
+	if m.Multimap == nil {
-	for k := range m {
+		return fmt.Errorf("no target (nil pointer to map[string][]string)")
-		delete(m, k)
+	}
+	// allow explicit nil multimap
+	if value == "nil" {
+		*m.Multimap = nil
+		m.initialized = true
+		return nil
+	}
+	if !m.initialized || *m.Multimap == nil {
+		// clear default values, or allocate if no existing map
+		*m.Multimap = make(map[string][]string)
+		m.initialized = true
 	}
 	for _, pair := range strings.Split(value, ",") {
 		if len(pair) == 0 {
@@ -46,19 +69,23 @@ func (m ColonSeparatedMultimapStringString) Set(value string) error {
 		}
 		k := strings.TrimSpace(kv[0])
 		v := strings.TrimSpace(kv[1])
-		m[k] = append(m[k], v)
+		(*m.Multimap)[k] = append((*m.Multimap)[k], v)
 	}
 	return nil
 }
 
 // String implements github.com/spf13/pflag.Value
-func (m ColonSeparatedMultimapStringString) String() string {
+func (m *ColonSeparatedMultimapStringString) String() string {
+	if *m.Multimap == nil {
+		return "nil"
+	}
+
 	type kv struct {
 		k string
 		v string
 	}
-	kvs := make([]kv, 0, len(m))
+	kvs := make([]kv, 0, len(*m.Multimap))
-	for k, vs := range m {
+	for k, vs := range *m.Multimap {
 		for i := range vs {
 			kvs = append(kvs, kv{k: k, v: vs[i]})
 		}
@@ -75,6 +102,6 @@ func (m ColonSeparatedMultimapStringString) String() string {
 }
 
 // Type implements github.com/spf13/pflag.Value
-func (m ColonSeparatedMultimapStringString) Type() string {
+func (m *ColonSeparatedMultimapStringString) Type() string {
 	return "colonSeparatedMultimapStringString"
 }

staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string_test.go

@@ -22,17 +22,43 @@ import (
 )
 
 func TestStringColonSeparatedMultimapStringString(t *testing.T) {
+	var nilMap map[string][]string
 	cases := []struct {
 		desc   string
-		m      ColonSeparatedMultimapStringString
+		m      *ColonSeparatedMultimapStringString
 		expect string
 	}{
-		{"empty", ColonSeparatedMultimapStringString{}, ""},
+		{"nil", NewColonSeparatedMultimapStringString(&nilMap), "nil"},
-		{"empty key", ColonSeparatedMultimapStringString{"": []string{"foo"}}, ":foo"},
+		{"empty", NewColonSeparatedMultimapStringString(&map[string][]string{}), ""},
-		{"one key", ColonSeparatedMultimapStringString{"one": []string{"foo"}}, "one:foo"},
+		{"empty key", NewColonSeparatedMultimapStringString(
-		{"two keys", ColonSeparatedMultimapStringString{"one": []string{"foo"}, "two": []string{"bar"}}, "one:foo,two:bar"},
+			&map[string][]string{
-		{"two keys, multiple items in one key", ColonSeparatedMultimapStringString{"one": []string{"foo", "baz"}, "two": []string{"bar"}}, "one:foo,one:baz,two:bar"},
+				"": {"foo"},
-		{"three keys, multiple items in one key", ColonSeparatedMultimapStringString{"a": []string{"hello"}, "b": []string{"again", "beautiful"}, "c": []string{"world"}}, "a:hello,b:again,b:beautiful,c:world"},
+			}),
+			":foo"},
+		{"one key", NewColonSeparatedMultimapStringString(
+			&map[string][]string{
+				"one": {"foo"},
+			}),
+			"one:foo"},
+		{"two keys", NewColonSeparatedMultimapStringString(
+			&map[string][]string{
+				"one": {"foo"},
+				"two": {"bar"},
+			}),
+			"one:foo,two:bar"},
+		{"two keys, multiple items in one key", NewColonSeparatedMultimapStringString(
+			&map[string][]string{
+				"one": {"foo", "baz"},
+				"two": {"bar"},
+			}),
+			"one:foo,one:baz,two:bar"},
+		{"three keys, multiple items in one key", NewColonSeparatedMultimapStringString(
+			&map[string][]string{
+				"a": {"hello"},
+				"b": {"again", "beautiful"},
+				"c": {"world"},
+			}),
+			"a:hello,b:again,b:beautiful,c:world"},
 	}
 	for _, c := range cases {
 		t.Run(c.desc, func(t *testing.T) {
@@ -45,53 +71,125 @@ func TestStringColonSeparatedMultimapStringString(t *testing.T) {
 }
 
 func TestSetColonSeparatedMultimapStringString(t *testing.T) {
+	var nilMap map[string][]string
 	cases := []struct {
 		desc   string
-		val    string
+		vals   []string
-		expect ColonSeparatedMultimapStringString
+		start  *ColonSeparatedMultimapStringString
+		expect *ColonSeparatedMultimapStringString
 		err    string
 	}{
-		{"empty", "", ColonSeparatedMultimapStringString{}, ""},
+		// we initialize the map with a default key that should be cleared by Set
-		{"empty key", ":foo", ColonSeparatedMultimapStringString{
+		{"clears defaults", []string{""},
-			"": []string{"foo"},
+			NewColonSeparatedMultimapStringString(&map[string][]string{"default": {}}),
-		}, ""},
+			&ColonSeparatedMultimapStringString{
-		{"one key", "one:foo", ColonSeparatedMultimapStringString{
+				initialized: true,
-			"one": []string{"foo"}}, ""},
+				Multimap:    &map[string][]string{}}, ""},
-		{"two keys", "one:foo,two:bar", ColonSeparatedMultimapStringString{
+		{"explicitly nil", []string{"nil"},
-			"one": []string{"foo"},
+			NewColonSeparatedMultimapStringString(&map[string][]string{"default": {}}),
-			"two": []string{"bar"},
+			&ColonSeparatedMultimapStringString{
-		}, ""},
+				initialized: true,
-		{"two keys with space", "one:foo, two:bar", ColonSeparatedMultimapStringString{
+				Multimap:    &nilMap,
-			"one": []string{"foo"},
+			}, ""},
-			"two": []string{"bar"},
+		// make sure we still allocate for "initialized" multimaps where Multimap was initially set to a nil map
-		}, ""},
+		{"allocates map if currently nil", []string{""},
-		{"two keys, multiple items in one key", "one: foo, two:bar, one:baz", ColonSeparatedMultimapStringString{
+			&ColonSeparatedMultimapStringString{initialized: true, Multimap: &nilMap},
-			"one": []string{"foo", "baz"},
+			&ColonSeparatedMultimapStringString{
-			"two": []string{"bar"},
+				initialized: true,
-		}, ""},
+				Multimap:    &map[string][]string{},
-		{"three keys, multiple items in one key", "a:hello,b:again,c:world,b:beautiful", ColonSeparatedMultimapStringString{
+			}, ""},
-			"a": []string{"hello"},
+		// for most cases, we just reuse nilMap, which should be allocated by Set, and is reset before each test case
-			"b": []string{"again", "beautiful"},
+		{"empty", []string{""},
-			"c": []string{"world"},
+			NewColonSeparatedMultimapStringString(&nilMap),
-		}, ""},
+			&ColonSeparatedMultimapStringString{
-		{"missing value", "one", ColonSeparatedMultimapStringString{}, "malformed pair, expect string:string"},
+				initialized: true,
+				Multimap:    &map[string][]string{}}, ""},
+		{"empty key", []string{":foo"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"": {"foo"},
+				}}, ""},
+		{"one key", []string{"one:foo"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"one": {"foo"},
+				}}, ""},
+		{"two keys", []string{"one:foo,two:bar"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"one": {"foo"},
+					"two": {"bar"},
+				}}, ""},
+		{"two keys with space", []string{"one:foo, two:bar"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"one": {"foo"},
+					"two": {"bar"},
+				}}, ""},
+		{"two keys, multiple items in one key", []string{"one: foo, two:bar, one:baz"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"one": {"foo", "baz"},
+					"two": {"bar"},
+				}}, ""},
+		{"three keys, multiple items in one key", []string{"a:hello,b:again,c:world,b:beautiful"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"a": {"hello"},
+					"b": {"again", "beautiful"},
+					"c": {"world"},
+				}}, ""},
+		{"three keys, multiple items in one key, multiple Set invocations", []string{"a:hello,b:again", "c:world", "b:beautiful"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap: &map[string][]string{
+					"a": {"hello"},
+					"b": {"again", "beautiful"},
+					"c": {"world"},
+				}}, ""},
+		{"missing value", []string{"a"},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			nil,
+			"malformed pair, expect string:string"},
+		{"no target", []string{"a:foo"},
+			NewColonSeparatedMultimapStringString(nil),
+			nil,
+			"no target (nil pointer to map[string][]string)"},
 	}
 
 	for _, c := range cases {
+		nilMap = nil
 		t.Run(c.desc, func(t *testing.T) {
-			// we initialize the map with a default key that should be cleared by Set (no test cases specify "default")
+			var err error
-			m := ColonSeparatedMultimapStringString{"default": []string{}}
+			for _, val := range c.vals {
-			err := m.Set(c.val)
+				err = c.start.Set(val)
+				if err != nil {
+					break
+				}
+			}
 			if c.err != "" {
-				if err.Error() != c.err {
+				if err == nil || err.Error() != c.err {
 					t.Fatalf("expect error %s but got %v", c.err, err)
 				}
 				return
 			} else if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
-			if !reflect.DeepEqual(c.expect, m) {
+			if !reflect.DeepEqual(c.expect, c.start) {
-				t.Fatalf("expect %#v but got %#v", c.expect, m)
+				t.Fatalf("expect %#v but got %#v", c.expect, c.start)
 			}
 		})
 	}
@@ -100,22 +198,25 @@ func TestSetColonSeparatedMultimapStringString(t *testing.T) {
 func TestRoundTripColonSeparatedMultimapStringString(t *testing.T) {
 	cases := []struct {
 		desc   string
-		val    string
+		vals   []string
 		expect string
 	}{
-		{"empty", "", ""},
+		{"empty", []string{""}, ""},
-		{"empty key", ":foo", ":foo"},
+		{"empty key", []string{":foo"}, ":foo"},
-		{"one key", "one:foo", "one:foo"},
+		{"one key", []string{"one:foo"}, "one:foo"},
-		{"two keys", "one:foo,two:bar", "one:foo,two:bar"},
+		{"two keys", []string{"one:foo,two:bar"}, "one:foo,two:bar"},
-		{"two keys, multiple items in one key", "one:foo, two:bar, one:baz", "one:foo,one:baz,two:bar"},
+		{"two keys, multiple items in one key", []string{"one:foo, two:bar, one:baz"}, "one:foo,one:baz,two:bar"},
-		{"three keys, multiple items in one key", "a:hello,b:again,c:world,b:beautiful", "a:hello,b:again,b:beautiful,c:world"},
+		{"three keys, multiple items in one key", []string{"a:hello,b:again,c:world,b:beautiful"}, "a:hello,b:again,b:beautiful,c:world"},
+		{"three keys, multiple items in one key, multiple Set invocations", []string{"a:hello,b:again", "c:world", "b:beautiful"}, "a:hello,b:again,b:beautiful,c:world"},
 	}
 
 	for _, c := range cases {
 		t.Run(c.desc, func(t *testing.T) {
-			m := ColonSeparatedMultimapStringString{}
+			m := NewColonSeparatedMultimapStringString(&map[string][]string{})
-			if err := m.Set(c.val); err != nil {
+			for _, val := range c.vals {
-				t.Fatalf("unexpected error: %v", err)
+				if err := m.Set(val); err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
 			}
 			str := m.String()
 			if c.expect != str {