From ef45ccecd6f5751c29ff6c8b01610dfbbc68b32a Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Fri, 31 Oct 2025 18:11:07 -0400 Subject: [PATCH 1/8] added the numeric bounds linter --- docs/linters.md | 63 +++++ pkg/analysis/numericbounds/analyzer.go | 219 ++++++++++++++++++ pkg/analysis/numericbounds/analyzer_test.go | 28 +++ pkg/analysis/numericbounds/doc.go | 54 +++++ pkg/analysis/numericbounds/initializer.go | 35 +++ .../numericbounds/testdata/src/a/a.go | 152 ++++++++++++ pkg/registration/doc.go | 1 + 7 files changed, 552 insertions(+) create mode 100644 pkg/analysis/numericbounds/analyzer.go create mode 100644 pkg/analysis/numericbounds/analyzer_test.go create mode 100644 pkg/analysis/numericbounds/doc.go create mode 100644 pkg/analysis/numericbounds/initializer.go create mode 100644 pkg/analysis/numericbounds/testdata/src/a/a.go diff --git a/docs/linters.md b/docs/linters.md index 594f8407..80143ef6 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -12,6 +12,7 @@ - [JSONTags](#jsontags) - Ensures proper JSON tag formatting - [MaxLength](#maxlength) - Checks for maximum length constraints on strings and arrays - [NamingConventions](#namingconventions) - Ensures field names adhere to user-defined naming conventions +- [NumericBounds](#numericbounds) - Validates numeric fields have appropriate bounds validation markers - [NoBools](#nobools) - Prevents usage of boolean types - [NoDurations](#nodurations) - Prevents usage of duration types - [NoFloats](#nofloats) - Prevents usage of floating-point types @@ -477,6 +478,68 @@ linterConfig: message: prefer 'colour' over 'color' when referring to colours in field names ``` +## NumericBounds + +The `numericbounds` linter checks that numeric fields (`int32` and `int64`) have appropriate bounds validation markers. + +According to Kubernetes API conventions, numeric fields should have bounds checking to prevent values that are too small, negative (when not intended), or too large. + +This linter ensures that: +- `int32` and `int64` fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers +- `int64` fields with bounds outside the JavaScript safe integer range are flagged + +### JavaScript Safe Integer Range + +For `int64` fields, the linter checks if the bounds exceed the JavaScript safe integer range of `-(2^53)` to `(2^53)` (specifically, `-9007199254740991` to `9007199254740991`). + +JavaScript represents all numbers as IEEE 754 double-precision floating-point values, which can only safely represent integers in this range. Values outside this range may lose precision when processed by JavaScript clients. + +When an `int64` field has bounds that exceed this range, the linter will suggest using a string type instead to avoid precision loss. + +### Examples + +**Valid:** Numeric field with proper bounds markers +```go +type Example struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=100 + Count int32 +} +``` + +**Valid:** Int64 field with JavaScript-safe bounds +```go +type Example struct { + // +kubebuilder:validation:Minimum=-9007199254740991 + // +kubebuilder:validation:Maximum=9007199254740991 + Timestamp int64 +} +``` + +**Invalid:** Missing bounds markers +```go +type Example struct { + Count int32 // want: should have minimum and maximum bounds validation markers +} +``` + +**Invalid:** Only one bound specified +```go +type Example struct { + // +kubebuilder:validation:Minimum=0 + Count int32 // want: has minimum but is missing maximum bounds validation marker +} +``` + +**Invalid:** Int64 with bounds exceeding JavaScript safe range +```go +type Example struct { + // +kubebuilder:validation:Minimum=-10000000000000000 + // +kubebuilder:validation:Maximum=10000000000000000 + LargeNumber int64 // want: bounds exceed JavaScript safe integer range +} +``` + ## NoBools The `nobools` linter checks that fields in the API types do not contain a `bool` type. diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go new file mode 100644 index 00000000..852db19e --- /dev/null +++ b/pkg/analysis/numericbounds/analyzer.go @@ -0,0 +1,219 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package numericbounds + +import ( + "errors" + "fmt" + "go/ast" + "strconv" + + "golang.org/x/tools/go/analysis" + kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" + markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/utils" + "sigs.k8s.io/kube-api-linter/pkg/markers" +) + +const name = "numericbounds" + +// JavaScript safe integer bounds (2^53 - 1 and -(2^53 - 1)) +const ( + maxSafeInt = 9007199254740991 // 2^53 - 1 + minSafeInt = -9007199254740991 // -(2^53 - 1) +) + +var errMarkerMissingValue = errors.New("marker value not found") + +// Analyzer is the analyzer for the numericbounds package. +// It checks that numeric fields have appropriate bounds validation markers. +var Analyzer = &analysis.Analyzer{ + Name: name, + Doc: "Checks that numeric fields (int32, int64) have appropriate minimum and maximum bounds validation markers", + Run: run, + Requires: []*analysis.Analyzer{inspector.Analyzer}, +} + +func run(pass *analysis.Pass) (any, error) { + inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector) + if !ok { + return nil, kalerrors.ErrCouldNotGetInspector + } + + inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) { + checkField(pass, field, markersAccess) + }) + + return nil, nil //nolint:nilnil +} + +func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) { + if field == nil || len(field.Names) == 0 { + return + } + + // Unwrap pointers and slices to get the underlying type + fieldType, isSlice := unwrapType(field.Type) + + // Get the underlying numeric type identifier (int32 or int64) + ident := getNumericTypeIdent(pass, fieldType) + if ident == nil { + return + } + + // Only check int32 and int64 types + if ident.Name != "int32" && ident.Name != "int64" { + return + } + + fieldName := utils.FieldName(field) + fieldMarkers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field) + + // Determine which markers to look for based on whether the field is a slice + minMarker, maxMarker := getMarkerNames(isSlice) + + // Get minimum and maximum marker values + minimum, minErr := getMarkerNumericValue(fieldMarkers, minMarker) + maximum, maxErr := getMarkerNumericValue(fieldMarkers, maxMarker) + + // Check if markers are missing + minMissing := errors.Is(minErr, errMarkerMissingValue) + maxMissing := errors.Is(maxErr, errMarkerMissingValue) + + // Report any invalid marker values (e.g., non-numeric values) + if minErr != nil && !minMissing { + pass.Reportf(field.Pos(), "field %s has an invalid minimum marker: %v", fieldName, minErr) + return + } + if maxErr != nil && !maxMissing { + pass.Reportf(field.Pos(), "field %s has an invalid maximum marker: %v", fieldName, maxErr) + return + } + + // Report if both markers are missing + if minMissing && maxMissing { + pass.Reportf(field.Pos(), "field %s of type %s should have minimum and maximum bounds validation markers", fieldName, ident.Name) + return + } + + // Report if only one marker is present + if minMissing { + pass.Reportf(field.Pos(), "field %s of type %s has maximum but is missing minimum bounds validation marker", fieldName, ident.Name) + return + } + if maxMissing { + pass.Reportf(field.Pos(), "field %s of type %s has minimum but is missing maximum bounds validation marker", fieldName, ident.Name) + return + } + + // For int64 fields, check if bounds are within JavaScript safe integer range + checkJavaScriptSafeBounds(pass, field, fieldName, ident.Name, minimum, maximum) +} + +// unwrapType unwraps pointers and slices to get the underlying type. +// Returns the unwrapped type and a boolean indicating if it's a slice. +func unwrapType(expr ast.Expr) (ast.Expr, bool) { + isSlice := false + + // Unwrap pointer if present (e.g., *int32) + if starExpr, ok := expr.(*ast.StarExpr); ok { + expr = starExpr.X + } + + // Check if it's a slice and unwrap (e.g., []int32) + if arrayType, ok := expr.(*ast.ArrayType); ok { + isSlice = true + expr = arrayType.Elt + + // Handle pointer inside slice (e.g., []*int32) + if starExpr, ok := expr.(*ast.StarExpr); ok { + expr = starExpr.X + } + } + + return expr, isSlice +} + +// getMarkerNames returns the appropriate minimum and maximum marker names +// based on whether the field is a slice. +func getMarkerNames(isSlice bool) (minMarker, maxMarker string) { + if isSlice { + return markers.KubebuilderItemsMinimumMarker, markers.KubebuilderItemsMaximumMarker + } + return markers.KubebuilderMinimumMarker, markers.KubebuilderMaximumMarker +} + +// checkJavaScriptSafeBounds checks if int64 bounds are within JavaScript safe integer range. +func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeName string, minimum, maximum float64) { + if typeName != "int64" { + return + } + + if minimum < minSafeInt || maximum > maxSafeInt { + pass.Reportf(field.Pos(), + "field %s of type int64 has bounds [%d, %d] that exceed safe integer range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients", + fieldName, int64(minimum), int64(maximum), minSafeInt, maxSafeInt) + } +} + +// getMarkerNumericValue extracts the numeric value from the first instance of the marker with the given name. +func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerName string) (float64, error) { + markerList := markerSet.Get(markerName) + if len(markerList) == 0 { + return 0, errMarkerMissingValue + } + + marker := markerList[0] + rawValue, ok := marker.Expressions[""] + if !ok { + return 0, errMarkerMissingValue + } + + // Parse as float64 using strconv for better error handling + value, err := strconv.ParseFloat(rawValue, 64) + if err != nil { + return 0, fmt.Errorf("error converting value to number: %w", err) + } + + return value, nil +} + +// getNumericTypeIdent returns the identifier for int32 or int64 types. +// It handles type aliases by looking up the underlying type. +// Note: This function expects pointers and slices to already be unwrapped. +func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { + ident, ok := expr.(*ast.Ident) + if !ok { + return nil + } + + // Check if it's a basic int32 or int64 type + if ident.Name == "int32" || ident.Name == "int64" { + return ident + } + + // Check if it's a type alias to int32 or int64 + if !utils.IsBasicType(pass, ident) { + typeSpec, ok := utils.LookupTypeSpec(pass, ident) + if ok { + return getNumericTypeIdent(pass, typeSpec.Type) + } + } + + return nil +} diff --git a/pkg/analysis/numericbounds/analyzer_test.go b/pkg/analysis/numericbounds/analyzer_test.go new file mode 100644 index 00000000..b0c829db --- /dev/null +++ b/pkg/analysis/numericbounds/analyzer_test.go @@ -0,0 +1,28 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package numericbounds_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + "sigs.k8s.io/kube-api-linter/pkg/analysis/numericbounds" +) + +func Test(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, numericbounds.Analyzer, "a") +} diff --git a/pkg/analysis/numericbounds/doc.go b/pkg/analysis/numericbounds/doc.go new file mode 100644 index 00000000..ad41361b --- /dev/null +++ b/pkg/analysis/numericbounds/doc.go @@ -0,0 +1,54 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* +numericbounds is an analyzer that checks for proper bounds validation on numeric fields. + +According to Kubernetes API conventions, numeric fields should have appropriate bounds +checking to prevent values that are too small, negative (when not intended), or too large. + +This analyzer ensures that: + - int32 and int64 fields have both minimum and maximum bounds markers + - For slices of numeric types, the analyzer checks for items:Minimum and items:Maximum markers + - Type aliases to int32 or int64 are also checked + - Pointer types (e.g., *int32, []*int64) are unwrapped and validated + - int64 fields with values outside the JavaScript safe integer range (-(2^53-1) to (2^53-1)) + are flagged, as they may cause precision loss in JavaScript clients + +The analyzer checks for the presence of +kubebuilder:validation:Minimum and ++kubebuilder:validation:Maximum markers on numeric fields, or the items: variants for slices. + +For int64 fields, if the bounds exceed the JavaScript safe integer range of +[-9007199254740991, 9007199254740991], the analyzer suggests using a string type instead +to avoid precision loss in JavaScript environments. + +Examples of valid and invalid code: + +Valid: + + type Example struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=100 + Count int32 + } + +Invalid: + + type Example struct { + Count int32 // Missing minimum and maximum markers + } +*/ +package numericbounds diff --git a/pkg/analysis/numericbounds/initializer.go b/pkg/analysis/numericbounds/initializer.go new file mode 100644 index 00000000..cfd1b616 --- /dev/null +++ b/pkg/analysis/numericbounds/initializer.go @@ -0,0 +1,35 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package numericbounds + +import ( + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" + "sigs.k8s.io/kube-api-linter/pkg/analysis/registry" +) + +func init() { + registry.DefaultRegistry().RegisterLinter(Initializer()) +} + +// Initializer returns the AnalyzerInitializer for this +// Analyzer so that it can be added to the registry. +func Initializer() initializer.AnalyzerInitializer { + return initializer.NewInitializer( + name, + Analyzer, + true, + ) +} diff --git a/pkg/analysis/numericbounds/testdata/src/a/a.go b/pkg/analysis/numericbounds/testdata/src/a/a.go new file mode 100644 index 00000000..cc9e7830 --- /dev/null +++ b/pkg/analysis/numericbounds/testdata/src/a/a.go @@ -0,0 +1,152 @@ +package a + +// ValidInt32WithBounds has proper bounds validation +type ValidInt32WithBounds struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=100 + Count int32 +} + +// ValidInt64WithBounds has proper bounds validation +type ValidInt64WithBounds struct { + // +kubebuilder:validation:Minimum=-1000 + // +kubebuilder:validation:Maximum=1000 + Value int64 +} + +// ValidInt64WithJSSafeBounds has bounds within JavaScript safe integer range +type ValidInt64WithJSSafeBounds struct { + // +kubebuilder:validation:Minimum=-9007199254740991 + // +kubebuilder:validation:Maximum=9007199254740991 + SafeValue int64 +} + +// InvalidInt32NoBounds should have bounds markers +type InvalidInt32NoBounds struct { + NoBounds int32 // want "field NoBounds of type int32 should have minimum and maximum bounds validation markers" +} + +// InvalidInt64NoBounds should have bounds markers +type InvalidInt64NoBounds struct { + NoBounds int64 // want "field NoBounds of type int64 should have minimum and maximum bounds validation markers" +} + +// InvalidInt32OnlyMin should have maximum marker +type InvalidInt32OnlyMin struct { + // +kubebuilder:validation:Minimum=0 + OnlyMin int32 // want "field OnlyMin of type int32 has minimum but is missing maximum bounds validation marker" +} + +// InvalidInt32OnlyMax should have minimum marker +type InvalidInt32OnlyMax struct { + // +kubebuilder:validation:Maximum=100 + OnlyMax int32 // want "field OnlyMax of type int32 has maximum but is missing minimum bounds validation marker" +} + +// InvalidInt64OnlyMin should have maximum marker +type InvalidInt64OnlyMin struct { + // +kubebuilder:validation:Minimum=0 + OnlyMin int64 // want "field OnlyMin of type int64 has minimum but is missing maximum bounds validation marker" +} + +// InvalidInt64OnlyMax should have minimum marker +type InvalidInt64OnlyMax struct { + // +kubebuilder:validation:Maximum=100 + OnlyMax int64 // want "field OnlyMax of type int64 has maximum but is missing minimum bounds validation marker" +} + +// InvalidInt64ExceedsJSMaxBounds has maximum that exceeds JavaScript safe integer range +type InvalidInt64ExceedsJSMaxBounds struct { + // +kubebuilder:validation:Minimum=-1000 + // +kubebuilder:validation:Maximum=9007199254740992 + UnsafeMax int64 // want "field UnsafeMax of type int64 has bounds \\[-1000, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" +} + +// InvalidInt64ExceedsJSMinBounds has minimum that exceeds JavaScript safe integer range +type InvalidInt64ExceedsJSMinBounds struct { + // +kubebuilder:validation:Minimum=-9007199254740992 + // +kubebuilder:validation:Maximum=1000 + UnsafeMin int64 // want "field UnsafeMin of type int64 has bounds \\[-9007199254740992, 1000\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" +} + +// InvalidInt64ExceedsJSBothBounds has both bounds exceeding JavaScript safe integer range +type InvalidInt64ExceedsJSBothBounds struct { + // +kubebuilder:validation:Minimum=-9007199254740992 + // +kubebuilder:validation:Maximum=9007199254740992 + UnsafeBoth int64 // want "field UnsafeBoth of type int64 has bounds \\[-9007199254740992, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" +} + +// IgnoredStringField should not be checked +type IgnoredStringField struct { + Name string +} + +// IgnoredBoolField should not be checked +type IgnoredBoolField struct { + Enabled bool +} + +// IgnoredFloat64Field should not be checked +type IgnoredFloat64Field struct { + Value float64 +} + +// MixedFields has both valid and invalid fields +type MixedFields struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=100 + ValidCount int32 + + InvalidCount int32 // want "field InvalidCount of type int32 should have minimum and maximum bounds validation markers" + + Name string + + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=1000 + ValidValue int64 + + InvalidValue int64 // want "field InvalidValue of type int64 should have minimum and maximum bounds validation markers" +} + +// PointerFields with pointers should also be checked +type PointerFields struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=100 + ValidPointerWithBounds *int32 + + InvalidPointer *int32 // want "field InvalidPointer of type int32 should have minimum and maximum bounds validation markers" + + InvalidPointer64 *int64 // want "field InvalidPointer64 of type int64 should have minimum and maximum bounds validation markers" +} + +// SliceFields with slices should check the element type +type SliceFields struct { + ValidSlice []string + + InvalidSlice []int32 // want "field InvalidSlice of type int32 should have minimum and maximum bounds validation markers" + + InvalidSlice64 []int64 // want "field InvalidSlice64 of type int64 should have minimum and maximum bounds validation markers" + + // +kubebuilder:validation:items:Minimum=0 + // +kubebuilder:validation:items:Maximum=100 + ValidSliceWithBounds []int32 +} + +// TypeAliasFields with type aliases should be checked +type Int32Alias int32 +type Int64Alias int64 + +type TypeAliasFields struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=100 + ValidAlias Int32Alias + + InvalidAlias Int32Alias // want "field InvalidAlias of type int32 should have minimum and maximum bounds validation markers" + + InvalidAlias64 Int64Alias // want "field InvalidAlias64 of type int64 should have minimum and maximum bounds validation markers" +} + +// PointerSliceFields with pointer slices should also be checked +type PointerSliceFields struct { + InvalidPointerSlice []*int32 // want "field InvalidPointerSlice of type int32 should have minimum and maximum bounds validation markers" +} diff --git a/pkg/registration/doc.go b/pkg/registration/doc.go index 439a7513..916b54b6 100644 --- a/pkg/registration/doc.go +++ b/pkg/registration/doc.go @@ -42,6 +42,7 @@ import ( _ "sigs.k8s.io/kube-api-linter/pkg/analysis/nophase" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/noreferences" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/notimestamp" + _ "sigs.k8s.io/kube-api-linter/pkg/analysis/numericbounds" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/optionalfields" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/optionalorrequired" _ "sigs.k8s.io/kube-api-linter/pkg/analysis/preferredmarkers" From fcd8ab62fdf821ca4d2d60eb71eebbda5bf617e9 Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Sun, 2 Nov 2025 19:50:50 -0500 Subject: [PATCH 2/8] reorganize code to mimic project conventions --- pkg/analysis/numericbounds/analyzer.go | 70 +++++++++++++------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index 852db19e..8470e408 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -63,7 +63,8 @@ func run(pass *analysis.Pass) (any, error) { } func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) { - if field == nil || len(field.Names) == 0 { + fieldName := utils.FieldName(field) + if fieldName == "" { return } @@ -81,7 +82,6 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp return } - fieldName := utils.FieldName(field) fieldMarkers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field) // Determine which markers to look for based on whether the field is a slice @@ -125,6 +125,31 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp checkJavaScriptSafeBounds(pass, field, fieldName, ident.Name, minimum, maximum) } +// getNumericTypeIdent returns the identifier for int32 or int64 types. +// It handles type aliases by looking up the underlying type. +// Note: This function expects pointers and slices to already be unwrapped. +func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { + ident, ok := expr.(*ast.Ident) + if !ok { + return nil + } + + // Check if it's a basic int32 or int64 type + if ident.Name == "int32" || ident.Name == "int64" { + return ident + } + + // Check if it's a type alias to int32 or int64 + if !utils.IsBasicType(pass, ident) { + typeSpec, ok := utils.LookupTypeSpec(pass, ident) + if ok { + return getNumericTypeIdent(pass, typeSpec.Type) + } + } + + return nil +} + // unwrapType unwraps pointers and slices to get the underlying type. // Returns the unwrapped type and a boolean indicating if it's a slice. func unwrapType(expr ast.Expr) (ast.Expr, bool) { @@ -158,19 +183,6 @@ func getMarkerNames(isSlice bool) (minMarker, maxMarker string) { return markers.KubebuilderMinimumMarker, markers.KubebuilderMaximumMarker } -// checkJavaScriptSafeBounds checks if int64 bounds are within JavaScript safe integer range. -func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeName string, minimum, maximum float64) { - if typeName != "int64" { - return - } - - if minimum < minSafeInt || maximum > maxSafeInt { - pass.Reportf(field.Pos(), - "field %s of type int64 has bounds [%d, %d] that exceed safe integer range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients", - fieldName, int64(minimum), int64(maximum), minSafeInt, maxSafeInt) - } -} - // getMarkerNumericValue extracts the numeric value from the first instance of the marker with the given name. func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerName string) (float64, error) { markerList := markerSet.Get(markerName) @@ -193,27 +205,15 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerName string) return value, nil } -// getNumericTypeIdent returns the identifier for int32 or int64 types. -// It handles type aliases by looking up the underlying type. -// Note: This function expects pointers and slices to already be unwrapped. -func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { - ident, ok := expr.(*ast.Ident) - if !ok { - return nil - } - - // Check if it's a basic int32 or int64 type - if ident.Name == "int32" || ident.Name == "int64" { - return ident +// checkJavaScriptSafeBounds checks if int64 bounds are within JavaScript safe integer range. +func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeName string, minimum, maximum float64) { + if typeName != "int64" { + return } - // Check if it's a type alias to int32 or int64 - if !utils.IsBasicType(pass, ident) { - typeSpec, ok := utils.LookupTypeSpec(pass, ident) - if ok { - return getNumericTypeIdent(pass, typeSpec.Type) - } + if minimum < minSafeInt || maximum > maxSafeInt { + pass.Reportf(field.Pos(), + "field %s of type int64 has bounds [%d, %d] that exceed safe integer range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients", + fieldName, int64(minimum), int64(maximum), minSafeInt, maxSafeInt) } - - return nil } From 626dbb74d7b51c02c680252d991e0c7ab3da4e73 Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Tue, 4 Nov 2025 15:23:48 -0500 Subject: [PATCH 3/8] Added float32/64, moved docs, and followed other conventions as discussed in reviews --- docs/linters.md | 67 ++----- pkg/analysis/numericbounds/analyzer.go | 166 ++++++++++++------ pkg/analysis/numericbounds/doc.go | 75 +++++++- pkg/analysis/numericbounds/initializer.go | 2 +- .../numericbounds/testdata/src/a/a.go | 116 +++++++++--- 5 files changed, 296 insertions(+), 130 deletions(-) diff --git a/docs/linters.md b/docs/linters.md index 80143ef6..ce4b892a 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -480,65 +480,34 @@ linterConfig: ## NumericBounds -The `numericbounds` linter checks that numeric fields (`int32` and `int64`) have appropriate bounds validation markers. +The `numericbounds` linter checks that numeric fields (`int32`, `int64`, `float32`, `float64`) have appropriate bounds validation markers. According to Kubernetes API conventions, numeric fields should have bounds checking to prevent values that are too small, negative (when not intended), or too large. This linter ensures that: -- `int32` and `int64` fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers -- `int64` fields with bounds outside the JavaScript safe integer range are flagged +- Numeric fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers +- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported +- Bounds values are within the type's valid range (e.g., int32 bounds must fit in int32) +- `int64` fields with bounds outside the JavaScript safe integer range (±2^53-1) are flagged -### JavaScript Safe Integer Range +**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. -For `int64` fields, the linter checks if the bounds exceed the JavaScript safe integer range of `-(2^53)` to `(2^53)` (specifically, `-9007199254740991` to `9007199254740991`). - -JavaScript represents all numbers as IEEE 754 double-precision floating-point values, which can only safely represent integers in this range. Values outside this range may lose precision when processed by JavaScript clients. - -When an `int64` field has bounds that exceed this range, the linter will suggest using a string type instead to avoid precision loss. - -### Examples - -**Valid:** Numeric field with proper bounds markers -```go -type Example struct { - // +kubebuilder:validation:Minimum=0 - // +kubebuilder:validation:Maximum=100 - Count int32 -} -``` - -**Valid:** Int64 field with JavaScript-safe bounds -```go -type Example struct { - // +kubebuilder:validation:Minimum=-9007199254740991 - // +kubebuilder:validation:Maximum=9007199254740991 - Timestamp int64 -} -``` +### Configuration -**Invalid:** Missing bounds markers -```go -type Example struct { - Count int32 // want: should have minimum and maximum bounds validation markers -} -``` +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. -**Invalid:** Only one bound specified -```go -type Example struct { - // +kubebuilder:validation:Minimum=0 - Count int32 // want: has minimum but is missing maximum bounds validation marker -} +To enable in `.golangci.yml`: +```yaml +linters-settings: + custom: + kubeapilinter: + settings: + linters: + enable: + - numericbounds ``` -**Invalid:** Int64 with bounds exceeding JavaScript safe range -```go -type Example struct { - // +kubebuilder:validation:Minimum=-10000000000000000 - // +kubebuilder:validation:Maximum=10000000000000000 - LargeNumber int64 // want: bounds exceed JavaScript safe integer range -} -``` +See the [package documentation](https://pkg.go.dev/sigs.k8s.io/kube-api-linter/pkg/analysis/numericbounds) for detailed examples and usage. ## NoBools diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index 8470e408..f76b73b2 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -30,12 +30,24 @@ import ( "sigs.k8s.io/kube-api-linter/pkg/markers" ) -const name = "numericbounds" +const ( + name = "numericbounds" +) + +// Type bounds for validation +const ( + maxInt32 = 2147483647 // 2^31 - 1 + minInt32 = -2147483648 // -2^31 + maxFloat32 = 3.40282346638528859811704183484516925440e+38 // max float32 + minFloat32 = -3.40282346638528859811704183484516925440e+38 // min float32 +) // JavaScript safe integer bounds (2^53 - 1 and -(2^53 - 1)) const ( - maxSafeInt = 9007199254740991 // 2^53 - 1 - minSafeInt = -9007199254740991 // -(2^53 - 1) + maxSafeInt32 = 2147483647 // 2^31 - 1 (fits in JS Number) + minSafeInt32 = -2147483648 // -2^31 (fits in JS Number) + maxSafeInt64 = 9007199254740991 // 2^53 - 1 (max safe integer in JS) + minSafeInt64 = -9007199254740991 // -(2^53 - 1) (min safe integer in JS) ) var errMarkerMissingValue = errors.New("marker value not found") @@ -44,7 +56,7 @@ var errMarkerMissingValue = errors.New("marker value not found") // It checks that numeric fields have appropriate bounds validation markers. var Analyzer = &analysis.Analyzer{ Name: name, - Doc: "Checks that numeric fields (int32, int64) have appropriate minimum and maximum bounds validation markers", + Doc: "Checks that numeric fields (int32, int64, float32, float64) have appropriate minimum and maximum bounds validation markers", Run: run, Requires: []*analysis.Analyzer{inspector.Analyzer}, } @@ -71,25 +83,31 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp // Unwrap pointers and slices to get the underlying type fieldType, isSlice := unwrapType(field.Type) - // Get the underlying numeric type identifier (int32 or int64) + // Get the underlying numeric type identifier (int32, int64, float32, float64) ident := getNumericTypeIdent(pass, fieldType) if ident == nil { return } - // Only check int32 and int64 types - if ident.Name != "int32" && ident.Name != "int64" { + // Only check int32, int64, float32, and float64 types + if ident.Name != "int32" && ident.Name != "int64" && ident.Name != "float32" && ident.Name != "float64" { return } + // Create type description that clarifies array element types + typeDesc := ident.Name + if isSlice { + typeDesc = fmt.Sprintf("array element type of %s", ident.Name) + } + fieldMarkers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field) // Determine which markers to look for based on whether the field is a slice - minMarker, maxMarker := getMarkerNames(isSlice) + minMarkers, maxMarkers := getMarkerNames(isSlice) // Get minimum and maximum marker values - minimum, minErr := getMarkerNumericValue(fieldMarkers, minMarker) - maximum, maxErr := getMarkerNumericValue(fieldMarkers, maxMarker) + minimum, minErr := getMarkerNumericValue(fieldMarkers, minMarkers) + maximum, maxErr := getMarkerNumericValue(fieldMarkers, maxMarkers) // Check if markers are missing minMissing := errors.Is(minErr, errMarkerMissingValue) @@ -98,34 +116,32 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp // Report any invalid marker values (e.g., non-numeric values) if minErr != nil && !minMissing { pass.Reportf(field.Pos(), "field %s has an invalid minimum marker: %v", fieldName, minErr) - return } if maxErr != nil && !maxMissing { pass.Reportf(field.Pos(), "field %s has an invalid maximum marker: %v", fieldName, maxErr) - return } - // Report if both markers are missing - if minMissing && maxMissing { - pass.Reportf(field.Pos(), "field %s of type %s should have minimum and maximum bounds validation markers", fieldName, ident.Name) - return - } - - // Report if only one marker is present + // Report if markers are missing if minMissing { - pass.Reportf(field.Pos(), "field %s of type %s has maximum but is missing minimum bounds validation marker", fieldName, ident.Name) - return + pass.Reportf(field.Pos(), "field %s %s is missing minimum bounds validation marker", fieldName, typeDesc) } if maxMissing { - pass.Reportf(field.Pos(), "field %s of type %s has minimum but is missing maximum bounds validation marker", fieldName, ident.Name) + pass.Reportf(field.Pos(), "field %s %s is missing maximum bounds validation marker", fieldName, typeDesc) + } + + // If any markers are missing or invalid, don't continue with bounds checks + if minErr != nil || maxErr != nil { return } + // Validate bounds are within the type's range + checkBoundsWithinTypeRange(pass, field, fieldName, typeDesc, minimum, maximum) + // For int64 fields, check if bounds are within JavaScript safe integer range - checkJavaScriptSafeBounds(pass, field, fieldName, ident.Name, minimum, maximum) + checkJavaScriptSafeBounds(pass, field, fieldName, typeDesc, minimum, maximum) } -// getNumericTypeIdent returns the identifier for int32 or int64 types. +// getNumericTypeIdent returns the identifier for numeric types (int32, int64, float32, float64). // It handles type aliases by looking up the underlying type. // Note: This function expects pointers and slices to already be unwrapped. func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { @@ -134,12 +150,12 @@ func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { return nil } - // Check if it's a basic int32 or int64 type - if ident.Name == "int32" || ident.Name == "int64" { + // Check if it's a basic numeric type we care about + if ident.Name == "int32" || ident.Name == "int64" || ident.Name == "float32" || ident.Name == "float64" { return ident } - // Check if it's a type alias to int32 or int64 + // Check if it's a type alias to a numeric type if !utils.IsBasicType(pass, ident) { typeSpec, ok := utils.LookupTypeSpec(pass, ident) if ok { @@ -152,6 +168,9 @@ func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { // unwrapType unwraps pointers and slices to get the underlying type. // Returns the unwrapped type and a boolean indicating if it's a slice. +// When the field is a slice, we extract the element type since that's what +// needs bounds validation (e.g., []int32 -> int32). The isSlice flag allows +// the caller to report errors with "array element type" for clarity. func unwrapType(expr ast.Expr) (ast.Expr, bool) { isSlice := false @@ -160,12 +179,12 @@ func unwrapType(expr ast.Expr) (ast.Expr, bool) { expr = starExpr.X } - // Check if it's a slice and unwrap (e.g., []int32) + // Check if it's a slice and unwrap to get element type (e.g., []int32 -> int32) if arrayType, ok := expr.(*ast.ArrayType); ok { isSlice = true expr = arrayType.Elt - // Handle pointer inside slice (e.g., []*int32) + // Handle pointer inside slice (e.g., []*int32 -> int32) if starExpr, ok := expr.(*ast.StarExpr); ok { expr = starExpr.X } @@ -176,44 +195,87 @@ func unwrapType(expr ast.Expr) (ast.Expr, bool) { // getMarkerNames returns the appropriate minimum and maximum marker names // based on whether the field is a slice. -func getMarkerNames(isSlice bool) (minMarker, maxMarker string) { +// Returns both kubebuilder and k8s declarative validation markers. +func getMarkerNames(isSlice bool) (minMarkers, maxMarkers []string) { if isSlice { - return markers.KubebuilderItemsMinimumMarker, markers.KubebuilderItemsMaximumMarker + return []string{markers.KubebuilderItemsMinimumMarker}, []string{markers.KubebuilderItemsMaximumMarker} } - return markers.KubebuilderMinimumMarker, markers.KubebuilderMaximumMarker + return []string{markers.KubebuilderMinimumMarker, markers.K8sMinimumMarker}, []string{markers.KubebuilderMaximumMarker, markers.K8sMaximumMarker} } -// getMarkerNumericValue extracts the numeric value from the first instance of the marker with the given name. -func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerName string) (float64, error) { - markerList := markerSet.Get(markerName) - if len(markerList) == 0 { - return 0, errMarkerMissingValue - } +// getMarkerNumericValue extracts the numeric value from the first instance of any of the given marker names. +// Checks multiple marker names to support both kubebuilder and k8s declarative validation markers. +func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []string) (float64, error) { + for _, markerName := range markerNames { + markerList := markerSet.Get(markerName) + if len(markerList) == 0 { + continue + } - marker := markerList[0] - rawValue, ok := marker.Expressions[""] - if !ok { - return 0, errMarkerMissingValue - } + marker := markerList[0] + rawValue, ok := marker.Expressions[""] + if !ok { + continue + } - // Parse as float64 using strconv for better error handling - value, err := strconv.ParseFloat(rawValue, 64) - if err != nil { - return 0, fmt.Errorf("error converting value to number: %w", err) + // Parse as float64 using strconv for better error handling + value, err := strconv.ParseFloat(rawValue, 64) + if err != nil { + return 0, fmt.Errorf("error converting value to number: %w", err) + } + + return value, nil } - return value, nil + return 0, errMarkerMissingValue +} + +// checkBoundsWithinTypeRange validates that the bounds are within the valid range for the type. +func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, fieldName, typeDesc string, minimum, maximum float64) { + // Extract the actual type name from typeDesc (e.g., "array element type of int32" -> "int32") + typeName := extractTypeName(typeDesc) + + switch typeName { + case "int32": + if minimum < minInt32 || minimum > maxInt32 { + 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) + } + if maximum < minInt32 || maximum > maxInt32 { + 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) + } + case "float32": + if minimum < minFloat32 || minimum > maxFloat32 { + pass.Reportf(field.Pos(), "field %s %s has minimum bound %v that is outside the valid float32 range", fieldName, typeDesc, minimum) + } + if maximum < minFloat32 || maximum > maxFloat32 { + pass.Reportf(field.Pos(), "field %s %s has maximum bound %v that is outside the valid float32 range", fieldName, typeDesc, maximum) + } + } } // checkJavaScriptSafeBounds checks if int64 bounds are within JavaScript safe integer range. -func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeName string, minimum, maximum float64) { +func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeDesc string, minimum, maximum float64) { + // Extract the actual type name from typeDesc + typeName := extractTypeName(typeDesc) + if typeName != "int64" { return } - if minimum < minSafeInt || maximum > maxSafeInt { + if minimum < minSafeInt64 || maximum > maxSafeInt64 { pass.Reportf(field.Pos(), - "field %s of type int64 has bounds [%d, %d] that exceed safe integer range [%d, %d]. Consider using a string type to avoid precision loss in JavaScript clients", - fieldName, int64(minimum), int64(maximum), minSafeInt, maxSafeInt) + "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", + fieldName, typeDesc, int64(minimum), int64(maximum), minSafeInt64, maxSafeInt64) + } +} + +// extractTypeName extracts the base type name from a type description. +// E.g., "array element type of int32" -> "int32", "int64" -> "int64" +func extractTypeName(typeDesc string) string { + // Check if it's an array element type description + const prefix = "array element type of " + if len(typeDesc) > len(prefix) && typeDesc[:len(prefix)] == prefix { + return typeDesc[len(prefix):] } + return typeDesc } diff --git a/pkg/analysis/numericbounds/doc.go b/pkg/analysis/numericbounds/doc.go index ad41361b..7a1ebbed 100644 --- a/pkg/analysis/numericbounds/doc.go +++ b/pkg/analysis/numericbounds/doc.go @@ -21,23 +21,27 @@ According to Kubernetes API conventions, numeric fields should have appropriate checking to prevent values that are too small, negative (when not intended), or too large. This analyzer ensures that: - - int32 and int64 fields have both minimum and maximum bounds markers - - For slices of numeric types, the analyzer checks for items:Minimum and items:Maximum markers - - Type aliases to int32 or int64 are also checked + - int32, int64, float32, and float64 fields have both minimum and maximum bounds markers + - For slices of numeric types, the analyzer checks the element type for items:Minimum and items:Maximum markers + (e.g., []int32 checks if the int32 elements have bounds, not the array itself) + - Type aliases to numeric types are recursively resolved and checked - Pointer types (e.g., *int32, []*int64) are unwrapped and validated - - int64 fields with values outside the JavaScript safe integer range (-(2^53-1) to (2^53-1)) + - Both kubebuilder and k8s declarative validation markers are supported + - Bounds values are validated to be within the type's range (e.g., int32 bounds must fit in int32) + - int64 fields with values outside the JavaScript safe integer range (±2^53-1) are flagged, as they may cause precision loss in JavaScript clients The analyzer checks for the presence of +kubebuilder:validation:Minimum and +kubebuilder:validation:Maximum markers on numeric fields, or the items: variants for slices. +It also supports +k8s:minimum and +k8s:maximum for declarative validation. For int64 fields, if the bounds exceed the JavaScript safe integer range of [-9007199254740991, 9007199254740991], the analyzer suggests using a string type instead to avoid precision loss in JavaScript environments. -Examples of valid and invalid code: +# Examples -Valid: +## Valid: Numeric field with proper bounds markers type Example struct { // +kubebuilder:validation:Minimum=0 @@ -45,10 +49,65 @@ Valid: Count int32 } -Invalid: +## Valid: Int64 field with JavaScript-safe bounds type Example struct { - Count int32 // Missing minimum and maximum markers + // +kubebuilder:validation:Minimum=-9007199254740991 + // +kubebuilder:validation:Maximum=9007199254740991 + Timestamp int64 + } + +## Valid: Float field with bounds + + type Example struct { + // +kubebuilder:validation:Minimum=0.0 + // +kubebuilder:validation:Maximum=100.0 + Ratio float32 + } + +## Valid: Slice with items bounds + + type Example struct { + // +kubebuilder:validation:items:Minimum=1 + // +kubebuilder:validation:items:Maximum=65535 + Ports []int32 + } + +## Valid: Using k8s declarative validation markers + + type Example struct { + // +k8s:minimum=0 + // +k8s:maximum=100 + Count int32 + } + +## Invalid: Missing bounds markers + + type Example struct { + Count int32 // want: should have minimum and maximum bounds validation markers + } + +## Invalid: Only one bound specified + + type Example struct { + // +kubebuilder:validation:Minimum=0 + Count int32 // want: has minimum but is missing maximum bounds validation marker + } + +## Invalid: Int64 with bounds exceeding JavaScript safe range + + type Example struct { + // +kubebuilder:validation:Minimum=-10000000000000000 + // +kubebuilder:validation:Maximum=10000000000000000 + LargeNumber int64 // want: bounds exceed JavaScript safe integer range + } + +## Invalid: Int32 bounds outside valid int32 range + + type Example struct { + // +kubebuilder:validation:Minimum=-3000000000 + // +kubebuilder:validation:Maximum=3000000000 + Count int32 // want: bounds outside valid int32 range } */ package numericbounds diff --git a/pkg/analysis/numericbounds/initializer.go b/pkg/analysis/numericbounds/initializer.go index cfd1b616..c81b4fc9 100644 --- a/pkg/analysis/numericbounds/initializer.go +++ b/pkg/analysis/numericbounds/initializer.go @@ -30,6 +30,6 @@ func Initializer() initializer.AnalyzerInitializer { return initializer.NewInitializer( name, Analyzer, - true, + false, // For now, CRD only, and so not on by default. ) } diff --git a/pkg/analysis/numericbounds/testdata/src/a/a.go b/pkg/analysis/numericbounds/testdata/src/a/a.go index cc9e7830..2ccebb4d 100644 --- a/pkg/analysis/numericbounds/testdata/src/a/a.go +++ b/pkg/analysis/numericbounds/testdata/src/a/a.go @@ -23,57 +23,57 @@ type ValidInt64WithJSSafeBounds struct { // InvalidInt32NoBounds should have bounds markers type InvalidInt32NoBounds struct { - NoBounds int32 // want "field NoBounds of type int32 should have minimum and maximum bounds validation markers" + NoBounds int32 // want "field NoBounds int32 is missing minimum bounds validation marker" "field NoBounds int32 is missing maximum bounds validation marker" } // InvalidInt64NoBounds should have bounds markers type InvalidInt64NoBounds struct { - NoBounds int64 // want "field NoBounds of type int64 should have minimum and maximum bounds validation markers" + NoBounds int64 // want "field NoBounds int64 is missing minimum bounds validation marker" "field NoBounds int64 is missing maximum bounds validation marker" } // InvalidInt32OnlyMin should have maximum marker type InvalidInt32OnlyMin struct { // +kubebuilder:validation:Minimum=0 - OnlyMin int32 // want "field OnlyMin of type int32 has minimum but is missing maximum bounds validation marker" + OnlyMin int32 // want "field OnlyMin int32 is missing maximum bounds validation marker" } // InvalidInt32OnlyMax should have minimum marker type InvalidInt32OnlyMax struct { // +kubebuilder:validation:Maximum=100 - OnlyMax int32 // want "field OnlyMax of type int32 has maximum but is missing minimum bounds validation marker" + OnlyMax int32 // want "field OnlyMax int32 is missing minimum bounds validation marker" } // InvalidInt64OnlyMin should have maximum marker type InvalidInt64OnlyMin struct { // +kubebuilder:validation:Minimum=0 - OnlyMin int64 // want "field OnlyMin of type int64 has minimum but is missing maximum bounds validation marker" + OnlyMin int64 // want "field OnlyMin int64 is missing maximum bounds validation marker" } // InvalidInt64OnlyMax should have minimum marker type InvalidInt64OnlyMax struct { // +kubebuilder:validation:Maximum=100 - OnlyMax int64 // want "field OnlyMax of type int64 has maximum but is missing minimum bounds validation marker" + OnlyMax int64 // want "field OnlyMax int64 is missing minimum bounds validation marker" } // InvalidInt64ExceedsJSMaxBounds has maximum that exceeds JavaScript safe integer range type InvalidInt64ExceedsJSMaxBounds struct { // +kubebuilder:validation:Minimum=-1000 // +kubebuilder:validation:Maximum=9007199254740992 - UnsafeMax int64 // want "field UnsafeMax of type int64 has bounds \\[-1000, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeMax int64 // want "field UnsafeMax int64 has bounds \\[-1000, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // InvalidInt64ExceedsJSMinBounds has minimum that exceeds JavaScript safe integer range type InvalidInt64ExceedsJSMinBounds struct { // +kubebuilder:validation:Minimum=-9007199254740992 // +kubebuilder:validation:Maximum=1000 - UnsafeMin int64 // want "field UnsafeMin of type int64 has bounds \\[-9007199254740992, 1000\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeMin int64 // want "field UnsafeMin int64 has bounds \\[-9007199254740992, 1000\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // InvalidInt64ExceedsJSBothBounds has both bounds exceeding JavaScript safe integer range type InvalidInt64ExceedsJSBothBounds struct { // +kubebuilder:validation:Minimum=-9007199254740992 // +kubebuilder:validation:Maximum=9007199254740992 - UnsafeBoth int64 // want "field UnsafeBoth of type int64 has bounds \\[-9007199254740992, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeBoth int64 // want "field UnsafeBoth int64 has bounds \\[-9007199254740992, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // IgnoredStringField should not be checked @@ -86,18 +86,37 @@ type IgnoredBoolField struct { Enabled bool } -// IgnoredFloat64Field should not be checked -type IgnoredFloat64Field struct { +// ValidFloat64WithBounds has proper bounds validation +type ValidFloat64WithBounds struct { + // +kubebuilder:validation:Minimum=-1000.5 + // +kubebuilder:validation:Maximum=1000.5 Value float64 } +// ValidFloat32WithBounds has proper bounds validation +type ValidFloat32WithBounds struct { + // +kubebuilder:validation:Minimum=0.0 + // +kubebuilder:validation:Maximum=100.0 + Ratio float32 +} + +// InvalidFloat64NoBounds should have bounds markers +type InvalidFloat64NoBounds struct { + Value float64 // want "field Value float64 is missing minimum bounds validation marker" "field Value float64 is missing maximum bounds validation marker" +} + +// InvalidFloat32NoBounds should have bounds markers +type InvalidFloat32NoBounds struct { + Ratio float32 // want "field Ratio float32 is missing minimum bounds validation marker" "field Ratio float32 is missing maximum bounds validation marker" +} + // MixedFields has both valid and invalid fields type MixedFields struct { // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=100 ValidCount int32 - InvalidCount int32 // want "field InvalidCount of type int32 should have minimum and maximum bounds validation markers" + InvalidCount int32 // want "field InvalidCount int32 is missing minimum bounds validation marker" "field InvalidCount int32 is missing maximum bounds validation marker" Name string @@ -105,7 +124,7 @@ type MixedFields struct { // +kubebuilder:validation:Maximum=1000 ValidValue int64 - InvalidValue int64 // want "field InvalidValue of type int64 should have minimum and maximum bounds validation markers" + InvalidValue int64 // want "field InvalidValue int64 is missing minimum bounds validation marker" "field InvalidValue int64 is missing maximum bounds validation marker" } // PointerFields with pointers should also be checked @@ -114,18 +133,18 @@ type PointerFields struct { // +kubebuilder:validation:Maximum=100 ValidPointerWithBounds *int32 - InvalidPointer *int32 // want "field InvalidPointer of type int32 should have minimum and maximum bounds validation markers" + InvalidPointer *int32 // want "field InvalidPointer int32 is missing minimum bounds validation marker" "field InvalidPointer int32 is missing maximum bounds validation marker" - InvalidPointer64 *int64 // want "field InvalidPointer64 of type int64 should have minimum and maximum bounds validation markers" + InvalidPointer64 *int64 // want "field InvalidPointer64 int64 is missing minimum bounds validation marker" "field InvalidPointer64 int64 is missing maximum bounds validation marker" } // SliceFields with slices should check the element type type SliceFields struct { ValidSlice []string - InvalidSlice []int32 // want "field InvalidSlice of type int32 should have minimum and maximum bounds validation markers" + InvalidSlice []int32 // want "field InvalidSlice array element type of int32 is missing minimum bounds validation marker" "field InvalidSlice array element type of int32 is missing maximum bounds validation marker" - InvalidSlice64 []int64 // want "field InvalidSlice64 of type int64 should have minimum and maximum bounds validation markers" + InvalidSlice64 []int64 // want "field InvalidSlice64 array element type of int64 is missing minimum bounds validation marker" "field InvalidSlice64 array element type of int64 is missing maximum bounds validation marker" // +kubebuilder:validation:items:Minimum=0 // +kubebuilder:validation:items:Maximum=100 @@ -135,18 +154,75 @@ type SliceFields struct { // TypeAliasFields with type aliases should be checked type Int32Alias int32 type Int64Alias int64 +type Float32Alias float32 +type Float64Alias float64 + +// Type aliases with bounds on the type itself +// +kubebuilder:validation:Minimum=0 +// +kubebuilder:validation:Maximum=255 +type BoundedInt32Alias int32 + +// +kubebuilder:validation:Minimum=-1000 +// +kubebuilder:validation:Maximum=1000 +type BoundedInt64Alias int64 + +// +kubebuilder:validation:Minimum=0.0 +// +kubebuilder:validation:Maximum=1.0 +type BoundedFloat32Alias float32 + +// +kubebuilder:validation:Minimum=-100.5 +// +kubebuilder:validation:Maximum=100.5 +type BoundedFloat64Alias float64 type TypeAliasFields struct { // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=100 ValidAlias Int32Alias - InvalidAlias Int32Alias // want "field InvalidAlias of type int32 should have minimum and maximum bounds validation markers" + InvalidAlias Int32Alias // want "field InvalidAlias int32 is missing minimum bounds validation marker" "field InvalidAlias int32 is missing maximum bounds validation marker" + + InvalidAlias64 Int64Alias // want "field InvalidAlias64 int64 is missing minimum bounds validation marker" "field InvalidAlias64 int64 is missing maximum bounds validation marker" + + InvalidAliasFloat32 Float32Alias // want "field InvalidAliasFloat32 float32 is missing minimum bounds validation marker" "field InvalidAliasFloat32 float32 is missing maximum bounds validation marker" - InvalidAlias64 Int64Alias // want "field InvalidAlias64 of type int64 should have minimum and maximum bounds validation markers" + InvalidAliasFloat64 Float64Alias // want "field InvalidAliasFloat64 float64 is missing minimum bounds validation marker" "field InvalidAliasFloat64 float64 is missing maximum bounds validation marker" + + // Valid: bounds are on the type alias itself + ValidBoundedAlias BoundedInt32Alias + + ValidBoundedAlias64 BoundedInt64Alias + + ValidBoundedFloat32 BoundedFloat32Alias + + ValidBoundedFloat64 BoundedFloat64Alias } // PointerSliceFields with pointer slices should also be checked type PointerSliceFields struct { - InvalidPointerSlice []*int32 // want "field InvalidPointerSlice of type int32 should have minimum and maximum bounds validation markers" + InvalidPointerSlice []*int32 // want "field InvalidPointerSlice array element type of int32 is missing minimum bounds validation marker" "field InvalidPointerSlice array element type of int32 is missing maximum bounds validation marker" +} + +// K8sDeclarativeValidation with k8s declarative validation markers should work +type K8sDeclarativeValidation struct { + // +k8s:minimum=0 + // +k8s:maximum=100 + ValidWithK8sMarkers int32 + + // +k8s:minimum=-1000 + // +k8s:maximum=1000 + ValidInt64WithK8s int64 +} + +// InvalidInt32BoundsOutOfRange has int32 bounds outside the valid int32 range +type InvalidInt32BoundsOutOfRange struct { + // +kubebuilder:validation:Minimum=-3000000000 + // +kubebuilder:validation:Maximum=3000000000 + OutOfRange int32 // want "field OutOfRange int32 has minimum bound -3e\\+09 that is outside the valid int32 range" "field OutOfRange int32 has maximum bound 3e\\+09 that is outside the valid int32 range" +} + +// MixedMarkersKubebuilderAndK8s can use either kubebuilder or k8s markers +type MixedMarkersKubebuilderAndK8s struct { + // +kubebuilder:validation:Minimum=0 + // +k8s:maximum=100 + MixedMarkers int32 } From e73e30ae1233be857caed6915231b7d1b4f8e056 Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Fri, 7 Nov 2025 10:36:57 -0500 Subject: [PATCH 4/8] added float64 and but removed unused int64. Updated Docs, initializer, and used utils --- docs/linters.md | 25 +-- pkg/analysis/numericbounds/analyzer.go | 211 ++++++------------ pkg/analysis/numericbounds/doc.go | 105 ++------- pkg/analysis/numericbounds/initializer.go | 2 +- .../numericbounds/testdata/src/a/a.go | 46 ++-- pkg/analysis/utils/utils.go | 2 +- pkg/analysis/utils/zero_value.go | 21 +- 7 files changed, 133 insertions(+), 279 deletions(-) diff --git a/docs/linters.md b/docs/linters.md index ce4b892a..4b203156 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -486,28 +486,15 @@ According to Kubernetes API conventions, numeric fields should have bounds check This linter ensures that: - Numeric fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers -- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported -- Bounds values are within the type's valid range (e.g., int32 bounds must fit in int32) -- `int64` fields with bounds outside the JavaScript safe integer range (±2^53-1) are flagged +- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported +- Bounds values are within the type's valid range: + - int32: full int32 range (±2^31-1) + - int64: JavaScript-safe range (±2^53-1) per Kubernetes API conventions + - float32/float64: within their respective ranges **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. -### Configuration - -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. - -To enable in `.golangci.yml`: -```yaml -linters-settings: - custom: - kubeapilinter: - settings: - linters: - enable: - - numericbounds -``` - -See the [package documentation](https://pkg.go.dev/sigs.k8s.io/kube-api-linter/pkg/analysis/numericbounds) for detailed examples and usage. +This linter is **not enabled by default** as it is primarily focused on CRD validation with kubebuilder markers. ## NoBools diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index f76b73b2..23359cef 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" "go/ast" - "strconv" "golang.org/x/tools/go/analysis" kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" @@ -34,18 +33,20 @@ const ( name = "numericbounds" ) -// Type bounds for validation +// Type bounds for validation. const ( - maxInt32 = 2147483647 // 2^31 - 1 - minInt32 = -2147483648 // -2^31 - maxFloat32 = 3.40282346638528859811704183484516925440e+38 // max float32 - minFloat32 = -3.40282346638528859811704183484516925440e+38 // min float32 + maxInt32 = 2147483647 // 2^31 - 1 + minInt32 = -2147483648 // -2^31 + maxFloat32 = 3.40282346638528859811704183484516925440e+38 // max float32 + minFloat32 = -3.40282346638528859811704183484516925440e+38 // min float32 + maxFloat64 = 1.797693134862315708145274237317043567981e+308 // max float64 + minFloat64 = -1.797693134862315708145274237317043567981e+308 // min float64 ) -// JavaScript safe integer bounds (2^53 - 1 and -(2^53 - 1)) +// JavaScript safe integer bounds for int64 (±2^53-1). +// Per Kubernetes API conventions, int64 fields should use bounds within this range +// to ensure compatibility with JavaScript clients. const ( - maxSafeInt32 = 2147483647 // 2^31 - 1 (fits in JS Number) - minSafeInt32 = -2147483648 // -2^31 (fits in JS Number) maxSafeInt64 = 9007199254740991 // 2^53 - 1 (max safe integer in JS) minSafeInt64 = -9007199254740991 // -(2^53 - 1) (min safe integer in JS) ) @@ -67,41 +68,35 @@ func run(pass *analysis.Pass) (any, error) { return nil, kalerrors.ErrCouldNotGetInspector } - inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) { - checkField(pass, field, markersAccess) + inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, _ string) { + // Create TypeChecker with closure capturing markersAccess + typeChecker := utils.NewTypeChecker(func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) { + checkNumericType(pass, ident, node, prefix, markersAccess) + }) + + typeChecker.CheckNode(pass, field) }) return nil, nil //nolint:nilnil } -func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) { - fieldName := utils.FieldName(field) - if fieldName == "" { - return - } - - // Unwrap pointers and slices to get the underlying type - fieldType, isSlice := unwrapType(field.Type) - - // Get the underlying numeric type identifier (int32, int64, float32, float64) - ident := getNumericTypeIdent(pass, fieldType) - if ident == nil { - return - } - +//nolint:cyclop +func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string, markersAccess markershelper.Markers) { // Only check int32, int64, float32, and float64 types if ident.Name != "int32" && ident.Name != "int64" && ident.Name != "float32" && ident.Name != "float64" { return } - // Create type description that clarifies array element types - typeDesc := ident.Name - if isSlice { - typeDesc = fmt.Sprintf("array element type of %s", ident.Name) + field, ok := node.(*ast.Field) + if !ok { + return } fieldMarkers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field) + // Determine if this is a slice/array field + isSlice := utils.IsArrayTypeOrAlias(pass, field) + // Determine which markers to look for based on whether the field is a slice minMarkers, maxMarkers := getMarkerNames(isSlice) @@ -115,18 +110,20 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp // Report any invalid marker values (e.g., non-numeric values) if minErr != nil && !minMissing { - pass.Reportf(field.Pos(), "field %s has an invalid minimum marker: %v", fieldName, minErr) + pass.Reportf(field.Pos(), "%s has an invalid minimum marker: %v", prefix, minErr) } + if maxErr != nil && !maxMissing { - pass.Reportf(field.Pos(), "field %s has an invalid maximum marker: %v", fieldName, maxErr) + pass.Reportf(field.Pos(), "%s has an invalid maximum marker: %v", prefix, maxErr) } // Report if markers are missing if minMissing { - pass.Reportf(field.Pos(), "field %s %s is missing minimum bounds validation marker", fieldName, typeDesc) + pass.Reportf(field.Pos(), "%s is missing minimum bounds validation marker", prefix) } + if maxMissing { - pass.Reportf(field.Pos(), "field %s %s is missing maximum bounds validation marker", fieldName, typeDesc) + pass.Reportf(field.Pos(), "%s is missing maximum bounds validation marker", prefix) } // If any markers are missing or invalid, don't continue with bounds checks @@ -134,63 +131,8 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelp return } - // Validate bounds are within the type's range - checkBoundsWithinTypeRange(pass, field, fieldName, typeDesc, minimum, maximum) - - // For int64 fields, check if bounds are within JavaScript safe integer range - checkJavaScriptSafeBounds(pass, field, fieldName, typeDesc, minimum, maximum) -} - -// getNumericTypeIdent returns the identifier for numeric types (int32, int64, float32, float64). -// It handles type aliases by looking up the underlying type. -// Note: This function expects pointers and slices to already be unwrapped. -func getNumericTypeIdent(pass *analysis.Pass, expr ast.Expr) *ast.Ident { - ident, ok := expr.(*ast.Ident) - if !ok { - return nil - } - - // Check if it's a basic numeric type we care about - if ident.Name == "int32" || ident.Name == "int64" || ident.Name == "float32" || ident.Name == "float64" { - return ident - } - - // Check if it's a type alias to a numeric type - if !utils.IsBasicType(pass, ident) { - typeSpec, ok := utils.LookupTypeSpec(pass, ident) - if ok { - return getNumericTypeIdent(pass, typeSpec.Type) - } - } - - return nil -} - -// unwrapType unwraps pointers and slices to get the underlying type. -// Returns the unwrapped type and a boolean indicating if it's a slice. -// When the field is a slice, we extract the element type since that's what -// needs bounds validation (e.g., []int32 -> int32). The isSlice flag allows -// the caller to report errors with "array element type" for clarity. -func unwrapType(expr ast.Expr) (ast.Expr, bool) { - isSlice := false - - // Unwrap pointer if present (e.g., *int32) - if starExpr, ok := expr.(*ast.StarExpr); ok { - expr = starExpr.X - } - - // Check if it's a slice and unwrap to get element type (e.g., []int32 -> int32) - if arrayType, ok := expr.(*ast.ArrayType); ok { - isSlice = true - expr = arrayType.Elt - - // Handle pointer inside slice (e.g., []*int32 -> int32) - if starExpr, ok := expr.(*ast.StarExpr); ok { - expr = starExpr.X - } - } - - return expr, isSlice + // Validate bounds are within the type's valid range + checkBoundsWithinTypeRange(pass, field, prefix, ident.Name, minimum, maximum) } // getMarkerNames returns the appropriate minimum and maximum marker names @@ -200,6 +142,7 @@ func getMarkerNames(isSlice bool) (minMarkers, maxMarkers []string) { if isSlice { return []string{markers.KubebuilderItemsMinimumMarker}, []string{markers.KubebuilderItemsMaximumMarker} } + return []string{markers.KubebuilderMinimumMarker, markers.K8sMinimumMarker}, []string{markers.KubebuilderMaximumMarker, markers.K8sMaximumMarker} } @@ -212,16 +155,14 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri continue } - marker := markerList[0] - rawValue, ok := marker.Expressions[""] - if !ok { - continue - } - - // Parse as float64 using strconv for better error handling - value, err := strconv.ParseFloat(rawValue, 64) + // Use the exported utils function to parse the marker value + value, err := utils.GetMarkerNumericValue[float64](markerList[0]) if err != nil { - return 0, fmt.Errorf("error converting value to number: %w", err) + if errors.Is(err, errMarkerMissingValue) { + continue + } + + return 0, fmt.Errorf("error getting marker value: %w", err) } return value, nil @@ -231,51 +172,45 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri } // checkBoundsWithinTypeRange validates that the bounds are within the valid range for the type. -func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, fieldName, typeDesc string, minimum, maximum float64) { - // Extract the actual type name from typeDesc (e.g., "array element type of int32" -> "int32") - typeName := extractTypeName(typeDesc) - +// For int64, enforces JavaScript-safe bounds as per Kubernetes API conventions to ensure +// compatibility with JavaScript clients. +// +//nolint:cyclop +func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, prefix, typeName string, minimum, maximum float64) { switch typeName { case "int32": - if minimum < minInt32 || minimum > maxInt32 { - 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) + if minimum < float64(minInt32) || minimum > float64(maxInt32) { + pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid int32 range [%d, %d]", prefix, minimum, minInt32, maxInt32) } - if maximum < minInt32 || maximum > maxInt32 { - 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) - } - case "float32": - if minimum < minFloat32 || minimum > maxFloat32 { - pass.Reportf(field.Pos(), "field %s %s has minimum bound %v that is outside the valid float32 range", fieldName, typeDesc, minimum) + + if maximum < float64(minInt32) || maximum > float64(maxInt32) { + pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid int32 range [%d, %d]", prefix, maximum, minInt32, maxInt32) } - if maximum < minFloat32 || maximum > maxFloat32 { - pass.Reportf(field.Pos(), "field %s %s has maximum bound %v that is outside the valid float32 range", fieldName, typeDesc, maximum) + case "int64": + // Kubernetes API conventions enforce JavaScript-safe bounds for int64 (±2^53-1) + // to ensure compatibility with JavaScript clients + if minimum < float64(minSafeInt64) { + 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) } - } -} -// checkJavaScriptSafeBounds checks if int64 bounds are within JavaScript safe integer range. -func checkJavaScriptSafeBounds(pass *analysis.Pass, field *ast.Field, fieldName, typeDesc string, minimum, maximum float64) { - // Extract the actual type name from typeDesc - typeName := extractTypeName(typeDesc) - - if typeName != "int64" { - return - } + if maximum > float64(maxSafeInt64) { + 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) + } + case "float32": + if minimum < float64(minFloat32) || minimum > float64(maxFloat32) { + pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid float32 range", prefix, minimum) + } - if minimum < minSafeInt64 || maximum > maxSafeInt64 { - pass.Reportf(field.Pos(), - "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", - fieldName, typeDesc, int64(minimum), int64(maximum), minSafeInt64, maxSafeInt64) - } -} + if maximum < float64(minFloat32) || maximum > float64(maxFloat32) { + pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid float32 range", prefix, maximum) + } + case "float64": + if minimum < minFloat64 || minimum > maxFloat64 { + pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid float64 range", prefix, minimum) + } -// extractTypeName extracts the base type name from a type description. -// E.g., "array element type of int32" -> "int32", "int64" -> "int64" -func extractTypeName(typeDesc string) string { - // Check if it's an array element type description - const prefix = "array element type of " - if len(typeDesc) > len(prefix) && typeDesc[:len(prefix)] == prefix { - return typeDesc[len(prefix):] + if maximum < minFloat64 || maximum > maxFloat64 { + pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid float64 range", prefix, maximum) + } } - return typeDesc } diff --git a/pkg/analysis/numericbounds/doc.go b/pkg/analysis/numericbounds/doc.go index 7a1ebbed..7cd08e9e 100644 --- a/pkg/analysis/numericbounds/doc.go +++ b/pkg/analysis/numericbounds/doc.go @@ -15,99 +15,30 @@ limitations under the License. */ /* -numericbounds is an analyzer that checks for proper bounds validation on numeric fields. +numericbounds is an analyzer that checks numeric fields have appropriate bounds validation markers. -According to Kubernetes API conventions, numeric fields should have appropriate bounds -checking to prevent values that are too small, negative (when not intended), or too large. +According to Kubernetes API conventions, numeric fields should have bounds checking to prevent +values that are too small, negative (when not intended), or too large. -This analyzer ensures that: - - int32, int64, float32, and float64 fields have both minimum and maximum bounds markers - - For slices of numeric types, the analyzer checks the element type for items:Minimum and items:Maximum markers - (e.g., []int32 checks if the int32 elements have bounds, not the array itself) - - Type aliases to numeric types are recursively resolved and checked - - Pointer types (e.g., *int32, []*int64) are unwrapped and validated - - Both kubebuilder and k8s declarative validation markers are supported - - Bounds values are validated to be within the type's range (e.g., int32 bounds must fit in int32) - - int64 fields with values outside the JavaScript safe integer range (±2^53-1) - are flagged, as they may cause precision loss in JavaScript clients +The analyzer checks that int32, int64, float32, and float64 fields have both minimum and +maximum bounds markers. It supports both kubebuilder markers (+kubebuilder:validation:Minimum/Maximum) +and k8s declarative validation markers (+k8s:minimum/maximum). -The analyzer checks for the presence of +kubebuilder:validation:Minimum and -+kubebuilder:validation:Maximum markers on numeric fields, or the items: variants for slices. -It also supports +k8s:minimum and +k8s:maximum for declarative validation. +For slices of numeric types, the analyzer checks the element type for items:Minimum and items:Maximum markers. -For int64 fields, if the bounds exceed the JavaScript safe integer range of -[-9007199254740991, 9007199254740991], the analyzer suggests using a string type instead -to avoid precision loss in JavaScript environments. +Type aliases are resolved and checked. Pointer types are unwrapped and validated. -# Examples +Bounds values are validated to be within the type's range: + - int32: full int32 range (±2^31-1) + - int64: JavaScript-safe range (±2^53-1) per Kubernetes API conventions + - float32/float64: within their respective ranges -## Valid: Numeric field with proper bounds markers +For int64 fields, Kubernetes API conventions enforce JavaScript-safe bounds (±2^53-1) +to ensure compatibility with JavaScript clients and prevent precision loss. - type Example struct { - // +kubebuilder:validation:Minimum=0 - // +kubebuilder:validation:Maximum=100 - Count int32 - } - -## Valid: Int64 field with JavaScript-safe bounds - - type Example struct { - // +kubebuilder:validation:Minimum=-9007199254740991 - // +kubebuilder:validation:Maximum=9007199254740991 - Timestamp int64 - } - -## Valid: Float field with bounds - - type Example struct { - // +kubebuilder:validation:Minimum=0.0 - // +kubebuilder:validation:Maximum=100.0 - Ratio float32 - } - -## Valid: Slice with items bounds - - type Example struct { - // +kubebuilder:validation:items:Minimum=1 - // +kubebuilder:validation:items:Maximum=65535 - Ports []int32 - } - -## Valid: Using k8s declarative validation markers - - type Example struct { - // +k8s:minimum=0 - // +k8s:maximum=100 - Count int32 - } - -## Invalid: Missing bounds markers - - type Example struct { - Count int32 // want: should have minimum and maximum bounds validation markers - } - -## Invalid: Only one bound specified - - type Example struct { - // +kubebuilder:validation:Minimum=0 - Count int32 // want: has minimum but is missing maximum bounds validation marker - } - -## Invalid: Int64 with bounds exceeding JavaScript safe range - - type Example struct { - // +kubebuilder:validation:Minimum=-10000000000000000 - // +kubebuilder:validation:Maximum=10000000000000000 - LargeNumber int64 // want: bounds exceed JavaScript safe integer range - } - -## Invalid: Int32 bounds outside valid int32 range - - type Example struct { - // +kubebuilder:validation:Minimum=-3000000000 - // +kubebuilder:validation:Maximum=3000000000 - Count int32 // want: bounds outside valid int32 range - } +For arrays of numeric types, the minimum/maximum of each element can be set using ++kubebuilder:validation:items:Minimum and +kubebuilder:validation:items:Maximum markers. +Alternatively, if the array uses a numeric type alias, the markers can be placed on the +alias type definition itself. */ package numericbounds diff --git a/pkg/analysis/numericbounds/initializer.go b/pkg/analysis/numericbounds/initializer.go index c81b4fc9..02c2baf9 100644 --- a/pkg/analysis/numericbounds/initializer.go +++ b/pkg/analysis/numericbounds/initializer.go @@ -30,6 +30,6 @@ func Initializer() initializer.AnalyzerInitializer { return initializer.NewInitializer( name, Analyzer, - false, // For now, CRD only, and so not on by default. + false, ) } diff --git a/pkg/analysis/numericbounds/testdata/src/a/a.go b/pkg/analysis/numericbounds/testdata/src/a/a.go index 2ccebb4d..14001442 100644 --- a/pkg/analysis/numericbounds/testdata/src/a/a.go +++ b/pkg/analysis/numericbounds/testdata/src/a/a.go @@ -23,57 +23,57 @@ type ValidInt64WithJSSafeBounds struct { // InvalidInt32NoBounds should have bounds markers type InvalidInt32NoBounds struct { - NoBounds int32 // want "field NoBounds int32 is missing minimum bounds validation marker" "field NoBounds int32 is missing maximum bounds validation marker" + NoBounds int32 // want "field NoBounds is missing minimum bounds validation marker" "field NoBounds is missing maximum bounds validation marker" } // InvalidInt64NoBounds should have bounds markers type InvalidInt64NoBounds struct { - NoBounds int64 // want "field NoBounds int64 is missing minimum bounds validation marker" "field NoBounds int64 is missing maximum bounds validation marker" + NoBounds int64 // want "field NoBounds is missing minimum bounds validation marker" "field NoBounds is missing maximum bounds validation marker" } // InvalidInt32OnlyMin should have maximum marker type InvalidInt32OnlyMin struct { // +kubebuilder:validation:Minimum=0 - OnlyMin int32 // want "field OnlyMin int32 is missing maximum bounds validation marker" + OnlyMin int32 // want "field OnlyMin is missing maximum bounds validation marker" } // InvalidInt32OnlyMax should have minimum marker type InvalidInt32OnlyMax struct { // +kubebuilder:validation:Maximum=100 - OnlyMax int32 // want "field OnlyMax int32 is missing minimum bounds validation marker" + OnlyMax int32 // want "field OnlyMax is missing minimum bounds validation marker" } // InvalidInt64OnlyMin should have maximum marker type InvalidInt64OnlyMin struct { // +kubebuilder:validation:Minimum=0 - OnlyMin int64 // want "field OnlyMin int64 is missing maximum bounds validation marker" + OnlyMin int64 // want "field OnlyMin is missing maximum bounds validation marker" } // InvalidInt64OnlyMax should have minimum marker type InvalidInt64OnlyMax struct { // +kubebuilder:validation:Maximum=100 - OnlyMax int64 // want "field OnlyMax int64 is missing minimum bounds validation marker" + OnlyMax int64 // want "field OnlyMax is missing minimum bounds validation marker" } // InvalidInt64ExceedsJSMaxBounds has maximum that exceeds JavaScript safe integer range type InvalidInt64ExceedsJSMaxBounds struct { // +kubebuilder:validation:Minimum=-1000 // +kubebuilder:validation:Maximum=9007199254740992 - UnsafeMax int64 // want "field UnsafeMax int64 has bounds \\[-1000, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeMax int64 // want "field UnsafeMax has maximum bound 9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // InvalidInt64ExceedsJSMinBounds has minimum that exceeds JavaScript safe integer range type InvalidInt64ExceedsJSMinBounds struct { // +kubebuilder:validation:Minimum=-9007199254740992 // +kubebuilder:validation:Maximum=1000 - UnsafeMin int64 // want "field UnsafeMin int64 has bounds \\[-9007199254740992, 1000\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeMin int64 // want "field UnsafeMin has minimum bound -9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // InvalidInt64ExceedsJSBothBounds has both bounds exceeding JavaScript safe integer range type InvalidInt64ExceedsJSBothBounds struct { // +kubebuilder:validation:Minimum=-9007199254740992 // +kubebuilder:validation:Maximum=9007199254740992 - UnsafeBoth int64 // want "field UnsafeBoth int64 has bounds \\[-9007199254740992, 9007199254740992\\] that exceed safe integer range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeBoth int64 // want "field UnsafeBoth has minimum bound -9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" "field UnsafeBoth has maximum bound 9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // IgnoredStringField should not be checked @@ -102,12 +102,12 @@ type ValidFloat32WithBounds struct { // InvalidFloat64NoBounds should have bounds markers type InvalidFloat64NoBounds struct { - Value float64 // want "field Value float64 is missing minimum bounds validation marker" "field Value float64 is missing maximum bounds validation marker" + Value float64 // want "field Value is missing minimum bounds validation marker" "field Value is missing maximum bounds validation marker" } // InvalidFloat32NoBounds should have bounds markers type InvalidFloat32NoBounds struct { - Ratio float32 // want "field Ratio float32 is missing minimum bounds validation marker" "field Ratio float32 is missing maximum bounds validation marker" + Ratio float32 // want "field Ratio is missing minimum bounds validation marker" "field Ratio is missing maximum bounds validation marker" } // MixedFields has both valid and invalid fields @@ -116,7 +116,7 @@ type MixedFields struct { // +kubebuilder:validation:Maximum=100 ValidCount int32 - InvalidCount int32 // want "field InvalidCount int32 is missing minimum bounds validation marker" "field InvalidCount int32 is missing maximum bounds validation marker" + InvalidCount int32 // want "field InvalidCount is missing minimum bounds validation marker" "field InvalidCount is missing maximum bounds validation marker" Name string @@ -124,7 +124,7 @@ type MixedFields struct { // +kubebuilder:validation:Maximum=1000 ValidValue int64 - InvalidValue int64 // want "field InvalidValue int64 is missing minimum bounds validation marker" "field InvalidValue int64 is missing maximum bounds validation marker" + InvalidValue int64 // want "field InvalidValue is missing minimum bounds validation marker" "field InvalidValue is missing maximum bounds validation marker" } // PointerFields with pointers should also be checked @@ -133,18 +133,18 @@ type PointerFields struct { // +kubebuilder:validation:Maximum=100 ValidPointerWithBounds *int32 - InvalidPointer *int32 // want "field InvalidPointer int32 is missing minimum bounds validation marker" "field InvalidPointer int32 is missing maximum bounds validation marker" + InvalidPointer *int32 // want "field InvalidPointer pointer is missing minimum bounds validation marker" "field InvalidPointer pointer is missing maximum bounds validation marker" - InvalidPointer64 *int64 // want "field InvalidPointer64 int64 is missing minimum bounds validation marker" "field InvalidPointer64 int64 is missing maximum bounds validation marker" + InvalidPointer64 *int64 // want "field InvalidPointer64 pointer is missing minimum bounds validation marker" "field InvalidPointer64 pointer is missing maximum bounds validation marker" } // SliceFields with slices should check the element type type SliceFields struct { ValidSlice []string - InvalidSlice []int32 // want "field InvalidSlice array element type of int32 is missing minimum bounds validation marker" "field InvalidSlice array element type of int32 is missing maximum bounds validation marker" + InvalidSlice []int32 // want "field InvalidSlice array element is missing minimum bounds validation marker" "field InvalidSlice array element is missing maximum bounds validation marker" - InvalidSlice64 []int64 // want "field InvalidSlice64 array element type of int64 is missing minimum bounds validation marker" "field InvalidSlice64 array element type of int64 is missing maximum bounds validation marker" + InvalidSlice64 []int64 // want "field InvalidSlice64 array element is missing minimum bounds validation marker" "field InvalidSlice64 array element is missing maximum bounds validation marker" // +kubebuilder:validation:items:Minimum=0 // +kubebuilder:validation:items:Maximum=100 @@ -179,13 +179,13 @@ type TypeAliasFields struct { // +kubebuilder:validation:Maximum=100 ValidAlias Int32Alias - InvalidAlias Int32Alias // want "field InvalidAlias int32 is missing minimum bounds validation marker" "field InvalidAlias int32 is missing maximum bounds validation marker" + InvalidAlias Int32Alias // want "field InvalidAlias type Int32Alias is missing minimum bounds validation marker" "field InvalidAlias type Int32Alias is missing maximum bounds validation marker" - InvalidAlias64 Int64Alias // want "field InvalidAlias64 int64 is missing minimum bounds validation marker" "field InvalidAlias64 int64 is missing maximum bounds validation marker" + InvalidAlias64 Int64Alias // want "field InvalidAlias64 type Int64Alias is missing minimum bounds validation marker" "field InvalidAlias64 type Int64Alias is missing maximum bounds validation marker" - InvalidAliasFloat32 Float32Alias // want "field InvalidAliasFloat32 float32 is missing minimum bounds validation marker" "field InvalidAliasFloat32 float32 is missing maximum bounds validation marker" + InvalidAliasFloat32 Float32Alias // want "field InvalidAliasFloat32 type Float32Alias is missing minimum bounds validation marker" "field InvalidAliasFloat32 type Float32Alias is missing maximum bounds validation marker" - InvalidAliasFloat64 Float64Alias // want "field InvalidAliasFloat64 float64 is missing minimum bounds validation marker" "field InvalidAliasFloat64 float64 is missing maximum bounds validation marker" + InvalidAliasFloat64 Float64Alias // want "field InvalidAliasFloat64 type Float64Alias is missing minimum bounds validation marker" "field InvalidAliasFloat64 type Float64Alias is missing maximum bounds validation marker" // Valid: bounds are on the type alias itself ValidBoundedAlias BoundedInt32Alias @@ -199,7 +199,7 @@ type TypeAliasFields struct { // PointerSliceFields with pointer slices should also be checked type PointerSliceFields struct { - InvalidPointerSlice []*int32 // want "field InvalidPointerSlice array element type of int32 is missing minimum bounds validation marker" "field InvalidPointerSlice array element type of int32 is missing maximum bounds validation marker" + 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" } // K8sDeclarativeValidation with k8s declarative validation markers should work @@ -217,7 +217,7 @@ type K8sDeclarativeValidation struct { type InvalidInt32BoundsOutOfRange struct { // +kubebuilder:validation:Minimum=-3000000000 // +kubebuilder:validation:Maximum=3000000000 - OutOfRange int32 // want "field OutOfRange int32 has minimum bound -3e\\+09 that is outside the valid int32 range" "field OutOfRange int32 has maximum bound 3e\\+09 that is outside the valid int32 range" + 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" } // MixedMarkersKubebuilderAndK8s can use either kubebuilder or k8s markers diff --git a/pkg/analysis/utils/utils.go b/pkg/analysis/utils/utils.go index 71a481f5..1b27879d 100644 --- a/pkg/analysis/utils/utils.go +++ b/pkg/analysis/utils/utils.go @@ -363,7 +363,7 @@ func isTypeBasic(t types.Type) bool { // It returns a nil value when the marker is not present, and an error // when the marker is present, but malformed. func GetMinProperties(markerSet markershelper.MarkerSet) (*int, error) { - minProperties, err := getMarkerNumericValueByName[int](markerSet, markers.KubebuilderMinPropertiesMarker) + minProperties, err := GetMarkerNumericValueByName[int](markerSet, markers.KubebuilderMinPropertiesMarker) if err != nil && !errors.Is(err, errMarkerMissingValue) { return nil, fmt.Errorf("invalid format for minimum properties marker: %w", err) } diff --git a/pkg/analysis/utils/zero_value.go b/pkg/analysis/utils/zero_value.go index 94701664..71144b0b 100644 --- a/pkg/analysis/utils/zero_value.go +++ b/pkg/analysis/utils/zero_value.go @@ -221,7 +221,7 @@ func isArrayZeroValueValid(pass *analysis.Pass, field *ast.Field, arrayType *ast fieldMarkers := TypeAwareMarkerCollectionForField(pass, markersAccess, field) // For arrays, we can use a zero value if the array is not required to have a minimum number of items. - minItems, err := getMarkerNumericValueByName[int](fieldMarkers, markers.KubebuilderMinItemsMarker) + minItems, err := GetMarkerNumericValueByName[int](fieldMarkers, markers.KubebuilderMinItemsMarker) if err != nil && !errors.Is(err, errMarkerMissingValue) { return false, false } @@ -257,13 +257,13 @@ type number interface { func isNumericZeroValueValid[N number](pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) (bool, bool) { fieldMarkers := TypeAwareMarkerCollectionForField(pass, markersAccess, field) - minimum, err := getMarkerNumericValueByName[N](fieldMarkers, markers.KubebuilderMinimumMarker) + minimum, err := GetMarkerNumericValueByName[N](fieldMarkers, markers.KubebuilderMinimumMarker) if err != nil && !errors.Is(err, errMarkerMissingValue) { pass.Reportf(field.Pos(), "field %s has an invalid minimum marker: %v", FieldName(field), err) return false, false } - maximum, err := getMarkerNumericValueByName[N](fieldMarkers, markers.KubebuilderMaximumMarker) + maximum, err := GetMarkerNumericValueByName[N](fieldMarkers, markers.KubebuilderMaximumMarker) if err != nil && !errors.Is(err, errMarkerMissingValue) { pass.Reportf(field.Pos(), "field %s has an invalid maximum marker: %v", FieldName(field), err) return false, false @@ -276,15 +276,16 @@ func isNumericZeroValueValid[N number](pass *analysis.Pass, field *ast.Field, ma return ptr.Deref(minimum, -1) <= 0 && ptr.Deref(maximum, 1) >= 0, hasCompleteRange || hasGreaterThanZeroMinimum || hasLessThanZeroMaximum } -// getMarkerNumericValueByName extracts the numeric value from the first instance of the marker with the given name. -// Works for markers like MaxLength, MinLength, etc. -func getMarkerNumericValueByName[N number](marker markershelper.MarkerSet, markerName string) (*N, error) { +// GetMarkerNumericValueByName extracts a numeric value from a marker with the given name. +// Returns a pointer to the value, or nil if the marker is not found. +// Works for markers like MaxLength, MinLength, Minimum, Maximum, etc. +func GetMarkerNumericValueByName[N number](marker markershelper.MarkerSet, markerName string) (*N, error) { markerList := marker.Get(markerName) if len(markerList) == 0 { return nil, errMarkerMissingValue } - markerValue, err := getMarkerNumericValue[N](markerList[0]) + markerValue, err := GetMarkerNumericValue[N](markerList[0]) if err != nil { return nil, fmt.Errorf("error getting marker value: %w", err) } @@ -292,9 +293,9 @@ func getMarkerNumericValueByName[N number](marker markershelper.MarkerSet, marke return &markerValue, nil } -// getMarkerNumericValue extracts a numeric value from the default value of a marker. -// Works for markers like MaxLength, MinLength, etc. -func getMarkerNumericValue[N number](marker markershelper.Marker) (N, error) { +// GetMarkerNumericValue extracts a numeric value from the default value of a marker. +// Works for markers like MaxLength, MinLength, Minimum, Maximum, etc. +func GetMarkerNumericValue[N number](marker markershelper.Marker) (N, error) { if marker.Payload.Value == "" { return N(0), errMarkerMissingValue } From 798e62ab7504145aea5207d27af8e8cf922b4587 Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Fri, 7 Nov 2025 10:48:13 -0500 Subject: [PATCH 5/8] Type casting for safeInt64 --- pkg/analysis/numericbounds/analyzer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index 23359cef..1c5189b5 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -190,11 +190,11 @@ func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, prefix, t // Kubernetes API conventions enforce JavaScript-safe bounds for int64 (±2^53-1) // to ensure compatibility with JavaScript clients if minimum < float64(minSafeInt64) { - 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) + 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)) } if maximum > float64(maxSafeInt64) { - 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) + 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)) } case "float32": if minimum < float64(minFloat32) || minimum > float64(maxFloat32) { From a6609de39ef588ba7fa9b24de97c7ec8856b630b Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Wed, 12 Nov 2025 14:19:50 -0500 Subject: [PATCH 6/8] generics for bound checks, typos, updated tests for qualified field names, and other reviews --- docs/linters.md | 12 +-- go.mod | 1 + pkg/analysis/numericbounds/analyzer.go | 86 ++++++++++--------- .../numericbounds/testdata/src/a/a.go | 82 ++++++++++-------- 4 files changed, 99 insertions(+), 82 deletions(-) diff --git a/docs/linters.md b/docs/linters.md index 4b203156..ef39c2aa 100644 --- a/docs/linters.md +++ b/docs/linters.md @@ -485,12 +485,12 @@ The `numericbounds` linter checks that numeric fields (`int32`, `int64`, `float3 According to Kubernetes API conventions, numeric fields should have bounds checking to prevent values that are too small, negative (when not intended), or too large. This linter ensures that: -- Numeric fields have both `+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum` markers -- K8s declarative validation markers (`+k8s:minimum` and `+k8s:maximum`) are also supported -- Bounds values are within the type's valid range: - - int32: full int32 range (±2^31-1) - - int64: JavaScript-safe range (±2^53-1) per Kubernetes API conventions - - float32/float64: within their respective ranges +- Numeric fields have both `+k8s:minimum` and `+k8s:maximum` markers +- Kubebuilder validation markers (`+kubebuilder:validation:Minimum` and `+kubebuilder:validation:Maximum`) are also supported +- Bounds values are validated: + - int32: within int32 range (±2^31-1) + - int64: within JavaScript-safe range (±2^53-1) per K8s API conventions for JSON compatibility + - float32/float64: marker values are valid (within type ranges) **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. diff --git a/go.mod b/go.mod index 633691d4..af908fdc 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/google/go-cmp v0.7.0 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.38.0 + golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 golang.org/x/tools v0.37.0 k8s.io/apimachinery v0.32.3 k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index 1c5189b5..7451b3ec 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -20,6 +20,7 @@ import ( "fmt" "go/ast" + "golang.org/x/exp/constraints" "golang.org/x/tools/go/analysis" kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors" "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" @@ -68,10 +69,10 @@ func run(pass *analysis.Pass) (any, error) { return nil, kalerrors.ErrCouldNotGetInspector } - inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, _ string) { - // Create TypeChecker with closure capturing markersAccess - typeChecker := utils.NewTypeChecker(func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) { - checkNumericType(pass, ident, node, prefix, markersAccess) + inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, qualifiedFieldName string) { + // Create TypeChecker with closure capturing markersAccess and qualifiedFieldName + typeChecker := utils.NewTypeChecker(func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, _ string) { + checkNumericType(pass, ident, node, markersAccess, qualifiedFieldName) }) typeChecker.CheckNode(pass, field) @@ -81,7 +82,7 @@ func run(pass *analysis.Pass) (any, error) { } //nolint:cyclop -func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string, markersAccess markershelper.Markers) { +func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, markersAccess markershelper.Markers, qualifiedFieldName string) { // Only check int32, int64, float32, and float64 types if ident.Name != "int32" && ident.Name != "int64" && ident.Name != "float32" && ident.Name != "float64" { return @@ -94,7 +95,7 @@ func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, pref fieldMarkers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field) - // Determine if this is a slice/array field + // Check if this is an array/slice field isSlice := utils.IsArrayTypeOrAlias(pass, field) // Determine which markers to look for based on whether the field is a slice @@ -110,20 +111,20 @@ func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, pref // Report any invalid marker values (e.g., non-numeric values) if minErr != nil && !minMissing { - pass.Reportf(field.Pos(), "%s has an invalid minimum marker: %v", prefix, minErr) + pass.Reportf(field.Pos(), "%s has an invalid minimum marker: %v", qualifiedFieldName, minErr) } if maxErr != nil && !maxMissing { - pass.Reportf(field.Pos(), "%s has an invalid maximum marker: %v", prefix, maxErr) + pass.Reportf(field.Pos(), "%s has an invalid maximum marker: %v", qualifiedFieldName, maxErr) } // Report if markers are missing if minMissing { - pass.Reportf(field.Pos(), "%s is missing minimum bounds validation marker", prefix) + pass.Reportf(field.Pos(), "%s is missing minimum bound validation marker", qualifiedFieldName) } if maxMissing { - pass.Reportf(field.Pos(), "%s is missing maximum bounds validation marker", prefix) + pass.Reportf(field.Pos(), "%s is missing maximum bound validation marker", qualifiedFieldName) } // If any markers are missing or invalid, don't continue with bounds checks @@ -132,7 +133,7 @@ func checkNumericType(pass *analysis.Pass, ident *ast.Ident, node ast.Node, pref } // Validate bounds are within the type's valid range - checkBoundsWithinTypeRange(pass, field, prefix, ident.Name, minimum, maximum) + checkBoundsWithinTypeRange(pass, field, qualifiedFieldName, ident.Name, minimum, maximum) } // getMarkerNames returns the appropriate minimum and maximum marker names @@ -148,6 +149,8 @@ func getMarkerNames(isSlice bool) (minMarkers, maxMarkers []string) { // getMarkerNumericValue extracts the numeric value from the first instance of any of the given marker names. // Checks multiple marker names to support both kubebuilder and k8s declarative validation markers. +// Precedence: Markers checked in the order provided and first valid value found is returned. +// We require a valid numeric value (not just marker presence) for both minimum and maximum markers. func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []string) (float64, error) { for _, markerName := range markerNames { markerList := markerSet.Get(markerName) @@ -155,7 +158,7 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri continue } - // Use the exported utils function to parse the marker value + // Use the exported utils.GetMarkerNumericValue function to parse the marker value value, err := utils.GetMarkerNumericValue[float64](markerList[0]) if err != nil { if errors.Is(err, errMarkerMissingValue) { @@ -174,43 +177,42 @@ func getMarkerNumericValue(markerSet markershelper.MarkerSet, markerNames []stri // checkBoundsWithinTypeRange validates that the bounds are within the valid range for the type. // For int64, enforces JavaScript-safe bounds as per Kubernetes API conventions to ensure // compatibility with JavaScript clients. -// -//nolint:cyclop func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, prefix, typeName string, minimum, maximum float64) { switch typeName { case "int32": - if minimum < float64(minInt32) || minimum > float64(maxInt32) { - pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid int32 range [%d, %d]", prefix, minimum, minInt32, maxInt32) - } - - if maximum < float64(minInt32) || maximum > float64(maxInt32) { - pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid int32 range [%d, %d]", prefix, maximum, minInt32, maxInt32) - } + checkBoundInRange(pass, field, prefix, minimum, minInt32, maxInt32, "minimum", "int32") + checkBoundInRange(pass, field, prefix, maximum, minInt32, maxInt32, "maximum", "int32") case "int64": - // Kubernetes API conventions enforce JavaScript-safe bounds for int64 (±2^53-1) - // to ensure compatibility with JavaScript clients - if minimum < float64(minSafeInt64) { - 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)) - } - - if maximum > float64(maxSafeInt64) { - 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)) - } + // K8s API conventions enforce JavaScript-safe bounds for int64 (±2^53-1) + checkBoundInRange(pass, field, prefix, minimum, int64(minSafeInt64), int64(maxSafeInt64), "minimum", "JavaScript-safe int64", + "Consider using a string type to avoid precision loss in JavaScript clients") + checkBoundInRange(pass, field, prefix, maximum, int64(minSafeInt64), int64(maxSafeInt64), "maximum", "JavaScript-safe int64", + "Consider using a string type to avoid precision loss in JavaScript clients") case "float32": - if minimum < float64(minFloat32) || minimum > float64(maxFloat32) { - pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid float32 range", prefix, minimum) - } - - if maximum < float64(minFloat32) || maximum > float64(maxFloat32) { - pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid float32 range", prefix, maximum) - } + checkFloatBoundInRange(pass, field, prefix, minimum, minFloat32, maxFloat32, "minimum", "float32") + checkFloatBoundInRange(pass, field, prefix, maximum, minFloat32, maxFloat32, "maximum", "float32") case "float64": - if minimum < minFloat64 || minimum > maxFloat64 { - pass.Reportf(field.Pos(), "%s has minimum bound %v that is outside the valid float64 range", prefix, minimum) - } + checkFloatBoundInRange(pass, field, prefix, minimum, minFloat64, maxFloat64, "minimum", "float64") + checkFloatBoundInRange(pass, field, prefix, maximum, minFloat64, maxFloat64, "maximum", "float64") + } +} - if maximum < minFloat64 || maximum > maxFloat64 { - pass.Reportf(field.Pos(), "%s has maximum bound %v that is outside the valid float64 range", prefix, maximum) +// checkBoundInRange checks if an integer bound value is within the valid range. +// Uses generics to avoid type conversions. +func checkBoundInRange[T constraints.Integer](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string, extraMsg ...string) { + if value < float64(minBound) || value > float64(maxBound) { + msg := fmt.Sprintf("%s has %s bound %%v that is outside the %s range [%%d, %%d]", prefix, boundType, typeName) + if len(extraMsg) > 0 { + msg += ". " + extraMsg[0] } + + pass.Reportf(field.Pos(), msg, value, minBound, maxBound) + } +} + +// checkFloatBoundInRange checks if a float bound value is within the valid range. +func checkFloatBoundInRange[T constraints.Float](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string) { + if value < float64(minBound) || value > float64(maxBound) { + pass.Reportf(field.Pos(), "%s has %s bound %v that is outside the valid %s range", prefix, boundType, value, typeName) } } diff --git a/pkg/analysis/numericbounds/testdata/src/a/a.go b/pkg/analysis/numericbounds/testdata/src/a/a.go index 14001442..4fd1ff69 100644 --- a/pkg/analysis/numericbounds/testdata/src/a/a.go +++ b/pkg/analysis/numericbounds/testdata/src/a/a.go @@ -1,79 +1,79 @@ package a -// ValidInt32WithBounds has proper bounds validation -type ValidInt32WithBounds struct { +// ValidInt32WithBound has proper bounds validation +type ValidInt32WithBound struct { // +kubebuilder:validation:Minimum=0 // +kubebuilder:validation:Maximum=100 Count int32 } -// ValidInt64WithBounds has proper bounds validation -type ValidInt64WithBounds struct { +// ValidInt64WithBound has proper bounds validation +type ValidInt64WithBound struct { // +kubebuilder:validation:Minimum=-1000 // +kubebuilder:validation:Maximum=1000 Value int64 } -// ValidInt64WithJSSafeBounds has bounds within JavaScript safe integer range -type ValidInt64WithJSSafeBounds struct { +// ValidInt64WithJSSafeBound has bounds within JavaScript safe integer range +type ValidInt64WithJSSafeBound struct { // +kubebuilder:validation:Minimum=-9007199254740991 // +kubebuilder:validation:Maximum=9007199254740991 SafeValue int64 } -// InvalidInt32NoBounds should have bounds markers -type InvalidInt32NoBounds struct { - NoBounds int32 // want "field NoBounds is missing minimum bounds validation marker" "field NoBounds is missing maximum bounds validation marker" +// InvalidInt32NoBound should have bounds markers +type InvalidInt32NoBound struct { + NoBound int32 // want "InvalidInt32NoBound.NoBound is missing minimum bound validation marker" "InvalidInt32NoBound.NoBound is missing maximum bound validation marker" } -// InvalidInt64NoBounds should have bounds markers -type InvalidInt64NoBounds struct { - NoBounds int64 // want "field NoBounds is missing minimum bounds validation marker" "field NoBounds is missing maximum bounds validation marker" +// InvalidInt64NoBound should have bounds markers +type InvalidInt64NoBound struct { + NoBound int64 // want "InvalidInt64NoBound.NoBound is missing minimum bound validation marker" "InvalidInt64NoBound.NoBound is missing maximum bound validation marker" } // InvalidInt32OnlyMin should have maximum marker type InvalidInt32OnlyMin struct { // +kubebuilder:validation:Minimum=0 - OnlyMin int32 // want "field OnlyMin is missing maximum bounds validation marker" + OnlyMin int32 // want "InvalidInt32OnlyMin.OnlyMin is missing maximum bound validation marker" } // InvalidInt32OnlyMax should have minimum marker type InvalidInt32OnlyMax struct { // +kubebuilder:validation:Maximum=100 - OnlyMax int32 // want "field OnlyMax is missing minimum bounds validation marker" + OnlyMax int32 // want "InvalidInt32OnlyMax.OnlyMax is missing minimum bound validation marker" } // InvalidInt64OnlyMin should have maximum marker type InvalidInt64OnlyMin struct { // +kubebuilder:validation:Minimum=0 - OnlyMin int64 // want "field OnlyMin is missing maximum bounds validation marker" + OnlyMin int64 // want "InvalidInt64OnlyMin.OnlyMin is missing maximum bound validation marker" } // InvalidInt64OnlyMax should have minimum marker type InvalidInt64OnlyMax struct { // +kubebuilder:validation:Maximum=100 - OnlyMax int64 // want "field OnlyMax is missing minimum bounds validation marker" + OnlyMax int64 // want "InvalidInt64OnlyMax.OnlyMax is missing minimum bound validation marker" } // InvalidInt64ExceedsJSMaxBounds has maximum that exceeds JavaScript safe integer range type InvalidInt64ExceedsJSMaxBounds struct { // +kubebuilder:validation:Minimum=-1000 // +kubebuilder:validation:Maximum=9007199254740992 - UnsafeMax int64 // want "field UnsafeMax has maximum bound 9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeMax int64 // want "InvalidInt64ExceedsJSMaxBounds.UnsafeMax has maximum bound 9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // InvalidInt64ExceedsJSMinBounds has minimum that exceeds JavaScript safe integer range type InvalidInt64ExceedsJSMinBounds struct { // +kubebuilder:validation:Minimum=-9007199254740992 // +kubebuilder:validation:Maximum=1000 - UnsafeMin int64 // want "field UnsafeMin has minimum bound -9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeMin int64 // want "InvalidInt64ExceedsJSMinBounds.UnsafeMin has minimum bound -9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // InvalidInt64ExceedsJSBothBounds has both bounds exceeding JavaScript safe integer range type InvalidInt64ExceedsJSBothBounds struct { // +kubebuilder:validation:Minimum=-9007199254740992 // +kubebuilder:validation:Maximum=9007199254740992 - UnsafeBoth int64 // want "field UnsafeBoth has minimum bound -9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" "field UnsafeBoth has maximum bound 9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" + UnsafeBoth int64 // want "InvalidInt64ExceedsJSBothBounds.UnsafeBoth has minimum bound -9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" "InvalidInt64ExceedsJSBothBounds.UnsafeBoth has maximum bound 9\\.007199254740992e\\+15 that is outside the JavaScript-safe int64 range \\[-9007199254740991, 9007199254740991\\]\\. Consider using a string type to avoid precision loss in JavaScript clients" } // IgnoredStringField should not be checked @@ -102,12 +102,12 @@ type ValidFloat32WithBounds struct { // InvalidFloat64NoBounds should have bounds markers type InvalidFloat64NoBounds struct { - Value float64 // want "field Value is missing minimum bounds validation marker" "field Value is missing maximum bounds validation marker" + Value float64 // want "InvalidFloat64NoBounds.Value is missing minimum bound validation marker" "InvalidFloat64NoBounds.Value is missing maximum bound validation marker" } // InvalidFloat32NoBounds should have bounds markers type InvalidFloat32NoBounds struct { - Ratio float32 // want "field Ratio is missing minimum bounds validation marker" "field Ratio is missing maximum bounds validation marker" + Ratio float32 // want "InvalidFloat32NoBounds.Ratio is missing minimum bound validation marker" "InvalidFloat32NoBounds.Ratio is missing maximum bound validation marker" } // MixedFields has both valid and invalid fields @@ -116,7 +116,7 @@ type MixedFields struct { // +kubebuilder:validation:Maximum=100 ValidCount int32 - InvalidCount int32 // want "field InvalidCount is missing minimum bounds validation marker" "field InvalidCount is missing maximum bounds validation marker" + InvalidCount int32 // want "MixedFields.InvalidCount is missing minimum bound validation marker" "MixedFields.InvalidCount is missing maximum bound validation marker" Name string @@ -124,7 +124,7 @@ type MixedFields struct { // +kubebuilder:validation:Maximum=1000 ValidValue int64 - InvalidValue int64 // want "field InvalidValue is missing minimum bounds validation marker" "field InvalidValue is missing maximum bounds validation marker" + InvalidValue int64 // want "MixedFields.InvalidValue is missing minimum bound validation marker" "MixedFields.InvalidValue is missing maximum bound validation marker" } // PointerFields with pointers should also be checked @@ -133,18 +133,18 @@ type PointerFields struct { // +kubebuilder:validation:Maximum=100 ValidPointerWithBounds *int32 - InvalidPointer *int32 // want "field InvalidPointer pointer is missing minimum bounds validation marker" "field InvalidPointer pointer is missing maximum bounds validation marker" + InvalidPointer *int32 // want "PointerFields.InvalidPointer is missing minimum bound validation marker" "PointerFields.InvalidPointer is missing maximum bound validation marker" - InvalidPointer64 *int64 // want "field InvalidPointer64 pointer is missing minimum bounds validation marker" "field InvalidPointer64 pointer is missing maximum bounds validation marker" + InvalidPointer64 *int64 // want "PointerFields.InvalidPointer64 is missing minimum bound validation marker" "PointerFields.InvalidPointer64 is missing maximum bound validation marker" } // SliceFields with slices should check the element type type SliceFields struct { ValidSlice []string - InvalidSlice []int32 // want "field InvalidSlice array element is missing minimum bounds validation marker" "field InvalidSlice array element is missing maximum bounds validation marker" + InvalidSlice []int32 // want "SliceFields.InvalidSlice is missing minimum bound validation marker" "SliceFields.InvalidSlice is missing maximum bound validation marker" - InvalidSlice64 []int64 // want "field InvalidSlice64 array element is missing minimum bounds validation marker" "field InvalidSlice64 array element is missing maximum bounds validation marker" + InvalidSlice64 []int64 // want "SliceFields.InvalidSlice64 is missing minimum bound validation marker" "SliceFields.InvalidSlice64 is missing maximum bound validation marker" // +kubebuilder:validation:items:Minimum=0 // +kubebuilder:validation:items:Maximum=100 @@ -179,13 +179,13 @@ type TypeAliasFields struct { // +kubebuilder:validation:Maximum=100 ValidAlias Int32Alias - InvalidAlias Int32Alias // want "field InvalidAlias type Int32Alias is missing minimum bounds validation marker" "field InvalidAlias type Int32Alias is missing maximum bounds validation marker" + InvalidAlias Int32Alias // want "TypeAliasFields.InvalidAlias is missing minimum bound validation marker" "TypeAliasFields.InvalidAlias is missing maximum bound validation marker" - InvalidAlias64 Int64Alias // want "field InvalidAlias64 type Int64Alias is missing minimum bounds validation marker" "field InvalidAlias64 type Int64Alias is missing maximum bounds validation marker" + InvalidAlias64 Int64Alias // want "TypeAliasFields.InvalidAlias64 is missing minimum bound validation marker" "TypeAliasFields.InvalidAlias64 is missing maximum bound validation marker" - InvalidAliasFloat32 Float32Alias // want "field InvalidAliasFloat32 type Float32Alias is missing minimum bounds validation marker" "field InvalidAliasFloat32 type Float32Alias is missing maximum bounds validation marker" + InvalidAliasFloat32 Float32Alias // want "TypeAliasFields.InvalidAliasFloat32 is missing minimum bound validation marker" "TypeAliasFields.InvalidAliasFloat32 is missing maximum bound validation marker" - InvalidAliasFloat64 Float64Alias // want "field InvalidAliasFloat64 type Float64Alias is missing minimum bounds validation marker" "field InvalidAliasFloat64 type Float64Alias is missing maximum bounds validation marker" + InvalidAliasFloat64 Float64Alias // want "TypeAliasFields.InvalidAliasFloat64 is missing minimum bound validation marker" "TypeAliasFields.InvalidAliasFloat64 is missing maximum bound validation marker" // Valid: bounds are on the type alias itself ValidBoundedAlias BoundedInt32Alias @@ -199,10 +199,10 @@ type TypeAliasFields struct { // PointerSliceFields with pointer slices should also be checked type PointerSliceFields struct { - 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" + InvalidPointerSlice []*int32 // want "PointerSliceFields.InvalidPointerSlice is missing minimum bound validation marker" "PointerSliceFields.InvalidPointerSlice is missing maximum bound validation marker" } -// K8sDeclarativeValidation with k8s declarative validation markers should work +// K8sDeclarativeValidation with k8s declarative validation markers type K8sDeclarativeValidation struct { // +k8s:minimum=0 // +k8s:maximum=100 @@ -217,7 +217,7 @@ type K8sDeclarativeValidation struct { type InvalidInt32BoundsOutOfRange struct { // +kubebuilder:validation:Minimum=-3000000000 // +kubebuilder:validation:Maximum=3000000000 - 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" + OutOfRange int32 // want "InvalidInt32BoundsOutOfRange.OutOfRange has minimum bound -3e\\+09 that is outside the int32 range \\[-2147483648, 2147483647\\]" "InvalidInt32BoundsOutOfRange.OutOfRange has maximum bound 3e\\+09 that is outside the int32 range \\[-2147483648, 2147483647\\]" } // MixedMarkersKubebuilderAndK8s can use either kubebuilder or k8s markers @@ -226,3 +226,17 @@ type MixedMarkersKubebuilderAndK8s struct { // +k8s:maximum=100 MixedMarkers int32 } + +// InvalidMarkerNonNumericMin has a non-numeric minimum value +type InvalidMarkerNonNumericMin struct { + // +kubebuilder:validation:Minimum=invalid + // +kubebuilder:validation:Maximum=100 + BadMin int32 // want "InvalidMarkerNonNumericMin.BadMin has an invalid minimum marker" +} + +// InvalidMarkerNonNumericMax has a non-numeric maximum value +type InvalidMarkerNonNumericMax struct { + // +kubebuilder:validation:Minimum=0 + // +kubebuilder:validation:Maximum=notanumber + BadMax int64 // want "InvalidMarkerNonNumericMax.BadMax has an invalid maximum marker" +} From 830ee74a2c8416d5198b158521a08777559a8201 Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Wed, 12 Nov 2025 15:33:59 -0500 Subject: [PATCH 7/8] checkBoundInRange: one function for int and float --- pkg/analysis/numericbounds/analyzer.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index 7451b3ec..5c844396 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -189,19 +189,19 @@ func checkBoundsWithinTypeRange(pass *analysis.Pass, field *ast.Field, prefix, t checkBoundInRange(pass, field, prefix, maximum, int64(minSafeInt64), int64(maxSafeInt64), "maximum", "JavaScript-safe int64", "Consider using a string type to avoid precision loss in JavaScript clients") case "float32": - checkFloatBoundInRange(pass, field, prefix, minimum, minFloat32, maxFloat32, "minimum", "float32") - checkFloatBoundInRange(pass, field, prefix, maximum, minFloat32, maxFloat32, "maximum", "float32") + checkBoundInRange(pass, field, prefix, minimum, minFloat32, maxFloat32, "minimum", "float32") + checkBoundInRange(pass, field, prefix, maximum, minFloat32, maxFloat32, "maximum", "float32") case "float64": - checkFloatBoundInRange(pass, field, prefix, minimum, minFloat64, maxFloat64, "minimum", "float64") - checkFloatBoundInRange(pass, field, prefix, maximum, minFloat64, maxFloat64, "maximum", "float64") + checkBoundInRange(pass, field, prefix, minimum, minFloat64, maxFloat64, "minimum", "float64") + checkBoundInRange(pass, field, prefix, maximum, minFloat64, maxFloat64, "maximum", "float64") } } -// checkBoundInRange checks if an integer bound value is within the valid range. -// Uses generics to avoid type conversions. -func checkBoundInRange[T constraints.Integer](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string, extraMsg ...string) { +// checkBoundInRange checks if a bound value is within the valid range. +// Uses generics to work with both integer and float types. +func checkBoundInRange[T constraints.Integer | constraints.Float](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string, extraMsg ...string) { if value < float64(minBound) || value > float64(maxBound) { - msg := fmt.Sprintf("%s has %s bound %%v that is outside the %s range [%%d, %%d]", prefix, boundType, typeName) + msg := fmt.Sprintf("%s has %s bound %%v that is outside the %s range [%%v, %%v]", prefix, boundType, typeName) if len(extraMsg) > 0 { msg += ". " + extraMsg[0] } @@ -209,10 +209,3 @@ func checkBoundInRange[T constraints.Integer](pass *analysis.Pass, field *ast.Fi pass.Reportf(field.Pos(), msg, value, minBound, maxBound) } } - -// checkFloatBoundInRange checks if a float bound value is within the valid range. -func checkFloatBoundInRange[T constraints.Float](pass *analysis.Pass, field *ast.Field, prefix string, value float64, minBound, maxBound T, boundType, typeName string) { - if value < float64(minBound) || value > float64(maxBound) { - pass.Reportf(field.Pos(), "%s has %s bound %v that is outside the valid %s range", prefix, boundType, value, typeName) - } -} From bdb99e509d7d4666153d4544a8c336b0053ecf20 Mon Sep 17 00:00:00 2001 From: Krish Agarwal Date: Thu, 13 Nov 2025 10:14:19 -0500 Subject: [PATCH 8/8] typechecker uses qualifiedFieldName --- pkg/analysis/numericbounds/analyzer.go | 1 + pkg/analysis/numericbounds/testdata/src/a/a.go | 4 ---- pkg/analysis/utils/type_check.go | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/analysis/numericbounds/analyzer.go b/pkg/analysis/numericbounds/analyzer.go index 5c844396..d57a338a 100644 --- a/pkg/analysis/numericbounds/analyzer.go +++ b/pkg/analysis/numericbounds/analyzer.go @@ -71,6 +71,7 @@ func run(pass *analysis.Pass) (any, error) { inspect.InspectFields(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markershelper.Markers, qualifiedFieldName string) { // Create TypeChecker with closure capturing markersAccess and qualifiedFieldName + // Ignore TypeChecker's prefix since we use qualifiedFieldName from inspector typeChecker := utils.NewTypeChecker(func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, _ string) { checkNumericType(pass, ident, node, markersAccess, qualifiedFieldName) }) diff --git a/pkg/analysis/numericbounds/testdata/src/a/a.go b/pkg/analysis/numericbounds/testdata/src/a/a.go index 4fd1ff69..8b25ccc0 100644 --- a/pkg/analysis/numericbounds/testdata/src/a/a.go +++ b/pkg/analysis/numericbounds/testdata/src/a/a.go @@ -145,10 +145,6 @@ type SliceFields struct { InvalidSlice []int32 // want "SliceFields.InvalidSlice is missing minimum bound validation marker" "SliceFields.InvalidSlice is missing maximum bound validation marker" InvalidSlice64 []int64 // want "SliceFields.InvalidSlice64 is missing minimum bound validation marker" "SliceFields.InvalidSlice64 is missing maximum bound validation marker" - - // +kubebuilder:validation:items:Minimum=0 - // +kubebuilder:validation:items:Maximum=100 - ValidSliceWithBounds []int32 } // TypeAliasFields with type aliases should be checked diff --git a/pkg/analysis/utils/type_check.go b/pkg/analysis/utils/type_check.go index d6b8e1b4..47e372d4 100644 --- a/pkg/analysis/utils/type_check.go +++ b/pkg/analysis/utils/type_check.go @@ -31,14 +31,14 @@ type TypeChecker interface { } // NewTypeChecker returns a new TypeChecker with the provided checkFunc. -func NewTypeChecker(checkFunc func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string)) TypeChecker { +func NewTypeChecker(checkFunc func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, qualifiedFieldName string)) TypeChecker { return &typeChecker{ checkFunc: checkFunc, } } type typeChecker struct { - checkFunc func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) + checkFunc func(pass *analysis.Pass, ident *ast.Ident, node ast.Node, qualifiedFieldName string) } // CheckNode checks the provided node for built-in types.