feat: migrate bootstrap-only validations to CEL

api/v1alpha1/nodereadinessrule_types.go

@@ -67,6 +67,8 @@ const (
 //
 // +kubebuilder:validation:XValidation:rule="(!has(oldSelf.conditionPolicy) ? 'allOf' : oldSelf.conditionPolicy) == (!has(self.conditionPolicy) ? 'allOf' : self.conditionPolicy)",message="conditionPolicy is immutable"
 // +kubebuilder:validation:XValidation:rule="!has(self.conditionPolicy) || self.conditionPolicy != 'anyOf' || self.conditions.all(c, !has(c.defaultStatus))",message="defaultStatus is not supported when conditionPolicy is anyOf"
+// +kubebuilder:validation:XValidation:rule="!has(self.enforcementMode) || self.enforcementMode != 'bootstrap-only' || !has(self.conditionPolicy) || self.conditionPolicy != 'anyOf'",message="anyOf conditionPolicy is not supported with bootstrap-only enforcementMode"
+// +kubebuilder:validation:XValidation:rule="!has(self.enforcementMode) || self.enforcementMode != 'bootstrap-only' || !has(self.conditions) || self.conditions.all(c, !has(c.defaultStatus))",message="defaultStatus should not be used with bootstrap-only enforcementMode"
 type NodeReadinessRuleSpec struct {
 	// conditions contains a list of the Node conditions that defines the specific
 	// criteria that must be met for taints to be managed on the target Node.

charts/node-readiness-controller/crds/nodereadinessrules.readiness.node.x-k8s.io.yaml

@@ -269,6 +269,13 @@ spec:
             - message: defaultStatus is not supported when conditionPolicy is anyOf
               rule: '!has(self.conditionPolicy) || self.conditionPolicy != ''anyOf''
                 || self.conditions.all(c, !has(c.defaultStatus))'
+            - message: anyOf conditionPolicy is not supported with bootstrap-only
+                enforcementMode
+              rule: '!has(self.enforcementMode) || self.enforcementMode != ''bootstrap-only''
+                || !has(self.conditionPolicy) || self.conditionPolicy != ''anyOf'''
+            - message: defaultStatus should not be used with bootstrap-only enforcementMode
+              rule: '!has(self.enforcementMode) || self.enforcementMode != ''bootstrap-only''
+                || !has(self.conditions) || self.conditions.all(c, !has(c.defaultStatus))'
           status:
             description: status defines the observed state of NodeReadinessRule
             minProperties: 1

config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml

@@ -269,6 +269,13 @@ spec:
             - message: defaultStatus is not supported when conditionPolicy is anyOf
               rule: '!has(self.conditionPolicy) || self.conditionPolicy != ''anyOf''
                 || self.conditions.all(c, !has(c.defaultStatus))'
+            - message: anyOf conditionPolicy is not supported with bootstrap-only
+                enforcementMode
+              rule: '!has(self.enforcementMode) || self.enforcementMode != ''bootstrap-only''
+                || !has(self.conditionPolicy) || self.conditionPolicy != ''anyOf'''
+            - message: defaultStatus should not be used with bootstrap-only enforcementMode
+              rule: '!has(self.enforcementMode) || self.enforcementMode != ''bootstrap-only''
+                || !has(self.conditions) || self.conditions.all(c, !has(c.defaultStatus))'
           status:
             description: status defines the observed state of NodeReadinessRule
             minProperties: 1

docs/book/src/user-guide/concepts.md

@@ -86,7 +86,7 @@ The choice of `defaultStatus` has critical interaction with the rule's `enforcem
 >
 > `defaultStatus` is useful when a condition may never appear on a node in its healthy state, effectively treating its absence as a known-good signal. However, `bootstrap-only` mode exists precisely to *wait* for conditions to be explicitly reported before completing the bootstrap gate.
 >
- > Because these two features serve opposing purposes, using them together can lead to unintended behavior, such as completing the bootstrap phase before a condition is actually verified. To prevent  this, the admission webhook explicitly rejects this combination.
+ > Because these two features serve opposing purposes, using them together can lead to unintended behavior, such as completing the bootstrap phase before a condition is actually verified. To prevent this, the controller explicitly rejects this combination.
 
 ## Readiness Condition Reporting
 

internal/controller/nodereadinessrule_controller_test.go

@@ -2114,6 +2114,43 @@ var _ = Describe("NodeReadinessRule Controller", func() {
 		})
 	})
 
