Skip to content

Commit ff9998b

Browse files
sats-23sats-23
committed
feat: Restrict NodeReadinessRuleSpec.Taint to "readiness.k8s.io/" prefix
Signed-off-by: Sathvik <Sathvik.S@ibm.com>
1 parent dc4a0b1 commit ff9998b

File tree

10 files changed

+194
-57
lines changed

10 files changed

+194
-57
lines changed

‎api/v1alpha1/nodereadinessrule_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ type NodeReadinessRuleSpec struct {
7070
// on Nodes that meet the defined condition criteria.
7171
//
7272
// +required
73+
// +kubebuilder:validation:XValidation:rule="self.key.startsWith('readiness.k8s.io/')",message="taint key must start with 'readiness.k8s.io/'"
74+
// +kubebuilder:validation:XValidation:rule="self.key.size() >= 17 && self.key.size() <= 253",message="taint key length must be between 17 and 253 characters"
75+
// +kubebuilder:validation:XValidation:rule="!has(self.value) || self.value.size() <= 63",message="taint value length must be at most 63 characters"
76+
// +kubebuilder:validation:XValidation:rule="self.effect in ['NoSchedule', 'PreferNoSchedule', 'NoExecute']",message="taint effect must be one of 'NoSchedule', 'PreferNoSchedule', 'NoExecute'"
7377
Taint corev1.Taint `json:"taint,omitempty,omitzero"`
7478

7579
// nodeSelector limits the scope of this rule to a specific subset of Nodes.

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,16 @@ spec:
179179
- effect
180180
- key
181181
type: object
182+
x-kubernetes-validations:
183+
- message: taint key must start with 'readiness.k8s.io/'
184+
rule: self.key.startsWith('readiness.k8s.io/')
185+
- message: taint key length must be between 17 and 253 characters
186+
rule: self.key.size() >= 17 && self.key.size() <= 253
187+
- message: taint value length must be at most 63 characters
188+
rule: '!has(self.value) || self.value.size() <= 63'
189+
- message: taint effect must be one of 'NoSchedule', 'PreferNoSchedule',
190+
'NoExecute'
191+
rule: self.effect in ['NoSchedule', 'PreferNoSchedule', 'NoExecute']
182192
required:
183193
- conditions
184194
- enforcementMode

‎internal/controller/node_controller.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -241,11 +241,7 @@ func (r *RuleReadinessController) hasTaintBySpec(node *corev1.Node, taintSpec co
241241
// addTaintBySpec adds a taint to a node.
242242
func (r *RuleReadinessController) addTaintBySpec(ctx context.Context, node *corev1.Node, taintSpec corev1.Taint) error {
243243
patch := client.StrategicMergeFrom(node.DeepCopy())
244-
node.Spec.Taints = append(node.Spec.Taints, corev1.Taint{
245-
Key: taintSpec.Key,
246-
Value: taintSpec.Value,
247-
Effect: taintSpec.Effect,
248-
})
244+
node.Spec.Taints = append(node.Spec.Taints, taintSpec)
249245
return r.Patch(ctx, node, patch)
250246
}
251247

‎internal/controller/node_controller_test.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ var _ = Describe("Node Controller", func() {
3636
const (
3737
nodeName = "node-controller-test-node"
3838
ruleName = "node-controller-test-rule"
39-
taintKey = "test-taint"
39+
taintKey = "readiness.k8s.io/test-taint"
4040
conditionType = "TestCondition"
4141
)
4242

@@ -66,19 +66,19 @@ var _ = Describe("Node Controller", func() {
6666

6767
It("should correctly compare node taints", func() {
6868
taint1 := []corev1.Taint{
69-
{Key: "key1", Effect: corev1.TaintEffectNoSchedule, Value: "value1"},
70-
{Key: "key2", Effect: corev1.TaintEffectNoExecute, Value: "value2"},
69+
{Key: "readiness.k8s.io/key1", Effect: corev1.TaintEffectNoSchedule, Value: "value1"},
70+
{Key: "readiness.k8s.io/key2", Effect: corev1.TaintEffectNoExecute, Value: "value2"},
7171
}
7272
taint2 := []corev1.Taint{
73-
{Key: "key1", Effect: corev1.TaintEffectNoSchedule, Value: "value1"},
74-
{Key: "key2", Effect: corev1.TaintEffectNoExecute, Value: "value2"},
73+
{Key: "readiness.k8s.io/key1", Effect: corev1.TaintEffectNoSchedule, Value: "value1"},
74+
{Key: "readiness.k8s.io/key2", Effect: corev1.TaintEffectNoExecute, Value: "value2"},
7575
}
7676
taint3 := []corev1.Taint{
77-
{Key: "key1", Effect: corev1.TaintEffectNoSchedule, Value: "different"},
78-
{Key: "key2", Effect: corev1.TaintEffectNoExecute, Value: "value2"},
77+
{Key: "readiness.k8s.io/key1", Effect: corev1.TaintEffectNoSchedule, Value: "different"},
78+
{Key: "readiness.k8s.io/key2", Effect: corev1.TaintEffectNoExecute, Value: "value2"},
7979
}
8080
taint4 := []corev1.Taint{
81-
{Key: "key1", Effect: corev1.TaintEffectNoSchedule, Value: "value1"},
81+
{Key: "readiness.k8s.io/key1", Effect: corev1.TaintEffectNoSchedule, Value: "value1"},
8282
}
8383

8484
Expect(taintsEqual(taint1, taint2)).To(BeTrue(), "identical taints should be equal")
@@ -547,7 +547,7 @@ var _ = Describe("Node Controller", func() {
547547
},
548548
Spec: corev1.NodeSpec{
549549
Taints: []corev1.Taint{
550-
{Key: "status-test-taint", Effect: corev1.TaintEffectNoSchedule, Value: "pending"},
550+
{Key: "readiness.k8s.io/status-test-taint", Effect: corev1.TaintEffectNoSchedule, Value: "pending"},
551551
},
552552
},
553553
Status: corev1.NodeStatus{
@@ -557,6 +557,7 @@ var _ = Describe("Node Controller", func() {
557557
},
558558
}
559559

560+
val := "pending"
560561
rule = &nodereadinessiov1alpha1.NodeReadinessRule{
561562
ObjectMeta: metav1.ObjectMeta{
562563
Name: "status-test-rule",
@@ -566,9 +567,9 @@ var _ = Describe("Node Controller", func() {
566567
{Type: "StatusTestCondition", RequiredStatus: corev1.ConditionTrue},
567568
},
568569
Taint: corev1.Taint{
569-
Key: "status-test-taint",
570+
Key: "readiness.k8s.io/status-test-taint",
570571
Effect: corev1.TaintEffectNoSchedule,
571-
Value: "pending",
572+
Value: val,
572573
},
573574
NodeSelector: metav1.LabelSelector{
574575
MatchLabels: map[string]string{"test-group": "status"},

‎internal/controller/nodereadinessrule_controller_test.go‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
)
3535

3636
const (
37-
selectorChangeTaintKey = "selector-change-taint"
37+
selectorChangeTaintKey = "readiness.k8s.io/selector-change-taint"
3838
)
3939

4040
var _ = Describe("NodeReadinessRule Controller", func() {
@@ -90,7 +90,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
9090
},
9191
},
9292
Taint: corev1.Taint{
93-
Key: "test-taint",
93+
Key: "readiness.k8s.io/test-taint",
9494
Effect: corev1.TaintEffectNoSchedule,
9595
},
9696
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
@@ -133,7 +133,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
133133
},
134134
},
135135
Taint: corev1.Taint{
136-
Key: "test-taint",
136+
Key: "readiness.k8s.io/test-taint",
137137
Effect: corev1.TaintEffectNoSchedule,
138138
},
139139
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
@@ -154,7 +154,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
154154
cachedRule, exists := readinessController.ruleCache["test-rule"]
155155
readinessController.ruleCacheMutex.RUnlock()
156156
Expect(exists).To(BeTrue())
157-
Expect(cachedRule.Spec.Taint.Key).To(Equal("test-taint"))
157+
Expect(cachedRule.Spec.Taint.Key).To(Equal("readiness.k8s.io/test-taint"))
158158

159159
// Cleanup
160160
Expect(k8sClient.Delete(ctx, rule)).To(Succeed())
@@ -176,7 +176,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
176176
},
177177
},
178178
Taint: corev1.Taint{
179-
Key: "test-taint",
179+
Key: "readiness.k8s.io/test-taint",
180180
Effect: corev1.TaintEffectNoSchedule,
181181
},
182182
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
@@ -237,7 +237,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
237237
{Type: "TestCondition", RequiredStatus: corev1.ConditionTrue},
238238
},
239239
Taint: corev1.Taint{
240-
Key: "immediate-test-taint",
240+
Key: "readiness.k8s.io/immediate-test-taint",
241241
Effect: corev1.TaintEffectNoSchedule,
242242
},
243243
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
@@ -265,7 +265,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
265265
return false
266266
}
267267
for _, taint := range updatedNode.Spec.Taints {
268-
if taint.Key == "immediate-test-taint" && taint.Effect == corev1.TaintEffectNoSchedule {
268+
if taint.Key == "readiness.k8s.io/immediate-test-taint" && taint.Effect == corev1.TaintEffectNoSchedule {
269269
return true
270270
}
271271
}
@@ -299,7 +299,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
299299
},
300300
},
301301
Taint: corev1.Taint{
302-
Key: "dry-run-taint",
302+
Key: "readiness.k8s.io/dry-run-taint",
303303
Effect: corev1.TaintEffectNoSchedule,
304304
},
305305
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
@@ -369,7 +369,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
369369
{Type: "Ready", RequiredStatus: corev1.ConditionTrue},
370370
},
371371
Taint: corev1.Taint{
372-
Key: "node-test-taint",
372+
Key: "readiness.k8s.io/node-test-taint",
373373
Effect: corev1.TaintEffectNoSchedule,
374374
},
375375
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeBootstrapOnly,
@@ -423,14 +423,14 @@ var _ = Describe("NodeReadinessRule Controller", func() {
423423
node := &corev1.Node{
424424
Spec: corev1.NodeSpec{
425425
Taints: []corev1.Taint{
426-
{Key: "test-key", Effect: corev1.TaintEffectNoSchedule, Value: "test-value"},
427-
{Key: "another-key", Effect: corev1.TaintEffectNoExecute},
426+
{Key: "readiness.k8s.io/test-key", Effect: corev1.TaintEffectNoSchedule, Value: "test-value"},
427+
{Key: "readiness.k8s.io/another-key", Effect: corev1.TaintEffectNoExecute},
428428
},
429429
},
430430
}
431431

432432
taintSpec := corev1.Taint{
433-
Key: "test-key",
433+
Key: "readiness.k8s.io/test-key",
434434
Effect: corev1.TaintEffectNoSchedule,
435435
}
436436

@@ -439,7 +439,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
439439

440440
// Test non-existent taint
441441
nonExistentTaint := corev1.Taint{
442-
Key: "missing-key",
442+
Key: "readiness.k8s.io/missing-key",
443443
Effect: corev1.TaintEffectNoSchedule,
444444
}
445445

@@ -534,7 +534,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
534534
},
535535
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
536536
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "DBReady", RequiredStatus: corev1.ConditionTrue}},
537-
Taint: corev1.Taint{Key: "db-unready", Effect: corev1.TaintEffectNoSchedule},
537+
Taint: corev1.Taint{Key: "readiness.k8s.io/db-unready", Effect: corev1.TaintEffectNoSchedule},
538538
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
539539
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"app": "backend"}},
540540
},
@@ -590,7 +590,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
590590
},
591591
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
592592
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "TestReady", RequiredStatus: corev1.ConditionTrue}},
593-
Taint: corev1.Taint{Key: "test-unready", Effect: corev1.TaintEffectNoSchedule},
593+
Taint: corev1.Taint{Key: "readiness.k8s.io/test-unready", Effect: corev1.TaintEffectNoSchedule},
594594
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
595595
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"node-group": "new-workers"}},
596596
},
@@ -664,7 +664,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
664664
},
665665
Spec: corev1.NodeSpec{
666666
Taints: []corev1.Taint{
667-
{Key: "cleanup-taint", Effect: corev1.TaintEffectNoSchedule, Value: "pending"},
667+
{Key: "readiness.k8s.io/cleanup-taint", Effect: corev1.TaintEffectNoSchedule, Value: "pending"},
668668
},
669669
},
670670
Status: corev1.NodeStatus{
@@ -679,7 +679,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
679679
Spec: nodereadinessiov1alpha1.NodeReadinessRuleSpec{
680680
Conditions: []nodereadinessiov1alpha1.ConditionRequirement{{Type: "TestReady", RequiredStatus: corev1.ConditionTrue}},
681681
NodeSelector: metav1.LabelSelector{MatchLabels: map[string]string{"kubernetes.io/hostname": "cleanup-test-node"}},
682-
Taint: corev1.Taint{Key: "cleanup-taint", Effect: corev1.TaintEffectNoSchedule},
682+
Taint: corev1.Taint{Key: "readiness.k8s.io/cleanup-taint", Effect: corev1.TaintEffectNoSchedule},
683683
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
684684
},
685685
}
@@ -710,7 +710,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
710710
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "cleanup-test-node"}, updatedNode)).To(Succeed())
711711
hasTaint := false
712712
for _, taint := range updatedNode.Spec.Taints {
713-
if taint.Key == "cleanup-taint" {
713+
if taint.Key == "readiness.k8s.io/cleanup-taint" {
714714
hasTaint = true
715715
break
716716
}
@@ -731,7 +731,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
731731
return false
732732
}
733733
for _, taint := range updatedNode.Spec.Taints {
734-
if taint.Key == "cleanup-taint" {
734+
if taint.Key == "readiness.k8s.io/cleanup-taint" {
735735
return false // Taint still exists
736736
}
737737
}
@@ -767,7 +767,7 @@ var _ = Describe("NodeReadinessRule Controller", func() {
767767
},
768768
},
769769
},
770-
Taint: corev1.Taint{Key: "unready", Effect: corev1.TaintEffectNoSchedule},
770+
Taint: corev1.Taint{Key: "readiness.k8s.io/unready", Effect: corev1.TaintEffectNoSchedule},
771771
EnforcementMode: nodereadinessiov1alpha1.EnforcementModeContinuous,
772772
},
773773
}

