public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module
@ 2023-03-27 15:18 Lukas Wagner
  2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 01/18] add proxmox-notification crate Lukas Wagner
                   ` (18 more replies)
  0 siblings, 19 replies; 22+ messages in thread
From: Lukas Wagner @ 2023-03-27 15:18 UTC (permalink / raw)
  To: pve-devel

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





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

end of thread, other threads:[~2023-04-14  9:47 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-27 15:18 [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 01/18] add proxmox-notification crate Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 02/18] notification: implement sendmail endpoint Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 03/18] notification: add notification filter mechanism Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 04/18] notification: implement gotify endpoint Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 05/18] notification: allow adding new sendmail endpoints / filters Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox 06/18] notification: add debian packaging Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox-perl-rs 07/18] log: set default log level to 'info', add product specfig logging env var1 Lukas Wagner
2023-03-31  9:17   ` Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH proxmox-perl-rs 08/18] add basic bindings for the proxmox_notification crate Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-cluster 09/18] cluster files: add notifications.cfg Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 10/18] test: fix names of .PHONY targets Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 11/18] add PVE::Notification module Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 12/18] vzdump: send notifications via new notification module Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 13/18] vzdump: rename 'sendmail' sub to 'send_notification' Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 14/18] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 15/18] api: apt: send notification via new notification module Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-manager 16/18] api: replication: send notifications " Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-ha-manager 17/18] manager: " Lukas Wagner
2023-03-27 15:18 ` [pve-devel] [PATCH pve-ha-manager 18/18] manager: rename 'sendmail' --> 'send_notification' Lukas Wagner
2023-04-14  6:19 ` [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module Thomas Lamprecht
2023-04-14  9:47   ` 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