+	Context("CEL Validation for bootstrap-only constraints", func() {
+		It("should reject rule creation when enforcementMode is bootstrap-only and defaultStatus is set", func() {
+			rule := &nodereadinessiov1alpha1.NodeReadinessRule{
+				ObjectMeta: metav1.ObjectMeta{Name: "cel-test-reject-bootstrap-defaultstatus"},
+				Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
+					EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
+					Taint:           corev1.Taint{Key: "readiness.k8s.io/cel-test", Effect: corev1.TaintEffectNoSchedule},
+					NodeSelector:    metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+					Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
+						{Type: "TestReady", RequiredStatus: corev1.ConditionTrue, DefaultStatus: corev1.ConditionFalse},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, rule)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("defaultStatus should not be used with bootstrap-only enforcementMode"))
+		})
+
+		It("should reject rule creation when enforcementMode is bootstrap-only and conditionPolicy is anyOf", func() {
+			rule := &nodereadinessiov1alpha1.NodeReadinessRule{
+				ObjectMeta: metav1.ObjectMeta{Name: "cel-test-reject-bootstrap-anyof"},
+				Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
+					EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
+					Taint:           corev1.Taint{Key: "readiness.k8s.io/cel-test", Effect: corev1.TaintEffectNoSchedule},
+					ConditionPolicy: nodereadinessiov1alpha1.ConditionPolicyAnyOf,
+					NodeSelector:    metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
+					Conditions: []nodereadinessiov1alpha1.ConditionRequirement{
+						{Type: "TestReady", RequiredStatus: corev1.ConditionTrue},
+					},
+				},
+			}
+			err := k8sClient.Create(ctx, rule)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("anyOf conditionPolicy is not supported with bootstrap-only enforcementMode"))
+		})
+	})
+
 	Context("when existing rule is updated", func() {
 		var rule *nodereadinessiov1alpha1.NodeReadinessRule
 		var node *corev1.Node

internal/webhook/nodereadinessgaterule_webhook.go

@@ -47,7 +47,7 @@ func (w *NodeReadinessRuleWebhook) validateNodeReadinessRule(ctx context.Context
 	allErrs := make(field.ErrorList, 0, 4)
 
 	// Validate basic fields
-	allErrs = append(allErrs, w.validateSpec(rule.Spec, isUpdate)...)
+	allErrs = append(allErrs, w.validateSpec(rule.Spec)...)
 
 	// Check for conflicting rules (same taint key)
 	allErrs = append(allErrs, w.validateTaintConflicts(ctx, rule, isUpdate)...)
@@ -58,7 +58,6 @@ func (w *NodeReadinessRuleWebhook) validateNodeReadinessRule(ctx context.Context
 // validateSpec validates the spec fields that CRD CEL based XValidation cannot handle.
 func (w *NodeReadinessRuleWebhook) validateSpec(
 	spec readinessv1alpha1.NodeReadinessRuleSpec,
-	isUpdate bool,
 ) field.ErrorList {
 	var allErrs field.ErrorList
 
@@ -71,33 +70,6 @@ func (w *NodeReadinessRuleWebhook) validateSpec(
 		allErrs = append(allErrs, field.Required(field.NewPath("spec", "nodeSelector"), "nodeSelector must not be empty"))
 	}
 
-	// skip below checks for update because `enforcementMode`, `conditions`,
-	// and `conditionPolicy` are immutable as constrained by CEL XValidation rules.
-	if isUpdate {
-		return allErrs
-	}
-
-	// validate defaultStatus is not used in bootstrap-only mode
-	if spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly {
-		for i, cond := range spec.Conditions {
-			if cond.DefaultStatus != "" {
-				allErrs = append(allErrs, field.Forbidden(
-					field.NewPath("spec", "conditions").Index(i).Child("defaultStatus"),
-					"defaultStatus should not be used with bootstrap-only enforcementMode",
-				))
-			}
-		}
-	}
-
-	// validate anyOf conditionPolicy is not combined with bootstrap-only mode.
-	if spec.ConditionPolicy == readinessv1alpha1.ConditionPolicyAnyOf &&
-		spec.EnforcementMode == readinessv1alpha1.EnforcementModeBootstrapOnly {
-		allErrs = append(allErrs, field.Forbidden(
-			field.NewPath("spec", "conditionPolicy"),
-			"anyOf conditionPolicy is not supported with bootstrap-only enforcementMode",
-		))
-	}
-
 	return allErrs
 }
 

internal/webhook/nodereadinessgaterule_webhook_test.go

@@ -63,8 +63,7 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
 					},
 				},
 			}
-			// using isUpdate true to keep the test isolated to nodeSelector validation
-			allErrs := webhook.validateSpec(rule.Spec, true)
+			allErrs := webhook.validateSpec(rule.Spec)
 			Expect(allErrs).To(HaveLen(1))
 			Expect(allErrs[0].Field).To(Equal("spec.nodeSelector"))
 		})
@@ -79,9 +78,7 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
 					},
 				},
 			}
