fix #6143: notify: allow overriding notification templates

Previously, notification templates could be modified by the user, but
these were overwritten again with installing newer package versions of
pve-manager and proxmox-backup.

Now override templates can be created cluster-wide in the path
“/etc/{pve,proxmox-backup}/notification-templates/{namespace}”, which
are used with priority. The folder structure has to be created and
populated manually (e.g. /etc/pve/notification-templates/default).

If override templates are not existing or their rendering fails, the
vendor templates in
"/usr/share/{pve-manager,proxmox-backup}/templates/default/" are used.

Sequence: [override html -> vendor html ->] override txt -> vendor txt

An error is only returned if none of the template candidates could be
used. Using an override template gets not logged.

Signed-off-by: Alexander Zeidler <a.zeidler@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Link: https://lore.proxmox.com/20250321133341.151340-1-a.zeidler@proxmox.com

proxmox-notify/src/context/mod.rs

@@ -1,6 +1,7 @@
 use std::fmt::Debug;
 use std::sync::Mutex;
 
+use crate::renderer::TemplateSource;
 use crate::Error;
 
 #[cfg(any(feature = "pve-context", feature = "pbs-context"))]
@@ -24,11 +25,12 @@ pub trait Context: Send + Sync + Debug {
     fn http_proxy_config(&self) -> Option<String>;
     /// Return default config for built-in targets/matchers.
     fn default_config(&self) -> &'static str;
-    /// Lookup a template in a certain (optional) namespace
+    /// Return the path of `filename` from `source` and a certain (optional) `namespace`
     fn lookup_template(
         &self,
         filename: &str,
         namespace: Option<&str>,
+        source: TemplateSource,
     ) -> Result<Option<String>, Error>;
 }
 

proxmox-notify/src/context/pbs.rs

