public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: pdm-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com
Subject: [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker
Date: Thu,  9 Apr 2026 15:27:19 +0200	[thread overview]
Message-ID: <20260409132721.272178-4-l.wagner@proxmox.com> (raw)
In-Reply-To: <20260409132721.272178-1-l.wagner@proxmox.com>

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





  parent reply	other threads:[~2026-04-09 13:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 ` Lukas Wagner [this message]
2026-04-09 15:03   ` [PATCH proxmox-mail-forward 3/5] forward using PBS' notification worker 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260409132721.272178-4-l.wagner@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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