fix: Fix budget validation through CEL (#1006)

pkg/apis/crds/karpenter.sh_nodepools.yaml

@@ -109,7 +109,7 @@ spec:
                       type: array
                       x-kubernetes-validations:
                         - message: '''schedule'' must be set with ''duration'''
-                          rule: '!self.all(x, (has(x.schedule) && !has(x.duration)) || (!has(x.schedule) && has(x.duration)))'
+                          rule: self.all(x, has(x.schedule) == has(x.duration))
                     consolidateAfter:
                       description: |-
                         ConsolidateAfter is the duration the controller will wait

pkg/apis/v1beta1/nodepool.go

@@ -91,7 +91,7 @@ type Disruption struct {
 	// If there are multiple active budgets, Karpenter uses
 	// the most restrictive value. If left undefined,
 	// this will default to one budget with a value to 10%.
-	// +kubebuilder:validation:XValidation:message="'schedule' must be set with 'duration'",rule="!self.all(x, (has(x.schedule) && !has(x.duration)) || (!has(x.schedule) && has(x.duration)))"
+	// +kubebuilder:validation:XValidation:message="'schedule' must be set with 'duration'",rule="self.all(x, has(x.schedule) == has(x.duration))"
 	// +kubebuilder:default:={{nodes: "10%"}}
 	// +kubebuilder:validation:MaxItems=50
 	// +optional

pkg/apis/v1beta1/nodepool_validation_cel_test.go

@@ -188,7 +188,7 @@ var _ = Describe("CEL/Validation", func() {
 			}}
 			Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
 		})
-		It("should fail when creating two budgets where one is invalid", func() {
+		It("should fail when creating two budgets where one has an invalid crontab", func() {
 			nodePool.Spec.Disruption.Budgets = []Budget{
 				{
 					Nodes:    "10",
@@ -202,6 +202,23 @@ var _ = Describe("CEL/Validation", func() {
 				}}
 			Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
 		})
+		It("should fail when creating multiple budgets where one doesn't have both schedule and duration", func() {
+			nodePool.Spec.Disruption.Budgets = []Budget{
+				{
+					Nodes:    "10",
+					Duration: &metav1.Duration{Duration: lo.Must(time.ParseDuration("20m"))},
+				},
+				{
+					Nodes:    "10",
+					Schedule: ptr.String("* * * * *"),
+					Duration: &metav1.Duration{Duration: lo.Must(time.ParseDuration("20m"))},
+				},
+				{
+					Nodes: "10",
+				},
+			}
+			Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+		})
 	})
 	Context("KubeletConfiguration", func() {
 		It("should succeed on kubeReserved with invalid keys", func() {

pkg/apis/v1beta1/nodepool_validation_webhook_test.go

@@ -131,7 +131,7 @@ var _ = Describe("Webhook/Validation", func() {
 			}}
 			Expect(nodePool.Validate(ctx)).To(Succeed())
 		})
-		It("should fail to validate two budgets where one is invalid", func() {
+		It("should fail when creating two budgets where one has an invalid crontab", func() {
 			nodePool.Spec.Disruption.Budgets = []Budget{
 				{
 					Nodes:    "10",
@@ -145,6 +145,23 @@ var _ = Describe("Webhook/Validation", func() {
 				}}
 			Expect(nodePool.Validate(ctx)).ToNot(Succeed())
 		})
+		It("should fail when creating multiple budgets where one doesn't have both schedule and duration", func() {
+			nodePool.Spec.Disruption.Budgets = []Budget{
+				{
+					Nodes:    "10",
+					Duration: &metav1.Duration{Duration: lo.Must(time.ParseDuration("20m"))},
+				},
+				{
+					Nodes:    "10",
+					Schedule: ptr.String("* * * * *"),
+					Duration: &metav1.Duration{Duration: lo.Must(time.ParseDuration("20m"))},
+				},
+				{
+					Nodes: "10",
+				},
+			}
+			Expect(nodePool.Validate(ctx)).ToNot(Succeed())
+		})
 	})
 	Context("Limits", func() {
 		It("should allow undefined limits", func() {