From de6dfb60682795aefd1d52307d43a89bd454fdd5 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 6 Nov 2025 15:12:19 +0100 Subject: [PATCH] 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 --- .../serialization/serialization_check.go | 13 ++++++++ .../src/pointers_when_required/strings.go | 32 +++++++++++++++++++ .../pointers_when_required/strings.go.golden | 32 +++++++++++++++++++ pkg/analysis/utils/zero_value.go | 29 +++++++++++++++++ 4 files changed, 106 insertions(+) diff --git a/pkg/analysis/utils/serialization/serialization_check.go b/pkg/analysis/utils/serialization/serialization_check.go index 91439ac5..fc92a681 100644 --- a/pkg/analysis/utils/serialization/serialization_check.go +++ b/pkg/analysis/utils/serialization/serialization_check.go @@ -96,6 +96,19 @@ func (s *serializationCheck) Check(pass *analysis.Pass, field *ast.Field, marker isPointer, underlying := utils.IsStarExpr(field.Type) isStruct := utils.IsStructType(pass, field.Type) + // Check if this struct should be treated as a non-struct type (e.g., Type=string marker). + // This handles structs with custom marshalling that serialize as other types. + if isStruct { + typeValue := utils.GetTypeMarkerValue(pass, field, markersAccess) + // If the type marker indicates this is not a struct, treat it accordingly. + // Type "object" means it's still a struct/object type in the OpenAPI sense. + // Other types (string, number, integer, boolean, array) indicate custom marshalling + // that changes the serialization format from a struct to that type. + if typeValue != "" && typeValue != "object" { + isStruct = false + } + } + switch s.pointerPreference { case PointersPreferenceAlways: // The field must always be a pointer, pointers require omitempty, so enforce that too. diff --git a/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go b/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go index 6f92241c..fe700ba3 100644 --- a/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go +++ b/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go @@ -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 { + 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." } diff --git a/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go.golden b/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go.golden index d7b796e0..25000ec9 100644 --- a/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go.golden +++ b/pkg/analysis/utils/serialization/testdata/src/pointers_when_required/strings.go.golden @@ -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 { + ID string `json:"-"` + Secret string `json:"-"` +} + type TestStrings struct { 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." @@ -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,omitempty"` // want "field Token should have the omitempty tag." + + TokenWithOmitEmpty BootstrapTokenString `json:"tokenWithOmitEmpty,omitempty"` + + 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." + + 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,omitempty"` // want "field TokenStruct should have the omitempty tag." + + TokenStructWithOmitEmpty BootstrapTokenStruct `json:"tokenStructWithOmitEmpty,omitempty"` + + 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." + + TokenStructPtrWithOmitEmpty BootstrapTokenStruct `json:"tokenStructPtrWithOmitEmpty,omitempty"` // want "field TokenStructPtrWithOmitEmpty does not allow the zero value. The field does not need to be a pointer." } diff --git a/pkg/analysis/utils/zero_value.go b/pkg/analysis/utils/zero_value.go index f80f4af4..3796700a 100644 --- a/pkg/analysis/utils/zero_value.go +++ b/pkg/analysis/utils/zero_value.go @@ -73,14 +73,43 @@ func getUnderlyingType(expr ast.Expr) ast.Expr { return expr } +// GetTypeMarkerValue returns the value of the kubebuilder Type marker for a field. +// Returns empty string if no Type marker is present. +// The Type marker indicates how the field serializes (e.g., "string", "number", "object"). +func GetTypeMarkerValue(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) string { + fieldMarkers := TypeAwareMarkerCollectionForField(pass, markersAccess, field) + typeMarkers := fieldMarkers.Get(markers.KubebuilderTypeMarker) + + for _, typeMarker := range typeMarkers { + // The value might be "string" (with quotes) or string (without quotes) + typeValue := strings.Trim(typeMarker.Payload.Value, `"`) + if typeValue != "" { + return typeValue + } + } + + return "" +} + // isStructZeroValueValid checks if the zero value of a struct is valid. // It checks if all non-omitted fields within the struct accept their zero values. // 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. +// Special case: If the struct has Type=string marker with string validation markers (MinLength/MaxLength), +// treat it as a string for validation purposes (e.g., for structs with custom marshalling). func isStructZeroValueValid(pass *analysis.Pass, field *ast.Field, structType *ast.StructType, markersAccess markershelper.Markers, considerOmitzero bool) (bool, bool) { if structType == nil { return false, false } + // Check if this struct should be validated as a string (Type=string marker). + // This handles structs with custom marshalling that serialize as strings. + if GetTypeMarkerValue(pass, field, markersAccess) == "string" { + // Use string validation logic instead of struct validation logic. + // This ensures that string-specific validation markers (MinLength, MaxLength, Pattern) + // are properly evaluated for structs that marshal as strings. + return isStringZeroValueValid(pass, field, markersAccess) + } + jsonTagInfo, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags) if !ok { panic("could not get struct field tags from pass result")