Skip to content

Commit c532ff6

Browse files
committed
linters: add forbiddenmarkers and nonullable linters
Signed-off-by: Bryce Palmer <bpalmer@redhat.com>
1 parent 1b29e82 commit c532ff6

File tree

17 files changed

+1404
-1
lines changed

17 files changed

+1404
-1
lines changed

docs/linters.md

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
- [Conditions](#conditions) - Checks that `Conditions` fields are correctly formatted
44
- [CommentStart](#commentstart) - Ensures comments start with the serialized form of the type
55
- [DuplicateMarkers](#duplicatemarkers) - Checks for exact duplicates of markers
6+
- [ForbiddenMarkers](#forbiddenmarkers) - Checks that no forbidden markers are present on types/fields.
67
- [Integers](#integers) - Validates usage of supported integer types
78
- [JSONTags](#jsontags) - Ensures proper JSON tag formatting
89
- [MaxLength](#maxlength) - Checks for maximum length constraints on strings and arrays
910
- [NoBools](#nobools) - Prevents usage of boolean types
1011
- [NoFloats](#nofloats) - Prevents usage of floating-point types
1112
- [Nomaps](#nomaps) - Restricts usage of map types
13+
- [NoNullable](#nonullable) - Prevents usage of the nullable marker
1214
- [Nophase](#nophase) - Prevents usage of 'Phase' fields
1315
- [Notimestamp](#notimestamp) - Prevents usage of 'TimeStamp' fields
1416
- [OptionalFields](#optionalfields) - Validates optional field conventions
@@ -98,6 +100,134 @@ will not.
98100
The `duplicatemarkers` linter can automatically fix all markers that are exact match to another markers.
99101
If there are duplicates across fields and their underlying type, the marker on the type will be preferred and the marker on the field will be removed.
100102

103+
## ForbiddenMarkers
104+
105+
The `forbiddenmarkers` linter ensures that types and fields do not contain any markers that are forbidden.
106+
107+
By default, `forbiddenmarkers` is not enabled.
108+
109+
### Configuation
110+
111+
It can be configured with a list of marker identifiers and optionally their attributes and values that are forbidden.
112+
113+
Some examples configurations explained:
114+
115+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker`
116+
117+
```yaml
118+
linterConfig:
119+
forbiddenmarkers:
120+
markers:
121+
- identifier: "forbidden:marker"
122+
```
123+
124+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` containing the attribute `fruit`
125+
126+
```yaml
127+
linterConfig:
128+
forbiddenmarkers:
129+
markers:
130+
- identifier: "forbidden:marker"
131+
ruleSets:
132+
- attributes:
133+
- name: "fruit"
134+
```
135+
136+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` containing the `fruit` AND `color` attributes
137+
138+
```yaml
139+
linterConfig:
140+
forbiddenmarkers:
141+
markers:
142+
- identifier: "forbidden:marker"
143+
ruleSets:
144+
- attributes:
145+
- name: "fruit"
146+
- name: "color"
147+
```
148+
149+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` where the `fruit` attribute is set to one of `apple`, `banana`, or `orange`
150+
151+
```yaml
152+
linterConfig:
153+
forbiddenmarkers:
154+
markers:
155+
- identifier: "forbidden:marker"
156+
ruleSets:
157+
- attributes:
158+
- name: "fruit"
159+
values:
160+
- "apple"
161+
- "banana"
162+
- "orange"
163+
```
164+
165+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` where the `fruit` attribute is set to one of `apple`, `banana`, or `orange` AND the `color` attribute is set to one of `red`, `green`, or `blue`
166+
167+
```yaml
168+
linterConfig:
169+
forbiddenmarkers:
170+
markers:
171+
- identifier: "forbidden:marker"
172+
ruleSets:
173+
- attributes:
174+
- name: "fruit"
175+
values:
176+
- "apple"
177+
- "banana"
178+
- "orange"
179+
- name: "color"
180+
values:
181+
- "red"
182+
- "blue"
183+
- "green"
184+
```
185+
186+
**Scenario:** forbid all instances of the marker with the identifier `forbidden:marker` where:
187+
188+
- The `fruit` attribute is set to `apple` and the `color` attribute is set to one of `blue` or `orange` (allow any other color apple)
189+
190+
_OR_
191+
192+
- The `fruit` attribute is set to `orange` and the `color` attribute is set to one of `blue`, `red`, or `green` (allow any other color orange)
193+
194+
_OR_
195+
196+
- The `fruit` attribute is set to `banana` (no bananas allowed)
197+
198+
```yaml
199+
linterConfig:
200+
forbiddenmarkers:
201+
markers:
202+
- identifier: "forbidden:marker"
203+
ruleSets:
204+
- attributes:
205+
- name: "fruit"
206+
values:
207+
- "apple"
208+
- name: "color"
209+
values:
210+
- "blue"
211+
- "orange"
212+
- attributes:
213+
- name: "fruit"
214+
values:
215+
- "orange"
216+
- name: "color"
217+
values:
218+
- "blue"
219+
- "red"
220+
- "green"
221+
- attributes:
222+
- name: "fruit"
223+
values:
224+
- "banana"
225+
```
226+
227+
### Fixes
228+
229+
Fixes are suggested to remove all markers that are forbidden.
230+
101231
## Integers
102232

103233
The `integers` linter checks for usage of unsupported integer types.
@@ -161,6 +291,14 @@ lintersConfig:
161291
policy: Enforce | AllowStringToStringMaps | Ignore # Determines how the linter should handle maps of simple types. Defaults to AllowStringToStringMaps.
162292
```
163293

294+
## NoNullable
295+
296+
The `nonullable` linter ensures that types and fields do not have the `nullable` marker.
297+
298+
### Fixes
299+
300+
Fixes are suggested to remove the `nullable` marker.
301+
164302
## Notimestamp
165303

166304
The `notimestamp` linter checks that the fields in the API are not named with the word 'Timestamp'.
@@ -334,10 +472,12 @@ linter will suggest adding the comment `// +kubebuilder:subresource:status` abov
334472

335473
The `uniquemarkers` linter ensures that types and fields do not contain more than a single definition of a marker that should only be present once.
336474

337-
Because this linter has no way of determining which marker definition was intended it does not suggest any fixes
475+
Because this linter has no way of determining which marker definition was intended it does not suggest any fixes
338476

339477
### Configuration
478+
340479
It can configured to include a set of custom markers in the analysis by setting:
480+
341481
```yaml
342482
lintersConfig:
343483
uniquemarkers:
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
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 forbiddenmarkers
17+
18+
import (
19+
"fmt"
20+
"go/ast"
21+
"slices"
22+
23+
"golang.org/x/tools/go/analysis"
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+
"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
29+
)
30+
31+
const name = "forbiddenmarkers"
32+
33+
type analyzer struct {
34+
forbiddenMarkers []Marker
35+
}
36+
37+
// NewAnalyzer creates a new analysis.Analyzer for the forbiddenmarkers
38+
// linter based on the provided config.ForbiddenMarkersConfig.
39+
func newAnalyzer(cfg *Config) *analysis.Analyzer {
40+
a := &analyzer{
41+
forbiddenMarkers: cfg.Markers,
42+
}
43+
44+
analyzer := &analysis.Analyzer{
45+
Name: name,
46+
Doc: "Check that no forbidden markers are present on types and fields.",
47+
Run: a.run,
48+
Requires: []*analysis.Analyzer{inspector.Analyzer},
49+
}
50+
51+
for _, marker := range a.forbiddenMarkers {
52+
markers.DefaultRegistry().Register(marker.Identifier)
53+
}
54+
55+
return analyzer
56+
}
57+
58+
func (a *analyzer) run(pass *analysis.Pass) (any, error) {
59+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
60+
if !ok {
61+
return nil, kalerrors.ErrCouldNotGetInspector
62+
}
63+
64+
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
65+
checkField(pass, field, markersAccess, a.forbiddenMarkers)
66+
})
67+
68+
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
69+
checkType(pass, typeSpec, markersAccess, a.forbiddenMarkers)
70+
})
71+
72+
return nil, nil //nolint:nilnil
73+
}
74+
75+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, forbiddenMarkers []Marker) {
76+
if field == nil || len(field.Names) == 0 {
77+
return
78+
}
79+
80+
markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
81+
check(markers, forbiddenMarkers, reportField(pass, field))
82+
}
83+
84+
func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers, forbiddenMarkers []Marker) {
85+
if typeSpec == nil {
86+
return
87+
}
88+
89+
markers := markersAccess.TypeMarkers(typeSpec)
90+
check(markers, forbiddenMarkers, reportType(pass, typeSpec))
91+
}
92+
93+
func check(markerSet markers.MarkerSet, forbiddenMarkers []Marker, reportFunc func(marker markers.Marker)) {
94+
for _, marker := range forbiddenMarkers {
95+
marks := markerSet.Get(marker.Identifier)
96+
for _, mark := range marks {
97+
if len(marker.RuleSets) == 0 {
98+
reportFunc(mark)
99+
continue
100+
}
101+
102+
for _, ruleSet := range marker.RuleSets {
103+
if markerMatchesAttributeRules(mark, ruleSet.Attributes...) {
104+
reportFunc(mark)
105+
}
106+
}
107+
}
108+
}
109+
}
110+
111+
func markerMatchesAttributeRules(marker markers.Marker, attrRules ...MarkerAttribute) bool {
112+
for _, attrRule := range attrRules {
113+
// if the marker doesn't contain the attribute for a specified rule it fails the AND
114+
// operation.
115+
val, ok := marker.Expressions[attrRule.Name]
116+
if !ok {
117+
return false
118+
}
119+
120+
// if the value doesn't match one of the forbidden ones, this marker is not forbidden
121+
if len(attrRule.Values) > 0 && !slices.Contains(attrRule.Values, val) {
122+
return false
123+
}
124+
}
125+
126+
return true
127+
}
128+
129+
func reportField(pass *analysis.Pass, field *ast.Field) func(marker markers.Marker) {
130+
return func(marker markers.Marker) {
131+
pass.Report(analysis.Diagnostic{
132+
Pos: field.Pos(),
133+
Message: fmt.Sprintf("field %s has forbidden marker %q", field.Names[0].Name, marker.String()),
134+
SuggestedFixes: []analysis.SuggestedFix{
135+
{
136+
Message: fmt.Sprintf("remove forbidden marker %q", marker.String()),
137+
TextEdits: []analysis.TextEdit{
138+
{
139+
Pos: marker.Pos,
140+
End: marker.End,
141+
},
142+
},
143+
},
144+
},
145+
})
146+
}
147+
}
148+
149+
func reportType(pass *analysis.Pass, typeSpec *ast.TypeSpec) func(marker markers.Marker) {
150+
return func(marker markers.Marker) {
151+
pass.Report(analysis.Diagnostic{
152+
Pos: typeSpec.Pos(),
153+
Message: fmt.Sprintf("type %s has forbidden marker %q", typeSpec.Name, marker.String()),
154+
SuggestedFixes: []analysis.SuggestedFix{
155+
{
156+
Message: fmt.Sprintf("remove forbidden marker %q", marker.String()),
157+
TextEdits: []analysis.TextEdit{
158+
{
159+
Pos: marker.Pos,
160+
End: marker.End,
161+
},
162+
},
163+
},
164+
},
165+
})
166+
}
167+
}

0 commit comments

Comments
 (0)