@@ -7,6 +7,7 @@ use proxmox_schema::{ObjectSchema, Schema, StringSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
 
 use crate::context::{common, Context};
+use crate::renderer::TemplateSource;
 use crate::Error;
 
 const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
@@ -109,8 +110,14 @@ impl Context for PBSContext {
         &self,
         filename: &str,
         namespace: Option<&str>,
+        source: TemplateSource,
     ) -> Result<Option<String>, Error> {
-        let path = Path::new("/usr/share/proxmox-backup/templates")
+        let path = match source {
+            TemplateSource::Vendor => "/usr/share/proxmox-backup/templates",
+            TemplateSource::Override => "/etc/proxmox-backup/notification-templates",
+        };
+
+        let path = Path::new(&path)
             .join(namespace.unwrap_or("default"))
             .join(filename);
 

proxmox-notify/src/context/pve.rs

@@ -1,4 +1,5 @@
 use crate::context::{common, Context};
+use crate::renderer::TemplateSource;
 use crate::Error;
 use std::path::Path;
 
@@ -58,10 +59,17 @@ impl Context for PVEContext {
         &self,
         filename: &str,
         namespace: Option<&str>,
+        source: TemplateSource,
     ) -> Result<Option<String>, Error> {
-        let path = Path::new("/usr/share/pve-manager/templates")
+        let path = match source {
+            TemplateSource::Vendor => "/usr/share/pve-manager/templates",
+            TemplateSource::Override => "/etc/pve/notification-templates",
+        };
+
+        let path = Path::new(&path)
             .join(namespace.unwrap_or("default"))
             .join(filename);
+
         let template_string = proxmox_sys::fs::file_read_optional_string(path)
             .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
         Ok(template_string)

proxmox-notify/src/context/test.rs

@@ -1,4 +1,5 @@
 use crate::context::Context;
+use crate::renderer::TemplateSource;
 use crate::Error;
 
 #[derive(Debug)]
@@ -29,6 +30,7 @@ impl Context for TestContext {
         &self,
         _filename: &str,
         _namespace: Option<&str>,
+        _source: TemplateSource,
     ) -> Result<Option<String>, Error> {
         Ok(Some(String::new()))
     }

proxmox-notify/src/renderer/mod.rs

@@ -1,6 +1,6 @@
 //! Module for rendering notification templates.
 
-use std::time::Duration;
+use std::{fmt::Display, time::Duration};
 
 use handlebars::{
     Context, Handlebars, Helper, HelperDef, HelperResult, Output, RenderContext,
@@ -190,11 +190,29 @@ impl ValueRenderFunction {
     }
 }
 
-/// Available template types
+/// Choose between the provided `vendor` template or its by the user optionally created `override`
 #[derive(Copy, Clone)]
+pub enum TemplateSource {
+    Vendor,
+    Override,
+}
+
+impl Display for TemplateSource {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            TemplateSource::Vendor => f.write_str("vendor"),
+            TemplateSource::Override => f.write_str("override"),
+        }
+    }
+}
+
+/// Available template types
+#[derive(Copy, Clone, PartialEq)]
 pub enum TemplateType {
     /// HTML body template
     HtmlBody,
+    /// Fallback HTML body, based on the `PlaintextBody` template
+    HtmlBodyFromPlaintext,
     /// Plaintext body template
     PlaintextBody,
     /// Subject template
@@ -205,15 +223,25 @@ impl TemplateType {
     fn file_suffix(&self) -> &'static str {
         match self {
             TemplateType::HtmlBody => "body.html.hbs",
+            TemplateType::HtmlBodyFromPlaintext => "body.txt.hbs",
             TemplateType::PlaintextBody => "body.txt.hbs",
             TemplateType::Subject => "subject.txt.hbs",
         }
     }
 
     fn postprocess(&self, mut rendered: String) -> String {
-        if let Self::Subject = self {
+        match self {
+            TemplateType::HtmlBodyFromPlaintext => {
+                rendered = format!(
+                    "<html><body><pre>{}</pre></body></html>",
+                    handlebars::html_escape(&rendered)
+                )
+            }
+            TemplateType::Subject => {
                 rendered = rendered.replace('\n', " ");
             }
+            _ => {}
+        }
 
         rendered
     }
@@ -221,6 +249,7 @@ impl TemplateType {
     fn block_render_fns(&self) -> BlockRenderFunctions {
         match self {
             TemplateType::HtmlBody => html::block_render_functions(),
+            TemplateType::HtmlBodyFromPlaintext => plaintext::block_render_functions(),
             TemplateType::Subject => plaintext::block_render_functions(),
             TemplateType::PlaintextBody => plaintext::block_render_functions(),
         }
@@ -231,6 +260,7 @@ impl TemplateType {
             TemplateType::PlaintextBody => handlebars::no_escape,
             TemplateType::Subject => handlebars::no_escape,
             TemplateType::HtmlBody => handlebars::html_escape,
+            TemplateType::HtmlBodyFromPlaintext => handlebars::no_escape,
         }
     }
 }
@@ -250,10 +280,14 @@ impl BlockRenderFunctions {
 }
 
 fn render_template_impl(
-    template: &str,
     data: &Value,
     renderer: TemplateType,
-) -> Result<String, Error> {
+    filename: &str,
+    source: TemplateSource,
+) -> Result<Option<String>, Error> {
+    let template_string = context::context().lookup_template(&filename, None, source)?;
+
+    if let Some(template_string) = template_string {
         let mut handlebars = Handlebars::new();
         handlebars.register_escape_fn(renderer.escape_fn());
 
@@ -268,52 +302,78 @@ fn render_template_impl(
         );
 
         let rendered_template = handlebars
-        .render_template(template, data)
+            .render_template(&template_string, data)
             .map_err(|err| Error::RenderError(err.into()))?;
 
-    Ok(rendered_template)
+        let rendered_template = renderer.postprocess(rendered_template);
+
+        Ok(Some(rendered_template))
+    } else {
+        Ok(None)
+    }
 }
 
 /// Render a template string.
 ///
-/// The output format can be chosen via the `renderer` parameter (see [TemplateType]
+/// The output format is chosen via the `ty` parameter (see [TemplateType] for
-/// for available options).
+/// available options). If an override template is found and renderable, it is
+/// used instead of the vendor one. If the [TemplateType] is `HtmlBody` but no
+/// HTML template is found or renderable, it falls back to use a plaintext
+/// template encapsulated in a pre-formatted HTML block (<pre>).
 pub fn render_template(
     mut ty: TemplateType,
     template: &str,
     data: &Value,
 ) -> Result<String, Error> {
+    let mut source = TemplateSource::Override;
+
+    loop {
         let filename = format!("{template}-{suffix}", suffix = ty.file_suffix());
+        let result = render_template_impl(data, ty, &filename, source);
 
-    let template_string = context::context().lookup_template(&filename, None)?;
+        match result {
+            Ok(Some(s)) => {
+                return Ok(s);
+            }
+            Ok(None) => {}
+            Err(err) => {
+                tracing::error!("failed to render {source} template '{filename}': {err}");
+            }
+        }
 
-    let (template_string, fallback) = match (template_string, ty) {
+        match (ty, source) {
-        (None, TemplateType::HtmlBody) => {
-            ty = TemplateType::PlaintextBody;
-            let plaintext_filename = format!("{template}-{suffix}", suffix = ty.file_suffix());
             (
-                context::context().lookup_template(&plaintext_filename, None)?,
+                TemplateType::HtmlBody
-                true,
+                | TemplateType::HtmlBodyFromPlaintext
-            )
+                | TemplateType::PlaintextBody
+                | TemplateType::Subject,
+                TemplateSource::Override,
+            ) => {
+                // Override template not found or renderable, try the vendor one instead
+                source = TemplateSource::Vendor;
+            }
+            (TemplateType::HtmlBody, TemplateSource::Vendor) => {
+                // Override and vendor HTML templates not found or renderable,
+                // try next the override plaintext as fallback
+                ty = TemplateType::HtmlBodyFromPlaintext;
+                source = TemplateSource::Override;
+            }
+            (
+                TemplateType::HtmlBodyFromPlaintext
+                | TemplateType::PlaintextBody
+                | TemplateType::Subject,
+                TemplateSource::Vendor,
+            ) => {
+                // Return error, no suitable templates found or renderable
+                break;
+            }
         }
-        (template_string, _) => (template_string, false),
-    };
-
-    let template_string = template_string.ok_or(Error::Generic(format!(
-        "could not load template '{template}'"
-    )))?;
-
-    let mut rendered = render_template_impl(&template_string, data, ty)?;
-    rendered = ty.postprocess(rendered);
-
-    if fallback {
-        rendered = format!(
-            "<html><body><pre>{}</pre></body></html>",
-            handlebars::html_escape(&rendered)
-        );
     }
 
-    Ok(rendered)
+    Err(Error::Generic(
+        "failed to render notification template, all template candidates are erroneous or missing"
+            .into(),
+    ))
 }
 
 #[cfg(test)]