public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility
@ 2026-04-09 13:27 Lukas Wagner
  2026-04-09 13:27 ` [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue Lukas Wagner
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Lukas Wagner @ 2026-04-09 13:27 UTC (permalink / raw)
  To: pdm-devel, pbs-devel

Forwarding through the worker has a couple of benefits:
  - We don't need to bump proxmx-mail-forwarder for changes in proxmox-notify (at
    least as soon as PVE uses a similar approach as well)
  - any notification state (history, tokens) becomes easier to reason
    about if there is only one process touching it
  - we can move the Context trait implementations from proxmox_notify to the
    actual product code again

Technically we'd need a versioned break between proxmox-backup and
proxmox-mail-forward, due to the 'mail-forwarder' feature that was not enabled in PBS
before, as well as the changed spool directory. If proxmox-mail-forward is
updated without updating proxmox-backup, forwarding the mail would fail due to

  - /var/lib/proxmox-backup/notifications/queue not existing
  - proxmox-backup not being able to deserialize notifications of the 'Forwarded Mail' kind

I don't think there is a way around this breakage, but we *could* defuse it by:

  - keep sending the old way (by directly calling into proxmox_notify)
  - but still change PBS to enable the 'mail-forwarder' feature and use the new spool-dir path
  - at some point in the future we switch proxmox-mail-forward to use the
    queue-based/worker approach, making it less likely that someone still
    holds back a proxmox-backup update, which would break forwarding

I'll leave this decision to the PBS maintainers, just tell me which approach is
preferrable, then I'll adapt the series if needed.


proxmox-backup:

Lukas Wagner (2):
  notifications: move spool directory to
    /var/lib/proxmox-backup/notifications/queue
  pull in 'mail-forwarder' feature for proxmox-notify

 Cargo.toml                      |  2 +-
 src/server/notifications/mod.rs | 30 ++++++++++++++++++++++++------
 2 files changed, 25 insertions(+), 7 deletions(-)


proxmox-mail-forward:

Lukas Wagner (3):
  forward using PBS' notification worker
  forward mails on PDM installations as well
  only ever forward for one product, favoring PVE over PBS over PDM

 Cargo.toml  |   3 +-
 src/main.rs | 129 +++++++++++++++++++++++++++++-----------------------
 2 files changed, 74 insertions(+), 58 deletions(-)


Summary over all repositories:
  4 files changed, 99 insertions(+), 65 deletions(-)

-- 
Generated by murpp 0.11.0




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue
  2026-04-09 13:27 [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility Lukas Wagner
@ 2026-04-09 13:27 ` Lukas Wagner
  2026-04-09 14:52   ` Arthur Bied-Charreton
  2026-04-09 13:27 ` [PATCH proxmox-backup 2/5] pull in 'mail-forwarder' feature for proxmox-notify Lukas Wagner
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Lukas Wagner @ 2026-04-09 13:27 UTC (permalink / raw)
  To: pdm-devel, pbs-devel

In anticipation of more notification-related data that might be stored
soon (notification history, oauth tokens, etc.), we move the spool
directory that is used to queue notifications that are sent by the proxy
process to a separate subdirectory.

