Skip to content

Commit f12c55e

Browse files
committed
Fix zero value detection for custom types with validation markers
Fixes issue #138 where the linter was not detecting invalid zero values for structs with Type=string markers and string validation constraints. The problem occurred because structs with custom marshalling (indicated by the Type=string marker) were being validated as regular structs instead of as strings, causing MinLength/MaxLength markers to be ignored. Signed-off-by: Sascha Grunert <[email protected]>
1 parent 33e0e43 commit f12c55e

File tree

4 files changed

+106
-0
lines changed

4 files changed

+106
-0
lines changed

pkg/analysis/utils/serialization/serialization_check.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,19 @@ func (s *serializationCheck) Check(pass *analysis.Pass, field *ast.Field, marker
9696
isPointer, underlying := utils.IsStarExpr(field.Type)
9797
isStruct := utils.IsStructType(pass, field.Type)
9898

99+
// Check if this struct should be treated as a non-struct type (e.g., Type=string marker).
100+
// This handles structs with custom marshalling that serialize as other types.
101+
if isStruct {
102+
typeValue := utils.GetTypeMarkerValue(pass, field, markersAccess)
103+
// If the type marker indicates this is not a struct, treat it accordingly.
104+
// Type "object" means it's still a struct/object type in the OpenAPI sense.
105+
// Other types (string, number, integer, boolean, array) indicate custom marshalling
106+
// that changes the serialization format from a struct to that type.
107+
if typeValue != "" && typeValue != "object" {
108+
isStruct = false
109+
}
110+
}
111+
99112
switch s.pointerPreference {
100113
case PointersPreferenceAlways:
101114
// The field must always be a pointer, pointers require omitempty, so enforce that too.

pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
package a
22

3+
// +kubebuilder:validation:MinLength=1
4+
// +kubebuilder:validation:MaxLength=23
5+
type BootstrapTokenString string
6+
7+
// BootstrapTokenStruct is a struct with Type=string marker (issue #138).
8+
// This struct has custom JSON marshalling that serializes it as a string, not an object.
9+
// +kubebuilder:validation:Type=string
10+
// +kubebuilder:validation:MinLength=1
11+
// +kubebuilder:validation:MaxLength=23
12+
type BootstrapTokenStruct struct {
13+
ID string `json:"-"`
14+
Secret string `json:"-"`
15+
}
16+
317
type TestStrings struct {
418
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."
519

@@ -56,4 +70,22 @@ type TestStrings struct {
5670

5771
// +kubebuilder:validation:Enum=a;b;c;""
5872
EnumValidEmptyStringPtrWithOmitEmpty *string `json:"enumValidEmptyStringPtrWithOmitEmpty,omitempty"`
73+
74+
// Test for issue #138: custom string type with validation markers on the type definition
75+
Token BootstrapTokenString `json:"token"` // want "field Token should have the omitempty tag."
76+
77+
TokenWithOmitEmpty BootstrapTokenString `json:"tokenWithOmitEmpty,omitempty"`
78+
79+
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."
80+
81+
TokenPtrWithOmitEmpty *BootstrapTokenString `json:"tokenPtrWithOmitEmpty,omitempty"` // want "field TokenPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer."
82+
83+
// Test for issue #138: struct with Type=string marker
84+
TokenStruct BootstrapTokenStruct `json:"tokenStruct"` // want "field TokenStruct should have the omitempty tag."
85+
86+
TokenStructWithOmitEmpty BootstrapTokenStruct `json:"tokenStructWithOmitEmpty,omitempty"`
87+
88+
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."
89+
90+
TokenStructPtrWithOmitEmpty *BootstrapTokenStruct `json:"tokenStructPtrWithOmitEmpty,omitempty"` // want "field TokenStructPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer."
5991
}

pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go.golden

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,19 @@
11
package a
22

3+
// +kubebuilder:validation:MinLength=1
4+
// +kubebuilder:validation:MaxLength=23
5+
type BootstrapTokenString string
6+
7+
// BootstrapTokenStruct is a struct with Type=string marker (issue #138).
8+
// This struct has custom JSON marshalling that serializes it as a string, not an object.
9+
// +kubebuilder:validation:Type=string
10+
// +kubebuilder:validation:MinLength=1
11+
// +kubebuilder:validation:MaxLength=23
12+
type BootstrapTokenStruct struct {
13+
ID string `json:"-"`
14+
Secret string `json:"-"`
15+
}
16+
317
type TestStrings struct {
418
String *string `json:"string,omitempty"` // 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."
519

@@ -56,4 +70,22 @@ type TestStrings struct {
5670

5771
// +kubebuilder:validation:Enum=a;b;c;""
5872
EnumValidEmptyStringPtrWithOmitEmpty *string `json:"enumValidEmptyStringPtrWithOmitEmpty,omitempty"`
73+
74+
// Test for issue #138: custom string type with validation markers on the type definition
75+
Token BootstrapTokenString `json:"token,omitempty"` // want "field Token should have the omitempty tag."
76+
77+
TokenWithOmitEmpty BootstrapTokenString `json:"tokenWithOmitEmpty,omitempty"`
78+
79+
TokenPtr BootstrapTokenString `json:"tokenPtr,omitempty"` // 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."
80+
81+
TokenPtrWithOmitEmpty BootstrapTokenString `json:"tokenPtrWithOmitEmpty,omitempty"` // want "field TokenPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer."
82+
83+
// Test for issue #138: struct with Type=string marker
84+
TokenStruct BootstrapTokenStruct `json:"tokenStruct,omitempty"` // want "field TokenStruct should have the omitempty tag."
85+
86+
TokenStructWithOmitEmpty BootstrapTokenStruct `json:"tokenStructWithOmitEmpty,omitempty"`
87+
88+
TokenStructPtr BootstrapTokenStruct `json:"tokenStructPtr,omitempty"` // 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."
89+
90+
TokenStructPtrWithOmitEmpty BootstrapTokenStruct `json:"tokenStructPtrWithOmitEmpty,omitempty"` // want "field TokenStructPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer."
5991
}

pkg/analysis/utils/zero_value.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,43 @@ func getUnderlyingType(expr ast.Expr) ast.Expr {
7373
return expr
7474
}
7575

76+
// GetTypeMarkerValue returns the value of the kubebuilder Type marker for a field.
77+
// Returns empty string if no Type marker is present.
78+
// The Type marker indicates how the field serializes (e.g., "string", "number", "object").
79+
func GetTypeMarkerValue(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) string {
80+
fieldMarkers := TypeAwareMarkerCollectionForField(pass, markersAccess, field)
81+
typeMarkers := fieldMarkers.Get(markers.KubebuilderTypeMarker)
82+
83+
for _, typeMarker := range typeMarkers {
84+
// The value might be "string" (with quotes) or string (without quotes)
85+
typeValue := strings.Trim(typeMarker.Payload.Value, `"`)
86+
if typeValue != "" {
87+
return typeValue
88+
}
89+
}
90+
91+
return ""
92+
}
93+
7694
// isStructZeroValueValid checks if the zero value of a struct is valid.
7795
// It checks if all non-omitted fields within the struct accept their zero values.
7896
// It also checks if the struct has a minProperties marker, and if so, whether the number of non-omitted fields is greater than or equal to the minProperties value.
97+
// Special case: If the struct has Type=string marker with string validation markers (MinLength/MaxLength),
98+
// treat it as a string for validation purposes (e.g., for structs with custom marshalling).
7999
func isStructZeroValueValid(pass *analysis.Pass, field *ast.Field, structType *ast.StructType, markersAccess markershelper.Markers, considerOmitzero bool) (bool, bool) {
80100
if structType == nil {
81101
return false, false
82102
}
83103

104+
// Check if this struct should be validated as a string (Type=string marker).
105+
// This handles structs with custom marshalling that serialize as strings.
106+
if GetTypeMarkerValue(pass, field, markersAccess) == "string" {
107+
// Use string validation logic instead of struct validation logic.
108+
// This ensures that string-specific validation markers (MinLength, MaxLength, Pattern)
109+
// are properly evaluated for structs that marshal as strings.
110+
return isStringZeroValueValid(pass, field, markersAccess)
111+
}
112+
84113
jsonTagInfo, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
85114
if !ok {
86115
panic("could not get struct field tags from pass result")

0 commit comments

Comments
 (0)