Skip to content

Commit

Permalink
selector data type 🔥
Browse files Browse the repository at this point in the history 
Signed-off-by: Eguzki Astiz Lezaun <[email protected]>
  • Loading branch information
@eguzki
eguzki committed Nov 8, 2024
1 parent ce3a829 commit 21b2fd7
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 287 deletions.
165 changes: 13 additions & 152 deletions src/configuration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::configuration::action_set::ActionSet;
use crate::configuration::action_set_index::ActionSetIndex;
use crate::data;
use crate::data::Predicate;
use crate::data::PropertyPath;
use crate::service::GrpcService;
use cel_interpreter::functions::duration;
use cel_interpreter::Value;
Expand Down Expand Up @@ -37,39 +36,6 @@ impl ExpressionItem {
}
}

#[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<String>,

// 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<String>,

#[serde(skip_deserializing)]
path: OnceCell<PropertyPath>,
}

impl SelectorItem {
pub fn compile(&self) -> Result<(), String> {
self.path
.set(self.selector.as_str().into())
.map_err(|p| format!("Err on {p:?}"))
}

pub fn path(&self) -> &PropertyPath {
self.path
.get()
.expect("SelectorItem wasn't previously compiled!")
}
}

#[derive(Deserialize, Debug, Clone)]
pub struct StaticItem {
pub value: String,
Expand All @@ -81,15 +47,13 @@ pub struct StaticItem {
#[serde(rename_all = "lowercase")]
pub enum DataType {
Static(StaticItem),
Selector(SelectorItem),
Expression(ExpressionItem),
}

impl DataType {
pub fn compile(&self) -> Result<(), String> {
match self {
DataType::Static(_) => Ok(()),
DataType::Selector(selector) => selector.compile(),
DataType::Expression(exp) => exp.compile(),
}
}
Expand Down Expand Up @@ -405,118 +369,7 @@ mod test {
}

#[test]
fn parse_config_data_selector() {
let config = r#"{
"services": {
"limitador": {
"type": "ratelimit",
"endpoint": "limitador-cluster",
"failureMode": "deny"
}
},
"actionSets": [
{
"name": "rlp-ns-A/rlp-name-A",
"routeRuleConditions": {
"hostnames": ["*.toystore.com", "example.com"]
},
"actions": [
{
"service": "limitador",
"scope": "rlp-ns-A/rlp-name-A",
"data": [
{
"selector": {
"selector": "my.selector.path",
"key": "mykey",
"default": "my_selector_default_value"
}
}]
}]
}]
}"#;
let res = serde_json::from_str::<PluginConfiguration>(config);
if let Err(ref e) = res {
eprintln!("{e}");
}
assert!(res.is_ok());

let filter_config = res.unwrap();
assert_eq!(filter_config.action_sets.len(), 1);

let actions = &filter_config.action_sets[0].actions;
assert_eq!(actions.len(), 1);

let data_items = &actions[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 {
panic!();
}
}

#[test]
fn parse_config_condition_selector_operators() {
let config = r#"{
"services": {
"limitador": {
"type": "ratelimit",
"endpoint": "limitador-cluster",
"failureMode": "deny"
}
},
"actionSets": [
{
"name": "rlp-ns-A/rlp-name-A",
"routeRuleConditions": {
"hostnames": ["*.toystore.com", "example.com"],
"predicates": [
"request.path == '/admin/toy'",
"request.method != 'POST'",
"request.host.startsWith('cars.')",
"request.host.endsWith('.com')",
"request.host.matches('*.com')"
]
},
"actions": [
{
"service": "limitador",
"scope": "rlp-ns-A/rlp-name-A",
"predicates": [
"auth.metadata.username == 'alice'",
"request.path.endsWith('/car')"
],
"data": [ { "selector": { "selector": "my.selector.path" } }]
}]
}]
}"#;
let res = serde_json::from_str::<PluginConfiguration>(config);
if let Err(ref e) = res {
eprintln!("{e}");
}
assert!(res.is_ok());

let filter_config = res.unwrap();
assert_eq!(filter_config.action_sets.len(), 1);

let predicates = &filter_config.action_sets[0]
.route_rule_conditions
.predicates;
assert_eq!(predicates.len(), 5);

let predicates = &filter_config.action_sets[0].actions[0].predicates;
assert_eq!(predicates.len(), 2);
}

#[test]
fn parse_config_conditions_optional() {
fn parse_config_predicates_optional() {
let config = r#"{
"services": {
"limitador": {
Expand All @@ -543,8 +396,9 @@ mod test {
}
},
{
"selector": {
"selector": "auth.metadata.username"
"expression": {
"key": "username",
"value": "auth.metadata.username"
}
}]
}]
Expand All @@ -569,6 +423,12 @@ mod test {
.route_rule_conditions
.predicates;
assert_eq!(predicates.len(), 0);

let actions = &filter_config.action_sets[0].actions;
assert_eq!(actions.len(), 1);

let action_predicates = &actions[0].predicates;
assert_eq!(action_predicates.len(), 0);
}

#[test]
Expand Down Expand Up @@ -598,8 +458,9 @@ mod test {
"key": "rlp-ns-A/rlp-name-A",
"value": "1"
},
"selector": {
"selector": "auth.metadata.username"
"expression": {
"key": "username",
"value": "auth.metadata.username"
}
}]
}]
Expand Down
33 changes: 1 addition & 32 deletions src/configuration/action.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::configuration::{DataItem, DataType};
use crate::data::Predicate;
use crate::envoy::{RateLimitDescriptor, RateLimitDescriptor_Entry};
use cel_interpreter::Value;
use log::{debug, error};
use log::error;
use protobuf::RepeatedField;
use serde::Deserialize;
use std::cell::OnceCell;
Expand Down Expand Up @@ -80,37 +80,6 @@ impl Action {
}
},
),
DataType::Selector(selector_item) => {
let descriptor_key = match &selector_item.key {
None => selector_item.path().to_string(),
Some(key) => key.to_owned(),
};

let value = match crate::data::get_attribute::<String>(selector_item.path())
.expect("Error!")
{
//TODO(didierofrivia): Replace hostcalls by DI
None => {
debug!(
"build_single_descriptor: selector not found: {}",
selector_item.path()
);
match &selector_item.default {
None => return None, // skipping the entire descriptor
Some(default_value) => default_value.clone(),
}
}
// TODO(eastizle): not all fields are strings
// https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/advanced/attributes
Some(attr_str) => attr_str,
// Alternative implementation (for rust >= 1.76)
// Attribute::parse(attribute_bytes)
// .inspect_err(|e| debug!("#{} build_single_descriptor: failed to parse selector value: {}, error: {}",
// filter.context_id, attribute_path, e))
// .ok()?,
};
(descriptor_key, value)
}
};
let mut descriptor_entry = RateLimitDescriptor_Entry::new();
descriptor_entry.set_key(key);
Expand Down
105 changes: 2 additions & 103 deletions tests/rate_limited.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ fn it_passes_additional_headers() {

#[test]
#[serial]
fn it_rate_limits_with_empty_conditions() {
fn it_rate_limits_with_empty_predicates() {
let args = tester::MockSettings {
wasm_path: wasm_module(),
quiet: false,
Expand Down Expand Up @@ -496,108 +496,7 @@ fn it_rate_limits_with_empty_conditions() {

#[test]
#[serial]
fn it_does_not_rate_limits_when_selector_does_not_exist_and_misses_default_value() {
let args = tester::MockSettings {
wasm_path: wasm_module(),
quiet: false,
allow_unexpected: false,
};
let mut module = tester::mock(args).unwrap();

module
.call_start()
.execute_and_expect(ReturnType::None)
.unwrap();

let root_context = 1;
let cfg = r#"{
"services": {
"limitador": {
"type": "ratelimit",
"endpoint": "limitador-cluster",
"failureMode": "deny",
"timeout": "5s"
}
},
"actionSets": [
{
"name": "some-name",
"routeRuleConditions": {
"hostnames": ["*.com"]
},
"actions": [
{
"service": "limitador",
"scope": "RLS-domain",
"data": [
{
"selector": {
"selector": "unknown.path"
}
}
]
}]
}]
}"#;

module
.call_proxy_on_context_create(root_context, 0)
.expect_log(Some(LogLevel::Info), Some("#1 set_root_context"))
.execute_and_expect(ReturnType::None)
.unwrap();
module
.call_proxy_on_configure(root_context, 0)
.expect_log(Some(LogLevel::Info), Some("#1 on_configure"))
.expect_get_buffer_bytes(Some(BufferType::PluginConfiguration))
.returning(Some(cfg.as_bytes()))
.expect_log(Some(LogLevel::Info), None)
.execute_and_expect(ReturnType::Bool(true))
.unwrap();

let http_context = 2;
module
.call_proxy_on_context_create(http_context, root_context)
.expect_log(Some(LogLevel::Debug), Some("#2 create_http_context"))
.execute_and_expect(ReturnType::None)
.unwrap();

module
.call_proxy_on_request_headers(http_context, 0, false)
.expect_log(Some(LogLevel::Debug), Some("#2 on_http_request_headers"))
.expect_get_header_map_value(Some(MapType::HttpRequestHeaders), Some(":authority"))
.returning(Some("a.com"))
.expect_log(
Some(LogLevel::Debug),
Some("#2 action_set selected some-name"),
)
// retrieving properties for RateLimitRequest
.expect_log(
Some(LogLevel::Debug),
Some("get_property: path: [\"unknown\", \"path\"]"),
)
.expect_get_property(Some(vec!["unknown", "path"]))
.returning(None)
.expect_log(
Some(LogLevel::Debug),
Some("build_single_descriptor: selector not found: unknown.path"),
)
.expect_log(
Some(LogLevel::Debug),
Some("grpc_message_request: empty descriptors"),
)
.execute_and_expect(ReturnType::Action(Action::Continue))
.unwrap();

module
.call_proxy_on_response_headers(http_context, 0, false)
.expect_log(Some(LogLevel::Debug), Some("#2 on_http_response_headers"))
.execute_and_expect(ReturnType::Action(Action::Continue))
.unwrap();
}

#[test]
#[serial]
fn it_does_not_rate_limits_when_condition_does_not_match() {
fn it_does_not_rate_limits_when_predicates_does_not_match() {
let args = tester::MockSettings {
wasm_path: wasm_module(),
quiet: false,
Expand Down

0 comments on commit 21b2fd7

Please sign in to comment.