public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify
@ 2023-10-02  8:06 Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2 Lukas Wagner
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

The aim of this patch series is to adapt `proxmox-mail-forward` 
so that it forwards emails that were sent to the local root user
through the `proxmox_notify` crate.

A short summary of the status quo:
Any mail that is sent to the local `root` user is forwarded by
postfix to the `proxmox-mail-forward` binary, which receives the
mail via STDIN. `proxmox-mail-forward` looks up the email address 
configured for the `root@pam` user in /etc/{proxmox-backup,pve}/user.cfg 
and then forwards the mail to this address by calling `sendmail`

This patch series modifies `proxmox-mail-forward` in the following way:
`proxmox-mail-forward` instantiates the configuration for `proxmox_notify`
by reading `/etc/{proxmox-backup,pve}/notifications.cfg. Also, it looks 
up the policy for system mail (target/if to forward at all) in 
`node.cfg/datacenter.cfg`. Following that, the mail is passed to 
`proxmox_notify`, which sends it to the specified target(s).
If no target is configured/configuration files do not exist, then
the mail is forwarded using the `mail-to-root` target, which always exists.
In this way the changes should be 100% backwards compatible.

One small change in behavior can occur if PBS is co-installed on a PVE host.
Here it could happen that a mail is forwarded twice: Once for for 
notification configuration for PVE, and once for the config for PBS.
Unfortunately there is no easy way to perform any useful 'deduplication'
there (by target name does not really work, since they could have different
configuration/recipients; by 'mail-address' would work for mail-based targets,
however this involves some pretty invasive changes and still does not work for 
targets that are not mail-based). 

Personally I feel that we should just add  a section about this behavior 
in the docs (once proxmox_notify is fully integrated in PBS), 
instructing the user to set `system-mail` to `never` in `node.cfg` 
(don't forward mails). Alternatively we could try to detect co-installations
and only forward for the target of one of both products. 
However, I prefer the first option.

`proxmox-notify` now depends on a new crate `mail-parser` to parse 
email headers (something I *really* don't want to implement myself
from scratch). The new dependency is not packaged yet, the necessary
debcargo-conf changes are included in the first patch.
@TESTERS: I can provide a pre-built deb for `mail-parser`.

Changelog:
  - v1 -> v2:
    - Rebased
    - Apply the same fix for the PVE context as in [1]


[1] https://lists.proxmox.com/pipermail/pve-devel/2023-October/059294.html



debcargo-conf:

Lukas Wagner (1):
  package mail-parser 0.8.2

 src/mail-parser/debian/changelog              |  6 ++
 src/mail-parser/debian/copyright              | 49 ++++++++++++
 .../debian/copyright.debcargo.hint            | 77 +++++++++++++++++++
 src/mail-parser/debian/debcargo.toml          |  2 +
 4 files changed, 134 insertions(+)
 create mode 100644 src/mail-parser/debian/changelog
 create mode 100644 src/mail-parser/debian/copyright
 create mode 100644 src/mail-parser/debian/copyright.debcargo.hint
 create mode 100644 src/mail-parser/debian/debcargo.toml


proxmox:

Lukas Wagner (4):
  sys: email: add `forward`
  notify: introduce Error::Generic
  notify: add mechanisms for email message forwarding
  notify: add PVE/PBS context

 Cargo.toml                                    |   1 +
 proxmox-notify/Cargo.toml                     |   5 +-
 proxmox-notify/src/context/common.rs          |  27 ++++
 .../src/{context.rs => context/mod.rs}        |  14 +-
 proxmox-notify/src/context/pbs.rs             | 130 ++++++++++++++++++
 proxmox-notify/src/context/pve.rs             |  82 +++++++++++
 proxmox-notify/src/endpoints/gotify.rs        |  21 +--
 proxmox-notify/src/endpoints/sendmail.rs      |  62 +++++----
 proxmox-notify/src/filter.rs                  |   8 +-
 proxmox-notify/src/lib.rs                     | 109 +++++++++++++--
 proxmox-sys/src/email.rs                      |  52 ++++++-
 11 files changed, 451 insertions(+), 60 deletions(-)
 create mode 100644 proxmox-notify/src/context/common.rs
 rename proxmox-notify/src/{context.rs => context/mod.rs} (54%)
 create mode 100644 proxmox-notify/src/context/pbs.rs
 create mode 100644 proxmox-notify/src/context/pve.rs


proxmox-perl-rs:

Lukas Wagner (2):
  notify: construct Notification via constructor
  pve-rs: notify: remove notify_context for PVE

 common/src/notify.rs         |   8 +--
 pve-rs/Cargo.toml            |   2 +-
 pve-rs/src/lib.rs            |   7 ++-
 pve-rs/src/notify_context.rs | 117 -----------------------------------
 4 files changed, 6 insertions(+), 128 deletions(-)
 delete mode 100644 pve-rs/src/notify_context.rs


pve-cluster:

Lukas Wagner (1):
  datacenter config: add new parameters for system mail forwarding

 src/PVE/DataCenterConfig.pm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)


pve-manager:

Lukas Wagner (1):
  ui: notify: add system-mail settings, configuring mail forwarding

 www/manager6/dc/NotificationEvents.js | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)


proxmox-mail-forward:

Lukas Wagner (1):
  feed forwarded mails into proxmox_notify

 Cargo.toml  |   8 +-
 src/main.rs | 348 +++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 238 insertions(+), 118 deletions(-)


pve-docs:

Lukas Wagner (1):
  notification: add docs for system mail forwarding

 notifications.adoc | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)


Summary over all repositories:
  24 files changed, 899 insertions(+), 313 deletions(-)

-- 
murpp v0.4.0





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

* [pve-devel] [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-20 15:36   ` [pve-devel] applied: " Thomas Lamprecht
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 02/11] sys: email: add `forward` Lukas Wagner
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/mail-parser/debian/changelog              |  6 ++
 src/mail-parser/debian/copyright              | 49 ++++++++++++
 .../debian/copyright.debcargo.hint            | 77 +++++++++++++++++++
 src/mail-parser/debian/debcargo.toml          |  2 +
 4 files changed, 134 insertions(+)
 create mode 100644 src/mail-parser/debian/changelog
 create mode 100644 src/mail-parser/debian/copyright
 create mode 100644 src/mail-parser/debian/copyright.debcargo.hint
 create mode 100644 src/mail-parser/debian/debcargo.toml