‎internal/webhook/nodereadinessgaterule_webhook.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ package webhook
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223

24+
corev1 "k8s.io/api/core/v1"
2325
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2426
"k8s.io/apimachinery/pkg/runtime"
2527
"k8s.io/apimachinery/pkg/util/validation/field"
@@ -93,9 +95,36 @@ func (w *NodeReadinessRuleWebhook) validateSpec(spec readinessv1alpha1.NodeReadi
9395
taintField := specField.Child("taint")
9496
if spec.Taint.Key == "" {
9597
allErrs = append(allErrs, field.Required(taintField.Child("key"), "taint key cannot be empty"))
98+
} else {
99+
// Validate key prefix
100+
if !strings.HasPrefix(spec.Taint.Key, "readiness.k8s.io/") {
101+
allErrs = append(allErrs, field.Invalid(taintField.Child("key"), spec.Taint.Key, "taint key must start with 'readiness.k8s.io/'"))
102+
}
103+
// Validate key length
104+
if len(spec.Taint.Key) < 17 || len(spec.Taint.Key) > 253 {
105+
allErrs = append(allErrs, field.Invalid(taintField.Child("key"), spec.Taint.Key, "taint key length must be between 17 and 253 characters"))
106+
}
96107
}
108+
97109
if spec.Taint.Effect == "" {
98110
allErrs = append(allErrs, field.Required(taintField.Child("effect"), "taint effect cannot be empty"))
111+
} else {
112+
// Validate effect
113+
switch spec.Taint.Effect {
114+
case corev1.TaintEffectNoSchedule, corev1.TaintEffectPreferNoSchedule, corev1.TaintEffectNoExecute:
115+
// valid
116+
default:
117+
allErrs = append(allErrs, field.NotSupported(taintField.Child("effect"), spec.Taint.Effect, []string{
118+
string(corev1.TaintEffectNoSchedule),
119+
string(corev1.TaintEffectPreferNoSchedule),
120+
string(corev1.TaintEffectNoExecute),
121+
}))
122+
}
123+
}
124+
125+
// Validate value length
126+
if len(spec.Taint.Value) > 63 {
127+
allErrs = append(allErrs, field.Invalid(taintField.Child("value"), spec.Taint.Value, "taint value length must be at most 63 characters"))
99128
}
100129

101130
// Validate enforcement mode

0 commit comments

Comments
 (0)