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

[pbs-devel] [PATCH proxmox-backup v3 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

Changes since v2:
  - Add the `fqdn` variable to unify it with PVE

Changes since v1:
- Change some template variables for apt notifications
  - version -> available-version
  - old_version -> installed-version
  - name -> package-name
- Use anyhows alternate representation (e.g. format!("{err:#}"))
  to include error context in notifications (thx Wolfgang)

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}                                   | 357 ++++++++++--------
 src/server/notifications/template_data.rs     | 344 +++++++++++++++++
 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, 643 insertions(+), 245 deletions(-)
 rename src/server/{notifications.rs => notifications/mod.rs} (66%)
 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, 643 insertions(+), 245 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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Notes:
    Changes since v2:
     - include `fqdn` as a common variable

 src/server/notifications/mod.rs           |  50 ++++----
 src/server/notifications/template_data.rs | 135 ++++++++++++++++++++++
 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, 173 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..3d467b85 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(), format!("{err:#}"));
+            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..264fab44
--- /dev/null
+++ b/src/server/notifications/template_data.rs
@@ -0,0 +1,135 @@
+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 FQDN of the PBS host.
+    pub fqdn: 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(),
+            fqdn,
+            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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/server/notifications/mod.rs           | 24 ++++++++++++-----------
 src/server/notifications/template_data.rs | 11 +++++++++++
 templates/default/acme-err-body.txt.hbs   |  2 +-
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 3d467b85..a2730d71 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> {
@@ -489,24 +489,26 @@ pub fn send_updates_available(updates: &[&APTUpdateInfo]) -> Result<(), Error> {
 /// send email on certificate renewal failure.
 pub fn send_certificate_renewal_mail(result: &Result<(), Error>) -> Result<(), Error> {
     let error: String = match result {
-        Err(e) => e.to_string(),
+        Err(e) => format!("{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 264fab44..5455cc23 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -133,3 +133,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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@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 a2730d71..ba55e2b1 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 5455cc23..59a430c2 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`
@@ -144,3 +144,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.
+    package_name: String,
+    /// The new version which can be installed.
+    available_version: String,
+    /// The currently installed version.
+    installed_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 {
+                    package_name: info.package.clone(),
+                    available_version: info.version.clone(),
+                    installed_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..cf69ae69 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.package-name}}: {{this.installed-version}} -> {{this.available-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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@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 ba55e2b1..e0dd92d2 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: format!("{err:#}"),
+            };
+
+            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 59a430c2..1b04352e 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -183,3 +183,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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@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 e0dd92d2..9e069710 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: format!("{err:#}"),
+            };
+            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 1b04352e..81ee19c6 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -211,3 +211,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 v3 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>
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

[pbs-devel] [PATCH proxmox-backup v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@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 f15044a1..04790126 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 1e6c278b..a585401c 100644
--- a/src/server/notifications/template_data.rs
+++ b/src/server/notifications/template_data.rs
@@ -295,3 +295,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 v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/server/notifications/mod.rs              | 73 +++++++++-----------
 src/server/notifications/template_data.rs    | 28 ++++++++
 templates/default/verify-err-body.txt.hbs    |  8 +--
 templates/default/verify-err-subject.txt.hbs |  2 +-
 templates/default/verify-ok-body.txt.hbs     |  6 +-
 templates/default/verify-ok-subject.txt.hbs  |  2 +-
 6 files changed, 71 insertions(+), 48 deletions(-)

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



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

[pbs-devel] [PATCH proxmox-backup v3 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>
Reviewed-by: Maximiliano Sandoval <m.sandoval@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

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

[Thomas Lamprecht]

Am 28.03.25 um 11:22 schrieb 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.

Series seems alright to me, I'm just wondering a bit how that guarantee
will look like? But probably better part of the series that is actually
implementing [#6143].

> 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
> 

applied series, thanks!


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

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

[Lukas Wagner]

On  2025-04-02 14:45, Thomas Lamprecht wrote:
> Am 28.03.25 um 11:22 schrieb 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.
> 
> Series seems alright to me, I'm just wondering a bit how that guarantee
> will look like? But probably better part of the series that is actually
> implementing [#6143].

I would suggest:
  - no backward-incompatible changes in minor upgrades
  - for breaking changes in major upgrades, implement some best-effort 
    checks in pbsXtoY/pveXtoY which check any custom templates for
    anything that will be changed/removed


Incompatible changes are:
  - removing variables
  - changing type/representation of variables (e.g. switching from number of bytes to KiB, etc.)
  - removing helpers
  - non-trivial changes to a helper's behavior
  - incompatible changes to the rendering engine (e.g. switching from Handlebars to something else)

Backward-compatible changes would be:
  - adding new template variables
  - adding new template helpers
  - adding new, optional parameters to existing helpers
  - trivial changes to helpers (hypothetical example: "1KiB" -> "1 KiB" for `{{ human-bytes 1024 }}`)


What do you think?

-- 
- Lukas



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

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

[Thomas Lamprecht]

Am 02.04.25 um 15:27 schrieb Lukas Wagner:
> I would suggest:
>   - no backward-incompatible changes in minor upgrades
>   - for breaking changes in major upgrades, implement some best-effort 
>     checks in pbsXtoY/pveXtoY which check any custom templates for
>     anything that will be changed/removed
> 
> 
> Incompatible changes are:
>   - removing variables
>   - changing type/representation of variables (e.g. switching from number of bytes to KiB, etc.)
>   - removing helpers
>   - non-trivial changes to a helper's behavior
>   - incompatible changes to the rendering engine (e.g. switching from Handlebars to something else)
> 
> Backward-compatible changes would be:
>   - adding new template variables
>   - adding new template helpers
>   - adding new, optional parameters to existing helpers
>   - trivial changes to helpers (hypothetical example: "1KiB" -> "1 KiB" for `{{ human-bytes 1024 }}`)

The last one is really something where the "no breakage" means
"no report from users", but I'm fine with that with smaller adaptions.
 
> What do you think?

Sounds about right.

I'd probably also mention that we try to do changes by adding them as new
variable/helper/... if the resulting maintenance burden is somewhat
manageable, as with that we can then have co-existing old/new for a while
(e.g., the current major release) and drop the old one in the next major
release, which would simplify major upgrades combined with sharing templates
be it through pmxcfs or some configuration management stack the admin uses,
like Ansible, compared to rolling out such changes in one go with a major
upgrade.

This does not change what you state above w.r.t. what counts as breaking
change and is not so much relevant for the consumers of that info (those
overriding templates) but can be nice to state the general guideline for
devs to roll out such changes there nonetheless.


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