diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index 6a97f914a1a..e7d0e8ab7e0 100644 --- a/cmd/kubelet/app/options/options.go +++ b/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.") diff --git a/pkg/kubelet/apis/kubeletconfig/v1alpha1/defaults.go b/pkg/kubelet/apis/kubeletconfig/v1alpha1/defaults.go index 34e07f4c42b..e53665a90c1 100644 --- a/pkg/kubelet/apis/kubeletconfig/v1alpha1/defaults.go +++ b/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 { diff --git a/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string.go b/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string.go index 638f4ae9548..0619b0c9543 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string.go +++ b/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 { - // 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)") + } + // 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)) - 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 +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" } diff --git a/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string_test.go b/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string_test.go index c7b1629353a..e473217ea62 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string_test.go +++ b/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{}, ""}, - {"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), "nil"}, + {"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,125 @@ 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{}}, ""}, + {"explicitly nil", []string{"nil"}, + NewColonSeparatedMultimapStringString(&map[string][]string{"default": {}}), + &ColonSeparatedMultimapStringString{ + initialized: true, + Multimap: &nilMap, + }, ""}, + // 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 +198,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 {