From 37d3eaab1958399fc895fa671709edb4928fa1dc Mon Sep 17 00:00:00 2001 From: Adam Cattermole Date: Thu, 29 Aug 2024 11:19:12 +0100 Subject: [PATCH 1/3] Create service at configuration level Signed-off-by: Adam Cattermole --- src/configuration.rs | 9 +++++ src/filter/http_context.rs | 5 +-- src/service.rs | 75 ++++++++++++++++++++------------------ 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 9526f8cd..a3436dde 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -1,5 +1,6 @@ use std::cell::OnceCell; use std::fmt::{Debug, Display, Formatter}; +use std::rc::Rc; use std::sync::Arc; use cel_interpreter::objects::ValueType; @@ -10,6 +11,7 @@ use serde::Deserialize; use crate::attribute::Attribute; use crate::policy::Policy; use crate::policy_index::PolicyIndex; +use crate::service::GrpcService; #[derive(Deserialize, Debug, Clone)] pub struct SelectorItem { @@ -441,6 +443,7 @@ pub struct FilterConfig { pub index: PolicyIndex, // Deny/Allow request when faced with an irrecoverable failure. pub failure_mode: FailureMode, + pub service: Rc, } impl Default for FilterConfig { @@ -448,6 +451,7 @@ impl Default for FilterConfig { Self { index: PolicyIndex::new(), failure_mode: FailureMode::Deny, + service: Rc::new(GrpcService::default()), } } } @@ -480,9 +484,14 @@ impl TryFrom for FilterConfig { } } + // todo(adam-cattermole): retrieve from config + let rl_service = + GrpcService::new(ExtensionType::RateLimit, config.policies[0].service.clone()); + Ok(Self { index, failure_mode: config.failure_mode, + service: Rc::new(rl_service), }) } } diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index ba2fb690..531ab7f8 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -1,4 +1,4 @@ -use crate::configuration::{ExtensionType, FailureMode, FilterConfig}; +use crate::configuration::{FailureMode, FilterConfig}; use crate::envoy::{RateLimitResponse, RateLimitResponse_Code}; use crate::policy::Policy; use crate::service::rate_limit::RateLimitService; @@ -41,8 +41,7 @@ impl Filter { } let rls = GrpcServiceHandler::new( - ExtensionType::RateLimit, - rlp.service.clone(), + Rc::clone(&self.config.service), Rc::clone(&self.header_resolver), ); let message = RateLimitService::message(rlp.domain.clone(), descriptors); diff --git a/src/service.rs b/src/service.rs index 1f16198a..e0b01681 100644 --- a/src/service.rs +++ b/src/service.rs @@ -13,49 +13,52 @@ use std::cell::OnceCell; use std::rc::Rc; use std::time::Duration; -pub struct GrpcServiceHandler { +#[derive(Default)] +pub struct GrpcService { endpoint: String, - service_name: String, - method_name: String, + name: &'static str, + method: &'static str, +} + +impl GrpcService { + pub fn new(extension_type: ExtensionType, endpoint: String) -> Self { + match extension_type { + ExtensionType::Auth => Self { + endpoint, + name: AUTH_SERVICE_NAME, + method: AUTH_METHOD_NAME, + }, + ExtensionType::RateLimit => Self { + endpoint, + name: RATELIMIT_SERVICE_NAME, + method: RATELIMIT_METHOD_NAME, + }, + } + } + fn endpoint(&self) -> &str { + &self.endpoint + } + fn name(&self) -> &str { + self.name + } + fn method(&self) -> &str { + self.method + } +} + +pub struct GrpcServiceHandler { + service: Rc, header_resolver: Rc, } impl GrpcServiceHandler { - fn build( - endpoint: String, - service_name: &str, - method_name: &str, - header_resolver: Rc, - ) -> Self { + pub fn new(service: Rc, header_resolver: Rc) -> Self { Self { - endpoint: endpoint.to_owned(), - service_name: service_name.to_owned(), - method_name: method_name.to_owned(), + service, header_resolver, } } - pub fn new( - extension_type: ExtensionType, - endpoint: String, - header_resolver: Rc, - ) -> Self { - match extension_type { - ExtensionType::Auth => Self::build( - endpoint, - AUTH_SERVICE_NAME, - AUTH_METHOD_NAME, - header_resolver, - ), - ExtensionType::RateLimit => Self::build( - endpoint, - RATELIMIT_SERVICE_NAME, - RATELIMIT_METHOD_NAME, - header_resolver, - ), - } - } - pub fn send(&self, message: M) -> Result { let msg = Message::write_to_bytes(&message).unwrap(); let metadata = self @@ -66,9 +69,9 @@ impl GrpcServiceHandler { .collect(); dispatch_grpc_call( - self.endpoint.as_str(), - self.service_name.as_str(), - self.method_name.as_str(), + self.service.endpoint(), + self.service.name(), + self.service.method(), metadata, Some(&msg), Duration::from_secs(5), From 91822bed27d117902b5c086be83403b21463edeb Mon Sep 17 00:00:00 2001 From: Adam Cattermole Date: Thu, 29 Aug 2024 13:30:12 +0100 Subject: [PATCH 2/3] Add and use extensions from configuration Signed-off-by: Adam Cattermole --- src/configuration.rs | 122 ++++++++++++++++++++++++++----------- src/filter/http_context.rs | 28 ++++++--- src/policy.rs | 10 +-- src/policy_index.rs | 8 +-- src/service.rs | 17 +++++- 5 files changed, 126 insertions(+), 59 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index a3436dde..554c086d 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -1,4 +1,5 @@ use std::cell::OnceCell; +use std::collections::HashMap; use std::fmt::{Debug, Display, Formatter}; use std::rc::Rc; use std::sync::Arc; @@ -441,17 +442,14 @@ pub fn type_of(path: &str) -> Option { pub struct FilterConfig { pub index: PolicyIndex, - // Deny/Allow request when faced with an irrecoverable failure. - pub failure_mode: FailureMode, - pub service: Rc, + pub services: Rc>>, } impl Default for FilterConfig { fn default() -> Self { Self { index: PolicyIndex::new(), - failure_mode: FailureMode::Deny, - service: Rc::new(GrpcService::default()), + services: Rc::new(HashMap::new()), } } } @@ -484,21 +482,33 @@ impl TryFrom for FilterConfig { } } - // todo(adam-cattermole): retrieve from config - let rl_service = - GrpcService::new(ExtensionType::RateLimit, config.policies[0].service.clone()); + // configure grpc services from the extensions in config + let services = config + .extensions + .into_iter() + .map(|(name, ext)| { + ( + name, + Rc::new(GrpcService::new( + ext.extension_type, + ext.endpoint, + ext.failure_mode, + )), + ) + }) + .collect(); Ok(Self { index, - failure_mode: config.failure_mode, - service: Rc::new(rl_service), + services: Rc::new(services), }) } } -#[derive(Deserialize, Debug, Clone)] +#[derive(Deserialize, Debug, Clone, Default)] #[serde(rename_all = "lowercase")] pub enum FailureMode { + #[default] Deny, Allow, } @@ -513,8 +523,16 @@ pub enum ExtensionType { #[derive(Deserialize, Debug, Clone)] #[serde(rename_all = "camelCase")] pub struct PluginConfiguration { - #[serde(rename = "rateLimitPolicies")] + pub extensions: HashMap, pub policies: Vec, +} + +#[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "camelCase")] +pub struct Extension { + #[serde(rename = "type")] + pub extension_type: ExtensionType, + pub endpoint: String, // Deny/Allow request when faced with an irrecoverable failure. pub failure_mode: FailureMode, } @@ -524,12 +542,17 @@ mod test { use super::*; const CONFIG: &str = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -621,8 +644,8 @@ mod test { #[test] fn parse_config_min() { let config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [] + "extensions": {}, + "policies": [] }"#; let res = serde_json::from_str::(config); if let Err(ref e) = res { @@ -637,12 +660,17 @@ mod test { #[test] fn parse_config_data_selector() { let config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -687,12 +715,17 @@ mod test { #[test] fn parse_config_condition_selector_operators() { let config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -766,12 +799,17 @@ mod test { #[test] fn parse_config_conditions_optional() { let config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -810,12 +848,17 @@ mod test { fn parse_config_invalid_data() { // data item fields are mutually exclusive let bad_config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -837,8 +880,14 @@ mod test { // data item unknown fields are forbidden let bad_config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", @@ -861,12 +910,17 @@ mod test { // condition selector operator unknown let bad_config = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "rlp-ns-A/rlp-name-A", "domain": "rlp-ns-A/rlp-name-A", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index 531ab7f8..fa80e75c 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -40,13 +40,19 @@ impl Filter { return Action::Continue; } - let rls = GrpcServiceHandler::new( - Rc::clone(&self.config.service), - Rc::clone(&self.header_resolver), - ); + // todo(adam-cattermole): For now we just get the first GrpcService but we expect to have + // an action which links to the service that should be used + let rls = self + .config + .services + .values() + .next() + .expect("expect a value"); + + let handler = GrpcServiceHandler::new(Rc::clone(rls), Rc::clone(&self.header_resolver)); let message = RateLimitService::message(rlp.domain.clone(), descriptors); - match rls.send(message) { + match handler.send(message) { Ok(call_id) => { debug!( "#{} initiated gRPC call (id# {}) to Limitador", @@ -56,7 +62,7 @@ impl Filter { } Err(e) => { warn!("gRPC call to Limitador failed! {e:?}"); - if let FailureMode::Deny = self.config.failure_mode { + if let FailureMode::Deny = rls.failure_mode() { self.send_http_response(500, vec![], Some(b"Internal Server Error.\n")) } Action::Continue @@ -65,7 +71,15 @@ impl Filter { } fn handle_error_on_grpc_response(&self) { - match &self.config.failure_mode { + // todo(adam-cattermole): We need a method of knowing which service is the one currently + // being used (the current action) so that we can get the failure mode + let rls = self + .config + .services + .values() + .next() + .expect("expect a value"); + match rls.failure_mode() { FailureMode::Deny => { self.send_http_response(500, vec![], Some(b"Internal Server Error.\n")) } diff --git a/src/policy.rs b/src/policy.rs index 7b894c16..7505b8fd 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -26,24 +26,16 @@ pub struct Rule { pub struct Policy { pub name: String, pub domain: String, - pub service: String, pub hostnames: Vec, pub rules: Vec, } impl Policy { #[cfg(test)] - pub fn new( - name: String, - domain: String, - service: String, - hostnames: Vec, - rules: Vec, - ) -> Self { + pub fn new(name: String, domain: String, hostnames: Vec, rules: Vec) -> Self { Policy { name, domain, - service, hostnames, rules, } diff --git a/src/policy_index.rs b/src/policy_index.rs index 4a1bf607..58b31d94 100644 --- a/src/policy_index.rs +++ b/src/policy_index.rs @@ -41,13 +41,7 @@ mod tests { use crate::policy_index::PolicyIndex; fn build_ratelimit_policy(name: &str) -> Policy { - Policy::new( - name.to_owned(), - "".to_owned(), - "".to_owned(), - Vec::new(), - Vec::new(), - ) + Policy::new(name.to_owned(), "".to_owned(), Vec::new(), Vec::new()) } #[test] diff --git a/src/service.rs b/src/service.rs index e0b01681..e6b13d61 100644 --- a/src/service.rs +++ b/src/service.rs @@ -1,7 +1,7 @@ pub(crate) mod auth; pub(crate) mod rate_limit; -use crate::configuration::ExtensionType; +use crate::configuration::{ExtensionType, FailureMode}; use crate::service::auth::{AUTH_METHOD_NAME, AUTH_SERVICE_NAME}; use crate::service::rate_limit::{RATELIMIT_METHOD_NAME, RATELIMIT_SERVICE_NAME}; use crate::service::TracingHeader::{Baggage, Traceparent, Tracestate}; @@ -18,20 +18,23 @@ pub struct GrpcService { endpoint: String, name: &'static str, method: &'static str, + failure_mode: FailureMode, } impl GrpcService { - pub fn new(extension_type: ExtensionType, endpoint: String) -> Self { + pub fn new(extension_type: ExtensionType, endpoint: String, failure_mode: FailureMode) -> Self { match extension_type { ExtensionType::Auth => Self { endpoint, name: AUTH_SERVICE_NAME, method: AUTH_METHOD_NAME, + failure_mode, }, ExtensionType::RateLimit => Self { endpoint, name: RATELIMIT_SERVICE_NAME, method: RATELIMIT_METHOD_NAME, + failure_mode, }, } } @@ -44,8 +47,12 @@ impl GrpcService { fn method(&self) -> &str { self.method } + pub fn failure_mode(&self) -> &FailureMode { + &self.failure_mode + } } +#[derive(Default)] pub struct GrpcServiceHandler { service: Rc, header_resolver: Rc, @@ -83,6 +90,12 @@ pub struct HeaderResolver { headers: OnceCell>, } +impl Default for HeaderResolver { + fn default() -> Self { + Self::new() + } +} + impl HeaderResolver { pub fn new() -> Self { Self { From a78e96289fd8b94704e3b45531acdb7250af1328 Mon Sep 17 00:00:00 2001 From: Adam Cattermole Date: Thu, 29 Aug 2024 14:10:46 +0100 Subject: [PATCH 3/3] Update tests to use new config Signed-off-by: Adam Cattermole --- tests/rate_limited.rs | 48 ++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/tests/rate_limited.rs b/tests/rate_limited.rs index b25bbdda..d1421350 100644 --- a/tests/rate_limited.rs +++ b/tests/rate_limited.rs @@ -29,8 +29,8 @@ fn it_loads() { let root_context = 1; let cfg = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [] + "extensions": {}, + "policies": [] }"#; module @@ -90,12 +90,17 @@ fn it_limits() { let root_context = 1; let cfg = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "some-name", "domain": "RLS-domain", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -228,12 +233,17 @@ fn it_passes_additional_headers() { let root_context = 1; let cfg = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "some-name", "domain": "RLS-domain", - "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], "rules": [ { @@ -380,12 +390,17 @@ fn it_rate_limits_with_empty_conditions() { let root_context = 1; let cfg = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "some-name", "domain": "RLS-domain", - "service": "limitador-cluster", "hostnames": ["*.com"], "rules": [ { @@ -492,12 +507,17 @@ fn it_does_not_rate_limits_when_selector_does_not_exist_and_misses_default_value let root_context = 1; let cfg = r#"{ - "failureMode": "deny", - "rateLimitPolicies": [ + "extensions": { + "limitador": { + "type": "ratelimit", + "endpoint": "limitador-cluster", + "failureMode": "deny" + } + }, + "policies": [ { "name": "some-name", "domain": "RLS-domain", - "service": "limitador-cluster", "hostnames": ["*.com"], "rules": [ {