Merge pull request #3407 from Poor12/webhook-change

Add validation for failover

api/openapi-spec/swagger.json

@@ -15044,7 +15044,7 @@
           "format": "int32"
         },
         "decisionConditions": {
-          "description": "DecisionConditions indicates the decision conditions of performing the failover process. Only when all conditions are met can the failover process be performed. Currently, DecisionConditions includes several conditions: - TolerationSeconds (optional) - HealthyState (mandatory)",
+          "description": "DecisionConditions indicates the decision conditions of performing the failover process. Only when all conditions are met can the failover process be performed. Currently, DecisionConditions includes several conditions: - TolerationSeconds (optional)",
           "default": {},
           "$ref": "#/definitions/com.github.karmada-io.karmada.pkg.apis.policy.v1alpha1.DecisionConditions"
         },

charts/karmada/_crds/bases/policy.karmada.io_clusterpropagationpolicies.yaml

@@ -87,7 +87,7 @@ spec:
                           of performing the failover process. Only when all conditions
                           are met can the failover process be performed. Currently,
                           DecisionConditions includes several conditions: - TolerationSeconds
-                          (optional) - HealthyState (mandatory)'
+                          (optional)'
                         properties:
                           tolerationSeconds:
                             default: 300

charts/karmada/_crds/bases/policy.karmada.io_propagationpolicies.yaml

@@ -83,7 +83,7 @@ spec:
                           of performing the failover process. Only when all conditions
                           are met can the failover process be performed. Currently,
                           DecisionConditions includes several conditions: - TolerationSeconds
-                          (optional) - HealthyState (mandatory)'
+                          (optional)'
                         properties:
                           tolerationSeconds:
                             default: 300

charts/karmada/_crds/bases/work.karmada.io_clusterresourcebindings.yaml

@@ -291,7 +291,7 @@ spec:
                           of performing the failover process. Only when all conditions
                           are met can the failover process be performed. Currently,
                           DecisionConditions includes several conditions: - TolerationSeconds
-                          (optional) - HealthyState (mandatory)'
+                          (optional)'
                         properties:
                           tolerationSeconds:
                             default: 300

charts/karmada/_crds/bases/work.karmada.io_resourcebindings.yaml

@@ -291,7 +291,7 @@ spec:
                           of performing the failover process. Only when all conditions
                           are met can the failover process be performed. Currently,
                           DecisionConditions includes several conditions: - TolerationSeconds
-                          (optional) - HealthyState (mandatory)'
+                          (optional)'
                         properties:
                           tolerationSeconds:
                             default: 300

pkg/apis/policy/v1alpha1/propagation_types.go

