From 8abc54d2576037b92c0fcae7aa4c43128332801c Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Mon, 23 Apr 2018 00:16:13 +0200 Subject: [PATCH 1/2] make API.ControlPlaneEndpoint accept IP --- cmd/kubeadm/app/apis/kubeadm/types.go | 12 +- .../app/apis/kubeadm/v1alpha1/types.go | 12 +- .../kubeadm/validation/validation_test.go | 120 ++++- .../app/phases/certs/pkiutil/pki_helpers.go | 13 +- .../phases/certs/pkiutil/pki_helpers_test.go | 89 ++-- .../app/phases/kubeconfig/kubeconfig_test.go | 4 +- cmd/kubeadm/app/util/endpoint.go | 130 +++-- cmd/kubeadm/app/util/endpoint_test.go | 463 +++++++++--------- 8 files changed, 530 insertions(+), 313 deletions(-) diff --git a/cmd/kubeadm/app/apis/kubeadm/types.go b/cmd/kubeadm/app/apis/kubeadm/types.go index 9bf657f70fa..f45e5e42d8b 100644 --- a/cmd/kubeadm/app/apis/kubeadm/types.go +++ b/cmd/kubeadm/app/apis/kubeadm/types.go @@ -133,7 +133,17 @@ type MasterConfiguration struct { type API struct { // AdvertiseAddress sets the IP address for the API server to advertise. AdvertiseAddress string - // ControlPlaneEndpoint sets the DNS address with optional port for the API server + // ControlPlaneEndpoint sets a stable IP address or DNS name for the control plane; it + // can be a valid IP address or a RFC-1123 DNS subdomain, both with optional TCP port. + // In case the ControlPlaneEndpoint is not specified, the AdvertiseAddress + BindPort + // are used; in case the ControlPlaneEndpoint is specified but without a TCP port, + // the BindPort is used. + // Possible usages are: + // e.g. In an cluster with more than one control plane instances, this field should be + // assigned the address of the external load balancer in front of the + // control plane instances. + // e.g. in environments with enforced node recycling, the ControlPlaneEndpoint + // could be used for assigning a stable DNS to the control plane. ControlPlaneEndpoint string // BindPort sets the secure port for the API Server to bind to. // Defaults to 6443. diff --git a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/types.go b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/types.go index e677023dbe6..5eb5eea73a4 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1alpha1/types.go +++ b/cmd/kubeadm/app/apis/kubeadm/v1alpha1/types.go @@ -125,7 +125,17 @@ type MasterConfiguration struct { type API struct { // AdvertiseAddress sets the IP address for the API server to advertise. AdvertiseAddress string `json:"advertiseAddress"` - // ControlPlaneEndpoint sets the DNS address for the API server + // ControlPlaneEndpoint sets a stable IP address or DNS name for the control plane; it + // can be a valid IP address or a RFC-1123 DNS subdomain, both with optional TCP port. + // In case the ControlPlaneEndpoint is not specified, the AdvertiseAddress + BindPort + // are used; in case the ControlPlaneEndpoint is specified but without a TCP port, + // the BindPort is used. + // Possible usages are: + // e.g. In an cluster with more than one control plane instances, this field should be + // assigned the address of the external load balancer in front of the + // control plane instances. + // e.g. in environments with enforced node recycling, the ControlPlaneEndpoint + // could be used for assigning a stable DNS to the control plane. ControlPlaneEndpoint string `json:"controlPlaneEndpoint"` // BindPort sets the secure port for the API Server to bind to. // Defaults to 6443. diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index 87d82d1fda8..3c621dc4089 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go @@ -275,7 +275,73 @@ func TestValidateAPIEndpoint(t *testing.T) { expected: false, }, { - name: "Valid IPv4 address and default port", + name: "Valid DNS ControlPlaneEndpoint (with port), AdvertiseAddress and default port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "cp.k8s.io:8081", + AdvertiseAddress: "4.5.6.7", + BindPort: 6443, + }, + }, + expected: true, + }, + { + name: "Valid IPv4 ControlPlaneEndpoint (with port), AdvertiseAddress and default port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "1.2.3.4:8081", + AdvertiseAddress: "4.5.6.7", + BindPort: 6443, + }, + }, + expected: true, + }, + { + name: "Valid IPv6 ControlPlaneEndpoint (with port), ControlPlaneEndpoint and port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "[2001:db7::1]:8081", + AdvertiseAddress: "2001:db7::2", + BindPort: 6443, + }, + }, + expected: true, + }, + { + name: "Valid DNS ControlPlaneEndpoint (without port), AdvertiseAddress and default port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "cp.k8s.io", + AdvertiseAddress: "4.5.6.7", + BindPort: 6443, + }, + }, + expected: true, + }, + { + name: "Valid IPv4 ControlPlaneEndpoint (without port), AdvertiseAddress and default port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "1.2.3.4", + AdvertiseAddress: "4.5.6.7", + BindPort: 6443, + }, + }, + expected: true, + }, + { + name: "Valid IPv6 ControlPlaneEndpoint (without port), ControlPlaneEndpoint and port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "2001:db7::1", + AdvertiseAddress: "2001:db7::2", + BindPort: 6443, + }, + }, + expected: true, + }, + { + name: "Valid IPv4 AdvertiseAddress and default port", s: &kubeadm.MasterConfiguration{ API: kubeadm.API{ AdvertiseAddress: "1.2.3.4", @@ -285,7 +351,7 @@ func TestValidateAPIEndpoint(t *testing.T) { expected: true, }, { - name: "Valid IPv6 address and port", + name: "Valid IPv6 AdvertiseAddress and port", s: &kubeadm.MasterConfiguration{ API: kubeadm.API{ AdvertiseAddress: "2001:db7::1", @@ -295,7 +361,7 @@ func TestValidateAPIEndpoint(t *testing.T) { expected: true, }, { - name: "Invalid IPv4 address", + name: "Invalid IPv4 AdvertiseAddress", s: &kubeadm.MasterConfiguration{ API: kubeadm.API{ AdvertiseAddress: "1.2.34", @@ -305,7 +371,7 @@ func TestValidateAPIEndpoint(t *testing.T) { expected: false, }, { - name: "Invalid IPv6 address", + name: "Invalid IPv6 AdvertiseAddress", s: &kubeadm.MasterConfiguration{ API: kubeadm.API{ AdvertiseAddress: "2001:db7:1", @@ -314,6 +380,52 @@ func TestValidateAPIEndpoint(t *testing.T) { }, expected: false, }, + { + name: "Invalid BindPort", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + AdvertiseAddress: "1.2.3.4", + BindPort: 0, + }, + }, + expected: false, + }, + { + name: "Invalid DNS ControlPlaneEndpoint", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "bad!!.k8s.io", + }, + }, + expected: false, + }, + { + name: "Invalid ipv4 ControlPlaneEndpoint", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "1..3.4", + }, + }, + expected: false, + }, + { + name: "Invalid ipv6 ControlPlaneEndpoint", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "1200::AB00:1234::2552:7777:1313", + }, + }, + expected: false, + }, + { + name: "Invalid ControlPlaneEndpoint port", + s: &kubeadm.MasterConfiguration{ + API: kubeadm.API{ + ControlPlaneEndpoint: "1.2.3.4:0", + }, + }, + expected: false, + }, } for _, rt := range tests { actual := ValidateAPIEndpoint(&rt.s.API, nil) diff --git a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go index 996688ee685..6cebbe3a92e 100644 --- a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go +++ b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers.go @@ -29,6 +29,7 @@ import ( certutil "k8s.io/client-go/util/cert" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/pkg/registry/core/service/ipallocator" ) @@ -286,9 +287,17 @@ func GetAPIServerAltNames(cfg *kubeadmapi.MasterConfiguration) (*certutil.AltNam }, } - // add api server dns advertise address + // add api server controlPlaneEndpoint if present (dns or ip) if len(cfg.API.ControlPlaneEndpoint) > 0 { - altNames.DNSNames = append(altNames.DNSNames, cfg.API.ControlPlaneEndpoint) + if host, _, err := kubeadmutil.ParseHostPort(cfg.API.ControlPlaneEndpoint); err == nil { + if ip := net.ParseIP(host); ip != nil { + altNames.IPs = append(altNames.IPs, ip) + } else { + altNames.DNSNames = append(altNames.DNSNames, host) + } + } else { + return nil, fmt.Errorf("error parsing API api.controlPlaneEndpoint %q: %s", cfg.API.ControlPlaneEndpoint, err) + } } appendSANsToAltNames(altNames, cfg.APIServerCertSANs, kubeadmconstants.APIServerCertName) diff --git a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go index ba4057c6c92..2ff497c1550 100644 --- a/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go +++ b/cmd/kubeadm/app/phases/certs/pkiutil/pki_helpers_test.go @@ -436,48 +436,69 @@ func TestPathForPublicKey(t *testing.T) { } func TestGetAPIServerAltNames(t *testing.T) { - hostname := "valid-hostname" - advertiseIP := "1.2.3.4" - controlPlaneEndpoint := "api.k8s.io" - cfg := &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{AdvertiseAddress: advertiseIP, ControlPlaneEndpoint: controlPlaneEndpoint}, - Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, - NodeName: hostname, - APIServerCertSANs: []string{"10.1.245.94", "10.1.245.95", "1.2.3.L", "invalid,commas,in,DNS"}, + + var tests = []struct { + name string + cfg *kubeadmapi.MasterConfiguration + expectedDNSNames []string + expectedIPAddresses []string + }{ + { + name: "ControlPlaneEndpoint DNS", + cfg: &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4", ControlPlaneEndpoint: "api.k8s.io:6443"}, + Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, + NodeName: "valid-hostname", + APIServerCertSANs: []string{"10.1.245.94", "10.1.245.95", "1.2.3.L", "invalid,commas,in,DNS"}, + }, + expectedDNSNames: []string{"valid-hostname", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local", "api.k8s.io"}, + expectedIPAddresses: []string{"10.96.0.1", "1.2.3.4", "10.1.245.94", "10.1.245.95"}, + }, + { + name: "ControlPlaneEndpoint IP", + cfg: &kubeadmapi.MasterConfiguration{ + API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4", ControlPlaneEndpoint: "4.5.6.7:6443"}, + Networking: kubeadmapi.Networking{ServiceSubnet: "10.96.0.0/12", DNSDomain: "cluster.local"}, + NodeName: "valid-hostname", + APIServerCertSANs: []string{"10.1.245.94", "10.1.245.95", "1.2.3.L", "invalid,commas,in,DNS"}, + }, + expectedDNSNames: []string{"valid-hostname", "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local"}, + expectedIPAddresses: []string{"10.96.0.1", "1.2.3.4", "10.1.245.94", "10.1.245.95", "4.5.6.7"}, + }, } - altNames, err := GetAPIServerAltNames(cfg) - if err != nil { - t.Fatalf("failed calling GetAPIServerAltNames: %v", err) - } + for _, rt := range tests { + altNames, err := GetAPIServerAltNames(rt.cfg) + if err != nil { + t.Fatalf("failed calling GetAPIServerAltNames: %s: %v", rt.name, err) + } - expectedDNSNames := []string{hostname, "kubernetes", "kubernetes.default", "kubernetes.default.svc", "kubernetes.default.svc.cluster.local", controlPlaneEndpoint} - for _, DNSName := range expectedDNSNames { - found := false - for _, val := range altNames.DNSNames { - if val == DNSName { - found = true - break + for _, DNSName := range rt.expectedDNSNames { + found := false + for _, val := range altNames.DNSNames { + if val == DNSName { + found = true + break + } + } + + if !found { + t.Errorf("%s: altNames does not contain DNSName %s but %v", rt.name, DNSName, altNames.DNSNames) } } - if !found { - t.Errorf("altNames does not contain DNSName %s", DNSName) - } - } - - expectedIPAddresses := []string{"10.96.0.1", advertiseIP, "10.1.245.94", "10.1.245.95"} - for _, IPAddress := range expectedIPAddresses { - found := false - for _, val := range altNames.IPs { - if val.Equal(net.ParseIP(IPAddress)) { - found = true - break + for _, IPAddress := range rt.expectedIPAddresses { + found := false + for _, val := range altNames.IPs { + if val.Equal(net.ParseIP(IPAddress)) { + found = true + break + } } - } - if !found { - t.Errorf("altNames does not contain IPAddress %s", IPAddress) + if !found { + t.Errorf("%s: altNames does not contain IPAddress %s but %v", rt.name, IPAddress, altNames.IPs) + } } } } diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go index 9b6fccd5cb7..3b47f4e627f 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go @@ -72,12 +72,12 @@ func TestGetKubeConfigSpecs(t *testing.T) { NodeName: "valid-node-name", }, { - API: kubeadmapi.API{ControlPlaneEndpoint: "api.k8s.io", BindPort: 1234}, + API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4", ControlPlaneEndpoint: "api.k8s.io", BindPort: 1234}, CertificatesDir: pkidir, NodeName: "valid-node-name", }, { - API: kubeadmapi.API{ControlPlaneEndpoint: "api.k8s.io:4321", BindPort: 1234}, + API: kubeadmapi.API{AdvertiseAddress: "1.2.3.4", ControlPlaneEndpoint: "api.k8s.io:4321", BindPort: 1234}, CertificatesDir: pkidir, NodeName: "valid-node-name", }, diff --git a/cmd/kubeadm/app/util/endpoint.go b/cmd/kubeadm/app/util/endpoint.go index ed2c2eb7fb9..1e46ad9e6d5 100644 --- a/cmd/kubeadm/app/util/endpoint.go +++ b/cmd/kubeadm/app/util/endpoint.go @@ -19,67 +19,101 @@ package util import ( "fmt" "net" + "net/url" "strconv" - "strings" "k8s.io/apimachinery/pkg/util/validation" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" ) -// GetMasterEndpoint returns a properly formatted Master Endpoint -// or passes the error from GetMasterHostPort. +// GetMasterEndpoint returns a properly formatted endpoint for the control plane built according following rules: +// - If the api.ControlPlaneEndpoint is defined, use it. +// - if the api.ControlPlaneEndpoint is defined but without a port number, use the api.ControlPlaneEndpoint + api.BindPort is used. +// - Otherwise, in case the api.ControlPlaneEndpoint is not defined, use the api.AdvertiseAddress + the api.BindPort. func GetMasterEndpoint(api *kubeadmapi.API) (string, error) { - - hostPort, err := GetMasterHostPort(api) - if err != nil { - return "", err + // parse the bind port + var bindPort = strconv.Itoa(int(api.BindPort)) + if _, err := parsePort(bindPort); err != nil { + return "", fmt.Errorf("invalid value %q given for api.bindPort: %s", api.BindPort, err) } - return fmt.Sprintf("https://%s", hostPort), nil -} -// GetMasterHostPort returns a properly formatted Master hostname or IP and port pair, or error -// if the hostname or IP address can not be parsed or port is outside the valid TCP range. -func GetMasterHostPort(api *kubeadmapi.API) (string, error) { - var masterIP string - var portStr string + // parse the AdvertiseAddress + var ip = net.ParseIP(api.AdvertiseAddress) + if ip == nil { + return "", fmt.Errorf("invalid value `%s` given for api.advertiseAddress", api.AdvertiseAddress) + } + + // set the master url using cfg.API.AdvertiseAddress + the cfg.API.BindPort + masterURL := &url.URL{ + Scheme: "https", + Host: net.JoinHostPort(ip.String(), bindPort), + } + + // if the controlplane endpoint is defined if len(api.ControlPlaneEndpoint) > 0 { - if strings.Contains(api.ControlPlaneEndpoint, ":") { - var err error - masterIP, portStr, err = net.SplitHostPort(api.ControlPlaneEndpoint) - if err != nil { - return "", fmt.Errorf("invalid value `%s` given for `ControlPlaneEndpoint`: %s", api.ControlPlaneEndpoint, err) - } + // parse the controlplane endpoint + var host, port string + var err error + if host, port, err = ParseHostPort(api.ControlPlaneEndpoint); err != nil { + return "", fmt.Errorf("invalid value %q given for api.controlPlaneEndpoint: %s", api.ControlPlaneEndpoint, err) + } + + // if a port is provided within the controlPlaneAddress warn the users we are using it, else use the bindport + if port != "" { + fmt.Println("[endpoint] WARNING: port specified in api.controlPlaneEndpoint overrides api.bindPort in the controlplane address") } else { - masterIP = api.ControlPlaneEndpoint + port = bindPort } - errs := validation.IsDNS1123Subdomain(masterIP) - if len(errs) > 0 { - return "", fmt.Errorf("error parsing `ControlPlaneEndpoint` to valid dns subdomain with errors: %s", errs) + + // overrides the master url using the controlPlaneAddress (and eventually the bindport) + masterURL = &url.URL{ + Scheme: "https", + Host: net.JoinHostPort(host, port), } - } else { - ip := net.ParseIP(api.AdvertiseAddress) - if ip == nil { - return "", fmt.Errorf("error parsing address %s", api.AdvertiseAddress) - } - masterIP = ip.String() } - var port int32 - 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 = 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 + return masterURL.String(), nil +} + +// ParseHostPort parses a network address of the form "host:port", "ipv4:port", "[ipv6]:port" into host and port; +// ":port" can be eventually omitted. +// If the string is not a valid representation of network address, ParseHostPort returns an error. +func ParseHostPort(hostport string) (string, string, error) { + var host, port string + var err error + + // try to split host and port + if host, port, err = net.SplitHostPort(hostport); err != nil { + // if SplitHostPort returns an error, the entire hostport is considered as host + host = hostport + } + + // if port is defined, parse and validate it + if port != "" { + if _, err := parsePort(port); err != nil { + return "", "", fmt.Errorf("port must be a valid number between 1 and 65535, inclusive") + } + } + + // if host is a valid IP, returns it + if ip := net.ParseIP(host); ip != nil { + return host, port, nil + } + + // if host is a validate RFC-1123 subdomain, returns it + if errs := validation.IsDNS1123Subdomain(host); len(errs) == 0 { + return host, port, nil + } + + return "", "", fmt.Errorf("host must be a valid IP address or a valid RFC-1123 DNS subdomain") +} + +// ParsePort parses a string representing a TCP port. +// If the string is not a valid representation of a TCP port, ParsePort returns an error. +func parsePort(port string) (int, error) { + if portInt, err := strconv.Atoi(port); err == nil && (1 <= portInt && portInt <= 65535) { + return portInt, nil + } + + return 0, fmt.Errorf("port must be a valid number between 1 and 65535, inclusive") } diff --git a/cmd/kubeadm/app/util/endpoint_test.go b/cmd/kubeadm/app/util/endpoint_test.go index ba37d1d1989..1bf738c0962 100644 --- a/cmd/kubeadm/app/util/endpoint_test.go +++ b/cmd/kubeadm/app/util/endpoint_test.go @@ -24,301 +24,322 @@ import ( func TestGetMasterEndpoint(t *testing.T) { var tests = []struct { - name string - cfg *kubeadmapi.MasterConfiguration - endpoint string - expected bool + name string + api *kubeadmapi.API + expectedEndpoint string + expectedError bool }{ { - name: "bad controlplane endpooint dns", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "bad!!cp.k8s.io", - BindPort: 1234, - }, + name: "use ControlPlaneEndpoint (dns) if fully defined", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "cp.k8s.io:1234", + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:1234", - expected: false, + expectedEndpoint: "https://cp.k8s.io:1234", }, { - name: "both DNS and IP passed", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - ControlPlaneEndpoint: "cp.k8s.io", - BindPort: 1234, - }, + name: "use ControlPlaneEndpoint (ipv4) if fully defined", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "1.2.3.4:1234", + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:1234", - expected: true, + expectedEndpoint: "https://1.2.3.4:1234", }, { - name: "valid DNS endpoint", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "cp.k8s.io", - BindPort: 1234, - }, + name: "use ControlPlaneEndpoint (ipv6) if fully defined", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "[2001:db8::1]:1234", + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:1234", - expected: true, + expectedEndpoint: "https://[2001:db8::1]:1234", }, { - name: "valid DNS endpoint with port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "cp.k8s.io:443", - BindPort: 1234, - }, + name: "use ControlPlaneEndpoint (dns) + BindPort if ControlPlaneEndpoint defined without port", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "cp.k8s.io", + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:443", - expected: true, + expectedEndpoint: "https://cp.k8s.io:4567", }, { - 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, - }, + name: "use ControlPlaneEndpoint (ipv4) + BindPort if ControlPlaneEndpoint defined without port", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "1.2.3.4", + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:443", - expected: true, + expectedEndpoint: "https://1.2.3.4:4567", }, { - name: "DNS endpoint with malformed port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "cp.k8s.io:443:443", - BindPort: 1234, - }, + name: "use ControlPlaneEndpoint (ipv6) + BindPort if ControlPlaneEndpoint defined without port", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "2001:db8::1", + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:443:443", - expected: false, + expectedEndpoint: "https://[2001:db8::1]:4567", }, { - name: "DNS endpoint with colon and missing port uses bind port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "cp.k8s.io:", - BindPort: 1234, - }, + name: "use AdvertiseAddress (ipv4) + BindPort if ControlPlaneEndpoint is not defined", + api: &kubeadmapi.API{ + BindPort: 4567, + AdvertiseAddress: "4.5.6.7", }, - endpoint: "https://cp.k8s.io:1234", - expected: true, + expectedEndpoint: "https://4.5.6.7:4567", }, { - name: "DNS endpoint with non numeric port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "cp.k8s.io:port", - BindPort: 1234, - }, + name: "use AdvertiseAddress (ipv6) + BindPort if ControlPlaneEndpoint is not defined", + api: &kubeadmapi.API{ + BindPort: 4567, + AdvertiseAddress: "2001:db8::1", }, - endpoint: "https://cp.k8s.io:port", - expected: false, + expectedEndpoint: "https://[2001:db8::1]:4567", }, { - name: "DNS endpoint with invalid port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - ControlPlaneEndpoint: "cp.k8s.io:987654321", - BindPort: 1234, - }, + name: "fail if invalid BindPort", + api: &kubeadmapi.API{ + BindPort: 0, }, - endpoint: "https://cp.k8s.io:987654321", - expected: false, + expectedError: true, }, { - name: "valid IPv4 endpoint", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - BindPort: 1234, - }, + name: "fail if invalid ControlPlaneEndpoint (dns)", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "bad!!.cp.k8s.io", + BindPort: 4567, }, - endpoint: "https://1.2.3.4:1234", - expected: true, + expectedError: true, }, { - name: "valid IPv6 endpoint", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "2001:db8::1", - BindPort: 4321, - }, + name: "fail if invalid ControlPlaneEndpoint (ip4)", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "1..0", + BindPort: 4567, }, - endpoint: "https://[2001:db8::1]:4321", - expected: true, + expectedError: true, }, { - name: "invalid IPv4 endpoint", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - BindPort: 1234, - }, + name: "fail if invalid ControlPlaneEndpoint (ip6)", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "1200::AB00:1234::2552:7777:1313", + BindPort: 4567, }, - endpoint: "https://[1.2.3.4]:1234", - expected: false, + expectedError: true, }, { - name: "invalid IPv6 endpoint", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "2001:db8::1", - BindPort: 4321, - }, + name: "fail if invalid ControlPlaneEndpoint (port)", + api: &kubeadmapi.API{ + ControlPlaneEndpoint: "cp.k8s.io:0", + BindPort: 4567, }, - endpoint: "https://2001:db8::1:4321", - expected: false, + expectedError: true, }, { - name: "invalid IPv4 AdvertiseAddress", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.34", - BindPort: 1234, - }, + name: "fail if invalid AdvertiseAddress (ip4)", + api: &kubeadmapi.API{ + AdvertiseAddress: "1..0", + BindPort: 4567, }, - endpoint: "https://1.2.3.4:1234", - expected: false, + expectedError: true, }, { - name: "invalid IPv6 AdvertiseAddress", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "2001::db8::1", - BindPort: 4321, - }, + name: "fail if invalid AdvertiseAddress (ip6)", + api: &kubeadmapi.API{ + AdvertiseAddress: "1200::AB00:1234::2552:7777:1313", + BindPort: 4567, }, - endpoint: "https://[2001:db8::1]:4321", - expected: false, + expectedError: true, }, } + for _, rt := range tests { - actual, err := GetMasterEndpoint(&rt.cfg.API) - if err != nil && rt.expected { - t.Error(err) + actualEndpoint, actualError := GetMasterEndpoint(rt.api) + + if (actualError != nil) && !rt.expectedError { + t.Errorf("%s unexpected failure: %v", rt.name, actualError) + continue + } else if (actualError == nil) && rt.expectedError { + t.Errorf("%s passed when expected to fail", rt.name) + continue } - if actual != rt.endpoint && rt.expected { - t.Errorf( - "%s test case failed:\n\texpected: %s\n\t actual: %s", - rt.name, - rt.endpoint, - (actual), - ) + + if actualEndpoint != rt.expectedEndpoint { + t.Errorf("%s returned invalid endpoint %s, expected %s", rt.name, actualEndpoint, rt.expectedEndpoint) } } } -func TestGetMasterHostPort(t *testing.T) { +func TestParseHostPort(t *testing.T) { + var tests = []struct { - name string - cfg *kubeadmapi.MasterConfiguration - hostPort string - expected bool + name string + hostport string + expectedHost string + expectedPort string + expectedError bool }{ { - name: "valid IPv4 master host and port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - BindPort: 1234, - }, - }, - hostPort: "1.2.3.4:1234", - expected: true, + name: "valid dns", + hostport: "cp.k8s.io", + expectedHost: "cp.k8s.io", + expectedPort: "", }, { - name: "valid IPv6 master host port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "2001:db8::1", - BindPort: 4321, - }, - }, - hostPort: "[2001:db8::1]:4321", - expected: true, + name: "valid dns:port", + hostport: "cp.k8s.io:1234", + expectedHost: "cp.k8s.io", + expectedPort: "1234", }, { - name: "invalid IPv4 address", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.34", - BindPort: 1234, - }, - }, - hostPort: "1.2.3.4:1234", - expected: false, + name: "valid ip4", + hostport: "1.2.3.4", + expectedHost: "1.2.3.4", + expectedPort: "", }, { - name: "invalid IPv6 address", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "2001::db8::1", - BindPort: 4321, - }, - }, - hostPort: "[2001:db8::1]:4321", - expected: false, + name: "valid ipv4:port", + hostport: "1.2.3.4:1234", + expectedHost: "1.2.3.4", + expectedPort: "1234", }, { - name: "invalid TCP port number", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - BindPort: 987654321, - }, - }, - hostPort: "1.2.3.4:987654321", - expected: false, + name: "valid ipv6", + hostport: "2001:db8::1", + expectedHost: "2001:db8::1", + expectedPort: "", }, { - name: "invalid negative TCP port number", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - BindPort: -987654321, - }, - }, - hostPort: "1.2.3.4:-987654321", - expected: false, + name: "valid ipv6:port", + hostport: "[2001:db8::1]:1234", + expectedHost: "2001:db8::1", + expectedPort: "1234", }, { - name: "unspecified IPv4 TCP port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1.2.3.4", - }, - }, - hostPort: "1.2.3.4:0", - expected: true, + name: "invalid port(not a number)", + hostport: "cp.k8s.io:aaa", + expectedError: true, }, { - name: "unspecified IPv6 TCP port", - cfg: &kubeadmapi.MasterConfiguration{ - API: kubeadmapi.API{ - AdvertiseAddress: "1:2:3::4", - }, - }, - hostPort: "[1:2:3::4]:0", - expected: true, + name: "invalid port(out of range, positive port number)", + hostport: "cp.k8s.io:987654321", + expectedError: true, + }, + { + name: "invalid port(out of range, negative port number)", + hostport: "cp.k8s.io:-987654321", + expectedError: true, + }, + { + name: "invalid port(out of range, negative port number)", + hostport: "cp.k8s.io:123:123", + expectedError: true, + }, + { + name: "invalid dns", + hostport: "bad!!cp.k8s.io", + expectedError: true, + }, + { + name: "invalid valid dns:port", + hostport: "bad!!cp.k8s.io:1234", + expectedError: true, + }, + { + name: "invalid ip4, but valid DNS", + hostport: "259.2.3.4", + expectedHost: "259.2.3.4", + }, + { + name: "invalid ip4", + hostport: "1..3.4", + expectedError: true, + }, + { + name: "invalid ip4(2):port", + hostport: "1..3.4:1234", + expectedError: true, + }, + { + name: "invalid ipv6", + hostport: "1200::AB00:1234::2552:7777:1313", + expectedError: true, + }, + { + name: "invalid ipv6:port", + hostport: "[1200::AB00:1234::2552:7777:1313]:1234", + expectedError: true, }, } + for _, rt := range tests { - actual, err := GetMasterHostPort(&rt.cfg.API) - if err != nil && rt.expected { - t.Error(err) + actualHost, actualPort, actualError := ParseHostPort(rt.hostport) + + if (actualError != nil) && !rt.expectedError { + t.Errorf("%s unexpected failure: %v", rt.name, actualError) + continue + } else if (actualError == nil) && rt.expectedError { + t.Errorf("%s passed when expected to fail", rt.name) + continue } - if actual != rt.hostPort && rt.expected { - t.Errorf( - "%s test case failed:\n\texpected: %s\n\t actual: %s", - rt.name, - rt.hostPort, - (actual), - ) + + if actualHost != rt.expectedHost { + t.Errorf("%s returned invalid host %s, expected %s", rt.name, actualHost, rt.expectedHost) + continue + } + + if actualPort != rt.expectedPort { + t.Errorf("%s returned invalid port %s, expected %s", rt.name, actualPort, rt.expectedPort) + } + } +} + +func TestParsePort(t *testing.T) { + + var tests = []struct { + name string + port string + expectedPort int + expectedError bool + }{ + { + name: "valid port", + port: "1234", + expectedPort: 1234, + }, + { + name: "invalid port (not a number)", + port: "a", + expectedError: true, + }, + { + name: "invalid port (<1)", + port: "-10", + expectedError: true, + }, + { + name: "invalid port (>65535)", + port: "66535", + expectedError: true, + }, + } + + for _, rt := range tests { + actualPort, actualError := parsePort(rt.port) + + if (actualError != nil) && !rt.expectedError { + t.Errorf("%s unexpected failure: %v", rt.name, actualError) + continue + } else if (actualError == nil) && rt.expectedError { + t.Errorf("%s passed when expected to fail", rt.name) + continue + } + + if actualPort != rt.expectedPort { + t.Errorf("%s returned invalid port %d, expected %d", rt.name, actualPort, rt.expectedPort) } } } From 8f838d9e424786d71de4bd3f7dcc6ee56234d91d Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Mon, 23 Apr 2018 00:16:30 +0200 Subject: [PATCH 2/2] autogenerated files --- cmd/kubeadm/app/phases/certs/pkiutil/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/kubeadm/app/phases/certs/pkiutil/BUILD b/cmd/kubeadm/app/phases/certs/pkiutil/BUILD index f4c8a743178..28d664faf23 100644 --- a/cmd/kubeadm/app/phases/certs/pkiutil/BUILD +++ b/cmd/kubeadm/app/phases/certs/pkiutil/BUILD @@ -23,6 +23,7 @@ go_library( deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", "//cmd/kubeadm/app/constants:go_default_library", + "//cmd/kubeadm/app/util:go_default_library", "//pkg/registry/core/service/ipallocator:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//vendor/k8s.io/client-go/util/cert:go_default_library",