Reconcile external konnectivity address

cmd/controller/controller.go

@@ -6,6 +6,7 @@
 package controller
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"fmt"
@@ -537,15 +538,11 @@ func (c *command) start(ctx context.Context, flags *config.ControllerOptions, de
 	}
 
 	if enableKonnectivity {
-		konnectivityHost := nodeConfig.Spec.API.APIAddress()
-		if nodeConfig.Spec.Konnectivity.ExternalAddress != "" {
-			konnectivityHost = nodeConfig.Spec.Konnectivity.ExternalAddress
-		}
 		clusterComponents.Add(ctx, &controller.KonnectivityAgent{
-			K0sVars:       c.K0sVars,
-			APIServerHost: konnectivityHost,
-			EventEmitter:  prober.NewEventEmitter(),
-			ServerCount:   numActiveControllers.Peek,
+			ManifestsDir:           c.K0sVars.ManifestsDir,
+			KonnectivityServerHost: cmp.Or(nodeConfig.Spec.API.ExternalHost(), nodeConfig.Spec.API.Address),
+			EventEmitter:           prober.NewEventEmitter(),
+			ServerCount:            numActiveControllers.Peek,
 		})
 	}
 

internal/pkg/net/hostport.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"net"
+	"net/netip"
 	"strconv"
 
 	"github.com/asaskevich/govalidator"
