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

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

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

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

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

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