-
Notifications
You must be signed in to change notification settings - Fork 24
Fix zero value detection for custom types with validation markers #184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,19 @@ | ||
| package a | ||
|
|
||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=23 | ||
| type BootstrapTokenString string | ||
|
|
||
| // BootstrapTokenStruct is a struct with Type=string marker (issue #138). | ||
| // This struct has custom JSON marshalling that serializes it as a string, not an object. | ||
| // +kubebuilder:validation:Type=string | ||
| // +kubebuilder:validation:MinLength=1 | ||
| // +kubebuilder:validation:MaxLength=23 | ||
| type BootstrapTokenStruct struct { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this would typically have a custom unmarshal function right?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I initially tried to add actual (un)marshal methods to the testdata file to make this more explicit, but the test framework expects the data to remain in a specific state. |
||
| ID string `json:"-"` | ||
| Secret string `json:"-"` | ||
| } | ||
|
|
||
| type TestStrings struct { | ||
| String string `json:"string"` // want "field String should have the omitempty tag." "field String has a valid zero value \\(\"\"\\), but the validation is not complete \\(e.g. minimum length\\). The field should be a pointer to allow the zero value to be set. If the zero value is not a valid use case, complete the validation and remove the pointer." | ||
|
|
||
|
|
@@ -56,4 +70,22 @@ type TestStrings struct { | |
|
|
||
| // +kubebuilder:validation:Enum=a;b;c;"" | ||
| EnumValidEmptyStringPtrWithOmitEmpty *string `json:"enumValidEmptyStringPtrWithOmitEmpty,omitempty"` | ||
|
|
||
| // Test for issue #138: custom string type with validation markers on the type definition | ||
| Token BootstrapTokenString `json:"token"` // want "field Token should have the omitempty tag." | ||
|
|
||
| TokenWithOmitEmpty BootstrapTokenString `json:"tokenWithOmitEmpty,omitempty"` | ||
|
|
||
| TokenPtr *BootstrapTokenString `json:"tokenPtr"` // want "field TokenPtr should have the omitempty tag." "field TokenPtr does not allow the zero value. The field does not need to be a pointer." | ||
|
|
||
| TokenPtrWithOmitEmpty *BootstrapTokenString `json:"tokenPtrWithOmitEmpty,omitempty"` // want "field TokenPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer." | ||
|
|
||
| // Test for issue #138: struct with Type=string marker | ||
| TokenStruct BootstrapTokenStruct `json:"tokenStruct"` // want "field TokenStruct should have the omitempty tag." | ||
|
|
||
| TokenStructWithOmitEmpty BootstrapTokenStruct `json:"tokenStructWithOmitEmpty,omitempty"` | ||
|
|
||
| TokenStructPtr *BootstrapTokenStruct `json:"tokenStructPtr"` // want "field TokenStructPtr should have the omitempty tag." "field TokenStructPtr does not allow the zero value. The field does not need to be a pointer." | ||
|
|
||
| TokenStructPtrWithOmitEmpty *BootstrapTokenStruct `json:"tokenStructPtrWithOmitEmpty,omitempty"` // want "field TokenStructPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer." | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to set these on non struct types? Could other types have custom marshalling and custom types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, type markers can technically be set on non-struct custom types (e.g.,
type MyInt int), and non-struct types can have custom marshalling.For non-struct types, validation markers are already collected via
TypeAwareMarkerCollectionForFieldand handled correctly byIsZeroValueValidwithout needing special treatment in the serialization check.