public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system
@ 2023-08-03 12:16 Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module Lukas Wagner
                   ` (31 more replies)
  0 siblings, 32 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

# Overview

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.
The patch series also includes groups. They allow to send a notification
to multiple endpoints at the same time. Furthermore, there are filters.
Endpoints and groups can configure filters to determine if a notification
should be sent. For now, filters can only be configured based on notification
severity.

A short summary of what is included in this patch series:
  - Sendmail endpoint plugin: uses the system's `sendmail` command 
    to send - well - mail. The sendmail plugin sends multi-part mails
    containing HTML as well as plain text.
  - Gotify endpoint plugin: sends a notification to a gotify server
  - Groups: As for any notification event one is only able to select a single
    target, groups can be created to notify multiply endpoints at the same time
  - Filters: Endpoints and groups can also have filtering: The filter 
    can match on the notification's metadata (only severity for now) to 
    determine if it will be sent or not. Filters can be easily extended in 
    the future to match on other structured metadata as well.
  - REST API for managing endpoints, groups and filters
  - Overhauled GUI for backup jobs/one-off backups - here the use can now 
    select a notification target
  - GUI for configuring the other notification events 
    (APT, replication, fencing) - here the user can configure *when* and
    *where* to send a notification
  - Notification rendering based on templates: 
    From a single template, the system can render notifications to either
    plain text or HTML.

# Configuration

For every single notification event (backup jobs, replication, APT, fencing),
it is now possible to configure
  a.) whether to send a notification at all
  b.) where to send the notification (target)

For backup jobs, this is configured in the respective backup job config in 
`jobs.cfg`, everything else is configured using an extended `notify`
configuration key in `datacenter.cfg`:

    root@pve:~# cat /etc/pve/datacenter.cfg 
    keyboard: en-us
    notify: target-fencing=foo,fencing=always,package-updates=auto,target-package-updates=bar
  
    root@pve:~# cat /etc/pve/jobs.cfg
    vzdump: backup-2e021835-88ea
      ...
      notes-template {{guestname}}
      notification-policy always
      notification-target mail-to-root
      ...

Targets (endpoint plugins or groups) and filters are configured in two new 
configuration files: `notifications.cfg` and `priv/notifications.cfg` - 
the latter is only readable by root and is used to store any sensitive
information for notification endpoint plugins (tokens, passwords, etc.).
These two new configuration files are opaque to the Perl code, it only 
interacts with it through the bindings to the Rust implementation.

    root@pve:~# cat /etc/pve/notifications.cfg
    filter: only-errors
      min-severity error

    sendmail: sm1
      mailto-user root@pam
      mailto user1@example.com
      mailto user2@example.com
      filter on-errors

    gotify: gt1
      filter only-errors
      server https://<IP:port>
  
    root@pve:~# cat /etc/pve/priv/notifications.cfg
    gotify: gt1
      token foobar

# Permissions

The new notification module is fully integrated in the permission system.
For every target or filter, there exists a corresponding ACL path
/mapping/notifications/<name>. A user must have the Mapping.Use permissions
on that path to send the notification/use the filter. Mapping.Modify and 
Mapping.Audit are needed for writing/reading a target's/filter's config.
For groups, the user must have the Mapping.Use permission for every 
single endpoint included in the group. If a group/endpoint has a filter,
the user must have the Mapping.Use permissions for that as well.

# Backwards Compatibility

Great care was taken so that existing setups work exactly as before.
Also, every new feature of this series is strictly opt-in, meaning that 
if no settings are changed, the system *should* behave exactly as before with
regards to sending notifications. The formatting of the notification emails 
has changed *slightly*. If any users relied on that, they might require some
changes (e.g. if they scraped the mails to get the results from backup jobs).

# A note to testers

Since this patch series covers a lot of different repos, it's a bit 
cumbersome to set it up correctly. Thus, I've built .deb files that you can
just install (staging repos should be configured). 
The packages are at `iso/packages/notification` on nasi. I'll try 
to rebuild them regularly with my changes rebased on the current master
branches. Holler at me if you think anything is broken or if the packages
do not install properly.

The recommend setup to test this is a 3 node cluster (fencing notifications)
with ZFS storage (for replication notifications). For update notifications,
you can use `pvesh create /node/<hostname>/apt/update --notify 1`.
You might need to clear/rm `/var/lib/pve-manager/pkgupdates` first.

# Follow-up work (in no particular order)

  - Revisit the template helpers. Once they are considered 'stable', we 
    could offer an API for users to send notifications themselves.

  - Maybe add some fancier styling to the HTML mails? 
    e.g. a light, but recognizable Proxmox-colored style? Unfortunately 
    HTML mails have no proper support for CSS and require inline styling

  - In the future, the API might be changed/extended so that supports
    "registering" notifications.
    This also allows us to specify a 'contract' on what properties
    are guaranteed to be included with a specific notification.
    Consequently, this allows us to:
      a.) generate a list of all possible notification sources in the system 
      b.) add more sophisticated filtering (e.g. match property for 
      c.) add other endpoints types where a fixed set of metadata fields makes
          sense (e.g. webhook)

    In my head, using the notification module could look like this
    then:

        # Global context
        my backup_failed_notification = PVE::Notify::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::Notify::send(target, backup_failed_notification->instantiate({
          'host' => 'earth',
          'vmlist' => ... ,
          'logs' => ... ,
        }));

  - 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 - the 'chattier' ones should probably be 
    disabled by default
  - Add other endpoints, e.g. webhook, a generic mail endpoint that 
    sends emails directly via SMTP, etc.
  - Integrate the new module into the other products
    - for PBS, this should be relatively straight-forward

# Version bumps
- pve-manager requires:
    widget-toolkit
    pve-doc-generator
- pve-ha-manager requires libpve-notify-perl

[1] https://gotify.net/
[2] https://bugzilla.proxmox.com/show_bug.cgi?id=4526

# Changelog
Changes since v5:
  - pve-manager:
    - Fix bug in permission check in the API for testing targets
    - Fix eslint warnings (missing trailing commas)
    - Reflowed some commit messages (70 chars)
  - widget-toolkit:
    - Increase width of the endpoint edit window in preparation for endpoints
      configuration panels with 2 columns

Changes since v4:
  - pve-manager:
    - Minor code style improvements in pve-manager, based on Wolfgang's and 
      Thomas' feedback - thanks!
    - Extended some commit messages
    - Change naming of anonymous, virtual groups/endpoints
    - Allow any user to see/test the 'mail-to-root' target.
      The target is exempt from permission checks to ensure backwards compat,
      so it makes sense to display it in the notification target panel.

Changes since v3:
  - tried to document needed version bumps in cover letter
  - proxmox-perl-rs: Fixed an issue with unicode encoding by taking
    the config file contents as &[u8] and decoding instead of &str.
  - proxmox-section-config: derive Clone for SectionConfigData, so that 
    proxmox-notify's `Config` can derive Clone as well
  - proxmox-schema: add schema for comments
  - proxmox-notify: update d/copyright to new format
  - proxmox-notify: use `OnceCell` instead of `Mutex` for `Context`
  - minor stylistic code improvements, based on the feedback from the reviews
    (thanks!) - see changelog in commits for details

Changes since v2:
  - Transformed 'channels' into 'groups'. Allow to notify via a single endpoint
    or via a group (containing multiple endpoints)
  - Dropped some of the features for filters for now, such as sub-filters and 
    property-matchers. The property matching would require use to stabilize
    notification properties, which will be done later. Right now, the
    notifications only have the properties required for rendering the 
    notification template.
  - Groups can now also have filters
  - Check if a filter/group/endpoint is still used before deleting it
  - Ensure that filter/group/endpoint names are unique
  - Add new options to datacenter.cfg, allowing to configure the target
    and whether to notify at all for all notification events
  - Added new GUI panels, one for modifying the new settings in 
    datacenter.cfg, the other one for adding/modifying/deleting notification
    targets and filters
  - Integrate notification targets in the backup job UI
  - Full integration with the permission system
  - Allow users as mail recipients, looking up their email address in the user
    configuration file `user.cfg`.
  - Look up mail-from and author from datacenter.cfg, if it is not configured 
    for a sendmail endpoint
  - Support proxies for gotify endpoints. Proxy config is read from 
    datacenter.cfg
  - Move `PVE::Notify` to `pve-cluster`, building a new `libpve-notify-perl`
    package
  - The API now uses the new array type
  - Add always-available `mail-to-root` target, which is used as a fallback
    if no other target is configured for an event
  - Template rendering: Don't HTML-escape if rendering to plain text
  - many other minor changes
    
Changes since v1:
  - Some renaming:
    - PVE::Notification -> PVE::Notify
    - proxmox-notification -> proxmox-notify
  - Split configuration for gotify endpoints into a public part in
    `notifications.cfg` and a private part for the token in 
    `priv/notifications.cfg`
  - Add template-based notification rendering (`proxmox`), including helpers 
    for: 
    - tables
    - pretty printed JSON
    - duration, timestamps
    - byte sizes
  - Add notification channels (repo `proxmox`)
  - Add API routes for channels, endpoints, filters (implementation in 
    `proxmox-notify`, glue code in `proxmox-perl-rs` and handler in 
    `pve-manager`)
  - Integrated new notification channels in backup jobs/one-off backups (repo 
    `pve-manager`)
  - Replication/APT/Fencing use an 'anonymous' channel with a temporary 
    sendmail endpoint, sending mails to `root`
  - Added new options for backup jobs
  - Reworked git history

Versions of this patch series:
v5: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058467.html
v4: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058315.html
v3: https://lists.proxmox.com/pipermail/pve-devel/2023-July/058158.html
v2: https://lists.proxmox.com/pipermail/pve-devel/2023-May/056927.html
v1: https://lists.proxmox.com/pipermail/pve-devel/2023-March/056445.html



pve-ha-manager:

Lukas Wagner (1):
  manager: send notifications via new notification module

 debian/control           |  2 ++
 src/PVE/HA/Env.pm        |  6 ++---
 src/PVE/HA/Env/PVE2.pm   | 21 +++++++++-------
 src/PVE/HA/NodeStatus.pm | 52 ++++++++++++++++++++++++----------------
 src/PVE/HA/Sim/Env.pm    | 10 ++++++--
 5 files changed, 57 insertions(+), 34 deletions(-)


pve-manager:

Lukas Wagner (23):
  d/control: add dependency to `libpve-notify-perl`
  vzdump: send notifications via new notification module
  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
  api: prepare api handler module for notification config
  api: notification: add api routes for groups
  api: notification: add api routes for sendmail endpoints
  api: notification: add api routes for gotify endpoints
  api: notification: add api routes for filters
  api: notification: allow fetching notification targets
  api: notification: allow to test targets
  api: notification: disallow removing targets if they are used
  ui: backup: allow to select notification target for jobs
  ui: backup: adapt backup job details to new notification params
  ui: backup: allow to set notification-target for one-off backups
  ui: allow to configure notification event -> target mapping
  ui: add notification target configuration panel
  ui: perm path: add ACL paths for notifications, usb and pci mappings
  ui: perm path: increase width of the perm path selector combobox
  ui: dc: remove notify key from datacenter option view
  vzdump: use <name> as a convention for virtual endpoints/groups
  api: notification: make the 'mail-to-root' target visible to any user

 PVE/API2/APT.pm                               |   99 +-
 PVE/API2/Cluster.pm                           |    7 +
 PVE/API2/Cluster/Makefile                     |    1 +
 PVE/API2/Cluster/Notifications.pm             | 1332 +++++++++++++++++
 PVE/API2/Replication.pm                       |   63 +-
 PVE/API2/VZDump.pm                            |   10 +-
 PVE/VZDump.pm                                 |  335 +++--
 debian/control                                |    2 +
 test/Makefile                                 |    8 +-
 ...il_test.pl => vzdump_notification_test.pl} |   36 +-
 www/manager6/Makefile                         |    5 +-
 www/manager6/data/PermPathStore.js            |    3 +
 www/manager6/dc/Backup.js                     |   84 +-
 www/manager6/dc/BackupJobDetail.js            |   20 +-
 www/manager6/dc/Config.js                     |   28 +
 www/manager6/dc/NotificationEvents.js         |  277 ++++
 www/manager6/dc/OptionView.js                 |   20 -
 www/manager6/form/NotificationModeSelector.js |    8 +
 ...ector.js => NotificationPolicySelector.js} |    1 +
 .../form/NotificationTargetSelector.js        |   54 +
 www/manager6/form/PermPathSelector.js         |    1 +
 www/manager6/window/Backup.js                 |   35 +-
 22 files changed, 2176 insertions(+), 253 deletions(-)
 create mode 100644 PVE/API2/Cluster/Notifications.pm
 rename test/{mail_test.pl => vzdump_notification_test.pl} (62%)
 create mode 100644 www/manager6/dc/NotificationEvents.js
 create mode 100644 www/manager6/form/NotificationModeSelector.js
 rename www/manager6/form/{EmailNotificationSelector.js => NotificationPolicySelector.js} (87%)
 create mode 100644 www/manager6/form/NotificationTargetSelector.js


proxmox-widget-toolkit:

Lukas Wagner (5):
  notification: add gui for sendmail notification endpoints
  notification: add gui for gotify notification endpoints
  notification: add gui for notification groups
  notification: allow to select filter for notification targets
  notification: add ui for managing notification filters

 src/Makefile                            |   8 +
 src/Schema.js                           |  18 ++
 src/data/model/NotificationConfig.js    |  17 ++
 src/form/NotificationFilterSelector.js  |  58 +++++
 src/panel/GotifyEditPanel.js            |  53 ++++
 src/panel/NotificationConfigView.js     | 319 ++++++++++++++++++++++++
 src/panel/NotificationGroupEditPanel.js | 183 ++++++++++++++
 src/panel/SendmailEditPanel.js          | 139 +++++++++++
 src/window/EndpointEditBase.js          |  57 +++++
 src/window/NotificationFilterEdit.js    | 109 ++++++++
 10 files changed, 961 insertions(+)
 create mode 100644 src/data/model/NotificationConfig.js
 create mode 100644 src/form/NotificationFilterSelector.js
 create mode 100644 src/panel/GotifyEditPanel.js
 create mode 100644 src/panel/NotificationConfigView.js
 create mode 100644 src/panel/NotificationGroupEditPanel.js
 create mode 100644 src/panel/SendmailEditPanel.js
 create mode 100644 src/window/EndpointEditBase.js
 create mode 100644 src/window/NotificationFilterEdit.js


pve-docs:

Lukas Wagner (1):
  add documentation for the new notification system

 notifications.adoc   | 159 +++++++++++++++++++++++++++++++++++++++++++
 pve-admin-guide.adoc |   2 +
 pve-gui.adoc         |   2 +
 vzdump.adoc          |   5 ++
 4 files changed, 168 insertions(+)
 create mode 100644 notifications.adoc


Summary over all repositories:
  41 files changed, 3362 insertions(+), 287 deletions(-)

-- 
murpp v0.4.0





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

* [pve-devel] [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 15:39   ` [pve-devel] applied: " Thomas Lamprecht
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 02/30] d/control: add dependency to `libpve-notify-perl` Lukas Wagner
                   ` (30 subsequent siblings)
  31 siblings, 1 reply; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

... instead of using sendmail directly.

If the new 'notify.target-fencing' parameter from datacenter config
is set, we use it as a target for notifications. If it is not set,
we send the notification to the default target (mail-to-root).

There is also a new 'notify.fencing' paramter which controls if
notifications should be sent at all. If it is not set, we
default to the old behavior, which is to send.

Also add dependency to the `libpve-notify-perl` package to d/control.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 debian/control           |  2 ++
 src/PVE/HA/Env.pm        |  6 ++---
 src/PVE/HA/Env/PVE2.pm   | 21 +++++++++-------
 src/PVE/HA/NodeStatus.pm | 52 ++++++++++++++++++++++++----------------
 src/PVE/HA/Sim/Env.pm    | 10 ++++++--
 5 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/debian/control b/debian/control
index 5bff56a..ffa9c1c 100644
--- a/debian/control
+++ b/debian/control
@@ -8,6 +8,7 @@ Build-Depends: debhelper-compat (= 13),
                libpve-access-control,
                libpve-cluster-perl,
                libpve-common-perl,
+               libpve-notify-perl,
                libpve-rs-perl (>= 0.7.3),
                lintian,
                pve-cluster,
@@ -21,6 +22,7 @@ Architecture: any
 Depends: libjson-perl,
          libpve-cluster-perl,
          libpve-common-perl,
+         libpve-notify-perl,
          libpve-rs-perl (>= 0.7.3),
          pve-cluster (>= 3.0-17),
          pve-container (>= 5.0.1),
diff --git a/src/PVE/HA/Env.pm b/src/PVE/HA/Env.pm
index 16603ec..b7060a4 100644
--- a/src/PVE/HA/Env.pm
+++ b/src/PVE/HA/Env.pm
@@ -144,10 +144,10 @@ sub log {
     return $self->{plug}->log($level, @args);
 }
 
-sub sendmail {
-    my ($self, $subject, $text) = @_;
+sub send_notification {
+    my ($self, $subject, $text, $properties) = @_;
 
-    return $self->{plug}->sendmail($subject, $text);
+    return $self->{plug}->send_notification($subject, $text, $properties);
 }
 
 # acquire a cluster wide manager lock
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index f6ebfeb..ea9e6e4 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -13,6 +13,7 @@ use PVE::Cluster qw(cfs_register_file cfs_read_file cfs_write_file cfs_lock_file
 use PVE::DataCenterConfig;
 use PVE::INotify;
 use PVE::RPCEnvironment;
+use PVE::Notify;
 
 use PVE::HA::Tools ':exit_codes';
 use PVE::HA::Env;
@@ -219,16 +220,20 @@ sub log {
     syslog($level, $msg);
 }
 
-sub sendmail {
-    my ($self, $subject, $text) = @_;
+sub send_notification {
+    my ($self, $subject, $text, $properties) = @_;
 
-    # Leave it to postfix to append the correct hostname
-    my $mailfrom = 'root';
-    # /root/.forward makes pvemailforward redirect the
-    # mail to the address configured in the datacenter
-    my $mailto = 'root';
+    eval {
+	my $dc_config = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	my $target = $dc_config->{notify}->{'target-fencing'} // PVE::Notify::default_target();
+	my $notify = $dc_config->{notify}->{fencing} // 'always';
+
+	if ($notify eq 'always') {
+	    PVE::Notify::error($target, $subject, $text, $properties);
+	}
+    };
 
-    PVE::Tools::sendmail($mailto, $subject, $text, undef, $mailfrom);
+    $self->log("warning", "could not notify: $@") if $@;
 }
 
 my $last_lock_status_hash = {};
diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
index ee5be8e..b264a36 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -188,35 +188,45 @@ sub update {
    }
 }
 
-# assembles a commont text for fence emails
-my $send_fence_state_email = sub {
-    my ($self, $subject_prefix, $subject, $node) = @_;
-
-    my $haenv = $self->{haenv};
-
-    my $mail_text = <<EOF
-The node '$node' failed and needs manual intervention.
+my $body_template = <<EOT;
+{{#verbatim}}
+The node '{{node}}' failed and needs manual intervention.
 
-The PVE HA manager tries  to fence it and recover the
-configured HA resources to a healthy node if possible.
+The PVE HA manager tries  to fence it and recover the configured HA resources to
+a healthy node if possible.
 
-Current fence status:  $subject_prefix
-$subject
+Current fence status: {{subject-prefix}}
+{{subject}}
+{{/verbatim}}
 
+{{heading-2 "Overall Cluster status:"}}
+{{object status-data}}
+EOT
 
-Overall Cluster status:
------------------------
+my $subject_template = "{{subject-prefix}}: {{subject}}";
 
-EOF
-;
-    my $mail_subject = $subject_prefix . ': ' . $subject;
+# assembles a commont text for fence emails
+my $send_fence_state_email = sub {
+    my ($self, $subject_prefix, $subject, $node) = @_;
 
+    my $haenv = $self->{haenv};
     my $status = $haenv->read_manager_status();
-    my $data = { manager_status => $status, node_status => $self->{status} };
-
-    $mail_text .= to_json($data, { pretty => 1, canonical => 1});
 
-    $haenv->sendmail($mail_subject, $mail_text);
+    my $notification_properties = {
+	"status-data"    => {
+	    manager_status => $status,
+	    node_status    => $self->{status}
+	},
+	"node"           => $node,
+	"subject-prefix" => $subject_prefix,
+	"subject"        => $subject,
+    };
+
+    $haenv->send_notification(
+	$subject_template,
+	$body_template,
+	$notification_properties
+    );
 };
 
 
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index c6ea73c..d3aea8d 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -288,8 +288,14 @@ sub log {
     printf("%-5s %5d %12s: $msg\n", $level, $time, "$self->{nodename}/$self->{log_id}");
 }
 
-sub sendmail {
-    my ($self, $subject, $text) = @_;
+sub send_notification {
+    my ($self, $subject, $text, $properties) = @_;
+
+    # The template for the subject is "{{subject-prefix}}: {{subject}}"
+    # We have to perform poor-man's template rendering to pass the test cases.
+
+    $subject = $subject =~ s/\{\{subject-prefix}}/$properties->{"subject-prefix"}/r;
+    $subject = $subject =~ s/\{\{subject}}/$properties->{"subject"}/r;
 
     # only log subject, do not spam the logs
     $self->log('email', $subject);
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 02/30] d/control: add dependency to `libpve-notify-perl`
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 03/30] vzdump: send notifications via new notification module Lukas Wagner
                   ` (29 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Did not add version number since I do not know which it will be yet.

 debian/control | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/debian/control b/debian/control
index 3206b514..b807dbfe 100644
--- a/debian/control
+++ b/debian/control
@@ -14,6 +14,7 @@ Build-Depends: debhelper-compat (= 13),
                libpve-common-perl (>= 7.2-6),
                libpve-guest-common-perl (>= 5.0.2),
                libpve-http-server-perl (>= 2.0-12),
+               libpve-notify-perl,
                libpve-rs-perl (>= 0.7.1),
                libpve-storage-perl (>= 6.3-2),
                libtemplate-perl,
@@ -61,6 +62,7 @@ Depends: apt (>= 1.5~),
          libpve-common-perl (>= 7.2-7),
          libpve-guest-common-perl (>= 5.0.2),
          libpve-http-server-perl (>= 4.1-1),
+         libpve-notify-perl,
          libpve-rs-perl (>= 0.7.1),
          libpve-storage-perl (>= 7.2-12),
          librados2-perl (>= 1.3-1),
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 03/30] vzdump: send notifications via new notification module
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 02/30] d/control: add dependency to `libpve-notify-perl` Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 04/30] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
                   ` (28 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

... instead of using sendmail directly.

If the new 'notification-target' parameter is set,
we send the notification to this endpoint or group.
If 'mailto' is set, we add a temporary endpoint and a
temporary group containg both targets.

This commit also refactors the old 'sendmail' sub heavily:
  - Use template-based notification text instead of endless
    string concatenations
  - Removing the old plaintext/HTML table rendering in favor of
    the new template/property-based approach offered by the
    `proxmox-notify` crate.
  - Rename `sendmail` sub to `send_notification`
  - Breaking out some of the code into helper subs, hopefully
    reducing the spaghetti factor a bit

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/VZDump.pm |  10 +-
 PVE/VZDump.pm      | 335 +++++++++++++++++++++++++--------------------
 test/mail_test.pl  |  36 ++---
 3 files changed, 214 insertions(+), 167 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index e3dcd0bd..3886772e 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -44,7 +44,9 @@ __PACKAGE__->register_method ({
 	    ."'Datastore.AllocateSpace' on the backup storage. The 'tmpdir', 'dumpdir' and "
 	    ."'script' parameters are restricted to the 'root\@pam' user. The 'maxfiles' and "
 	    ."'prune-backups' settings require 'Datastore.Allocate' on the backup storage. The "
-	    ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'.",
+	    ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. "
+	    ."If 'notification-target' is set, then the 'Mapping.Use' permission is needed on "
+	    ."'/mapping/notification/<target>'.",
 	user => 'all',
     },
     protected => 1,
@@ -113,6 +115,10 @@ __PACKAGE__->register_method ({
 	    $rpcenv->check($user, "/storage/$storeid", [ 'Datastore.AllocateSpace' ]);
 	}
 
+	if (my $target = $param->{'notification-target'}) {
+	    PVE::Notify::check_may_use_target($target, $rpcenv);
+	}
+
 	my $worker = sub {
 	    my $upid = shift;
 
@@ -127,7 +133,7 @@ __PACKAGE__->register_method ({
 		$vzdump->getlock($upid); # only one process allowed
 	    };
 	    if (my $err = $@) {
-		$vzdump->sendmail([], 0, $err);
+		$vzdump->send_notification([], 0, $err);
 		exit(-1);
 	    }
 
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index c58e5f78..7dc9f31e 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -19,6 +19,7 @@ use PVE::Exception qw(raise_param_exc);
 use PVE::HA::Config;
 use PVE::HA::Env::PVE2;
 use PVE::JSONSchema qw(get_standard_option);
+use PVE::Notify;
 use PVE::RPCEnvironment;
 use PVE::Storage;
 use PVE::VZDump::Common;
@@ -317,21 +318,90 @@ sub read_vzdump_defaults {
     return $res;
 }
 
-use constant MAX_MAIL_SIZE => 1024*1024;
-sub sendmail {
-    my ($self, $tasklist, $totaltime, $err, $detail_pre, $detail_post) = @_;
+sub read_backup_task_logs {
+    my ($task_list) = @_;
 
-    my $opts = $self->{opts};
+    my $task_logs = "";
 
-    my $mailto = $opts->{mailto};
+    for my $task (@$task_list) {
+	my $vmid = $task->{vmid};
+	my $log_file = $task->{tmplog};
+	if (!$task->{tmplog}) {
+	    $task_logs .= "$vmid: no log available\n\n";
+	    next;
+	}
+	if (open (my $TMP, '<', "$log_file")) {
+	    while (my $line = <$TMP>) {
+		next if $line =~ /^status: \d+/; # not useful in mails
+		$task_logs .= encode8bit ("$vmid: $line");
+	    }
+	    close ($TMP);
+	} else {
+	    $task_logs .= "$vmid: Could not open log file\n\n";
+	}
+	$task_logs .= "\n";
+    }
 
-    return if !($mailto && scalar(@$mailto));
+    return $task_logs;
+}
 
-    my $cmdline = $self->{cmdline};
+sub build_guest_table {
+    my ($task_list) = @_;
+
+    my $table = {
+	schema => {
+	    columns => [
+		{
+		    label => "VMID",
+		    id  => "vmid"
+		},
+		{
+		    label => "Name",
+		    id  => "name"
+		},
+		{
+		    label => "Status",
+		    id  => "status"
+		},
+		{
+		    label => "Time",
+		    id  => "time",
+		    renderer => "duration"
+		},
+		{
+		    label => "Size",
+		    id  => "size",
+		    renderer => "human-bytes"
+		},
+		{
+		    label => "Filename",
+		    id  => "filename"
+		},
+	    ]
+	},
+	data => []
+    };
+
+    for my $task (@$task_list) {
+	my $successful = $task->{state} eq 'ok';
+	my $size = $successful ? $task->{size} : 0;
+	my $filename = $successful ? $task->{target} : undef;
+	push @{$table->{data}}, {
+	    "vmid" => $task->{vmid},
+	    "name" => $task->{hostname},
+	    "status" => $task->{state},
+	    "time" => $task->{backuptime},
+	    "size" => $size,
+	    "filename" => $filename,
+	};
+    }
+
+    return $table;
+}
 
-    my $ecount = 0;
-    foreach my $task (@$tasklist) {
-	$ecount++ if $task->{state} ne 'ok';
+sub sanitize_task_list {
+    my ($task_list) = @_;
+    for my $task (@$task_list) {
 	chomp $task->{msg} if $task->{msg};
 	$task->{backuptime} = 0 if !$task->{backuptime};
 	$task->{size} = 0 if !$task->{size};
@@ -342,164 +412,133 @@ sub sendmail {
 	    $task->{msg} = 'aborted';
 	}
     }
+}
 
-    my $notify = $opts->{mailnotification} || 'always';
-    return if (!$ecount && !$err && ($notify eq 'failure'));
+sub count_failed_tasks {
+    my ($tasklist) = @_;
 
-    my $stat = ($ecount || $err) ? 'backup failed' : 'backup successful';
-    if ($err) {
-	if ($err =~ /\n/) {
-	    $stat .= ": multiple problems";
-	} else {
-	    $stat .= ": $err";
-	    $err = undef;
-	}
+    my $error_count = 0;
+    for my $task (@$tasklist) {
+	$error_count++ if $task->{state} ne 'ok';
     }
 
+    return $error_count;
+}
+
+sub get_hostname {
     my $hostname = `hostname -f` || PVE::INotify::nodename();
     chomp $hostname;
+    return $hostname;
+}
 
-    # text part
-    my $text = $err ? "$err\n\n" : '';
-    my $namelength = 20;
-    $text .= sprintf (
-	"%-10s %-${namelength}s %-6s %10s %10s  %s\n",
-	qw(VMID NAME STATUS TIME SIZE FILENAME)
-    );
-    foreach my $task (@$tasklist) {
-	my $name = substr($task->{hostname}, 0, $namelength);
-	my $successful = $task->{state} eq 'ok';
-	my $size = $successful ? format_size ($task->{size}) : 0;
-	my $filename = $successful ? $task->{target} : '-';
-	my $size_fmt = $successful ? "%10s": "%8.2fMB";
-	$text .= sprintf(
-	    "%-10s %-${namelength}s %-6s %10s $size_fmt  %s\n",
-	    $task->{vmid},
-	    $name,
-	    $task->{state},
-	    format_time($task->{backuptime}),
-	    $size,
-	    $filename,
-	);
-    }
+my $subject_template = "vzdump backup status ({{hostname}}): {{status-text}}";
 
-    my $text_log_part;
-    $text_log_part .= "\nDetailed backup logs:\n\n";
-    $text_log_part .= "$cmdline\n\n";
+my $body_template = <<EOT;
+{{error-message}}
+{{heading-1 "Details"}}
+{{table guest-table}}
 
-    $text_log_part .= $detail_pre . "\n" if defined($detail_pre);
-    foreach my $task (@$tasklist) {
-	my $vmid = $task->{vmid};
-	my $log = $task->{tmplog};
-	if (!$log) {
-	    $text_log_part .= "$vmid: no log available\n\n";
-	    next;
-	}
-	if (open (my $TMP, '<', "$log")) {
-	    while (my $line = <$TMP>) {
-		next if $line =~ /^status: \d+/; # not useful in mails
-		$text_log_part .= encode8bit ("$vmid: $line");
-	    }
-	    close ($TMP);
-	} else {
-	    $text_log_part .= "$vmid: Could not open log file\n\n";
-	}
-	$text_log_part .= "\n";
-    }
-    $text_log_part .= $detail_post if defined($detail_post);
+Total running time: {{duration total-time}}
 
-    # html part
-    my $html = "<html><body>\n";
-    $html .= "<p>" . (escape_html($err) =~ s/\n/<br>/gr) . "</p>\n" if $err;
-    $html .= "<table border=1 cellpadding=3>\n";
-    $html .= "<tr><td>VMID<td>NAME<td>STATUS<td>TIME<td>SIZE<td>FILENAME</tr>\n";
+{{heading-1 "Logs"}}
+{{verbatim-monospaced logs}}
+EOT
 
-    my $ssize = 0;
-    foreach my $task (@$tasklist) {
-	my $vmid = $task->{vmid};
-	my $name = $task->{hostname};
-
-	if  ($task->{state} eq 'ok') {
-	    $ssize += $task->{size};
-
-	    $html .= sprintf (
-	        "<tr><td>%s<td>%s<td>OK<td>%s<td align=right>%s<td>%s</tr>\n",
-	        $vmid,
-	        $name,
-	        format_time($task->{backuptime}),
-	        format_size ($task->{size}),
-	        escape_html ($task->{target}),
-	    );
-	} else {
-	    $html .= sprintf (
-	        "<tr><td>%s<td>%s<td><font color=red>FAILED<td>%s<td colspan=2>%s</tr>\n",
-	        $vmid,
-	        $name,
-	        format_time($task->{backuptime}),
-	        escape_html ($task->{msg}),
-	    );
-	}
-    }
+use constant MAX_LOG_SIZE => 1024*1024;
 
-    $html .= sprintf ("<tr><td align=left colspan=3>TOTAL<td>%s<td>%s<td></tr>",
- format_time ($totaltime), format_size ($ssize));
+sub send_notification {
+    my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
 
-    $html .= "\n</table><br><br>\n";
-    my $html_log_part;
-    $html_log_part .= "Detailed backup logs:<br /><br />\n";
-    $html_log_part .= "<pre>\n";
-    $html_log_part .= escape_html($cmdline) . "\n\n";
+    my $opts = $self->{opts};
+    my $mailto = $opts->{mailto};
+    my $cmdline = $self->{cmdline};
+    my $target = $opts->{"notification-target"};
+    # Fall back to 'mailnotification' if 'notification-policy' is not set.
+    # If both are set, 'notification-policy' takes precedence
+    my $policy = $opts->{"notification-policy"} // $opts->{mailnotification} // 'always';
 
-    $html_log_part .= escape_html($detail_pre) . "\n" if defined($detail_pre);
-    foreach my $task (@$tasklist) {
-	my $vmid = $task->{vmid};
-	my $log = $task->{tmplog};
-	if (!$log) {
-	    $html_log_part .= "$vmid: no log available\n\n";
-	    next;
-	}
-	if (open (my $TMP, '<', "$log")) {
-	    while (my $line = <$TMP>) {
-		next if $line =~ /^status: \d+/; # not useful in mails
-		if ($line =~ m/^\S+\s\d+\s+\d+:\d+:\d+\s+(ERROR|WARN):/) {
-		    $html_log_part .= encode8bit ("$vmid: <font color=red>".
-			escape_html ($line) . "</font>");
-		} else {
-		    $html_log_part .= encode8bit ("$vmid: " . escape_html ($line));
-		}
-	    }
-	    close ($TMP);
+    return if ($policy eq 'never');
+
+    sanitize_task_list($tasklist);
+    my $error_count = count_failed_tasks($tasklist);
+
+    my $failed = ($error_count || $err);
+
+    return if (!$failed && ($policy eq 'failure'));
+
+    my $status_text = $failed ? 'backup failed' : 'backup successful';
+
+    if ($err) {
+	if ($err =~ /\n/) {
+	    $status_text .= ": multiple problems";
 	} else {
-	    $html_log_part .= "$vmid: Could not open log file\n\n";
+	    $status_text .= ": $err";
+	    $err = undef;
 	}
-	$html_log_part .= "\n";
     }
-    $html_log_part .= escape_html($detail_post) if defined($detail_post);
-    $html_log_part .= "</pre>";
-    my $html_end = "\n</body></html>\n";
-    # end html part
-
-    if (length($text) + length($text_log_part) +
-	length($html) + length($html_log_part) +
-	length($html_end) < MAX_MAIL_SIZE)
+
+    my $text_log_part = "$cmdline\n\n";
+    $text_log_part .= $detail_pre . "\n" if defined($detail_pre);
+    $text_log_part .= read_backup_task_logs($tasklist);
+    $text_log_part .= $detail_post if defined($detail_post);
+
+    if (length($text_log_part)  > MAX_LOG_SIZE)
     {
-	$html .= $html_log_part;
-	$html .= $html_end;
-	$text .= $text_log_part;
-    } else {
-	my $msg = "Log output was too long to be sent by mail. ".
+	# Let's limit the maximum length of included logs
+	$text_log_part = "Log output was too long to be sent. ".
 	    "See Task History for details!\n";
-	$text .= $msg;
-	$html .= "<p>$msg</p>";
-	$html .= $html_end;
+    };
+
+    my $notification_props = {
+	"hostname"      => get_hostname(),
+	"error-message" => $err,
+	"guest-table"   => build_guest_table($tasklist),
+	"logs"          => $text_log_part,
+	"status-text"   => $status_text,
+	"total-time"    => $total_time,
+    };
+
+    my $notification_config = PVE::Notify::read_config();
+
+    if ($mailto && scalar(@$mailto)) {
+	# <, >, @ is not allowed in endpoint names, but only it is only
+	# verified once the config is serialized. That means that
+	# we can rely on that fact that no other endpoint with this name exists.
+	my $endpoint_name = "mail-to-<" . join(",", @$mailto) . ">";
+	$notification_config->add_sendmail_endpoint(
+	    $endpoint_name,
+	    $mailto,
+	    undef,
+	    undef,
+	    "vzdump backup tool");
+
+	my $endpoints = [$endpoint_name];
+
+	# Create an anonymous group containing the sendmail endpoint and the
+	# $target endpoint, if specified
+	if ($target) {
+	    push @$endpoints, $target;
+	}
+
+	$target = "group-$endpoint_name";
+	$notification_config->add_group(
+	    $target,
+	    $endpoints,
+	);
     }
 
-    my $subject = "vzdump backup status ($hostname) : $stat";
+    return if (!$target);
 
-    my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
-    my $mailfrom = $dcconf->{email_from} || "root";
+    my $severity = $failed ? "error" : "info";
 
-    PVE::Tools::sendmail($mailto, $subject, $text, $html, $mailfrom, "vzdump backup tool");
+    PVE::Notify::notify(
+	$target,
+	$severity,
+	$subject_template,
+	$body_template,
+	$notification_props,
+	$notification_config
+    );
 };
 
 sub new {
@@ -632,7 +671,7 @@ sub new {
     }
 
     if ($errors) {
-	eval { $self->sendmail([], 0, $errors); };
+	eval { $self->send_notification([], 0, $errors); };
 	debugmsg ('err', $@) if $@;
 	die "$errors\n";
     }
@@ -1322,11 +1361,11 @@ sub exec_backup {
     my $totaltime = time() - $starttime;
 
     eval {
-	# otherwise $self->sendmail() will interpret it as multiple problems
+	# otherwise $self->send_notification() will interpret it as multiple problems
 	my $chomped_err = $err;
 	chomp($chomped_err) if $chomped_err;
 
-	$self->sendmail(
+	$self->send_notification(
 	    $tasklist,
 	    $totaltime,
 	    $chomped_err,
diff --git a/test/mail_test.pl b/test/mail_test.pl
index d0114441..0635ebb7 100755
--- a/test/mail_test.pl
+++ b/test/mail_test.pl
@@ -5,7 +5,7 @@ use warnings;
 
 use lib '..';
 
-use Test::More tests => 5;
+use Test::More tests => 3;
 use Test::MockModule;
 
 use PVE::VZDump;
@@ -29,17 +29,19 @@ sub prepare_mail_with_status {
 sub prepare_long_mail {
     open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
     # 0.5 MB * 2 parts + the overview tables gives more than 1 MB mail
-    print TEST_FILE "a" x (1024*1024/2);
+    print TEST_FILE "a" x (1024*1024);
     close(TEST_FILE);
 }
 
-my ($result_text, $result_html);
+my $result_text;
+my $result_properties;
+
+my $mock_notification_module = Test::MockModule->new('PVE::Notify');
+$mock_notification_module->mock('send_notification', sub {
+    my ($channel, $severity, $title, $text, $properties) = @_;
 
-my $mock_tools_module = Test::MockModule->new('PVE::Tools');
-$mock_tools_module->mock('sendmail', sub {
-    my (undef, undef, $text, $html, undef, undef) = @_;
     $result_text = $text;
-    $result_html = $html;
+    $result_properties = $properties;
 });
 
 my $mock_cluster_module = Test::MockModule->new('PVE::Cluster');
@@ -47,7 +49,9 @@ $mock_cluster_module->mock('cfs_read_file', sub {
     my $path = shift;
 
     if ($path eq 'datacenter.cfg') {
-	return {};
+        return {};
+    } elsif ($path eq 'notifications.cfg' || $path eq 'priv/notifications.cfg') {
+        return '';
     } else {
 	die "unexpected cfs_read_file\n";
     }
@@ -62,28 +66,26 @@ my $SELF = {
 my $task = { state => 'ok', vmid => '100', };
 my $tasklist;
 sub prepare_test {
-    $result_text = $result_html = undef;
+    $result_text = undef;
     $task->{tmplog} = shift;
     $tasklist = [ $task ];
 }
 
 {
     prepare_test($TEST_FILE_WRONG_PATH);
-    PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef);
-    like($result_text, $NO_LOGFILE, "Missing logfile is detected");
+    PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
+    like($result_properties->{logs}, $NO_LOGFILE, "Missing logfile is detected");
 }
 {
     prepare_test($TEST_FILE_PATH);
     prepare_mail_with_status();
-    PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef);
-    unlike($result_text, $STATUS, "Status are not in text part of mails");
-    unlike($result_html, $STATUS, "Status are not in HTML part of mails");
+    PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
+    unlike($result_properties->{"status-text"}, $STATUS, "Status are not in text part of mails");
 }
 {
     prepare_test($TEST_FILE_PATH);
     prepare_long_mail();
-    PVE::VZDump::sendmail($SELF, $tasklist, 0, undef, undef, undef);
-    like($result_text, $LOG_TOO_LONG, "Text part of mails gets shortened");
-    like($result_html, $LOG_TOO_LONG, "HTML part of mails gets shortened");
+    PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
+    like($result_properties->{logs}, $LOG_TOO_LONG, "Text part of mails gets shortened");
 }
 unlink $TEST_FILE_PATH;
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 04/30] test: rename mail_test.pl to vzdump_notification_test.pl
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (2 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 03/30] vzdump: send notifications via new notification module Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 05/30] api: apt: send notification via new notification module Lukas Wagner
                   ` (27 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 test/Makefile                                      | 8 ++++----
 test/{mail_test.pl => vzdump_notification_test.pl} | 0
 2 files changed, 4 insertions(+), 4 deletions(-)
 rename test/{mail_test.pl => vzdump_notification_test.pl} (100%)

diff --git a/test/Makefile b/test/Makefile
index cccdc1c9..62d75050 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,7 +5,7 @@ all:
 export PERLLIB=..
 
 .PHONY: check
-check: test-replication test-balloon test-mail test-vzdump test-osd
+check: test-replication test-balloon test-vzdump-notification test-vzdump test-osd
 
 .PHONY: test-balloon
 test-balloon:
@@ -17,9 +17,9 @@ test-replication: replication1.t replication2.t replication3.t replication4.t re
 replication%.t: replication_test%.pl
 	./$<
 
-.PHONY: test-mail
-test-mail:
-	./mail_test.pl
+.PHONY: test-vzdump-notification
+test-vzdump-notification:
+	./vzdump_notification_test.pl
 
 .PHONY: test-vzdump
 test-vzdump: test-vzdump-guest-included test-vzdump-new
diff --git a/test/mail_test.pl b/test/vzdump_notification_test.pl
similarity index 100%
rename from test/mail_test.pl
rename to test/vzdump_notification_test.pl
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 05/30] api: apt: send notification via new notification module
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (3 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 04/30] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 06/30] api: replication: send notifications " Lukas Wagner
                   ` (26 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

... instead of using sendmail directly

If the new 'target-package-updates' is set, we send a notification to
this target. If not, we continue to send a mail to root@pam (if the
mail address is configured)

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/APT.pm | 99 ++++++++++++++++++++++++++++++++++---------------
 1 file changed, 70 insertions(+), 29 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index f73535e1..a213fc59 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -19,6 +19,7 @@ use PVE::DataCenterConfig;
 use PVE::SafeSyslog;
 use PVE::INotify;
 use PVE::Exception;
+use PVE::Notify;
 use PVE::RESTHandler;
 use PVE::RPCEnvironment;
 use PVE::API2Tools;
@@ -272,6 +273,12 @@ __PACKAGE__->register_method({
 	return $pkglist;
     }});
 
+my $updates_available_subject_template = "New software packages available ({{hostname}})";
+my $updates_available_body_template = <<EOT;
+The following updates are available:
+{{table updates}}
+EOT
+
 __PACKAGE__->register_method({
     name => 'update_database',
     path => 'update',
@@ -279,6 +286,8 @@ __PACKAGE__->register_method({
     description => "This is used to resynchronize the package index files from their sources (apt-get update).",
     permissions => {
 	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
+	description => "If 'notify: target-package-updates' is set, then the user must have the "
+	    . "'Mapping.Use' permission on '/mapping/notification/<target>'",
     },
     protected => 1,
     proxyto => 'node',
@@ -307,6 +316,17 @@ __PACKAGE__->register_method({
 	my ($param) = @_;
 
 	my $rpcenv = PVE::RPCEnvironment::get();
+	my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	my $target = $dcconf->{notify}->{'target-package-updates'} //
+	    PVE::Notify::default_target();
+
+	if ($param->{notify} && $target ne PVE::Notify::default_target()) {
+	    # If we notify via anything other than the default target (mail to root),
+	    # then the user must have the proper permissions for the target.
+	    # The mail-to-root target does not require these, as otherwise
+	    # we would break compatibility.
+	    PVE::Notify::check_may_use_target($target, $rpcenv);
+	}
 
 	my $authuser = $rpcenv->get_user();
 
@@ -314,7 +334,6 @@ __PACKAGE__->register_method({
 	    my $upid = shift;
 
 	    # setup proxy for apt
-	    my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
 
 	    my $aptconf = "// no proxy configured\n";
 	    if ($dcconf->{http_proxy}) {
@@ -336,39 +355,59 @@ __PACKAGE__->register_method({
 	    my $pkglist = &$update_pve_pkgstatus();
 
 	    if ($param->{notify} && scalar(@$pkglist)) {
+		my $updates_table = {
+		    schema => {
+			columns => [
+			    {
+				label => "Package",
+				id    => "package",
+			    },
+			    {
+				label => "Old Version",
+				id    => "old-version",
+			    },
+			    {
+				label => "New Version",
+				id    => "new-version",
+			    }
+			]
+		    },
+		    data => []
+		};
+
+		my $hostname = `hostname -f` || PVE::INotify::nodename();
+		chomp $hostname;
+
+		my $count = 0;
+		foreach my $p (sort {$a->{Package} cmp $b->{Package} } @$pkglist) {
+		    next if $p->{NotifyStatus} && $p->{NotifyStatus} eq $p->{Version};
+		    $count++;
+
+		    push @{$updates_table->{data}}, {
+			"package"     => $p->{Package},
+			"old-version" => $p->{OldVersion},
+			"new-version" => $p->{Version}
+		    };
+		}
 
-		my $usercfg = PVE::Cluster::cfs_read_file("user.cfg");
-		my $rootcfg = $usercfg->{users}->{'root@pam'} || {};
-		my $mailto = $rootcfg->{email};
-
-		if ($mailto) {
-		    my $hostname = `hostname -f` || PVE::INotify::nodename();
-		    chomp $hostname;
-		    my $mailfrom = $dcconf->{email_from} || "root";
-		    my $subject = "New software packages available ($hostname)";
-
-		    my $data = "The following updates are available:\n\n";
-
-		    my $count = 0;
-		    foreach my $p (sort {$a->{Package} cmp $b->{Package} } @$pkglist) {
-			next if $p->{NotifyStatus} && $p->{NotifyStatus} eq $p->{Version};
-			$count++;
-			if ($p->{OldVersion}) {
-			    $data .= "$p->{Package}: $p->{OldVersion} ==> $p->{Version}\n";
-			} else {
-			    $data .= "$p->{Package}: $p->{Version} (new)\n";
-			}
-		    }
+		return if !$count;
 
-		    return if !$count;
+		my $properties = {
+		    updates  => $updates_table,
+		    hostname => $hostname,
+		};
 
-		    PVE::Tools::sendmail($mailto, $subject, $data, undef, $mailfrom, '');
+		PVE::Notify::info(
+		    $target,
+		    $updates_available_subject_template,
+		    $updates_available_body_template,
+		    $properties,
+		);
 
-		    foreach my $pi (@$pkglist) {
-			$pi->{NotifyStatus} = $pi->{Version};
-		    }
-		    PVE::Tools::file_set_contents($pve_pkgstatus_fn, encode_json($pkglist));
+		foreach my $pi (@$pkglist) {
+		    $pi->{NotifyStatus} = $pi->{Version};
 		}
+		PVE::Tools::file_set_contents($pve_pkgstatus_fn, encode_json($pkglist));
 	    }
 
 	    return;
@@ -378,6 +417,8 @@ __PACKAGE__->register_method({
 
     }});
 
+
+
 __PACKAGE__->register_method({
     name => 'changelog',
     path => 'changelog',
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 06/30] api: replication: send notifications via new notification module
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (4 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 05/30] api: apt: send notification via new notification module Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 07/30] api: prepare api handler module for notification config Lukas Wagner
                   ` (25 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

If the new 'target-replication' option in datacenter.cfg is set to a
notification target, we send notifications that way. If it is not set,
we continue send a notification to the default target (mail to
root@pam).

There is also a new 'replication' option. It controls whether to send
a notification at all.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Replication.pm | 63 ++++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 89c5a802..d61518ba 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -15,6 +15,7 @@ use PVE::QemuConfig;
 use PVE::QemuServer;
 use PVE::LXC::Config;
 use PVE::LXC;
+use PVE::Notify;
 
 use PVE::RESTHandler;
 
@@ -91,6 +92,24 @@ my sub _should_mail_at_failcount {
     return $i * 48 == $fail_count;
 };
 
+my $replication_error_subject_template = "Replication Job: '{{job-id}}' failed";
+my $replication_error_body_template = <<EOT;
+{{#verbatim}}
+Replication job '{{job-id}}' with target '{{job-target}}' and schedule '{{job-schedule}}' failed!
+
+Last successful sync: {{timestamp last-sync}}
+Next sync try: {{timestamp next-sync}}
+Failure count: {{failure-count}}
+
+{{#if (eq failure-count 3)}}
+Note: The system  will now reduce the frequency of error reports, as the job
+appears to be stuck.
+{{/if}}
+Error:
+{{verbatim-monospaced error}}
+{{/verbatim}}
+EOT
+
 my sub _handle_job_err {
     my ($job, $err, $mail) = @_;
 
@@ -103,33 +122,37 @@ my sub _handle_job_err {
 
     return if !_should_mail_at_failcount($fail_count);
 
-    my $schedule = $job->{schedule} // '*/15';
-
-    my $msg = "Replication job $job->{id} with target '$job->{target}' and schedule";
-    $msg .= " '$schedule' failed!\n";
-
-    $msg .= "  Last successful sync: ";
-    if (my $last_sync = $jobstate->{last_sync}) {
-	$msg .= render_timestamp($last_sync) ."\n";
-    } else {
-	$msg .= "None/Unknown\n";
-    }
     # not yet updated, so $job->next_sync here is actually the current one.
     # NOTE: Copied from PVE::ReplicationState::job_status()
     my $next_sync = $job->{next_sync} + 60 * ($fail_count <= 3 ? 5 * $fail_count : 30);
-    $msg .= "  Next sync try: " . render_timestamp($next_sync) ."\n";
-    $msg .= "  Failure count: $fail_count\n";
-
 
-    if ($fail_count == 3) {
-	$msg .= "\nNote: The system will now reduce the frequency of error reports,";
-	$msg .= " as the job appears to be stuck.\n";
-    }
+    # The replication job is run every 15 mins if no schedule is set.
+    my $schedule = $job->{schedule} // '*/15';
 
-    $msg .= "\nError:\n$err";
+    my $properties = {
+	"failure-count" => $fail_count,
+	"last-sync"     => $jobstate->{last_sync},
+	"next-sync"     => $next_sync,
+	"job-id"        => $job->{id},
+	"job-target"    => $job->{target},
+	"job-schedule"  => $schedule,
+	"error"         => $err,
+    };
 
     eval {
-	PVE::Tools::sendmail('root', "Replication Job: $job->{id} failed", $msg)
+	my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	my $target = $dcconf->{notify}->{'target-replication'} // PVE::Notify::default_target();
+	my $notify = $dcconf->{notify}->{'replication'} // 'always';
+
+	if ($notify eq 'always') {
+	    PVE::Notify::error(
+		$target,
+		$replication_error_subject_template,
+		$replication_error_body_template,
+		$properties
+	    );
+	}
+
     };
     warn ": $@" if $@;
 }
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 07/30] api: prepare api handler module for notification config
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (5 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 06/30] api: replication: send notifications " Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 08/30] api: notification: add api routes for groups Lukas Wagner
                   ` (24 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

This commit adds a new Perl module, PVE::API2::Cluster::Notification.
The module will contain all API handlers for the new notification
subsystem.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Cluster.pm               |  7 +++
 PVE/API2/Cluster/Makefile         |  1 +
 PVE/API2/Cluster/Notifications.pm | 71 +++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 PVE/API2/Cluster/Notifications.pm

diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
index 3daf6ae5..2e4cd9cc 100644
--- a/PVE/API2/Cluster.pm
+++ b/PVE/API2/Cluster.pm
@@ -29,6 +29,7 @@ use PVE::API2::Cluster::Ceph;
 use PVE::API2::Cluster::Mapping;
 use PVE::API2::Cluster::Jobs;
 use PVE::API2::Cluster::MetricServer;
+use PVE::API2::Cluster::Notifications;
 use PVE::API2::ClusterConfig;
 use PVE::API2::Firewall::Cluster;
 use PVE::API2::HAConfig;
@@ -52,6 +53,11 @@ __PACKAGE__->register_method ({
     path => 'metrics',
 });
 
+__PACKAGE__->register_method ({
+    subclass => "PVE::API2::Cluster::Notifications",
+    path => 'notifications',
+});
+
 __PACKAGE__->register_method ({
     subclass => "PVE::API2::ClusterConfig",
     path => 'config',
@@ -149,6 +155,7 @@ __PACKAGE__->register_method ({
 	    { name => 'log' },
 	    { name => 'mapping' },
 	    { name => 'metrics' },
+	    { name => 'notifications' },
 	    { name => 'nextid' },
 	    { name => 'options' },
 	    { name => 'replication' },
diff --git a/PVE/API2/Cluster/Makefile b/PVE/API2/Cluster/Makefile
index 0c52a241..b109e5cb 100644
--- a/PVE/API2/Cluster/Makefile
+++ b/PVE/API2/Cluster/Makefile
@@ -8,6 +8,7 @@ PERLSOURCE= 			\
 	BackupInfo.pm		\
 	MetricServer.pm		\
 	Mapping.pm		\
+	Notifications.pm		\
 	Jobs.pm			\
 	Ceph.pm
 
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
new file mode 100644
index 00000000..1efebbc1
--- /dev/null
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -0,0 +1,71 @@
+package PVE::API2::Cluster::Notifications;
+
+use warnings;
+use strict;
+
+use Storable qw(dclone);
+use JSON;
+
+use PVE::Tools qw(extract_param);
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RESTHandler;
+use PVE::Notify;
+
+use base qw(PVE::RESTHandler);
+
+sub make_properties_optional {
+    my ($properties) = @_;
+    $properties = dclone($properties);
+
+    for my $key (keys %$properties) {
+	$properties->{$key}->{optional} = 1 if $key ne 'name';
+    }
+
+    return $properties;
+}
+
+sub raise_api_error {
+    my ($api_error) = @_;
+
+    if (!(ref($api_error) eq 'HASH' && $api_error->{message} && $api_error->{code})) {
+	die $api_error;
+    }
+
+    my $msg = "$api_error->{message}\n";
+    my $exc = PVE::Exception->new($msg, code => $api_error->{code});
+
+    my (undef, $filename, $line) = caller;
+
+    $exc->{filename} = $filename;
+    $exc->{line} = $line;
+
+    die $exc;
+}
+
+__PACKAGE__->register_method ({
+    name => 'index',
+    path => '',
+    method => 'GET',
+    description => 'Index for notification-related API endpoints.',
+    permissions => { user => 'all' },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {},
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $result = [
+	];
+
+	return $result;
+    }
+});
+
+1;
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 08/30] api: notification: add api routes for groups
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (6 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 07/30] api: prepare api handler module for notification config Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 09/30] api: notification: add api routes for sendmail endpoints Lukas Wagner
                   ` (23 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

The Perl part of the API methods primarily defines the API schema,
checks for any needed priviledges and then calls the actual Rust
implementation exposed via perlmod. Any errors returned by the Rust
code are translated into PVE::Exception, so that the API call fails
with the correct HTTP error code.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v4:
      - Explain the changes a bit more in the commit message
      - Factor out permission checks into a common helper
      - Minor code style improvements

 PVE/API2/Cluster/Notifications.pm | 263 ++++++++++++++++++++++++++++++
 1 file changed, 263 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 1efebbc1..55dd650d 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -42,6 +42,24 @@ sub raise_api_error {
     die $exc;
 }
 
+sub filter_entities_by_privs {
+    my ($rpcenv, $entities) = @_;
+    my $authuser = $rpcenv->get_user();
+
+    my $can_see_mapping_privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit'];
+
+    my $filtered = [grep {
+	$rpcenv->check_any(
+	    $authuser,
+	    "/mapping/notification/$_->{name}",
+	    $can_see_mapping_privs,
+	    1
+	)
+    } @$entities];
+
+    return $filtered;
+}
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -62,10 +80,255 @@ __PACKAGE__->register_method ({
     },
     code => sub {
 	my $result = [
+	    { name => 'groups' },
 	];
 
 	return $result;
     }
 });
 
+my $group_properties = {
+    name => {
+	description => 'Name of the group.',
+	type => 'string',
+	format => 'pve-configid',
+    },
+    'endpoint' => {
+	type => 'array',
+	items => {
+	    type => 'string',
+	    format => 'pve-configid',
+	},
+	description => 'List of included endpoints',
+    },
+    'comment' => {
+	description => 'Comment',
+	type => 'string',
+	optional => 1,
+    },
+    filter => {
+	description => 'Name of the filter that should be applied.',
+	type => 'string',
+	format => 'pve-configid',
+	optional => 1,
+    },
+};
+
+__PACKAGE__->register_method ({
+    name => 'get_groups',
+    path => 'groups',
+    method => 'GET',
+    description => 'Returns a list of all groups',
+    protected => 1,
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => $group_properties,
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $config = PVE::Notify::read_config();
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $entities = eval {
+	    $config->get_groups();
+	};
+	raise_api_error($@) if $@;
+
+	return filter_entities_by_privs($rpcenv, $entities);
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_group',
+    path => 'groups/{name}',
+    method => 'GET',
+    description => 'Return a specific group',
+    protected => 1,
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    %$group_properties,
+	    digest => get_standard_option('pve-config-digest'),
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	my $config = PVE::Notify::read_config();
+
+	my $group = eval {
+	    $config->get_group($name)
+	};
+
+	raise_api_error($@) if $@;
+	$group->{digest} = $config->digest();
+
+	return $group;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'create_group',
+    path => 'groups',
+    protected => 1,
+    method => 'POST',
+    description => 'Create a new group',
+    permissions => {
+	check => ['perm', '/mapping/notification', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => $group_properties,
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $endpoint = extract_param($param, 'endpoint');
+	my $comment = extract_param($param, 'comment');
+	my $filter = extract_param($param, 'filter');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->add_group(
+		    $name,
+		    $endpoint,
+		    $comment,
+		    $filter,
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'update_group',
+    path => 'groups/{name}',
+    protected => 1,
+    method => 'PUT',
+    description => 'Update existing group',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    %{ make_properties_optional($group_properties) },
+	    delete => {
+		type => 'array',
+		items => {
+		    type => 'string',
+		    format => 'pve-configid',
+		},
+		optional => 1,
+		description => 'A list of settings you want to delete.',
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+	},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $endpoint = extract_param($param, 'endpoint');
+	my $comment = extract_param($param, 'comment');
+	my $filter = extract_param($param, 'filter');
+	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->update_group(
+		    $name,
+		    $endpoint,
+		    $comment,
+		    $filter,
+		    $delete,
+		    $digest,
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete_group',
+    protected => 1,
+    path => 'groups/{name}',
+    method => 'DELETE',
+    description => 'Remove group',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+		$config->delete_group($name);
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
 1;
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 09/30] api: notification: add api routes for sendmail endpoints
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (7 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 08/30] api: notification: add api routes for groups Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 10/30] api: notification: add api routes for gotify endpoints Lukas Wagner
                   ` (22 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

The Perl part of the API methods primarily defines the API schema,
checks for any needed priviledges and then calls the actual Rust
implementation exposed via perlmod. Any errors returned by the Rust
code are translated into PVE::Exception, so that the API call fails
with the correct HTTP error code.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v4:
      - Explain the changes a bit more in the commit message
      - Factor out permission checks into a common helper
      - Minor code style improvements

 PVE/API2/Cluster/Notifications.pm | 305 ++++++++++++++++++++++++++++++
 1 file changed, 305 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 55dd650d..2d907c35 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -80,6 +80,7 @@ __PACKAGE__->register_method ({
     },
     code => sub {
 	my $result = [
+	    { name => 'endpoints' },
 	    { name => 'groups' },
 	];
 
@@ -87,6 +88,33 @@ __PACKAGE__->register_method ({
     }
 });
 
+__PACKAGE__->register_method ({
+    name => 'endpoints_index',
+    path => 'endpoints',
+    method => 'GET',
+    description => 'Index for all available endpoint types.',
+    permissions => { user => 'all' },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {},
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $result = [
+	    { name => 'sendmail' },
+	];
+
+	return $result;
+    }
+});
+
 my $group_properties = {
     name => {
 	description => 'Name of the group.',
@@ -331,4 +359,281 @@ __PACKAGE__->register_method ({
     }
 });
 
+my $sendmail_properties = {
+    name => {
+	description => 'The name of the endpoint.',
+	type => 'string',
+	format => 'pve-configid',
+    },
+    mailto => {
+	type => 'array',
+	items => {
+	    type => 'string',
+	    format => 'email-or-username',
+	},
+	description => 'List of email recipients',
+	optional => 1,
+    },
+    'mailto-user' => {
+	type => 'array',
+	items => {
+	    type => 'string',
+	    format => 'pve-userid',
+	},
+	description => 'List of users',
+	optional => 1,
+    },
+    'from-address' => {
+	description => '`From` address for the mail',
+	type => 'string',
+	optional => 1,
+    },
+    author => {
+	description => 'Author of the mail',
+	type => 'string',
+	optional => 1,
+    },
+    'comment' => {
+	description => 'Comment',
+	type        => 'string',
+	optional    => 1,
+    },
+    filter => {
+	description => 'Name of the filter that should be applied.',
+	type => 'string',
+	format => 'pve-configid',
+	optional => 1,
+    },
+};
+
+__PACKAGE__->register_method ({
+    name => 'get_sendmail_endpoints',
+    path => 'endpoints/sendmail',
+    method => 'GET',
+    description => 'Returns a list of all sendmail endpoints',
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
+	user => 'all',
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => $sendmail_properties,
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $config = PVE::Notify::read_config();
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $entities = eval {
+	    $config->get_sendmail_endpoints();
+	};
+	raise_api_error($@) if $@;
+
+	return filter_entities_by_privs($rpcenv, $entities);
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_sendmail_endpoint',
+    path => 'endpoints/sendmail/{name}',
+    method => 'GET',
+    description => 'Return a specific sendmail endpoint',
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
+	],
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    %$sendmail_properties,
+	    digest => get_standard_option('pve-config-digest'),
+	}
+
+    },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	my $config = PVE::Notify::read_config();
+	my $endpoint = eval {
+	    $config->get_sendmail_endpoint($name)
+	};
+
+	raise_api_error($@) if $@;
+	$endpoint->{digest} = $config->digest();
+
+	return $endpoint;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'create_sendmail_endpoint',
+    path => 'endpoints/sendmail',
+    protected => 1,
+    method => 'POST',
+    description => 'Create a new sendmail endpoint',
+    permissions => {
+	check => ['perm', '/mapping/notification', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => $sendmail_properties,
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $mailto = extract_param($param, 'mailto');
+	my $mailto_user = extract_param($param, 'mailto-user');
+	my $from_address = extract_param($param, 'from-address');
+	my $author = extract_param($param, 'author');
+	my $comment = extract_param($param, 'comment');
+	my $filter = extract_param($param, 'filter');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->add_sendmail_endpoint(
+		    $name,
+		    $mailto,
+		    $mailto_user,
+		    $from_address,
+		    $author,
+		    $comment,
+		    $filter
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'update_sendmail_endpoint',
+    path => 'endpoints/sendmail/{name}',
+    protected => 1,
+    method => 'PUT',
+    description => 'Update existing sendmail endpoint',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    %{ make_properties_optional($sendmail_properties) },
+	    delete => {
+		type => 'array',
+		items => {
+		    type => 'string',
+		    format => 'pve-configid',
+		},
+		optional => 1,
+		description => 'A list of settings you want to delete.',
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $mailto = extract_param($param, 'mailto');
+	my $mailto_user = extract_param($param, 'mailto-user');
+	my $from_address = extract_param($param, 'from-address');
+	my $author = extract_param($param, 'author');
+	my $comment = extract_param($param, 'comment');
+	my $filter = extract_param($param, 'filter');
+
+	my $delete = extract_param($param, 'delete');
+	my $digest = extract_param($param, 'digest');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->update_sendmail_endpoint(
+		    $name,
+		    $mailto,
+		    $mailto_user,
+		    $from_address,
+		    $author,
+		    $comment,
+		    $filter,
+		    $delete,
+		    $digest,
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete_sendmail_endpoint',
+    protected => 1,
+    path => 'endpoints/sendmail/{name}',
+    method => 'DELETE',
+    description => 'Remove sendmail endpoint',
+    permissions => {
+	check => ['perm', '/mapping/notification', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+		$config->delete_sendmail_endpoint($param->{name});
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if ($@);
+	return;
+    }
+});
+
 1;
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 10/30] api: notification: add api routes for gotify endpoints
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (8 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 09/30] api: notification: add api routes for sendmail endpoints Lukas Wagner
@ 2023-08-03 12:16 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 11/30] api: notification: add api routes for filters Lukas Wagner
                   ` (21 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:16 UTC (permalink / raw)
  To: pve-devel

The Perl part of the API methods primarily defines the API schema,
checks for any needed priviledges and then calls the actual Rust
implementation exposed via perlmod. Any errors returned by the Rust
code are translated into PVE::Exception, so that the API call fails
with the correct HTTP error code.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v4:
      - Explain the changes a bit more in the commit message
      - Factor out permission checks into a common helper
      - Minor code style improvements

 PVE/API2/Cluster/Notifications.pm | 262 ++++++++++++++++++++++++++++++
 1 file changed, 262 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 2d907c35..b65c6957 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -24,6 +24,19 @@ sub make_properties_optional {
     return $properties;
 }
 
+sub remove_protected_properties {
+    my ($properties, $to_remove) = @_;
+    $properties = dclone($properties);
+
+    for my $key (keys %$properties) {
+	if (grep /^$key$/, @$to_remove) {
+	    delete $properties->{$key};
+	}
+    }
+
+    return $properties;
+}
+
 sub raise_api_error {
     my ($api_error) = @_;
 
@@ -108,6 +121,7 @@ __PACKAGE__->register_method ({
     },
     code => sub {
 	my $result = [
+	    { name => 'gotify' },
 	    { name => 'sendmail' },
 	];
 
@@ -636,4 +650,252 @@ __PACKAGE__->register_method ({
     }
 });
 
+my $gotify_properties = {
+    name => {
+	description => 'The name of the endpoint.',
+	type => 'string',
+	format => 'pve-configid',
+    },
+    'server' => {
+	description => 'Server URL',
+	type => 'string',
+    },
+    'token' => {
+	description => 'Secret token',
+	type => 'string',
+    },
+    'comment' => {
+	description => 'Comment',
+	type        => 'string',
+	optional    => 1,
+    },
+    'filter' => {
+	description => 'Name of the filter that should be applied.',
+	type => 'string',
+	format => 'pve-configid',
+	optional => 1,
+    }
+};
+
+__PACKAGE__->register_method ({
+    name => 'get_gotify_endpoints',
+    path => 'endpoints/gotify',
+    method => 'GET',
+    description => 'Returns a list of all gotify endpoints',
+    protected => 1,
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => remove_protected_properties($gotify_properties, ['token']),
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $config = PVE::Notify::read_config();
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $entities = eval {
+	    $config->get_gotify_endpoints();
+	};
+	raise_api_error($@) if $@;
+
+	return filter_entities_by_privs($rpcenv, $entities);
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_gotify_endpoint',
+    path => 'endpoints/gotify/{name}',
+    method => 'GET',
+    description => 'Return a specific gotify endpoint',
+    protected => 1,
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+		description => 'Name of the endpoint.'
+	    },
+	}
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    %{ remove_protected_properties($gotify_properties, ['token']) },
+	    digest => get_standard_option('pve-config-digest'),
+	}
+    },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	my $config = PVE::Notify::read_config();
+	my $endpoint = eval {
+	    $config->get_gotify_endpoint($name)
+	};
+
+	raise_api_error($@) if $@;
+	$endpoint->{digest} = $config->digest();
+
+	return $endpoint;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'create_gotify_endpoint',
+    path => 'endpoints/gotify',
+    protected => 1,
+    method => 'POST',
+    description => 'Create a new gotify endpoint',
+    permissions => {
+	check => ['perm', '/mapping/notification', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => $gotify_properties,
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $server = extract_param($param, 'server');
+	my $token = extract_param($param, 'token');
+	my $comment = extract_param($param, 'comment');
+	my $filter = extract_param($param, 'filter');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->add_gotify_endpoint(
+		    $name,
+		    $server,
+		    $token,
+		    $comment,
+		    $filter
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'update_gotify_endpoint',
+    path => 'endpoints/gotify/{name}',
+    protected => 1,
+    method => 'PUT',
+    description => 'Update existing gotify endpoint',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    %{ make_properties_optional($gotify_properties) },
+	    delete => {
+		type => 'array',
+		items => {
+		    type => 'string',
+		    format => 'pve-configid',
+		},
+		optional => 1,
+		description => 'A list of settings you want to delete.',
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $server = extract_param($param, 'server');
+	my $token = extract_param($param, 'token');
+	my $comment = extract_param($param, 'comment');
+	my $filter = extract_param($param, 'filter');
+
+	my $delete = extract_param($param, 'delete');
+	my $digest = extract_param($param, 'digest');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->update_gotify_endpoint(
+		    $name,
+		    $server,
+		    $token,
+		    $comment,
+		    $filter,
+		    $delete,
+		    $digest,
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete_gotify_endpoint',
+    protected => 1,
+    path => 'endpoints/gotify/{name}',
+    method => 'DELETE',
+    description => 'Remove gotify endpoint',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+		$config->delete_gotify_endpoint($name);
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
 1;
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 11/30] api: notification: add api routes for filters
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (9 preceding siblings ...)
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 10/30] api: notification: add api routes for gotify endpoints Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 12/30] api: notification: allow fetching notification targets Lukas Wagner
                   ` (20 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

The Perl part of the API methods primarily defines the API schema,
checks for any needed priviledges and then calls the actual Rust
implementation exposed via perlmod. Any errors returned by the Rust
code are translated into PVE::Exception, so that the API call fails
with the correct HTTP error code.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v4:
      - Explain the changes a bit more in the commit message
      - Factor out permission checks into a common helper
      - Minor code style improvements

 PVE/API2/Cluster/Notifications.pm | 255 ++++++++++++++++++++++++++++++
 1 file changed, 255 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index b65c6957..b4db7f8e 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -94,6 +94,7 @@ __PACKAGE__->register_method ({
     code => sub {
 	my $result = [
 	    { name => 'endpoints' },
+	    { name => 'filters' },
 	    { name => 'groups' },
 	];
 
@@ -898,4 +899,258 @@ __PACKAGE__->register_method ({
 	return;
     }
 });
+
+my $filter_properties = {
+    name => {
+	description => 'Name of the endpoint.',
+	type => 'string',
+	format => 'pve-configid',
+    },
+    'min-severity' => {
+	type => 'string',
+	description => 'Minimum severity to match',
+	optional => 1,
+	enum => [qw(info notice warning error)],
+    },
+    mode => {
+	type => 'string',
+	description => "Choose between 'and' and 'or' for when multiple properties are specified",
+	optional => 1,
+	enum => [qw(and or)],
+	default => 'and',
+    },
+    'invert-match' => {
+	type => 'boolean',
+	description => 'Invert match of the whole filter',
+	optional => 1,
+    },
+    'comment' => {
+	description => 'Comment',
+	type        => 'string',
+	optional    => 1,
+    },
+};
+
+__PACKAGE__->register_method ({
+    name => 'get_filters',
+    path => 'filters',
+    method => 'GET',
+    description => 'Returns a list of all filters',
+    protected => 1,
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
+	user => 'all',
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => $filter_properties,
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $config = PVE::Notify::read_config();
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $entities = eval {
+	    $config->get_filters();
+	};
+	raise_api_error($@) if $@;
+
+	return filter_entities_by_privs($rpcenv, $entities);
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_filter',
+    path => 'filters/{name}',
+    method => 'GET',
+    description => 'Return a specific filter',
+    protected => 1,
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    %$filter_properties,
+	    digest => get_standard_option('pve-config-digest'),
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	my $config = PVE::Notify::read_config();
+
+	my $filter = eval {
+	    $config->get_filter($name)
+	};
+
+	raise_api_error($@) if $@;
+	$filter->{digest} = $config->digest();
+
+	return $filter;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'create_filter',
+    path => 'filters',
+    protected => 1,
+    method => 'POST',
+    description => 'Create a new filter',
+    protected => 1,
+    permissions => {
+	check => ['perm', '/mapping/notification', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => $filter_properties,
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $min_severity = extract_param($param, 'min-severity');
+	my $mode = extract_param($param, 'mode');
+	my $invert_match = extract_param($param, 'invert-match');
+	my $comment = extract_param($param, 'comment');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->add_filter(
+		    $name,
+		    $min_severity,
+		    $mode,
+		    $invert_match,
+		    $comment,
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'update_filter',
+    path => 'filters/{name}',
+    protected => 1,
+    method => 'PUT',
+    description => 'Update existing filter',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    %{ make_properties_optional($filter_properties) },
+	    delete => {
+		type => 'array',
+		items => {
+		    type => 'string',
+		    format => 'pve-configid',
+		},
+		optional => 1,
+		description => 'A list of settings you want to delete.',
+	    },
+	    digest => get_standard_option('pve-config-digest'),
+	},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+
+	my $name = extract_param($param, 'name');
+	my $min_severity = extract_param($param, 'min-severity');
+	my $mode = extract_param($param, 'mode');
+	my $invert_match = extract_param($param, 'invert-match');
+	my $comment = extract_param($param, 'comment');
+	my $digest = extract_param($param, 'digest');
+	my $delete = extract_param($param, 'delete');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+
+		$config->update_filter(
+		    $name,
+		    $min_severity,
+		    $mode,
+		    $invert_match,
+		    $comment,
+		    $delete,
+		    $digest,
+		);
+
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'delete_filter',
+    protected => 1,
+    path => 'filters/{name}',
+    method => 'DELETE',
+    description => 'Remove filter',
+    permissions => {
+	check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		type => 'string',
+		format => 'pve-configid',
+	    },
+	}
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	eval {
+	    PVE::Notify::lock_config(sub {
+		my $config = PVE::Notify::read_config();
+		$config->delete_filter($name);
+		PVE::Notify::write_config($config);
+	    });
+	};
+
+	raise_api_error($@) if $@;
+	return;
+    }
+});
+
 1;
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 12/30] api: notification: allow fetching notification targets
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (10 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 11/30] api: notification: add api routes for filters Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 13/30] api: notification: allow to test targets Lukas Wagner
                   ` (19 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

The API call returns all entities that can be used as notification
targets (endpoints, groups). Only targets for which the user has
appropriate permissions are returned.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Cluster/Notifications.pm | 81 +++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index b4db7f8e..d6f29291 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -96,6 +96,7 @@ __PACKAGE__->register_method ({
 	    { name => 'endpoints' },
 	    { name => 'filters' },
 	    { name => 'groups' },
+	    { name => 'targets' },
 	];
 
 	return $result;
@@ -130,6 +131,86 @@ __PACKAGE__->register_method ({
     }
 });
 
+__PACKAGE__->register_method ({
+    name => 'get_all_targets',
+    path => 'targets',
+    method => 'GET',
+    description => 'Returns a list of all entities that can be used as notification targets' .
+	' (endpoints and groups).',
+    permissions => {
+	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
+	user => 'all',
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		name => {
+		    description => 'Name of the endpoint/group.',
+		    type => 'string',
+		    format => 'pve-configid',
+		},
+		'type' => {
+		    description => 'Type of the endpoint or group.',
+		    type  => 'string',
+		    enum => [qw(sendmail gotify group)],
+		},
+		'comment' => {
+		    description => 'Comment',
+		    type        => 'string',
+		    optional    => 1,
+		},
+	    },
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	my $config = PVE::Notify::read_config();
+	my $rpcenv = PVE::RPCEnvironment::get();
+
+	my $targets = eval {
+	    my $result = [];
+
+	    for my $target (@{$config->get_sendmail_endpoints()}) {
+		push @$result, {
+		    name => $target->{name},
+		    comment => $target->{comment},
+		    type => 'sendmail',
+		};
+	    }
+
+	    for my $target (@{$config->get_gotify_endpoints()}) {
+		push @$result, {
+		    name => $target->{name},
+		    comment => $target->{comment},
+		    type => 'gotify',
+		};
+	    }
+
+	    for my $target (@{$config->get_groups()}) {
+		push @$result, {
+		    name => $target->{name},
+		    comment => $target->{comment},
+		    type => 'group',
+		};
+	    }
+
+	    $result
+	};
+
+	raise_api_error($@) if $@;
+
+	return filter_entities_by_privs($rpcenv, $targets);
+    }
+});
+
 my $group_properties = {
     name => {
 	description => 'Name of the group.',
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 13/30] api: notification: allow to test targets
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (11 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 12/30] api: notification: allow fetching notification targets Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 14/30] api: notification: disallow removing targets if they are used Lukas Wagner
                   ` (18 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

This API call allows the user to test a notification target.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Cluster/Notifications.pm | 40 +++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index d6f29291..065d6690 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -211,6 +211,46 @@ __PACKAGE__->register_method ({
     }
 });
 
+__PACKAGE__->register_method ({
+    name => 'test_target',
+    path => 'targets/{name}/test',
+    protected => 1,
+    method => 'POST',
+    description => 'Send a test notification to a provided target.',
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Use']],
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
+	    ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
+	],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    name => {
+		description => 'Name of the target.',
+		type => 'string',
+		format => 'pve-configid'
+	    },
+	},
+    },
+    returns => { type => 'null' },
+    code => sub {
+	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	my $config = PVE::Notify::read_config();
+
+	eval {
+	    $config->test_target($name);
+	};
+
+	raise_api_error($@) if $@;
+
+	return;
+    }
+});
+
 my $group_properties = {
     name => {
 	description => 'Name of the group.',
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 14/30] api: notification: disallow removing targets if they are used
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (12 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 13/30] api: notification: allow to test targets Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 15/30] ui: backup: allow to select notification target for jobs Lukas Wagner
                   ` (17 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Check notification targets configured in datacenter.cfg and jobs.cfg,
failing if the group/endpoint to be removed is still in use there.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Cluster/Notifications.pm | 44 ++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 065d6690..9c9e8243 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -6,6 +6,7 @@ use strict;
 use Storable qw(dclone);
 use JSON;
 
+use PVE::Exception qw(raise_param_exc);
 use PVE::Tools qw(extract_param);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::RESTHandler;
@@ -73,6 +74,31 @@ sub filter_entities_by_privs {
     return $filtered;
 }
 
+sub target_used_by {
+    my ($target) = @_;
+
+    my $used_by = [];
+
+    # Check keys in datacenter.cfg
+    my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
+    for my $key (qw(target-package-updates target-replication target-fencing)) {
+	if ($dc_conf->{notify} && $dc_conf->{notify}->{$key} eq $target) {
+	    push @$used_by, $key;
+	}
+    }
+
+    # Check backup jobs
+    my $jobs_conf = PVE::Cluster::cfs_read_file('jobs.cfg');
+    for my $key (keys %{$jobs_conf->{ids}}) {
+	my $job = $jobs_conf->{ids}->{$key};
+	if ($job->{'notification-target'} eq $target) {
+	    push @$used_by, $key;
+	}
+    }
+
+    return join(', ', @$used_by);
+}
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -482,6 +508,11 @@ __PACKAGE__->register_method ({
 	my ($param) = @_;
 	my $name = extract_param($param, 'name');
 
+	my $used_by = target_used_by($name);
+	if ($used_by) {
+	    raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"});
+	}
+
 	eval {
 	    PVE::Notify::lock_config(sub {
 		my $config = PVE::Notify::read_config();
@@ -758,11 +789,17 @@ __PACKAGE__->register_method ({
     returns => { type => 'null' },
     code => sub {
 	my ($param) = @_;
+	my $name = extract_param($param, 'name');
+
+	my $used_by = target_used_by($name);
+	if ($used_by) {
+	    raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"});
+	}
 
 	eval {
 	    PVE::Notify::lock_config(sub {
 		my $config = PVE::Notify::read_config();
-		$config->delete_sendmail_endpoint($param->{name});
+		$config->delete_sendmail_endpoint($name);
 		PVE::Notify::write_config($config);
 	    });
 	};
@@ -1008,6 +1045,11 @@ __PACKAGE__->register_method ({
 	my ($param) = @_;
 	my $name = extract_param($param, 'name');
 
+	my $used_by = target_used_by($name);
+	if ($used_by) {
+	    raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"});
+	}
+
 	eval {
 	    PVE::Notify::lock_config(sub {
 		my $config = PVE::Notify::read_config();
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 15/30] ui: backup: allow to select notification target for jobs
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (13 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 14/30] api: notification: disallow removing targets if they are used Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 16/30] ui: backup: adapt backup job details to new notification params Lukas Wagner
                   ` (16 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

This commit adds a possibility to choose between different options
for notifications for backup jobs:
    - Notify via email, in the same manner as before
    - Notify via an endpoint/group

If 'notify via mail' is selected, a text field where an email address
can be entered is displayed:

    Notify:         | Always notify  v |
    Notify via:     | E-Mail         v |
    Send Mail to:   | foo@example.com  |
    Compression:    | .....          v |

If the other option is selected selected, a combo picker for selecting
a channel is displayed:

    Notify:         | Always notify  v |
    Notify via:     | Endpoint/Group v |
    Target:         | endpoint-foo   v |
    Compression:    | .....          v |

The code has also been adapted to use the newly introduced
'notification-policy' parameter, which replaces the 'mailnotification'
paramter for backup jobs. Some logic which automatically migrates from
'mailnotification' has been added.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/Makefile                         |  4 +-
 www/manager6/dc/Backup.js                     | 84 +++++++++++++++++--
 www/manager6/form/NotificationModeSelector.js |  8 ++
 ...ector.js => NotificationPolicySelector.js} |  1 +
 .../form/NotificationTargetSelector.js        | 54 ++++++++++++
 5 files changed, 143 insertions(+), 8 deletions(-)
 create mode 100644 www/manager6/form/NotificationModeSelector.js
 rename www/manager6/form/{EmailNotificationSelector.js => NotificationPolicySelector.js} (87%)
 create mode 100644 www/manager6/form/NotificationTargetSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 7ec9d7a5..5ea4e4a2 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -36,7 +36,6 @@ JSSRC= 							\
 	form/DayOfWeekSelector.js			\
 	form/DiskFormatSelector.js			\
 	form/DiskStorageSelector.js			\
-	form/EmailNotificationSelector.js		\
 	form/FileSelector.js				\
 	form/FirewallPolicySelector.js			\
 	form/GlobalSearchField.js			\
@@ -51,6 +50,9 @@ JSSRC= 							\
 	form/MultiPCISelector.js			\
 	form/NetworkCardSelector.js			\
 	form/NodeSelector.js				\
+	form/NotificationModeSelector.js		\
+	form/NotificationTargetSelector.js		\
+	form/NotificationPolicySelector.js		\
 	form/PCISelector.js				\
 	form/PCIMapSelector.js				\
 	form/PermPathSelector.js			\
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 03a02651..625b5430 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -36,6 +36,30 @@ Ext.define('PVE.dc.BackupEdit', {
 		delete values.node;
 	    }
 
+	    if (!isCreate) {
+		// 'mailnotification' is deprecated in favor of 'notification-policy'
+		// -> Migration to the new parameter happens in init, so we are
+		//    safe to remove the old parameter here.
+		Proxmox.Utils.assemble_field_data(values, { 'delete': 'mailnotification' });
+
+		// If sending notifications via mail, remove the current value of
+		// 'notification-target'
+		if (values['notification-mode'] === "mailto") {
+		    Proxmox.Utils.assemble_field_data(
+			values,
+			{ 'delete': 'notification-target' },
+		    );
+		} else {
+		    // and vice versa...
+		    Proxmox.Utils.assemble_field_data(
+			values,
+			{ 'delete': 'mailto' },
+		    );
+		}
+	    }
+
+	    delete values['notification-mode'];
+
 	    if (!values.id && isCreate) {
 		values.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
 	    }
@@ -146,6 +170,22 @@ Ext.define('PVE.dc.BackupEdit', {
 		    success: function(response, _options) {
 			let data = response.result.data;
 
+			// 'mailnotification' is deprecated. Let's automatically
+			// migrate to the compatible 'notification-policy' parameter
+			if (data.mailnotification) {
+			    if (!data["notification-policy"]) {
+				data["notification-policy"] = data.mailnotification;
+			    }
+
+			    delete data.mailnotification;
+			}
+
+			if (data['notification-target']) {
+			    data['notification-mode'] = 'notification-target';
+			} else if (data.mailto) {
+			    data['notification-mode'] = 'mailto';
+			}
+
 			if (data.exclude) {
 			    data.vmid = data.exclude;
 			    data.selMode = 'exclude';
@@ -188,11 +228,13 @@ Ext.define('PVE.dc.BackupEdit', {
     viewModel: {
 	data: {
 	    selMode: 'include',
+	    notificationMode: 'notification-target',
 	},
 
 	formulas: {
 	    poolMode: (get) => get('selMode') === 'pool',
 	    disableVMSelection: (get) => get('selMode') !== 'include' && get('selMode') !== 'exclude',
+	    mailNotificationSelected: (get) => get('notificationMode') === 'mailto',
 	},
     },
 
@@ -282,20 +324,48 @@ Ext.define('PVE.dc.BackupEdit', {
 				},
 			    ],
 			    column2: [
-				{
-				    xtype: 'textfield',
-				    fieldLabel: gettext('Send email to'),
-				    name: 'mailto',
-				},
 				{
 				    xtype: 'pveEmailNotificationSelector',
-				    fieldLabel: gettext('Email'),
-				    name: 'mailnotification',
+				    fieldLabel: gettext('Notify'),
+				    name: 'notification-policy',
 				    cbind: {
 					value: (get) => get('isCreate') ? 'always' : '',
 					deleteEmpty: '{!isCreate}',
 				    },
 				},
+				{
+				    xtype: 'pveNotificationModeSelector',
+				    fieldLabel: gettext('Notify via'),
+				    name: 'notification-mode',
+				    bind: {
+					value: '{notificationMode}',
+				    },
+				},
+				{
+				    xtype: 'pveNotificationTargetSelector',
+				    fieldLabel: gettext('Notification Target'),
+				    name: 'notification-target',
+				    allowBlank: true,
+				    editable: true,
+				    autoSelect: false,
+				    bind: {
+					hidden: '{mailNotificationSelected}',
+					disabled: '{mailNotificationSelected}',
+				    },
+				    cbind: {
+					deleteEmpty: '{!isCreate}',
+				    },
+				},
+				{
+				    xtype: 'textfield',
+				    fieldLabel: gettext('Send email to'),
+				    name: 'mailto',
+				    hidden: true,
+				    bind: {
+					hidden: '{!mailNotificationSelected}',
+					disabled: '{!mailNotificationSelected}',
+				    },
+				},
 				{
 				    xtype: 'pveCompressionSelector',
 				    reference: 'compressionSelector',
diff --git a/www/manager6/form/NotificationModeSelector.js b/www/manager6/form/NotificationModeSelector.js
new file mode 100644
index 00000000..58fddd56
--- /dev/null
+++ b/www/manager6/form/NotificationModeSelector.js
@@ -0,0 +1,8 @@
+Ext.define('PVE.form.NotificationModeSelector', {
+    extend: 'Proxmox.form.KVComboBox',
+    alias: ['widget.pveNotificationModeSelector'],
+    comboItems: [
+	['notification-target', gettext('Target')],
+	['mailto', gettext('E-Mail')],
+    ],
+});
diff --git a/www/manager6/form/EmailNotificationSelector.js b/www/manager6/form/NotificationPolicySelector.js
similarity index 87%
rename from www/manager6/form/EmailNotificationSelector.js
rename to www/manager6/form/NotificationPolicySelector.js
index f318ea18..68087275 100644
--- a/www/manager6/form/EmailNotificationSelector.js
+++ b/www/manager6/form/NotificationPolicySelector.js
@@ -4,5 +4,6 @@ Ext.define('PVE.form.EmailNotificationSelector', {
     comboItems: [
 	['always', gettext('Notify always')],
 	['failure', gettext('On failure only')],
+	['never', gettext('Notify never')],
     ],
 });
diff --git a/www/manager6/form/NotificationTargetSelector.js b/www/manager6/form/NotificationTargetSelector.js
new file mode 100644
index 00000000..9ead28e7
--- /dev/null
+++ b/www/manager6/form/NotificationTargetSelector.js
@@ -0,0 +1,54 @@
+Ext.define('PVE.form.NotificationTargetSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: ['widget.pveNotificationTargetSelector'],
+
+    // set default value to empty array, else it inits it with
+    // null and after the store load it is an empty array,
+    // triggering dirtychange
+    value: [],
+    valueField: 'name',
+    displayField: 'name',
+    deleteEmpty: true,
+    skipEmptyText: true,
+
+    store: {
+	    fields: ['name', 'type', 'comment'],
+	    proxy: {
+		type: 'proxmox',
+		url: '/api2/json/cluster/notifications/targets',
+	    },
+	    sorters: [
+		{
+		    property: 'name',
+		    direction: 'ASC',
+		},
+	    ],
+	    autoLoad: true,
+	},
+
+    listConfig: {
+	columns: [
+	    {
+		header: gettext('Target'),
+		dataIndex: 'name',
+		sortable: true,
+		hideable: false,
+		flex: 1,
+	    },
+	    {
+		header: gettext('Type'),
+		dataIndex: 'type',
+		sortable: true,
+		hideable: false,
+		flex: 1,
+	    },
+	    {
+		header: gettext('Comment'),
+		dataIndex: 'comment',
+		sortable: true,
+		hideable: false,
+		flex: 2,
+	    },
+	],
+    },
+});
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 16/30] ui: backup: adapt backup job details to new notification params
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (14 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 15/30] ui: backup: allow to select notification target for jobs Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 17/30] ui: backup: allow to set notification-target for one-off backups Lukas Wagner
                   ` (15 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Adapt the backup job detail view so that it shows notification
targets.

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

diff --git a/www/manager6/dc/BackupJobDetail.js b/www/manager6/dc/BackupJobDetail.js
index 880784a2..e154fec1 100644
--- a/www/manager6/dc/BackupJobDetail.js
+++ b/www/manager6/dc/BackupJobDetail.js
@@ -202,15 +202,27 @@ Ext.define('PVE.dc.BackupInfo', {
     column2: [
 	{
 	    xtype: 'displayfield',
-	    name: 'mailnotification',
+	    name: 'notification-policy',
 	    fieldLabel: gettext('Notification'),
 	    renderer: function(value) {
-		let mailto = this.up('pveBackupInfo')?.record?.mailto || 'root@localhost';
+		let record = this.up('pveBackupInfo')?.record;
+
+		// Fall back to old value, in case this option is not migrated yet.
+		let policy = value || record?.mailnotification || 'always';
+
 		let when = gettext('Always');
-		if (value === 'failure') {
+		if (policy === 'failure') {
 		    when = gettext('On failure only');
+		} else if (policy === 'never') {
+		    when = gettext('Never');
 		}
-		return `${when} (${mailto})`;
+
+		// Notification-target takes precedence
+		let target = record?.['notification-target'] ||
+		    record?.mailto ||
+		    gettext('No target configured');
+
+		return `${when} (${target})`;
 	    },
 	},
 	{
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 17/30] ui: backup: allow to set notification-target for one-off backups
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (15 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 16/30] ui: backup: adapt backup job details to new notification params Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 18/30] ui: allow to configure notification event -> target mapping Lukas Wagner
                   ` (14 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

In essence the same change as for backup jobs.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/window/Backup.js | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 4b21c746..17a37e12 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -30,12 +30,32 @@ Ext.define('PVE.window.Backup', {
 	    name: 'mode',
 	});
 
+	let notificationTargetSelector = Ext.create('PVE.form.NotificationTargetSelector', {
+	    fieldLabel: gettext('Notification target'),
+	    name: 'notification-target',
+	    emptyText: Proxmox.Utils.noneText,
+	    hidden: true,
+	});
+
 	let mailtoField = Ext.create('Ext.form.field.Text', {
 	    fieldLabel: gettext('Send email to'),
 	    name: 'mailto',
 	    emptyText: Proxmox.Utils.noneText,
 	});
 
+	let notificationModeSelector = Ext.create('PVE.form.NotificationModeSelector', {
+	    fieldLabel: gettext('Notify via'),
+	    value: 'mailto',
+	    name: 'notification-mode',
+	    listeners: {
+		change: function(f, v) {
+		    let mailSelected = v === 'mailto';
+		    notificationTargetSelector.setHidden(mailSelected);
+		    mailtoField.setHidden(!mailSelected);
+		},
+	    },
+	});
+
 	const keepNames = [
 	    ['keep-last', gettext('Keep Last')],
 	    ['keep-hourly', gettext('Keep Hourly')],
@@ -107,6 +127,12 @@ Ext.define('PVE.window.Backup', {
 			success: function(response, opts) {
 			    const data = response.result.data;
 
+			    if (!initialDefaults && data['notification-mode'] !== undefined) {
+				notificationModeSelector.setValue(data['notification-mode']);
+			    }
+			    if (!initialDefaults && data['notification-channel'] !== undefined) {
+				notificationTargetSelector.setValue(data['notification-channel']);
+			    }
 			    if (!initialDefaults && data.mailto !== undefined) {
 				mailtoField.setValue(data.mailto);
 			    }
@@ -176,6 +202,8 @@ Ext.define('PVE.window.Backup', {
 	    ],
 	    column2: [
 		compressionSelector,
+		notificationModeSelector,
+		notificationTargetSelector,
 		mailtoField,
 		removeCheckbox,
 	    ],
@@ -252,10 +280,15 @@ Ext.define('PVE.window.Backup', {
 		    remove: values.remove,
 		};
 
-		if (values.mailto) {
+		if (values.mailto && values['notification-mode'] === 'mailto') {
 		    params.mailto = values.mailto;
 		}
 
+		if (values['notification-target'] &&
+		    values['notification-mode'] === 'notification-target') {
+		    params['notification-target'] = values['notification-target'];
+		}
+
 		if (values.compress) {
 		    params.compress = values.compress;
 		}
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 18/30] ui: allow to configure notification event -> target mapping
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (16 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 17/30] ui: backup: allow to set notification-target for one-off backups Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 19/30] ui: add notification target configuration panel Lukas Wagner
                   ` (13 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

This commit adds a new view that allows configuring notification
targets for all existing notification events (replication, updates,
fencing).

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Fixed missing trailing commas
    
    Changes since v4:
      - No changes
    
    Changes since v3:
      - Show warnings only if 'never' is selected
      - Also show a warning for disabled package update notifications
      - Some code style touch ups
      - Added some comments

 www/manager6/Makefile                 |   1 +
 www/manager6/dc/Config.js             |  12 ++
 www/manager6/dc/NotificationEvents.js | 277 ++++++++++++++++++++++++++
 3 files changed, 290 insertions(+)
 create mode 100644 www/manager6/dc/NotificationEvents.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 5ea4e4a2..59a5d8a7 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -159,6 +159,7 @@ JSSRC= 							\
 	dc/Health.js					\
 	dc/Log.js					\
 	dc/NodeView.js					\
+	dc/NotificationEvents.js			\
 	dc/OptionView.js				\
 	dc/PermissionView.js				\
 	dc/PoolEdit.js					\
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 04ed04f0..aa025c8d 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -317,6 +317,18 @@ Ext.define('PVE.dc.Config', {
 	    );
 	}
 
+	if (caps.dc['Sys.Audit']) {
+	    me.items.push(
+		{
+		    xtype: 'pveNotificationEvents',
+		    title: gettext('Notifications'),
+		    onlineHelp: 'notification_events',
+		    iconCls: 'fa fa-bell-o',
+		    itemId: 'notifications',
+		},
+	    );
+	}
+
 	if (caps.dc['Sys.Audit']) {
 	    me.items.push({
 		xtype: 'pveDcSupport',
diff --git a/www/manager6/dc/NotificationEvents.js b/www/manager6/dc/NotificationEvents.js
new file mode 100644
index 00000000..f2ee12e0
--- /dev/null
+++ b/www/manager6/dc/NotificationEvents.js
@@ -0,0 +1,277 @@
+Ext.define('PVE.dc.NotificationEventsPolicySelector', {
+    alias: ['widget.pveNotificationEventsPolicySelector'],
+    extend: 'Proxmox.form.KVComboBox',
+    deleteEmpty: false,
+    value: '__default__',
+
+    config: {
+	warningRef: null,
+	warnIfValIs: null,
+    },
+
+    listeners: {
+	change: function(field, newValue) {
+	    let me = this;
+	    if (!me.warningRef && !me.warnIfValIs) {
+		return;
+	    }
+
+	    let warningField = field.nextSibling(
+		`displayfield[reference=${me.warningRef}]`,
+	    );
+	    warningField.setVisible(newValue === me.warnIfValIs);
+	},
+    },
+});
+
+Ext.define('PVE.dc.NotificationEventDisabledWarning', {
+    alias: ['widget.pveNotificationEventDisabledWarning'],
+    extend: 'Ext.form.field.Display',
+    userCls: 'pmx-hint',
+    hidden: true,
+    value: gettext('Disabling notifications is not ' +
+        'recommended for production systems!'),
+});
+
+Ext.define('PVE.dc.NotificationEventsTargetSelector', {
+    alias: ['widget.pveNotificationEventsTargetSelector'],
+    extend: 'PVE.form.NotificationTargetSelector',
+    fieldLabel: gettext('Notification Target'),
+    allowBlank: true,
+    editable: true,
+    autoSelect: false,
+    deleteEmpty: false,
+    emptyText: `${Proxmox.Utils.defaultText} (${gettext("mail-to-root")})`,
+});
+
+Ext.define('PVE.dc.NotificationEvents', {
+    extend: 'Proxmox.grid.ObjectGrid',
+    alias: ['widget.pveNotificationEvents'],
+
+    // Taken from OptionView.js, but adapted slightly.
+    // The modified version allows us to have multiple rows in the ObjectGrid
+    // for the same underlying property (notify).
+    // Every setting is eventually stored as a property string in the
+    // notify key of datacenter.cfg.
+    // When updating 'notify', all properties that were already set
+    // also have to be submitted, even if they were not modified.
+    // This means that we need to save the old value somewhere.
+    addInputPanelRow: function(name, propertyName, text, opts) {
+	let me = this;
+
+	opts = opts || {};
+	me.rows = me.rows || {};
+
+	me.rows[name] = {
+	    required: true,
+	    defaultValue: opts.defaultValue,
+	    header: text,
+	    renderer: opts.renderer,
+	    name: propertyName,
+	    editor: {
+		xtype: 'proxmoxWindowEdit',
+		width: opts.width || 400,
+		subject: text,
+		onlineHelp: opts.onlineHelp,
+		fieldDefaults: {
+		    labelWidth: opts.labelWidth || 150,
+		},
+		setValues: function(values) {
+		    let value = values[propertyName];
+
+		    if (opts.parseBeforeSet) {
+			value = PVE.Parser.parsePropertyString(value);
+		    }
+
+		    Ext.Array.each(this.query('inputpanel'), function(panel) {
+			panel.setValues(value);
+
+			// Save the original value
+			panel.originalValue = {
+			    ...value,
+			};
+		    });
+		},
+		url: opts.url,
+		items: [{
+		    xtype: 'inputpanel',
+		    onGetValues: function(values) {
+			let fields = this.config.items.map(field => field.name).filter(n => n);
+
+			// Restore old, unchanged values
+			for (const [key, value] of Object.entries(this.originalValue)) {
+			    if (!fields.includes(key)) {
+				values[key] = value;
+			    }
+			}
+
+			let value = {};
+			if (Object.keys(values).length > 0) {
+			    value[propertyName] = PVE.Parser.printPropertyString(values);
+			} else {
+			    Proxmox.Utils.assemble_field_data(value, { 'delete': propertyName });
+			}
+
+			return value;
+		    },
+		    items: opts.items,
+		}],
+	    },
+	};
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	// Helper function for rendering the property
+	// Needed since the actual value is always stored in the 'notify' property
+	let render_value = (store, target_key, mode_key, default_val) => {
+	    let value = store.getById('notify')?.get('value') ?? {};
+	    let target = value[target_key] ?? gettext('mail-to-root');
+	    let template;
+
+	    switch (value[mode_key]) {
+		case 'always':
+		    template = gettext('Always, notify via target \'{0}\'');
+		    break;
+		case 'never':
+		    template = gettext('Never');
+		    break;
+		case 'auto':
+		    template = gettext('Automatically, notify via target \'{0}\'');
+		    break;
+		default:
+		    template = gettext('{1} ({2}), notify via target \'{0}\'');
+		    break;
+	    }
+
+	    return Ext.String.format(template, target, Proxmox.Utils.defaultText, default_val);
+	};
+
+	me.addInputPanelRow('fencing', 'notify', gettext('Node Fencing'), {
+	    renderer: (value, metaData, record, rowIndex, colIndex, store) =>
+		render_value(store, 'target-fencing', 'fencing', gettext('Always')),
+	    url: "/api2/extjs/cluster/options",
+	    items: [
+		{
+		    xtype: 'pveNotificationEventsPolicySelector',
+		    name: 'fencing',
+		    fieldLabel: gettext('Notify'),
+		    comboItems: [
+			['__default__', `${Proxmox.Utils.defaultText} (${gettext('Always')})`],
+			['always', gettext('Always')],
+			['never', gettext('Never')],
+		    ],
+		    warningRef: 'warning',
+		    warnIfValIs: 'never',
+		},
+		{
+		    xtype: 'pveNotificationEventsTargetSelector',
+		    name: 'target-fencing',
+		},
+		{
+		    xtype: 'pveNotificationEventDisabledWarning',
+		    reference: 'warning',
+		},
+	    ],
+	});
+
+	me.addInputPanelRow('replication', 'notify', gettext('Replication'), {
+	    renderer: (value, metaData, record, rowIndex, colIndex, store) =>
+		render_value(store, 'target-replication', 'replication', gettext('Always')),
+	    url: "/api2/extjs/cluster/options",
+	    items: [
+		{
+		    xtype: 'pveNotificationEventsPolicySelector',
+		    name: 'replication',
+		    fieldLabel: gettext('Notify'),
+		    comboItems: [
+			['__default__', `${Proxmox.Utils.defaultText} (${gettext('Always')})`],
+			['always', gettext('Always')],
+			['never', gettext('Never')],
+		    ],
+		    warningRef: 'warning',
+		    warnIfValIs: 'never',
+		},
+		{
+		    xtype: 'pveNotificationEventsTargetSelector',
+		    name: 'target-replication',
+		},
+		{
+		    xtype: 'pveNotificationEventDisabledWarning',
+		    reference: 'warning',
+		},
+	    ],
+	});
+
+	me.addInputPanelRow('updates', 'notify', gettext('Package Updates'), {
+	    renderer: (value, metaData, record, rowIndex, colIndex, store) =>
+		render_value(
+		    store,
+		    'target-package-updates',
+		    'package-updates',
+		    gettext('Automatically'),
+		),
+	    url: "/api2/extjs/cluster/options",
+	    items: [
+		{
+		    xtype: 'pveNotificationEventsPolicySelector',
+		    name: 'package-updates',
+		    fieldLabel: gettext('Notify'),
+		    comboItems: [
+			[
+			    '__default__',
+			    `${Proxmox.Utils.defaultText} (${gettext('Automatically')})`,
+			],
+			['auto', gettext('Automatically')],
+			['always', gettext('Always')],
+			['never', gettext('Never')],
+		    ],
+		    warningRef: 'warning',
+		    warnIfValIs: 'never',
+		},
+		{
+		    xtype: 'pveNotificationEventsTargetSelector',
+		    name: 'target-package-updates',
+		},
+		{
+		    xtype: 'pveNotificationEventDisabledWarning',
+		    reference: 'warning',
+		},
+	    ],
+	});
+
+	// Hack: Also load the notify property to make it accessible
+	// for our render functions.
+	me.rows.notify = {
+	    visible: false,
+	};
+
+	me.selModel = Ext.create('Ext.selection.RowModel', {});
+
+	Ext.apply(me, {
+	    tbar: [{
+		text: gettext('Edit'),
+		xtype: 'proxmoxButton',
+		disabled: true,
+		handler: () => me.run_editor(),
+		selModel: me.selModel,
+	    }],
+	    url: "/api2/json/cluster/options",
+	    editorConfig: {
+		url: "/api2/extjs/cluster/options",
+	    },
+	    interval: 5000,
+	    cwidth1: 200,
+	    listeners: {
+		itemdblclick: me.run_editor,
+	    },
+	});
+
+	me.callParent();
+
+	me.on('activate', me.rstore.startUpdate);
+	me.on('destroy', me.rstore.stopUpdate);
+	me.on('deactivate', me.rstore.stopUpdate);
+    },
+});
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 19/30] ui: add notification target configuration panel
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (17 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 18/30] ui: allow to configure notification event -> target mapping Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 20/30] ui: perm path: add ACL paths for notifications, usb and pci mappings Lukas Wagner
                   ` (12 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Embed the new notification target configuration panel, implemented in
proxmox-widget-toolkit.

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

diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index aa025c8d..9ba7b301 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -329,6 +329,22 @@ Ext.define('PVE.dc.Config', {
 	    );
 	}
 
+	if (caps.mapping['Mapping.Audit'] ||
+	    caps.mapping['Mapping.Use'] ||
+	    caps.mapping['Mapping.Modify']) {
+	    me.items.push(
+		{
+		    xtype: 'pmxNotificationConfigView',
+		    title: gettext('Notification Targets'),
+		    onlineHelp: 'notification_targets',
+		    itemId: 'notification-targets',
+		    iconCls: 'fa fa-dot-circle-o',
+		    baseUrl: '/cluster/notifications',
+		    groups: ['notifications'],
+		},
+	    );
+	}
+
 	if (caps.dc['Sys.Audit']) {
 	    me.items.push({
 		xtype: 'pveDcSupport',
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 20/30] ui: perm path: add ACL paths for notifications, usb and pci mappings
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (18 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 19/30] ui: add notification target configuration panel Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 21/30] ui: perm path: increase width of the perm path selector combobox Lukas Wagner
                   ` (11 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Suggested-by: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    In future, we could create a new API endpoint that returns all possible ACL
    and then use a normal store for the perm path combobox?
    
    Changes since v3:
      - Removed API calls that fetch targets/filters, instead
        add static paths.

 www/manager6/data/PermPathStore.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/www/manager6/data/PermPathStore.js b/www/manager6/data/PermPathStore.js
index c3ac7f0e..64ab2f03 100644
--- a/www/manager6/data/PermPathStore.js
+++ b/www/manager6/data/PermPathStore.js
@@ -9,6 +9,9 @@ Ext.define('PVE.data.PermPathStore', {
 	{ 'value': '/access/groups' },
 	{ 'value': '/access/realm' },
 	{ 'value': '/mapping' },
+	{ 'value': '/mapping/notification' },
+	{ 'value': '/mapping/pci' },
+	{ 'value': '/mapping/usb' },
 	{ 'value': '/nodes' },
 	{ 'value': '/pool' },
 	{ 'value': '/sdn/zones' },
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 21/30] ui: perm path: increase width of the perm path selector combobox
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (19 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 20/30] ui: perm path: add ACL paths for notifications, usb and pci mappings Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 22/30] ui: dc: remove notify key from datacenter option view Lukas Wagner
                   ` (10 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

ACL paths for notification targets can become quite long, e.g.:
/mappings/notifications/<target name>

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/form/PermPathSelector.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/form/PermPathSelector.js b/www/manager6/form/PermPathSelector.js
index c20d8b65..e8d395fc 100644
--- a/www/manager6/form/PermPathSelector.js
+++ b/www/manager6/form/PermPathSelector.js
@@ -6,6 +6,7 @@ Ext.define('PVE.form.PermPathSelector', {
     displayField: 'value',
     typeAhead: true,
     queryMode: 'local',
+    width: 380,
 
     store: {
 	type: 'pvePermPath',
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 22/30] ui: dc: remove notify key from datacenter option view
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (20 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 21/30] ui: perm path: increase width of the perm path selector combobox Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 23/30] vzdump: use <name> as a convention for virtual endpoints/groups Lukas Wagner
                   ` (9 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Settings for notifications have been moved to their own view.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v3:
      - New in v4

 www/manager6/dc/OptionView.js | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/www/manager6/dc/OptionView.js b/www/manager6/dc/OptionView.js
index 8b7aca32..4717277f 100644
--- a/www/manager6/dc/OptionView.js
+++ b/www/manager6/dc/OptionView.js
@@ -91,26 +91,6 @@ Ext.define('PVE.dc.OptionView', {
 	    vtype: 'proxmoxMail',
 	    defaultValue: 'root@$hostname',
 	});
-	me.add_inputpanel_row('notify', gettext('Notify'), {
-	    renderer: v => !v ? 'package-updates=auto' : PVE.Parser.printPropertyString(v),
-	    labelWidth: 120,
-	    url: "/api2/extjs/cluster/options",
-	    //onlineHelp: 'ha_manager_shutdown_policy',
-	    items: [{
-		xtype: 'proxmoxKVComboBox',
-		name: 'package-updates',
-		fieldLabel: gettext('Package Updates'),
-		deleteEmpty: false,
-		value: '__default__',
-		comboItems: [
-		    ['__default__', Proxmox.Utils.defaultText + ' (auto)'],
-		    ['auto', gettext('Automatically')],
-		    ['always', gettext('Always')],
-		    ['never', gettext('Never')],
-		],
-		defaultValue: '__default__',
-	    }],
-	});
 	me.add_text_row('mac_prefix', gettext('MAC address prefix'), {
 	    deleteEmpty: true,
 	    vtype: 'MacPrefix',
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 23/30] vzdump: use <name> as a convention for virtual endpoints/groups
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (21 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 22/30] ui: dc: remove notify key from datacenter option view Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 24/30] api: notification: make the 'mail-to-root' target visible to any user Lukas Wagner
                   ` (8 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel; +Cc: Lukas Wagner, Wolfgang Bumiller

Virtual (or anonymous) endpoints/groups are used for sending
one-off notifications to a target that does not exist in the
config.

VZDump uses this to send out notification mails to those addresses
configured by the `mailto` parameter.

Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v4:
      - new in v5

 PVE/VZDump.pm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 7dc9f31e..2671e3b1 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -501,10 +501,10 @@ sub send_notification {
     my $notification_config = PVE::Notify::read_config();
 
     if ($mailto && scalar(@$mailto)) {
-	# <, >, @ is not allowed in endpoint names, but only it is only
+	# <, >, @ are not allowed in endpoint names, but that is only
 	# verified once the config is serialized. That means that
 	# we can rely on that fact that no other endpoint with this name exists.
-	my $endpoint_name = "mail-to-<" . join(",", @$mailto) . ">";
+	my $endpoint_name = "<mail-to-" . join(",", @$mailto) . ">";
 	$notification_config->add_sendmail_endpoint(
 	    $endpoint_name,
 	    $mailto,
@@ -520,7 +520,7 @@ sub send_notification {
 	    push @$endpoints, $target;
 	}
 
-	$target = "group-$endpoint_name";
+	$target = "<group-$endpoint_name>";
 	$notification_config->add_group(
 	    $target,
 	    $endpoints,
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-manager 24/30] api: notification: make the 'mail-to-root' target visible to any user
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (22 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 23/30] vzdump: use <name> as a convention for virtual endpoints/groups Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 25/30] notification: add gui for sendmail notification endpoints Lukas Wagner
                   ` (7 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Since the target does not require Mapping.Use, it should also be
visible and testable by all users.

Short explanation why the 'mail-to-root' is exempt from priv checks:

To ensure backwards compatibility, the 'mail-to-root' target does not
require the `Mapping.Use` privs. This is needed due to the fact that
this target is used as a fallback in case no other target is
configured for an event. For instance, the /node/<name>/apt/update API
call only requires Sys.Modify for the node, but it can also send a
notification. If we were to require Mapping.Use, we could break the
apt/update API compat in the case that a notification shall be sent,
but without any configured notification target (which will then
default to 'mail-to-root').

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Fix bug in permission check

 PVE/API2/Cluster/Notifications.pm | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 9c9e8243..ec666903 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -68,7 +68,7 @@ sub filter_entities_by_privs {
 	    "/mapping/notification/$_->{name}",
 	    $can_see_mapping_privs,
 	    1
-	)
+	) || $_->{name} eq PVE::Notify::default_target();
     } @$entities];
 
     return $filtered;
@@ -165,7 +165,8 @@ __PACKAGE__->register_method ({
 	' (endpoints and groups).',
     permissions => {
 	description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
-	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'."
+	    . " The special 'mail-to-root' target is available to all users.",
 	user => 'all',
     },
     protected => 1,
@@ -244,11 +245,10 @@ __PACKAGE__->register_method ({
     method => 'POST',
     description => 'Send a test notification to a provided target.',
     permissions => {
-	check => ['or',
-	    ['perm', '/mapping/notification/{name}', ['Mapping.Use']],
-	    ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
-	    ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
-	],
+	description => "The user requires 'Mapping.Modify', 'Mapping.Use' or"
+	    . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'."
+	    . " The special 'mail-to-root' target can be accessed by all users.",
+	user => 'all',
     },
     parameters => {
 	additionalProperties => 0,
@@ -264,10 +264,23 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 	my $name = extract_param($param, 'name');
-
-	my $config = PVE::Notify::read_config();
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $authuser = $rpcenv->get_user();
+
+	my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit'];
+
+	if ($name ne PVE::Notify::default_target()) {
+	    # Due to backwards compatibility reasons the 'mail-to-root'
+	    # target must be accessible for any user
+	    $rpcenv->check_any(
+		$authuser,
+		"/mapping/notification/$name",
+		$privs,
+	    );
+	}
 
 	eval {
+	    my $config = PVE::Notify::read_config();
 	    $config->test_target($name);
 	};
 
-- 
2.39.2





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

* [pve-devel] [PATCH v6 proxmox-widget-toolkit 25/30] notification: add gui for sendmail notification endpoints
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (23 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 24/30] api: notification: make the 'mail-to-root' target visible to any user Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 26/30] notification: add gui for gotify " Lukas Wagner
                   ` (6 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

This commit adds a new panel 'NotificationConfigView' that is supposed
to be embedded in the datacenter configuration side-bar.
This new view lists all notification endpoints, allowing to
add/modify/delete/test them.

Furthermore, this commits adds the dialog for adding/modifying
sendmail endpoints. The dialog is 'plugin-in' based, meaning that it
consists of a base window (EndpointEditBase) and a panel that holds
the actual fields for the endpoint type configuration. This will show
be beneficial once the GUI for other endpoint types is added.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Extended commit message
      - Changed width of dialog window to 700px, in preparation for
        future endpoint configuration panels that will contain two panels
    
    Changes since v3:
      - extracted validator function
      - use items/advancedItems  instead of columns

 src/Makefile                         |   4 +
 src/Schema.js                        |   8 ++
 src/data/model/NotificationConfig.js |   8 ++
 src/panel/NotificationConfigView.js  | 196 +++++++++++++++++++++++++++
 src/panel/SendmailEditPanel.js       | 130 ++++++++++++++++++
 src/window/EndpointEditBase.js       |  52 +++++++
 6 files changed, 398 insertions(+)
 create mode 100644 src/data/model/NotificationConfig.js
 create mode 100644 src/panel/NotificationConfigView.js
 create mode 100644 src/panel/SendmailEditPanel.js
 create mode 100644 src/window/EndpointEditBase.js

diff --git a/src/Makefile b/src/Makefile
index baa90ec..e83038f 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -22,6 +22,7 @@ JSSRC=					\
 	data/ObjectStore.js		\
 	data/RRDStore.js		\
 	data/TimezoneStore.js		\
+	data/model/NotificationConfig.js	\
 	data/model/Realm.js		\
 	data/model/Certificates.js	\
 	data/model/ACME.js		\
@@ -59,6 +60,7 @@ JSSRC=					\
 	panel/InfoWidget.js		\
 	panel/LogView.js		\
 	panel/NodeInfoRepoStatus.js	\
+	panel/NotificationConfigView.js	\
 	panel/JournalView.js		\
 	panel/PermissionView.js		\
 	panel/PruneKeepPanel.js		\
@@ -68,6 +70,7 @@ JSSRC=					\
 	panel/ACMEAccount.js		\
 	panel/ACMEPlugin.js		\
 	panel/ACMEDomains.js		\
+	panel/SendmailEditPanel.js	\
 	panel/StatusView.js		\
 	panel/TfaView.js		\
 	panel/NotesView.js		\
@@ -83,6 +86,7 @@ JSSRC=					\
 	window/ACMEAccount.js		\
 	window/ACMEPluginEdit.js	\
 	window/ACMEDomains.js		\
+	window/EndpointEditBase.js		\
 	window/FileBrowser.js		\
 	window/AuthEditBase.js		\
 	window/AuthEditOpenId.js	\
diff --git a/src/Schema.js b/src/Schema.js
index b247b1e..99bb3fa 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -37,6 +37,14 @@ Ext.define('Proxmox.Schema', { // a singleton
 	}
     },
 
+    notificationEndpointTypes: {
+	sendmail: {
+	    name: gettext('Sendmail'),
+	    ipanel: 'pmxSendmailEditPanel',
+	    iconCls: 'fa-envelope-o',
+	},
+    },
+
     pxarFileTypes: {
 	b: { icon: 'cube', label: gettext('Block Device') },
 	c: { icon: 'tty', label: gettext('Character Device') },
diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js
new file mode 100644
index 0000000..c2ce843
--- /dev/null
+++ b/src/data/model/NotificationConfig.js
@@ -0,0 +1,8 @@
+Ext.define('proxmox-notification-endpoints', {
+    extend: 'Ext.data.Model',
+    fields: ['name', 'type', 'comment'],
+    proxy: {
+        type: 'proxmox',
+    },
+    idProperty: 'name',
+});
diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js
new file mode 100644
index 0000000..9282ccd
--- /dev/null
+++ b/src/panel/NotificationConfigView.js
@@ -0,0 +1,196 @@
+Ext.define('Proxmox.panel.NotificationConfigView', {
+    extend: 'Ext.panel.Panel',
+    alias: 'widget.pmxNotificationConfigView',
+    mixins: ['Proxmox.Mixin.CBind'],
+    layout: {
+	type: 'border',
+    },
+
+    items: [
+	{
+	    region: 'center',
+	    border: false,
+	    xtype: 'pmxNotificationEndpointView',
+	    cbind: {
+		baseUrl: '{baseUrl}',
+	    },
+	},
+    ],
+});
+
+Ext.define('Proxmox.panel.NotificationEndpointView', {
+    extend: 'Ext.grid.Panel',
+    alias: 'widget.pmxNotificationEndpointView',
+
+    title: gettext('Notification Targets'),
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	openEditWindow: function(endpointType, endpoint) {
+	    let me = this;
+
+	    if (endpoint === 'mail-to-root') {
+		return;
+	    }
+
+	    Ext.create('Proxmox.window.EndpointEditBase', {
+		baseUrl: me.getView().baseUrl,
+		type: endpointType,
+
+		name: endpoint,
+		autoShow: true,
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	},
+
+	openEditForSelectedItem: function() {
+	    let me = this;
+	    let view = me.getView();
+
+	    let selection = view.getSelection();
+	    if (selection.length < 1) {
+		return;
+	    }
+
+	    me.openEditWindow(selection[0].data.type, selection[0].data.name);
+	},
+
+	reload: function() {
+	    let me = this;
+	    let view = me.getView();
+	    view.getStore().rstore.load();
+	},
+
+	testEndpoint: function() {
+	    let me = this;
+	    let view = me.getView();
+
+	    let selection = view.getSelection();
+	    if (selection.length < 1) {
+		return;
+	    }
+
+	    let target = selection[0].data.name;
+
+	    Ext.Msg.confirm(
+		gettext("Notification Target Test"),
+		gettext(`Do you want to send a test notification to '${target}'?`),
+		function(decision) {
+		    if (decision !== "yes") {
+			return;
+		    }
+
+		    Proxmox.Utils.API2Request({
+			method: 'POST',
+			url: `${view.baseUrl}/targets/${target}/test`,
+
+			success: function(response, opt) {
+			    Ext.Msg.show({
+				title: gettext('Notification Target Test'),
+				message: gettext(`Sent test notification to '${target}'.`),
+				buttons: Ext.Msg.OK,
+				icon: Ext.Msg.INFO,
+			    });
+			},
+			autoErrorAlert: true,
+		    });
+	    });
+	},
+    },
+
+    listeners: {
+	itemdblclick: 'openEditForSelectedItem',
+	activate: 'reload',
+    },
+
+    emptyText: gettext('No notification targets configured'),
+
+    columns: [
+	{
+	    dataIndex: 'name',
+	    text: gettext('Target Name'),
+	    renderer: Ext.String.htmlEncode,
+	    flex: 1,
+	},
+	{
+	    dataIndex: 'type',
+	    text: gettext('Type'),
+	    renderer: Ext.String.htmlEncode,
+	    flex: 1,
+	},
+	{
+	    dataIndex: 'comment',
+	    text: gettext('Comment'),
+	    renderer: Ext.String.htmlEncode,
+	    flex: 1,
+	},
+    ],
+
+    store: {
+	type: 'diff',
+	autoDestroy: true,
+	autoDestroyRstore: true,
+	rstore: {
+	    type: 'update',
+	    storeid: 'proxmox-notification-endpoints',
+	    model: 'proxmox-notification-endpoints',
+	    autoStart: true,
+	},
+	sorters: 'name',
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	let menuItems = [];
+	for (const [endpointType, config] of Object.entries(
+	    Proxmox.Schema.notificationEndpointTypes).sort()) {
+	    menuItems.push({
+		text: config.name,
+		iconCls: 'fa fa-fw ' + (config.iconCls || 'fa-bell-o'),
+		handler: () => me.controller.openEditWindow(endpointType),
+	    });
+	}
+
+	Ext.apply(me, {
+	    tbar: [
+		{
+		    text: gettext('Add'),
+		    menu: menuItems,
+		},
+		{
+		    xtype: 'proxmoxButton',
+		    text: gettext('Modify'),
+		    handler: 'openEditForSelectedItem',
+		    enableFn: rec => rec.data.name !== 'mail-to-root',
+		    disabled: true,
+		},
+		{
+		    xtype: 'proxmoxStdRemoveButton',
+		    callback: 'reload',
+		    enableFn: rec => rec.data.name !== 'mail-to-root',
+		    getUrl: function(rec) {
+			if (rec.data.type === 'group') {
+			    return `${me.baseUrl}/groups/${rec.getId()}`;
+			}
+
+			return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`;
+		    },
+		},
+		'-',
+		{
+		    xtype: 'proxmoxButton',
+		    text: gettext('Test'),
+		    handler: 'testEndpoint',
+		    disabled: true,
+		},
+	    ],
+	});
+
+	me.callParent();
+	me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/targets`);
+    },
+});
diff --git a/src/panel/SendmailEditPanel.js b/src/panel/SendmailEditPanel.js
new file mode 100644
index 0000000..33ac482
--- /dev/null
+++ b/src/panel/SendmailEditPanel.js
@@ -0,0 +1,130 @@
+Ext.define('Proxmox.panel.SendmailEditPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pmxSendmailEditPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    type: 'sendmail',
+
+    mailValidator: function() {
+	let mailto_user = this.down(`[name=mailto-user]`);
+	let mailto = this.down(`[name=mailto]`);
+
+	if (!mailto_user.getValue()?.length && !mailto.getValue()) {
+	    return gettext('Either mailto or mailto-user must be set');
+	}
+
+	return true;
+    },
+
+    items: [
+	{
+	    xtype: 'pmxDisplayEditField',
+	    name: 'name',
+	    cbind: {
+		value: '{name}',
+		editable: '{isCreate}',
+	    },
+	    fieldLabel: gettext('Endpoint Name'),
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'pmxUserSelector',
+	    name: 'mailto-user',
+	    reference: 'mailto-user',
+	    multiSelect: true,
+	    allowBlank: true,
+	    editable: false,
+	    skipEmptyText: true,
+	    fieldLabel: gettext('User(s)'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	    validator: function() {
+		return this.up('pmxSendmailEditPanel').mailValidator();
+	    },
+	    listConfig: {
+		width: 600,
+		columns: [
+		    {
+			header: gettext('User'),
+			sortable: true,
+			dataIndex: 'userid',
+			renderer: Ext.String.htmlEncode,
+			flex: 1,
+		    },
+		    {
+			header: gettext('E-Mail'),
+			sortable: true,
+			dataIndex: 'email',
+			renderer: Ext.String.htmlEncode,
+			flex: 1,
+		    },
+		    {
+			header: gettext('Comment'),
+			sortable: false,
+			dataIndex: 'comment',
+			renderer: Ext.String.htmlEncode,
+			flex: 1,
+		    },
+		],
+	    },
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    fieldLabel: gettext('Additional Recipient(s)'),
+	    name: 'mailto',
+	    reference: 'mailto',
+	    allowBlank: true,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	    autoEl: {
+		tag: 'div',
+		'data-qtip': gettext(
+		    'Multiple recipients must be separated by spaces, commas or semicolons',
+		),
+	    },
+	    validator: function() {
+		return this.up('pmxSendmailEditPanel').mailValidator();
+	    },
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'comment',
+	    fieldLabel: gettext('Comment'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+    ],
+
+    advancedItems: [
+	{
+	    xtype: 'proxmoxtextfield',
+	    fieldLabel: gettext('Author'),
+	    name: 'author',
+	    allowBlank: true,
+	    emptyText: gettext('Proxmox VE'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    fieldLabel: gettext('From Address'),
+	    name: 'from-address',
+	    allowBlank: true,
+	    emptyText: gettext('Defaults to datacenter configuration, or root@$hostname'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+    ],
+
+    onGetValues: (values) => {
+	if (values.mailto) {
+	    values.mailto = values.mailto.split(/[\s,;]+/);
+	}
+	return values;
+    },
+});
diff --git a/src/window/EndpointEditBase.js b/src/window/EndpointEditBase.js
new file mode 100644
index 0000000..7e4c910
--- /dev/null
+++ b/src/window/EndpointEditBase.js
@@ -0,0 +1,52 @@
+Ext.define('Proxmox.window.EndpointEditBase', {
+    extend: 'Proxmox.window.Edit',
+
+    isAdd: true,
+
+    fieldDefaults: {
+	labelWidth: 120,
+    },
+
+    width: 700,
+
+    initComponent: function() {
+	let me = this;
+
+	me.isCreate = !me.name;
+
+	if (!me.baseUrl) {
+	    throw "baseUrl not set";
+	}
+
+	me.url = `/api2/extjs${me.baseUrl}/endpoints/${me.type}`;
+
+	if (me.isCreate) {
+	    me.method = 'POST';
+	} else {
+	    me.url += `/${me.name}`;
+	    me.method = 'PUT';
+	}
+
+	let endpointConfig = Proxmox.Schema.notificationEndpointTypes[me.type];
+	if (!endpointConfig) {
+	    throw 'unknown endpoint type';
+	}
+
+	me.subject = endpointConfig.name;
+
+	Ext.apply(me, {
+	    items: [{
+		name: me.name,
+		xtype: endpointConfig.ipanel,
+		isCreate: me.isCreate,
+		type: me.type,
+	    }],
+	});
+
+	me.callParent();
+
+	if (!me.isCreate) {
+	    me.load();
+	}
+    },
+});
-- 
2.39.2





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

* [pve-devel] [PATCH v6 proxmox-widget-toolkit 26/30] notification: add gui for gotify notification endpoints
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (24 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 25/30] notification: add gui for sendmail notification endpoints Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 27/30] notification: add gui for notification groups Lukas Wagner
                   ` (5 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

The GUI is based on the 'plugin-based' dialog window EndpointEditBase
that was introduced in an earlier commit.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Extended commit message
    
    Changes since v3:
      - Use items/advancedItems instead of columns

 src/Makefile                 |  1 +
 src/Schema.js                |  5 ++++
 src/panel/GotifyEditPanel.js | 44 ++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)
 create mode 100644 src/panel/GotifyEditPanel.js

diff --git a/src/Makefile b/src/Makefile
index e83038f..2e620e3 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -66,6 +66,7 @@ JSSRC=					\
 	panel/PruneKeepPanel.js		\
 	panel/RRDChart.js		\
 	panel/GaugeWidget.js		\
+	panel/GotifyEditPanel.js	\
 	panel/Certificates.js		\
 	panel/ACMEAccount.js		\
 	panel/ACMEPlugin.js		\
diff --git a/src/Schema.js b/src/Schema.js
index 99bb3fa..37ecd88 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -43,6 +43,11 @@ Ext.define('Proxmox.Schema', { // a singleton
 	    ipanel: 'pmxSendmailEditPanel',
 	    iconCls: 'fa-envelope-o',
 	},
+	gotify: {
+	    name: gettext('Gotify'),
+	    ipanel: 'pmxGotifyEditPanel',
+	    iconCls: 'fa-bell-o',
+	},
     },
 
     pxarFileTypes: {
diff --git a/src/panel/GotifyEditPanel.js b/src/panel/GotifyEditPanel.js
new file mode 100644
index 0000000..5d814e5
--- /dev/null
+++ b/src/panel/GotifyEditPanel.js
@@ -0,0 +1,44 @@
+Ext.define('Proxmox.panel.GotifyEditPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pmxGotifyEditPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    type: 'gotify',
+
+    items: [
+	{
+	    xtype: 'pmxDisplayEditField',
+	    name: 'name',
+	    cbind: {
+		value: '{name}',
+		editable: '{isCreate}',
+	    },
+	    fieldLabel: gettext('Endpoint Name'),
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    fieldLabel: gettext('Server URL'),
+	    name: 'server',
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    inputType: 'password',
+	    fieldLabel: gettext('API Token'),
+	    name: 'token',
+	    cbind: {
+		emptyText: get => !get('isCreate') ? gettext('Unchanged') : '',
+		allowBlank: '{!isCreate}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'comment',
+	    fieldLabel: gettext('Comment'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+    ],
+});
-- 
2.39.2





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

* [pve-devel] [PATCH v6 proxmox-widget-toolkit 27/30] notification: add gui for notification groups
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (25 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 26/30] notification: add gui for gotify " Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 28/30] notification: allow to select filter for notification targets Lukas Wagner
                   ` (4 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

The GUI is based on the 'plugin-based' dialog window EndpointEditBase
that was introduced in an earlier commit.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Extended commit message
    
    Changes since v3:
      - Use items/advancedItems instead of columns
      - Call initField in EndpointSelector
      - Minor code style improvements

 src/Makefile                            |   1 +
 src/Schema.js                           |   5 +
 src/panel/NotificationGroupEditPanel.js | 174 ++++++++++++++++++++++++
 src/window/EndpointEditBase.js          |   6 +-
 4 files changed, 185 insertions(+), 1 deletion(-)
 create mode 100644 src/panel/NotificationGroupEditPanel.js

diff --git a/src/Makefile b/src/Makefile
index 2e620e3..829081d 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -61,6 +61,7 @@ JSSRC=					\
 	panel/LogView.js		\
 	panel/NodeInfoRepoStatus.js	\
 	panel/NotificationConfigView.js	\
+	panel/NotificationGroupEditPanel.js	\
 	panel/JournalView.js		\
 	panel/PermissionView.js		\
 	panel/PruneKeepPanel.js		\
diff --git a/src/Schema.js b/src/Schema.js
index 37ecd88..a7ffdf8 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -48,6 +48,11 @@ Ext.define('Proxmox.Schema', { // a singleton
 	    ipanel: 'pmxGotifyEditPanel',
 	    iconCls: 'fa-bell-o',
 	},
+	group: {
+	    name: gettext('Notification Group'),
+	    ipanel: 'pmxNotificationGroupEditPanel',
+	    iconCls: 'fa-bell-o',
+	},
     },
 
     pxarFileTypes: {
diff --git a/src/panel/NotificationGroupEditPanel.js b/src/panel/NotificationGroupEditPanel.js
new file mode 100644
index 0000000..910d15a
--- /dev/null
+++ b/src/panel/NotificationGroupEditPanel.js
@@ -0,0 +1,174 @@
+Ext.define('Proxmox.panel.NotificationGroupEditPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pmxNotificationGroupEditPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    type: 'group',
+
+    items: [
+	{
+	    xtype: 'pmxDisplayEditField',
+	    name: 'name',
+	    cbind: {
+		value: '{name}',
+		editable: '{isCreate}',
+	    },
+	    fieldLabel: gettext('Group Name'),
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'pmxNotificationEndpointSelector',
+	    name: 'endpoint',
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'comment',
+	    fieldLabel: gettext('Comment'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+    ],
+});
+
+Ext.define('Proxmox.form.NotificationEndpointSelector', {
+    extend: 'Ext.grid.Panel',
+    alias: 'widget.pmxNotificationEndpointSelector',
+
+    mixins: {
+	field: 'Ext.form.field.Field',
+    },
+
+    padding: '0 0 10 0',
+
+    allowBlank: true,
+    selectAll: false,
+    isFormField: true,
+
+    store: {
+	autoLoad: true,
+	model: 'proxmox-notification-endpoints',
+	sorters: 'name',
+	filters: item => item.data.type !== 'group',
+    },
+
+    columns: [
+	{
+	    header: gettext('Endpoint Name'),
+	    dataIndex: 'name',
+	    flex: 1,
+	},
+	{
+	    header: gettext('Type'),
+	    dataIndex: 'type',
+	    flex: 1,
+	},
+	{
+	    header: gettext('Comment'),
+	    dataIndex: 'comment',
+	    flex: 3,
+	},
+    ],
+
+    selModel: {
+	selType: 'checkboxmodel',
+	mode: 'SIMPLE',
+    },
+
+    checkChangeEvents: [
+	'selectionchange',
+	'change',
+    ],
+
+    listeners: {
+	selectionchange: function() {
+	    // to trigger validity and error checks
+	    this.checkChange();
+	},
+    },
+
+    getSubmitData: function() {
+	let me = this;
+	let res = {};
+	res[me.name] = me.getValue();
+	return res;
+    },
+
+    getValue: function() {
+	let me = this;
+	if (me.savedValue !== undefined) {
+	    return me.savedValue;
+	}
+	let sm = me.getSelectionModel();
+	return (sm.getSelection() ?? []).map(item => item.data.name);
+    },
+
+    setValueSelection: function(value) {
+	let me = this;
+
+	let store = me.getStore();
+
+	let notFound = [];
+	let selection = value.map(item => {
+	    let found = store.findRecord('name', item, 0, false, true, true);
+	    if (!found) {
+		notFound.push(item);
+	    }
+	    return found;
+	}).filter(r => r);
+
+	for (const name of notFound) {
+	    let rec = store.add({
+		name,
+		type: '-',
+		comment: gettext('Included endpoint does not exist!'),
+	    });
+	    selection.push(rec[0]);
+	}
+
+	let sm = me.getSelectionModel();
+	if (selection.length) {
+	    sm.select(selection);
+	} else {
+	    sm.deselectAll();
+	}
+	// to correctly trigger invalid class
+	me.getErrors();
+    },
+
+    setValue: function(value) {
+	let me = this;
+
+	let store = me.getStore();
+	if (!store.isLoaded()) {
+	    me.savedValue = value;
+	    store.on('load', function() {
+		me.setValueSelection(value);
+		delete me.savedValue;
+	    }, { single: true });
+	} else {
+	    me.setValueSelection(value);
+	}
+	return me.mixins.field.setValue.call(me, value);
+    },
+
+    getErrors: function(value) {
+	let me = this;
+	if (!me.isDisabled() && me.allowBlank === false &&
+	    me.getSelectionModel().getCount() === 0) {
+	    me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+	    return [gettext('No endpoint selected')];
+	}
+
+	me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+	return [];
+    },
+
+    initComponent: function() {
+	let me = this;
+	me.callParent();
+	me.initField();
+    },
+
+});
diff --git a/src/window/EndpointEditBase.js b/src/window/EndpointEditBase.js
index 7e4c910..9230b99 100644
--- a/src/window/EndpointEditBase.js
+++ b/src/window/EndpointEditBase.js
@@ -18,7 +18,11 @@ Ext.define('Proxmox.window.EndpointEditBase', {
 	    throw "baseUrl not set";
 	}
 
-	me.url = `/api2/extjs${me.baseUrl}/endpoints/${me.type}`;
+	if (me.type === 'group') {
+	    me.url = `/api2/extjs${me.baseUrl}/groups`;
+	} else {
+	    me.url = `/api2/extjs${me.baseUrl}/endpoints/${me.type}`;
+	}
 
 	if (me.isCreate) {
 	    me.method = 'POST';
-- 
2.39.2





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

* [pve-devel] [PATCH v6 proxmox-widget-toolkit 28/30] notification: allow to select filter for notification targets
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (26 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 27/30] notification: add gui for notification groups Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 29/30] notification: add ui for managing notification filters Lukas Wagner
                   ` (3 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

This commit adds a new selector field for existing endpoint
configuration where one is able to select a notification filter.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Extended commit message

 src/Makefile                            |  1 +
 src/form/NotificationFilterSelector.js  | 58 +++++++++++++++++++++++++
 src/panel/GotifyEditPanel.js            |  9 ++++
 src/panel/NotificationConfigView.js     |  4 ++
 src/panel/NotificationGroupEditPanel.js |  9 ++++
 src/panel/SendmailEditPanel.js          |  9 ++++
 src/window/EndpointEditBase.js          |  1 +
 7 files changed, 91 insertions(+)
 create mode 100644 src/form/NotificationFilterSelector.js

diff --git a/src/Makefile b/src/Makefile
index 829081d..f661bb6 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -44,6 +44,7 @@ JSSRC=					\
 	form/RoleSelector.js		\
 	form/DiskSelector.js		\
 	form/MultiDiskSelector.js	\
+	form/NotificationFilterSelector.js	\
 	form/TaskTypeSelector.js	\
 	form/ACME.js			\
 	form/UserSelector.js		\
diff --git a/src/form/NotificationFilterSelector.js b/src/form/NotificationFilterSelector.js
new file mode 100644
index 0000000..d2ab8be
--- /dev/null
+++ b/src/form/NotificationFilterSelector.js
@@ -0,0 +1,58 @@
+Ext.define('Proxmox.form.NotificationFilterSelector', {
+    extend: 'Proxmox.form.ComboGrid',
+    alias: ['widget.pmxNotificationFilterSelector'],
+
+    // set default value to empty array, else it inits it with
+    // null and after the store load it is an empty array,
+    // triggering dirtychange
+    value: [],
+    valueField: 'name',
+    displayField: 'name',
+    deleteEmpty: true,
+    skipEmptyText: true,
+    allowBlank: true,
+    editable: false,
+    autoSelect: false,
+
+    listConfig: {
+	columns: [
+	    {
+		header: gettext('Filter'),
+		dataIndex: 'name',
+		sortable: true,
+		hideable: false,
+		flex: 1,
+	    },
+	    {
+		header: gettext('Comment'),
+		dataIndex: 'comment',
+		sortable: true,
+		hideable: false,
+		flex: 2,
+	    },
+	],
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	Ext.apply(me, {
+	    store: {
+		fields: ['name', 'comment'],
+		proxy: {
+		    type: 'proxmox',
+		    url: `/api2/json/${me.baseUrl}/filters`,
+		},
+		sorters: [
+		    {
+			property: 'name',
+			direction: 'ASC',
+		    },
+		],
+		autoLoad: true,
+	    },
+	});
+
+	me.callParent();
+    },
+});
diff --git a/src/panel/GotifyEditPanel.js b/src/panel/GotifyEditPanel.js
index 5d814e5..3ddcc4d 100644
--- a/src/panel/GotifyEditPanel.js
+++ b/src/panel/GotifyEditPanel.js
@@ -32,6 +32,15 @@ Ext.define('Proxmox.panel.GotifyEditPanel', {
 		allowBlank: '{!isCreate}',
 	    },
 	},
+	{
+	    xtype: 'pmxNotificationFilterSelector',
+	    name: 'filter',
+	    fieldLabel: gettext('Filter'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+		baseUrl: '{baseUrl}',
+	    },
+	},
 	{
 	    xtype: 'proxmoxtextfield',
 	    name: 'comment',
diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js
index 9282ccd..80e38f1 100644
--- a/src/panel/NotificationConfigView.js
+++ b/src/panel/NotificationConfigView.js
@@ -145,6 +145,10 @@ Ext.define('Proxmox.panel.NotificationEndpointView', {
     initComponent: function() {
 	let me = this;
 
+	if (!me.baseUrl) {
+	    throw "baseUrl is not set!";
+	}
+
 	let menuItems = [];
 	for (const [endpointType, config] of Object.entries(
 	    Proxmox.Schema.notificationEndpointTypes).sort()) {
diff --git a/src/panel/NotificationGroupEditPanel.js b/src/panel/NotificationGroupEditPanel.js
index 910d15a..aa76810 100644
--- a/src/panel/NotificationGroupEditPanel.js
+++ b/src/panel/NotificationGroupEditPanel.js
@@ -21,6 +21,15 @@ Ext.define('Proxmox.panel.NotificationGroupEditPanel', {
 	    name: 'endpoint',
 	    allowBlank: false,
 	},
+	{
+	    xtype: 'pmxNotificationFilterSelector',
+	    name: 'filter',
+	    fieldLabel: gettext('Filter'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+		baseUrl: '{baseUrl}',
+	    },
+	},
 	{
 	    xtype: 'proxmoxtextfield',
 	    name: 'comment',
diff --git a/src/panel/SendmailEditPanel.js b/src/panel/SendmailEditPanel.js
index 33ac482..b814f39 100644
--- a/src/panel/SendmailEditPanel.js
+++ b/src/panel/SendmailEditPanel.js
@@ -88,6 +88,15 @@ Ext.define('Proxmox.panel.SendmailEditPanel', {
 		return this.up('pmxSendmailEditPanel').mailValidator();
 	    },
 	},
+	{
+	    xtype: 'pmxNotificationFilterSelector',
+	    name: 'filter',
+	    fieldLabel: gettext('Filter'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+		baseUrl: '{baseUrl}',
+	    },
+	},
 	{
 	    xtype: 'proxmoxtextfield',
 	    name: 'comment',
diff --git a/src/window/EndpointEditBase.js b/src/window/EndpointEditBase.js
index 9230b99..f42d0ea 100644
--- a/src/window/EndpointEditBase.js
+++ b/src/window/EndpointEditBase.js
@@ -43,6 +43,7 @@ Ext.define('Proxmox.window.EndpointEditBase', {
 		name: me.name,
 		xtype: endpointConfig.ipanel,
 		isCreate: me.isCreate,
+		baseUrl: me.baseUrl,
 		type: me.type,
 	    }],
 	});
-- 
2.39.2





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

* [pve-devel] [PATCH v6 proxmox-widget-toolkit 29/30] notification: add ui for managing notification filters
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (27 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 28/30] notification: allow to select filter for notification targets Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-docs 30/30] add documentation for the new notification system Lukas Wagner
                   ` (2 subsequent siblings)
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

This commit adds a new dialog window, containing all fields necessary
to configure notification filters.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---

Notes:
    Changes since v5:
      - Extended commit message
    
    Changes since v3:
      - use items/advancedItems instead of columns

 src/Makefile                         |   3 +-
 src/data/model/NotificationConfig.js |   9 ++
 src/panel/NotificationConfigView.js  | 119 +++++++++++++++++++++++++++
 src/window/NotificationFilterEdit.js | 109 ++++++++++++++++++++++++
 4 files changed, 239 insertions(+), 1 deletion(-)
 create mode 100644 src/window/NotificationFilterEdit.js

diff --git a/src/Makefile b/src/Makefile
index f661bb6..21fbe76 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -89,7 +89,8 @@ JSSRC=					\
 	window/ACMEAccount.js		\
 	window/ACMEPluginEdit.js	\
 	window/ACMEDomains.js		\
-	window/EndpointEditBase.js		\
+	window/EndpointEditBase.js	\
+	window/NotificationFilterEdit.js \
 	window/FileBrowser.js		\
 	window/AuthEditBase.js		\
 	window/AuthEditOpenId.js	\
diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js
index c2ce843..bb4ef85 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -6,3 +6,12 @@ Ext.define('proxmox-notification-endpoints', {
     },
     idProperty: 'name',
 });
+
+Ext.define('proxmox-notification-filters', {
+    extend: 'Ext.data.Model',
+    fields: ['name', 'comment'],
+    proxy: {
+        type: 'proxmox',
+    },
+    idProperty: 'name',
+});
diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js
index 80e38f1..6586524 100644
--- a/src/panel/NotificationConfigView.js
+++ b/src/panel/NotificationConfigView.js
@@ -15,6 +15,17 @@ Ext.define('Proxmox.panel.NotificationConfigView', {
 		baseUrl: '{baseUrl}',
 	    },
 	},
+	{
+	    region: 'south',
+	    height: '50%',
+	    border: false,
+	    collapsible: true,
+	    animCollapse: false,
+	    xtype: 'pmxNotificationFilterView',
+	    cbind: {
+		baseUrl: '{baseUrl}',
+	    },
+	},
     ],
 });
 
@@ -198,3 +209,111 @@ Ext.define('Proxmox.panel.NotificationEndpointView', {
 	me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/targets`);
     },
 });
+
+Ext.define('Proxmox.panel.NotificationFilterView', {
+    extend: 'Ext.grid.Panel',
+    alias: 'widget.pmxNotificationFilterView',
+
+    title: gettext('Notification Filters'),
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	openEditWindow: function(filter) {
+	    let me = this;
+
+	    Ext.create('Proxmox.window.NotificationFilterEdit', {
+		baseUrl: me.getView().baseUrl,
+		name: filter,
+		autoShow: true,
+		listeners: {
+		    destroy: () => me.reload(),
+		},
+	    });
+	},
+
+	openEditForSelectedItem: function() {
+	    let me = this;
+	    let view = me.getView();
+
+	    let selection = view.getSelection();
+	    if (selection.length < 1) {
+		return;
+	    }
+
+	    me.openEditWindow(selection[0].data.name);
+	},
+
+	reload: function() {
+	    this.getView().getStore().rstore.load();
+	},
+    },
+
+    listeners: {
+	itemdblclick: 'openEditForSelectedItem',
+	activate: 'reload',
+    },
+
+    emptyText: gettext('No notification filters configured'),
+
+    columns: [
+	{
+	    dataIndex: 'name',
+	    text: gettext('Filter Name'),
+	    renderer: Ext.String.htmlEncode,
+	    flex: 1,
+	},
+	{
+	    dataIndex: 'comment',
+	    text: gettext('Comment'),
+	    renderer: Ext.String.htmlEncode,
+	    flex: 2,
+	},
+    ],
+
+    store: {
+	type: 'diff',
+	autoDestroy: true,
+	autoDestroyRstore: true,
+	rstore: {
+	    type: 'update',
+	    storeid: 'proxmox-notification-filters',
+	    model: 'proxmox-notification-filters',
+	    autoStart: true,
+	},
+	sorters: 'name',
+    },
+
+    initComponent: function() {
+	let me = this;
+
+	if (!me.baseUrl) {
+	    throw "baseUrl is not set!";
+	}
+
+	Ext.apply(me, {
+	    tbar: [
+		{
+		    xtype: 'proxmoxButton',
+		    text: gettext('Add'),
+		    handler: () => me.getController().openEditWindow(),
+		    selModel: false,
+		},
+		{
+		    xtype: 'proxmoxButton',
+		    text: gettext('Modify'),
+		    handler: 'openEditForSelectedItem',
+		    disabled: true,
+		},
+		{
+		    xtype: 'proxmoxStdRemoveButton',
+		    callback: 'reload',
+		    baseurl: `${me.baseUrl}/filters`,
+		},
+	    ],
+	});
+
+	me.callParent();
+	me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/filters`);
+    },
+});
diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationFilterEdit.js
new file mode 100644
index 0000000..703a9e2
--- /dev/null
+++ b/src/window/NotificationFilterEdit.js
@@ -0,0 +1,109 @@
+Ext.define('Proxmox.panel.NotificationFilterEditPanel', {
+    extend: 'Proxmox.panel.InputPanel',
+    xtype: 'pmxNotificationFilterEditPanel',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    items: [
+	{
+	    xtype: 'pmxDisplayEditField',
+	    name: 'name',
+	    cbind: {
+		value: '{name}',
+		editable: '{isCreate}',
+	    },
+	    fieldLabel: gettext('Filter Name'),
+	    allowBlank: false,
+	},
+	{
+	    xtype: 'proxmoxKVComboBox',
+	    name: 'min-severity',
+	    fieldLabel: gettext('Minimum Severity'),
+	    value: null,
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	    comboItems: [
+		['info', 'info'],
+		['notice', 'notice'],
+		['warning', 'warning'],
+		['error', 'error'],
+	    ],
+	    triggers: {
+		clear: {
+		    cls: 'pmx-clear-trigger',
+		    weight: -1,
+		    hidden: false,
+		    handler: function() {
+			this.setValue('');
+		    },
+		},
+	    },
+	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    fieldLabel: gettext('Invert match'),
+	    name: 'invert-match',
+	    uncheckedValue: 0,
+	    defaultValue: 0,
+	    cbind: {
+		deleteDefaultValue: '{!isCreate}',
+	    },
+	},
+	{
+	    xtype: 'proxmoxtextfield',
+	    name: 'comment',
+	    fieldLabel: gettext('Comment'),
+	    cbind: {
+		deleteEmpty: '{!isCreate}',
+	    },
+	},
+    ],
+});
+
+Ext.define('Proxmox.window.NotificationFilterEdit', {
+    extend: 'Proxmox.window.Edit',
+
+    isAdd: true,
+
+    fieldDefaults: {
+	labelWidth: 120,
+    },
+
+    width: 500,
+
+    initComponent: function() {
+	let me = this;
+
+	me.isCreate = !me.name;
+
+	if (!me.baseUrl) {
+	    throw "baseUrl not set";
+	}
+
+	me.url = `/api2/extjs${me.baseUrl}/filters`;
+
+	if (me.isCreate) {
+	    me.method = 'POST';
+	} else {
+	    me.url += `/${me.name}`;
+	    me.method = 'PUT';
+	}
+
+	me.subject = gettext('Notification Filter');
+
+	Ext.apply(me, {
+	    items: [{
+		name: me.name,
+		xtype: 'pmxNotificationFilterEditPanel',
+		isCreate: me.isCreate,
+		baseUrl: me.baseUrl,
+	    }],
+	});
+
+	me.callParent();
+
+	if (!me.isCreate) {
+	    me.load();
+	}
+    },
+});
-- 
2.39.2





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

* [pve-devel] [PATCH v6 pve-docs 30/30] add documentation for the new notification system
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (28 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 29/30] notification: add ui for managing notification filters Lukas Wagner
@ 2023-08-03 12:17 ` Lukas Wagner
  2023-08-11 11:22 ` [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce " Dominik Csapak
  2023-08-16 10:14 ` [pve-devel] applied-series: " Wolfgang Bumiller
  31 siblings, 0 replies; 34+ messages in thread
From: Lukas Wagner @ 2023-08-03 12:17 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc   | 159 +++++++++++++++++++++++++++++++++++++++++++
 pve-admin-guide.adoc |   2 +
 pve-gui.adoc         |   2 +
 vzdump.adoc          |   5 ++
 4 files changed, 168 insertions(+)
 create mode 100644 notifications.adoc

diff --git a/notifications.adoc b/notifications.adoc
new file mode 100644
index 0000000..c4d2931
--- /dev/null
+++ b/notifications.adoc
@@ -0,0 +1,159 @@
+[[chapter_notifications]]
+Notifications
+=============
+ifndef::manvolnum[]
+:pve-toplevel:
+endif::manvolnum[]
+
+[[notification_events]]
+Notification Events
+-------------------
+
+{pve} will attempt to notify system administrators in case of certain events,
+such as:
+
+[width="100%",options="header"]
+|===========================================================================
+| Event name        | Description                             | Severity
+| `package-updates` | System updates are available            | `info`
+| `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)
+|===========================================================================
+
+In the 'Notification' 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
+behavior (*when* to send a notification) and the notification target (*where* to
+send the notification).
+
+
+See also:
+
+* xref:datacenter_configuration_file[Datacenter Configuration]
+* xref:datacenter_configuration_file[vzdump]
+
+[[notification_targets]]
+Notification Targets
+--------------------
+
+Notification targets can be configured in the 'Notification Targets' panel.
+
+NOTE: The `mail-to-root` target is always available and cannot be modified or
+removed. It sends a mail the `root@pam` user by using the `sendmail` command and
+serves as a fallback target if no other target is configured for an event.
+
+Sendmail
+~~~~~~~~
+The sendmail binary is a program commonly found on Unix-like operating systems
+that handles the sending of email messages.
+It is a command-line utility that allows users and applications to send emails
+directly from the command line or from within scripts.
+
+The sendmail notification target uses the `sendmail` binary to send emails.
+
+
+NOTE: In standard {pve} installations, the `sendmail` binary is provided by
+Postfix. For this type of target to work correctly, it might be necessary to
+change Postfix's configuration so that it can correctly deliver emails.
+For cluster setups it is necessary to have a working Postfix configuration on
+every single cluster node.
+
+The configuration for Sendmail target plugins has the following options:
+
+* `mailto`: E-Mail address to which the notification shall be sent to. Can be
+set multiple times to accomodate multiple recipients.
+* `mailto-user`: Users to which emails shall be sent to. The user's email
+address will be looked up in `users.cfg`. Can be set multiple times to
+accomodate multiple recipients.
+* `author`: Sets the author of the E-Mail. Defaults to `Proxmox VE`.
+* `from-address`: Sets the from address of the E-Mail. If the parameter is not
+set, the plugin will fall back to the `email_from` setting from
+`datacenter.cfg`. If that is also not set, the plugin will default to
+`root@$hostname`, where `$hostname` is the hostname of the node.
+
+* `filter`: The name of the filter to use for this target.
+
+Gotify
+~~~~~~
+
+http://gotify.net[Gotify] is an open-source self-hosted notification server that
+allows you to send and receive push notifications to various devices and
+applications. It provides a simple API and web interface, making it easy to
+integrate with different platforms and services.
+
+The configuration for Gotify target plugins has the following options:
+
+* `server`: The base URL of the Gotify server, e.g. `http://<ip>:8888`
+* `token`: The authentication token. Tokens can be generated within the Gotify
+web interface.
+* `filter`: The name of the filter to use for this target.
+
+NOTE: The Gotify target plugin will respect the HTTP proxy settings from the
+ xref:datacenter_configuration_file[datacenter configuration]
+
+Group
+~~~~~
+
+One can only select a single target for notification events.
+To notify via multiple targets at the same time, a group can be created.
+A group can reference multiple targets. If a group is used as a target,
+the notification will be sent to all referenced targets. Groups can reference
+all targets except other groups.
+
+
+Notification Filters
+--------------------
+A notification target can be configured to use a *notification filter*.
+If a notification is sent to a target with a filter, the
+filter will determine if the notification will be actually sent or not.
+
+The following matchers are available:
+
+* `min-severity`: Matches notifications with equal or higher severity
+
+It is also possible to configure the evaluation of the individual matchers:
+
+* `invert-match`: Inverts the result of the whole filter
+* `mode`: Sets the logical operator used to connect the individual matchers to
+`and` or `or`. Defaults to `and`.
+
+The `mode` option also influences the evaluation of filters without any
+matchers. If set to `or`, an empty filter evaluates to `false` (do not notify).
+If set to `and`, the result is `true` (send a notification).
+----
+filter: always-matches
+    mode and
+
+filter: never-matches
+    mode or
+----
+
+Permissions
+-----------
+
+For every target or filter, there exists a corresponding ACL path
+`/mapping/notification/<name>`.
+If an operation can be triggered by a user (e.g. via the GUI or API) and if
+that operation is configured to notify via a given target, then
+the user must have the `Mapping.Use` permission on the corresponding
+node in the ACL tree.
+`Mapping.Modify` and `Mapping.Audit` are needed for
+writing/reading the configuration of a target or filter.
+
+NOTE: For backwards-compatibility, the special `mail-to-root` target
+does not require `Mapping.Use`.
+
+NOTE: When sending notifications via a group target,
+the user must have the `Mapping.Use` permission for every single endpoint
+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.
+
+
+
+
+
+
+
diff --git a/pve-admin-guide.adoc b/pve-admin-guide.adoc
index 5bac3d6..ce21d8f 100644
--- a/pve-admin-guide.adoc
+++ b/pve-admin-guide.adoc
@@ -51,6 +51,8 @@ include::ha-manager.adoc[]
 
 include::vzdump.adoc[]
 
+include::notifications.adoc[]
+
 // Return to normal title levels.
 :leveloffset: 0
 
diff --git a/pve-gui.adoc b/pve-gui.adoc
index e0fc873..9f63a7e 100644
--- a/pve-gui.adoc
+++ b/pve-gui.adoc
@@ -246,6 +246,8 @@ On the datacenter level, you can access cluster-wide settings and information.
 
 * *Metric Server:* define external metric servers for {pve}.
 
+* *Notifications:* configurate notification behavior and targets for  {pve}.
+
 * *Support:* display information about your support subscription.
 
 
diff --git a/vzdump.adoc b/vzdump.adoc
index f3eadcd..a7c3d1e 100644
--- a/vzdump.adoc
+++ b/vzdump.adoc
@@ -552,6 +552,11 @@ Backup all guest systems and send notification mails to root and admin.
 
  # vzdump --all --mode suspend --mailto root --mailto admin
 
+Backup guest 777 and notify via the `notify-admins` notification target on
+failure.
+
+ # vzdump 777  --notification-target notify-admins --notification-policy failure
+
 Use snapshot mode (no downtime) and non-default dump directory.
 
  # vzdump 777 --dumpdir /mnt/backup --mode snapshot
-- 
2.39.2





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

* [pve-devel] applied: [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module
  2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module Lukas Wagner
@ 2023-08-03 15:39   ` Thomas Lamprecht
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Lamprecht @ 2023-08-03 15:39 UTC (permalink / raw)
  To: Proxmox VE development discussion, Lukas Wagner

Am 03/08/2023 um 14:16 schrieb Lukas Wagner:
> ... instead of using sendmail directly.
> 
> If the new 'notify.target-fencing' parameter from datacenter config
> is set, we use it as a target for notifications. If it is not set,
> we send the notification to the default target (mail-to-root).
> 
> There is also a new 'notify.fencing' paramter which controls if
> notifications should be sent at all. If it is not set, we
> default to the old behavior, which is to send.
> 
> Also add dependency to the `libpve-notify-perl` package to d/control.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  debian/control           |  2 ++
>  src/PVE/HA/Env.pm        |  6 ++---
>  src/PVE/HA/Env/PVE2.pm   | 21 +++++++++-------
>  src/PVE/HA/NodeStatus.pm | 52 ++++++++++++++++++++++++----------------
>  src/PVE/HA/Sim/Env.pm    | 10 ++++++--
>  5 files changed, 57 insertions(+), 34 deletions(-)
> 
>

applied, thanks!




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

* Re: [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (29 preceding siblings ...)
  2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-docs 30/30] add documentation for the new notification system Lukas Wagner
@ 2023-08-11 11:22 ` Dominik Csapak
  2023-08-16 10:14 ` [pve-devel] applied-series: " Wolfgang Bumiller
  31 siblings, 0 replies; 34+ messages in thread
From: Dominik Csapak @ 2023-08-11 11:22 UTC (permalink / raw)
  To: Proxmox VE development discussion, Lukas Wagner

did not look through the api/backend part, but the gui part is now ok from my side,
so consider the manger ui patches (15-22) and the wt patches (25-29)

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>




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

* [pve-devel] applied-series: [PATCH v6 many 00/30] fix #4156: introduce new notification system
  2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
                   ` (30 preceding siblings ...)
  2023-08-11 11:22 ` [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce " Dominik Csapak
@ 2023-08-16 10:14 ` Wolfgang Bumiller
  31 siblings, 0 replies; 34+ messages in thread
From: Wolfgang Bumiller @ 2023-08-16 10:14 UTC (permalink / raw)
  To: Lukas Wagner; +Cc: pve-devel

applied series, thanks

fixed up the mocking in tests & bumped dependencies accordingly




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

end of thread, other threads:[~2023-08-16 10:14 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-03 12:16 [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce new notification system Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-ha-manager 01/30] manager: send notifications via new notification module Lukas Wagner
2023-08-03 15:39   ` [pve-devel] applied: " Thomas Lamprecht
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 02/30] d/control: add dependency to `libpve-notify-perl` Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 03/30] vzdump: send notifications via new notification module Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 04/30] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 05/30] api: apt: send notification via new notification module Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 06/30] api: replication: send notifications " Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 07/30] api: prepare api handler module for notification config Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 08/30] api: notification: add api routes for groups Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 09/30] api: notification: add api routes for sendmail endpoints Lukas Wagner
2023-08-03 12:16 ` [pve-devel] [PATCH v6 pve-manager 10/30] api: notification: add api routes for gotify endpoints Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 11/30] api: notification: add api routes for filters Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 12/30] api: notification: allow fetching notification targets Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 13/30] api: notification: allow to test targets Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 14/30] api: notification: disallow removing targets if they are used Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 15/30] ui: backup: allow to select notification target for jobs Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 16/30] ui: backup: adapt backup job details to new notification params Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 17/30] ui: backup: allow to set notification-target for one-off backups Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 18/30] ui: allow to configure notification event -> target mapping Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 19/30] ui: add notification target configuration panel Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 20/30] ui: perm path: add ACL paths for notifications, usb and pci mappings Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 21/30] ui: perm path: increase width of the perm path selector combobox Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 22/30] ui: dc: remove notify key from datacenter option view Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 23/30] vzdump: use <name> as a convention for virtual endpoints/groups Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-manager 24/30] api: notification: make the 'mail-to-root' target visible to any user Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 25/30] notification: add gui for sendmail notification endpoints Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 26/30] notification: add gui for gotify " Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 27/30] notification: add gui for notification groups Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 28/30] notification: allow to select filter for notification targets Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 proxmox-widget-toolkit 29/30] notification: add ui for managing notification filters Lukas Wagner
2023-08-03 12:17 ` [pve-devel] [PATCH v6 pve-docs 30/30] add documentation for the new notification system Lukas Wagner
2023-08-11 11:22 ` [pve-devel] [PATCH v6 many 00/30] fix #4156: introduce " Dominik Csapak
2023-08-16 10:14 ` [pve-devel] applied-series: " Wolfgang Bumiller

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