Skip to content

Commit

Permalink
🔥 feature lenient_conditions
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Snaps <[email protected]>
  • Loading branch information
alexsnaps committed Nov 27, 2024
1 parent 323bc8a commit 91fbf64
Show file tree
Hide file tree
Showing 6 changed files with 3 additions and 118 deletions.
19 changes: 0 additions & 19 deletions doc/migrations/conditions.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,3 @@ case `foo` was the identifier of the variable, while `bar` was the value to eval
after the operator `==` would be equally important. SO that `foo == bar` would test for a `foo ` variable being equal
to ` bar` where the trailing whitespace after the identifier, and the one prefixing the value, would have been
evaluated.

## Server binary users

The server still allows for the deprecated syntax, but warns about its usage. You can easily migrate your limits file,
using the following command:

```commandline
limitador-server --validate old_limits.yaml > updated_limits.yaml
```

Which should output `Deprecated syntax for conditions corrected!` to `stderr` while `stdout` would be the limits using
the new syntax. It is recommended you manually verify the resulting `LIMITS_FILE`.


## Crate users

A feature `lenient_conditions` has been added, which lets you use the syntax used in previous version of the crate.
The function `limitador::limit::check_deprecated_syntax_usages_and_reset()` lets you verify if the deprecated syntax
has been used as `limit::Limit`s are created with their condition strings using the deprecated syntax.
2 changes: 1 addition & 1 deletion limitador-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ distributed_storage = ["limitador/distributed_storage"]
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
limitador = { path = "../limitador", features = ['lenient_conditions'] }
limitador = { path = "../limitador" }
tokio = { version = "1", features = ["full"] }
thiserror = "2"
tonic = "0.12.3"
Expand Down
7 changes: 0 additions & 7 deletions limitador-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,6 @@ impl Limiter {
Self::Blocking(limiter) => limiter.configure_with(limits)?,
Self::Async(limiter) => limiter.configure_with(limits).await?,
}
if limitador::limit::check_deprecated_syntax_usages_and_reset() {
error!("You are using deprecated syntax for your conditions! See the migration guide https://docs.kuadrant.io/limitador/doc/migrations/conditions/")
}
Ok(())
}
Err(e) => Err(LimitadorServerError::ConfigFile(format!(
Expand Down Expand Up @@ -597,10 +594,6 @@ fn create_config() -> (Configuration, &'static str) {
let parsed_limits: Result<Vec<Limit>, _> = serde_yaml::from_reader(f);
match parsed_limits {
Ok(limits) => {
if limitador::limit::check_deprecated_syntax_usages_and_reset() {
eprintln!("Deprecated syntax for conditions corrected!\n")
}

let output: Vec<http_api::LimitVO> =
limits.iter().map(|l| l.into()).collect();
match serde_yaml::to_string(&output) {
Expand Down
1 change: 0 additions & 1 deletion limitador/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ default = ["disk_storage", "redis_storage"]
disk_storage = ["rocksdb"]
distributed_storage = ["tokio", "tokio-stream", "h2", "base64", "uuid", "tonic", "tonic-reflection", "prost", "prost-types"]
redis_storage = ["redis", "r2d2", "tokio"]
lenient_conditions = []

[dependencies]
moka = { version = "0.12", features = ["sync"] }
Expand Down
1 change: 0 additions & 1 deletion limitador/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,4 @@ For the complete documentation of the crate's API, please refer to [docs.rs](htt

* `redis_storage`: support for using Redis as the data storage backend.
* `disk_storage`: support for using RocksDB as a local disk storage backend.
* `lenient_conditions`: support for the deprecated syntax of `Condition`s
* `default`: `redis_storage`.
91 changes: 2 additions & 89 deletions limitador/src/limit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,8 @@ use std::error::Error;
use std::fmt::{Debug, Display, Formatter};
use std::hash::{Hash, Hasher};

#[cfg(feature = "lenient_conditions")]
mod deprecated {
use std::sync::atomic::{AtomicBool, Ordering};

static DEPRECATED_SYNTAX: AtomicBool = AtomicBool::new(false);

pub fn check_deprecated_syntax_usages_and_reset() -> bool {
match DEPRECATED_SYNTAX.compare_exchange(true, false, Ordering::Relaxed, Ordering::Relaxed)
{
Ok(previous) => previous,
Err(previous) => previous,
}
}

pub fn deprecated_syntax_used() {
DEPRECATED_SYNTAX.fetch_or(true, Ordering::SeqCst);
}
}

use crate::errors::LimitadorError;
use crate::LimitadorResult;
#[cfg(feature = "lenient_conditions")]
pub use deprecated::check_deprecated_syntax_usages_and_reset;

#[derive(Debug, Hash, Eq, PartialEq, Clone, PartialOrd, Ord, Serialize, Deserialize)]
pub struct Namespace(String);
Expand Down Expand Up @@ -167,44 +146,6 @@ impl TryFrom<String> for Condition {
)
}
}
#[cfg(feature = "lenient_conditions")]
(TokenType::Identifier, TokenType::EqualEqual, TokenType::Identifier) => {
if let (
Some(Literal::Identifier(var_name)),
Some(Literal::Identifier(operand)),
) = (&tokens[0].literal, &tokens[2].literal)
{
deprecated::deprecated_syntax_used();
Ok(Condition {
var_name: var_name.clone(),
predicate: Predicate::Equal,
operand: operand.clone(),
})
} else {
panic!(
"Unexpected state {tokens:?} returned from Scanner for: `{value}`"
)
}
}
#[cfg(feature = "lenient_conditions")]
(TokenType::Identifier, TokenType::EqualEqual, TokenType::Number) => {
if let (
Some(Literal::Identifier(var_name)),
Some(Literal::Number(operand)),
) = (&tokens[0].literal, &tokens[2].literal)
{
deprecated::deprecated_syntax_used();
Ok(Condition {
var_name: var_name.clone(),
predicate: Predicate::Equal,
operand: operand.to_string(),
})
} else {
panic!(
"Unexpected state {tokens:?} returned from Scanner for: `{value}`"
)
}
}
(t1, t2, _) => {
let faulty = match (t1, t2) {
(
Expand Down Expand Up @@ -902,32 +843,6 @@ mod tests {
assert!(!limit.applies(&values))
}

#[test]
#[cfg(feature = "lenient_conditions")]
fn limit_does_not_apply_when_cond_is_false_deprecated_style() {
let limit = Limit::new("test_namespace", 10, 60, vec!["x == 5"], vec!["y"])
.expect("This must be a valid limit!");

let mut values: HashMap<String, String> = HashMap::new();
values.insert("x".into(), "1".into());
values.insert("y".into(), "1".into());

assert!(!limit.applies(&values));
assert!(check_deprecated_syntax_usages_and_reset());
assert!(!check_deprecated_syntax_usages_and_reset());

let limit = Limit::new("test_namespace", 10, 60, vec!["x == foobar"], vec!["y"])
.expect("This must be a valid limit!");

let mut values: HashMap<String, String> = HashMap::new();
values.insert("x".into(), "foobar".into());
values.insert("y".into(), "1".into());

assert!(limit.applies(&values));
assert!(check_deprecated_syntax_usages_and_reset());
assert!(!check_deprecated_syntax_usages_and_reset());
}

#[test]
fn limit_does_not_apply_when_cond_var_is_not_set() {
let limit = Limit::new("test_namespace", 10, 60, vec!["x == \"5\""], vec!["y"])
Expand Down Expand Up @@ -1027,11 +942,9 @@ mod tests {
}

Check warning on line 943 in limitador/src/limit.rs

View workflow job for this annotation

GitHub Actions / Rustfmt

Diff in /home/runner/work/limitador/limitador/limitador/src/limit.rs
#[test]
#[cfg(not(feature = "lenient_conditions"))]
fn invalid_deprecated_condition_parsing() {
let _result = serde_json::from_str::<Condition>(r#""x == 5""#)
.err()
.expect("Should fail!");
serde_json::from_str::<Condition>(r#""x == 5""#)
.expect_err("Should fail!");
}

#[test]
Expand Down

0 comments on commit 91fbf64

Please sign in to comment.