Skip to content

Commit 9487576

Browse files
added float64 and but removed unused int64. Updated Docs, initializer, and used utils
1 parent 1ac8dbc commit 9487576

File tree

8 files changed

+134
-280
lines changed

8 files changed

+134
-280
lines changed

docs/linters.md

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -485,28 +485,15 @@ According to Kubernetes API conventions, numeric fields should have bounds check
485485

486486
This linter ensures that:
487487
- Numeric fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers
488-
- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported
489-
- Bounds values are within the type's valid range (e.g., int32 bounds must fit in int32)
490-
- `int64` fields with bounds outside the JavaScript safe integer range (±2^53-1) are flagged
488+
- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported
489+
- Bounds values are within the type's valid range:
490+
- int32: full int32 range (±2^31-1)
491+
- int64: JavaScript-safe range (±2^53-1) per Kubernetes API conventions
492+
- float32/float64: within their respective ranges
491493

492494
**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.
493495

494-
### Configuration
495-
496-
This linter is **not enabled by default** as it is primarily focused on CRD validation with kubebuilder markers. It can be enabled in your configuration.
497-
498-
To enable in `.golangci.yml`:
499-
```yaml
500-
linters-settings:
501-
custom:
502-
kubeapilinter:
503-
settings:
504-
linters:
505-
enable:
506-
- numericbounds
507-
```
508-
509-
See the [package documentation](https://pkg.go.dev/sigs.k8s.io/kube-api-linter/pkg/analysis/numericbounds) for detailed examples and usage.
496+
This linter is **not enabled by default** as it is primarily focused on CRD validation with kubebuilder markers.
510497

511498
## NoBools
512499

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go 1.24.0
55
require (
66
github.com/golangci/golangci-lint/v2 v2.5.0
77
github.com/golangci/plugin-module-register v0.1.2
8+
github.com/google/go-cmp v0.7.0
89
github.com/onsi/ginkgo/v2 v2.23.4
910
github.com/onsi/gomega v1.38.0
1011
golang.org/x/tools v0.37.0
@@ -100,7 +101,6 @@ require (
100101
github.com/golangci/revgrep v0.8.0 // indirect
101102
github.com/golangci/swaggoswag v0.0.0-20250504205917-77f2aca3143e // indirect
102103
github.com/golangci/unconvert v0.0.0-20250410112200-a129a6e6413e // indirect
103-
github.com/google/go-cmp v0.7.0 // indirect
104104
github.com/google/pprof v0.0.0-20250607225305-033d6d78b36a // indirect
105105
github.com/gordonklaus/ineffassign v0.2.0 // indirect
106106
github.com/gostaticanalysis/analysisutil v0.7.1 // indirect

pkg/analysis/numericbounds/analyzer.go

Lines changed: 73 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"errors"
2020
"fmt"
2121
"go/ast"
22-
"strconv"
2322

2423
"golang.org/x/tools/go/analysis"
2524
kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
@@ -34,18 +33,20 @@ const (
3433
name = "numericbounds"
3534
)
3635

37-
// Type bounds for validation
36+
// Type bounds for validation.
3837
const (
39-
maxInt32 = 2147483647 // 2^31 - 1
40-
minInt32 = -2147483648 // -2^31
41-
maxFloat32 = 3.40282346638528859811704183484516925440e+38 // max float32
42-
minFloat32 = -3.40282346638528859811704183484516925440e+38 // min float32
38+
maxInt32 = 2147483647 // 2^31 - 1
39+
minInt32 = -2147483648 // -2^31
40+
maxFloat32 = 3.40282346638528859811704183484516925440e+38 // max float32
41+
minFloat32 = -3.40282346638528859811704183484516925440e+38 // min float32
42+
maxFloat64 = 1.797693134862315708145274237317043567981e+308 // max float64
43+
minFloat64 = -1.797693134862315708145274237317043567981e+308 // min float64
4344
)
4445

45-
// JavaScript safe integer bounds (2^53 - 1 and -(2^53 - 1))
46+
// JavaScript safe integer bounds for int64 (±2^53-1).
47+
// Per Kubernetes API conventions, int64 fields should use bounds within this range
48+
// to ensure compatibility with JavaScript clients.
4649
const (
47-
maxSafeInt32 = 2147483647 // 2^31 - 1 (fits in JS Number)
48-
minSafeInt32 = -2147483648 // -2^31 (fits in JS Number)
4950
maxSafeInt64 = 9007199254740991 // 2^53 - 1 (max safe integer in JS)
5051
minSafeInt64 = -9007199254740991 // -(2^53 - 1) (min safe integer in JS)
5152
)
@@ -67,41 +68,35 @@ func run(pass *analysis.Pass) (any, error) {
6768
return nil, kalerrors.ErrCouldNotGetInspector
6869
}
6970

70-
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) {
71-
checkField(pass, field, markersAccess)
71+
inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, _ string) {
72+
// Create TypeChecker with closure capturing markersAccess
73+
typeChecker := utils.NewTypeChecker(func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
74+
checkNumericType(pass, ident, node, prefix, markersAccess)
75+
})
76+
77+
typeChecker.CheckNode(pass, field)
7278
})
7379

7480
return nil, nil //nolint:nilnil
7581
}
7682

77-
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) {
78-
fieldName := utils.FieldName(field)
79-
if fieldName == "" {
80-
return
81-
}
82-
83-
// Unwrap pointers and slices to get the underlying type
84-
fieldType, isSlice := unwrapType(field.Type)
85-
86-
// Get the underlying numeric type identifier (int32, int64, float32, float64)
87-
ident := getNumericTypeIdent(pass, fieldType)
88-
if ident == nil {
89-
return
90-
}
91-
83+
//nolint:cyclop
84+
func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string, markersAccess markershelper.Markers) {
9285
// Only check int32, int64, float32, and float64 types
9386
if ident.Name != "int32" && ident.Name != "int64" && ident.Name != "float32" && ident.Name != "float64" {
9487
return
9588
}
9689

97-
// Create type description that clarifies array element types
98-
typeDesc := ident.Name
99-
if isSlice {
100-
typeDesc = fmt.Sprintf("array element type of %s", ident.Name)
90+
field, ok := node.(*ast.Field)
91+
if !ok {
92+
return
10193
}
10294

10395
fieldMarkers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
10496

97+
// Determine if this is a slice/array field
98+
isSlice := utils.IsArrayTypeOrAlias(pass, field)
99+
105100
// Determine which markers to look for based on whether the field is a slice
106101
minMarkers, maxMarkers := getMarkerNames(isSlice)
107102

@@ -115,82 +110,29 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp
115110

116111
// Report any invalid marker values (e.g., non-numeric values)
117112
if minErr != nil && !minMissing {
118-
pass.Reportf(field.Pos(), "field %s has an invalid minimum marker: %v", fieldName, minErr)
113+
pass.Reportf(field.Pos(), "%s has an invalid minimum marker: %v", prefix, minErr)
119114
}
115+
120116
if maxErr != nil && !maxMissing {
121-
pass.Reportf(field.Pos(), "field %s has an invalid maximum marker: %v", fieldName, maxErr)
117+
pass.Reportf(field.Pos(), "%s has an invalid maximum marker: %v", prefix, maxErr)
122118
}
123119

124120
// Report if markers are missing
125121
if minMissing {
126-
pass.Reportf(field.Pos(), "field %s %s is missing minimum bounds validation marker", fieldName, typeDesc)
122+
pass.Reportf(field.Pos(), "%s is missing minimum bounds validation marker", prefix)
127123
}
124+
128125
if maxMissing {
129-
pass.Reportf(field.Pos(), "field %s %s is missing maximum bounds validation marker", fieldName, typeDesc)
126+
pass.Reportf(field.Pos(), "%s is missing maximum bounds validation marker", prefix)
130127
}
131128

132129
// If any markers are missing or invalid, don't continue with bounds checks
133130
if minErr != nil || maxErr != nil {
134131
return
135132
}
136133

137-
// Validate bounds are within the type's range
138-
checkBoundsWithinTypeRange(pass, field, fieldName, typeDesc, minimum, maximum)
139-
140-
// For int64 fields, check if bounds are within JavaScript safe integer range
141-
checkJavaScriptSafeBounds(pass, field, fieldName, typeDesc, minimum, maximum)
142-
}
143-
144-
// getNumericTypeIdent returns the identifier for numeric types (int32, int64, float32, float64).
145-
// It handles type aliases by looking up the underlying type.
146-
// Note: This function expects pointers and slices to already be unwrapped.
147-
func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident {
148-
ident, ok := expr.(*ast.Ident)
149-
if !ok {
150-
return nil
151-
}
152-
153-
// Check if it's a basic numeric type we care about
154-
if ident.Name == "int32" || ident.Name == "int64" || ident.Name == "float32" || ident.Name == "float64" {
155-
return ident
156-
}
157-
158-
// Check if it's a type alias to a numeric type
159-
if !utils.IsBasicType(pass, ident) {
160-
typeSpec, ok := utils.LookupTypeSpec(pass, ident)
161-
if ok {
162-
return getNumericTypeIdent(pass, typeSpec.Type)
163-
}
164-
}
165-
166-
return nil
167-
}
168-
169-
// unwrapType unwraps pointers and slices to get the underlying type.
170-
// Returns the unwrapped type and a boolean indicating if it's a slice.
171-
// When the field is a slice, we extract the element type since that's what
172-
// needs bounds validation (e.g., []int32 -> int32). The isSlice flag allows
173-
// the caller to report errors with "array element type" for clarity.
174-
func unwrapType(expr ast.Expr) (ast.Expr, bool) {
175-
isSlice := false
176-
177-
// Unwrap pointer if present (e.g., *int32)
178-
if starExpr, ok := expr.(*ast.StarExpr); ok {
179-
expr = starExpr.X
180-
}
181-
182-
// Check if it's a slice and unwrap to get element type (e.g., []int32 -> int32)
183-
if arrayType, ok := expr.(*ast.ArrayType); ok {
184-
isSlice = true
185-
expr = arrayType.Elt
186-
187-
// Handle pointer inside slice (e.g., []*int32 -> int32)
188-
if starExpr, ok := expr.(*ast.StarExpr); ok {
189-
expr = starExpr.X
190-
}
191-
}
192-
193-
return expr, isSlice
134+
// Validate bounds are within the type's valid range
135+
checkBoundsWithinTypeRange(pass, field, prefix, ident.Name, minimum, maximum)
194136
}
195137

196138
// getMarkerNames returns the appropriate minimum and maximum marker names
@@ -200,6 +142,7 @@ func getMarkerNames(isSlice bool) (minMarkers, maxMarkers []string) {
200142
if isSlice {
201143
return []string{markers.KubebuilderItemsMinimumMarker}, []string{markers.KubebuilderItemsMaximumMarker}
202144
}
145+
203146
return []string{markers.KubebuilderMinimumMarker, markers.K8sMinimumMarker}, []string{markers.KubebuilderMaximumMarker, markers.K8sMaximumMarker}
204147
}
205148

@@ -212,16 +155,14 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri
212155
continue
213156
}
214157

