From 6c6cfba7735ef1bc3f5e850d5abf258fb7786f38 Mon Sep 17 00:00:00 2001
From: Simon Bein <simontheleg@gmail.com>
Date: Tue, 19 Mar 2024 17:35:57 +0100
Subject: [PATCH] add preflight check for hostnames

---
 pkg/tasks/preflight_checks.go      | 30 ++++++++++++++++++
 pkg/tasks/preflight_checks_test.go | 50 ++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/pkg/tasks/preflight_checks.go b/pkg/tasks/preflight_checks.go
index 094bc72f4..defb3023b 100644
--- a/pkg/tasks/preflight_checks.go
+++ b/pkg/tasks/preflight_checks.go
@@ -32,6 +32,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/validation"
 	dynclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -70,6 +71,15 @@ func runPreflightChecks(s *state.State) error {
 		}
 	}
 
+	s.Logger.Infoln("Verifying that hostnames are valid Kubernetes node names...")
+	allHostnames := make([]string, len(s.Cluster.ControlPlane.Hosts)+len(s.Cluster.StaticWorkers.Hosts))
+	for i, host := range append(s.Cluster.ControlPlane.Hosts, s.Cluster.StaticWorkers.Hosts...) {
+		allHostnames[i] = host.Hostname
+	}
+	if err := checkHostnames(allHostnames); err != nil {
+		return err
+	}
+
 	return nil
 }
 
@@ -210,3 +220,23 @@ func checkVersionSkew(reqVer, currVer *semver.Version, diff uint64) error {
 
 	return nil
 }
+
+func checkHostnames(hostnames []string) error {
+	// According to k8s spec, the node hostname has to be a valid RFC 1123 DNS Subdomain (https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#dns-subdomain-names)
+	allErrors := []string{}
+
+	for _, hostname := range hostnames {
+		err := validation.IsDNS1123Subdomain(hostname)
+		if len(err) > 0 {
+			allErrors = append(allErrors, fmt.Sprintf("- hostname %q cannot be used as Kubernetes node: %s", hostname, err))
+		}
+	}
+
+	if len(allErrors) > 0 {
+		allErrors = append(allErrors, "Please rename host(s)")
+
+		return fail.Runtime(fmt.Errorf(strings.Join(allErrors, "\n")), "validating node hostnames")
+	}
+
+	return nil
+}
diff --git a/pkg/tasks/preflight_checks_test.go b/pkg/tasks/preflight_checks_test.go
index 86bc196dd..5b7e38097 100644
--- a/pkg/tasks/preflight_checks_test.go
+++ b/pkg/tasks/preflight_checks_test.go
@@ -267,3 +267,53 @@ func TestCheckVersionSkewInvalid(t *testing.T) {
 		})
 	}
 }
+
+func TestCheckHostname(t *testing.T) {
+	t.Parallel()
+
+	testcases := []struct {
+		name      string
+		hostnames []string
+		expError  error
+	}{
+		{
+			name:      "valid hostname",
+			hostnames: []string{"valid-hostname"},
+			expError:  nil,
+		},
+		{
+			name:      "invalid hostname",
+			hostnames: []string{"ALL-CAPS"},
+			expError: fmt.Errorf(`runtime: validating node hostnames
+- hostname "ALL-CAPS" cannot be used as Kubernetes node: [a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')]
+Please rename host(s)`),
+		},
+		{
+			name:      "multiple errors",
+			hostnames: []string{"ALL-CAPS", "inv@lid-symbol"},
+			expError: fmt.Errorf(`runtime: validating node hostnames
+- hostname "ALL-CAPS" cannot be used as Kubernetes node: [a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')]
+- hostname "inv@lid-symbol" cannot be used as Kubernetes node: [a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')]
+Please rename host(s)`),
+		},
+	}
+
+	for _, tc := range testcases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			err := checkHostnames(tc.hostnames)
+
+			exp, act := "", ""
+			if tc.expError != nil {
+				exp = tc.expError.Error()
+			}
+			if err != nil {
+				act = err.Error()
+			}
+			if exp != act {
+				t.Fatalf("want error\n%q\ngot\n%q\n", tc.expError, err)
+			}
+		})
+	}
+}