Skip to content

Commit 68d9c70

Browse files
krishagarwal278krishagarwal278
committed
generics for bound checks, typos, and other reviews
1 parent 798e62a commit 68d9c70

File tree

4 files changed

+91
-69
lines changed

4 files changed

+91
-69
lines changed

‎docs/linters.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -486,11 +486,11 @@ According to Kubernetes API conventions, numeric fields should have bounds check
486486

487487
This linter ensures that:
488488
- Numeric fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers
489-
- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported
490-
- Bounds values are within the type's valid range:
491-
- int32: full int32 range (±2^31-1)
492-
- int64: JavaScript-safe range (±2^53-1) per Kubernetes API conventions
493-
- float32/float64: within their respective ranges
489+
- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported
490+
- Bounds values are validated:
491+
- int32: within int32 range (±2^31-1)
492+
- int64: within JavaScript-safe range (±2^53-1) per K8s API conventions for JSON compatibility
493+
- float32/float64: marker values are valid (within type ranges)
494494

495495
**Note:** While `+k8s:minimum` is documented in the official Kubernetes declarative validation spec, `+k8s:maximum` is not yet officially documented but is supported by this linter for forward compatibility and consistency.
496496

‎go.mod‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
github.com/google/go-cmp v0.7.0
99
github.com/onsi/ginkgo/v2 v2.23.4
1010
github.com/onsi/gomega v1.38.0
11+
golang.org/x/exp v0.0.0-20240909161429-701f63a606c0
1112
golang.org/x/tools v0.37.0
1213
k8s.io/apimachinery v0.32.3
1314
k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b

‎pkg/analysis/numericbounds/analyzer.go‎

