From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pbs-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 8AD451FF183 for <inbox@lore.proxmox.com>; Wed, 21 May 2025 16:23:52 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EE38C1A638; Wed, 21 May 2025 16:23:51 +0200 (CEST) From: Lukas Wagner <l.wagner@proxmox.com> To: pbs-devel@lists.proxmox.com Date: Wed, 21 May 2025 16:23:06 +0200 Message-Id: <20250521142309.264719-2-l.wagner@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250521142309.264719-1-l.wagner@proxmox.com> References: <20250521142309.264719-1-l.wagner@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.020 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox 1/2] notify: matcher: allow to evaluate other matchers via `eval-matcher` X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion <pbs-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/> List-Post: <mailto:pbs-devel@lists.proxmox.com> List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com> This commit adds support for nested matchers, allowing to build arbitrary matching 'formulas'. A matcher can now have one or more `eval-matcher` directives. These take the name of another matcher as a parameter. When the matcher is evaluated, we first check any referenced nested matcher and use their results to compute the final verdict for the matcher. If one wants to use a matcher via `eval-matcher`, the matcher must be marked as nested. Any nested matcher is only evaluated when another matcher references it. Any configured targets for a nested matcher are ignored, only the 'top-most' matcher's targets are notified if the notification matches. Direct/indirect recursion is not allowed (A -> A, A -> B -> A) an raises an error. A simple cache is introduced to make sure that we don't have to evaluate any nested matcher more than once. Simple config example: matcher: top-level target mail-to-root mode any eval-matcher a eval-matcher b matcher: a nested true mode all match-field exact:datastore=store match-field exact:type=gc match-severity error matcher: b nested true mode all match-field exact:datastore=store match-field exact:type=prune match-severity error Signed-off-by: Lukas Wagner <l.wagner@proxmox.com> --- proxmox-notify/src/lib.rs | 20 ++- proxmox-notify/src/matcher.rs | 309 ++++++++++++++++++++++++++++++++-- 2 files changed, 304 insertions(+), 25 deletions(-) diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 12e59474..bb5bb605 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -376,7 +376,7 @@ impl Config { #[derive(Default)] pub struct Bus { endpoints: HashMap<String, Box<dyn Endpoint>>, - matchers: Vec<MatcherConfig>, + matchers: HashMap<String, MatcherConfig>, } #[allow(unused_macros)] @@ -514,10 +514,14 @@ impl Bus { ); } - let matchers = config - .config - .convert_to_typed_array(MATCHER_TYPENAME) - .map_err(|err| Error::ConfigDeserialization(err.into()))?; + let matchers = HashMap::from_iter( + config + .config + .convert_to_typed_array(MATCHER_TYPENAME) + .map_err(|err| Error::ConfigDeserialization(err.into()))? + .into_iter() + .map(|e: MatcherConfig| (e.name.clone(), e)), + ); Ok(Bus { endpoints, @@ -531,8 +535,8 @@ impl Bus { } #[cfg(test)] - pub fn add_matcher(&mut self, filter: MatcherConfig) { - self.matchers.push(filter) + pub fn add_matcher(&mut self, matcher: MatcherConfig) { + self.matchers.insert(matcher.name.clone(), matcher); } /// Send a notification. Notification matchers will determine which targets will receive @@ -540,7 +544,7 @@ impl Bus { /// /// Any errors will not be returned but only logged. pub fn send(&self, notification: &Notification) { - let targets = matcher::check_matches(self.matchers.as_slice(), notification); + let targets = matcher::check_matches(&self.matchers, notification); for target in targets { if let Some(endpoint) = self.endpoints.get(target) { diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index 083c2dbd..3cc0189a 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::fmt::Debug; use std::str::FromStr; @@ -98,6 +98,13 @@ pub const MATCH_FIELD_ENTRY_SCHEMA: Schema = StringSchema::new("Match metadata f }, optional: true, }, + "eval-matcher": { + type: Array, + items: { + schema: ENTITY_NAME_SCHEMA, + }, + optional: true, + }, "target": { type: Array, items: { @@ -106,7 +113,7 @@ pub const MATCH_FIELD_ENTRY_SCHEMA: Schema = StringSchema::new("Match metadata f optional: true, }, })] -#[derive(Debug, Serialize, Deserialize, Updater, Default)] +#[derive(Clone, Debug, Serialize, Deserialize, Updater, Default)] #[serde(rename_all = "kebab-case")] /// Config for Sendmail notification endpoints pub struct MatcherConfig { @@ -136,6 +143,11 @@ pub struct MatcherConfig { #[serde(skip_serializing_if = "Option::is_none")] pub invert_match: Option<bool>, + /// List of nested matchers to evaluate. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub eval_matcher: Vec<String>, + /// Targets to notify. #[serde(default, skip_serializing_if = "Vec::is_empty")] #[updater(serde(skip_serializing_if = "Option::is_none"))] @@ -149,6 +161,13 @@ pub struct MatcherConfig { #[serde(skip_serializing_if = "Option::is_none")] pub disable: Option<bool>, + /// Determine if this matcher can be used as a nested matcher. + /// Nested matchers are only evaluated if they are referenced by + /// another matcher. Any configured targets of a nested matcher + /// are ignored. + #[serde(skip_serializing_if = "Option::is_none")] + pub nested: Option<bool>, + /// Origin of this config entry. #[serde(skip_serializing_if = "Option::is_none")] #[updater(skip)] @@ -282,7 +301,30 @@ impl FromStr for FieldMatcher { } impl MatcherConfig { - pub fn matches(&self, notification: &Notification) -> Result<Option<&[String]>, Error> { + pub fn matches( + &self, + notification: &Notification, + all_matchers: &HashMap<String, MatcherConfig>, + cache: &mut HashMap<String, bool>, + ) -> Result<Option<&[String]>, Error> { + let mut trace = HashSet::new(); + trace.insert(self.name.clone()); + let result = self.matches_impl(notification, all_matchers, &mut trace, cache)?; + + if result { + Ok(Some(&self.target)) + } else { + Ok(None) + } + } + + fn matches_impl( + &self, + notification: &Notification, + all_matchers: &HashMap<String, MatcherConfig>, + trace: &mut HashSet<String>, + cache: &mut HashMap<String, bool>, + ) -> Result<bool, Error> { let mode = self.mode.unwrap_or_default(); let mut is_match = mode.neutral_element(); @@ -311,13 +353,52 @@ impl MatcherConfig { ); } - let invert_match = self.invert_match.unwrap_or_default(); + for matcher_name in &self.eval_matcher { + no_matchers = false; + match all_matchers.get(matcher_name) { + Some(matcher) if matcher.nested.unwrap_or_default() => { + if trace.contains(matcher_name) { + return Err(Error::FilterFailed( + "recursive sub-matcher definition".into(), + )); + } - Ok(if is_match != invert_match || no_matchers { - Some(&self.target) - } else { - None - }) + if matcher.disable.unwrap_or_default() { + continue; + } + + trace.insert(matcher.name.clone()); + + if let Some(cached_result) = cache.get(&matcher.name) { + is_match = mode.apply(is_match, *cached_result); + } else { + is_match = mode.apply( + is_match, + matcher.matches_impl(notification, all_matchers, trace, cache)?, + ); + } + + trace.remove(matcher_name); + } + Some(_) => { + return Err(Error::FilterFailed( + "referenced matcher is not declared as sub-matcher".into(), + )); + } + None => { + return Err(Error::FilterFailed( + "referenced sub-matcher does not exist".into(), + )); + } + } + } + + let invert_match = self.invert_match.unwrap_or_default(); + is_match = is_match != invert_match || no_matchers; + + cache.insert(self.name.clone(), is_match); + + Ok(is_match) } /// Check if given `MatchDirectives` match a notification. @@ -438,24 +519,31 @@ pub enum DeleteableMatcherProperty { } pub fn check_matches<'a>( - matchers: &'a [MatcherConfig], + matchers: &'a HashMap<String, MatcherConfig>, notification: &Notification, ) -> HashSet<&'a str> { let mut targets = HashSet::new(); + let mut cache = HashMap::new(); - for matcher in matchers { - if matcher.disable.unwrap_or_default() { - // Skip this matcher if it is disabled - info!("skipping disabled matcher '{name}'", name = matcher.name); + for (name, matcher) in matchers { + if matcher.nested.unwrap_or_default() { + // Matchers which are declared are only evaluated if a + // top-level, non-nested matcher references it. continue; } - match matcher.matches(notification) { + if matcher.disable.unwrap_or_default() { + // Skip this matcher if it is disabled + info!("skipping disabled matcher '{name}'"); + continue; + } + + match matcher.matches(notification, matchers, &mut cache) { Ok(t) => { let t = t.unwrap_or_default(); targets.extend(t.iter().map(|s| s.as_str())); } - Err(err) => error!("matcher '{matcher}' failed: {err}", matcher = matcher.name), + Err(err) => error!("matcher '{name}' failed: {err}"), } } @@ -505,6 +593,7 @@ mod tests { assert!("regex:'3=b.*".parse::<FieldMatcher>().is_err()); assert!("invalid:'bar=b.*".parse::<FieldMatcher>().is_err()); } + #[test] fn test_severities() { let notification = @@ -526,7 +615,193 @@ mod tests { ..Default::default() }; - assert!(config.matches(¬ification).unwrap().is_some()) + let mut all_matchers = HashMap::new(); + all_matchers.insert("default".to_string(), config.clone()); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .unwrap() + .is_some()) } } + + #[test] + fn test_submatcher() { + let mut all_matchers = HashMap::new(); + + let config = MatcherConfig { + name: "sub-1".to_string(), + nested: Some(true), + mode: Some(MatchModeOperator::All), + match_severity: vec!["error".parse().unwrap()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let config = MatcherConfig { + name: "sub-2".to_string(), + nested: Some(true), + mode: Some(MatchModeOperator::All), + match_field: vec!["exact:datastore=backups".parse().unwrap()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let config = MatcherConfig { + name: "top".to_string(), + eval_matcher: vec!["sub-1".into(), "sub-2".into()], + mode: Some(MatchModeOperator::Any), + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let notification = Notification::from_template( + Severity::Notice, + "test", + Value::Null, + HashMap::from_iter([("datastore".into(), "backups".into())]), + ); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .unwrap() + .is_some()); + + let notification = Notification::from_template( + Severity::Error, + "test", + Value::Null, + HashMap::from_iter([("datastore".into(), "other".into())]), + ); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .unwrap() + .is_some()); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .unwrap() + .is_some()); + + let notification = Notification::from_template( + Severity::Warning, + "test", + Value::Null, + HashMap::from_iter([("datastore".into(), "other".into())]), + ); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .unwrap() + .is_none()); + } + + #[test] + fn test_submatcher_recursion_direct() { + let mut all_matchers = HashMap::new(); + + let config = MatcherConfig { + name: "sub-1".to_string(), + nested: Some(true), + eval_matcher: vec!["sub-1".into()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let config = MatcherConfig { + name: "top".to_string(), + eval_matcher: vec!["sub-1".into()], + mode: Some(MatchModeOperator::Any), + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let notification = + Notification::from_template(Severity::Notice, "test", Value::Null, HashMap::new()); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .is_err()); + } + + #[test] + fn test_submatcher_recursion_indirect() { + let mut all_matchers = HashMap::new(); + + let config = MatcherConfig { + name: "sub-1".to_string(), + nested: Some(true), + eval_matcher: vec!["sub-2".into()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let config = MatcherConfig { + name: "sub-2".to_string(), + nested: Some(true), + eval_matcher: vec!["sub-1".into()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let config = MatcherConfig { + name: "top".to_string(), + eval_matcher: vec!["sub-1".into()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let notification = + Notification::from_template(Severity::Notice, "test", Value::Null, HashMap::new()); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .is_err()); + } + + #[test] + fn test_submatcher_does_not_exist() { + let mut all_matchers = HashMap::new(); + + let config = MatcherConfig { + name: "top".to_string(), + eval_matcher: vec!["doesntexist".into()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let notification = + Notification::from_template(Severity::Notice, "test", Value::Null, HashMap::new()); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .is_err()); + } + + #[test] + fn test_submatcher_does_not_declared_as_submatcher() { + let mut all_matchers = HashMap::new(); + + let config = MatcherConfig { + name: "sub-1".to_string(), + nested: Some(true), + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let config = MatcherConfig { + name: "top".to_string(), + eval_matcher: vec!["doesntexist".into()], + ..Default::default() + }; + all_matchers.insert(config.name.clone(), config.clone()); + + let notification = + Notification::from_template(Severity::Notice, "test", Value::Null, HashMap::new()); + + assert!(config + .matches(¬ification, &all_matchers, &mut HashMap::new()) + .is_err()); + } } -- 2.39.5 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel