Repository navigation
Conversation
|
Label |
50d3a86 to
c80a053
Compare
An integer driver setting such as Kubernetes `tolerationSeconds` can become a fractional JSON number during protobuf decoding and fail typed configuration parsing. This preserves whole-number values through the shared converter and the actual Kubernetes rendering path. Signed-off-by: Shiju <shiju@nvidia.com>
f9245d5 to
de0645c
Compare
Karthik-Chowdary
left a comment
There was a problem hiding this comment.
Additional wire-level evidence for de0645c23: I exercised the unmodified shared converter from base cc0642fa and this head in an isolated Rust harness, using real prost::Message Struct encode/decode before typed serde parsing.
- A nested PodSpec with
tolerationSecondsof0,512, and9007199254740994(2^53+2) decoded into the actualk8s_openapi::api::core::v1::PodSpecand matched direct JSON on head. Base failed withinvalid type: floating point ... expected i64. - Nested lists of six negative
i64values (including −(2^53+2) andi64::MIN) and all 256i8values also matched direct typed JSON on head; both fail on base. Float/null/bool/string controls remained equal. - Control checks still rejected i8 overflow/fractions and nonexact producer integers; those are existing serde/producer behavior, not new guarantees of this PR.
The five added checks give base 2 passed / 3 failed, head 5 passed. The head helper’s existing ten unit tests also passed. Direct dependencies were pinned to the PR lockfile versions (prost/prost-types 0.14.3, serde 1.0.229, serde_json 1.0.149, k8s-openapi 0.24.0). This verifies binary transport plus shared typed conversion, not driver factories, Kubernetes admission, or live sandbox behavior.
Reproducer: import the unchanged crates/openshell-core/src/proto_struct.rs; form JSON {"spec":{"containers":[],"tolerations":[{"operator":"Exists","effect":"NoExecute","tolerationSeconds":9007199254740994}]}}; call json_object_to_struct, then Struct::decode(proto.encode_to_vec().as_slice()), convert using the production API, and deserialize the resulting spec as PodSpec.
| /// | ||
| /// Reject non-finite numbers anywhere in the configuration instead of turning | ||
| /// them into null, which could silently clear an optional constraint. | ||
| pub fn struct_to_typed_json_value( |
There was a problem hiding this comment.
This now sits next to struct_to_json_value / struct_to_json_object / value_to_json, and all four are documented as "for typed serde decoding". Both struct_to_json_value and struct_to_typed_json_value return the same untyped serde_json::Value; "typed" describes what the caller does next, not what the function does. The actual difference (whole numbers stay integers, non-finite numbers are rejected instead of becoming null) is not visible from the names, so it is hard to tell which one a new caller should use. The shorter, infallible one is the easy pick, and that reintroduces #5106.
Every call site in this PR immediately does serde_json::from_value::<T>(json) and never uses the intermediate JSON, so what about folding that in and keeping the conversion private?
pub fn from_struct<T: DeserializeOwned>(
config: &prost_types::Struct,
) -> Result<T, StructDecodeError> {
Ok(serde_json::from_value(struct_to_exact_json(config)?)?)
}with StructDecodeError having NonFiniteNumber and Deserialize(#[from] serde_json::Error) variants. Benefits:
- The name says what you get, a typed value, so there is one obvious choice for decoding config.
- Each driver call site goes from two
map_errs to one, e.g.proto_struct::from_struct(config).map_err(|err| format!("invalid docker driver_config: {err}")). - The exact-JSON conversion does not need to be public; a caller that really wants the JSON can use
from_struct::<serde_json::Value>.
The existing functions still have general-purpose callers (CLI display, OPA input, interceptor patches), so I would keep them but reword their docs to say they are lossy: whole numbers become floats and non-finite numbers become null.
Summary
A valid integer driver setting, such as Kubernetes
tolerationSeconds, can arrive as a floating-point JSON number after protobuf decoding and fail typed parsing. This preserves exactly represented whole numbers through the shared converter and the Kubernetes rendering path.Related Issue
Closes #5106. Part of #5114.
Changes
Release Notes
Per-sandbox integer driver settings retain their JSON number type. The separate gateway TOML PID-limit path is unchanged.
Testing
Checklist