Skip to content

Commit b5dcc00

Browse files
authored
Merge pull request #162 from saschagrunert/arrayofstruct
Add arrayofstruct linter
2 parents 1dea13b + db1e14a commit b5dcc00

File tree

7 files changed

+458
-0
lines changed

7 files changed

+458
-0
lines changed

docs/linters.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Linters
22

3+
- [ArrayOfStruct](#arrayofstruct) - Ensures arrays of structs have at least one required field
34
- [Conditions](#conditions) - Checks that `Conditions` fields are correctly formatted
45
- [CommentStart](#commentstart) - Ensures comments start with the serialized form of the type
56
- [ConflictingMarkers](#conflictingmarkers) - Detects mutually exclusive markers on the same field
@@ -24,6 +25,27 @@
2425
- [StatusSubresource](#statussubresource) - Validates status subresource configuration
2526
- [UniqueMarkers](#uniquemarkers) - Ensures unique marker definitions
2627

28+
## ArrayOfStruct
29+
30+
The `arrayofstruct` linter checks that arrays containing structs have at least one required field to prevent ambiguous YAML representations.
31+
32+
When an array contains structs where all fields are optional or unmarked, it becomes difficult to distinguish between different array elements in YAML configurations. This can lead to ambiguous or confusing API definitions.
33+
34+
The linter enforces that at least one field in the struct must be marked with one of the following required markers:
35+
- `// +required`
36+
- `// +kubebuilder:validation:Required`
37+
- `// +k8s:required`
38+
39+
This applies to:
40+
- Direct array fields containing struct types
41+
- Arrays of pointers to struct types
42+
- Arrays of inline struct definitions
43+
- Arrays using type aliases that resolve to struct types
44+
45+
The linter does not check:
46+
- Arrays of primitive types (strings, integers, etc.)
47+
- Arrays of types from external packages (cannot inspect their fields)
48+
2749
## Conditions
2850

2951
The `conditions` linter checks that `Conditions` fields in the API types are correctly formatted.
Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package arrayofstruct
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
22+
"golang.org/x/tools/go/analysis"
23+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
24+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
26+
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
28+
"sigs.k8s.io/kube-api-linter/pkg/markers"
29+
)
30+
31+
const name = "arrayofstruct"
32+
33+
// Analyzer is the analyzer for the arrayofstruct package.
34+
// It checks that arrays containing structs have at least one required field.
35+
var Analyzer = &analysis.Analyzer{
36+
Name: name,
37+
Doc: "Arrays containing structs must have at least one required field to prevent ambiguous YAML representations",
38+
Run: run,
39+
Requires: []*analysis.Analyzer{inspector.Analyzer},
40+
}
41+
42+
func run(pass *analysis.Pass) (any, error) {
43+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
44+
if !ok {
45+
return nil, kalerrors.ErrCouldNotGetInspector
46+
}
47+
48+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) {
49+
checkField(pass, field, markersAccess)
50+
})
51+
52+
return nil, nil //nolint:nilnil
53+
}
54+
55+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) {
56+
// Get the element type of the array
57+
elementType := getArrayElementType(pass, field)
58+
if elementType == nil {
59+
return
60+
}
61+
62+
// Check if this is an array of objects (not primitives)
63+
if !isObjectType(pass, elementType) {
64+
return
65+
}
66+
67+
// Handle pointer types (e.g., []*MyStruct)
68+
if starExpr, ok := elementType.(*ast.StarExpr); ok {
69+
elementType = starExpr.X
70+
}
71+
72+
// Get the struct type definition
73+
structType := getStructType(pass, elementType)
74+
if structType == nil {
75+
return
76+
}
77+
78+
// Check if at least one field in the struct has a required marker
79+
if hasRequiredField(structType, markersAccess) {
80+
return
81+
}
82+
83+
reportArrayOfStructIssue(pass, field)
84+
}
85+
86+
// getArrayElementType extracts the element type from an array field.
87+
// Returns nil if the field is not an array.
88+
func getArrayElementType(pass *analysis.Pass, field *ast.Field) ast.Expr {
89+
switch fieldType := field.Type.(type) {
90+
case *ast.ArrayType:
91+
return fieldType.Elt
92+
case *ast.Ident:
93+
// For type aliases to arrays, we need to resolve the underlying type
94+
typeSpec, ok := utils.LookupTypeSpec(pass, fieldType)
95+
if !ok {
96+
return nil
97+
}
98+
99+
arrayType, ok := typeSpec.Type.(*ast.ArrayType)
100+
if !ok {
101+
return nil
102+
}
103+
104+
return arrayType.Elt
105+
default:
106+
return nil
107+
}
108+
}
109+
110+
// reportArrayOfStructIssue reports a diagnostic for an array of structs without required fields.
111+
func reportArrayOfStructIssue(pass *analysis.Pass, field *ast.Field) {
112+
fieldName := utils.FieldName(field)
113+
structName := utils.GetStructNameForField(pass, field)
114+
115+
var prefix string
116+
if structName != "" {
117+
prefix = fmt.Sprintf("%s.%s", structName, fieldName)
118+
} else {
119+
prefix = fieldName
120+
}
121+
122+
message := fmt.Sprintf("%s is an array of structs, but the struct has no required fields. At least one field should be marked as %s to prevent ambiguous YAML configurations", prefix, markers.RequiredMarker)
123+
124+
pass.Report(analysis.Diagnostic{
125+
Pos: field.Pos(),
126+
Message: message,
127+
})
128+
}
129+
130+
// isObjectType checks if the given expression represents an object type (not a primitive).
131+
func isObjectType(pass *analysis.Pass, expr ast.Expr) bool {
132+
switch et := expr.(type) {
133+
case *ast.StructType:
134+
// Inline struct definition
135+
return true
136+
case *ast.Ident:
137+
// Check if it's a basic type
138+
if utils.IsBasicType(pass, et) {
139+
return false
140+
}
141+
// It's a named type, check if it's a struct
142+
typeSpec, ok := utils.LookupTypeSpec(pass, et)
143+
if !ok {
144+
// Might be from another package, assume it's an object
145+
return true
146+
}
147+
// Recursively check the underlying type
148+
return isObjectType(pass, typeSpec.Type)
149+
case *ast.StarExpr:
150+
// Pointer to something, check what it points to
151+
return isObjectType(pass, et.X)
152+
case *ast.SelectorExpr:
153+
// Type from another package, we can't inspect it
154+
// Return false to be conservative and skip checking these fields
155+
return false
156+
default:
157+
return false
158+
}
159+
}
160+
161+
// getStructType resolves the given expression to a struct type,
162+
// following type aliases and handling inline structs.
163+
func getStructType(pass *analysis.Pass, expr ast.Expr) *ast.StructType {
164+
switch et := expr.(type) {
165+
case *ast.StructType:
166+
// Inline struct definition
167+
return et
168+
case *ast.Ident:
169+
// Named struct type or type alias
170+
typeSpec, ok := utils.LookupTypeSpec(pass, et)
171+
if !ok {
172+
// This might be a type from another package or a built-in type
173+
// In this case, we can't inspect it, so we return nil
174+
return nil
175+
}
176+
177+
// Check if the typeSpec.Type is a struct
178+
if structType, ok := typeSpec.Type.(*ast.StructType); ok {
179+
return structType
180+
}
181+
182+
// If not a struct, it might be an alias to another type
183+
// Recursively resolve it
184+
return getStructType(pass, typeSpec.Type)
185+
case *ast.SelectorExpr:
186+
// Type from another package, we can't inspect it
187+
return nil
188+
default:
189+
return nil
190+
}
191+
}
192+
193+
// hasRequiredField checks if at least one field in the struct has a required marker.
194+
func hasRequiredField(structType *ast.StructType, markersAccess markershelper.Markers) bool {
195+
if structType.Fields == nil {
196+
return false
197+
}
198+
199+
for _, field := range structType.Fields.List {
200+
fieldMarkers := markersAccess.FieldMarkers(field)
201+
202+
// Check for any of the required markers
203+
if fieldMarkers.Has(markers.RequiredMarker) ||
204+
fieldMarkers.Has(markers.KubebuilderRequiredMarker) ||
205+
fieldMarkers.Has(markers.K8sRequiredMarker) {
206+
return true
207+
}
208+
}
209+
210+
return false
211+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package arrayofstruct_test
17+
18+
import (
19+
"testing"
20+
21+
"golang.org/x/tools/go/analysis/analysistest"
22+
"sigs.k8s.io/kube-api-linter/pkg/analysis/arrayofstruct"
23+
)
24+
25+
func Test(t *testing.T) {
26+
testdata := analysistest.TestData()
27+
analysistest.Run(t, testdata, arrayofstruct.Analyzer, "a")
28+
}

pkg/analysis/arrayofstruct/doc.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package arrayofstruct checks that arrays containing structs have at least
18+
// one required field in the struct definition.
19+
//
20+
// This prevents ambiguous YAML representations where the absence of required
21+
// fields can lead to configurations that are unclear or have dramatically
22+
// different meanings. For example, in NetworkPolicy, the difference between
23+
// "match all packets to 10.0.0.1, port 80" vs "match all packets to 10.0.0.1
24+
// on any port, and also match all packets to port 80 on any IP" can be subtle
25+
// but critical.
26+
package arrayofstruct
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
package arrayofstruct
17+
18+
import (
19+
"sigs.k8s.io/kube-api-linter/pkg/analysis/initializer"
20+
"sigs.k8s.io/kube-api-linter/pkg/analysis/registry"
21+
)
22+
23+
func init() {
24+
registry.DefaultRegistry().RegisterLinter(Initializer())
25+
}
26+
27+
// Initializer returns the AnalyzerInitializer for this
28+
// Analyzer so that it can be added to the registry.
29+
func Initializer() initializer.AnalyzerInitializer {
30+
return initializer.NewInitializer(
31+
name,
32+
Analyzer,
33+
// Enable by default as this check prevents ambiguous YAML configurations
34+
true,
35+
)
36+
}

0 commit comments

Comments
 (0)