-
-			// using isUpdate true to keep the test isolated to nodeSelector validation
-			allErrs := webhook.validateSpec(rule.Spec, true)
+			allErrs := webhook.validateSpec(rule.Spec)
 			Expect(allErrs).To(BeEmpty())
 		})
 
@@ -99,9 +96,7 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
 						EnforcementMode: readinessv1alpha1.EnforcementModeContinuous,
 					},
 				}
-
-				// using isUpdate true to keep the test isolated to nodeSelector validation
-				allErrs := webhook.validateSpec(rule.Spec, true)
+				allErrs := webhook.validateSpec(rule.Spec)
 				Expect(allErrs).To(HaveLen(1))
 				Expect(allErrs[0].Field).To(Equal("spec.nodeSelector"))
 				Expect(allErrs[0].Type).To(Equal(field.ErrorTypeRequired))
@@ -124,111 +119,13 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
 						EnforcementMode: readinessv1alpha1.EnforcementModeContinuous,
 					},
 				}
-
-				// using isUpdate true to keep the test isolated to nodeSelector validation
-				allErrs := webhook.validateSpec(rule.Spec, true)
+				allErrs := webhook.validateSpec(rule.Spec)
 				Expect(allErrs).To(HaveLen(1))
 				Expect(allErrs[0].Field).To(Equal("spec.nodeSelector"))
 				Expect(allErrs[0].Type).To(Equal(field.ErrorTypeInvalid))
 			})
 		})
 
-		Context("defaultStatus enforcement", func() {
-			var spec readinessv1alpha1.NodeReadinessRuleSpec
-
-			BeforeEach(func() {
-				spec = readinessv1alpha1.NodeReadinessRuleSpec{
-					NodeSelector: metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"node-role.kubernetes.io/worker": "",
-						},
-					},
-					Conditions: []readinessv1alpha1.ConditionRequirement{
-						{
-							Type:           "Ready",
-							RequiredStatus: corev1.ConditionTrue,
-							DefaultStatus:  corev1.ConditionTrue, // offending field
-						},
-					},
-					EnforcementMode: readinessv1alpha1.EnforcementModeBootstrapOnly,
-				}
-			})
-
-			It("should skip defaultStatus check on update (early return)", func() {
-				allErrs := webhook.validateSpec(spec, true)
-				Expect(allErrs).To(BeEmpty())
-			})
-
-			It("should skip defaultStatus check for continuous enforcement", func() {
-				spec.EnforcementMode = readinessv1alpha1.EnforcementModeContinuous
-				allErrs := webhook.validateSpec(spec, false)
-				Expect(allErrs).To(BeEmpty())
-			})
-
-			It("should forbid defaultStatus with bootstrap-only enforcement", func() {
-				allErrs := webhook.validateSpec(spec, false)
-				Expect(allErrs).To(HaveLen(1))
-				Expect(allErrs[0].Field).To(Equal("spec.conditions[0].defaultStatus"))
-				Expect(allErrs[0].Type).To(Equal(field.ErrorTypeForbidden))
-			})
-		})
-
-		Context("conditionPolicy with bootstrap-only", func() {
-			var spec readinessv1alpha1.NodeReadinessRuleSpec
-
-			BeforeEach(func() {
-				spec = readinessv1alpha1.NodeReadinessRuleSpec{
-					NodeSelector:    metav1.LabelSelector{MatchLabels: map[string]string{"foo": "bar"}},
-					ConditionPolicy: readinessv1alpha1.ConditionPolicyAnyOf,
-					EnforcementMode: readinessv1alpha1.EnforcementModeBootstrapOnly,
-				}
-			})
-
-			It("should skip conditionPolicy check on update (early return)", func() {
-				allErrs := webhook.validateSpec(spec, true)
-				Expect(allErrs).To(BeEmpty())
-			})
-
-			It("should allow anyOf conditionPolicy for continuous enforcement", func() {
-				spec.EnforcementMode = readinessv1alpha1.EnforcementModeContinuous
-				allErrs := webhook.validateSpec(spec, false)
-				Expect(allErrs).To(BeEmpty())
-			})
-
-			It("should forbid anyOf conditionPolicy with bootstrap-only enforcement", func() {
-				allErrs := webhook.validateSpec(spec, false)
-				Expect(allErrs).To(HaveLen(1))
-				Expect(allErrs[0].Field).To(Equal("spec.conditionPolicy"))
-				Expect(allErrs[0].Type).To(Equal(field.ErrorTypeForbidden))
-			})
-		})
-
-		It("should accumulate errors across nodeSelector, defaultStatus, and conditionPolicy violations", func() {
-			spec := readinessv1alpha1.NodeReadinessRuleSpec{
-				NodeSelector:    metav1.LabelSelector{},                 // empty → ErrorTypeRequired
-				ConditionPolicy: readinessv1alpha1.ConditionPolicyAnyOf, // anyOf + bootstrap-only → ErrorTypeForbidden
-				Conditions: []readinessv1alpha1.ConditionRequirement{
-					{
-						Type:           "Ready",
-						RequiredStatus: corev1.ConditionTrue,
-						DefaultStatus:  corev1.ConditionFalse,
-					},
-					{Type: "NetworkReady",
-						RequiredStatus: corev1.ConditionTrue,
-						DefaultStatus:  corev1.ConditionTrue,
-					},
-				},
-				EnforcementMode: readinessv1alpha1.EnforcementModeBootstrapOnly,
-			}
-
-			allErrs := webhook.validateSpec(spec, false)
-			Expect(allErrs).To(HaveLen(4))
-			Expect(allErrs[0].Field).To(Equal("spec.nodeSelector"))
-			Expect(allErrs[1].Field).To(Equal("spec.conditions[0].defaultStatus"))
-			Expect(allErrs[2].Field).To(Equal("spec.conditions[1].defaultStatus"))
-			Expect(allErrs[3].Field).To(Equal("spec.conditionPolicy"))
-		})
-
 		It("should pass validation for valid spec", func() {
 			rule := &readinessv1alpha1.NodeReadinessRule{
 				Spec: readinessv1alpha1.NodeReadinessRuleSpec{
@@ -250,7 +147,7 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
 				},
 			}
 
-			allErrs := webhook.validateSpec(rule.Spec, false)
+			allErrs := webhook.validateSpec(rule.Spec)
 			Expect(allErrs).To(BeEmpty())
 		})
 	})
@@ -893,60 +790,6 @@ var _ = Describe("NodeReadinessRule Validation Webhook", func() {
 			Expect(allErrs).To(HaveLen(1)) // Empty nodeSelector validation
 			Expect(allErrs[0].Field).To(Equal("spec.nodeSelector"))
 
-			// Test defaultStatus used with bootstrap-only enforcement
-			bootstrapDefaultStatusRule := &readinessv1alpha1.NodeReadinessRule{
-				ObjectMeta: metav1.ObjectMeta{Name: "bootstrap-defaultstatus-comprehensive"},
-				Spec: readinessv1alpha1.NodeReadinessRuleSpec{
-					Conditions: []readinessv1alpha1.ConditionRequirement{
-						{
-							Type:           "Ready",
-							RequiredStatus: corev1.ConditionTrue,
-							DefaultStatus:  corev1.ConditionFalse,
-						},
-					},
-					NodeSelector: metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"node-role.kubernetes.io/worker": "",
-						},
-					},
-					Taint: corev1.Taint{
-						Key:    "readiness.k8s.io/test-key",
-						Effect: corev1.TaintEffectNoSchedule,
-					},
-					EnforcementMode: readinessv1alpha1.EnforcementModeBootstrapOnly,
-				},
-			}
-
-			allErrs = webhook.validateNodeReadinessRule(ctx, bootstrapDefaultStatusRule, false)
-			Expect(allErrs).To(HaveLen(1))
-			Expect(allErrs[0].Field).To(Equal("spec.conditions[0].defaultStatus"))
-			Expect(allErrs[0].Type).To(Equal(field.ErrorTypeForbidden))
-
-			// Test anyOf conditionPolicy used with bootstrap-only enforcement
-			bootstrapAnyOfPolicyRule := &readinessv1alpha1.NodeReadinessRule{
-				ObjectMeta: metav1.ObjectMeta{Name: "bootstrap-anyof-comprehensive"},
-				Spec: readinessv1alpha1.NodeReadinessRuleSpec{
-					Conditions: []readinessv1alpha1.ConditionRequirement{
-						{Type: "Ready", RequiredStatus: corev1.ConditionTrue},
-					},
-					ConditionPolicy: readinessv1alpha1.ConditionPolicyAnyOf,
-					NodeSelector: metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							"node-role.kubernetes.io/worker": "",
-						},
-					},
-					Taint: corev1.Taint{
-						Key:    "readiness.k8s.io/test-key-2",
-						Effect: corev1.TaintEffectNoSchedule,
-					},
-					EnforcementMode: readinessv1alpha1.EnforcementModeBootstrapOnly,
-				},
-			}
-
-			allErrs = webhook.validateNodeReadinessRule(ctx, bootstrapAnyOfPolicyRule, false)
-			Expect(allErrs).To(HaveLen(1))
-			Expect(allErrs[0].Field).To(Equal("spec.conditionPolicy"))
-			Expect(allErrs[0].Type).To(Equal(field.ErrorTypeForbidden))
 		})
 	})
 })