[pbs-devel] [PATCH proxmox-backup 00/10] notifications: cleanup in preparation of overridable templates

[pbs-devel] [PATCH proxmox-backup 00/10] notifications: cleanup in preparation of overridable templates

[Lukas Wagner]

When the notification system was brought to PBS, the template strings
were moved to the template files as they were, without any changes.
The original templates were an implementation detail which were not
exposed to the user in any way.
They were bit inconsistent with regards to how template variables were
named (e.g. '{{datastore}}', '{{store}}', '{{job.store}}' for
referring to a datastore), as well es how variables/helpers 
were accessed ({{ var }} vs {{var}}).

With [#6143] on the horizon, notification templates, template variables
and template helpers become part of our public API and as such
we should provide some stability guarantees for them.
As a result, we use this opportunity to do a 'final' cleanup.

The aims of this series are:
  - cleanup inconsistencies in the existing templates
  - add custom types which are used to pass template variables
    to the notification system, serving as documentation of what
    is passed exactly to each template, as well as protection
    against accidentally leaking interal code changes into
    the template rendering process

This series also removes the HTML version of the test notification template.
In PBS, this was the only template for which we shipped an HTML version.
This might bee a bit confusing for users writing their own templates,
hence it is removed. If we ever add HTML templates for the other notification
types we can add it back.

[#6143] https://bugzilla.proxmox.com/show_bug.cgi?id=6143

proxmox-backup:

Lukas Wagner (10):
  notifications: move make notifications module a dir-style module
  notifications: add type for GC notification template data
  notifications: add type for ACME notification template data
  notifications: add type for APT notification template data
  notifications: add type for prune notification template data
  notifications: add type for sync notification template data
  notifications: add type for tape backup notification template data
  notifications: add type for tape load notification template data
  notifications: add type for verify notification template data
  notifications: remove HTML template for test notification

 debian/proxmox-backup-server.install          |   1 -
 .../mod.rs}                                   | 355 ++++++++++--------
 src/server/notifications/template_data.rs     | 341 +++++++++++++++++
 templates/Makefile                            |   1 -
 templates/default/acme-err-body.txt.hbs       |   2 +-
 templates/default/gc-err-body.txt.hbs         |   2 +-
 templates/default/gc-err-subject.txt.hbs      |   2 +-
 templates/default/gc-ok-body.txt.hbs          |  22 +-
 templates/default/gc-ok-subject.txt.hbs       |   2 +-
 .../default/package-updates-body.txt.hbs      |   8 +-
 .../default/package-updates-subject.txt.hbs   |   2 +-
 templates/default/prune-err-body.txt.hbs      |   6 +-
 templates/default/prune-err-subject.txt.hbs   |   2 +-
 templates/default/prune-ok-body.txt.hbs       |   6 +-
 templates/default/prune-ok-subject.txt.hbs    |   2 +-
 templates/default/sync-err-body.txt.hbs       |  14 +-
 templates/default/sync-err-subject.txt.hbs    |   8 +-
 templates/default/sync-ok-body.txt.hbs        |  14 +-
 templates/default/sync-ok-subject.txt.hbs     |   6 +-
 .../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 +-
 templates/default/tape-load-body.txt.hbs      |  14 +-
 templates/default/tape-load-subject.txt.hbs   |   2 +-
 templates/default/test-body.html.hbs          |   1 -
 templates/default/test-body.txt.hbs           |   2 +-
 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 +-
 31 files changed, 639 insertions(+), 244 deletions(-)
 rename src/server/{notifications.rs => notifications/mod.rs} (67%)
 create mode 100644 src/server/notifications/template_data.rs
 delete mode 100644 templates/default/test-body.html.hbs


Summary over all repositories:
  31 files changed, 639 insertions(+), 244 deletions(-)

-- 
Generated by git-murpp 0.8.0


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

[pbs-devel] [PATCH proxmox-backup 01/10] notifications: move make notifications module a dir-style module

[Lukas Wagner]

The next commit is going to add a separate submodule for notification
template data types.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/server/{notifications.rs => notifications/mod.rs} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename src/server/{notifications.rs => notifications/mod.rs} (100%)

diff --git a/src/server/notifications.rs b/src/server/notifications/mod.rs
similarity index 100%
rename from src/server/notifications.rs
rename to src/server/notifications/mod.rs
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 02/10] notifications: add type for GC notification template data

[Lukas Wagner]

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           |  50 ++++----
 src/server/notifications/template_data.rs | 132 ++++++++++++++++++++++
 templates/default/gc-err-body.txt.hbs     |   2 +-
 templates/default/gc-err-subject.txt.hbs  |   2 +-
 templates/default/gc-ok-body.txt.hbs      |  22 ++--
 templates/default/gc-ok-subject.txt.hbs   |   2 +-
 6 files changed, 170 insertions(+), 40 deletions(-)
 create mode 100644 src/server/notifications/template_data.rs

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index eea55202..182af213 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -21,6 +21,10 @@ use proxmox_notify::{Endpoint, Notification, Severity};
 
 const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
 
+mod template_data;
+
+use template_data::{GcErrTemplateData, GcOkTemplateData};
+
 /// Initialize the notification system by setting context in proxmox_notify
 pub fn init() -> Result<(), Error> {
     proxmox_notify::context::set_context(&PBS_CONTEXT);
@@ -146,38 +150,32 @@ pub fn send_gc_status(
     status: &GarbageCollectionStatus,
     result: &Result<(), Error>,
 ) -> Result<(), Error> {
-    let (fqdn, port) = get_server_url();
-    let mut data = json!({
-        "datastore": datastore,
-        "fqdn": fqdn,
-        "port": port,
-    });
-
-    let (severity, template) = match result {
-        Ok(()) => {
-            let deduplication_factor = if status.disk_bytes > 0 {
-                (status.index_data_bytes as f64) / (status.disk_bytes as f64)
-            } else {
-                1.0
-            };
-
-            data["status"] = json!(status);
-            data["deduplication-factor"] = format!("{:.2}", deduplication_factor).into();
-
-            (Severity::Info, "gc-ok")
-        }
-        Err(err) => {
-            data["error"] = err.to_string().into();
-            (Severity::Error, "gc-err")
-        }
-    };
     let metadata = HashMap::from([
         ("datastore".into(), datastore.into()),
         ("hostname".into(), proxmox_sys::nodename().into()),
         ("type".into(), "gc".into()),
     ]);
 
-    let notification = Notification::from_template(severity, template, data, metadata);
+    let notification = match result {
+        Ok(()) => {
+            let template_data = GcOkTemplateData::new(datastore.to_string(), status);
+            Notification::from_template(
+                Severity::Info,
+                "gc-ok",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+        Err(err) => {
+            let template_data = GcErrTemplateData::new(datastore.to_string(), err.to_string());
+            Notification::from_template(
+                Severity::Error,
+                "gc-err",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+    };
 
     let (email, notify, mode) = lookup_datastore_notify_settings(datastore);
     match mode {
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
new file mode 100644
index 00000000..2d87b435
--- /dev/null
+++ b/src/server/notifications/template_data.rs
@@ -0,0 +1,132 @@
+use pbs_api_types::GarbageCollectionStatus;
+use serde::Serialize;
+
+// NOTE: For some of these types, the `XyzOkTemplateData` and `XyzErrTemplateData`
+// types are almost identical except for the `error` member.
+// While at first glance I might make sense
+// to consolidate the two and make `error` an `Option`, I would argue
+// that it is actually quite nice to have a single, distinct type for
+// each template. This makes it 100% clear which params are accessible
+// for every single template, at the cost of some boilerplate code.
+
+/// Template data which should be available in *all* notifications.
+/// The fields of this struct will be flattened into the individual
+/// *TemplateData structs.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct CommonData {
+    /// The hostname of the PBS host.
+    pub hostname: String,
+    /// The base URL for building links to the web interface.
+    pub base_url: String,
+}
+
+impl CommonData {
+    pub fn new() -> CommonData {
+        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);
+            }
+        }
+
+        // TODO: Some users might want to be able to override this.
+        let base_url = format!("https://{fqdn}:8007");
+
+        CommonData {
+            hostname: nodename.into(),
+            base_url,
+        }
+    }
+}
+
+/// Template data for the gc-ok template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct GcOkTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The task's UPID.
+    pub upid: Option<String>,
+    /// Number of processed index files.
+    pub index_file_count: usize,
+    /// Sum of bytes referred by index files.
+    pub index_data_bytes: u64,
+    /// Bytes used on disk.
+    pub disk_bytes: u64,
+    /// Chunks used on disk.
+    pub disk_chunks: usize,
+    /// Sum of removed bytes.
+    pub removed_bytes: u64,
+    /// Number of removed chunks.
+    pub removed_chunks: usize,
+    /// Sum of pending bytes (pending removal - kept for safety).
+    pub pending_bytes: u64,
+    /// Number of pending chunks (pending removal - kept for safety).
+    pub pending_chunks: usize,
+    /// Number of chunks marked as .bad by verify that have been removed by GC.
+    pub removed_bad: usize,
+    /// Number of chunks still marked as .bad after garbage collection.
+    pub still_bad: usize,
+    /// Factor of deduplication.
+    pub deduplication_factor: String,
+}
+
+impl GcOkTemplateData {
+    /// Create new a new instance.
+    pub fn new(datastore: String, status: &GarbageCollectionStatus) -> Self {
+        let deduplication_factor = if status.disk_bytes > 0 {
+            (status.index_data_bytes as f64) / (status.disk_bytes as f64)
+        } else {
+            1.0
+        };
+        let deduplication_factor = format!("{:.2}", deduplication_factor);
+
+        Self {
+            common: CommonData::new(),
+            datastore,
+            upid: status.upid.clone(),
+            index_file_count: status.index_file_count,
+            index_data_bytes: status.index_data_bytes,
+            disk_bytes: status.disk_bytes,
+            disk_chunks: status.disk_chunks,
+            removed_bytes: status.removed_bytes,
+            removed_chunks: status.removed_chunks,
+            pending_bytes: status.pending_bytes,
+            pending_chunks: status.pending_chunks,
+            removed_bad: status.removed_bad,
+            still_bad: status.still_bad,
+            deduplication_factor,
+        }
+    }
+}
+
+/// Template data for the gc-err template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct GcErrTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The error that occured during the GC job.
+    pub error: String,
+}
+
+impl GcErrTemplateData {
+    /// Create new a new instance.
+    pub fn new(datastore: String, error: String) -> Self {
+        Self {
+            common: CommonData::new(),
+            datastore,
+            error,
+        }
+    }
+}
diff --git a/templates/default/gc-err-body.txt.hbs b/templates/default/gc-err-body.txt.hbs
index d6c2d0bc..107f9e2e 100644
--- a/templates/default/gc-err-body.txt.hbs
+++ b/templates/default/gc-err-body.txt.hbs
@@ -5,4 +5,4 @@ Garbage collection failed: {{error}}
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#pbsServerAdministration:tasks>
+<{{base-url}}/#pbsServerAdministration:tasks>
diff --git a/templates/default/gc-err-subject.txt.hbs b/templates/default/gc-err-subject.txt.hbs
index ebf49f3b..f02873d1 100644
--- a/templates/default/gc-err-subject.txt.hbs
+++ b/templates/default/gc-err-subject.txt.hbs
@@ -1 +1 @@
-Garbage Collect Datastore '{{ datastore }}' failed
+Garbage Collect Datastore '{{datastore}}' failed
diff --git a/templates/default/gc-ok-body.txt.hbs b/templates/default/gc-ok-body.txt.hbs
index d2f7cd81..b3aaedf4 100644
--- a/templates/default/gc-ok-body.txt.hbs
+++ b/templates/default/gc-ok-body.txt.hbs
@@ -1,17 +1,17 @@
 Datastore:            {{datastore}}
-Task ID:              {{status.upid}}
-Index file count:     {{status.index-file-count}}
+Task ID:              {{upid}}
+Index file count:     {{index-file-count}}
 
-Removed garbage:      {{human-bytes status.removed-bytes}}
-Removed chunks:       {{status.removed-chunks}}
-Removed bad chunks:   {{status.removed-bad}}
+Removed garbage:      {{human-bytes removed-bytes}}
+Removed chunks:       {{removed-chunks}}
+Removed bad chunks:   {{removed-bad}}
 
-Leftover bad chunks:  {{status.still-bad}}
-Pending removals:     {{human-bytes status.pending-bytes}} (in {{status.pending-chunks}} chunks)
+Leftover bad chunks:  {{still-bad}}
+Pending removals:     {{human-bytes pending-bytes}} (in {{pending-chunks}} chunks)
 
-Original Data usage:  {{human-bytes status.index-data-bytes}}
-On-Disk usage:        {{human-bytes status.disk-bytes}} ({{relative-percentage status.disk-bytes status.index-data-bytes}})
-On-Disk chunks:       {{status.disk-chunks}}
+Original Data usage:  {{human-bytes index-data-bytes}}
+On-Disk usage:        {{human-bytes disk-bytes}} ({{relative-percentage disk-bytes index-data-bytes}})
+On-Disk chunks:       {{disk-chunks}}
 
 Deduplication Factor: {{deduplication-factor}}
 
@@ -20,4 +20,4 @@ Garbage collection successful.
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#DataStore-{{datastore}}>
+<{{base-url}}/#DataStore-{{datastore}}>
diff --git a/templates/default/gc-ok-subject.txt.hbs b/templates/default/gc-ok-subject.txt.hbs
index 538e3700..ee27ec50 100644
--- a/templates/default/gc-ok-subject.txt.hbs
+++ b/templates/default/gc-ok-subject.txt.hbs
@@ -1 +1 @@
-Garbage Collect Datastore '{{ datastore }}' successful
+Garbage Collect 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

[pbs-devel] [PATCH proxmox-backup 03/10] notifications: add type for ACME notification template data

[Lukas Wagner]

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           | 22 ++++++++++++----------
 src/server/notifications/template_data.rs | 11 +++++++++++
 templates/default/acme-err-body.txt.hbs   |  2 +-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 182af213..864c6e9f 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -23,7 +23,7 @@ const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/noti
 
 mod template_data;
 
-use template_data::{GcErrTemplateData, GcOkTemplateData};
+use template_data::{AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData};
 
 /// Initialize the notification system by setting context in proxmox_notify
 pub fn init() -> Result<(), Error> {
@@ -493,20 +493,22 @@ pub fn send_certificate_renewal_mail(result: &Result<(), Error>) -> Result<(), E
         _ => return Ok(()),
     };
 
-    let (fqdn, port) = get_server_url();
-
-    let data = json!({
-        "fqdn": fqdn,
-        "port": port,
-        "error": error,
-    });
-
     let metadata = HashMap::from([
         ("hostname".into(), proxmox_sys::nodename().into()),
         ("type".into(), "acme".into()),
     ]);
 
-    let notification = Notification::from_template(Severity::Info, "acme-err", data, metadata);
+    let template_data = AcmeErrTemplateData {
+        common: CommonData::new(),
+        error,
+    };
+
+    let notification = Notification::from_template(
+        Severity::Info,
+        "acme-err",
+        serde_json::to_value(template_data)?,
+        metadata,
+    );
 
     send_notification(notification)?;
     Ok(())
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index 2d87b435..c3e31367 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -130,3 +130,14 @@ impl GcErrTemplateData {
         }
     }
 }
