From 316c3696df63ff6c052786f700e2468fc869d164 Mon Sep 17 00:00:00 2001 From: Zihong Zheng Date: Wed, 31 Jan 2018 14:01:43 -0800 Subject: [PATCH] kube-proxy: Fix flag validation for healthz-bind-address and metrics-bind-address --- cmd/kube-proxy/app/server.go | 4 +- pkg/apis/componentconfig/helpers.go | 50 ++++++++ pkg/apis/componentconfig/helpers_test.go | 110 ++++++++++++++++-- .../componentconfig/zz_generated.deepcopy.go | 25 ++++ 4 files changed, 179 insertions(+), 10 deletions(-) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index 3337dcdaa62..42fe8a7a5e2 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -128,8 +128,8 @@ func AddFlags(options *Options, fs *pflag.FlagSet) { fs.Var(componentconfig.IPVar{Val: &options.config.BindAddress}, "bind-address", "The IP address for the proxy server to serve on (set to `0.0.0.0` for all IPv4 interfaces and `::` for all IPv6 interfaces)") fs.StringVar(&options.master, "master", options.master, "The address of the Kubernetes API server (overrides any value in kubeconfig)") fs.Int32Var(&options.healthzPort, "healthz-port", options.healthzPort, "The port to bind the health check server. Use 0 to disable.") - fs.Var(componentconfig.IPVar{Val: &options.config.HealthzBindAddress}, "healthz-bind-address", "The IP address and port for the health check server to serve on (set to `0.0.0.0` for all IPv4 interfaces and `::` for all IPv6 interfaces)") - fs.Var(componentconfig.IPVar{Val: &options.config.MetricsBindAddress}, "metrics-bind-address", "The IP address and port for the metrics server to serve on (set to `0.0.0.0` for all IPv4 interfaces and `::` for all IPv6 interfaces)") + fs.Var(componentconfig.IPPortVar{Val: &options.config.HealthzBindAddress}, "healthz-bind-address", "The IP address and port for the health check server to serve on (set to `0.0.0.0` for all IPv4 interfaces and `::` for all IPv6 interfaces)") + fs.Var(componentconfig.IPPortVar{Val: &options.config.MetricsBindAddress}, "metrics-bind-address", "The IP address and port for the metrics server to serve on (set to `0.0.0.0` for all IPv4 interfaces and `::` for all IPv6 interfaces)") fs.Int32Var(options.config.OOMScoreAdj, "oom-score-adj", utilpointer.Int32PtrDerefOr(options.config.OOMScoreAdj, int32(qos.KubeProxyOOMScoreAdj)), "The oom-score-adj value for kube-proxy process. Values must be within the range [-1000, 1000]") fs.StringVar(&options.config.ResourceContainer, "resource-container", options.config.ResourceContainer, "Absolute name of the resource-only container to create and run the Kube-proxy in (Default: /kube-proxy).") fs.MarkDeprecated("resource-container", "This feature will be removed in a later release.") diff --git a/pkg/apis/componentconfig/helpers.go b/pkg/apis/componentconfig/helpers.go index a39ec138c81..a8fb98c1fad 100644 --- a/pkg/apis/componentconfig/helpers.go +++ b/pkg/apis/componentconfig/helpers.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "net" + "strconv" "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -61,6 +62,55 @@ func (v IPVar) Type() string { return "ip" } +// IPPortVar allows IP or IP:port formats. +type IPPortVar struct { + Val *string +} + +func (v IPPortVar) Set(s string) error { + if len(s) == 0 { + v.Val = nil + return nil + } + + if v.Val == nil { + // it's okay to panic here since this is programmer error + panic("the string pointer passed into IPPortVar should not be nil") + } + + // Both IP and IP:port are valid. + // Attempt to parse into IP first. + if net.ParseIP(s) != nil { + *v.Val = s + return nil + } + + // Can not parse into IP, now assume IP:port. + host, port, err := net.SplitHostPort(s) + if err != nil { + return fmt.Errorf("%q is not in a valid format (ip or ip:port): %v", s, err) + } + if net.ParseIP(host) == nil { + return fmt.Errorf("%q is not a valid IP address", host) + } + if _, err := strconv.Atoi(port); err != nil { + return fmt.Errorf("%q is not a valid number", port) + } + *v.Val = s + return nil +} + +func (v IPPortVar) String() string { + if v.Val == nil { + return "" + } + return *v.Val +} + +func (v IPPortVar) Type() string { + return "ipport" +} + type PortRangeVar struct { Val *string } diff --git a/pkg/apis/componentconfig/helpers_test.go b/pkg/apis/componentconfig/helpers_test.go index 12a8c6a8cef..66e38e5a4c6 100644 --- a/pkg/apis/componentconfig/helpers_test.go +++ b/pkg/apis/componentconfig/helpers_test.go @@ -25,7 +25,7 @@ import ( func TestIPVar(t *testing.T) { defaultIP := "0.0.0.0" - cases := []struct { + testCases := []struct { argc string expectErr bool expectVal string @@ -41,7 +41,7 @@ func TestIPVar(t *testing.T) { expectVal: defaultIP, }, } - for _, c := range cases { + for _, tc := range testCases { fs := pflag.NewFlagSet("blah", pflag.PanicOnError) ip := defaultIP fs.Var(IPVar{&ip}, "ip", "the ip") @@ -53,19 +53,113 @@ func TestIPVar(t *testing.T) { err = r.(error) } }() - fs.Parse(strings.Split(c.argc, " ")) + fs.Parse(strings.Split(tc.argc, " ")) }() - if c.expectErr && err == nil { + if tc.expectErr && err == nil { t.Errorf("did not observe an expected error") continue } - if !c.expectErr && err != nil { - t.Errorf("observed an unexpected error") + if !tc.expectErr && err != nil { + t.Errorf("observed an unexpected error: %v", err) continue } - if c.expectVal != ip { - t.Errorf("unexpected ip: expected %q, saw %q", c.expectVal, ip) + if tc.expectVal != ip { + t.Errorf("unexpected ip: expected %q, saw %q", tc.expectVal, ip) + } + } +} + +func TestIPPortVar(t *testing.T) { + defaultIPPort := "0.0.0.0:8080" + testCases := []struct { + desc string + argc string + expectErr bool + expectVal string + }{ + + { + desc: "valid ipv4 1", + argc: "blah --ipport=0.0.0.0", + expectVal: "0.0.0.0", + }, + { + desc: "valid ipv4 2", + argc: "blah --ipport=127.0.0.1", + expectVal: "127.0.0.1", + }, + + { + desc: "invalid IP", + argc: "blah --ipport=invalidip", + expectErr: true, + expectVal: defaultIPPort, + }, + { + desc: "valid ipv4 with port", + argc: "blah --ipport=0.0.0.0:8080", + expectVal: "0.0.0.0:8080", + }, + { + desc: "invalid ipv4 with invalid port", + argc: "blah --ipport=0.0.0.0:invalidport", + expectErr: true, + expectVal: defaultIPPort, + }, + { + desc: "invalid IP with port", + argc: "blah --ipport=invalidip:8080", + expectErr: true, + expectVal: defaultIPPort, + }, + { + desc: "valid ipv6 1", + argc: "blah --ipport=::1", + expectVal: "::1", + }, + { + desc: "valid ipv6 2", + argc: "blah --ipport=::", + expectVal: "::", + }, + { + desc: "valid ipv6 with port", + argc: "blah --ipport=[::1]:8080", + expectVal: "[::1]:8080", + }, + { + desc: "invalid ipv6 with port without bracket", + argc: "blah --ipport=fd00:f00d:600d:f00d:8080", + expectErr: true, + expectVal: defaultIPPort, + }, + } + for _, tc := range testCases { + fs := pflag.NewFlagSet("blah", pflag.PanicOnError) + ipport := defaultIPPort + fs.Var(IPPortVar{&ipport}, "ipport", "the ip:port") + + var err error + func() { + defer func() { + if r := recover(); r != nil { + err = r.(error) + } + }() + fs.Parse(strings.Split(tc.argc, " ")) + }() + + if tc.expectErr && err == nil { + t.Errorf("%q: Did not observe an expected error", tc.desc) + continue + } + if !tc.expectErr && err != nil { + t.Errorf("%q: Observed an unexpected error: %v", tc.desc, err) + continue + } + if tc.expectVal != ipport { + t.Errorf("%q: Unexpected ipport: expected %q, saw %q", tc.desc, tc.expectVal, ipport) } } } diff --git a/pkg/apis/componentconfig/zz_generated.deepcopy.go b/pkg/apis/componentconfig/zz_generated.deepcopy.go index 31fad659dfc..8b33440897b 100644 --- a/pkg/apis/componentconfig/zz_generated.deepcopy.go +++ b/pkg/apis/componentconfig/zz_generated.deepcopy.go @@ -56,6 +56,31 @@ func (in *GroupResource) DeepCopy() *GroupResource { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPPortVar) DeepCopyInto(out *IPPortVar) { + *out = *in + if in.Val != nil { + in, out := &in.Val, &out.Val + if *in == nil { + *out = nil + } else { + *out = new(string) + **out = **in + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPPortVar. +func (in *IPPortVar) DeepCopy() *IPPortVar { + if in == nil { + return nil + } + out := new(IPPortVar) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *IPVar) DeepCopyInto(out *IPVar) { *out = *in