From 08f099748f90a0bd63dcc53bec09b4ac4582955c Mon Sep 17 00:00:00 2001
From: Jacob Lindgren <jacob.lindgren@suse.com>
Date: Tue, 27 Jun 2023 16:43:40 -0500
Subject: [PATCH] Validate machine pool names < 63 characters on create

---
 .../v1/cluster/validator.go                   | 28 ++++++++
 .../v1/cluster/validator_test.go              | 69 +++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go
index c2686b61..d7711887 100644
--- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go
+++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go
@@ -88,6 +88,10 @@ func (p *provisioningAdmitter) Admit(request *admission.Request) (*admissionv1.A
 			return response, err
 		}
 
+		if err := p.validateMachinePoolNames(request, response, cluster); err != nil || response.Result != nil {
+			return response, err
+		}
+
 		if response.Result = common.CheckCreatorID(request, oldCluster, cluster); response.Result != nil {
 			return response, nil
 		}
@@ -184,6 +188,30 @@ func (p *provisioningAdmitter) validateClusterName(request *admission.Request, r
 	return nil
 }
 
+func (p *provisioningAdmitter) validateMachinePoolNames(request *admission.Request, response *admissionv1.AdmissionResponse, cluster *v1.Cluster) error {
+	if request.Operation != admissionv1.Create {
+		return nil
+	}
+
+	if cluster.Spec.RKEConfig == nil {
+		return nil
+	}
+
+	for _, pool := range cluster.Spec.RKEConfig.MachinePools {
+		if len(pool.Name) > 63 {
+			response.Result = &metav1.Status{
+				Status:  "Failure",
+				Message: "pool name must be 63 characters or fewer",
+				Reason:  metav1.StatusReasonInvalid,
+				Code:    http.StatusUnprocessableEntity,
+			}
+			break
+		}
+	}
+
+	return nil
+}
+
 // validatePSACT validate if the cluster and underlying secret are configured properly when PSACT is enabled or disabled
 func (p *provisioningAdmitter) validatePSACT(request *admission.Request, response *admissionv1.AdmissionResponse, cluster *v1.Cluster) error {
 	if cluster.Name == "local" || cluster.Spec.RKEConfig == nil {
diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go
index 40df6b88..b5e8ba00 100644
--- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go
+++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go
@@ -1,7 +1,12 @@
 package cluster
 
 import (
+	"strings"
 	"testing"
+
+	v1 "github.com/rancher/rancher/pkg/apis/provisioning.cattle.io/v1"
+	"github.com/rancher/webhook/pkg/admission"
+	admissionv1 "k8s.io/api/admission/v1"
 )
 
 func Test_isValidName(t *testing.T) {
@@ -131,3 +136,67 @@ func Test_isValidName(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateMachinePoolName(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name, value string
+		fail        bool
+	}{
+		{
+			name:  "muchTooLong",
+			value: strings.Repeat("12345678", 8),
+			fail:  true,
+		},
+		{
+			name:  "barelyUnderLimit",
+			value: strings.Repeat("12345678", 7),
+			fail:  false,
+		},
+		{
+			name:  "regularLookingString",
+			value: "regular-string-test",
+			fail:  false,
+		},
+	}
+
+	a := provisioningAdmitter{}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			resp := admissionv1.AdmissionResponse{}
+
+			err := a.validateMachinePoolNames(
+				&admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create}},
+				&resp,
+				&v1.Cluster{
+					Spec: v1.ClusterSpec{
+						RKEConfig: &v1.RKEConfig{
+							MachinePools: []v1.RKEMachinePool{{Name: tt.value}},
+						},
+					},
+				},
+			)
+
+			if err != nil {
+				t.Errorf("got error when none was expected: %v", err)
+			}
+
+			if tt.fail {
+				if resp.Result == nil {
+					t.Error("got no result on response when one was expected")
+				}
+				if resp.Result.Status != "Failure" {
+					t.Errorf("got %v when Failure was expected", resp.Result.Status)
+				}
+			} else {
+				if resp.Result != nil {
+					t.Error("got result on response when none was expected")
+				}
+			}
+		})
+	}
+}