Skip to content

Commit 4e25e61

Browse files
committed
Skip statussubresource linting for Kubernetes List types
The statussubresource linter was incorrectly flagging Kubernetes List types that don't have a status field. List types follow a different pattern (TypeMeta, ListMeta, Items) and don't use the status subresource. Signed-off-by: Sascha Grunert <[email protected]>
1 parent 5fca1e8 commit 4e25e61

File tree

3 files changed

+303
-4
lines changed

3 files changed

+303
-4
lines changed

pkg/analysis/statussubresource/analyzer.go

Lines changed: 93 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"fmt"
2020
"go/ast"
2121
"go/token"
22+
"strings"
2223

2324
"golang.org/x/tools/go/analysis"
2425
"golang.org/x/tools/go/analysis/passes/inspect"
@@ -32,7 +33,11 @@ import (
3233
const (
3334
name = "statussubresource"
3435

35-
statusJSONTag = "status"
36+
statusJSONTag = "status"
37+
listMetaName = "ListMeta"
38+
typeMetaName = "TypeMeta"
39+
itemsFieldName = "Items"
40+
listNameSuffix = "List"
3641
)
3742

3843
// Analyzer is a analyzer for the statussubresource package.
@@ -88,7 +93,7 @@ func run(pass *analysis.Pass) (any, error) {
8893
return nil, nil //nolint:nilnil
8994
}
9095

91-
func checkStruct(pass *analysis.Pass, sTyp *ast.StructType, name string, structMarkers markershelper.MarkerSet, jsonTags extractjsontags.StructFieldTags) {
96+
func checkStruct(pass *analysis.Pass, sTyp *ast.StructType, name string, structMarkers markershelper.MarkerSet, jsonTags extractjsontags.StructFieldTags) { //nolint:cyclop
9297
if sTyp == nil {
9398
return
9499
}
@@ -97,6 +102,12 @@ func checkStruct(pass *analysis.Pass, sTyp *ast.StructType, name string, structM
97102
return
98103
}
99104

105+
// Skip Kubernetes List types as they follow a different pattern
106+
// and don't use the status subresource
107+
if isKubernetesListType(sTyp, name) {
108+
return
109+
}
110+
100111
hasStatusSubresourceMarker := structMarkers.Has(markers.KubebuilderStatusSubresourceMarker)
101112
hasStatusField := hasStatusField(sTyp, jsonTags)
102113

@@ -151,3 +162,83 @@ func hasStatusField(sTyp *ast.StructType, jsonTags extractjsontags.StructFieldTa
151162
func formatKubeBuilderMarkerWithValue(marker, value string) string {
152163
return fmt.Sprintf("%s:=%s", marker, value)
153164
}
165+
166+
// isKubernetesListType checks if a struct is a Kubernetes List type.
167+
// A Kubernetes List type has:
168+
// - Name ending with "List"
169+
// - Exactly 3 fields
170+
// - TypeMeta (embedded)
171+
// - ListMeta
172+
// - Items (slice type)
173+
//
174+
// Example:
175+
//
176+
// type FooList struct {
177+
// metav1.TypeMeta `json:",inline"`
178+
// metav1.ListMeta `json:"metadata,omitempty"`
179+
// Items []Foo `json:"items"`
180+
// }
181+
func isKubernetesListType(sTyp *ast.StructType, name string) bool {
182+
if !isValidListStructure(sTyp, name) {
183+
return false
184+
}
185+
186+
return hasRequiredListFields(sTyp)
187+
}
188+
189+
// isValidListStructure checks if the struct has the basic structure of a List type.
190+
func isValidListStructure(sTyp *ast.StructType, name string) bool {
191+
if !strings.HasSuffix(name, listNameSuffix) {
192+
return false
193+
}
194+
195+
if sTyp == nil || sTyp.Fields == nil || sTyp.Fields.List == nil {
196+
return false
197+
}
198+
199+
// List types must have exactly 3 fields
200+
return len(sTyp.Fields.List) == 3
201+
}
202+
203+
// hasRequiredListFields checks if the struct has the required fields for a List type.
204+
func hasRequiredListFields(sTyp *ast.StructType) bool {
205+
hasTypeMeta := false
206+
hasListMeta := false
207+
hasItems := false
208+
209+
for _, field := range sTyp.Fields.List {
210+
// Check for embedded TypeMeta (field with no name and TypeMeta type)
211+
hasTypeMeta = hasTypeMeta || (len(field.Names) == 0 && hasTypeMetaType(field))
212+
checkNamedListFields(field, &hasListMeta, &hasItems)
213+
}
214+
215+
return hasTypeMeta && hasListMeta && hasItems
216+
}
217+
218+
// checkNamedListFields checks if a field contains ListMeta or Items fields.
219+
func checkNamedListFields(field *ast.Field, hasListMeta, hasItems *bool) {
220+
for _, fieldName := range field.Names {
221+
switch fieldName.Name {
222+
case itemsFieldName:
223+
if _, ok := field.Type.(*ast.ArrayType); ok {
224+
*hasItems = true
225+
}
226+
case listMetaName:
227+
*hasListMeta = true
228+
}
229+
}
230+
}
231+
232+
// hasTypeMetaType checks if a field's type is TypeMeta.
233+
// It handles both simple identifiers (e.g., TypeMeta) and qualified identifiers (e.g., metav1.TypeMeta).
234+
func hasTypeMetaType(field *ast.Field) bool {
235+
if ident, ok := field.Type.(*ast.Ident); ok {
236+
return ident.Name == typeMetaName
237+
}
238+
239+
if sel, ok := field.Type.(*ast.SelectorExpr); ok {
240+
return sel.Sel.Name == typeMetaName
241+
}
242+
243+
return false
244+
}

pkg/analysis/statussubresource/testdata/src/a/a.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,105 @@ type FooBarBaz struct { // want "root object type \"FooBarBaz\" is marked to ena
5858
type FooBarBazSpec struct {
5959
Name string `json:"name"`
6060
}
61+
62+
// Test that List types are skipped (issue #53)
63+
// This is a Kubernetes List type with TypeMeta, ListMeta, and Items
64+
// Even with space between marker and type definition, it should be skipped
65+
66+
// +kubebuilder:object:root=true
67+
type ClusterList struct {
68+
TypeMeta `json:",inline"`
69+
ListMeta `json:"metadata,omitempty"`
70+
Items []Cluster `json:"items"`
71+
}
72+
73+
type TypeMeta struct {
74+
Kind string `json:"kind,omitempty"`
75+
APIVersion string `json:"apiVersion,omitempty"`
76+
}
77+
78+
type ListMeta struct {
79+
ResourceVersion string `json:"resourceVersion,omitempty"`
80+
}
81+
82+
type Cluster struct {
83+
Name string `json:"name"`
84+
}
85+
86+
// Test List type with qualified types (metav1.TypeMeta, metav1.ListMeta)
87+
// +kubebuilder:object:root=true
88+
type PodList struct {
89+
metav1TypeMeta `json:",inline"`
90+
metav1ListMeta `json:"metadata,omitempty"`
91+
Items []Pod `json:"items"`
92+
}
93+
94+
type metav1TypeMeta struct {
95+
Kind string `json:"kind,omitempty"`
96+
APIVersion string `json:"apiVersion,omitempty"`
97+
}
98+
99+
type metav1ListMeta struct {
100+
ResourceVersion string `json:"resourceVersion,omitempty"`
101+
}
102+
103+
type Pod struct {
104+
Name string `json:"name"`
105+
}
106+
107+
// Test that non-List types are not skipped even if they have 3 fields
108+
// This should trigger the linter because it has a status field but no marker
109+
// +kubebuilder:object:root=true
110+
type NotAList struct { // want "root object type \"NotAList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
111+
Spec NotAListSpec `json:"spec"`
112+
Status NotAListStatus `json:"status"`
113+
Extra string `json:"extra"`
114+
}
115+
116+
type NotAListSpec struct {
117+
Name string `json:"name"`
118+
}
119+
120+
type NotAListStatus struct {
121+
Ready bool `json:"ready"`
122+
}
123+
124+
// Test that types ending with "List" but not matching the pattern are not skipped
125+
// This has 4 fields instead of 3, so it should not be treated as a List type
126+
// +kubebuilder:object:root=true
127+
type BadList struct { // want "root object type \"BadList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
128+
TypeMeta `json:",inline"`
129+
ListMeta `json:"metadata,omitempty"`
130+
Items []string `json:"items"`
131+
Status BadListStatus `json:"status"`
132+
}
133+
134+
type BadListStatus struct {
135+
Count int `json:"count"`
136+
}
137+
138+
// Test that types ending with "List" but missing Items field are not skipped
139+
// +kubebuilder:object:root=true
140+
type IncompleteList struct { // want "root object type \"IncompleteList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
141+
TypeMeta `json:",inline"`
142+
ListMeta `json:"metadata,omitempty"`
143+
Status IncompleteListStatus `json:"status"`
144+
}
145+
146+
type IncompleteListStatus struct {
147+
Ready bool `json:"ready"`
148+
}
149+
150+
// Test that types ending with "List" but with non-slice Items field are not skipped
151+
// Items must be a slice type to be considered a valid List type
152+
// +kubebuilder:object:root=true
153+
type NonSliceItemsList struct { // want "root object type \"NonSliceItemsList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
154+
TypeMeta `json:",inline"`
155+
ListMeta `json:"metadata,omitempty"`
156+
Items string `json:"items"`
157+
Status NonSliceStatus `json:"status"`
158+
}
159+
160+
type NonSliceStatus struct {
161+
Count int `json:"count"`
162+
}

pkg/analysis/statussubresource/testdata/src/a/a.go.golden

Lines changed: 108 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,115 @@ type FooBarStatus struct {
5353
// +kubebuilder:object:root=true
5454
// +kubebuilder:subresource:status
5555
type FooBarBaz struct { // want "root object type \"FooBarBaz\" is marked to enable the status subresource with marker \"kubebuilder:subresource:status\" but has no status field"
56-
Spec FooBarBazSpec `json:"spec"`
56+
Spec FooBarBazSpec `json:"spec"`
5757
}
5858

5959
type FooBarBazSpec struct {
60-
Name string `json:"name"`
60+
Name string `json:"name"`
61+
}
62+
63+
// Test that List types are skipped (issue #53)
64+
// This is a Kubernetes List type with TypeMeta, ListMeta, and Items
65+
// Even with space between marker and type definition, it should be skipped
66+
67+
// +kubebuilder:object:root=true
68+
type ClusterList struct {
69+
TypeMeta `json:",inline"`
70+
ListMeta `json:"metadata,omitempty"`
71+
Items []Cluster `json:"items"`
72+
}
73+
74+
type TypeMeta struct {
75+
Kind string `json:"kind,omitempty"`
76+
APIVersion string `json:"apiVersion,omitempty"`
77+
}
78+
79+
type ListMeta struct {
80+
ResourceVersion string `json:"resourceVersion,omitempty"`
81+
}
82+
83+
type Cluster struct {
84+
Name string `json:"name"`
85+
}
86+
87+
// Test List type with qualified types (metav1.TypeMeta, metav1.ListMeta)
88+
// +kubebuilder:object:root=true
89+
type PodList struct {
90+
metav1TypeMeta `json:",inline"`
91+
metav1ListMeta `json:"metadata,omitempty"`
92+
Items []Pod `json:"items"`
93+
}
94+
95+
type metav1TypeMeta struct {
96+
Kind string `json:"kind,omitempty"`
97+
APIVersion string `json:"apiVersion,omitempty"`
98+
}
99+
100+
type metav1ListMeta struct {
101+
ResourceVersion string `json:"resourceVersion,omitempty"`
102+
}
103+
104+
type Pod struct {
105+
Name string `json:"name"`
106+
}
107+
108+
// Test that non-List types are not skipped even if they have 3 fields
109+
// This should trigger the linter because it has a status field but no marker
110+
// +kubebuilder:object:root=true
111+
// +kubebuilder:subresource:status
112+
type NotAList struct { // want "root object type \"NotAList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
113+
Spec NotAListSpec `json:"spec"`
114+
Status NotAListStatus `json:"status"`
115+
Extra string `json:"extra"`
116+
}
117+
118+
type NotAListSpec struct {
119+
Name string `json:"name"`
120+
}
121+
122+
type NotAListStatus struct {
123+
Ready bool `json:"ready"`
124+
}
125+
126+
// Test that types ending with "List" but not matching the pattern are not skipped
127+
// This has 4 fields instead of 3, so it should not be treated as a List type
128+
// +kubebuilder:object:root=true
129+
// +kubebuilder:subresource:status
130+
type BadList struct { // want "root object type \"BadList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
131+
TypeMeta `json:",inline"`
132+
ListMeta `json:"metadata,omitempty"`
133+
Items []string `json:"items"`
134+
Status BadListStatus `json:"status"`
135+
}
136+
137+
type BadListStatus struct {
138+
Count int `json:"count"`
139+
}
140+
141+
// Test that types ending with "List" but missing Items field are not skipped
142+
// +kubebuilder:object:root=true
143+
// +kubebuilder:subresource:status
144+
type IncompleteList struct { // want "root object type \"IncompleteList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
145+
TypeMeta `json:",inline"`
146+
ListMeta `json:"metadata,omitempty"`
147+
Status IncompleteListStatus `json:"status"`
148+
}
149+
150+
type IncompleteListStatus struct {
151+
Ready bool `json:"ready"`
152+
}
153+
154+
// Test that types ending with "List" but with non-slice Items field are not skipped
155+
// Items must be a slice type to be considered a valid List type
156+
// +kubebuilder:object:root=true
157+
// +kubebuilder:subresource:status
158+
type NonSliceItemsList struct { // want "root object type \"NonSliceItemsList\" has a status field but does not have the marker \"kubebuilder:subresource:status\" to enable the status subresource"
159+
TypeMeta `json:",inline"`
160+
ListMeta `json:"metadata,omitempty"`
161+
Items string `json:"items"`
162+
Status NonSliceStatus `json:"status"`
163+
}
164+
165+
type NonSliceStatus struct {
166+
Count int `json:"count"`
61167
}

0 commit comments

Comments
 (0)