@@ -210,7 +210,6 @@ type ApplicationFailoverBehavior struct {
 	// Only when all conditions are met can the failover process be performed.
 	// Currently, DecisionConditions includes several conditions:
 	// - TolerationSeconds (optional)
-	// - HealthyState (mandatory)
 	// +required
 	DecisionConditions DecisionConditions `json:"decisionConditions"`
 

pkg/generated/openapi/zz_generated.openapi.go

@@ -2164,7 +2164,7 @@ func schema_pkg_apis_policy_v1alpha1_ApplicationFailoverBehavior(ref common.Refe
 					},
 					"decisionConditions": {
 						SchemaProps: spec.SchemaProps{
-							Description: "DecisionConditions indicates the decision conditions of performing the failover process. Only when all conditions are met can the failover process be performed. Currently, DecisionConditions includes several conditions: - TolerationSeconds (optional) - HealthyState (mandatory)",
+							Description: "DecisionConditions indicates the decision conditions of performing the failover process. Only when all conditions are met can the failover process be performed. Currently, DecisionConditions includes several conditions: - TolerationSeconds (optional)",
 							Default:     map[string]interface{}{},
 							Ref:         ref("github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1.DecisionConditions"),
 						},

pkg/util/validation/validation.go

@@ -21,6 +21,7 @@ const LabelValueMaxLength int = 63
 func ValidatePropagationSpec(spec policyv1alpha1.PropagationSpec) field.ErrorList {
 	var allErrs field.ErrorList
 	allErrs = append(allErrs, ValidatePlacement(spec.Placement, field.NewPath("spec").Child("placement"))...)
+	allErrs = append(allErrs, ValidateFailover(spec.Failover, field.NewPath("spec").Child("failover"))...)
 	return allErrs
 }
 
@@ -142,6 +143,42 @@ func ValidateSpreadConstraint(spreadConstraints []policyv1alpha1.SpreadConstrain
 	return allErrs
 }
 
+// ValidateFailover validates that the failoverBehavior is correctly defined.
+func ValidateFailover(failoverBehavior *policyv1alpha1.FailoverBehavior, fldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if failoverBehavior == nil {
+		return nil
+	}
+
+	allErrs = append(allErrs, ValidateApplicationFailover(failoverBehavior.Application, fldPath.Child("application"))...)
+	return allErrs
+}
+
+// ValidateApplicationFailover validates that the application failover is correctly defined.
+func ValidateApplicationFailover(applicationFailoverBehavior *policyv1alpha1.ApplicationFailoverBehavior, fldPath *field.Path) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if applicationFailoverBehavior == nil {
+		return nil
+	}
+
+	preConditions := applicationFailoverBehavior.PreConditions
+	if preConditions != nil && preConditions.DelaySeconds != nil && *preConditions.DelaySeconds < 0 {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("preConditions").Child("delaySeconds"), *preConditions.DelaySeconds, "must be greater than or equal to 0"))
+	}
+
+	if *applicationFailoverBehavior.DecisionConditions.TolerationSeconds < 0 {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("decisionConditions").Child("tolerationSeconds"), *applicationFailoverBehavior.DecisionConditions.TolerationSeconds, "must be greater than or equal to 0"))
+	}
+
+	if *applicationFailoverBehavior.BlockPredecessorSeconds < 0 {
+		allErrs = append(allErrs, field.Invalid(fldPath.Child("blockPredecessorSeconds"), *applicationFailoverBehavior.BlockPredecessorSeconds, "must be greater than or equal to 0"))
+	}
+
+	return allErrs
+}
+
 // ValidateOverrideSpec validates that the overrider specification is correctly defined.
 func ValidateOverrideSpec(overrideSpec *policyv1alpha1.OverrideSpec) field.ErrorList {
 	var allErrs field.ErrorList

pkg/util/validation/validation_test.go

@@ -5,6 +5,8 @@ import (
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/utils/pointer"
 
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 	"github.com/karmada-io/karmada/pkg/util"
@@ -554,3 +556,74 @@ func TestValidatePropagationSpec(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateApplicationFailover(t *testing.T) {
+	tests := []struct {
+		name                        string
+		applicationFailoverBehavior *policyv1alpha1.ApplicationFailoverBehavior
+		expectedErr                 string
+	}{
+		{
+			name:                        "application failover is nil",
+			applicationFailoverBehavior: nil,
+			expectedErr:                 "",
+		},
+		{
+			name: "the delaySeconds is less than zero",
+			applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
+				BlockPredecessorSeconds: pointer.Int32(100),
+				DecisionConditions: policyv1alpha1.DecisionConditions{
+					TolerationSeconds: pointer.Int32(100),
+				},
+				PreConditions: &policyv1alpha1.PreConditions{
+					DelaySeconds: pointer.Int32(-100),
+				},
+			},
+			expectedErr: "spec.failover.application.preConditions.delaySeconds: Invalid value: -100: must be greater than or equal to 0",
+		},
+		{
+			name: "the tolerationSeconds is less than zero",
+			applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
+				BlockPredecessorSeconds: pointer.Int32(100),
+				DecisionConditions: policyv1alpha1.DecisionConditions{
+					TolerationSeconds: pointer.Int32(-100),
+				},
+			},
+			expectedErr: "spec.failover.application.decisionConditions.tolerationSeconds: Invalid value: -100: must be greater than or equal to 0",
+		},
+		{
+			name: "the blockPredecessorSeconds is less than zero",
+			applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
+				BlockPredecessorSeconds: pointer.Int32(-100),
+				DecisionConditions: policyv1alpha1.DecisionConditions{
+					TolerationSeconds: pointer.Int32(100),
+				},
+			},
+			expectedErr: "spec.failover.application.blockPredecessorSeconds: Invalid value: -100: must be greater than or equal to 0",
+		},
+		{
+			name: "application behavior is correctly defined",
+			applicationFailoverBehavior: &policyv1alpha1.ApplicationFailoverBehavior{
+				BlockPredecessorSeconds: pointer.Int32(100),
+				DecisionConditions: policyv1alpha1.DecisionConditions{
+					TolerationSeconds: pointer.Int32(100),
+				},
+				PreConditions: &policyv1alpha1.PreConditions{
+					DelaySeconds: pointer.Int32(100),
+				},
+			},
+			expectedErr: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			errs := ValidateApplicationFailover(tt.applicationFailoverBehavior, field.NewPath("spec").Child("failover").Child("application"))
+			err := errs.ToAggregate()
+			if err != nil && err.Error() != tt.expectedErr {
+				t.Errorf("expected error:\n  %s, but got:\n  %s", tt.expectedErr, err.Error())
+			} else if err == nil && tt.expectedErr != "" {
+				t.Errorf("expected error:\n  %s, but got no error\n", tt.expectedErr)
+			}
+		})
+	}
+}