From 3e7a42801595bd558224d29ba1fb66aed1b52a17 Mon Sep 17 00:00:00 2001 From: Alex Snaps Date: Wed, 27 Nov 2024 06:58:42 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A5=20feature=20lenient=5Fconditions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Alex Snaps --- doc/migrations/conditions.md | 19 -------- limitador-server/Cargo.toml | 2 +- limitador-server/src/main.rs | 7 --- limitador/Cargo.toml | 1 - limitador/README.md | 1 - limitador/src/limit.rs | 90 +----------------------------------- 6 files changed, 2 insertions(+), 118 deletions(-) diff --git a/doc/migrations/conditions.md b/doc/migrations/conditions.md index 29fd1a8b..ffd4bb49 100644 --- a/doc/migrations/conditions.md +++ b/doc/migrations/conditions.md @@ -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. diff --git a/limitador-server/Cargo.toml b/limitador-server/Cargo.toml index 6ed286c9..5608e939 100644 --- a/limitador-server/Cargo.toml +++ b/limitador-server/Cargo.toml @@ -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" diff --git a/limitador-server/src/main.rs b/limitador-server/src/main.rs index 99193219..ac8e0861 100644 --- a/limitador-server/src/main.rs +++ b/limitador-server/src/main.rs @@ -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!( @@ -597,10 +594,6 @@ fn create_config() -> (Configuration, &'static str) { let parsed_limits: Result, _> = 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 = limits.iter().map(|l| l.into()).collect(); match serde_yaml::to_string(&output) { diff --git a/limitador/Cargo.toml b/limitador/Cargo.toml index a2dc0033..fdaaf03a 100644 --- a/limitador/Cargo.toml +++ b/limitador/Cargo.toml @@ -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"] } diff --git a/limitador/README.md b/limitador/README.md index fd8f6f95..305f9d45 100644 --- a/limitador/README.md +++ b/limitador/README.md @@ -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`. diff --git a/limitador/src/limit.rs b/limitador/src/limit.rs index f19b5041..b6729634 100644 --- a/limitador/src/limit.rs +++ b/limitador/src/limit.rs @@ -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); @@ -167,44 +146,6 @@ impl TryFrom 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) { ( @@ -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 = 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 = 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"]) @@ -1027,11 +942,8 @@ mod tests { } #[test] - #[cfg(not(feature = "lenient_conditions"))] fn invalid_deprecated_condition_parsing() { - let _result = serde_json::from_str::(r#""x == 5""#) - .err() - .expect("Should fail!"); + serde_json::from_str::(r#""x == 5""#).expect_err("Should fail!"); } #[test]