From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 403511FF380 for ; Fri, 19 Apr 2024 16:19:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 86AE7B4A8; Fri, 19 Apr 2024 16:18:28 +0200 (CEST) From: Lukas Wagner To: pve-devel@lists.proxmox.com Date: Fri, 19 Apr 2024 16:17:06 +0200 Message-Id: <20240419141723.377507-4-l.wagner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240419141723.377507-1-l.wagner@proxmox.com> References: <20240419141723.377507-1-l.wagner@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.002 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: [pve-devel] [PATCH proxmox v2 03/20] notify: convert Option> -> Vec in config structs X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" Suggested-by: Wolfgang Bumiller Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes Reviewed-by: Fiona Ebner --- proxmox-notify/src/api/matcher.rs | 27 +++++++-------- proxmox-notify/src/api/mod.rs | 22 +++++------- proxmox-notify/src/api/sendmail.rs | 22 ++++++------ proxmox-notify/src/api/smtp.rs | 24 ++++++------- proxmox-notify/src/endpoints/common/mail.rs | 20 ++++------- proxmox-notify/src/endpoints/sendmail.rs | 14 ++++---- proxmox-notify/src/endpoints/smtp.rs | 18 +++++----- proxmox-notify/src/lib.rs | 10 +++--- proxmox-notify/src/matcher.rs | 38 ++++++++++++--------- 9 files changed, 96 insertions(+), 99 deletions(-) diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs index f0eabd9..63ec73d 100644 --- a/proxmox-notify/src/api/matcher.rs +++ b/proxmox-notify/src/api/matcher.rs @@ -38,10 +38,7 @@ pub fn get_matcher(config: &Config, name: &str) -> Result Result<(), HttpError> { super::ensure_unique(config, &matcher_config.name)?; - - if let Some(targets) = matcher_config.target.as_deref() { - super::ensure_endpoints_exist(config, targets)?; - } + super::ensure_endpoints_exist(config, &matcher_config.target)?; config .config @@ -78,10 +75,10 @@ pub fn update_matcher( if let Some(delete) = delete { for deleteable_property in delete { match deleteable_property { - DeleteableMatcherProperty::MatchSeverity => matcher.match_severity = None, - DeleteableMatcherProperty::MatchField => matcher.match_field = None, - DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar = None, - DeleteableMatcherProperty::Target => matcher.target = None, + DeleteableMatcherProperty::MatchSeverity => matcher.match_severity.clear(), + DeleteableMatcherProperty::MatchField => matcher.match_field.clear(), + DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar.clear(), + DeleteableMatcherProperty::Target => matcher.target.clear(), DeleteableMatcherProperty::Mode => matcher.mode = None, DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None, DeleteableMatcherProperty::Comment => matcher.comment = None, @@ -91,15 +88,15 @@ pub fn update_matcher( } if let Some(match_severity) = matcher_updater.match_severity { - matcher.match_severity = Some(match_severity); + matcher.match_severity = match_severity; } if let Some(match_field) = matcher_updater.match_field { - matcher.match_field = Some(match_field); + matcher.match_field = match_field; } if let Some(match_calendar) = matcher_updater.match_calendar { - matcher.match_calendar = Some(match_calendar); + matcher.match_calendar = match_calendar; } if let Some(mode) = matcher_updater.mode { @@ -120,7 +117,7 @@ pub fn update_matcher( if let Some(target) = matcher_updater.target { super::ensure_endpoints_exist(config, target.as_slice())?; - matcher.target = Some(target); + matcher.target = target; } config @@ -244,9 +241,9 @@ matcher: matcher2 let matcher = get_matcher(&config, "matcher1")?; assert_eq!(matcher.invert_match, None); - assert!(matcher.match_severity.is_none()); - assert!(matches!(matcher.match_field, None)); - assert_eq!(matcher.target, None); + assert!(matcher.match_severity.is_empty()); + assert!(matcher.match_field.is_empty()); + assert!(matcher.target.is_empty()); assert!(matcher.mode.is_none()); assert_eq!(matcher.comment, None); diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs index bb0371d..a2cf29e 100644 --- a/proxmox-notify/src/api/mod.rs +++ b/proxmox-notify/src/api/mod.rs @@ -102,10 +102,8 @@ fn get_referrers(config: &Config, entity: &str) -> Result, HttpE let mut referrers = HashSet::new(); for matcher in matcher::get_matchers(config)? { - if let Some(targets) = matcher.target { - if targets.iter().any(|target| target == entity) { - referrers.insert(matcher.name.clone()); - } + if matcher.target.iter().any(|target| target == entity) { + referrers.insert(matcher.name.clone()); } } @@ -149,11 +147,9 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet { let mut new = HashSet::new(); for entity in entities { - if let Ok(group) = matcher::get_matcher(config, entity) { - if let Some(targets) = group.target { - for target in targets { - new.insert(target.clone()); - } + if let Ok(matcher) = matcher::get_matcher(config, entity) { + for target in matcher.target { + new.insert(target.clone()); } } } @@ -219,7 +215,7 @@ mod tests { &mut config, SendmailConfig { name: "sendmail".to_string(), - mailto: Some(vec!["foo@example.com".to_string()]), + mailto: vec!["foo@example.com".to_string()], ..Default::default() }, )?; @@ -228,7 +224,7 @@ mod tests { &mut config, SendmailConfig { name: "builtin".to_string(), - mailto: Some(vec!["foo@example.com".to_string()]), + mailto: vec!["foo@example.com".to_string()], origin: Some(Origin::Builtin), ..Default::default() }, @@ -251,11 +247,11 @@ mod tests { &mut config, MatcherConfig { name: "matcher".to_string(), - target: Some(vec![ + target: vec![ "sendmail".to_string(), "gotify".to_string(), "builtin".to_string(), - ]), + ], ..Default::default() }, )?; diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs index 3285ac7..c20a3e5 100644 --- a/proxmox-notify/src/api/sendmail.rs +++ b/proxmox-notify/src/api/sendmail.rs @@ -40,7 +40,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result Result<(), HttpError> { super::ensure_unique(config, &endpoint.name)?; - if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() { + if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() { http_bail!( BAD_REQUEST, "must at least provide one recipient, either in mailto or in mailto-user" @@ -83,19 +83,19 @@ pub fn update_endpoint( DeleteableSendmailProperty::FromAddress => endpoint.from_address = None, DeleteableSendmailProperty::Author => endpoint.author = None, DeleteableSendmailProperty::Comment => endpoint.comment = None, - DeleteableSendmailProperty::Mailto => endpoint.mailto = None, - DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user = None, + DeleteableSendmailProperty::Mailto => endpoint.mailto.clear(), + DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user.clear(), DeleteableSendmailProperty::Disable => endpoint.disable = None, } } } if let Some(mailto) = updater.mailto { - endpoint.mailto = Some(mailto); + endpoint.mailto = mailto; } if let Some(mailto_user) = updater.mailto_user { - endpoint.mailto_user = Some(mailto_user); + endpoint.mailto_user = mailto_user; } if let Some(from_address) = updater.from_address { @@ -114,7 +114,7 @@ pub fn update_endpoint( endpoint.disable = Some(disable); } - if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() { + if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() { http_bail!( BAD_REQUEST, "must at least provide one recipient, either in mailto or in mailto-user" @@ -164,8 +164,8 @@ pub mod tests { config, SendmailConfig { name: name.into(), - mailto: Some(vec!["user1@example.com".into()]), - mailto_user: None, + mailto: vec!["user1@example.com".into()], + mailto_user: vec![], from_address: Some("from@example.com".into()), author: Some("root".into()), comment: Some("Comment".into()), @@ -248,12 +248,12 @@ pub mod tests { assert_eq!( endpoint.mailto, - Some(vec![ + vec![ "user2@example.com".to_string(), "user3@example.com".to_string() - ]) + ] ); - assert_eq!(endpoint.mailto_user, Some(vec!["root@pam".to_string(),])); + assert_eq!(endpoint.mailto_user, vec!["root@pam".to_string(),]); assert_eq!(endpoint.from_address, Some("root@example.com".to_string())); assert_eq!(endpoint.author, Some("newauthor".to_string())); assert_eq!(endpoint.comment, Some("new comment".to_string())); diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs index 16d103c..7a58677 100644 --- a/proxmox-notify/src/api/smtp.rs +++ b/proxmox-notify/src/api/smtp.rs @@ -50,7 +50,7 @@ pub fn add_endpoint( super::ensure_unique(config, &endpoint_config.name)?; - if endpoint_config.mailto.is_none() && endpoint_config.mailto_user.is_none() { + if endpoint_config.mailto.is_empty() && endpoint_config.mailto_user.is_empty() { http_bail!( BAD_REQUEST, "must at least provide one recipient, either in mailto or in mailto-user" @@ -101,8 +101,8 @@ pub fn update_endpoint( DeleteableSmtpProperty::Author => endpoint.author = None, DeleteableSmtpProperty::Comment => endpoint.comment = None, DeleteableSmtpProperty::Disable => endpoint.disable = None, - DeleteableSmtpProperty::Mailto => endpoint.mailto = None, - DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user = None, + DeleteableSmtpProperty::Mailto => endpoint.mailto.clear(), + DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user.clear(), DeleteableSmtpProperty::Password => super::set_private_config_entry( config, SmtpPrivateConfig { @@ -119,10 +119,10 @@ pub fn update_endpoint( } if let Some(mailto) = updater.mailto { - endpoint.mailto = Some(mailto); + endpoint.mailto = mailto; } if let Some(mailto_user) = updater.mailto_user { - endpoint.mailto_user = Some(mailto_user); + endpoint.mailto_user = mailto_user; } if let Some(from_address) = updater.from_address { endpoint.from_address = from_address; @@ -163,7 +163,7 @@ pub fn update_endpoint( endpoint.disable = Some(disable); } - if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() { + if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() { http_bail!( BAD_REQUEST, "must at least provide one recipient, either in mailto or in mailto-user" @@ -211,8 +211,8 @@ pub mod tests { config, SmtpConfig { name: name.into(), - mailto: Some(vec!["user1@example.com".into()]), - mailto_user: None, + mailto: vec!["user1@example.com".into()], + mailto_user: vec![], from_address: "from@example.com".into(), author: Some("root".into()), comment: Some("Comment".into()), @@ -311,12 +311,12 @@ pub mod tests { assert_eq!( endpoint.mailto, - Some(vec![ + vec![ "user2@example.com".to_string(), "user3@example.com".to_string() - ]) + ] ); - assert_eq!(endpoint.mailto_user, Some(vec!["root@pam".to_string(),])); + assert_eq!(endpoint.mailto_user, vec!["root@pam".to_string(),]); assert_eq!(endpoint.from_address, "root@example.com".to_string()); assert_eq!(endpoint.author, Some("newauthor".to_string())); assert_eq!(endpoint.comment, Some("new comment".to_string())); @@ -343,7 +343,7 @@ pub mod tests { assert_eq!(endpoint.comment, None); assert_eq!(endpoint.port, None); assert_eq!(endpoint.username, None); - assert_eq!(endpoint.mailto_user, None); + assert!(endpoint.mailto_user.is_empty()); Ok(()) } diff --git a/proxmox-notify/src/endpoints/common/mail.rs b/proxmox-notify/src/endpoints/common/mail.rs index 0929d7c..b1c4823 100644 --- a/proxmox-notify/src/endpoints/common/mail.rs +++ b/proxmox-notify/src/endpoints/common/mail.rs @@ -2,22 +2,16 @@ use std::collections::HashSet; use crate::context; -pub(crate) fn get_recipients( - email_addrs: Option<&[String]>, - users: Option<&[String]>, -) -> HashSet { +pub(crate) fn get_recipients(email_addrs: &[String], users: &[String]) -> HashSet { let mut recipients = HashSet::new(); - if let Some(mailto_addrs) = email_addrs { - for addr in mailto_addrs { - recipients.insert(addr.clone()); - } + for addr in email_addrs { + recipients.insert(addr.clone()); } - if let Some(users) = users { - for user in users { - if let Some(addr) = context::context().lookup_email_for_user(user) { - recipients.insert(addr); - } + + for user in users { + if let Some(addr) = context::context().lookup_email_for_user(user) { + recipients.insert(addr); } } recipients diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs index 6178b5e..fa04002 100644 --- a/proxmox-notify/src/endpoints/sendmail.rs +++ b/proxmox-notify/src/endpoints/sendmail.rs @@ -44,11 +44,13 @@ pub struct SendmailConfig { #[updater(skip)] pub name: String, /// Mail recipients - #[serde(skip_serializing_if = "Option::is_none")] - pub mailto: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub mailto: Vec, /// Mail recipients - #[serde(skip_serializing_if = "Option::is_none")] - pub mailto_user: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub mailto_user: Vec, /// `From` address for the mail #[serde(skip_serializing_if = "Option::is_none")] pub from_address: Option, @@ -90,8 +92,8 @@ pub struct SendmailEndpoint { impl Endpoint for SendmailEndpoint { fn send(&self, notification: &Notification) -> Result<(), Error> { let recipients = mail::get_recipients( - self.config.mailto.as_deref(), - self.config.mailto_user.as_deref(), + self.config.mailto.as_slice(), + self.config.mailto_user.as_slice(), ); let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect(); diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs index f0c836a..ded5baf 100644 --- a/proxmox-notify/src/endpoints/smtp.rs +++ b/proxmox-notify/src/endpoints/smtp.rs @@ -44,8 +44,8 @@ pub enum SmtpMode { }, mailto: { type: Array, - items: { - schema: EMAIL_SCHEMA, + items: { + schema: EMAIL_SCHEMA, }, optional: true, }, @@ -80,11 +80,13 @@ pub struct SmtpConfig { #[serde(skip_serializing_if = "Option::is_none")] pub username: Option, /// Mail recipients - #[serde(skip_serializing_if = "Option::is_none")] - pub mailto: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub mailto: Vec, /// Mail recipients - #[serde(skip_serializing_if = "Option::is_none")] - pub mailto_user: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub mailto_user: Vec, /// `From` address for the mail pub from_address: String, /// Author of the mail @@ -177,8 +179,8 @@ impl Endpoint for SmtpEndpoint { let transport = transport_builder.build(); let recipients = mail::get_recipients( - self.config.mailto.as_deref(), - self.config.mailto_user.as_deref(), + self.config.mailto.as_slice(), + self.config.mailto_user.as_slice(), ); let mail_from = self.config.from_address.clone(); diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index 9f8683f..35dcb17 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -611,7 +611,7 @@ mod tests { bus.add_endpoint(Box::new(mock.clone())); let matcher = MatcherConfig { - target: Some(vec!["endpoint".into()]), + target: vec!["endpoint".into()], ..Default::default() }; @@ -642,15 +642,15 @@ mod tests { bus.add_matcher(MatcherConfig { name: "matcher1".into(), - match_severity: Some(vec!["warning,error".parse()?]), - target: Some(vec!["mock1".into()]), + match_severity: vec!["warning,error".parse()?], + target: vec!["mock1".into()], ..Default::default() }); bus.add_matcher(MatcherConfig { name: "matcher2".into(), - match_severity: Some(vec!["error".parse()?]), - target: Some(vec!["mock2".into()]), + match_severity: vec!["error".parse()?], + target: vec!["mock2".into()], ..Default::default() }); diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs index d6051ac..b387fef 100644 --- a/proxmox-notify/src/matcher.rs +++ b/proxmox-notify/src/matcher.rs @@ -113,17 +113,19 @@ pub struct MatcherConfig { pub name: String, /// List of matched metadata fields - #[serde(skip_serializing_if = "Option::is_none")] - pub match_field: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub match_field: Vec, /// List of matched severity levels - #[serde(skip_serializing_if = "Option::is_none")] - pub match_severity: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub match_severity: Vec, /// List of matched severity levels - #[serde(skip_serializing_if = "Option::is_none")] - pub match_calendar: Option>, - + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub match_calendar: Vec, /// Decide if 'all' or 'any' match statements must match #[serde(skip_serializing_if = "Option::is_none")] pub mode: Option, @@ -133,8 +135,9 @@ pub struct MatcherConfig { pub invert_match: Option, /// Targets to notify - #[serde(skip_serializing_if = "Option::is_none")] - pub target: Option>, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + #[updater(serde(skip_serializing_if = "Option::is_none"))] + pub target: Vec, /// Comment #[serde(skip_serializing_if = "Option::is_none")] @@ -284,29 +287,32 @@ impl MatcherConfig { // If there are no matching directives, the matcher will always match let mut no_matchers = true; - if let Some(severity_matchers) = self.match_severity.as_deref() { + if !self.match_severity.is_empty() { no_matchers = false; is_match = mode.apply( is_match, - self.check_matches(notification, severity_matchers)?, + self.check_matches(notification, &self.match_severity)?, ); } - if let Some(field_matchers) = self.match_field.as_deref() { + if !self.match_field.is_empty() { no_matchers = false; - is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?); + is_match = mode.apply( + is_match, + self.check_matches(notification, &self.match_field)?, + ); } - if let Some(calendar_matchers) = self.match_calendar.as_deref() { + if !self.match_calendar.is_empty() { no_matchers = false; is_match = mode.apply( is_match, - self.check_matches(notification, calendar_matchers)?, + self.check_matches(notification, &self.match_calendar)?, ); } let invert_match = self.invert_match.unwrap_or_default(); Ok(if is_match != invert_match || no_matchers { - Some(self.target.as_deref().unwrap_or_default()) + Some(&self.target) } else { None }) -- 2.39.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel