From 3fbaae49755d7aefcd66a92dc50d43fa0d19a05d Mon Sep 17 00:00:00 2001 From: Priyanka Saggu Date: Sun, 8 Mar 2026 02:07:04 +0530 Subject: [PATCH 1/2] add CEL validation for taint key format to match k8s qualified name rules exactly one '/' separator (prefix/name format), 1-63 character, alphanumeric, '-', '_', '.', must start/end with alphanumeric ref: git.k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/api/validate/content/kube.go#L24-L72 --- api/v1alpha1/nodereadinessrule_types.go | 8 ++++++++ ...readiness.node.x-k8s.io_nodereadinessrules.yaml | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/api/v1alpha1/nodereadinessrule_types.go b/api/v1alpha1/nodereadinessrule_types.go index 5944f29..54d23e8 100644 --- a/api/v1alpha1/nodereadinessrule_types.go +++ b/api/v1alpha1/nodereadinessrule_types.go @@ -69,6 +69,11 @@ type NodeReadinessRuleSpec struct { // taint defines the specific Taint (Key, Value, and Effect) to be managed // on Nodes that meet the defined condition criteria. // + // The taint key must follow Kubernetes qualified name format: prefix/name + // where prefix is 'readiness.k8s.io' (DNS subdomain) and name is a qualified + // name (max 63 chars, alphanumeric, '-', '_', '.', must start and end with alphanumeric). + // ref: git.k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/api/validate/content/kube.go#L24-L72 + // // Supported effects: NoSchedule, PreferNoSchedule, NoExecute. // Caution: NoExecute evicts existing pods and can cause significant disruption // when combined with continuous enforcement mode. Prefer NoSchedule for most use cases. @@ -76,6 +81,9 @@ type NodeReadinessRuleSpec struct { // +required // +kubebuilder:validation:XValidation:rule="self.key.startsWith('readiness.k8s.io/')",message="taint key must start with 'readiness.k8s.io/'" // +kubebuilder:validation:XValidation:rule="self.key.size() <= 253",message="taint key length must be at most 253 characters" + // +kubebuilder:validation:XValidation:rule="size(self.key.split('/')) == 2",message="taint key must have exactly one '/' separator (prefix/name format)" + // +kubebuilder:validation:XValidation:rule="size(self.key.split('/')[1]) > 0 && size(self.key.split('/')[1]) <= 63",message="taint key name part must be 1-63 characters" + // +kubebuilder:validation:XValidation:rule="self.key.split('/')[1].matches('^[A-Za-z0-9]([-A-Za-z0-9_.]*[A-Za-z0-9])?$')",message="taint key name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character" // +kubebuilder:validation:XValidation:rule="!has(self.value) || self.value.size() <= 63",message="taint value length must be at most 63 characters" // +kubebuilder:validation:XValidation:rule="self.effect in ['NoSchedule', 'PreferNoSchedule', 'NoExecute']",message="taint effect must be one of 'NoSchedule', 'PreferNoSchedule', 'NoExecute'" Taint corev1.Taint `json:"taint,omitempty,omitzero"` diff --git a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml index 4e44244..464b638 100644 --- a/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml +++ b/config/crd/bases/readiness.node.x-k8s.io_nodereadinessrules.yaml @@ -158,6 +158,11 @@ spec: taint defines the specific Taint (Key, Value, and Effect) to be managed on Nodes that meet the defined condition criteria. + The taint key must follow Kubernetes qualified name format: prefix/name + where prefix is 'readiness.k8s.io' (DNS subdomain) and name is a qualified + name (max 63 chars, alphanumeric, '-', '_', '.', must start and end with alphanumeric). + ref: git.k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/api/validate/content/kube.go#L24-L72 + Supported effects: NoSchedule, PreferNoSchedule, NoExecute. Caution: NoExecute evicts existing pods and can cause significant disruption when combined with continuous enforcement mode. Prefer NoSchedule for most use cases. @@ -188,6 +193,15 @@ spec: rule: self.key.startsWith('readiness.k8s.io/') - message: taint key length must be at most 253 characters rule: self.key.size() <= 253 + - message: taint key must have exactly one '/' separator (prefix/name + format) + rule: size(self.key.split('/')) == 2 + - message: taint key name part must be 1-63 characters + rule: size(self.key.split('/')[1]) > 0 && size(self.key.split('/')[1]) + <= 63 + - message: taint key name part must consist of alphanumeric characters, + '-', '_' or '.', and must start and end with an alphanumeric character + rule: self.key.split('/')[1].matches('^[A-Za-z0-9]([-A-Za-z0-9_.]*[A-Za-z0-9])?$') - message: taint value length must be at most 63 characters rule: '!has(self.value) || self.value.size() <= 63' - message: taint effect must be one of 'NoSchedule', 'PreferNoSchedule', From d0e8b0eb605f14328235f6c290935ddfedf859a2 Mon Sep 17 00:00:00 2001 From: Priyanka Saggu Date: Sun, 8 Mar 2026 02:33:53 +0530 Subject: [PATCH 2/2] add e2e tests for taint key validation in the NRR CRD --- test/e2e/taint_validation_test.go | 251 ++++++++++++++++++++++++++++++ 1 file changed, 251 insertions(+) create mode 100644 test/e2e/taint_validation_test.go diff --git a/test/e2e/taint_validation_test.go b/test/e2e/taint_validation_test.go new file mode 100644 index 0000000..56dcc14 --- /dev/null +++ b/test/e2e/taint_validation_test.go @@ -0,0 +1,251 @@ +//go:build e2e +// +build e2e + +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package e2e + +import ( + "fmt" + "os/exec" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "sigs.k8s.io/node-readiness-controller/test/utils" +) + +var _ = Describe("NodeReadinessRule Taint Key Validation", Ordered, func() { + BeforeAll(func() { + By("installing CRDs for validation tests") + cmd := exec.Command("make", "install") + _, err := utils.Run(cmd) + Expect(err).NotTo(HaveOccurred(), "Failed to install CRDs") + }) + + AfterAll(func() { + By("uninstalling CRDs after validation tests") + cmd := exec.Command("make", "uninstall") + _, _ = utils.Run(cmd) + }) + + Context("When creating a NodeReadinessRule", func() { + AfterEach(func() { + // Clean up any test resources + _ = exec.Command("kubectl", "delete", "nodereadinessrule", "--all", "--ignore-not-found=true").Run() + }) + + It("should reject taint keys with multiple slashes", func() { + manifest := ` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-multiple-slashes +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "readiness.k8s.io/invalid/multiple-slashes" + effect: "NoSchedule" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +` + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := cmd.CombinedOutput() + + Expect(err).To(HaveOccurred(), "Should fail to create NodeReadinessRule with multiple slashes") + Expect(string(output)).To(ContainSubstring("exactly one '/' separator")) + }) + + It("should reject taint keys with name starting with dash", func() { + manifest := ` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-dash-start +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "readiness.k8s.io/-invalid-start" + effect: "NoSchedule" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +` + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := cmd.CombinedOutput() + + Expect(err).To(HaveOccurred(), "Should fail with name starting with dash") + Expect(string(output)).To(ContainSubstring("must start and end with an alphanumeric character")) + }) + + It("should reject taint keys with name ending with dash", func() { + manifest := ` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-dash-end +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "readiness.k8s.io/invalid-end-" + effect: "NoSchedule" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +` + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := cmd.CombinedOutput() + + Expect(err).To(HaveOccurred(), "Should fail with name ending with dash") + Expect(string(output)).To(ContainSubstring("must start and end with an alphanumeric character")) + }) + + It("should reject taint keys with name longer than 63 characters", func() { + longName := strings.Repeat("a", 64) + manifest := fmt.Sprintf(` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-long-name +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "readiness.k8s.io/%s" + effect: "NoSchedule" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +`, longName) + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := cmd.CombinedOutput() + + Expect(err).To(HaveOccurred(), "Should fail with name longer than 63 characters") + Expect(string(output)).To(ContainSubstring("1-63 characters")) + }) + + It("should accept valid taint keys", func() { + validKeys := []string{ + "readiness.k8s.io/simple", + "readiness.k8s.io/with-dashes", + "readiness.k8s.io/with_underscores", + "readiness.k8s.io/with.dots", + "readiness.k8s.io/Mixed-Case_123.OK", + "readiness.k8s.io/security-agent-ready", + } + + for i, key := range validKeys { + manifest := fmt.Sprintf(` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-valid-%d +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "%s" + effect: "NoSchedule" + value: "pending" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +`, i, key) + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := utils.Run(cmd) + + Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("Should accept valid taint key: %s. Output: %s", key, output)) + } + + // Clean up + cmd := exec.Command("kubectl", "delete", "nodereadinessrule", "--all") + _, _ = utils.Run(cmd) + }) + + It("should reject empty name part", func() { + manifest := ` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-empty-name +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "readiness.k8s.io/" + effect: "NoSchedule" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +` + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := cmd.CombinedOutput() + + Expect(err).To(HaveOccurred(), "Should fail with empty name part") + Expect(string(output)).To(ContainSubstring("1-63 characters")) + }) + + It("should reject taint keys with special characters in name", func() { + manifest := ` +apiVersion: readiness.node.x-k8s.io/v1alpha1 +kind: NodeReadinessRule +metadata: + name: test-special-chars +spec: + conditions: + - type: "test.condition" + requiredStatus: "True" + taint: + key: "readiness.k8s.io/invalid@special" + effect: "NoSchedule" + enforcementMode: "continuous" + nodeSelector: + matchLabels: + kubernetes.io/os: linux +` + cmd := exec.Command("kubectl", "apply", "-f", "-") + cmd.Stdin = strings.NewReader(manifest) + output, err := cmd.CombinedOutput() + + Expect(err).To(HaveOccurred(), "Should fail with special characters") + Expect(string(output)).To(ContainSubstring("must consist of alphanumeric characters")) + }) + }) +})