[pbs-devel] [PATCH proxmox-backup v3 09/10] notifications: add type for verify notification template data

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

This commit adds a separate type for the data passed to this type of
notification template. Also we make sure that we do not expose any
non-primitive types to the template renderer, any data
needed in the template is mapped into the new dedicated
template data type.
This ensures that any changes in types defined in other places
do not leak into the template rendering process by accident.

This commit also tries to unify the style and naming of template
variables.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/server/notifications/mod.rs              | 73 +++++++++-----------
 src/server/notifications/template_data.rs    | 28 ++++++++
 templates/default/verify-err-body.txt.hbs    |  8 +--
 templates/default/verify-err-subject.txt.hbs |  2 +-
 templates/default/verify-ok-body.txt.hbs     |  6 +-
 templates/default/verify-ok-subject.txt.hbs  |  2 +-
 6 files changed, 71 insertions(+), 48 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 04790126..801e65ec 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -5,7 +5,6 @@ use std::time::{Duration, Instant};
 use anyhow::Error;
 use const_format::concatcp;
 use nix::unistd::Uid;
-use serde_json::json;
 
 use proxmox_notify::context::pbs::PBS_CONTEXT;
 use proxmox_schema::ApiType;
@@ -27,6 +26,7 @@ use template_data::{
     AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData,
     PackageUpdatesTemplateData, PruneErrTemplateData, PruneOkTemplateData, SyncErrTemplateData,
     SyncOkTemplateData, TapeBackupErrTemplateData, TapeBackupOkTemplateData, TapeLoadTemplateData,
+    VerifyErrTemplateData, VerifyOkTemplateData,
 };
 
 /// Initialize the notification system by setting context in proxmox_notify
@@ -206,25 +206,6 @@ pub fn send_verify_status(
     job: VerificationJobConfig,
     result: &Result<Vec<String>, Error>,
 ) -> Result<(), Error> {
-    let (fqdn, port) = get_server_url();
-    let mut data = json!({
-        "job": job,
-        "fqdn": fqdn,
-        "port": port,
-    });
-
-    let (template, severity) = match result {
-        Ok(errors) if errors.is_empty() => ("verify-ok", Severity::Info),
-        Ok(errors) => {
-            data["errors"] = json!(errors);
-            ("verify-err", Severity::Error)
-        }
-        Err(_) => {
-            // aborted job - do not send any notification
-            return Ok(());
-        }
-    };
-
     let metadata = HashMap::from([
         ("job-id".into(), job.id.clone()),
         ("datastore".into(), job.store.clone()),
@@ -232,7 +213,39 @@ pub fn send_verify_status(
         ("type".into(), "verify".into()),
     ]);
 
-    let notification = Notification::from_template(severity, template, data, metadata);
+    let notification = match result {
+        Err(_) => {
+            // aborted job - do not send any notification
+            return Ok(());
+        }
+        Ok(errors) if errors.is_empty() => {
+            let template_data = VerifyOkTemplateData {
+                common: CommonData::new(),
+                datastore: job.store.clone(),
+                job_id: job.id.clone(),
+            };
+            Notification::from_template(
+                Severity::Info,
+                "verify-ok",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+        Ok(errors) => {
+            let template_data = VerifyErrTemplateData {
+                common: CommonData::new(),
+                datastore: job.store.clone(),
+                job_id: job.id.clone(),
+                failed_snapshot_list: errors.clone(),
+            };
+            Notification::from_template(
+                Severity::Error,
+                "verify-err",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+    };
 
     let (email, notify, mode) = lookup_datastore_notify_settings(&job.store);
     match mode {
@@ -509,24 +522,6 @@ pub fn send_load_media_notification(
     Ok(())
 }
 
-fn get_server_url() -> (String, usize) {
-    // user will surely request that they can change this
-
-    let nodename = proxmox_sys::nodename();
-    let mut fqdn = nodename.to_owned();
-
-    if let Ok(resolv_conf) = crate::api2::node::dns::read_etc_resolv_conf() {
-        if let Some(search) = resolv_conf["search"].as_str() {
-            fqdn.push('.');
-            fqdn.push_str(search);
-        }
-    }
-
-    let port = 8007;
-
-    (fqdn, port)
-}
-
 pub fn send_updates_available(updates: &[&APTUpdateInfo]) -> Result<(), Error> {
     let hostname = proxmox_sys::nodename().to_string();
 
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index a585401c..319aad10 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -314,3 +314,31 @@ pub struct TapeLoadTemplateData {
     /// The label of the tape.
     pub tape_label: String,
 }
+
+/// Template data for the verify-ok template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct VerifyOkTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The ID of the job.
+    pub job_id: String,
+}
+
+/// Template data for the verify-err template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct VerifyErrTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The ID of the job.
+    pub job_id: String,
+    /// The list of snapshots that failed to verify.
+    pub failed_snapshot_list: Vec<String>,
+}
diff --git a/templates/default/verify-err-body.txt.hbs b/templates/default/verify-err-body.txt.hbs
index d07b5ce0..96922eee 100644
--- a/templates/default/verify-err-body.txt.hbs
+++ b/templates/default/verify-err-body.txt.hbs
@@ -1,14 +1,14 @@
 
-Job ID:    {{job.id}}
-Datastore: {{job.store}}
+Job ID:    {{job-id}}
+Datastore: {{datastore}}
 
 Verification failed on these snapshots/groups:
 
-{{#each errors}}
+{{#each failed-snapshot-list}}
     {{this~}}
 {{/each}}
 
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#pbsServerAdministration:tasks>
+<{{base-url}}/#pbsServerAdministration:tasks>
diff --git a/templates/default/verify-err-subject.txt.hbs b/templates/default/verify-err-subject.txt.hbs
index 00a2d07f..f5b3f620 100644
--- a/templates/default/verify-err-subject.txt.hbs
+++ b/templates/default/verify-err-subject.txt.hbs
@@ -1 +1 @@
-Verify Datastore '{{ job.store }}' failed
+Verify Datastore '{{datastore}}' failed
diff --git a/templates/default/verify-ok-body.txt.hbs b/templates/default/verify-ok-body.txt.hbs
index 7560582e..fe776515 100644
--- a/templates/default/verify-ok-body.txt.hbs
+++ b/templates/default/verify-ok-body.txt.hbs
@@ -1,10 +1,10 @@
 
-Job ID:    {{job.id}}
-Datastore: {{job.store}}
+Job ID:    {{job-id}}
+Datastore: {{datastore}}
 
 Verification successful.
 
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#DataStore-{{job.store}}>
+<{{base-url}}/#DataStore-{{datastore}}>
diff --git a/templates/default/verify-ok-subject.txt.hbs b/templates/default/verify-ok-subject.txt.hbs
index 6020874c..0e4c17da 100644
--- a/templates/default/verify-ok-subject.txt.hbs
+++ b/templates/default/verify-ok-subject.txt.hbs
@@ -1 +1 @@
-Verify Datastore '{{ job.store }}' successful
+Verify Datastore '{{datastore}}' successful
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel