From 793ee32077525abcb981c0dd099071f47a99ad3e Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Mon, 10 Jul 2023 11:52:16 +0200 Subject: [PATCH 01/14] wasm configuration v2 --- src/configuration.rs | 552 ++++++++++++++++++++++++++++++------- src/filter/http_context.rs | 288 +++++++------------ src/utils.rs | 136 +-------- 3 files changed, 556 insertions(+), 420 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 1b61dabc..718fa0d2 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -1,71 +1,133 @@ -use crate::envoy::RLA_action_specifier; use crate::policy_index::PolicyIndex; use serde::Deserialize; #[derive(Deserialize, Debug, Clone)] -pub struct Rule { - #[serde(default)] - pub paths: Vec, +pub struct SelectorItem { + // Selector of an attribute from the contextual properties provided by kuadrant + // during request and connection processing + pub selector: String, + + // If not set it defaults to `selector` field value as the descriptor key. #[serde(default)] - pub hosts: Vec, + pub key: Option, + + // An optional value to use if the selector is not found in the context. + // If not set and the selector is not found in the context, then no data is generated. #[serde(default)] - pub methods: Vec, + pub default: Option, } #[derive(Deserialize, Debug, Clone)] -pub struct Configuration { - #[serde(default)] - pub actions: Vec, +pub struct StaticItem { + pub value: String, + pub key: String, } +// Mutually exclusive struct fields #[derive(Deserialize, Debug, Clone)] -pub struct GatewayAction { - #[serde(default)] - pub rules: Vec, +#[serde(rename_all = "lowercase")] +pub enum DataType { + Static(StaticItem), + Selector(SelectorItem), +} + +#[derive(Deserialize, Debug, Clone)] +#[serde(deny_unknown_fields)] +pub struct DataItem { + #[serde(flatten)] + pub item: DataType, +} + +#[derive(Deserialize, PartialEq, Debug, Clone)] +pub enum WhenConditionOperator { + #[serde(rename = "eq")] + EqualOperator, + #[serde(rename = "neq")] + NotEqualOperator, + #[serde(rename = "starts_with")] + StartsWithOperator, + #[serde(rename = "ends_with")] + EndsWithOperator, + #[serde(rename = "matches")] + MatchesOperator, +} + +impl WhenConditionOperator { + pub fn eval(&self, value: &str, attr_value: &str) -> bool { + match *self { + WhenConditionOperator::EqualOperator => value.eq(attr_value), + WhenConditionOperator::NotEqualOperator => !value.eq(attr_value), + WhenConditionOperator::StartsWithOperator => attr_value.starts_with(value), + WhenConditionOperator::EndsWithOperator => attr_value.ends_with(value), + // TODO(eastizle): implement other operators + _ => true, + } + } +} + +#[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "camelCase")] +pub struct PatternExpression { + pub selector: String, + pub operator: WhenConditionOperator, + pub value: String, +} + +#[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "camelCase")] +pub struct Condition { + pub all_of: Vec, +} + +#[derive(Deserialize, Debug, Clone)] +pub struct Rule { + // #[serde(default)] - pub configurations: Vec, + pub conditions: Vec, + // + pub data: Vec, } #[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "camelCase")] pub struct RateLimitPolicy { pub name: String, - pub rate_limit_domain: String, - pub upstream_cluster: String, + pub domain: String, + pub service: String, pub hostnames: Vec, - #[serde(default)] - pub gateway_actions: Vec, + pub rules: Vec, } impl RateLimitPolicy { #[cfg(test)] pub fn new( name: String, - rate_limit_domain: String, - upstream_cluster: String, + domain: String, + service: String, hostnames: Vec, - gateway_actions: Vec, + rules: Vec, ) -> Self { RateLimitPolicy { name, - rate_limit_domain, - upstream_cluster, + domain, + service, hostnames, - gateway_actions, + rules, } } } pub struct FilterConfig { pub index: PolicyIndex, - // Deny request when faced with an irrecoverable failure. - pub failure_mode_deny: bool, + // Deny/Allow request when faced with an irrecoverable failure. + pub failure_mode: FailureMode, } impl FilterConfig { pub fn new() -> Self { Self { index: PolicyIndex::new(), - failure_mode_deny: true, + failure_mode: FailureMode::Deny, } } @@ -80,20 +142,24 @@ impl FilterConfig { Self { index, - failure_mode_deny: config.failure_mode_deny, + failure_mode: config.failure_mode, } } } -// TODO(rahulanand16nov): We can convert the structure of config in such a way -// that it's optimized for lookup in the runtime. For e.g., keying on virtualhost -// to sort through ratelimitpolicies and then further operations. +#[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "lowercase")] +pub enum FailureMode { + Deny, + Allow, +} #[derive(Deserialize, Debug, Clone)] +#[serde(rename_all = "camelCase")] pub struct PluginConfiguration { pub rate_limit_policies: Vec, - // Deny request when faced with an irrecoverable failure. - pub failure_mode_deny: bool, + // Deny/Allow request when faced with an irrecoverable failure. + pub failure_mode: FailureMode, } #[cfg(test)] @@ -101,63 +167,52 @@ mod test { use super::*; const CONFIG: &str = r#"{ - "failure_mode_deny": true, - "rate_limit_policies": [ + "failureMode": "deny", + "rateLimitPolicies": [ { - "name": "some-name", - "rate_limit_domain": "RLS-domain", - "upstream_cluster": "limitador-cluster", + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], - "gateway_actions": [ + "rules": [ { - "rules": [ + "conditions": [ { - "paths": ["/admin/toy"], - "hosts": ["cars.toystore.com"], - "methods": ["POST"] - }], - "configurations": [ - { - "actions": [ + "allOf": [ { - "generic_key": { - "descriptor_key": "admin", - "descriptor_value": "1" - } + "selector": "request.path", + "operator": "eq", + "value": "/admin/toy" }, { - "metadata": { - "descriptor_key": "user-id", - "default_value": "no-user", - "metadata_key": { - "key": "envoy.filters.http.ext_authz", - "path": [ - { - "segment": { - "key": "ext_auth_data" - } - }, - { - "segment": { - "key": "user_id" - } - } - ] - }, - "source": "DYNAMIC" - } + "selector": "request.method", + "operator": "eq", + "value": "POST" + }, + { + "selector": "request.host", + "operator": "eq", + "value": "cars.toystore.com" + }] + }], + "data": [ + { + "static": { + "key": "rlp-ns-A/rlp-name-A", + "value": "1" } - ] - } - ] - } - ] - } - ] + }, + { + "selector": { + "selector": "auth.metadata.username" + } + }] + }] + }] }"#; #[test] - fn parse_config() { + fn parse_config_happy_path() { let res = serde_json::from_str::(CONFIG); if let Err(ref e) = res { eprintln!("{e}"); @@ -167,35 +222,340 @@ mod test { let filter_config = res.unwrap(); assert_eq!(filter_config.rate_limit_policies.len(), 1); - let gateway_actions = &filter_config.rate_limit_policies[0].gateway_actions; - assert_eq!(gateway_actions.len(), 1); + let rules = &filter_config.rate_limit_policies[0].rules; + assert_eq!(rules.len(), 1); + + let conditions = &rules[0].conditions; + assert_eq!(conditions.len(), 1); - let configurations = &gateway_actions[0].configurations; - assert_eq!(configurations.len(), 1); + let all_of_conditions = &conditions[0].all_of; + assert_eq!(all_of_conditions.len(), 3); - let actions = &configurations[0].actions; - assert_eq!(actions.len(), 2); - assert!(std::matches!( - actions[0], - RLA_action_specifier::generic_key(_) - )); + let data_items = &rules[0].data; + assert_eq!(data_items.len(), 2); - if let RLA_action_specifier::metadata(ref metadata_action) = actions[1] { - let metadata_key = metadata_action.get_metadata_key(); - assert_eq!(metadata_key.get_key(), "envoy.filters.http.ext_authz"); + // TODO(eastizle): DataItem does not implement PartialEq, add it only for testing? + //assert_eq!( + // data_items[0], + // DataItem { + // item: DataType::Static(StaticItem { + // key: String::from("rlp-ns-A/rlp-name-A"), + // value: String::from("1") + // }) + // } + //); - let metadata_path = metadata_key.get_path(); - assert_eq!(metadata_path.len(), 2); - assert_eq!(metadata_path[0].get_key(), "ext_auth_data"); - assert_eq!(metadata_path[1].get_key(), "user_id"); + if let DataType::Static(static_item) = &data_items[0].item { + assert_eq!(static_item.key, "rlp-ns-A/rlp-name-A"); + assert_eq!(static_item.value, "1"); } else { - panic!("wrong action type: expected metadata type"); + assert!(false); + } + + if let DataType::Selector(selector_item) = &data_items[1].item { + assert_eq!(selector_item.selector, "auth.metadata.username"); + assert!(selector_item.key.is_none()); + assert!(selector_item.default.is_none()); + } else { + assert!(false); } } + #[test] + fn parse_config_min() { + let config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [] + }"#; + let res = serde_json::from_str::(config); + if let Err(ref e) = res { + eprintln!("{e}"); + } + assert!(res.is_ok()); + + let filter_config = res.unwrap(); + assert_eq!(filter_config.rate_limit_policies.len(), 0); + } + + #[test] + fn parse_config_data_selector() { + let config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [ + { + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", + "hostnames": ["*.toystore.com", "example.com"], + "rules": [ + { + "data": [ + { + "selector": { + "selector": "my.selector.path", + "key": "mykey", + "default": "my_selector_default_value" + } + }] + }] + }] + }"#; + let res = serde_json::from_str::(config); + if let Err(ref e) = res { + eprintln!("{e}"); + } + assert!(res.is_ok()); + + let filter_config = res.unwrap(); + assert_eq!(filter_config.rate_limit_policies.len(), 1); + + let rules = &filter_config.rate_limit_policies[0].rules; + assert_eq!(rules.len(), 1); + + let data_items = &rules[0].data; + assert_eq!(data_items.len(), 1); + + if let DataType::Selector(selector_item) = &data_items[0].item { + assert_eq!(selector_item.selector, "my.selector.path"); + assert_eq!(selector_item.key.as_ref().unwrap(), "mykey"); + assert_eq!( + selector_item.default.as_ref().unwrap(), + "my_selector_default_value" + ); + } else { + assert!(false); + } + } + + #[test] + fn parse_config_condition_selector_operators() { + let config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [ + { + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", + "hostnames": ["*.toystore.com", "example.com"], + "rules": [ + { + "conditions": [ + { + "allOf": [ + { + "selector": "request.path", + "operator": "eq", + "value": "/admin/toy" + }, + { + "selector": "request.method", + "operator": "neq", + "value": "POST" + }, + { + "selector": "request.host", + "operator": "starts_with", + "value": "cars." + }, + { + "selector": "request.host", + "operator": "ends_with", + "value": ".com" + }, + { + "selector": "request.host", + "operator": "matches", + "value": "*.com" + }] + }], + "data": [ { "selector": { "selector": "my.selector.path" } }] + }] + }] + }"#; + let res = serde_json::from_str::(config); + if let Err(ref e) = res { + eprintln!("{e}"); + } + assert!(res.is_ok()); + + let filter_config = res.unwrap(); + assert_eq!(filter_config.rate_limit_policies.len(), 1); + + let rules = &filter_config.rate_limit_policies[0].rules; + assert_eq!(rules.len(), 1); + + let conditions = &rules[0].conditions; + assert_eq!(conditions.len(), 1); + + let all_of_conditions = &conditions[0].all_of; + assert_eq!(all_of_conditions.len(), 5); + + let expected_conditions = [ + // selector, value, operator + ( + "request.path", + "/admin/toy", + WhenConditionOperator::EqualOperator, + ), + ( + "request.method", + "POST", + WhenConditionOperator::NotEqualOperator, + ), + ( + "request.host", + "cars.", + WhenConditionOperator::StartsWithOperator, + ), + ( + "request.host", + ".com", + WhenConditionOperator::EndsWithOperator, + ), + ( + "request.host", + "*.com", + WhenConditionOperator::MatchesOperator, + ), + ]; + + for i in 0..expected_conditions.len() { + assert_eq!(all_of_conditions[i].selector, expected_conditions[i].0); + assert_eq!(all_of_conditions[i].value, expected_conditions[i].1); + assert_eq!(all_of_conditions[i].operator, expected_conditions[i].2); + } + } + + #[test] + fn parse_config_conditions_optional() { + let config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [ + { + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", + "hostnames": ["*.toystore.com", "example.com"], + "rules": [ + { + "data": [ + { + "static": { + "key": "rlp-ns-A/rlp-name-A", + "value": "1" + } + }, + { + "selector": { + "selector": "auth.metadata.username" + } + }] + }] + }] + }"#; + let res = serde_json::from_str::(config); + if let Err(ref e) = res { + eprintln!("{e}"); + } + assert!(res.is_ok()); + + let filter_config = res.unwrap(); + assert_eq!(filter_config.rate_limit_policies.len(), 1); + + let rules = &filter_config.rate_limit_policies[0].rules; + assert_eq!(rules.len(), 1); + + let conditions = &rules[0].conditions; + assert_eq!(conditions.len(), 0); + } + + #[test] + fn parse_config_invalid_data() { + // data item fields are mutually exclusive + let bad_config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [ + { + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", + "hostnames": ["*.toystore.com", "example.com"], + "rules": [ + { + "data": [ + { + "static": { + "key": "rlp-ns-A/rlp-name-A", + "value": "1" + }, + "selector": { + "selector": "auth.metadata.username" + } + }] + }] + }] + }"#; + let res = serde_json::from_str::(bad_config); + assert!(res.is_err()); + + // data item unknown fields are forbidden + let bad_config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [ + { + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", + "hostnames": ["*.toystore.com", "example.com"], + "rules": [ + { + "data": [ + { + "unknown": { + "key": "rlp-ns-A/rlp-name-A", + "value": "1" + } + }] + }] + }] + }"#; + let res = serde_json::from_str::(bad_config); + assert!(res.is_err()); + + // condition selector operator unknown + let bad_config = r#"{ + "failureMode": "deny", + "rateLimitPolicies": [ + { + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador-cluster", + "hostnames": ["*.toystore.com", "example.com"], + "rules": [ + { + "conditions": [ + { + "allOf": [ + { + "selector": "request.path", + "operator": "unknown", + "value": "/admin/toy" + }] + }], + "data": [ { "selector": { "selector": "my.selector.path" } }] + }] + }] + }"#; + let res = serde_json::from_str::(bad_config); + assert!(res.is_err()); + } + #[test] fn filter_config_from_configuration() { let res = serde_json::from_str::(CONFIG); + if let Err(ref e) = res { + eprintln!("{e}"); + } assert!(res.is_ok()); let filter_config = FilterConfig::from(res.unwrap()); diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index 02ea8895..55c16884 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -1,14 +1,15 @@ -use crate::configuration::{Configuration, FilterConfig, RateLimitPolicy, Rule}; +use crate::configuration::{ + Condition, DataItem, DataType, FilterConfig, PatternExpression, RateLimitPolicy, +}; use crate::envoy::{ - RLA_action_specifier, RateLimitDescriptor, RateLimitDescriptor_Entry, RateLimitRequest, - RateLimitResponse, RateLimitResponse_Code, + RateLimitDescriptor, RateLimitDescriptor_Entry, RateLimitRequest, RateLimitResponse, + RateLimitResponse_Code, }; -use crate::utils::{match_headers, path_match, request_process_failure, subdomain_match}; +use crate::utils::request_process_failure; use log::{debug, info, warn}; use protobuf::Message; use proxy_wasm::traits::{Context, HttpContext}; use proxy_wasm::types::Action; -use std::collections::HashMap; use std::rc::Rc; use std::time::Duration; @@ -22,26 +23,6 @@ pub struct Filter { } impl Filter { - fn request_path(&self) -> String { - match self.get_http_request_header(":path") { - None => { - warn!(":path header not found"); - String::new() - } - Some(path) => path, - } - } - - fn request_method(&self) -> String { - match self.get_http_request_header(":method") { - None => { - warn!(":method header not found"); - String::new() - } - Some(method) => method, - } - } - fn request_authority(&self) -> String { match self.get_http_request_header(":authority") { None => { @@ -63,14 +44,14 @@ impl Filter { } let mut rl_req = RateLimitRequest::new(); - rl_req.set_domain(rlp.rate_limit_domain.clone()); + rl_req.set_domain(rlp.domain.clone()); rl_req.set_hits_addend(1); rl_req.set_descriptors(descriptors); let rl_req_serialized = Message::write_to_bytes(&rl_req).unwrap(); // TODO(rahulanand16nov): Error Handling match self.dispatch_grpc_call( - rlp.upstream_cluster.as_str(), + rlp.service.as_str(), RATELIMIT_SERVICE_NAME, RATELIMIT_METHOD_NAME, Vec::new(), @@ -80,7 +61,7 @@ impl Filter { Ok(call_id) => info!("Initiated gRPC call (id# {}) to Limitador", call_id), Err(e) => { warn!("gRPC call to Limitador failed! {:?}", e); - request_process_failure(self.config.failure_mode_deny); + request_process_failure(&self.config.failure_mode); } } Action::Pause @@ -91,195 +72,114 @@ impl Filter { rlp: &RateLimitPolicy, ) -> protobuf::RepeatedField { //::protobuf::RepeatedField::default() - rlp.gateway_actions + rlp.rules .iter() - .filter(|ga| self.filter_configurations_by_rules(&ga.rules)) - // flatten the vec to vec - .flat_map(|ga| &ga.configurations) + .filter(|rule| self.filter_rule_by_conditions(&rule.conditions)) + // flatten the vec to vec + .flat_map(|rule| &rule.data) // All actions cannot be flatten! each vec of actions defines one potential descriptor - .flat_map(|configuration| self.build_descriptor(configuration)) + .flat_map(|data| self.build_descriptor(data)) .collect() } - fn filter_configurations_by_rules(&self, rules: &[Rule]) -> bool { - if rules.is_empty() { - // no rules is equivalent to matching all the requests. + fn filter_rule_by_conditions(&self, conditions: &[Condition]) -> bool { + if conditions.is_empty() { + // no conditions is equivalent to matching all the requests. return true; } - rules.iter().any(|rule| self.rule_applies(rule)) + conditions + .iter() + .any(|condition| self.condition_applies(condition)) } - fn rule_applies(&self, rule: &Rule) -> bool { - if !rule.paths.is_empty() - && !rule - .paths - .iter() - .any(|path| path_match(path, self.request_path().as_str())) - { - return false; - } - - if !rule.methods.is_empty() - && !rule - .methods - .iter() - .any(|method| self.request_method().eq(method)) - { - return false; - } + fn condition_applies(&self, condition: &Condition) -> bool { + condition + .all_of + .iter() + .all(|pattern_expression| self.pattern_expression_applies(pattern_expression)) + } - if !rule.hosts.is_empty() - && !rule - .hosts - .iter() - .any(|subdomain| subdomain_match(subdomain, self.request_authority().as_str())) - { - return false; + fn pattern_expression_applies(&self, p_e: &PatternExpression) -> bool { + let attribute_path = p_e.selector.split(".").collect(); + match self.get_property(attribute_path) { + None => { + debug!( + "[context_id: {}]: selector not found: {}", + self.context_id, p_e.selector + ); + false + } + // TODO(eastizle): not all fields are strings + // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes + Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { + Err(e) => { + debug!( + "[context_id: {}]: failed to parse selector value: {}, error: {}", + self.context_id, p_e.selector, e + ); + false + } + Ok(attribute_value) => p_e + .operator + .eval(p_e.value.as_str(), attribute_value.as_str()), + }, } - - true } - fn build_descriptor(&self, configuration: &Configuration) -> Option { + fn build_descriptor(&self, data: &DataItem) -> Option { let mut entries = ::protobuf::RepeatedField::default(); - for action in configuration.actions.iter() { - let mut descriptor_entry = RateLimitDescriptor_Entry::new(); - match action { - RLA_action_specifier::source_cluster(_) => { - match self.get_property(vec!["connection", "requested_server_name"]) { - None => { - debug!("requested service name not found"); - return None; - } - Some(src_cluster_bytes) => { - match String::from_utf8(src_cluster_bytes) { - // NOTE(rahulanand16nov): not sure if it's correct. - Ok(src_cluster) => { - descriptor_entry.set_key("source_cluster".into()); - descriptor_entry.set_value(src_cluster); - entries.push(descriptor_entry); - } - Err(e) => { - warn!("source_cluster action parsing error! {:?}", e); - return None; - } - } - } - } - } - RLA_action_specifier::destination_cluster(_) => { - match self.get_property(vec!["cluster_name"]) { - None => { - debug!("cluster name not found"); - return None; - } - Some(cluster_name_bytes) => match String::from_utf8(cluster_name_bytes) { - Ok(cluster_name) => { - descriptor_entry.set_key("destination_cluster".into()); - descriptor_entry.set_value(cluster_name); + + match &data.item { + DataType::Static(static_item) => { + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(static_item.key.to_owned()); + descriptor_entry.set_value(static_item.value.to_owned()); + entries.push(descriptor_entry); + } + DataType::Selector(selector_item) => { + let descriptor_key = match &selector_item.key { + None => selector_item.selector.to_owned(), + Some(key) => key.to_owned(), + }; + + let attribute_path = selector_item.selector.split(".").collect(); + + match self.get_property(attribute_path) { + None => { + debug!( + "[context_id: {}]: selector not found: {}", + self.context_id, selector_item.selector + ); + match &selector_item.default { + None => return None, // skipping descriptors + Some(default_value) => { + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(descriptor_key); + descriptor_entry.set_value(default_value.to_owned()); entries.push(descriptor_entry); } - Err(e) => { - warn!("cluster_name action parsing error! {:?}", e); - return None; - } - }, - } - } - RLA_action_specifier::request_headers(rh) => { - match self.get_http_request_header(rh.get_header_name()) { - None => { - debug!("header name {} not found", rh.get_header_name()); - return None; - } - Some(header_value) => { - descriptor_entry.set_key(rh.get_descriptor_key().into()); - descriptor_entry.set_value(header_value); - entries.push(descriptor_entry); } } - } - RLA_action_specifier::remote_address(_) => { - match self.get_http_request_header("x-forwarded-for") { - None => { - debug!("x-forwarded-for header not found"); + Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { + Err(e) => { + debug!( + "[context_id: {}]: failed to parse selector value: {}, error: {}", + self.context_id, selector_item.selector, e + ); return None; } - Some(remote_addess) => { - descriptor_entry.set_key("remote_address".into()); - descriptor_entry.set_value(remote_addess); + Ok(attribute_value) => { + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(descriptor_key); + descriptor_entry.set_value(attribute_value); entries.push(descriptor_entry); } - } + }, } - RLA_action_specifier::generic_key(gk) => { - descriptor_entry.set_key(gk.get_descriptor_key().into()); - descriptor_entry.set_value(gk.get_descriptor_value().into()); - entries.push(descriptor_entry); - } - RLA_action_specifier::header_value_match(hvm) => { - let request_headers: HashMap<_, _> = - self.get_http_request_headers().into_iter().collect(); - - if hvm.get_expect_match().get_value() - == match_headers(&request_headers, hvm.get_headers()) - { - descriptor_entry.set_key("header_match".into()); - descriptor_entry.set_value(hvm.get_descriptor_value().into()); - entries.push(descriptor_entry); - } else { - debug!("header_value_match does not add entry"); - return None; - } - } - RLA_action_specifier::dynamic_metadata(_) => todo!(), - RLA_action_specifier::metadata(md) => { - // Note(rahul): defaulting to dynamic metadata source right now. - let metadata_key = &md.get_metadata_key().key; - let mut metadata_path: Vec<&str> = md - .get_metadata_key() - .get_path() - .iter() - .map(|path_segment| path_segment.get_key()) - .collect(); - let default_value = md.get_default_value(); - let descriptor_key = md.get_descriptor_key(); - - descriptor_entry.set_key(descriptor_key.into()); - - let mut property_path = vec!["metadata", "filter_metadata", metadata_key]; - property_path.append(&mut metadata_path); - debug!("metadata property_path {:?}", property_path); - match self.get_property(property_path) { - None => { - debug!("metadata key not found"); - if default_value.is_empty() { - debug!("skipping descriptor because no metadata and default value present"); - return None; - } - descriptor_entry.set_value(default_value.into()); - entries.push(descriptor_entry); - } - Some(metadata_bytes) => match String::from_utf8(metadata_bytes) { - Err(e) => { - debug!("failed to parse metadata value: {}", e); - if default_value.is_empty() { - debug!("skipping descriptor because no metadata and default value present"); - return None; - } - descriptor_entry.set_value(default_value.into()); - } - Ok(metadata_value) => { - descriptor_entry.set_value(metadata_value); - entries.push(descriptor_entry); - } - }, - } - } - RLA_action_specifier::extension(_) => todo!(), } } + let mut res = RateLimitDescriptor::new(); res.set_entries(entries); Some(res) @@ -325,7 +225,7 @@ impl Context for Filter { Some(bytes) => bytes, None => { warn!("grpc response body is empty!"); - request_process_failure(self.config.failure_mode_deny); + request_process_failure(&self.config.failure_mode); return; } }; @@ -337,7 +237,7 @@ impl Context for Filter { "failed to parse grpc response body into RateLimitResponse message: {}", e ); - request_process_failure(self.config.failure_mode_deny); + request_process_failure(&self.config.failure_mode); return; } }; @@ -347,7 +247,7 @@ impl Context for Filter { overall_code: RateLimitResponse_Code::UNKNOWN, .. } => { - request_process_failure(self.config.failure_mode_deny); + request_process_failure(&self.config.failure_mode); return; } RateLimitResponse { diff --git a/src/utils.rs b/src/utils.rs index 6de4457e..89810bb6 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,6 +1,5 @@ -use crate::envoy::{HeaderMatcher, HeaderMatcher_specifier, StringMatcher_pattern}; +use crate::configuration::FailureMode; use proxy_wasm::hostcalls::{resume_http_request, send_http_response}; -use std::collections::HashMap; #[derive(Debug, thiserror::Error)] pub enum UtilsErr { @@ -15,134 +14,11 @@ pub enum UtilsErr { } // Helper function to handle failure during processing. -pub fn request_process_failure(failure_mode_deny: bool) { - if failure_mode_deny { - send_http_response(500, vec![], Some(b"Internal Server Error.\n")).unwrap(); - } - resume_http_request().unwrap(); -} - -pub fn match_headers( - req_headers: &HashMap, - config_headers: &[HeaderMatcher], -) -> bool { - for header_matcher in config_headers { - let invert_match = header_matcher.get_invert_match(); - if let Some(req_header_value) = req_headers.get(header_matcher.get_name()) { - if let Some(hm_specifier) = &header_matcher.header_match_specifier { - let mut is_match = false; - match hm_specifier { - HeaderMatcher_specifier::exact_match(str) => is_match = str == req_header_value, - HeaderMatcher_specifier::safe_regex_match(_regex_matcher) => todo!(), // TODO(rahulanand16nov): not implemented. - HeaderMatcher_specifier::range_match(range) => { - if let Ok(val) = req_header_value.parse::() { - is_match = range.get_start() <= val && val < range.get_end(); - } - } - HeaderMatcher_specifier::present_match(should_be_present) => { - is_match = *should_be_present - } - HeaderMatcher_specifier::prefix_match(prefix) => { - is_match = req_header_value.starts_with(prefix.as_str()) - } - HeaderMatcher_specifier::suffix_match(suffix) => { - is_match = req_header_value.ends_with(suffix.as_str()) - } - HeaderMatcher_specifier::contains_match(str) => { - is_match = req_header_value.contains(str.as_str()) - } - HeaderMatcher_specifier::string_match(str_matcher) => { - let ignore_case = str_matcher.get_ignore_case(); - if let Some(pattern) = &str_matcher.match_pattern { - match pattern { - StringMatcher_pattern::exact(str) => { - is_match = if ignore_case { - str.eq_ignore_ascii_case(req_header_value) - } else { - str == req_header_value - } - } - StringMatcher_pattern::prefix(str) => { - is_match = if ignore_case { - req_header_value - .to_lowercase() - .starts_with(&str.to_lowercase()) - } else { - req_header_value.starts_with(str.as_str()) - } - } - StringMatcher_pattern::suffix(str) => { - is_match = if ignore_case { - req_header_value - .to_lowercase() - .ends_with(&str.to_lowercase()) - } else { - req_header_value.ends_with(str.as_str()) - } - } - StringMatcher_pattern::safe_regex(_) => todo!(), // TODO(rahulanand16nov): not implemented. - StringMatcher_pattern::contains(str) => { - is_match = if ignore_case { - req_header_value - .to_lowercase() - .contains(&str.to_lowercase()) - } else { - req_header_value.contains(str.as_str()) - } - } - } - } else { - return false; - } - } - } - if is_match ^ invert_match { - return false; - } - } else { - return false; - } - } else { - return false; +pub fn request_process_failure(failure_mode: &FailureMode) { + match failure_mode { + FailureMode::Deny => { + send_http_response(500, vec![], Some(b"Internal Server Error.\n")).unwrap() } - } - true -} - -pub fn subdomain_match(subdomain: &str, authority: &str) -> bool { - authority.ends_with(subdomain.replace('*', "").as_str()) -} - -pub fn path_match(path_pattern: &str, request_path: &str) -> bool { - if let Some(stripped_path_pattern) = path_pattern.strip_suffix('*') { - request_path.starts_with(stripped_path_pattern) - } else { - request_path.eq(path_pattern) - } -} - -#[cfg(test)] -mod tests { - use crate::utils; - - #[test] - fn subdomain_match() { - assert!(utils::subdomain_match("*.example.com", "test.example.com")); - assert!(!utils::subdomain_match("*.example.com", "example.com")); - assert!(utils::subdomain_match("*", "test1.test2.example.com")); - assert!(utils::subdomain_match("example.com", "example.com")); - } - - #[test] - fn path_match() { - assert!(utils::path_match("/cats", "/cats")); - assert!(utils::path_match("/", "/")); - assert!(utils::path_match("/*", "/")); - assert!(utils::path_match("/*", "/cats/something")); - assert!(utils::path_match("/cats/*", "/cats/")); - assert!(utils::path_match("/cats/*", "/cats/something")); - assert!(!utils::path_match("/cats/*", "/cats")); - assert!(utils::path_match("/cats*", "/catsanddogs")); - assert!(utils::path_match("/cats*", "/cats/dogs")); + FailureMode::Allow => resume_http_request().unwrap(), } } From 1eac6593c0a5297431dac57530a7b51bd0d9f8b3 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 19 Jul 2023 17:13:44 +0200 Subject: [PATCH 02/14] "matches" operator in conditions --- Cargo.lock | 11 ------ Cargo.toml | 1 - src/configuration.rs | 15 +++++++-- src/filter/http_context.rs | 7 ++-- src/filter/root_context.rs | 2 +- src/glob.rs | 69 ++------------------------------------ 6 files changed, 19 insertions(+), 86 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 814cf69e..8dfc599c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1381,16 +1381,6 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_regex" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8136f1a4ea815d7eac4101cfd0b16dc0cb5e1fe1b8609dfd728058656b7badf" -dependencies = [ - "regex", - "serde", -] - [[package]] name = "serial_test" version = "2.0.0" @@ -1707,7 +1697,6 @@ dependencies = [ "regex", "serde", "serde_json", - "serde_regex", "serial_test", "thiserror", ] diff --git a/Cargo.toml b/Cargo.toml index fe1be274..98628cbe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,6 @@ prost-types = "0.11" protobuf = { version = "2.27", features = ["with-serde"] } thiserror = "1.0" regex = "1" -serde_regex = "1.1.0" radix_trie = "0.2.1" [dev-dependencies] diff --git a/src/configuration.rs b/src/configuration.rs index 718fa0d2..879e2b0f 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -1,4 +1,6 @@ +use crate::glob::GlobPattern; use crate::policy_index::PolicyIndex; +use log::warn; use serde::Deserialize; #[derive(Deserialize, Debug, Clone)] @@ -59,8 +61,17 @@ impl WhenConditionOperator { WhenConditionOperator::NotEqualOperator => !value.eq(attr_value), WhenConditionOperator::StartsWithOperator => attr_value.starts_with(value), WhenConditionOperator::EndsWithOperator => attr_value.ends_with(value), - // TODO(eastizle): implement other operators - _ => true, + WhenConditionOperator::MatchesOperator => match GlobPattern::try_from(value) { + // TODO(eastizle): regexp being compiled and validated at request time. + // Validations and possibly regexp compilation should happen at boot time instead. + // In addition, if the regexp is not valid, the only consequence is that + // the current condition would not apply + Ok(glob_pattern) => glob_pattern.is_match(attr_value), + Err(e) => { + warn!("failed to parse regexp: {value}, error: {e:?}"); + false + } + }, } } } diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index 55c16884..d73e2cf6 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -145,6 +145,8 @@ impl Filter { let attribute_path = selector_item.selector.split(".").collect(); + // TODO(eastizle): not all fields are strings + // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes match self.get_property(attribute_path) { None => { debug!( @@ -233,10 +235,7 @@ impl Context for Filter { let rl_resp: RateLimitResponse = match Message::parse_from_bytes(&res_body_bytes) { Ok(res) => res, Err(e) => { - warn!( - "failed to parse grpc response body into RateLimitResponse message: {}", - e - ); + warn!("failed to parse grpc response body into RateLimitResponse message: {e}"); request_process_failure(&self.config.failure_mode); return; } diff --git a/src/filter/root_context.rs b/src/filter/root_context.rs index cf66872e..a0740946 100644 --- a/src/filter/root_context.rs +++ b/src/filter/root_context.rs @@ -31,7 +31,7 @@ impl RootContext for FilterRoot { Some(c) => c, None => return false, }; - match serde_json::from_slice::(configuration.as_ref()) { + match serde_json::from_slice::(&configuration) { Ok(config) => { info!("plugin config parsed: {:?}", config); self.config = Rc::new(FilterConfig::from(config)); diff --git a/src/glob.rs b/src/glob.rs index ede4be56..d5113d5c 100644 --- a/src/glob.rs +++ b/src/glob.rs @@ -8,8 +8,7 @@ // use std::convert::TryFrom; -use regex::{Regex, RegexSet}; -use serde::{Deserialize, Serialize}; +use regex::Regex; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -18,8 +17,7 @@ pub enum Error { } #[repr(transparent)] -#[derive(Debug, Clone, Deserialize, Serialize)] -#[serde(try_from = "String", into = "String")] +#[derive(Debug, Clone)] pub struct GlobPattern(Regex); impl<'a> TryFrom<&'a str> for GlobPattern { @@ -129,73 +127,10 @@ fn regex_unescape_specials(specials: &[char], regex_s: &str) -> String { }) } -#[repr(transparent)] -#[derive(Debug, Clone, Deserialize, Serialize)] -#[serde(try_from = "Vec", into = "Vec")] -pub struct GlobPatternSet(RegexSet); - -impl TryFrom> for GlobPatternSet { - type Error = Error; - - fn try_from(v: Vec) -> Result { - Self::new(v.iter().map(|pat| GlobPattern::glob_pattern(pat.as_str()))) - } -} - -impl<'a> TryFrom<&'a str> for GlobPatternSet { - type Error = Error; - - fn try_from(value: &'a str) -> Result { - Self::new(core::iter::once(GlobPattern::glob_pattern(value))) - } -} - -impl From for Vec { - fn from(gs: GlobPatternSet) -> Self { - gs.0.patterns().to_vec() - } -} -// The default value will match everything -impl Default for GlobPatternSet { - fn default() -> Self { - // panic: won't panic since "*" is always valid - Self::new([".*"].iter()).unwrap() - } -} - -impl GlobPatternSet { - pub fn new(exprs: I) -> Result - where - S: AsRef, - I: IntoIterator, - { - Ok(Self(RegexSet::new(exprs)?)) - } - - pub fn is_match(&self, s: &str) -> bool { - self.0.is_match(s) - } - - pub fn regex_set(&self) -> &RegexSet { - &self.0 - } - - pub fn into_inner(self) -> RegexSet { - self.0 - } -} - #[cfg(test)] mod test { use super::*; - #[test] - fn default_global_pattern_set() -> Result<(), Error> { - let pattern_set = GlobPatternSet::default(); - assert!(pattern_set.is_match("should_match_all")); - Ok(()) - } - #[test] fn glob_pattern_matches() -> Result<(), Error> { let fixtures = [ From 8c512a5583b242921bd1e2ec4f17ac197a73b7ac Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Wed, 19 Jul 2023 22:58:48 +0200 Subject: [PATCH 03/14] dev env: new plugin struct v2 --- README.md | 6 +- src/filter/http_context.rs | 3 +- utils/docker-compose/envoy.yaml | 155 +++++++++++-------------------- utils/docker-compose/limits.yaml | 12 +-- 4 files changed, 65 insertions(+), 111 deletions(-) diff --git a/README.md b/README.md index 0a24a6f8..d22b7897 100644 --- a/README.md +++ b/README.md @@ -56,19 +56,19 @@ make development Three rate limit policies defined for e2e testing: -* `rlp-a`: Actions should not generate descriptors. Hence, rate limiting service should **not** be called. +* `rlp-a`: Only one data item. Data selector should not generate return any value. Thus, descriptor should be empty and rate limiting service should **not** be called. ``` curl -H "Host: test.a.com" http://127.0.0.1:18000/get ``` -* `rlp-b`: Rules do not match. Hence, rate limiting service should **not** be called. +* `rlp-b`: Conditions do not match. Hence, rate limiting service should **not** be called. ``` curl -H "Host: test.b.com" http://127.0.0.1:18000/get ``` -* `rlp-c`: Five descriptors from multiple action types should be generated. Hence, rate limiting service should be called. +* `rlp-c`: Five descriptors from multiple data items should be generated. Hence, rate limiting service should be called. ``` curl -H "Host: test.c.com" -H "x-forwarded-for: 127.0.0.1" -H "My-Custom-Header-01: my-custom-header-value-01" -H "My-Custom-Header-02: my-custom-header-value-02" -H "x-dyn-user-id: bob" http://127.0.0.1:18000/get diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index d73e2cf6..b158ec1a 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -77,7 +77,8 @@ impl Filter { .filter(|rule| self.filter_rule_by_conditions(&rule.conditions)) // flatten the vec to vec .flat_map(|rule| &rule.data) - // All actions cannot be flatten! each vec of actions defines one potential descriptor + // WRONG: each rule generates one descriptor + jdsjd .flat_map(|data| self.build_descriptor(data)) .collect() } diff --git a/utils/docker-compose/envoy.yaml b/utils/docker-compose/envoy.yaml index 6efce2e2..dd88e576 100644 --- a/utils/docker-compose/envoy.yaml +++ b/utils/docker-compose/envoy.yaml @@ -50,137 +50,94 @@ static_resources: "@type": "type.googleapis.com/google.protobuf.StringValue" value: > { - "failure_mode_deny": true, - "rate_limit_policies": [ + "failureMode": "deny", + "rateLimitPolicies": [ { - "name": "rlp-a", - "rate_limit_domain": "rls-domain-a", - "upstream_cluster": "limitador", + "name": "rlp-ns-A/rlp-name-A", + "domain": "rlp-ns-A/rlp-name-A", + "service": "limitador", "hostnames": ["*.a.com"], - "gateway_actions": [ + "rules": [ { - "configurations": [ + "data": [ { - "actions": [ - { - "request_headers": { - "header_name": "not-existing-header", - "skip_if_absent": true, - "descriptor_key": "my_not_existing_header" - } - } - ] + "selector": { + "selector": "unknown.path" + } } ] } ] }, { - "name": "rlp-b", - "rate_limit_domain": "rls-domain-b", - "upstream_cluster": "limitador", + "name": "rlp-ns-B/rlp-name-B", + "domain": "rlp-ns-B/rlp-name-B", + "service": "limitador", "hostnames": ["*.b.com"], - "gateway_actions": [ + "rules": [ { - "rules": [ - { - "paths": ["/not/exising/path", "/any*"] - } - ], - "configurations": [ + "conditions": [ { - "actions": [ + "allOf": [ { - "generic_key": { - "descriptor_key": "some_generic_key", - "descriptor_value": "some_generic_value" - } + "selector": "request.url_path", + "operator": "starts_with", + "value": "/unknown-path" } ] } + ], + "data": [ + { + "static": { + "key": "rlp-ns-B/rlp-name-B/limit-not-to-be-activated", + "value": "1" + } + } ] } ] }, { - "name": "rlp-c", - "rate_limit_domain": "rls-domain-c", - "upstream_cluster": "limitador", + "name": "rlp-ns-C/rlp-name-C", + "domain": "rlp-ns-C/rlp-name-C", + "service": "limitador", "hostnames": ["*.c.com"], - "gateway_actions": [ + "rules": [ { - "rules": [ - { - "paths": ["/get"], - "hosts": ["test.c.com"], - "method": ["GET"] - } - ], - "configurations": [ + "conditions": [ { - "actions": [ + "allOf": [ { - "generic_key": { - "descriptor_key": "some_generic_key_for_c", - "descriptor_value": "some_generic_value_for_c" - } - } - ] - }, - { - "actions": [ + "selector": "request.url_path", + "operator": "starts_with", + "value": "/get" + }, { - "remote_address": {} - } - ] - }, - { - "actions": [ + "selector": "request.host", + "operator": "eq", + "value": "test.c.com" + }, { - "header_value_match": { - "descriptor_value": "header_value_match_value", - "headers": [ - { - "name": "My-Custom-Header-01", - "exact_match": "my-custom-header-value", - "invert_match": false - } - ] - } - } + "selector": "request.method", + "operator": "eq", + "value": "GET" + }, ] - }, + } + ], + "data": [ { - "actions": [ - { - "request_headers": { - "header_name": "My-Custom-Header-02", - "skip_if_absent": true, - "descriptor_key": "my_custom_header_key" - } - } - ] + "selector": { + "key": "souce", + "value": "1" + } }, { - "actions": [ - { - "metadata": { - "descriptor_key": "user_id", - "default_value": "no-user", - "metadata_key": { - "key": "envoy.filters.http.header_to_metadata", - "path": [ - { - "segment": { - "key": "user-id" - } - } - ] - }, - "source": "DYNAMIC" - } - } - ] + "static": { + "key": "rlp-ns-C/rlp-name-C/limit-to-be-activated", + "value": "1" + } } ] } diff --git a/utils/docker-compose/limits.yaml b/utils/docker-compose/limits.yaml index 70a67dd3..36356409 100644 --- a/utils/docker-compose/limits.yaml +++ b/utils/docker-compose/limits.yaml @@ -1,12 +1,8 @@ --- -- namespace: rls-domain-b - max_value: 5 - seconds: 10 - conditions: - - "some_generic_key == 'some_generic_value'" - variables: [] -- namespace: rls-domain-c +- namespace: rlp-ns-C/rlp-name-C max_value: 2 seconds: 10 - conditions: ["user_id == 'bob'"] + conditions: + - "rlp-ns-C/rlp-name-C/limit-to-be-activated == '1'" + - "user_id == 'bob'" variables: [] From 9fe81fcf6e229b2c32839ee22e6066557d98dc1b Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 20 Jul 2023 17:52:52 +0200 Subject: [PATCH 04/14] fix build descriptors --- Cargo.lock | 22 +---- Cargo.toml | 2 +- README.md | 4 +- src/filter/http_context.rs | 136 +++++++++++++++++-------------- src/lib.rs | 1 - src/utils.rs | 24 ------ utils/docker-compose/envoy.yaml | 96 ++++++++++++++++++++-- utils/docker-compose/limits.yaml | 2 +- 8 files changed, 174 insertions(+), 113 deletions(-) delete mode 100644 src/utils.rs diff --git a/Cargo.lock b/Cargo.lock index 8dfc599c..6589fa65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,17 +11,6 @@ dependencies = [ "gimli", ] -[[package]] -name = "ahash" -version = "0.7.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" -dependencies = [ - "getrandom 0.2.7", - "once_cell", - "version_check", -] - [[package]] name = "ahash" version = "0.8.3" @@ -643,9 +632,6 @@ name = "hashbrown" version = "0.12.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" -dependencies = [ - "ahash 0.7.6", -] [[package]] name = "hashbrown" @@ -653,7 +639,7 @@ version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "43a3c133739dddd0d2990f9a4bdf8eb4b21ef50e4851ca85ab661199821d510e" dependencies = [ - "ahash 0.8.3", + "ahash", ] [[package]] @@ -1091,11 +1077,11 @@ dependencies = [ [[package]] name = "proxy-wasm" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3927081c2674366adadef4d5c5d34c4d849ab764a17bfe4ff2bd04436efb593d" +checksum = "823b744520cd4a54ba7ebacbffe4562e839d6dcd8f89209f96a1ace4f5229cd4" dependencies = [ - "hashbrown 0.12.3", + "hashbrown 0.13.2", "log", ] diff --git a/Cargo.toml b/Cargo.toml index 98628cbe..a06794c1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ default = ["with-serde"] with-serde = ["protobuf/with-serde"] [dependencies] -proxy-wasm = "0.2" +proxy-wasm = "0.2.1" serde_json = "1.0" log = "0.4" serde = { version = "1.0", features = ["derive"] } diff --git a/README.md b/README.md index d22b7897..c3c81c47 100644 --- a/README.md +++ b/README.md @@ -68,10 +68,10 @@ curl -H "Host: test.a.com" http://127.0.0.1:18000/get curl -H "Host: test.b.com" http://127.0.0.1:18000/get ``` -* `rlp-c`: Five descriptors from multiple data items should be generated. Hence, rate limiting service should be called. +* `rlp-c`: Four descriptors from multiple rules should be generated. Hence, rate limiting service should be called. ``` -curl -H "Host: test.c.com" -H "x-forwarded-for: 127.0.0.1" -H "My-Custom-Header-01: my-custom-header-value-01" -H "My-Custom-Header-02: my-custom-header-value-02" -H "x-dyn-user-id: bob" http://127.0.0.1:18000/get +curl -H "Host: test.c.com" -H "x-forwarded-for: 127.0.0.1" -H "My-Custom-Header-01: my-custom-header-value-01" -H "x-dyn-user-id: bob" http://127.0.0.1:18000/get ``` **Note:** Dynamic metadata can also be set with `user-id` as the key if you add the header `x-dyn-user-id`. This is provided using [Header-To-Metadata filter](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/header_to_metadata_filter#config-http-filters-header-to-metadata). diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index b158ec1a..241ba47c 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -1,11 +1,11 @@ use crate::configuration::{ - Condition, DataItem, DataType, FilterConfig, PatternExpression, RateLimitPolicy, + Condition, DataItem, DataType, FailureMode, FilterConfig, PatternExpression, RateLimitPolicy, + Rule, }; use crate::envoy::{ RateLimitDescriptor, RateLimitDescriptor_Entry, RateLimitRequest, RateLimitResponse, RateLimitResponse_Code, }; -use crate::utils::request_process_failure; use log::{debug, info, warn}; use protobuf::Message; use proxy_wasm::traits::{Context, HttpContext}; @@ -58,28 +58,30 @@ impl Filter { Some(&rl_req_serialized), Duration::from_secs(5), ) { - Ok(call_id) => info!("Initiated gRPC call (id# {}) to Limitador", call_id), + Ok(call_id) => { + info!("Initiated gRPC call (id# {}) to Limitador", call_id); + Action::Pause + } Err(e) => { - warn!("gRPC call to Limitador failed! {:?}", e); - request_process_failure(&self.config.failure_mode); + warn!("gRPC call to Limitador failed! {e:?}"); + if let FailureMode::Deny = self.config.failure_mode { + self.send_http_response(500, vec![], Some(b"Internal Server Error.\n")) + } + Action::Continue } } - Action::Pause } fn build_descriptors( &self, rlp: &RateLimitPolicy, ) -> protobuf::RepeatedField { - //::protobuf::RepeatedField::default() rlp.rules .iter() - .filter(|rule| self.filter_rule_by_conditions(&rule.conditions)) - // flatten the vec to vec - .flat_map(|rule| &rule.data) - // WRONG: each rule generates one descriptor - jdsjd - .flat_map(|data| self.build_descriptor(data)) + .filter(|rule: &&Rule| self.filter_rule_by_conditions(&rule.conditions)) + // Mapping 1 Rule -> 1 Descriptor + // Filter out empty descriptors + .filter_map(|rule| self.build_single_descriptor(&rule.data)) .collect() } @@ -128,57 +130,61 @@ impl Filter { } } - fn build_descriptor(&self, data: &DataItem) -> Option { + fn build_single_descriptor(&self, data_list: &Vec) -> Option { let mut entries = ::protobuf::RepeatedField::default(); - match &data.item { - DataType::Static(static_item) => { - let mut descriptor_entry = RateLimitDescriptor_Entry::new(); - descriptor_entry.set_key(static_item.key.to_owned()); - descriptor_entry.set_value(static_item.value.to_owned()); - entries.push(descriptor_entry); - } - DataType::Selector(selector_item) => { - let descriptor_key = match &selector_item.key { - None => selector_item.selector.to_owned(), - Some(key) => key.to_owned(), - }; + // iterate over data items to allow any data item to skip the entire descriptor + for data in data_list.iter() { + match &data.item { + DataType::Static(static_item) => { + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(static_item.key.to_owned()); + descriptor_entry.set_value(static_item.value.to_owned()); + entries.push(descriptor_entry); + } + DataType::Selector(selector_item) => { + let descriptor_key = match &selector_item.key { + None => selector_item.selector.to_owned(), + Some(key) => key.to_owned(), + }; - let attribute_path = selector_item.selector.split(".").collect(); + let attribute_path = selector_item.selector.split(".").collect(); - // TODO(eastizle): not all fields are strings - // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes - match self.get_property(attribute_path) { - None => { - debug!( - "[context_id: {}]: selector not found: {}", - self.context_id, selector_item.selector - ); - match &selector_item.default { - None => return None, // skipping descriptors - Some(default_value) => { - let mut descriptor_entry = RateLimitDescriptor_Entry::new(); - descriptor_entry.set_key(descriptor_key); - descriptor_entry.set_value(default_value.to_owned()); - entries.push(descriptor_entry); + // TODO(eastizle): not all fields are strings + // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes + match self.get_property(attribute_path) { + None => { + debug!( + "[context_id: {}]: selector not found: {}", + self.context_id, selector_item.selector + ); + match &selector_item.default { + // skipping the entire descriptor + None => return None, + Some(default_value) => { + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(descriptor_key); + descriptor_entry.set_value(default_value.to_owned()); + entries.push(descriptor_entry); + } } } - } - Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { - Err(e) => { - debug!( + Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { + Err(e) => { + debug!( "[context_id: {}]: failed to parse selector value: {}, error: {}", self.context_id, selector_item.selector, e ); - return None; - } - Ok(attribute_value) => { - let mut descriptor_entry = RateLimitDescriptor_Entry::new(); - descriptor_entry.set_key(descriptor_key); - descriptor_entry.set_value(attribute_value); - entries.push(descriptor_entry); - } - }, + return None; + } + Ok(attribute_value) => { + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(descriptor_key); + descriptor_entry.set_value(attribute_value); + entries.push(descriptor_entry); + } + }, + } } } } @@ -187,6 +193,15 @@ impl Filter { res.set_entries(entries); Some(res) } + + fn handle_error_on_grpc_response(&self) { + match &self.config.failure_mode { + FailureMode::Deny => { + self.send_http_response(500, vec![], Some(b"Internal Server Error.\n")) + } + FailureMode::Allow => self.resume_http_request(), + } + } } impl HttpContext for Filter { @@ -210,6 +225,7 @@ impl HttpContext for Filter { } fn on_http_response_headers(&mut self, _num_headers: usize, _end_of_stream: bool) -> Action { + debug!("on_http_response_headers #{}", self.context_id); for (name, value) in &self.response_headers_to_add { self.add_http_response_header(name, value); } @@ -220,15 +236,15 @@ impl HttpContext for Filter { impl Context for Filter { fn on_grpc_call_response(&mut self, token_id: u32, status_code: u32, resp_size: usize) { info!( - "received gRPC call response: token: {}, status: {}", - token_id, status_code + "on_grpc_call_response #{}: received gRPC call response: token: {token_id}, status: {status_code}", + self.context_id ); let res_body_bytes = match self.get_grpc_call_response_body(0, resp_size) { Some(bytes) => bytes, None => { warn!("grpc response body is empty!"); - request_process_failure(&self.config.failure_mode); + self.handle_error_on_grpc_response(); return; } }; @@ -237,7 +253,7 @@ impl Context for Filter { Ok(res) => res, Err(e) => { warn!("failed to parse grpc response body into RateLimitResponse message: {e}"); - request_process_failure(&self.config.failure_mode); + self.handle_error_on_grpc_response(); return; } }; @@ -247,7 +263,7 @@ impl Context for Filter { overall_code: RateLimitResponse_Code::UNKNOWN, .. } => { - request_process_failure(&self.config.failure_mode); + self.handle_error_on_grpc_response(); return; } RateLimitResponse { diff --git a/src/lib.rs b/src/lib.rs index f9cfa6c6..2cde111f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,6 @@ mod envoy; mod filter; mod glob; mod policy_index; -mod utils; #[cfg(test)] mod tests { diff --git a/src/utils.rs b/src/utils.rs deleted file mode 100644 index 89810bb6..00000000 --- a/src/utils.rs +++ /dev/null @@ -1,24 +0,0 @@ -use crate::configuration::FailureMode; -use proxy_wasm::hostcalls::{resume_http_request, send_http_response}; - -#[derive(Debug, thiserror::Error)] -pub enum UtilsErr { - #[error("failed to create string from utf8 data")] - Utf8Fail(#[from] std::string::FromUtf8Error), - #[error("problem while handing protobuf")] - ProtobufErr(#[from] protobuf::error::ProtobufError), - #[error("failed to get i64 from slice")] - SliceToI64(#[from] std::array::TryFromSliceError), - #[error("failed to convert from i64 to u32")] - I64ToU32(#[from] std::num::TryFromIntError), -} - -// Helper function to handle failure during processing. -pub fn request_process_failure(failure_mode: &FailureMode) { - match failure_mode { - FailureMode::Deny => { - send_http_response(500, vec![], Some(b"Internal Server Error.\n")).unwrap() - } - FailureMode::Allow => resume_http_request().unwrap(), - } -} diff --git a/utils/docker-compose/envoy.yaml b/utils/docker-compose/envoy.yaml index dd88e576..912c8841 100644 --- a/utils/docker-compose/envoy.yaml +++ b/utils/docker-compose/envoy.yaml @@ -122,21 +122,105 @@ static_resources: "selector": "request.method", "operator": "eq", "value": "GET" + } + ] + } + ], + "data": [ + { + "static": { + "key": "limit-to-be-activated", + "value": "1" + } + } + ] + }, + { + "conditions": [ + { + "allOf": [ + { + "selector": "request.url_path", + "operator": "starts_with", + "value": "/get" }, + { + "selector": "request.host", + "operator": "eq", + "value": "test.c.com" + }, + { + "selector": "request.method", + "operator": "eq", + "value": "GET" + } ] } ], "data": [ { "selector": { - "key": "souce", - "value": "1" + "selector": "source.address" } - }, + } + ] + }, + { + "conditions": [ { - "static": { - "key": "rlp-ns-C/rlp-name-C/limit-to-be-activated", - "value": "1" + "allOf": [ + { + "selector": "request.url_path", + "operator": "starts_with", + "value": "/get" + }, + { + "selector": "request.host", + "operator": "eq", + "value": "test.c.com" + }, + { + "selector": "request.method", + "operator": "eq", + "value": "GET" + } + ] + } + ], + "data": [ + { + "selector": { + "selector": "request.headers.My-Custom-Header-01" + } + } + ] + }, + { + "conditions": [ + { + "allOf": [ + { + "selector": "request.url_path", + "operator": "starts_with", + "value": "/get" + }, + { + "selector": "request.host", + "operator": "eq", + "value": "test.c.com" + }, + { + "selector": "request.method", + "operator": "eq", + "value": "GET" + } + ] + } + ], + "data": [ + { + "selector": { + "selector": "metadata.filter_metadata.\"envoy.filters.http.header_to_metadata\".user-id" } } ] diff --git a/utils/docker-compose/limits.yaml b/utils/docker-compose/limits.yaml index 36356409..31a68f61 100644 --- a/utils/docker-compose/limits.yaml +++ b/utils/docker-compose/limits.yaml @@ -3,6 +3,6 @@ max_value: 2 seconds: 10 conditions: - - "rlp-ns-C/rlp-name-C/limit-to-be-activated == '1'" + - "limit_to_be_activated == '1'" - "user_id == 'bob'" variables: [] From 2350f7e9f4996d58a2f2f92b098590efdb6aec19 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 21 Jul 2023 16:12:50 +0200 Subject: [PATCH 05/14] Tokenize selector fields with escape char --- README.md | 19 +++++++++- src/filter/http_context.rs | 18 ++++++---- src/lib.rs | 1 + src/utils.rs | 62 +++++++++++++++++++++++++++++++++ utils/docker-compose/envoy.yaml | 2 +- 5 files changed, 93 insertions(+), 9 deletions(-) create mode 100644 src/utils.rs diff --git a/README.md b/README.md index c3c81c47..7ae1442f 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,25 @@ curl -H "Host: test.b.com" http://127.0.0.1:18000/get curl -H "Host: test.c.com" -H "x-forwarded-for: 127.0.0.1" -H "My-Custom-Header-01: my-custom-header-value-01" -H "x-dyn-user-id: bob" http://127.0.0.1:18000/get ``` -**Note:** Dynamic metadata can also be set with `user-id` as the key if you add the header `x-dyn-user-id`. This is provided using [Header-To-Metadata filter](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/header_to_metadata_filter#config-http-filters-header-to-metadata). +The expected descriptors: +``` +RateLimitDescriptor { entries: [Entry { key: "limit-to-be-activated", value: "1" }], limit: None } +``` + +``` +RateLimitDescriptor { entries: [Entry { key: "source.address", value: "127.0.0.1:0" }], limit: None } +``` + +``` +RateLimitDescriptor { entries: [Entry { key: "request.headers.My-Custom-Header-01", value: "my-custom-header-value-01" }], limit: None } +``` + +``` +RateLimitDescriptor { entries: [Entry { key: "metadata.filter_metadata.envoy\\.filters\\.http\\.header_to_metadata.user-id", value: "bob" }], limit: None } +``` + +**Note:** Using [Header-To-Metadata filter](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/header_to_metadata_filter#config-http-filters-header-to-metadata), `x-dyn-user-id` header value is available in the metadata struct with the `user-id` key. Clean up all resources diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index 241ba47c..3a69431e 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -6,6 +6,7 @@ use crate::envoy::{ RateLimitDescriptor, RateLimitDescriptor_Entry, RateLimitRequest, RateLimitResponse, RateLimitResponse_Code, }; +use crate::utils::tokenize_with_escaping; use log::{debug, info, warn}; use protobuf::Message; use proxy_wasm::traits::{Context, HttpContext}; @@ -104,8 +105,10 @@ impl Filter { } fn pattern_expression_applies(&self, p_e: &PatternExpression) -> bool { - let attribute_path = p_e.selector.split(".").collect(); - match self.get_property(attribute_path) { + let attribute_path = tokenize_with_escaping(&p_e.selector, '.', '\\'); + // convert a Vec to Vec<&str> + // attribute_path.iter().map(AsRef::as_ref).collect() + match self.get_property(attribute_path.iter().map(AsRef::as_ref).collect()) { None => { debug!( "[context_id: {}]: selector not found: {}", @@ -148,11 +151,10 @@ impl Filter { Some(key) => key.to_owned(), }; - let attribute_path = selector_item.selector.split(".").collect(); - - // TODO(eastizle): not all fields are strings - // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes - match self.get_property(attribute_path) { + let attribute_path = tokenize_with_escaping(&selector_item.selector, '.', '\\'); + // convert a Vec to Vec<&str> + // attribute_path.iter().map(AsRef::as_ref).collect() + match self.get_property(attribute_path.iter().map(AsRef::as_ref).collect()) { None => { debug!( "[context_id: {}]: selector not found: {}", @@ -169,6 +171,8 @@ impl Filter { } } } + // TODO(eastizle): not all fields are strings + // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { Err(e) => { debug!( diff --git a/src/lib.rs b/src/lib.rs index 2cde111f..f9cfa6c6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,6 +3,7 @@ mod envoy; mod filter; mod glob; mod policy_index; +mod utils; #[cfg(test)] mod tests { diff --git a/src/utils.rs b/src/utils.rs new file mode 100644 index 00000000..abd234d5 --- /dev/null +++ b/src/utils.rs @@ -0,0 +1,62 @@ +// Split a string at each non-escaped occurrence of a separator character. +pub fn tokenize_with_escaping(string: &str, separator: char, escape: char) -> Vec { + let mut token = String::new(); + let mut tokens: Vec = Vec::new(); + let mut chars = string.chars(); + while let Some(ch) = chars.next() { + match ch { + x if x == separator => { + tokens.push(token); + token = String::new(); + } + x if x == escape => { + if let Some(next) = chars.next() { + token.push(next); + } + } + _ => token.push(ch), + } + } + tokens.push(token); + tokens +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn tokenize_with_scaping_basic() { + let string = r"one\.two..three\\\\.four\\\.\five."; + let tokens = tokenize_with_escaping(string, '.', '\\'); + assert_eq!(tokens, vec!["one.two", "", r"three\\", r"four\.five", ""]); + } + + #[test] + fn tokenize_with_scaping_ends_with_separator() { + let string = r"one."; + let tokens = tokenize_with_escaping(string, '.', '\\'); + assert_eq!(tokens, vec!["one", ""]); + } + + #[test] + fn tokenize_with_scaping_ends_with_escape() { + let string = r"one\"; + let tokens = tokenize_with_escaping(string, '.', '\\'); + assert_eq!(tokens, vec!["one"]); + } + + #[test] + fn tokenize_with_scaping_starts_with_separator() { + let string = r".one"; + let tokens = tokenize_with_escaping(string, '.', '\\'); + assert_eq!(tokens, vec!["", "one"]); + } + + #[test] + fn tokenize_with_scaping_starts_with_escape() { + let string = r"\one"; + let tokens = tokenize_with_escaping(string, '.', '\\'); + assert_eq!(tokens, vec!["one"]); + } +} diff --git a/utils/docker-compose/envoy.yaml b/utils/docker-compose/envoy.yaml index 912c8841..9bca8d9c 100644 --- a/utils/docker-compose/envoy.yaml +++ b/utils/docker-compose/envoy.yaml @@ -220,7 +220,7 @@ static_resources: "data": [ { "selector": { - "selector": "metadata.filter_metadata.\"envoy.filters.http.header_to_metadata\".user-id" + "selector": "metadata.filter_metadata.envoy\\.filters\\.http\\.header_to_metadata.user-id" } } ] From e256a5fada1204ed83e2a9302f505850c8998c62 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 21 Jul 2023 17:25:47 +0200 Subject: [PATCH 06/14] fixed dev environment --- README.md | 21 ++++++++++++++++++--- utils/docker-compose/envoy.yaml | 7 ++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 7ae1442f..25fb24b5 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ curl -H "Host: test.c.com" -H "x-forwarded-for: 127.0.0.1" -H "My-Custom-Header- The expected descriptors: ``` -RateLimitDescriptor { entries: [Entry { key: "limit-to-be-activated", value: "1" }], limit: None } +RateLimitDescriptor { entries: [Entry { key: "limit_to_be_activated", value: "1" }], limit: None } ``` ``` @@ -89,12 +89,27 @@ RateLimitDescriptor { entries: [Entry { key: "request.headers.My-Custom-Header-0 ``` ``` -RateLimitDescriptor { entries: [Entry { key: "metadata.filter_metadata.envoy\\.filters\\.http\\.header_to_metadata.user-id", value: "bob" }], limit: None } +RateLimitDescriptor { entries: [Entry { key: "user_id", value: "bob" }], limit: None } ``` **Note:** Using [Header-To-Metadata filter](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/header_to_metadata_filter#config-http-filters-header-to-metadata), `x-dyn-user-id` header value is available in the metadata struct with the `user-id` key. -Clean up all resources +According to the defined limits: + +```yaml +--- +- namespace: rlp-ns-C/rlp-name-C + max_value: 2 + seconds: 10 + conditions: + - "limit_to_be_activated == '1'" + - "user_id == 'bob'" + variables: [] +``` + +The third request in less than 10 seconds should return `429 Too Many Requests`. + +### Clean up all resources ``` make stop-development diff --git a/utils/docker-compose/envoy.yaml b/utils/docker-compose/envoy.yaml index 9bca8d9c..8feac0ca 100644 --- a/utils/docker-compose/envoy.yaml +++ b/utils/docker-compose/envoy.yaml @@ -30,7 +30,7 @@ static_resources: request_rules: - header: x-dyn-user-id on_header_present: - key: user-id + key: user_id type: STRING remove: false - name: envoy.filters.http.wasm @@ -129,7 +129,7 @@ static_resources: "data": [ { "static": { - "key": "limit-to-be-activated", + "key": "limit_to_be_activated", "value": "1" } } @@ -220,7 +220,8 @@ static_resources: "data": [ { "selector": { - "selector": "metadata.filter_metadata.envoy\\.filters\\.http\\.header_to_metadata.user-id" + "selector": "metadata.filter_metadata.envoy\\.filters\\.http\\.header_to_metadata.user_id", + "key": "user_id" } } ] From b2d9f0de408b448df02f3b8c1ae71c20bb5a6f86 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Fri, 21 Jul 2023 17:36:40 +0200 Subject: [PATCH 07/14] fix current e2e tests --- tests/rate_limited.rs | 151 ++++++++++++++++++++++++------------------ 1 file changed, 85 insertions(+), 66 deletions(-) diff --git a/tests/rate_limited.rs b/tests/rate_limited.rs index 2d340e91..325ebbdd 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#"{ - "failure_mode_deny": true, - "rate_limit_policies": [] + "failureMode": "deny", + "rateLimitPolicies": [] }"#; module @@ -68,6 +68,7 @@ fn it_loads() { module .call_proxy_on_response_headers(http_context, 0, false) + .expect_log(Some(LogLevel::Debug), Some("on_http_response_headers #2")) .execute_and_expect(ReturnType::Action(Action::Continue)) .unwrap(); } @@ -89,37 +90,45 @@ fn it_limits() { let root_context = 1; let cfg = r#"{ - "failure_mode_deny": true, - "rate_limit_policies": [ - { + "failureMode": "deny", + "rateLimitPolicies": [ + { "name": "some-name", - "rate_limit_domain": "RLS-domain", - "upstream_cluster": "limitador-cluster", + "domain": "RLS-domain", + "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], - "gateway_actions": [ + "rules": [ { - "rules": [ - { - "paths": ["/admin/toy"], - "hosts": ["cars.toystore.com"], - "methods": ["POST"] - }], - "configurations": [ - { - "actions": [ + "conditions": [ { - "generic_key": { - "descriptor_key": "admin", - "descriptor_value": "1" - } + "allOf": [ + { + "selector": "request.url_path", + "operator": "starts_with", + "value": "/admin/toy" + }, + { + "selector": "request.host", + "operator": "eq", + "value": "cars.toystore.com" + }, + { + "selector": "request.method", + "operator": "eq", + "value": "POST" + }] } - ] - } + ], + "data": [ + { + "static": { + "key": "admin", + "value": "1" + } + } ] - } - ] - } - ] + }] + }] }"#; module @@ -148,12 +157,12 @@ fn it_limits() { .expect_log(Some(LogLevel::Info), Some("on_http_request_headers #2")) .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority")) .returning(Some("cars.toystore.com")) - .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":path")) - .returning(Some("/admin/toy")) - .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":method")) - .returning(Some("POST")) - .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority")) - .returning(Some("cars.toystore.com")) + .expect_get_property(Some(vec!["request", "url_path"])) + .returning(Some("/admin/toy".as_bytes())) + .expect_get_property(Some(vec!["request", "host"])) + .returning(Some("cars.toystore.com".as_bytes())) + .expect_get_property(Some(vec!["request", "method"])) + .returning(Some("POST".as_bytes())) .expect_grpc_call( Some("limitador-cluster"), Some("envoy.service.ratelimit.v3.RateLimitService"), @@ -178,7 +187,7 @@ fn it_limits() { .call_proxy_on_grpc_receive(http_context, 42, grpc_response.len() as i32) .expect_log( Some(LogLevel::Info), - Some("received gRPC call response: token: 42, status: 0"), + Some("on_grpc_call_response #2: received gRPC call response: token: 42, status: 0"), ) .expect_get_buffer_bytes(Some(BufferType::GrpcReceiveBuffer)) .returning(Some(&grpc_response)) @@ -187,6 +196,7 @@ fn it_limits() { module .call_proxy_on_response_headers(http_context, 0, false) + .expect_log(Some(LogLevel::Debug), Some("on_http_response_headers #2")) .execute_and_expect(ReturnType::Action(Action::Continue)) .unwrap(); } @@ -208,37 +218,45 @@ fn it_passes_additional_headers() { let root_context = 1; let cfg = r#"{ - "failure_mode_deny": true, - "rate_limit_policies": [ - { + "failureMode": "deny", + "rateLimitPolicies": [ + { "name": "some-name", - "rate_limit_domain": "RLS-domain", - "upstream_cluster": "limitador-cluster", + "domain": "RLS-domain", + "service": "limitador-cluster", "hostnames": ["*.toystore.com", "example.com"], - "gateway_actions": [ + "rules": [ { - "rules": [ - { - "paths": ["/admin/toy"], - "hosts": ["cars.toystore.com"], - "methods": ["POST"] - }], - "configurations": [ - { - "actions": [ + "conditions": [ { - "generic_key": { - "descriptor_key": "admin", - "descriptor_value": "1" - } + "allOf": [ + { + "selector": "request.url_path", + "operator": "starts_with", + "value": "/admin/toy" + }, + { + "selector": "request.host", + "operator": "eq", + "value": "cars.toystore.com" + }, + { + "selector": "request.method", + "operator": "eq", + "value": "POST" + }] } - ] - } + ], + "data": [ + { + "static": { + "key": "admin", + "value": "1" + } + } ] - } - ] - } - ] + }] + }] }"#; module @@ -267,12 +285,12 @@ fn it_passes_additional_headers() { .expect_log(Some(LogLevel::Info), Some("on_http_request_headers #2")) .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority")) .returning(Some("cars.toystore.com")) - .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":path")) - .returning(Some("/admin/toy")) - .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":method")) - .returning(Some("POST")) - .expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority")) - .returning(Some("cars.toystore.com")) + .expect_get_property(Some(vec!["request", "url_path"])) + .returning(Some("/admin/toy".as_bytes())) + .expect_get_property(Some(vec!["request", "host"])) + .returning(Some("cars.toystore.com".as_bytes())) + .expect_get_property(Some(vec!["request", "method"])) + .returning(Some("POST".as_bytes())) .expect_grpc_call( Some("limitador-cluster"), Some("envoy.service.ratelimit.v3.RateLimitService"), @@ -301,7 +319,7 @@ fn it_passes_additional_headers() { .call_proxy_on_grpc_receive(http_context, 42, grpc_response.len() as i32) .expect_log( Some(LogLevel::Info), - Some("received gRPC call response: token: 42, status: 0"), + Some("on_grpc_call_response #2: received gRPC call response: token: 42, status: 0"), ) .expect_get_buffer_bytes(Some(BufferType::GrpcReceiveBuffer)) .returning(Some(&grpc_response)) @@ -310,6 +328,7 @@ fn it_passes_additional_headers() { module .call_proxy_on_response_headers(http_context, 0, false) + .expect_log(Some(LogLevel::Debug), Some("on_http_response_headers #2")) .expect_add_header_map_value( Some(MapType::HttpResponseHeaders), Some("test"), From 3e445fa11c9c959bc96a286cd6a711648ac1f8a9 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 25 Jul 2023 17:32:42 +0200 Subject: [PATCH 08/14] replace starts_with and ends_with operators with startsWith and endsWith --- src/configuration.rs | 8 ++++---- tests/rate_limited.rs | 4 ++-- utils/docker-compose/envoy.yaml | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 879e2b0f..49f90fac 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -46,9 +46,9 @@ pub enum WhenConditionOperator { EqualOperator, #[serde(rename = "neq")] NotEqualOperator, - #[serde(rename = "starts_with")] + #[serde(rename = "startsWith")] StartsWithOperator, - #[serde(rename = "ends_with")] + #[serde(rename = "endsWith")] EndsWithOperator, #[serde(rename = "matches")] MatchesOperator, @@ -365,12 +365,12 @@ mod test { }, { "selector": "request.host", - "operator": "starts_with", + "operator": "startsWith", "value": "cars." }, { "selector": "request.host", - "operator": "ends_with", + "operator": "endsWith", "value": ".com" }, { diff --git a/tests/rate_limited.rs b/tests/rate_limited.rs index 325ebbdd..db2b81b8 100644 --- a/tests/rate_limited.rs +++ b/tests/rate_limited.rs @@ -104,7 +104,7 @@ fn it_limits() { "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/admin/toy" }, { @@ -232,7 +232,7 @@ fn it_passes_additional_headers() { "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/admin/toy" }, { diff --git a/utils/docker-compose/envoy.yaml b/utils/docker-compose/envoy.yaml index 8feac0ca..d53c7007 100644 --- a/utils/docker-compose/envoy.yaml +++ b/utils/docker-compose/envoy.yaml @@ -81,7 +81,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/unknown-path" } ] @@ -110,7 +110,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/get" }, { @@ -141,7 +141,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/get" }, { @@ -171,7 +171,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/get" }, { @@ -201,7 +201,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "starts_with", + "operator": "startsWith", "value": "/get" }, { From 5e66c64da6d97081d07237e4dbfe0b19c0e752cd Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 25 Jul 2023 18:09:36 +0200 Subject: [PATCH 09/14] doc new wasm config struct for RLPv2 --- README.md | 105 +++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 89 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 25fb24b5..22d8a441 100644 --- a/README.md +++ b/README.md @@ -4,24 +4,91 @@ A Proxy-Wasm module written in Rust, acting as a shim between Envoy and Limitado Following is a sample configuration used by the shim. ```yaml -failure_mode_deny: true -rate_limit_policies: - - name: toystore - rate_limit_domain: toystore-app - upstream_cluster: rate-limit-cluster +failureMode: deny +rateLimitPolicies: + - name: rlp-ns-A/rlp-name-A + domain: rlp-ns-A/rlp-name-A + service: rate-limit-cluster hostnames: ["*.toystore.com"] - gateway_actions: - - rules: - - paths: ["/admin/toy"] - methods: ["GET"] - hosts: ["pets.toystore.com"] - configurations: - - actions: - - generic_key: - descriptor_key: admin - descriptor_value: "1" + rules: + - conditions: + - allOf: + - selector: request.url_path + operator: startsWith + value: /get + - selector: request.host + operator: eq + value: test.toystore.com + - selector: request.method + operator: eq + value: GET + data: + - selector: + selector: request.headers.My-Custom-Header + - static: + key: admin + value: "1" ``` +## Features + +#### Condition operators implemented + +```Rust +#[derive(Deserialize, PartialEq, Debug, Clone)] +pub enum WhenConditionOperator { + #[serde(rename = "eq")] + EqualOperator, + #[serde(rename = "neq")] + NotEqualOperator, + #[serde(rename = "startsWith")] + StartsWithOperator, + #[serde(rename = "endsWith")] + EndsWithOperator, + #[serde(rename = "matches")] + MatchesOperator, +} +``` + +The `matches` operator is a a simple globbing pattern implementation based on regular expressions. +The only characters taken into account are: +* `?`: 0 or 1 characters +* `*`: 0 or more characters +* `+`: 1 or more characters + +#### Selectors + +The struct is + +```Rust +#[derive(Deserialize, Debug, Clone)] +pub struct SelectorItem { + // Selector of an attribute from the contextual properties provided by kuadrant + // during request and connection processing + pub selector: String, + + // If not set it defaults to `selector` field value as the descriptor key. + #[serde(default)] + pub key: Option, + + // An optional value to use if the selector is not found in the context. + // If not set and the selector is not found in the context, then no data is generated. + #[serde(default)] + pub default: Option, +} +``` + +Selectors are tokenized at each non-escaped occurrence of a separator character `.`. +Example: + +``` +Input: this.is.a.exam\.ple -> Retuns: ["this", "is", "a", "exam.ple"]. +``` + +Some path segments include dot `.` char in them. For instance envoy filter names: `envoy.filters.http.header_to_metadata`. +In that particular cases, the dot chat (separator), needs to be escaped. + + ## Building Prerequisites: @@ -44,7 +111,13 @@ Build the WASM module in release mode make build BUILD=release ``` -## Running/Testing locally +## Testing + +``` +cargo test +``` + +## Running local development environment `docker` and `docker-compose` required. From 96e513a8205405a3860dbeb5cf2473bc937d80bc Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 25 Jul 2023 18:12:24 +0200 Subject: [PATCH 10/14] replace startsWith and endsWith operators with startswith and endswith --- README.md | 6 +++--- src/configuration.rs | 6 +++--- tests/rate_limited.rs | 4 ++-- utils/docker-compose/envoy.yaml | 10 +++++----- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 22d8a441..10f8a229 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ rateLimitPolicies: - conditions: - allOf: - selector: request.url_path - operator: startsWith + operator: startswith value: /get - selector: request.host operator: eq @@ -41,9 +41,9 @@ pub enum WhenConditionOperator { EqualOperator, #[serde(rename = "neq")] NotEqualOperator, - #[serde(rename = "startsWith")] + #[serde(rename = "startswith")] StartsWithOperator, - #[serde(rename = "endsWith")] + #[serde(rename = "endswith")] EndsWithOperator, #[serde(rename = "matches")] MatchesOperator, diff --git a/src/configuration.rs b/src/configuration.rs index 49f90fac..c36f3f6b 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -46,7 +46,7 @@ pub enum WhenConditionOperator { EqualOperator, #[serde(rename = "neq")] NotEqualOperator, - #[serde(rename = "startsWith")] + #[serde(rename = "startswith")] StartsWithOperator, #[serde(rename = "endsWith")] EndsWithOperator, @@ -365,12 +365,12 @@ mod test { }, { "selector": "request.host", - "operator": "startsWith", + "operator": "startswith", "value": "cars." }, { "selector": "request.host", - "operator": "endsWith", + "operator": "endswith", "value": ".com" }, { diff --git a/tests/rate_limited.rs b/tests/rate_limited.rs index db2b81b8..59c7343a 100644 --- a/tests/rate_limited.rs +++ b/tests/rate_limited.rs @@ -104,7 +104,7 @@ fn it_limits() { "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/admin/toy" }, { @@ -232,7 +232,7 @@ fn it_passes_additional_headers() { "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/admin/toy" }, { diff --git a/utils/docker-compose/envoy.yaml b/utils/docker-compose/envoy.yaml index d53c7007..e2b8b25d 100644 --- a/utils/docker-compose/envoy.yaml +++ b/utils/docker-compose/envoy.yaml @@ -81,7 +81,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/unknown-path" } ] @@ -110,7 +110,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/get" }, { @@ -141,7 +141,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/get" }, { @@ -171,7 +171,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/get" }, { @@ -201,7 +201,7 @@ static_resources: "allOf": [ { "selector": "request.url_path", - "operator": "startsWith", + "operator": "startswith", "value": "/get" }, { From 76eff9a63d20dd8a70d79dc5cea5c4b9140493f9 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 25 Jul 2023 18:39:17 +0200 Subject: [PATCH 11/14] doc: clarify selectors --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 10f8a229..ef74ce62 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,11 @@ The only characters taken into account are: #### Selectors +Selector of an attribute from the contextual properties provided by kuadrant. +Currently, only some of the +[Envoy Attributes](https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes) +can be used. + The struct is ```Rust From e522bf65cee70b5e719f78e3af53f2b76e02cdcb Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 25 Jul 2023 19:05:56 +0200 Subject: [PATCH 12/14] fix clippy issues --- Cargo.lock | 2 +- src/configuration.rs | 52 ++++++++++++-------------------------- src/filter/http_context.rs | 2 +- 3 files changed, 18 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6589fa65..07553432 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1088,7 +1088,7 @@ dependencies = [ [[package]] name = "proxy-wasm-test-framework" version = "0.1.0" -source = "git+https://github.com/Kuadrant/wasm-test-framework.git?branch=kuadrant#904904c6f5232cdb02c7310d4254b715aa524699" +source = "git+https://github.com/Kuadrant/wasm-test-framework.git?branch=kuadrant#a9c07dd5b25f31bd649bd3155a247a96e3dcf5d7" dependencies = [ "anyhow", "lazy_static", diff --git a/src/configuration.rs b/src/configuration.rs index c36f3f6b..efc09e11 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -43,25 +43,25 @@ pub struct DataItem { #[derive(Deserialize, PartialEq, Debug, Clone)] pub enum WhenConditionOperator { #[serde(rename = "eq")] - EqualOperator, + Equal, #[serde(rename = "neq")] - NotEqualOperator, + NotEqual, #[serde(rename = "startswith")] - StartsWithOperator, - #[serde(rename = "endsWith")] - EndsWithOperator, + StartsWith, + #[serde(rename = "endswith")] + EndsWith, #[serde(rename = "matches")] - MatchesOperator, + Matches, } impl WhenConditionOperator { pub fn eval(&self, value: &str, attr_value: &str) -> bool { match *self { - WhenConditionOperator::EqualOperator => value.eq(attr_value), - WhenConditionOperator::NotEqualOperator => !value.eq(attr_value), - WhenConditionOperator::StartsWithOperator => attr_value.starts_with(value), - WhenConditionOperator::EndsWithOperator => attr_value.ends_with(value), - WhenConditionOperator::MatchesOperator => match GlobPattern::try_from(value) { + WhenConditionOperator::Equal => value.eq(attr_value), + WhenConditionOperator::NotEqual => !value.eq(attr_value), + WhenConditionOperator::StartsWith => attr_value.starts_with(value), + WhenConditionOperator::EndsWith => attr_value.ends_with(value), + WhenConditionOperator::Matches => match GlobPattern::try_from(value) { // TODO(eastizle): regexp being compiled and validated at request time. // Validations and possibly regexp compilation should happen at boot time instead. // In addition, if the regexp is not valid, the only consequence is that @@ -403,31 +403,11 @@ mod test { let expected_conditions = [ // selector, value, operator - ( - "request.path", - "/admin/toy", - WhenConditionOperator::EqualOperator, - ), - ( - "request.method", - "POST", - WhenConditionOperator::NotEqualOperator, - ), - ( - "request.host", - "cars.", - WhenConditionOperator::StartsWithOperator, - ), - ( - "request.host", - ".com", - WhenConditionOperator::EndsWithOperator, - ), - ( - "request.host", - "*.com", - WhenConditionOperator::MatchesOperator, - ), + ("request.path", "/admin/toy", WhenConditionOperator::Equal), + ("request.method", "POST", WhenConditionOperator::NotEqual), + ("request.host", "cars.", WhenConditionOperator::StartsWith), + ("request.host", ".com", WhenConditionOperator::EndsWith), + ("request.host", "*.com", WhenConditionOperator::Matches), ]; for i in 0..expected_conditions.len() { diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index 3a69431e..48be40e2 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -133,7 +133,7 @@ impl Filter { } } - fn build_single_descriptor(&self, data_list: &Vec) -> Option { + fn build_single_descriptor(&self, data_list: &[DataItem]) -> Option { let mut entries = ::protobuf::RepeatedField::default(); // iterate over data items to allow any data item to skip the entire descriptor From 3ba1b8f40589acd979088f13e66748bd07b82793 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Tue, 25 Jul 2023 19:13:46 +0200 Subject: [PATCH 13/14] fix more clippy issues --- src/configuration.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index efc09e11..5e0b69db 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -260,7 +260,7 @@ mod test { assert_eq!(static_item.key, "rlp-ns-A/rlp-name-A"); assert_eq!(static_item.value, "1"); } else { - assert!(false); + panic!(); } if let DataType::Selector(selector_item) = &data_items[1].item { @@ -268,7 +268,7 @@ mod test { assert!(selector_item.key.is_none()); assert!(selector_item.default.is_none()); } else { - assert!(false); + panic!(); } } @@ -334,7 +334,7 @@ mod test { "my_selector_default_value" ); } else { - assert!(false); + panic!(); } } From 59033b0b4f89290391c8eee9acf00fde51fe6654 Mon Sep 17 00:00:00 2001 From: Alex Snaps Date: Fri, 28 Jul 2023 14:00:06 -0400 Subject: [PATCH 14/14] Use empty string ("") when querying a non existing property --- src/filter/http_context.rs | 67 ++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/src/filter/http_context.rs b/src/filter/http_context.rs index 48be40e2..84a0a5f9 100644 --- a/src/filter/http_context.rs +++ b/src/filter/http_context.rs @@ -108,29 +108,30 @@ impl Filter { let attribute_path = tokenize_with_escaping(&p_e.selector, '.', '\\'); // convert a Vec to Vec<&str> // attribute_path.iter().map(AsRef::as_ref).collect() - match self.get_property(attribute_path.iter().map(AsRef::as_ref).collect()) { - None => { - debug!( - "[context_id: {}]: selector not found: {}", - self.context_id, p_e.selector - ); - false - } - // TODO(eastizle): not all fields are strings - // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes - Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { - Err(e) => { + let attribute_value = + match self.get_property(attribute_path.iter().map(AsRef::as_ref).collect()) { + None => { debug!( - "[context_id: {}]: failed to parse selector value: {}, error: {}", - self.context_id, p_e.selector, e + "[context_id: {}]: selector not found: {}, defaulting to ``", + self.context_id, p_e.selector ); - false + "".to_string() } - Ok(attribute_value) => p_e - .operator - .eval(p_e.value.as_str(), attribute_value.as_str()), - }, - } + // TODO(eastizle): not all fields are strings + // https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes + Some(attribute_bytes) => match String::from_utf8(attribute_bytes) { + Err(e) => { + debug!( + "[context_id: {}]: failed to parse selector value: {}, error: {}", + self.context_id, p_e.selector, e + ); + return false; + } + Ok(attribute_value) => attribute_value, + }, + }; + p_e.operator + .eval(p_e.value.as_str(), attribute_value.as_str()) } fn build_single_descriptor(&self, data_list: &[DataItem]) -> Option { @@ -154,7 +155,9 @@ impl Filter { let attribute_path = tokenize_with_escaping(&selector_item.selector, '.', '\\'); // convert a Vec to Vec<&str> // attribute_path.iter().map(AsRef::as_ref).collect() - match self.get_property(attribute_path.iter().map(AsRef::as_ref).collect()) { + let value = match self + .get_property(attribute_path.iter().map(AsRef::as_ref).collect()) + { None => { debug!( "[context_id: {}]: selector not found: {}", @@ -162,13 +165,8 @@ impl Filter { ); match &selector_item.default { // skipping the entire descriptor - None => return None, - Some(default_value) => { - let mut descriptor_entry = RateLimitDescriptor_Entry::new(); - descriptor_entry.set_key(descriptor_key); - descriptor_entry.set_value(default_value.to_owned()); - entries.push(descriptor_entry); - } + None => "".to_string(), + Some(default_value) => default_value.clone(), } } // TODO(eastizle): not all fields are strings @@ -181,14 +179,13 @@ impl Filter { ); return None; } - Ok(attribute_value) => { - let mut descriptor_entry = RateLimitDescriptor_Entry::new(); - descriptor_entry.set_key(descriptor_key); - descriptor_entry.set_value(attribute_value); - entries.push(descriptor_entry); - } + Ok(attribute_value) => attribute_value, }, - } + }; + let mut descriptor_entry = RateLimitDescriptor_Entry::new(); + descriptor_entry.set_key(descriptor_key); + descriptor_entry.set_value(value); + entries.push(descriptor_entry); } } }