convert some override webhook logic to CEL & add tests

Signed-off-by: Britania Rodriguez Reyes <[email protected]>

Author: britaniar

.github/.copilot/breadcrumbs/2025-07-01-utc-cel-clusterresourceoverride.md

@@ -0,0 +1,46 @@
+# Breadcrumb for CEL Conversion: ClusterResourceOverride
+
+## Requirements
+- Convert webhook validation logic for ClusterResourceOverride to CEL XValidation rules in the CRD.
+- Remove/replace webhook logic that is now enforced by CEL.
+- Ensure all constraints that can be enforced by CEL are migrated.
+- Document what remains in webhook (cross-object checks).
+
+## Additional comments from user
+- User approved the plan to proceed with CEL conversion for ClusterResourceOverride.
+
+## Plan
+### Phase 1: Analyze and Write CEL Expressions
+- [ ] Task 1.1: Identify all webhook validation logic that can be enforced by CEL.
+- [ ] Task 1.2: Write CEL XValidation rules for:
+  - No labelSelector in clusterResourceSelectors
+  - Name is required in clusterResourceSelectors
+  - No duplicate clusterResourceSelectors
+- [ ] Task 1.3: Add CEL rules to the CRD YAML.
+
+### Phase 2: Remove/Refactor Webhook Logic
+- [ ] Task 2.1: Remove/replace Go validation logic now enforced by CEL.
+- [ ] Task 2.2: Update webhook to only perform cross-object checks (e.g., uniqueness across objects).
+
+### Phase 3: Test and Document
+- [ ] Task 3.1: Test CRD validation with CEL rules.
+- [ ] Task 3.2: Update documentation and this breadcrumb with before/after comparison.
+
+## Decisions
+- CEL will be used for all per-object validation that does not require access to other objects.
+- Webhook will remain for cross-object uniqueness checks.
+
+## Implementation Details
+- CEL rules will be added under `x-kubernetes-validations` in the CRD.
+- Go code will be refactored to avoid duplicate validation.
+
+## Changes Made
+- (To be updated after implementation)
+
+## Before/After Comparison
+- (To be updated after implementation)
+
+## References
+- `/pkg/utils/validator/clusterresourceoverride.go` (Go validation logic)
+- `/config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml` (CRD definition)
+- [Kubernetes CEL docs](https://kubernetes.io/docs/reference/using-api/cel/)

apis/cluster/v1beta1/zz_generated.deepcopy.go

@@ -5,7 +5,7 @@ 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
+	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,
@@ -91,6 +91,8 @@ type OverridePolicy struct {
 }
 
 // OverrideRule defines how to override the selected resources on the target clusters.
+// +kubebuilder:validation:XValidation:rule="!(has(self.overrideType) && self.overrideType == 'Delete' && self.jsonPatchOverrides.size() != 0)", message="jsonPatchOverrides cannot be set when the override type is Delete."
+// +kubebuilder:validation:XValidation:rule="!(has(self.overrideType) && self.overrideType == 'JSONPatch' && self.jsonPatchOverrides.size() == 0)", message="jsonPatchOverrides must be set when the override type is JSONPatch."
 type OverrideRule struct {
 	// ClusterSelectors selects the target clusters.
 	// The resources will be overridden before applying to the matching clusters.
@@ -198,6 +200,10 @@ type JSONPatchOverride struct {
 	// Path defines the target location.
 	// Note: override will fail if the resource path does not exist.
 	// +required
+	// +kubebuilder:validation:XValidation:rule="!(self == '/kind' || self == '/apiVersion')",message="Path cannot target typeMeta fields (kind, apiVersion)"
+	// +kubebuilder:validation:XValidation:rule="!(self.startsWith('/metadata') && (!(self.startsWith('/metadata/annotations') || self.startsWith('/metadata/labels'))) || self == '/metadata')",message="Path can only target annotations and labels under metadata"
+	// +kubebuilder:validation:XValidation:rule="!self.startsWith('/status')",message="Path cannot target status fields"
+	// +kubebuilder:validation:XValidation:rule="self.startsWith('/')",message="Path must start with /"
 	Path string `json:"path"`
 	// Value defines the content to be applied on the target location.
 	// Value should be empty when operator is `remove`.

apis/placement/v1alpha1/override_types.go

@@ -542,8 +542,8 @@ spec:
                         managed by Fleet (i.e., specified in the hub cluster manifest). This is the default
                         option.
 
-                        Note that this option would revert any ad-hoc changes made on the member cluster side in
-                        the managed fields; if you would like to make temporary edits on the member cluster side
+                        Note that this option would revert any ad-hoc changes made on the member cluster side in the
+                        managed fields; if you would like to make temporary edits on the member cluster side
                         in the managed fields, switch to IfNotDrifted option. Note that changes in unmanaged
                         fields will be left alone; if you use the FullDiff compare option, such changes will
                         be reported as drifts.

apis/placement/v1alpha1/zz_generated.deepcopy.go

@@ -327,6 +327,19 @@ spec:
                                   Path defines the target location.
                                   Note: override will fail if the resource path does not exist.
                                 type: string
+                                x-kubernetes-validations:
+                                - message: Path cannot target typeMeta fields (kind,
+                                    apiVersion)
+                                  rule: '!(self == ''/kind'' || self == ''/apiVersion'')'
+                                - message: Path can only target annotations and labels
+                                    under metadata
+                                  rule: '!(self.startsWith(''/metadata'') && (!(self.startsWith(''/metadata/annotations'')
+                                    || self.startsWith(''/metadata/labels''))) ||
+                                    self == ''/metadata'')'
+                                - message: Path cannot target status fields
+                                  rule: '!self.startsWith(''/status'')'
+                                - message: Path must start with /
+                                  rule: self.startsWith('/')
                               value:
                                 description: |-
                                   Value defines the content to be applied on the target location.
@@ -352,6 +365,15 @@ spec:
                           - Delete
                           type: string
                       type: object
+                      x-kubernetes-validations:
+                      - message: jsonPatchOverrides cannot be set when the override
+                          type is Delete.
+                        rule: '!(has(self.overrideType) && self.overrideType == ''Delete''
+                          && self.jsonPatchOverrides.size() != 0)'
+                      - message: jsonPatchOverrides must be set when the override
+                          type is JSONPatch.
+                        rule: '!(has(self.overrideType) && self.overrideType == ''JSONPatch''
+                          && self.jsonPatchOverrides.size() == 0)'
                     maxItems: 20
                     minItems: 1
                     type: array

apis/placement/v1beta1/zz_generated.deepcopy.go

@@ -341,6 +341,19 @@ spec:
                                       Path defines the target location.
                                       Note: override will fail if the resource path does not exist.
                                     type: string
+                                    x-kubernetes-validations:
+                                    - message: Path cannot target typeMeta fields
+                                        (kind, apiVersion)
+                                      rule: '!(self == ''/kind'' || self == ''/apiVersion'')'
+                                    - message: Path can only target annotations and
+                                        labels under metadata
+                                      rule: '!(self.startsWith(''/metadata'') && (!(self.startsWith(''/metadata/annotations'')
+                                        || self.startsWith(''/metadata/labels'')))
+                                        || self == ''/metadata'')'
+                                    - message: Path cannot target status fields
+                                      rule: '!self.startsWith(''/status'')'
+                                    - message: Path must start with /
+                                      rule: self.startsWith('/')
                                   value:
                                     description: |-
                                       Value defines the content to be applied on the target location.
@@ -366,6 +379,15 @@ spec:
                               - Delete
                               type: string
                           type: object
+                          x-kubernetes-validations:
+                          - message: jsonPatchOverrides cannot be set when the override
+                              type is Delete.
+                            rule: '!(has(self.overrideType) && self.overrideType ==
+                              ''Delete'' && self.jsonPatchOverrides.size() != 0)'
+                          - message: jsonPatchOverrides must be set when the override
+                              type is JSONPatch.
+                            rule: '!(has(self.overrideType) && self.overrideType ==
+                              ''JSONPatch'' && self.jsonPatchOverrides.size() == 0)'
                         maxItems: 20
                         minItems: 1
                         type: array

apis/v1alpha1/zz_generated.deepcopy.go

@@ -1807,8 +1807,8 @@ spec:
                             managed by Fleet (i.e., specified in the hub cluster manifest). This is the default
                             option.
 
-                            Note that this option would revert any ad-hoc changes made on the member cluster side in
-                            the managed fields; if you would like to make temporary edits on the member cluster side
+                            Note that this option would revert any ad-hoc changes made on the member cluster side in the
+                            managed fields; if you would like to make temporary edits on the member cluster side
                             in the managed fields, switch to IfNotDrifted option. Note that changes in unmanaged
                             fields will be left alone; if you use the FullDiff compare option, such changes will
                             be reported as drifts.