[feat] validate application name lenth

Signed-off-by: Arsen Gumin <gumin@live.ru>

Author: aagumin

internal/webhook/scheduledsparkapplication_validator.go

@@ -45,7 +45,7 @@ func (v *ScheduledSparkApplicationValidator) ValidateCreate(ctx context.Context,
 		return nil, nil
 	}
 	logger.Info("Validating SchedulingSparkApplication create", "name", app.Name, "namespace", app.Namespace)
-	if err := v.validate(app); err != nil {
+	if err := v.validate(ctx, app); err != nil {
 		return nil, err
 	}
 	return nil, nil
@@ -58,7 +58,7 @@ func (v *ScheduledSparkApplicationValidator) ValidateUpdate(ctx context.Context,
 		return nil, nil
 	}
 	logger.Info("Validating SchedulingSparkApplication update", "name", newApp.Name, "namespace", newApp.Namespace)
-	if err := v.validate(newApp); err != nil {
+	if err := v.validate(ctx, newApp); err != nil {
 		return nil, err
 	}
 	return nil, nil
@@ -74,7 +74,11 @@ func (v *ScheduledSparkApplicationValidator) ValidateDelete(ctx context.Context,
 	return nil, nil
 }
 
-func (v *ScheduledSparkApplicationValidator) validate(_ *v1beta2.ScheduledSparkApplication) error {
-	// TODO: implement validate logic
+func (v *ScheduledSparkApplicationValidator) validate(ctx context.Context, app *v1beta2.ScheduledSparkApplication) error {
+	meta := app.ObjectMeta
+	err := validateNameLength(ctx, meta)
+	if err != nil {
+		return err
+	}
 	return nil
 }

internal/webhook/sparkapplication_validator.go

@@ -34,6 +34,11 @@ import (
 // Modifying the path for an invalid path can cause API server errors; failing to locate the webhook.
 // +kubebuilder:webhook:admissionReviewVersions=v1,failurePolicy=fail,groups=sparkoperator.k8s.io,matchPolicy=Exact,mutating=false,name=validate-sparkapplication.sparkoperator.k8s.io,path=/validate-sparkoperator-k8s-io-v1beta2-sparkapplication,reinvocationPolicy=Never,resources=sparkapplications,sideEffects=None,verbs=create;update,versions=v1beta2,webhookVersions=v1
 
+const (
+	// 63 (Kubernetes limit) - 10 (max suffix length, e.g., "-driver-svc")
+	maxAppNameLength = 53
+)
+
 type SparkApplicationValidator struct {
 	client client.Client
 
@@ -58,6 +63,11 @@ func (v *SparkApplicationValidator) ValidateCreate(ctx context.Context, obj runt
 		return nil, nil
 	}
 	logger.Info("Validating SparkApplication create", "name", app.Name, "namespace", app.Namespace, "state", util.GetApplicationState(app))
+
+	if err := v.validateMetadata(ctx, app); err != nil {
+		return nil, err
+	}
+
 	if err := v.validateSpec(ctx, app); err != nil {
 		return nil, err
 	}
@@ -90,6 +100,10 @@ func (v *SparkApplicationValidator) ValidateUpdate(ctx context.Context, oldObj r
 		return nil, nil
 	}
 
+	if err := v.validateMetadata(ctx, newApp); err != nil {
+		return nil, err
+	}
+
 	if err := v.validateSpec(ctx, newApp); err != nil {
 		return nil, err
 	}
@@ -185,3 +199,12 @@ func (v *SparkApplicationValidator) validateResourceUsage(ctx context.Context, a
 
 	return nil
 }
+
+func (v *SparkApplicationValidator) validateMetadata(ctx context.Context, app *v1beta2.SparkApplication) error {
+	meta := app.ObjectMeta
+	err := validateNameLength(ctx, meta)
+	if err != nil {
+		return err
+	}
+	return nil
+}

internal/webhook/webhook.go

@@ -17,6 +17,10 @@ limitations under the License.
 package webhook
 
 import (
+	"context"
+	"fmt"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
@@ -35,3 +39,19 @@ type Options struct {
 	WebhookMetricsBindAddress      string
 	EnableResourceQuotaEnforcement bool
 }
+
+// validateNameLength checks if the application name exceeds the limit for Kubernetes labels.
+// The RFC 1123 DNS label regex is handled by Kubernetes itself,
+// we only need to check the length here to provide a more specific
+// and early error message.
+func validateNameLength(_ context.Context, appMeta metav1.ObjectMeta) error {
+	var allErrs field.ErrorList
+	if len(appMeta.Name) > maxAppNameLength {
+		allErrs = append(allErrs, field.Invalid(field.NewPath("metadata", "name"), appMeta.Name,
+			fmt.Sprintf("name must be no more than %d characters to allow for resource suffixes", maxAppNameLength)))
+	}
+	if allErrs != nil {
+		return allErrs.ToAggregate()
+	}
+	return nil
+}

internal/webhook/webhook_test.go

@@ -0,0 +1,80 @@
+package webhook
+
+import (
+	"context"
+	"fmt"
+	"strings"
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// Note: The maxAppNameLength constant must also be defined in your main code.
+// We define it here for the test. The value 63 is a common length
+// limit for labels in Kubernetes.
+
+func TestValidateNameLength(t *testing.T) {
+	// Define test cases
+	testCases := []struct {
+		name        string
+		appName     string
+		expectError bool
+		errorMsg    string
+	}{
+		{
+			name:        "name with valid length",
+			appName:     "my-awesome-application",
+			expectError: false,
+		},
+		{
+			name:        "empty name",
+			appName:     "",
+			expectError: false, // Length is 0, which is less than the limit
+		},
+		{
+			name:        "name at max length boundary",
+			appName:     strings.Repeat("a", maxAppNameLength),
+			expectError: false,
+		},
+		{
+			name:        "name exceeding max length by 1 character",
+			appName:     strings.Repeat("a", maxAppNameLength+1),
+			expectError: true,
+			errorMsg:    fmt.Sprintf("metadata.name: Invalid value: %q: name must be no more than %d characters to allow for resource suffixes", strings.Repeat("a", maxAppNameLength+1), maxAppNameLength),
+		},
+		{
+			name:        "long name significantly over the limit",
+			appName:     "this-is-a-very-very-very-long-application-name-that-definitely-exceeds-the-kubernetes-label-length-limit",
+			expectError: true,
+			errorMsg:    fmt.Sprintf("metadata.name: Invalid value: %q: name must be no more than %d characters to allow for resource suffixes", "this-is-a-very-very-very-long-application-name-that-definitely-exceeds-the-kubernetes-label-length-limit", maxAppNameLength),
+		},
+	}
+
+	// Run tests in a loop
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Prepare test data
+			appMeta := metav1.ObjectMeta{
+				Name: tc.appName,
+			}
+
+			// Call the function under test
+			err := validateNameLength(context.Background(), appMeta)
+
+			// Check the result
+			if tc.expectError {
+				if err == nil {
+					t.Errorf("expected an error but got nil")
+					return
+				}
+				if err.Error() != tc.errorMsg {
+					t.Errorf("expected error message:\n%q\ngot:\n%q", tc.errorMsg, err.Error())
+				}
+			} else {
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+				}
+			}
+		})
+	}
+}

test/e2e/bad_examples/long-name-validation.yaml

@@ -0,0 +1,27 @@
+apiVersion: sparkoperator.k8s.io/v1beta2
+kind: SparkApplication
+metadata:
+  name: this-is-a-very-very-very-long-application-name-that-exceeds-the-limit
+  namespace: default
+spec:
+  type: Scala
+  mode: cluster
+  image: docker.io/library/spark:4.0.0
+  imagePullPolicy: IfNotPresent
+  mainClass: org.apache.spark.examples.SparkPi
+  mainApplicationFile: local:///opt/spark/examples/jars/spark-examples.jar
+  arguments:
+  - "5000"
+  sparkVersion: 4.0.0
+  driver:
+    labels:
+      version: 4.0.0
+    cores: 1
+    memory: 512m
+    serviceAccount: spark-operator-spark
+  executor:
+    labels:
+      version: 4.0.0
+    instances: 1
+    cores: 1
+    memory: 512m

test/e2e/sparkapplication_test.go

@@ -393,4 +393,28 @@ var _ = Describe("Example SparkApplication", func() {
 			Expect(strings.Contains(string(bytes), "Pi is roughly 3")).To(BeTrue())
 		})
 	})
+
+	Context("long-name-validation", func() {
+		ctx := context.Background()
+		path := filepath.Join("bad_examples", "long-name-validation.yaml")
+		app := &v1beta2.SparkApplication{}
+
+		BeforeEach(func() {
+			By("Parsing SparkApplication from file")
+			file, err := os.Open(path)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(file).NotTo(BeNil())
+
+			decoder := yaml.NewYAMLOrJSONDecoder(file, 100)
+			Expect(decoder).NotTo(BeNil())
+			Expect(decoder.Decode(app)).NotTo(HaveOccurred())
+		})
+
+		It("Should be rejected by the webhook due to name length exceeding the limit", func() {
+			By("Attempting to create SparkApplication with a name that exceeds the maximum length")
+			err := k8sClient.Create(ctx, app)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("name must be no more than 53 characters"))
+		})
+	})
 })