From de0645c23f2e9ce743f0f03a3e1b644a8cce4e2a Mon Sep 17 00:00:00 2001 From: Shiju Date: Tue, 3 Nov 2026 17:10:58 +0530 Subject: [PATCH] fix(compute): preserve protobuf configuration integers 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 --- crates/openshell-core/src/proto_struct.rs | 193 ++++++++++++++++++ crates/openshell-driver-docker/src/lib.rs | 7 +- .../openshell-driver-kubernetes/src/driver.rs | 14 +- crates/openshell-driver-mxc/src/driver.rs | 12 +- .../openshell-driver-podman/src/container.rs | 8 +- crates/openshell-driver-vm/src/driver.rs | 7 +- 6 files changed, 226 insertions(+), 15 deletions(-) diff --git a/crates/openshell-core/src/proto_struct.rs b/crates/openshell-core/src/proto_struct.rs index ce95c65f0c..7ada7ccf86 100644 --- a/crates/openshell-core/src/proto_struct.rs +++ b/crates/openshell-core/src/proto_struct.rs @@ -13,6 +13,11 @@ pub enum ProtoStructError { UnrepresentableNumber(serde_json::Number), } +/// A non-finite protobuf number cannot be used as typed configuration. +#[derive(Debug, thiserror::Error)] +#[error("protobuf configuration contains a non-finite number")] +pub struct TypedProtoStructError; + /// Convert a JSON object into a protobuf Struct. pub fn json_object_to_struct( object: serde_json::Map, @@ -87,6 +92,77 @@ pub fn struct_to_json_value(config: &prost_types::Struct) -> serde_json::Value { serde_json::Value::Object(struct_to_json_object(config)) } +/// Convert protobuf configuration into JSON for typed serde decoding. +/// +/// Finite integral doubles in the JSON integer range become integers; fractions +/// and larger finite numbers remain floats. This preserves exactly represented +/// large integers accepted by [`json_value_to_proto`]. A receiver cannot detect +/// precision already lost before transport; the producer must enforce exactness. +/// General-purpose conversions keep their existing float and null behavior. +/// +/// # Errors +/// +/// 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( + config: &prost_types::Struct, +) -> Result { + config + .fields + .iter() + .map(|(key, value)| typed_value_to_json(value).map(|value| (key.clone(), value))) + .collect::, _>>() + .map(serde_json::Value::Object) +} + +// The casts below are preceded by finite, integral and exclusive range guards. +#[allow( + clippy::cast_possible_truncation, + clippy::cast_precision_loss, + clippy::cast_sign_loss +)] +fn typed_value_to_json( + value: &prost_types::Value, +) -> Result { + use prost_types::value::Kind; + use serde_json::{Number, Value}; + + Ok(match value.kind.as_ref() { + Some(Kind::NumberValue(number)) => { + if !number.is_finite() { + return Err(TypedProtoStructError); + } + // Exclusive upper bounds are intentional: u64::MAX and i64::MAX + // round UP when converted to f64. Guard before any integer cast. + let integer = if number.fract() == 0.0 { + if *number >= 0.0 && *number < u64::MAX as f64 { + Some(Number::from(*number as u64)) + } else if *number >= i64::MIN as f64 && *number < 0.0 { + Some(Number::from(*number as i64)) + } else { + None + } + } else { + None + }; + Value::Number( + integer + .or_else(|| Number::from_f64(*number)) + .ok_or(TypedProtoStructError)?, + ) + } + Some(Kind::StructValue(config)) => struct_to_typed_json_value(config)?, + Some(Kind::ListValue(list)) => Value::Array( + list.values + .iter() + .map(typed_value_to_json) + .collect::>()?, + ), + // Only numeric values differ from the compatible general conversion. + _ => value_to_json(value), + }) +} + /// Convert a protobuf Value into a JSON value for typed serde decoding. #[must_use] pub fn value_to_json(value: &prost_types::Value) -> serde_json::Value { @@ -177,6 +253,123 @@ mod tests { ); } + #[test] + fn typed_configuration_round_trips_recursive_integer_fields() { + #[derive(Debug, Deserialize, PartialEq)] + struct Limits { + max_processes: Option, + max_resident_bytes: u64, + } + #[derive(Debug, Deserialize, PartialEq)] + struct Config { + service_ports: Vec, + limits: Limits, + nested: Vec, + } + let json = serde_json::json!({ + "service_ports": [8080, 65535], + "limits": {"max_processes": 128, "max_resident_bytes": 1_073_741_824}, + "nested": [{"max_processes": null, "max_resident_bytes": 0}] + }); + let expected: Config = serde_json::from_value(json.clone()).unwrap(); + let proto = json_object_to_struct(json.as_object().unwrap().clone()).unwrap(); + let actual: Config = + serde_json::from_value(struct_to_typed_json_value(&proto).unwrap()).unwrap(); + assert_eq!(actual, expected); + // The general-purpose conversion retains its existing float semantics. + assert!(struct_to_json_value(&proto)["service_ports"][0].is_f64()); + } + + #[test] + fn typed_configuration_rejects_nonfinite_numbers() { + for number in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] { + let mut proto = json_object_to_struct( + serde_json::json!({"limits": [{"max_processes": 128}]}) + .as_object() + .unwrap() + .clone(), + ) + .unwrap(); + let Some(prost_types::value::Kind::ListValue(list)) = + proto.fields.get_mut("limits").unwrap().kind.as_mut() + else { + unreachable!(); + }; + let Some(prost_types::value::Kind::StructValue(limits)) = list.values[0].kind.as_mut() + else { + unreachable!(); + }; + limits.fields.get_mut("max_processes").unwrap().kind = + Some(prost_types::value::Kind::NumberValue(number)); + assert!(struct_to_typed_json_value(&proto).is_err()); + assert!(struct_to_json_value(&proto)["limits"][0]["max_processes"].is_null()); + } + } + + #[test] + fn typed_configuration_preserves_numeric_boundaries() { + for integer in [ + 0, + 9_007_199_254_740_992, + 9_007_199_254_740_994, + 1_u64 << 63, + u64::MAX - 2047, + ] { + let proto = json_object_to_struct( + serde_json::json!({"limit": integer}) + .as_object() + .unwrap() + .clone(), + ) + .unwrap(); + let value = struct_to_typed_json_value(&proto).unwrap(); + assert_eq!( + serde_json::from_value::(value["limit"].clone()).unwrap(), + integer + ); + } + let proto = json_object_to_struct( + serde_json::json!({"limit": i64::MIN}) + .as_object() + .unwrap() + .clone(), + ) + .unwrap(); + let value = struct_to_typed_json_value(&proto).unwrap(); + assert_eq!( + serde_json::from_value::(value["limit"].clone()).unwrap(), + i64::MIN + ); + // Fractions and doubles outside the integer range must not be truncated + // or saturated into a valid integer configuration value. + for number in [1.5, -1.0, 18_446_744_073_709_551_616.0, f64::MAX] { + let proto = prost_types::Struct { + fields: [( + "limit".to_owned(), + prost_types::Value { + kind: Some(prost_types::value::Kind::NumberValue(number)), + }, + )] + .into(), + }; + let value = struct_to_typed_json_value(&proto).unwrap(); + assert!(serde_json::from_value::(value["limit"].clone()).is_err()); + } + let below_i64_min = prost_types::Struct { + fields: [( + "limit".to_owned(), + prost_types::Value { + kind: Some(prost_types::value::Kind::NumberValue( + -9_223_372_036_854_777_856.0, + )), + }, + )] + .into(), + }; + let value = struct_to_typed_json_value(&below_i64_min).unwrap(); + assert!(serde_json::from_value::(value["limit"].clone()).is_err()); + } + #[derive(Debug, Default, Deserialize)] #[serde(default)] struct TestConfig { diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 6b761f2f59..c57eed7c01 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -55,7 +55,7 @@ use openshell_core::proto::compute::v1::{ WatchSandboxesSandboxEvent, compute_driver_server::ComputeDriver, watch_sandboxes_event, }; use openshell_core::proto_struct::{ - deserialize_optional_non_empty_string_list, struct_to_json_value, + deserialize_optional_non_empty_string_list, struct_to_typed_json_value, }; use openshell_core::{ AppArmorProfile, Error, ImagePullPolicy, NetworkSupervisorTrustBundle, Result as CoreResult, @@ -783,8 +783,9 @@ impl DockerSandboxDriverConfig { return Ok(Self::default()); }; - serde_json::from_value(struct_to_json_value(config)) - .map_err(|err| format!("invalid docker driver_config: {err}")) + let json = struct_to_typed_json_value(config) + .map_err(|err| format!("invalid docker driver_config: {err}"))?; + serde_json::from_value(json).map_err(|err| format!("invalid docker driver_config: {err}")) } } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index afc06e58c2..5e9a14bef6 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -69,7 +69,7 @@ use openshell_core::proto::compute::v1::{ WatchSandboxesDeletedEvent, WatchSandboxesEvent, WatchSandboxesPlatformEvent, WatchSandboxesSandboxEvent, watch_sandboxes_event, }; -use openshell_core::proto_struct::{struct_to_json_object, value_to_json}; +use openshell_core::proto_struct::{struct_to_typed_json_value, value_to_json}; use openshell_isolation_interface::contract::ResolvedWorkloadIdentity; use openshell_sandbox_backend::boundary_protocol::{ GatewayVerificationKey, SandboxTlsClientConfig, SandboxTlsServerConfig, @@ -711,7 +711,8 @@ impl KubernetesSandboxDriverConfig { return Ok(Self::default()); }; - let json = serde_json::Value::Object(struct_to_json_object(config)); + let json = struct_to_typed_json_value(config) + .map_err(|err| format!("invalid kubernetes driver_config: {err}"))?; let config: Self = serde_json::from_value(json) .map_err(|err| format!("invalid kubernetes driver_config: {err}"))?; config @@ -12891,6 +12892,11 @@ mod tests { "key": "nvidia.com/gpu", "operator": "Exists", "effect": "NoSchedule" + }, { + "key": "maintenance.example.com/pool", + "operator": "Exists", + "effect": "NoExecute", + "tolerationSeconds": 512 }] }, "containers": { @@ -12933,6 +12939,10 @@ mod tests { pod_template["spec"]["tolerations"][0]["key"], serde_json::json!("nvidia.com/gpu") ); + let toleration: k8s_openapi::api::core::v1::Toleration = + serde_json::from_value(pod_template["spec"]["tolerations"][1].clone()) + .expect("transport preserves the integer expected by the Kubernetes API"); + assert_eq!(toleration.toleration_seconds, Some(512)); assert_eq!( pod_template["spec"]["containers"][0]["resources"]["requests"]["vendor.example/gpu-memory"], serde_json::json!("8Gi") diff --git a/crates/openshell-driver-mxc/src/driver.rs b/crates/openshell-driver-mxc/src/driver.rs index 2772f1a69c..d5f15697ee 100644 --- a/crates/openshell-driver-mxc/src/driver.rs +++ b/crates/openshell-driver-mxc/src/driver.rs @@ -14,7 +14,7 @@ use openshell_core::proto::compute::v1::{ GetCapabilitiesResponse, WatchSandboxesDeletedEvent, WatchSandboxesEvent, WatchSandboxesPlatformEvent, WatchSandboxesSandboxEvent, watch_sandboxes_event, }; -use openshell_core::proto_struct::struct_to_json_value; +use openshell_core::proto_struct::struct_to_typed_json_value; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::net::SocketAddr; @@ -239,10 +239,12 @@ fn sandbox_config(sandbox: &DriverSandbox) -> Result