From 4eb2ea65eba3ce2c6e50469457a8f2d5d8698899 Mon Sep 17 00:00:00 2001 From: Alex Snaps Date: Wed, 8 Jun 2022 16:04:01 -0400 Subject: [PATCH] Just call passing in the Namespace --- limitador-server/src/envoy_rls/server.rs | 6 +- limitador-server/src/http_api/server.rs | 30 +++-- limitador/benches/bench.rs | 14 ++- limitador/src/lib.rs | 149 +++++++++-------------- limitador/tests/helpers/tests_limiter.rs | 36 ++++-- 5 files changed, 114 insertions(+), 121 deletions(-) diff --git a/limitador-server/src/envoy_rls/server.rs b/limitador-server/src/envoy_rls/server.rs index f6f89e70..0ee4d91c 100644 --- a/limitador-server/src/envoy_rls/server.rs +++ b/limitador-server/src/envoy_rls/server.rs @@ -46,6 +46,8 @@ impl RateLimitService for MyRateLimiter { })); } + let namespace = namespace.into(); + for descriptor in &req.descriptors { for entry in &descriptor.entries { values.insert(entry.key.clone(), entry.value.clone()); @@ -62,11 +64,11 @@ impl RateLimitService for MyRateLimiter { let is_rate_limited_res = match &*self.limiter { Limiter::Blocking(limiter) => { - limiter.check_rate_limited_and_update(namespace, &values, i64::from(hits_addend)) + limiter.check_rate_limited_and_update(&namespace, &values, i64::from(hits_addend)) } Limiter::Async(limiter) => { limiter - .check_rate_limited_and_update(namespace, &values, i64::from(hits_addend)) + .check_rate_limited_and_update(&namespace, &values, i64::from(hits_addend)) .await } }; diff --git a/limitador-server/src/http_api/server.rs b/limitador-server/src/http_api/server.rs index 82c0541d..d9d5a4dc 100644 --- a/limitador-server/src/http_api/server.rs +++ b/limitador-server/src/http_api/server.rs @@ -73,9 +73,10 @@ async fn get_limits( data: web::Data>, namespace: web::Path, ) -> Result>, ErrorResponse> { + let namespace = &namespace.into_inner().into(); let get_limits_result = match data.get_ref().as_ref() { - Limiter::Blocking(limiter) => limiter.get_limits(namespace.into_inner()), - Limiter::Async(limiter) => limiter.get_limits(namespace.into_inner()).await, + Limiter::Blocking(limiter) => limiter.get_limits(namespace), + Limiter::Async(limiter) => limiter.get_limits(namespace).await, }; match get_limits_result { @@ -108,9 +109,10 @@ async fn delete_limits( data: web::Data>, namespace: web::Path, ) -> Result, ErrorResponse> { + let namespace = namespace.into_inner().into(); let delete_limits_result = match data.get_ref().as_ref() { - Limiter::Blocking(limiter) => limiter.delete_limits(namespace.into_inner()), - Limiter::Async(limiter) => limiter.delete_limits(namespace.into_inner()).await, + Limiter::Blocking(limiter) => limiter.delete_limits(&namespace), + Limiter::Async(limiter) => limiter.delete_limits(&namespace).await, }; match delete_limits_result { @@ -124,9 +126,10 @@ async fn get_counters( data: web::Data>, namespace: web::Path, ) -> Result>, ErrorResponse> { + let namespace = namespace.into_inner().into(); let get_counters_result = match data.get_ref().as_ref() { - Limiter::Blocking(limiter) => limiter.get_counters(namespace.into_inner()), - Limiter::Async(limiter) => limiter.get_counters(namespace.into_inner()).await, + Limiter::Blocking(limiter) => limiter.get_counters(&namespace), + Limiter::Async(limiter) => limiter.get_counters(&namespace).await, }; match get_counters_result { @@ -151,9 +154,10 @@ async fn check( values, delta, } = request.into_inner(); + let namespace = namespace.into(); let is_rate_limited_result = match state.get_ref().as_ref() { - Limiter::Blocking(limiter) => limiter.is_rate_limited(namespace, &values, delta), - Limiter::Async(limiter) => limiter.is_rate_limited(namespace, &values, delta).await, + Limiter::Blocking(limiter) => limiter.is_rate_limited(&namespace, &values, delta), + Limiter::Async(limiter) => limiter.is_rate_limited(&namespace, &values, delta).await, }; match is_rate_limited_result { @@ -178,9 +182,10 @@ async fn report( values, delta, } = request.into_inner(); + let namespace = namespace.into(); let update_counters_result = match data.get_ref().as_ref() { - Limiter::Blocking(limiter) => limiter.update_counters(namespace, &values, delta), - Limiter::Async(limiter) => limiter.update_counters(namespace, &values, delta).await, + Limiter::Blocking(limiter) => limiter.update_counters(&namespace, &values, delta), + Limiter::Async(limiter) => limiter.update_counters(&namespace, &values, delta).await, }; match update_counters_result { @@ -199,13 +204,14 @@ async fn check_and_report( values, delta, } = request.into_inner(); + let namespace = namespace.into(); let rate_limited_and_update_result = match data.get_ref().as_ref() { Limiter::Blocking(limiter) => { - limiter.check_rate_limited_and_update(namespace, &values, delta) + limiter.check_rate_limited_and_update(&namespace, &values, delta) } Limiter::Async(limiter) => { limiter - .check_rate_limited_and_update(namespace, &values, delta) + .check_rate_limited_and_update(&namespace, &values, delta) .await } }; diff --git a/limitador/benches/bench.rs b/limitador/benches/bench.rs index e8afc071..60bebe82 100644 --- a/limitador/benches/bench.rs +++ b/limitador/benches/bench.rs @@ -130,7 +130,11 @@ fn bench_is_rate_limited(b: &mut Bencher, test_scenario: &TestScenario, storage: black_box( rate_limiter - .is_rate_limited(params.namespace.as_ref(), ¶ms.values, params.delta) + .is_rate_limited( + ¶ms.namespace.to_owned().into(), + ¶ms.values, + params.delta, + ) .unwrap(), ) }) @@ -145,7 +149,11 @@ fn bench_update_counters(b: &mut Bencher, test_scenario: &TestScenario, storage: black_box( rate_limiter - .update_counters(params.namespace.as_ref(), ¶ms.values, params.delta) + .update_counters( + ¶ms.namespace.to_owned().into(), + ¶ms.values, + params.delta, + ) .unwrap(), ) }) @@ -165,7 +173,7 @@ fn bench_check_rate_limited_and_update( black_box( rate_limiter .check_rate_limited_and_update( - params.namespace.as_ref(), + ¶ms.namespace.to_owned().into(), ¶ms.values, params.delta, ) diff --git a/limitador/src/lib.rs b/limitador/src/lib.rs index 4b65ba9d..81927189 100644 --- a/limitador/src/lib.rs +++ b/limitador/src/lib.rs @@ -81,10 +81,11 @@ //! rate_limiter.delete_limit(&limit); //! //! // Get all the limits in a namespace -//! rate_limiter.get_limits("my_namespace"); +//! let namespace = "my_namespace".into(); +//! rate_limiter.get_limits(&namespace); //! //! // Delete all the limits in a namespace -//! rate_limiter.delete_limits("my_namespace"); +//! rate_limiter.delete_limits(&namespace); //! ``` //! //! # Apply limits @@ -112,22 +113,23 @@ //! values_to_report.insert("user_id".to_string(), "1".to_string()); //! //! // Check if we can report -//! assert!(!rate_limiter.is_rate_limited("my_namespace", &values_to_report, 1).unwrap()); +//! let namespace = "my_namespace".into(); +//! assert!(!rate_limiter.is_rate_limited(&namespace, &values_to_report, 1).unwrap()); //! //! // Report -//! rate_limiter.update_counters("my_namespace", &values_to_report, 1).unwrap(); +//! rate_limiter.update_counters(&namespace, &values_to_report, 1).unwrap(); //! //! // Check and report again -//! assert!(!rate_limiter.is_rate_limited("my_namespace", &values_to_report, 1).unwrap()); -//! rate_limiter.update_counters("my_namespace", &values_to_report, 1).unwrap(); +//! assert!(!rate_limiter.is_rate_limited(&namespace, &values_to_report, 1).unwrap()); +//! rate_limiter.update_counters(&namespace, &values_to_report, 1).unwrap(); //! //! // We've already reported 2, so reporting another one should not be allowed -//! assert!(rate_limiter.is_rate_limited("my_namespace", &values_to_report, 1).unwrap()); +//! assert!(rate_limiter.is_rate_limited(&namespace, &values_to_report, 1).unwrap()); //! //! // You can also check and report if not limited in a single call. It's useful //! // for example, when calling Limitador from a proxy. Instead of doing 2 //! // separate calls, we can issue just one: -//! rate_limiter.check_rate_limited_and_update("my_namespace", &values_to_report, 1).unwrap(); +//! rate_limiter.check_rate_limited_and_update(&namespace, &values_to_report, 1).unwrap(); //! ``` //! //! # Async @@ -160,7 +162,7 @@ //! use limitador::limit::Limit; //! use limitador::storage::redis::AsyncRedisStorage; //! let limit = Limit::new( -//! "my_namespace", +//! "my_namespace", //! 10, //! 60, //! vec!["req.method == GET"], @@ -188,7 +190,6 @@ // TODO this needs review to reduce the bloat pulled in by dependencies #![allow(clippy::multiple_crate_versions)] -use core::fmt::Debug; use std::collections::{HashMap, HashSet}; use crate::counter::Counter; @@ -319,45 +320,30 @@ impl RateLimiter { self.storage.delete_limit(limit).map_err(|err| err.into()) } - pub fn get_limits>( - &self, - namespace: N, - ) -> Result, LimitadorError> - where - >::Error: Debug, - { - self.storage - .get_limits(&namespace.into()) - .map_err(|err| err.into()) + pub fn get_limits(&self, namespace: &Namespace) -> Result, LimitadorError> { + self.storage.get_limits(namespace).map_err(|err| err.into()) } - pub fn delete_limits>(&self, namespace: N) -> Result<(), LimitadorError> - where - >::Error: Debug, - { + pub fn delete_limits(&self, namespace: &Namespace) -> Result<(), LimitadorError> { self.storage - .delete_limits(&namespace.into()) + .delete_limits(namespace) .map_err(|err| err.into()) } - pub fn is_rate_limited>( + pub fn is_rate_limited( &self, - namespace: N, + namespace: &Namespace, values: &HashMap, delta: i64, - ) -> Result - where - >::Error: Debug, - { - let namespace = namespace.into(); - let counters = self.counters_that_apply(namespace.clone(), values)?; + ) -> Result { + let counters = self.counters_that_apply(namespace, values)?; for counter in counters { match self.storage.is_within_limits(&counter, delta) { Ok(within_limits) => { if !within_limits { self.prometheus_metrics - .incr_limited_calls(&namespace, counter.limit().name()); + .incr_limited_calls(namespace, counter.limit().name()); return Ok(true); } } @@ -365,19 +351,16 @@ impl RateLimiter { } } - self.prometheus_metrics.incr_authorized_calls(&namespace); + self.prometheus_metrics.incr_authorized_calls(namespace); Ok(false) } - pub fn update_counters>( + pub fn update_counters( &self, - namespace: N, + namespace: &Namespace, values: &HashMap, delta: i64, - ) -> Result<(), LimitadorError> - where - >::Error: Debug, - { + ) -> Result<(), LimitadorError> { let counters = self.counters_that_apply(namespace, values)?; counters @@ -386,20 +369,16 @@ impl RateLimiter { .map_err(|err| err.into()) } - pub fn check_rate_limited_and_update>( + pub fn check_rate_limited_and_update( &self, - namespace: N, + namespace: &Namespace, values: &HashMap, delta: i64, - ) -> Result - where - >::Error: Debug, - { - let namespace = namespace.into(); - let counters = self.counters_that_apply(namespace.clone(), values)?; + ) -> Result { + let counters = self.counters_that_apply(namespace, values)?; if counters.is_empty() { - self.prometheus_metrics.incr_authorized_calls(&namespace); + self.prometheus_metrics.incr_authorized_calls(namespace); return Ok(false); } @@ -409,23 +388,20 @@ impl RateLimiter { match check_result { Authorization::Ok => { - self.prometheus_metrics.incr_authorized_calls(&namespace); + self.prometheus_metrics.incr_authorized_calls(namespace); Ok(false) } Authorization::Limited(c) => { self.prometheus_metrics - .incr_limited_calls(&namespace, c.limit().name()); + .incr_limited_calls(namespace, c.limit().name()); Ok(true) } } } - pub fn get_counters( - &self, - namespace: impl Into, - ) -> Result, LimitadorError> { + pub fn get_counters(&self, namespace: &Namespace) -> Result, LimitadorError> { self.storage - .get_counters(&namespace.into()) + .get_counters(namespace) .map_err(|err| err.into()) } @@ -445,7 +421,7 @@ impl RateLimiter { .get_namespaces()? .union(&namespaces_limits_to_keep_or_create) { - let limits_in_namespace = self.get_limits(namespace.clone())?; + let limits_in_namespace = self.get_limits(namespace)?; let limits_to_keep_in_ns: HashSet = limits_to_keep_or_create .get(namespace) .cloned() @@ -467,14 +443,11 @@ impl RateLimiter { self.prometheus_metrics.gather_metrics() } - fn counters_that_apply>( + fn counters_that_apply( &self, - namespace: N, + namespace: &Namespace, values: &HashMap, - ) -> Result, LimitadorError> - where - >::Error: Debug, - { + ) -> Result, LimitadorError> { let limits = self.get_limits(namespace)?; let counters = limits @@ -529,39 +502,35 @@ impl AsyncRateLimiter { pub async fn get_limits( &self, - namespace: impl Into, + namespace: &Namespace, ) -> Result, LimitadorError> { self.storage - .get_limits(&namespace.into()) + .get_limits(namespace) .await .map_err(|err| err.into()) } - pub async fn delete_limits( - &self, - namespace: impl Into, - ) -> Result<(), LimitadorError> { + pub async fn delete_limits(&self, namespace: &Namespace) -> Result<(), LimitadorError> { self.storage - .delete_limits(&namespace.into()) + .delete_limits(namespace) .await .map_err(|err| err.into()) } pub async fn is_rate_limited( &self, - namespace: impl Into, + namespace: &Namespace, values: &HashMap, delta: i64, ) -> Result { - let namespace = namespace.into(); - let counters = self.counters_that_apply(namespace.clone(), values).await?; + let counters = self.counters_that_apply(namespace, values).await?; for counter in counters { match self.storage.is_within_limits(&counter, delta).await { Ok(within_limits) => { if !within_limits { self.prometheus_metrics - .incr_limited_calls(&namespace, counter.limit().name()); + .incr_limited_calls(namespace, counter.limit().name()); return Ok(true); } } @@ -569,13 +538,13 @@ impl AsyncRateLimiter { } } - self.prometheus_metrics.incr_authorized_calls(&namespace); + self.prometheus_metrics.incr_authorized_calls(namespace); Ok(false) } pub async fn update_counters( &self, - namespace: impl Into, + namespace: &Namespace, values: &HashMap, delta: i64, ) -> Result<(), LimitadorError> { @@ -588,21 +557,17 @@ impl AsyncRateLimiter { Ok(()) } - pub async fn check_rate_limited_and_update>( + pub async fn check_rate_limited_and_update( &self, - namespace: N, + namespace: &Namespace, values: &HashMap, delta: i64, - ) -> Result - where - >::Error: Debug, - { + ) -> Result { // the above where-clause is needed in order to call unwrap(). - let namespace = namespace.into(); - let counters = self.counters_that_apply(namespace.clone(), values).await?; + let counters = self.counters_that_apply(namespace, values).await?; if counters.is_empty() { - self.prometheus_metrics.incr_authorized_calls(&namespace); + self.prometheus_metrics.incr_authorized_calls(namespace); return Ok(false); } @@ -613,12 +578,12 @@ impl AsyncRateLimiter { match check_result { Authorization::Ok => { - self.prometheus_metrics.incr_authorized_calls(&namespace); + self.prometheus_metrics.incr_authorized_calls(namespace); Ok(false) } Authorization::Limited(c) => { self.prometheus_metrics - .incr_limited_calls(&namespace, c.limit().name()); + .incr_limited_calls(namespace, c.limit().name()); Ok(true) } } @@ -626,10 +591,10 @@ impl AsyncRateLimiter { pub async fn get_counters( &self, - namespace: impl Into, + namespace: &Namespace, ) -> Result, LimitadorError> { self.storage - .get_counters(&namespace.into()) + .get_counters(namespace) .await .map_err(|err| err.into()) } @@ -651,7 +616,7 @@ impl AsyncRateLimiter { .await? .union(&namespaces_limits_to_keep_or_create) { - let limits_in_namespace = self.get_limits(namespace.clone()).await?; + let limits_in_namespace = self.get_limits(namespace).await?; let limits_to_keep_in_ns: HashSet = limits_to_keep_or_create .get(namespace) .cloned() @@ -675,7 +640,7 @@ impl AsyncRateLimiter { async fn counters_that_apply( &self, - namespace: impl Into, + namespace: &Namespace, values: &HashMap, ) -> Result, LimitadorError> { let limits = self.get_limits(namespace).await?; diff --git a/limitador/tests/helpers/tests_limiter.rs b/limitador/tests/helpers/tests_limiter.rs index 0246bfc7..471c0c21 100644 --- a/limitador/tests/helpers/tests_limiter.rs +++ b/limitador/tests/helpers/tests_limiter.rs @@ -55,15 +55,15 @@ impl TestsLimiter { pub async fn get_limits(&self, namespace: &str) -> Result, LimitadorError> { match &self.limiter_impl { - LimiterImpl::Blocking(limiter) => limiter.get_limits(namespace), - LimiterImpl::Async(limiter) => limiter.get_limits(namespace).await, + LimiterImpl::Blocking(limiter) => limiter.get_limits(&namespace.into()), + LimiterImpl::Async(limiter) => limiter.get_limits(&namespace.into()).await, } } pub async fn delete_limits(&self, namespace: &str) -> Result<(), LimitadorError> { match &self.limiter_impl { - LimiterImpl::Blocking(limiter) => limiter.delete_limits(namespace), - LimiterImpl::Async(limiter) => limiter.delete_limits(namespace).await, + LimiterImpl::Blocking(limiter) => limiter.delete_limits(&namespace.into()), + LimiterImpl::Async(limiter) => limiter.delete_limits(&namespace.into()).await, } } @@ -74,8 +74,14 @@ impl TestsLimiter { delta: i64, ) -> Result { match &self.limiter_impl { - LimiterImpl::Blocking(limiter) => limiter.is_rate_limited(namespace, values, delta), - LimiterImpl::Async(limiter) => limiter.is_rate_limited(namespace, values, delta).await, + LimiterImpl::Blocking(limiter) => { + limiter.is_rate_limited(&namespace.into(), values, delta) + } + LimiterImpl::Async(limiter) => { + limiter + .is_rate_limited(&namespace.into(), values, delta) + .await + } } } @@ -86,8 +92,14 @@ impl TestsLimiter { delta: i64, ) -> Result<(), LimitadorError> { match &self.limiter_impl { - LimiterImpl::Blocking(limiter) => limiter.update_counters(namespace, values, delta), - LimiterImpl::Async(limiter) => limiter.update_counters(namespace, values, delta).await, + LimiterImpl::Blocking(limiter) => { + limiter.update_counters(&namespace.into(), values, delta) + } + LimiterImpl::Async(limiter) => { + limiter + .update_counters(&namespace.into(), values, delta) + .await + } } } @@ -99,11 +111,11 @@ impl TestsLimiter { ) -> Result { match &self.limiter_impl { LimiterImpl::Blocking(limiter) => { - limiter.check_rate_limited_and_update(namespace, values, delta) + limiter.check_rate_limited_and_update(&namespace.into(), values, delta) } LimiterImpl::Async(limiter) => { limiter - .check_rate_limited_and_update(namespace, values, delta) + .check_rate_limited_and_update(&namespace.into(), values, delta) .await } } @@ -111,8 +123,8 @@ impl TestsLimiter { pub async fn get_counters(&self, namespace: &str) -> Result, LimitadorError> { match &self.limiter_impl { - LimiterImpl::Blocking(limiter) => limiter.get_counters(namespace), - LimiterImpl::Async(limiter) => limiter.get_counters(namespace).await, + LimiterImpl::Blocking(limiter) => limiter.get_counters(&namespace.into()), + LimiterImpl::Async(limiter) => limiter.get_counters(&namespace.into()).await, } }