From df4858e989a78c49d0f2024635e31e718e95ca32 Mon Sep 17 00:00:00 2001 From: Lukas Wagner Date: Tue, 14 Nov 2023 13:59:12 +0100 Subject: [PATCH] notify: factor out notification content into its own type 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 | 24 ++++--- proxmox-notify/src/endpoints/sendmail.rs | 60 +++++++++--------- proxmox-notify/src/filter.rs | 10 +-- proxmox-notify/src/lib.rs | 81 ++++++++++++++---------- proxmox-notify/src/renderer/mod.rs | 15 ++--- 6 files changed, 107 insertions(+), 87 deletions(-) diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs index c0a6f27b..d705fd0c 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 83df41f7..af86f9c5 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 26e2a175..c5409257 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(); + 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)?; - 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())) + 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 748ec4e8..e014a59c 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 f7d480cf..d40d017f 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 24f14aa5..e9f36e6d 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.