diff --git a/tonic-xds/src/xds/resource/cluster.rs b/tonic-xds/src/xds/resource/cluster.rs index a21619e32..2f1c5a494 100644 --- a/tonic-xds/src/xds/resource/cluster.rs +++ b/tonic-xds/src/xds/resource/cluster.rs @@ -8,6 +8,9 @@ use xds_client::{Error, Resource}; use super::security::{ClusterSecurityConfig, parse_transport_socket}; +// Re-exported so other modules keep using the `cluster::LbPolicy` path. +pub(crate) use super::lb_policy::{LbPolicy, RingHashSettings}; + /// Validated Cluster resource. #[derive(Debug, Clone)] pub(crate) struct ClusterResource { @@ -22,13 +25,6 @@ pub(crate) struct ClusterResource { pub security: Option, } -/// Load balancing policies. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum LbPolicy { - RoundRobin, - LeastRequest, -} - impl Resource for ClusterResource { type Message = Cluster; @@ -58,6 +54,9 @@ impl Resource for ClusterResource { let lb_policy = match cluster::LbPolicy::try_from(message.lb_policy) { Ok(cluster::LbPolicy::RoundRobin) => LbPolicy::RoundRobin, Ok(cluster::LbPolicy::LeastRequest) => LbPolicy::LeastRequest, + Ok(cluster::LbPolicy::RingHash) => { + LbPolicy::RingHash(RingHashSettings::validate(message.lb_config)?) + } _ => { return Err(Error::Validation(format!( "unsupported load balancing policy: {}", @@ -143,9 +142,10 @@ mod tests { #[test] fn test_unsupported_lb_policy_is_rejected() { + // A policy we don't support (e.g. MAGLEV) is still NACKed. let cluster = Cluster { - name: "rh-cluster".to_string(), - lb_policy: cluster::LbPolicy::RingHash as i32, + name: "mg-cluster".to_string(), + lb_policy: cluster::LbPolicy::Maglev as i32, ..Default::default() }; let err = ClusterResource::validate(cluster).unwrap_err(); @@ -155,6 +155,24 @@ mod tests { ); } + #[test] + fn test_ring_hash_lb_policy() { + // A RING_HASH cluster validates to LbPolicy::RingHash with the parsed + // (here defaulted) settings. Detailed ring_hash_lb_config validation is + // covered in the `lb_policy` module. + let cluster = Cluster { + name: "rh-cluster".to_string(), + lb_policy: cluster::LbPolicy::RingHash as i32, + ..Default::default() + }; + let validated = ClusterResource::validate(cluster).unwrap(); + let LbPolicy::RingHash(settings) = validated.lb_policy else { + panic!("expected RingHash, got {:?}", validated.lb_policy); + }; + assert_eq!(settings.min_ring_size(), 1024); + assert_eq!(settings.max_ring_size(), 4096); + } + #[test] fn test_validate_empty_name() { let cluster = make_cluster(""); diff --git a/tonic-xds/src/xds/resource/lb_policy.rs b/tonic-xds/src/xds/resource/lb_policy.rs new file mode 100644 index 000000000..abe58fe82 --- /dev/null +++ b/tonic-xds/src/xds/resource/lb_policy.rs @@ -0,0 +1,209 @@ +//! Cluster load-balancing policy (CDS) and ring-hash config validation. + +use envoy_types::pb::envoy::config::cluster::v3::cluster::{ + self, ring_hash_lb_config::HashFunction, +}; +use xds_client::Error; + +/// Load balancing policies. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum LbPolicy { + RoundRobin, + LeastRequest, + /// Ring-hash (gRFC A42), carrying the validated ring bounds. + RingHash(RingHashSettings), +} + +/// Validated gRFC A42 ring-hash sizing parsed from `ring_hash_lb_config`. +/// +/// Fields are private so a `RingHashSettings` can only be obtained from +/// [`RingHashSettings::validate`] — i.e. every value is already within bounds. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct RingHashSettings { + min_ring_size: u64, + max_ring_size: u64, +} + +/// gRFC A42 ring-hash sizing. `minimum_ring_size` defaults to 1024 and +/// `maximum_ring_size` to the 8M ceiling when unset; both are then clamped to +/// the local cap of 4096, and any configured value above the ceiling is +/// rejected. Defaulting an unset max to the ceiling (rather than directly to +/// the cap) keeps the `min > max` check from rejecting a `min` in the +/// `(cap, ceiling]` range when `max` is unset. +pub(crate) const RING_HASH_DEFAULT_MIN_SIZE: u64 = 1024; +pub(crate) const RING_HASH_SIZE_CAP: u64 = 4096; +pub(crate) const RING_HASH_SIZE_CEILING: u64 = 8 * 1024 * 1024; + +impl RingHashSettings { + pub(crate) fn min_ring_size(&self) -> u64 { + self.min_ring_size + } + + pub(crate) fn max_ring_size(&self) -> u64 { + self.max_ring_size + } + + /// Validate a Cluster's `lb_config` oneof as `ring_hash_lb_config` (gRFC + /// A42). + /// + /// Rejects a `hash_function` other than `XX_HASH`, any ring size above the + /// 8M ceiling, and `min_ring_size > max_ring_size`. Unset sizes take the + /// defaults (1024 / 8M); the resolved bounds are then clamped to the local + /// cap. + pub(crate) fn validate(lb_config: Option) -> xds_client::Result { + let (min_field, max_field, hash_function) = match lb_config { + Some(cluster::LbConfig::RingHashLbConfig(c)) => { + (c.minimum_ring_size, c.maximum_ring_size, c.hash_function) + } + // No ring_hash_lb_config → defaults (XX_HASH is the proto default). + _ => (None, None, HashFunction::XxHash as i32), + }; + + match HashFunction::try_from(hash_function) { + Ok(HashFunction::XxHash) => {} + Ok(other) => { + return Err(Error::Validation(format!( + "unsupported ring_hash hash function: {}", + other.as_str_name() + ))); + } + Err(_) => { + return Err(Error::Validation(format!( + "unknown ring_hash hash function: {hash_function}" + ))); + } + } + + let min = min_field.map_or(RING_HASH_DEFAULT_MIN_SIZE, |v| v.value); + let max = max_field.map_or(RING_HASH_SIZE_CEILING, |v| v.value); + if min > RING_HASH_SIZE_CEILING || max > RING_HASH_SIZE_CEILING { + return Err(Error::Validation(format!( + "ring_hash ring size exceeds the maximum of {RING_HASH_SIZE_CEILING} \ + (min_ring_size={min}, max_ring_size={max})" + ))); + } + // Checked on the resolved sizes before the cap is applied. + if min > max { + return Err(Error::Validation(format!( + "ring_hash min_ring_size ({min}) is greater than max_ring_size ({max})" + ))); + } + + Ok(RingHashSettings { + min_ring_size: min.min(RING_HASH_SIZE_CAP), + max_ring_size: max.min(RING_HASH_SIZE_CAP), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use envoy_types::pb::google::protobuf::UInt64Value; + + fn ring_size(value: u64) -> UInt64Value { + UInt64Value { value } + } + + fn lb_config(config: cluster::RingHashLbConfig) -> Option { + Some(cluster::LbConfig::RingHashLbConfig(config)) + } + + #[test] + fn ring_hash_defaults() { + // No ring_hash_lb_config → min 1024, max defaults to the cap; XX_HASH + // is the default hash function. + let settings = RingHashSettings::validate(None).unwrap(); + assert_eq!( + settings, + RingHashSettings { + min_ring_size: 1024, + max_ring_size: 4096, + } + ); + } + + #[test] + fn ring_hash_custom_sizes_within_cap() { + let settings = RingHashSettings::validate(lb_config(cluster::RingHashLbConfig { + minimum_ring_size: Some(ring_size(2048)), + maximum_ring_size: Some(ring_size(3000)), + ..Default::default() + })) + .unwrap(); + assert_eq!( + settings, + RingHashSettings { + min_ring_size: 2048, + max_ring_size: 3000, + } + ); + } + + #[test] + fn ring_hash_sizes_clamped_to_cap() { + // Sizes within the 8M ceiling but above the local cap clamp to 4096. + let settings = RingHashSettings::validate(lb_config(cluster::RingHashLbConfig { + minimum_ring_size: Some(ring_size(100_000)), + maximum_ring_size: Some(ring_size(100_000)), + ..Default::default() + })) + .unwrap(); + assert_eq!( + settings, + RingHashSettings { + min_ring_size: 4096, + max_ring_size: 4096, + } + ); + } + + #[test] + fn ring_hash_rejects_non_xx_hash() { + let err = RingHashSettings::validate(lb_config(cluster::RingHashLbConfig { + hash_function: cluster::ring_hash_lb_config::HashFunction::MurmurHash2 as i32, + ..Default::default() + })) + .unwrap_err(); + assert!(err.to_string().contains("hash function")); + } + + #[test] + fn ring_hash_rejects_size_above_ceiling() { + let err = RingHashSettings::validate(lb_config(cluster::RingHashLbConfig { + maximum_ring_size: Some(ring_size(8 * 1024 * 1024 + 1)), + ..Default::default() + })) + .unwrap_err(); + assert!(err.to_string().contains("exceeds the maximum")); + } + + #[test] + fn ring_hash_rejects_min_greater_than_max() { + let err = RingHashSettings::validate(lb_config(cluster::RingHashLbConfig { + minimum_ring_size: Some(ring_size(3000)), + maximum_ring_size: Some(ring_size(2000)), + ..Default::default() + })) + .unwrap_err(); + assert!(err.to_string().contains("greater than max_ring_size")); + } + + #[test] + fn ring_hash_min_above_cap_with_unset_max_is_accepted() { + // min in (cap, ceiling] with max unset must not NACK: max defaults to + // 8M, so min <= max, and both then clamp to the cap. + let settings = RingHashSettings::validate(lb_config(cluster::RingHashLbConfig { + minimum_ring_size: Some(ring_size(5000)), + ..Default::default() + })) + .unwrap(); + assert_eq!( + settings, + RingHashSettings { + min_ring_size: 4096, + max_ring_size: 4096, + } + ); + } +} diff --git a/tonic-xds/src/xds/resource/mod.rs b/tonic-xds/src/xds/resource/mod.rs index e3e90daea..daf9f5627 100644 --- a/tonic-xds/src/xds/resource/mod.rs +++ b/tonic-xds/src/xds/resource/mod.rs @@ -13,6 +13,7 @@ pub(crate) mod cluster; pub(crate) mod endpoints; pub(crate) mod hash_policy; +pub(crate) mod lb_policy; pub(crate) mod listener; pub(crate) mod outlier_detection; pub(crate) mod route_config;