Merge pull request #62314 from rjosephwright/endpoint-port

Automatic merge from submit-queue (batch tested with PRs 58540, 62314). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add ability to specify port for kubeadm `API.ControlPlaneEndpoint`

**What this PR does / why we need it**:

When `API.ControlPlaneEndpoint` is used, the `BindPort` of the
apiserver is currently assumed, which means a load balancer cannot
listen on a different port than the apiserver. This extends the
`ControlPlaneEndpoint` to take an optional port which may differ
from the apiserver's `BindPort`.

**Release note**:

```release-note
The kubeadm config option `API.ControlPlaneEndpoint` has been extended to take an optional port which may differ from the apiserver's bind port.
```

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -133,7 +133,7 @@ type MasterConfiguration struct {
 type API struct {
 	// AdvertiseAddress sets the IP address for the API server to advertise.
 	AdvertiseAddress string
-	// ControlPlaneEndpoint sets the DNS address for the API server
+	// ControlPlaneEndpoint sets the DNS address with optional port for the API server
 	ControlPlaneEndpoint string
 	// BindPort sets the secure port for the API Server to bind to.
 	// Defaults to 6443.

cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go

@@ -64,61 +64,76 @@ func TestGetKubeConfigSpecs(t *testing.T) {
 	// Adds a pki folder with a ca certs to the temp folder
 	pkidir := testutil.SetupPkiDirWithCertificateAuthorithy(t, tmpdir)
 
-	// Creates a Master Configuration pointing to the pkidir folder
+	// Creates Master Configurations pointing to the pkidir folder
-	cfg := &kubeadmapi.MasterConfiguration{
+	cfgs := []*kubeadmapi.MasterConfiguration{
-		API:             kubeadmapi.API{AdvertiseAddress: "1.2.3.4", BindPort: 1234},
-		CertificatesDir: pkidir,
-		NodeName:        "valid-node-name",
-	}
-
-	// Creates a Master Configuration pointing to the pkidir folder
-	cfgDNS := &kubeadmapi.MasterConfiguration{
-		API:             kubeadmapi.API{ControlPlaneEndpoint: "api.k8s.io", BindPort: 1234},
-		CertificatesDir: pkidir,
-		NodeName:        "valid-node-name",
-	}
-
-	// Executes getKubeConfigSpecs
-	specs, err := getKubeConfigSpecs(cfg)
-	if err != nil {
-		t.Fatal("getKubeConfigSpecs failed!")
-	}
-
-	// Executes getKubeConfigSpecs
-	specsDNS, err := getKubeConfigSpecs(cfgDNS)
-	if err != nil {
-		t.Fatal("getKubeConfigSpecs failed!")
-	}
-
-	var assertions = []struct {
-		kubeConfigFile string
-		clientName     string
-		organizations  []string
-	}{
 		{
-			kubeConfigFile: kubeadmconstants.AdminKubeConfigFileName,
+			API:             kubeadmapi.API{AdvertiseAddress: "1.2.3.4", BindPort: 1234},
-			clientName:     "kubernetes-admin",
+			CertificatesDir: pkidir,
-			organizations:  []string{kubeadmconstants.MastersGroup},
+			NodeName:        "valid-node-name",
 		},
 		{
-			kubeConfigFile: kubeadmconstants.KubeletKubeConfigFileName,
+			API:             kubeadmapi.API{ControlPlaneEndpoint: "api.k8s.io", BindPort: 1234},
-			clientName:     fmt.Sprintf("system:node:%s", cfg.NodeName),
+			CertificatesDir: pkidir,
-			organizations:  []string{kubeadmconstants.NodesGroup},
+			NodeName:        "valid-node-name",
 		},
 		{
-			kubeConfigFile: kubeadmconstants.ControllerManagerKubeConfigFileName,
+			API:             kubeadmapi.API{ControlPlaneEndpoint: "api.k8s.io:4321", BindPort: 1234},
-			clientName:     kubeadmconstants.ControllerManagerUser,
+			CertificatesDir: pkidir,
+			NodeName:        "valid-node-name",
 		},
 		{
-			kubeConfigFile: kubeadmconstants.SchedulerKubeConfigFileName,
+			API:             kubeadmapi.API{AdvertiseAddress: "1.2.3.4", ControlPlaneEndpoint: "api.k8s.io", BindPort: 1234},
-			clientName:     kubeadmconstants.SchedulerUser,
+			CertificatesDir: pkidir,
+			NodeName:        "valid-node-name",
+		},
+		{
+			API:             kubeadmapi.API{AdvertiseAddress: "1.2.3.4", ControlPlaneEndpoint: "api.k8s.io:4321", BindPort: 1234},
+			CertificatesDir: pkidir,
+			NodeName:        "valid-node-name",
 		},
 	}
 
-	for _, assertion := range assertions {
+	for _, cfg := range cfgs {
+		var assertions = []struct {
+			kubeConfigFile string
+			clientName     string
+			organizations  []string
+		}{
+			{
+				kubeConfigFile: kubeadmconstants.AdminKubeConfigFileName,
+				clientName:     "kubernetes-admin",
+				organizations:  []string{kubeadmconstants.MastersGroup},
+			},
+			{
+				kubeConfigFile: kubeadmconstants.KubeletKubeConfigFileName,
+				clientName:     fmt.Sprintf("system:node:%s", cfg.NodeName),
+				organizations:  []string{kubeadmconstants.NodesGroup},
+			},
+			{
+				kubeConfigFile: kubeadmconstants.ControllerManagerKubeConfigFileName,
+				clientName:     kubeadmconstants.ControllerManagerUser,
+			},
+			{
+				kubeConfigFile: kubeadmconstants.SchedulerKubeConfigFileName,
+				clientName:     kubeadmconstants.SchedulerUser,
+			},
+		}
 
-		// assert the spec for the kubeConfigFile exists
+		for _, assertion := range assertions {
-		if spec, ok := specs[assertion.kubeConfigFile]; ok {
+			// Executes getKubeConfigSpecs
+			specs, err := getKubeConfigSpecs(cfg)
+			if err != nil {
+				t.Fatal("getKubeConfigSpecs failed!")
+			}
+
+			var spec *kubeConfigSpec
+			var ok bool
+
+			// assert the spec for the kubeConfigFile exists
+			if spec, ok = specs[assertion.kubeConfigFile]; !ok {
+				t.Errorf("getKubeConfigSpecs didn't create spec for %s ", assertion.kubeConfigFile)
+				continue
+			}
 
 			// Assert clientName
 			if spec.ClientName != assertion.clientName {
@@ -146,41 +161,6 @@ func TestGetKubeConfigSpecs(t *testing.T) {
 			if spec.ClientCertAuth == nil || spec.ClientCertAuth.CAKey == nil {
 				t.Errorf("getKubeConfigSpecs didn't loaded CAKey into spec for %s!", assertion.kubeConfigFile)
 			}
-		} else {
-			t.Errorf("getKubeConfigSpecs didn't create spec for %s ", assertion.kubeConfigFile)
-		}
-
-		// assert the spec for the kubeConfigFile exists
-		if spec, ok := specsDNS[assertion.kubeConfigFile]; ok {
-
-			// Assert clientName
-			if spec.ClientName != assertion.clientName {
-				t.Errorf("getKubeConfigSpecs for %s clientName is %s, expected %s", assertion.kubeConfigFile, spec.ClientName, assertion.clientName)
-			}
-
-			// Assert Organizations
-			if spec.ClientCertAuth == nil || !reflect.DeepEqual(spec.ClientCertAuth.Organizations, assertion.organizations) {
-				t.Errorf("getKubeConfigSpecs for %s Organizations is %v, expected %v", assertion.kubeConfigFile, spec.ClientCertAuth.Organizations, assertion.organizations)
-			}
-
-			// Asserts MasterConfiguration values injected into spec
-			masterEndpoint, err := kubeadmutil.GetMasterEndpoint(cfgDNS)
-			if err != nil {
-				t.Error(err)
-			}
-			if spec.APIServer != masterEndpoint {
-				t.Errorf("getKubeConfigSpecs didn't injected cfg.APIServer endpoint into spec for %s", assertion.kubeConfigFile)
-			}
-
-			// Asserts CA certs and CA keys loaded into specs
-			if spec.CACert == nil {
-				t.Errorf("getKubeConfigSpecs didn't loaded CACert into spec for %s!", assertion.kubeConfigFile)
-			}
-			if spec.ClientCertAuth == nil || spec.ClientCertAuth.CAKey == nil {
-				t.Errorf("getKubeConfigSpecs didn't loaded CAKey into spec for %s!", assertion.kubeConfigFile)
-			}
-		} else {
-			t.Errorf("getKubeConfigSpecs didn't create spec for %s ", assertion.kubeConfigFile)
 		}
 	}
 }

cmd/kubeadm/app/util/endpoint.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net"
 	"strconv"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/util/validation"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -36,16 +37,25 @@ func GetMasterEndpoint(cfg *kubeadmapi.MasterConfiguration) (string, error) {
 	return fmt.Sprintf("https://%s", hostPort), nil
 }
 
-// GetMasterHostPort returns a properly formatted Master IP/port pair or error
+// GetMasterHostPort returns a properly formatted Master hostname or IP and port pair, or error
-// if the IP address can not be parsed or port is outside the valid TCP range.
+// if the hostname or IP address can not be parsed or port is outside the valid TCP range.
 func GetMasterHostPort(cfg *kubeadmapi.MasterConfiguration) (string, error) {
 	var masterIP string
+	var portStr string
 	if len(cfg.API.ControlPlaneEndpoint) > 0 {
-		errs := validation.IsDNS1123Subdomain(cfg.API.ControlPlaneEndpoint)
+		if strings.Contains(cfg.API.ControlPlaneEndpoint, ":") {
+			var err error
+			masterIP, portStr, err = net.SplitHostPort(cfg.API.ControlPlaneEndpoint)
+			if err != nil {
+				return "", fmt.Errorf("invalid value `%s` given for `ControlPlaneEndpoint`: %s", cfg.API.ControlPlaneEndpoint, err)
+			}
+		} else {
+			masterIP = cfg.API.ControlPlaneEndpoint
+		}
+		errs := validation.IsDNS1123Subdomain(masterIP)
 		if len(errs) > 0 {
 			return "", fmt.Errorf("error parsing `ControlPlaneEndpoint` to valid dns subdomain with errors: %s", errs)
 		}
-		masterIP = cfg.API.ControlPlaneEndpoint
 	} else {
 		ip := net.ParseIP(cfg.API.AdvertiseAddress)
 		if ip == nil {
@@ -54,10 +64,22 @@ func GetMasterHostPort(cfg *kubeadmapi.MasterConfiguration) (string, error) {
 		masterIP = ip.String()
 	}
 
-	if cfg.API.BindPort < 0 || cfg.API.BindPort > 65535 {
+	var port int32
-		return "", fmt.Errorf("api server port must be between 0 and 65535")
+	if len(portStr) > 0 {
+		portInt, err := strconv.Atoi(portStr)
+		if err != nil {
+			return "", fmt.Errorf("error parsing `ControlPlaneEndpoint` port value `%s`: %s", portStr, err.Error())
+		}
+		port = int32(portInt)
+		fmt.Println("[endpoint] WARNING: specifying a port for `ControlPlaneEndpoint` overrides `BindPort`")
+	} else {
+		port = cfg.API.BindPort
 	}
 
-	hostPort := net.JoinHostPort(masterIP, strconv.Itoa(int(cfg.API.BindPort)))
+	if port < 0 || port > 65535 {
+		return "", fmt.Errorf("api server port must be between 0 and 65535, %d was given", port)
+	}
+
+	hostPort := net.JoinHostPort(masterIP, strconv.Itoa(int(port)))
 	return hostPort, nil
 }

cmd/kubeadm/app/util/endpoint_test.go

@@ -63,6 +63,73 @@ func TestGetMasterEndpoint(t *testing.T) {
 			endpoint: "https://cp.k8s.io:1234",
 			expected: true,
 		},
+		{
+			name: "valid DNS endpoint with port",
+			cfg: &kubeadmapi.MasterConfiguration{
+				API: kubeadmapi.API{
+					ControlPlaneEndpoint: "cp.k8s.io:443",
+					BindPort:             1234,
+				},
+			},
+			endpoint: "https://cp.k8s.io:443",
+			expected: true,
+		},
+		{
+			name: "valid DNS endpoint and IP with port",
+			cfg: &kubeadmapi.MasterConfiguration{
+				API: kubeadmapi.API{
+					AdvertiseAddress:     "1.2.3.4",
+					ControlPlaneEndpoint: "cp.k8s.io:443",
+					BindPort:             1234,
+				},
+			},
+			endpoint: "https://cp.k8s.io:443",
+			expected: true,
+		},
+		{
+			name: "DNS endpoint with malformed port",
+			cfg: &kubeadmapi.MasterConfiguration{
+				API: kubeadmapi.API{
+					ControlPlaneEndpoint: "cp.k8s.io:443:443",
+					BindPort:             1234,
+				},
+			},
+			endpoint: "https://cp.k8s.io:443:443",
+			expected: false,
+		},
+		{
+			name: "DNS endpoint with colon and missing port uses bind port",
+			cfg: &kubeadmapi.MasterConfiguration{
+				API: kubeadmapi.API{
+					ControlPlaneEndpoint: "cp.k8s.io:",
+					BindPort:             1234,
+				},
+			},
+			endpoint: "https://cp.k8s.io:1234",
+			expected: true,
+		},
+		{
+			name: "DNS endpoint with non numeric port",
+			cfg: &kubeadmapi.MasterConfiguration{
+				API: kubeadmapi.API{
+					ControlPlaneEndpoint: "cp.k8s.io:port",
+					BindPort:             1234,
+				},
+			},
+			endpoint: "https://cp.k8s.io:port",
+			expected: false,
+		},
+		{
+			name: "DNS endpoint with invalid port",
+			cfg: &kubeadmapi.MasterConfiguration{
+				API: kubeadmapi.API{
+					ControlPlaneEndpoint: "cp.k8s.io:987654321",
+					BindPort:             1234,
+				},
+			},
+			endpoint: "https://cp.k8s.io:987654321",
+			expected: false,
+		},
 		{
 			name: "valid IPv4 endpoint",
 			cfg: &kubeadmapi.MasterConfiguration{