diff --git a/crates/openshell-core/src/proto_struct.rs b/crates/openshell-core/src/proto_struct.rs index ce95c65f0c..672590a692 100644 --- a/crates/openshell-core/src/proto_struct.rs +++ b/crates/openshell-core/src/proto_struct.rs @@ -3,7 +3,10 @@ //! Helpers for converting `google.protobuf.Struct` values to and from JSON. -use serde::{Deserialize, Deserializer, de::Error as _}; +use serde::{ + Deserialize, Deserializer, + de::{DeserializeOwned, Error as _}, +}; /// Errors converting JSON values into protobuf well-known types. #[derive(Debug, thiserror::Error)] @@ -13,6 +16,17 @@ pub enum ProtoStructError { UnrepresentableNumber(serde_json::Number), } +/// Errors decoding protobuf configuration into a Rust type. +#[derive(Debug, thiserror::Error)] +pub enum StructDecodeError { + /// A non-finite protobuf number cannot be used as typed configuration. + #[error("protobuf configuration contains a non-finite number")] + NonFiniteNumber, + /// The configuration does not match the requested Rust type. + #[error(transparent)] + Deserialize(#[from] serde_json::Error), +} + /// Convert a JSON object into a protobuf Struct. pub fn json_object_to_struct( object: serde_json::Map, @@ -69,7 +83,10 @@ fn integer_is_exact_in_f64(integer: u64) -> bool { <= f64::MANTISSA_DIGITS } -/// Convert a protobuf Struct into a JSON object for typed serde decoding. +/// Convert a protobuf Struct into a JSON object with lossy numeric conversion. +/// +/// All numbers become floats, and non-finite numbers become null. Use +/// [`from_struct`] to decode typed configuration while preserving integers. #[must_use] pub fn struct_to_json_object( config: &prost_types::Struct, @@ -81,13 +98,95 @@ pub fn struct_to_json_object( .collect() } -/// Convert a protobuf Struct into a JSON value for typed serde decoding. +/// Convert a protobuf Struct into a JSON value with lossy numeric conversion. +/// +/// All numbers become floats, and non-finite numbers become null. Use +/// [`from_struct`] to decode typed configuration while preserving integers. #[must_use] pub fn struct_to_json_value(config: &prost_types::Struct) -> serde_json::Value { serde_json::Value::Object(struct_to_json_object(config)) } -/// Convert a protobuf Value into a JSON value for typed serde decoding. +/// Deserialize protobuf configuration into a Rust type, preserving integers. +/// +/// 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. +/// Use `from_struct::` to obtain the intermediate JSON. +/// +/// # Errors +/// +/// Return [`StructDecodeError::NonFiniteNumber`] for non-finite numbers anywhere +/// in the configuration, or [`StructDecodeError::Deserialize`] if the +/// configuration does not match the requested type. +pub fn from_struct( + config: &prost_types::Struct, +) -> Result { + Ok(serde_json::from_value(struct_to_exact_json(config)?)?) +} + +fn struct_to_exact_json( + config: &prost_types::Struct, +) -> Result { + config + .fields + .iter() + .map(|(key, value)| value_to_exact_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 value_to_exact_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(StructDecodeError::NonFiniteNumber); + } + // 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(StructDecodeError::NonFiniteNumber)?, + ) + } + Some(Kind::StructValue(config)) => struct_to_exact_json(config)?, + Some(Kind::ListValue(list)) => Value::Array( + list.values + .iter() + .map(value_to_exact_json) + .collect::>()?, + ), + // Only numeric values differ from the compatible general conversion. + _ => value_to_json(value), + }) +} + +/// Convert a protobuf Value into a JSON value with lossy numeric conversion. +/// +/// All numbers become floats, and non-finite numbers become null. Use +/// [`from_struct`] to decode typed configuration while preserving integers. #[must_use] pub fn value_to_json(value: &prost_types::Value) -> serde_json::Value { match value.kind.as_ref() { @@ -177,6 +276,128 @@ 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 = from_struct(&proto).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!(matches!( + from_struct::(&proto), + Err(StructDecodeError::NonFiniteNumber) + )); + assert!(struct_to_json_value(&proto)["limits"][0]["max_processes"].is_null()); + } + } + + #[test] + fn typed_configuration_preserves_numeric_boundaries() { + #[derive(Debug, Deserialize)] + struct Limit { + limit: T, + } + + 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 config: Limit = from_struct(&proto).unwrap(); + assert_eq!(config.limit, integer); + } + let proto = json_object_to_struct( + serde_json::json!({"limit": i64::MIN}) + .as_object() + .unwrap() + .clone(), + ) + .unwrap(); + let config: Limit = from_struct(&proto).unwrap(); + assert_eq!(config.limit, 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(), + }; + assert!(matches!( + from_struct::>(&proto), + Err(StructDecodeError::Deserialize(_)) + )); + } + 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(), + }; + assert!(matches!( + from_struct::>(&below_i64_min), + Err(StructDecodeError::Deserialize(_)) + )); + } + #[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..a75c06f726 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -54,9 +54,7 @@ use openshell_core::proto::compute::v1::{ WatchSandboxesEvent, WatchSandboxesPlatformEvent, WatchSandboxesRequest, WatchSandboxesSandboxEvent, compute_driver_server::ComputeDriver, watch_sandboxes_event, }; -use openshell_core::proto_struct::{ - deserialize_optional_non_empty_string_list, struct_to_json_value, -}; +use openshell_core::proto_struct::{deserialize_optional_non_empty_string_list, from_struct}; use openshell_core::{ AppArmorProfile, Error, ImagePullPolicy, NetworkSupervisorTrustBundle, Result as CoreResult, UpstreamProxyConfig, @@ -783,8 +781,7 @@ impl DockerSandboxDriverConfig { return Ok(Self::default()); }; - serde_json::from_value(struct_to_json_value(config)) - .map_err(|err| format!("invalid docker driver_config: {err}")) + from_struct(config).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..c0539144ec 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::{from_struct, value_to_json}; use openshell_isolation_interface::contract::ResolvedWorkloadIdentity; use openshell_sandbox_backend::boundary_protocol::{ GatewayVerificationKey, SandboxTlsClientConfig, SandboxTlsServerConfig, @@ -711,8 +711,7 @@ impl KubernetesSandboxDriverConfig { return Ok(Self::default()); }; - let json = serde_json::Value::Object(struct_to_json_object(config)); - let config: Self = serde_json::from_value(json) + let config: Self = from_struct(config) .map_err(|err| format!("invalid kubernetes driver_config: {err}"))?; config .validate() @@ -12891,6 +12890,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 +12937,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..ee2de97e98 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::from_struct; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::net::SocketAddr; @@ -239,10 +239,9 @@ fn sandbox_config(sandbox: &DriverSandbox) -> Result