Skip to content

Commit 57e7fda

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #122 from sivchari/followup-ssatag
Followup ssatag
2 parents 029010b + 9b814cb commit 57e7fda

File tree

9 files changed

+502
-65
lines changed

9 files changed

+502
-65
lines changed

‎docs/linters.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ The linter checks for:
281281

282282
```yaml
283283
lintersConfig:
284-
ssaTags:
284+
ssatags:
285285
listTypeSetUsage: Warn | Ignore # The policy for listType=set usage on object arrays. Defaults to `Warn`.
286286
```
287287

‎pkg/analysis/ssatags/analyzer.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,31 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, markersAcce
8383
return
8484
}
8585

86+
// If the field is a byte array, we cannot use listType markers with it.
87+
if utils.IsByteArray(pass, field) {
88+
listTypeMarkers := fieldMarkers.Get(kubebuildermarkers.KubebuilderListTypeMarker)
89+
for _, marker := range listTypeMarkers {
90+
pass.Report(analysis.Diagnostic{
91+
Pos: field.Pos(),
92+
Message: fmt.Sprintf("%s is a byte array, which does not support the listType marker. Remove the listType marker", utils.FieldName(field)),
93+
SuggestedFixes: []analysis.SuggestedFix{
94+
{
95+
Message: fmt.Sprintf("Remove listType marker from %s", utils.FieldName(field)),
96+
TextEdits: []analysis.TextEdit{
97+
{
98+
Pos: marker.Pos,
99+
End: marker.End + 1,
100+
NewText: []byte(""),
101+
},
102+
},
103+
},
104+
},
105+
})
106+
}
107+
108+
return
109+
}
110+
86111
fieldName := utils.FieldName(field)
87112
listTypeMarkers := fieldMarkers.Get(kubebuildermarkers.KubebuilderListTypeMarker)
88113

‎pkg/analysis/ssatags/testdata/src/a/a.go

Lines changed: 49 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,54 +19,57 @@ type IntAlias = int
1919
type ObjectAlias = TestObject
2020
type StringArrayAlias = []string
2121

