[pve-devel] [PATCH many v9 00/13] notifications: notification metadata matching improvements

[pve-devel] [PATCH many v9 00/13] 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:
  - 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)
  - 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-manager needs bumped pve-guest-common (thx @Fabian)

Changelog:
  - v9: fix typos in commit message, add @Max's R-b trailer - thx!
  - v8: incorporate feedback from @Fabian, thx a lot!
    - Made 'job-id' API param usable by root@pam only - this should prevent
      abuse by spoofing job-id, potentially bothering other users with bogus
      notifications.
    - Don't set 'job-id' when starting a backup job via 'Run now' in the UI
    - Add a note to the docs explaining when job-id is set and when not.
    - Drop already applied patches
  - 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 (5):
  api: jobs: vzdump: pass job 'job-id' parameter
  ui: dc: backup: allow to set custom job id in  advanced settings
  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/Backup.pm                          |   2 +-
 PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
 PVE/API2/VZDump.pm                          |  13 +-
 PVE/Jobs/VZDump.pm                          |   4 +-
 PVE/VZDump.pm                               |   6 +-
 debian/control                              |   2 +-
 www/manager6/Utils.js                       |  11 ++
 www/manager6/dc/Backup.js                   |   4 -
 www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
 9 files changed, 192 insertions(+), 12 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 (3):
  notifications: describe new notification metadata fields
  notifications: match-field 'exact'-mode can now match multiple values
  notifications: add note regarding when 'job-id' is set for backups

 notifications.adoc | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)


Summary over all repositories:
  13 files changed, 655 insertions(+), 218 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 v9 01/13] 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
can be considered 'internal'.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@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 1996c5b..2532b42 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -503,7 +503,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 v9 02/13] api: jobs: vzdump: pass job 'job-id' parameter

[Lukas Wagner]

This allows us to access the backup job id in the send_notification
function, where we can set it as metadata for the notification.
The 'job-id' parameter can only be used by 'root@pam' to prevent
abuse. This has the side effect that manually triggered backup jobs
cannot have the 'job-id' parameter at the moment. To mitigate that,
manually triggered backup jobs could be changed so that they
are not performed by a direct API call by the UI, but by requesting
pvescheduler to execute the job in the near future (similar to how
manually triggered replication jobs work).

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
---
 PVE/API2/Backup.pm |  2 +-
 PVE/API2/VZDump.pm | 13 +++++++++++--
 PVE/Jobs/VZDump.pm |  4 +++-
 PVE/VZDump.pm      |  6 +++---
 4 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
index 88140323..48598b8f 100644
--- a/PVE/API2/Backup.pm
+++ b/PVE/API2/Backup.pm
@@ -45,7 +45,7 @@ sub assert_param_permission_common {
     my ($rpcenv, $user, $param, $is_delete) = @_;
     return if $user eq 'root@pam'; # always OK
 
-    for my $key (qw(tmpdir dumpdir script)) {
+    for my $key (qw(tmpdir dumpdir script job-id)) {
 	raise_param_exc({ $key => "Only root may set this option."}) if exists $param->{$key};
     }
 
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 7f92e7ec..15c9b0dc 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -42,8 +42,8 @@ __PACKAGE__->register_method ({
     permissions => {
 	description => "The user needs 'VM.Backup' permissions on any VM, and "
 	    ."'Datastore.AllocateSpace' on the backup storage (and fleecing storage when fleecing "
-	    ."is used). The 'tmpdir', 'dumpdir' and 'script' parameters are restricted to the "
-	    ."'root\@pam' user. The 'maxfiles' and 'prune-backups' settings require "
+	    ."is used). The 'tmpdir', 'dumpdir', 'script' and 'job-id' parameters are restricted "
+	    ."to the 'root\@pam' user. The 'maxfiles' and 'prune-backups' settings require "
 	    ."'Datastore.Allocate' on the backup storage. The 'bwlimit', 'performance' and "
 	    ."'ionice' parameters require 'Sys.Modify' on '/'.",
 	user => 'all',
@@ -53,6 +53,15 @@ __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. Only root\@pam"
+		    . " can set this parameter.",
+		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 8dbcc4a9..f1a6b220 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';
@@ -528,13 +529,12 @@ 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 (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;
 
     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] [PATCH manager v9 03/13] 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>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
---
 www/manager6/dc/Backup.js                   |  4 ----
 www/manager6/panel/BackupAdvancedOptions.js | 23 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4ba80b31..381402ca 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 7dd19f96..acb2fbd0 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) {
@@ -108,6 +112,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 v9 04/13] 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>
Reviewed-by: Max Carrara <m.carrara@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 v9 05/13] ui: utils: add overrides for translatable notification fields/values

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@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 f5608944..5b9d86ca 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2059,6 +2059,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 v9 06/13] 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>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
---
 debian/control | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index d4c254d4..bc9c7218 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 v9 07/13] 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>
Reviewed-by: Max Carrara <m.carrara@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 v9 08/13] 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>
Reviewed-by: Max Carrara <m.carrara@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 v9 09/13] 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>
Reviewed-by: Max Carrara <m.carrara@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 v9 10/13] 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>
Reviewed-by: Max Carrara <m.carrara@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 v9 11/13] notifications: describe new notification metadata fields

