feat: update the placement v1 with PR and CPRS by ryanzhang-oss · Pull Request #484 · kubefleet-dev/kubefleet

ryanzhang-oss · 2026-03-05T01:53:09Z

Description of your changes

update the placement v1 with PR and CPRS

Fixes #

I have:

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

How has this code been tested

Special notes for your reviewer

Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

codecov · 2026-03-05T02:33:43Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

britaniar

Update failed tests:

ytimocin · 2026-03-05T18:06:26Z

apis/placement/v1/clusterresourceplacement_types.go

-// ClusterResourcePlacementSpec defines the desired state of ClusterResourcePlacement.
-type ClusterResourcePlacementSpec struct {
+// PlacementSpec defines the desired state of ClusterResourcePlacement.
+type PlacementSpec struct {


Do we have a repo-wide naming convention?

this is k8s api so we follow k8s naming convention

Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

michaelawyu

LGTM ;) I can submit additional API progression E2E test cases if needed.

weng271190436 · 2026-03-06T17:45:21Z

apis/placement/v1/clusterresourceplacement_types.go

 	// The desired state of ClusterResourcePlacement.
 	// +required
-	Spec ClusterResourcePlacementSpec `json:"spec"`
+	Spec PlacementSpec `json:"spec"`


I added some CEL to v1beta1 PlacementSpec and Policy

https://github.com/kubefleet-dev/kubefleet/pull/315/changes

Should we include those as well?

We can have yetkin add it in his PR.

britaniar · 2026-03-11T18:11:21Z

apis/placement/v1/clusterresourceplacement_types.go

@@ -885,15 +935,26 @@ type EnvelopeType string
 const (
 	// ConfigMapEnvelopeType means the envelope object is of type `ConfigMap`.
 	ConfigMapEnvelopeType EnvelopeType = "ConfigMap"


Non-blocking but is it time to drop this?

kubefleet/apis/placement/v1beta1/clusterresourceplacement_types.go

Line 1105 in 6c46f12

// TO-DO (chenyu1): drop this type after the configMap-based envelopes become obsolete.

britaniar · 2026-03-11T18:15:26Z

apis/placement/v1/clusterresourceplacement_types.go

@@ -1203,6 +1275,57 @@ const (
 	// - "Unknown" means we haven't finished the apply yet so that we cannot check the resource availability.
 	ResourcesAvailableConditionType ResourcePlacementConditionType = "Available"


Think this and the ones above should have been replaced by the ones below.

ryanzhang-oss · 2026-03-12T02:35:40Z

Code review

Found 5 issues:

ClusterResourcePlacementStatus type name reused for a new namespaced CRD — previously this was the status struct embedded in ClusterResourcePlacement (now renamed to PlacementStatus). Any external code that imported v1.ClusterResourcePlacementStatus as a status struct now silently refers to the new CRD object instead. The generated deepcopy confirms this: the new type embeds TypeMeta, ObjectMeta, PlacementStatus, and LastUpdatedTime — an entirely different shape.

https://github.com/kubefleet-dev/kubefleet/blob/2e3152b7dc4a085d0b372a1a9d9673868f6b0787/apis/placement/v1/clusterresourceplacement_types.go#L1578-L1598

Contradictory +kubebuilder:validation:Required and json:"...,omitempty" on ClusterResourcePlacementStatus fields — both PlacementStatus (tagged sourceStatus,omitempty) and LastUpdatedTime (tagged lastUpdatedTime,omitempty) are marked Required, but omitempty causes the Go JSON serializer to drop zero-valued fields. The CRD schema will reject objects missing these fields while the serializer happily omits them — a serialization/validation mismatch that will cause runtime failures.

https://github.com/kubefleet-dev/kubefleet/blob/2e3152b7dc4a085d0b372a1a9d9673868f6b0787/apis/placement/v1/clusterresourceplacement_types.go#L1583-L1598

Missing CEL immutability validations present in v1beta1 — v1beta1's ClusterResourcePlacement.Spec has three +kubebuilder:validation:XValidation rules absent from v1: (a) policy cannot be removed once set, (b) statusReportingScope is immutable, (c) placementType is immutable. Without these, users can freely mutate fields intended to be immutable, breaking controller invariants.

kubefleet/apis/placement/v1/clusterresourceplacement_types.go

Lines 1480 to 1498 in 2e3152b

    
           // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="ResourcePlacementWorkSynchronized")].observedGeneration`,name="Work-Synchronized-Gen",priority=1,type=string 
        
           // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="ResourcePlacementAvailable")].status`,name="Available",type=string 
        
           // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="ResourcePlacementAvailable")].observedGeneration`,name="Available-Gen",type=string 
        
           // +kubebuilder:printcolumn:JSONPath=`.metadata.creationTimestamp`,name="Age",type=date 
        
           // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object 
        
           // ResourcePlacement is used to select namespace scoped resources, including built-in resources and custom resources, 
        
           // and placement them onto selected member clusters in a fleet. 
        
           // `SchedulingPolicySnapshot` and `ResourceSnapshot` objects are created in the same namespace when there are changes in the 
        
           // system to keep the history of the changes affecting a `ResourcePlacement`. We will also create `ResourceBinding` objects in the same namespace. 
        
           type ResourcePlacement struct { 
        
           	metav1.TypeMeta   `json:",inline"` 
        
           	metav1.ObjectMeta `json:"metadata,omitempty"` 
        
           	// The desired state of ResourcePlacement. 
        
           	// +required 
        
           	Spec PlacementSpec `json:"spec"` 
        
           	// The observed status of ResourcePlacement.

Missing CEL rule: exactly one namespace selector required when StatusReportingScope=NamespaceAccessible — v1beta1 enforces this with !(self.statusReportingScope == 'NamespaceAccessible' && size(self.resourceSelectors.filter(x, x.kind == 'Namespace')) != 1). Without it, users can set NamespaceAccessible while targeting zero or multiple namespaces, causing runtime failures when the controller tries to create the ClusterResourcePlacementStatus mirror object.

kubefleet/apis/placement/v1/clusterresourceplacement_types.go

Lines 1480 to 1498 in 2e3152b

    
           // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="ResourcePlacementWorkSynchronized")].observedGeneration`,name="Work-Synchronized-Gen",priority=1,type=string 
        
           // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="ResourcePlacementAvailable")].status`,name="Available",type=string 
        
           // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="ResourcePlacementAvailable")].observedGeneration`,name="Available-Gen",type=string 
        
           // +kubebuilder:printcolumn:JSONPath=`.metadata.creationTimestamp`,name="Age",type=date 
        
           // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object 
        
           // ResourcePlacement is used to select namespace scoped resources, including built-in resources and custom resources, 
        
           // and placement them onto selected member clusters in a fleet. 
        
           // `SchedulingPolicySnapshot` and `ResourceSnapshot` objects are created in the same namespace when there are changes in the 
        
           // system to keep the history of the changes affecting a `ResourcePlacement`. We will also create `ResourceBinding` objects in the same namespace. 
        
           type ResourcePlacement struct { 
        
           	metav1.TypeMeta   `json:",inline"` 
        
           	metav1.ObjectMeta `json:"metadata,omitempty"` 
        
           	// The desired state of ResourcePlacement. 
        
           	// +required 
        
           	Spec PlacementSpec `json:"spec"` 
        
           	// The observed status of ResourcePlacement.

ResourcePlacementList and ClusterResourcePlacementStatusList missing +kubebuilder:object:root=true — ClusterResourcePlacementList in the same file has this marker; both new list types do not. Without it, controller-gen will not register these list types correctly, breaking CRD generation and scheme registration.

kubefleet/apis/placement/v1/clusterresourceplacement_types.go

Lines 1530 to 1537 in 2e3152b

    
           func (m *ResourcePlacement) SetPlacementSpec(spec PlacementSpec) { 
        
           	spec.DeepCopyInto(&m.Spec) 
        
           } 
        
           // GetPlacementStatus returns the placement status. 
        
           func (m *ResourcePlacement) GetPlacementStatus() *PlacementStatus { 
        
           	return &m.Status 
        
           }

- Add CEL immutability validations to CRP Spec (matching v1beta1): policy cannot be removed once set, statusReportingScope immutable, placementType immutable, and namespace selector requirement when StatusReportingScope=NamespaceAccessible - Fix contradictory +kubebuilder:validation:Required + omitempty on ClusterResourcePlacementStatus fields (PlacementStatus, LastUpdatedTime) - Add missing +kubebuilder:object:root=true to ResourcePlacementList and ClusterResourcePlacementStatusList Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

…esilient Two CI fixes: 1. Makefile: pin GOIMPORTS_VER from 'latest' to v0.42.0. golang.org/x/tools v0.43.0 (latest) now requires Go >= 1.25.0, but CI runs Go 1.24.13, causing the unit-and-integration-tests job to fail at setup with: go: golang.org/x/tools/cmd/goimports@latest: requires go >= 1.25.0 2. pkg/controllers/workgenerator/envelope.go: when multiple work objects are found for the same envelope (possible transiently if the same CRP name is reused quickly after deletion), self-heal by deleting all but the most recently created work and proceeding, instead of returning an UnexpectedBehaviorError that causes WorkSynchronized=False on the binding. The previous hard-fail path caused e2e test flakiness in the 'mixed availability statefulset' case where cluster-1's workgenerator would encounter leftover work objects from the prior test context. Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

ryanzhang-oss added 2 commits March 4, 2026 17:52

update the placement v1 with PR and CPRS

95da00f

Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

update the placement v1 with PR and CPRS

c06a6a4

Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

britaniar approved these changes Mar 5, 2026

View reviewed changes

britaniar requested changes Mar 5, 2026

View reviewed changes

ytimocin reviewed Mar 5, 2026

View reviewed changes

fix the test now that we aded per cluster resource index

2e3152b

Signed-off-by: Ryan Zhang <yangzhangrice@hotmail.com>

michaelawyu reviewed Mar 6, 2026

View reviewed changes

weng271190436 previously approved these changes Mar 6, 2026

View reviewed changes

sjwaight linked an issue Mar 11, 2026 that may be closed by this pull request

Feature request: provide mechanism for namespace-only users to view status for ClusterResourcePlacement #508

Open

britaniar reviewed Mar 11, 2026

View reviewed changes

ryanzhang-oss dismissed weng271190436’s stale review via 3433d12 March 12, 2026 03:29

ryanzhang-oss force-pushed the placement-v1 branch from 3433d12 to 2d86616 Compare March 12, 2026 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: update the placement v1 with PR and CPRS#484

feat: update the placement v1 with PR and CPRS#484
ryanzhang-oss wants to merge 5 commits intokubefleet-dev:mainfrom
ryanzhang-oss:placement-v1

ryanzhang-oss commented Mar 5, 2026

Uh oh!

codecov bot commented Mar 5, 2026

Uh oh!

britaniar left a comment

Uh oh!

ytimocin Mar 5, 2026

Uh oh!

ryanzhang-oss Mar 6, 2026

Uh oh!

michaelawyu left a comment

Uh oh!

weng271190436 Mar 6, 2026

Uh oh!

britaniar Mar 11, 2026

Uh oh!

britaniar Mar 11, 2026

Uh oh!

britaniar Mar 11, 2026

Uh oh!

ryanzhang-oss commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		@@ -1203,6 +1275,57 @@ const (
		// - "Unknown" means we haven't finished the apply yet so that we cannot check the resource availability.
		ResourcesAvailableConditionType ResourcePlacementConditionType = "Available"

Conversation

ryanzhang-oss commented Mar 5, 2026

Description of your changes

How has this code been tested

Special notes for your reviewer

Uh oh!

codecov bot commented Mar 5, 2026

Codecov Report

Uh oh!

britaniar left a comment

Choose a reason for hiding this comment

Uh oh!

ytimocin Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

ryanzhang-oss Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

michaelawyu left a comment

Choose a reason for hiding this comment

Uh oh!

weng271190436 Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

britaniar Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

britaniar Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

britaniar Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

ryanzhang-oss commented Mar 12, 2026

Code review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants