Merge pull request #55254 from mtaufen/iterative-manifest-url-header

Automatic merge from submit-queue (batch tested with PRs 55254, 55525, 50108, 54674, 55263). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

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"]}`

Related: #53833, https://github.com/kubernetes/kubernetes/pull/54643#discussion_r148651257

```release-note
NONE
```

Kubernetes-commit: dc315b8e37a73e5c036701eaee868909aa9333f7

pkg/util/flag/colon_separated_multimap_string_string.go

@@ -28,13 +28,30 @@ 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 {
-	// clear old values
-	for k := range m {
-		delete(m, k)
+func (m *ColonSeparatedMultimapStringString) Set(value string) error {
+	if m.Multimap == nil {
+		return fmt.Errorf("no target (nil pointer to map[string][]string)")
+	}
+	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 +63,19 @@ 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 {
 	type kv struct {
 		k string
 		v string
 	}
-	kvs := make([]kv, 0, len(m))
-	for k, vs := range m {
+	kvs := make([]kv, 0, len(*m.Multimap))
+	for k, vs := range *m.Multimap {
 		for i := range vs {
 			kvs = append(kvs, kv{k: k, v: vs[i]})
 		}
@@ -75,6 +92,11 @@ func (m ColonSeparatedMultimapStringString) String() string {
 }
 
 // Type implements github.com/spf13/pflag.Value
-func (m ColonSeparatedMultimapStringString) Type() string {
+func (m *ColonSeparatedMultimapStringString) Type() string {
 	return "colonSeparatedMultimapStringString"
 }
+
+// Empty implements OmitEmpty
+func (m *ColonSeparatedMultimapStringString) Empty() bool {
+	return len(*m.Multimap) == 0
+}

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{}, ""},
-		{"empty key", ColonSeparatedMultimapStringString{"": []string{"foo"}}, ":foo"},
-		{"one key", ColonSeparatedMultimapStringString{"one": []string{"foo"}}, "one:foo"},
-		{"two keys", ColonSeparatedMultimapStringString{"one": []string{"foo"}, "two": []string{"bar"}}, "one:foo,two:bar"},
-		{"two keys, multiple items in one key", ColonSeparatedMultimapStringString{"one": []string{"foo", "baz"}, "two": []string{"bar"}}, "one:foo,one:baz,two:bar"},
-		{"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"},
+		{"nil", NewColonSeparatedMultimapStringString(&nilMap), ""},
+		{"empty", NewColonSeparatedMultimapStringString(&map[string][]string{}), ""},
+		{"empty key", NewColonSeparatedMultimapStringString(
+			&map[string][]string{
+				"": {"foo"},
+			}),
+			":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,119 @@ func TestStringColonSeparatedMultimapStringString(t *testing.T) {
 }
 
 func TestSetColonSeparatedMultimapStringString(t *testing.T) {
+	var nilMap map[string][]string
 	cases := []struct {
 		desc   string
-		val    string
-		expect ColonSeparatedMultimapStringString
+		vals   []string
+		start  *ColonSeparatedMultimapStringString
+		expect *ColonSeparatedMultimapStringString
 		err    string
 	}{
-		{"empty", "", ColonSeparatedMultimapStringString{}, ""},
-		{"empty key", ":foo", ColonSeparatedMultimapStringString{
-			"": []string{"foo"},
-		}, ""},
-		{"one key", "one:foo", ColonSeparatedMultimapStringString{
-			"one": []string{"foo"}}, ""},
-		{"two keys", "one:foo,two:bar", ColonSeparatedMultimapStringString{
-			"one": []string{"foo"},
-			"two": []string{"bar"},
-		}, ""},
-		{"two keys with space", "one:foo, two:bar", ColonSeparatedMultimapStringString{
-			"one": []string{"foo"},
-			"two": []string{"bar"},
-		}, ""},
-		{"two keys, multiple items in one key", "one: foo, two:bar, one:baz", ColonSeparatedMultimapStringString{
-			"one": []string{"foo", "baz"},
-			"two": []string{"bar"},
-		}, ""},
-		{"three keys, multiple items in one key", "a:hello,b:again,c:world,b:beautiful", ColonSeparatedMultimapStringString{
-			"a": []string{"hello"},
-			"b": []string{"again", "beautiful"},
-			"c": []string{"world"},
-		}, ""},
-		{"missing value", "one", ColonSeparatedMultimapStringString{}, "malformed pair, expect string:string"},
+		// we initialize the map with a default key that should be cleared by Set
+		{"clears defaults", []string{""},
+			NewColonSeparatedMultimapStringString(&map[string][]string{"default": {}}),
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap:    &map[string][]string{}}, ""},
+		// make sure we still allocate for "initialized" multimaps where Multimap was initially set to a nil map
+		{"allocates map if currently nil", []string{""},
+			&ColonSeparatedMultimapStringString{initialized: true, Multimap: &nilMap},
+			&ColonSeparatedMultimapStringString{
+				initialized: true,
+				Multimap:    &map[string][]string{},
+			}, ""},
+		// for most cases, we just reuse nilMap, which should be allocated by Set, and is reset before each test case
+		{"empty", []string{""},
+			NewColonSeparatedMultimapStringString(&nilMap),
+			&ColonSeparatedMultimapStringString{
+				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")
-			m := ColonSeparatedMultimapStringString{"default": []string{}}
-			err := m.Set(c.val)
+			var err error
+			for _, val := range c.vals {
+				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) {
-				t.Fatalf("expect %#v but got %#v", c.expect, m)
+			if !reflect.DeepEqual(c.expect, c.start) {
+				t.Fatalf("expect %#v but got %#v", c.expect, c.start)
 			}
 		})
 	}
@@ -100,22 +192,25 @@ func TestSetColonSeparatedMultimapStringString(t *testing.T) {
 func TestRoundTripColonSeparatedMultimapStringString(t *testing.T) {
 	cases := []struct {
 		desc   string
-		val    string
+		vals   []string
 		expect string
 	}{
-		{"empty", "", ""},
-		{"empty key", ":foo", ":foo"},
-		{"one key", "one:foo", "one:foo"},
-		{"two keys", "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"},
-		{"three keys, multiple items in one key", "a:hello,b:again,c:world,b:beautiful", "a:hello,b:again,b:beautiful,c:world"},
+		{"empty", []string{""}, ""},
+		{"empty key", []string{":foo"}, ":foo"},
+		{"one key", []string{"one:foo"}, "one:foo"},
+		{"two keys", []string{"one:foo,two:bar"}, "one:foo,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", []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{}
-			if err := m.Set(c.val); err != nil {
-				t.Fatalf("unexpected error: %v", err)
+			m := NewColonSeparatedMultimapStringString(&map[string][]string{})
+			for _, val := range c.vals {
+				if err := m.Set(val); err != nil {
+					t.Fatalf("unexpected error: %v", err)
+				}
 			}
 			str := m.String()
 			if c.expect != str {
@@ -124,3 +219,24 @@ func TestRoundTripColonSeparatedMultimapStringString(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyColonSeparatedMultimapStringString(t *testing.T) {
+	var nilMap map[string][]string
+	cases := []struct {
+		desc   string
+		val    *ColonSeparatedMultimapStringString
+		expect bool
+	}{
+		{"nil", NewColonSeparatedMultimapStringString(&nilMap), true},
+		{"empty", NewColonSeparatedMultimapStringString(&map[string][]string{}), true},
+		{"populated", NewColonSeparatedMultimapStringString(&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)
+			}
+		})
+	}
+}