feat: CEL Validation for NodeClaim Kubelet (aws#596)

Makefile

@@ -32,6 +32,7 @@ licenses: download ## Verifies dependency licenses
 verify: ## Verify code. Includes codegen, dependencies, linting, formatting, etc
 	go mod tidy
 	go generate ./...
+	hack/validation/kubelet.sh
 	@# Use perl instead of sed due to https://stackoverflow.com/questions/4247068/sed-command-with-i-option-failing-on-mac-but-works-on-linux
 	@# We need to do this "sed replace" until controller-tools fixes this parameterized types issue: https://github.com/kubernetes-sigs/controller-tools/issues/756
 	@perl -i -pe 's/sets.Set/sets.Set[string]/g' pkg/scheduling/zz_generated.deepcopy.go

hack/validation/kubelet.sh

@@ -0,0 +1,11 @@
+# Kubelet Validation 
+
+# The regular expression will be adding a check for if kublet.evictionHard and kubelet.evictionSoft are percentage or a quantity value
+# Adding validation for nodeclaim 
+yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.kubelet.properties.evictionHard.additionalProperties.pattern = "^((\d{1,2}(\.\d{1,2})?|100(\.0{1,2})?)%||(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?)$"' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml 
+yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.kubelet.properties.evictionSoft.additionalProperties.pattern = "^((\d{1,2}(\.\d{1,2})?|100(\.0{1,2})?)%||(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?)$"' -i pkg/apis/crds/karpenter.sh_nodeclaims.yaml 
+
+# The regular expression will be adding a check for if kublet.evictionHard and kubelet.evictionSoft are percentage or a quantity value
+# Adding validation for nodepool
+yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.kubelet.properties.evictionHard.additionalProperties.pattern = "^((\d{1,2}(\.\d{1,2})?|100(\.0{1,2})?)%||(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?)$"' -i pkg/apis/crds/karpenter.sh_nodepools.yaml 
+yq eval '.spec.versions[0].schema.openAPIV3Schema.properties.spec.properties.template.properties.spec.properties.kubelet.properties.evictionSoft.additionalProperties.pattern = "^((\d{1,2}(\.\d{1,2})?|100(\.0{1,2})?)%||(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?)$"' -i pkg/apis/crds/karpenter.sh_nodepools.yaml 

pkg/apis/v1beta1/nodeclaim.go

@@ -40,6 +40,9 @@ type NodeClaimSpec struct {
 	// Kubelet defines args to be used when configuring kubelet on provisioned nodes.
 	// They are a subset of the upstream types, recognizing not all options may be supported.
 	// Wherever possible, the types and names should reflect the upstream kubelet types.
+	// +kubebuilder:validation:XValidation:message="imageGCHighThresholdPercent must be greater than imageGCLowThresholdPercent",rule="has(self.imageGCHighThresholdPercent) && has(self.imageGCLowThresholdPercent) ?  self.imageGCHighThresholdPercent > self.imageGCLowThresholdPercent  : true"
+	// +kubebuilder:validation:XValidation:message="evictionSoft OwnerKey does not have a matching evictionSoftGracePeriod",rule="has(self.evictionSoft) ? self.evictionSoft.all(e, (e in self.evictionSoftGracePeriod)):true"
+	// +kubebuilder:validation:XValidation:message="evictionSoftGracePeriod OwnerKey does not have a matching evictionSoft",rule="has(self.evictionSoftGracePeriod) ? self.evictionSoftGracePeriod.all(e, (e in self.evictionSoft)):true"
 	Kubelet *KubeletConfiguration `json:"kubelet,omitempty"`
 	// NodeClassRef is a reference to an object that defines provider specific configuration
 	// +required
@@ -83,18 +86,25 @@ type KubeletConfiguration struct {
 	// +optional
 	PodsPerCore *int32 `json:"podsPerCore,omitempty"`
 	// SystemReserved contains resources reserved for OS system daemons and kernel memory.
+	// +kubebuilder:validation:XValidation:message="valid keys for systemReserved are ['cpu','memory','ephemeral-storage','pid']",rule="self.all(x, x=='cpu' || x=='memory' || x=='ephemeral-storage' || x=='pid')"
+	// +kubebuilder:validation:XValidation:message="systemReserved value cannot be a negative resource quantity",rule="self.all(x, !self[x].startsWith('-'))"
 	// +optional
 	SystemReserved v1.ResourceList `json:"systemReserved,omitempty"`
 	// KubeReserved contains resources reserved for Kubernetes system components.
+	// +kubebuilder:validation:XValidation:message="valid keys for kubeReserved are ['cpu','memory','ephemeral-storage','pid']",rule="self.all(x, x=='cpu' || x=='memory' || x=='ephemeral-storage' || x=='pid')"
+	// +kubebuilder:validation:XValidation:message="kubeReserved value cannot be a negative resource quantity",rule="self.all(x, !self[x].startsWith('-'))"
 	// +optional
 	KubeReserved v1.ResourceList `json:"kubeReserved,omitempty"`
 	// EvictionHard is the map of signal names to quantities that define hard eviction thresholds
+	// +kubebuilder:validation:XValidation:message="valid keys for evictionHard are ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available']",rule="self.all(x, x in ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available'])"
 	// +optional
 	EvictionHard map[string]string `json:"evictionHard,omitempty"`
 	// EvictionSoft is the map of signal names to quantities that define soft eviction thresholds
+	// +kubebuilder:validation:XValidation:message="valid keys for evictionSoft are ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available']",rule="self.all(x, x in ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available'])"
 	// +optional
 	EvictionSoft map[string]string `json:"evictionSoft,omitempty"`
 	// EvictionSoftGracePeriod is the map of signal names to quantities that define grace periods for each eviction signal
+	// +kubebuilder:validation:XValidation:message="valid keys for evictionSoftGracePeriod are ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available']",rule="self.all(x, x in ['memory.available','nodefs.available','nodefs.inodesFree','imagefs.available','imagefs.inodesFree','pid.available'])"
 	// +optional
 	EvictionSoftGracePeriod map[string]metav1.Duration `json:"evictionSoftGracePeriod,omitempty"`
 	// EvictionMaxPodGracePeriod is the maximum allowed grace period (in seconds) to use when terminating pods in

pkg/apis/v1beta1/nodeclaim_validation_test.go

@@ -307,35 +307,25 @@ var _ = Describe("Validation", func() {
 			})
 		})
 		Context("GCThresholdPercent", func() {
-			Context("ImageGCHighThresholdPercent", func() {
-				It("should succeed on a imageGCHighThresholdPercent", func() {
-					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
-						ImageGCHighThresholdPercent: ptr.Int32(10),
-					}
-					Expect(nodeClaim.Validate(ctx)).To(Succeed())
-				})
-				It("should fail when imageGCHighThresholdPercent is less than imageGCLowThresholdPercent", func() {
-					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
-						ImageGCHighThresholdPercent: ptr.Int32(50),
-						ImageGCLowThresholdPercent:  ptr.Int32(60),
-					}
-					Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
-				})
+			It("should succeed on a valid imageGCHighThresholdPercent", func() {
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
+					ImageGCHighThresholdPercent: ptr.Int32(10),
+				}
+				Expect(nodeClaim.Validate(ctx)).To(Succeed())
 			})
-			Context("ImageGCLowThresholdPercent", func() {
-				It("should succeed on a imageGCLowThresholdPercent", func() {
-					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
-						ImageGCLowThresholdPercent: ptr.Int32(10),
-					}
-					Expect(nodeClaim.Validate(ctx)).To(Succeed())
-				})
-				It("should fail when imageGCLowThresholdPercent is greather than imageGCHighThresheldPercent", func() {
-					nodeClaim.Spec.Kubelet = &KubeletConfiguration{
-						ImageGCHighThresholdPercent: ptr.Int32(50),
-						ImageGCLowThresholdPercent:  ptr.Int32(60),
-					}
-					Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
-				})
+			It("should fail when imageGCHighThresholdPercent is less than imageGCLowThresholdPercent", func() {
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
+					ImageGCHighThresholdPercent: ptr.Int32(50),
+					ImageGCLowThresholdPercent:  ptr.Int32(60),
+				}
+				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
+			})
+			It("should fail when imageGCLowThresholdPercent is greather than imageGCHighThresheldPercent", func() {
+				nodeClaim.Spec.Kubelet = &KubeletConfiguration{
+					ImageGCHighThresholdPercent: ptr.Int32(50),
+					ImageGCLowThresholdPercent:  ptr.Int32(60),
+				}
+				Expect(nodeClaim.Validate(ctx)).ToNot(Succeed())
 			})
 		})
 		Context("Eviction Soft Grace Period", func() {

pkg/apis/v1beta1/nodepool_validation_cel_test.go

@@ -23,7 +23,9 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/samber/lo"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/ptr"
 
 	. "github.com/aws/karpenter-core/pkg/apis/v1beta1"
 )
@@ -97,4 +99,267 @@ var _ = Describe("CEL/Validation", func() {
 			Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
 		})
 	})
