[pbs-devel] [PATCH proxmox 1/2] notify: matcher: allow to evaluate other matchers via `eval-matcher`

[Lukas Wagner <l.wagner@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(&notification).unwrap().is_some())
+            let mut all_matchers = HashMap::new();
+            all_matchers.insert("default".to_string(), config.clone());
+
+            assert!(config
+                .matches(&notification, &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(&notification, &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(&notification, &all_matchers, &mut HashMap::new())
+            .unwrap()
+            .is_some());
+
+        assert!(config
+            .matches(&notification, &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(&notification, &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(&notification, &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(&notification, &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(&notification, &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(&notification, &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