Allow floating-point values in validations

pkg/crd/markers/validation.go

@@ -17,9 +17,9 @@ limitations under the License.
 package markers
 
 import (
-	"fmt"
-
 	"encoding/json"
+	"fmt"
+	"math"
 
 	apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 
@@ -37,7 +37,7 @@ const (
 // reusable and writing complex validations on slice items.
 var ValidationMarkers = mustMakeAllWithPrefix("kubebuilder:validation", markers.DescribesField,
 
-	// integer markers
+	// numeric markers
 
 	Maximum(0),
 	Minimum(0),
@@ -122,11 +122,19 @@ func init() {
 
 // +controllertools:marker:generateHelp:category="CRD validation"
 // Maximum specifies the maximum numeric value that this field can have.
-type Maximum int
+type Maximum float64
+
+func (m Maximum) Value() float64 {
+	return float64(m)
+}
 
 // +controllertools:marker:generateHelp:category="CRD validation"
-// Minimum specifies the minimum numeric value that this field can have. Negative integers are supported.
-type Minimum int
+// Minimum specifies the minimum numeric value that this field can have. Negative numbers are supported.
+type Minimum float64
+
+func (m Minimum) Value() float64 {
+	return float64(m)
+}
 
 // +controllertools:marker:generateHelp:category="CRD validation"
 // ExclusiveMinimum indicates that the minimum is "up to" but not including that value.
@@ -138,7 +146,11 @@ type ExclusiveMaximum bool
 
 // +controllertools:marker:generateHelp:category="CRD validation"
 // MultipleOf specifies that this field must have a numeric value that's a multiple of this one.
-type MultipleOf int
+type MultipleOf float64
+
+func (m MultipleOf) Value() float64 {
+	return float64(m)
+}
 
 // +controllertools:marker:generateHelp:category="CRD validation"
 // MaxLength specifies the maximum length for this string.
@@ -251,41 +263,69 @@ type XIntOrString struct{}
 // to be used only as a last resort.
 type Schemaless struct{}
 
+func hasNumericType(schema *apiext.JSONSchemaProps) bool {
+	return schema.Type == "integer" || schema.Type == "number"
+}
+
+func isIntegral(value float64) bool {
+	return value == math.Trunc(value) && !math.IsNaN(value) && !math.IsInf(value, 0)
+}
+
 func (m Maximum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
-	if schema.Type != "integer" {
-		return fmt.Errorf("must apply maximum to an integer")
+	if !hasNumericType(schema) {
+		return fmt.Errorf("must apply maximum to a numeric value, found %s", schema.Type)
+	}
+
+	if schema.Type == "integer" && !isIntegral(m.Value()) {
+		return fmt.Errorf("cannot apply non-integral maximum validation (%v) to integer value", m.Value())
 	}
-	val := float64(m)
+
+	val := m.Value()
 	schema.Maximum = &val
 	return nil
 }
+
 func (m Minimum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
-	if schema.Type != "integer" {
-		return fmt.Errorf("must apply minimum to an integer")
+	if !hasNumericType(schema) {
+		return fmt.Errorf("must apply minimum to a numeric value, found %s", schema.Type)
+	}
+
+	if schema.Type == "integer" && !isIntegral(m.Value()) {
+		return fmt.Errorf("cannot apply non-integral minimum validation (%v) to integer value", m.Value())
 	}
-	val := float64(m)
+
+	val := m.Value()
 	schema.Minimum = &val
 	return nil
 }
+
 func (m ExclusiveMaximum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
-	if schema.Type != "integer" {
-		return fmt.Errorf("must apply exclusivemaximum to an integer")
+	if !hasNumericType(schema) {
+		return fmt.Errorf("must apply exclusivemaximum to a numeric value, found %s", schema.Type)
 	}
 	schema.ExclusiveMaximum = bool(m)
 	return nil
 }
+
 func (m ExclusiveMinimum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
-	if schema.Type != "integer" {
-		return fmt.Errorf("must apply exclusiveminimum to an integer")
+	if !hasNumericType(schema) {
+		return fmt.Errorf("must apply exclusiveminimum to a numeric value, found %s", schema.Type)
 	}
+
 	schema.ExclusiveMinimum = bool(m)
 	return nil
 }
+
 func (m MultipleOf) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
-	if schema.Type != "integer" {
-		return fmt.Errorf("must apply multipleof to an integer")
+	if !hasNumericType(schema) {
+		return fmt.Errorf("must apply multipleof to a numeric value, found %s", schema.Type)
 	}
-	val := float64(m)
+
+	if schema.Type == "integer" && !isIntegral(m.Value()) {
+		return fmt.Errorf("cannot apply non-integral multipleof validation (%v) to integer value", m.Value())
+	}
+
+	val := m.Value()
 	schema.MultipleOf = &val
 	return nil
 }
@@ -298,6 +338,7 @@ func (m MaxLength) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.MaxLength = &val
 	return nil
 }
+
 func (m MinLength) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	if schema.Type != "string" {
 		return fmt.Errorf("must apply minlength to a string")
@@ -306,6 +347,7 @@ func (m MinLength) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.MinLength = &val
 	return nil
 }
+
 func (m Pattern) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	// Allow string types or IntOrStrings. An IntOrString will still
 	// apply the pattern validation when a string is detected, the pattern
@@ -325,6 +367,7 @@ func (m MaxItems) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.MaxItems = &val
 	return nil
 }
+
 func (m MinItems) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	if schema.Type != "array" {
 		return fmt.Errorf("must apply minitems to an array")
@@ -333,6 +376,7 @@ func (m MinItems) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.MinItems = &val
 	return nil
 }
+
 func (m UniqueItems) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	if schema.Type != "array" {
 		return fmt.Errorf("must apply uniqueitems to an array")
@@ -376,6 +420,7 @@ func (m Enum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.Enum = vals
 	return nil
 }
+
 func (m Format) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
 	schema.Format = string(m)
 	return nil

pkg/crd/parser_integration_test.go

@@ -72,8 +72,9 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
 		reg := &markers.Registry{}
 		Expect(crdmarkers.Register(reg)).To(Succeed())
 		parser := &crd.Parser{
-			Collector: &markers.Collector{Registry: reg},
-			Checker:   &loader.TypeChecker{},
+			Collector:           &markers.Collector{Registry: reg},
+			Checker:             &loader.TypeChecker{},
+			AllowDangerousTypes: true, // need to allow “dangerous types” in this file for testing
 		}
 		crd.AddKnownTypes(parser)
 
@@ -188,6 +189,5 @@ var _ = Describe("CRD Generation From Parsing to CustomResourceDefinition", func
 
 		By("checking that no errors occurred along the way (expect for type errors)")
 		Expect(packageErrors(cronJobPkg, packages.TypeError)).NotTo(HaveOccurred())
-
 	})
 })

pkg/crd/testdata/cronjob_types.go

@@ -16,7 +16,7 @@ limitations under the License.
 // TODO(directxman12): test this across both versions (right now we're just
 // trusting k/k conversion, which is probably fine though)
 
-//go:generate ../../../.run-controller-gen.sh crd paths=./;./deprecated;./unserved output:dir=.
+//go:generate ../../../.run-controller-gen.sh crd:allowDangerousTypes=true paths=./;./deprecated;./unserved output:dir=.
 
 // +groupName=testdata.kubebuilder.io
 // +versionName=v1
@@ -178,6 +178,26 @@ type CronJobSpec struct {
 
 	// Checks that maps containing types that contain maps work
 	ContainsNestedMapMap map[string]ContainsNestedMap `json:"nestedMapInStruct,omitempty"`
+
+	// +kubebuilder:validation:Minimum=-0.5
+	// +kubebuilder:validation:Maximum=1.5
+	// +kubebuilder:validation:MultipleOf=0.5
+	FloatWithValidations float64 `json:"floatWithValidations"`
+
+	// +kubebuilder:validation:Minimum=-0.5
+	// +kubebuilder:validation:Maximum=1.5
+	// +kubebuilder:validation:MultipleOf=0.5
+	Float64WithValidations float64 `json:"float64WithValidations"`
+
+	// +kubebuilder:validation:Minimum=-2
+	// +kubebuilder:validation:Maximum=2
+	// +kubebuilder:validation:MultipleOf=2
+	IntWithValidations int `json:"intWithValidations"`
+
+	// +kubebuilder:validation:Minimum=-2
+	// +kubebuilder:validation:Maximum=2
+	// +kubebuilder:validation:MultipleOf=2
+	Int32WithValidations int32 `json:"int32WithValidations"`
 }
 
 type ContainsNestedMap struct {

pkg/crd/testdata/testdata.kubebuilder.io_cronjobs.yaml

@@ -125,6 +125,22 @@ spec:
                   a pointer to distinguish between explicit zero and not specified.
                 format: int32
                 type: integer
+              float64WithValidations:
+                maximum: 1.5
+                minimum: -0.5
+                multipleOf: 0.5
+                type: number
+              floatWithValidations:
+                maximum: 1.5
+                minimum: -0.5
+                multipleOf: 0.5
+                type: number
+              int32WithValidations:
+                format: int32
+                maximum: 2
+                minimum: -2
+                multipleOf: 2
+                type: integer
               intOrStringWithAPattern:
                 anyOf:
                 - type: integer
@@ -135,6 +151,11 @@ spec:
                   for having a pattern on this type.
                 pattern: ^((100|[0-9]{1,2})%|[0-9]+)$
                 x-kubernetes-int-or-string: true
+              intWithValidations:
+                maximum: 2
+                minimum: -2
+                multipleOf: 2
+                type: integer
               jobTemplate:
                 description: Specifies the job that will be created when executing
                   a CronJob.
@@ -7312,6 +7333,10 @@ spec:
             - defaultedSlice
             - defaultedString
             - embeddedResource
+            - float64WithValidations
+            - floatWithValidations
+            - int32WithValidations
+            - intWithValidations
             - jobTemplate
             - mapOfInfo
             - patternObject

pkg/markers/parse.go

@@ -84,6 +84,8 @@ const (
 	InvalidType ArgumentType = iota
 	// IntType is an int
 	IntType
+	// NumberType is a float64
+	NumberType
 	// StringType is a string
 	StringType
 	// BoolType is a bool
@@ -127,6 +129,8 @@ func (a Argument) typeString(out *strings.Builder) {
 		out.WriteString("<invalid>")
 	case IntType:
 		out.WriteString("int")
+	case NumberType:
+		out.WriteString("float64")
 	case StringType:
 		out.WriteString("string")
 	case BoolType:
@@ -180,6 +184,8 @@ func makeSliceType(itemType Argument) (reflect.Type, error) {
 	switch itemType.Type {
 	case IntType:
 		itemReflectedType = reflect.TypeOf(int(0))
+	case NumberType:
+		itemReflectedType = reflect.TypeOf(float64(0))
 	case StringType:
 		itemReflectedType = reflect.TypeOf("")
 	case BoolType:
@@ -215,6 +221,8 @@ func makeMapType(itemType Argument) (reflect.Type, error) {
 	switch itemType.Type {
 	case IntType:
 		itemReflectedType = reflect.TypeOf(int(0))
+	case NumberType:
+		itemReflectedType = reflect.TypeOf(float64(0))
 	case StringType:
 		itemReflectedType = reflect.TypeOf("")
 	case BoolType:
@@ -346,8 +354,11 @@ func guessType(scanner *sc.Scanner, raw string, allowSlice bool) *Argument {
 		if nextTok == '-' {
 			nextTok = subScanner.Scan()
 		}
+
 		if nextTok == sc.Int {
 			return &Argument{Type: IntType}
+		} else if nextTok == sc.Float {
+			return &Argument{Type: NumberType}
 		}
 	}
 
@@ -471,7 +482,7 @@ func (a *Argument) parseMap(scanner *sc.Scanner, raw string, out reflect.Value)
 func (a *Argument) parse(scanner *sc.Scanner, raw string, out reflect.Value, inSlice bool) {
 	// nolint:gocyclo
 	if a.Type == InvalidType {
-		scanner.Error(scanner, fmt.Sprintf("cannot parse invalid type"))
+		scanner.Error(scanner, "cannot parse invalid type")
 		return
 	}
 	if a.Pointer {
@@ -485,6 +496,32 @@ func (a *Argument) parse(scanner *sc.Scanner, raw string, out reflect.Value, inS
 		// consume everything else
 		for tok := scanner.Scan(); tok != sc.EOF; tok = scanner.Scan() {
 		}
+	case NumberType:
+		nextChar := scanner.Peek()
+		isNegative := false
+		if nextChar == '-' {
+			isNegative = true
+			scanner.Scan() // eat the '-'
+		}
+
+		tok := scanner.Scan()
+		if tok != sc.Float && tok != sc.Int {
+			scanner.Error(scanner, fmt.Sprintf("expected integer or float, got %q", scanner.TokenText()))
+			return
+		}
+
+		text := scanner.TokenText()
+		if isNegative {
+			text = "-" + text
+		}
+
+		val, err := strconv.ParseFloat(text, 64)
+		if err != nil {
+			scanner.Error(scanner, fmt.Sprintf("unable to parse number: %v", err))
+			return
+		}
+
+		castAndSet(out, reflect.ValueOf(val))
 	case IntType:
 		nextChar := scanner.Peek()
 		isNegative := false
@@ -597,6 +634,8 @@ func ArgumentFromType(rawType reflect.Type) (Argument, error) {
 		arg.Type = StringType
 	case reflect.Int, reflect.Int32: // NB(directxman12): all ints in kubernetes are int32, so explicitly support that
 		arg.Type = IntType
+	case reflect.Float64:
+		arg.Type = NumberType
 	case reflect.Bool:
 		arg.Type = BoolType
 	case reflect.Slice:
@@ -755,7 +794,7 @@ func (d *Definition) loadFields() error {
 func parserScanner(raw string, err func(*sc.Scanner, string)) *sc.Scanner {
 	scanner := &sc.Scanner{}
 	scanner.Init(bytes.NewBufferString(raw))
-	scanner.Mode = sc.ScanIdents | sc.ScanInts | sc.ScanStrings | sc.ScanRawStrings | sc.SkipComments
+	scanner.Mode = sc.ScanIdents | sc.ScanInts | sc.ScanStrings | sc.ScanRawStrings | sc.SkipComments | sc.ScanFloats
 	scanner.Error = err
 
 	return scanner