22+
type ByteArray []byte
23+
type ByteArrayAlias = []byte
24+
2225
type SSATagsTestSpec struct {
2326
// Valid atomic list - should pass
24-
// +kubebuilder:listType=atomic
27+
// +listType=atomic
2528
AtomicStringList []string `json:"atomicStringList,omitempty"`
2629

2730
// Valid atomic object list - should pass
28-
// +kubebuilder:listType=atomic
31+
// +listType=atomic
2932
AtomicObjectList []TestObject `json:"atomicObjectList,omitempty"`
3033

3134
// Valid set for primitive list - should pass (no warning for primitive lists)
32-
// +kubebuilder:listType=set
35+
// +listType=set
3336
SetPrimitiveList []string `json:"setPrimitiveList,omitempty"`
3437

3538
// Invalid set for object list - should warn about listType=set compatibility issues
36-
// +kubebuilder:listType=set
39+
// +listType=set
3740
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"
3841

3942
// Invalid map on primitive list - should error
40-
// +kubebuilder:listType=map
41-
// +kubebuilder:listMapKey=name
43+
// +listType=map
44+
// +listMapKey=name
4245
MapPrimitiveList []string `json:"mapPrimitiveList,omitempty"` // want "MapPrimitiveList with listType=map can only be used for object lists, not primitive lists"
4346

4447
// Valid map on object list with proper listMapKey - should pass
45-
// +kubebuilder:listType=map
46-
// +kubebuilder:listMapKey=name
48+
// +listType=map
49+
// +listMapKey=name
4750
MapObjectList []TestObject `json:"mapObjectList,omitempty"`
4851

4952
// Invalid map on object list without listMapKey - should error
50-
// +kubebuilder:listType=map
53+
// +listType=map
5154
MapObjectListNoKey []TestObject `json:"mapObjectListNoKey,omitempty"` // want "MapObjectListNoKey with listType=map must have at least one listMapKey marker"
5255

5356
// Invalid map on object list with non-existent listMapKey - should error
54-
// +kubebuilder:listType=map
55-
// +kubebuilder:listMapKey=nonexistent
57+
// +listType=map
58+
// +listMapKey=nonexistent
5659
MapObjectListInvalidKey []TestObject `json:"mapObjectListInvalidKey,omitempty"` // want "MapObjectListInvalidKey listMapKey \"nonexistent\" does not exist as a field in the struct"
5760

5861
// Valid map with correct JSON tag name - should pass
59-
// +kubebuilder:listType=map
60-
// +kubebuilder:listMapKey=name
62+
// +listType=map
63+
// +listMapKey=name
6164
MapObjectListValid []SimpleObject `json:"mapObjectListValid,omitempty"`
6265

6366
// Invalid map with non-existent JSON tag name - should error
64-
// +kubebuilder:listType=map
65-
// +kubebuilder:listMapKey=invalid_name
67+
// +listType=map
68+
// +listMapKey=invalid_name
6669
MapObjectListInvalidName []SimpleObject `json:"mapObjectListInvalidName,omitempty"` // want "MapObjectListInvalidName listMapKey \"invalid_name\" does not exist as a field in the struct"
6770

6871
// Invalid listType value - should error
69-
// +kubebuilder:listType=invalid
72+
// +listType=invalid
7073
InvalidListType []string `json:"invalidListType,omitempty"` // want "InvalidListType has invalid listType \"invalid\", must be one of: atomic, set, map"
7174

7275
// Missing listType on primitive array - should warn
@@ -82,17 +85,17 @@ type SSATagsTestSpec struct {
8285
PointerArrayNoMarker []*TestObject `json:"pointerArrayNoMarker,omitempty"` // want "PointerArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
8386

8487
// Defined type tests - defined types should behave as their underlying types
85-
// +kubebuilder:listType=atomic
88+
// +listType=atomic
8689
StringAtomicList []String `json:"stringAtomicList,omitempty"`
8790

88-
// +kubebuilder:listType=set
91+
// +listType=set
8992
StringSetList []String `json:"stringSetList,omitempty"`
9093

9194
// Defined type to object should behave as object list
92-
// +kubebuilder:listType=atomic
95+
// +listType=atomic
9396
ObjectAtomicList []Object `json:"objectAtomicList,omitempty"`
9497

95-
// +kubebuilder:listType=set
98+
// +listType=set
9699
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"
97100

98101
// Missing listType on defined type to basic type - should warn
@@ -102,24 +105,24 @@ type SSATagsTestSpec struct {
102105
ObjectNoMarker []Object `json:"objectNoMarker,omitempty"` // want "ObjectNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
103106

104107
// Pointer to defined type - should behave same as defined type
105-
// +kubebuilder:listType=atomic
108+
// +listType=atomic
106109
PointerToString []*String `json:"pointerToString,omitempty"`
107110

108-
// +kubebuilder:listType=set
111+
// +listType=set
109112
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"
110113

111114
// Type alias tests - aliases to basic types should behave as primitive lists
112-
// +kubebuilder:listType=atomic
115+
// +listType=atomic
113116
StringAliasAtomicList []StringAlias `json:"stringAliasAtomicList,omitempty"`
114117

115-
// +kubebuilder:listType=set
118+
// +listType=set
116119
StringAliasSetList []StringAlias `json:"stringAliasSetList,omitempty"`
117120

118121
// Type alias to object should behave as object list
119-
// +kubebuilder:listType=atomic
122+
// +listType=atomic
120123
ObjectAliasAtomicList []ObjectAlias `json:"objectAliasAtomicList,omitempty"`
121124

122-
// +kubebuilder:listType=set
125+
// +listType=set
123126
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"
124127

125128
// Missing listType on alias to basic type - should warn
@@ -129,32 +132,46 @@ type SSATagsTestSpec struct {
129132
ObjectAliasNoMarker []ObjectAlias `json:"objectAliasNoMarker,omitempty"` // want "ObjectAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
130133

131134
// Pointer to alias - should behave same as alias
132-
// +kubebuilder:listType=atomic
135+
// +listType=atomic
133136
PointerToStringAlias []*StringAlias `json:"pointerToStringAlias,omitempty"`
134137

135-
// +kubebuilder:listType=set
138+
// +listType=set
136139
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"
137140

138141
// Multiple pointer levels
139142
PointerToPointerObject []*(*TestObject) `json:"pointerToPointerObject,omitempty"` // want "PointerToPointerObject should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
140143

141144
// Array defined type tests - defined types to array types should behave as array lists
142-
// +kubebuilder:listType=atomic
145+
// +listType=atomic
143146
StringArrayAtomicList StringArray `json:"stringArrayAtomicList,omitempty"`
144147

145-
// +kubebuilder:listType=set
148+
// +listType=set
146149
StringArraySetList StringArray `json:"stringArraySetList,omitempty"`
147150

148151
// Missing listType on array defined type - should warn
149152
StringArrayNoMarker StringArray `json:"stringArrayNoMarker,omitempty"` // want "StringArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
150153

151154
// Array type alias tests - aliases to array types should behave as array lists
152-
// +kubebuilder:listType=atomic
155+
// +listType=atomic
153156
StringArrayAliasAtomicList StringArrayAlias `json:"stringArrayAliasAtomicList,omitempty"`
154157

155-
// +kubebuilder:listType=set
158+
// +listType=set
156159
StringArrayAliasSetList StringArrayAlias `json:"stringArrayAliasSetList,omitempty"`
157160

158161
// Missing listType on array alias - should warn
159162
StringArrayAliasNoMarker StringArrayAlias `json:"stringArrayAliasNoMarker,omitempty"` // want "StringArrayAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
163+
164+
// Byte array tests - these should be skipped by IsByteArray condition
165+
// Direct byte array - should be ignored (no listType required)
166+
ByteArrayDirect []byte `json:"byteArrayDirect,omitempty"`
167+
168+
// Defined type byte array - should be ignored (no listType required)
169+
ByteArrayDefinedType ByteArray `json:"byteArrayDefinedType,omitempty"`
170+
171+
// Aliased byte array - should be ignored (no listType required)
172+
ByteArrayAliased ByteArrayAlias `json:"byteArrayAliased,omitempty"`
173+
174+
// Byte array with markers - should be ignored even if markers are present
175+
// +listType=atomic
176+
ByteArrayWithMarker []byte `json:"byteArrayWithMarker,omitempty"` // want "ByteArrayWithMarker is a byte array, which does not support the listType marker. Remove the listType marker"
160177
}

‎pkg/analysis/ssatags/testdata/src/a/a.go.golden

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
package a
2+
3+
type TestObject struct {
4+
Name string `json:"name"`
5+
ID int `json:"id"`
6+
}
7+
8+
type SimpleObject struct {
9+
Name string `json:"name"`
10+
}
11+
12+
type String string
13+
type Int int
14+
type Object TestObject
15+
type StringArray []string
16+
17+
type StringAlias = string
18+
type IntAlias = int
19+
type ObjectAlias = TestObject
20+
type StringArrayAlias = []string
21+
22+
type ByteArray []byte
23+
type ByteArrayAlias = []byte
24+
25+
type SSATagsTestSpec struct {
26+
// Valid atomic list - should pass
27+
// +listType=atomic
28+
AtomicStringList []string `json:"atomicStringList,omitempty"`
29+
30+
// Valid atomic object list - should pass
31+
// +listType=atomic
32+
AtomicObjectList []TestObject `json:"atomicObjectList,omitempty"`
33+
34+
// Valid set for primitive list - should pass (no warning for primitive lists)
35+
// +listType=set
36+
SetPrimitiveList []string `json:"setPrimitiveList,omitempty"`
37+
38+
// Invalid set for object list - should warn about listType=set compatibility issues
39+
// +listType=set
40+
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"
41+
42+
// Invalid map on primitive list - should error
43+
// +listType=map
44+
// +listMapKey=name
45+
MapPrimitiveList []string `json:"mapPrimitiveList,omitempty"` // want "MapPrimitiveList with listType=map can only be used for object lists, not primitive lists"
46+
47+
// Valid map on object list with proper listMapKey - should pass
48+
// +listType=map
49+
// +listMapKey=name
50+
MapObjectList []TestObject `json:"mapObjectList,omitempty"`
51+
52+
// Invalid map on object list without listMapKey - should error
53+
// +listType=map
54+
MapObjectListNoKey []TestObject `json:"mapObjectListNoKey,omitempty"` // want "MapObjectListNoKey with listType=map must have at least one listMapKey marker"
55+
56+
// Invalid map on object list with non-existent listMapKey - should error
57+
// +listType=map
58+
// +listMapKey=nonexistent
59+
MapObjectListInvalidKey []TestObject `json:"mapObjectListInvalidKey,omitempty"` // want "MapObjectListInvalidKey listMapKey \"nonexistent\" does not exist as a field in the struct"
60+
61+
// Valid map with correct JSON tag name - should pass
62+
// +listType=map
63+
// +listMapKey=name
64+
MapObjectListValid []SimpleObject `json:"mapObjectListValid,omitempty"`
65+
66+
// Invalid map with non-existent JSON tag name - should error
67+
// +listType=map
68+
// +listMapKey=invalid_name
69+
MapObjectListInvalidName []SimpleObject `json:"mapObjectListInvalidName,omitempty"` // want "MapObjectListInvalidName listMapKey \"invalid_name\" does not exist as a field in the struct"
70+
71+
// Invalid listType value - should error
72+
// +listType=invalid
73+
InvalidListType []string `json:"invalidListType,omitempty"` // want "InvalidListType has invalid listType \"invalid\", must be one of: atomic, set, map"
74+
75+
// Missing listType on primitive array - should warn
76+
PrimitiveArrayNoMarker []string `json:"primitiveArrayNoMarker,omitempty"` // want "PrimitiveArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
77+
78+
// Missing listType on object array - should warn
79+
ObjectArrayNoMarker []TestObject `json:"objectArrayNoMarker,omitempty"` // want "ObjectArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
80+
81+
// Non-array field - should be ignored
82+
SingleValue string `json:"singleValue,omitempty"`
83+
84+
// Pointer array field - should behave same as non-pointer
85+
PointerArrayNoMarker []*TestObject `json:"pointerArrayNoMarker,omitempty"` // want "PointerArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
86+
87+
// Defined type tests - defined types should behave as their underlying types
88+
// +listType=atomic
89+
StringAtomicList []String `json:"stringAtomicList,omitempty"`
90+
91+
// +listType=set
92+
StringSetList []String `json:"stringSetList,omitempty"`
93+
94+
// Defined type to object should behave as object list
95+
// +listType=atomic
96+
ObjectAtomicList []Object `json:"objectAtomicList,omitempty"`
97+
98+
// +listType=set
99+
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"
100+
101+
// Missing listType on defined type to basic type - should warn
102+
StringNoMarker []String `json:"stringNoMarker,omitempty"` // want "StringNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
103+
104+
// Missing listType on defined type to object - should warn
105+
ObjectNoMarker []Object `json:"objectNoMarker,omitempty"` // want "ObjectNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
106+
107+
// Pointer to defined type - should behave same as defined type
108+
// +listType=atomic
109+
PointerToString []*String `json:"pointerToString,omitempty"`
110+
111+
// +listType=set
112+
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"
113+
114+
// Type alias tests - aliases to basic types should behave as primitive lists
115+
// +listType=atomic
116+
StringAliasAtomicList []StringAlias `json:"stringAliasAtomicList,omitempty"`
117+
118+
// +listType=set
119+
StringAliasSetList []StringAlias `json:"stringAliasSetList,omitempty"`
120+
121+
// Type alias to object should behave as object list
122+
// +listType=atomic
123+
ObjectAliasAtomicList []ObjectAlias `json:"objectAliasAtomicList,omitempty"`
124+
125+
// +listType=set
126+
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"
127+
128+
// Missing listType on alias to basic type - should warn
129+
StringAliasNoMarker []StringAlias `json:"stringAliasNoMarker,omitempty"` // want "StringAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
130+
131+
// Missing listType on alias to object - should warn
132+
ObjectAliasNoMarker []ObjectAlias `json:"objectAliasNoMarker,omitempty"` // want "ObjectAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
133+
134+
// Pointer to alias - should behave same as alias
135+
// +listType=atomic
136+
PointerToStringAlias []*StringAlias `json:"pointerToStringAlias,omitempty"`
137+
138+
// +listType=set
139+
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"
140+
141+
// Multiple pointer levels
142+
PointerToPointerObject []*(*TestObject) `json:"pointerToPointerObject,omitempty"` // want "PointerToPointerObject should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
143+
144+
// Array defined type tests - defined types to array types should behave as array lists
145+
// +listType=atomic
146+
StringArrayAtomicList StringArray `json:"stringArrayAtomicList,omitempty"`
147+
148+
// +listType=set
149+
StringArraySetList StringArray `json:"stringArraySetList,omitempty"`
150+
151+
// Missing listType on array defined type - should warn
152+
StringArrayNoMarker StringArray `json:"stringArrayNoMarker,omitempty"` // want "StringArrayNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
153+
154+
// Array type alias tests - aliases to array types should behave as array lists
155+
// +listType=atomic
156+
StringArrayAliasAtomicList StringArrayAlias `json:"stringArrayAliasAtomicList,omitempty"`
157+
158+
// +listType=set
159+
StringArrayAliasSetList StringArrayAlias `json:"stringArrayAliasSetList,omitempty"`
160+
161+
// Missing listType on array alias - should warn
162+
StringArrayAliasNoMarker StringArrayAlias `json:"stringArrayAliasNoMarker,omitempty"` // want "StringArrayAliasNoMarker should have a listType marker for proper Server-Side Apply behavior \\(atomic, set, or map\\)"
163+
164+
// Byte array tests - these should be skipped by IsByteArray condition
165+
// Direct byte array - should be ignored (no listType required)
166+
ByteArrayDirect []byte `json:"byteArrayDirect,omitempty"`
167+
168+
// Defined type byte array - should be ignored (no listType required)
169+
ByteArrayDefinedType ByteArray `json:"byteArrayDefinedType,omitempty"`
170+
171+
// Aliased byte array - should be ignored (no listType required)
172+
ByteArrayAliased ByteArrayAlias `json:"byteArrayAliased,omitempty"`
173+
174+
// Byte array with markers - should be ignored even if markers are present
175+
ByteArrayWithMarker []byte `json:"byteArrayWithMarker,omitempty"` // want "ByteArrayWithMarker is a byte array, which does not support the listType marker. Remove the listType marker"
176+
}

0 commit comments

Comments
 (0)