From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 550FA1FF164
	for <inbox@lore.proxmox.com>; Fri, 28 Mar 2025 11:23:04 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 44C2136C39;
	Fri, 28 Mar 2025 11:22:55 +0100 (CET)
From: Lukas Wagner <l.wagner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Fri, 28 Mar 2025 11:22:39 +0100
Message-Id: <20250328102242.75539-8-l.wagner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250328102242.75539-1-l.wagner@proxmox.com>
References: <20250328102242.75539-1-l.wagner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -1.037 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
 KAM_MAILER                  2 Automated Mailer Tag Left in Email
 PROLO_LEO1                0.1 Meta Catches all Leo drug variations so far
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH proxmox-backup v3 07/10] notifications: add type
 for tape backup notification template data
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.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               | 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 9e069710..f15044a1 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: format!("{err:#}"),
+            };
+
+            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 81ee19c6..1e6c278b 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -247,3 +247,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