We still read from the old directory, for the rare cases that there
might still be any unsent notifications before the package is upgraded.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/server/notifications/mod.rs | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
index 95ff9ef18..15de5ccc5 100644
--- a/src/server/notifications/mod.rs
+++ b/src/server/notifications/mod.rs
@@ -18,7 +18,11 @@ use pbs_api_types::{
 use proxmox_notify::endpoints::sendmail::{SendmailConfig, SendmailEndpoint};
 use proxmox_notify::{Endpoint, Notification, Severity};
 
-const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
+const SPOOL_DIR_OLD: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
+const SPOOL_DIR: &str = concatcp!(
+    pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR,
+    "/notifications/queue"
+);
 
 mod template_data;
 
@@ -43,14 +47,15 @@ pub fn create_spool_dir() -> Result<(), Error> {
         .owner(backup_user.uid)
         .group(backup_user.gid);
 
-    create_path(SPOOL_DIR, None, Some(opts))?;
+    create_path(SPOOL_DIR, Some(opts), Some(opts))?;
     Ok(())
 }
 
-async fn send_queued_notifications() -> Result<(), Error> {
-    let mut read_dir = tokio::fs::read_dir(SPOOL_DIR).await?;
-
-    let mut notifications = Vec::new();
+async fn read_notifications_from_spool_dir(
+    notifications: &mut Vec<Notification>,
+    path: &str,
+) -> Result<(), Error> {
+    let mut read_dir = tokio::fs::read_dir(path).await?;
 
     while let Some(entry) = read_dir.next_entry().await? {
         let path = entry.path();
@@ -71,6 +76,19 @@ async fn send_queued_notifications() -> Result<(), Error> {
         }
     }
 
+    Ok(())
+}
+
+async fn send_queued_notifications() -> Result<(), Error> {
+    let mut notifications = Vec::new();
+
+    // Also read left-over notifications from the old directory -- we should be able to remove this
+    // pretty soon.
+    // FIXME: Remove this with the next minor update.
+    read_notifications_from_spool_dir(&mut notifications, SPOOL_DIR_OLD).await?;
+
+    read_notifications_from_spool_dir(&mut notifications, SPOOL_DIR).await?;
+
     if notifications.is_empty() {
         return Ok(());
     }
-- 
2.47.3





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH proxmox-backup 2/5] pull in 'mail-forwarder' feature for proxmox-notify
  2026-04-09 13:27 [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility Lukas Wagner
  2026-04-09 13:27 ` [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue Lukas Wagner
@ 2026-04-09 13:27 ` Lukas Wagner
  2026-04-09 14:54   ` Arthur Bied-Charreton
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker Lukas Wagner
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Lukas Wagner @ 2026-04-09 13:27 UTC (permalink / raw)
  To: pdm-devel, pbs-devel

Since the notification worker is now in charge of sending notifications
which were queued by proxmox-mail-forward.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 Cargo.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Cargo.toml b/Cargo.toml
index 2347e85e1..448b775e0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -233,7 +233,7 @@ proxmox-ldap.workspace = true
 proxmox-metrics.workspace = true
 proxmox-network-api = { workspace = true, features = [ "impl" ] }
 proxmox-network-types.workspace = true
-proxmox-notify = { workspace = true, features = [ "pbs-context" ] }
+proxmox-notify = { workspace = true, features = [ "pbs-context", "mail-forwarder" ] }
 proxmox-openid.workspace = true
 proxmox-product-config.workspace = true
 proxmox-rate-limiter = { workspace = true, features = [ "shared-rate-limiter" ] }
-- 
2.47.3





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker
  2026-04-09 13:27 [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility Lukas Wagner
  2026-04-09 13:27 ` [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue Lukas Wagner
  2026-04-09 13:27 ` [PATCH proxmox-backup 2/5] pull in 'mail-forwarder' feature for proxmox-notify Lukas Wagner
@ 2026-04-09 13:27 ` Lukas Wagner
  2026-04-09 15:03   ` Arthur Bied-Charreton
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 4/5] forward mails on PDM installations as well Lukas Wagner
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 5/5] only ever forward for one product, favoring PVE over PBS over PDM Lukas Wagner
  4 siblings, 1 reply; 11+ messages in thread
From: Lukas Wagner @ 2026-04-09 13:27 UTC (permalink / raw)
  To: pdm-devel, pbs-devel

Instead of sending the notification ourselves, use the fact that we
already have a 'notification queue' handled by a dedicated worker task
in PBS. Up until now, it was only used for sending notifications from
the unprivileged proxy process, but nothing speaks against doing the
same for forwarded emails.

At some point, we should also introduce this worker in PVE -- then we
can use the same approach here for PVE as well. When this is done, we
gain the following advantages:

  - we don't need to bump proxmox-mail-forward if there are changes in
    proxmox-notify (as soon as PVE uses the same approach as well)

  - we can drop the notification context implementations from
    proxmox_notify and move them to the actual product code, where they
    belong

  - proxmox-mail-forward becomes extremely simple and light-weight --
    the only thing from proxmox-notify is the serializable Notification
    struct, which could be put into a separate crate or made available
    via a feature flag.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 Cargo.toml  |   3 +-
 src/main.rs | 101 +++++++++++++++++++++++++---------------------------
 2 files changed, 50 insertions(+), 54 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 6d13a84..3e12db4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -16,9 +16,10 @@ exclude = [ "debian" ]
 [dependencies]
 anyhow = "1.0"
 nix = "0.29"
+serde_json = "1.0"
 
 proxmox-log = "1"
-proxmox-notify = {version = "1", features = ["mail-forwarder", "pve-context", "pbs-context"] }
+proxmox-notify = {version = "1", features = ["mail-forwarder", "pve-context"] }
 proxmox-sys = "1"
 
 # Not directly used, this is just to force it to the new version:
diff --git a/src/main.rs b/src/main.rs
index a95d75c..0509cc4 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6,38 +6,29 @@
 //! The message is passed via stdin.
 //! The binary is installed with setuid permissions and will thus run as
 //! root (euid ~ root, ruid ~ nobody)
-//!
-//! The forwarding behavior is the following:
-//!   - PVE installed: Use PVE's notifications.cfg
-//!   - PBS installed: Use PBS's notifications.cfg if present. If not,
-//!     use an empty configuration and add a default sendmail target and
-//!     a matcher - this is needed because notifications are not yet
-//!     integrated in PBS.
-//!   - PVE/PBS co-installed: Use PVE's config *and* PBS's config, but if
-//!     PBS's config does not exist, a default sendmail target will *not* be
-//!     added. We assume that PVE's config contains the desired notification
-//!     behavior for system mails.
-//!
+
 use std::io::Read;
 use std::path::Path;
 
+use anyhow::Context;
 use anyhow::Error;
+use nix::unistd::User;
 
 use proxmox_log::LevelFilter;
 use proxmox_log::Logger;
 use proxmox_log::error;
 use proxmox_notify::Config;
-use proxmox_notify::context::pbs::PBS_CONTEXT;
 use proxmox_notify::context::pve::PVE_CONTEXT;
 use proxmox_sys::fs;
+use proxmox_sys::fs::CreateOptions;
 
 const PVE_CFG_PATH: &str = "/etc/pve";
 const PVE_PUB_NOTIFICATION_CFG_FILENAME: &str = "/etc/pve/notifications.cfg";
 const PVE_PRIV_NOTIFICATION_CFG_FILENAME: &str = "/etc/pve/priv/notifications.cfg";
 
+const PBS_UNPRIV_USER: &str = "backup";
 const PBS_CFG_PATH: &str = "/etc/proxmox-backup";
-const PBS_PUB_NOTIFICATION_CFG_FILENAME: &str = "/etc/proxmox-backup/notifications.cfg";
-const PBS_PRIV_NOTIFICATION_CFG_FILENAME: &str = "/etc/proxmox-backup/notifications-priv.cfg";
+const PBS_SPOOL_DIR: &str = "/var/lib/proxmox-backup/notifications/queue";
 
 /// Wrapper around `proxmox_sys::fs::file_read_optional_string` which also returns `None` upon error
 /// after logging it.
@@ -61,18 +52,6 @@ fn read_stdin() -> Result<Vec<u8>, Error> {
     Ok(input)
 }
 
-fn forward_common(mail: &[u8], config: &Config) -> Result<(), Error> {
-    let real_uid = nix::unistd::getuid();
-    // The uid is passed so that `sendmail` can be called as the a correct user.
-    // (sendmail will show a warning if called from a setuid process)
-    let notification =
-        proxmox_notify::Notification::new_forwarded_mail(mail, Some(real_uid.as_raw()))?;
-
-    proxmox_notify::api::common::send(config, &notification)?;
-
-    Ok(())
-}
-
 /// Forward a mail to PVE's notification system
 fn forward_for_pve(mail: &[u8]) -> Result<(), Error> {
     proxmox_notify::context::set_context(&PVE_CONTEXT);
@@ -80,40 +59,59 @@ fn forward_for_pve(mail: &[u8]) -> Result<(), Error> {
     let priv_config = attempt_file_read(PVE_PRIV_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
 
     let config = Config::new(&config, &priv_config)?;
+    let real_uid = nix::unistd::getuid();
 
-    forward_common(mail, &config)
+    // The uid is passed so that `sendmail` can be called as the a correct user.
+    // (sendmail will show a warning if called from a setuid process)
+    let notification =
+        proxmox_notify::Notification::new_forwarded_mail(mail, Some(real_uid.as_raw()))?;
+
+    proxmox_notify::api::common::send(&config, &notification)?;
+
+    Ok(())
 }
 
 /// Forward a mail to PBS's notification system
-fn forward_for_pbs(mail: &[u8], has_pve: bool) -> Result<(), Error> {
-    proxmox_notify::context::set_context(&PBS_CONTEXT);
+fn forward_for_pbs(mail: &[u8]) -> Result<(), Error> {
+    let unpriv_user = lookup_user(PBS_UNPRIV_USER)?;
+    let opts = CreateOptions::new()
+        .owner(unpriv_user.uid)
+        .group(unpriv_user.gid);
 
-    let config = if Path::new(PBS_PUB_NOTIFICATION_CFG_FILENAME).exists() {
-        let config = attempt_file_read(PBS_PUB_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
-        let priv_config = attempt_file_read(PBS_PRIV_NOTIFICATION_CFG_FILENAME).unwrap_or_default();
+    forward_via_spooldir(Path::new(PBS_SPOOL_DIR), opts, mail)
+}
 
-        Config::new(&config, &priv_config)?
-    } else {
-        // Instantiate empty config.
-        // Note: This will contain the default built-in targets/matchers.
-        let config = Config::new("", "")?;
-        if has_pve {
-            // Skip forwarding if we are co-installed with PVE AND
-            // we do not have our own notifications.cfg file yet
-            // --> We assume that PVE has a sane matcher configured that
-            // forwards the mail properly
-            // TODO: This can be removed once PBS has full notification integration
+/// Forward the email by queuing it in a spool directory.
+///
+/// In the product, there is a worker that periodically handles enqueued notifications stored
+/// in this directory and feeds them into `proxmox_notify`.
+fn forward_via_spooldir(
+    spooldir: &Path,
+    create_options: CreateOptions,
+    mail: &[u8],
+) -> Result<(), Error> {
+    let notification = proxmox_notify::Notification::new_forwarded_mail(mail, None)?;
+    let mut path = spooldir.join(notification.id().to_string());
+    path.set_extension("json");
 
-            return Ok(());
-        }
-        config
-    };
+    let data = serde_json::to_vec(&notification)?;
+    proxmox_sys::fs::replace_file(path, &data, create_options, true)?;
 
-    forward_common(mail, &config)?;
+    proxmox_log::info!(
+        "queued forwarded mail for later consumption by notification worker (id={id})",
+        id = notification.id()
+    );
 
     Ok(())
 }
 
+/// Helper to look up a [`nix::unistd::User`] from a given username.
+fn lookup_user(username: &str) -> Result<User, Error> {
+    User::from_name(username)
+        .with_context(|| format!("unable to lookup '{username}' user"))?
+        .with_context(|| format!("'{username}' user does not exist"))
+}
+
 fn main() {
     if let Err(err) = Logger::from_env("PROXMOX_LOG", LevelFilter::INFO)
         .journald()
@@ -125,11 +123,8 @@ fn main() {
     // Read the mail that is to be forwarded from stdin
     match read_stdin() {
         Ok(mail) => {
-            let mut has_pve = false;
-
             // Assume a PVE installation if /etc/pve exists
             if Path::new(PVE_CFG_PATH).exists() {
-                has_pve = true;
                 if let Err(err) = forward_for_pve(&mail) {
                     error!("could not forward mail for Proxmox VE: {err:#}");
                 }
@@ -137,7 +132,7 @@ fn main() {
 
             // Assume a PBS installation if /etc/proxmox-backup exists
             if Path::new(PBS_CFG_PATH).exists() {
-                if let Err(err) = forward_for_pbs(&mail, has_pve) {
+                if let Err(err) = forward_for_pbs(&mail) {
                     error!("could not forward mail for Proxmox Backup Server: {err:#}");
                 }
             }
-- 
2.47.3





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH proxmox-mail-forward 4/5] forward mails on PDM installations as well
  2026-04-09 13:27 [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility Lukas Wagner
                   ` (2 preceding siblings ...)
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker Lukas Wagner
@ 2026-04-09 13:27 ` Lukas Wagner
  2026-04-09 15:06   ` Arthur Bied-Charreton
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 5/5] only ever forward for one product, favoring PVE over PBS over PDM Lukas Wagner
  4 siblings, 1 reply; 11+ messages in thread
From: Lukas Wagner @ 2026-04-09 13:27 UTC (permalink / raw)
  To: pdm-devel, pbs-devel

Use the same approach as for PBS installations, namely by storing the
notification in a spool directory which is then later handled by a
worker in the PDM proxy process.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/main.rs | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/main.rs b/src/main.rs
index 0509cc4..92a2804 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -30,6 +30,10 @@ const PBS_UNPRIV_USER: &str = "backup";
 const PBS_CFG_PATH: &str = "/etc/proxmox-backup";
 const PBS_SPOOL_DIR: &str = "/var/lib/proxmox-backup/notifications/queue";
 
+const PDM_UNPRIV_USER: &str = "www-data";
+const PDM_CFG_PATH: &str = "/etc/proxmox-datacenter-manager";
+const PDM_SPOOL_DIR: &str = "/var/lib/proxmox-datacenter-manager/notifications/queue";
+
 /// Wrapper around `proxmox_sys::fs::file_read_optional_string` which also returns `None` upon error
 /// after logging it.
 fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
@@ -81,6 +85,16 @@ fn forward_for_pbs(mail: &[u8]) -> Result<(), Error> {
     forward_via_spooldir(Path::new(PBS_SPOOL_DIR), opts, mail)
 }
 
+/// Forward a mail to PDM's notification system
+fn forward_for_pdm(mail: &[u8]) -> Result<(), Error> {
+    let unpriv_user = lookup_user(PDM_UNPRIV_USER)?;
+    let opts = CreateOptions::new()
+        .owner(unpriv_user.uid)
+        .group(unpriv_user.gid);
+
+    forward_via_spooldir(Path::new(PDM_SPOOL_DIR), opts, mail)
+}
+
 /// Forward the email by queuing it in a spool directory.
 ///
 /// In the product, there is a worker that periodically handles enqueued notifications stored
@@ -136,6 +150,13 @@ fn main() {
                     error!("could not forward mail for Proxmox Backup Server: {err:#}");
                 }
             }
+
+            // Assume a PDM installation if /etc/proxmox-datacenter-manager exists
+            if Path::new(PDM_CFG_PATH).exists() {
+                if let Err(err) = forward_for_pdm(&mail) {
+                    error!("could not forward mail for Proxmox Datacenter Manager: {err:#}");
+                }
+            }
         }
         Err(err) => {
             error!("could not read mail from STDIN: {err:#}")
-- 
2.47.3





^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH proxmox-mail-forward 5/5] only ever forward for one product, favoring PVE over PBS over PDM
  2026-04-09 13:27 [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility Lukas Wagner
                   ` (3 preceding siblings ...)
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 4/5] forward mails on PDM installations as well Lukas Wagner
@ 2026-04-09 13:27 ` Lukas Wagner
  2026-04-09 15:15   ` Arthur Bied-Charreton
  4 siblings, 1 reply; 11+ messages in thread
From: Lukas Wagner @ 2026-04-09 13:27 UTC (permalink / raw)
  To: pdm-devel, pbs-devel

Co-installations of products should hopefully be not that common, but a
defined order might be easier to grasp for users than having to deal
with notifications being sent through multiple products.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/main.rs | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/main.rs b/src/main.rs
index 92a2804..b089bb0 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6,6 +6,9 @@
 //! The message is passed via stdin.
 //! The binary is installed with setuid permissions and will thus run as
 //! root (euid ~ root, ruid ~ nobody)
+//!
+//! If multiple products are co-installed, only one of the products is ever responsible for
+//! forwarding the email. The order is PVE > PBS > PDM.
 
 use std::io::Read;
 use std::path::Path;
@@ -137,22 +140,18 @@ fn main() {
     // Read the mail that is to be forwarded from stdin
     match read_stdin() {
         Ok(mail) => {
-            // Assume a PVE installation if /etc/pve exists
             if Path::new(PVE_CFG_PATH).exists() {
+                // Assume a PVE installation if /etc/pve exists
                 if let Err(err) = forward_for_pve(&mail) {
                     error!("could not forward mail for Proxmox VE: {err:#}");
                 }
-            }
-
-            // Assume a PBS installation if /etc/proxmox-backup exists
-            if Path::new(PBS_CFG_PATH).exists() {
+            } else if Path::new(PBS_CFG_PATH).exists() {
+                // Assume a PBS installation if /etc/proxmox-backup exists
                 if let Err(err) = forward_for_pbs(&mail) {
                     error!("could not forward mail for Proxmox Backup Server: {err:#}");
                 }
-            }
-
-            // Assume a PDM installation if /etc/proxmox-datacenter-manager exists
-            if Path::new(PDM_CFG_PATH).exists() {
+            } else if Path::new(PDM_CFG_PATH).exists() {
+                // Assume a PDM installation if /etc/proxmox-datacenter-manager exists
                 if let Err(err) = forward_for_pdm(&mail) {
                     error!("could not forward mail for Proxmox Datacenter Manager: {err:#}");
                 }
-- 
2.47.3





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue
  2026-04-09 13:27 ` [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue Lukas Wagner
@ 2026-04-09 14:52   ` Arthur Bied-Charreton
  0 siblings, 0 replies; 11+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-09 14:52 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pdm-devel, pbs-devel

On Thu, Apr 09, 2026 at 03:27:17PM +0200, Lukas Wagner wrote:
> In anticipation of more notification-related data that might be stored
> soon (notification history, oauth tokens, etc.), we move the spool
> directory that is used to queue notifications that are sent by the proxy
> process to a separate subdirectory.
> 
> We still read from the old directory, for the rare cases that there
> might still be any unsent notifications before the package is upgraded.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  src/server/notifications/mod.rs | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/src/server/notifications/mod.rs b/src/server/notifications/mod.rs
> index 95ff9ef18..15de5ccc5 100644
> --- a/src/server/notifications/mod.rs
> +++ b/src/server/notifications/mod.rs
> @@ -18,7 +18,11 @@ use pbs_api_types::{
>  use proxmox_notify::endpoints::sendmail::{SendmailConfig, SendmailEndpoint};
>  use proxmox_notify::{Endpoint, Notification, Severity};
>  
> -const SPOOL_DIR: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
> +const SPOOL_DIR_OLD: &str = concatcp!(pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR, "/notifications");
> +const SPOOL_DIR: &str = concatcp!(
> +    pbs_buildcfg::PROXMOX_BACKUP_STATE_DIR,
> +    "/notifications/queue"
> +);
>  
>  mod template_data;
>  
> @@ -43,14 +47,15 @@ pub fn create_spool_dir() -> Result<(), Error> {
>          .owner(backup_user.uid)
>          .group(backup_user.gid);
>  
> -    create_path(SPOOL_DIR, None, Some(opts))?;
> +    create_path(SPOOL_DIR, Some(opts), Some(opts))?;
>      Ok(())
>  }
>  
> -async fn send_queued_notifications() -> Result<(), Error> {
> -    let mut read_dir = tokio::fs::read_dir(SPOOL_DIR).await?;
> -
> -    let mut notifications = Vec::new();
> +async fn read_notifications_from_spool_dir(
> +    notifications: &mut Vec<Notification>,
> +    path: &str,
> +) -> Result<(), Error> {
> +    let mut read_dir = tokio::fs::read_dir(path).await?;
>  
>      while let Some(entry) = read_dir.next_entry().await? {
>          let path = entry.path();
> @@ -71,6 +76,19 @@ async fn send_queued_notifications() -> Result<(), Error> {
>          }
>      }
>  
> +    Ok(())
> +}
> +
> +async fn send_queued_notifications() -> Result<(), Error> {
> +    let mut notifications = Vec::new();
> +
> +    // Also read left-over notifications from the old directory -- we should be able to remove this
> +    // pretty soon.
> +    // FIXME: Remove this with the next minor update.
> +    read_notifications_from_spool_dir(&mut notifications, SPOOL_DIR_OLD).await?;
> +
> +    read_notifications_from_spool_dir(&mut notifications, SPOOL_DIR).await?;
> +
>      if notifications.is_empty() {
>          return Ok(());
>      }
> -- 
> 2.47.3
> 
> 
> 
> 
> 
Tested with both the old & the new proxmox-mail-forward binaries, lgtm

Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH proxmox-backup 2/5] pull in 'mail-forwarder' feature for proxmox-notify
  2026-04-09 13:27 ` [PATCH proxmox-backup 2/5] pull in 'mail-forwarder' feature for proxmox-notify Lukas Wagner
@ 2026-04-09 14:54   ` Arthur Bied-Charreton
  0 siblings, 0 replies; 11+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-09 14:54 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pdm-devel, pbs-devel

On Thu, Apr 09, 2026 at 03:27:18PM +0200, Lukas Wagner wrote:
> Since the notification worker is now in charge of sending notifications
> which were queued by proxmox-mail-forward.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  Cargo.toml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Cargo.toml b/Cargo.toml
> index 2347e85e1..448b775e0 100644
> --- a/Cargo.toml
> +++ b/Cargo.toml
> @@ -233,7 +233,7 @@ proxmox-ldap.workspace = true
>  proxmox-metrics.workspace = true
>  proxmox-network-api = { workspace = true, features = [ "impl" ] }
>  proxmox-network-types.workspace = true
> -proxmox-notify = { workspace = true, features = [ "pbs-context" ] }
> +proxmox-notify = { workspace = true, features = [ "pbs-context", "mail-forwarder" ] }
>  proxmox-openid.workspace = true
>  proxmox-product-config.workspace = true
>  proxmox-rate-limiter = { workspace = true, features = [ "shared-rate-limiter" ] }
> -- 
> 2.47.3
> 
> 
> 
> 
>
compiles with all features permutations, lgtm

Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker Lukas Wagner
@ 2026-04-09 15:03   ` Arthur Bied-Charreton
  0 siblings, 0 replies; 11+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-09 15:03 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pdm-devel, pbs-devel

On Thu, Apr 09, 2026 at 03:27:19PM +0200, Lukas Wagner wrote:
> Instead of sending the notification ourselves, use the fact that we
> already have a 'notification queue' handled by a dedicated worker task
> in PBS. Up until now, it was only used for sending notifications from
> the unprivileged proxy process, but nothing speaks against doing the
> same for forwarded emails.
> 
> At some point, we should also introduce this worker in PVE -- then we
> can use the same approach here for PVE as well. When this is done, we
> gain the following advantages:
> 
>   - we don't need to bump proxmox-mail-forward if there are changes in
>     proxmox-notify (as soon as PVE uses the same approach as well)
> 
>   - we can drop the notification context implementations from
>     proxmox_notify and move them to the actual product code, where they
>     belong
yay!
> 
>   - proxmox-mail-forward becomes extremely simple and light-weight --
>     the only thing from proxmox-notify is the serializable Notification
>     struct, which could be put into a separate crate or made available
>     via a feature flag.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
[...]

Tested these changes on both PVE and PBS, did not run into any issues.
As mentioned in the cover letter, there is breakage when using an
upgraded proxmox-mail-forward on a non-upgraded PBS instance (i.e. 
that does not yet have the 
/var/lib/proxmox-backup/notifications/queue directory).

Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH proxmox-mail-forward 4/5] forward mails on PDM installations as well
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 4/5] forward mails on PDM installations as well Lukas Wagner
@ 2026-04-09 15:06   ` Arthur Bied-Charreton
  0 siblings, 0 replies; 11+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-09 15:06 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pdm-devel, pbs-devel

On Thu, Apr 09, 2026 at 03:27:20PM +0200, Lukas Wagner wrote:
> Use the same approach as for PBS installations, namely by storing the
> notification in a spool directory which is then later handled by a
> worker in the PDM proxy process.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
[...]

Looks good to me (obviously not tested end-to-end since we do not yet 
have notifications in PDM)

Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH proxmox-mail-forward 5/5] only ever forward for one product, favoring PVE over PBS over PDM
  2026-04-09 13:27 ` [PATCH proxmox-mail-forward 5/5] only ever forward for one product, favoring PVE over PBS over PDM Lukas Wagner
@ 2026-04-09 15:15   ` Arthur Bied-Charreton
  0 siblings, 0 replies; 11+ messages in thread
From: Arthur Bied-Charreton @ 2026-04-09 15:15 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pdm-devel, pbs-devel

On Thu, Apr 09, 2026 at 03:27:21PM +0200, Lukas Wagner wrote:
> Co-installations of products should hopefully be not that common, but a
> defined order might be easier to grasp for users than having to deal
> with notifications being sent through multiple products.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
[...]

Tested by creating the different config files on a PBS installation,
lgtm.

Tested-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
Reviewed-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-04-09 15:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-09 13:27 [PATCH proxmox{-backup,-mail-forward} 0/5] forward mails through notification worker; PDM compatibility Lukas Wagner
2026-04-09 13:27 ` [PATCH proxmox-backup 1/5] notifications: move spool directory to /var/lib/proxmox-backup/notifications/queue Lukas Wagner
2026-04-09 14:52   ` Arthur Bied-Charreton
2026-04-09 13:27 ` [PATCH proxmox-backup 2/5] pull in 'mail-forwarder' feature for proxmox-notify Lukas Wagner
2026-04-09 14:54   ` Arthur Bied-Charreton
2026-04-09 13:27 ` [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker Lukas Wagner
2026-04-09 15:03   ` Arthur Bied-Charreton
2026-04-09 13:27 ` [PATCH proxmox-mail-forward 4/5] forward mails on PDM installations as well Lukas Wagner
2026-04-09 15:06   ` Arthur Bied-Charreton
2026-04-09 13:27 ` [PATCH proxmox-mail-forward 5/5] only ever forward for one product, favoring PVE over PBS over PDM Lukas Wagner
2026-04-09 15:15   ` Arthur Bied-Charreton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal