feat: add CEL validation rules to placement override API types

[Yetkin Timocin (ytimocin)]

Description of your changes

This pull request adds comprehensive validation rules to the override CRDs and their Go types, improving the safety and correctness of resource and cluster override definitions. The main changes ensure required fields are present, selectors are unique and well-formed, and JSON patch operations are restricted to valid paths and operations. These validations are applied consistently across both the Go API types and the generated CRDs for both v1 and v1beta1 versions.

Validation rule enhancements:

• Added XValidation rules to enforce that labelSelector is not used and name is required in clusterResourceSelectors, and that each selector is unique in ClusterResourceOverrideSpec and ResourceOverrideSpec (override_types.go for both v1 and v1beta1) [1] [2] [3] [4].
• Enforced that jsonPatchOverrides must be empty for Delete overrideType and non-empty for JSONPatch overrideType. Also required only labelSelector in cluster selectors (OverrideRule in both API versions) [1] [2].

JSONPatchOverride restrictions:

• Introduced validation rules to prohibit patching certain paths (/kind, /apiVersion, /status, most of /metadata except labels and annotations), and to disallow empty/whitespace path segments, trailing slashes, and values on remove operations (JSONPatchOverride in both API versions) [1] [2].
• Added pattern and maxLength constraints for the path field (JSONPatchOverride in both API versions) [1] [2].

CRD schema updates:

• Updated the generated CRD YAML (placement.kubernetes-fleet.io_clusterresourceoverrides.yaml) to include all new validation rules, including maxLength/pattern for path, required fields, and all custom validation logic for selectors and JSONPatchOverrides [1] [2] [3] [4] [5] [6].

These changes significantly improve the robustness of the override APIs by catching configuration errors early, both at the API and CRD levels.

How has this code been tested

N/A

Special notes for your reviewer

N/A

I have:

• Run make reviewable to ensure this PR is ready for review.

[codecov]

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/utils/validator/clusterresourceoverride.go	0.00%	1 Missing ⚠️
pkg/utils/validator/resourceoverride.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Britania Rodriguez Reyes (britaniar)]

We no longer support v1alpha1 so feel free to not include those APIs.

[Copilot]

Pull request overview

This PR adds CEL-based CRD validations for Placement Override APIs (defense-in-depth) so invalid ClusterResourceOverride / ResourceOverride specs are rejected at CRD admission time and align more closely with existing webhook/validator behavior.

Changes:

• Added CEL/XValidation rules to override API types (e.g., overrideType↔jsonPatchOverrides consistency, JSON patch path restrictions, CRO placement scope restriction).
• Regenerated CRD schemas to include the new validations (pattern/maxLength and x-kubernetes-validations).
• Updated validator logic/tests and adjusted integration/e2e tests to reflect earlier (CEL/CRD) rejection behavior.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
apis/placement/v1/override_types.go	Adds XValidation rules for override rule consistency and JSONPatch path constraints; adds CRO placement-scope restriction.
apis/placement/v1beta1/override_types.go	Same as v1; includes placement immutability rule at spec-level and new JSONPatch path validations.
config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml	CRD schema regen: adds CRO placement-scope validation + JSONPatch path rules + overrideType/jsonPatchOverrides rules.
config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml	CRD schema regen for snapshot CRD to mirror validation rules.
config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml	CRD schema regen for RO: JSONPatch path pattern/maxLength + validations; overrideType/jsonPatchOverrides rules.
config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml	Same schema regen for RO snapshots.
pkg/utils/validator/clusterresourceoverride.go	Adds CRO placement-scope validation in validator.
pkg/utils/validator/clusterresourceoverride_test.go	Adds unit test coverage for namespaced placement-scope rejection.
pkg/utils/validator/resourceoverride.go	Adds max JSON patch path length validation (512).
pkg/utils/validator/resourceoverride_test.go	Adds unit test for max-length JSON patch path.
pkg/controllers/overrider/clusterresource_controller_integration_test.go	Updates test JSON patch paths to valid RFC6902-style paths (/spec/replicas).
pkg/controllers/overrider/resource_controller_integration_test.go	Same path updates for namespaced overrides.
test/apis/placement/v1beta1/api_validation_integration_test.go	Adjusts helper data used by integration tests (CRO selector now uses Namespace).
test/apis/placement/v1beta1/override_cel_validation_integration_test.go	Adds envtest suite validating the new CEL/CRD behavior for override objects.
test/e2e/webhook_test.go	Removes webhook expectations for cases now rejected earlier by CEL/CRD validation; updates failing inputs accordingly.

Comments suppressed due to low confidence (1)

pkg/utils/validator/clusterresourceoverride.go:40

• ValidateClusterResourceOverride() returns early on validateClusterResourceSelectors() errors, which drops any placement-scope validation errors collected earlier. Consider appending the selector error to allErr and returning an aggregate so users see all independent validation failures (placement scope isn’t dependent on selector validity).

	if err := validateClusterResourceOverridePlacement(cro.Spec.Placement); err != nil {
		allErr = append(allErr, err)
	}

	// Check if the resource is being selected by resource name
	if err := validateClusterResourceSelectors(cro); err != nil {
		// Skip other checks because the check is only valid if resource is selected by name
		return err
	}

[Copilot]

The new XValidation says jsonPatchOverrides "must be empty" when overrideType is Delete, but jsonPatchOverrides still has MinItems=1, which makes an explicitly empty list invalid and can surface a less helpful minItems error instead of this message. Either require the field to be unset for Delete (adjust rule/message) or remove MinItems and rely on XValidation for the JSONPatch/non-empty requirement.

[Copilot]

The new XValidation says jsonPatchOverrides "must be empty" when overrideType is Delete, but jsonPatchOverrides still has MinItems=1, which makes an explicitly empty list invalid and can surface a less helpful minItems error instead of this message. Either require the field to be unset for Delete (adjust rule/message) or remove MinItems and rely on XValidation for the JSONPatch/non-empty requirement.

[Copilot]

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

[Copilot]

These assertions expect both the cross-object selector conflict error (from the webhook validator) and the "only labelSelector is supported" error (now enforced via CRD CEL) in the same response. With validations split between webhook vs CRD schema, only one of these sources will typically be returned for a given request (depending on validation/admission ordering), making this test brittle. Consider splitting into separate test cases: one that reaches the webhook to assert cross-object/remove-op errors, and another that isolates the CEL clusterSelector constraint (no cross-object conflict / no webhook-only violations).

[Copilot]

This UPDATE test currently asserts both CEL-enforced clusterSelector constraints ("only labelSelector is supported") and webhook-enforced/cross-object validations (duplicate selected resource, remove-op value). Since the webhook validator no longer checks the clusterSelector constraint, you’ll generally either get the webhook denial (no CEL message) or the CEL denial (no webhook-only messages). Split or restructure the test so each invalid condition is asserted in isolation (e.g., a valid selector set when testing CEL-only errors, and a valid clusterSelector when testing webhook-only errors).

[Copilot]

Similar to the CRO tests above, this UPDATE assertion mixes CEL-only validation (clusterSelector propertySelector/propertySorter restriction) with webhook-only validations (cross-object selector conflict + remove-op value). With the clusterSelector constraint moved to CRD CEL and removed from the webhook validator, you typically won't see all these substrings in a single error response. Restructure into separate cases so the request reaches the intended validator (CEL vs webhook) for each assertion.

Suggested change

Expect(statusErr.Status().Message).Should(MatchRegexp("only labelSelector is supported"))