Re: [PATCH proxmox 14/29] notify: migrate PBS's and PVE's default matcher to expression syntax

["Lukas Wagner" <l.wagner@proxmox.com>]

On Fri Jul 24, 2026 at 8:47 AM CEST, Arthur Bied-Charreton wrote:
> On Thu, Jul 09, 2026 at 01:57:01PM +0200, Lukas Wagner wrote:
>> Instead of providing the default matcher expression as a JSON blob, we
>> programmatically construct the appropricate SectionConfigData entity.
>> This required a change of the Context trait, now the default_config
>> method returns &'static SectionConfigData instead of &'static str.
>> 
>> The actual SectionConfigData entity is only constructed once and then
>> stored in a OnceLock.
>> 
>> A nice side-effect of this change is that the config-parsing code path
>> is more efficient, since we don't have to deserialize the default config
>> over and over again.
>> 
> one comment inline 
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>>  proxmox-notify/src/context/mod.rs      |  4 +-
>>  proxmox-notify/src/context/pbs.rs      | 87 ++++++++++++++++++++------
>>  proxmox-notify/src/context/pve.rs      | 67 +++++++++++++++-----
>>  proxmox-notify/src/context/test.rs     |  9 ++-
>>  proxmox-notify/src/lib.rs              |  6 +-
>>  proxmox-notify/src/matcher/calendar.rs |  9 ++-
>>  proxmox-notify/src/matcher/field.rs    | 11 +++-
>>  proxmox-notify/src/matcher/severity.rs | 10 ++-
>>  8 files changed, 157 insertions(+), 46 deletions(-)
>> 
>> diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs
>> index 87a2a716..70c3a567 100644
>> --- a/proxmox-notify/src/context/mod.rs
>> +++ b/proxmox-notify/src/context/mod.rs
>> @@ -1,6 +1,8 @@
>>  use std::fmt::Debug;
>>  use std::sync::Mutex;
>>  
>> +use proxmox_section_config::SectionConfigData;
>> +
>>  use crate::Error;
>>  use crate::renderer::TemplateSource;
>>  
>> @@ -24,7 +26,7 @@ pub trait Context: Send + Sync + Debug {
>>      /// Proxy configuration for the current node
>>      fn http_proxy_config(&self) -> Option<String>;
>>      /// Return default config for built-in targets/matchers.
>> -    fn default_config(&self) -> &'static str;
>> +    fn default_config(&self) -> &'static SectionConfigData;
>>      /// Return the path of `filename` from `source` and a certain (optional) `namespace`
>>      fn lookup_template(
>>          &self,
>> diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
>> index a9121548..b2561ad1 100644
>> --- a/proxmox-notify/src/context/pbs.rs
>> +++ b/proxmox-notify/src/context/pbs.rs
>> @@ -1,14 +1,19 @@
>>  use std::path::Path;
>> +use std::sync::OnceLock;
>>  
>>  use serde::Deserialize;
>>  use tracing::error;
>>  
>>  use proxmox_schema::{ObjectSchema, Schema, StringSchema};
>> -use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
>> +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
>>  
>> -use crate::Error;
>>  use crate::context::{Context, common};
>> +use crate::endpoints::sendmail::{SENDMAIL_TYPENAME, SendmailConfig};
>> +use crate::matcher::field::FieldMatcher;
>> +use crate::matcher::severity::SeverityMatcher;
>> +use crate::matcher::{MATCHER_TYPENAME, MatcherConfig};
>>  use crate::renderer::TemplateSource;
>> +use crate::{Error, Severity};
>>  
>>  const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
>>  const PBS_NODE_CFG_FILENAME: &str = "/etc/proxmox-backup/node.cfg";
>> @@ -60,21 +65,6 @@ fn lookup_mail_address(content: &str, username: &str) -> Option<String> {
>>      }
>>  }
>>  
>> -const DEFAULT_CONFIG: &str = "\
>> -sendmail: mail-to-root
>> -    comment Send mails to root@pam's email address
>> -    mailto-user root@pam
>> -
>> -
>> -matcher: default-matcher
>> -    mode all
>> -    invert-match true
>> -    match-field exact:type=prune
>> -    match-severity info
>> -    target mail-to-root
>> -    comment Route everything but successful prune job notifications to mail-to-root
>> -";
>> -
>>  #[derive(Debug)]
>>  pub struct PBSContext;
>>  
>> @@ -102,8 +92,67 @@ impl Context for PBSContext {
>>          content.and_then(|content| common::lookup_datacenter_config_key(&content, "http-proxy"))
>>      }
>>  
>> -    fn default_config(&self) -> &'static str {
>> -        DEFAULT_CONFIG
>> +    fn default_config(&self) -> &'static SectionConfigData {
>> +        static DEFAULT_CONFIG: OnceLock<SectionConfigData> = OnceLock::new();
>> +        DEFAULT_CONFIG.get_or_init(|| {
>> +            // FIXME: Long-term we want to move the trait implementation to the product,
>> +            // maybe add some nice builder to construct the default config.
>> +            // Moving this as-is to the product would expose a lot of internals
>> +            // to the product.
>> +
>> +            let mut config = SectionConfigData::default();
>> +            config
>> +                .set_data(
>> +                    "mail-to-root",
>> +                    SENDMAIL_TYPENAME,
>> +                    SendmailConfig {
>> +                        name: "mail-to-root".into(),
>> +                        mailto_user: vec!["root@pam".into()],
>> +                        comment: Some("Send mails to root@pam's email address".into()),
>> +                        ..Default::default()
>> +                    },
>> +                )
>> +                .expect("failed to set 'mail-to-root' in default config");
>> +
>> +            use proxmox_match_expression::{not, any_of, all_of};
>> +
>> +            let expression = any_of![
>> +                not!(
>> +                    FieldMatcher::Exact {
>> +                        field: "type".into(),
>> +                        values: vec!["prune".into()],
>> +                    }.into()
>> +                ),
>> +                all_of![
>> +                    FieldMatcher::Exact {
>> +                        field: "type".into(),
>> +                        values: vec!["prune".into()],
>> +                    }.into(),
>> +                    SeverityMatcher {
>> +                        severities: vec![Severity::Error, Severity::Warning],
>> +                    }.into(),
>> +                ],
>> +            ];
>> +
>> +            let expression = serde_json::to_string(&expression)
>> +                .expect("failed serialize expression for 'default-matcher'");
>> +
>> +            config
>> +                .set_data(
>> +                    "default-matcher",
>> +                    MATCHER_TYPENAME,
>> +                    MatcherConfig {
>> +                        name: "default-matcher".into(),
>> +                        expression: Some(expression),
>> +                        target: vec!["mail-to-root".into()],
>> +                        comment: Some("Route everything but successful prune job notifications to mail-to-root".into()),
>> +                        ..Default::default()
>> +                    },
>> +                )
>> +                .expect("failed to set 'default-matcher' in default config");
>> +
>> +            config
>> +        })
>>      }
> afaict the default changes from 'suppress prune info notifications'
> to 'for prune notifications, only notify on error or warning'. not
> very familiar with the notifications sent by PBS, i assume prune jobs
> never send notice-level notifications? 

Indeed, we actually don't use 'notice' at all at the moment. I think
including 'notice' in the default policy is actually the right thing to
do, even this is a change from the previous behavior. Now, as I said,
this does not change anything for any existing notifications.

I will document the slight change in the commit message!
>>  
>>      fn lookup_template(
> [...]