From 66860ab25d9c5735daa41ea78b7d88f67ea901d3 Mon Sep 17 00:00:00 2001
From: Tapajit Chandra Paul <tapajit@appscode.com>
Date: Thu, 7 Sep 2023 16:33:03 +0600
Subject: [PATCH] Add proper validation for nodepool name

Signed-off-by: Tapajit Chandra Paul <tapajit@appscode.com>
---
 .../azuremanagedmachinepool_webhook.go        | 54 +++++++++++++++++--
 .../azuremanagedmachinepool_webhook_test.go   | 10 ++--
 2 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/api/v1beta1/azuremanagedmachinepool_webhook.go b/api/v1beta1/azuremanagedmachinepool_webhook.go
index 86be164f2aa..b97cf9defd3 100644
--- a/api/v1beta1/azuremanagedmachinepool_webhook.go
+++ b/api/v1beta1/azuremanagedmachinepool_webhook.go
@@ -23,6 +23,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"unicode"
 
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -363,14 +364,59 @@ func (m *AzureManagedMachinePool) validateOSType() error {
 }
 
 func (m *AzureManagedMachinePool) validateName() error {
-	if m.Spec.OSType != nil && *m.Spec.OSType == WindowsOS &&
-		m.Spec.Name != nil && len(*m.Spec.Name) > 6 {
+	var name *string
+	var fieldNameMessage string
+	if m.Spec.Name == nil || *m.Spec.Name == "" {
+		name = &m.Name
+		fieldNameMessage = "when spec.name is empty, metadata.name"
+	} else {
+		name = m.Spec.Name
+		fieldNameMessage = "spec.name"
+	}
+
+	if err := validateNameLength(m.Spec.OSType, name, fieldNameMessage); err != nil {
+		return err
+	}
+	return validateNamePattern(name, fieldNameMessage)
+}
+
+func validateNameLength(osType *string, name *string, fieldNameMessage string) error {
+	if osType != nil && *osType == WindowsOS &&
+		name != nil && len(*name) > 6 {
+		return field.Invalid(
+			field.NewPath("Spec", "Name"),
+			name,
+			fmt.Sprintf("For OSType Windows, %s can not be longer than 6 characters.", fieldNameMessage))
+	} else if (osType == nil || *osType == LinuxOS) &&
+		(name != nil && len(*name) > 12) {
 		return field.Invalid(
 			field.NewPath("Spec", "Name"),
-			m.Spec.Name,
-			"Windows agent pool name can not be longer than 6 characters.")
+			osType,
+			fmt.Sprintf("For OSType Linux, %s can not be longer than 12 characters.", fieldNameMessage))
 	}
+	return nil
+}
 
+func validateNamePattern(name *string, fieldNameMessage string) error {
+	if name == nil || *name == "" {
+		return nil
+	}
+
+	if !unicode.IsLower(rune((*name)[0])) {
+		return field.Invalid(
+			field.NewPath("Spec", "Name"),
+			name,
+			fmt.Sprintf("%s must begin with a lowercase letter.", fieldNameMessage))
+	}
+
+	for _, char := range *name {
+		if !(unicode.IsLower(char) || unicode.IsNumber(char)) {
+			return field.Invalid(
+				field.NewPath("Spec", "Name"),
+				name,
+				fmt.Sprintf("%s may only contain lowercase alphanumeric characters.", fieldNameMessage))
+		}
+	}
 	return nil
 }
 
diff --git a/api/v1beta1/azuremanagedmachinepool_webhook_test.go b/api/v1beta1/azuremanagedmachinepool_webhook_test.go
index 0dae9bf34ac..823b41b6769 100644
--- a/api/v1beta1/azuremanagedmachinepool_webhook_test.go
+++ b/api/v1beta1/azuremanagedmachinepool_webhook_test.go
@@ -39,7 +39,7 @@ func TestAzureManagedMachinePoolDefaultingWebhook(t *testing.T) {
 	t.Logf("Testing ammp defaulting webhook with mode system")
 	ammp := &AzureManagedMachinePool{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: "fooName",
+			Name: "fooname",
 		},
 		Spec: AzureManagedMachinePoolSpec{
 			Mode:         "System",
@@ -57,7 +57,7 @@ func TestAzureManagedMachinePoolDefaultingWebhook(t *testing.T) {
 	val, ok := ammp.Labels[LabelAgentPoolMode]
 	g.Expect(ok).To(BeTrue())
 	g.Expect(val).To(Equal("System"))
-	g.Expect(*ammp.Spec.Name).To(Equal("fooName"))
+	g.Expect(*ammp.Spec.Name).To(Equal("fooname"))
 	g.Expect(*ammp.Spec.OSType).To(Equal(LinuxOS))
 
 	t.Logf("Testing ammp defaulting webhook with empty string name specified in Spec")
@@ -65,14 +65,14 @@ func TestAzureManagedMachinePoolDefaultingWebhook(t *testing.T) {
 	ammp.Spec.Name = &emptyName
 	err = mw.Default(context.Background(), ammp)
 	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(*ammp.Spec.Name).To(Equal("fooName"))
+	g.Expect(*ammp.Spec.Name).To(Equal("fooname"))
 
 	t.Logf("Testing ammp defaulting webhook with normal name specified in Spec")
-	normalName := "barName"
+	normalName := "barname"
 	ammp.Spec.Name = &normalName
 	err = mw.Default(context.Background(), ammp)
 	g.Expect(err).NotTo(HaveOccurred())
-	g.Expect(*ammp.Spec.Name).To(Equal("barName"))
+	g.Expect(*ammp.Spec.Name).To(Equal("barname"))
 
 	t.Logf("Testing ammp defaulting webhook with normal OsDiskType specified in Spec")
 	normalOsDiskType := "Ephemeral"