Skip to content

Commit 3db7fa8

Browse files
committed
Add statusoptional linter
This commit introduces a new linter, `statusoptional`, which checks that all first-level children fields within a status struct are marked as optional. It adds functionality to automatically suggest fixes for fields that are missing the appropriate markers.
1 parent 2c83ed3 commit 3db7fa8

File tree

13 files changed

+572
-2
lines changed

13 files changed

+572
-2
lines changed

README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,20 @@ It will suggest to remove the pointer from the field, and update the `json` tag
279279
If you prefer not to suggest fixes for pointers in required fields, you can change the `pointerPolicy` to `Warn`.
280280
The linter will then only suggest to remove the `omitempty` value from the `json` tag.
281281

282+
## StatusOptional
283+
284+
The `statusoptional` linter checks that all first-level children fields within a status struct are marked as optional.
285+
286+
This is important because status fields should be optional to allow for partial updates and backward compatibility.
287+
The linter ensures that all direct child fields of any status struct have either the `// +optional` or
288+
`// +kubebuilder:validation:Optional` marker.
289+
290+
### Fixes
291+
292+
The `statusoptional` linter can automatically fix fields in status structs that are not marked as optional.
293+
294+
It will suggest adding the `// +optional` marker to any status field that is missing it.
295+
282296
## StatusSubresource
283297

284298
The `statussubresource` linter checks that the status subresource is configured correctly for

debug.test89409974

9.23 MB
Binary file not shown.

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
55
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
66
github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI=
77
github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8=
8-
github.com/golangci/golangci-lint/v2 v2.0.0 h1:RQWk8VCuMQv9bBDy3x23yds2yf9aRZ86C9MWGIdNRuU=
9-
github.com/golangci/golangci-lint/v2 v2.0.0/go.mod h1:ptNNMeGBQrbves0Qq38xvfdJg18PzxmT+7KRCOpm6i8=
108
github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c=
119
github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc=
1210
github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=

pkg/analysis/registry.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"sigs.k8s.io/kube-api-linter/pkg/analysis/nophase"
3131
"sigs.k8s.io/kube-api-linter/pkg/analysis/optionalorrequired"
3232
"sigs.k8s.io/kube-api-linter/pkg/analysis/requiredfields"
33+
"sigs.k8s.io/kube-api-linter/pkg/analysis/statusoptional"
3334
"sigs.k8s.io/kube-api-linter/pkg/analysis/statussubresource"
3435
"sigs.k8s.io/kube-api-linter/pkg/config"
3536

@@ -83,6 +84,7 @@ func NewRegistry() Registry {
8384
nophase.Initializer(),
8485
optionalorrequired.Initializer(),
8586
requiredfields.Initializer(),
87+
statusoptional.Initializer(),
8688
statussubresource.Initializer(),
8789
},
8890
}

