From 7cb21746c09cd7dd8e2beaaa03f17ba41563a318 Mon Sep 17 00:00:00 2001 From: Michael Taufen Date: Thu, 19 Oct 2017 15:42:07 -0700 Subject: [PATCH] Lift embedded structure out of ManifestURLHeader field --- cmd/kubelet/app/options/options.go | 2 +- pkg/kubelet/apis/kubeletconfig/types.go | 4 +- .../apis/kubeletconfig/v1alpha1/types.go | 4 +- .../v1alpha1/zz_generated.conversion.go | 4 +- .../v1alpha1/zz_generated.deepcopy.go | 12 ++ .../kubeletconfig/zz_generated.deepcopy.go | 12 ++ pkg/kubelet/kubelet.go | 10 +- .../src/k8s.io/apiserver/pkg/util/flag/BUILD | 2 + .../colon_separated_multimap_string_string.go | 80 +++++++++++ ...n_separated_multimap_string_string_test.go | 126 ++++++++++++++++++ 10 files changed, 244 insertions(+), 12 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string.go create mode 100644 staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string_test.go diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index 08e09e80aa5..22d89e87bae 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -337,7 +337,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.StringVar(&c.ManifestURLHeader, "manifest-url-header", c.ManifestURLHeader, "HTTP header to use when accessing the manifest URL, with the key separated from the value with a ':', as in 'key:value'") + 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.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/types.go b/pkg/kubelet/apis/kubeletconfig/types.go index 575a991cd97..bcaff8dc807 100644 --- a/pkg/kubelet/apis/kubeletconfig/types.go +++ b/pkg/kubelet/apis/kubeletconfig/types.go @@ -70,7 +70,7 @@ type KubeletConfiguration struct { ManifestURL string // manifestURLHeader is the HTTP header to use when accessing the manifest // URL, with the key separated from the value with a ':', as in 'key:value' - ManifestURLHeader string + ManifestURLHeader map[string][]string // enableServer enables the Kubelet's server EnableServer bool // address is the IP address for the Kubelet to serve on (set to 0.0.0.0 @@ -227,7 +227,7 @@ type KubeletConfiguration struct { // In cluster mode, this is obtained from the master. PodCIDR string // ResolverConfig is the resolver configuration file used as the basis - // for the container DNS resolution configuration."), [] + // for the container DNS resolution configuration. ResolverConfig string // cpuCFSQuota is Enable CPU CFS quota enforcement for containers that // specify CPU limits diff --git a/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go b/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go index 8921b193637..83491fc9412 100644 --- a/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go +++ b/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go @@ -66,7 +66,7 @@ type KubeletConfiguration struct { ManifestURL string `json:"manifestURL"` // manifestURLHeader is the HTTP header to use when accessing the manifest // URL, with the key separated from the value with a ':', as in 'key:value' - ManifestURLHeader string `json:"manifestURLHeader"` + ManifestURLHeader map[string][]string `json:"manifestURLHeader"` // enableServer enables the Kubelet's server EnableServer *bool `json:"enableServer"` // address is the IP address for the Kubelet to serve on (set to 0.0.0.0 @@ -219,7 +219,7 @@ type KubeletConfiguration struct { // In cluster mode, this is obtained from the master. PodCIDR string `json:"podCIDR"` // ResolverConfig is the resolver configuration file used as the basis - // for the container DNS resolution configuration."), [] + // for the container DNS resolution configuration. ResolverConfig string `json:"resolvConf"` // cpuCFSQuota is Enable CPU CFS quota enforcement for containers that // specify CPU limits diff --git a/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.conversion.go b/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.conversion.go index 51c98995f52..794f9671a6d 100644 --- a/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.conversion.go +++ b/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.conversion.go @@ -148,7 +148,7 @@ func autoConvert_v1alpha1_KubeletConfiguration_To_kubeletconfig_KubeletConfigura out.FileCheckFrequency = in.FileCheckFrequency out.HTTPCheckFrequency = in.HTTPCheckFrequency out.ManifestURL = in.ManifestURL - out.ManifestURLHeader = in.ManifestURLHeader + out.ManifestURLHeader = *(*map[string][]string)(unsafe.Pointer(&in.ManifestURLHeader)) if err := v1.Convert_Pointer_bool_To_bool(&in.EnableServer, &out.EnableServer, s); err != nil { return err } @@ -288,7 +288,7 @@ func autoConvert_kubeletconfig_KubeletConfiguration_To_v1alpha1_KubeletConfigura out.FileCheckFrequency = in.FileCheckFrequency out.HTTPCheckFrequency = in.HTTPCheckFrequency out.ManifestURL = in.ManifestURL - out.ManifestURLHeader = in.ManifestURLHeader + out.ManifestURLHeader = *(*map[string][]string)(unsafe.Pointer(&in.ManifestURLHeader)) if err := v1.Convert_bool_To_Pointer_bool(&in.EnableServer, &out.EnableServer, s); err != nil { return err } diff --git a/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.deepcopy.go b/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.deepcopy.go index df855d99c01..4671e50fdb3 100644 --- a/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/kubelet/apis/kubeletconfig/v1alpha1/zz_generated.deepcopy.go @@ -146,6 +146,18 @@ func (in *KubeletConfiguration) DeepCopyInto(out *KubeletConfiguration) { out.SyncFrequency = in.SyncFrequency out.FileCheckFrequency = in.FileCheckFrequency out.HTTPCheckFrequency = in.HTTPCheckFrequency + if in.ManifestURLHeader != nil { + in, out := &in.ManifestURLHeader, &out.ManifestURLHeader + *out = make(map[string][]string, len(*in)) + for key, val := range *in { + if val == nil { + (*out)[key] = nil + } else { + (*out)[key] = make([]string, len(val)) + copy((*out)[key], val) + } + } + } if in.EnableServer != nil { in, out := &in.EnableServer, &out.EnableServer if *in == nil { diff --git a/pkg/kubelet/apis/kubeletconfig/zz_generated.deepcopy.go b/pkg/kubelet/apis/kubeletconfig/zz_generated.deepcopy.go index 9be3d24a47c..bf64fd145b8 100644 --- a/pkg/kubelet/apis/kubeletconfig/zz_generated.deepcopy.go +++ b/pkg/kubelet/apis/kubeletconfig/zz_generated.deepcopy.go @@ -137,6 +137,18 @@ func (in *KubeletConfiguration) DeepCopyInto(out *KubeletConfiguration) { out.SyncFrequency = in.SyncFrequency out.FileCheckFrequency = in.FileCheckFrequency out.HTTPCheckFrequency = in.HTTPCheckFrequency + if in.ManifestURLHeader != nil { + in, out := &in.ManifestURLHeader, &out.ManifestURLHeader + *out = make(map[string][]string, len(*in)) + for key, val := range *in { + if val == nil { + (*out)[key] = nil + } else { + (*out)[key] = make([]string, len(val)) + copy((*out)[key], val) + } + } + } out.Authentication = in.Authentication out.Authorization = in.Authorization if in.HostNetworkSources != nil { diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index b95ca191855..53e2b98ada5 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -272,12 +272,12 @@ type Dependencies struct { // KubeletConfiguration or returns an error. func makePodSourceConfig(kubeCfg *kubeletconfiginternal.KubeletConfiguration, kubeDeps *Dependencies, nodeName types.NodeName) (*config.PodConfig, error) { manifestURLHeader := make(http.Header) - if kubeCfg.ManifestURLHeader != "" { - pieces := strings.Split(kubeCfg.ManifestURLHeader, ":") - if len(pieces) != 2 { - return nil, fmt.Errorf("manifest-url-header must have a single ':' key-value separator, got %q", kubeCfg.ManifestURLHeader) + if len(kubeCfg.ManifestURLHeader) > 0 { + for k, v := range kubeCfg.ManifestURLHeader { + for i := range v { + manifestURLHeader.Add(k, v[i]) + } } - manifestURLHeader.Set(pieces[0], pieces[1]) } // source of all configuration diff --git a/staging/src/k8s.io/apiserver/pkg/util/flag/BUILD b/staging/src/k8s.io/apiserver/pkg/util/flag/BUILD index 274da4786ba..9bf629d1aa2 100644 --- a/staging/src/k8s.io/apiserver/pkg/util/flag/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/util/flag/BUILD @@ -9,6 +9,7 @@ load( go_test( name = "go_default_test", srcs = [ + "colon_separated_multimap_string_string_test.go", "map_string_bool_test.go", "namedcertkey_flag_test.go", ], @@ -24,6 +25,7 @@ go_test( go_library( name = "go_default_library", srcs = [ + "colon_separated_multimap_string_string.go", "configuration_map.go", "flags.go", "map_string_bool.go", 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 new file mode 100644 index 00000000000..638f4ae9548 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string.go @@ -0,0 +1,80 @@ +/* +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 + +import ( + "fmt" + "sort" + "strings" +) + +// ColonSeparatedMultimapStringString supports setting a map[string][]string from an encoding +// that separates keys from values with ':' and separates key-value pairs with ','. +// A key can be repeated multiple times, in which case the values are appended to a +// 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 + +// Set implements github.com/spf13/pflag.Value +func (m ColonSeparatedMultimapStringString) Set(value string) error { + // clear old values + for k := range m { + delete(m, k) + } + for _, pair := range strings.Split(value, ",") { + if len(pair) == 0 { + continue + } + kv := strings.SplitN(pair, ":", 2) + if len(kv) != 2 { + return fmt.Errorf("malformed pair, expect string:string") + } + k := strings.TrimSpace(kv[0]) + v := strings.TrimSpace(kv[1]) + m[k] = append(m[k], v) + } + return nil +} + +// String implements github.com/spf13/pflag.Value +func (m ColonSeparatedMultimapStringString) String() string { + type kv struct { + k string + v string + } + kvs := make([]kv, 0, len(m)) + for k, vs := range m { + for i := range vs { + kvs = append(kvs, kv{k: k, v: vs[i]}) + } + } + // stable sort by keys, order of values should be preserved + sort.SliceStable(kvs, func(i, j int) bool { + return kvs[i].k < kvs[j].k + }) + pairs := make([]string, 0, len(kvs)) + for i := range kvs { + pairs = append(pairs, fmt.Sprintf("%s:%s", kvs[i].k, kvs[i].v)) + } + return strings.Join(pairs, ",") +} + +// Type implements github.com/spf13/pflag.Value +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 new file mode 100644 index 00000000000..c7b1629353a --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/util/flag/colon_separated_multimap_string_string_test.go @@ -0,0 +1,126 @@ +/* +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 + +import ( + "reflect" + "testing" +) + +func TestStringColonSeparatedMultimapStringString(t *testing.T) { + cases := []struct { + desc string + 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"}, + } + for _, c := range cases { + t.Run(c.desc, func(t *testing.T) { + str := c.m.String() + if c.expect != str { + t.Fatalf("expect %q but got %q", c.expect, str) + } + }) + } +} + +func TestSetColonSeparatedMultimapStringString(t *testing.T) { + cases := []struct { + desc string + val string + 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"}, + } + + for _, c := range cases { + 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) + if c.err != "" { + if 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) + } + }) + } +} + +func TestRoundTripColonSeparatedMultimapStringString(t *testing.T) { + cases := []struct { + desc string + val 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"}, + } + + 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) + } + str := m.String() + if c.expect != str { + t.Fatalf("expect %q but got %q", c.expect, str) + } + }) + } +}