Merge pull request #76320 from JieJhih/config/kube_proxy

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"
@@ -213,8 +214,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.
@@ -359,38 +360,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)
+		}
+	}
+}