+
+/// Template data for the acme-err template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct AcmeErrTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The error that occured when trying to request the certificate.
+    pub error: String,
+}
diff --git a/templates/default/acme-err-body.txt.hbs b/templates/default/acme-err-body.txt.hbs
index 3cbfea4a..b9f52a25 100644
--- a/templates/default/acme-err-body.txt.hbs
+++ b/templates/default/acme-err-body.txt.hbs
@@ -4,4 +4,4 @@ Error: {{error}}
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#pbsCertificateConfiguration>
+<{{base-url}}/#pbsCertificateConfiguration>
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 04/10] notifications: add type for APT notification template data

[Lukas Wagner]

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               | 23 ++++++-----
 src/server/notifications/template_data.rs     | 41 ++++++++++++++++++-
 .../default/package-updates-body.txt.hbs      |  8 ++--
 .../default/package-updates-subject.txt.hbs   |  2 +-
 4 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 864c6e9f..480db365 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -23,7 +23,10 @@ const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/noti
 
 mod template_data;
 
-use template_data::{AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData};
+use template_data::{
+    AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData,
+    PackageUpdatesTemplateData,
+};
 
 /// Initialize the notification system by setting context in proxmox_notify
 pub fn init() -> Result<(), Error> {
@@ -464,23 +467,21 @@ fn get_server_url() -> (String, usize) {
 }
 
 pub fn send_updates_available(updates: &[&APTUpdateInfo]) -> Result<(), Error> {
-    let (fqdn, port) = get_server_url();
     let hostname = proxmox_sys::nodename().to_string();
 
-    let data = json!({
-        "fqdn": fqdn,
-        "hostname": &hostname,
-        "port": port,
-        "updates": updates,
-    });
-
     let metadata = HashMap::from([
         ("hostname".into(), hostname),
         ("type".into(), "package-updates".into()),
     ]);
 
-    let notification =
-        Notification::from_template(Severity::Info, "package-updates", data, metadata);
+    let template_data = PackageUpdatesTemplateData::new(updates);
+
+    let notification = Notification::from_template(
+        Severity::Info,
+        "package-updates",
+        serde_json::to_value(template_data)?,
+        metadata,
+    );
 
     send_notification(notification)?;
     Ok(())
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index c3e31367..98b39c10 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -1,4 +1,4 @@
-use pbs_api_types::GarbageCollectionStatus;
+use pbs_api_types::{APTUpdateInfo, GarbageCollectionStatus};
 use serde::Serialize;
 
 // NOTE: For some of these types, the `XyzOkTemplateData` and `XyzErrTemplateData`
@@ -141,3 +141,42 @@ pub struct AcmeErrTemplateData {
     /// The error that occured when trying to request the certificate.
     pub error: String,
 }
+
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+/// A single package which can be upgraded.
+pub struct UpgradablePackage {
+    /// The name of the package.
+    name: String,
+    /// The new version which can be installed.
+    version: String,
+    /// The currently installed version.
+    old_version: String,
+}
+
+/// Template data for the package-updates template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct PackageUpdatesTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    pub available_updates: Vec<UpgradablePackage>,
+}
+
+impl PackageUpdatesTemplateData {
+    /// Create new a new instance.
+    pub fn new(updates: &[&APTUpdateInfo]) -> Self {
+        Self {
+            common: CommonData::new(),
+            available_updates: updates
+                .iter()
+                .map(|info| UpgradablePackage {
+                    name: info.package.clone(),
+                    version: info.version.clone(),
+                    old_version: info.old_version.clone(),
+                })
+                .collect(),
+        }
+    }
+}
diff --git a/templates/default/package-updates-body.txt.hbs b/templates/default/package-updates-body.txt.hbs
index 62f9c7c4..8a31f320 100644
--- a/templates/default/package-updates-body.txt.hbs
+++ b/templates/default/package-updates-body.txt.hbs
@@ -1,8 +1,8 @@
 Proxmox Backup Server has the following updates available:
-{{#each updates }}
-    {{Package}}: {{OldVersion}} -> {{Version~}}
-{{/each }}
+{{#each available-updates}}
+    {{this.name}}: {{this.old-version}} -> {{this.version~}}
+{{/each}}
 
 To upgrade visit the web interface:
 
-<https://{{fqdn}}:{{port}}/#pbsServerAdministration:updates>
+<{{base-url}}/#pbsServerAdministration:updates>
diff --git a/templates/default/package-updates-subject.txt.hbs b/templates/default/package-updates-subject.txt.hbs
index c8a775d5..556a67b8 100644
--- a/templates/default/package-updates-subject.txt.hbs
+++ b/templates/default/package-updates-subject.txt.hbs
@@ -1 +1 @@
-New software packages available ({{ hostname }})
+New software packages available ({{hostname}})
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 05/10] notifications: add type for prune notification template data

[Lukas Wagner]

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             | 50 +++++++++++++--------
 src/server/notifications/template_data.rs   | 28 ++++++++++++
 templates/default/prune-err-body.txt.hbs    |  6 +--
 templates/default/prune-err-subject.txt.hbs |  2 +-
 templates/default/prune-ok-body.txt.hbs     |  6 +--
 templates/default/prune-ok-subject.txt.hbs  |  2 +-
 6 files changed, 68 insertions(+), 26 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 480db365..d1f547c6 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -25,7 +25,7 @@ mod template_data;
 
 use template_data::{
     AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData,
-    PackageUpdatesTemplateData,
+    PackageUpdatesTemplateData, PruneErrTemplateData, PruneOkTemplateData,
 };
 
 /// Initialize the notification system by setting context in proxmox_notify
@@ -259,22 +259,6 @@ pub fn send_prune_status(
     jobname: &str,
     result: &Result<(), Error>,
 ) -> Result<(), Error> {
-    let (fqdn, port) = get_server_url();
-    let mut data = json!({
-        "jobname": jobname,
-        "store": store,
-        "fqdn": fqdn,
-        "port": port,
-    });
-
-    let (template, severity) = match result {
-        Ok(()) => ("prune-ok", Severity::Info),
-        Err(err) => {
-            data["error"] = err.to_string().into();
-            ("prune-err", Severity::Error)
-        }
-    };
-
     let metadata = HashMap::from([
         ("job-id".into(), jobname.to_string()),
         ("datastore".into(), store.into()),
@@ -282,7 +266,37 @@ pub fn send_prune_status(
         ("type".into(), "prune".into()),
     ]);
 
-    let notification = Notification::from_template(severity, template, data, metadata);
+    let notification = match result {
+        Ok(()) => {
+            let template_data = PruneOkTemplateData {
+                common: CommonData::new(),
+                datastore: store.to_string(),
+                job_id: jobname.to_string(),
+            };
+
+            Notification::from_template(
+                Severity::Info,
+                "prune-ok",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+        Err(err) => {
+            let template_data = PruneErrTemplateData {
+                common: CommonData::new(),
+                datastore: store.to_string(),
+                job_id: jobname.to_string(),
+                error: err.to_string(),
+            };
+
+            Notification::from_template(
+                Severity::Error,
+                "prune-err",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+    };
 
     let (email, notify, mode) = lookup_datastore_notify_settings(store);
     match mode {
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index 98b39c10..0dead942 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -180,3 +180,31 @@ impl PackageUpdatesTemplateData {
         }
     }
 }
+
+/// Template data for the prune-ok template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct PruneOkTemplateData {
+    /// 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 prune-err template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct PruneErrTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The ID of the job.
+    pub job_id: String,
+    /// The error that occured during the prune job.
+    pub error: String,
+}
diff --git a/templates/default/prune-err-body.txt.hbs b/templates/default/prune-err-body.txt.hbs
index 0973c3d9..4ea8d7a9 100644
--- a/templates/default/prune-err-body.txt.hbs
+++ b/templates/default/prune-err-body.txt.hbs
@@ -1,10 +1,10 @@
 
-Job ID:       {{jobname}}
-Datastore:    {{store}}
+Job ID:       {{job-id}}
+Datastore:    {{datastore}}
 
 Pruning failed: {{error}}
 
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#pbsServerAdministration:tasks>
+<{{base-url}}/#pbsServerAdministration:tasks>
diff --git a/templates/default/prune-err-subject.txt.hbs b/templates/default/prune-err-subject.txt.hbs
index 836ae722..6af35ea0 100644
--- a/templates/default/prune-err-subject.txt.hbs
+++ b/templates/default/prune-err-subject.txt.hbs
@@ -1 +1 @@
-Pruning datastore '{{ store }}' failed
+Pruning datastore '{{datastore}}' failed
diff --git a/templates/default/prune-ok-body.txt.hbs b/templates/default/prune-ok-body.txt.hbs
index b7e449e3..9e119c0e 100644
--- a/templates/default/prune-ok-body.txt.hbs
+++ b/templates/default/prune-ok-body.txt.hbs
@@ -1,10 +1,10 @@
 
-Job ID:       {{jobname}}
-Datastore:    {{store}}
+Job ID:       {{job-id}}
+Datastore:    {{datastore}}
 
 Pruning successful.
 
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#DataStore-{{store}}>
+<{{base-url}}/#DataStore-{{datastore}}>
diff --git a/templates/default/prune-ok-subject.txt.hbs b/templates/default/prune-ok-subject.txt.hbs
index 3227a062..25a854ad 100644
--- a/templates/default/prune-ok-subject.txt.hbs
+++ b/templates/default/prune-ok-subject.txt.hbs
@@ -1 +1 @@
-Pruning datastore '{{ store }}' successful
+Pruning 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

[pbs-devel] [PATCH proxmox-backup 06/10] notifications: add type for sync notification template data

[Lukas Wagner]

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            | 52 +++++++++++++++-------
 src/server/notifications/template_data.rs  | 36 +++++++++++++++
 templates/default/sync-err-body.txt.hbs    | 14 +++---
 templates/default/sync-err-subject.txt.hbs |  8 ++--
 templates/default/sync-ok-body.txt.hbs     | 14 +++---
 templates/default/sync-ok-subject.txt.hbs  |  6 +--
 6 files changed, 92 insertions(+), 38 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index d1f547c6..3301c239 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -25,7 +25,8 @@ mod template_data;
 
 use template_data::{
     AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData,
-    PackageUpdatesTemplateData, PruneErrTemplateData, PruneOkTemplateData,
+    PackageUpdatesTemplateData, PruneErrTemplateData, PruneOkTemplateData, SyncErrTemplateData,
+    SyncOkTemplateData,
 };
 
 /// Initialize the notification system by setting context in proxmox_notify
@@ -320,21 +321,6 @@ pub fn send_prune_status(
 }
 
 pub fn send_sync_status(job: &SyncJobConfig, result: &Result<(), 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(()) => ("sync-ok", Severity::Info),
-        Err(err) => {
-            data["error"] = err.to_string().into();
-            ("sync-err", Severity::Error)
-        }
-    };
-
     let metadata = HashMap::from([
         ("job-id".into(), job.id.clone()),
         ("datastore".into(), job.store.clone()),
@@ -342,7 +328,39 @@ pub fn send_sync_status(job: &SyncJobConfig, result: &Result<(), Error>) -> Resu
         ("type".into(), "sync".into()),
     ]);
 
-    let notification = Notification::from_template(severity, template, data, metadata);
+    let notification = match result {
+        Ok(()) => {
+            let template_data = SyncOkTemplateData {
+                common: CommonData::new(),
+                datastore: job.store.clone(),
+                job_id: job.id.clone(),
+                remote: job.remote.clone(),
+                remote_datastore: job.remote_store.clone(),
+            };
+            Notification::from_template(
+                Severity::Info,
+                "sync-ok",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+        Err(err) => {
+            let template_data = SyncErrTemplateData {
+                common: CommonData::new(),
+                datastore: job.store.clone(),
+                job_id: job.id.clone(),
+                remote: job.remote.clone(),
+                remote_datastore: job.remote_store.clone(),
+                error: err.to_string(),
+            };
+            Notification::from_template(
+                Severity::Error,
+                "sync-err",
+                serde_json::to_value(template_data)?,
+                metadata,
+            )
+        }
+    };
 
     let (email, notify, mode) = lookup_datastore_notify_settings(&job.store);
     match mode {
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index 0dead942..3df9ed17 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -208,3 +208,39 @@ pub struct PruneErrTemplateData {
     /// The error that occured during the prune job.
     pub error: String,
 }
+
+/// Template data for the sync-ok template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct SyncOkTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The ID of the job.
+    pub job_id: String,
+    /// The remote.
+    pub remote: Option<String>,
+    /// The remote datastore we synced to/from.
+    pub remote_datastore: String,
+}
+
+/// Template data for the sync-err template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct SyncErrTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The datastore.
+    pub datastore: String,
+    /// The ID of the job.
+    pub job_id: String,
+    /// The remote.
+    pub remote: Option<String>,
+    /// The remote datastore we synced to/from.
+    pub remote_datastore: String,
+    /// The error that occurred during the sync job.
+    pub error: String,
+}
diff --git a/templates/default/sync-err-body.txt.hbs b/templates/default/sync-err-body.txt.hbs
index a56d9d22..d47a5d6c 100644
--- a/templates/default/sync-err-body.txt.hbs
+++ b/templates/default/sync-err-body.txt.hbs
@@ -1,14 +1,14 @@
-Job ID:             {{job.id}}
-Datastore:          {{job.store}}
-{{#if job.remote~}}
-Remote:             {{job.remote}}
-Remote Store:       {{job.remote-store}}
+Job ID:             {{job-id}}
+Datastore:          {{datastore}}
+{{#if remote~}}
+Remote:             {{remote}}
+Remote Store:       {{remote-datastore}}
 {{else~}}
-Local Source Store: {{job.remote-store}}
+Local Source Store: {{remote-datastore}}
 {{/if}}
 Synchronization failed: {{error}}
 
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#pbsServerAdministration:tasks>
+<{{base-url}}/#pbsServerAdministration:tasks>
diff --git a/templates/default/sync-err-subject.txt.hbs b/templates/default/sync-err-subject.txt.hbs
index a1464802..a4aee3e4 100644
--- a/templates/default/sync-err-subject.txt.hbs
+++ b/templates/default/sync-err-subject.txt.hbs
@@ -1,5 +1,5 @@
-{{#if job.remote~}}
-Sync remote '{{ job.remote }}' datastore '{{ job.remote-store }}' failed
+{{#if remote~}}
+Sync remote '{{remote}}' datastore '{{remote-datastore}}' failed
 {{else~}}
-Sync local datastore '{{ job.remote-store }}' failed
-{{/if}}
+Sync local datastore '{{remote-datastore}}' failed
+{{/if}} 
diff --git a/templates/default/sync-ok-body.txt.hbs b/templates/default/sync-ok-body.txt.hbs
index 25c4b33b..6ef5993d 100644
--- a/templates/default/sync-ok-body.txt.hbs
+++ b/templates/default/sync-ok-body.txt.hbs
@@ -1,14 +1,14 @@
-Job ID:             {{job.id}}
-Datastore:          {{job.store}}
-{{#if job.remote~}}
-Remote:             {{job.remote}}
-Remote Store:       {{job.remote-store}}
+Job ID:             {{job-id}}
+Datastore:          {{datastore}}
+{{#if remote~}}
+Remote:             {{remote}}
+Remote Store:       {{remote-datastore}}
 {{else~}}
-Local Source Store: {{job.remote-store}}
+Local Source Store: {{remote-datastore}}
 {{/if}}
 Synchronization successful.
 
 
 Please visit the web interface for further details:
 
-<https://{{fqdn}}:{{port}}/#DataStore-{{job.store}}>
+<{{base-url}}/#DataStore-{{datastore}}>
diff --git a/templates/default/sync-ok-subject.txt.hbs b/templates/default/sync-ok-subject.txt.hbs
index 76616b5c..35fac2ce 100644
--- a/templates/default/sync-ok-subject.txt.hbs
+++ b/templates/default/sync-ok-subject.txt.hbs
@@ -1,5 +1,5 @@
-{{#if job.remote~}}
-Sync remote '{{ job.remote }}' datastore '{{ job.remote-store }}' successful
+{{#if remote~}}
+Sync remote '{{remote}}' datastore '{{remote-datastore}}' successful
 {{else~}}
-Sync local datastore '{{ job.remote-store }}' successful
+Sync local datastore '{{remote-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

[pbs-devel] [PATCH proxmox-backup 07/10] notifications: add type for tape backup notification template data

[Lukas Wagner]

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

[pbs-devel] [PATCH proxmox-backup 08/10] notifications: add type for tape load notification template data

[Lukas Wagner]

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             | 31 +++++++++++++--------
 src/server/notifications/template_data.rs   | 19 +++++++++++++
 templates/default/tape-load-body.txt.hbs    | 14 +++++-----
 templates/default/tape-load-subject.txt.hbs |  2 +-
 4 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index e8ce60a2..92c5eb4d 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, TapeBackupErrTemplateData, TapeBackupOkTemplateData,
+    SyncOkTemplateData, TapeBackupErrTemplateData, TapeBackupOkTemplateData, TapeLoadTemplateData,
 };
 
 /// Initialize the notification system by setting context in proxmox_notify
@@ -470,21 +470,28 @@ pub fn send_load_media_notification(
     label_text: &str,
     reason: Option<String>,
 ) -> Result<(), Error> {
-    let device_type = if changer { "changer" } else { "drive" };
-
-    let data = json!({
-        "device-type": device_type,
-        "device": device,
-        "label-text": label_text,
-        "reason": reason,
-        "is-changer": changer,
-    });
-
     let metadata = HashMap::from([
         ("hostname".into(), proxmox_sys::nodename().into()),
         ("type".into(), "tape-load".into()),
     ]);
-    let notification = Notification::from_template(Severity::Notice, "tape-load", data, metadata);
+
+    let device_type = if changer { "changer" } else { "drive" };
+
+    let template_data = TapeLoadTemplateData {
+        common: CommonData::new(),
+        load_reason: reason,
+        tape_drive: device.into(),
+        drive_type: device_type.into(),
+        drive_is_changer: changer,
+        tape_label: label_text.into(),
+    };
+
+    let notification = Notification::from_template(
+        Severity::Notice,
+        "tape-load",
+        serde_json::to_value(template_data)?,
+        metadata,
+    );
 
     match mode {
         TapeNotificationMode::LegacySendmail { notify_user } => {
diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
index e24a5ef3..b6b3f293 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -292,3 +292,22 @@ pub struct TapeBackupErrTemplateData {
     /// The error that happend during the backup job.
     pub error: String,
 }
+
+/// Template data for the tape-load template.
+#[derive(Serialize)]
+#[serde(rename_all = "kebab-case")]
+pub struct TapeLoadTemplateData {
+    /// Common properties.
+    #[serde(flatten)]
+    pub common: CommonData,
+    /// The reason why the tape must be loaded.
+    pub load_reason: Option<String>,
+    /// The tape drive.
+    pub tape_drive: String,
+    /// The type of the drive (changer/drive)
+    pub drive_type: String,
+    /// The drive is a tape changer.
+    pub drive_is_changer: bool,
+    /// The label of the tape.
+    pub tape_label: String,
+}
diff --git a/templates/default/tape-load-body.txt.hbs b/templates/default/tape-load-body.txt.hbs
index ddc8a9e1..1e2562c9 100644
--- a/templates/default/tape-load-body.txt.hbs
+++ b/templates/default/tape-load-body.txt.hbs
@@ -1,15 +1,15 @@
-{{#if reason~}}
-The {{ device-type }} has the wrong or no tape(s) inserted. Error:
-{{ reason }}
+{{#if load-reason~}}
+The {{drive-type}} has the wrong or no tape(s) inserted. Error:
+{{load-reason}}
 
 {{/if~}}
-{{#if is-changer~}}
+{{#if drive-is-changer~}}
 Please insert the requested media into the changer.
 
-Changer: {{ device }}
+Changer: {{tape-drive}}
 {{else}}
 Please insert the requested media into the backup drive.
 
-Drive: {{ device }}
+Drive: {{tape-drive}}
 {{/if}}
-Media: {{ label-text }}
+Media: {{tape-label}}
diff --git a/templates/default/tape-load-subject.txt.hbs b/templates/default/tape-load-subject.txt.hbs
index 10f6a02e..a680415b 100644
--- a/templates/default/tape-load-subject.txt.hbs
+++ b/templates/default/tape-load-subject.txt.hbs
@@ -1 +1 @@
-Load Media '{{ label-text }}' request for {{ device-type }} '{{ device }}'
+Load Media '{{tape-label}}' request for {{drive-type}} '{{tape-drive}}'
-- 
2.39.5



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

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

[Lukas Wagner]

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              | 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 92c5eb4d..e34484c3 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 b6b3f293..917f53de 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -311,3 +311,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

[pbs-devel] [PATCH proxmox-backup 10/10] notifications: remove HTML template for test notification

[Lukas Wagner]

The template files for this one have simply been copied from PVE, including
the HTML template.
In PBS we actually don't provide any HTML templates for any other type
of notification, so especially with the template override mechanism on
the horizon, it's probably better to remove this template until we also
provide an HTML version for the other types as well.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 debian/proxmox-backup-server.install | 1 -
 templates/Makefile                   | 1 -
 templates/default/test-body.html.hbs | 1 -
 templates/default/test-body.txt.hbs  | 2 +-
 4 files changed, 1 insertion(+), 4 deletions(-)
 delete mode 100644 templates/default/test-body.html.hbs

diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
index a562d5e8..45f8d903 100644
--- a/debian/proxmox-backup-server.install
+++ b/debian/proxmox-backup-server.install
@@ -63,7 +63,6 @@ usr/share/proxmox-backup/templates/default/tape-backup-ok-body.txt.hbs
 usr/share/proxmox-backup/templates/default/tape-backup-ok-subject.txt.hbs
 usr/share/proxmox-backup/templates/default/tape-load-body.txt.hbs
 usr/share/proxmox-backup/templates/default/tape-load-subject.txt.hbs
-usr/share/proxmox-backup/templates/default/test-body.html.hbs
 usr/share/proxmox-backup/templates/default/test-body.txt.hbs
 usr/share/proxmox-backup/templates/default/test-subject.txt.hbs
 usr/share/proxmox-backup/templates/default/verify-err-body.txt.hbs
diff --git a/templates/Makefile b/templates/Makefile
index 0f8ad72c..0539902e 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -24,7 +24,6 @@ NOTIFICATION_TEMPLATES=						\
 	default/tape-load-body.txt.hbs			\
 	default/tape-load-subject.txt.hbs		\
 	default/test-body.txt.hbs				\
-	default/test-body.html.hbs				\
 	default/test-subject.txt.hbs			\
 	default/verify-err-body.txt.hbs			\
 	default/verify-ok-body.txt.hbs			\
diff --git a/templates/default/test-body.html.hbs b/templates/default/test-body.html.hbs
deleted file mode 100644
index 26a43dde..00000000
--- a/templates/default/test-body.html.hbs
+++ /dev/null
@@ -1 +0,0 @@
-This is a test of the notification target '{{ target }}'.
diff --git a/templates/default/test-body.txt.hbs b/templates/default/test-body.txt.hbs
index 26a43dde..24454433 100644
--- a/templates/default/test-body.txt.hbs
+++ b/templates/default/test-body.txt.hbs
@@ -1 +1 @@
-This is a test of the notification target '{{ target }}'.
+This is a test of the notification target '{{target}}'.
-- 
2.39.5



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

Re: [pbs-devel] [PATCH proxmox-backup 00/10] notifications: cleanup in preparation of overridable templates

[Maximiliano Sandoval]

Lukas Wagner <l.wagner@proxmox.com> writes:

> When the notification system was brought to PBS, the template strings
> were moved to the template files as they were, without any changes.
> The original templates were an implementation detail which were not
> exposed to the user in any way.
> They were bit inconsistent with regards to how template variables were
> named (e.g. '{{datastore}}', '{{store}}', '{{job.store}}' for
> referring to a datastore), as well es how variables/helpers
> were accessed ({{ var }} vs {{var}}).
>
> With [#6143] on the horizon, notification templates, template variables
> and template helpers become part of our public API and as such
> we should provide some stability guarantees for them.
> As a result, we use this opportunity to do a 'final' cleanup.
>
> The aims of this series are:
>   - cleanup inconsistencies in the existing templates
>   - add custom types which are used to pass template variables
>     to the notification system, serving as documentation of what
>     is passed exactly to each template, as well as protection
>     against accidentally leaking interal code changes into
>     the template rendering process
>
> This series also removes the HTML version of the test notification template.
> In PBS, this was the only template for which we shipped an HTML version.
> This might bee a bit confusing for users writing their own templates,
> hence it is removed. If we ever add HTML templates for the other notification
> types we can add it back.
>
> [#6143] https://bugzilla.proxmox.com/show_bug.cgi?id=6143

I went through this patch series and it looks good to me.

Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>


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

Re: [pbs-devel] [PATCH proxmox-backup 02/10] notifications: add type for GC notification template data

[Wolfgang Bumiller]

On Fri, Mar 21, 2025 at 01:25:13PM +0100, Lukas Wagner wrote:
> 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           |  50 ++++----
>  src/server/notifications/template_data.rs | 132 ++++++++++++++++++++++
>  templates/default/gc-err-body.txt.hbs     |   2 +-
>  templates/default/gc-err-subject.txt.hbs  |   2 +-
>  templates/default/gc-ok-body.txt.hbs      |  22 ++--
>  templates/default/gc-ok-subject.txt.hbs   |   2 +-
>  6 files changed, 170 insertions(+), 40 deletions(-)
>  create mode 100644 src/server/notifications/template_data.rs
> 
> diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
> index eea55202..182af213 100644
> --- a/src/server/notifications/mod.rs
> +++ b/src/server/notifications/mod.rs
> @@ -21,6 +21,10 @@ use proxmox_notify::{Endpoint, Notification, Severity};
>  
>  const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
>  
> +mod template_data;
> +
> +use template_data::{GcErrTemplateData, GcOkTemplateData};
> +
>  /// Initialize the notification system by setting context in proxmox_notify
>  pub fn init() -> Result<(), Error> {
>      proxmox_notify::context::set_context(&PBS_CONTEXT);
> @@ -146,38 +150,32 @@ pub fn send_gc_status(
>      status: &GarbageCollectionStatus,
>      result: &Result<(), Error>,
>  ) -> Result<(), Error> {
> -    let (fqdn, port) = get_server_url();
> -    let mut data = json!({
> -        "datastore": datastore,
> -        "fqdn": fqdn,
> -        "port": port,
> -    });
> -
> -    let (severity, template) = match result {
> -        Ok(()) => {
> -            let deduplication_factor = if status.disk_bytes > 0 {
> -                (status.index_data_bytes as f64) / (status.disk_bytes as f64)
> -            } else {
> -                1.0
> -            };
> -
> -            data["status"] = json!(status);
> -            data["deduplication-factor"] = format!("{:.2}", deduplication_factor).into();
> -
> -            (Severity::Info, "gc-ok")
> -        }
> -        Err(err) => {
> -            data["error"] = err.to_string().into();
> -            (Severity::Error, "gc-err")
> -        }
> -    };
>      let metadata = HashMap::from([
>          ("datastore".into(), datastore.into()),
>          ("hostname".into(), proxmox_sys::nodename().into()),
>          ("type".into(), "gc".into()),
>      ]);
>  
> -    let notification = Notification::from_template(severity, template, data, metadata);
> +    let notification = match result {
> +        Ok(()) => {
> +            let template_data = GcOkTemplateData::new(datastore.to_string(), status);
> +            Notification::from_template(
> +                Severity::Info,
> +                "gc-ok",
> +                serde_json::to_value(template_data)?,
> +                metadata,
> +            )
> +        }
> +        Err(err) => {
> +            let template_data = GcErrTemplateData::new(datastore.to_string(), err.to_string());

^ While at it, we could consider switching this error to use
`format!{"{err:#}")` which will include the context as a short form.
This goes for all the templates/commits - but can just be a follow up.
Noticing this since Chris got me looking at anyhow's display
representations[1]

[1] https://docs.rs/anyhow/latest/anyhow/struct.Error.html#display-representations

> +            Notification::from_template(
> +                Severity::Error,
> +                "gc-err",
> +                serde_json::to_value(template_data)?,
> +                metadata,
> +            )
> +        }
> +    };
>  
>      let (email, notify, mode) = lookup_datastore_notify_settings(datastore);
>      match mode {
> diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
> new file mode 100644
> index 00000000..2d87b435
> --- /dev/null
> +++ b/src/server/notifications/template_data.rs
> @@ -0,0 +1,132 @@
> +use pbs_api_types::GarbageCollectionStatus;
> +use serde::Serialize;
> +
> +// NOTE: For some of these types, the `XyzOkTemplateData` and `XyzErrTemplateData`
> +// types are almost identical except for the `error` member.
> +// While at first glance I might make sense
> +// to consolidate the two and make `error` an `Option`, I would argue
> +// that it is actually quite nice to have a single, distinct type for
> +// each template. This makes it 100% clear which params are accessible
> +// for every single template, at the cost of some boilerplate code.
> +
> +/// Template data which should be available in *all* notifications.
> +/// The fields of this struct will be flattened into the individual
> +/// *TemplateData structs.
> +#[derive(Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct CommonData {
> +    /// The hostname of the PBS host.
> +    pub hostname: String,
> +    /// The base URL for building links to the web interface.
> +    pub base_url: String,
> +}
> +
> +impl CommonData {
> +    pub fn new() -> CommonData {
> +        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);
> +            }
> +        }
> +
> +        // TODO: Some users might want to be able to override this.
> +        let base_url = format!("https://{fqdn}:8007");
> +
> +        CommonData {
> +            hostname: nodename.into(),
> +            base_url,
> +        }
> +    }
> +}
> +
> +/// Template data for the gc-ok template.
> +#[derive(Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct GcOkTemplateData {
> +    /// Common properties.
> +    #[serde(flatten)]
> +    pub common: CommonData,
> +    /// The datastore.
> +    pub datastore: String,
> +    /// The task's UPID.
> +    pub upid: Option<String>,
> +    /// Number of processed index files.
> +    pub index_file_count: usize,
> +    /// Sum of bytes referred by index files.
> +    pub index_data_bytes: u64,
> +    /// Bytes used on disk.
> +    pub disk_bytes: u64,
> +    /// Chunks used on disk.
> +    pub disk_chunks: usize,
> +    /// Sum of removed bytes.
> +    pub removed_bytes: u64,
> +    /// Number of removed chunks.
> +    pub removed_chunks: usize,
> +    /// Sum of pending bytes (pending removal - kept for safety).
> +    pub pending_bytes: u64,
> +    /// Number of pending chunks (pending removal - kept for safety).
> +    pub pending_chunks: usize,
> +    /// Number of chunks marked as .bad by verify that have been removed by GC.
> +    pub removed_bad: usize,
> +    /// Number of chunks still marked as .bad after garbage collection.
> +    pub still_bad: usize,
> +    /// Factor of deduplication.
> +    pub deduplication_factor: String,
> +}
> +
> +impl GcOkTemplateData {
> +    /// Create new a new instance.
> +    pub fn new(datastore: String, status: &GarbageCollectionStatus) -> Self {
> +        let deduplication_factor = if status.disk_bytes > 0 {
> +            (status.index_data_bytes as f64) / (status.disk_bytes as f64)
> +        } else {
> +            1.0
> +        };
> +        let deduplication_factor = format!("{:.2}", deduplication_factor);
> +
> +        Self {
> +            common: CommonData::new(),
> +            datastore,
> +            upid: status.upid.clone(),
> +            index_file_count: status.index_file_count,
> +            index_data_bytes: status.index_data_bytes,
> +            disk_bytes: status.disk_bytes,
> +            disk_chunks: status.disk_chunks,
> +            removed_bytes: status.removed_bytes,
> +            removed_chunks: status.removed_chunks,
> +            pending_bytes: status.pending_bytes,
> +            pending_chunks: status.pending_chunks,
> +            removed_bad: status.removed_bad,
> +            still_bad: status.still_bad,
> +            deduplication_factor,
> +        }
> +    }
> +}
> +
> +/// Template data for the gc-err template.
> +#[derive(Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct GcErrTemplateData {
> +    /// Common properties.
> +    #[serde(flatten)]
> +    pub common: CommonData,
> +    /// The datastore.
> +    pub datastore: String,
> +    /// The error that occured during the GC job.
> +    pub error: String,
> +}
> +
> +impl GcErrTemplateData {
> +    /// Create new a new instance.
> +    pub fn new(datastore: String, error: String) -> Self {
> +        Self {
> +            common: CommonData::new(),
> +            datastore,
> +            error,
> +        }
> +    }
> +}
> diff --git a/templates/default/gc-err-body.txt.hbs b/templates/default/gc-err-body.txt.hbs
> index d6c2d0bc..107f9e2e 100644
> --- a/templates/default/gc-err-body.txt.hbs
> +++ b/templates/default/gc-err-body.txt.hbs
> @@ -5,4 +5,4 @@ Garbage collection failed: {{error}}
>  
>  Please visit the web interface for further details:
>  
> -<https://{{fqdn}}:{{port}}/#pbsServerAdministration:tasks>
> +<{{base-url}}/#pbsServerAdministration:tasks>
> diff --git a/templates/default/gc-err-subject.txt.hbs b/templates/default/gc-err-subject.txt.hbs
> index ebf49f3b..f02873d1 100644
> --- a/templates/default/gc-err-subject.txt.hbs
> +++ b/templates/default/gc-err-subject.txt.hbs
> @@ -1 +1 @@
> -Garbage Collect Datastore '{{ datastore }}' failed
> +Garbage Collect Datastore '{{datastore}}' failed
> diff --git a/templates/default/gc-ok-body.txt.hbs b/templates/default/gc-ok-body.txt.hbs
> index d2f7cd81..b3aaedf4 100644
> --- a/templates/default/gc-ok-body.txt.hbs
> +++ b/templates/default/gc-ok-body.txt.hbs
> @@ -1,17 +1,17 @@
>  Datastore:            {{datastore}}
> -Task ID:              {{status.upid}}
> -Index file count:     {{status.index-file-count}}
> +Task ID:              {{upid}}
> +Index file count:     {{index-file-count}}
>  
> -Removed garbage:      {{human-bytes status.removed-bytes}}
> -Removed chunks:       {{status.removed-chunks}}
> -Removed bad chunks:   {{status.removed-bad}}
> +Removed garbage:      {{human-bytes removed-bytes}}
> +Removed chunks:       {{removed-chunks}}
> +Removed bad chunks:   {{removed-bad}}
>  
> -Leftover bad chunks:  {{status.still-bad}}
> -Pending removals:     {{human-bytes status.pending-bytes}} (in {{status.pending-chunks}} chunks)
> +Leftover bad chunks:  {{still-bad}}
> +Pending removals:     {{human-bytes pending-bytes}} (in {{pending-chunks}} chunks)
>  
> -Original Data usage:  {{human-bytes status.index-data-bytes}}
> -On-Disk usage:        {{human-bytes status.disk-bytes}} ({{relative-percentage status.disk-bytes status.index-data-bytes}})
> -On-Disk chunks:       {{status.disk-chunks}}
> +Original Data usage:  {{human-bytes index-data-bytes}}
> +On-Disk usage:        {{human-bytes disk-bytes}} ({{relative-percentage disk-bytes index-data-bytes}})
> +On-Disk chunks:       {{disk-chunks}}
>  
>  Deduplication Factor: {{deduplication-factor}}
>  
> @@ -20,4 +20,4 @@ Garbage collection successful.
>  
>  Please visit the web interface for further details:
>  
> -<https://{{fqdn}}:{{port}}/#DataStore-{{datastore}}>
> +<{{base-url}}/#DataStore-{{datastore}}>
> diff --git a/templates/default/gc-ok-subject.txt.hbs b/templates/default/gc-ok-subject.txt.hbs
> index 538e3700..ee27ec50 100644
> --- a/templates/default/gc-ok-subject.txt.hbs
> +++ b/templates/default/gc-ok-subject.txt.hbs
> @@ -1 +1 @@
> -Garbage Collect Datastore '{{ datastore }}' successful
> +Garbage Collect 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

Re: [pbs-devel] [PATCH proxmox-backup 02/10] notifications: add type for GC notification template data

[Lukas Wagner]

On  2025-03-21 14:28, Wolfgang Bumiller wrote:
> On Fri, Mar 21, 2025 at 01:25:13PM +0100, Lukas Wagner wrote:
>> 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           |  50 ++++----
>>  src/server/notifications/template_data.rs | 132 ++++++++++++++++++++++
>>  templates/default/gc-err-body.txt.hbs     |   2 +-
>>  templates/default/gc-err-subject.txt.hbs  |   2 +-
>>  templates/default/gc-ok-body.txt.hbs      |  22 ++--
>>  templates/default/gc-ok-subject.txt.hbs   |   2 +-
>>  6 files changed, 170 insertions(+), 40 deletions(-)
>>  create mode 100644 src/server/notifications/template_data.rs
>>
>> diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
>> index eea55202..182af213 100644
>> --- a/src/server/notifications/mod.rs
>> +++ b/src/server/notifications/mod.rs
>> @@ -21,6 +21,10 @@ use proxmox_notify::{Endpoint, Notification, Severity};
>>  
>>  const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
>>  
>> +mod template_data;
>> +
>> +use template_data::{GcErrTemplateData, GcOkTemplateData};
>> +
>>  /// Initialize the notification system by setting context in proxmox_notify
>>  pub fn init() -> Result<(), Error> {
>>      proxmox_notify::context::set_context(&PBS_CONTEXT);
>> @@ -146,38 +150,32 @@ pub fn send_gc_status(
>>      status: &GarbageCollectionStatus,
>>      result: &Result<(), Error>,
>>  ) -> Result<(), Error> {
>> -    let (fqdn, port) = get_server_url();
>> -    let mut data = json!({
>> -        "datastore": datastore,
>> -        "fqdn": fqdn,
>> -        "port": port,
>> -    });
>> -
>> -    let (severity, template) = match result {
>> -        Ok(()) => {
>> -            let deduplication_factor = if status.disk_bytes > 0 {
>> -                (status.index_data_bytes as f64) / (status.disk_bytes as f64)
>> -            } else {
>> -                1.0
>> -            };
>> -
>> -            data["status"] = json!(status);
>> -            data["deduplication-factor"] = format!("{:.2}", deduplication_factor).into();
>> -
>> -            (Severity::Info, "gc-ok")
>> -        }
>> -        Err(err) => {
>> -            data["error"] = err.to_string().into();
>> -            (Severity::Error, "gc-err")
>> -        }
>> -    };
>>      let metadata = HashMap::from([
>>          ("datastore".into(), datastore.into()),
>>          ("hostname".into(), proxmox_sys::nodename().into()),
>>          ("type".into(), "gc".into()),
>>      ]);
>>  
>> -    let notification = Notification::from_template(severity, template, data, metadata);
>> +    let notification = match result {
>> +        Ok(()) => {
>> +            let template_data = GcOkTemplateData::new(datastore.to_string(), status);
>> +            Notification::from_template(
>> +                Severity::Info,
>> +                "gc-ok",
>> +                serde_json::to_value(template_data)?,
>> +                metadata,
>> +            )
>> +        }
>> +        Err(err) => {
>> +            let template_data = GcErrTemplateData::new(datastore.to_string(), err.to_string());
> 
> ^ While at it, we could consider switching this error to use
> `format!{"{err:#}")` which will include the context as a short form.
> This goes for all the templates/commits - but can just be a follow up.
> Noticing this since Chris got me looking at anyhow's display
> representations[1]
> 
> [1] https://docs.rs/anyhow/latest/anyhow/struct.Error.html#display-representations
> 

Good point, I'll check it out. Will do that in a followup if nothing else comes up.

Thanks!

-- 
- Lukas



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

Re: [pbs-devel] [PATCH proxmox-backup 04/10] notifications: add type for APT notification template data

[Lukas Wagner]

On  2025-03-21 13:25, Lukas Wagner wrote:
> 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               | 23 ++++++-----
>  src/server/notifications/template_data.rs     | 41 ++++++++++++++++++-
>  .../default/package-updates-body.txt.hbs      |  8 ++--
>  .../default/package-updates-subject.txt.hbs   |  2 +-
>  4 files changed, 57 insertions(+), 17 deletions(-)
> 
> diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
> index 864c6e9f..480db365 100644
> --- a/src/server/notifications/mod.rs
> +++ b/src/server/notifications/mod.rs
> @@ -23,7 +23,10 @@ const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/noti
>  
>  mod template_data;
>  
> -use template_data::{AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData};
> +use template_data::{
> +    AcmeErrTemplateData, CommonData, GcErrTemplateData, GcOkTemplateData,
> +    PackageUpdatesTemplateData,
> +};
>  
>  /// Initialize the notification system by setting context in proxmox_notify
>  pub fn init() -> Result<(), Error> {
> @@ -464,23 +467,21 @@ fn get_server_url() -> (String, usize) {
>  }
>  
>  pub fn send_updates_available(updates: &[&APTUpdateInfo]) -> Result<(), Error> {
> -    let (fqdn, port) = get_server_url();
>      let hostname = proxmox_sys::nodename().to_string();
>  
> -    let data = json!({
> -        "fqdn": fqdn,
> -        "hostname": &hostname,
> -        "port": port,
> -        "updates": updates,
> -    });
> -
>      let metadata = HashMap::from([
>          ("hostname".into(), hostname),
>          ("type".into(), "package-updates".into()),
>      ]);
>  
> -    let notification =
> -        Notification::from_template(Severity::Info, "package-updates", data, metadata);
> +    let template_data = PackageUpdatesTemplateData::new(updates);
> +
> +    let notification = Notification::from_template(
> +        Severity::Info,
> +        "package-updates",
> +        serde_json::to_value(template_data)?,
> +        metadata,
> +    );
>  
>      send_notification(notification)?;
>      Ok(())
> diff --git a/src/server/notifications/template_data.rs b/src/server/notifications/template_data.rs
> index c3e31367..98b39c10 100644
> --- a/src/server/notifications/template_data.rs
> +++ b/src/server/notifications/template_data.rs
> @@ -1,4 +1,4 @@
> -use pbs_api_types::GarbageCollectionStatus;
> +use pbs_api_types::{APTUpdateInfo, GarbageCollectionStatus};
>  use serde::Serialize;
>  
>  // NOTE: For some of these types, the `XyzOkTemplateData` and `XyzErrTemplateData`
> @@ -141,3 +141,42 @@ pub struct AcmeErrTemplateData {
>      /// The error that occured when trying to request the certificate.
>      pub error: String,
>  }
> +
> +#[derive(Serialize)]
> +#[serde(rename_all = "kebab-case")]
> +/// A single package which can be upgraded.
> +pub struct UpgradablePackage {
> +    /// The name of the package.
> +    name: String,
> +    /// The new version which can be installed.
> +    version: String,
> +    /// The currently installed version.
> +    old_version: String,
> +}

I'll do a

name -> package_name
old_version -> installed_version
version -> available_version

in a v2 so that this the same as in the PVE template which I am auditing right now.

I'll send it once I am sure that there are no other changes I want to make
in order to make it more consistent with PVE's templates.

-- 
- Lukas



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

[pbs-devel] superseded: [PATCH proxmox-backup 00/10] notifications: cleanup in preparation of overridable templates

[Lukas Wagner]

Sent v2!

Superseded-by: https://lore.proxmox.com/pbs-devel/20250327145533.274348-1-l.wagner@proxmox.com/T/#t 
-- 
- Lukas



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