fix: allow FQDNs as controlPlaneEndpoint (#157)

* fix: allow FQDNs as controlPlaneEndpoint

* add testcases with plain ip4/ip6 endpoints

Author: pborn-ionos

internal/webhook/proxmoxcluster_webhook.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"fmt"
 	"net/netip"
+	"regexp"
 	"strings"
 
 	infrav1 "github.com/ionos-cloud/cluster-api-provider-proxmox/api/v1alpha1"
@@ -63,7 +64,7 @@ func (*ProxmoxCluster) ValidateCreate(_ context.Context, obj runtime.Object) (wa
 		return warnings, err
 	}
 
-	if err := validateIPs(cluster); err != nil {
+	if err := validateControlPlaneEndpoint(cluster); err != nil {
 		warnings = append(warnings, fmt.Sprintf("cannot create proxmox cluster %s", cluster.GetName()))
 		return warnings, err
 	}
@@ -83,44 +84,62 @@ func (*ProxmoxCluster) ValidateUpdate(_ context.Context, _ runtime.Object, newOb
 		return warnings, apierrors.NewBadRequest(fmt.Sprintf("expected a ProxmoxCluster but got %T", newCluster))
 	}
 
-	if err := validateIPs(newCluster); err != nil {
+	if err := validateControlPlaneEndpoint(newCluster); err != nil {
 		warnings = append(warnings, fmt.Sprintf("cannot update proxmox cluster %s", newCluster.GetName()))
 		return warnings, err
 	}
 
 	return warnings, nil
 }
 
-func validateIPs(cluster *infrav1.ProxmoxCluster) error {
+func validateControlPlaneEndpoint(cluster *infrav1.ProxmoxCluster) error {
 	ep := cluster.Spec.ControlPlaneEndpoint
 
 	gk, name := cluster.GroupVersionKind().GroupKind(), cluster.GetName()
 
-	endpointHostIP := ep.Host
+	endpoint := ep.Host
+
+	addr, err := netip.ParseAddr(endpoint)
+
+	/*
+	   No further validation is done on hostnames. Checking DNS records
+	   incures a lot of complexity. To list a few of the problems:
+	    - DNS TTL will lead to incorrect results
+	    - IP addresses can be PTR records
+	    - Both A and AAAA records would need checking
+	    - A record can have multiple entries, each of which need to be checked
+	    - A valid record can start with _, but that is not a valid hostname
+	    - ...
+	   Most importantly, cluster-api does not validate controlPlaneEndpoint
+	   at all.
+	*/
+	match := isHostname(endpoint)
+	if match {
+		return nil
+	}
 
-	addr, err := netip.ParseAddr(endpointHostIP)
 	if err != nil {
 		return apierrors.NewInvalid(
 			gk,
 			name,
 			field.ErrorList{
 				field.Invalid(
-					field.NewPath("spec", "controlplaneEndpoint"), endpointHostIP, "provided endpoint address is not a valid IP"),
+					field.NewPath("spec", "controlplaneEndpoint"), endpoint, "provided endpoint address is not a valid IP or FQDN"),
 			})
 	}
 	// If the passed control-plane endppoint is an IPv6 address, wrap it in [], so it can properly pass ParseAddrPort validation
 	if addr.Is6() {
-		endpointHostIP = fmt.Sprintf("[%s]", ep.Host)
+		endpoint = fmt.Sprintf("[%s]", endpoint)
 	}
 
-	ipAddr, err := netip.ParseAddrPort(fmt.Sprintf("%s:%d", endpointHostIP, ep.Port))
+	ipAddr, err := netip.ParseAddrPort(fmt.Sprintf("%s:%d", endpoint, ep.Port))
 	if err != nil {
 		return apierrors.NewInvalid(
 			gk,
 			name,
 			field.ErrorList{
 				field.Invalid(
-					field.NewPath("spec", "controlplaneEndpoint"), fmt.Sprintf("%s:%d", endpointHostIP, ep.Port), "provided endpoint is not in a valid IP and port format"),
+					field.NewPath("spec", "controlplaneEndpoint"), fmt.Sprintf("%s:%d", endpoint, ep.Port), "provided endpoint is not in a valid IP and port format"),
 			})
 	}
 
@@ -213,3 +232,17 @@ func buildSetFromAddresses(addresses []string) (*netipx.IPSet, error) {
 func hasNoIPPoolConfig(cluster *infrav1.ProxmoxCluster) bool {
 	return cluster.Spec.IPv4Config == nil && cluster.Spec.IPv6Config == nil
 }
+
+func isHostname(h string) bool {
+	// shortname is up to 253 bytes long
+	shortname := `([a-z0-9]{1,253}|[a-z0-9][a-z0-9-]{1,251}[a-z0-9])`
+	// hostname is optional in a domain
+	hostname := `([a-z0-9]{1,63}|[a-z0-9][a-z0-9-]{1,61}[a-z0-9]\.)?`
+	domain := `((([a-z0-9]{1,63}|[a-z0-9][a-z0-9-]{1,61}[a-z0-9])\.)+?[a-z]{2,63})`
+
+	// make hostname match case insensitive, match complete string
+	hostmatch := `(?i)^(` + shortname + `|` + hostname + domain + `)$`
+
+	match, _ := regexp.Match(hostmatch, []byte(h))
+	return match
+}

internal/webhook/proxmoxcluster_webhook_test.go

@@ -43,10 +43,40 @@ var _ = Describe("Controller Test", func() {
 			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("at least one ip config must be set")))
 		})
 
-		It("should disallow invalid endpoint IP", func() {
+		It("should disallow invalid endpoint FQDN", func() {
 			cluster := invalidProxmoxCluster("test-cluster")
-			cluster.Spec.ControlPlaneEndpoint.Host = "invalid"
-			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint address is not a valid IP")))
+			cluster.Spec.ControlPlaneEndpoint.Host = "_this.is.a.txt.record"
+			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint address is not a valid IP or FQDN")))
+		})
+
+		It("should disallow invalid endpoint short hostname", func() {
+			cluster := invalidProxmoxCluster("test-cluster")
+			cluster.Spec.ControlPlaneEndpoint.Host = "invalid-"
+			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(MatchError(ContainSubstring("provided endpoint address is not a valid IP or FQDN")))
+		})
+
+		It("should allow valid endpoint FQDN", func() {
+			cluster := validProxmoxCluster("succeed-test-cluster-with-fqdn")
+			cluster.Spec.ControlPlaneEndpoint.Host = "host.example.com"
+			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
+		})
+
+		It("should allow valid upper case endpoint FQDN", func() {
+			cluster := validProxmoxCluster("succeed-test-cluster-with-uppercase-fqdn")
+			cluster.Spec.ControlPlaneEndpoint.Host = "HOST.EXAMPLE.COM"
+			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
+		})
+
+		It("should allow valid endpoint IP4", func() {
+			cluster := validProxmoxCluster("succeed-test-cluster-with-ip4")
+			cluster.Spec.ControlPlaneEndpoint.Host = "127.0.0.1"
+			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
+		})
+
+		It("should allow valid endpoint IP6", func() {
+			cluster := validProxmoxCluster("succeed-test-cluster-with-ip6")
+			cluster.Spec.ControlPlaneEndpoint.Host = "::1"
+			g.Expect(k8sClient.Create(testEnv.GetContext(), &cluster)).To(Succeed())
 		})
 
 		It("should disallow invalid endpoint IP + port combination", func() {