[pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements

[pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements

[Lukas Wagner]

This patch series attempts to improve the user experience when creating
notification matchers.

Some of the noteworthy changes:
  - Fixup inconsistent 'hostname' field. Some notification events sent
  the hostname including a domain, while other did not.
  This series unifies the behavior, now the field only includes the hostname
  without a domain. Also updated the docs to reflect this change.
  - Allow setting a custom backup job ID, similar how we handle it for
  sync/prune jobs in PBS (to allow recognizable names used in matchers)
  - Adding columns for backup job ID/replication job ID in the UI
  - New metadata fields:
    - job-id: Job ID for backup-jobs or replication-jobs
  - Add an API that enumerates known notification metadata fields/values
  - Suggest known fields/values in match rule window
  - Some code clean up for match rule edit window
  - Extended the 'exact' match-field mode - it now allows setting multiple
    allowed values, separated via ',':
      e.g. `match-field exact:type=replication,fencing
    Originally, I created a separate 'list' match type for this, but
    since the semantics for a list with one value and 'exact' mode
    are identical, I decided to just extend 'exact'.
    This is a safe change since there are are no values where a ','
    makes any sense (config IDs, hostnames)

NOTE: Might need a versionened break, since the widget-toolkit-patches
depend on new APIs provided by pve-manager. If the API is not present,
creating matchers with 'match-field' does not work (cannot load lists
of known values/fields)

Inter-Dependencies:
  - the widget-toolkit dep in pve-manager needs to 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", otherwise the UI breaks
    with the pve-manager patches applied) --> already included a patch for
    this
  - widget-toolkit relies on a new API endpoint provided by pve-manager:
    --> we require a versioned break in widget-toolkit on pve-manager

Changelog:
  - v7: incorporated some more feedback from @Fiona, thx!
    - Fixed error when switching from 'exact' to 'regex' if the text field
      was empty
    - rebased to latest master
    - 'backport' doc improvements from PBS
    - bumped widget-toolkit dep
  - v6: incorporate feedback from @Fiona, thx!
    - rename 'id' -> 'job-id' in VZDump API handler
    - consolidate 'replication-job'/'backup-job' to 'job-id'
    - Move 'job-id' setting to advanced tab in backup job edit.
    - Don't use 'internal' flag to mark translatable fields, since
      the only field where that's necessary is 'type' for now - so
      just add a hardcoded check
  - v5:
    - Rebased onto latest master, resolving some small conflict
  - v4:
    - widget-toolkit: break out changes for the utils module so that they
      can be applied ahead of time to ease dep bumping
    - don't show Job IDs in the backup/replication job columns
  - v3:
    - Drop already applied patches for `proxmox`
    - Rebase onto latest master - minor conflict resolution was needed
  - v2:
    - include 'type' metadata field for forwarded mails
      --> otherwise it's not possible to match them
    - include Maximilliano's T-b trailer in UI patches

pve-guest-common:

Lukas Wagner (1):
  vzdump: common: allow 'job-id' as a parameter without being in schema

 src/PVE/VZDump/Common.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


pve-manager:

Lukas Wagner (9):
  api: jobs: vzdump: pass job 'job-id' parameter
  ui: dc: backup: send 'job-id' property when starting a backup job
    manually
  ui: dc: backup: allow to set custom job id in  advanced settings
  api: replication: add 'job-id' to notification metadata
  vzdump: apt: notification: do not include domain in 'hostname' field
  api: replication: include 'hostname' field for notifications
  api: notification: add API for getting known metadata fields/values
  ui: utils: add overrides for translatable notification fields/values
  d/control: bump proxmox-widget-toolkit dependency to 4.1.4

 PVE/API2/APT.pm                             |   3 +-
 PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
 PVE/API2/Replication.pm                     |   4 +-
 PVE/API2/VZDump.pm                          |   8 ++
 PVE/Jobs/VZDump.pm                          |   4 +-
 PVE/VZDump.pm                               |  14 +-
 debian/control                              |   2 +-
 www/manager6/Utils.js                       |  12 ++
 www/manager6/dc/Backup.js                   |   7 +-
 www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
 10 files changed, 200 insertions(+), 16 deletions(-)


proxmox-widget-toolkit:

Lukas Wagner (4):
  notification: matcher: match-field: show known fields/values
  notification: matcher: move match-field formulas to local viewModel
  notification: matcher: move match-calendar fields to panel
  notification: matcher: move match-severity fields to panel

 src/data/model/NotificationConfig.js  |  12 +
 src/window/NotificationMatcherEdit.js | 613 ++++++++++++++++++--------
 2 files changed, 441 insertions(+), 184 deletions(-)


pve-docs:

Lukas Wagner (5):
  notification: clarify that 'hostname' does not include the domain
  notifications: describe new notification metadata fields
  notifications: match-field 'exact'-mode can now match multiple values
  notifications: fix typo in 'notification'
  notifications: backport some rephrased parts from PBS docs

 notifications.adoc | 137 +++++++++++++++++++++++++++------------------
 1 file changed, 84 insertions(+), 53 deletions(-)


Summary over all repositories:
  14 files changed, 726 insertions(+), 254 deletions(-)

-- 
Generated by git-murpp 0.7.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH pve-guest-common v7 01/19] vzdump: common: allow 'job-id' as a parameter without being in schema

[Lukas Wagner]

'job-id' is passed when a backup as started as a job and will be
passed to the notification system as matchable metadata. It it
can be considered 'internal'.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 src/PVE/VZDump/Common.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 1539444..e835b05 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -496,7 +496,7 @@ sub command_line {
 
     foreach my $p (keys %$param) {
 	next if $p eq 'id' || $p eq 'vmid' || $p eq 'starttime' ||
-	        $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled';
+	        $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled' || $p eq 'job-id';
 	my $v = $param->{$p};
 	my $pd = $confdesc->{$p} || die "no such vzdump option '$p'\n";
 	if ($p eq 'exclude-path') {
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 02/19] api: jobs: vzdump: pass job 'job-id' parameter

[Lukas Wagner]

This allows us to access us the backup job id in the send_notification
function, where we can set it as metadata for the notification.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/VZDump.pm | 8 ++++++++
 PVE/Jobs/VZDump.pm | 4 +++-
 PVE/VZDump.pm      | 6 +++---
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 7f92e7ec..84dbc100 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -53,6 +53,14 @@ __PACKAGE__->register_method ({
     parameters => {
 	additionalProperties => 0,
 	properties => PVE::VZDump::Common::json_config_properties({
+	    'job-id' => {
+		description => "The ID of the backup job. If set, the 'backup-job' metadata field"
+		    . " of the backup notification will be set to this value.",
+		type => 'string',
+		format => 'pve-configid',
+		maxLength => 256,
+		optional => 1,
+	    },
 	    stdout => {
 		type => 'boolean',
 		description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2dad3f55 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
 use base qw(PVE::VZDump::JobBase);
 
 sub run {
-    my ($class, $conf) = @_;
+    my ($class, $conf, $job_id) = @_;
 
     my $props = $class->properties();
     # remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
 	delete $conf->{$opt} if !defined($props->{$opt});
     }
 
+    $conf->{'job-id'} = $job_id;
+
     # Required as string parameters # FIXME why?! we could just check ref()
     for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
 	if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 5b7080f3..2167b289 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -483,6 +483,7 @@ sub send_notification {
     my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
 
     my $opts = $self->{opts};
+    my $job_id = $opts->{'job-id'};
     my $mailto = $opts->{mailto};
     my $cmdline = $self->{cmdline};
     my $policy = $opts->{mailnotification} // 'always';
@@ -529,12 +530,11 @@ sub send_notification {
     };
 
     my $fields = {
-	# TODO: There is no straight-forward way yet to get the
-	# backup job id here... (I think pvescheduler would need
-	# to pass that to the vzdump call?)
 	type => "vzdump",
 	hostname => $hostname,
     };
+    # Add backup-job metadata field in case this is a backup job.
+    $fields->{'job-id'} = $job_id if $job_id;
 
     my $severity = $failed ? "error" : "info";
     my $email_configured = $mailto && scalar(@$mailto);
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH manager v7 02/19] api: jobs: vzdump: pass job 'job-id' parameter

[Fabian Grünbichler]

Quoting Lukas Wagner (2024-06-10 10:40:21)
> This allows us to access us the backup job id in the send_notification
> function, where we can set it as metadata for the notification.

should we have some sort of safeguard against passing in a bogus/fake job-id?
e.g., right now, I could call this API endpoint with arbitrary job-id values
and (potentially) trigger notifications to other users..

some possible avenues would be:
- limit the parameter to root (but that means only scheduled executions can set
  it, manual invocations can't)
- limit to existing job-ids (doesn't provide much benefit)
- ..

> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  PVE/API2/VZDump.pm | 8 ++++++++
>  PVE/Jobs/VZDump.pm | 4 +++-
>  PVE/VZDump.pm      | 6 +++---
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
> index 7f92e7ec..84dbc100 100644
> --- a/PVE/API2/VZDump.pm
> +++ b/PVE/API2/VZDump.pm
> @@ -53,6 +53,14 @@ __PACKAGE__->register_method ({
>      parameters => {
>         additionalProperties => 0,
>         properties => PVE::VZDump::Common::json_config_properties({
> +           'job-id' => {
> +               description => "The ID of the backup job. If set, the 'backup-job' metadata field"
> +                   . " of the backup notification will be set to this value.",
> +               type => 'string',
> +               format => 'pve-configid',
> +               maxLength => 256,
> +               optional => 1,
> +           },
>             stdout => {
>                 type => 'boolean',
>                 description => "Write tar to stdout, not to a file.",
> diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
> index b8e57945..2dad3f55 100644
> --- a/PVE/Jobs/VZDump.pm
> +++ b/PVE/Jobs/VZDump.pm
> @@ -12,7 +12,7 @@ use PVE::API2::VZDump;
>  use base qw(PVE::VZDump::JobBase);
>  
>  sub run {
> -    my ($class, $conf) = @_;
> +    my ($class, $conf, $job_id) = @_;
>  
>      my $props = $class->properties();
>      # remove all non vzdump related options
> @@ -20,6 +20,8 @@ sub run {
>         delete $conf->{$opt} if !defined($props->{$opt});
>      }
>  
> +    $conf->{'job-id'} = $job_id;
> +
>      # Required as string parameters # FIXME why?! we could just check ref()
>      for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
>         if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 5b7080f3..2167b289 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -483,6 +483,7 @@ sub send_notification {
>      my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
>  
>      my $opts = $self->{opts};
> +    my $job_id = $opts->{'job-id'};
>      my $mailto = $opts->{mailto};
>      my $cmdline = $self->{cmdline};
>      my $policy = $opts->{mailnotification} // 'always';
> @@ -529,12 +530,11 @@ sub send_notification {
>      };
>  
>      my $fields = {
> -       # TODO: There is no straight-forward way yet to get the
> -       # backup job id here... (I think pvescheduler would need
> -       # to pass that to the vzdump call?)
>         type => "vzdump",
>         hostname => $hostname,
>      };
> +    # Add backup-job metadata field in case this is a backup job.
> +    $fields->{'job-id'} = $job_id if $job_id;
>  
>      my $severity = $failed ? "error" : "info";
>      my $email_configured = $mailto && scalar(@$mailto);
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 03/19] ui: dc: backup: send 'job-id' property when starting a backup job manually

[Lukas Wagner]

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

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4ba80b31..4b45b5c6 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -604,11 +604,12 @@ Ext.define('PVE.dc.BackupView', {
 	    job = Ext.clone(job);
 
 	    let jobNode = job.node;
+	    job['job-id'] = job.id;
+	    delete job.id;
 	    // Remove properties related to scheduling
 	    delete job.enabled;
 	    delete job.starttime;
 	    delete job.dow;
-	    delete job.id;
 	    delete job.schedule;
 	    delete job.type;
 	    delete job.node;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 04/19] ui: dc: backup: allow to set custom job id in advanced settings

[Lukas Wagner]

This might be useful if somebody wants to match on the new
'backup-job' field in a notification match rule.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 www/manager6/Utils.js                       |  1 +
 www/manager6/dc/Backup.js                   |  4 ----
 www/manager6/panel/BackupAdvancedOptions.js | 23 +++++++++++++++++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index f5608944..5b0d51eb 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1963,6 +1963,7 @@ Ext.define('PVE.Utils', {
     singleton: true,
     constructor: function() {
 	var me = this;
+
 	Ext.apply(me, me.utilities);
 
 	Proxmox.Utils.override_task_descriptions({
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4b45b5c6..5975cb1c 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -45,10 +45,6 @@ Ext.define('PVE.dc.BackupEdit', {
 		Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-target' });
 	    }
 
-	    if (!values.id && isCreate) {
-		values.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
-	    }
-
 	    let selMode = values.selMode;
 	    delete values.selMode;
 
diff --git a/www/manager6/panel/BackupAdvancedOptions.js b/www/manager6/panel/BackupAdvancedOptions.js
index 1026c6f4..8860e42c 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -37,6 +37,10 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
 	    return {};
 	}
 
+	if (!formValues.id && me.isCreate) {
+	    formValues.id = 'backup-' + Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
+	}
+
 	let options = {};
 
 	if (!me.isCreate) {
@@ -105,6 +109,25 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
     },
 
     items: [
+	{
+	    xtype: 'pveTwoColumnContainer',
+	    startColumn: {
+		xtype: 'pmxDisplayEditField',
+		vtype: 'ConfigId',
+		fieldLabel: gettext('Job ID'),
+		emptyText: gettext('Autogenerate'),
+		name: 'id',
+		allowBlank: true,
+		cbind: {
+		    editable: '{isCreate}',
+		},
+	    },
+	    endFlex: 2,
+	    endColumn: {
+		xtype: 'displayfield',
+		value: gettext('Can be used in notification matchers to match this job.'),
+	    },
+	},
 	{
 	    xtype: 'pveTwoColumnContainer',
 	    startColumn: {
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 05/19] api: replication: add 'job-id' to notification metadata

[Lukas Wagner]

This allows users to create notification match rules for specific
replication jobs, if they so desire.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/Replication.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index d84ac1ab..6208a1a2 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -123,8 +123,8 @@ my sub _handle_job_err {
     };
 
     my $metadata_fields = {
-	# TODO: Add job-id?
 	type => "replication",
+	"job-id" => $job->{id},
     };
 
     eval {
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH manager v7 05/19] api: replication: add 'job-id' to notification metadata

[Fabian Grünbichler]

since this one is independent of the things with open questions.

Quoting Lukas Wagner (2024-06-10 10:40:24)
> This allows users to create notification match rules for specific
> replication jobs, if they so desire.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  PVE/API2/Replication.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
> index d84ac1ab..6208a1a2 100644
> --- a/PVE/API2/Replication.pm
> +++ b/PVE/API2/Replication.pm
> @@ -123,8 +123,8 @@ my sub _handle_job_err {
>      };
>  
>      my $metadata_fields = {
> -       # TODO: Add job-id?
>         type => "replication",
> +       "job-id" => $job->{id},
>      };
>  
>      eval {
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 06/19] vzdump: apt: notification: do not include domain in 'hostname' field

[Lukas Wagner]

 - The man page warns about the usage of `hostname -f`, since a host
   may have multiple domains (or none at all)
 - The fallback PVE::INotify::nodename() already only returned the
   hostname without the domain part
 - Fencing notifications didn't include the domain part anyway

This may result in soft-breakage for any users who have already relied
on the domain being present. If there is need for it, it could include
a fqdn metadata field.

The hostname property used for rendering the notification template
is unaffected for now.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/API2/APT.pm | 3 ++-
 PVE/VZDump.pm   | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index ec7c21b2..47c50961 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -348,7 +348,8 @@ __PACKAGE__->register_method({
 		# matchers.
 		my $metadata_fields = {
 		    type => 'package-updates',
-		    hostname => $hostname,
+		    # Hostname (without domain part)
+		    hostname => PVE::INotify::nodename(),
 		};
 
 		PVE::Notify::info(
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 2167b289..db3a02a9 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -517,10 +517,9 @@ sub send_notification {
 	    "See Task History for details!\n";
     };
 
-    my $hostname = get_hostname();
-
     my $notification_props = {
-	"hostname" => $hostname,
+	# Hostname, might include domain part
+	"hostname" => get_hostname(),
 	"error-message" => $err,
 	"guest-table" => build_guest_table($tasklist),
 	"logs" => $text_log_part,
@@ -531,7 +530,8 @@ sub send_notification {
 
     my $fields = {
 	type => "vzdump",
-	hostname => $hostname,
+	# Hostname (without domain part)
+	hostname => PVE::INotify::nodename(),
     };
     # Add backup-job metadata field in case this is a backup job.
     $fields->{'job-id'} = $job_id if $job_id;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH manager v7 06/19] vzdump: apt: notification: do not include domain in 'hostname' field

[Fabian Grünbichler]

independent as well..

Quoting Lukas Wagner (2024-06-10 10:40:25)
>  - The man page warns about the usage of `hostname -f`, since a host
>    may have multiple domains (or none at all)
>  - The fallback PVE::INotify::nodename() already only returned the
>    hostname without the domain part
>  - Fencing notifications didn't include the domain part anyway
> 
> This may result in soft-breakage for any users who have already relied
> on the domain being present. If there is need for it, it could include
> a fqdn metadata field.
> 
> The hostname property used for rendering the notification template
> is unaffected for now.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  PVE/API2/APT.pm | 3 ++-
>  PVE/VZDump.pm   | 8 ++++----
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
> index ec7c21b2..47c50961 100644
> --- a/PVE/API2/APT.pm
> +++ b/PVE/API2/APT.pm
> @@ -348,7 +348,8 @@ __PACKAGE__->register_method({
>                 # matchers.
>                 my $metadata_fields = {
>                     type => 'package-updates',
> -                   hostname => $hostname,
> +                   # Hostname (without domain part)
> +                   hostname => PVE::INotify::nodename(),
>                 };
>  
>                 PVE::Notify::info(
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 2167b289..db3a02a9 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -517,10 +517,9 @@ sub send_notification {
>             "See Task History for details!\n";
>      };
>  
> -    my $hostname = get_hostname();
> -
>      my $notification_props = {
> -       "hostname" => $hostname,
> +       # Hostname, might include domain part
> +       "hostname" => get_hostname(),
>         "error-message" => $err,
>         "guest-table" => build_guest_table($tasklist),
>         "logs" => $text_log_part,
> @@ -531,7 +530,8 @@ sub send_notification {
>  
>      my $fields = {
>         type => "vzdump",
> -       hostname => $hostname,
> +       # Hostname (without domain part)
> +       hostname => PVE::INotify::nodename(),
>      };
>      # Add backup-job metadata field in case this is a backup job.
>      $fields->{'job-id'} = $job_id if $job_id;
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 07/19] api: replication: include 'hostname' field for notifications

[Lukas Wagner]

The field contains the hostname of the host (without any domain part)
which sends the notification. This field can be used in match-field
match rules.

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

diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 6208a1a2..e4a7180f 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -125,6 +125,8 @@ my sub _handle_job_err {
     my $metadata_fields = {
 	type => "replication",
 	"job-id" => $job->{id},
+	# Hostname (without domain part)
+	hostname => PVE::INotify::nodename(),
     };
 
     eval {
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH manager v7 07/19] api: replication: include 'hostname' field for notifications

[Fabian Grünbichler]

and this one as well..

Quoting Lukas Wagner (2024-06-10 10:40:26)
> The field contains the hostname of the host (without any domain part)
> which sends the notification. This field can be used in match-field
> match rules.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  PVE/API2/Replication.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
> index 6208a1a2..e4a7180f 100644
> --- a/PVE/API2/Replication.pm
> +++ b/PVE/API2/Replication.pm
> @@ -125,6 +125,8 @@ my sub _handle_job_err {
>      my $metadata_fields = {
>         type => "replication",
>         "job-id" => $job->{id},
> +       # Hostname (without domain part)
> +       hostname => PVE::INotify::nodename(),
>      };
>  
>      eval {
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 08/19] api: notification: add API for getting known metadata fields/values

[Lukas Wagner]

This new API route returns known notification metadata fields and
a list of known possible values. This will be used by the UI to
provide suggestions when adding/modifying match rules.

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

diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..2b202c28 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,151 @@ __PACKAGE__->register_method ({
 	    { name => 'endpoints' },
 	    { name => 'matchers' },
 	    { name => 'targets' },
+	    { name => 'matcher-fields' },
+	    { name => 'matcher-field-values' },
 	];
 
 	return $result;
     }
 });
 
+__PACKAGE__->register_method ({
+    name => 'get_matcher_fields',
+    path => 'matcher-fields',
+    method => 'GET',
+    description => 'Returns known notification metadata fields',
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notifications', ['Mapping.Modify']],
+	    ['perm', '/mapping/notifications', ['Mapping.Audit']],
+	],
+    },
+    protected => 0,
+    parameters => {
+	additionalProperties => 0,
+	properties => {},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		name => {
+		    description => 'Name of the field.',
+		    type => 'string',
+		},
+	    },
+	},
+	links => [ { rel => 'child', href => '{name}' } ],
+    },
+    code => sub {
+	# TODO: Adapt this API handler once we have a 'notification registry'
+
+	my $result = [
+	    { name => 'type' },
+	    { name => 'hostname' },
+	    { name => 'job-id' },
+	];
+
+	return $result;
+    }
+});
+
+__PACKAGE__->register_method ({
+    name => 'get_matcher_field_values',
+    path => 'matcher-field-values',
+    method => 'GET',
+    description => 'Returns known notification metadata fields and their known values',
+    permissions => {
+	check => ['or',
+	    ['perm', '/mapping/notifications', ['Mapping.Modify']],
+	    ['perm', '/mapping/notifications', ['Mapping.Audit']],
+	],
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => 'object',
+	    properties => {
+		'value' => {
+		    description => 'Notification metadata value known by the system.',
+		    type => 'string'
+		},
+		'comment' => {
+		    description => 'Additional comment for this value.',
+		    type => 'string',
+		    optional => 1,
+		},
+		'field' => {
+		    description => 'Field this value belongs to.',
+		    type => 'string',
+		},
+	    },
+	},
+    },
+    code => sub {
+	# TODO: Adapt this API handler once we have a 'notification registry'
+	my $rpcenv = PVE::RPCEnvironment::get();
+	my $user = $rpcenv->get_user();
+
+	my $values = [
+	    {
+		value => 'package-updates',
+		field => 'type',
+	    },
+	    {
+		value => 'fencing',
+		field => 'type',
+	    },
+	    {
+		value => 'replication',
+		field => 'type',
+	    },
+	    {
+		value => 'vzdump',
+		field => 'type',
+	    },
+	    {
+		value => 'system-mail',
+		field => 'type',
+	    },
+	];
+
+	# Here we need a manual permission check.
+	if ($rpcenv->check($user, "/", ["Sys.Audit"], 1)) {
+	    for my $backup_job (@{PVE::API2::Backup->index({})}) {
+		push @$values, {
+		    value => $backup_job->{id},
+		    comment => $backup_job->{comment},
+		    field => 'job-id'
+		};
+	    }
+	}
+	# The API call returns only returns jobs for which the user
+	# has adequate permissions.
+	for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+	    push @$values, {
+		value => $sync_job->{id},
+		comment => $sync_job->{comment},
+		field => 'job-id'
+	    };
+	}
+
+	for my $node (@{PVE::Cluster::get_nodelist()}) {
+	    push @$values, {
+		value => $node,
+		field => 'hostname',
+	    }
+	}
+
+	return $values;
+    }
+});
+
 __PACKAGE__->register_method ({
     name => 'endpoints_index',
     path => 'endpoints',
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 09/19] ui: utils: add overrides for translatable notification fields/values

[Lukas Wagner]

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

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 5b0d51eb..ea448bfb 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2060,6 +2060,17 @@ Ext.define('PVE.Utils', {
 	    zfscreate: [gettext('ZFS Storage'), gettext('Create')],
 	    zfsremove: ['ZFS Pool', gettext('Remove')],
 	});
+
+	Proxmox.Utils.overrideNotificationFieldName({
+	    'job-id': gettext('Job ID'),
+	});
+
+	Proxmox.Utils.overrideNotificationFieldValue({
+	    'package-updates': gettext('Package updates are available'),
+	    'vzdump': gettext('Backup notifications'),
+	    'replication': gettext('Replication job notifications'),
+	    'fencing': gettext('Node fencing notifications'),
+	});
     },
 
 });
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v7 10/19] d/control: bump proxmox-widget-toolkit dependency to 4.1.4

[Lukas Wagner]

We need
  "utils: add mechanism to add and override translatable notification
  event descriptions in the product specific UIs"
otherwise there is an error in the browser console.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 debian/control | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index 4988a7d2..e400ddc5 100644
--- a/debian/control
+++ b/debian/control
@@ -20,7 +20,7 @@ Build-Depends: debhelper-compat (= 13),
                libtemplate-perl,
                libtest-mockmodule-perl,
                lintian,
-               proxmox-widget-toolkit (>= 4.0.7),
+               proxmox-widget-toolkit (>= 4.1.4),
                pve-cluster,
                pve-container,
                pve-doc-generator (>= 8.0.5),
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH widget-toolkit v7 11/19] notification: matcher: match-field: show known fields/values

[Lukas Wagner]

These changes introduce combogrid pickers for the 'field' and 'value'
form elements for 'match-field' match rules. The 'field' picker shows
a list of all known metadata fields, while the 'value' picker shows a
list of all known values, filtered depending on the current value of
'field'.

The list of known fields/values is retrieved from new API endpoints.
Some values are marked 'internal' by the backend. This means that the
'value' field was not user-created (counter example: backup job
IDs) and can therefore be used as a base for translations.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/data/model/NotificationConfig.js  |  12 ++
 src/window/NotificationMatcherEdit.js | 297 +++++++++++++++++++++-----
 2 files changed, 253 insertions(+), 56 deletions(-)

diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js
index e8ebf28..03cf317 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -15,3 +15,15 @@ Ext.define('proxmox-notification-matchers', {
     },
     idProperty: 'name',
 });
+
+Ext.define('proxmox-notification-fields', {
+    extend: 'Ext.data.Model',
+    fields: ['name', 'description'],
+    idProperty: 'name',
+});
+
+Ext.define('proxmox-notification-field-values', {
+    extend: 'Ext.data.Model',
+    fields: ['value', 'comment', 'field'],
+    idProperty: 'value',
+});
diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index e717ad7..be33efe 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -79,7 +79,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
 	labelWidth: 120,
     },
 
-    width: 700,
+    width: 800,
 
     initComponent: function() {
 	let me = this;
@@ -416,10 +416,22 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    let me = this;
 		    let record = me.get('selectedRecord');
 		    let currentData = record.get('data');
+
+		    let newValue = [];
+
+		    // Build equivalent regular expression if switching
+		    // to 'regex' mode
+		    if (value === 'regex') {
+			let regexVal = "^(";
+			regexVal += currentData.value.join('|') + ")$";
+			newValue.push(regexVal);
+		    }
+
 		    record.set({
 			data: {
 			    ...currentData,
 			    type: value,
+			    value: newValue,
 			},
 		    });
 		},
@@ -441,6 +453,8 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 			data: {
 			    ...currentData,
 			    field: value,
+			    // Reset value if field changes
+			    value: [],
 			},
 		    });
 		},
@@ -549,6 +563,9 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
     column2: [
 	{
 	    xtype: 'pmxNotificationMatchRuleSettings',
+	    cbind: {
+		baseUrl: '{baseUrl}',
+	    },
 	},
 
     ],
@@ -601,7 +618,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
 		let value = data.value;
 		text = Ext.String.format(gettext("Match field: {0}={1}"), field, value);
 		iconCls = 'fa fa-square-o';
-		if (!field || !value) {
+		if (!field || !value || (Ext.isArray(value) && !value.length)) {
 		    iconCls += ' internal-error';
 		}
 	    } break;
@@ -821,6 +838,11 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
 		if (type === undefined) {
 		    type = "exact";
 		}
+
+		if (type === 'exact') {
+		    matchedValue = matchedValue.split(',');
+		}
+
 		return {
 		    type: 'match-field',
 		    data: {
@@ -982,7 +1004,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
 Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
     extend: 'Ext.panel.Panel',
     xtype: 'pmxNotificationMatchRuleSettings',
+    mixins: ['Proxmox.Mixin.CBind'],
     border: false,
+    layout: 'anchor',
 
     items: [
 	{
@@ -1000,6 +1024,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 		['notall', gettext('At least one rule does not match')],
 		['notany', gettext('No rule matches')],
 	    ],
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		hidden: '{!showMatchingMode}',
 		disabled: '{!showMatchingMode}',
@@ -1011,7 +1037,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    fieldLabel: gettext('Node type'),
 	    isFormField: false,
 	    allowBlank: false,
-
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		value: '{nodeType}',
 		hidden: '{!showMatcherType}',
@@ -1025,57 +1052,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    ],
 	},
 	{
-	    fieldLabel: gettext('Match Type'),
-	    xtype: 'proxmoxKVComboBox',
-	    reference: 'type',
-	    isFormField: false,
-	    allowBlank: false,
-	    submitValue: false,
-	    field: 'type',
-
-	    bind: {
-		hidden: '{!typeIsMatchField}',
-		disabled: '{!typeIsMatchField}',
-		value: '{matchFieldType}',
-	    },
-
-	    comboItems: [
-		['exact', gettext('Exact')],
-		['regex', gettext('Regex')],
-	    ],
-	},
-	{
-	    fieldLabel: gettext('Field'),
-	    xtype: 'proxmoxKVComboBox',
-	    isFormField: false,
-	    submitValue: false,
-	    allowBlank: false,
-	    editable: true,
-	    displayField: 'key',
-	    field: 'field',
-	    bind: {
-		hidden: '{!typeIsMatchField}',
-		disabled: '{!typeIsMatchField}',
-		value: '{matchFieldField}',
-	    },
-	    // TODO: Once we have a 'notification registry', we should
-	    // retrive those via an API call.
-	    comboItems: [
-		['type', ''],
-		['hostname', ''],
-	    ],
-	},
-	{
-	    fieldLabel: gettext('Value'),
-	    xtype: 'textfield',
-	    isFormField: false,
-	    submitValue: false,
-	    allowBlank: false,
-	    field: 'value',
-	    bind: {
-		hidden: '{!typeIsMatchField}',
-		disabled: '{!typeIsMatchField}',
-		value: '{matchFieldValue}',
+	    xtype: 'pmxNotificationMatchFieldSettings',
+	    cbind: {
+		baseUrl: '{baseUrl}',
 	    },
 	},
 	{
@@ -1085,7 +1064,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    allowBlank: true,
 	    multiSelect: true,
 	    field: 'value',
-
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		value: '{matchSeverityValue}',
 		hidden: '{!typeIsMatchSeverity}',
@@ -1108,7 +1088,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    editable: true,
 	    displayField: 'key',
 	    field: 'value',
-
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
 	    bind: {
 		value: '{matchCalendarValue}',
 		hidden: '{!typeIsMatchCalendar}',
@@ -1122,3 +1103,207 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	},
     ],
 });
+
+Ext.define('Proxmox.panel.MatchFieldSettings', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pmxNotificationMatchFieldSettings',
+    border: false,
+    layout: 'anchor',
+    // Hide initially to avoid glitches when opening the window
+    hidden: true,
+    bind: {
+	hidden: '{!typeIsMatchField}',
+    },
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	control: {
+	    'field[reference=fieldSelector]': {
+		change: function(field) {
+		    let view = this.getView();
+		    let valueField = view.down('field[reference=valueSelector]');
+		    let store = valueField.getStore();
+		    let val = field.getValue();
+
+		    if (val) {
+			store.setFilters([
+			    {
+				property: 'field',
+				value: val,
+			    },
+			]);
+		    }
+		},
+	    },
+	},
+    },
+
+
+    initComponent: function() {
+	let me = this;
+
+	let store = Ext.create('Ext.data.Store', {
+	    model: 'proxmox-notification-fields',
+	    autoLoad: true,
+	    proxy: {
+		type: 'proxmox',
+		url: `/api2/json/${me.baseUrl}/matcher-fields`,
+	    },
+	    listeners: {
+		'load': function() {
+		    this.each(function(record) {
+			record.set({
+			    description:
+				Proxmox.Utils.formatNotificationFieldName(
+				    record.get('name'),
+				),
+			});
+		    });
+
+		    // Commit changes so that the description field is not marked
+		    // as dirty
+		    this.commitChanges();
+		},
+	    },
+	});
+
+	let valueStore = Ext.create('Ext.data.Store', {
+	    model: 'proxmox-notification-field-values',
+	    autoLoad: true,
+	    proxy: {
+		type: 'proxmox',
+
+		url: `/api2/json/${me.baseUrl}/matcher-field-values`,
+	    },
+	    listeners: {
+		'load': function() {
+		    this.each(function(record) {
+			if (record.get('field') === 'type') {
+			    record.set({
+				comment:
+				    Proxmox.Utils.formatNotificationFieldValue(
+					record.get('value'),
+				    ),
+			    });
+			}
+		    }, this, true);
+
+		    // Commit changes so that the description field is not marked
+		    // as dirty
+		    this.commitChanges();
+		},
+	    },
+	});
+
+	Ext.apply(me, {
+	    viewModel: Ext.create('Ext.app.ViewModel', {
+		parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+		formulas: {
+		    isRegex: function(get) {
+			return get('matchFieldType') === 'regex';
+		    },
+		},
+	    }),
+	    items: [
+		{
+		    fieldLabel: gettext('Match Type'),
+		    xtype: 'proxmoxKVComboBox',
+		    reference: 'type',
+		    isFormField: false,
+		    allowBlank: false,
+		    submitValue: false,
+		    field: 'type',
+
+		    bind: {
+			value: '{matchFieldType}',
+		    },
+
+		    comboItems: [
+			['exact', gettext('Exact')],
+			['regex', gettext('Regex')],
+		    ],
+		},
+		{
+		    fieldLabel: gettext('Field'),
+		    reference: 'fieldSelector',
+		    xtype: 'proxmoxComboGrid',
+		    isFormField: false,
+		    submitValue: false,
+		    allowBlank: false,
+		    editable: false,
+		    store: store,
+		    queryMode: 'local',
+		    valueField: 'name',
+		    displayField: 'description',
+		    field: 'field',
+		    bind: {
+			value: '{matchFieldField}',
+		    },
+		    listConfig: {
+			columns: [
+			    {
+				header: gettext('Description'),
+				dataIndex: 'description',
+				flex: 2,
+			    },
+			    {
+				header: gettext('Field Name'),
+				dataIndex: 'name',
+				flex: 1,
+			    },
+			],
+		    },
+		},
+		{
+		    fieldLabel: gettext('Value'),
+		    reference: 'valueSelector',
+		    xtype: 'proxmoxComboGrid',
+		    autoSelect: false,
+		    editable: false,
+		    isFormField: false,
+		    submitValue: false,
+		    allowBlank: false,
+		    showClearTrigger: true,
+		    field: 'value',
+		    store: valueStore,
+		    valueField: 'value',
+		    displayField: 'value',
+		    notFoundIsValid: false,
+		    multiSelect: true,
+		    bind: {
+			value: '{matchFieldValue}',
+			hidden: '{isRegex}',
+		    },
+		    listConfig: {
+			columns: [
+			    {
+				header: gettext('Value'),
+				dataIndex: 'value',
+				flex: 1,
+			    },
+			    {
+				header: gettext('Comment'),
+				dataIndex: 'comment',
+				flex: 2,
+			    },
+			],
+		    },
+		},
+		{
+		    fieldLabel: gettext('Regex'),
+		    xtype: 'proxmoxtextfield',
+		    editable: true,
+		    isFormField: false,
+		    submitValue: false,
+		    allowBlank: false,
+		    field: 'value',
+		    bind: {
+			value: '{matchFieldValue}',
+			hidden: '{!isRegex}',
+		    },
+		},
+	    ],
+	});
+	me.callParent();
+    },
+});
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH widget-toolkit v7 12/19] notification: matcher: move match-field formulas to local viewModel

[Lukas Wagner]

This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/window/NotificationMatcherEdit.js | 189 +++++++++++++-------------
 1 file changed, 95 insertions(+), 94 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index be33efe..559b405 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,15 +380,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		}
 		return !record.isRoot();
 	    },
-	    typeIsMatchField: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		get: function(record) {
-		    return record?.get('type') === 'match-field';
-		},
-	    },
 	    typeIsMatchSeverity: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -407,89 +398,13 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    return record?.get('type') === 'match-calendar';
 		},
 	    },
-	    matchFieldType: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-
-		    let newValue = [];
-
-		    // Build equivalent regular expression if switching
-		    // to 'regex' mode
-		    if (value === 'regex') {
-			let regexVal = "^(";
-			regexVal += currentData.value.join('|') + ")$";
-			newValue.push(regexVal);
-		    }
-
-		    record.set({
-			data: {
-			    ...currentData,
-			    type: value,
-			    value: newValue,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.type;
-		},
-	    },
-	    matchFieldField: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-
-		    record.set({
-			data: {
-			    ...currentData,
-			    field: value,
-			    // Reset value if field changes
-			    value: [],
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.field;
-		},
-	    },
-	    matchFieldValue: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-		    record.set({
-			data: {
-			    ...currentData,
-			    value: value,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.value;
-		},
-	    },
 	    matchSeverityValue: {
 		bind: {
 		    bindTo: '{selectedRecord}',
 		    deep: true,
 		},
 		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
+		    let record = this.get('selectedRecord');
 		    let currentData = record.get('data');
 		    record.set({
 			data: {
@@ -1137,7 +1052,98 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
 	    },
 	},
     },
+    viewModel: {
+	// parent is set in `initComponents`
+	formulas: {
+	    typeIsMatchField: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		get: function(record) {
+		    return record?.get('type') === 'match-field';
+		},
+	    },
+	    isRegex: function(get) {
+		return get('matchFieldType') === 'regex';
+	    },
+	    matchFieldType: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
+
+		    let newValue = [];
+
+		    // Build equivalent regular expression if switching
+		    // to 'regex' mode
+		    if (value === 'regex') {
+			let regexVal = "^";
+			if (currentData.value && currentData.value.length) {
+			    regexVal += `(${currentData.value.join('|')})`;
+			}
+			regexVal += "$";
+			newValue.push(regexVal);
+		    }
+
+		    record.set({
+			data: {
+			    ...currentData,
+			    type: value,
+			    value: newValue,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.type;
+		},
+	    },
+	    matchFieldField: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
 
+		    record.set({
+			data: {
+			    ...currentData,
+			    field: value,
+			    // Reset value if field changes
+			    value: [],
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.field;
+		},
+	    },
+	    matchFieldValue: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
+		    record.set({
+			data: {
+			    ...currentData,
+			    value: value,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.value;
+		},
+	    },
+	},
+    },
 
     initComponent: function() {
 	let me = this;
@@ -1195,15 +1201,10 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
 	    },
 	});
 
+	Ext.apply(me.viewModel, {
+	    parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+	});
 	Ext.apply(me, {
-	    viewModel: Ext.create('Ext.app.ViewModel', {
-		parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
-		formulas: {
-		    isRegex: function(get) {
-			return get('matchFieldType') === 'regex';
-		    },
-		},
-	    }),
 	    items: [
 		{
 		    fieldLabel: gettext('Match Type'),
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH widget-toolkit v7 13/19] notification: matcher: move match-calendar fields to panel

[Lukas Wagner]

Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/window/NotificationMatcherEdit.js | 92 +++++++++++++++++----------
 1 file changed, 60 insertions(+), 32 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index 559b405..50145e3 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -389,15 +389,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    return record?.get('type') === 'match-severity';
 		},
 	    },
-	    typeIsMatchCalendar: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		get: function(record) {
-		    return record?.get('type') === 'match-calendar';
-		},
-	    },
 	    matchSeverityValue: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -417,26 +408,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		    return record?.get('data')?.value;
 		},
 	    },
-	    matchCalendarValue: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let me = this;
-		    let record = me.get('selectedRecord');
-		    let currentData = record.get('data');
-		    record.set({
-			data: {
-			    ...currentData,
-			    value: value,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.value;
-		},
-	    },
 	    rootMode: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -995,6 +966,58 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 		['unknown', gettext('Unknown')],
 	    ],
 	},
+	{
+	    xtype: 'pmxNotificationMatchCalendarSettings',
+	},
+    ],
+});
+
+Ext.define('Proxmox.panel.MatchCalendarSettings', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pmxNotificationMatchCalendarSettings',
+    border: false,
+    layout: 'anchor',
+    // Hide initially to avoid glitches when opening the window
+    hidden: true,
+    bind: {
+	hidden: '{!typeIsMatchCalendar}',
+    },
+    viewModel: {
+	// parent is set in `initComponents`
+	formulas: {
+	    typeIsMatchCalendar: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		get: function(record) {
+		    return record?.get('type') === 'match-calendar';
+		},
+	    },
+
+	    matchCalendarValue: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let me = this;
+		    let record = me.get('selectedRecord');
+		    let currentData = record.get('data');
+		    record.set({
+			data: {
+			    ...currentData,
+			    value: value,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.value;
+		},
+	    },
+	},
+    },
+    items: [
 	{
 	    xtype: 'proxmoxKVComboBox',
 	    fieldLabel: gettext('Timespan to match'),
@@ -1003,11 +1026,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    editable: true,
 	    displayField: 'key',
 	    field: 'value',
-	    // Hide initially to avoid glitches when opening the window
-	    hidden: true,
 	    bind: {
 		value: '{matchCalendarValue}',
-		hidden: '{!typeIsMatchCalendar}',
 		disabled: '{!typeIsMatchCalender}',
 	    },
 
@@ -1017,6 +1037,14 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    ],
 	},
     ],
+
+    initComponent: function() {
+	let me = this;
+	Ext.apply(me.viewModel, {
+	    parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+	});
+	me.callParent();
+    },
 });
 
 Ext.define('Proxmox.panel.MatchFieldSettings', {
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH widget-toolkit v7 14/19] notification: matcher: move match-severity fields to panel

[Lukas Wagner]

Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/window/NotificationMatcherEdit.js | 129 ++++++++++++++++----------
 1 file changed, 80 insertions(+), 49 deletions(-)

diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index 50145e3..9ab443f 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,34 +380,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
 		}
 		return !record.isRoot();
 	    },
-	    typeIsMatchSeverity: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		get: function(record) {
-		    return record?.get('type') === 'match-severity';
-		},
-	    },
-	    matchSeverityValue: {
-		bind: {
-		    bindTo: '{selectedRecord}',
-		    deep: true,
-		},
-		set: function(value) {
-		    let record = this.get('selectedRecord');
-		    let currentData = record.get('data');
-		    record.set({
-			data: {
-			    ...currentData,
-			    value: value,
-			},
-		    });
-		},
-		get: function(record) {
-		    return record?.get('data')?.value;
-		},
-	    },
+
 	    rootMode: {
 		bind: {
 		    bindTo: '{selectedRecord}',
@@ -944,27 +917,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
 	    },
 	},
 	{
-	    xtype: 'proxmoxKVComboBox',
-	    fieldLabel: gettext('Severities to match'),
-	    isFormField: false,
-	    allowBlank: true,
-	    multiSelect: true,
-	    field: 'value',
-	    // Hide initially to avoid glitches when opening the window
-	    hidden: true,
-	    bind: {
-		value: '{matchSeverityValue}',
-		hidden: '{!typeIsMatchSeverity}',
-		disabled: '{!typeIsMatchSeverity}',
-	    },
-
-	    comboItems: [
-		['info', gettext('Info')],
-		['notice', gettext('Notice')],
-		['warning', gettext('Warning')],
-		['error', gettext('Error')],
-		['unknown', gettext('Unknown')],
-	    ],
+	    xtype: 'pmxNotificationMatchSeveritySettings',
 	},
 	{
 	    xtype: 'pmxNotificationMatchCalendarSettings',
@@ -1047,6 +1000,84 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
     },
 });
 
+Ext.define('Proxmox.panel.MatchSeveritySettings', {
+    extend: 'Ext.panel.Panel',
+    xtype: 'pmxNotificationMatchSeveritySettings',
+    border: false,
+    layout: 'anchor',
+    // Hide initially to avoid glitches when opening the window
+    hidden: true,
+    bind: {
+	hidden: '{!typeIsMatchSeverity}',
+    },
+    viewModel: {
+	// parent is set in `initComponents`
+	formulas: {
+	    typeIsMatchSeverity: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		get: function(record) {
+		    return record?.get('type') === 'match-severity';
+		},
+	    },
+	    matchSeverityValue: {
+		bind: {
+		    bindTo: '{selectedRecord}',
+		    deep: true,
+		},
+		set: function(value) {
+		    let record = this.get('selectedRecord');
+		    let currentData = record.get('data');
+		    record.set({
+			data: {
+			    ...currentData,
+			    value: value,
+			},
+		    });
+		},
+		get: function(record) {
+		    return record?.get('data')?.value;
+		},
+	    },
+	},
+    },
+    items: [
+	{
+	    xtype: 'proxmoxKVComboBox',
+	    fieldLabel: gettext('Severities to match'),
+	    isFormField: false,
+	    allowBlank: true,
+	    multiSelect: true,
+	    field: 'value',
+	    // Hide initially to avoid glitches when opening the window
+	    hidden: true,
+	    bind: {
+		value: '{matchSeverityValue}',
+		hidden: '{!typeIsMatchSeverity}',
+		disabled: '{!typeIsMatchSeverity}',
+	    },
+
+	    comboItems: [
+		['info', gettext('Info')],
+		['notice', gettext('Notice')],
+		['warning', gettext('Warning')],
+		['error', gettext('Error')],
+		['unknown', gettext('Unknown')],
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+	let me = this;
+	Ext.apply(me.viewModel, {
+	    parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+	});
+	me.callParent();
+    },
+});
+
 Ext.define('Proxmox.panel.MatchFieldSettings', {
     extend: 'Ext.panel.Panel',
     xtype: 'pmxNotificationMatchFieldSettings',
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs v7 15/19] notification: clarify that 'hostname' does not include the domain

[Lukas Wagner]

This was a bit inconsistent between the different notification types:
  - APT/VZDump included the domain part
  - fence notifications did not

A decision has been made to unify this by removing the domain part
from APT/VZDump notifications.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/notifications.adoc b/notifications.adoc
index 46aff6a..57053c8 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -301,7 +301,7 @@ Notification Events
 |=======================================================================
 | Field name | Description
 | `type`     | Type of the notifcation
-| `hostname` | Hostname, including domain (e.g. `pve1.example.com`)
+| `hostname` | Hostname, without domain (e.g. `pve1`)
 |=======================================================================
 
 System Mail Forwarding
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH docs v7 15/19] notification: clarify that 'hostname' does not include the domain

[Fabian Grünbichler]

Quoting Lukas Wagner (2024-06-10 10:40:34)
> This was a bit inconsistent between the different notification types:
>   - APT/VZDump included the domain part
>   - fence notifications did not
> 
> A decision has been made to unify this by removing the domain part
> from APT/VZDump notifications.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  notifications.adoc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/notifications.adoc b/notifications.adoc
> index 46aff6a..57053c8 100644
> --- a/notifications.adoc
> +++ b/notifications.adoc
> @@ -301,7 +301,7 @@ Notification Events
>  |=======================================================================
>  | Field name | Description
>  | `type`     | Type of the notifcation
> -| `hostname` | Hostname, including domain (e.g. `pve1.example.com`)
> +| `hostname` | Hostname, without domain (e.g. `pve1`)
>  |=======================================================================
>  
>  System Mail Forwarding
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs v7 16/19] notifications: describe new notification metadata fields

[Lukas Wagner]

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

diff --git a/notifications.adoc b/notifications.adoc
index 57053c8..dec878a 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -289,19 +289,21 @@ Notification Events
 
 [width="100%",options="header"]
 |===========================================================================
-| Event                        | `type`            | Severity | Metadata fields (in addition to `type`)
-| System updates available     |`package-updates`  | `info`   | `hostname`
-| Cluster node fenced          |`fencing`          | `error`  | `hostname`
-| Storage replication failed   |`replication`      | `error`  | -
-| Backup finished              |`vzdump`           | `info` (`error` on failure) | `hostname`
-| Mail for root                |`system-mail`      | `unknown`| -
+| Event                            | `type`            | Severity | Metadata fields (in addition to `type`)
+| System updates available         |`package-updates`  | `info`   | `hostname`
+| Cluster node fenced              |`fencing`          | `error`  | `hostname`
+| Storage replication job failed   |`replication`      | `error`  | `hostname`, `job-id`
+| Backup succeeded                 |`vzdump`           | `info`   | `hostname`, `job-id` (only for backup jobs)
+| Backup failed                    |`vzdump`           | `error`  | `hostname`, `job-id` (only for backup jobs)
+| Mail for root                    |`system-mail`      | `unknown`| `hostname`
 |===========================================================================
 
 [width="100%",options="header"]
 |=======================================================================
-| Field name | Description
-| `type`     | Type of the notifcation
-| `hostname` | Hostname, without domain (e.g. `pve1`)
+| Field name        | Description
+| `type`            | Type of the notifcation
+| `hostname`        | Hostname, without domain (e.g. `pve1`)
+| `job-id`          | Job ID
 |=======================================================================
 
 System Mail Forwarding
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs v7 17/19] notifications: match-field 'exact'-mode can now match multiple values

[Lukas Wagner]

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

diff --git a/notifications.adoc b/notifications.adoc
index dec878a..07f0b3e 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -221,11 +221,16 @@ configurable schedule.
 Field Matching Rules
 ~~~~~~~~~~~~~~~~~~~~
 Notifications have a selection of metadata fields that can be matched.
+When using `exact` as a matching mode, a `,` can be used as a separator.
+The matching rule then matches if the metadata field has *any* of the specified
+values.
 
 * `match-field exact:type=vzdump` Only match notifications about backups.
+* `match-field exact:type=replication,fencing` Match `replication` and `fencing` notifications.
 * `match-field regex:hostname=^.+\.example\.com$` Match the hostname of
 the node.
 
+
 If a matched metadata field does not exist, the notification will not be
 matched.
 For instance, a `match-field regex:hostname=.*` directive will only match
@@ -267,18 +272,7 @@ matcher: backup-failures
         comment Send notifications about backup failures to one group of admins
 
 matcher: cluster-failures
-        match-field exact:type=replication
-        match-field exact:type=fencing
-        mode any
-        target cluster-admins
-        comment Send cluster-related notifications to other group of admins
-----
-
-The last matcher could also be rewritten using a field matcher with a regular
-expression:
-----
-matcher: cluster-failures
-        match-field regex:type=^(replication|fencing)$
+        match-field exact:type=replication,fencing
         target cluster-admins
         comment Send cluster-related notifications to other group of admins
 ----
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs v7 18/19] notifications: fix typo in 'notification'

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 notifications.adoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/notifications.adoc b/notifications.adoc
index 07f0b3e..9c5228c 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -295,7 +295,7 @@ Notification Events
 [width="100%",options="header"]
 |=======================================================================
 | Field name        | Description
-| `type`            | Type of the notifcation
+| `type`            | Type of the notification
 | `hostname`        | Hostname, without domain (e.g. `pve1`)
 | `job-id`          | Job ID
 |=======================================================================
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH docs v7 18/19] notifications: fix typo in 'notification'

[Fabian Grünbichler]

with some context adaptations

Quoting Lukas Wagner (2024-06-10 10:40:37)
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  notifications.adoc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/notifications.adoc b/notifications.adoc
> index 07f0b3e..9c5228c 100644
> --- a/notifications.adoc
> +++ b/notifications.adoc
> @@ -295,7 +295,7 @@ Notification Events
>  [width="100%",options="header"]
>  |=======================================================================
>  | Field name        | Description
> -| `type`            | Type of the notifcation
> +| `type`            | Type of the notification
>  | `hostname`        | Hostname, without domain (e.g. `pve1`)
>  | `job-id`          | Job ID
>  |=======================================================================
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH docs v7 19/19] notifications: backport some rephrased parts from PBS docs

[Lukas Wagner]

Most of the changes were done when adapting the PVE docs to
the new PBS notification system, so now we 'backport' those
improvements.

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

diff --git a/notifications.adoc b/notifications.adoc
index 9c5228c..bdfebd0 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -9,37 +9,38 @@ Overview
 --------
 [thumbnail="screenshot/gui-datacenter-notification-overview.png"]
 
-{pve} will send notifications if case of noteworthy events in the system.
-
-There are a number of different xref:notification_events[notification events],
-each with their own set of metadata fields that can be used in
-notification matchers.
-
-A xref:notification_matchers[notification matcher] determines
-_which_ notifications shall be sent _where_.
-A matcher has _match rules_, that can be used to
-match on certain notification properties (e.g. timestamp, severity,
-metadata fields).
-If a matcher matches a notification, the notification will be routed
-to a configurable set of notification targets.
-
-A xref:notification_targets[notification target] is an abstraction for a
-destination where a notification should be sent to - for instance,
-a Gotify server instance, or a set of email addresses.
-There are multiple types of notification targets, including
-`sendmail`, which uses the system's sendmail command to send emails,
-or `gotify`, which sends a notification to a Gotify instance.
+* {pve} emits xref:notification_events[Notification Events] in case of
+  storage replication failures, node fencing, finished/failed backups
+  and other events.
+  These events are handled by the notification system. A notification
+  event has metadata, for example a timestamp, a severity level,
+  a type, and other optional metadata fields.
+* xref:notification_matchers[Notification Matchers] route a notification
+  event to one or more notification targets. A matcher can have match
+  rules to selectively route based on the metadata of a notification event.
+* xref:notification_targets[Notification Targets] are a destination to
+  which a notification event is routed to by a matcher.
+  There are multiple types of target, mail-based (Sendmail and SMTP)
+  and Gotify.
+
+Backup jobs have a configurable xref:notification_mode[Notification Mode].
+It allows you to choose between the notification system and a legacy mode
+for sending notification emails. The legacy mode is equivalent to the
+way notifications were handled before {pve} 8.1.
 
 The notification system can be configured in the GUI under
-Datacenter -> Notifications. The configuration is stored in
+Datacenter → Notifications. The configuration is stored in
 `/etc/pve/notifications.cfg` and `/etc/pve/priv/notifications.cfg` -
 the latter contains sensitive configuration options such as
-passwords or authentication tokens for notification targets.
+passwords or authentication tokens for notification targets and can
+only be read by `root`.
 
 [[notification_targets]]
 Notification Targets
 --------------------
 
+{pve} offers multiple types of notification targets.
+
 [[notification_targets_sendmail]]
 Sendmail
 ~~~~~~~~
@@ -50,14 +51,19 @@ 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.
-
+The sendmail notification target uses the `sendmail` binary to send emails to a
+list of configured users or email addresses. If a user is selected as a recipient,
+the email address configured in user's settings will be used.
+For the `root@pam` user, this is the email address entered during installation.
+A user's email address can be configured in
+`Datacenter → Permissions → Users`.
+If a user has no associated email address, no email will be sent.
 
 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.
+Postfix. It may be necessary to configure Postfix so that it can deliver
+mails correctly - for example by setting an external mail relay (smart host).
+In case of failed delivery, check the system logs for messages logged by
+the Postfix daemon.
 
 The configuration for Sendmail target plugins has the following options:
 
@@ -90,6 +96,12 @@ SMTP
 [thumbnail="screenshot/gui-datacenter-notification-smtp.png"]
 
 SMTP notification targets can send emails directly to an SMTP mail relay.
+This target does not use the system's MTA to deliver emails.
+Similar to sendmail targets, if a user is selected as a recipient, the user's configured
+email address will be used.
+
+NOTE: Unlike sendmail targets, SMTP targets do not have any queuing/retry mechanism
+in case of a failed mail delivery.
 
 The configuration for SMTP target plugins has the following options:
 
@@ -308,9 +320,8 @@ that are initially directed to the local `root` user. {pve} will
 feed these mails into the notification system as a notification of
 type `system-mail` and with severity `unknown`.
 
-When the forwarding process involves an email-based target
-(like `sendmail` or `smtp`), the email is forwarded exactly as received, with all
-original mail headers remaining intact. For all other targets,
+When the email is forwarded to a sendmail target, the mail's content and headers
+are forwarded as-is. For all other targets,
 the system tries to extract both a subject line and the main text body
 from the email content. In instances where emails solely consist of HTML
 content, they will be transformed into plain text format during this process.
@@ -318,9 +329,33 @@ content, they will be transformed into plain text format during this process.
 Permissions
 -----------
 
-In order to modify/view the configuration for notification targets,
+To modify/view the configuration for notification targets,
 the `Mapping.Modify/Mapping.Audit` permissions are required for the
 `/mapping/notifications` ACL node.
 
 Testing a target requires `Mapping.Use`, `Mapping.Audit` or `Mapping.Modify`
 permissions on `/mapping/notifications`
+
+[[notification_mode]]
+Notification Mode
+-----------------
+A backup job configuration has the `notification-mode`
+option which can have one of three values.
+
+* `auto`: Use the `legacy-sendmail` mode if no email address is entered in the
+  `mailto`/`Send email to` field. If no email address is entered,
+  the `notification-system` mode is used.
+
+* `legacy-sendmail`: Send notification emails via the system's `sendmail` command.
+  The notification system will be bypassed and any configured
+  targets/matchers will be ignored.
+  This mode is equivalent to the notification behavior for version before
+  {pve} 8.1 .
+
+* `notification-system`: Use the new, flexible notification system.
+
+If the `notification-mode` option is not set, {pve} will default
+to `auto`.
+
+The `legacy-sendmail` mode might be removed in a later release of
+{pve}.
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied: [PATCH docs v7 19/19] notifications: backport some rephrased parts from PBS docs

[Fabian Grünbichler]

Quoting Lukas Wagner (2024-06-10 10:40:38)
> Most of the changes were done when adapting the PVE docs to
> the new PBS notification system, so now we 'backport' those
> improvements.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  notifications.adoc | 99 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 67 insertions(+), 32 deletions(-)
> 
> diff --git a/notifications.adoc b/notifications.adoc
> index 9c5228c..bdfebd0 100644
> --- a/notifications.adoc
> +++ b/notifications.adoc
> @@ -9,37 +9,38 @@ Overview
>  --------
>  [thumbnail="screenshot/gui-datacenter-notification-overview.png"]
>  
> -{pve} will send notifications if case of noteworthy events in the system.
> -
> -There are a number of different xref:notification_events[notification events],
> -each with their own set of metadata fields that can be used in
> -notification matchers.
> -
> -A xref:notification_matchers[notification matcher] determines
> -_which_ notifications shall be sent _where_.
> -A matcher has _match rules_, that can be used to
> -match on certain notification properties (e.g. timestamp, severity,
> -metadata fields).
> -If a matcher matches a notification, the notification will be routed
> -to a configurable set of notification targets.
> -
> -A xref:notification_targets[notification target] is an abstraction for a
> -destination where a notification should be sent to - for instance,
> -a Gotify server instance, or a set of email addresses.
> -There are multiple types of notification targets, including
> -`sendmail`, which uses the system's sendmail command to send emails,
> -or `gotify`, which sends a notification to a Gotify instance.
> +* {pve} emits xref:notification_events[Notification Events] in case of
> +  storage replication failures, node fencing, finished/failed backups
> +  and other events.
> +  These events are handled by the notification system. A notification
> +  event has metadata, for example a timestamp, a severity level,
> +  a type, and other optional metadata fields.
> +* xref:notification_matchers[Notification Matchers] route a notification
> +  event to one or more notification targets. A matcher can have match
> +  rules to selectively route based on the metadata of a notification event.
> +* xref:notification_targets[Notification Targets] are a destination to
> +  which a notification event is routed to by a matcher.
> +  There are multiple types of target, mail-based (Sendmail and SMTP)
> +  and Gotify.
> +
> +Backup jobs have a configurable xref:notification_mode[Notification Mode].
> +It allows you to choose between the notification system and a legacy mode
> +for sending notification emails. The legacy mode is equivalent to the
> +way notifications were handled before {pve} 8.1.
>  
>  The notification system can be configured in the GUI under
> -Datacenter -> Notifications. The configuration is stored in
> +Datacenter → Notifications. The configuration is stored in
>  `/etc/pve/notifications.cfg` and `/etc/pve/priv/notifications.cfg` -
>  the latter contains sensitive configuration options such as
> -passwords or authentication tokens for notification targets.
> +passwords or authentication tokens for notification targets and can
> +only be read by `root`.
>  
>  [[notification_targets]]
>  Notification Targets
>  --------------------
>  
> +{pve} offers multiple types of notification targets.
> +
>  [[notification_targets_sendmail]]
>  Sendmail
>  ~~~~~~~~
> @@ -50,14 +51,19 @@ 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.
> -
> +The sendmail notification target uses the `sendmail` binary to send emails to a
> +list of configured users or email addresses. If a user is selected as a recipient,
> +the email address configured in user's settings will be used.
> +For the `root@pam` user, this is the email address entered during installation.
> +A user's email address can be configured in
> +`Datacenter → Permissions → Users`.
> +If a user has no associated email address, no email will be sent.
>  
>  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.
> +Postfix. It may be necessary to configure Postfix so that it can deliver
> +mails correctly - for example by setting an external mail relay (smart host).
> +In case of failed delivery, check the system logs for messages logged by
> +the Postfix daemon.
>  
>  The configuration for Sendmail target plugins has the following options:
>  
> @@ -90,6 +96,12 @@ SMTP
>  [thumbnail="screenshot/gui-datacenter-notification-smtp.png"]
>  
>  SMTP notification targets can send emails directly to an SMTP mail relay.
> +This target does not use the system's MTA to deliver emails.
> +Similar to sendmail targets, if a user is selected as a recipient, the user's configured
> +email address will be used.
> +
> +NOTE: Unlike sendmail targets, SMTP targets do not have any queuing/retry mechanism
> +in case of a failed mail delivery.
>  
>  The configuration for SMTP target plugins has the following options:
>  
> @@ -308,9 +320,8 @@ that are initially directed to the local `root` user. {pve} will
>  feed these mails into the notification system as a notification of
>  type `system-mail` and with severity `unknown`.
>  
> -When the forwarding process involves an email-based target
> -(like `sendmail` or `smtp`), the email is forwarded exactly as received, with all
> -original mail headers remaining intact. For all other targets,
> +When the email is forwarded to a sendmail target, the mail's content and headers
> +are forwarded as-is. For all other targets,
>  the system tries to extract both a subject line and the main text body
>  from the email content. In instances where emails solely consist of HTML
>  content, they will be transformed into plain text format during this process.
> @@ -318,9 +329,33 @@ content, they will be transformed into plain text format during this process.
>  Permissions
>  -----------
>  
> -In order to modify/view the configuration for notification targets,
> +To modify/view the configuration for notification targets,
>  the `Mapping.Modify/Mapping.Audit` permissions are required for the
>  `/mapping/notifications` ACL node.
>  
>  Testing a target requires `Mapping.Use`, `Mapping.Audit` or `Mapping.Modify`
>  permissions on `/mapping/notifications`
> +
> +[[notification_mode]]
> +Notification Mode
> +-----------------
> +A backup job configuration has the `notification-mode`
> +option which can have one of three values.
> +
> +* `auto`: Use the `legacy-sendmail` mode if no email address is entered in the
> +  `mailto`/`Send email to` field. If no email address is entered,
> +  the `notification-system` mode is used.
> +
> +* `legacy-sendmail`: Send notification emails via the system's `sendmail` command.
> +  The notification system will be bypassed and any configured
> +  targets/matchers will be ignored.
> +  This mode is equivalent to the notification behavior for version before
> +  {pve} 8.1 .
> +
> +* `notification-system`: Use the new, flexible notification system.
> +
> +If the `notification-mode` option is not set, {pve} will default
> +to `auto`.
> +
> +The `legacy-sendmail` mode might be removed in a later release of
> +{pve}.
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements

[Fabian Grünbichler]

Quoting Lukas Wagner (2024-06-10 10:40:19)
> This patch series attempts to improve the user experience when creating
> notification matchers.
> 
> Some of the noteworthy changes:
>   - Fixup inconsistent 'hostname' field. Some notification events sent
>   the hostname including a domain, while other did not.
>   This series unifies the behavior, now the field only includes the hostname
>   without a domain. Also updated the docs to reflect this change.
>   - Allow setting a custom backup job ID, similar how we handle it for
>   sync/prune jobs in PBS (to allow recognizable names used in matchers)
>   - Adding columns for backup job ID/replication job ID in the UI
>   - New metadata fields:
>     - job-id: Job ID for backup-jobs or replication-jobs
>   - Add an API that enumerates known notification metadata fields/values
>   - Suggest known fields/values in match rule window
>   - Some code clean up for match rule edit window
>   - Extended the 'exact' match-field mode - it now allows setting multiple
>     allowed values, separated via ',':
>       e.g. `match-field exact:type=replication,fencing
>     Originally, I created a separate 'list' match type for this, but
>     since the semantics for a list with one value and 'exact' mode
>     are identical, I decided to just extend 'exact'.
>     This is a safe change since there are are no values where a ','
>     makes any sense (config IDs, hostnames)
> 
> NOTE: Might need a versionened break, since the widget-toolkit-patches
> depend on new APIs provided by pve-manager. If the API is not present,
> creating matchers with 'match-field' does not work (cannot load lists
> of known values/fields)
> 
> Inter-Dependencies:
>   - the widget-toolkit dep in pve-manager needs to 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", otherwise the UI breaks
>     with the pve-manager patches applied) --> already included a patch for
>     this
>   - widget-toolkit relies on a new API endpoint provided by pve-manager:
>     --> we require a versioned break in widget-toolkit on pve-manager

pve-guest-common is also needed by pve-manager AFAICT?

 and manual invocations of backup jobs are broken in a cluster if the target
node is not yet upgraded, since that would set the still unknown job-id
parameter.. combined with the "job-id value can't be trusted" aspect, it might
be better to skip setting it for manual invocations?

> 
> Changelog:
>   - v7: incorporated some more feedback from @Fiona, thx!
>     - Fixed error when switching from 'exact' to 'regex' if the text field
>       was empty
>     - rebased to latest master
>     - 'backport' doc improvements from PBS
>     - bumped widget-toolkit dep
>   - v6: incorporate feedback from @Fiona, thx!
>     - rename 'id' -> 'job-id' in VZDump API handler
>     - consolidate 'replication-job'/'backup-job' to 'job-id'
>     - Move 'job-id' setting to advanced tab in backup job edit.
>     - Don't use 'internal' flag to mark translatable fields, since
>       the only field where that's necessary is 'type' for now - so
>       just add a hardcoded check
>   - v5:
>     - Rebased onto latest master, resolving some small conflict
>   - v4:
>     - widget-toolkit: break out changes for the utils module so that they
>       can be applied ahead of time to ease dep bumping
>     - don't show Job IDs in the backup/replication job columns
>   - v3:
>     - Drop already applied patches for `proxmox`
>     - Rebase onto latest master - minor conflict resolution was needed
>   - v2:
>     - include 'type' metadata field for forwarded mails
>       --> otherwise it's not possible to match them
>     - include Maximilliano's T-b trailer in UI patches
> 
> pve-guest-common:
> 
> Lukas Wagner (1):
>   vzdump: common: allow 'job-id' as a parameter without being in schema
> 
>  src/PVE/VZDump/Common.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> pve-manager:
> 
> Lukas Wagner (9):
>   api: jobs: vzdump: pass job 'job-id' parameter
>   ui: dc: backup: send 'job-id' property when starting a backup job
>     manually
>   ui: dc: backup: allow to set custom job id in  advanced settings
>   api: replication: add 'job-id' to notification metadata
>   vzdump: apt: notification: do not include domain in 'hostname' field
>   api: replication: include 'hostname' field for notifications
>   api: notification: add API for getting known metadata fields/values
>   ui: utils: add overrides for translatable notification fields/values
>   d/control: bump proxmox-widget-toolkit dependency to 4.1.4
> 
>  PVE/API2/APT.pm                             |   3 +-
>  PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
>  PVE/API2/Replication.pm                     |   4 +-
>  PVE/API2/VZDump.pm                          |   8 ++
>  PVE/Jobs/VZDump.pm                          |   4 +-
>  PVE/VZDump.pm                               |  14 +-
>  debian/control                              |   2 +-
>  www/manager6/Utils.js                       |  12 ++
>  www/manager6/dc/Backup.js                   |   7 +-
>  www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
>  10 files changed, 200 insertions(+), 16 deletions(-)
> 
> 
> proxmox-widget-toolkit:
> 
> Lukas Wagner (4):
>   notification: matcher: match-field: show known fields/values
>   notification: matcher: move match-field formulas to local viewModel
>   notification: matcher: move match-calendar fields to panel
>   notification: matcher: move match-severity fields to panel
> 
>  src/data/model/NotificationConfig.js  |  12 +
>  src/window/NotificationMatcherEdit.js | 613 ++++++++++++++++++--------
>  2 files changed, 441 insertions(+), 184 deletions(-)
> 
> 
> pve-docs:
> 
> Lukas Wagner (5):
>   notification: clarify that 'hostname' does not include the domain
>   notifications: describe new notification metadata fields
>   notifications: match-field 'exact'-mode can now match multiple values
>   notifications: fix typo in 'notification'
>   notifications: backport some rephrased parts from PBS docs
> 
>  notifications.adoc | 137 +++++++++++++++++++++++++++------------------
>  1 file changed, 84 insertions(+), 53 deletions(-)
> 
> 
> Summary over all repositories:
>   14 files changed, 726 insertions(+), 254 deletions(-)
> 
> -- 
> Generated by git-murpp 0.7.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements

[Lukas Wagner]

On  2024-07-04 14:56, Fabian Grünbichler wrote:
> Quoting Lukas Wagner (2024-06-10 10:40:19)
>> This patch series attempts to improve the user experience when creating
>> notification matchers.
>>
>> Some of the noteworthy changes:
>>   - Fixup inconsistent 'hostname' field. Some notification events sent
>>   the hostname including a domain, while other did not.
>>   This series unifies the behavior, now the field only includes the hostname
>>   without a domain. Also updated the docs to reflect this change.
>>   - Allow setting a custom backup job ID, similar how we handle it for
>>   sync/prune jobs in PBS (to allow recognizable names used in matchers)
>>   - Adding columns for backup job ID/replication job ID in the UI
>>   - New metadata fields:
>>     - job-id: Job ID for backup-jobs or replication-jobs
>>   - Add an API that enumerates known notification metadata fields/values
>>   - Suggest known fields/values in match rule window
>>   - Some code clean up for match rule edit window
>>   - Extended the 'exact' match-field mode - it now allows setting multiple
>>     allowed values, separated via ',':
>>       e.g. `match-field exact:type=replication,fencing
>>     Originally, I created a separate 'list' match type for this, but
>>     since the semantics for a list with one value and 'exact' mode
>>     are identical, I decided to just extend 'exact'.
>>     This is a safe change since there are are no values where a ','
>>     makes any sense (config IDs, hostnames)
>>
>> NOTE: Might need a versionened break, since the widget-toolkit-patches
>> depend on new APIs provided by pve-manager. If the API is not present,
>> creating matchers with 'match-field' does not work (cannot load lists
>> of known values/fields)
>>
>> Inter-Dependencies:
>>   - the widget-toolkit dep in pve-manager needs to 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", otherwise the UI breaks
>>     with the pve-manager patches applied) --> already included a patch for
>>     this
>>   - widget-toolkit relies on a new API endpoint provided by pve-manager:
>>     --> we require a versioned break in widget-toolkit on pve-manager
> 
> pve-guest-common is also needed by pve-manager AFAICT?

Oh yes, of course. Always a bit hard to keep track of everything in
large patch series' like this one ;)

>  and manual invocations of backup jobs are broken in a cluster if the target
> node is not yet upgraded, since that would set the still unknown job-id
> parameter.. combined with the "job-id value can't be trusted" aspect, it might
> be better to skip setting it for manual invocations?

Short summary of our off-list discussion:
We agreed to make 'job-id' usable by root only to prevent abuse (e.g.
setting it to the job-id of other backup jobs, or some random value)
and to stop setting for manually triggered backup jobs.
That slightly worsens UX when e.g. triggering a backup job
to test matcher settings. To mitigate that, a follow up
could change the 'Run Backup Job' in such a way that it does not do a
direct vzdump API call, but requests execution of the backup job in the
near future from pvescheduler - similar how the 'Run now' button
for storage replication works.

Thanks a lot for the feedback!

-- 
- Lukas


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] superseded: [PATCH many v7 00/19] notifications: notification metadata matching improvements

[Lukas Wagner]

superseded by v8!

On  2024-06-10 10:40, Lukas Wagner wrote:
> This patch series attempts to improve the user experience when creating
> notification matchers.
> 
> Some of the noteworthy changes:
>   - Fixup inconsistent 'hostname' field. Some notification events sent
>   the hostname including a domain, while other did not.
>   This series unifies the behavior, now the field only includes the hostname
>   without a domain. Also updated the docs to reflect this change.
>   - Allow setting a custom backup job ID, similar how we handle it for
>   sync/prune jobs in PBS (to allow recognizable names used in matchers)
>   - Adding columns for backup job ID/replication job ID in the UI
>   - New metadata fields:
>     - job-id: Job ID for backup-jobs or replication-jobs
>   - Add an API that enumerates known notification metadata fields/values
>   - Suggest known fields/values in match rule window
>   - Some code clean up for match rule edit window
>   - Extended the 'exact' match-field mode - it now allows setting multiple
>     allowed values, separated via ',':
>       e.g. `match-field exact:type=replication,fencing
>     Originally, I created a separate 'list' match type for this, but
>     since the semantics for a list with one value and 'exact' mode
>     are identical, I decided to just extend 'exact'.
>     This is a safe change since there are are no values where a ','
>     makes any sense (config IDs, hostnames)
> 
> NOTE: Might need a versionened break, since the widget-toolkit-patches
> depend on new APIs provided by pve-manager. If the API is not present,
> creating matchers with 'match-field' does not work (cannot load lists
> of known values/fields)
> 
> Inter-Dependencies:
>   - the widget-toolkit dep in pve-manager needs to 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", otherwise the UI breaks
>     with the pve-manager patches applied) --> already included a patch for
>     this
>   - widget-toolkit relies on a new API endpoint provided by pve-manager:
>     --> we require a versioned break in widget-toolkit on pve-manager
> 
> Changelog:
>   - v7: incorporated some more feedback from @Fiona, thx!
>     - Fixed error when switching from 'exact' to 'regex' if the text field
>       was empty
>     - rebased to latest master
>     - 'backport' doc improvements from PBS
>     - bumped widget-toolkit dep
>   - v6: incorporate feedback from @Fiona, thx!
>     - rename 'id' -> 'job-id' in VZDump API handler
>     - consolidate 'replication-job'/'backup-job' to 'job-id'
>     - Move 'job-id' setting to advanced tab in backup job edit.
>     - Don't use 'internal' flag to mark translatable fields, since
>       the only field where that's necessary is 'type' for now - so
>       just add a hardcoded check
>   - v5:
>     - Rebased onto latest master, resolving some small conflict
>   - v4:
>     - widget-toolkit: break out changes for the utils module so that they
>       can be applied ahead of time to ease dep bumping
>     - don't show Job IDs in the backup/replication job columns
>   - v3:
>     - Drop already applied patches for `proxmox`
>     - Rebase onto latest master - minor conflict resolution was needed
>   - v2:
>     - include 'type' metadata field for forwarded mails
>       --> otherwise it's not possible to match them
>     - include Maximilliano's T-b trailer in UI patches
> 
> pve-guest-common:
> 
> Lukas Wagner (1):
>   vzdump: common: allow 'job-id' as a parameter without being in schema
> 
>  src/PVE/VZDump/Common.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> pve-manager:
> 
> Lukas Wagner (9):
>   api: jobs: vzdump: pass job 'job-id' parameter
>   ui: dc: backup: send 'job-id' property when starting a backup job
>     manually
>   ui: dc: backup: allow to set custom job id in  advanced settings
>   api: replication: add 'job-id' to notification metadata
>   vzdump: apt: notification: do not include domain in 'hostname' field
>   api: replication: include 'hostname' field for notifications
>   api: notification: add API for getting known metadata fields/values
>   ui: utils: add overrides for translatable notification fields/values
>   d/control: bump proxmox-widget-toolkit dependency to 4.1.4
> 
>  PVE/API2/APT.pm                             |   3 +-
>  PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
>  PVE/API2/Replication.pm                     |   4 +-
>  PVE/API2/VZDump.pm                          |   8 ++
>  PVE/Jobs/VZDump.pm                          |   4 +-
>  PVE/VZDump.pm                               |  14 +-
>  debian/control                              |   2 +-
>  www/manager6/Utils.js                       |  12 ++
>  www/manager6/dc/Backup.js                   |   7 +-
>  www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
>  10 files changed, 200 insertions(+), 16 deletions(-)
> 
> 
> proxmox-widget-toolkit:
> 
> Lukas Wagner (4):
>   notification: matcher: match-field: show known fields/values
>   notification: matcher: move match-field formulas to local viewModel
>   notification: matcher: move match-calendar fields to panel
>   notification: matcher: move match-severity fields to panel
> 
>  src/data/model/NotificationConfig.js  |  12 +
>  src/window/NotificationMatcherEdit.js | 613 ++++++++++++++++++--------
>  2 files changed, 441 insertions(+), 184 deletions(-)
> 
> 
> pve-docs:
> 
> Lukas Wagner (5):
>   notification: clarify that 'hostname' does not include the domain
>   notifications: describe new notification metadata fields
>   notifications: match-field 'exact'-mode can now match multiple values
>   notifications: fix typo in 'notification'
>   notifications: backport some rephrased parts from PBS docs
> 
>  notifications.adoc | 137 +++++++++++++++++++++++++++------------------
>  1 file changed, 84 insertions(+), 53 deletions(-)
> 
> 
> Summary over all repositories:
>   14 files changed, 726 insertions(+), 254 deletions(-)
> 

-- 
- Lukas


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel