From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id E37BE9F978 for ; Tue, 7 Nov 2023 11:19:04 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BDCB730204 for ; Tue, 7 Nov 2023 11:19:04 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 7 Nov 2023 11:19:03 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id F11A046986 for ; Tue, 7 Nov 2023 11:19:02 +0100 (CET) From: Lukas Wagner To: pve-devel@lists.proxmox.com Date: Tue, 7 Nov 2023 11:18:02 +0100 Message-Id: <20231107101827.340100-3-l.wagner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20231107101827.340100-1-l.wagner@proxmox.com> References: <20231107101827.340100-1-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.017 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pve-devel] [PATCH proxmox 02/27] notify: factor out notification content into its own type 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: , X-List-Received-Date: Tue, 07 Nov 2023 10:19:05 -0000 This will be useful later for system mail forwarding, where the content of the mail should be forwarded unchanged. This moves notification properties into this new type and calls them 'data'. They will exclusively used for template rendering. `Notification` will receive a separate field for metadata, which will be useful for notification filtering. This decouples template rendering and filtering, which enables us to be very precise about which metadata fields we allow to be used in filters. Signed-off-by: Lukas Wagner --- proxmox-notify/examples/render.rs | 4 +- proxmox-notify/src/endpoints/gotify.rs | 26 +++++--- proxmox-notify/src/endpoints/sendmail.rs | 62 +++++++++--------- proxmox-notify/src/filter.rs | 10 +-- proxmox-notify/src/lib.rs | 81 ++++++++++++++---------- proxmox-notify/src/renderer/mod.rs | 15 ++--- 6 files changed, 109 insertions(+), 89 deletions(-) diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs index c0a6f27..d705fd0 100644 --- a/proxmox-notify/examples/render.rs +++ b/proxmox-notify/examples/render.rs @@ -53,10 +53,10 @@ fn main() -> Result<(), Error> { } }); - let output = render_template(TemplateRenderer::Html, TEMPLATE, Some(&properties))?; + let output = render_template(TemplateRenderer::Html, TEMPLATE, &properties)?; println!("{output}"); - let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, Some(&properties))?; + let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, &properties)?; println!("{output}"); Ok(()) diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs index 83df41f..af86f9c 100644 --- a/proxmox-notify/src/endpoints/gotify.rs +++ b/proxmox-notify/src/endpoints/gotify.rs @@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater}; use crate::context::context; use crate::renderer::TemplateRenderer; use crate::schema::ENTITY_NAME_SCHEMA; -use crate::{renderer, Endpoint, Error, Notification, Severity}; +use crate::{renderer, Content, Endpoint, Error, Notification, Severity}; fn severity_to_priority(level: Severity) -> u32 { match level { @@ -85,15 +85,21 @@ pub enum DeleteableGotifyProperty { impl Endpoint for GotifyEndpoint { fn send(&self, notification: &Notification) -> Result<(), Error> { - let properties = notification.properties.as_ref(); - - let title = renderer::render_template( - TemplateRenderer::Plaintext, - ¬ification.title, - properties, - )?; - let message = - renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?; + + let (title, message) = match ¬ification.content { + Content::Template { + title_template, + body_template, + data + } => { + let rendered_title = + renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?; + let rendered_message = + renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?; + + (rendered_title, rendered_message) + } + }; // We don't have a TemplateRenderer::Markdown yet, so simply put everything // in code tags. Otherwise tables etc. are not formatted properly diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs index 26e2a17..c540925 100644 --- a/proxmox-notify/src/endpoints/sendmail.rs +++ b/proxmox-notify/src/endpoints/sendmail.rs @@ -8,7 +8,7 @@ use proxmox_schema::{api, Updater}; use crate::context::context; use crate::renderer::TemplateRenderer; use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA}; -use crate::{renderer, Endpoint, Error, Notification}; +use crate::{renderer, Content, Endpoint, Error, Notification}; pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail"; @@ -102,41 +102,43 @@ impl Endpoint for SendmailEndpoint { } } - let properties = notification.properties.as_ref(); - - let subject = renderer::render_template( - TemplateRenderer::Plaintext, - ¬ification.title, - properties, - )?; - let html_part = - renderer::render_template(TemplateRenderer::Html, ¬ification.body, properties)?; - let text_part = - renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?; - - let author = self - .config - .author - .clone() - .unwrap_or_else(|| context().default_sendmail_author()); - + let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect(); let mailfrom = self .config .from_address .clone() .unwrap_or_else(|| context().default_sendmail_from()); - let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect(); - - proxmox_sys::email::sendmail( - &recipients_str, - &subject, - Some(&text_part), - Some(&html_part), - Some(&mailfrom), - Some(&author), - ) - .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into())) + match ¬ification.content { + Content::Template { + title_template, + body_template, + data, + } => { + let subject = + renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?; + let html_part = + renderer::render_template(TemplateRenderer::Html, body_template, data)?; + let text_part = + renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?; + + let author = self + .config + .author + .clone() + .unwrap_or_else(|| context().default_sendmail_author()); + + proxmox_sys::email::sendmail( + &recipients_str, + &subject, + Some(&text_part), + Some(&html_part), + Some(&mailfrom), + Some(&author), + ) + .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into())) + } + } } fn name(&self) -> &str { diff --git a/proxmox-notify/src/filter.rs b/proxmox-notify/src/filter.rs index 748ec4e..e014a59 100644 --- a/proxmox-notify/src/filter.rs +++ b/proxmox-notify/src/filter.rs @@ -160,7 +160,7 @@ impl<'a> FilterMatcher<'a> { #[cfg(test)] mod tests { use super::*; - use crate::config; + use crate::{config, Content}; fn parse_filters(config: &str) -> Result, Error> { let (config, _) = config::config(config)?; @@ -169,10 +169,12 @@ mod tests { fn empty_notification_with_severity(severity: Severity) -> Notification { Notification { - title: String::new(), - body: String::new(), + content: Content::Template { + title_template: String::new(), + body_template: String::new(), + data: Default::default(), + }, severity, - properties: Default::default(), } } diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs index f7d480c..d40d017 100644 --- a/proxmox-notify/src/lib.rs +++ b/proxmox-notify/src/lib.rs @@ -116,17 +116,44 @@ pub trait Endpoint { fn filter(&self) -> Option<&str>; } +#[derive(Debug, Clone)] +pub enum Content { + /// Title and body will be rendered as a template + Template { + /// Template for the notification title. + title_template: String, + /// Template for the notification body. + body_template: String, + /// Data that can be used for template rendering. + data: Value, + }, +} + #[derive(Debug, Clone)] /// Notification which can be sent pub struct Notification { /// Notification severity - pub severity: Severity, - /// The title of the notification - pub title: String, - /// Notification text - pub body: String, - /// Additional metadata for the notification - pub properties: Option, + severity: Severity, + /// Notification content + content: Content, +} + +impl Notification { + pub fn new_templated>( + severity: Severity, + title: S, + body: S, + properties: Value, + ) -> Self { + Self { + severity, + content: Content::Template { + title_template: title.as_ref().to_string(), + body_template: body.as_ref().to_string(), + data: properties, + }, + } + } } /// Notification configuration @@ -384,9 +411,11 @@ impl Bus { pub fn test_target(&self, target: &str) -> Result<(), Error> { let notification = Notification { severity: Severity::Info, - title: "Test notification".into(), - body: "This is a test of the notification target '{{ target }}'".into(), - properties: Some(json!({ "target": target })), + content: Content::Template { + title_template: "Test notification".into(), + body_template: "This is a test of the notification target '{{ target }}'".into(), + data: json!({ "target": target }), + }, }; let mut errors: Vec> = Vec::new(); @@ -473,12 +502,7 @@ mod tests { // Send directly to endpoint bus.send( "endpoint", - &Notification { - title: "Title".into(), - body: "Body".into(), - severity: Severity::Info, - properties: Default::default(), - }, + &Notification::new_templated(Severity::Info, "Title", "Body", Default::default()), ); let messages = mock.messages(); assert_eq!(messages.len(), 1); @@ -511,15 +535,9 @@ mod tests { bus.add_endpoint(Box::new(endpoint2.clone())); let send_to_group = |channel| { - bus.send( - channel, - &Notification { - title: "Title".into(), - body: "Body".into(), - severity: Severity::Info, - properties: Default::default(), - }, - ) + let notification = + Notification::new_templated(Severity::Info, "Title", "Body", Default::default()); + bus.send(channel, ¬ification) }; send_to_group("group1"); @@ -579,15 +597,10 @@ mod tests { }); let send_with_severity = |severity| { - bus.send( - "channel1", - &Notification { - title: "Title".into(), - body: "Body".into(), - severity, - properties: Default::default(), - }, - ); + let notification = + Notification::new_templated(severity, "Title", "Body", Default::default()); + + bus.send("channel1", ¬ification); }; send_with_severity(Severity::Info); diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs index 24f14aa..e9f36e6 100644 --- a/proxmox-notify/src/renderer/mod.rs +++ b/proxmox-notify/src/renderer/mod.rs @@ -228,11 +228,9 @@ impl BlockRenderFunctions { fn render_template_impl( template: &str, - properties: Option<&Value>, + data: &Value, renderer: TemplateRenderer, ) -> Result { - let properties = properties.unwrap_or(&Value::Null); - let mut handlebars = Handlebars::new(); handlebars.register_escape_fn(renderer.escape_fn()); @@ -242,7 +240,7 @@ fn render_template_impl( ValueRenderFunction::register_helpers(&mut handlebars); let rendered_template = handlebars - .render_template(template, properties) + .render_template(template, data) .map_err(|err| Error::RenderError(err.into()))?; Ok(rendered_template) @@ -255,11 +253,11 @@ fn render_template_impl( pub fn render_template( renderer: TemplateRenderer, template: &str, - properties: Option<&Value>, + data: &Value, ) -> Result { let mut rendered_template = String::from(renderer.prefix()); - rendered_template.push_str(&render_template_impl(template, properties, renderer)?); + rendered_template.push_str(&render_template_impl(template, data, renderer)?); rendered_template.push_str(renderer.postfix()); Ok(rendered_template) @@ -314,7 +312,7 @@ mod tests { #[test] fn test_render_template() -> Result<(), Error> { - let properties = json!({ + let data = json!({ "dur": 12345, "size": 1024 * 15, @@ -370,8 +368,7 @@ val1 val2 val3 val4 "#; - let rendered_plaintext = - render_template(TemplateRenderer::Plaintext, template, Some(&properties))?; + let rendered_plaintext = render_template(TemplateRenderer::Plaintext, template, &data)?; // Let's not bother about testing the HTML output, too fragile. -- 2.39.2