[Lukas Wagner]

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

diff --git a/notifications.adoc b/notifications.adoc
index 25a9391..acca19b 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -301,19 +301,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 notification
-| `hostname` | Hostname, without domain (e.g. `pve1`)
+| Field name        | Description
+| `type`            | Type of the notification
+| `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 v9 12/13] notifications: match-field 'exact'-mode can now match multiple values

[Lukas Wagner]

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

diff --git a/notifications.adoc b/notifications.adoc
index acca19b..bdfebd0 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -233,11 +233,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
@@ -279,18 +284,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 v9 13/13] notifications: add note regarding when 'job-id' is set for backups

[Lukas Wagner]

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
---
 notifications.adoc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/notifications.adoc b/notifications.adoc
index bdfebd0..6425e6c 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -312,6 +312,10 @@ Notification Events
 | `job-id`          | Job ID
 |=======================================================================
 
+NOTE: Backup job notifications only have `job-id` set if the backup job
+  was executed automatically based on its schedule, but not if it was triggered
+  manually by the 'Run now' button in the UI.
+
 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 pve-guest-common v9 01/13] vzdump: common: allow 'job-id' as a parameter without being in schema

[Thomas Lamprecht]

Am 08/07/2024 um 11:38 schrieb 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
> can be considered 'internal'.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> Reviewed-by: Max Carrara <m.carrara@proxmox.com>
> ---
>  src/PVE/VZDump/Common.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


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

[pve-devel] applied: [PATCH manager v9 02/13] api: jobs: vzdump: pass job 'job-id' parameter

[Thomas Lamprecht]

Am 08/07/2024 um 11:38 schrieb Lukas Wagner:
> This allows us to access the backup job id in the send_notification
> function, where we can set it as metadata for the notification.
> The 'job-id' parameter can only be used by 'root@pam' to prevent
> abuse. This has the side effect that manually triggered backup jobs
> cannot have the 'job-id' parameter at the moment. To mitigate that,
> manually triggered backup jobs could be changed so that they
> are not performed by a direct API call by the UI, but by requesting
> pvescheduler to execute the job in the near future (similar to how
> manually triggered replication jobs work).
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> Reviewed-by: Max Carrara <m.carrara@proxmox.com>
> ---
>  PVE/API2/Backup.pm |  2 +-
>  PVE/API2/VZDump.pm | 13 +++++++++++--
>  PVE/Jobs/VZDump.pm |  4 +++-
>  PVE/VZDump.pm      |  6 +++---
>  4 files changed, 18 insertions(+), 7 deletions(-)
> 
>

applied with the d/control bump for guest-common squashed in, thanks!


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

[pve-devel] partially-applied: [PATCH many v9 00/13] notifications: notification metadata matching improvements

[Thomas Lamprecht]

Am 08/07/2024 um 11:37 schrieb Lukas Wagner:
> This patch series attempts to improve the user experience when creating
> notification matchers.
> 
> Some of the noteworthy changes:
>   - 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)
>   - 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

not sure if I really want to do that as it's not really useful for clusters
anyway, where the loaded wtk can be newer than the manager of a (proxied)
node. But in any way: many thanks for mentioning it.

>   - pve-manager needs bumped pve-guest-common (thx @Fabian)
> 
> Changelog:
>   - v9: fix typos in commit message, add @Max's R-b trailer - thx!
>   - v8: incorporate feedback from @Fabian, thx a lot!
>     - Made 'job-id' API param usable by root@pam only - this should prevent
>       abuse by spoofing job-id, potentially bothering other users with bogus
>       notifications.
>     - Don't set 'job-id' when starting a backup job via 'Run now' in the UI
>     - Add a note to the docs explaining when job-id is set and when not.
>     - Drop already applied patches
>   - 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 (5):
>   api: jobs: vzdump: pass job 'job-id' parameter
>   ui: dc: backup: allow to set custom job id in  advanced settings
>   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/Backup.pm                          |   2 +-
>  PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
>  PVE/API2/VZDump.pm                          |  13 +-
>  PVE/Jobs/VZDump.pm                          |   4 +-
>  PVE/VZDump.pm                               |   6 +-
>  debian/control                              |   2 +-
>  www/manager6/Utils.js                       |  11 ++
>  www/manager6/dc/Backup.js                   |   4 -
>  www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
>  9 files changed, 192 insertions(+), 12 deletions(-)
> 

applied above for now, we probably should bump manager soonish and then can
apply below.

> 
> 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 (3):
>   notifications: describe new notification metadata fields
>   notifications: match-field 'exact'-mode can now match multiple values
>   notifications: add note regarding when 'job-id' is set for backups
> 
>  notifications.adoc | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> 
> Summary over all repositories:
>   13 files changed, 655 insertions(+), 218 deletions(-)
> 


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

Re: [pve-devel] partially-applied: [PATCH many v9 00/13] notifications: notification metadata matching improvements

[Lukas Wagner]

On  2024-07-22 19:36, Thomas Lamprecht wrote:
>> Lukas Wagner (5):
>>   api: jobs: vzdump: pass job 'job-id' parameter
>>   ui: dc: backup: allow to set custom job id in  advanced settings
>>   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/Backup.pm                          |   2 +-
>>  PVE/API2/Cluster/Notifications.pm           | 139 ++++++++++++++++++++
>>  PVE/API2/VZDump.pm                          |  13 +-
>>  PVE/Jobs/VZDump.pm                          |   4 +-
>>  PVE/VZDump.pm                               |   6 +-
>>  debian/control                              |   2 +-
>>  www/manager6/Utils.js                       |  11 ++
>>  www/manager6/dc/Backup.js                   |   4 -
>>  www/manager6/panel/BackupAdvancedOptions.js |  23 ++++
>>  9 files changed, 192 insertions(+), 12 deletions(-)
>>
> 
> applied above for now, we probably should bump manager soonish and then can
> apply below.
> 

pve-manager has been bumped in the meanwhile, I guess we could now merge the
remaining patches for pve-docs and proxmox-widget-toolkit?
They still apply cleanly and a quick test also showed that everything still
works as expected.


-- 
- Lukas


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

[pve-devel] applied-series: partially-applied: [PATCH many v9 00/13] notifications: notification metadata matching improvements

[Thomas Lamprecht]

Am 23/09/2024 um 13:27 schrieb Lukas Wagner:
> pve-manager has been bumped in the meanwhile, I guess we could now merge the
> remaining patches for pve-docs and proxmox-widget-toolkit?
> They still apply cleanly and a quick test also showed that everything still
> works as expected.

thanks for the reminder, applied the remaining docs and wtk patches now.


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