fix: validate RayCluster name with validating webhook (ray-project#1732)

kevin85421 · Dec 11, 2023 · d49e052 · d49e052
1 parent eee9d94
commit d49e052
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 0 deletions.
diff --git a/ray-operator/apis/ray/v1/raycluster_webhook.go b/ray-operator/apis/ray/v1/raycluster_webhook.go
@@ -1,6 +1,8 @@
 package v1
 
 import (
+	"regexp"
+
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -13,6 +15,7 @@ import (
 
 // log is for logging in this package.
 var rayclusterlog = logf.Log.WithName("raycluster-resource")
+var nameRegex, _ = regexp.Compile("^[a-z]([-a-z0-9]*[a-z0-9])?$")
 
 func (r *RayCluster) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
@@ -45,9 +48,15 @@ func (r *RayCluster) ValidateDelete() (admission.Warnings, error) {
 
 func (r *RayCluster) validateRayCluster() error {
 	var allErrs field.ErrorList
+
+	if err := r.validateName(); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	if err := r.validateWorkerGroups(); err != nil {
 		allErrs = append(allErrs, err)
 	}
+
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -57,6 +66,13 @@ func (r *RayCluster) validateRayCluster() error {
 		r.Name, allErrs)
 }
 
+func (r *RayCluster) validateName() *field.Error {
+	if !nameRegex.MatchString(r.Name) {
+		return field.Invalid(field.NewPath("metadata").Child("name"), r.Name, "name must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name',  or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')")
+	}
+	return nil
+}
+
 func (r *RayCluster) validateWorkerGroups() *field.Error {
 	workerGroupNames := make(map[string]bool)
 

diff --git a/ray-operator/apis/ray/v1/webhook_suite_test.go b/ray-operator/apis/ray/v1/webhook_suite_test.go
@@ -120,6 +120,33 @@ var _ = BeforeSuite(func() {
 })
 
 var _ = Describe("RayCluster validating webhook", func() {
+	Context("when name is invalid", func() {
+		It("should return error", func() {
+			rayCluster := RayCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "invalid.name",
+				},
+				Spec: RayClusterSpec{
+					HeadGroupSpec: HeadGroupSpec{
+						RayStartParams: map[string]string{"DEADBEEF": "DEADBEEF"},
+						Template: corev1.PodTemplateSpec{
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{},
+							},
+						},
+					},
+					WorkerGroupSpecs: []WorkerGroupSpec{},
+				},
+			}
+
+			err := k8sClient.Create(context.TODO(), &rayCluster)
+			Expect(err).To(HaveOccurred())
+
+			Expect(err.Error()).To(ContainSubstring("RayCluster.ray.io \"invalid.name\" is invalid: metadata.name:"))
+		})
+	})
+
 	Context("when groupNames are not unique", func() {
 		var name, namespace string
 		var rayCluster RayCluster