215-
marker := markerList[0]
216-
rawValue, ok := marker.Expressions[""]
217-
if !ok {
218-
continue
219-
}
220-
221-
// Parse as float64 using strconv for better error handling
222-
value, err := strconv.ParseFloat(rawValue, 64)
158+
// Use the exported utils function to parse the marker value
159+
value, err := utils.GetMarkerNumericValue[float64](markerList[0])
223160
if err != nil {
224-
return 0, fmt.Errorf("error converting value to number: %w", err)
161+
if errors.Is(err, errMarkerMissingValue) {
162+
continue
163+
}
164+
165+
return 0, fmt.Errorf("error getting marker value: %w", err)
225166
}
226167

227168
return value, nil
@@ -231,51 +172,45 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri
231172
}
232173

233174
// checkBoundsWithinTypeRange validates that the bounds are within the valid range for the type.
234-
func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, fieldName, typeDesc string, minimum, maximum float64) {
235-
// Extract the actual type name from typeDesc (e.g., "array element type of int32" -> "int32")
236-
typeName := extractTypeName(typeDesc)
237-
175+
// For int64, enforces JavaScript-safe bounds as per Kubernetes API conventions to ensure
176+
// compatibility with JavaScript clients.
177+
//
178+
//nolint:cyclop
179+
func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, prefix, typeName string, minimum, maximum float64) {
238180
switch typeName {
239181
case "int32":
240-
if minimum < minInt32 || minimum > maxInt32 {
241-
pass.Reportf(field.Pos(), "field %s %s has minimum bound %v that is outside the valid int32 range [%d, %d]", fieldName, typeDesc, minimum, minInt32, maxInt32)
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)
242184
}
243-
if maximum < minInt32 || maximum > maxInt32 {
244-
pass.Reportf(field.Pos(), "field %s %s has maximum bound %v that is outside the valid int32 range [%d, %d]", fieldName, typeDesc, maximum, minInt32, maxInt32)
245-
}
246-
case "float32":
247-
if minimum < minFloat32 || minimum > maxFloat32 {
248-
pass.Reportf(field.Pos(), "field %s %s has minimum bound %v that is outside the valid float32 range", fieldName, typeDesc, minimum)
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)
249188
}
250-
if maximum < minFloat32 || maximum > maxFloat32 {
251-
pass.Reportf(field.Pos(), "field %s %s has maximum bound %v that is outside the valid float32 range", fieldName, typeDesc, maximum)
189+
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, minSafeInt64, maxSafeInt64)
252194
}
253-
}
254-
}
255195

256-
// checkJavaScriptSafeBounds checks if int64 bounds are within JavaScript safe integer range.
257-
func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeDesc string, minimum, maximum float64) {
258-
// Extract the actual type name from typeDesc
259-
typeName := extractTypeName(typeDesc)
260-
261-
if typeName != "int64" {
262-
return
263-
}
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, minSafeInt64, maxSafeInt64)
198+
}
199+
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+
}
264203

265-
if minimum < minSafeInt64 || maximum > maxSafeInt64 {
266-
pass.Reportf(field.Pos(),
267-
"field %s %s has bounds [%d, %d] that exceed safe integer range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients",
268-
fieldName, typeDesc, int64(minimum), int64(maximum), minSafeInt64, maxSafeInt64)
269-
}
270-
}
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+
}
207+
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+
}
271211

272-
// extractTypeName extracts the base type name from a type description.
273-
// E.g., "array element type of int32" -> "int32", "int64" -> "int64"
274-
func extractTypeName(typeDesc string) string {
275-
// Check if it's an array element type description
276-
const prefix = "array element type of "
277-
if len(typeDesc) > len(prefix) && typeDesc[:len(prefix)] == prefix {
278-
return typeDesc[len(prefix):]
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)
214+
}
279215
}
280-
return typeDesc
281216
}

0 commit comments

Comments
 (0)