@@ -23,10 +24,9 @@ func (h *HostPort) Host() string { return h.host }
 func (h *HostPort) Port() uint16 { return h.port }
 
 func NewHostPort(host string, port uint16) (*HostPort, error) {
-	if !govalidator.IsIP(host) && !govalidator.IsDNSName(host) {
-		return nil, errors.New("host is neither an IP address nor a DNS name")
+	if err := validateHost(host); err != nil {
+		return nil, err
 	}
-
 	if port == 0 {
 		return nil, errors.New("port is zero")
 	}
@@ -39,19 +39,35 @@ func ParseHostPort(hostPort string) (*HostPort, error) {
 }
 
 func ParseHostPortWithDefault(hostPort string, defaultPort uint16) (*HostPort, error) {
+	if _, err := netip.ParseAddr(hostPort); err == nil {
+		if defaultPort == 0 {
+			return nil, errors.New("missing port in address")
+		}
+		return &HostPort{hostPort, defaultPort}, nil
+	}
+
 	var port uint16
 	host, portStr, err := net.SplitHostPort(hostPort)
 	if err != nil {
 		addrErr := &net.AddrError{}
-		if errors.As(err, &addrErr) {
-			if defaultPort != 0 && addrErr.Err == "missing port in address" {
+		if !errors.As(err, &addrErr) {
+			return nil, err
+		}
+
+		if addrErr.Err == "missing port in address" {
+			if defaultPort != 0 {
 				host = addrErr.Addr
 				port = defaultPort
 			} else {
+				if _, ok := unwrapIPv6Literal(addrErr.Addr); !ok {
+					if err := validateHost(addrErr.Addr); err != nil {
+						return nil, err
+					}
+				}
 				return nil, errors.New(addrErr.Err)
 			}
 		} else {
-			return nil, err
+			return nil, errors.New(addrErr.Err)
 		}
 	} else {
 		parsed, err := strconv.ParseUint(portStr, 10, 16)
@@ -70,6 +86,10 @@ func ParseHostPortWithDefault(hostPort string, defaultPort uint16) (*HostPort, e
 		port = uint16(parsed)
 	}
 
+	if literal, ok := unwrapIPv6Literal(host); ok {
+		return &HostPort{literal, port}, nil
+	}
+
 	return NewHostPort(host, port)
 }
 
@@ -96,3 +116,25 @@ func (h *HostPort) UnmarshalText(text []byte) error {
 	*h = *parsed
 	return err
 }
+
+func unwrapIPv6Literal(host string) (string, bool) {
+	if len := len(host); len > 2 && host[0] == '[' && host[len-1] == ']' {
+		host := host[1 : len-1]
+		if addr, err := netip.ParseAddr(host); err == nil && addr.Is6() {
+			return host, true
+		}
+	}
+
+	return host, false
+}
+
+func validateHost(host string) error {
+	if govalidator.IsDNSName(host) {
+		return nil
+	}
+	if _, err := netip.ParseAddr(host); err == nil {
+		return nil
+	}
+
+	return errors.New("host is neither an IP address nor a DNS name")
+}

internal/pkg/net/hostport_test.go

@@ -19,6 +19,7 @@ func TestNewHostPort(t *testing.T) {
 	}{
 		{"ipv4", "127.0.0.1", 4711, "127.0.0.1", "127.0.0.1:4711"},
 		{"ipv6", "::1", 4711, "::1", "[::1]:4711"},
+		{"ipv6_zone", "fe80::1%eth0", 4711, "fe80::1%eth0", "[fe80::1%eth0]:4711"},
 		{"dns", "example.com", 4711, "example.com", "example.com:4711"},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -59,6 +60,7 @@ func TestParseHostPort(t *testing.T) {
 	}{
 		{"ipv4", "127.0.0.1:4711", "127.0.0.1", 4711, "127.0.0.1:4711"},
 		{"ipv6", "[::1]:4711", "::1", 4711, "[::1]:4711"},
+		{"ipv6_zone", "[fe80::1%eth0]:4711", "fe80::1%eth0", 4711, "[fe80::1%eth0]:4711"},
 		{"dns", "example.com:4711", "example.com", 4711, "example.com:4711"},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -74,6 +76,12 @@ func TestParseHostPort(t *testing.T) {
 	for _, test := range []struct{ name, hostPort, errMsg string }{
 		{"spaces", "f o o:4711", "host is neither an IP address nor a DNS name"},
 		{"missing_port", "foo", "missing port in address"},
+		{"missing_ipv4_port", "127.0.0.1", "missing port in address"},
+		{"missing_ipv6_port_unspecified", "::", "missing port in address"},
+		{"missing_ipv6_port_localhost", "::1", "missing port in address"},
+		{"missing_ipv6_port_full", "0:0:0:0:0:0:0:1", "missing port in address"},
+		{"missing_ipv6_port_host", "[::1]", "missing port in address"},
+		{"ipv4_address_as_ipv6_host", "[127.0.0.1]", "host is neither an IP address nor a DNS name"},
 		{"empty_port", "foo:", `port is not a positive number: ""`},
 		{"zero_port", "foo:0", "port is zero"},
 		{"negative_port", "foo:-1", `port is not a positive number: "-1"`},
@@ -90,16 +98,35 @@ func TestParseHostPort(t *testing.T) {
 }
 
 func TestParseHostPortWithDefault(t *testing.T) {
-	hostPort, err := net.ParseHostPortWithDefault("yep", 4711)
-	if assert.NoError(t, err) && assert.NotNil(t, hostPort) {
-		assert.Equal(t, "yep", hostPort.Host())
-		assert.Equal(t, uint16(4711), hostPort.Port())
-		assert.Equal(t, "yep:4711", hostPort.String())
-	}
+	for _, test := range []struct{ name, host, parsedHost, defaultHostPort, hostPort string }{
+		{"ipv4", "127.0.0.1", "127.0.0.1", "127.0.0.1:4711", "127.0.0.1:1337"},
+		{"ipv6_unspecified", "::", "::", "[::]:4711", "[::]:1337"},
+		{"ipv6_localhost", "::1", "::1", "[::1]:4711", "[::1]:1337"},
+		{"ipv6_full", "0:0:0:0:0:0:0:1", "0:0:0:0:0:0:0:1", "[0:0:0:0:0:0:0:1]:4711", "[0:0:0:0:0:0:0:1]:1337"},
+		{"ipv6_zone", "fe80::1%eth0", "fe80::1%eth0", "[fe80::1%eth0]:4711", "[fe80::1%eth0]:1337"},
+		{"ipv6_host", "[::1]", "::1", "[::1]:4711", "[::1]:1337"},
+		{"host", "yep", "yep", "yep:4711", "yep:1337"},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			hostPort, err := net.ParseHostPortWithDefault(test.host, 0)
+			assert.Nil(t, hostPort)
+			if assert.Error(t, err) {
+				assert.Equal(t, "missing port in address", err.Error())
+			}
 
-	hostPort, err = net.ParseHostPortWithDefault("yep", 0)
-	assert.Nil(t, hostPort)
-	if assert.Error(t, err) {
-		assert.Equal(t, "missing port in address", err.Error())
+			hostPort, err = net.ParseHostPortWithDefault(test.host, 4711)
+			if assert.NoError(t, err) && assert.NotNil(t, hostPort) {
+				assert.Equal(t, test.parsedHost, hostPort.Host())
+				assert.Equal(t, uint16(4711), hostPort.Port())
+				assert.Equal(t, test.defaultHostPort, hostPort.String())
+			}
+
+			hostPort, err = net.ParseHostPortWithDefault(test.hostPort, 4711)
+			if assert.NoError(t, err) && assert.NotNil(t, hostPort) {
+				assert.Equal(t, test.parsedHost, hostPort.Host())
+				assert.Equal(t, uint16(1337), hostPort.Port())
+				assert.Equal(t, test.hostPort, hostPort.String())
+			}
+		})
 	}
 }

pkg/apis/k0s/v1beta1/api.go

@@ -4,6 +4,7 @@
 package v1beta1
 
 import (
+	"cmp"
 	"encoding/json"
 	"fmt"
 	"net"
@@ -80,14 +81,6 @@ func (a *APISpec) LocalURL() *url.URL {
 	return &url.URL{Scheme: "https", Host: host}
 }
 
-// APIAddress ...
-func (a *APISpec) APIAddress() string {
-	if a.ExternalAddress != "" {
-		return a.ExternalHost()
-	}
-	return a.Address
-}
-
 func (a *APISpec) APIServerHostPort() (*k0snet.HostPort, error) {
 	if a.ExternalAddress != "" {
 		if ip := net.ParseIP(a.ExternalAddress); ip != nil {
@@ -134,8 +127,7 @@ func (a *APISpec) APIAddressURL() string {
 // If the address used to detect it, isn't an IP address but a hostname or if
 // both are unset, it will default to IPv4
 func (a *APISpec) DetectPrimaryAddressFamily() PrimaryAddressFamilyType {
-	addr := a.APIAddress()
-	if ip := net.ParseIP(addr); ip != nil && ip.To4() == nil {
+	if ip := net.ParseIP(cmp.Or(a.ExternalHost(), a.Address)); ip != nil && ip.To4() == nil {
 		return PrimaryFamilyIPv6
 	}
 	return PrimaryFamilyIPv4

pkg/component/controller/konnectivityagent.go

@@ -6,26 +6,24 @@ package controller
 import (
 	"context"
 	"fmt"
-	"net"
 	"path/filepath"
-	"strconv"
 	"time"
 
 	"github.com/sirupsen/logrus"
 
 	"github.com/k0sproject/k0s/internal/pkg/dir"
+	k0snet "github.com/k0sproject/k0s/internal/pkg/net"
 	"github.com/k0sproject/k0s/internal/pkg/templatewriter"
 	"github.com/k0sproject/k0s/pkg/apis/k0s/v1beta1"
 	"github.com/k0sproject/k0s/pkg/component/manager"
 	"github.com/k0sproject/k0s/pkg/component/prober"
-	"github.com/k0sproject/k0s/pkg/config"
 	"github.com/k0sproject/k0s/pkg/constant"
 )
 
 type KonnectivityAgent struct {
-	K0sVars       *config.CfgVars
-	APIServerHost string
-	ServerCount   func() (uint, <-chan struct{})
+	ManifestsDir           string
+	KonnectivityServerHost string
+	ServerCount            func() (uint, <-chan struct{})
 
 	configChangeChan chan *v1beta1.ClusterConfig
 	log              *logrus.Entry
@@ -101,37 +99,27 @@ func (k *KonnectivityAgent) Stop() error {
 }
 
 func (k *KonnectivityAgent) writeKonnectivityAgent(clusterConfig *v1beta1.ClusterConfig, serverCount uint) error {
-	konnectivityDir := filepath.Join(k.K0sVars.ManifestsDir, "konnectivity")
+	konnectivityDir := filepath.Join(k.ManifestsDir, "konnectivity")
 	err := dir.Init(konnectivityDir, constant.ManifestsDirMode)
 	if err != nil {
 		return err
 	}
 
-	proxyServerHost := k.APIServerHost
-	proxyServerPort := clusterConfig.Spec.Konnectivity.AgentPort
-
-	// We don't use k0snet.ParseHostPortWithDefault here because the API server host might be an IP
-	// literal (IPv6). We don't want to change the current behavior and fail here. So we
-	// just use the standard library function and change default values only if we successfully parsed host and port.
-	host, port, _ := net.SplitHostPort(k.APIServerHost)
-	if host != "" {
-		proxyServerHost = host
-	}
-	if p, _ := strconv.Atoi(port); p != 0 {
-		proxyServerPort = int32(p)
-	}
-
 	cfg := konnectivityAgentConfig{
-		// Since the konnectivity server runs with hostNetwork=true this is the
-		// IP address of the master machine
-		ProxyServerHost: proxyServerHost,
-		ProxyServerPort: uint16(proxyServerPort),
+		ProxyServerHost: k.KonnectivityServerHost,
+		ProxyServerPort: uint16(clusterConfig.Spec.Konnectivity.AgentPort),
 		Image:           clusterConfig.Spec.Images.Konnectivity.URI(),
 		ServerCount:     serverCount,
 		PullPolicy:      clusterConfig.Spec.Images.DefaultPullPolicy,
 	}
 
-	if clusterConfig.Spec.Network != nil {
+	if externalAddress := clusterConfig.Spec.Konnectivity.ExternalAddress; externalAddress != "" {
+		serverHostPort, err := k0snet.ParseHostPortWithDefault(externalAddress, cfg.ProxyServerPort)
+		if err != nil {
+			return fmt.Errorf("failed to determine proxy server host and port (%q, %d): %w", externalAddress, cfg.ProxyServerPort, err)
+		}
+		cfg.ProxyServerHost, cfg.ProxyServerPort = serverHostPort.Host(), serverHostPort.Port()
+	} else if clusterConfig.Spec.Network != nil {
 		nllb := clusterConfig.Spec.Network.NodeLocalLoadBalancing
 		if nllb.IsEnabled() {
 			switch nllb.Type {
@@ -195,7 +183,6 @@ func (k *KonnectivityAgent) writeKonnectivityAgent(clusterConfig *v1beta1.Cluste
 type konnectivityAgentConfig struct {
 	ProxyServerHost string
 	ProxyServerPort uint16
-	AgentPort       uint16
 	Image           string
 	ServerCount     uint
 	PullPolicy      string