pkg/analysis/registry_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ var _ = Describe("Registry", func() {
4040
"nophase",
4141
"optionalorrequired",
4242
"requiredfields",
43+
"statusoptional",
4344
))
4445
})
4546
})
@@ -59,6 +60,7 @@ var _ = Describe("Registry", func() {
5960
"nophase",
6061
"optionalorrequired",
6162
"requiredfields",
63+
"statusoptional",
6264
"statussubresource",
6365
))
6466
})
Lines changed: 260 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,260 @@
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 statusoptional
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
22+
"golang.org/x/tools/go/analysis"
23+
24+
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
28+
)
29+
30+
const (
31+
name = "statusoptional"
32+
33+
statusJSONTag = "status"
34+
35+
// OptionalMarker is the marker that indicates that a field is optional.
36+
OptionalMarker = "optional"
37+
38+
// KubebuilderOptionalMarker is the marker that indicates that a field is optional in kubebuilder.
39+
KubebuilderOptionalMarker = "kubebuilder:validation:Optional"
40+
41+
// RequiredMarker is the marker that indicates that a field is required.
42+
RequiredMarker = "required"
43+
44+
// KubebuilderRequiredMarker is the marker that indicates that a field is required in kubebuilder.
45+
KubebuilderRequiredMarker = "kubebuilder:validation:Required"
46+
)
47+
48+
func init() {
49+
markers.DefaultRegistry().Register(
50+
OptionalMarker,
51+
KubebuilderOptionalMarker,
52+
RequiredMarker,
53+
KubebuilderRequiredMarker,
54+
)
55+
}
56+
57+
type analyzer struct {
58+
preferredOptionalMarker string
59+
}
60+
61+
// newAnalyzer creates a new analyzer.
62+
func newAnalyzer(preferredOptionalMarker string) *analysis.Analyzer {
63+
if preferredOptionalMarker == "" {
64+
preferredOptionalMarker = OptionalMarker
65+
}
66+
67+
a := &analyzer{
68+
preferredOptionalMarker: preferredOptionalMarker,
69+
}
70+
71+
return &analysis.Analyzer{
72+
Name: name,
73+
Doc: "Checks that all first-level children fields within status struct are marked as optional",
74+
Run: a.run,
75+
Requires: []*analysis.Analyzer{inspector.Analyzer, extractjsontags.Analyzer},
76+
}
77+
}
78+
79+
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
80+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
81+
if !ok {
82+
return nil, kalerrors.ErrCouldNotGetInspector
83+
}
84+
85+
jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
86+
if !ok {
87+
return nil, kalerrors.ErrCouldNotGetJSONTags
88+
}
89+
90+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
91+
if jsonTagInfo.Name != statusJSONTag {
92+
return
93+
}
94+
95+
statusStructType := getStructFromField(field)
96+
if statusStructType == nil {
97+
return
98+
}
99+
100+
a.checkStatusStruct(pass, statusStructType, markersAccess, jsonTags)
101+
})
102+
103+
return nil, nil //nolint:nilnil
104+
}
105+
106+
func (a *analyzer) checkStatusStruct(pass *analysis.Pass, statusType *ast.StructType, markersAccess markers.Markers, jsonTags extractjsontags.StructFieldTags) {
107+
if statusType.Fields == nil || statusType.Fields.List == nil {
108+
return
109+
}
110+
111+
// Check each child field of the status struct
112+
for _, childField := range statusType.Fields.List {
113+
fieldName := a.getFieldName(childField)
114+
if fieldName == "" {
115+
continue
116+
}
117+
118+
jsonTagInfo := jsonTags.FieldTags(childField)
119+
if jsonTagInfo.Ignored {
120+
continue
121+
}
122+
123+
if !jsonTagInfo.Inline {
124+
// Check if the field has the required optional markers
125+
a.checkFieldOptionalMarker(pass, childField, fieldName, markersAccess)
126+
} else {
127+
// Inline fields should not have names
128+
if len(childField.Names) > 0 {
129+
continue
130+
}
131+
132+
inLineStruct := getStructFromField(childField)
133+
if inLineStruct == nil {
134+
continue
135+
}
136+
137+
// Check embedded structs recursively
138+
a.checkStatusStruct(pass, inLineStruct, markersAccess, jsonTags)
139+
}
140+
}
141+
}
142+
143+
// getFieldName extracts the field name from an AST field.
144+
// If the field is an embedded field, it returns the name of the embedded struct.
145+
// If the field is a pointer to an embedded struct, it returns the
146+
// name of the embedded struct preceded by a "*".
147+
// TODO: move it to utils package.
148+
func (a *analyzer) getFieldName(field *ast.Field) string {
149+
if len(field.Names) > 0 {
150+
return field.Names[0].Name
151+
}
152+
153+
switch typ := field.Type.(type) {
154+
case *ast.Ident:
155+
return typ.Name
156+
case *ast.StarExpr:
157+
if ident, ok := typ.X.(*ast.Ident); ok {
158+
return fmt.Sprintf("*%s", ident.Name)
159+
}
160+
}
161+
162+
return ""
163+
}
164+
165+
// checkFieldOptionalMarker checks if a field has the required optional markers.
166+
// If the field has a required marker, it will be replaced with the preferred optional marker.
167+
// If the field does not have an optional marker, it will be added.
168+
func (a *analyzer) checkFieldOptionalMarker(pass *analysis.Pass, field *ast.Field, fieldName string, markersAccess markers.Markers) {
169+
fieldMarkers := markersAccess.FieldMarkers(field)
170+
// Check if the field has either the optional or kubebuilder:validation:Optional marker
171+
hasOptionalMarker := fieldMarkers.Has(OptionalMarker) || fieldMarkers.Has(KubebuilderOptionalMarker)
172+
if hasOptionalMarker {
173+
return
174+
}
175+
176+
// Check if the field has required markers that need to be replaced
177+
hasRequiredMarker := fieldMarkers.Has(RequiredMarker) || fieldMarkers.Has(KubebuilderRequiredMarker)
178+
if hasRequiredMarker {
179+
// Collect all required markers that need to be removed
180+
var textEdits []analysis.TextEdit
181+
182+
// Handle standard required markers
183+
if fieldMarkers.Has(RequiredMarker) {
184+
for _, marker := range fieldMarkers[RequiredMarker] {
185+
textEdits = append(textEdits, analysis.TextEdit{
186+
Pos: marker.Pos,
187+
End: marker.End,
188+
// Remove the marker completely (will add preferred one separately)
189+
NewText: []byte(""),
190+
})
191+
}
192+
}
193+
194+
// Handle kubebuilder required markers
195+
if fieldMarkers.Has(KubebuilderRequiredMarker) {
196+
for _, marker := range fieldMarkers[KubebuilderRequiredMarker] {
197+
textEdits = append(textEdits, analysis.TextEdit{
198+
Pos: marker.Pos,
199+
End: marker.End,
200+
// Remove the marker completely (will add preferred one separately)
201+
NewText: []byte(""),
202+
})
203+
}
204+
}
205+
206+
// Add the preferred optional marker at the beginning of the field
207+
textEdits = append(textEdits, analysis.TextEdit{
208+
Pos: field.Pos(),
209+
NewText: []byte(fmt.Sprintf("// +%s\n", a.preferredOptionalMarker)),
210+
})
211+
212+
pass.Report(analysis.Diagnostic{
213+
Pos: field.Pos(),
214+
Message: fmt.Sprintf("status field %q must be marked as optional, not required", fieldName),
215+
SuggestedFixes: []analysis.SuggestedFix{{
216+
Message: fmt.Sprintf("replace required marker(s) with %s", a.preferredOptionalMarker),
217+
TextEdits: textEdits,
218+
}},
219+
})
220+
} else {
221+
// Report the error and suggest a fix to add the optional marker
222+
pass.Report(analysis.Diagnostic{
223+
Pos: field.Pos(),
224+
Message: fmt.Sprintf("status field %q must be marked as optional", fieldName),
225+
SuggestedFixes: []analysis.SuggestedFix{
226+
{
227+
Message: "add the optional marker",
228+
TextEdits: []analysis.TextEdit{
229+
{
230+
// Position at the beginning of the line of the field
231+
Pos: field.Pos(),
232+
// Insert the marker before the field
233+
NewText: []byte(fmt.Sprintf("// +%s\n", a.preferredOptionalMarker)),
234+
},
235+
},
236+
},
237+
},
238+
})
239+
}
240+
}
241+
242+
// getStructFromField extracts the struct type from an AST Field.
243+
func getStructFromField(field *ast.Field) *ast.StructType {
244+
ident, ok := field.Type.(*ast.Ident)
245+
if !ok {
246+
return nil
247+
}
248+
249+
typeSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)
250+
if !ok {
251+
return nil
252+
}
253+
254+
structType, ok := typeSpec.Type.(*ast.StructType)
255+
if !ok {
256+
return nil
257+
}
258+
259+
return structType
260+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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 statusoptional
17+
18+
import (
19+
"testing"
20+
21+
"golang.org/x/tools/go/analysis/analysistest"
22+
)
23+
24+
func Test(t *testing.T) {
25+
testdata := analysistest.TestData()
26+
analysistest.RunWithSuggestedFixes(t, testdata, newAnalyzer("optional"), "a")
27+
}

pkg/analysis/statusoptional/doc.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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+
/*
18+
The statusoptional linter ensures that all first-level children fields within a status struct
19+
are marked as optional.
20+
21+
This is important because status fields should be optional to allow for partial updates
22+
and backward compatibility.
23+
24+
This linter checks:
25+
1. For structs with a JSON tag of "status"
26+
2. All direct child fields of the status struct
27+
3. Ensures each child field has an optional marker
28+
29+
The linter will report an issue if any field in the status struct is not marked as optional
30+
and will suggest a fix to add the appropriate optional marker.
31+
*/
32+
package statusoptional

0 commit comments

Comments
 (0)