From 20d5b46ca68e2f5b7b740e61cfea44e9b4879723 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Fri, 4 Oct 2024 16:36:51 +0200 Subject: [PATCH 1/3] [feat] Setting timeout as Duration Signed-off-by: dd di cesare --- src/configuration.rs | 72 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 7 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 3dc43715..062314bc 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -4,19 +4,21 @@ use std::fmt::{Debug, Display, Formatter}; use std::rc::Rc; use std::sync::Arc; +use crate::attribute::Attribute; +use crate::envoy::{RateLimitDescriptor, RateLimitDescriptor_Entry}; +use crate::policy::Policy; +use crate::policy_index::PolicyIndex; +use crate::service::GrpcService; +use cel_interpreter::functions::duration; use cel_interpreter::objects::ValueType; use cel_interpreter::{Context, Expression, Value}; use cel_parser::{Atom, RelationOp}; use log::debug; use protobuf::RepeatedField; use proxy_wasm::hostcalls; -use serde::Deserialize; - -use crate::attribute::Attribute; -use crate::envoy::{RateLimitDescriptor, RateLimitDescriptor_Entry}; -use crate::policy::Policy; -use crate::policy_index::PolicyIndex; -use crate::service::GrpcService; +use serde::de::{Error, Visitor}; +use serde::{Deserialize, Deserializer}; +use std::time::Duration; #[derive(Deserialize, Debug, Clone)] pub struct SelectorItem { @@ -534,6 +536,52 @@ pub struct Extension { pub endpoint: String, // Deny/Allow request when faced with an irrecoverable failure. pub failure_mode: FailureMode, + #[serde(default)] + pub timeout: Timeout, +} + +#[derive(Debug, Clone, PartialEq)] +pub struct Timeout(pub Duration); +impl Default for Timeout { + fn default() -> Self { + Timeout(Duration::from_millis(20)) + } +} + +impl<'de> Deserialize<'de> for Timeout { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + deserializer.deserialize_str(TimeoutVisitor) + } +} + +struct TimeoutVisitor; +impl<'de> Visitor<'de> for TimeoutVisitor { + type Value = Timeout; + + fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result { + formatter.write_str("DurationString -> Sign? Number Unit String? Sign -> '-' Number -> Digit+ ('.' Digit+)? Digit -> '0' | '1' | '2' | '3' | '4' | '5' | '6' | '7' | '8' | '9' Unit -> 'h' | 'm' | 's' | 'ms' | 'us' | 'ns' String -> DurationString") + } + + fn visit_str(self, string: &str) -> Result + where + E: Error, + { + self.visit_string(String::from(string)) + } + + fn visit_string(self, string: String) -> Result + where + E: Error, + { + match duration(Arc::new(string)) { + Ok(Value::Duration(duration)) => Ok(Timeout(duration.to_std().unwrap())), + Err(e) => Err(E::custom(e)), + _ => Err(E::custom("Unsupported Duration Value")), + } + } } #[derive(Deserialize, Debug, Clone)] @@ -628,11 +676,13 @@ mod test { "type": "auth", "endpoint": "authorino-cluster", "failureMode": "deny" + "timeout": "24ms" }, "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", "failureMode": "allow" + "timeout": "42ms" } }, "policies": [ @@ -703,6 +753,7 @@ mod test { assert_eq!(auth_extension.extension_type, ExtensionType::Auth); assert_eq!(auth_extension.endpoint, "authorino-cluster"); assert_eq!(auth_extension.failure_mode, FailureMode::Deny); + assert_eq!(auth_extension.timeout, Timeout(Duration::from_millis(24))); } else { panic!() } @@ -711,6 +762,7 @@ mod test { assert_eq!(rl_extension.extension_type, ExtensionType::RateLimit); assert_eq!(rl_extension.endpoint, "limitador-cluster"); assert_eq!(rl_extension.failure_mode, FailureMode::Allow); + assert_eq!(rl_extension.timeout, Timeout(Duration::from_millis(42))); } else { panic!() } @@ -979,6 +1031,12 @@ mod test { let filter_config = res.unwrap(); assert_eq!(filter_config.policies.len(), 1); + let extensions = &filter_config.extensions; + assert_eq!( + extensions.get("limitador").unwrap().timeout, + Timeout(Duration::from_millis(20)) + ); + let rules = &filter_config.policies[0].rules; assert_eq!(rules.len(), 1); From e6d485f1fe57de6b079ef5c3d4ec7f551cc8cfc9 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Fri, 4 Oct 2024 18:01:34 +0200 Subject: [PATCH 2/3] [refactor] Using Timeout when sending request message Signed-off-by: dd di cesare --- src/configuration.rs | 8 ++++---- src/operation_dispatcher.rs | 11 ++++++++--- src/service.rs | 3 ++- tests/auth.rs | 3 ++- tests/multi.rs | 6 ++++-- tests/rate_limited.rs | 12 ++++++++---- 6 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 062314bc..fbe2abed 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -675,13 +675,13 @@ mod test { "authorino": { "type": "auth", "endpoint": "authorino-cluster", - "failureMode": "deny" + "failureMode": "deny", "timeout": "24ms" }, "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", - "failureMode": "allow" + "failureMode": "allow", "timeout": "42ms" } }, @@ -753,7 +753,7 @@ mod test { assert_eq!(auth_extension.extension_type, ExtensionType::Auth); assert_eq!(auth_extension.endpoint, "authorino-cluster"); assert_eq!(auth_extension.failure_mode, FailureMode::Deny); - assert_eq!(auth_extension.timeout, Timeout(Duration::from_millis(24))); + assert_eq!(auth_extension.timeout, Timeout(Duration::from_millis(24))) } else { panic!() } @@ -762,7 +762,7 @@ mod test { assert_eq!(rl_extension.extension_type, ExtensionType::RateLimit); assert_eq!(rl_extension.endpoint, "limitador-cluster"); assert_eq!(rl_extension.failure_mode, FailureMode::Allow); - assert_eq!(rl_extension.timeout, Timeout(Duration::from_millis(42))); + assert_eq!(rl_extension.timeout, Timeout(Duration::from_millis(42))) } else { panic!() } diff --git a/src/operation_dispatcher.rs b/src/operation_dispatcher.rs index 8668d077..3bb87710 100644 --- a/src/operation_dispatcher.rs +++ b/src/operation_dispatcher.rs @@ -60,9 +60,12 @@ impl Operation { fn trigger(&self) -> Result { if let Some(message) = (self.grpc_message_build_fn)(self.get_extension_type(), &self.action) { - let res = self - .service - .send(self.get_map_values_bytes_fn, self.grpc_call_fn, message); + let res = self.service.send( + self.get_map_values_bytes_fn, + self.grpc_call_fn, + message, + self.extension.timeout.0, + ); self.set_result(res); self.next_state(); res @@ -229,6 +232,7 @@ fn grpc_message_build_fn( #[cfg(test)] mod tests { use super::*; + use crate::configuration::Timeout; use crate::envoy::RateLimitRequest; use protobuf::RepeatedField; use std::time::Duration; @@ -283,6 +287,7 @@ mod tests { extension_type, endpoint: "local".to_string(), failure_mode: FailureMode::Deny, + timeout: Timeout(Duration::from_millis(42)), }), action: Action { extension: "local".to_string(), diff --git a/src/service.rs b/src/service.rs index 3d0e7cc2..c8c4eed1 100644 --- a/src/service.rs +++ b/src/service.rs @@ -125,6 +125,7 @@ impl GrpcServiceHandler { get_map_values_bytes_fn: GetMapValuesBytesFn, grpc_call_fn: GrpcCallFn, message: GrpcMessageRequest, + timeout: Duration, ) -> Result { let msg = Message::write_to_bytes(&message).unwrap(); let metadata = self @@ -140,7 +141,7 @@ impl GrpcServiceHandler { self.service.method(), metadata, Some(&msg), - Duration::from_secs(5), + timeout, ) } diff --git a/tests/auth.rs b/tests/auth.rs index 8727aa1a..85006da8 100644 --- a/tests/auth.rs +++ b/tests/auth.rs @@ -9,7 +9,8 @@ const CONFIG: &str = r#"{ "authorino": { "type": "auth", "endpoint": "authorino-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" } }, "policies": [ diff --git a/tests/multi.rs b/tests/multi.rs index 3c2604c8..3c6525f4 100644 --- a/tests/multi.rs +++ b/tests/multi.rs @@ -10,12 +10,14 @@ const CONFIG: &str = r#"{ "authorino": { "type": "auth", "endpoint": "authorino-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" }, "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" } }, "policies": [ diff --git a/tests/rate_limited.rs b/tests/rate_limited.rs index a8b92718..7daeb930 100644 --- a/tests/rate_limited.rs +++ b/tests/rate_limited.rs @@ -87,7 +87,8 @@ fn it_limits() { "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" } }, "policies": [ @@ -242,7 +243,8 @@ fn it_passes_additional_headers() { "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" } }, "policies": [ @@ -411,7 +413,8 @@ fn it_rate_limits_with_empty_conditions() { "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" } }, "policies": [ @@ -529,7 +532,8 @@ fn it_does_not_rate_limits_when_selector_does_not_exist_and_misses_default_value "limitador": { "type": "ratelimit", "endpoint": "limitador-cluster", - "failureMode": "deny" + "failureMode": "deny", + "timeout": "5s" } }, "policies": [ From c970dd3f2438f4cf885716d81ff947d85e05ce35 Mon Sep 17 00:00:00 2001 From: dd di cesare Date: Fri, 4 Oct 2024 18:12:06 +0200 Subject: [PATCH 3/3] [docs] Added timeout to README Signed-off-by: dd di cesare --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 7f8ccd66..a3eb6d47 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,7 @@ extensions: type: auth endpoint: auth-cluster failureMode: deny + timeout: 10ms ratelimit-ext: type: ratelimit endpoint: ratelimit-cluster