🐛 (fix) Priority field to use pointer type with correct markers #2413
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| // | ||
| // +kubebuilder:default:=0 | ||
| // +kubebuilder:validation:minimum:=-2147483648 | ||
| // +kubebuilder:validation:maximum:=2147483647 |
There was a problem hiding this comment.
The above ones are invalid.
Therefore, we are fixing the validation here.
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| // +kubebuilder:validation:Format:=int32 | ||
| // +optional | ||
| Priority int32 `json:"priority"` | ||
| Priority *int32 `json:"priority,omitempty"` |
There was a problem hiding this comment.
- Pointer = distinguish between "not set" (nil) vs "set to 0"
- omitempty = don't show field in YAML when not set (cleaner)
- kubebuilder default = API server fills in 0 when user omits it
Change ClusterCatalog Priority field from int32 to *int32 to properly represent an optional field with a default value. - Update Priority to *int32 with json:"priority,omitempty" - Fix kubebuilder markers: Minimum/Maximum capitalization - Add Format:=int32 marker for OpenAPI schema - Add GetPriority() helper method for safe nil handling - Update resolver to use GetPriority() instead of direct access - Update tests to use ptr.To[int32]() for pointer values The kubebuilder default marker ensures the API server sets Priority to 0 when omitted, while the pointer type allows proper detection of user-specified vs defaulted values. Assisted-by: CLAUDE
camilamacedo86
force-pushed
the
fix-marker-minmax
branch
from
84c7ea1 to
c88aa54
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The lowest possible value is -2147483648. | ||
| // The highest possible value is 2147483647. | ||
| // | ||
| // +kubebuilder:default:=0 |
There was a problem hiding this comment.
The +kubebuilder:default:=0 marker combined with the pointer type *int32 creates a contradiction. When this default is applied by the API server, the Go code will receive a non-nil pointer to 0 whether the user explicitly set priority to 0 or omitted it entirely. This defeats the purpose of using a pointer type, which is typically to distinguish between "not set" (nil) and "set to zero value" (non-nil pointer to 0). Consider removing the default marker if the goal is to detect user-specified vs omitted values, or changing back to a non-pointer type if a default value should always be present.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2413 +/- ##
=======================================
Coverage 68.82% 68.82%
=======================================
Files 100 100
Lines 7641 7648 +7
=======================================
+ Hits 5259 5264 +5
- Misses 1947 1948 +1
- Partials 435 436 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Change ClusterCatalog Priority field from int32 to *int32 to properly
represent an optional field with a default value.
The kubebuilder default marker ensures the API server sets Priority
to 0 when omitted, while the pointer type allows proper detection of
user-specified vs defaulted values.