Ensure valid node names (#194)

* add utils.CleanHostname
* clean hostname when joining nodes or creating tokens
* do not hard-code timeouts in join process

src/k8s/go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/onsi/gomega v1.30.0
 	github.com/sirupsen/logrus v1.9.3
 	github.com/spf13/cobra v1.8.0
+	golang.org/x/net v0.21.0
 	gopkg.in/yaml.v2 v2.4.0
 	helm.sh/helm/v3 v3.14.2
 	k8s.io/api v0.29.0
@@ -146,7 +147,6 @@ require (
 	go.opentelemetry.io/otel/trace v1.23.1 // indirect
 	go.starlark.net v0.0.0-20230912135651-745481cf39ed // indirect
 	golang.org/x/crypto v0.19.0 // indirect
-	golang.org/x/net v0.21.0 // indirect
 	golang.org/x/oauth2 v0.15.0 // indirect
 	golang.org/x/sync v0.6.0 // indirect
 	golang.org/x/sys v0.17.0 // indirect

src/k8s/pkg/k8s/client/cluster.go

@@ -8,6 +8,7 @@ import (
 
 	apiv1 "github.com/canonical/k8s/api/v1"
 	"github.com/canonical/k8s/pkg/config"
+	"github.com/canonical/k8s/pkg/utils"
 	"github.com/canonical/k8s/pkg/utils/control"
 	"github.com/canonical/k8s/pkg/utils/k8s"
 	"github.com/canonical/lxd/lxd/util"
@@ -23,29 +24,32 @@ func (c *k8sdClient) IsBootstrapped(ctx context.Context) bool {
 // Bootstrap bootstraps the k8s cluster
 func (c *k8sdClient) Bootstrap(ctx context.Context, bootstrapConfig apiv1.BootstrapConfig) (apiv1.NodeStatus, error) {
 	// Get system hostname.
-	hostname, err := os.Hostname()
+	rawHostname, err := os.Hostname()
 	if err != nil {
 		return apiv1.NodeStatus{}, fmt.Errorf("failed to retrieve system hostname: %w", err)
 	}
+	// TODO: this should be done on the server side, but we cannot currently hijack the microcluster bootstrap endpoint.
+	hostname, err := utils.CleanHostname(rawHostname)
+	if err != nil {
+		return apiv1.NodeStatus{}, fmt.Errorf("invalid hostname %q: %w", rawHostname, err)
+	}
 
 	// Get system addrPort.
 	addrPort := util.CanonicalNetworkAddress(util.NetworkInterfaceAddress(), config.DefaultPort)
 
-	timeToWait := 30
-	// If a context timeout is set, use this instead.
-	deadline, set := ctx.Deadline()
-	if set {
-		timeToWait = int(deadline.Sub(time.Now()).Seconds())
+	timeout := 30 * time.Second
+	if deadline, set := ctx.Deadline(); set {
+		timeout = time.Until(deadline)
 	}
 
-	if err := c.m.Ready(timeToWait); err != nil {
+	if err := c.m.Ready(int(timeout / time.Second)); err != nil {
 		return apiv1.NodeStatus{}, fmt.Errorf("cluster did not come up in time: %w", err)
 	}
 	config, err := bootstrapConfig.ToMap()
 	if err != nil {
 		return apiv1.NodeStatus{}, fmt.Errorf("failed to convert bootstrap config to map: %w", err)
 	}
-	if err := c.m.NewCluster(hostname, addrPort, config, time.Duration(timeToWait)*time.Second); err != nil {
+	if err := c.m.NewCluster(hostname, addrPort, config, timeout); err != nil {
 		// TODO(neoaggelos): print message that bootstrap failed, and that we are cleaning up
 		fmt.Fprintln(os.Stderr, "Failed with error:", err)
 		c.CleanupNode(ctx, c.opts.Snap, hostname)

src/k8s/pkg/k8sd/api/cluster_join.go

@@ -8,6 +8,7 @@ import (
 
 	apiv1 "github.com/canonical/k8s/api/v1"
 	"github.com/canonical/k8s/pkg/k8sd/types"
+	"github.com/canonical/k8s/pkg/utils"
 	"github.com/canonical/lxd/lxd/response"
 	"github.com/canonical/microcluster/microcluster"
 	"github.com/canonical/microcluster/state"
@@ -19,15 +20,25 @@ func postClusterJoin(m *microcluster.MicroCluster, s *state.State, r *http.Reque
 		return response.BadRequest(fmt.Errorf("failed to parse request: %w", err))
 	}
 
+	hostname, err := utils.CleanHostname(req.Name)
+	if err != nil {
+		return response.BadRequest(fmt.Errorf("invalid hostname %q: %w", req.Name, err))
+	}
+
+	timeout := 30 * time.Second
+	if deadline, set := s.Context.Deadline(); set {
+		timeout = time.Until(deadline)
+	}
+
 	// differentiate between control plane and worker node tokens
 	info := &types.InternalWorkerNodeToken{}
 	if info.Decode(req.Token) == nil {
 		// valid worker node token
-		if err := m.NewCluster(req.Name, req.Address, map[string]string{"workerToken": req.Token}, time.Second*180); err != nil {
+		if err := m.NewCluster(hostname, req.Address, map[string]string{"workerToken": req.Token}, timeout); err != nil {
 			return response.InternalError(fmt.Errorf("failed to join k8sd cluster as worker: %w", err))
 		}
 	} else {
-		if err := m.JoinCluster(req.Name, req.Address, req.Token, nil, time.Second*180); err != nil {
+		if err := m.JoinCluster(hostname, req.Address, req.Token, nil, timeout); err != nil {
 			return response.InternalError(fmt.Errorf("failed to join k8sd cluster as control plane: %w", err))
 		}
 	}

src/k8s/pkg/k8sd/api/cluster_tokens.go

@@ -10,6 +10,7 @@ import (
 	apiv1 "github.com/canonical/k8s/api/v1"
 	"github.com/canonical/k8s/pkg/k8sd/database"
 	"github.com/canonical/k8s/pkg/k8sd/types"
+	"github.com/canonical/k8s/pkg/utils"
 	"github.com/canonical/lxd/lxd/response"
 	"github.com/canonical/microcluster/microcluster"
 	"github.com/canonical/microcluster/state"
@@ -21,14 +22,16 @@ func postClusterJoinTokens(m *microcluster.MicroCluster, s *state.State, r *http
 		return response.BadRequest(fmt.Errorf("failed to parse request: %w", err))
 	}
 
-	var (
-		token string
-		err   error
-	)
+	hostname, err := utils.CleanHostname(req.Name)
+	if err != nil {
+		return response.BadRequest(fmt.Errorf("invalid hostname %q: %w", req.Name, err))
+	}
+
+	var token string
 	if req.Worker {
 		token, err = createWorkerToken(s)
 	} else {
-		token, err = m.NewJoinToken(req.Name)
+		token, err = m.NewJoinToken(hostname)
 	}
 	if err != nil {
 		return response.InternalError(fmt.Errorf("failed to create token: %w", err))

src/k8s/pkg/utils/hostname.go

@@ -0,0 +1,20 @@
+package utils
+
+import (
+	"fmt"
+	"strings"
+
+	"golang.org/x/net/idna"
+)
+
+// CleanHostname sanitises hostnames.
+func CleanHostname(hostname string) (string, error) {
+	clean, err := idna.Lookup.ToASCII(hostname)
+	if err != nil {
+		return "", fmt.Errorf("failed to parse hostname %q: %w", hostname, err)
+	}
+	if strings.HasSuffix(hostname, ".") {
+		return "", fmt.Errorf("hostname cannot end with a dot (%q)", ".")
+	}
+	return clean, nil
+}

src/k8s/pkg/utils/hostname_test.go

@@ -0,0 +1,38 @@
+package utils_test
+
+import (
+	"testing"
+
+	"github.com/canonical/k8s/pkg/utils"
+	. "github.com/onsi/gomega"
+)
+
+func TestCleanHostname(t *testing.T) {
+	for _, tc := range []struct {
+		hostname       string
+		expectHostname string
+		expectValid    bool
+	}{
+		{hostname: "w1", expectHostname: "w1", expectValid: true},
+		{hostname: "w1.internal", expectHostname: "w1.internal", expectValid: true},
+		{hostname: "w1.test.domain", expectHostname: "w1.test.domain", expectValid: true},
+		{hostname: "w1-with-dash", expectHostname: "w1-with-dash", expectValid: true},
+		{hostname: "Capital", expectHostname: "capital", expectValid: true},
+		{hostname: "dash-end-"},
+		{hostname: "dot-end."},
+		{hostname: "w1-with_underscore"},
+		{hostname: "spaces 123"},
+		{hostname: "special!@*!^%#*&$"},
+	} {
+		t.Run(tc.hostname, func(t *testing.T) {
+			g := NewWithT(t)
+			hostname, err := utils.CleanHostname(tc.hostname)
+			if tc.expectValid {
+				g.Expect(err).To(BeNil())
+				g.Expect(hostname).To(Equal(tc.expectHostname))
+			} else {
+				g.Expect(err).To(Not(BeNil()))
+			}
+		})
+	}
+}