Skip to content

Commit ca856e2

Browse files
ytimocinytimocin
committed
feat: add CEL validation rules to MemberCluster taint API types
Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
1 parent df220ae commit ca856e2

9 files changed

Lines changed: 1256 additions & 154 deletions

File tree

‎apis/cluster/v1/membercluster_types.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ type MemberClusterSpec struct {
7171
//
7272
// This field is beta-level and is for the taints and tolerations feature.
7373
// +kubebuilder:validation:MaxItems=100
74+
// +kubebuilder:validation:XValidation:rule="self.all(t, self.filter(u, u.key == t.key && (has(u.value) ? u.value : '') == (has(t.value) ? t.value : '') && u.effect == t.effect).size() == 1)",message="taints must be unique"
7475
// +optional
7576
Taints []Taint `json:"taints,omitempty"`
7677
}
@@ -136,12 +137,21 @@ type MemberClusterStatus struct {
136137

137138
// Taint attached to MemberCluster has the "effect" on
138139
// any ClusterResourcePlacement that does not tolerate the Taint.
140+
// +kubebuilder:validation:XValidation:rule="(self.key.contains('/') ? self.key.substring(self.key.indexOf('/') + 1) : self.key).matches('^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$')",message="taint key name segment must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character"
141+
// +kubebuilder:validation:XValidation:rule="!self.key.contains('/') || self.key.substring(0, self.key.indexOf('/')).matches('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$')",message="taint key prefix must be a lowercase DNS subdomain"
142+
// +kubebuilder:validation:XValidation:rule="(self.key.contains('/') ? self.key.size() - self.key.indexOf('/') - 1 : self.key.size()) <= 63",message="taint key name segment must be 63 characters or less"
143+
// +kubebuilder:validation:XValidation:rule="!self.key.contains('/') || self.key.indexOf('/') <= 253",message="taint key prefix must be 253 characters or less"
144+
// +kubebuilder:validation:XValidation:rule="!has(self.value) || size(self.value) == 0 || self.value.matches('^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$')",message="taint value must be a valid label value"
139145
type Taint struct {
140146
// The taint key to be applied to a MemberCluster.
147+
// MaxLength is 253 (prefix) + 1 (slash) + 63 (name segment) = 317.
148+
// +kubebuilder:validation:MaxLength=317
149+
// +kubebuilder:validation:MinLength=1
141150
// +required
142151
Key string `json:"key"`
143152

144153
// The taint value corresponding to the taint key.
154+
// +kubebuilder:validation:MaxLength=63
145155
// +optional
146156
Value string `json:"value,omitempty"`
147157

‎apis/cluster/v1beta1/membercluster_types.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ type MemberClusterSpec struct {
7272
//
7373
// This field is beta-level and is for the taints and tolerations feature.
7474
// +kubebuilder:validation:MaxItems=100
75+
// +kubebuilder:validation:XValidation:rule="self.all(t, self.filter(u, u.key == t.key && (has(u.value) ? u.value : '') == (has(t.value) ? t.value : '') && u.effect == t.effect).size() == 1)",message="taints must be unique"
7576
// +optional
7677
Taints []Taint `json:"taints,omitempty"`
7778

@@ -156,12 +157,21 @@ type MemberClusterStatus struct {
156157

157158
// Taint attached to MemberCluster has the "effect" on
158159
// any ClusterResourcePlacement that does not tolerate the Taint.
160+
// +kubebuilder:validation:XValidation:rule="(self.key.contains('/') ? self.key.substring(self.key.indexOf('/') + 1) : self.key).matches('^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$')",message="taint key name segment must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character"
161+
// +kubebuilder:validation:XValidation:rule="!self.key.contains('/') || self.key.substring(0, self.key.indexOf('/')).matches('^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$')",message="taint key prefix must be a lowercase DNS subdomain"
162+
// +kubebuilder:validation:XValidation:rule="(self.key.contains('/') ? self.key.size() - self.key.indexOf('/') - 1 : self.key.size()) <= 63",message="taint key name segment must be 63 characters or less"
163+
// +kubebuilder:validation:XValidation:rule="!self.key.contains('/') || self.key.indexOf('/') <= 253",message="taint key prefix must be 253 characters or less"
164+
// +kubebuilder:validation:XValidation:rule="!has(self.value) || size(self.value) == 0 || self.value.matches('^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$')",message="taint value must be a valid label value"
159165
type Taint struct {
160166
// The taint key to be applied to a MemberCluster.
167+
// MaxLength is 253 (prefix) + 1 (slash) + 63 (name segment) = 317.
168+
// +kubebuilder:validation:MaxLength=317
169+
// +kubebuilder:validation:MinLength=1
161170
// +required
162171
Key string `json:"key"`
163172

164173
// The taint value corresponding to the taint key.
174+
// +kubebuilder:validation:MaxLength=63
165175
// +optional
166176
Value string `json:"value,omitempty"`
167177

‎config/crd/bases/cluster.kubernetes-fleet.io_memberclusters.yaml‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,17 +128,43 @@ spec:
128128
- NoSchedule
129129
type: string
130130
key:
131-
description: The taint key to be applied to a MemberCluster.
131+
description: |-
132+
The taint key to be applied to a MemberCluster.
133+
MaxLength is 253 (prefix) + 1 (slash) + 63 (name segment) = 317.
134+
maxLength: 317
135+
minLength: 1
132136
type: string
133137
value:
134138
description: The taint value corresponding to the taint key.
139+
maxLength: 63
135140
type: string
136141
required:
137142
- effect
138143
- key
139144
type: object
145+
x-kubernetes-validations:
146+
- message: taint key name segment must consist of alphanumeric characters,
147+
'-', '_' or '.', and must start and end with an alphanumeric
148+
character
149+
rule: '(self.key.contains(''/'') ? self.key.substring(self.key.indexOf(''/'')
150+
+ 1) : self.key).matches(''^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$'')'
151+
- message: taint key prefix must be a lowercase DNS subdomain
152+
rule: '!self.key.contains(''/'') || self.key.substring(0, self.key.indexOf(''/'')).matches(''^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$'')'
153+
- message: taint key name segment must be 63 characters or less
154+
rule: '(self.key.contains(''/'') ? self.key.size() - self.key.indexOf(''/'')
155+
- 1 : self.key.size()) <= 63'
156+
- message: taint key prefix must be 253 characters or less
157+
rule: '!self.key.contains(''/'') || self.key.indexOf(''/'') <=
158+
253'
159+
- message: taint value must be a valid label value
160+
rule: '!has(self.value) || size(self.value) == 0 || self.value.matches(''^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$'')'
140161
maxItems: 100
141162
type: array
163+
x-kubernetes-validations:
164+
- message: taints must be unique
165+
rule: 'self.all(t, self.filter(u, u.key == t.key && (has(u.value)
166+
? u.value : '''') == (has(t.value) ? t.value : '''') && u.effect
167+
== t.effect).size() == 1)'
142168
required:
143169
- identity
144170
type: object
@@ -500,17 +526,43 @@ spec:
500526
- NoSchedule
501527
type: string
502528
key:
503-
description: The taint key to be applied to a MemberCluster.
529+
description: |-
530+
The taint key to be applied to a MemberCluster.
531+
MaxLength is 253 (prefix) + 1 (slash) + 63 (name segment) = 317.
532+
maxLength: 317
533+
minLength: 1
504534
type: string
505535
value:
506536
description: The taint value corresponding to the taint key.
537+
maxLength: 63
507538
type: string
508539
required:
509540
- effect
510541
- key
511542
type: object
543+
x-kubernetes-validations:
544+
- message: taint key name segment must consist of alphanumeric characters,
545+
'-', '_' or '.', and must start and end with an alphanumeric
546+
character
547+
rule: '(self.key.contains(''/'') ? self.key.substring(self.key.indexOf(''/'')
548+
+ 1) : self.key).matches(''^[A-Za-z0-9]([A-Za-z0-9._-]*[A-Za-z0-9])?$'')'
549+
- message: taint key prefix must be a lowercase DNS subdomain
550+
rule: '!self.key.contains(''/'') || self.key.substring(0, self.key.indexOf(''/'')).matches(''^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$'')'
551+
- message: taint key name segment must be 63 characters or less
552+
rule: '(self.key.contains(''/'') ? self.key.size() - self.key.indexOf(''/'')
553+
- 1 : self.key.size()) <= 63'
554+
- message: taint key prefix must be 253 characters or less
555+
rule: '!self.key.contains(''/'') || self.key.indexOf(''/'') <=
556+
253'
557+
- message: taint value must be a valid label value
558+
rule: '!has(self.value) || size(self.value) == 0 || self.value.matches(''^[a-zA-Z0-9]([a-zA-Z0-9._-]*[a-zA-Z0-9])?$'')'
512559
maxItems: 100
513560
type: array
561+
x-kubernetes-validations:
562+
- message: taints must be unique
563+
rule: 'self.all(t, self.filter(u, u.key == t.key && (has(u.value)
564+
? u.value : '''') == (has(t.value) ? t.value : '''') && u.effect
565+
== t.effect).size() == 1)'
514566
required:
515567
- identity
516568
type: object

