fix(api): reject non-integral bounds for Int search spaces

[sanskar-singh-2403]

What this PR does / why we need it

UniformSpace and LogUniformSpace both carry a type field (Int or Float), but their min and max are Double strings constrained only by the Double pattern and by the CEL rule double(self.min) < double(self.max). Nothing connected type: Int to integral bounds, so all of these were accepted by the API server:

min	max	type	accepted before
1.5	8.7	Int	yes
1.2	1.8	Int	yes, and contains no integer at all
1e-3	1e2	Int	yes
0.5	10	Int	yes
1	10.5	Int	yes

The 1.2/1.8 case is the sharpest one: there is no integer in [1.2, 1.8], so the search space is unsatisfiable, and the object is still created.

The failure was then deferred to the suggestion backend. Katib's search space conversion takes the INTEGER branch and calls Python int() directly on the bounds:

https://github.com/kubeflow/katib/blob/master/pkg/suggestion/v1beta1/internal/search_space.py#L53-L55

Python int() rejects every fractional and scientific-notation string the Trainer API accepts, so the user gets an opaque ValueError traceback from inside the suggestion container with nothing pointing at the offending field. This is the same failure class that #3862 exists to eliminate for grid search.

The KEP already puts this in scope. proposals/2605-optimization-job-crd/README.md says the discriminated union exists so that mathematical CEL validations (double(), int()) are possible. The double() half shipped; the int() half was never implemented.

Design notes

One CEL rule per type, guarded on self.type != 'Int'. The Float path is untouched. type is defaulted to Float before CEL runs, so the guard is always evaluable, including when the user omits type entirely.

Integrality is tested by a float round-trip, double(x) == double(int(double(x))), so "1", "1.0" and "1e2" are all correctly treated as whole numbers. The check is on the value, not on its spelling.

The rule also clamps magnitude to 2^53, and this is load-bearing rather than decorative. CEL's int() conversion errors on a double outside int64 range, and an erroring rule surfaces as type conversion error from 'double' to 'int' evaluating rule: <message>, which names the rule but describes the wrong problem. Folding the range check into the same rule means an out-of-range Int bound gets a message about its actual problem. 2^53 rather than int64 max because above 2^53 a float64 cannot represent consecutive integers, so "is this a whole number" stops being a meaningful question about the user's input.

Not addressed here: a bound above 1e308 overflows double() in the pre-existing min < max rule and reports type conversion error ... evaluating rule: min must be strictly less than max, which names ordering when the problem is magnitude. That comes from the Double type's MaxLength=64 pattern, affects Float equally, and is better fixed on Double itself. I noted it in #3906 and it is left for a follow-up.

Testing

New envtest table in test/integration/webhooks/optimizationjob_int_bounds_test.go, 16 cases:

min	max	type	expected
1.5	8.7	Int	rejected
1.2	1.8	Int	rejected, no integer in range
0.5	10	Int	rejected, min only
1	10.5	Int	rejected, max only
1e-3	1e2	Int	rejected
1	1e300	Int	rejected, beyond 2^53
1	8	Int	accepted
1.0	8.0	Int	accepted
1e2	1e3	Int	accepted
-8	-1	Int	accepted
0.001	0.1	Float	accepted
0.001	0.1	omitted	accepted, defaulting
1.5	8.7	Int logUniform	rejected
1.2	1.8	Int logUniform	rejected
1	1024	Int logUniform	accepted
0.0001	0.1	Float logUniform	accepted

Generated assets

XValidation markers do not flow into zz_generated.openapi.go or swagger.json, so the regenerated surface is only the two CRD YAMLs (manifests/base/crds/ and the Helm chart copy). Produced by make manifests.

Fixes #3906

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign terrytangyuan for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[HarshPopat23]

Thanks @sanskar-singh-2403 for addressing this validation gap. The overall direction and test coverage look good, but I found one blocking contract issue.

The new rule accepts "1.0" and "1e2" as integral values, while the cited Katib path still calls int() on the original strings. Since CEL validation does not rewrite the stored bounds, both values will still fail at runtime with ValueError.

The float round-trip can also misclassify fractional input because of IEEE-754 rounding. For example, double("1.0000000000000001") becomes 1.0, so this value can pass the rule even though its original value is non-integral. The 2^53 magnitude limit does not prevent precision loss like this.

Could we define the intended wire-format contract and then either:

• require canonical integer strings for Int bounds using an anchored integer regex, or
• normalize/parse the values exactly before passing them to the suggestion provider?

Please also add regression coverage for "1.0000000000000001" and update the "1.0"/"1e2" expectations according to the chosen contract.

PTAL : @andreyvelich

[google-oss-prow]

@HarshPopat23: changing LGTM is restricted to collaborators

Details

In response to this:

Thanks for addressing this validation gap. The overall direction and test coverage look good, but I found one blocking contract issue.

The new rule accepts "1.0" and "1e2" as integral values, while the cited Katib path still calls int() on the original strings. Since CEL validation does not rewrite the stored bounds, both values will still fail at runtime with ValueError.

The float round-trip can also misclassify fractional input because of IEEE-754 rounding. For example, double("1.0000000000000001") becomes 1.0, so this value can pass the rule even though its original value is non-integral. The 2^53 magnitude limit does not prevent precision loss like this.

Could we define the intended wire-format contract and then either:

• require canonical integer strings for Int bounds using an anchored integer regex, or
• normalize/parse the values exactly before passing them to the suggestion provider?

Please also add regression coverage for "1.0000000000000001" and update the "1.0"/"1e2" expectations according to the chosen contract.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.