support ipv6 in bind address

use split host port func instead trim specific character

add unit test for metrics and healthz bind address

recover import package

refactor set default kube proxy configuration

fix ipv4 condition

fix set default port condition

rewrite call function occasion to reduce error

set ipv6 default value

move get GetBindAddressHostPort to util

use one func to handle deprecated series

update bazel

define address type

return earlier in the error case

refactor set default kube proxy configuration logic

recover import package

preserve some of the original comments

add get default address func

add append port if needed unit test

rewrite unit test for deprecated flags

remove unused codes

Author: LexCC

cmd/kube-proxy/app/BUILD

@@ -31,6 +31,7 @@ go_library(
         "//pkg/proxy/iptables:go_default_library",
         "//pkg/proxy/ipvs:go_default_library",
         "//pkg/proxy/userspace:go_default_library",
+        "//pkg/proxy/util:go_default_library",
         "//pkg/util/configz:go_default_library",
         "//pkg/util/filesystem:go_default_library",
         "//pkg/util/flag:go_default_library",

cmd/kube-proxy/app/server.go

@@ -61,6 +61,7 @@ import (
 	"k8s.io/kubernetes/pkg/proxy/iptables"
 	"k8s.io/kubernetes/pkg/proxy/ipvs"
 	"k8s.io/kubernetes/pkg/proxy/userspace"
+	proxyutil "k8s.io/kubernetes/pkg/proxy/util"
 	"k8s.io/kubernetes/pkg/util/configz"
 	"k8s.io/kubernetes/pkg/util/filesystem"
 	utilflag "k8s.io/kubernetes/pkg/util/flag"
@@ -212,8 +213,8 @@ func NewOptions() *Options {
 func (o *Options) Complete() error {
 	if len(o.ConfigFile) == 0 && len(o.WriteConfigTo) == 0 {
 		klog.Warning("WARNING: all flags other than --config, --write-config-to, and --cleanup are deprecated. Please begin using a config file ASAP.")
-		o.applyDeprecatedHealthzPortToConfig()
-		o.applyDeprecatedMetricsPortToConfig()
+		o.config.HealthzBindAddress = addressFromDeprecatedFlags(o.config.HealthzBindAddress, o.healthzPort)
+		o.config.MetricsBindAddress = addressFromDeprecatedFlags(o.config.MetricsBindAddress, o.metricsPort)
 	}
 
 	// Load the config file here in Complete, so that Validate validates the fully-resolved config.
@@ -357,38 +358,15 @@ func (o *Options) writeConfigFile() error {
 	return nil
 }
 
-// applyDeprecatedHealthzPortToConfig sets o.config.HealthzBindAddress from
-// flags passed on the command line based on the following rules:
-//
-// 1. If --healthz-port is 0, disable the healthz server.
-// 2. Otherwise, use the value of --healthz-port for the port portion of
-//    o.config.HealthzBindAddress
-func (o *Options) applyDeprecatedHealthzPortToConfig() {
-	if o.healthzPort == 0 {
-		o.config.HealthzBindAddress = ""
-		return
+// addressFromDeprecatedFlags returns server address from flags
+// passed on the command line based on the following rules:
+// 1. If port is 0, disable the server (e.g. set address to empty).
+// 2. Otherwise, set the port portion of the config accordingly.
+func addressFromDeprecatedFlags(addr string, port int32) string {
+	if port == 0 {
+		return ""
 	}
-
-	index := strings.Index(o.config.HealthzBindAddress, ":")
-	if index != -1 {
-		o.config.HealthzBindAddress = o.config.HealthzBindAddress[0:index]
-	}
-
-	o.config.HealthzBindAddress = fmt.Sprintf("%s:%d", o.config.HealthzBindAddress, o.healthzPort)
-}
-
-func (o *Options) applyDeprecatedMetricsPortToConfig() {
-	if o.metricsPort == 0 {
-		o.config.MetricsBindAddress = ""
-		return
-	}
-
-	index := strings.Index(o.config.MetricsBindAddress, ":")
-	if index != -1 {
-		o.config.MetricsBindAddress = o.config.MetricsBindAddress[0:index]
-	}
-
-	o.config.MetricsBindAddress = fmt.Sprintf("%s:%d", o.config.MetricsBindAddress, o.metricsPort)
+	return proxyutil.AppendPortIfNeeded(addr, port)
 }
 
 // loadConfigFromFile loads the contents of file and decodes it as a

cmd/kube-proxy/app/server_test.go

@@ -549,3 +549,79 @@ func (s *fakeProxyServerError) Run() error {
 		return fmt.Errorf("mocking error from ProxyServer.Run()")
 	}
 }
+
+func TestAddressFromDeprecatedFlags(t *testing.T) {
+	testCases := []struct {
+		name               string
+		healthzPort        int32
+		healthzBindAddress string
+		metricsPort        int32
+		metricsBindAddress string
+		expHealthz         string
+		expMetrics         string
+	}{
+		{
+			name:               "IPv4 bind address",
+			healthzBindAddress: "1.2.3.4",
+			healthzPort:        12345,
+			metricsBindAddress: "2.3.4.5",
+			metricsPort:        23456,
+			expHealthz:         "1.2.3.4:12345",
+			expMetrics:         "2.3.4.5:23456",
+		},
+		{
+			name:               "IPv4 bind address has port",
+			healthzBindAddress: "1.2.3.4:12345",
+			healthzPort:        23456,
+			metricsBindAddress: "2.3.4.5:12345",
+			metricsPort:        23456,
+			expHealthz:         "1.2.3.4:12345",
+			expMetrics:         "2.3.4.5:12345",
+		},
+		{
+			name:               "IPv6 bind address",
+			healthzBindAddress: "fd00:1::5",
+			healthzPort:        12345,
+			metricsBindAddress: "fd00:1::6",
+			metricsPort:        23456,
+			expHealthz:         "[fd00:1::5]:12345",
+			expMetrics:         "[fd00:1::6]:23456",
+		},
+		{
+			name:               "IPv6 bind address has port",
+			healthzBindAddress: "[fd00:1::5]:12345",
+			healthzPort:        56789,
+			metricsBindAddress: "[fd00:1::6]:56789",
+			metricsPort:        12345,
+			expHealthz:         "[fd00:1::5]:12345",
+			expMetrics:         "[fd00:1::6]:56789",
+		},
+		{
+			name:               "Invalid IPv6 Config",
+			healthzBindAddress: "[fd00:1::5]",
+			healthzPort:        12345,
+			metricsBindAddress: "[fd00:1::6]",
+			metricsPort:        56789,
+			expHealthz:         "[fd00:1::5]",
+			expMetrics:         "[fd00:1::6]",
+		},
+	}
+
+	for i := range testCases {
+		gotHealthz := addressFromDeprecatedFlags(testCases[i].healthzBindAddress, testCases[i].healthzPort)
+		gotMetrics := addressFromDeprecatedFlags(testCases[i].metricsBindAddress, testCases[i].metricsPort)
+
+		errFn := func(name, except, got string) {
+			t.Errorf("case %s: expected %v, got %v", name, except, got)
+		}
+
+		if gotHealthz != testCases[i].expHealthz {
+			errFn(testCases[i].name, testCases[i].expHealthz, gotHealthz)
+		}
+
+		if gotMetrics != testCases[i].expMetrics {
+			errFn(testCases[i].name, testCases[i].expMetrics, gotMetrics)
+		}
+
+	}
+}

pkg/proxy/apis/config/v1alpha1/BUILD

@@ -20,6 +20,7 @@ go_library(
         "//pkg/kubelet/qos:go_default_library",
         "//pkg/master/ports:go_default_library",
         "//pkg/proxy/apis/config:go_default_library",
+        "//pkg/proxy/util:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/conversion:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",

pkg/proxy/apis/config/v1alpha1/defaults.go

@@ -18,14 +18,16 @@ package v1alpha1
 
 import (
 	"fmt"
-	"strings"
+	"net"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kruntime "k8s.io/apimachinery/pkg/runtime"
 	kubeproxyconfigv1alpha1 "k8s.io/kube-proxy/config/v1alpha1"
+
 	"k8s.io/kubernetes/pkg/kubelet/qos"
 	"k8s.io/kubernetes/pkg/master/ports"
+	proxyutil "k8s.io/kubernetes/pkg/proxy/util"
 	"k8s.io/utils/pointer"
 )
 
@@ -34,19 +36,24 @@ func addDefaultingFuncs(scheme *kruntime.Scheme) error {
 }
 
 func SetDefaults_KubeProxyConfiguration(obj *kubeproxyconfigv1alpha1.KubeProxyConfiguration) {
+
 	if len(obj.BindAddress) == 0 {
 		obj.BindAddress = "0.0.0.0"
 	}
+
+	defaultHealthzAddress, defaultMetricsAddress := getDefaultAddresses(obj.BindAddress)
+
 	if obj.HealthzBindAddress == "" {
-		obj.HealthzBindAddress = fmt.Sprintf("0.0.0.0:%v", ports.ProxyHealthzPort)
-	} else if !strings.Contains(obj.HealthzBindAddress, ":") {
-		obj.HealthzBindAddress += fmt.Sprintf(":%v", ports.ProxyHealthzPort)
+		obj.HealthzBindAddress = fmt.Sprintf("%s:%v", defaultHealthzAddress, ports.ProxyHealthzPort)
+	} else {
+		obj.HealthzBindAddress = proxyutil.AppendPortIfNeeded(obj.HealthzBindAddress, ports.ProxyHealthzPort)
 	}
 	if obj.MetricsBindAddress == "" {
-		obj.MetricsBindAddress = fmt.Sprintf("127.0.0.1:%v", ports.ProxyStatusPort)
-	} else if !strings.Contains(obj.MetricsBindAddress, ":") {
-		obj.MetricsBindAddress += fmt.Sprintf(":%v", ports.ProxyStatusPort)
+		obj.MetricsBindAddress = fmt.Sprintf("%s:%v", defaultMetricsAddress, ports.ProxyStatusPort)
+	} else {
+		obj.MetricsBindAddress = proxyutil.AppendPortIfNeeded(obj.MetricsBindAddress, ports.ProxyStatusPort)
 	}
+
 	if obj.OOMScoreAdj == nil {
 		temp := int32(qos.KubeProxyOOMScoreAdj)
 		obj.OOMScoreAdj = &temp
@@ -121,3 +128,13 @@ func SetDefaults_KubeProxyConfiguration(obj *kubeproxyconfigv1alpha1.KubeProxyCo
 		obj.FeatureGates = make(map[string]bool)
 	}
 }
+
+// getDefaultAddresses returns default address of healthz and metrics server
+// based on the given bind address. IPv6 addresses are enclosed in square
+// brackets for appending port.
+func getDefaultAddresses(bindAddress string) (defaultHealthzAddress, defaultMetricsAddress string) {
+	if net.ParseIP(bindAddress).To4() != nil {
+		return "0.0.0.0", "127.0.0.1"
+	}
+	return "[::]", "[::1]"
+}

pkg/proxy/util/utils.go

@@ -214,3 +214,24 @@ func filterWithCondition(strs []string, expectedCondition bool, conditionFunc fu
 	}
 	return corrects, incorrects
 }
+
+// AppendPortIfNeeded appends the given port to IP address unless it is already in
+// "ipv4:port" or "[ipv6]:port" format.
+func AppendPortIfNeeded(addr string, port int32) string {
+	// Return if address is already in "ipv4:port" or "[ipv6]:port" format.
+	if _, _, err := net.SplitHostPort(addr); err == nil {
+		return addr
+	}
+
+	// Simply return for invalid case. This should be caught by validation instead.
+	ip := net.ParseIP(addr)
+	if ip == nil {
+		return addr
+	}
+
+	// Append port to address.
+	if ip.To4() != nil {
+		return fmt.Sprintf("%s:%d", addr, port)
+	}
+	return fmt.Sprintf("[%s]:%d", addr, port)
+}

pkg/proxy/util/utils_test.go

@@ -396,3 +396,50 @@ func TestGetNodeAddressses(t *testing.T) {
 		}
 	}
 }
+
+func TestAppendPortIfNeeded(t *testing.T) {
+	testCases := []struct {
+		name   string
+		addr   string
+		port   int32
+		expect string
+	}{
+		{
+			name:   "IPv4 all-zeros bind address has port",
+			addr:   "0.0.0.0:12345",
+			port:   23456,
+			expect: "0.0.0.0:12345",
+		},
+		{
+			name:   "non-zeros IPv4 config",
+			addr:   "9.8.7.6",
+			port:   12345,
+			expect: "9.8.7.6:12345",
+		},
+		{
+			name:   "IPv6 \"[::]\" bind address has port",
+			addr:   "[::]:12345",
+			port:   23456,
+			expect: "[::]:12345",
+		},
+		{
+			name:   "IPv6 config",
+			addr:   "fd00:1::5",
+			port:   23456,
+			expect: "[fd00:1::5]:23456",
+		},
+		{
+			name:   "Invalid IPv6 Config",
+			addr:   "[fd00:1::5]",
+			port:   12345,
+			expect: "[fd00:1::5]",
+		},
+	}
+
+	for i := range testCases {
+		got := AppendPortIfNeeded(testCases[i].addr, testCases[i].port)
+		if testCases[i].expect != got {
+			t.Errorf("case %s: expected %v, got %v", testCases[i].name, testCases[i].expect, got)
+		}
+	}
+}