diff --git a/src/mail-parser/debian/changelog b/src/mail-parser/debian/changelog
new file mode 100644
index 000000000..2ed51934d
--- /dev/null
+++ b/src/mail-parser/debian/changelog
@@ -0,0 +1,6 @@
+rust-mail-parser (0.8.2-1) UNRELEASED-FIXME-AUTOGENERATED-DEBCARGO; urgency=medium
+
+  * Team upload.
+  * Package mail-parser 0.8.2 from crates.io using debcargo 2.6.0
+
+ -- Lukas Wagner <l.wagner@proxmox.com>  Fri, 25 Aug 2023 14:10:10 +0200
diff --git a/src/mail-parser/debian/copyright b/src/mail-parser/debian/copyright
new file mode 100644
index 000000000..28fc2722c
--- /dev/null
+++ b/src/mail-parser/debian/copyright
@@ -0,0 +1,49 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+Upstream-Name: mail-parser
+Upstream-Contact: Stalwart Labs <hello@stalw.art>
+Source: https://github.com/stalwartlabs/mail-parser
+
+Files: *
+Copyright: 2020-2022 Stalwart Labs <hello@stalw.art>
+License: Apache-2.0 or MIT
+
+Files: tests/legacy/*
+Copyright: 2010 Hunny Software, Inc.
+License: UNKNOWN-LICENSE;
+
+Files: tests/malformed/*
+Copyright: 2003-2018 Dovecot authors, licensed under MIT.
+License: MIT
+
+Files: tests/thirdparty/*
+Copyright: 2003-2018 Dovecot authors, licensed under MIT.
+License: MIT
+
+Files: debian/*
+Copyright:
+ 2023 Debian Rust Maintainers <pkg-rust-maintainers@alioth-lists.debian.net>
+ 2023 Lukas Wagner <l.wagner@proxmox.com>
+License: Apache-2.0 or MIT
+
+License: Apache-2.0
+ Debian systems provide the Apache 2.0 license in
+ /usr/share/common-licenses/Apache-2.0
+
+License: MIT
+ Permission is hereby granted, free of charge, to any person obtaining a copy
+ of this software and associated documentation files (the "Software"), to deal
+ in the Software without restriction, including without limitation the rights
+ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ copies of the Software, and to permit persons to whom the Software is
+ furnished to do so, subject to the following conditions:
+ .
+ The above copyright notice and this permission notice shall be included in all
+ copies or substantial portions of the Software.
+ .
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
+ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ SOFTWARE.
diff --git a/src/mail-parser/debian/copyright.debcargo.hint b/src/mail-parser/debian/copyright.debcargo.hint
new file mode 100644
index 000000000..f0379206a
--- /dev/null
+++ b/src/mail-parser/debian/copyright.debcargo.hint
@@ -0,0 +1,77 @@
+Format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/
+Upstream-Name: mail-parser
+Upstream-Contact: Stalwart Labs <hello@stalw.art>
+Source: https://github.com/stalwartlabs/mail-parser
+
+Files: *
+Copyright: FIXME (overlay) UNKNOWN-YEARS Stalwart Labs <hello@stalw.art>
+License: Apache-2.0 or MIT
+Comment:
+ FIXME (overlay): Since upstream copyright years are not available in
+ Cargo.toml, they were extracted from the upstream Git repository. This may not
+ be correct information so you should review and fix this before uploading to
+ the archive.
+
+Files: README.md
+Copyright: 2020-2022, Stalwart Labs Ltd.
+License: UNKNOWN-LICENSE; FIXME (overlay)
+Comment:
+ FIXME (overlay): These notices are extracted from files. Please review them
+ before uploading to the archive.
+
+Files: src/decoders/base64.rs
+Copyright: 2005, 2006, 2007 Nick Galbreath -- nickg [at] modp [dot] com
+License: UNKNOWN-LICENSE; FIXME (overlay)
+Comment:
+ FIXME (overlay): These notices are extracted from files. Please review them
+ before uploading to the archive.
+
+Files: tests/legacy/COPYING
+Copyright: 2010 Hunny Software, Inc.
+License: UNKNOWN-LICENSE; FIXME (overlay)
+Comment:
+ FIXME (overlay): These notices are extracted from files. Please review them
+ before uploading to the archive.
+
+Files: tests/malformed/COPYING
+Copyright: 2003-2018 Dovecot authors, licensed under MIT.
+License: UNKNOWN-LICENSE; FIXME (overlay)
+Comment:
+ FIXME (overlay): These notices are extracted from files. Please review them
+ before uploading to the archive.
+
+Files: tests/thirdparty/COPYING
+Copyright: 2003-2018 Dovecot authors, licensed under MIT.
+License: UNKNOWN-LICENSE; FIXME (overlay)
+Comment:
+ FIXME (overlay): These notices are extracted from files. Please review them
+ before uploading to the archive.
+
+Files: debian/*
+Copyright:
+ 2023 Debian Rust Maintainers <pkg-rust-maintainers@alioth-lists.debian.net>
+ 2023 Lukas Wagner <l.wagner@proxmox.com>
+License: Apache-2.0 or MIT
+
+License: Apache-2.0
+ Debian systems provide the Apache 2.0 license in
+ /usr/share/common-licenses/Apache-2.0
+
+License: MIT
+ Permission is hereby granted, free of charge, to any person obtaining a copy
+ of this software and associated documentation files (the "Software"), to deal
+ in the Software without restriction, including without limitation the rights
+ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ copies of the Software, and to permit persons to whom the Software is
+ furnished to do so, subject to the following conditions:
+ .
+ The above copyright notice and this permission notice shall be included in all
+ copies or substantial portions of the Software.
+ .
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
+ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ SOFTWARE.
diff --git a/src/mail-parser/debian/debcargo.toml b/src/mail-parser/debian/debcargo.toml
new file mode 100644
index 000000000..d69a1a27b
--- /dev/null
+++ b/src/mail-parser/debian/debcargo.toml
@@ -0,0 +1,2 @@
+overlay = "."
+uploaders = ["Lukas Wagner <l.wagner@proxmox.com>"]
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox 02/11] sys: email: add `forward`
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2 Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 03/11] notify: introduce Error::Generic Lukas Wagner
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

This new function forwards an email to new recipients.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-sys/src/email.rs | 52 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/proxmox-sys/src/email.rs b/proxmox-sys/src/email.rs
index 8b3a1b6..c94f634 100644
--- a/proxmox-sys/src/email.rs
+++ b/proxmox-sys/src/email.rs
@@ -3,7 +3,7 @@
 use std::io::Write;
 use std::process::{Command, Stdio};
 
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
 
 /// Sends multi-part mail with text and/or html to a list of recipients
 ///
@@ -110,6 +110,56 @@ pub fn sendmail(
     Ok(())
 }
 
+/// Forwards an email message to a given list of recipients.
+///
+/// ``sendmail`` is used for sending the mail, thus `message` must be
+/// compatible with that (the message is piped into stdin unmodified).
+pub fn forward(
+    mailto: &[&str],
+    mailfrom: &str,
+    message: &[u8],
+    uid: Option<u32>,
+) -> Result<(), Error> {
+    use std::os::unix::process::CommandExt;
+
+    if mailto.is_empty() {
+        bail!("At least one recipient has to be specified!")
+    }
+
+    let mut builder = Command::new("/usr/sbin/sendmail");
+
+    builder
+        .args([
+            "-N", "never", // never send DSN (avoid mail loops)
+            "-f", mailfrom, "--",
+        ])
+        .args(mailto)
+        .stdin(Stdio::piped())
+        .stdout(Stdio::null())
+        .stderr(Stdio::null());
+
+    if let Some(uid) = uid {
+        builder.uid(uid);
+    }
+
+    let mut process = builder
+        .spawn()
+        .map_err(|err| format_err!("could not spawn sendmail process: {err}"))?;
+
+    process
+        .stdin
+        .take()
+        .unwrap()
+        .write_all(message)
+        .map_err(|err| format_err!("couldn't write to sendmail stdin: {err}"))?;
+
+    process
+        .wait()
+        .map_err(|err| format_err!("sendmail did not exit successfully: {err}"))?;
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod test {
     use crate::email::sendmail;
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox 03/11] notify: introduce Error::Generic
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2 Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 02/11] sys: email: add `forward` Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 04/11] notify: add mechanisms for email message forwarding Lukas Wagner
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

... as leaf error-type for anything for which we do not necessarily
want a separate enum variant.

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

diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 7500778..f7d480c 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -25,13 +25,22 @@ mod config;
 
 #[derive(Debug)]
 pub enum Error {
+    /// There was an error serializing the config
     ConfigSerialization(Box<dyn StdError + Send + Sync>),
+    /// There was an error deserializing the config
     ConfigDeserialization(Box<dyn StdError + Send + Sync>),
+    /// An endpoint failed to send a notification
     NotifyFailed(String, Box<dyn StdError + Send + Sync>),
+    /// A target does not exist
     TargetDoesNotExist(String),
+    /// Testing one or more notification targets failed
     TargetTestFailed(Vec<Box<dyn StdError + Send + Sync>>),
+    /// A filter could not be applied
     FilterFailed(String),
+    /// The notification's template string could not be rendered
     RenderError(Box<dyn StdError + Send + Sync>),
+    /// Generic error for anything else
+    Generic(String),
 }
 
 impl Display for Error {
@@ -60,6 +69,7 @@ impl Display for Error {
                 write!(f, "could not apply filter: {message}")
             }
             Error::RenderError(err) => write!(f, "could not render notification template: {err}"),
+            Error::Generic(message) => f.write_str(message),
         }
     }
 }
@@ -74,6 +84,7 @@ impl StdError for Error {
             Error::TargetTestFailed(errs) => Some(&*errs[0]),
             Error::FilterFailed(_) => None,
             Error::RenderError(err) => Some(&**err),
+            Error::Generic(_) => None,
         }
     }
 }
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox 04/11] notify: add mechanisms for email message forwarding
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (2 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 03/11] notify: introduce Error::Generic Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 05/11] notify: add PVE/PBS context Lukas Wagner
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

As preparation for the integration of `proxmox-mail-foward` into the
notification system, this commit makes a few changes that allow us to
forward raw email messages (as passed from postfix).

For mail-based notification targets, the email will be forwarded
as-is, including all headers. The only thing that changes is the
message envelope.
For other notification targets, the mail is parsed using the
`mail-parser` crate, which allows us to extract a subject and a body.
As a body we use the plain-text version of the mail. If an email is
HTML-only, the `mail-parser` crate will automatically attempt to
transform the HTML into readable plain text.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 Cargo.toml                               |  1 +
 proxmox-notify/Cargo.toml                |  2 +
 proxmox-notify/src/endpoints/gotify.rs   | 21 +++--
 proxmox-notify/src/endpoints/sendmail.rs | 62 ++++++++-------
 proxmox-notify/src/filter.rs             |  8 +-
 proxmox-notify/src/lib.rs                | 98 ++++++++++++++++++++----
 6 files changed, 138 insertions(+), 54 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index e334ac1..9adfe59 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -61,6 +61,7 @@ lazy_static = "1.4"
 ldap3 = { version = "0.11", default-features = false }
 libc = "0.2.107"
 log = "0.4.17"
+mail-parser = "0.8.2"
 native-tls = "0.2"
 nix = "0.26.1"
 once_cell = "1.3.1"
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 1541b8b..441b6e1 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -11,6 +11,7 @@ exclude.workspace = true
 handlebars = { workspace = true }
 lazy_static.workspace = true
 log.workspace = true
+mail-parser = { workspace = true, optional = true }
 once_cell.workspace = true
 openssl.workspace = true
 proxmox-http = { workspace = true, features = ["client-sync"], optional = true }
@@ -26,5 +27,6 @@ serde_json.workspace = true
 
 [features]
 default = ["sendmail", "gotify"]
+mail-forwarder = ["dep:mail-parser"]
 sendmail = ["dep:proxmox-sys"]
 gotify = ["dep:proxmox-http"]
diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs
index 83df41f..261573b 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater};
 use crate::context::context;
 use crate::renderer::TemplateRenderer;
 use crate::schema::ENTITY_NAME_SCHEMA;
-use crate::{renderer, Endpoint, Error, Notification, Severity};
+use crate::{renderer, Content, Endpoint, Error, Notification, Severity};
 
 fn severity_to_priority(level: Severity) -> u32 {
     match level {
@@ -87,13 +87,18 @@ impl Endpoint for GotifyEndpoint {
     fn send(&self, notification: &Notification) -> Result<(), Error> {
         let properties = notification.properties.as_ref();
 
-        let title = renderer::render_template(
-            TemplateRenderer::Plaintext,
-            &notification.title,
-            properties,
-        )?;
-        let message =
-            renderer::render_template(TemplateRenderer::Plaintext, &notification.body, properties)?;
+        let (title, message) = match &notification.content {
+            Content::Template { title, body } => {
+                let rendered_title =
+                    renderer::render_template(TemplateRenderer::Plaintext, title, properties)?;
+                let rendered_message =
+                    renderer::render_template(TemplateRenderer::Plaintext, body, properties)?;
+
+                (rendered_title, rendered_message)
+            }
+            #[cfg(feature = "mail-forwarder")]
+            Content::ForwardedMail { title, body, .. } => (title.clone(), body.clone()),
+        };
 
         // We don't have a TemplateRenderer::Markdown yet, so simply put everything
         // in code tags. Otherwise tables etc. are not formatted properly
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index 26e2a17..9cc3f31 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -8,7 +8,7 @@ use proxmox_schema::{api, Updater};
 use crate::context::context;
 use crate::renderer::TemplateRenderer;
 use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA};
-use crate::{renderer, Endpoint, Error, Notification};
+use crate::{renderer, Content, Endpoint, Error, Notification};
 
 pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail";
 
@@ -103,40 +103,44 @@ impl Endpoint for SendmailEndpoint {
         }
 
         let properties = notification.properties.as_ref();
-
-        let subject = renderer::render_template(
-            TemplateRenderer::Plaintext,
-            &notification.title,
-            properties,
-        )?;
-        let html_part =
-            renderer::render_template(TemplateRenderer::Html, &notification.body, properties)?;
-        let text_part =
-            renderer::render_template(TemplateRenderer::Plaintext, &notification.body, properties)?;
-
-        let author = self
-            .config
-            .author
-            .clone()
-            .unwrap_or_else(|| context().default_sendmail_author());
-
+        let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
         let mailfrom = self
             .config
             .from_address
             .clone()
             .unwrap_or_else(|| context().default_sendmail_from());
 
-        let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
-
-        proxmox_sys::email::sendmail(
-            &recipients_str,
-            &subject,
-            Some(&text_part),
-            Some(&html_part),
-            Some(&mailfrom),
-            Some(&author),
-        )
-        .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+        match &notification.content {
+            Content::Template { title, body } => {
+                let subject =
+                    renderer::render_template(TemplateRenderer::Plaintext, title, properties)?;
+                let html_part =
+                    renderer::render_template(TemplateRenderer::Html, body, properties)?;
+                let text_part =
+                    renderer::render_template(TemplateRenderer::Plaintext, body, properties)?;
+
+                let author = self
+                    .config
+                    .author
+                    .clone()
+                    .unwrap_or_else(|| context().default_sendmail_author());
+
+                proxmox_sys::email::sendmail(
+                    &recipients_str,
+                    &subject,
+                    Some(&text_part),
+                    Some(&html_part),
+                    Some(&mailfrom),
+                    Some(&author),
+                )
+                .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+            }
+            #[cfg(feature = "mail-forwarder")]
+            Content::ForwardedMail { raw, uid, .. } => {
+                proxmox_sys::email::forward(&recipients_str, &mailfrom, raw, *uid)
+                    .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+            }
+        }
     }
 
     fn name(&self) -> &str {
diff --git a/proxmox-notify/src/filter.rs b/proxmox-notify/src/filter.rs
index 748ec4e..d052512 100644
--- a/proxmox-notify/src/filter.rs
+++ b/proxmox-notify/src/filter.rs
@@ -160,7 +160,7 @@ impl<'a> FilterMatcher<'a> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::config;
+    use crate::{config, Content};
 
     fn parse_filters(config: &str) -> Result<Vec<FilterConfig>, Error> {
         let (config, _) = config::config(config)?;
@@ -169,8 +169,10 @@ mod tests {
 
     fn empty_notification_with_severity(severity: Severity) -> Notification {
         Notification {
-            title: String::new(),
-            body: String::new(),
+            content: Content::Template {
+                title: String::new(),
+                body: String::new(),
+            },
             severity,
             properties: Default::default(),
         }
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index f7d480c..eebc57a 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -116,17 +116,79 @@ pub trait Endpoint {
     fn filter(&self) -> Option<&str>;
 }
 
+#[derive(Debug, Clone)]
+pub enum Content {
+    /// Title and body will be rendered as a template
+    Template { title: String, body: String },
+    /// A special content type for forwarded mails. Contains the raw content of the original mail
+    /// as well as a (non-template) fallback title and body for endpoints that are not based
+    /// on email.
+    #[cfg(feature = "mail-forwarder")]
+    ForwardedMail {
+        /// Raw mail contents
+        raw: Vec<u8>,
+        /// Fallback title
+        title: String,
+        /// Fallback body
+        body: String,
+        /// UID to use when calling sendmail
+        #[allow(dead_code)] // Unused in some feature flag permutations
+        uid: Option<u32>,
+    },
+}
+
 #[derive(Debug, Clone)]
 /// Notification which can be sent
 pub struct Notification {
     /// Notification severity
-    pub severity: Severity,
-    /// The title of the notification
-    pub title: String,
-    /// Notification text
-    pub body: String,
+    severity: Severity,
+    /// Notification content
+    #[allow(dead_code)] // Unused in some feature flag permutations
+    content: Content,
     /// Additional metadata for the notification
-    pub properties: Option<Value>,
+    #[allow(dead_code)] // Unused in some feature flag permutations
+    properties: Option<Value>,
+}
+
+impl Notification {
+    pub fn new_templated<S: AsRef<str>>(
+        severity: Severity,
+        title: S,
+        body: S,
+        properties: Option<Value>,
+    ) -> Self {
+        Self {
+            severity,
+            content: Content::Template {
+                title: title.as_ref().to_string(),
+                body: body.as_ref().to_string(),
+            },
+            properties,
+        }
+    }
+
+    #[cfg(feature = "mail-forwarder")]
+    pub fn new_forwarded_mail(raw_mail: &[u8], uid: Option<u32>) -> Result<Self, Error> {
+        let message = mail_parser::Message::parse(raw_mail)
+            .ok_or_else(|| Error::Generic("could not parse forwarded email".to_string()))?;
+
+        let title = message.subject().unwrap_or_default().into();
+        let body = message.body_text(0).unwrap_or_default().into();
+
+        Ok(Self {
+            // Unfortunately we cannot reasonably infer the severity from the
+            // mail contents, so just set it to the highest for now so that
+            // it is not filtered out.
+            severity: Severity::Error,
+            content: Content::ForwardedMail {
+                raw: raw_mail.into(),
+                title,
+                body,
+                uid,
+            },
+            properties: None,
+        })
+    }
 }
 
 /// Notification configuration
@@ -384,8 +446,10 @@ impl Bus {
     pub fn test_target(&self, target: &str) -> Result<(), Error> {
         let notification = Notification {
             severity: Severity::Info,
-            title: "Test notification".into(),
-            body: "This is a test of the notification target '{{ target }}'".into(),
+            content: Content::Template {
+                title: "Test notification".into(),
+                body: "This is a test of the notification target '{{ target }}'".into(),
+            },
             properties: Some(json!({ "target": target })),
         };
 
@@ -474,8 +538,10 @@ mod tests {
         bus.send(
             "endpoint",
             &Notification {
-                title: "Title".into(),
-                body: "Body".into(),
+                content: Content::Template {
+                    title: "Title".into(),
+                    body: "Body".into(),
+                },
                 severity: Severity::Info,
                 properties: Default::default(),
             },
@@ -514,8 +580,10 @@ mod tests {
             bus.send(
                 channel,
                 &Notification {
-                    title: "Title".into(),
-                    body: "Body".into(),
+                    content: Content::Template {
+                        title: "Title".into(),
+                        body: "Body".into(),
+                    },
                     severity: Severity::Info,
                     properties: Default::default(),
                 },
@@ -582,8 +650,10 @@ mod tests {
             bus.send(
                 "channel1",
                 &Notification {
-                    title: "Title".into(),
-                    body: "Body".into(),
+                    content: Content::Template {
+                        title: "Title".into(),
+                        body: "Body".into(),
+                    },
                     severity,
                     properties: Default::default(),
                 },
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox 05/11] notify: add PVE/PBS context
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (3 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 04/11] notify: add mechanisms for email message forwarding Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-perl-rs 06/11] notify: construct Notification via constructor Lukas Wagner
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

This commit moves PVEContext from `proxmox-perl-rs` into the
`proxmox-notify` crate, since we now also need to access it from
`promxox-mail-forward`. The context is now hidden behind a feature
flag `pve-context`, ensuring that we only compile it when needed.

This commit adds PBSContext, since we now require it for
`proxmox-mail-forward`.

This commit also changes the global context from being stored in a
`once_cell` to a regular `Mutex`, since we now need to set/reset
the context in `proxmox-mail-forward`.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-notify/Cargo.toml                     |   3 +-
 proxmox-notify/src/context/common.rs          |  27 ++++
 .../src/{context.rs => context/mod.rs}        |  14 +-
 proxmox-notify/src/context/pbs.rs             | 130 ++++++++++++++++++
 proxmox-notify/src/context/pve.rs             |  82 +++++++++++
 5 files changed, 251 insertions(+), 5 deletions(-)
 create mode 100644 proxmox-notify/src/context/common.rs
 rename proxmox-notify/src/{context.rs => context/mod.rs} (54%)
 create mode 100644 proxmox-notify/src/context/pbs.rs
 create mode 100644 proxmox-notify/src/context/pve.rs

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 441b6e1..8d8caaf 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -12,7 +12,6 @@ handlebars = { workspace = true }
 lazy_static.workspace = true
 log.workspace = true
 mail-parser = { workspace = true, optional = true }
-once_cell.workspace = true
 openssl.workspace = true
 proxmox-http = { workspace = true, features = ["client-sync"], optional = true }
 proxmox-http-error.workspace = true
@@ -30,3 +29,5 @@ default = ["sendmail", "gotify"]
 mail-forwarder = ["dep:mail-parser"]
 sendmail = ["dep:proxmox-sys"]
 gotify = ["dep:proxmox-http"]
+pve-context = ["dep:proxmox-sys"]
+pbs-context = ["dep:proxmox-sys"]
diff --git a/proxmox-notify/src/context/common.rs b/proxmox-notify/src/context/common.rs
new file mode 100644
index 0000000..7580bd1
--- /dev/null
+++ b/proxmox-notify/src/context/common.rs
@@ -0,0 +1,27 @@
+use std::path::Path;
+
+pub(crate) fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
+    match proxmox_sys::fs::file_read_optional_string(path) {
+        Ok(contents) => contents,
+        Err(err) => {
+            log::error!("{err}");
+            None
+        }
+    }
+}
+
+pub(crate) fn lookup_datacenter_config_key(content: &str, key: &str) -> Option<String> {
+    let key_prefix = format!("{key}:");
+    normalize_for_return(
+        content
+            .lines()
+            .find_map(|line| line.strip_prefix(&key_prefix)),
+    )
+}
+
+pub(crate) fn normalize_for_return(s: Option<&str>) -> Option<String> {
+    match s?.trim() {
+        "" => None,
+        s => Some(s.to_string()),
+    }
+}
diff --git a/proxmox-notify/src/context.rs b/proxmox-notify/src/context/mod.rs
similarity index 54%
rename from proxmox-notify/src/context.rs
rename to proxmox-notify/src/context/mod.rs
index 370c7ee..00de2b0 100644
--- a/proxmox-notify/src/context.rs
+++ b/proxmox-notify/src/context/mod.rs
@@ -1,6 +1,12 @@
 use std::fmt::Debug;
+use std::sync::Mutex;
 
-use once_cell::sync::OnceCell;
+#[cfg(any(feature = "pve-context", feature = "pbs-context"))]
+pub mod common;
+#[cfg(feature = "pbs-context")]
+pub mod pbs;
+#[cfg(feature = "pve-context")]
+pub mod pve;
 
 pub trait Context: Send + Sync + Debug {
     fn lookup_email_for_user(&self, user: &str) -> Option<String>;
@@ -9,13 +15,13 @@ pub trait Context: Send + Sync + Debug {
     fn http_proxy_config(&self) -> Option<String>;
 }
 
-static CONTEXT: OnceCell<&'static dyn Context> = OnceCell::new();
+static CONTEXT: Mutex<Option<&'static dyn Context>> = Mutex::new(None);
 
 pub fn set_context(context: &'static dyn Context) {
-    CONTEXT.set(context).expect("context has already been set");
+    *CONTEXT.lock().unwrap() = Some(context);
 }
 
 #[allow(unused)] // context is not used if all endpoint features are disabled
 pub(crate) fn context() -> &'static dyn Context {
-    *CONTEXT.get().expect("context has not been yet")
+    (*CONTEXT.lock().unwrap()).expect("context for proxmox-notify has not been set yet")
 }
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
new file mode 100644
index 0000000..1e79566
--- /dev/null
+++ b/proxmox-notify/src/context/pbs.rs
@@ -0,0 +1,130 @@
+use serde::Deserialize;
+
+use proxmox_schema::{ObjectSchema, Schema, StringSchema};
+use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
+
+use crate::context::{common, Context};
+
+const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
+const PBS_NODE_CFG_FILENAME: &str = "/etc/proxmox-backup/node.cfg";
+
+// FIXME: Switch to the actual schema when possible in terms of dependency.
+// It's safe to assume that the config was written with the actual schema restrictions, so parsing
+// it with the less restrictive schema should be enough for the purpose of getting the mail address.
+const DUMMY_ID_SCHEMA: Schema = StringSchema::new("dummy ID").min_length(3).schema();
+const DUMMY_EMAIL_SCHEMA: Schema = StringSchema::new("dummy email").schema();
+const DUMMY_USER_SCHEMA: ObjectSchema = ObjectSchema {
+    description: "minimal PBS user",
+    properties: &[
+        ("userid", false, &DUMMY_ID_SCHEMA),
+        ("email", true, &DUMMY_EMAIL_SCHEMA),
+    ],
+    additional_properties: true,
+    default_key: None,
+};
+
+#[derive(Deserialize)]
+struct DummyPbsUser {
+    pub email: Option<String>,
+}
+
+/// Extract the root user's email address from the PBS user config.
+fn lookup_mail_address(content: &str, username: &str) -> Option<String> {
+    let mut config = SectionConfig::new(&DUMMY_ID_SCHEMA).allow_unknown_sections(true);
+    let user_plugin = SectionConfigPlugin::new(
+        "user".to_string(),
+        Some("userid".to_string()),
+        &DUMMY_USER_SCHEMA,
+    );
+    config.register_plugin(user_plugin);
+
+    match config.parse(PBS_USER_CFG_FILENAME, content) {
+        Ok(parsed) => {
+            parsed.sections.get(username)?;
+            match parsed.lookup::<DummyPbsUser>("user", username) {
+                Ok(user) => common::normalize_for_return(user.email.as_deref()),
+                Err(err) => {
+                    log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
+                    None
+                }
+            }
+        }
+        Err(err) => {
+            log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
+            None
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct PBSContext;
+
+pub static PBS_CONTEXT: PBSContext = PBSContext;
+
+impl Context for PBSContext {
+    fn lookup_email_for_user(&self, user: &str) -> Option<String> {
+        let content = common::attempt_file_read(PBS_USER_CFG_FILENAME);
+        content.and_then(|content| lookup_mail_address(&content, user))
+    }
+
+    fn default_sendmail_author(&self) -> String {
+        "Proxmox Backup Server".into()
+    }
+
+    fn default_sendmail_from(&self) -> String {
+        let content = common::attempt_file_read(PBS_NODE_CFG_FILENAME);
+        content
+            .and_then(|content| common::lookup_datacenter_config_key(&content, "email-from"))
+            .unwrap_or_else(|| String::from("root"))
+    }
+
+    fn http_proxy_config(&self) -> Option<String> {
+        let content = common::attempt_file_read(PBS_NODE_CFG_FILENAME);
+        content.and_then(|content| common::lookup_datacenter_config_key(&content, "http-proxy"))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const USER_CONFIG: &str = "
+user: root@pam
+	email root@example.com
+
+user: test@pbs
+	enable true
+	expire 0
+    ";
+
+    #[test]
+    fn test_parse_mail() {
+        assert_eq!(
+            lookup_mail_address(USER_CONFIG, "root@pam"),
+            Some("root@example.com".to_string())
+        );
+        assert_eq!(lookup_mail_address(USER_CONFIG, "test@pbs"), None);
+    }
+
+    const NODE_CONFIG: &str = "
+default-lang: de
+email-from: root@example.com
+http-proxy: http://localhost:1234
+    ";
+
+    #[test]
+    fn test_parse_node_config() {
+        assert_eq!(
+            common::lookup_datacenter_config_key(NODE_CONFIG, "email-from"),
+            Some("root@example.com".to_string())
+        );
+        assert_eq!(
+            common::lookup_datacenter_config_key(NODE_CONFIG, "http-proxy"),
+            Some("http://localhost:1234".to_string())
+        );
+        assert_eq!(
+            common::lookup_datacenter_config_key(NODE_CONFIG, "foo"),
+            None
+        );
+    }
+}
diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
new file mode 100644
index 0000000..f263c95
--- /dev/null
+++ b/proxmox-notify/src/context/pve.rs
@@ -0,0 +1,82 @@
+use crate::context::{common, Context};
+
+fn lookup_mail_address(content: &str, user: &str) -> Option<String> {
+    common::normalize_for_return(content.lines().find_map(|line| {
+        let fields: Vec<&str> = line.split(':').collect();
+        #[allow(clippy::get_first)] // to keep expression style consistent
+        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == user {
+            true => fields.get(6).copied(),
+            false => None,
+        }
+    }))
+}
+
+#[derive(Debug)]
+pub struct PVEContext;
+
+impl Context for PVEContext {
+    fn lookup_email_for_user(&self, user: &str) -> Option<String> {
+        let content = common::attempt_file_read("/etc/pve/user.cfg");
+        content.and_then(|content| lookup_mail_address(&content, user))
+    }
+
+    fn default_sendmail_author(&self) -> String {
+        "Proxmox VE".into()
+    }
+
+    fn default_sendmail_from(&self) -> String {
+        let content = common::attempt_file_read("/etc/pve/datacenter.cfg");
+        content
+            .and_then(|content| common::lookup_datacenter_config_key(&content, "email_from"))
+            .unwrap_or_else(|| String::from("root"))
+    }
+
+    fn http_proxy_config(&self) -> Option<String> {
+        let content = common::attempt_file_read("/etc/pve/datacenter.cfg");
+        content.and_then(|content| common::lookup_datacenter_config_key(&content, "http_proxy"))
+    }
+}
+
+pub static PVE_CONTEXT: PVEContext = PVEContext;
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    const USER_CONFIG: &str = "
+user:root@pam:1:0:::root@example.com:::
+user:test@pve:1:0:::test@example.com:::
+user:no-mail@pve:1:0::::::
+    ";
+
+    #[test]
+    fn test_parse_mail() {
+        assert_eq!(
+            lookup_mail_address(USER_CONFIG, "root@pam"),
+            Some("root@example.com".to_string())
+        );
+        assert_eq!(
+            lookup_mail_address(USER_CONFIG, "test@pve"),
+            Some("test@example.com".to_string())
+        );
+        assert_eq!(lookup_mail_address(USER_CONFIG, "no-mail@pve"), None);
+    }
+
+    const DC_CONFIG: &str = "
+email_from: user@example.com
+http_proxy: http://localhost:1234
+keyboard: en-us
+";
+    #[test]
+    fn test_parse_dc_config() {
+        assert_eq!(
+            common::lookup_datacenter_config_key(DC_CONFIG, "email_from"),
+            Some("user@example.com".to_string())
+        );
+        assert_eq!(
+            common::lookup_datacenter_config_key(DC_CONFIG, "http_proxy"),
+            Some("http://localhost:1234".to_string())
+        );
+        assert_eq!(common::lookup_datacenter_config_key(DC_CONFIG, "foo"), None);
+    }
+}
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox-perl-rs 06/11] notify: construct Notification via constructor
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (4 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 05/11] notify: add PVE/PBS context Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-perl-rs 07/11] pve-rs: notify: remove notify_context for PVE Lukas Wagner
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

This keeps us isolated from any further changes in the
proxmox_notify::Notification struct.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 common/src/notify.rs | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/common/src/notify.rs b/common/src/notify.rs
index 9f44225..203acca 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -94,13 +94,7 @@ mod export {
         properties: Option<JSONValue>,
     ) -> Result<(), HttpError> {
         let config = this.config.lock().unwrap();
-
-        let notification = Notification {
-            severity,
-            title,
-            body,
-            properties,
-        };
+        let notification = Notification::new_templated(severity, title, body, properties);
 
         api::common::send(&config, channel, &notification)
     }
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox-perl-rs 07/11] pve-rs: notify: remove notify_context for PVE
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (5 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-perl-rs 06/11] notify: construct Notification via constructor Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-cluster 08/11] datacenter config: add new parameters for system mail forwarding Lukas Wagner
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

The context has now been moved to `proxmox-notify` due to the fact
that we also need it in `proxmox-mail-forward` now.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 pve-rs/Cargo.toml            |   2 +-
 pve-rs/src/lib.rs            |   7 ++-
 pve-rs/src/notify_context.rs | 117 -----------------------------------
 3 files changed, 5 insertions(+), 121 deletions(-)
 delete mode 100644 pve-rs/src/notify_context.rs

diff --git a/pve-rs/Cargo.toml b/pve-rs/Cargo.toml
index f9e3291..f734b3b 100644
--- a/pve-rs/Cargo.toml
+++ b/pve-rs/Cargo.toml
@@ -36,7 +36,7 @@ perlmod = { version = "0.13", features = [ "exporter" ] }
 proxmox-apt = "0.10.6"
 proxmox-http = { version = "0.9", features = ["client-sync", "client-trait"] }
 proxmox-http-error = "0.1.0"
-proxmox-notify = "0.2"
+proxmox-notify = { version = "0.2", features = ["pve-context"] }
 proxmox-openid = "0.10"
 proxmox-resource-scheduling = "0.3.0"
 proxmox-subscription = "0.4"
diff --git a/pve-rs/src/lib.rs b/pve-rs/src/lib.rs
index d1915c9..42be39e 100644
--- a/pve-rs/src/lib.rs
+++ b/pve-rs/src/lib.rs
@@ -4,18 +4,19 @@
 pub mod common;
 
 pub mod apt;
-pub mod notify_context;
 pub mod openid;
 pub mod resource_scheduling;
 pub mod tfa;
 
 #[perlmod::package(name = "Proxmox::Lib::PVE", lib = "pve_rs")]
 mod export {
-    use crate::{common, notify_context};
+    use proxmox_notify::context::pve::PVE_CONTEXT;
+
+    use crate::common;
 
     #[export]
     pub fn init() {
         common::logger::init("PVE_LOG", "info");
-        notify_context::init();
+        proxmox_notify::context::set_context(&PVE_CONTEXT)
     }
 }
diff --git a/pve-rs/src/notify_context.rs b/pve-rs/src/notify_context.rs
deleted file mode 100644
index 48623fd..0000000
--- a/pve-rs/src/notify_context.rs
+++ /dev/null
@@ -1,117 +0,0 @@
-use log;
-use std::path::Path;
-
-use proxmox_notify::context::Context;
-
-// Some helpers borrowed and slightly adapted from `proxmox-mail-forward`
-
-fn normalize_for_return(s: Option<&str>) -> Option<String> {
-    match s?.trim() {
-        "" => None,
-        s => Some(s.to_string()),
-    }
-}
-
-fn attempt_file_read<P: AsRef<Path>>(path: P) -> Option<String> {
-    match proxmox_sys::fs::file_read_optional_string(path) {
-        Ok(contents) => contents,
-        Err(err) => {
-            log::error!("{err}");
-            None
-        }
-    }
-}
-
-fn lookup_mail_address(content: &str, user: &str) -> Option<String> {
-    normalize_for_return(content.lines().find_map(|line| {
-        let fields: Vec<&str> = line.split(':').collect();
-        #[allow(clippy::get_first)] // to keep expression style consistent
-        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == user {
-            true => fields.get(6).copied(),
-            false => None,
-        }
-    }))
-}
-
-fn lookup_datacenter_config_key(content: &str, key: &str) -> Option<String> {
-    let key_prefix = format!("{key}:");
-    normalize_for_return(
-        content
-            .lines()
-            .find_map(|line| line.strip_prefix(&key_prefix)),
-    )
-}
-
-#[derive(Debug)]
-struct PVEContext;
-
-impl Context for PVEContext {
-    fn lookup_email_for_user(&self, user: &str) -> Option<String> {
-        let content = attempt_file_read("/etc/pve/user.cfg");
-        content.and_then(|content| lookup_mail_address(&content, user))
-    }
-
-    fn default_sendmail_author(&self) -> String {
-        "Proxmox VE".into()
-    }
-
-    fn default_sendmail_from(&self) -> String {
-        let content = attempt_file_read("/etc/pve/datacenter.cfg");
-        content
-            .and_then(|content| lookup_datacenter_config_key(&content, "mail_from"))
-            .unwrap_or_else(|| String::from("root"))
-    }
-
-    fn http_proxy_config(&self) -> Option<String> {
-        let content = attempt_file_read("/etc/pve/datacenter.cfg");
-        content.and_then(|content| lookup_datacenter_config_key(&content, "http_proxy"))
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    const USER_CONFIG: &str = "
-user:root@pam:1:0:::root@example.com:::
-user:test@pve:1:0:::test@example.com:::
-user:no-mail@pve:1:0::::::
-    ";
-
-    #[test]
-    fn test_parse_mail() {
-        assert_eq!(
-            lookup_mail_address(USER_CONFIG, "root@pam"),
-            Some("root@example.com".to_string())
-        );
-        assert_eq!(
-            lookup_mail_address(USER_CONFIG, "test@pve"),
-            Some("test@example.com".to_string())
-        );
-        assert_eq!(lookup_mail_address(USER_CONFIG, "no-mail@pve"), None);
-    }
-
-    const DC_CONFIG: &str = "
-email_from: user@example.com
-http_proxy: http://localhost:1234
-keyboard: en-us
-";
-    #[test]
-    fn test_parse_dc_config() {
-        assert_eq!(
-            lookup_datacenter_config_key(DC_CONFIG, "email_from"),
-            Some("user@example.com".to_string())
-        );
-        assert_eq!(
-            lookup_datacenter_config_key(DC_CONFIG, "http_proxy"),
-            Some("http://localhost:1234".to_string())
-        );
-        assert_eq!(lookup_datacenter_config_key(DC_CONFIG, "foo"), None);
-    }
-}
-
-static CONTEXT: PVEContext = PVEContext;
-
-pub fn init() {
-    proxmox_notify::context::set_context(&CONTEXT)
-}
-- 
2.39.2





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

* [pve-devel] [PATCH v2 pve-cluster 08/11] datacenter config: add new parameters for system mail forwarding
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (6 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-perl-rs 07/11] pve-rs: notify: remove notify_context for PVE Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-manager 09/11] ui: notify: add system-mail settings, configuring " Lukas Wagner
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

This commit adds two new paramters to the 'notify' property string:
  - 'system-mail': Determine whether mails to root should be forwarded
    by the notification system
  - 'system-mail-target': Determine the target to which the
    notification should be forwarded to.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/PVE/DataCenterConfig.pm | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/src/PVE/DataCenterConfig.pm b/src/PVE/DataCenterConfig.pm
index 09be6eb..e2619c5 100644
--- a/src/PVE/DataCenterConfig.pm
+++ b/src/PVE/DataCenterConfig.pm
@@ -116,6 +116,28 @@ my $notification_format = {
 	    . " to root via a 'sendmail' notification endpoint.",
 	optional => 1,
     },
+    'system-mail' => {
+	type => 'string',
+	enum => ['always', 'never'],
+	description => "Control if mails to the 'root' user should be forwarded.",
+	verbose_description => "Control if mails to the 'root' user should be forwarded.\n"
+	    . "* 'always' forward always\n"
+	    . "* 'never' forward never.\n"
+	    . "For production systems, turning off mail forwarding is not"
+	    . "recommended!\n",
+	default => 'always',
+	optional => 1,
+    },
+    'target-system-mail' => {
+	type => 'string',
+	format_description => 'TARGET',
+	description => "Control where mails to the 'root' user should be forwarded to.",
+	verbose_description => "Control where mails to the 'root' user should be forwarded to."
+	    . " Has to be the name of a notification target (endpoint or notification group)."
+	    . " If the 'target-system-mail' parameter is not set, the system will send mails"
+	    . " to root via a 'sendmail' notification endpoint.",
+	optional => 1,
+    },
 };
 
 register_standard_option('pve-ha-shutdown-policy', {
-- 
2.39.2





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

* [pve-devel] [PATCH v2 pve-manager 09/11] ui: notify: add system-mail settings, configuring mail forwarding
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (7 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-cluster 08/11] datacenter config: add new parameters for system mail forwarding Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-mail-forward 10/11] feed forwarded mails into proxmox_notify Lukas Wagner
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

The 'Notifications' panel in Datacenter view now features a new entry
'System mail', allowing the user to configure target and policy for
mails sent to the local root user.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/dc/NotificationEvents.js | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/www/manager6/dc/NotificationEvents.js b/www/manager6/dc/NotificationEvents.js
index 18816290..40e9a65f 100644
--- a/www/manager6/dc/NotificationEvents.js
+++ b/www/manager6/dc/NotificationEvents.js
@@ -240,6 +240,33 @@ Ext.define('PVE.dc.NotificationEvents', {
 	    ],
 	});
 
+	me.addInputPanelRow('system-mail', 'notify', gettext('System Mail'), {
+	    renderer: (value, metaData, record, rowIndex, colIndex, store) =>
+		render_value(store, 'target-system-mail', 'system-mail', gettext('Always')),
+	    url: "/api2/extjs/cluster/options",
+	    items: [
+		{
+		    xtype: 'pveNotificationEventsPolicySelector',
+		    name: 'system-mail',
+		    fieldLabel: gettext('Notify'),
+		    comboItems: [
+			['__default__', `${Proxmox.Utils.defaultText} (${gettext('Always')})`],
+			['always', gettext('Always')],
+			['never', gettext('Never')],
+		    ],
+		    warningRef: 'warning',
+		    warnIfValIs: 'never',
+		},
+		{
+		    xtype: 'pveNotificationEventsTargetSelector',
+		    name: 'target-system-mail',
+		},
+		{
+		    xtype: 'pveNotificationEventDisabledWarning',
+		    reference: 'warning',
+		},
+	    ],
+	});
 	// Hack: Also load the notify property to make it accessible
 	// for our render functions.
 	me.rows.notify = {
-- 
2.39.2





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

* [pve-devel] [PATCH v2 proxmox-mail-forward 10/11] feed forwarded mails into proxmox_notify
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (8 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-manager 09/11] ui: notify: add system-mail settings, configuring " Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-docs 11/11] notification: add docs for system mail forwarding Lukas Wagner
  2023-10-17  7:28 ` [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

This allows us to send notifications for events from daemons that are
not under our control, e.g. zed, smartd, cron. etc...

For mail-based notification targets (sendmail, soon smtp) the mail is
forwarded as is, including all headers.
All other target types will try to parse the email to extra subject
and text body.

On PBS, where proxmox-notify is not yet fully integrated, we also use
proxmox-notify. There, we simply fall back to the default
target/policy (mail-to-root, which looks up root@pam's mail address).
Once notification support is built into PBS, proxmox-mail-forward
should automatically work in the same way as for PVE.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 Cargo.toml  |   8 +-
 src/main.rs | 348 +++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 238 insertions(+), 118 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index c68e802..64f8d47 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -3,6 +3,7 @@ name = "proxmox-mail-forward"
 version = "0.2.0"
 authors = [
     "Fiona Ebner <f.ebner@proxmox.com>",
+    "Lukas Wagner <l.wagner@proxmox.com>",
     "Proxmox Support Team <support@proxmox.com>",
 ]
 edition = "2021"
@@ -17,9 +18,10 @@ anyhow = "1.0"
 log = "0.4.17"
 nix = "0.26"
 serde = { version = "1.0", features = ["derive"] }
-#serde_json = "1.0"
+serde_json = "1.0"
 syslog = "6.0"
 
-proxmox-schema = "1.3"
-proxmox-section-config = "1.0.2"
+proxmox-schema = { version = "2.0", features = ["api-macro"] }
+proxmox-section-config = "2.0"
 proxmox-sys = "0.5"
+proxmox-notify = {version = "0.2", features = ["mail-forwarder", "pve-context", "pbs-context"] }
diff --git a/src/main.rs b/src/main.rs
index f3d4193..d3b7b6a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -1,39 +1,58 @@
+use std::io::Read;
 use std::path::Path;
-use std::process::Command;
 
-use anyhow::{bail, format_err, Error};
+use anyhow::Error;
 use serde::Deserialize;
 
-use proxmox_schema::{ObjectSchema, Schema, StringSchema};
-use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
+use proxmox_notify::context::pbs::PBS_CONTEXT;
+use proxmox_notify::context::pve::PVE_CONTEXT;
+use proxmox_notify::endpoints::sendmail::SendmailConfig;
+use proxmox_notify::Config;
+use proxmox_schema::{api, ApiType};
 use proxmox_sys::fs;
 
-const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
-const PBS_ROOT_USER: &str = "root@pam";
-
-// FIXME: Switch to the actual schema when possible in terms of dependency.
-// It's safe to assume that the config was written with the actual schema restrictions, so parsing
-// it with the less restrictive schema should be enough for the purpose of getting the mail address.
-const DUMMY_ID_SCHEMA: Schema = StringSchema::new("dummy ID").min_length(3).schema();
-const DUMMY_EMAIL_SCHEMA: Schema = StringSchema::new("dummy email").schema();
-const DUMMY_USER_SCHEMA: ObjectSchema = ObjectSchema {
-    description: "minimal PBS user",
-    properties: &[
-        ("userid", false, &DUMMY_ID_SCHEMA),
-        ("email", true, &DUMMY_EMAIL_SCHEMA),
-    ],
-    additional_properties: true,
-    default_key: None,
-};
+const PVE_CFG_PATH: &str = "/etc/pve";
+const PVE_DATACENTER_CFG_FILENAME: &str = "/etc/pve/datacenter.cfg";
+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_CFG_PATH: &str = "/etc/proxmox-backup";
+const PBS_NODE_CFG_FILENAME: &str = "/etc/proxmox-backup/node.cfg";
+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";
 
-#[derive(Deserialize)]
-struct DummyPbsUser {
-    pub email: Option<String>,
+#[api]
+#[derive(Deserialize, Default, Debug, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+enum Policy {
+    #[default]
+    /// Always send a notification.
+    Always,
+    /// Never send a notification.
+    Never,
 }
 
-const PVE_USER_CFG_FILENAME: &str = "/etc/pve/user.cfg";
-const PVE_DATACENTER_CFG_FILENAME: &str = "/etc/pve/datacenter.cfg";
-const PVE_ROOT_USER: &str = "root@pam";
+#[api(
+    properties: {},
+    additional_properties: true,
+)]
+#[derive(Deserialize, Default, Debug)]
+#[serde(rename_all = "kebab-case")]
+/// Parsed property string for the `notify` key in `datacenter.cfg`
+struct NotifyPropertyString {
+    /// Target for redirected system mails.
+    target_system_mail: Option<String>,
+    /// Notification policy for system mails
+    system_mail: Option<Policy>,
+}
+
+/// Forwarding policy for system mail
+struct NotificationPolicy {
+    /// Target for redirected system mails.
+    target: String,
+    /// Notification policy for system mails
+    policy: Policy,
+}
 
 /// Convenience helper to get the trimmed contents of an optional &str, mapping blank ones to `None`
 /// and creating a String from it for returning.
@@ -44,89 +63,143 @@ fn normalize_for_return(s: Option<&str>) -> Option<String> {
     }
 }
 
-/// Extract the root user's email address from the PBS user config.
-fn get_pbs_mail_to(content: &str) -> Option<String> {
-    let mut config = SectionConfig::new(&DUMMY_ID_SCHEMA).allow_unknown_sections(true);
-    let user_plugin = SectionConfigPlugin::new(
-        "user".to_string(),
-        Some("userid".to_string()),
-        &DUMMY_USER_SCHEMA,
-    );
-    config.register_plugin(user_plugin);
-
-    match config.parse(PBS_USER_CFG_FILENAME, content) {
-        Ok(parsed) => {
-            parsed.sections.get(PBS_ROOT_USER)?;
-            match parsed.lookup::<DummyPbsUser>("user", PBS_ROOT_USER) {
-                Ok(user) => normalize_for_return(user.email.as_deref()),
-                Err(err) => {
-                    log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
-                    None
-                }
-            }
-        }
+/// 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> {
+    match fs::file_read_optional_string(path) {
+        Ok(contents) => contents,
         Err(err) => {
-            log::error!("unable to parse {} - {}", PBS_USER_CFG_FILENAME, err);
+            log::error!("{}", err);
             None
         }
     }
 }
 
-/// Extract the root user's email address from the PVE user config.
-fn get_pve_mail_to(content: &str) -> Option<String> {
-    normalize_for_return(content.lines().find_map(|line| {
-        let fields: Vec<&str> = line.split(':').collect();
-        #[allow(clippy::get_first)] // to keep expression style consistent
-        match fields.get(0)?.trim() == "user" && fields.get(1)?.trim() == PVE_ROOT_USER {
-            true => fields.get(6).copied(),
-            false => None,
-        }
-    }))
-}
-
-/// Extract the From-address configured in the PVE datacenter config.
-fn get_pve_mail_from(content: &str) -> Option<String> {
+/// Look up a the value of a given key in PVE datacenter config
+fn lookup_datacenter_config_key(content: &str, key: &str) -> Option<String> {
+    let key_prefix = format!("{key}:");
     normalize_for_return(
         content
             .lines()
-            .find_map(|line| line.strip_prefix("email_from:")),
+            .find_map(|line| line.strip_prefix(&key_prefix)),
     )
 }
 
-/// Executes sendmail as a child process with the specified From/To-addresses, expecting the mail
-/// contents to be passed via stdin inherited from this program.
-fn forward_mail(mail_from: String, mail_to: Vec<String>) -> Result<(), Error> {
-    if mail_to.is_empty() {
-        bail!("user 'root@pam' does not have an email address");
+/// Read data from stdin, until EOF is encountered.
+fn read_stdin() -> Result<Vec<u8>, Error> {
+    let mut input = Vec::new();
+    let stdin = std::io::stdin();
+    let mut handle = stdin.lock();
+
+    handle.read_to_end(&mut input)?;
+    Ok(input)
+}
+
+/// Parse node.cfg/datacenter.cfg and extract system mail target/policy.
+fn system_mail_target_policy(datacenter_config: Option<&str>) -> NotificationPolicy {
+    let cfg: Result<_, Error> = datacenter_config
+        .and_then(|cfg| lookup_datacenter_config_key(cfg, "notify"))
+        .map(|val| {
+            let val = NotifyPropertyString::API_SCHEMA.parse_property_string(&val)?;
+            let config: NotifyPropertyString = serde_json::from_value(val)?;
+            Ok(config)
+        })
+        .transpose();
+
+    let cfg = match cfg {
+        Ok(cfg) => cfg.unwrap_or_default(),
+        Err(err) => {
+            log::error!(
+                "could not read system mail notification settings from datacenter.cfg: {err}"
+            );
+
+            Default::default()
+        }
+    };
+
+    NotificationPolicy {
+        target: cfg
+            .target_system_mail
+            .unwrap_or_else(|| String::from("mail-to-root")),
+        policy: cfg.system_mail.unwrap_or_default(),
     }
+}
 
-    log::info!("forward mail to <{}>", mail_to.join(","));
+fn forward_common(
+    mail: &[u8],
+    config: &mut Config,
+    target: &NotificationPolicy,
+) -> Result<(), Error> {
+    let real_uid = nix::unistd::getuid();
+    // The uid is passed so that `sendmail` can be called as the a correct user.
+    let notification =
+        proxmox_notify::Notification::new_forwarded_mail(mail, Some(real_uid.as_raw()))?;
 
-    let mut cmd = Command::new("sendmail");
-    cmd.args([
-        "-bm", "-N", "never", // never send DSN (avoid mail loops)
-        "-f", &mail_from, "--",
-    ]);
-    cmd.args(mail_to);
-    cmd.env("PATH", "/sbin:/bin:/usr/sbin:/usr/bin");
+    // TODO: mail-to-root should *always* be always available, but right now that is only ensured
+    // in PVE::Notify - maybe we should move that to proxmox_notify? So that we don't have to
+    // add it here?
+    proxmox_notify::api::sendmail::add_endpoint(
+        config,
+        &SendmailConfig {
+            name: "mail-to-root".to_string(),
+            mailto_user: Some(vec!["root@pam".to_string()]),
+            ..Default::default()
+        },
+    )?;
 
-    // with status(), child inherits stdin
-    cmd.status()
-        .map_err(|err| format_err!("command {:?} failed - {}", cmd, err))?;
+    if matches!(target.policy, Policy::Always) {
+        let target = &target.target;
+        log::info!("attempting to forward mail via target {target}");
+        proxmox_notify::api::common::send(config, target, &notification)?;
+    } else {
+        log::info!(
+            "'notify: system-mail=never' set in datacenter.cfg/node.cfg, mail is not forwarded"
+        );
+    }
 
     Ok(())
 }
 
-/// 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> {
-    match fs::file_read_optional_string(path) {
-        Ok(contents) => contents,
-        Err(err) => {
-            log::error!("{}", err);
-            None
-        }
-    }
+/// Forward the mail to root@pam for a PVE installation. If no mail address is configured or
+/// if the user.cfg file does not exist (e.g. if PBS is not actually installed), nothing will
+/// happen.
+fn forward_for_pve(mail: &[u8]) -> Result<(), Error> {
+    let mut config = notification_config(
+        PVE_PUB_NOTIFICATION_CFG_FILENAME,
+        PVE_PRIV_NOTIFICATION_CFG_FILENAME,
+    )?;
+    let datacenter_config = attempt_file_read(PVE_DATACENTER_CFG_FILENAME);
+    let target = system_mail_target_policy(datacenter_config.as_deref());
+
+    proxmox_notify::context::set_context(&PVE_CONTEXT);
+
+    forward_common(mail, &mut config, &target)
+}
+
+/// Forward the mail to root@pam for a PBS installation. If no mail address is configured or
+/// if the user.cfg file does not exist (e.g. if PBS is not actually installed), nothing will
+/// happen.
+fn forward_for_pbs(mail: &[u8]) -> Result<(), Error> {
+    let mut config = notification_config(
+        PBS_PUB_NOTIFICATION_CFG_FILENAME,
+        PBS_PRIV_NOTIFICATION_CFG_FILENAME,
+    )?;
+    let node_config = attempt_file_read(PBS_NODE_CFG_FILENAME);
+    let target = system_mail_target_policy(node_config.as_deref());
+
+    proxmox_notify::context::set_context(&PBS_CONTEXT);
+
+    forward_common(mail, &mut config, &target)
+}
+
+/// Parse notification config.
+///
+/// If the configuration file does not exist, this will return an empty configuration.
+fn notification_config(public_path: &str, private_path: &str) -> Result<Config, Error> {
+    let public_contents = attempt_file_read(public_path).unwrap_or_default();
+    let private_contents = attempt_file_read(private_path).unwrap_or_default();
+
+    Config::new(&public_contents, &private_contents).map_err(Into::into)
 }
 
 fn main() {
@@ -135,40 +208,85 @@ fn main() {
         log::LevelFilter::Info,
         Some("proxmox-mail-forward"),
     ) {
-        eprintln!("unable to inititialize syslog - {}", err);
+        eprintln!("unable to initialize syslog - {}", err);
     }
 
-    let pbs_user_cfg_content = attempt_file_read(PBS_USER_CFG_FILENAME);
-    let pve_user_cfg_content = attempt_file_read(PVE_USER_CFG_FILENAME);
-    let pve_datacenter_cfg_content = attempt_file_read(PVE_DATACENTER_CFG_FILENAME);
+    // Read the mail that is to be forwarded from stdin
+    match read_stdin() {
+        Ok(mail) => {
+            // Detect if we are on a PVE system. False positives, e.g. from
+            // left-overs of a co-installation with PBS might lead to a mail
+            // being forwarded twice or error messages in the system logs (e.g.
+            // if we fall back to 'mail-to-root' but root@pam has no email address
+            // configured, then the sendmail target will complain about a missing recipient)
+            if Path::new(PVE_CFG_PATH).exists() {
+                if let Err(err) = forward_for_pve(&mail) {
+                    log::error!("could not forward mail for Proxmox VE: {err}");
+                }
+            }
 
-    let real_uid = nix::unistd::getuid();
-    if let Err(err) = nix::unistd::setresuid(real_uid, real_uid, real_uid) {
-        log::error!(
-            "mail forward failed: unable to set effective uid to {}: {}",
-            real_uid,
-            err
-        );
-        return;
+            // Same as above...
+            if Path::new(PBS_CFG_PATH).exists() {
+                if let Err(err) = forward_for_pbs(&mail) {
+                    log::error!("could not forward mail for Proxmox Backup Server: {err}");
+                }
+            }
+        }
+        Err(err) => {
+            log::error!("could not read mail from STDIN: {err}")
+        }
     }
+}
 
-    let pbs_mail_to = pbs_user_cfg_content.and_then(|content| get_pbs_mail_to(&content));
-    let pve_mail_to = pve_user_cfg_content.and_then(|content| get_pve_mail_to(&content));
-    let pve_mail_from = pve_datacenter_cfg_content.and_then(|content| get_pve_mail_from(&content));
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_notify_all_set() {
+        let config = Some(
+            "
+email_from: root@example.com
+notify: target-system-mail=foobar,system-mail=always
+keyboard: en-us
+        ",
+        );
 
-    let mail_from = pve_mail_from.unwrap_or_else(|| "root".to_string());
+        let policy = system_mail_target_policy(config);
 
-    let mut mail_to = vec![];
-    if let Some(pve_mail_to) = pve_mail_to {
-        mail_to.push(pve_mail_to);
+        assert_eq!(policy.target, "foobar");
+        assert_eq!(policy.policy, Policy::Always);
     }
-    if let Some(pbs_mail_to) = pbs_mail_to {
-        if !mail_to.contains(&pbs_mail_to) {
-            mail_to.push(pbs_mail_to);
-        }
+
+    #[test]
+    fn test_correct_defaults() {
+        let config = Some(
+            "
+email_from: root@example.com
+keyboard: en-us
+        ",
+        );
+
+        let policy = system_mail_target_policy(config);
+
+        assert_eq!(policy.target, "mail-to-root");
+        assert_eq!(policy.policy, Policy::Always);
     }
 
-    if let Err(err) = forward_mail(mail_from, mail_to) {
-        log::error!("mail forward failed: {}", err);
+    #[test]
+    fn test_invalid_policy() {
+        let config = Some(
+            "
+email_from: root@example.com
+notify: system-mail=invalid
+keyboard: en-us
+        ",
+        );
+
+        let policy = system_mail_target_policy(config);
+
+        assert_eq!(policy.target, "mail-to-root");
+        // Fall back to always if the policy is invalid
+        assert_eq!(policy.policy, Policy::Always);
     }
 }
-- 
2.39.2





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

* [pve-devel] [PATCH v2 pve-docs 11/11] notification: add docs for system mail forwarding
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (9 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-mail-forward 10/11] feed forwarded mails into proxmox_notify Lukas Wagner
@ 2023-10-02  8:06 ` Lukas Wagner
  2023-10-17  7:28 ` [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
  11 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-02  8:06 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/notifications.adoc b/notifications.adoc
index c4d2931..0b00b1e 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -19,9 +19,10 @@ such as:
 | `fencing`         | The {pve} HA manager has fenced a node  | `error`
 | `replication`     | A storage replication job has failed    | `error`
 | `vzdump`          | vzdump backup finished                  | `info` (`error` on failure)
+| `system-mail`     | An email was sent to the local root user| `error`
 |===========================================================================
 
-In the 'Notification' panel of the datacenter view, the system's behavior can be
+In the 'Notifications' panel of the datacenter view, the system's behavior can be
 configured for all events except backup jobs. For backup jobs,
 the settings can be found in the respective backup job configuration.
 For every notification event there is an option to configure the notification
@@ -151,9 +152,22 @@ included in the group. If a group/endpoint is configured to
 use a filter, the user must have the `Mapping.Use` permission for the filter
 as well.
 
-
-
-
-
-
-
+System Mail Forwarding
+---------------------
+
+Certain local system daemons, such as `smartd`, generate notification emails
+that are initially directed to the local `root` user. {pve} will
+forward these emails to a customizable notification target.
+
+By default, these emails are sent to the email address that is configured for
+the `root@pam` user using the predefined `mail-to-root` notification target.
+However, similar to any other notification event, this behavior can be
+adjusted within the 'Notifications' panel by making changes to the
+'System Mail' setting.
+
+When the forwarding process involves an email-based target
+(like `sendmail` or `smtp`), the email is forwarded exactly as received, with all
+original mail headers remaining intact. For all other targets,
+the system tries to extract both a subject line and the main text body
+from the email content. In instances where emails solely consist of HTML
+content, they will be transformed into plain text format during this process.
-- 
2.39.2





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

* Re: [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify
  2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
                   ` (10 preceding siblings ...)
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-docs 11/11] notification: add docs for system mail forwarding Lukas Wagner
@ 2023-10-17  7:28 ` Lukas Wagner
  2023-10-19  8:30   ` Lukas Wagner
  11 siblings, 1 reply; 15+ messages in thread
From: Lukas Wagner @ 2023-10-17  7:28 UTC (permalink / raw)
  To: pve-devel

Ping - would be great to get some reviews on this to get this merged for
the next release.

On 10/2/23 10:06, Lukas Wagner wrote:
> The aim of this patch series is to adapt `proxmox-mail-forward`
> so that it forwards emails that were sent to the local root user
> through the `proxmox_notify` crate.
> 


-- 
- Lukas




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

* Re: [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify
  2023-10-17  7:28 ` [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
@ 2023-10-19  8:30   ` Lukas Wagner
  0 siblings, 0 replies; 15+ messages in thread
From: Lukas Wagner @ 2023-10-19  8:30 UTC (permalink / raw)
  To: pve-devel



On 10/17/23 09:28, Lukas Wagner wrote:
> Ping - would be great to get some reviews on this to get this merged for
> the next release.
> 
> On 10/2/23 10:06, Lukas Wagner wrote:
>> The aim of this patch series is to adapt `proxmox-mail-forward`
>> so that it forwards emails that were sent to the local root user
>> through the `proxmox_notify` crate.
>>

There will be some conceptual changes to the notification system, 
requiring some changes to this series as well. A new version will
be posted at some point.

-- 
- Lukas




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

* [pve-devel] applied: [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2
  2023-10-02  8:06 ` [pve-devel] [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2 Lukas Wagner
@ 2023-10-20 15:36   ` Thomas Lamprecht
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2023-10-20 15:36 UTC (permalink / raw)
  To: Proxmox VE development discussion, Lukas Wagner

Am 02/10/2023 um 10:06 schrieb Lukas Wagner:
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  src/mail-parser/debian/changelog              |  6 ++
>  src/mail-parser/debian/copyright              | 49 ++++++++++++
>  .../debian/copyright.debcargo.hint            | 77 +++++++++++++++++++
>  src/mail-parser/debian/debcargo.toml          |  2 +
>  4 files changed, 134 insertions(+)
>  create mode 100644 src/mail-parser/debian/changelog
>  create mode 100644 src/mail-parser/debian/copyright
>  create mode 100644 src/mail-parser/debian/copyright.debcargo.hint
>  create mode 100644 src/mail-parser/debian/debcargo.toml
> 
>

applied this one already uploaded to our staging repo, thanks!




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

end of thread, other threads:[~2023-10-20 15:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-02  8:06 [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 debcargo-conf 01/11] package mail-parser 0.8.2 Lukas Wagner
2023-10-20 15:36   ` [pve-devel] applied: " Thomas Lamprecht
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 02/11] sys: email: add `forward` Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 03/11] notify: introduce Error::Generic Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 04/11] notify: add mechanisms for email message forwarding Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox 05/11] notify: add PVE/PBS context Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-perl-rs 06/11] notify: construct Notification via constructor Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-perl-rs 07/11] pve-rs: notify: remove notify_context for PVE Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-cluster 08/11] datacenter config: add new parameters for system mail forwarding Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-manager 09/11] ui: notify: add system-mail settings, configuring " Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 proxmox-mail-forward 10/11] feed forwarded mails into proxmox_notify Lukas Wagner
2023-10-02  8:06 ` [pve-devel] [PATCH v2 pve-docs 11/11] notification: add docs for system mail forwarding Lukas Wagner
2023-10-17  7:28 ` [pve-devel] [PATCH v2 many 00/11] notifications: feed system mails into proxmox_notify Lukas Wagner
2023-10-19  8:30   ` Lukas Wagner

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