[pbs-devel] [PATCH proxmox-backup 07/10] notifications: add type for tape backup 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>
---
 src/server/notifications/mod.rs               | 66 ++++++++++++-------
 src/server/notifications/template_data.rs     | 48 ++++++++++++++
 .../default/tape-backup-err-body.txt.hbs      | 18 ++---
 .../default/tape-backup-err-subject.txt.hbs   |  6 +-
 templates/default/tape-backup-ok-body.txt.hbs | 20 +++---
 .../default/tape-backup-ok-subject.txt.hbs    |  6 +-
 6 files changed, 117 insertions(+), 47 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 3301c239..e8ce60a2 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -26,7 +26,7 @@ mod template_data;
 use template_data::{
     AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData,
     PackageUpdatesTemplateData, PruneErrTemplateData, PruneOkTemplateData, SyncErrTemplateData,
-    SyncOkTemplateData,
+    SyncOkTemplateData, TapeBackupErrTemplateData, TapeBackupOkTemplateData,
 };
 
 /// Initialize the notification system by setting context in proxmox_notify
@@ -389,26 +389,6 @@ pub fn send_tape_backup_status(
     result: &Result<(), Error>,
     summary: TapeBackupJobSummary,
 ) -> Result<(), Error> {
-    let (fqdn, port) = get_server_url();
-    let duration: proxmox_time::TimeSpan = summary.duration.into();
-    let mut data = json!({
-        "job": job,
-        "fqdn": fqdn,
-        "port": port,
-        "id": id,
-        "snapshot-list": summary.snapshot_list,
-        "used-tapes": summary.used_tapes,
-        "job-duration": duration.to_string(),
-    });
-
-    let (template, severity) = match result {
-        Ok(()) => ("tape-backup-ok", Severity::Info),
-        Err(err) => {
-            data["error"] = err.to_string().into();
-            ("tape-backup-err", Severity::Error)
-        }
-    };
-
     let mut metadata = HashMap::from([
         ("datastore".into(), job.store.clone()),
         ("media-pool".into(), job.pool.clone()),
@@ -420,7 +400,49 @@ pub fn send_tape_backup_status(
         metadata.insert("job-id".into(), id.into());
     }
 
-    let notification = Notification::from_template(severity, template, data, metadata);
+    let duration = summary.duration.as_secs();
+
+    let notification = match result {
+        Ok(()) => {
+            let template_data = TapeBackupOkTemplateData {
+                common: CommonData::new(),
+                datastore: job.store.clone(),
+                job_id: id.map(|id| id.into()),
+                job_duration: duration,
+                tape_pool: job.pool.clone(),
+                tape_drive: job.drive.clone(),
+                used_tapes_list: summary.used_tapes.unwrap_or_default(),
+                snapshot_list: summary.snapshot_list,
+            };
+
+            Notification::from_template(
+                Severity::Info,
+                "tape-backup-ok",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+        Err(err) => {
+            let template_data = TapeBackupErrTemplateData {
+                common: CommonData::new(),
+                datastore: job.store.clone(),
+                job_id: id.map(|id| id.into()),
+                job_duration: duration,
+                tape_pool: job.pool.clone(),
+                tape_drive: job.drive.clone(),
+                used_tapes_list: summary.used_tapes.unwrap_or_default(),
+                snapshot_list: summary.snapshot_list,
+                error: err.to_string(),
+            };
+
+            Notification::from_template(
+                Severity::Error,
+                "tape-backup-err",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+    };
 
     let mode = TapeNotificationMode::from(job);
 
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index 3df9ed17..e24a5ef3 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -244,3 +244,51 @@ pub struct SyncErrTemplateData {
     /// The error that occurred during the sync job.
     pub error: String,
 }
+
+/// Template data for the tape-backup-ok template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct TapeBackupOkTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore that was backed up.
+    pub datastore: String,
+    /// The optional job id for this tape backup job.
+    pub job_id: Option<String>,
+    /// The total duration of the backup job in seconds.
+    pub job_duration: u64,
+    /// The tape pool.
+    pub tape_pool: String,
+    /// The name of the tape drive.
+    pub tape_drive: String,
+    /// The list of tapes which were used in this backup job.
+    pub used_tapes_list: Vec<String>,
+    /// The list of snapshots which were backed up.
+    pub snapshot_list: Vec<String>,
+}
+
+/// Template data for the tape-backup-err template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct TapeBackupErrTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore that was backed up.
+    pub datastore: String,
+    /// The optional job id for this tape backup job.
+    pub job_id: Option<String>,
+    /// The total duration of the backup job in seconds.
+    pub job_duration: u64,
+    /// The tape pool.
+    pub tape_pool: String,
+    /// The name of the tape drive.
+    pub tape_drive: String,
+    /// The list of tapes which were used in this backup job.
+    pub used_tapes_list: Vec<String>,
+    /// The list of snapshots which were backed up.
+    pub snapshot_list: Vec<String>,
+    /// The error that happend during the backup job.
+    pub error: String,
+}
diff --git a/templates/default/tape-backup-err-body.txt.hbs b/templates/default/tape-backup-err-body.txt.hbs
index cc45c882..387b2438 100644
--- a/templates/default/tape-backup-err-body.txt.hbs
+++ b/templates/default/tape-backup-err-body.txt.hbs
@@ -1,20 +1,20 @@
-{{#if id ~}}
-Job ID:     {{id}}
+{{#if job-id~}}
+Job ID:     {{job-id}}
 {{/if~}}
-Datastore:  {{job.store}}
-Tape Pool:  {{job.pool}}
-Tape Drive: {{job.drive}}
+Datastore:  {{datastore}}
+Tape Pool:  {{tape-pool}}
+Tape Drive: {{tape-drive}}
 
-{{#if snapshot-list ~}}
+{{#if snapshot-list~}}
 Snapshots included:
 
 {{#each snapshot-list~}}
 {{this}}
 {{/each~}}
 {{/if}}
-{{#if used-tapes }}
+{{#if used-tapes-list}}
 Used Tapes:
-{{#each used-tapes~}}
+{{#each used-tapes-list~}}
 {{this}}
 {{/each~}}
 {{/if}}
@@ -23,4 +23,4 @@ Tape Backup failed: {{error}}
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#pbsServerAdministration:tasks>
+<{{base-url}}/#pbsServerAdministration:tasks>
diff --git a/templates/default/tape-backup-err-subject.txt.hbs b/templates/default/tape-backup-err-subject.txt.hbs
index b52d338a..96c4f173 100644
--- a/templates/default/tape-backup-err-subject.txt.hbs
+++ b/templates/default/tape-backup-err-subject.txt.hbs
@@ -1,5 +1,5 @@
-{{#if id~}}
-Tape Backup '{{ id }}' datastore '{{ job.store }}' failed
+{{#if job-id~}}
+Tape Backup '{{job-id}}' datastore '{{datastore}}' failed
 {{else~}}
-Tape Backup datastore '{{ job.store }}' failed
+Tape Backup datastore '{{datastore}}' failed
 {{/if}}
diff --git a/templates/default/tape-backup-ok-body.txt.hbs b/templates/default/tape-backup-ok-body.txt.hbs
index ede51d05..33364142 100644
--- a/templates/default/tape-backup-ok-body.txt.hbs
+++ b/templates/default/tape-backup-ok-body.txt.hbs
@@ -1,21 +1,21 @@
-{{#if id ~}}
-Job ID:     {{id}}
+{{#if job-id~}}
+Job ID:     {{job-id}}
 {{/if~}}
-Datastore:  {{job.store}}
-Tape Pool:  {{job.pool}}
-Tape Drive: {{job.drive}}
+Datastore:  {{datastore}}
+Tape Pool:  {{tape-pool}}
+Tape Drive: {{tape-drive}}
 
-{{#if snapshot-list ~}}
+{{#if snapshot-list~}}
 Snapshots included:
 
 {{#each snapshot-list~}}
 {{this}}
 {{/each~}}
 {{/if}}
-Duration: {{job-duration}}
-{{#if used-tapes }}
+Duration: {{duration job-duration}}
+{{#if used-tapes-list}}
 Used Tapes:
-{{#each used-tapes~}}
+{{#each used-tapes-list~}}
 {{this}}
 {{/each~}}
 {{/if}}
@@ -24,4 +24,4 @@ Tape Backup successful.
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#DataStore-{{job.store}}>
+<{{base-url}}/#DataStore-{{datastore}}>
diff --git a/templates/default/tape-backup-ok-subject.txt.hbs b/templates/default/tape-backup-ok-subject.txt.hbs
index c475c05b..270e853f 100644
--- a/templates/default/tape-backup-ok-subject.txt.hbs
+++ b/templates/default/tape-backup-ok-subject.txt.hbs
@@ -1,5 +1,5 @@
-{{#if id~}}
-Tape Backup '{{ id }}' datastore '{{ job.store }}' successful
+{{#if job-id~}}
+Tape Backup '{{job-id}}' datastore '{{datastore}}' successful
 {{else~}}
-Tape Backup datastore '{{ job.store }}' successful
+Tape Backup datastore '{{datastore}}' successful
 {{/if}}
-- 
2.39.5



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