From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 568B691C3B for ; Mon, 27 Mar 2023 17:19:52 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 474E1D4F5 for ; Mon, 27 Mar 2023 17:19:25 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 27 Mar 2023 17:19:22 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E3A6D47002 for ; Mon, 27 Mar 2023 17:19:21 +0200 (CEST) From: Lukas Wagner To: pve-devel@lists.proxmox.com Date: Mon, 27 Mar 2023 17:18:39 +0200 Message-Id: <20230327151857.495565-1-l.wagner@proxmox.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.171 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Mar 2023 15:19:52 -0000 The purpose of this patch series is to overhaul the existing mail notification infrastructure in Proxmox VE. The series replaces calls to 'sendmail' with calls to a new, configurable notification module. The module was designed to support multiple notification endpoints, 'sendmail' using the system's sendmail command being the first one. As a proof of the extensibility of the current approach, the 'gotify' [1] plugin was also implemented in this series. The series also includes a first draft of how notification filtering based on severity and other properties could look like (more about properties will follow later in this cover letter). One thing that should be avoided is breaking existing mail notifications when these changes hit the repositories. This is a bit tricky, since currently, we send mails to 'root' (replication, HA - is forwarded by the mail forwarder to root@pam's mail address), root@pam's email address (APT), as well as to freely chooseable email addresses (vzdump). In this version of the patch series, a seamless transition is ensured by adding an 'ephemeral' sendmail notification endpoint right before sending a notification. A later patch could mark individual mail recipients in vzdump jobs as being deprecated in favor of the new notification endpoints. The drawback of this approach is that we might send a notification twice in case a user has created another sendmail endpoint with the same recipient. However, this is still better than not sending a notification at all. However, I'm open for suggestions for other approaches for maintaining compatibility. Alternative approaches that came to my mind are: - Explicitly break existing mail notifications with a major release, maybe create a default sendmail endpoint where *all* notifications are sent to root@pam via a sendmail endpoint. In this variant, the custom mail recipients for vzdump jobs would be removed - On update, create endpoints in the new configuration file for all possible email addresses, along with appropriate filters so that the behavior for every component currently sending mails remains unchanged. I don't really like this approach, as it might lead to a pretty messy 'notifications.cfg' with lots of endpoints/filters, if the user has lots of different backup jobs configured. Side effects of the changes: - vzdump loses the HTML table which lists VMs/CTs. Since different endpoints support different markup styles for formatting, it does not really make sense to support this in the notification API, at least not without 'cross-rendering' (e.g. markdown --> HTML) Short summary of the introduced changes per repo: - proxmox: Add new proxmox-notification crate, including configuration file parsing, two endpoints (sendmail, gotify) and per-endpoint filtering - proxmox-perl-rs: Add bindings for proxmox-notification, so that we can use it from perl. Also configure logging in such a way that log messages from proxmox-notification integrate nicely in task logs. - proxmox-cluster: Register new configuration file, 'notifications.cfg' - pve-manager: Add some more glue code, so that the notification functions are callable in a nice way. This is needed since we need to read the configuration file and feed the config to the rust bindings as a string. Replace calls to 'sendmail' in vzdump, apt, and replication with calls to the new notification module. - pve-ha-manager: Also replace calls to 'sendmail' with the new notification module Follow-up work (in no particular order): - Add CRUD API routes for managing notification endpoints and filters - right now, this can only be done by editing 'notifications.cfg' - Add a GUI and CLI using this API - Right now, the notification API is very simple. Sending a notification can be as simple as PVE::Notification::info("Title", "Body") In the future, the API might be changed/extended so that supports "registering" notifications. This allows us to a.) generate a list of all possible notification sources in the system b.) allows users to easily create filters for specific notification events. In my head, using the notification module could look like this then: # Global context my = PVE::Notification::register({ 'id' => 'backup-failed', 'severity' => 'error', 'properties' => ['host', 'vmlist', 'logs'], 'title' => '{{ host }}: Backup failed' 'body' => <<'EOF' A backup has failed for the following VMs: {{ vmlist }} {{ logs }} EOF }); # Later, to send the notification: PVE::Notification::send(->instantiate({ 'host' => 'earth', 'vmlist' => ... , 'logs' => ... , })); Title and body effectively become templates that can be instantiated with arbitrary properties. This has the following benefits: - This ensures that notifications could be easily translated in the future - Allows us to add functionality that allows users to customize notification text, as wished in [2]. - Properties can be used in filters (e.g. exact match, regex, etc.) - Properties can be listed in the list of notifications mentioned above, making it easier to create filters. - proxmox-mail-forward could be integrated as well. This would feed e.g. zfs-zed events into our notification infrastructure. Special care must be taken to not create recursive notification loops (e.g. zed sends to root, forwarder uses notification module, a configured sendmail endpoint sends to root, forwarder uses module --> loop) - Maybe add some CLI so that admins can send notifications in scripts (an API endpoint callable via pvesh might be enough for a start) - Add more notification events - Add other endpoints, e.g. webhook, a generic SMTP, etc. - Integrate the new module into the other products [1] https://gotify.net/ [2] https://bugzilla.proxmox.com/show_bug.cgi?id=4526 proxmox: Lukas Wagner (6): add proxmox-notification crate notification: implement sendmail endpoint notification: add notification filter mechanism notification: implement gotify endpoint notification: allow adding new sendmail endpoints / filters notification: add debian packaging Cargo.toml | 2 + proxmox-notification/Cargo.toml | 22 + proxmox-notification/debian/changelog | 5 + proxmox-notification/debian/control | 59 +++ proxmox-notification/debian/copyright | 16 + proxmox-notification/debian/debcargo.toml | 7 + proxmox-notification/src/config.rs | 71 +++ proxmox-notification/src/endpoints/gotify.rs | 80 ++++ proxmox-notification/src/endpoints/mod.rs | 2 + .../src/endpoints/sendmail.rs | 78 ++++ proxmox-notification/src/filter.rs | 427 ++++++++++++++++++ proxmox-notification/src/lib.rs | 307 +++++++++++++ proxmox-notification/src/methods.rs | 55 +++ 13 files changed, 1131 insertions(+) create mode 100644 proxmox-notification/Cargo.toml create mode 100644 proxmox-notification/debian/changelog create mode 100644 proxmox-notification/debian/control create mode 100644 proxmox-notification/debian/copyright create mode 100644 proxmox-notification/debian/debcargo.toml create mode 100644 proxmox-notification/src/config.rs create mode 100644 proxmox-notification/src/endpoints/gotify.rs create mode 100644 proxmox-notification/src/endpoints/mod.rs create mode 100644 proxmox-notification/src/endpoints/sendmail.rs create mode 100644 proxmox-notification/src/filter.rs create mode 100644 proxmox-notification/src/lib.rs create mode 100644 proxmox-notification/src/methods.rs proxmox-perl-rs: Lukas Wagner (2): log: set default log level to 'info', add product specfig logging env var1 add basic bindings for the proxmox_notification crate Cargo.toml | 2 + common/pkg/Makefile | 1 + common/src/logger.rs | 12 +++- pmg-rs/src/lib.rs | 2 +- pve-rs/Cargo.toml | 1 + pve-rs/src/lib.rs | 3 +- pve-rs/src/notification.rs | 128 +++++++++++++++++++++++++++++++++++++ 7 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 pve-rs/src/notification.rs pve-cluster: Lukas Wagner (1): cluster files: add notifications.cfg data/PVE/Cluster.pm | 1 + data/src/status.c | 1 + 2 files changed, 2 insertions(+) pve-manager: Lukas Wagner (7): test: fix names of .PHONY targets add PVE::Notification module vzdump: send notifications via new notification module vzdump: rename 'sendmail' sub to 'send_notification' test: rename mail_test.pl to vzdump_notification_test.pl api: apt: send notification via new notification module api: replication: send notifications via new notification module PVE/API2/APT.pm | 55 +++++--- PVE/API2/Replication.pm | 13 +- PVE/API2/VZDump.pm | 2 +- PVE/Makefile | 1 + PVE/Notification.pm | 60 +++++++++ PVE/VZDump.pm | 124 +++++------------- test/Makefile | 16 ++- ...il_test.pl => vzdump_notification_test.pl} | 28 ++-- 8 files changed, 169 insertions(+), 130 deletions(-) create mode 100644 PVE/Notification.pm rename test/{mail_test.pl => vzdump_notification_test.pl} (73%) pve-ha-manager: Lukas Wagner (2): manager: send notifications via new notification module manager: rename 'sendmail' --> 'send_notification' src/PVE/HA/Env.pm | 4 ++-- src/PVE/HA/Env/PVE2.pm | 15 +++++++++++++-- src/PVE/HA/NodeStatus.pm | 2 +- src/PVE/HA/Sim/Env.pm | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) Summary over all repositories: 34 files changed, 1464 insertions(+), 140 deletions(-) Generated by murpp v0.1.0 -- 2.30.2