+	Context("KubeletConfiguration", func() {
+		It("should succeed on kubeReserved with invalid keys", func() {
+			nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+				KubeReserved: v1.ResourceList{
+					v1.ResourceCPU: resource.MustParse("2"),
+				},
+			}
+			Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+		})
+		It("should succeed on systemReserved with invalid keys", func() {
+			nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+				SystemReserved: v1.ResourceList{
+					v1.ResourceCPU: resource.MustParse("2"),
+				},
+			}
+			Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+		})
+		It("should fail on kubeReserved with invalid keys", func() {
+			nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+				KubeReserved: v1.ResourceList{
+					v1.ResourcePods: resource.MustParse("2"),
+				},
+			}
+			Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+		})
+		It("should fail on systemReserved with invalid keys", func() {
+			nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+				SystemReserved: v1.ResourceList{
+					v1.ResourcePods: resource.MustParse("2"),
+				},
+			}
+			Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+		})
+		Context("Eviction Signals", func() {
+			Context("Eviction Hard", func() {
+				It("should succeed on evictionHard with valid keys and values", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						EvictionHard: map[string]string{
+							"memory.available":   "5%",
+							"nodefs.available":   "10%",
+							"nodefs.inodesFree":  "15%",
+							"imagefs.available":  "5%",
+							"imagefs.inodesFree": "5%",
+							"pid.available":      "5%",
+						},
+					}
+					Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+				})
+				It("should succeed on evictionHard with valid keys and values", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						EvictionHard: map[string]string{
+							"memory.available":   "20Mi",
+							"nodefs.available":   "34G",
+							"nodefs.inodesFree":  "25M",
+							"imagefs.available":  "20Gi",
+							"imagefs.inodesFree": "39Gi",
+							"pid.available":      "20G",
+						},
+					}
+					Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+				})
+				It("should fail on evictionHard with invalid keys", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						EvictionHard: map[string]string{
+							"memory": "5%",
+						},
+					}
+					Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+				})
+				It("should fail on invalid formatted percentage value in evictionHard", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						EvictionHard: map[string]string{
+							"memory.available": "5%3",
+						},
+					}
+					Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+				})
+				It("should fail on invalid percentage value (too large) in evictionHard", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						EvictionHard: map[string]string{
+							"memory.available": "110%",
+						},
+					}
+					Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+				})
+				It("should fail on invalid quantity value in evictionHard", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						EvictionHard: map[string]string{
+							"memory.available": "110GB",
+						},
+					}
+					Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+				})
+			})
+		})
+		Context("Eviction Soft", func() {
+			It("should succeed on evictionSoft with valid keys and values", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available":   "5%",
+						"nodefs.available":   "10%",
+						"nodefs.inodesFree":  "15%",
+						"imagefs.available":  "5%",
+						"imagefs.inodesFree": "5%",
+						"pid.available":      "5%",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available":   {Duration: time.Minute},
+						"nodefs.available":   {Duration: time.Second * 90},
+						"nodefs.inodesFree":  {Duration: time.Minute * 5},
+						"imagefs.available":  {Duration: time.Hour},
+						"imagefs.inodesFree": {Duration: time.Hour * 24},
+						"pid.available":      {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+			})
+			It("should succeed on evictionSoft with valid keys and values", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available":   "20Mi",
+						"nodefs.available":   "34G",
+						"nodefs.inodesFree":  "25M",
+						"imagefs.available":  "20Gi",
+						"imagefs.inodesFree": "39Gi",
+						"pid.available":      "20G",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available":   {Duration: time.Minute},
+						"nodefs.available":   {Duration: time.Second * 90},
+						"nodefs.inodesFree":  {Duration: time.Minute * 5},
+						"imagefs.available":  {Duration: time.Hour},
+						"imagefs.inodesFree": {Duration: time.Hour * 24},
+						"pid.available":      {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+			})
+			It("should fail on evictionSoft with invalid keys", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory": "5%",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory": {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+			It("should fail on invalid formatted percentage value in evictionSoft", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available": "5%3",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available": {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+			It("should fail on invalid percentage value (too large) in evictionSoft", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available": "110%",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available": {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+			It("should fail on invalid quantity value in evictionSoft", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available": "110GB",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available": {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+			It("should fail when eviction soft doesn't have matching grace period", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available": "200Mi",
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+		})
+		Context("GCThresholdPercent", func() {
+			Context("ImageGCHighThresholdPercent", func() {
+				It("should succeed on a imageGCHighThresholdPercent", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						ImageGCHighThresholdPercent: ptr.Int32(10),
+					}
+					Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+				})
+				It("should fail when imageGCHighThresholdPercent is less than imageGCLowThresholdPercent", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						ImageGCHighThresholdPercent: ptr.Int32(50),
+						ImageGCLowThresholdPercent:  ptr.Int32(60),
+					}
+					Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+				})
+			})
+			Context("ImageGCLowThresholdPercent", func() {
+				It("should succeed on a imageGCLowThresholdPercent", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						ImageGCLowThresholdPercent: ptr.Int32(10),
+					}
+					Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+				})
+				It("should fail when imageGCLowThresholdPercent is greather than imageGCHighThresheldPercent", func() {
+					nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+						ImageGCHighThresholdPercent: ptr.Int32(50),
+						ImageGCLowThresholdPercent:  ptr.Int32(60),
+					}
+					Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+				})
+			})
+		})
+		Context("Eviction Soft Grace Period", func() {
+			It("should succeed on evictionSoftGracePeriod with valid keys", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoft: map[string]string{
+						"memory.available":   "5%",
+						"nodefs.available":   "10%",
+						"nodefs.inodesFree":  "15%",
+						"imagefs.available":  "5%",
+						"imagefs.inodesFree": "5%",
+						"pid.available":      "5%",
+					},
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available":   {Duration: time.Minute},
+						"nodefs.available":   {Duration: time.Second * 90},
+						"nodefs.inodesFree":  {Duration: time.Minute * 5},
+						"imagefs.available":  {Duration: time.Hour},
+						"imagefs.inodesFree": {Duration: time.Hour * 24},
+						"pid.available":      {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).To(Succeed())
+			})
+			It("should fail on evictionSoftGracePeriod with invalid keys", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory": {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+			It("should fail when eviction soft grace period doesn't have matching threshold", func() {
+				nodePool.Spec.Template.Spec.Kubelet = &KubeletConfiguration{
+					EvictionSoftGracePeriod: map[string]metav1.Duration{
+						"memory.available": {Duration: time.Minute},
+					},
+				}
+				Expect(env.Client.Create(ctx, nodePool)).ToNot(Succeed())
+			})
+		})
+	})
 })

pkg/apis/v1beta1/nodepool_validation_webhook_test.go

@@ -517,7 +517,3 @@ var _ = Describe("Limits", func() {
 		Expect(provisioner.Spec.Limits.ExceededBy(provisioner.Status.Resources)).To(MatchError("cpu resource usage of 17 exceeds limit of 16"))
 	})
 })
-
-var _ = Describe("CEL/Validation", func() {
-
-})