Skip to content

Commit 18c3c08

Browse files
committed
Add linter for maxlength and maxitems
1 parent 48704a5 commit 18c3c08

File tree

8 files changed

+379
-0
lines changed

8 files changed

+379
-0
lines changed

README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,20 @@ lintersConfig:
201201
jsonTagRegex: "^[a-z][a-z0-9]*(?:[A-Z][a-z0-9]*)*$" # Provide a custom regex, which the json tag must match.
202202
```
203203

204+
## MaxLength
205+
206+
The `maxlength` linter checks that string and array fields in the API are bounded by a maximum length.
207+
208+
For strings, this means they have a `+kubebuilder:validation:MaxLength` marker.
209+
210+
For arrays, this means they have a `+kubebuilder:validation:MaxItems` marker.
211+
212+
For arrays of strings, the array element should also have a `+kubebuilder:validation:MaxLength` marker if the array element is a type alias,
213+
or `+kubebuilder:validation:items:MaxLenth` if the array is an element of the built-in string type.
214+
215+
Adding maximum lengths to strings and arrays not only ensures that the API is not abused (used to store overly large data, reduces DDOS etc.),
216+
but also allows CEL validation cost estimations to be kept within reasonable bounds.
217+
204218
## NoBools
205219

206220
The `nobools` linter checks that fields in the API types do not contain a `bool` type.

pkg/analysis/maxlength/analyzer.go

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
package maxlength
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"go/ast"
7+
8+
"github.com/JoelSpeed/kal/pkg/analysis/helpers/markers"
9+
"golang.org/x/tools/go/analysis"
10+
"golang.org/x/tools/go/analysis/passes/inspect"
11+
"golang.org/x/tools/go/ast/inspector"
12+
)
13+
14+
const (
15+
name = "maxlength"
16+
17+
kubebuilderMaxLength = "kubebuilder:validation:MaxLength"
18+
kubebuilderEnum = "kubebuilder:validation:Enum"
19+
kubebuilderFormat = "kubebuilder:validation:Format"
20+
21+
kubebuilderItemsMaxLength = "kubebuilder:validation:items:MaxLength"
22+
kubebuilderItemsEnum = "kubebuilder:validation:items:Enum"
23+
kubebuilderItemsFormat = "kubebuilder:validation:items:Format"
24+
25+
kubebuilderMaxItems = "kubebuilder:validation:MaxItems"
26+
)
27+
28+
var (
29+
errCouldNotGetInspector = errors.New("could not get inspector")
30+
errCouldNotGetMarkers = errors.New("could not get markers")
31+
)
32+
33+
// Analyzer is the analyzer for the maxlength package.
34+
// It checks that strings and arrays have maximum lengths and maximum items respectively.
35+
var Analyzer = &analysis.Analyzer{
36+
Name: name,
37+
Doc: "Checks that all strings formatted fields are marked with a maximum length, and that arrays are marked with max items.",
38+
Run: run,
39+
Requires: []*analysis.Analyzer{inspect.Analyzer, markers.Analyzer},
40+
}
41+
42+
func run(pass *analysis.Pass) (interface{}, error) {
43+
inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
44+
if !ok {
45+
return nil, errCouldNotGetInspector
46+
}
47+
48+
markersAccess, ok := pass.ResultOf[markers.Analyzer].(markers.Markers)
49+
if !ok {
50+
return nil, errCouldNotGetMarkers
51+
}
52+
53+
// Filter to structs so that we can iterate over the fields in a struct.
54+
nodeFilter := []ast.Node{
55+
(*ast.StructType)(nil),
56+
}
57+
58+
inspect.Preorder(nodeFilter, func(n ast.Node) {
59+
sTyp, ok := n.(*ast.StructType)
60+
if !ok {
61+
return
62+
}
63+
64+
if sTyp.Fields == nil {
65+
return
66+
}
67+
68+
for _, field := range sTyp.Fields.List {
69+
checkField(pass, field, markersAccess)
70+
}
71+
})
72+
73+
return nil, nil //nolint:nilnil
74+
}
75+
76+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers) {
77+
if len(field.Names) == 0 || field.Names[0] == nil {
78+
return
79+
}
80+
81+
fieldName := field.Names[0].Name
82+
prefix := fmt.Sprintf("field %s", fieldName)
83+
84+
checkTypeExpr(pass, field.Type, field, nil, markersAccess, prefix, kubebuilderMaxLength, needsStringMaxLength)
85+
}
86+
87+
func checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
88+
if ident.Obj == nil { // Built-in type
89+
if ident.Name == "string" {
90+
markers := getCombinedMarkers(markersAccess, node, aliases)
91+
92+
if needsMaxLength(markers) {
93+
pass.Reportf(node.Pos(), "%s must have a maximum length, add %s marker", prefix, marker)
94+
}
95+
}
96+
97+
return
98+
}
99+
100+
tSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)
101+
if !ok {
102+
return
103+
}
104+
105+
checkTypeSpec(pass, tSpec, node, append(aliases, tSpec), markersAccess, fmt.Sprintf("%s type", prefix), marker, needsMaxLength)
106+
}
107+
108+
func checkTypeSpec(pass *analysis.Pass, tSpec *ast.TypeSpec, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
109+
if tSpec.Name == nil {
110+
return
111+
}
112+
113+
typeName := tSpec.Name.Name
114+
prefix = fmt.Sprintf("%s %s", prefix, typeName)
115+
116+
checkTypeExpr(pass, tSpec.Type, node, aliases, markersAccess, prefix, marker, needsMaxLength)
117+
}
118+
119+
func checkTypeExpr(pass *analysis.Pass, typeExpr ast.Expr, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
120+
switch typ := typeExpr.(type) {
121+
case *ast.Ident:
122+
checkIdent(pass, typ, node, aliases, markersAccess, prefix, marker, needsMaxLength)
123+
case *ast.StarExpr:
124+
checkTypeExpr(pass, typ.X, node, aliases, markersAccess, prefix, marker, needsMaxLength)
125+
case *ast.ArrayType:
126+
checkArrayType(pass, typ, node, aliases, markersAccess, prefix)
127+
}
128+
}
129+
130+
func checkArrayType(pass *analysis.Pass, arrayType *ast.ArrayType, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix string) {
131+
if arrayType.Elt != nil {
132+
if ident, ok := arrayType.Elt.(*ast.Ident); ok {
133+
checkArrayElementIdent(pass, ident, node, aliases, markersAccess, fmt.Sprintf("%s array element", prefix))
134+
}
135+
}
136+
137+
markers := getCombinedMarkers(markersAccess, node, aliases)
138+
139+
if !markers.Has(kubebuilderMaxItems) {
140+
pass.Reportf(node.Pos(), "%s must have a maximum items, add %s marker", prefix, kubebuilderMaxItems)
141+
}
142+
}
143+
144+
func checkArrayElementIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix string) {
145+
if ident.Obj == nil { // Built-in type
146+
if ident.Name == "string" {
147+
markers := getCombinedMarkers(markersAccess, node, aliases)
148+
149+
if needsItemsMaxLength(markers) {
150+
pass.Reportf(node.Pos(), "%s must have a maximum length, add %s marker", prefix, kubebuilderItemsMaxLength)
151+
}
152+
}
153+
154+
return
155+
}
156+
157+
tSpec, ok := ident.Obj.Decl.(*ast.TypeSpec)
158+
if !ok {
159+
return
160+
}
161+
162+
// If the array element wasn't directly a string, allow a string alias to be used
163+
// with either the items style markers or the on alias style markers.
164+
checkTypeSpec(pass, tSpec, node, append(aliases, tSpec), markersAccess, fmt.Sprintf("%s type", prefix), kubebuilderMaxLength, func(ms markers.MarkerSet) bool {
165+
return needsStringMaxLength(ms) && needsItemsMaxLength(ms)
166+
})
167+
}
168+
169+
func getCombinedMarkers(markersAccess markers.Markers, node ast.Node, aliases []*ast.TypeSpec) markers.MarkerSet {
170+
base := markers.NewMarkerSet(getMarkers(markersAccess, node).UnsortedList()...)
171+
172+
for _, a := range aliases {
173+
base.Insert(getMarkers(markersAccess, a).UnsortedList()...)
174+
}
175+
176+
return base
177+
}
178+
179+
func getMarkers(markersAccess markers.Markers, node ast.Node) markers.MarkerSet {
180+
switch t := node.(type) {
181+
case *ast.Field:
182+
return markersAccess.FieldMarkers(t)
183+
case *ast.TypeSpec:
184+
return markersAccess.TypeMarkers(t)
185+
}
186+
187+
return nil
188+
}
189+
190+
// needsMaxLength returns true if the field needs a maximum length.
191+
// Fields do not need a maximum length if they are already marked with a maximum length,
192+
// or if they are an enum, or if they are a date, date-time or duration.
193+
func needsStringMaxLength(markerSet markers.MarkerSet) bool {
194+
switch {
195+
case markerSet.Has(kubebuilderMaxLength),
196+
markerSet.Has(kubebuilderEnum),
197+
markerSet.HasWithValue(kubebuilderFormatWithValue("date")),
198+
markerSet.HasWithValue(kubebuilderFormatWithValue("date-time")),
199+
markerSet.HasWithValue(kubebuilderFormatWithValue("duration")):
200+
return false
201+
}
202+
203+
return true
204+
}
205+
206+
func needsItemsMaxLength(markerSet markers.MarkerSet) bool {
207+
switch {
208+
case markerSet.Has(kubebuilderItemsMaxLength),
209+
markerSet.Has(kubebuilderItemsEnum),
210+
markerSet.HasWithValue(kubebuilderItemsFormatWithValue("date")),
211+
markerSet.HasWithValue(kubebuilderItemsFormatWithValue("date-time")),
212+
markerSet.HasWithValue(kubebuilderItemsFormatWithValue("duration")):
213+
return false
214+
}
215+
216+
return true
217+
}
218+
219+
func kubebuilderFormatWithValue(value string) string {
220+
return fmt.Sprintf("%s:=%s", kubebuilderFormat, value)
221+
}
222+
223+
func kubebuilderItemsFormatWithValue(value string) string {
224+
return fmt.Sprintf("%s:=%s", kubebuilderItemsFormat, value)
225+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package maxlength_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/JoelSpeed/kal/pkg/analysis/maxlength"
7+
"golang.org/x/tools/go/analysis/analysistest"
8+
)
9+
10+
func TestMaxLength(t *testing.T) {
11+
testdata := analysistest.TestData()
12+
13+
analysistest.Run(t, testdata, maxlength.Analyzer, "a")
14+
}

pkg/analysis/maxlength/doc.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/*
2+
maxlength is an analyzer that checks that all string fields have a maximum length, and that all array fields have a maximum number of items.
3+
4+
String fields that are not otherwise bound in length, through being an enum or formatted in a certain way, should have a maximum length.
5+
This ensures that CEL validations on the field are not overly costly in terms of time and memory.
6+
7+
Array fields should have a maximum number of items.
8+
This ensures that any CEL validations on the field are not overly costly in terms of time and memory.
9+
Where arrays are used to represent a list of structures, CEL rules may exist within the array.
10+
Limiting the array length ensures the cardinality of the rules within the array is not unbounded.
11+
12+
For strings, the maximum length can be set using the `kubebuilder:validation:MaxLength` tag.
13+
For arrays, the maximum number of items can be set using the `kubebuilder:validation:MaxItems` tag.
14+
15+
For arrays of strings, the maximum length of each string can be set using the `kubebuilder:validation:items:MaxLength` tag,
16+
on the array field itself.
17+
Or, if the array uses a string type alias, the `kubebuilder:validation:MaxLength` tag can be used on the alias.
18+
*/
19+
package maxlength
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package maxlength
2+
3+
import (
4+
"github.com/JoelSpeed/kal/pkg/config"
5+
"golang.org/x/tools/go/analysis"
6+
)
7+
8+
// Initializer returns the AnalyzerInitializer for this
9+
// Analyzer so that it can be added to the registry.
10+
func Initializer() initializer {
11+
return initializer{}
12+
}
13+
14+
// intializer implements the AnalyzerInitializer interface.
15+
type initializer struct{}
16+
17+
// Name returns the name of the Analyzer.
18+
func (initializer) Name() string {
19+
return name
20+
}
21+
22+
// Init returns the intialized Analyzer.
23+
func (initializer) Init(cfg config.LintersConfig) (*analysis.Analyzer, error) {
24+
return Analyzer, nil
25+
}
26+
27+
// Default determines whether this Analyzer is on by default, or not.
28+
func (initializer) Default() bool {
29+
return false // For now, CRD only, and so not on by default.
30+
}
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
package a
2+
3+
type MaxLength struct {
4+
// +kubebuilder:validation:MaxLength:=256
5+
StringWithMaxLength string
6+
7+
// +kubebuilder:validation:MaxLength:=128
8+
StringAliasWithMaxLengthOnField StringAlias
9+
10+
StringAliasWithMaxLengthOnAlias StringAliasWithMaxLength
11+
12+
StringWithoutMaxLength string // want "field StringWithoutMaxLength must have a maximum length, add kubebuilder:validation:MaxLength marker"
13+
14+
StringAliasWithoutMaxLength StringAlias // want "field StringAliasWithoutMaxLength type StringAlias must have a maximum length, add kubebuilder:validation:MaxLength marker"
15+
16+
// +kubebuilder:validation:Enum:="A";"B";"C"
17+
EnumString string
18+
19+
EnumStringAlias EnumStringAlias
20+
21+
// +kubebuilder:validation:Format:=duration
22+
DurationFormat string
23+
24+
// +kubebuilder:validation:Format:=date-time
25+
DateTimeFormat string
26+
27+
// +kubebuilder:validation:Format:=date
28+
DateFormat string
29+
30+
// +kubebuilder:validation:MaxItems:=256
31+
ArrayWithMaxItems []int
32+
33+
ArrayWithoutMaxItems []int // want "field ArrayWithoutMaxItems must have a maximum items, add kubebuilder:validation:MaxItems"
34+
35+
// +kubebuilder:validation:MaxItems:=128
36+
StringArrayWithMaxItemsWithoutMaxElementLength []string // want "field StringArrayWithMaxItemsWithoutMaxElementLength array element must have a maximum length, add kubebuilder:validation:items:MaxLength"
37+
38+
StringArrayWithoutMaxItemsWithoutMaxElementLength []string // want "field StringArrayWithoutMaxItemsWithoutMaxElementLength must have a maximum items, add kubebuilder:validation:MaxItems" "field StringArrayWithoutMaxItemsWithoutMaxElementLength array element must have a maximum length, add kubebuilder:validation:items:MaxLength"
39+
40+
// +kubebuilder:validation:MaxItems:=64
41+
// +kubebuilder:validation:items:MaxLength:=64
42+
StringArrayWithMaxItemsAndMaxElementLength []string
43+
44+
// +kubebuilder:validation:items:MaxLength:=512
45+
StringArrayWithoutMaxItemsWithMaxElementLength []string // want "field StringArrayWithoutMaxItemsWithMaxElementLength must have a maximum items, add kubebuilder:validation:MaxItems marker"
46+
47+
// +kubebuilder:validation:MaxItems:=128
48+
StringAliasArrayWithMaxItemsWithoutMaxElementLength []StringAlias // want "field StringAliasArrayWithMaxItemsWithoutMaxElementLength array element type StringAlias must have a maximum length, add kubebuilder:validation:MaxLength marker"
49+
50+
StringAliasArrayWithoutMaxItemsWithoutMaxElementLength []StringAlias // want "field StringAliasArrayWithoutMaxItemsWithoutMaxElementLength must have a maximum items, add kubebuilder:validation:MaxItems" "field StringAliasArrayWithoutMaxItemsWithoutMaxElementLength array element type StringAlias must have a maximum length, add kubebuilder:validation:MaxLength"
51+
52+
// +kubebuilder:validation:MaxItems:=64
53+
// +kubebuilder:validation:items:MaxLength:=64
54+
StringAliasArrayWithMaxItemsAndMaxElementLength []StringAlias
55+
56+
// +kubebuilder:validation:items:MaxLength:=512
57+
StringAliasArrayWithoutMaxItemsWithMaxElementLength []StringAlias // want "field StringAliasArrayWithoutMaxItemsWithMaxElementLength must have a maximum items, add kubebuilder:validation:MaxItems"
58+
59+
// +kubebuilder:validation:MaxItems:=64
60+
StringAliasArrayWithMaxItemsAndMaxElementLengthOnAlias []StringAliasWithMaxLength
61+
62+
StringAliasArrayWithoutMaxItemsWithMaxElementLengthOnAlias []StringAliasWithMaxLength // want "field StringAliasArrayWithoutMaxItemsWithMaxElementLengthOnAlias must have a maximum items, add kubebuilder:validation:MaxItems"
63+
}
64+
65+
// StringAlias is a string without a MaxLength.
66+
type StringAlias string
67+
68+
// StringAliasWithMaxLength is a string with a MaxLength.
69+
// +kubebuilder:validation:MaxLength:=512
70+
type StringAliasWithMaxLength string
71+
72+
// EnumStringAlias is a string alias that is an enum.
73+
// +kubebuilder:validation:Enum:="A";"B";"C"
74+
type EnumStringAlias string

0 commit comments

Comments
 (0)