Lines changed: 54 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import (
1919
"errors"
2020
"fmt"
2121
"go/ast"
22+
"strings"
2223

24+
"golang.org/x/exp/constraints"
2325
"golang.org/x/tools/go/analysis"
2426
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
2527
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
@@ -97,6 +99,12 @@ func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, pref
9799
// Determine if this is a slice/array field
98100
isSlice := utils.IsArrayTypeOrAlias(pass, field)
99101

102+
// For array elements, update prefix to include type information
103+
// e.g., "field Ports array element pointer" -> "field Ports array element type of int32"
104+
if isSlice {
105+
prefix = buildArrayElementPrefix(prefix, ident.Name)
106+
}
107+
100108
// Determine which markers to look for based on whether the field is a slice
101109
minMarkers, maxMarkers := getMarkerNames(isSlice)
102110

@@ -119,11 +127,11 @@ func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, pref
119127

120128
// Report if markers are missing
121129
if minMissing {
122-
pass.Reportf(field.Pos(), "%s is missing minimum bounds validation marker", prefix)
130+
pass.Reportf(field.Pos(), "%s is missing minimum bound validation marker", prefix)
123131
}
124132

125133
if maxMissing {
126-
pass.Reportf(field.Pos(), "%s is missing maximum bounds validation marker", prefix)
134+
pass.Reportf(field.Pos(), "%s is missing maximum bound validation marker", prefix)
127135
}
128136

129137
// If any markers are missing or invalid, don't continue with bounds checks
@@ -146,16 +154,30 @@ func getMarkerNames(isSlice bool) (minMarkers, maxMarkers []string) {
146154
return []string{markers.KubebuilderMinimumMarker, markers.K8sMinimumMarker}, []string{markers.KubebuilderMaximumMarker, markers.K8sMaximumMarker}
147155
}
148156

157+
// buildArrayElementPrefix updates the prefix for array elements to include type information.
158+
// Replaces TypeChecker's suffix (like "pointer" or "type X") with "type of {typeName}".
159+
// Example: "field Ports array element pointer" -> "field Ports array element type of int32".
160+
func buildArrayElementPrefix(prefix, typeName string) string {
161+
idx := strings.Index(prefix, "array element")
162+
if idx == -1 {
163+
return prefix
164+
}
165+
166+
return prefix[:idx+len("array element")] + " type of " + typeName
167+
}
168+
149169
// getMarkerNumericValue extracts the numeric value from the first instance of any of the given marker names.
150170
// Checks multiple marker names to support both kubebuilder and k8s declarative validation markers.
171+
// Precedence: Markers checked in the order provided and first valid value found is returned.
172+
// We require a valid numeric value (not just marker presence) for both minimum and maximum markers.
151173
func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []string) (float64, error) {
152174
for _, markerName := range markerNames {
153175
markerList := markerSet.Get(markerName)
154176
if len(markerList) == 0 {
155177
continue
156178
}
157179

158-
// Use the exported utils function to parse the marker value
180+
// Use the exported utils.GetMarkerNumericValue function to parse the marker value
159181
value, err := utils.GetMarkerNumericValue[float64](markerList[0])
160182
if err != nil {
161183
if errors.Is(err, errMarkerMissingValue) {
@@ -174,43 +196,42 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri
174196
// checkBoundsWithinTypeRange validates that the bounds are within the valid range for the type.
175197
// For int64, enforces JavaScript-safe bounds as per Kubernetes API conventions to ensure
176198
// compatibility with JavaScript clients.
177-
//
178-
//nolint:cyclop
179199
func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, prefix, typeName string, minimum, maximum float64) {
180200
switch typeName {
181201
case "int32":
182-
if minimum < float64(minInt32) || minimum > float64(maxInt32) {
183-
pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid int32 range [%d, %d]", prefix, minimum, minInt32, maxInt32)
184-
}
185-
186-
if maximum < float64(minInt32) || maximum > float64(maxInt32) {
187-
pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid int32 range [%d, %d]", prefix, maximum, minInt32, maxInt32)
188-
}
202+
checkBoundInRange(pass, field, prefix, minimum, minInt32, maxInt32, "minimum", "int32")
203+
checkBoundInRange(pass, field, prefix, maximum, minInt32, maxInt32, "maximum", "int32")
189204
case "int64":
190-
// Kubernetes API conventions enforce JavaScript-safe bounds for int64 (±2^53-1)
191-
// to ensure compatibility with JavaScript clients
192-
if minimum < float64(minSafeInt64) {
193-
pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the JavaScript-safe int64 range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients", prefix, minimum, int64(minSafeInt64), int64(maxSafeInt64))
194-
}
195-
196-
if maximum > float64(maxSafeInt64) {
197-
pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the JavaScript-safe int64 range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients", prefix, maximum, int64(minSafeInt64), int64(maxSafeInt64))
198-
}
205+
// K8s API conventions enforce JavaScript-safe bounds for int64 (±2^53-1)
206+
checkBoundInRange(pass, field, prefix, minimum, int64(minSafeInt64), int64(maxSafeInt64), "minimum", "JavaScript-safe int64",
207+
"Consider using a string type to avoid precision loss in JavaScript clients")
208+
checkBoundInRange(pass, field, prefix, maximum, int64(minSafeInt64), int64(maxSafeInt64), "maximum", "JavaScript-safe int64",
209+
"Consider using a string type to avoid precision loss in JavaScript clients")
199210
case "float32":
200-
if minimum < float64(minFloat32) || minimum > float64(maxFloat32) {
201-
pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid float32 range", prefix, minimum)
202-
}
203-
204-
if maximum < float64(minFloat32) || maximum > float64(maxFloat32) {
205-
pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid float32 range", prefix, maximum)
206-
}
211+
checkFloatBoundInRange(pass, field, prefix, minimum, minFloat32, maxFloat32, "minimum", "float32")
212+
checkFloatBoundInRange(pass, field, prefix, maximum, minFloat32, maxFloat32, "maximum", "float32")
207213
case "float64":
208-
if minimum < minFloat64 || minimum > maxFloat64 {
209-
pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid float64 range", prefix, minimum)
210-
}
214+
checkFloatBoundInRange(pass, field, prefix, minimum, minFloat64, maxFloat64, "minimum", "float64")
215+
checkFloatBoundInRange(pass, field, prefix, maximum, minFloat64, maxFloat64, "maximum", "float64")
216+
}
217+
}
211218

212-
if maximum < minFloat64 || maximum > maxFloat64 {
213-
pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid float64 range", prefix, maximum)
219+
// checkBoundInRange checks if an integer bound value is within the valid range.
220+
// Uses generics to avoid type conversions.
221+
func checkBoundInRange[T constraints.Integer](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string, extraMsg ...string) {
222+
if value < float64(minBound) || value > float64(maxBound) {
223+
msg := fmt.Sprintf("%s has %s bound %%v that is outside the %s range [%%d, %%d]", prefix, boundType, typeName)
224+
if len(extraMsg) > 0 {
225+
msg += ". " + extraMsg[0]
214226
}
227+
228+
pass.Reportf(field.Pos(), msg, value, minBound, maxBound)
229+
}
230+
}
231+
232+
// checkFloatBoundInRange checks if a float bound value is within the valid range.
233+
func checkFloatBoundInRange[T constraints.Float](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string) {
234+
if value < float64(minBound) || value > float64(maxBound) {
235+
pass.Reportf(field.Pos(), "%s has %s bound %v that is outside the valid %s range", prefix, boundType, value, typeName)
215236
}
216237
}

‎pkg/analysis/numericbounds/testdata/src/a/a.go‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,58 @@
11
package a
22

3-
// ValidInt32WithBounds has proper bounds validation
4-
type ValidInt32WithBounds struct {
3+
// ValidInt32WithBound has proper bounds validation
4+
type ValidInt32WithBound struct {
55
// +kubebuilder:validation:Minimum=0
66
// +kubebuilder:validation:Maximum=100
77
Count int32
88
}
99

10-
// ValidInt64WithBounds has proper bounds validation
11-
type ValidInt64WithBounds struct {
10+
// ValidInt64WithBound has proper bounds validation
11+
type ValidInt64WithBound struct {
1212
// +kubebuilder:validation:Minimum=-1000
1313
// +kubebuilder:validation:Maximum=1000
1414
Value int64
1515
}
1616

17-
// ValidInt64WithJSSafeBounds has bounds within JavaScript safe integer range
18-
type ValidInt64WithJSSafeBounds struct {
17+
// ValidInt64WithJSSafeBound has bounds within JavaScript safe integer range
18+
type ValidInt64WithJSSafeBound struct {
1919
// +kubebuilder:validation:Minimum=-9007199254740991
2020
// +kubebuilder:validation:Maximum=9007199254740991
2121
SafeValue int64
2222
}
2323

24-
// InvalidInt32NoBounds should have bounds markers
25-
type InvalidInt32NoBounds struct {
26-
NoBounds int32 // want "field NoBounds is missing minimum bounds validation marker" "field NoBounds is missing maximum bounds validation marker"
24+
// InvalidInt32NoBound should have bounds markers
25+
type InvalidInt32NoBound struct {
26+
NoBound int32 // want "field NoBound is missing minimum bound validation marker" "field NoBound is missing maximum bound validation marker"
2727
}
2828

29-
// InvalidInt64NoBounds should have bounds markers
30-
type InvalidInt64NoBounds struct {
31-
NoBounds int64 // want "field NoBounds is missing minimum bounds validation marker" "field NoBounds is missing maximum bounds validation marker"
29+
// InvalidInt64NoBound should have bounds markers
30+
type InvalidInt64NoBound struct {
31+
NoBound int64 // want "field NoBound is missing minimum bound validation marker" "field NoBound is missing maximum bound validation marker"
3232
}
3333

3434
// InvalidInt32OnlyMin should have maximum marker
3535
type InvalidInt32OnlyMin struct {
3636
// +kubebuilder:validation:Minimum=0
37-
OnlyMin int32 // want "field OnlyMin is missing maximum bounds validation marker"
37+
OnlyMin int32 // want "field OnlyMin is missing maximum bound validation marker"
3838
}
3939

4040
// InvalidInt32OnlyMax should have minimum marker
4141
type InvalidInt32OnlyMax struct {
4242
// +kubebuilder:validation:Maximum=100
43-
OnlyMax int32 // want "field OnlyMax is missing minimum bounds validation marker"
43+
OnlyMax int32 // want "field OnlyMax is missing minimum bound validation marker"
4444
}
4545

4646
// InvalidInt64OnlyMin should have maximum marker
4747
type InvalidInt64OnlyMin struct {
4848
// +kubebuilder:validation:Minimum=0
49-
OnlyMin int64 // want "field OnlyMin is missing maximum bounds validation marker"
49+
OnlyMin int64 // want "field OnlyMin is missing maximum bound validation marker"
5050
}
5151

5252
// InvalidInt64OnlyMax should have minimum marker
5353
type InvalidInt64OnlyMax struct {
5454
// +kubebuilder:validation:Maximum=100
55-
OnlyMax int64 // want "field OnlyMax is missing minimum bounds validation marker"
55+
OnlyMax int64 // want "field OnlyMax is missing minimum bound validation marker"
5656
}
5757

5858
// InvalidInt64ExceedsJSMaxBounds has maximum that exceeds JavaScript safe integer range
@@ -102,12 +102,12 @@ type ValidFloat32WithBounds struct {
102102

103103
// InvalidFloat64NoBounds should have bounds markers
104104
type InvalidFloat64NoBounds struct {
105-
Value float64 // want "field Value is missing minimum bounds validation marker" "field Value is missing maximum bounds validation marker"
105+
Value float64 // want "field Value is missing minimum bound validation marker" "field Value is missing maximum bound validation marker"
106106
}
107107

108108
// InvalidFloat32NoBounds should have bounds markers
109109
type InvalidFloat32NoBounds struct {
110-
Ratio float32 // want "field Ratio is missing minimum bounds validation marker" "field Ratio is missing maximum bounds validation marker"
110+
Ratio float32 // want "field Ratio is missing minimum bound validation marker" "field Ratio is missing maximum bound validation marker"
111111
}
112112

113113
// MixedFields has both valid and invalid fields
@@ -116,15 +116,15 @@ type MixedFields struct {
116116
// +kubebuilder:validation:Maximum=100
117117
ValidCount int32
118118

119-
InvalidCount int32 // want "field InvalidCount is missing minimum bounds validation marker" "field InvalidCount is missing maximum bounds validation marker"
119+
InvalidCount int32 // want "field InvalidCount is missing minimum bound validation marker" "field InvalidCount is missing maximum bound validation marker"
120120

121121
Name string
122122

123123
// +kubebuilder:validation:Minimum=0
124124
// +kubebuilder:validation:Maximum=1000
125125
ValidValue int64
126126

127-
InvalidValue int64 // want "field InvalidValue is missing minimum bounds validation marker" "field InvalidValue is missing maximum bounds validation marker"
127+
InvalidValue int64 // want "field InvalidValue is missing minimum bound validation marker" "field InvalidValue is missing maximum bound validation marker"
128128
}
129129

130130
// PointerFields with pointers should also be checked
@@ -133,18 +133,18 @@ type PointerFields struct {
133133
// +kubebuilder:validation:Maximum=100
134134
ValidPointerWithBounds *int32
135135

136-
InvalidPointer *int32 // want "field InvalidPointer pointer is missing minimum bounds validation marker" "field InvalidPointer pointer is missing maximum bounds validation marker"
136+
InvalidPointer *int32 // want "field InvalidPointer pointer is missing minimum bound validation marker" "field InvalidPointer pointer is missing maximum bound validation marker"
137137

138-
InvalidPointer64 *int64 // want "field InvalidPointer64 pointer is missing minimum bounds validation marker" "field InvalidPointer64 pointer is missing maximum bounds validation marker"
138+
InvalidPointer64 *int64 // want "field InvalidPointer64 pointer is missing minimum bound validation marker" "field InvalidPointer64 pointer is missing maximum bound validation marker"
139139
}
140140

141141
// SliceFields with slices should check the element type
142142
type SliceFields struct {
143143
ValidSlice []string
144144

145-
InvalidSlice []int32 // want "field InvalidSlice array element is missing minimum bounds validation marker" "field InvalidSlice array element is missing maximum bounds validation marker"
145+
InvalidSlice []int32 // want "field InvalidSlice array element type of int32 is missing minimum bound validation marker" "field InvalidSlice array element type of int32 is missing maximum bound validation marker"
146146

147-
InvalidSlice64 []int64 // want "field InvalidSlice64 array element is missing minimum bounds validation marker" "field InvalidSlice64 array element is missing maximum bounds validation marker"
147+
InvalidSlice64 []int64 // want "field InvalidSlice64 array element type of int64 is missing minimum bound validation marker" "field InvalidSlice64 array element type of int64 is missing maximum bound validation marker"
148148

149149
// +kubebuilder:validation:items:Minimum=0
150150
// +kubebuilder:validation:items:Maximum=100
@@ -179,13 +179,13 @@ type TypeAliasFields struct {
179179
// +kubebuilder:validation:Maximum=100
180180
ValidAlias Int32Alias
181181

182-
InvalidAlias Int32Alias // want "field InvalidAlias type Int32Alias is missing minimum bounds validation marker" "field InvalidAlias type Int32Alias is missing maximum bounds validation marker"
182+
InvalidAlias Int32Alias // want "field InvalidAlias type Int32Alias is missing minimum bound validation marker" "field InvalidAlias type Int32Alias is missing maximum bound validation marker"
183183

184-
InvalidAlias64 Int64Alias // want "field InvalidAlias64 type Int64Alias is missing minimum bounds validation marker" "field InvalidAlias64 type Int64Alias is missing maximum bounds validation marker"
184+
InvalidAlias64 Int64Alias // want "field InvalidAlias64 type Int64Alias is missing minimum bound validation marker" "field InvalidAlias64 type Int64Alias is missing maximum bound validation marker"
185185

186-
InvalidAliasFloat32 Float32Alias // want "field InvalidAliasFloat32 type Float32Alias is missing minimum bounds validation marker" "field InvalidAliasFloat32 type Float32Alias is missing maximum bounds validation marker"
186+
InvalidAliasFloat32 Float32Alias // want "field InvalidAliasFloat32 type Float32Alias is missing minimum bound validation marker" "field InvalidAliasFloat32 type Float32Alias is missing maximum bound validation marker"
187187

188-
InvalidAliasFloat64 Float64Alias // want "field InvalidAliasFloat64 type Float64Alias is missing minimum bounds validation marker" "field InvalidAliasFloat64 type Float64Alias is missing maximum bounds validation marker"
188+
InvalidAliasFloat64 Float64Alias // want "field InvalidAliasFloat64 type Float64Alias is missing minimum bound validation marker" "field InvalidAliasFloat64 type Float64Alias is missing maximum bound validation marker"
189189

190190
// Valid: bounds are on the type alias itself
191191
ValidBoundedAlias BoundedInt32Alias
@@ -199,10 +199,10 @@ type TypeAliasFields struct {
199199

200200
// PointerSliceFields with pointer slices should also be checked
201201
type PointerSliceFields struct {
202-
InvalidPointerSlice []*int32 // want "field InvalidPointerSlice array element pointer is missing minimum bounds validation marker" "field InvalidPointerSlice array element pointer is missing maximum bounds validation marker"
202+
InvalidPointerSlice []*int32 // want "field InvalidPointerSlice array element type of int32 is missing minimum bound validation marker" "field InvalidPointerSlice array element type of int32 is missing maximum bound validation marker"
203203
}
204204

205-
// K8sDeclarativeValidation with k8s declarative validation markers should work
205+
// K8sDeclarativeValidation with k8s declarative validation markers
206206
type K8sDeclarativeValidation struct {
207207
// +k8s:minimum=0
208208
// +k8s:maximum=100
@@ -217,7 +217,7 @@ type K8sDeclarativeValidation struct {
217217
type InvalidInt32BoundsOutOfRange struct {
218218
// +kubebuilder:validation:Minimum=-3000000000
219219
// +kubebuilder:validation:Maximum=3000000000
220-
OutOfRange int32 // want "field OutOfRange has minimum bound -3e\\+09 that is outside the valid int32 range" "field OutOfRange has maximum bound 3e\\+09 that is outside the valid int32 range"
220+
OutOfRange int32 // want "field OutOfRange has minimum bound -3e\\+09 that is outside the int32 range \\[-2147483648, 2147483647\\]" "field OutOfRange has maximum bound 3e\\+09 that is outside the int32 range \\[-2147483648, 2147483647\\]"
221221
}
222222

223223
// MixedMarkersKubebuilderAndK8s can use either kubebuilder or k8s markers

0 commit comments

Comments
 (0)