🐛 Update Kubernetes list type markers for conditions #413
Conversation
Replace deprecated +patchMergeKey and +patchStrategy markers with +listType=map and +listMapKey=type for condition arrays across all API versions. This ensures proper strategic merge patch behavior and aligns with current Kubernetes API conventions. Updates CRD YAML files with corresponding x-kubernetes-list-type and x-kubernetes-list-map-keys annotations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> Signed-off-by: Jian Qiu <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThis PR updates Kubernetes API definitions across multiple modules to standardize list handling by replacing patch-based merge annotations with map-based list semantics. Changes introduce Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/lgtm |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @addon/v1alpha1/types_managedclusteraddon.go:
- Around line 104-107: The Conditions field still includes deprecated struct
tags patchStrategy:"merge" and patchMergeKey:"type"; remove those two tags from
the Conditions field tag so it only keeps json:"conditions,omitempty" (and any
other non-deprecated tags), while leaving the kubebuilder markers (+listType=map
and +listMapKey=type) above the field intact; update the struct field
declaration for Conditions in types_managedclusteraddon.go accordingly.
In @addon/v1beta1/types_clustermanagementaddon.go:
- Around line 175-178: The Conditions field on the Clustermanagementaddon type
still has legacy struct tags; remove the old patchStrategy:"merge" and
patchMergeKey:"type" tags from the Conditions []metav1.Condition
`json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
declaration so only the new markers (+listType=map and +listMapKey=type) and the
json tag remain; update the Conditions field signature to use
`json:"conditions,omitempty"` (keeping the existing markers above the field) to
complete the migration.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yamladdon/v1alpha1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yamladdon/v1alpha1/types_clustermanagementaddon.goaddon/v1alpha1/types_managedclusteraddon.goaddon/v1beta1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yamladdon/v1beta1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yamladdon/v1beta1/types_clustermanagementaddon.goaddon/v1beta1/types_managedclusteraddon.gocluster/v1/0000_00_clusters.open-cluster-management.io_managedclusters.crd.yamlcluster/v1/types.gocluster/v1alpha1/types_addonplacementscore.gocluster/v1beta1/0000_02_clusters.open-cluster-management.io_placements.crd.yamlcluster/v1beta1/types_placement.gooperator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yamloperator/v1/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yamloperator/v1/types_clustermanager.gooperator/v1/types_klusterlet.gowork/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yamlwork/v1/types.gowork/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yamlwork/v1alpha1/types_manifestworkreplicaset.go
💤 Files with no reviewable changes (1)
- cluster/v1alpha1/types_addonplacementscore.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-01T01:10:01.745Z
Learnt from: haoqing0110
Repo: open-cluster-management-io/api PR: 407
File: addon/v1beta1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yaml:17-778
Timestamp: 2025-12-01T01:10:01.745Z
Learning: In the open-cluster-management-io/api repository, CRD files for addon resources (like ManagedClusterAddOn and ClusterManagementAddOn) intentionally omit conversion webhook configuration in their base definitions. The registration-operator patches these CRDs at deployment time with the appropriate webhook clientConfig and CA bundle. This is a deliberate architectural pattern to separate schema definitions from deployment-specific webhook configuration.
Applied to files:
addon/v1beta1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yamladdon/v1alpha1/types_managedclusteraddon.goaddon/v1beta1/types_managedclusteraddon.goaddon/v1beta1/types_clustermanagementaddon.goaddon/v1alpha1/types_clustermanagementaddon.goaddon/v1alpha1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yamlcluster/v1/0000_00_clusters.open-cluster-management.io_managedclusters.crd.yamlwork/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yamlwork/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yamloperator/v1/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yamladdon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yamladdon/v1beta1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yamloperator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yamlcluster/v1beta1/0000_02_clusters.open-cluster-management.io_placements.crd.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration
- GitHub Check: verify
🔇 Additional comments (27)
work/v1/0000_00_work.open-cluster-management.io_manifestworks.crd.yaml (3)
215-217: LGTM! Correct list-type annotations for conditionRules.The annotations properly configure
conditionRulesas a map keyed by theconditionfield, enabling strategic merge patch semantics. This aligns with Kubernetes API conventions for list fields with unique identifiers.
456-458: LGTM! Standard Kubernetes condition list-type annotations.The annotations correctly configure the
conditionsarray as a map keyed bytype, which is the standard pattern for Kubernetes condition arrays. This ensures proper strategic merge behavior when updating ManifestWork status.
538-540: LGTM! Consistent condition list-type annotations for manifest status.The annotations correctly configure the nested
conditionsarray as a map keyed bytype, maintaining consistency with the top-level status conditions. This ensures proper merge behavior for per-manifest condition updates.addon/v1alpha1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yaml (1)
195-197: LGTM! Proper list type annotations for conditions array.The addition of
x-kubernetes-list-map-keysandx-kubernetes-list-type: mapcorrectly enables map-based list semantics for the conditions array, replacing the deprecated patch strategy. This ensures proper strategic merge patch behavior where conditions are keyed by theirtypefield.operator/v1/0000_01_operator.open-cluster-management.io_clustermanagers.crd.yaml (1)
853-855: LGTM! Consistent list type annotations for ClusterManager conditions.The annotations correctly configure map-based list semantics for the conditions array, consistent with the pattern applied across other CRDs in this PR. This aligns with current Kubernetes API conventions for condition arrays.
addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml (1)
527-529: LGTM! Proper list annotations for installProgressions conditions.The annotations correctly enable map-based list semantics for the conditions array within installProgressions. This maintains consistency with the broader pattern of updating condition arrays across the API.
operator/v1/types_clustermanager.go (1)
502-504: LGTM! Clean migration to new list type markers.The kubebuilder markers are correctly added without any deprecated struct tag markers, demonstrating a complete and clean migration. This is the correct pattern for updating list type annotations.
cluster/v1/types.go (2)
148-150: LGTM! Correct list type markers for Conditions.The addition of
+listType=mapand+listMapKey=typemarkers toManagedClusterStatus.Conditionscorrectly replaces the deprecated patch markers and aligns with current Kubernetes API conventions for Condition arrays.
219-221: LGTM! Correct list type markers for Conditions.The addition of
+listType=mapand+listMapKey=typemarkers toClusterSetManagedNamespaceConfig.Conditionscorrectly replaces the deprecated patch markers and aligns with current Kubernetes API conventions for Condition arrays.work/v1alpha1/types_manifestworkreplicaset.go (1)
82-84: LGTM! Correct list type markers for Conditions.The addition of
+listType=mapand+listMapKey=typemarkers toManifestWorkReplicaSetStatus.Conditionscorrectly replaces the deprecated patch markers and aligns with current Kubernetes API conventions for Condition arrays.cluster/v1beta1/types_placement.go (1)
441-444: LGTM! Correct list type markers for Conditions.The addition of
+listType=mapand+listMapKey=typemarkers toPlacementStatus.Conditionscorrectly replaces the deprecated patch markers and aligns with current Kubernetes API conventions for Condition arrays.cluster/v1/0000_00_clusters.open-cluster-management.io_managedclusters.crd.yaml (2)
265-267: LGTM! Correct CRD annotations for map-type list.The addition of
x-kubernetes-list-map-keysandx-kubernetes-list-type: mapannotations to thestatus.conditionsarray correctly implements the map-type list semantics in the CRD schema, corresponding to the kubebuilder markers added in the Go type definitions.
336-338: LGTM! Correct CRD annotations for map-type list.The addition of
x-kubernetes-list-map-keysandx-kubernetes-list-type: mapannotations to themanagedNamespaces[].conditionsarray correctly implements the map-type list semantics in the CRD schema, corresponding to the kubebuilder markers added in the Go type definitions.work/v1/types.go (2)
473-475: LGTM! Correct list type markers for Conditions.The addition of
+listType=mapand+listMapKey=typemarkers toManifestWorkStatus.Conditionscorrectly replaces the deprecated patch markers and aligns with current Kubernetes API conventions for Condition arrays.
543-545: LGTM! Correct list type markers for Conditions.The addition of
+listType=mapand+listMapKey=typemarkers toManifestCondition.Conditionscorrectly replaces the deprecated patch markers and aligns with current Kubernetes API conventions for Condition arrays.operator/v1/types_klusterlet.go (1)
361-363: LGTM! Standard Kubernetes API convention applied correctly.The addition of
+listType=mapand+listMapKey=typemarkers is the correct approach for Condition arrays, replacing deprecated patch merge markers and aligning with current Kubernetes API conventions.work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml (1)
726-728: LGTM! CRD schema correctly updated for map-based list semantics.The addition of
x-kubernetes-list-type: mapandx-kubernetes-list-map-keysannotations ensures proper strategic merge patch behavior for the conditions array.addon/v1alpha1/types_clustermanagementaddon.go (1)
214-217: LGTM! Markers updated correctly while preserving struct tags.The new
+listType=mapand+listMapKey=typecomment markers are correctly added for CRD generation, while the existing struct tags are properly retained for JSON encoding.addon/v1beta1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml (2)
527-529: LGTM! v1alpha1 CRD schema correctly updated.The map-based list annotations are properly added to the conditions field in the v1alpha1 version.
1104-1106: LGTM! v1beta1 CRD schema consistently updated.The v1beta1 version correctly mirrors the same map-based list semantics as v1alpha1, ensuring consistency across API versions.
addon/v1beta1/types_managedclusteraddon.go (1)
136-139: LGTM! Correctly updated with proper marker and tag separation.The
+listType=mapand+listMapKey=typemarkers are correctly added for CRD schema generation, while the struct tags for JSON encoding are appropriately retained.cluster/v1beta1/0000_02_clusters.open-cluster-management.io_placements.crd.yaml (1)
604-606: LGTM!The CRD schema annotations correctly specify map-based list semantics for the conditions array, enabling proper strategic merge patch behavior with
typeas the map key.operator/v1/0000_00_operator.open-cluster-management.io_klusterlets.crd.yaml (1)
594-596: LGTM!The CRD schema annotations are correctly applied to the status.conditions array, consistent with the pattern used across other CRDs in this PR.
addon/v1beta1/0000_01_addon.open-cluster-management.io_managedclusteraddons.crd.yaml (4)
195-197: LGTM!The CRD schema annotations correctly implement map-based list semantics for the v1alpha1 status.conditions array.
393-396: LGTM!The supportedConfigs array correctly uses composite map keys (group and resource) for map-based merging, which aligns with the ConfigGroupResource structure.
551-553: LGTM!The CRD schema annotations correctly implement map-based list semantics for the v1beta1 status.conditions array, consistent with the v1alpha1 version.
764-767: LGTM!The supportedConfigs array in v1beta1 correctly uses composite map keys (group and resource), maintaining consistency with the v1alpha1 version.
| // +listType=map | ||
| // +listMapKey=type | ||
| // +optional | ||
| Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` |
There was a problem hiding this comment.
Remove deprecated struct tag markers.
The struct field still contains the deprecated patchStrategy:"merge" and patchMergeKey:"type" in the struct tag. Since you've added the new kubebuilder markers (+listType=map and +listMapKey=type), the old struct tag markers should be removed for consistency.
🔎 Proposed fix
// conditions describe the state of the managed and monitored components for the operator.
// +listType=map
// +listMapKey=type
// +optional
-Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
+Conditions []metav1.Condition `json:"conditions,omitempty"`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // +listType=map | |
| // +listMapKey=type | |
| // +optional | |
| Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` | |
| // +listType=map | |
| // +listMapKey=type | |
| // +optional | |
| Conditions []metav1.Condition `json:"conditions,omitempty"` |
🤖 Prompt for AI Agents
In @addon/v1alpha1/types_managedclusteraddon.go around lines 104-107, The
Conditions field still includes deprecated struct tags patchStrategy:"merge" and
patchMergeKey:"type"; remove those two tags from the Conditions field tag so it
only keeps json:"conditions,omitempty" (and any other non-deprecated tags),
while leaving the kubebuilder markers (+listType=map and +listMapKey=type) above
the field intact; update the struct field declaration for Conditions in
types_managedclusteraddon.go accordingly.
| // +listType=map | ||
| // +listMapKey=type | ||
| // +optional | ||
| Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for other Conditions fields in the codebase to check the pattern
rg -n -A1 -B1 'Conditions\s+\[\]metav1\.Condition' --type=goRepository: open-cluster-management-io/api
Length of output: 3360
Remove old struct tags now that new markers are in place.
The new markers +listType=map and +listMapKey=type have been added, but the old struct tags patchStrategy:"merge" patchMergeKey:"type" remain. This file is the only one in the codebase mixing both approaches. The rest of the codebase uses only the new markers (e.g., work/v1, operator/v1, cluster/v1) or only the old tags (other addon files not yet migrated). Remove the old struct tags to align with the modern Kubernetes API conventions and complete the migration to the new markers.
🤖 Prompt for AI Agents
In @addon/v1beta1/types_clustermanagementaddon.go around lines 175-178, The
Conditions field on the Clustermanagementaddon type still has legacy struct
tags; remove the old patchStrategy:"merge" and patchMergeKey:"type" tags from
the Conditions []metav1.Condition `json:"conditions,omitempty"
patchStrategy:"merge" patchMergeKey:"type"` declaration so only the new markers
(+listType=map and +listMapKey=type) and the json tag remain; update the
Conditions field signature to use `json:"conditions,omitempty"` (keeping the
existing markers above the field) to complete the migration.
openshift-merge-bot
bot
merged commit 7b34437
into
open-cluster-management-io:main
2 participants
Replace deprecated +patchMergeKey and +patchStrategy markers with +listType=map and +listMapKey=type for condition arrays across all API versions. This ensures proper strategic merge patch behavior and aligns with current Kubernetes API conventions.
Updates CRD YAML files with corresponding x-kubernetes-list-type and x-kubernetes-list-map-keys annotations.
🤖 Generated with Claude Code
Summary
Related issue(s)
Fixes #
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.