diff --git a/docs/linters.md b/docs/linters.md index 84245f8d..1e914939 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -281,7 +281,7 @@ The linter checks for: ```yaml lintersConfig: - ssaTags: + ssatags: listTypeSetUsage: Warn | Ignore # The policy for listType=set usage on object arrays. Defaults to `Warn`. ``` diff --git a/pkg/analysis/ssatags/analyzer.go b/pkg/analysis/ssatags/analyzer.go index 4a0e651e..46230b82 100644 --- a/pkg/analysis/ssatags/analyzer.go +++ b/pkg/analysis/ssatags/analyzer.go @@ -83,6 +83,31 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, markersAcce return } + // If the field is a byte array, we cannot use listType markers with it. + if utils.IsByteArray(pass, field) { + listTypeMarkers := fieldMarkers.Get(kubebuildermarkers.KubebuilderListTypeMarker) + for _, marker := range listTypeMarkers { + pass.Report(analysis.Diagnostic{ + Pos: field.Pos(), + Message: fmt.Sprintf("%s is a byte array, which does not support the listType marker. Remove the listType marker", utils.FieldName(field)), + SuggestedFixes: []analysis.SuggestedFix{ + { + Message: fmt.Sprintf("Remove listType marker from %s", utils.FieldName(field)), + TextEdits: []analysis.TextEdit{ + { + Pos: marker.Pos, + End: marker.End + 1, + NewText: []byte(""), + }, + }, + }, + }, + }) + } + + return + } + fieldName := utils.FieldName(field) listTypeMarkers := fieldMarkers.Get(kubebuildermarkers.KubebuilderListTypeMarker) diff --git a/pkg/analysis/ssatags/testdata/src/a/a.go b/pkg/analysis/ssatags/testdata/src/a/a.go index b744ef3d..db1a04a4 100644 --- a/pkg/analysis/ssatags/testdata/src/a/a.go +++ b/pkg/analysis/ssatags/testdata/src/a/a.go @@ -19,54 +19,57 @@ type IntAlias = int type ObjectAlias = TestObject type StringArrayAlias = []string +type ByteArray []byte +type ByteArrayAlias = []byte + type SSATagsTestSpec struct { // Valid atomic list - should pass - // +kubebuilder:listType=atomic + // +listType=atomic AtomicStringList []string `json:"atomicStringList,omitempty"` // Valid atomic object list - should pass - // +kubebuilder:listType=atomic + // +listType=atomic AtomicObjectList []TestObject `json:"atomicObjectList,omitempty"` // Valid set for primitive list - should pass (no warning for primitive lists) - // +kubebuilder:listType=set + // +listType=set SetPrimitiveList []string `json:"setPrimitiveList,omitempty"` // Invalid set for object list - should warn about listType=set compatibility issues - // +kubebuilder:listType=set + // +listType=set SetObjectList []TestObject `json:"setObjectList,omitempty"` // want "SetObjectList with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" // Invalid map on primitive list - should error - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=name + // +listType=map + // +listMapKey=name MapPrimitiveList []string `json:"mapPrimitiveList,omitempty"` // want "MapPrimitiveList with listType=map can only be used for object lists, not primitive lists" // Valid map on object list with proper listMapKey - should pass - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=name + // +listType=map + // +listMapKey=name MapObjectList []TestObject `json:"mapObjectList,omitempty"` // Invalid map on object list without listMapKey - should error - // +kubebuilder:listType=map + // +listType=map MapObjectListNoKey []TestObject `json:"mapObjectListNoKey,omitempty"` // want "MapObjectListNoKey with listType=map must have at least one listMapKey marker" // Invalid map on object list with non-existent listMapKey - should error - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=nonexistent + // +listType=map + // +listMapKey=nonexistent MapObjectListInvalidKey []TestObject `json:"mapObjectListInvalidKey,omitempty"` // want "MapObjectListInvalidKey listMapKey \"nonexistent\" does not exist as a field in the struct" // Valid map with correct JSON tag name - should pass - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=name + // +listType=map + // +listMapKey=name MapObjectListValid []SimpleObject `json:"mapObjectListValid,omitempty"` // Invalid map with non-existent JSON tag name - should error - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=invalid_name + // +listType=map + // +listMapKey=invalid_name MapObjectListInvalidName []SimpleObject `json:"mapObjectListInvalidName,omitempty"` // want "MapObjectListInvalidName listMapKey \"invalid_name\" does not exist as a field in the struct" // Invalid listType value - should error - // +kubebuilder:listType=invalid + // +listType=invalid InvalidListType []string `json:"invalidListType,omitempty"` // want "InvalidListType has invalid listType \"invalid\", must be one of: atomic, set, map" // Missing listType on primitive array - should warn @@ -82,17 +85,17 @@ type SSATagsTestSpec struct { PointerArrayNoMarker []*TestObject `json:"pointerArrayNoMarker,omitempty"` // want "PointerArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Defined type tests - defined types should behave as their underlying types - // +kubebuilder:listType=atomic + // +listType=atomic StringAtomicList []String `json:"stringAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringSetList []String `json:"stringSetList,omitempty"` // Defined type to object should behave as object list - // +kubebuilder:listType=atomic + // +listType=atomic ObjectAtomicList []Object `json:"objectAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set ObjectSetList []Object `json:"objectSetList,omitempty"` // want "ObjectSetList with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" // Missing listType on defined type to basic type - should warn @@ -102,24 +105,24 @@ type SSATagsTestSpec struct { ObjectNoMarker []Object `json:"objectNoMarker,omitempty"` // want "ObjectNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Pointer to defined type - should behave same as defined type - // +kubebuilder:listType=atomic + // +listType=atomic PointerToString []*String `json:"pointerToString,omitempty"` - // +kubebuilder:listType=set + // +listType=set PointerToObject []*Object `json:"pointerToObject,omitempty"` // want "PointerToObject with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" // Type alias tests - aliases to basic types should behave as primitive lists - // +kubebuilder:listType=atomic + // +listType=atomic StringAliasAtomicList []StringAlias `json:"stringAliasAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringAliasSetList []StringAlias `json:"stringAliasSetList,omitempty"` // Type alias to object should behave as object list - // +kubebuilder:listType=atomic + // +listType=atomic ObjectAliasAtomicList []ObjectAlias `json:"objectAliasAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set ObjectAliasSetList []ObjectAlias `json:"objectAliasSetList,omitempty"` // want "ObjectAliasSetList with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" // Missing listType on alias to basic type - should warn @@ -129,32 +132,46 @@ type SSATagsTestSpec struct { ObjectAliasNoMarker []ObjectAlias `json:"objectAliasNoMarker,omitempty"` // want "ObjectAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Pointer to alias - should behave same as alias - // +kubebuilder:listType=atomic + // +listType=atomic PointerToStringAlias []*StringAlias `json:"pointerToStringAlias,omitempty"` - // +kubebuilder:listType=set + // +listType=set PointerToObjectAlias []*ObjectAlias `json:"pointerToObjectAlias,omitempty"` // want "PointerToObjectAlias with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" // Multiple pointer levels PointerToPointerObject []*(*TestObject) `json:"pointerToPointerObject,omitempty"` // want "PointerToPointerObject should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Array defined type tests - defined types to array types should behave as array lists - // +kubebuilder:listType=atomic + // +listType=atomic StringArrayAtomicList StringArray `json:"stringArrayAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringArraySetList StringArray `json:"stringArraySetList,omitempty"` // Missing listType on array defined type - should warn StringArrayNoMarker StringArray `json:"stringArrayNoMarker,omitempty"` // want "StringArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Array type alias tests - aliases to array types should behave as array lists - // +kubebuilder:listType=atomic + // +listType=atomic StringArrayAliasAtomicList StringArrayAlias `json:"stringArrayAliasAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringArrayAliasSetList StringArrayAlias `json:"stringArrayAliasSetList,omitempty"` // Missing listType on array alias - should warn StringArrayAliasNoMarker StringArrayAlias `json:"stringArrayAliasNoMarker,omitempty"` // want "StringArrayAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Byte array tests - these should be skipped by IsByteArray condition + // Direct byte array - should be ignored (no listType required) + ByteArrayDirect []byte `json:"byteArrayDirect,omitempty"` + + // Defined type byte array - should be ignored (no listType required) + ByteArrayDefinedType ByteArray `json:"byteArrayDefinedType,omitempty"` + + // Aliased byte array - should be ignored (no listType required) + ByteArrayAliased ByteArrayAlias `json:"byteArrayAliased,omitempty"` + + // Byte array with markers - should be ignored even if markers are present + // +listType=atomic + ByteArrayWithMarker []byte `json:"byteArrayWithMarker,omitempty"` // want "ByteArrayWithMarker is a byte array, which does not support the listType marker. Remove the listType marker" } diff --git a/pkg/analysis/ssatags/testdata/src/a/a.go.golden b/pkg/analysis/ssatags/testdata/src/a/a.go.golden new file mode 100644 index 00000000..0dcb65f7 --- /dev/null +++ b/pkg/analysis/ssatags/testdata/src/a/a.go.golden @@ -0,0 +1,176 @@ +package a + +type TestObject struct { + Name string `json:"name"` + ID int `json:"id"` +} + +type SimpleObject struct { + Name string `json:"name"` +} + +type String string +type Int int +type Object TestObject +type StringArray []string + +type StringAlias = string +type IntAlias = int +type ObjectAlias = TestObject +type StringArrayAlias = []string + +type ByteArray []byte +type ByteArrayAlias = []byte + +type SSATagsTestSpec struct { + // Valid atomic list - should pass + // +listType=atomic + AtomicStringList []string `json:"atomicStringList,omitempty"` + + // Valid atomic object list - should pass + // +listType=atomic + AtomicObjectList []TestObject `json:"atomicObjectList,omitempty"` + + // Valid set for primitive list - should pass (no warning for primitive lists) + // +listType=set + SetPrimitiveList []string `json:"setPrimitiveList,omitempty"` + + // Invalid set for object list - should warn about listType=set compatibility issues + // +listType=set + SetObjectList []TestObject `json:"setObjectList,omitempty"` // want "SetObjectList with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" + + // Invalid map on primitive list - should error + // +listType=map + // +listMapKey=name + MapPrimitiveList []string `json:"mapPrimitiveList,omitempty"` // want "MapPrimitiveList with listType=map can only be used for object lists, not primitive lists" + + // Valid map on object list with proper listMapKey - should pass + // +listType=map + // +listMapKey=name + MapObjectList []TestObject `json:"mapObjectList,omitempty"` + + // Invalid map on object list without listMapKey - should error + // +listType=map + MapObjectListNoKey []TestObject `json:"mapObjectListNoKey,omitempty"` // want "MapObjectListNoKey with listType=map must have at least one listMapKey marker" + + // Invalid map on object list with non-existent listMapKey - should error + // +listType=map + // +listMapKey=nonexistent + MapObjectListInvalidKey []TestObject `json:"mapObjectListInvalidKey,omitempty"` // want "MapObjectListInvalidKey listMapKey \"nonexistent\" does not exist as a field in the struct" + + // Valid map with correct JSON tag name - should pass + // +listType=map + // +listMapKey=name + MapObjectListValid []SimpleObject `json:"mapObjectListValid,omitempty"` + + // Invalid map with non-existent JSON tag name - should error + // +listType=map + // +listMapKey=invalid_name + MapObjectListInvalidName []SimpleObject `json:"mapObjectListInvalidName,omitempty"` // want "MapObjectListInvalidName listMapKey \"invalid_name\" does not exist as a field in the struct" + + // Invalid listType value - should error + // +listType=invalid + InvalidListType []string `json:"invalidListType,omitempty"` // want "InvalidListType has invalid listType \"invalid\", must be one of: atomic, set, map" + + // Missing listType on primitive array - should warn + PrimitiveArrayNoMarker []string `json:"primitiveArrayNoMarker,omitempty"` // want "PrimitiveArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Missing listType on object array - should warn + ObjectArrayNoMarker []TestObject `json:"objectArrayNoMarker,omitempty"` // want "ObjectArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Non-array field - should be ignored + SingleValue string `json:"singleValue,omitempty"` + + // Pointer array field - should behave same as non-pointer + PointerArrayNoMarker []*TestObject `json:"pointerArrayNoMarker,omitempty"` // want "PointerArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Defined type tests - defined types should behave as their underlying types + // +listType=atomic + StringAtomicList []String `json:"stringAtomicList,omitempty"` + + // +listType=set + StringSetList []String `json:"stringSetList,omitempty"` + + // Defined type to object should behave as object list + // +listType=atomic + ObjectAtomicList []Object `json:"objectAtomicList,omitempty"` + + // +listType=set + ObjectSetList []Object `json:"objectSetList,omitempty"` // want "ObjectSetList with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" + + // Missing listType on defined type to basic type - should warn + StringNoMarker []String `json:"stringNoMarker,omitempty"` // want "StringNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Missing listType on defined type to object - should warn + ObjectNoMarker []Object `json:"objectNoMarker,omitempty"` // want "ObjectNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Pointer to defined type - should behave same as defined type + // +listType=atomic + PointerToString []*String `json:"pointerToString,omitempty"` + + // +listType=set + PointerToObject []*Object `json:"pointerToObject,omitempty"` // want "PointerToObject with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" + + // Type alias tests - aliases to basic types should behave as primitive lists + // +listType=atomic + StringAliasAtomicList []StringAlias `json:"stringAliasAtomicList,omitempty"` + + // +listType=set + StringAliasSetList []StringAlias `json:"stringAliasSetList,omitempty"` + + // Type alias to object should behave as object list + // +listType=atomic + ObjectAliasAtomicList []ObjectAlias `json:"objectAliasAtomicList,omitempty"` + + // +listType=set + ObjectAliasSetList []ObjectAlias `json:"objectAliasSetList,omitempty"` // want "ObjectAliasSetList with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" + + // Missing listType on alias to basic type - should warn + StringAliasNoMarker []StringAlias `json:"stringAliasNoMarker,omitempty"` // want "StringAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Missing listType on alias to object - should warn + ObjectAliasNoMarker []ObjectAlias `json:"objectAliasNoMarker,omitempty"` // want "ObjectAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Pointer to alias - should behave same as alias + // +listType=atomic + PointerToStringAlias []*StringAlias `json:"pointerToStringAlias,omitempty"` + + // +listType=set + PointerToObjectAlias []*ObjectAlias `json:"pointerToObjectAlias,omitempty"` // want "PointerToObjectAlias with listType=set is not recommended due to Server-Side Apply compatibility issues. Consider using listType=atomic or listType=map instead" + + // Multiple pointer levels + PointerToPointerObject []*(*TestObject) `json:"pointerToPointerObject,omitempty"` // want "PointerToPointerObject should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Array defined type tests - defined types to array types should behave as array lists + // +listType=atomic + StringArrayAtomicList StringArray `json:"stringArrayAtomicList,omitempty"` + + // +listType=set + StringArraySetList StringArray `json:"stringArraySetList,omitempty"` + + // Missing listType on array defined type - should warn + StringArrayNoMarker StringArray `json:"stringArrayNoMarker,omitempty"` // want "StringArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Array type alias tests - aliases to array types should behave as array lists + // +listType=atomic + StringArrayAliasAtomicList StringArrayAlias `json:"stringArrayAliasAtomicList,omitempty"` + + // +listType=set + StringArrayAliasSetList StringArrayAlias `json:"stringArrayAliasSetList,omitempty"` + + // Missing listType on array alias - should warn + StringArrayAliasNoMarker StringArrayAlias `json:"stringArrayAliasNoMarker,omitempty"` // want "StringArrayAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Byte array tests - these should be skipped by IsByteArray condition + // Direct byte array - should be ignored (no listType required) + ByteArrayDirect []byte `json:"byteArrayDirect,omitempty"` + + // Defined type byte array - should be ignored (no listType required) + ByteArrayDefinedType ByteArray `json:"byteArrayDefinedType,omitempty"` + + // Aliased byte array - should be ignored (no listType required) + ByteArrayAliased ByteArrayAlias `json:"byteArrayAliased,omitempty"` + + // Byte array with markers - should be ignored even if markers are present + ByteArrayWithMarker []byte `json:"byteArrayWithMarker,omitempty"` // want "ByteArrayWithMarker is a byte array, which does not support the listType marker. Remove the listType marker" +} diff --git a/pkg/analysis/ssatags/testdata/src/b/b.go b/pkg/analysis/ssatags/testdata/src/b/b.go index bd2c49c1..865e8f8a 100644 --- a/pkg/analysis/ssatags/testdata/src/b/b.go +++ b/pkg/analysis/ssatags/testdata/src/b/b.go @@ -19,39 +19,42 @@ type IntAlias = int type ObjectAlias = TestObject type StringArrayAlias = []string +type ByteArray []byte +type ByteArrayAlias = []byte + type SSATagsTestSpec struct { // Valid atomic list - should pass - // +kubebuilder:listType=atomic + // +listType=atomic AtomicStringList []string `json:"atomicStringList,omitempty"` // Valid atomic object list - should pass - // +kubebuilder:listType=atomic + // +listType=atomic AtomicObjectList []TestObject `json:"atomicObjectList,omitempty"` // Valid set for primitive list - should pass (no warning for primitive lists) - // +kubebuilder:listType=set + // +listType=set SetPrimitiveList []string `json:"setPrimitiveList,omitempty"` // Set for object list - no warning when ListTypeSetUsage=Ignore - // +kubebuilder:listType=set + // +listType=set SetObjectList []TestObject `json:"setObjectList,omitempty"` // Invalid map on primitive list - should error even when ListTypeSetUsage=Ignore - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=name + // +listType=map + // +listMapKey=name MapPrimitiveList []string `json:"mapPrimitiveList,omitempty"` // want "MapPrimitiveList with listType=map can only be used for object lists, not primitive lists" // Valid map on object list with proper listMapKey - should pass - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=name + // +listType=map + // +listMapKey=name MapObjectList []TestObject `json:"mapObjectList,omitempty"` // Map on object list without listMapKey - should error even when ListTypeSetUsage=Ignore - // +kubebuilder:listType=map + // +listType=map MapObjectListNoKey []TestObject `json:"mapObjectListNoKey,omitempty"` // want "MapObjectListNoKey with listType=map must have at least one listMapKey marker" // Invalid listType value - should error regardless of config - // +kubebuilder:listType=invalid + // +listType=invalid InvalidListType []string `json:"invalidListType,omitempty"` // want "InvalidListType has invalid listType \"invalid\", must be one of: atomic, set, map" // Missing listType on primitive array - should warn @@ -67,17 +70,17 @@ type SSATagsTestSpec struct { PointerArrayNoMarker []*TestObject `json:"pointerArrayNoMarker,omitempty"` // want "PointerArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Defined type tests - defined types should behave as their underlying types - // +kubebuilder:listType=atomic + // +listType=atomic StringAtomicList []String `json:"stringAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringSetList []String `json:"stringSetList,omitempty"` // Defined type to object should behave as object list - no set warning when ListTypeSetUsage=Ignore - // +kubebuilder:listType=atomic + // +listType=atomic ObjectAtomicList []Object `json:"objectAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set ObjectSetList []Object `json:"objectSetList,omitempty"` // Missing listType on defined type to basic type - should warn @@ -87,24 +90,24 @@ type SSATagsTestSpec struct { ObjectNoMarker []Object `json:"objectNoMarker,omitempty"` // want "ObjectNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Pointer to defined type - should behave same as defined type - // +kubebuilder:listType=atomic + // +listType=atomic PointerToString []*String `json:"pointerToString,omitempty"` - // +kubebuilder:listType=set + // +listType=set PointerToObject []*Object `json:"pointerToObject,omitempty"` // Type alias tests - aliases to basic types should behave as primitive lists - // +kubebuilder:listType=atomic + // +listType=atomic StringAliasAtomicList []StringAlias `json:"stringAliasAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringAliasSetList []StringAlias `json:"stringAliasSetList,omitempty"` // Type alias to object should behave as object list - no set warning when ListTypeSetUsage=Ignore - // +kubebuilder:listType=atomic + // +listType=atomic ObjectAliasAtomicList []ObjectAlias `json:"objectAliasAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set ObjectAliasSetList []ObjectAlias `json:"objectAliasSetList,omitempty"` // Missing listType on alias to basic type - should warn @@ -114,42 +117,56 @@ type SSATagsTestSpec struct { ObjectAliasNoMarker []ObjectAlias `json:"objectAliasNoMarker,omitempty"` // want "ObjectAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Pointer to alias - should behave same as alias - // +kubebuilder:listType=atomic + // +listType=atomic PointerToStringAlias []*StringAlias `json:"pointerToStringAlias,omitempty"` - // +kubebuilder:listType=set + // +listType=set PointerToObjectAlias []*ObjectAlias `json:"pointerToObjectAlias,omitempty"` // Multiple pointer levels PointerToPointerObject []*(*TestObject) `json:"pointerToPointerObject,omitempty"` // want "PointerToPointerObject should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Array defined type tests - defined types to array types should behave as array lists - // +kubebuilder:listType=atomic + // +listType=atomic StringArrayAtomicList StringArray `json:"stringArrayAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringArraySetList StringArray `json:"stringArraySetList,omitempty"` // Missing listType on array defined type - should warn StringArrayNoMarker StringArray `json:"stringArrayNoMarker,omitempty"` // want "StringArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Array type alias tests - aliases to array types should behave as array lists - // +kubebuilder:listType=atomic + // +listType=atomic StringArrayAliasAtomicList StringArrayAlias `json:"stringArrayAliasAtomicList,omitempty"` - // +kubebuilder:listType=set + // +listType=set StringArrayAliasSetList StringArrayAlias `json:"stringArrayAliasSetList,omitempty"` // Missing listType on array alias - should warn StringArrayAliasNoMarker StringArrayAlias `json:"stringArrayAliasNoMarker,omitempty"` // want "StringArrayAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" // Valid map with correct JSON tag name - should pass - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=name + // +listType=map + // +listMapKey=name MapObjectListValid []SimpleObject `json:"mapObjectListValid,omitempty"` // Invalid map with non-existent JSON tag name - should error - // +kubebuilder:listType=map - // +kubebuilder:listMapKey=invalid_name + // +listType=map + // +listMapKey=invalid_name MapObjectListInvalidName []SimpleObject `json:"mapObjectListInvalidName,omitempty"` // want "MapObjectListInvalidName listMapKey \"invalid_name\" does not exist as a field in the struct" + + // Byte array tests - these should be skipped by IsByteArray condition + // Direct byte array - should be ignored (no listType required) + ByteArrayDirect []byte `json:"byteArrayDirect,omitempty"` + + // Defined type byte array - should be ignored (no listType required) + ByteArrayDefinedType ByteArray `json:"byteArrayDefinedType,omitempty"` + + // Aliased byte array - should be ignored (no listType required) + ByteArrayAliased ByteArrayAlias `json:"byteArrayAliased,omitempty"` + + // Byte array with markers - should be ignored even if markers are present + // +listType=atomic + ByteArrayWithMarker []byte `json:"byteArrayWithMarker,omitempty"` // want "ByteArrayWithMarker is a byte array, which does not support the listType marker. Remove the listType marker" } diff --git a/pkg/analysis/ssatags/testdata/src/b/b.go.golden b/pkg/analysis/ssatags/testdata/src/b/b.go.golden new file mode 100644 index 00000000..c2ee81af --- /dev/null +++ b/pkg/analysis/ssatags/testdata/src/b/b.go.golden @@ -0,0 +1,172 @@ +package b + +type TestObject struct { + Name string `json:"name"` + ID int `json:"id"` +} + +type SimpleObject struct { + Name string `json:"name"` +} + +type String string +type Int int +type Object TestObject +type StringArray []string + +type StringAlias = string +type IntAlias = int +type ObjectAlias = TestObject +type StringArrayAlias = []string + +type ByteArray []byte +type ByteArrayAlias = []byte + +type SSATagsTestSpec struct { + // Valid atomic list - should pass + // +listType=atomic + AtomicStringList []string `json:"atomicStringList,omitempty"` + + // Valid atomic object list - should pass + // +listType=atomic + AtomicObjectList []TestObject `json:"atomicObjectList,omitempty"` + + // Valid set for primitive list - should pass (no warning for primitive lists) + // +listType=set + SetPrimitiveList []string `json:"setPrimitiveList,omitempty"` + + // Set for object list - no warning when ListTypeSetUsage=Ignore + // +listType=set + SetObjectList []TestObject `json:"setObjectList,omitempty"` + + // Invalid map on primitive list - should error even when ListTypeSetUsage=Ignore + // +listType=map + // +listMapKey=name + MapPrimitiveList []string `json:"mapPrimitiveList,omitempty"` // want "MapPrimitiveList with listType=map can only be used for object lists, not primitive lists" + + // Valid map on object list with proper listMapKey - should pass + // +listType=map + // +listMapKey=name + MapObjectList []TestObject `json:"mapObjectList,omitempty"` + + // Map on object list without listMapKey - should error even when ListTypeSetUsage=Ignore + // +listType=map + MapObjectListNoKey []TestObject `json:"mapObjectListNoKey,omitempty"` // want "MapObjectListNoKey with listType=map must have at least one listMapKey marker" + + // Invalid listType value - should error regardless of config + // +listType=invalid + InvalidListType []string `json:"invalidListType,omitempty"` // want "InvalidListType has invalid listType \"invalid\", must be one of: atomic, set, map" + + // Missing listType on primitive array - should warn + PrimitiveArrayNoMarker []string `json:"primitiveArrayNoMarker,omitempty"` // want "PrimitiveArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Missing listType on object array - should warn + ObjectArrayNoMarker []TestObject `json:"objectArrayNoMarker,omitempty"` // want "ObjectArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Non-array field - should be ignored + SingleValue string `json:"singleValue,omitempty"` + + // Pointer array field - should behave same as non-pointer + PointerArrayNoMarker []*TestObject `json:"pointerArrayNoMarker,omitempty"` // want "PointerArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Defined type tests - defined types should behave as their underlying types + // +listType=atomic + StringAtomicList []String `json:"stringAtomicList,omitempty"` + + // +listType=set + StringSetList []String `json:"stringSetList,omitempty"` + + // Defined type to object should behave as object list - no set warning when ListTypeSetUsage=Ignore + // +listType=atomic + ObjectAtomicList []Object `json:"objectAtomicList,omitempty"` + + // +listType=set + ObjectSetList []Object `json:"objectSetList,omitempty"` + + // Missing listType on defined type to basic type - should warn + StringNoMarker []String `json:"stringNoMarker,omitempty"` // want "StringNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Missing listType on defined type to object - should warn + ObjectNoMarker []Object `json:"objectNoMarker,omitempty"` // want "ObjectNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Pointer to defined type - should behave same as defined type + // +listType=atomic + PointerToString []*String `json:"pointerToString,omitempty"` + + // +listType=set + PointerToObject []*Object `json:"pointerToObject,omitempty"` + + // Type alias tests - aliases to basic types should behave as primitive lists + // +listType=atomic + StringAliasAtomicList []StringAlias `json:"stringAliasAtomicList,omitempty"` + + // +listType=set + StringAliasSetList []StringAlias `json:"stringAliasSetList,omitempty"` + + // Type alias to object should behave as object list - no set warning when ListTypeSetUsage=Ignore + // +listType=atomic + ObjectAliasAtomicList []ObjectAlias `json:"objectAliasAtomicList,omitempty"` + + // +listType=set + ObjectAliasSetList []ObjectAlias `json:"objectAliasSetList,omitempty"` + + // Missing listType on alias to basic type - should warn + StringAliasNoMarker []StringAlias `json:"stringAliasNoMarker,omitempty"` // want "StringAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Missing listType on alias to object - should warn + ObjectAliasNoMarker []ObjectAlias `json:"objectAliasNoMarker,omitempty"` // want "ObjectAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Pointer to alias - should behave same as alias + // +listType=atomic + PointerToStringAlias []*StringAlias `json:"pointerToStringAlias,omitempty"` + + // +listType=set + PointerToObjectAlias []*ObjectAlias `json:"pointerToObjectAlias,omitempty"` + + // Multiple pointer levels + PointerToPointerObject []*(*TestObject) `json:"pointerToPointerObject,omitempty"` // want "PointerToPointerObject should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Array defined type tests - defined types to array types should behave as array lists + // +listType=atomic + StringArrayAtomicList StringArray `json:"stringArrayAtomicList,omitempty"` + + // +listType=set + StringArraySetList StringArray `json:"stringArraySetList,omitempty"` + + // Missing listType on array defined type - should warn + StringArrayNoMarker StringArray `json:"stringArrayNoMarker,omitempty"` // want "StringArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Array type alias tests - aliases to array types should behave as array lists + // +listType=atomic + StringArrayAliasAtomicList StringArrayAlias `json:"stringArrayAliasAtomicList,omitempty"` + + // +listType=set + StringArrayAliasSetList StringArrayAlias `json:"stringArrayAliasSetList,omitempty"` + + // Missing listType on array alias - should warn + StringArrayAliasNoMarker StringArrayAlias `json:"stringArrayAliasNoMarker,omitempty"` // want "StringArrayAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)" + + // Valid map with correct JSON tag name - should pass + // +listType=map + // +listMapKey=name + MapObjectListValid []SimpleObject `json:"mapObjectListValid,omitempty"` + + // Invalid map with non-existent JSON tag name - should error + // +listType=map + // +listMapKey=invalid_name + MapObjectListInvalidName []SimpleObject `json:"mapObjectListInvalidName,omitempty"` // want "MapObjectListInvalidName listMapKey \"invalid_name\" does not exist as a field in the struct" + + // Byte array tests - these should be skipped by IsByteArray condition + // Direct byte array - should be ignored (no listType required) + ByteArrayDirect []byte `json:"byteArrayDirect,omitempty"` + + // Defined type byte array - should be ignored (no listType required) + ByteArrayDefinedType ByteArray `json:"byteArrayDefinedType,omitempty"` + + // Aliased byte array - should be ignored (no listType required) + ByteArrayAliased ByteArrayAlias `json:"byteArrayAliased,omitempty"` + + // Byte array with markers - should be ignored even if markers are present + ByteArrayWithMarker []byte `json:"byteArrayWithMarker,omitempty"` // want "ByteArrayWithMarker is a byte array, which does not support the listType marker. Remove the listType marker" +} + diff --git a/pkg/analysis/utils/utils.go b/pkg/analysis/utils/utils.go index 8be461b9..dfe73907 100644 --- a/pkg/analysis/utils/utils.go +++ b/pkg/analysis/utils/utils.go @@ -182,6 +182,35 @@ func IsObjectList(pass *analysis.Pass, field *ast.Field) bool { return false } +// IsByteArray checks if the field type is a byte array or an alias to a byte array. +func IsByteArray(pass *analysis.Pass, field *ast.Field) bool { + if arrayType, ok := field.Type.(*ast.ArrayType); ok { + if ident, ok := arrayType.Elt.(*ast.Ident); ok && types.Identical(pass.TypesInfo.TypeOf(ident), types.Typ[types.Byte]) { + return true + } + } + + if ident, ok := field.Type.(*ast.Ident); ok { + typeOf := pass.TypesInfo.TypeOf(ident) + if typeOf == nil { + return false + } + + switch typeOf := typeOf.(type) { + case *types.Alias: + if sliceType, ok := typeOf.Underlying().(*types.Slice); ok { + return types.Identical(sliceType.Elem(), types.Typ[types.Byte]) + } + case *types.Named: + if sliceType, ok := typeOf.Underlying().(*types.Slice); ok { + return types.Identical(sliceType.Elem(), types.Typ[types.Byte]) + } + } + } + + return false +} + func isArrayType(t types.Type) bool { if aliasType, ok := t.(*types.Alias); ok { return isArrayType(aliasType.Underlying()) diff --git a/pkg/markers/markers.go b/pkg/markers/markers.go index 1a9336e3..0d20ad04 100644 --- a/pkg/markers/markers.go +++ b/pkg/markers/markers.go @@ -151,10 +151,10 @@ const ( KubebuilderItemsXValidationMarker = "kubebuilder:validation:items:XValidation" // KubebuilderListTypeMarker is the marker used to specify the type of list for server-side apply operations. - KubebuilderListTypeMarker = "kubebuilder:listType" + KubebuilderListTypeMarker = "listType" // KubebuilderListMapKeyMarker is the marker used to specify the key field for map-type lists. - KubebuilderListMapKeyMarker = "kubebuilder:listMapKey" + KubebuilderListMapKeyMarker = "listMapKey" ) const ( diff --git a/pkg/registration/doc.go b/pkg/registration/doc.go index a97ad4cd..041a2730 100644 --- a/pkg/registration/doc.go +++ b/pkg/registration/doc.go @@ -36,6 +36,7 @@ import ( _ "sigs.k8s.io/kube-api-linter/pkg/analysis/optionalfields" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/optionalorrequired" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/requiredfields" + _ "sigs.k8s.io/kube-api-linter/pkg/analysis/ssatags" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/statusoptional" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/statussubresource" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/uniquemarkers"