Merge pull request #3093 from Poor12/add-validate

Add schedulerName validation
karmada-io · Feb 2, 2023 · c9886a0 · c9886a0
2 parents 1d32ab6 + b9e32de
commit c9886a0
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 4 deletions.
diff --git a/cmd/scheduler/app/options/validation.go b/cmd/scheduler/app/options/validation.go
@@ -27,5 +27,9 @@ func (o *Options) Validate() field.ErrorList {
 		errs = append(errs, field.Invalid(newPath.Child("SchedulerEstimatorTimeout"), o.SchedulerEstimatorTimeout, "must be greater than or equal to 0"))
 	}
 
+	if o.SchedulerName == "" {
+		errs = append(errs, field.Invalid(newPath.Child("SchedulerName"), o.SchedulerName, "should not be empty"))
+	}
+
 	return errs
 }
diff --git a/cmd/scheduler/app/options/validation_test.go b/cmd/scheduler/app/options/validation_test.go
@@ -25,6 +25,7 @@ func New(modifyOptions ModifyOptions) Options {
 		EnableSchedulerEstimator:  false,
 		SchedulerEstimatorTimeout: metav1.Duration{Duration: 1 * time.Second},
 		SchedulerEstimatorPort:    9001,
+		SchedulerName:             "default-scheduler",
 	}
 
 	if modifyOptions != nil {
@@ -45,10 +46,11 @@ func TestValidateKarmadaSchedulerConfiguration(t *testing.T) {
 			LeaderElection: componentbaseconfig.LeaderElectionConfiguration{
 				LeaderElect: false,
 			},
-			BindAddress:  "127.0.0.1",
-			SecurePort:   9000,
-			KubeAPIQPS:   40,
-			KubeAPIBurst: 30,
+			BindAddress:   "127.0.0.1",
+			SecurePort:    9000,
+			KubeAPIQPS:    40,
+			KubeAPIBurst:  30,
+			SchedulerName: "default-scheduler",
 		},
 	}
 
@@ -87,6 +89,12 @@ func TestValidateKarmadaSchedulerConfiguration(t *testing.T) {
 			}),
 			expectedErrs: field.ErrorList{field.Invalid(newPath.Child("SchedulerEstimatorTimeout"), metav1.Duration{Duration: -1 * time.Second}, "must be greater than or equal to 0")},
 		},
+		"invalid SchedulerName": {
+			opt: New(func(option *Options) {
+				option.SchedulerName = ""
+			}),
+			expectedErrs: field.ErrorList{field.Invalid(newPath.Child("SchedulerName"), "", "should not be empty")},
+		},
 	}
 
 	for _, testCase := range testCases {

diff --git a/pkg/webhook/clusterpropagationpolicy/validating.go b/pkg/webhook/clusterpropagationpolicy/validating.go
@@ -2,8 +2,10 @@ package clusterpropagationpolicy
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
+	admissionv1 "k8s.io/api/admission/v1"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
@@ -31,6 +33,20 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request)
 	}
 	klog.V(2).Infof("Validating ClusterPropagationPolicy(%s) for request: %s", policy.Name, req.Operation)
 
+	if req.Operation == admissionv1.Update {
+		oldPolicy := &policyv1alpha1.ClusterPropagationPolicy{}
+		err = v.decoder.DecodeRaw(req.OldObject, oldPolicy)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+
+		if policy.Spec.SchedulerName != oldPolicy.Spec.SchedulerName {
+			err = fmt.Errorf("the schedulerName should not be updated")
+			klog.Error(err)
+			return admission.Denied(err.Error())
+		}
+	}
+
 	if err := validation.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
 		klog.Error(err)
 		return admission.Denied(err.Error())

diff --git a/pkg/webhook/propagationpolicy/validating.go b/pkg/webhook/propagationpolicy/validating.go
@@ -2,8 +2,10 @@ package propagationpolicy
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 
+	admissionv1 "k8s.io/api/admission/v1"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
@@ -31,6 +33,20 @@ func (v *ValidatingAdmission) Handle(ctx context.Context, req admission.Request)
 	}
 	klog.V(2).Infof("Validating PropagationPolicy(%s/%s) for request: %s", policy.Namespace, policy.Name, req.Operation)
 
+	if req.Operation == admissionv1.Update {
+		oldPolicy := &policyv1alpha1.PropagationPolicy{}
+		err = v.decoder.DecodeRaw(req.OldObject, oldPolicy)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+
+		if policy.Spec.SchedulerName != oldPolicy.Spec.SchedulerName {
+			err = fmt.Errorf("the schedulerName should not be updated")
+			klog.Error(err)
+			return admission.Denied(err.Error())
+		}
+	}
+
 	if err := validation.ValidateSpreadConstraint(policy.Spec.Placement.SpreadConstraints); err != nil {
 		klog.Error(err)
 		return admission.Denied(err.Error())