From d1195d1d5faeade75289222c6c588485be63de36 Mon Sep 17 00:00:00 2001 From: 3u13r Date: Fri, 23 Dec 2022 18:59:15 +0100 Subject: [PATCH] join: make Azure instance names k8s compliant (#807) join: make Azure instance names k8s compliant --- .../internal/kubernetes/kubernetes.go | 21 +++++-- .../internal/kubernetes/kubernetes_test.go | 46 +++++++++----- joinservice/internal/kubernetes/kubernetes.go | 26 +++++++- .../internal/kubernetes/kubernetes_test.go | 63 +++++++++++++++++++ 4 files changed, 133 insertions(+), 23 deletions(-) create mode 100644 joinservice/internal/kubernetes/kubernetes_test.go diff --git a/bootstrapper/internal/kubernetes/kubernetes.go b/bootstrapper/internal/kubernetes/kubernetes.go index b949fdc87..c53e06735 100644 --- a/bootstrapper/internal/kubernetes/kubernetes.go +++ b/bootstrapper/internal/kubernetes/kubernetes.go @@ -14,6 +14,7 @@ import ( "errors" "fmt" "net" + "regexp" "strconv" "strings" "time" @@ -36,6 +37,8 @@ import ( kubeadm "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3" ) +var validHostnameRegex = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + // configReader provides kubeconfig as []byte. type configReader interface { ReadKubeconfig() ([]byte, error) @@ -110,7 +113,11 @@ func (k *KubeWrapper) InitCluster( if instance.VPCIP != "" { validIPs = append(validIPs, net.ParseIP(instance.VPCIP)) } - nodeName := k8sCompliantHostname(instance.Name) + nodeName, err := k8sCompliantHostname(instance.Name) + if err != nil { + return nil, fmt.Errorf("generating node name: %w", err) + } + nodeIP := instance.VPCIP subnetworkPodCIDR := instance.SecondaryIPRange if len(instance.AliasIPRanges) > 0 { @@ -278,7 +285,10 @@ func (k *KubeWrapper) JoinCluster(ctx context.Context, args *kubeadm.BootstrapTo } providerID := instance.ProviderID nodeInternalIP := instance.VPCIP - nodeName := k8sCompliantHostname(instance.Name) + nodeName, err := k8sCompliantHostname(instance.Name) + if err != nil { + return fmt.Errorf("generating node name: %w", err) + } loadbalancerEndpoint, err := k.providerMetadata.GetLoadBalancerEndpoint(ctx) if err != nil { @@ -401,10 +411,13 @@ func (k *KubeWrapper) setupInternalConfigMap(ctx context.Context, azureCVM strin // k8sCompliantHostname transforms a hostname to an RFC 1123 compliant, lowercase subdomain as required by Kubernetes node names. // The following regex is used by k8s for validation: /^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$/ . // Only a simple heuristic is used for now (to lowercase, replace underscores). -func k8sCompliantHostname(in string) string { +func k8sCompliantHostname(in string) (string, error) { hostname := strings.ToLower(in) hostname = strings.ReplaceAll(hostname, "_", "-") - return hostname + if !validHostnameRegex.MatchString(hostname) { + return "", fmt.Errorf("failed to generate a Kubernetes compliant hostname for %s", in) + } + return hostname, nil } // StartKubelet starts the kubelet service. diff --git a/bootstrapper/internal/kubernetes/kubernetes_test.go b/bootstrapper/internal/kubernetes/kubernetes_test.go index e60ccda57..3bcadef2f 100644 --- a/bootstrapper/internal/kubernetes/kubernetes_test.go +++ b/bootstrapper/internal/kubernetes/kubernetes_test.go @@ -10,7 +10,6 @@ import ( "context" "errors" "net" - "regexp" "strconv" "testing" @@ -465,22 +464,32 @@ func TestJoinCluster(t *testing.T) { } func TestK8sCompliantHostname(t *testing.T) { - compliantHostname := regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) testCases := map[string]struct { - hostname string - wantHostname string + input string + expected string + wantErr bool }{ - "azure scale set names work": { - hostname: "constellation-scale-set-bootstrappers-name_0", - wantHostname: "constellation-scale-set-bootstrappers-name-0", + "no change": { + input: "test", + expected: "test", }, - "compliant hostname is not modified": { - hostname: "abcd-123", - wantHostname: "abcd-123", + "uppercase": { + input: "TEST", + expected: "test", }, - "uppercase hostnames are lowercased": { - hostname: "ABCD", - wantHostname: "abcd", + "underscore": { + input: "test_node", + expected: "test-node", + }, + "empty": { + input: "", + expected: "", + wantErr: true, + }, + "error": { + input: "test_node_", + expected: "", + wantErr: true, }, } @@ -488,10 +497,13 @@ func TestK8sCompliantHostname(t *testing.T) { t.Run(name, func(t *testing.T) { assert := assert.New(t) - hostname := k8sCompliantHostname(tc.hostname) - - assert.Equal(tc.wantHostname, hostname) - assert.Regexp(compliantHostname, hostname) + actual, err := k8sCompliantHostname(tc.input) + if tc.wantErr { + assert.Error(err) + return + } + assert.NoError(err) + assert.Equal(tc.expected, actual) }) } } diff --git a/joinservice/internal/kubernetes/kubernetes.go b/joinservice/internal/kubernetes/kubernetes.go index c18986735..8beecb4b1 100644 --- a/joinservice/internal/kubernetes/kubernetes.go +++ b/joinservice/internal/kubernetes/kubernetes.go @@ -10,6 +10,8 @@ import ( "context" "encoding/json" "fmt" + "regexp" + "strings" "time" "github.com/edgelesssys/constellation/v2/internal/constants" @@ -85,7 +87,14 @@ func (c *Client) CreateConfigMap(ctx context.Context, configMap corev1.ConfigMap func (c *Client) AddNodeToJoiningNodes(ctx context.Context, nodeName string, componentsHash string, isControlPlane bool) error { joiningNode := &unstructured.Unstructured{} - objectMetadataName := nodeName + compliantNodeName, err := k8sCompliantHostname(nodeName) + if err != nil { + return fmt.Errorf("failed to get k8s compliant hostname: %w", err) + } + + // JoiningNodes referencing a worker node are named after the worker node. + // JoiningNodes referencing the control-plane node are named "control-plane". + objectMetadataName := compliantNodeName deadline := metav1.NewTime(time.Now().Add(48 * time.Hour)) if isControlPlane { objectMetadataName = "control-plane" @@ -99,7 +108,7 @@ func (c *Client) AddNodeToJoiningNodes(ctx context.Context, nodeName string, com "name": objectMetadataName, }, "spec": map[string]any{ - "name": nodeName, + "name": compliantNodeName, "componentshash": componentsHash, "iscontrolplane": isControlPlane, "deadline": deadline, @@ -142,3 +151,16 @@ func (c *Client) AddReferenceToK8sVersionConfigMap(ctx context.Context, k8sVersi } return nil } + +var validHostnameRegex = regexp.MustCompile(`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`) + +// k8sCompliantHostname transforms a hostname to an RFC 1123 compliant, lowercase subdomain as required by Kubernetes node names. +// Only a simple heuristic is used for now (to lowercase, replace underscores). +func k8sCompliantHostname(in string) (string, error) { + hostname := strings.ToLower(in) + hostname = strings.ReplaceAll(hostname, "_", "-") + if !validHostnameRegex.MatchString(hostname) { + return "", fmt.Errorf("failed to generate a Kubernetes compliant hostname for %s", in) + } + return hostname, nil +} diff --git a/joinservice/internal/kubernetes/kubernetes_test.go b/joinservice/internal/kubernetes/kubernetes_test.go new file mode 100644 index 000000000..225cd2a2d --- /dev/null +++ b/joinservice/internal/kubernetes/kubernetes_test.go @@ -0,0 +1,63 @@ +/* +Copyright (c) Edgeless Systems GmbH + +SPDX-License-Identifier: AGPL-3.0-only +*/ + +package kubernetes + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} + +func TestK8sCompliantHostname(t *testing.T) { + testCases := map[string]struct { + input string + expected string + wantErr bool + }{ + "no change": { + input: "test", + expected: "test", + }, + "uppercase": { + input: "TEST", + expected: "test", + }, + "underscore": { + input: "test_node", + expected: "test-node", + }, + "empty": { + input: "", + expected: "", + wantErr: true, + }, + "error": { + input: "test_node_", + expected: "", + wantErr: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + actual, err := k8sCompliantHostname(tc.input) + if tc.wantErr { + assert.Error(err) + return + } + assert.NoError(err) + assert.Equal(tc.expected, actual) + }) + } +}