‎pkg/utils/validator/membercluster.go‎

Lines changed: 0 additions & 41 deletions
This file was deleted.

‎pkg/utils/validator/membercluster_test.go‎

Lines changed: 0 additions & 100 deletions
This file was deleted.

‎pkg/webhook/membercluster/membercluster_validating_webhook.go‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515

1616
clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1"
1717
"github.com/kubefleet-dev/kubefleet/pkg/utils"
18-
"github.com/kubefleet-dev/kubefleet/pkg/utils/validator"
1918

2019
fleetnetworkingv1alpha1 "go.goms.io/fleet-networking/api/v1alpha1"
2120
)
@@ -78,14 +77,8 @@ func (v *memberClusterValidator) Handle(ctx context.Context, req admission.Reque
7877
return admission.Allowed("Member cluster is ready to leave")
7978
}
8079

81-
if err := v.decoder.Decode(req, &mc); err != nil {
82-
klog.ErrorS(err, "Failed to decode member cluster object for validating fields", "userName", req.UserInfo.Username, "groups", req.UserInfo.Groups)
83-
return admission.Errored(http.StatusBadRequest, err)
84-
}
85-
86-
if err := validator.ValidateMemberCluster(mc); err != nil {
87-
klog.V(2).ErrorS(err, "Member cluster has invalid fields, request is denied", "operation", req.Operation, "memberCluster", mcObjectName)
88-
return admission.Denied(err.Error())
89-
}
80+
// Taint validation is now handled by CRD-level CEL validation rules.
81+
// CREATE/UPDATE requests with invalid taints are rejected at schema
82+
// validation time before reaching this webhook.
9083
return admission.Allowed("Member cluster has valid fields")
9184
}

0 commit comments

Comments
 (0)