From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 0A41D1FF56B for ; Mon, 22 Apr 2024 16:25:00 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B572C13B44; Mon, 22 Apr 2024 16:25:04 +0200 (CEST) Date: Mon, 22 Apr 2024 16:24:51 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20240422123841.280675-1-l.wagner@proxmox.com> In-Reply-To: <20240422123841.280675-1-l.wagner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1713795522.9okq9bghe3.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.095 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH many v4 00/43] integrate notification system X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" On April 22, 2024 2:37 pm, Lukas Wagner wrote: > These patches integrate the notification system which was introduced in Proxmox VE > in 8.1 into Proxmox Backup Server. > > Some highlights/noteworthy details from the series: > - notification template files are installed in /usr/share/proxmox-backup/templates/default > and are rendered when a notification is sent > - since sending notifications needs to be done from a privileged context > (to read protected passwords/tokens from the priv config file), > we queue notifications to be sent in /var/lib/proxmox-backup/notifications > and periodically send any queued notifications via a worker in the > privileged process > - The API endpoint paths are prefixed with /config/notifications > - API endpoints which read/modify notification system config require > Sys.Audit (read) or Sys.Modify (modify/test endpoint) permissions > on /system/notifications > > - tape-{backup,restore} settings and datastore options now have a > 'notification-mode' parameter, which allows to choose between the > 'legacy' behavior (sendmail to a selected user's email address) and > the event-based notification system. If the parameter is not set, we > default to the legacy behavior in order to keep existing behavior > as is for now. For new datastores/tape backup jobs created > interactively from the UI, we set the parameter to 'notification-system' > and opt in into the new system by default. For the CLI/API we don't > so that we don't cause problems with any automations/scripts - > there I'd change the default to the new system with the next major release > > Rough edges: > - Datastore option view in UI could be improved. When 'notification-mode' is > set to 'notification-system', we should indicate that 'mailto-user' and the > other notification settings have no effect. Already did that in the edit > window, but in the grid panel I did not find a quick way to do that. in general this series looks mostly good to me, some rough edges I noticed while testing: - if the user lacks Sys.Audit, the notifications part is empty, even though there are built-in targets/matchers, this might cause complications for one-off tasks that allow selecting notification things, or cause confusion on the user's side - the datastore/job related checks when configuring notifications currently don't allow access to datastores with just Datastore.Backup, and not .Audit, even though such datastores/jobs are fully accessible by the user.. - docs/config/notifications/format.rst has partly invalid content - Sys.Modify is only contained in the Admin role atm, which has a lot of other privs - maybe we want to have our own NotificationsAdmin? not sure whether that makes sense.. maybe it would be better to think about this for a bit and maybe add a priv if we want to make this configurable by "medium privileged" users, since Sys.Modify is quite the heavy hammer anyway.. - as you said, the docs part for synopsis (is still) broken, this is a pre-existing issue though I pushed the collection of patches for PBS: - proxmox-widget-toolkit - proxmox/proxmox-notify (including a bump of the package) - proxmox-backup (including a bump of the proxmox-notify dep) to my staff repo(s), so that interested people don't have to redo all the application work ;) > > Prerequisites: > > - This patch series requires the patches for 'proxmox' from > https://lists.proxmox.com/pipermail/pve-devel/2024-April/063493.html > to be applied first > > - The dependency for proxmox-widget-toolkit should be bumped to at least > 4.1.4 (we need: "utils: add mechanism to add and override translatable notification event > descriptions in the product specific UIs") > > Changes since v3: > - Pull in UI changes from 'notification metadata matching improvments' as > component overrides - this can and should be removed once the > widget toolkit patches are applied. > - no other changes apart from that > > Changes since v2: > - consolidate verify/prune/sync/tape-job to job-id > - add a note to docs that package update notifications > are only sent by nodes with an active subscription > > Changes since v1: > - proxmox_notify: Change default-matcher to exclude 'prune success' notifications > - proxmox_notify: Move sendmail/forward functions from proxmox_sys > - proxmox_notify: Improve docs for API types > - proxmox-widget-toolkit: allow to override default mail author > - Add documentation, man pages, etc. (mostly copied from PVE, some > stylistic touchups, some rephrasing) > - Add additional matchable metadata fields, allowing to match on > individual jobs: > - verification-job > - prune-job > - sync-job > - tape-job > - Sort queued notifications by timestamp before sending > - Fix missing hostname in package-update notification > - Change default mail author in the UI from Proxmox VE to Proxmox Backup Server ($hostname) > > proxmox: > > Lukas Wagner (5): > notify: expose `config` module > notify: use std::sync::OnceCell instead of lazy_static! > notify: pbs-context: exclude successful prunes in default matcher > notify: endpoints: matcher: improve descriptions for API types > notify: add getter for notification timestamp > > proxmox-notify/Cargo.toml | 1 - > proxmox-notify/src/config.rs | 23 ++++++++++++------- > proxmox-notify/src/context/pbs.rs | 5 ++++- > proxmox-notify/src/endpoints/gotify.rs | 6 ++--- > proxmox-notify/src/endpoints/sendmail.rs | 15 ++++++++----- > proxmox-notify/src/endpoints/smtp.rs | 28 +++++++++++++++--------- > proxmox-notify/src/lib.rs | 11 ++++++---- > proxmox-notify/src/matcher.rs | 18 +++++++-------- > 8 files changed, 66 insertions(+), 41 deletions(-) > > > proxmox-widget-toolkit: > > Lukas Wagner (1): > sendmail: smtp: allow to overide default mail author > > src/Schema.js | 9 +++++++++ > src/panel/SendmailEditPanel.js | 2 +- > src/panel/SmtpEditPanel.js | 2 +- > src/window/EndpointEditBase.js | 1 + > 4 files changed, 12 insertions(+), 2 deletions(-) > > > proxmox-backup: > > Lukas Wagner (37): > pbs-config: add module for loading notification config > server: rename email_notifications module to notifications > notifications: allow sending notifications via proxmox_notify > buildsys: install templates for test notifications > pbs-config: acl: add /system/notifications as known ACL path > api: add endpoints for querying/testing notification targets > api: add endpoints for notification matchers > api: add endpoints for sendmail targets > api: add endpoints for smtp targets > api: add endpoints for gotify targets > api: add endpoints for querying known notification values/fields > api-types: api: datatore: add notification-mode parameter > api-types: api: tape: add notification-mode parameter > server: notifications: send GC notifications via notification system > server: notifications: send prune notifications via notification > system > server: notifications: send verify notifications via notification > system > server: notifications: send sync notifications via notification system > server: notifications: send update notifications via notification > system > server: notifications: send acme notifications via notification system > server: notifications: send tape notifications via notification system > ui: add notification config panel > ui: tape backup job: add selector for notification-mode > ui: tape backup: add selector for 'notification-mode' > ui: tape restore: add 'notification-mode' parameter > ui: datastore options: add 'notification-mode' parameter > ui: utils: add overrides for known notification metadata fields/values > ui: datastore edit: make new stores use notification system by default > ui: permissions paths: add /system/notifications to combobox > proxmox-backup-manager: add CLI for notification targets > proxmox-backup-manager: add CLI for notification matchers > proxmox-backup-manager: add CLI for gotify endpoints > proxmox-backup-manager: add CLI for sendmail endpoints > proxmox-backup-manager: add CLI for SMTP endpoints > docgen: generate synopsis for notifications{-priv,}.cfg > docs: add documentation for notification system > ui: util: override default mail author for sendmail/smtp targets > ui: notifications: pull in UX improvements for match rules creation > > Cargo.toml | 3 + > Makefile | 5 +- > debian/proxmox-backup-server.install | 31 + > docs/Makefile | 6 +- > docs/conf.py | 2 + > docs/config/notifications-priv/format.rst | 1 + > docs/config/notifications-priv/man5.rst | 24 + > docs/config/notifications/format.rst | 28 + > docs/config/notifications/man5.rst | 24 + > docs/configuration-files.rst | 30 + > docs/index.rst | 1 + > docs/notifications.rst | 213 ++++ > pbs-api-types/src/datastore.rs | 22 + > pbs-api-types/src/jobs.rs | 8 +- > pbs-config/Cargo.toml | 1 + > pbs-config/src/acl.rs | 3 +- > pbs-config/src/lib.rs | 1 + > pbs-config/src/notifications.rs | 41 + > src/api2/config/datastore.rs | 9 + > src/api2/config/mod.rs | 2 + > src/api2/config/notifications/gotify.rs | 190 +++ > src/api2/config/notifications/matchers.rs | 170 +++ > src/api2/config/notifications/mod.rs | 210 ++++ > src/api2/config/notifications/sendmail.rs | 178 +++ > src/api2/config/notifications/smtp.rs | 191 +++ > src/api2/config/notifications/targets.rs | 63 + > src/api2/config/tape_backup_job.rs | 8 + > src/api2/pull.rs | 10 +- > src/api2/tape/backup.rs | 62 +- > src/api2/tape/restore.rs | 46 +- > src/bin/docgen.rs | 4 + > src/bin/proxmox-backup-api.rs | 11 + > src/bin/proxmox-backup-manager.rs | 2 + > src/bin/proxmox-backup-proxy.rs | 1 + > src/bin/proxmox_backup_manager/mod.rs | 2 + > .../notifications/gotify.rs | 93 ++ > .../notifications/matchers.rs | 93 ++ > .../notifications/mod.rs | 21 + > .../notifications/sendmail.rs | 94 ++ > .../notifications/smtp.rs | 96 ++ > .../notifications/targets.rs | 51 + > src/server/email_notifications.rs | 763 ------------ > src/server/gc_job.rs | 10 +- > src/server/mod.rs | 4 +- > src/server/notifications.rs | 566 +++++++++ > src/server/verify_job.rs | 10 +- > src/tape/drive/mod.rs | 22 +- > src/tape/mod.rs | 27 + > src/tape/pool_writer/mod.rs | 11 +- > templates/Makefile | 41 + > templates/default/acme-err-body.txt.hbs | 7 + > templates/default/acme-err-subject.txt.hbs | 1 + > templates/default/gc-err-body.txt.hbs | 8 + > templates/default/gc-err-subject.txt.hbs | 1 + > templates/default/gc-ok-body.txt.hbs | 23 + > templates/default/gc-ok-subject.txt.hbs | 1 + > .../default/package-updates-body.txt.hbs | 8 + > .../default/package-updates-subject.txt.hbs | 1 + > templates/default/prune-err-body.txt.hbs | 10 + > templates/default/prune-err-subject.txt.hbs | 1 + > templates/default/prune-ok-body.txt.hbs | 10 + > templates/default/prune-ok-subject.txt.hbs | 1 + > templates/default/sync-err-body.txt.hbs | 14 + > templates/default/sync-err-subject.txt.hbs | 5 + > templates/default/sync-ok-body.txt.hbs | 14 + > templates/default/sync-ok-subject.txt.hbs | 5 + > .../default/tape-backup-err-body.txt.hbs | 26 + > .../default/tape-backup-err-subject.txt.hbs | 5 + > templates/default/tape-backup-ok-body.txt.hbs | 27 + > .../default/tape-backup-ok-subject.txt.hbs | 5 + > templates/default/tape-load-body.txt.hbs | 15 + > templates/default/tape-load-subject.txt.hbs | 1 + > templates/default/test-body.html.hbs | 1 + > templates/default/test-body.txt.hbs | 1 + > templates/default/test-subject.txt.hbs | 1 + > templates/default/verify-err-body.txt.hbs | 14 + > templates/default/verify-err-subject.txt.hbs | 1 + > templates/default/verify-ok-body.txt.hbs | 10 + > templates/default/verify-ok-subject.txt.hbs | 1 + > www/Makefile | 2 + > www/NavigationTree.js | 6 + > www/OnlineHelpInfo.js | 24 + > www/Utils.js | 39 + > www/config/NotificationConfigView.js | 11 + > www/datastore/OptionView.js | 15 + > www/form/PermissionPathSelector.js | 1 + > www/tape/window/TapeBackup.js | 25 + > www/tape/window/TapeBackupJob.js | 24 + > www/tape/window/TapeRestore.js | 17 + > www/window/DataStoreEdit.js | 13 + > www/window/NotificationMatcherOverride.js | 1105 +++++++++++++++++ > www/window/NotifyOptions.js | 38 + > 92 files changed, 4174 insertions(+), 869 deletions(-) > create mode 100644 docs/config/notifications-priv/format.rst > create mode 100644 docs/config/notifications-priv/man5.rst > create mode 100644 docs/config/notifications/format.rst > create mode 100644 docs/config/notifications/man5.rst > create mode 100644 docs/notifications.rst > create mode 100644 pbs-config/src/notifications.rs > create mode 100644 src/api2/config/notifications/gotify.rs > create mode 100644 src/api2/config/notifications/matchers.rs > create mode 100644 src/api2/config/notifications/mod.rs > create mode 100644 src/api2/config/notifications/sendmail.rs > create mode 100644 src/api2/config/notifications/smtp.rs > create mode 100644 src/api2/config/notifications/targets.rs > create mode 100644 src/bin/proxmox_backup_manager/notifications/gotify.rs > create mode 100644 src/bin/proxmox_backup_manager/notifications/matchers.rs > create mode 100644 src/bin/proxmox_backup_manager/notifications/mod.rs > create mode 100644 src/bin/proxmox_backup_manager/notifications/sendmail.rs > create mode 100644 src/bin/proxmox_backup_manager/notifications/smtp.rs > create mode 100644 src/bin/proxmox_backup_manager/notifications/targets.rs > delete mode 100644 src/server/email_notifications.rs > create mode 100644 src/server/notifications.rs > create mode 100644 templates/Makefile > create mode 100644 templates/default/acme-err-body.txt.hbs > create mode 100644 templates/default/acme-err-subject.txt.hbs > create mode 100644 templates/default/gc-err-body.txt.hbs > create mode 100644 templates/default/gc-err-subject.txt.hbs > create mode 100644 templates/default/gc-ok-body.txt.hbs > create mode 100644 templates/default/gc-ok-subject.txt.hbs > create mode 100644 templates/default/package-updates-body.txt.hbs > create mode 100644 templates/default/package-updates-subject.txt.hbs > create mode 100644 templates/default/prune-err-body.txt.hbs > create mode 100644 templates/default/prune-err-subject.txt.hbs > create mode 100644 templates/default/prune-ok-body.txt.hbs > create mode 100644 templates/default/prune-ok-subject.txt.hbs > create mode 100644 templates/default/sync-err-body.txt.hbs > create mode 100644 templates/default/sync-err-subject.txt.hbs > create mode 100644 templates/default/sync-ok-body.txt.hbs > create mode 100644 templates/default/sync-ok-subject.txt.hbs > create mode 100644 templates/default/tape-backup-err-body.txt.hbs > create mode 100644 templates/default/tape-backup-err-subject.txt.hbs > create mode 100644 templates/default/tape-backup-ok-body.txt.hbs > create mode 100644 templates/default/tape-backup-ok-subject.txt.hbs > create mode 100644 templates/default/tape-load-body.txt.hbs > create mode 100644 templates/default/tape-load-subject.txt.hbs > create mode 100644 templates/default/test-body.html.hbs > create mode 100644 templates/default/test-body.txt.hbs > create mode 100644 templates/default/test-subject.txt.hbs > create mode 100644 templates/default/verify-err-body.txt.hbs > create mode 100644 templates/default/verify-err-subject.txt.hbs > create mode 100644 templates/default/verify-ok-body.txt.hbs > create mode 100644 templates/default/verify-ok-subject.txt.hbs > create mode 100644 www/config/NotificationConfigView.js > create mode 100644 www/window/NotificationMatcherOverride.js > > > Summary over all repositories: > 104 files changed, 4252 insertions(+), 912 deletions(-) > > -- > Generated by git-murpp 0.7.1 > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel