Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints

2024-07-22 Thread Lukas Wagner


On  2024-07-22 14:10, Stefan Hanreich wrote:
> I got the following JS error when trying to save a secret/header/body
> template with the content: `qweqwe   ßẞ`
> 
> Uncaught DOMException: String contains an invalid character
> getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11924
> each ExtJS
> getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11920
> checkChange ExtJS
> itemChange https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:12009
> ExtJS 22
> proxmoxlib.js:11924
> 
> 
> Seems like the issue is with btoa and uppercase scharfes S?
> When using Umlauts, I at least get an error message, but cannot save.
> 
> 
> Other than that everything worked fine - consider this:
> Tested-By: Stefan Hanreich 
> 

Thanks a lot for testing, Stefan.

Indeed it looks like btoa has problems with unicode characters.
Luckily, MDN [1] has got our back with that one. I'll use that workaround
in an upcoming v3.

[1] https://developer.mozilla.org/en-US/docs/Glossary/Base64#the_unicode_problem

-- 
- Lukas


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


Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints

2024-07-22 Thread Lukas Wagner



On  2024-07-17 17:34, Max Carrara wrote:
> On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
>> Sending as an RFC because I don't want this merged yet; that being
>> said, the feature should be mostly finished at this point, I'd
>> appreciate any reviews and feedback.
>>
>> This series adds support for webhook notification targets to PVE
>> and PBS.
>>
>> A webhook is a HTTP API route provided by a third-party service that
>> can be used to inform the third-party about an event. In our case,
>> we can easily interact with various third-party notification/messaging
>> systems and send PVE/PBS notifications via this service.
>> The changes were tested against ntfy.sh, Discord and Slack.
>>
>> The configuration of webhook targets allows one to configure:
>>   - The URL
>>   - The HTTP method (GET/POST/PUT)
>>   - HTTP Headers
>>   - Body
>>
>> One can use handlebar templating to inject notification text and metadata
>> in the url, headers and body.
>>
>> One challenge is the handling of sensitve tokens and other secrets.
>> Since the endpoint is completely generic, we cannot know in advance
>> whether the body/header/url contains sensitive values.
>> Thus we add 'secrets' which are stored in the protected config only
>> accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
>> secrets are accessible in URLs/headers/body via templating:
>>
>>   Url: https://example.com/{{ secrets.token }}
>>
>> Secrets can only be set and updated, but never retrieved via the API.
>> In the UI, secrets are handled like other secret tokens/passwords.
>>
>> Bumps for PVE:
>>   - libpve-rs-perl needs proxmox-notify bumped
>>   - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
>>   - proxmox-mail-forward needs proxmox-notify bumped
>>
>> Bumps for PBS:
>>   - proxmox-backup needs proxmox-notify bumped
>>   - proxmox-mail-forward needs proxmox-notify bumped
> 
> Since this is an RFC, I mainly just did some proofreading; I haven't
> really spotted anything out of the ordinary, apart from a few *very
> small* things I commented on inline.
> 
> I like the overall idea of adding webhooks, so this looks pretty solid
> to me. At first I thought that this might be a bit of a niche use case,
> but I feel like it might actually be quite interesting for orgs that are
> e.g. on Slack: You could e.g. just "route" all notifications via a
> webhook to Slack, and Slack then sends a push notification to one's
> phone. The same can obviously done with other applications / services as
> well. So, pretty cool stuff :)
> 
> Not sure if this has been discussed somewhere already (off list etc.),
> but could you elaborate on why you don't want this merged yet? The
> patches look pretty solid to me, IMHO. Then again, I haven't really
> tested them yet due to all the required package bumps, so take this with
> a grain of salt.
> 
> If you want to have this RFC tested, I can of course give it a shot - do
> let me know if that's the case :)
> 

I posted this as an RFC because while I consider this as mostly finished,
it did not yet go through my own rigorous self-review/testing.
I had to switch to some other task and wanted to get this version out to get 
some
general feedback.

There are no changes planned unless I or somebody else discovers any issues,
so I'd very much welcome any testing :)

-- 
- Lukas


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



Re: [pve-devel] [pbs-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints

2024-07-22 Thread Lukas Wagner



On  2024-07-17 17:36, Max Carrara wrote:
> On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
>> These just call the API implementation via the perl-rs bindings.
>>
>> Signed-off-by: Lukas Wagner 
>> ---
>>  PVE/API2/Cluster/Notifications.pm | 263 +-
>>  1 file changed, 262 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/API2/Cluster/Notifications.pm 
>> b/PVE/API2/Cluster/Notifications.pm
>> index 10b611c9..eae2d436 100644
>> --- a/PVE/API2/Cluster/Notifications.pm
>> +++ b/PVE/API2/Cluster/Notifications.pm
>> @@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
>>  { name => 'gotify' },
>>  { name => 'sendmail' },
>>  { name => 'smtp' },
>> +{ name => 'webhook' },
>>  ];
>>  
>>  return $result;
>> @@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
>>  'type' => {
>>  description => 'Type of the target.',
>>  type  => 'string',
>> -enum => [qw(sendmail gotify smtp)],
>> +enum => [qw(sendmail gotify smtp webhook)],
>>  },
>>  'comment' => {
>>  description => 'Comment',
>> @@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
>>  }
>>  });
>>  
>> +my $webhook_properties = {
>> +name => {
>> +description => 'The name of the endpoint.',
>> +type => 'string',
>> +format => 'pve-configid',
>> +},
>> +url => {
>> +description => 'Server URL',
>> +type => 'string',
>> +},
>> +method => {
>> +description => 'HTTP method',
>> +type => 'string',
>> +enum => [qw(post put get)],
>> +},
>> +header => {
>> +description => 'HTTP headers to set. These have to be formatted as'
>> +  . ' a property string in the format name=,value=> value>',
>> +type => 'array',
>> +items => {
>> +type => 'string',
>> +},
>> +optional => 1,
>> +},
>> +body => {
>> +description => 'HTTP body, base64 encoded',
>> +type => 'string',
>> +optional => 1,
>> +},
>> +secret => {
>> +description => 'Secrets to set. These have to be formatted as'
>> +  . ' a property string in the format name=,value=> value>',
>> +type => 'array',
>> +items => {
>> +type => 'string',
>> +},
>> +optional => 1,
>> +},
>> +comment => {
>> +description => 'Comment',
>> +type => 'string',
>> +optional => 1,
>> +},
>> +disable => {
>> +description => 'Disable this target',
>> +type => 'boolean',
>> +optional => 1,
>> +default => 0,
>> +},
>> +};
>> +
>> +__PACKAGE__->register_method ({
>> +name => 'get_webhook_endpoints',
>> +path => 'endpoints/webhook',
>> +method => 'GET',
>> +description => 'Returns a list of all webhook endpoints',
>> +protected => 1,
>> +permissions => {
>> +check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
>> +check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
>> +},
>> +parameters => {
>> +additionalProperties => 0,
>> +properties => {},
>> +},
>> +returns => {
>> +type => 'array',
>> +items => {
>> +type => 'object',
>> +properties => {
>> +%$webhook_properties,
> 
> Would prefer `$webhook_properties->%*` here (postfix dereferencing) -
> even though not explicitly stated in our style guide, we use that kind
> of syntax for calling subroutines behind a reference, e.g.
> `$foo->($arg)` instead of `&$foo($arg)`.
> 

I kinda prefer the brevity of the prefix variant in this case. Are there
any pitfalls/problems with the prefix that I'm not aware of? If not, I'd prefer
to keep this as is, I used the syntax in many other spots in this file ;)

-- 
- Lukas


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



Re: [pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets

2024-07-22 Thread Lukas Wagner



On  2024-07-17 17:35, Max Carrara wrote:
>> +
>> +assert_eq!(secrets[1].name, "token".to_string());
>> +assert_eq!(secrets[1].value, Some(encode("secret")));
>> +assert_eq!(secrets[0].name, "token2".to_string());
>> +assert_eq!(secrets[0].value, Some(encode("newsecret")));
>> +
>> +// Test property deletion
>> +update_endpoint(
>> + config,
>> +"webhook-endpoint",
>> +Default::default(),
>> +Some(&[DeleteableWebhookProperty::Comment, 
>> DeleteableWebhookProperty::Secret]),
> 
> You missed a `cargo fmt` here ;)
> 


Right... Thanks! Was too used to RustRover autoformatting everything on save, 
which
I have not set up in my current nvim setup yet.

-- 
- Lukas


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



Re: [pve-devel] [pbs-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets

2024-07-22 Thread Lukas Wagner



On  2024-07-17 17:35, Max Carrara wrote:
>> +let handlebars = setup_handlebars();
>> +let body_template = 
>> self.base_64_decode(self.config.body.as_deref().unwrap_or_default())?;
>> +
>> +let body = handlebars
>> +.render_template(_template, )
>> +.map_err(|err| {
>> +// TODO: Cleanup error types, they have become a bit messy.
>> +// No user of the notify crate distinguish between the 
>> error types any way, so
>> +// we can refactor without any issues
>> +Error::Generic(format!("failed to render webhook body: 
>> {err}"))
> 
> I'm curious, how would you clean up the error types in particular?
> 

Right now, error handling is a bit messy... Some endpoints primarily use
the `NotifyFailed` variant, which wraps another error, while in some places
where I need a leaf error type that does not wrap any error I use the
`Generic` variant, which only stores a string.
I could have used the `NotifyFailed` variant here, but that one would
not have allowed me to add additional context ("failed to render webhook ..."),
unless wrapping another `Generic` variant...

I don't have yet made any detailed plans on how to clean that up though.

-- 
- Lukas


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



[pve-devel] [PATCH widget-toolkit v2 05/12] notification: add UI for adding/updating webhook targets

2024-07-12 Thread Lukas Wagner
The widgets for editing the headers/secrets were adapted from
the 'Tag Edit' dialog from PVE's datacenter options.

Apart from that, the new dialog is rather standard. I've decided
to put the http method and url in a single row, mostly to
save space and also to make it analogous to how an actual http request
is structured (VERB URL, followed by headers, followed by the body).

The secrets are a mechanism to store tokens/passwords in the
protected notification config. Secrets are accessible via
templating in the URL, headers and body via {{ secrets.NAME }}.
Secrets can only be set/updated, but not retrieved/displayed.

Signed-off-by: Lukas Wagner 
---
 src/Makefile  |   1 +
 src/Schema.js |   5 +
 src/panel/WebhookEditPanel.js | 417 ++
 3 files changed, 423 insertions(+)
 create mode 100644 src/panel/WebhookEditPanel.js

diff --git a/src/Makefile b/src/Makefile
index 0478251..cfaffd7 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -78,6 +78,7 @@ JSSRC=\
panel/StatusView.js \
panel/TfaView.js\
panel/NotesView.js  \
+   panel/WebhookEditPanel.js   \
window/Edit.js  \
window/PasswordEdit.js  \
window/SafeDestroy.js   \
diff --git a/src/Schema.js b/src/Schema.js
index 42541e0..cd1c306 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -65,6 +65,11 @@ Ext.define('Proxmox.Schema', { // a singleton
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
+   webhook: {
+   name: 'Webhook',
+   ipanel: 'pmxWebhookEditPanel',
+   iconCls: 'fa-bell-o',
+   },
 },
 
 // to add or change existing for product specific ones
diff --git a/src/panel/WebhookEditPanel.js b/src/panel/WebhookEditPanel.js
new file mode 100644
index 000..dfc7f3f
--- /dev/null
+++ b/src/panel/WebhookEditPanel.js
@@ -0,0 +1,417 @@
+Ext.define('Proxmox.panel.WebhookEditPanel', {
+extend: 'Proxmox.panel.InputPanel',
+xtype: 'pmxWebhookEditPanel',
+mixins: ['Proxmox.Mixin.CBind'],
+onlineHelp: 'notification_targets_webhook',
+
+type: 'webhook',
+
+columnT: [
+
+],
+
+column1: [
+   {
+   xtype: 'pmxDisplayEditField',
+   name: 'name',
+   cbind: {
+   value: '{name}',
+   editable: '{isCreate}',
+   },
+   fieldLabel: gettext('Endpoint Name'),
+   allowBlank: false,
+   },
+],
+
+column2: [
+   {
+   xtype: 'proxmoxcheckbox',
+   name: 'enable',
+   fieldLabel: gettext('Enable'),
+   allowBlank: false,
+   checked: true,
+   },
+],
+
+columnB: [
+   {
+   layout: 'hbox',
+   border: false,
+   margin: '0 0 5 0',
+   items: [
+   {
+   xtype: 'displayfield',
+   value: gettext('Method/URL:'),
+   width: 125,
+   },
+   {
+   xtype: 'proxmoxKVComboBox',
+   name: 'method',
+   //fieldLabel: gettext('Method'),
+   editable: false,
+   value: 'post',
+   comboItems: [
+   ['post', 'POST'],
+   ['put', 'PUT'],
+   ['get', 'GET'],
+   ],
+   width: 80,
+   margin: '0 5 0 0',
+   },
+   {
+   xtype: 'proxmoxtextfield',
+   //fieldLabel: gettext('URL'),
+   name: 'url',
+   allowBlank: false,
+   flex: 4,
+   },
+   ],
+   },
+   {
+   xtype: 'pmxWebhookKeyValueList',
+   name: 'header',
+   fieldLabel: gettext('Headers'),
+   maskValues: false,
+   cbind: {
+   isCreate: '{isCreate}',
+   },
+   },
+   {
+   xtype: 'textarea',
+   fieldLabel: gettext('Body'),
+   name: 'body',
+   allowBlank: true,
+   minHeight: '150',
+   fieldStyle: {
+   'font-family': 'monospace',
+   },
+   margin: '15 0 0 0',
+   },
+   {
+   xtype: 'pmxWebhookKeyValueList',
+   name: 'secret',
+   fieldLabel: gettext('Secrets'),
+   maskValues: true,
+   cbind: {
+   isCreate: '{isCreate}',
+   },
+   },
+   {
+   xtype: 'proxmoxtextfield',
+   name: 'comment',
+   fieldLabel: gettext('Comment'),
+   cbind: {
+   deleteEmpty: '{!isCreate}',
+   },
+   },
+],
+
+onSetValues: (values) => {
+   values.enable = !values.disable;
+
+   if (values.body) {
+   values.b

[pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets

2024-07-12 Thread Lukas Wagner
All in all pretty similar to other endpoint APIs.
One thing worth noting is how secrets are handled. We never ever
return the values of previously stored secrets in get_endpoint(s)
calls, but only a list of the names of all secrets. This is needed
to build the UI, where we display all secrets that were set before in
a table.

For update calls, one is supposed to send all secrets that should be
kept and updated. If the value should be updated, the name and value
is expected, and if the current value should preseved, only the name
is sent. If a secret's name is not present in the updater, it will be
dropped. If 'secret' is present in the 'delete' array, all secrets
will be dropped, apart from those which are also set/preserved in the
same update call.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/src/api/mod.rs |  20 ++
 proxmox-notify/src/api/webhook.rs | 406 ++
 2 files changed, 426 insertions(+)
 create mode 100644 proxmox-notify/src/api/webhook.rs

diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index a7f6261c..7f823bc7 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -15,6 +15,8 @@ pub mod matcher;
 pub mod sendmail;
 #[cfg(feature = "smtp")]
 pub mod smtp;
+#[cfg(feature = "webhook")]
+pub mod webhook;
 
 // We have our own, local versions of http_err and http_bail, because
 // we don't want to wrap the error in anyhow::Error. If we were to do that,
@@ -54,6 +56,9 @@ pub enum EndpointType {
 /// Gotify endpoint
 #[cfg(feature = "gotify")]
 Gotify,
+/// Webhook endpoint
+#[cfg(feature = "webhook")]
+Webhook,
 }
 
 #[api]
@@ -113,6 +118,17 @@ pub fn get_targets(config: ) -> Result, 
HttpError> {
 })
 }
 
+#[cfg(feature = "webhook")]
+for endpoint in webhook::get_endpoints(config)? {
+targets.push(Target {
+name: endpoint.name,
+origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+endpoint_type: EndpointType::Webhook,
+disable: endpoint.disable,
+comment: endpoint.comment,
+})
+}
+
 Ok(targets)
 }
 
@@ -145,6 +161,10 @@ fn ensure_endpoint_exists(#[allow(unused)] config: 
, name: ) -> Resul
 {
 exists = exists || smtp::get_endpoint(config, name).is_ok();
 }
+#[cfg(feature = "webhook")]
+{
+exists = exists || webhook::get_endpoint(config, name).is_ok();
+}
 
 if !exists {
 http_bail!(NOT_FOUND, "endpoint '{name}' does not exist")
diff --git a/proxmox-notify/src/api/webhook.rs 
b/proxmox-notify/src/api/webhook.rs
new file mode 100644
index ..b7f17c55
--- /dev/null
+++ b/proxmox-notify/src/api/webhook.rs
@@ -0,0 +1,406 @@
+use proxmox_http_error::HttpError;
+use proxmox_schema::property_string::PropertyString;
+
+use crate::api::http_err;
+use crate::endpoints::webhook::{
+DeleteableWebhookProperty, KeyAndBase64Val, WebhookConfig, 
WebhookConfigUpdater,
+WebhookPrivateConfig, WEBHOOK_TYPENAME,
+};
+use crate::{http_bail, Config};
+
+use super::remove_private_config_entry;
+use super::set_private_config_entry;
+
+/// Get a list of all webhook endpoints.
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns a list of all webhook endpoints or a `HttpError` if the config is
+/// erroneous (`500 Internal server error`).
+pub fn get_endpoints(config: ) -> Result, HttpError> 
{
+let mut endpoints: Vec = config
+.config
+.convert_to_typed_array(WEBHOOK_TYPENAME)
+.map_err(|e| http_err!(NOT_FOUND, "Could not fetch endpoints: {e}"))?;
+
+for endpoint in  endpoints {
+let priv_config: WebhookPrivateConfig = config
+.private_config
+.lookup(WEBHOOK_TYPENAME, )
+.unwrap_or_default();
+
+let mut secret_names = Vec::new();
+for secret in priv_config.secret {
+secret_names.push(
+KeyAndBase64Val {
+name: secret.name.clone(),
+value: None,
+}
+.into(),
+)
+}
+
+endpoint.secret = secret_names;
+}
+
+Ok(endpoints)
+}
+
+/// Get webhook endpoint with given `name`
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns the endpoint or a `HttpError` if the endpoint was not found (`404 
Not found`).
+pub fn get_endpoint(config: , name: ) -> Result {
+let mut endpoint: WebhookConfig = config
+.config
+.lookup(WEBHOOK_TYPENAME, name)
+.map_err(|_| http_err!(NOT_FOUND, "endpoint '{name}' not found"))?;
+
+let priv_config: Option = config
+.private_config
+.lookup(WEBHOOK_TYPENAME, )
+.ok();
+
+let mut secret_names = Vec::new();
+if let Some(priv_config) = priv_config {
+for secret in _

[pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency

2024-07-12 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 Cargo.toml | 2 +-
 debian/control | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index f39d118..49ca079 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,4 +20,4 @@ nix = "0.26"
 syslog = "6.0"
 
 proxmox-sys = "0.5.3"
-proxmox-notify = {version = "0.4", features = ["mail-forwarder", 
"pve-context", "pbs-context"] }
+proxmox-notify = {version = "0.5", features = ["mail-forwarder", 
"pve-context", "pbs-context"] }
diff --git a/debian/control b/debian/control
index 7329a24..0ab74a9 100644
--- a/debian/control
+++ b/debian/control
@@ -6,10 +6,10 @@ Build-Depends: cargo:native,
librust-anyhow-1+default-dev,
librust-log-0.4+default-dev (>= 0.4.17-~~),
librust-nix-0.26+default-dev,
-   librust-proxmox-notify-0.4+default-dev,
-   librust-proxmox-notify-0.4+mail-forwarder-dev,
-   librust-proxmox-notify-0.4+pbs-context-dev,
-   librust-proxmox-notify-0.4+pve-context-dev,
+   librust-proxmox-notify-0.5+default-dev,
+   librust-proxmox-notify-0.5+mail-forwarder-dev,
+   librust-proxmox-notify-0.5+pbs-context-dev,
+   librust-proxmox-notify-0.5+pve-context-dev,
librust-proxmox-sys-0.5+default-dev (>= 0.5.3-~~),
librust-syslog-6+default-dev,
libstd-rust-dev,
-- 
2.39.2



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



[pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints

2024-07-12 Thread Lukas Wagner
Sending as an RFC because I don't want this merged yet; that being
said, the feature should be mostly finished at this point, I'd
appreciate any reviews and feedback.

This series adds support for webhook notification targets to PVE
and PBS.

A webhook is a HTTP API route provided by a third-party service that
can be used to inform the third-party about an event. In our case,
we can easily interact with various third-party notification/messaging
systems and send PVE/PBS notifications via this service.
The changes were tested against ntfy.sh, Discord and Slack.

The configuration of webhook targets allows one to configure:
  - The URL
  - The HTTP method (GET/POST/PUT)
  - HTTP Headers
  - Body

One can use handlebar templating to inject notification text and metadata
in the url, headers and body.

One challenge is the handling of sensitve tokens and other secrets.
Since the endpoint is completely generic, we cannot know in advance
whether the body/header/url contains sensitive values.
Thus we add 'secrets' which are stored in the protected config only
accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
secrets are accessible in URLs/headers/body via templating:

  Url: https://example.com/{{ secrets.token }}

Secrets can only be set and updated, but never retrieved via the API.
In the UI, secrets are handled like other secret tokens/passwords.

Bumps for PVE:
  - libpve-rs-perl needs proxmox-notify bumped
  - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
  - proxmox-mail-forward needs proxmox-notify bumped

Bumps for PBS:
  - proxmox-backup needs proxmox-notify bumped
  - proxmox-mail-forward needs proxmox-notify bumped


Changes v1 -> v2:
  - Rebase proxmox-notify changes

proxmox:

Lukas Wagner (2):
  notify: implement webhook targets
  notify: add api for webhook targets

 proxmox-notify/Cargo.toml   |   9 +-
 proxmox-notify/src/api/mod.rs   |  20 +
 proxmox-notify/src/api/webhook.rs   | 406 +++
 proxmox-notify/src/config.rs|  23 ++
 proxmox-notify/src/endpoints/mod.rs |   2 +
 proxmox-notify/src/endpoints/webhook.rs | 509 
 proxmox-notify/src/lib.rs   |  17 +
 7 files changed, 983 insertions(+), 3 deletions(-)
 create mode 100644 proxmox-notify/src/api/webhook.rs
 create mode 100644 proxmox-notify/src/endpoints/webhook.rs


proxmox-perl-rs:

Lukas Wagner (2):
  common: notify: add bindings for webhook API routes
  common: notify: add bindings for get_targets

 common/src/notify.rs | 72 
 1 file changed, 72 insertions(+)


proxmox-widget-toolkit:

Lukas Wagner (1):
  notification: add UI for adding/updating webhook targets

 src/Makefile  |   1 +
 src/Schema.js |   5 +
 src/panel/WebhookEditPanel.js | 417 ++
 3 files changed, 423 insertions(+)
 create mode 100644 src/panel/WebhookEditPanel.js


pve-manager:

Lukas Wagner (2):
  api: notifications: use get_targets impl from proxmox-notify
  api: add routes for webhook notification endpoints

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


pve-docs:

Lukas Wagner (1):
  notification: add documentation for webhook target endpoints.

 notifications.adoc | 93 ++
 1 file changed, 93 insertions(+)


proxmox-backup:

Lukas Wagner (3):
  api: notification: add API routes for webhook targets
  ui: utils: enable webhook edit window
  docs: notification: add webhook endpoint documentation

 docs/notifications.rst   | 100 +
 src/api2/config/notifications/mod.rs |   2 +
 src/api2/config/notifications/webhook.rs | 175 +++
 www/Utils.js |   5 +
 4 files changed, 282 insertions(+)
 create mode 100644 src/api2/config/notifications/webhook.rs


proxmox-mail-forward:

Lukas Wagner (1):
  bump proxmox-notify dependency

 Cargo.toml | 2 +-
 debian/control | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)


Summary over all repositories:
  19 files changed, 2121 insertions(+), 42 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 proxmox v2 01/12] notify: implement webhook targets

2024-07-12 Thread Lukas Wagner
This target type allows users to perform HTTP requests to arbitrary
third party (notification) services, for instance
ntfy.sh/Discord/Slack.

The configuration for these endpoints allows one to freely configure
the URL, HTTP Method, headers and body. The URL, header values and
body support handlebars templating to inject notification text,
metadata and secrets. Secrets are stored in the protected
configuration file (e.g. /etc/pve/priv/notification.cfg) as key value
pairs, allowing users to protect sensitive tokens/passwords.
Secrets are accessible in handlebar templating via the secrets.*
namespace, e.g. if there is a secret named 'token', a body
could contain '{{ secrets.token }}' to inject the token into the
payload.

A couple of handlebars helpers are also provided:
  - url-encoding (useful for templating in URLs)
  - escape (escape any control characters in strings)
  - json (print a property as json)

In the configuration, the body, header values and secret values
are stored in base64 encoding so that we can store any string we want.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/Cargo.toml   |   9 +-
 proxmox-notify/src/config.rs|  23 ++
 proxmox-notify/src/endpoints/mod.rs |   2 +
 proxmox-notify/src/endpoints/webhook.rs | 509 
 proxmox-notify/src/lib.rs   |  17 +
 5 files changed, 557 insertions(+), 3 deletions(-)
 create mode 100644 proxmox-notify/src/endpoints/webhook.rs

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 7801814d..484aff19 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -9,13 +9,15 @@ exclude.workspace = true
 
 [dependencies]
 anyhow.workspace = true
-base64.workspace = true
+base64 = { workspace = true, optional = true }
 const_format.workspace = true
 handlebars = { workspace = true }
+http = { workspace = true, optional = true }
 lettre = { workspace = true, optional = true }
 log.workspace = true
 mail-parser = { workspace = true, optional = true }
 openssl.workspace = true
+percent-encoding = { workspace = true, optional = true }
 regex.workspace = true
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
@@ -31,10 +33,11 @@ proxmox-time.workspace = true
 proxmox-uuid = { workspace = true, features = ["serde"] }
 
 [features]
-default = ["sendmail", "gotify", "smtp"]
+default = ["sendmail", "gotify", "smtp", "webhook"]
 mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys"]
-sendmail = ["dep:proxmox-sys"]
+sendmail = ["dep:proxmox-sys", "dep:base64"]
 gotify = ["dep:proxmox-http"]
 pve-context = ["dep:proxmox-sys"]
 pbs-context = ["dep:proxmox-sys"]
 smtp = ["dep:lettre"]
+webhook = ["dep:base64", "dep:http", "dep:percent-encoding", 
"dep:proxmox-http"]
diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
index 789c4a7d..4d0b53f7 100644
--- a/proxmox-notify/src/config.rs
+++ b/proxmox-notify/src/config.rs
@@ -57,6 +57,17 @@ fn config_init() -> SectionConfig {
 GOTIFY_SCHEMA,
 ));
 }
+#[cfg(feature = "webhook")]
+{
+use crate::endpoints::webhook::{WebhookConfig, WEBHOOK_TYPENAME};
+
+const WEBHOOK_SCHEMA:  = 
WebhookConfig::API_SCHEMA.unwrap_object_schema();
+config.register_plugin(SectionConfigPlugin::new(
+WEBHOOK_TYPENAME.to_string(),
+Some(String::from("name")),
+WEBHOOK_SCHEMA,
+));
+}
 
 const MATCHER_SCHEMA:  = 
MatcherConfig::API_SCHEMA.unwrap_object_schema();
 config.register_plugin(SectionConfigPlugin::new(
@@ -110,6 +121,18 @@ fn private_config_init() -> SectionConfig {
 ));
 }
 
+#[cfg(feature = "webhook")]
+{
+use crate::endpoints::webhook::{WebhookPrivateConfig, 
WEBHOOK_TYPENAME};
+
+const WEBHOOK_SCHEMA:  =
+WebhookPrivateConfig::API_SCHEMA.unwrap_object_schema();
+config.register_plugin(SectionConfigPlugin::new(
+WEBHOOK_TYPENAME.to_string(),
+Some(String::from("name")),
+WEBHOOK_SCHEMA,
+));
+}
 config
 }
 
diff --git a/proxmox-notify/src/endpoints/mod.rs 
b/proxmox-notify/src/endpoints/mod.rs
index 97f79fcc..f20bee21 100644
--- a/proxmox-notify/src/endpoints/mod.rs
+++ b/proxmox-notify/src/endpoints/mod.rs
@@ -4,5 +4,7 @@ pub mod gotify;
 pub mod sendmail;
 #[cfg(feature = "smtp")]
 pub mod smtp;
+#[cfg(feature = "webhook")]
+pub mod webhook;
 
 mod common;
diff --git a/proxmox-notify/src/endpoints/webhook.rs 
b/proxmox-notify/src/endpoints/webhook.rs
new file mode 100644
index ..7e976f6b
--- /dev/null
+++ b/proxmox-notify/src/endpoints/webhook.rs
@@ -0,0 +1,509 @@
+use handlebars::{
+  

[pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints

2024-07-12 Thread Lukas Wagner
These just call the API implementation via the perl-rs bindings.

Signed-off-by: Lukas Wagner 
---
 PVE/API2/Cluster/Notifications.pm | 263 +-
 1 file changed, 262 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Cluster/Notifications.pm 
b/PVE/API2/Cluster/Notifications.pm
index 10b611c9..eae2d436 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
{ name => 'gotify' },
{ name => 'sendmail' },
{ name => 'smtp' },
+   { name => 'webhook' },
];
 
return $result;
@@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
'type' => {
description => 'Type of the target.',
type  => 'string',
-   enum => [qw(sendmail gotify smtp)],
+   enum => [qw(sendmail gotify smtp webhook)],
},
'comment' => {
description => 'Comment',
@@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
 }
 });
 
+my $webhook_properties = {
+name => {
+   description => 'The name of the endpoint.',
+   type => 'string',
+   format => 'pve-configid',
+},
+url => {
+   description => 'Server URL',
+   type => 'string',
+},
+method => {
+   description => 'HTTP method',
+   type => 'string',
+   enum => [qw(post put get)],
+},
+header => {
+   description => 'HTTP headers to set. These have to be formatted as'
+ . ' a property string in the format name=,value=',
+   type => 'array',
+   items => {
+   type => 'string',
+   },
+   optional => 1,
+},
+body => {
+   description => 'HTTP body, base64 encoded',
+   type => 'string',
+   optional => 1,
+},
+secret => {
+   description => 'Secrets to set. These have to be formatted as'
+ . ' a property string in the format name=,value=',
+   type => 'array',
+   items => {
+   type => 'string',
+   },
+   optional => 1,
+},
+comment => {
+   description => 'Comment',
+   type => 'string',
+   optional => 1,
+},
+disable => {
+   description => 'Disable this target',
+   type => 'boolean',
+   optional => 1,
+   default => 0,
+},
+};
+
+__PACKAGE__->register_method ({
+name => 'get_webhook_endpoints',
+path => 'endpoints/webhook',
+method => 'GET',
+description => 'Returns a list of all webhook endpoints',
+protected => 1,
+permissions => {
+   check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
+   check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
+},
+parameters => {
+   additionalProperties => 0,
+   properties => {},
+},
+returns => {
+   type => 'array',
+   items => {
+   type => 'object',
+   properties => {
+   %$webhook_properties,
+   'origin' => {
+   description => 'Show if this entry was created by a user or 
was built-in',
+   type  => 'string',
+   enum => [qw(user-created builtin modified-builtin)],
+   },
+   },
+   },
+   links => [ { rel => 'child', href => '{name}' } ],
+},
+code => sub {
+   my $config = PVE::Notify::read_config();
+   my $rpcenv = PVE::RPCEnvironment::get();
+
+   my $entities = eval {
+   $config->get_webhook_endpoints();
+   };
+   raise_api_error($@) if $@;
+
+   return $entities;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_webhook_endpoint',
+path => 'endpoints/webhook/{name}',
+method => 'GET',
+description => 'Return a specific webhook endpoint',
+protected => 1,
+permissions => {
+   check => ['or',
+   ['perm', '/mapping/notifications', ['Mapping.Modify']],
+   ['perm', '/mapping/notifications', ['Mapping.Audit']],
+   ],
+},
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   name => {
+   type => 'string',
+   format => 'pve-configid',
+   description => 'Name of the endpoint.'
+   },
+   }
+},
+returns => {
+   type => 'object',
+   properties => {
+   %$webhook_properties,
+   digest => get_standard_option('pve-config-digest'),
+   }
+},
+code => sub {
+   my ($param) = @_;
+   my $name = extract_param($param, 'name');
+
+   my $config = PVE::Notify::read_config();
+   my $endpoint = eval {
+   

[pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets

2024-07-12 Thread Lukas Wagner
Copied and adapted from the Gotify ones.

Signed-off-by: Lukas Wagner 
---
 src/api2/config/notifications/mod.rs |   2 +
 src/api2/config/notifications/webhook.rs | 175 +++
 2 files changed, 177 insertions(+)
 create mode 100644 src/api2/config/notifications/webhook.rs

diff --git a/src/api2/config/notifications/mod.rs 
b/src/api2/config/notifications/mod.rs
index dfe82ed0..81ca9800 100644
--- a/src/api2/config/notifications/mod.rs
+++ b/src/api2/config/notifications/mod.rs
@@ -22,6 +22,7 @@ pub mod matchers;
 pub mod sendmail;
 pub mod smtp;
 pub mod targets;
+pub mod webhook;
 
 #[sortable]
 const SUBDIRS: SubdirMap = !([
@@ -41,6 +42,7 @@ const ENDPOINT_SUBDIRS: SubdirMap = !([
 ("gotify", ::ROUTER),
 ("sendmail", ::ROUTER),
 ("smtp", ::ROUTER),
+("webhook", ::ROUTER),
 ]);
 
 const ENDPOINT_ROUTER: Router = Router::new()
diff --git a/src/api2/config/notifications/webhook.rs 
b/src/api2/config/notifications/webhook.rs
new file mode 100644
index ..4a040024
--- /dev/null
+++ b/src/api2/config/notifications/webhook.rs
@@ -0,0 +1,175 @@
+use anyhow::Error;
+use serde_json::Value;
+
+use proxmox_notify::endpoints::webhook::{
+DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
+};
+use proxmox_notify::schema::ENTITY_NAME_SCHEMA;
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, 
PROXMOX_CONFIG_DIGEST_SCHEMA};
+
+#[api(
+protected: true,
+input: {
+properties: {},
+},
+returns: {
+description: "List of webhook endpoints.",
+type: Array,
+items: { type: WebhookConfig },
+},
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_AUDIT, false),
+},
+)]
+/// List all webhook endpoints.
+pub fn list_endpoints(
+_param: Value,
+_rpcenv:  dyn RpcEnvironment,
+) -> Result, Error> {
+let config = pbs_config::notifications::config()?;
+
+let endpoints = proxmox_notify::api::webhook::get_endpoints()?;
+
+Ok(endpoints)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+name: {
+schema: ENTITY_NAME_SCHEMA,
+}
+},
+},
+returns: { type: WebhookConfig },
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_AUDIT, false),
+},
+)]
+/// Get a webhook endpoint.
+pub fn get_endpoint(name: String, rpcenv:  dyn RpcEnvironment) -> 
Result {
+let config = pbs_config::notifications::config()?;
+let endpoint = proxmox_notify::api::webhook::get_endpoint(, )?;
+
+rpcenv["digest"] = hex::encode(config.digest()).into();
+
+Ok(endpoint)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+endpoint: {
+type: WebhookConfig,
+flatten: true,
+},
+},
+},
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Add a new webhook endpoint.
+pub fn add_endpoint(
+endpoint: WebhookConfig,
+_rpcenv:  dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config()?;
+
+proxmox_notify::api::webhook::add_endpoint( config, endpoint)?;
+
+pbs_config::notifications::save_config(config)?;
+Ok(())
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+name: {
+schema: ENTITY_NAME_SCHEMA,
+},
+updater: {
+type: WebhookConfigUpdater,
+flatten: true,
+},
+delete: {
+description: "List of properties to delete.",
+type: Array,
+optional: true,
+items: {
+type: DeleteableWebhookProperty,
+}
+},
+digest: {
+optional: true,
+schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+},
+},
+},
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Update webhook endpoint.
+pub fn update_endpoint(
+name: String,
+updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option,
+_rpcenv:  dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config()?;
+let digest = digest.map(hex::decode).transpose()?;
+
+proxmox_notify::api::webhook::update_endpoint(
+ config,
+,
+updater,
+delete.as_deref()

[pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window

2024-07-12 Thread Lukas Wagner
This allows users to add/edit new webhook targets.

Signed-off-by: Lukas Wagner 
---
 www/Utils.js | 5 +
 1 file changed, 5 insertions(+)

diff --git a/www/Utils.js b/www/Utils.js
index 4853be36..b715972f 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -482,6 +482,11 @@ Ext.define('PBS.Utils', {
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
+   webhook: {
+   name: 'Webhook',
+   ipanel: 'pmxWebhookEditPanel',
+   iconCls: 'fa-bell-o',
+   },
};
 },
 
-- 
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 v2 06/12] api: notifications: use get_targets impl from proxmox-notify

2024-07-12 Thread Lukas Wagner
The get_targets API endpoint is now implemented in Rust.

Signed-off-by: Lukas Wagner 
---
 PVE/API2/Cluster/Notifications.pm | 34 +--
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/PVE/API2/Cluster/Notifications.pm 
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..10b611c9 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -170,39 +170,7 @@ __PACKAGE__->register_method ({
my $config = PVE::Notify::read_config();
 
my $targets = eval {
-   my $result = [];
-
-   for my $target (@{$config->get_sendmail_endpoints()}) {
-   push @$result, {
-   name => $target->{name},
-   comment => $target->{comment},
-   type => 'sendmail',
-   disable => $target->{disable},
-   origin => $target->{origin},
-   };
-   }
-
-   for my $target (@{$config->get_gotify_endpoints()}) {
-   push @$result, {
-   name => $target->{name},
-   comment => $target->{comment},
-   type => 'gotify',
-   disable => $target->{disable},
-   origin => $target->{origin},
-   };
-   }
-
-   for my $target (@{$config->get_smtp_endpoints()}) {
-   push @$result, {
-   name => $target->{name},
-   comment => $target->{comment},
-   type => 'smtp',
-   disable => $target->{disable},
-   origin => $target->{origin},
-   };
-   }
-
-   $result
+   $config->get_targets();
};
 
raise_api_error($@) if $@;
-- 
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 proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets

2024-07-12 Thread Lukas Wagner
This allows us to drop the impl of that function on the perl side.

Signed-off-by: Lukas Wagner 
---
 common/src/notify.rs | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/src/notify.rs b/common/src/notify.rs
index fe192d5..0f8a35d 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -27,6 +27,7 @@ mod export {
 MatcherConfigUpdater, SeverityMatcher,
 };
 use proxmox_notify::{api, Config, Notification, Severity};
+use proxmox_notify::api::Target;
 
 pub struct NotificationConfig {
 config: Mutex,
@@ -112,6 +113,14 @@ mod export {
 api::common::send(, )
 }
 
+#[export(serialize_error)]
+fn get_targets(
+#[try_from_ref] this: ,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::get_targets()
+}
+
 #[export(serialize_error)]
 fn test_target(
 #[try_from_ref] this: ,
-- 
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 proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes

2024-07-12 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 common/src/notify.rs | 63 
 1 file changed, 63 insertions(+)

diff --git a/common/src/notify.rs b/common/src/notify.rs
index e1b006b..fe192d5 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -19,6 +19,9 @@ mod export {
 DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, 
SmtpPrivateConfig,
 SmtpPrivateConfigUpdater,
 };
+use proxmox_notify::endpoints::webhook::{
+DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
+};
 use proxmox_notify::matcher::{
 CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, 
MatchModeOperator, MatcherConfig,
 MatcherConfigUpdater, SeverityMatcher,
@@ -393,6 +396,66 @@ mod export {
 api::smtp::delete_endpoint( config, name)
 }
 
+#[export(serialize_error)]
+fn get_webhook_endpoints(
+#[try_from_ref] this: ,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoints()
+}
+
+#[export(serialize_error)]
+fn get_webhook_endpoint(
+#[try_from_ref] this: ,
+id: ,
+) -> Result {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoint(, id)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn add_webhook_endpoint(
+#[try_from_ref] this: ,
+endpoint_config: WebhookConfig,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::add_endpoint(
+ config,
+endpoint_config,
+)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn update_webhook_endpoint(
+#[try_from_ref] this: ,
+name: ,
+config_updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option<>,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+let digest = decode_digest(digest)?;
+
+api::webhook::update_endpoint(
+ config,
+name,
+config_updater,
+delete.as_deref(),
+digest.as_deref(),
+)
+}
+
+#[export(serialize_error)]
+fn delete_webhook_endpoint(
+#[try_from_ref] this: ,
+name: ,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::delete_endpoint( config, name)
+}
+
 #[export(serialize_error)]
 fn get_matchers(
 #[try_from_ref] this: ,
-- 
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 08/12] notification: add documentation for webhook target endpoints.

2024-07-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 notifications.adoc | 93 ++
 1 file changed, 93 insertions(+)

diff --git a/notifications.adoc b/notifications.adoc
index 25a9391..b46f1d5 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -178,6 +178,99 @@ gotify: example
 token somesecrettoken
 
 
+[[notification_targets_webhook]]
+Webhook
+~~~
+
+Webhook notification targets perform HTTP requests to a configurable URL.
+
+The following configuration options are available:
+
+* `url`: The URL to which to perform the HTTP requests. 
+Supports templating to inject message contents, metadata and secrets.
+* `method`: HTTP Method to use (POST/PUT/GET)
+* `header`: Array of HTTP headers that should be set for the request.
+Supports templating to inject message contents, metadata and secrets.
+* `body`: HTTP body that should be sent.
+Supports templating to inject message contents, metadata and secrets.
+* `secret`: Array of secret key-value pairs. These will be stored in
+a protected configuration file only readable by root. Secrets can be
+accessed in body/header/URL templates via the `secrets` namespace.
+* `comment`: Comment for this target.
+
+For configuration options that support templating, the
+https://handlebarsjs.com/[Handlebars] syntax can be used to
+access the following properties:
+
+* `{{ title }}`: The rendered notification title
+* `{{ message }}`: The rendered notification body
+* `{{ severity }}`: The severity of the notification (`info`, `notice`, 
+`warning`, `error`, `unknown`)
+* `{{ timestamp }}`: The notification's timestamp as a UNIX epoch (in seconds).
+* `{{ fields. }}`: Sub-namespace for any metadata fields of the 
notification. 
+For instance, `fields.type` contains the notification type - for all available 
fields refer
+to xref:notification_events[Notification Events].
+* `{{ secrets. }}`: Sub-namespace for secrets. For instance, a secret 
named `token`
+is accessible via `secrets.token`.
+
+For convenience, the following helpers are available:
+
+* `{{ url-encode  }}`: URL-encode a property/literal.
+* `{{ escape  }}`: Escape any control characters that cannot be
+safely represented as a JSON string.
+* `{{ json  }}`: Render a value as JSON. This can be useful to
+pass a whole sub-namespace (e.g. `fields`) as a part of a JSON payload
+(e.g. `{{ json fields }}`).
+
+ Examples
+
+= `ntfy.sh`
+
+* Method: `POST` 
+* URL: `https://ntfy.sh/{{ secrets.channel }}`
+* Headers:
+** `Markdown`: `Yes`
+* Body:
+
+```
+{{ message }}
+```
+
+* Secrets:
+** `channel`: ``
+
+= Discord
+
+* Method: `POST`
+* URL: `https://discord.com/api/webhooks/{{ secrets.token }}`
+* Headers:
+** `Content-Type`: `application/json`
+* Body:
+
+{
+  "content": "``` {{ escape message }}```"
+}
+
+* Secrets:
+** `token`: ``
+
+= Slack
+
+* Method: `POST`
+* URL: `https://hooks.slack.com/services/{{ secrets.token }}`
+* Headers:
+** `Content-Type`: `application/json`
+* Body:
+
+{
+  "text": "``` {{escape message}}```",
+  "type": "mrkdwn"
+}
+
+* Secrets:
+** `token`: ``
+
+
 [[notification_matchers]]
 Notification Matchers
 -
-- 
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 proxmox-backup 11/12] docs: notification: add webhook endpoint documentation

2024-07-10 Thread Lukas Wagner
Same information as in pve-docs but translated to restructured text.

Signed-off-by: Lukas Wagner 
---
 docs/notifications.rst | 100 +
 1 file changed, 100 insertions(+)

diff --git a/docs/notifications.rst b/docs/notifications.rst
index 4ba8db86..d059fa76 100644
--- a/docs/notifications.rst
+++ b/docs/notifications.rst
@@ -85,6 +85,106 @@ integrate with different platforms and services.
 
 See :ref:`notifications.cfg` for all configuration options.
 
+.. _notification_targets_webhook:
+Webhook
+^^^
+Webhook notification targets perform HTTP requests to a configurable URL.
+
+The following configuration options are available:
+
+* ``url``: The URL to which to perform the HTTP requests. 
+  Supports templating to inject message contents, metadata and secrets.
+* ``method``: HTTP Method to use (POST/PUT/GET)
+* ``header``: Array of HTTP headers that should be set for the request.
+  Supports templating to inject message contents, metadata and secrets.
+* ``body``: HTTP body that should be sent.
+  Supports templating to inject message contents, metadata and secrets.
+* ``secret``: Array of secret key-value pairs. These will be stored in
+  a protected configuration file only readable by root. Secrets can be
+  accessed in body/header/URL templates via the ``secrets`` namespace.
+* ``comment``: Comment for this target.
+
+For configuration options that support templating, the
+`Handlebars <https://handlebarsjs.com>`_ syntax can be used to
+access the following properties:
+
+* ``{{ title }}``: The rendered notification title
+* ``{{ message }}``: The rendered notification body
+* ``{{ severity }}``: The severity of the notification (``info``, ``notice``, 
+  ``warning``, ``error``, ``unknown``)
+* ``{{ timestamp }}``: The notification's timestamp as a UNIX epoch (in 
seconds).
+* ``{{ fields. }}``: Sub-namespace for any metadata fields of the 
+  notification. For instance, ``fields.type`` contains the notification
+  type - for all available fields refer to :ref:`notification_events`.
+* ``{{ secrets. }}``: Sub-namespace for secrets. For instance, a secret
+  named ``token`` is accessible via ``secrets.token``.
+
+For convenience, the following helpers are available:
+
+* ``{{ url-encode  }}``: URL-encode a property/literal.
+* ``{{ escape  }}``: Escape any control characters that cannot
+  be safely represented as a JSON string.
+* ``{{ json  }}``: Render a value as JSON. This can be useful
+  to pass a whole sub-namespace (e.g. ``fields``) as a part of a JSON payload
+  (e.g. ``{{ json fields }}``).
+
+Example - ntfy.sh
+"""""""""""""""""
+
+* Method: ``POST``
+* URL: ``https://ntfy.sh/{{ secrets.channel }}``
+* Headers:
+
+  * ``Markdown``: ``Yes``
+* Body::
+
+```
+{{ message }}
+```
+
+* Secrets:
+
+  * ``channel``: 
+
+Example - Discord
+"""""""""""""""""
+
+* Method: ``POST``
+* URL: ``https://discord.com/api/webhooks/{{ secrets.token }}``
+* Headers:
+
+  * ``Content-Type``: ``application/json``
+
+* Body::
+
+{
+  "content": "``` {{ escape message }}```"
+}
+
+* Secrets:
+
+  * ``token``: 
+
+Example - Slack
+"""""""""""""""
+
+* Method: ``POST``
+* URL: ``https://hooks.slack.com/services/{{ secrets.token }}``
+* Headers:
+
+  * ``Content-Type``: ``application/json``
+
+* Body::
+
+{
+  "text": "``` {{escape message}}```",
+  "type": "mrkdwn"
+}
+
+* Secrets:
+
+  * ``token``: 
+
 .. _notification_matchers:
 
 Notification Matchers
-- 
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 05/12] notification: add UI for adding/updating webhook targets

2024-07-10 Thread Lukas Wagner
The widgets for editing the headers/secrets were adapted from
the 'Tag Edit' dialog from PVE's datacenter options.

Apart from that, the new dialog is rather standard. I've decided
to put the http method and url in a single row, mostly to
save space and also to make it analogous to how an actual http request
is structured (VERB URL, followed by headers, followed by the body).

The secrets are a mechanism to store tokens/passwords in the
protected notification config. Secrets are accessible via
templating in the URL, headers and body via {{ secrets.NAME }}.
Secrets can only be set/updated, but not retrieved/displayed.

Signed-off-by: Lukas Wagner 
---
 src/Makefile  |   1 +
 src/Schema.js |   5 +
 src/panel/WebhookEditPanel.js | 417 ++
 3 files changed, 423 insertions(+)
 create mode 100644 src/panel/WebhookEditPanel.js

diff --git a/src/Makefile b/src/Makefile
index 0478251..cfaffd7 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -78,6 +78,7 @@ JSSRC=\
panel/StatusView.js \
panel/TfaView.js\
panel/NotesView.js  \
+   panel/WebhookEditPanel.js   \
window/Edit.js  \
window/PasswordEdit.js  \
window/SafeDestroy.js   \
diff --git a/src/Schema.js b/src/Schema.js
index 42541e0..cd1c306 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -65,6 +65,11 @@ Ext.define('Proxmox.Schema', { // a singleton
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
+   webhook: {
+   name: 'Webhook',
+   ipanel: 'pmxWebhookEditPanel',
+   iconCls: 'fa-bell-o',
+   },
 },
 
 // to add or change existing for product specific ones
diff --git a/src/panel/WebhookEditPanel.js b/src/panel/WebhookEditPanel.js
new file mode 100644
index 000..dfc7f3f
--- /dev/null
+++ b/src/panel/WebhookEditPanel.js
@@ -0,0 +1,417 @@
+Ext.define('Proxmox.panel.WebhookEditPanel', {
+extend: 'Proxmox.panel.InputPanel',
+xtype: 'pmxWebhookEditPanel',
+mixins: ['Proxmox.Mixin.CBind'],
+onlineHelp: 'notification_targets_webhook',
+
+type: 'webhook',
+
+columnT: [
+
+],
+
+column1: [
+   {
+   xtype: 'pmxDisplayEditField',
+   name: 'name',
+   cbind: {
+   value: '{name}',
+   editable: '{isCreate}',
+   },
+   fieldLabel: gettext('Endpoint Name'),
+   allowBlank: false,
+   },
+],
+
+column2: [
+   {
+   xtype: 'proxmoxcheckbox',
+   name: 'enable',
+   fieldLabel: gettext('Enable'),
+   allowBlank: false,
+   checked: true,
+   },
+],
+
+columnB: [
+   {
+   layout: 'hbox',
+   border: false,
+   margin: '0 0 5 0',
+   items: [
+   {
+   xtype: 'displayfield',
+   value: gettext('Method/URL:'),
+   width: 125,
+   },
+   {
+   xtype: 'proxmoxKVComboBox',
+   name: 'method',
+   //fieldLabel: gettext('Method'),
+   editable: false,
+   value: 'post',
+   comboItems: [
+   ['post', 'POST'],
+   ['put', 'PUT'],
+   ['get', 'GET'],
+   ],
+   width: 80,
+   margin: '0 5 0 0',
+   },
+   {
+   xtype: 'proxmoxtextfield',
+   //fieldLabel: gettext('URL'),
+   name: 'url',
+   allowBlank: false,
+   flex: 4,
+   },
+   ],
+   },
+   {
+   xtype: 'pmxWebhookKeyValueList',
+   name: 'header',
+   fieldLabel: gettext('Headers'),
+   maskValues: false,
+   cbind: {
+   isCreate: '{isCreate}',
+   },
+   },
+   {
+   xtype: 'textarea',
+   fieldLabel: gettext('Body'),
+   name: 'body',
+   allowBlank: true,
+   minHeight: '150',
+   fieldStyle: {
+   'font-family': 'monospace',
+   },
+   margin: '15 0 0 0',
+   },
+   {
+   xtype: 'pmxWebhookKeyValueList',
+   name: 'secret',
+   fieldLabel: gettext('Secrets'),
+   maskValues: true,
+   cbind: {
+   isCreate: '{isCreate}',
+   },
+   },
+   {
+   xtype: 'proxmoxtextfield',
+   name: 'comment',
+   fieldLabel: gettext('Comment'),
+   cbind: {
+   deleteEmpty: '{!isCreate}',
+   },
+   },
+],
+
+onSetValues: (values) => {
+   values.enable = !values.disable;
+
+   if (values.body) {
+   values.b

[pve-devel] [PATCH manager 07/12] api: add routes for webhook notification endpoints

2024-07-10 Thread Lukas Wagner
These just call the API implementation via the perl-rs bindings.

Signed-off-by: Lukas Wagner 
---
 PVE/API2/Cluster/Notifications.pm | 263 +-
 1 file changed, 262 insertions(+), 1 deletion(-)

diff --git a/PVE/API2/Cluster/Notifications.pm 
b/PVE/API2/Cluster/Notifications.pm
index 10b611c9..eae2d436 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
{ name => 'gotify' },
{ name => 'sendmail' },
{ name => 'smtp' },
+   { name => 'webhook' },
];
 
return $result;
@@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
'type' => {
description => 'Type of the target.',
type  => 'string',
-   enum => [qw(sendmail gotify smtp)],
+   enum => [qw(sendmail gotify smtp webhook)],
},
'comment' => {
description => 'Comment',
@@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
 }
 });
 
+my $webhook_properties = {
+name => {
+   description => 'The name of the endpoint.',
+   type => 'string',
+   format => 'pve-configid',
+},
+url => {
+   description => 'Server URL',
+   type => 'string',
+},
+method => {
+   description => 'HTTP method',
+   type => 'string',
+   enum => [qw(post put get)],
+},
+header => {
+   description => 'HTTP headers to set. These have to be formatted as'
+ . ' a property string in the format name=,value=',
+   type => 'array',
+   items => {
+   type => 'string',
+   },
+   optional => 1,
+},
+body => {
+   description => 'HTTP body, base64 encoded',
+   type => 'string',
+   optional => 1,
+},
+secret => {
+   description => 'Secrets to set. These have to be formatted as'
+ . ' a property string in the format name=,value=',
+   type => 'array',
+   items => {
+   type => 'string',
+   },
+   optional => 1,
+},
+comment => {
+   description => 'Comment',
+   type => 'string',
+   optional => 1,
+},
+disable => {
+   description => 'Disable this target',
+   type => 'boolean',
+   optional => 1,
+   default => 0,
+},
+};
+
+__PACKAGE__->register_method ({
+name => 'get_webhook_endpoints',
+path => 'endpoints/webhook',
+method => 'GET',
+description => 'Returns a list of all webhook endpoints',
+protected => 1,
+permissions => {
+   check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
+   check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
+},
+parameters => {
+   additionalProperties => 0,
+   properties => {},
+},
+returns => {
+   type => 'array',
+   items => {
+   type => 'object',
+   properties => {
+   %$webhook_properties,
+   'origin' => {
+   description => 'Show if this entry was created by a user or 
was built-in',
+   type  => 'string',
+   enum => [qw(user-created builtin modified-builtin)],
+   },
+   },
+   },
+   links => [ { rel => 'child', href => '{name}' } ],
+},
+code => sub {
+   my $config = PVE::Notify::read_config();
+   my $rpcenv = PVE::RPCEnvironment::get();
+
+   my $entities = eval {
+   $config->get_webhook_endpoints();
+   };
+   raise_api_error($@) if $@;
+
+   return $entities;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_webhook_endpoint',
+path => 'endpoints/webhook/{name}',
+method => 'GET',
+description => 'Return a specific webhook endpoint',
+protected => 1,
+permissions => {
+   check => ['or',
+   ['perm', '/mapping/notifications', ['Mapping.Modify']],
+   ['perm', '/mapping/notifications', ['Mapping.Audit']],
+   ],
+},
+parameters => {
+   additionalProperties => 0,
+   properties => {
+   name => {
+   type => 'string',
+   format => 'pve-configid',
+   description => 'Name of the endpoint.'
+   },
+   }
+},
+returns => {
+   type => 'object',
+   properties => {
+   %$webhook_properties,
+   digest => get_standard_option('pve-config-digest'),
+   }
+},
+code => sub {
+   my ($param) = @_;
+   my $name = extract_param($param, 'name');
+
+   my $config = PVE::Notify::read_config();
+   my $endpoint = eval {
+   

[pve-devel] [PATCH proxmox-mail-forward 12/12] bump proxmox-notify dependency

2024-07-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 Cargo.toml | 2 +-
 debian/control | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index f39d118..49ca079 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,4 +20,4 @@ nix = "0.26"
 syslog = "6.0"
 
 proxmox-sys = "0.5.3"
-proxmox-notify = {version = "0.4", features = ["mail-forwarder", 
"pve-context", "pbs-context"] }
+proxmox-notify = {version = "0.5", features = ["mail-forwarder", 
"pve-context", "pbs-context"] }
diff --git a/debian/control b/debian/control
index 7329a24..0ab74a9 100644
--- a/debian/control
+++ b/debian/control
@@ -6,10 +6,10 @@ Build-Depends: cargo:native,
librust-anyhow-1+default-dev,
librust-log-0.4+default-dev (>= 0.4.17-~~),
librust-nix-0.26+default-dev,
-   librust-proxmox-notify-0.4+default-dev,
-   librust-proxmox-notify-0.4+mail-forwarder-dev,
-   librust-proxmox-notify-0.4+pbs-context-dev,
-   librust-proxmox-notify-0.4+pve-context-dev,
+   librust-proxmox-notify-0.5+default-dev,
+   librust-proxmox-notify-0.5+mail-forwarder-dev,
+   librust-proxmox-notify-0.5+pbs-context-dev,
+   librust-proxmox-notify-0.5+pve-context-dev,
librust-proxmox-sys-0.5+default-dev (>= 0.5.3-~~),
librust-syslog-6+default-dev,
libstd-rust-dev,
-- 
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 proxmox-backup 10/12] ui: utils: enable webhook edit window

2024-07-10 Thread Lukas Wagner
This allows users to add/edit new webhook targets.

Signed-off-by: Lukas Wagner 
---
 www/Utils.js | 5 +
 1 file changed, 5 insertions(+)

diff --git a/www/Utils.js b/www/Utils.js
index 4853be36..b715972f 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -482,6 +482,11 @@ Ext.define('PBS.Utils', {
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
+   webhook: {
+   name: 'Webhook',
+   ipanel: 'pmxWebhookEditPanel',
+   iconCls: 'fa-bell-o',
+   },
};
 },
 
-- 
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 proxmox-perl-rs 04/12] common: notify: add bindings for get_targets

2024-07-10 Thread Lukas Wagner
This allows us to drop the impl of that function on the perl side.

Signed-off-by: Lukas Wagner 
---
 common/src/notify.rs | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/src/notify.rs b/common/src/notify.rs
index fe192d5..0f8a35d 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -27,6 +27,7 @@ mod export {
 MatcherConfigUpdater, SeverityMatcher,
 };
 use proxmox_notify::{api, Config, Notification, Severity};
+use proxmox_notify::api::Target;
 
 pub struct NotificationConfig {
 config: Mutex,
@@ -112,6 +113,14 @@ mod export {
 api::common::send(, )
 }
 
+#[export(serialize_error)]
+fn get_targets(
+#[try_from_ref] this: ,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::get_targets()
+}
+
 #[export(serialize_error)]
 fn test_target(
 #[try_from_ref] this: ,
-- 
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 proxmox-perl-rs 03/12] common: notify: add bindings for webhook API routes

2024-07-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 common/src/notify.rs | 63 
 1 file changed, 63 insertions(+)

diff --git a/common/src/notify.rs b/common/src/notify.rs
index e1b006b..fe192d5 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -19,6 +19,9 @@ mod export {
 DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, 
SmtpPrivateConfig,
 SmtpPrivateConfigUpdater,
 };
+use proxmox_notify::endpoints::webhook::{
+DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
+};
 use proxmox_notify::matcher::{
 CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, 
MatchModeOperator, MatcherConfig,
 MatcherConfigUpdater, SeverityMatcher,
@@ -393,6 +396,66 @@ mod export {
 api::smtp::delete_endpoint( config, name)
 }
 
+#[export(serialize_error)]
+fn get_webhook_endpoints(
+#[try_from_ref] this: ,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoints()
+}
+
+#[export(serialize_error)]
+fn get_webhook_endpoint(
+#[try_from_ref] this: ,
+id: ,
+) -> Result {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoint(, id)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn add_webhook_endpoint(
+#[try_from_ref] this: ,
+endpoint_config: WebhookConfig,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::add_endpoint(
+ config,
+endpoint_config,
+)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn update_webhook_endpoint(
+#[try_from_ref] this: ,
+name: ,
+config_updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option<>,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+let digest = decode_digest(digest)?;
+
+api::webhook::update_endpoint(
+ config,
+name,
+config_updater,
+delete.as_deref(),
+digest.as_deref(),
+)
+}
+
+#[export(serialize_error)]
+fn delete_webhook_endpoint(
+#[try_from_ref] this: ,
+name: ,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::delete_endpoint( config, name)
+}
+
 #[export(serialize_error)]
 fn get_matchers(
 #[try_from_ref] this: ,
-- 
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 proxmox 01/12] notify: implement webhook targets

2024-07-10 Thread Lukas Wagner
This target type allows users to perform HTTP requests to arbitrary
third party (notification) services, for instance
ntfy.sh/Discord/Slack.

The configuration for these endpoints allows one to freely configure
the URL, HTTP Method, headers and body. The URL, header values and
body support handlebars templating to inject notification text,
metadata and secrets. Secrets are stored in the protected
configuration file (e.g. /etc/pve/priv/notification.cfg) as key value
pairs, allowing users to protect sensitive tokens/passwords.
Secrets are accessible in handlebar templating via the secrets.*
namespace, e.g. if there is a secret named 'token', a body
could contain '{{ secrets.token }}' to inject the token into the
payload.

A couple of handlebars helpers are also provided:
  - url-encoding (useful for templating in URLs)
  - escape (escape any control characters in strings)
  - json (print a property as json)

In the configuration, the body, header values and secret values
are stored in base64 encoding so that we can store any string we want.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/Cargo.toml   |   6 +-
 proxmox-notify/src/config.rs|  23 ++
 proxmox-notify/src/endpoints/mod.rs |   2 +
 proxmox-notify/src/endpoints/webhook.rs | 509 
 proxmox-notify/src/lib.rs   |  17 +
 5 files changed, 556 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-notify/src/endpoints/webhook.rs

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index d3eae584..d51969fa 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -9,12 +9,15 @@ exclude.workspace = true
 
 [dependencies]
 anyhow.workspace = true
+base64 = { workspace = true, optional = true }
 const_format.workspace = true
 handlebars = { workspace = true }
+http = { workspace = true, optional = true }
 lettre = { workspace = true, optional = true }
 log.workspace = true
 mail-parser = { workspace = true, optional = true }
 openssl.workspace = true
+percent-encoding = { workspace = true, optional = true }
 regex.workspace = true
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
@@ -30,10 +33,11 @@ proxmox-time.workspace = true
 proxmox-uuid = { workspace = true, features = ["serde"] }
 
 [features]
-default = ["sendmail", "gotify", "smtp"]
+default = ["sendmail", "gotify", "smtp", "webhook"]
 mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys"]
 sendmail = ["dep:proxmox-sys"]
 gotify = ["dep:proxmox-http"]
 pve-context = ["dep:proxmox-sys"]
 pbs-context = ["dep:proxmox-sys"]
 smtp = ["dep:lettre"]
+webhook = ["dep:base64", "dep:http", "dep:percent-encoding", 
"dep:proxmox-http"]
diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
index 789c4a7d..4d0b53f7 100644
--- a/proxmox-notify/src/config.rs
+++ b/proxmox-notify/src/config.rs
@@ -57,6 +57,17 @@ fn config_init() -> SectionConfig {
 GOTIFY_SCHEMA,
 ));
 }
+#[cfg(feature = "webhook")]
+{
+use crate::endpoints::webhook::{WebhookConfig, WEBHOOK_TYPENAME};
+
+const WEBHOOK_SCHEMA:  = 
WebhookConfig::API_SCHEMA.unwrap_object_schema();
+config.register_plugin(SectionConfigPlugin::new(
+WEBHOOK_TYPENAME.to_string(),
+Some(String::from("name")),
+WEBHOOK_SCHEMA,
+));
+}
 
 const MATCHER_SCHEMA:  = 
MatcherConfig::API_SCHEMA.unwrap_object_schema();
 config.register_plugin(SectionConfigPlugin::new(
@@ -110,6 +121,18 @@ fn private_config_init() -> SectionConfig {
 ));
 }
 
+#[cfg(feature = "webhook")]
+{
+use crate::endpoints::webhook::{WebhookPrivateConfig, 
WEBHOOK_TYPENAME};
+
+const WEBHOOK_SCHEMA:  =
+WebhookPrivateConfig::API_SCHEMA.unwrap_object_schema();
+config.register_plugin(SectionConfigPlugin::new(
+WEBHOOK_TYPENAME.to_string(),
+Some(String::from("name")),
+WEBHOOK_SCHEMA,
+));
+}
 config
 }
 
diff --git a/proxmox-notify/src/endpoints/mod.rs 
b/proxmox-notify/src/endpoints/mod.rs
index 97f79fcc..f20bee21 100644
--- a/proxmox-notify/src/endpoints/mod.rs
+++ b/proxmox-notify/src/endpoints/mod.rs
@@ -4,5 +4,7 @@ pub mod gotify;
 pub mod sendmail;
 #[cfg(feature = "smtp")]
 pub mod smtp;
+#[cfg(feature = "webhook")]
+pub mod webhook;
 
 mod common;
diff --git a/proxmox-notify/src/endpoints/webhook.rs 
b/proxmox-notify/src/endpoints/webhook.rs
new file mode 100644
index ..7e976f6b
--- /dev/null
+++ b/proxmox-notify/src/endpoints/webhook.rs
@@ -0,0 +1,509 @@
+use handlebars::{
+Context as HandlebarsContext, Handlebars, Helper, HelperResult, Output, 
RenderConte

[pve-devel] [PATCH proxmox 02/12] notify: add api for webhook targets

2024-07-10 Thread Lukas Wagner
All in all pretty similar to other endpoint APIs.
One thing worth noting is how secrets are handled. We never ever
return the values of previously stored secrets in get_endpoint(s)
calls, but only a list of the names of all secrets. This is needed
to build the UI, where we display all secrets that were set before in
a table.

For update calls, one is supposed to send all secrets that should be
kept and updated. If the value should be updated, the name and value
is expected, and if the current value should preseved, only the name
is sent. If a secret's name is not present in the updater, it will be
dropped. If 'secret' is present in the 'delete' array, all secrets
will be dropped, apart from those which are also set/preserved in the
same update call.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/src/api/mod.rs |  20 ++
 proxmox-notify/src/api/webhook.rs | 406 ++
 2 files changed, 426 insertions(+)
 create mode 100644 proxmox-notify/src/api/webhook.rs

diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index a7f6261c..7f823bc7 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -15,6 +15,8 @@ pub mod matcher;
 pub mod sendmail;
 #[cfg(feature = "smtp")]
 pub mod smtp;
+#[cfg(feature = "webhook")]
+pub mod webhook;
 
 // We have our own, local versions of http_err and http_bail, because
 // we don't want to wrap the error in anyhow::Error. If we were to do that,
@@ -54,6 +56,9 @@ pub enum EndpointType {
 /// Gotify endpoint
 #[cfg(feature = "gotify")]
 Gotify,
+/// Webhook endpoint
+#[cfg(feature = "webhook")]
+Webhook,
 }
 
 #[api]
@@ -113,6 +118,17 @@ pub fn get_targets(config: ) -> Result, 
HttpError> {
 })
 }
 
+#[cfg(feature = "webhook")]
+for endpoint in webhook::get_endpoints(config)? {
+targets.push(Target {
+name: endpoint.name,
+origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+endpoint_type: EndpointType::Webhook,
+disable: endpoint.disable,
+comment: endpoint.comment,
+})
+}
+
 Ok(targets)
 }
 
@@ -145,6 +161,10 @@ fn ensure_endpoint_exists(#[allow(unused)] config: 
, name: ) -> Resul
 {
 exists = exists || smtp::get_endpoint(config, name).is_ok();
 }
+#[cfg(feature = "webhook")]
+{
+exists = exists || webhook::get_endpoint(config, name).is_ok();
+}
 
 if !exists {
 http_bail!(NOT_FOUND, "endpoint '{name}' does not exist")
diff --git a/proxmox-notify/src/api/webhook.rs 
b/proxmox-notify/src/api/webhook.rs
new file mode 100644
index ..b7f17c55
--- /dev/null
+++ b/proxmox-notify/src/api/webhook.rs
@@ -0,0 +1,406 @@
+use proxmox_http_error::HttpError;
+use proxmox_schema::property_string::PropertyString;
+
+use crate::api::http_err;
+use crate::endpoints::webhook::{
+DeleteableWebhookProperty, KeyAndBase64Val, WebhookConfig, 
WebhookConfigUpdater,
+WebhookPrivateConfig, WEBHOOK_TYPENAME,
+};
+use crate::{http_bail, Config};
+
+use super::remove_private_config_entry;
+use super::set_private_config_entry;
+
+/// Get a list of all webhook endpoints.
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns a list of all webhook endpoints or a `HttpError` if the config is
+/// erroneous (`500 Internal server error`).
+pub fn get_endpoints(config: ) -> Result, HttpError> 
{
+let mut endpoints: Vec = config
+.config
+.convert_to_typed_array(WEBHOOK_TYPENAME)
+.map_err(|e| http_err!(NOT_FOUND, "Could not fetch endpoints: {e}"))?;
+
+for endpoint in  endpoints {
+let priv_config: WebhookPrivateConfig = config
+.private_config
+.lookup(WEBHOOK_TYPENAME, )
+.unwrap_or_default();
+
+let mut secret_names = Vec::new();
+for secret in priv_config.secret {
+secret_names.push(
+KeyAndBase64Val {
+name: secret.name.clone(),
+value: None,
+}
+.into(),
+)
+}
+
+endpoint.secret = secret_names;
+}
+
+Ok(endpoints)
+}
+
+/// Get webhook endpoint with given `name`
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns the endpoint or a `HttpError` if the endpoint was not found (`404 
Not found`).
+pub fn get_endpoint(config: , name: ) -> Result {
+let mut endpoint: WebhookConfig = config
+.config
+.lookup(WEBHOOK_TYPENAME, name)
+.map_err(|_| http_err!(NOT_FOUND, "endpoint '{name}' not found"))?;
+
+let priv_config: Option = config
+.private_config
+.lookup(WEBHOOK_TYPENAME, )
+.ok();
+
+let mut secret_names = Vec::new();
+if let Some(priv_config) = priv_config {
+for secret in _

[pve-devel] [PATCH manager 06/12] api: notifications: use get_targets impl from proxmox-notify

2024-07-10 Thread Lukas Wagner
The get_targets API endpoint is now implemented in Rust.

Signed-off-by: Lukas Wagner 
---
 PVE/API2/Cluster/Notifications.pm | 34 +--
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/PVE/API2/Cluster/Notifications.pm 
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..10b611c9 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -170,39 +170,7 @@ __PACKAGE__->register_method ({
my $config = PVE::Notify::read_config();
 
my $targets = eval {
-   my $result = [];
-
-   for my $target (@{$config->get_sendmail_endpoints()}) {
-   push @$result, {
-   name => $target->{name},
-   comment => $target->{comment},
-   type => 'sendmail',
-   disable => $target->{disable},
-   origin => $target->{origin},
-   };
-   }
-
-   for my $target (@{$config->get_gotify_endpoints()}) {
-   push @$result, {
-   name => $target->{name},
-   comment => $target->{comment},
-   type => 'gotify',
-   disable => $target->{disable},
-   origin => $target->{origin},
-   };
-   }
-
-   for my $target (@{$config->get_smtp_endpoints()}) {
-   push @$result, {
-   name => $target->{name},
-   comment => $target->{comment},
-   type => 'smtp',
-   disable => $target->{disable},
-   origin => $target->{origin},
-   };
-   }
-
-   $result
+   $config->get_targets();
};
 
raise_api_error($@) if $@;
-- 
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 proxmox-backup 09/12] api: notification: add API routes for webhook targets

2024-07-10 Thread Lukas Wagner
Copied and adapted from the Gotify ones.

Signed-off-by: Lukas Wagner 
---
 src/api2/config/notifications/mod.rs |   2 +
 src/api2/config/notifications/webhook.rs | 175 +++
 2 files changed, 177 insertions(+)
 create mode 100644 src/api2/config/notifications/webhook.rs

diff --git a/src/api2/config/notifications/mod.rs 
b/src/api2/config/notifications/mod.rs
index dfe82ed0..81ca9800 100644
--- a/src/api2/config/notifications/mod.rs
+++ b/src/api2/config/notifications/mod.rs
@@ -22,6 +22,7 @@ pub mod matchers;
 pub mod sendmail;
 pub mod smtp;
 pub mod targets;
+pub mod webhook;
 
 #[sortable]
 const SUBDIRS: SubdirMap = !([
@@ -41,6 +42,7 @@ const ENDPOINT_SUBDIRS: SubdirMap = !([
 ("gotify", ::ROUTER),
 ("sendmail", ::ROUTER),
 ("smtp", ::ROUTER),
+("webhook", ::ROUTER),
 ]);
 
 const ENDPOINT_ROUTER: Router = Router::new()
diff --git a/src/api2/config/notifications/webhook.rs 
b/src/api2/config/notifications/webhook.rs
new file mode 100644
index ..4a040024
--- /dev/null
+++ b/src/api2/config/notifications/webhook.rs
@@ -0,0 +1,175 @@
+use anyhow::Error;
+use serde_json::Value;
+
+use proxmox_notify::endpoints::webhook::{
+DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
+};
+use proxmox_notify::schema::ENTITY_NAME_SCHEMA;
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, 
PROXMOX_CONFIG_DIGEST_SCHEMA};
+
+#[api(
+protected: true,
+input: {
+properties: {},
+},
+returns: {
+description: "List of webhook endpoints.",
+type: Array,
+items: { type: WebhookConfig },
+},
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_AUDIT, false),
+},
+)]
+/// List all webhook endpoints.
+pub fn list_endpoints(
+_param: Value,
+_rpcenv:  dyn RpcEnvironment,
+) -> Result, Error> {
+let config = pbs_config::notifications::config()?;
+
+let endpoints = proxmox_notify::api::webhook::get_endpoints()?;
+
+Ok(endpoints)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+name: {
+schema: ENTITY_NAME_SCHEMA,
+}
+},
+},
+returns: { type: WebhookConfig },
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_AUDIT, false),
+},
+)]
+/// Get a webhook endpoint.
+pub fn get_endpoint(name: String, rpcenv:  dyn RpcEnvironment) -> 
Result {
+let config = pbs_config::notifications::config()?;
+let endpoint = proxmox_notify::api::webhook::get_endpoint(, )?;
+
+rpcenv["digest"] = hex::encode(config.digest()).into();
+
+Ok(endpoint)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+endpoint: {
+type: WebhookConfig,
+flatten: true,
+},
+},
+},
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Add a new webhook endpoint.
+pub fn add_endpoint(
+endpoint: WebhookConfig,
+_rpcenv:  dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config()?;
+
+proxmox_notify::api::webhook::add_endpoint( config, endpoint)?;
+
+pbs_config::notifications::save_config(config)?;
+Ok(())
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+name: {
+schema: ENTITY_NAME_SCHEMA,
+},
+updater: {
+type: WebhookConfigUpdater,
+flatten: true,
+},
+delete: {
+description: "List of properties to delete.",
+type: Array,
+optional: true,
+items: {
+type: DeleteableWebhookProperty,
+}
+},
+digest: {
+optional: true,
+schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+},
+},
+},
+access: {
+permission: ::Privilege(&["system", "notifications"], 
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Update webhook endpoint.
+pub fn update_endpoint(
+name: String,
+updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option,
+_rpcenv:  dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config()?;
+let digest = digest.map(hex::decode).transpose()?;
+
+proxmox_notify::api::webhook::update_endpoint(
+ config,
+,
+updater,
+delete.as_deref()

[pve-devel] [RFC many 00/12] notifications: add support for webhook endpoints

2024-07-10 Thread Lukas Wagner
Sending as an RFC because I don't want this merged yet; that being
said, the feature should be mostly finished at this point, I'd
appreciate any reviews and feedback.

This series adds support for webhook notification targets to PVE
and PBS.

A webhook is a HTTP API route provided by a third-party service that
can be used to inform the third-party about an event. In our case,
we can easily interact with various third-party notification/messaging
systems and send PVE/PBS notifications via this service.
The changes were tested against ntfy.sh, Discord and Slack.

The configuration of webhook targets allows one to configure:
  - The URL
  - The HTTP method (GET/POST/PUT)
  - HTTP Headers
  - Body

One can use handlebar templating to inject notification text and metadata
in the url, headers and body.

One challenge is the handling of sensitve tokens and other secrets.
Since the endpoint is completely generic, we cannot know in advance
whether the body/header/url contains sensitive values.
Thus we add 'secrets' which are stored in the protected config only
accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
secrets are accessible in URLs/headers/body via templating:

  Url: https://example.com/{{ secrets.token }}

Secrets can only be set and updated, but never retrieved via the API.
In the UI, secrets are handled like other secret tokens/passwords.

Bumps for PVE:
  - libpve-rs-perl needs proxmox-notify bumped
  - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
  - proxmox-mail-forward needs proxmox-notify bumped

Bumps for PBS:
  - proxmox-backup needs proxmox-notify bumped
  - proxmox-mail-forward needs proxmox-notify bumped

proxmox:

Lukas Wagner (2):
  notify: implement webhook targets
  notify: add api for webhook targets

 proxmox-notify/Cargo.toml   |   6 +-
 proxmox-notify/src/api/mod.rs   |  20 +
 proxmox-notify/src/api/webhook.rs   | 406 +++
 proxmox-notify/src/config.rs|  23 ++
 proxmox-notify/src/endpoints/mod.rs |   2 +
 proxmox-notify/src/endpoints/webhook.rs | 509 
 proxmox-notify/src/lib.rs   |  17 +
 7 files changed, 982 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-notify/src/api/webhook.rs
 create mode 100644 proxmox-notify/src/endpoints/webhook.rs


proxmox-perl-rs:

Lukas Wagner (2):
  common: notify: add bindings for webhook API routes
  common: notify: add bindings for get_targets

 common/src/notify.rs | 72 
 1 file changed, 72 insertions(+)


proxmox-widget-toolkit:

Lukas Wagner (1):
  notification: add UI for adding/updating webhook targets

 src/Makefile  |   1 +
 src/Schema.js |   5 +
 src/panel/WebhookEditPanel.js | 417 ++
 3 files changed, 423 insertions(+)
 create mode 100644 src/panel/WebhookEditPanel.js


pve-manager:

Lukas Wagner (2):
  api: notifications: use get_targets impl from proxmox-notify
  api: add routes for webhook notification endpoints

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


pve-docs:

Lukas Wagner (1):
  notification: add documentation for webhook target endpoints.

 notifications.adoc | 93 ++
 1 file changed, 93 insertions(+)


proxmox-backup:

Lukas Wagner (3):
  api: notification: add API routes for webhook targets
  ui: utils: enable webhook edit window
  docs: notification: add webhook endpoint documentation

 docs/notifications.rst   | 100 +
 src/api2/config/notifications/mod.rs |   2 +
 src/api2/config/notifications/webhook.rs | 175 +++
 www/Utils.js |   5 +
 4 files changed, 282 insertions(+)
 create mode 100644 src/api2/config/notifications/webhook.rs


proxmox-mail-forward:

Lukas Wagner (1):
  bump proxmox-notify dependency

 Cargo.toml | 2 +-
 debian/control | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)


Summary over all repositories:
  19 files changed, 2120 insertions(+), 40 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 widget-toolkit v9 10/13] notification: matcher: move match-severity fields to panel

2024-07-08 Thread 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 
Tested-by: Maximiliano Sandoval 
Reviewed-by: Max Carrara 
---
 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

[pve-devel] [PATCH widget-toolkit v9 08/13] notification: matcher: move match-field formulas to local viewModel

2024-07-08 Thread Lukas Wagner
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner 
Tested-by: Maximiliano Sandoval 
Reviewed-by: Max Carrara 
---
 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: {

[pve-devel] [PATCH docs v9 11/13] notifications: describe new notification metadata fields

2024-07-08 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
Reviewed-by: Max Carrara 
---
 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

2024-07-08 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
Reviewed-by: Max Carrara 
---
 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 manager v9 04/13] api: notification: add API for getting known metadata fields/values

2024-07-08 Thread 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 
Reviewed-by: Max Carrara 
---
 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()})

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

2024-07-08 Thread 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/Notificati

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

2024-07-08 Thread 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 
Reviewed-by: Max Carrara 
---
 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 05/13] ui: utils: add overrides for translatable notification fields/values

2024-07-08 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
Reviewed-by: Max Carrara 
---
 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 widget-toolkit v9 07/13] notification: matcher: match-field: show known fields/values

2024-07-08 Thread 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 
Tested-by: Maximiliano Sandoval 
Reviewed-by: Max Carrara 
---
 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: '{!show

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

2024-07-08 Thread 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 
Reviewed-by: Max Carrara 
---
 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 widget-toolkit v9 09/13] notification: matcher: move match-calendar fields to panel

2024-07-08 Thread 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 
Tested-by: Maximiliano Sandoval 
Reviewed-by: Max Carrara 
---
 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

[pve-devel] [PATCH manager v9 03/13] ui: dc: backup: allow to set custom job id in advanced settings

2024-07-08 Thread 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 
Reviewed-by: Max Carrara 
---
 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 06/13] d/control: bump proxmox-widget-toolkit dependency to 4.1.4

2024-07-08 Thread 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 
Reviewed-by: Max Carrara 
---
 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 docs v9 13/13] notifications: add note regarding when 'job-id' is set for backups

2024-07-08 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
Reviewed-by: Max Carrara 
---
 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



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

2024-07-08 Thread Lukas Wagner
On  2024-07-08 10:12, Max Carrara wrote:
> On Fri Jul 5, 2024 at 3:46 PM CEST, Lukas Wagner wrote:
>> This patch series attempts to improve the user experience when creating
>> notification matchers.
> 
> The below can pretty much just be considered "proofreading" as I haven't
> built and tested your changes, but since you already got a lot of
> feedback on the last couple versions, I think that's fine. ;) Just
> wanted to comment anyway.
> 
> The patches are rather easy to follow, and even though I'm no expert
> when it comes to Ext JS, the UI changes look fine to me too. The new UI
> logic feels (and is) much cleaner than before. There's nothing I can
> otherwise comment on; everything's pretty straight-forward.
> 
> The *only* things I have noticed are rather minor - there are two tiny
> typos in the commit messages of patch 01 and 02, but these can probably
> be fixed when applying the series:
>   01: Last sentence of message - "It it can be considered internal." 
>   02: First sentence of message - "This allows us to access us the [...]"
> 
> That's it otherwise from me - LGTM.
> 
> Reviewed-by: Max Carrara 
> 

Thanks a lot for your review, Max!

I'll send a v9 with the typos fixed and your R-b's added.

-- 
- 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

2024-07-05 Thread 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  

[pve-devel] [PATCH widget-toolkit v8 07/13] notification: matcher: match-field: show known fields/values

2024-07-05 Thread 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 
Tested-by: Maximiliano Sandoval 
---
 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: '{!show

[pve-devel] [PATCH widget-toolkit v8 10/13] notification: matcher: move match-severity fields to panel

2024-07-05 Thread 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 
Tested-by: Maximiliano Sandoval 
---
 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

[pve-devel] [PATCH widget-toolkit v8 08/13] notification: matcher: move match-field formulas to local viewModel

2024-07-05 Thread Lukas Wagner
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner 
Tested-by: Maximiliano Sandoval 
---
 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: {
+ 

[pve-devel] [PATCH docs v8 11/13] notifications: describe new notification metadata fields

2024-07-05 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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 v8 13/13] notifications: add note regarding when 'job-id' is set for backups

2024-07-05 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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] [PATCH many v8 00/13] notifications: notification metadata matching improvements

2024-07-05 Thread 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:
  - 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, 

[pve-devel] [PATCH manager v8 06/13] d/control: bump proxmox-widget-toolkit dependency to 4.1.4

2024-07-05 Thread 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 
---
 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 pve-guest-common v8 01/13] vzdump: common: allow 'job-id' as a parameter without being in schema

2024-07-05 Thread 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 
---
 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 widget-toolkit v8 09/13] notification: matcher: move match-calendar fields to panel

2024-07-05 Thread 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 
Tested-by: Maximiliano Sandoval 
---
 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 docs v8 12/13] notifications: match-field 'exact'-mode can now match multiple values

2024-07-05 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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 manager v8 05/13] ui: utils: add overrides for translatable notification fields/values

2024-07-05 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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 v8 04/13] api: notification: add API for getting known metadata fields/values

2024-07-05 Thread 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 
---
 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 => $

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

2024-07-05 Thread 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.
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 
---
 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 v8 03/13] ui: dc: backup: allow to set custom job id in advanced settings

2024-07-05 Thread 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 
---
 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



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

2024-07-04 Thread 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


[pve-devel] [PATCH proxmox 4/6] notify: move mail formatting to separate function

2024-06-24 Thread Lukas Wagner
This way we can test this in a sane manner and refactor
safely.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/src/endpoints/sendmail.rs | 109 +--
 1 file changed, 81 insertions(+), 28 deletions(-)

diff --git a/proxmox-notify/src/endpoints/sendmail.rs 
b/proxmox-notify/src/endpoints/sendmail.rs
index 0f7a61b0..241a2578 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -177,16 +177,13 @@ fn sendmail(
 mailfrom: ,
 author: ,
 ) -> Result<(), Error> {
-use std::fmt::Write as _;
-
 if mailto.is_empty() {
 return Err(Error::Generic(
 "At least one recipient has to be specified!".into(),
 ));
 }
-let recipients = mailto.join(",");
-
 let now = proxmox_time::epoch_i64();
+let body = format_mail(mailto, mailfrom, author, subject, text, html, 
now)?;
 
 let mut sendmail_process = match Command::new("/usr/sbin/sendmail")
 .arg("-B")
@@ -205,13 +202,46 @@ fn sendmail(
 }
 Ok(process) => process,
 };
+
+if let Err(err) = sendmail_process
+.stdin
+.take()
+.unwrap()
+.write_all(body.as_bytes())
+{
+return Err(Error::Generic(format!(
+"couldn't write to sendmail stdin: {err}"
+)));
+};
+
+// wait() closes stdin of the child
+if let Err(err) = sendmail_process.wait() {
+return Err(Error::Generic(format!(
+"sendmail did not exit successfully: {err}"
+)));
+}
+
+Ok(())
+}
+
+fn format_mail(
+mailto: &[],
+mailfrom: ,
+author: ,
+subject: ,
+text: Option<>,
+html: Option<>,
+timestamp: i64,
+) -> Result {
+use std::fmt::Write as _;
+
+let recipients = mailto.join(",");
 let mut is_multipart = false;
 if let (Some(_), Some(_)) = (text, html) {
 is_multipart = true;
 }
-
 let mut body = String::new();
-let boundary = format!("_=_NextPart_001_{}", now);
+let boundary = format!("_=_NextPart_001_{}", timestamp);
 if is_multipart {
 body.push_str("Content-Type: multipart/alternative;\n");
 let _ = writeln!(body, "\tboundary=\"{}\"", boundary);
@@ -226,11 +256,10 @@ fn sendmail(
 }
 let _ = writeln!(body, "From: {} <{}>", author, mailfrom);
 let _ = writeln!(body, "To: {}", );
-let rfc2822_date = proxmox_time::epoch_to_rfc2822(now)
+let rfc2822_date = proxmox_time::epoch_to_rfc2822(timestamp)
 .map_err(|err| Error::Generic(format!("failed to format time: 
{err}")))?;
 let _ = writeln!(body, "Date: {}", rfc2822_date);
 body.push_str("Auto-Submitted: auto-generated;\n");
-
 if is_multipart {
 body.push('\n');
 body.push_str("This is a multi-part message in MIME format.\n");
@@ -256,26 +285,7 @@ fn sendmail(
 let _ = write!(body, "\n--{}--", boundary);
 }
 }
-
-if let Err(err) = sendmail_process
-.stdin
-.take()
-.unwrap()
-.write_all(body.as_bytes())
-{
-return Err(Error::Generic(format!(
-"couldn't write to sendmail stdin: {err}"
-)));
-};
-
-// wait() closes stdin of the child
-if let Err(err) = sendmail_process.wait() {
-return Err(Error::Generic(format!(
-"sendmail did not exit successfully: {err}"
-)));
-}
-
-Ok(())
+Ok(body)
 }
 
 /// Forwards an email message to a given list of recipients.
@@ -342,4 +352,47 @@ mod test {
 );
 assert!(result.is_err());
 }
+
+#[test]
+fn test_format_mail_multipart() {
+let message = format_mail(
+&["Tony Est "],
+"foo...@example.com",
+"Fred Oobar",
+"This is the subject",
+Some("This is the plain body"),
+Some("This is the HTML body"),
+1718977850,
+)
+.expect("format_message failed");
+
+assert_eq!(
+message,
+r#"Content-Type: multipart/alternative;
+   boundary="_=_NextPart_001_1718977850"
+MIME-Version: 1.0
+Subject: This is the subject
+From: Fred Oobar 
+To: Tony Est 
+Date: Fri, 21 Jun 2024 15:50:50 +0200
+Auto-Submitted: auto-generated;
+
+This is a multi-part message in MIME format.
+
+--_=_NextPart_001_1718977850
+Content-Type: text/plain;
+   charset="UTF-8"
+Content-Transfer-Encoding: 8bit
+
+This is the plain body
+--_=_NextPart_001_1718977850
+Content-Type: text/html;
+   charset="UTF-8"
+Content-Transfer-Encoding: 8bit
+
+This is the HTML body
+--_=_NextPart_001_1718977850--"#
+.to_owned()
+);
+}
 }
-- 
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 proxmox 5/6] notify: sendmail: always send multi-part message

2024-06-24 Thread Lukas Wagner
Even if we don't have an HTML template available, we always
send an HTML part (the plain text part wrapped in ) to
improve rendering in certain mail clients. This means
we can simply message formatting, since we do not have to
distinguish between single-part and multi-part messages.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/src/endpoints/sendmail.rs | 81 +---
 1 file changed, 29 insertions(+), 52 deletions(-)

diff --git a/proxmox-notify/src/endpoints/sendmail.rs 
b/proxmox-notify/src/endpoints/sendmail.rs
index 241a2578..c28d9211 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -139,8 +139,8 @@ impl Endpoint for SendmailEndpoint {
 sendmail(
 _str,
 ,
-Some(_part),
-Some(_part),
+_part,
+_part,
 ,
 ,
 )
@@ -172,8 +172,8 @@ impl Endpoint for SendmailEndpoint {
 fn sendmail(
 mailto: &[],
 subject: ,
-text: Option<>,
-html: Option<>,
+text: ,
+html: ,
 mailfrom: ,
 author: ,
 ) -> Result<(), Error> {
@@ -229,26 +229,20 @@ fn format_mail(
 mailfrom: ,
 author: ,
 subject: ,
-text: Option<>,
-html: Option<>,
+text: ,
+html: ,
 timestamp: i64,
 ) -> Result {
 use std::fmt::Write as _;
 
 let recipients = mailto.join(",");
-let mut is_multipart = false;
-if let (Some(_), Some(_)) = (text, html) {
-is_multipart = true;
-}
 let mut body = String::new();
+
 let boundary = format!("_=_NextPart_001_{}", timestamp);
-if is_multipart {
-body.push_str("Content-Type: multipart/alternative;\n");
-let _ = writeln!(body, "\tboundary=\"{}\"", boundary);
-body.push_str("MIME-Version: 1.0\n");
-} else if !subject.is_ascii() {
-body.push_str("MIME-Version: 1.0\n");
-}
+body.push_str("Content-Type: multipart/alternative;\n");
+let _ = writeln!(body, "\tboundary=\"{}\"", boundary);
+body.push_str("MIME-Version: 1.0\n");
+
 if !subject.is_ascii() {
 let _ = writeln!(body, "Subject: =?utf-8?B?{}?=", 
base64::encode(subject));
 } else {
@@ -260,31 +254,21 @@ fn format_mail(
 .map_err(|err| Error::Generic(format!("failed to format time: 
{err}")))?;
 let _ = writeln!(body, "Date: {}", rfc2822_date);
 body.push_str("Auto-Submitted: auto-generated;\n");
-if is_multipart {
-body.push('\n');
-body.push_str("This is a multi-part message in MIME format.\n");
-let _ = write!(body, "\n--{}\n", boundary);
-}
-if let Some(text) = text {
-body.push_str("Content-Type: text/plain;\n");
-body.push_str("\tcharset=\"UTF-8\"\n");
-body.push_str("Content-Transfer-Encoding: 8bit\n");
-body.push('\n');
-body.push_str(text);
-if is_multipart {
-let _ = write!(body, "\n--{}\n", boundary);
-}
-}
-if let Some(html) = html {
-body.push_str("Content-Type: text/html;\n");
-body.push_str("\tcharset=\"UTF-8\"\n");
-body.push_str("Content-Transfer-Encoding: 8bit\n");
-body.push('\n');
-body.push_str(html);
-if is_multipart {
-let _ = write!(body, "\n--{}--", boundary);
-}
-}
+body.push('\n');
+body.push_str("This is a multi-part message in MIME format.\n");
+let _ = write!(body, "\n--{}\n", boundary);
+body.push_str("Content-Type: text/plain;\n");
+body.push_str("\tcharset=\"UTF-8\"\n");
+body.push_str("Content-Transfer-Encoding: 8bit\n");
+body.push('\n');
+body.push_str(text);
+let _ = write!(body, "\n--{}\n", boundary);
+body.push_str("Content-Type: text/html;\n");
+body.push_str("\tcharset=\"UTF-8\"\n");
+body.push_str("Content-Transfer-Encoding: 8bit\n");
+body.push('\n');
+body.push_str(html);
+let _ = write!(body, "\n--{}--", boundary);
 Ok(body)
 }
 
@@ -342,14 +326,7 @@ mod test {
 
 #[test]
 fn email_without_recipients() {
-let result = sendmail(
-&[],
-"Subject2",
-None,
-Some("HTML"),
-"root",
-"Proxmox",
-);
+let result = sendmail(&[], "Subject2", "", "HTML", "root", 
"Proxmox");
 assert!(result.is_err());
 }
 

[pve-devel] [PATCH proxmox 1/6] notify: copy sendmail/forward fn's from proxmox_sys

2024-06-24 Thread Lukas Wagner
proxmox_notify is the only user of those functions, so it makes
sense to move them here. A future commit will mark the
original functions from proxmox_sys as deprecated.

The functions were slightly modified, mostly to not
rely on anyhow for error reporting. Also they
are now private functions.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/Cargo.toml|   1 +
 proxmox-notify/src/endpoints/sendmail.rs | 189 ++-
 2 files changed, 188 insertions(+), 2 deletions(-)

diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index d3eae584..e55be0cc 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -9,6 +9,7 @@ exclude.workspace = true
 
 [dependencies]
 anyhow.workspace = true
+base64.workspace = true
 const_format.workspace = true
 handlebars = { workspace = true }
 lettre = { workspace = true, optional = true }
diff --git a/proxmox-notify/src/endpoints/sendmail.rs 
b/proxmox-notify/src/endpoints/sendmail.rs
index da0c0cc7..e75902fc 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -1,3 +1,6 @@
+use std::io::Write;
+use std::process::{Command, Stdio};
+
 use serde::{Deserialize, Serialize};
 
 use proxmox_schema::api_types::COMMENT_SCHEMA;
@@ -133,7 +136,7 @@ impl Endpoint for SendmailEndpoint {
 .clone()
 .unwrap_or_else(|| context().default_sendmail_author());
 
-proxmox_sys::email::sendmail(
+sendmail(
 _str,
 ,
 Some(_part),
@@ -145,7 +148,7 @@ impl Endpoint for SendmailEndpoint {
 }
 #[cfg(feature = "mail-forwarder")]
 Content::ForwardedMail { raw, uid, .. } => {
-proxmox_sys::email::forward(_str, , raw, 
*uid)
+forward(_str, , raw, *uid)
 .map_err(|err| 
Error::NotifyFailed(self.config.name.clone(), err.into()))
 }
 }
@@ -160,3 +163,185 @@ impl Endpoint for SendmailEndpoint {
 self.config.disable.unwrap_or_default()
 }
 }
+
+/// Sends multi-part mail with text and/or html to a list of recipients
+///
+/// Includes the header `Auto-Submitted: auto-generated`, so that auto-replies
+/// (i.e. OOO replies) won't trigger.
+/// ``sendmail`` is used for sending the mail.
+fn sendmail(
+mailto: &[],
+subject: ,
+text: Option<>,
+html: Option<>,
+mailfrom: Option<>,
+author: Option<>,
+) -> Result<(), Error> {
+use std::fmt::Write as _;
+
+if mailto.is_empty() {
+return Err(Error::Generic(
+"At least one recipient has to be specified!".into(),
+));
+}
+let mailfrom = mailfrom.unwrap_or("root");
+let recipients = mailto.join(",");
+let author = author.unwrap_or("Proxmox Backup Server");
+
+let now = proxmox_time::epoch_i64();
+
+let mut sendmail_process = match Command::new("/usr/sbin/sendmail")
+.arg("-B")
+.arg("8BITMIME")
+.arg("-f")
+.arg(mailfrom)
+.arg("--")
+.args(mailto)
+.stdin(Stdio::piped())
+.spawn()
+{
+Err(err) => {
+return Err(Error::Generic(format!(
+"could not spawn sendmail process: {err}"
+)))
+}
+Ok(process) => process,
+};
+let mut is_multipart = false;
+if let (Some(_), Some(_)) = (text, html) {
+is_multipart = true;
+}
+
+let mut body = String::new();
+let boundary = format!("_=_NextPart_001_{}", now);
+if is_multipart {
+body.push_str("Content-Type: multipart/alternative;\n");
+let _ = writeln!(body, "\tboundary=\"{}\"", boundary);
+body.push_str("MIME-Version: 1.0\n");
+} else if !subject.is_ascii() {
+body.push_str("MIME-Version: 1.0\n");
+}
+if !subject.is_ascii() {
+let _ = writeln!(body, "Subject: =?utf-8?B?{}?=", 
base64::encode(subject));
+} else {
+let _ = writeln!(body, "Subject: {}", subject);
+}
+let _ = writeln!(body, "From: {} <{}>", author, mailfrom);
+let _ = writeln!(body, "To: {}", );
+let rfc2822_date = proxmox_time::epoch_to_rfc2822(now)
+.map_err(|err| Error::Generic(format!("failed to format time: 
{err}")))?;
+let _ = writeln!(body, "Date: {}", rfc2822_date);
+body.push_str("Auto-Submitted: auto-generated;\n");
+
+if is_multipart {
+body.push('\n');
+body.push_str("This is a multi-part message in MIME format.\n");
+let _ = write!(body, "\n--{}\n", boundary);
+}
+if let Some(text) = text {
+body.push_str

[pve-devel] [PATCH proxmox 2/6] sys: mark email fn's as deprecated

2024-06-24 Thread Lukas Wagner
The only user was proxmox-notify which now uses its own
copies of these functions.

Also added #[allow(deprecated)] to the test case cause
we don't want any deprecation warnings when running the
test.

Signed-off-by: Lukas Wagner 
---
 proxmox-sys/src/email.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/proxmox-sys/src/email.rs b/proxmox-sys/src/email.rs
index 85d171d7..eb8fd10a 100644
--- a/proxmox-sys/src/email.rs
+++ b/proxmox-sys/src/email.rs
@@ -10,6 +10,7 @@ use anyhow::{bail, format_err, Error};
 /// Includes the header `Auto-Submitted: auto-generated`, so that auto-replies
 /// (i.e. OOO replies) won't trigger.
 /// ``sendmail`` is used for sending the mail.
+#[deprecated(note="Use proxmox-notify's abstractions instead")]
 pub fn sendmail(
 mailto: &[],
 subject: ,
@@ -114,6 +115,7 @@ pub fn sendmail(
 ///
 /// ``sendmail`` is used for sending the mail, thus `message` must be
 /// compatible with that (the message is piped into stdin unmodified).
+#[deprecated(note="Use proxmox-notify's abstractions instead")]
 pub fn forward(
 mailto: &[],
 mailfrom: ,
@@ -162,6 +164,7 @@ pub fn forward(
 
 #[cfg(test)]
 mod test {
+#![allow(deprecated)]
 use crate::email::sendmail;
 
 #[test]
-- 
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 proxmox 6/6] notify: sendmail: code style improvements

2024-06-24 Thread Lukas Wagner
No functional changes intended.

Signed-off-by: Lukas Wagner 
---
 proxmox-notify/src/endpoints/sendmail.rs | 57 +++-
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/proxmox-notify/src/endpoints/sendmail.rs 
b/proxmox-notify/src/endpoints/sendmail.rs
index c28d9211..42b2d3a8 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -185,7 +185,7 @@ fn sendmail(
 let now = proxmox_time::epoch_i64();
 let body = format_mail(mailto, mailfrom, author, subject, text, html, 
now)?;
 
-let mut sendmail_process = match Command::new("/usr/sbin/sendmail")
+let mut sendmail_process = Command::new("/usr/sbin/sendmail")
 .arg("-B")
 .arg("8BITMIME")
 .arg("-f")
@@ -194,32 +194,18 @@ fn sendmail(
 .args(mailto)
 .stdin(Stdio::piped())
 .spawn()
-{
-Err(err) => {
-return Err(Error::Generic(format!(
-"could not spawn sendmail process: {err}"
-)))
-}
-Ok(process) => process,
-};
+.map_err(|err| Error::Generic(format!("could not spawn sendmail 
process: {err}")))?;
 
-if let Err(err) = sendmail_process
+sendmail_process
 .stdin
 .take()
-.unwrap()
+.expect("stdin already taken")
 .write_all(body.as_bytes())
-{
-return Err(Error::Generic(format!(
-"couldn't write to sendmail stdin: {err}"
-)));
-};
-
-// wait() closes stdin of the child
-if let Err(err) = sendmail_process.wait() {
-return Err(Error::Generic(format!(
-"sendmail did not exit successfully: {err}"
-)));
-}
+.map_err(|err| Error::Generic(format!("couldn't write to sendmail 
stdin: {err}")))?;
+
+sendmail_process
+.wait()
+.map_err(|err| Error::Generic(format!("sendmail did not exit 
successfully: {err}")))?;
 
 Ok(())
 }
@@ -236,39 +222,46 @@ fn format_mail(
 use std::fmt::Write as _;
 
 let recipients = mailto.join(",");
+let boundary = format!("_=_NextPart_001_{timestamp}");
+
 let mut body = String::new();
 
-let boundary = format!("_=_NextPart_001_{}", timestamp);
+// Format email header
 body.push_str("Content-Type: multipart/alternative;\n");
-let _ = writeln!(body, "\tboundary=\"{}\"", boundary);
+let _ = writeln!(body, "\tboundary=\"{boundary}\"");
 body.push_str("MIME-Version: 1.0\n");
 
 if !subject.is_ascii() {
 let _ = writeln!(body, "Subject: =?utf-8?B?{}?=", 
base64::encode(subject));
 } else {
-let _ = writeln!(body, "Subject: {}", subject);
+let _ = writeln!(body, "Subject: {subject}");
 }
-let _ = writeln!(body, "From: {} <{}>", author, mailfrom);
-let _ = writeln!(body, "To: {}", );
+let _ = writeln!(body, "From: {author} <{mailfrom}>");
+let _ = writeln!(body, "To: {recipients}");
 let rfc2822_date = proxmox_time::epoch_to_rfc2822(timestamp)
 .map_err(|err| Error::Generic(format!("failed to format time: 
{err}")))?;
-let _ = writeln!(body, "Date: {}", rfc2822_date);
+let _ = writeln!(body, "Date: {rfc2822_date}");
 body.push_str("Auto-Submitted: auto-generated;\n");
 body.push('\n');
+
+// Format email body
 body.push_str("This is a multi-part message in MIME format.\n");
-let _ = write!(body, "\n--{}\n", boundary);
+let _ = write!(body, "\n--{boundary}\n");
+
 body.push_str("Content-Type: text/plain;\n");
 body.push_str("\tcharset=\"UTF-8\"\n");
 body.push_str("Content-Transfer-Encoding: 8bit\n");
 body.push('\n');
 body.push_str(text);
-let _ = write!(body, "\n--{}\n", boundary);
+let _ = write!(body, "\n--{boundary}\n");
+
 body.push_str("Content-Type: text/html;\n");
 body.push_str("\tcharset=\"UTF-8\"\n");
 body.push_str("Content-Transfer-Encoding: 8bit\n");
 body.push('\n');
 body.push_str(html);
-let _ = write!(body, "\n--{}--", boundary);
+let _ = write!(body, "\n--{boundary}--");
+
 Ok(body)
 }
 
-- 
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 proxmox 3/6] notify: sendmail: make mailfrom and author non-optional

2024-06-24 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 proxmox-notify/src/endpoints/sendmail.rs | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/proxmox-notify/src/endpoints/sendmail.rs 
b/proxmox-notify/src/endpoints/sendmail.rs
index e75902fc..0f7a61b0 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -141,8 +141,8 @@ impl Endpoint for SendmailEndpoint {
 ,
 Some(_part),
 Some(_part),
-Some(),
-Some(),
+,
+,
 )
 .map_err(|err| Error::NotifyFailed(self.config.name.clone(), 
err.into()))
 }
@@ -174,8 +174,8 @@ fn sendmail(
 subject: ,
 text: Option<>,
 html: Option<>,
-mailfrom: Option<>,
-author: Option<>,
+mailfrom: ,
+author: ,
 ) -> Result<(), Error> {
 use std::fmt::Write as _;
 
@@ -184,9 +184,7 @@ fn sendmail(
 "At least one recipient has to be specified!".into(),
 ));
 }
-let mailfrom = mailfrom.unwrap_or("root");
 let recipients = mailto.join(",");
-let author = author.unwrap_or("Proxmox Backup Server");
 
 let now = proxmox_time::epoch_i64();
 
@@ -339,8 +337,8 @@ mod test {
 "Subject2",
 None,
 Some("HTML"),
-None,
-Some("test1"),
+"root",
+"Proxmox",
 );
 assert!(result.is_err());
 }
-- 
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 proxmox-perl-rs 15/15] pmg-rs: acme: simplify acount config saving

2024-06-20 Thread Lukas Wagner
We already depend on proxmox_sys, so we can just use
`replace_file`. Fixing a clippy warning (missing
truncate setting for OpenOptions) is an added benefit.

Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/acme.rs | 62 ++
 1 file changed, 13 insertions(+), 49 deletions(-)

diff --git a/pmg-rs/src/acme.rs b/pmg-rs/src/acme.rs
index e2e7327..ca24f17 100644
--- a/pmg-rs/src/acme.rs
+++ b/pmg-rs/src/acme.rs
@@ -2,11 +2,9 @@
 //!
 //! The functions in here are perl bindings.
 
-use std::fs::OpenOptions;
-use std::io::{self, Write};
-use std::os::unix::fs::OpenOptionsExt;
-
 use anyhow::{format_err, Error};
+use nix::sys::stat::Mode;
+use proxmox_sys::fs::CreateOptions;
 use serde::{Deserialize, Serialize};
 
 use proxmox_acme::types::AccountData as AcmeAccountData;
@@ -90,19 +88,12 @@ impl Inner {
 let _account = self
 .client
 .new_account(contact, tos_agreed, rsa_bits, eab_creds)?;
-let file = OpenOptions::new()
-.write(true)
-.create(true)
-.mode(0o600)
-.open(_path)
-.map_err(|err| format_err!("failed to open {:?} for writing: {}", 
account_path, err))?;
-self.write_to(file).map_err(|err| {
-format_err!(
-"failed to write acme account to {:?}: {}",
-account_path,
-err
-)
-})?;
+
+let data = serde_json::to_vec(_account_data()?)?;
+let create_options = 
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(_path, , create_options, 
true)
+.map_err(|err| format_err!("failed to replace ACME account config: 
{err}"))?;
+
 self.account_path = Some(account_path);
 
 Ok(())
@@ -131,12 +122,6 @@ impl Inner {
 })
 }
 
-fn write_to( self, out: T) -> Result<(), Error> {
-let data = self.to_account_data()?;
-
-Ok(serde_json::to_writer_pretty(out, )?)
-}
-
 pub fn update_account( self, data: ) -> Result<(), 
Error> {
 let account_path = self
 .account_path
@@ -144,32 +129,11 @@ impl Inner {
 .ok_or_else(|| format_err!("missing account path"))?;
 self.client.update_account(data)?;
 
-let tmp_path = format!("{}.tmp", account_path);
-// FIXME: move proxmox::tools::replace_file & make_temp out into a 
nice *little* crate...
-let mut file = OpenOptions::new()
-.write(true)
-.create(true)
-.mode(0o600)
-.open(_path)
-.map_err(|err| format_err!("failed to open {:?} for writing: {}", 
tmp_path, err))?;
-self.write_to( file).map_err(|err| {
-format_err!("failed to write acme account to {:?}: {}", tmp_path, 
err)
-})?;
-file.flush().map_err(|err| {
-format_err!("failed to flush acme account file {:?}: {}", 
tmp_path, err)
-})?;
-
-// re-borrow since we needed `self` as mut earlier
-let account_path = self.account_path.as_deref().unwrap();
-std::fs::rename(_path, account_path).map_err(|err| {
-format_err!(
-"failed to rotate temp file into place ({:?} -> {:?}): {}",
-_path,
-account_path,
-err
-)
-})?;
-drop(file);
+let data = serde_json::to_vec(_account_data()?)?;
+let create_options = 
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(account_path, , create_options, 
true)
+.map_err(|err| format_err!("failed to replace ACME account config: 
{err}"))?;
+
 Ok(())
 }
 
-- 
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 proxmox-perl-rs 13/15] pmg-rs: tfa: clippy: useless conversion to the same type

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/tfa.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index af69721..4e9ce8f 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -178,7 +178,7 @@ mod export {
 #[try_from_ref] this: ,
 ) -> Result<(Option, Option), Error> {
 Ok(match this.inner.lock().unwrap().webauthn.clone() {
-Some(config) => (Some(hex::encode(config.digest())), 
Some(config.into())),
+Some(config) => (Some(hex::encode(config.digest())), Some(config)),
 None => (None, None),
 })
 }
-- 
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 proxmox-perl-rs 09/15] pmg-rs: tfa: clippy: unnecessary `pub(self)`

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/tfa.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index 1924488..0680baa 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -17,7 +17,7 @@ use anyhow::{bail, format_err, Error};
 use nix::errno::Errno;
 use nix::sys::stat::Mode;
 
-pub(self) use proxmox_tfa::api::{
+use proxmox_tfa::api::{
 RecoveryState, TfaChallenge, TfaConfig, TfaResponse, U2fConfig, 
UserChallengeAccess,
 WebauthnConfig,
 };
-- 
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 proxmox-perl-rs 10/15] pmg-rs: tfa: clippy: question mark operator is useless here

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/tfa.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index 0680baa..a97d171 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -441,11 +441,11 @@ mod export {
 #[export]
 fn api_unlock_tfa(#[raw] raw_this: Value, userid: ) -> Result {
 let this:  = (_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
  this.inner.lock().unwrap(),
 ::new(_this)?,
 userid,
-)?)
+)
 }
 
 #[derive(serde::Serialize)]
-- 
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 proxmox-perl-rs 14/15] pmg-rs: acme: clippy: reference is immediately deref'd by the compiler

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/acme.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pmg-rs/src/acme.rs b/pmg-rs/src/acme.rs
index 7ea78c6..e2e7327 100644
--- a/pmg-rs/src/acme.rs
+++ b/pmg-rs/src/acme.rs
@@ -403,7 +403,7 @@ pub mod export {
 this.inner
 .lock()
 .unwrap()
-.revoke_certificate(, reason)?;
+.revoke_certificate(data, reason)?;
 Ok(())
 }
 
-- 
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 proxmox-perl-rs 08/15] pve-rs: tfa: clippy: stripping a prefix manually

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 1054169..66dca3d 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -736,10 +736,10 @@ fn decode_old_oath_entry(
 let key = unsafe { std::str::from_utf8_unchecked(key) };
 
 // See PVE::OTP::oath_verify_otp
-let key = if key.starts_with("v2-0x") {
-hex::decode([5..]).map_err(|_| format_err!("bad v2 hex key in 
oath entry"))?
-} else if key.starts_with("v2-") {
-base32::decode(base32::Alphabet::RFC4648 { padding: true }, 
[3..])
+let key = if let Some(key) = key.strip_prefix("v2-0x") {
+hex::decode(key).map_err(|_| format_err!("bad v2 hex key in oath 
entry"))?
+} else if let Some(key) = key.strip_prefix("v2-") {
+base32::decode(base32::Alphabet::RFC4648 { padding: true }, key)
 .ok_or_else(|| format_err!("bad v2 base32 key in oath entry"))?
 } else if key.len() == 16 {
 base32::decode(base32::Alphabet::RFC4648 { padding: true }, key)
-- 
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 proxmox-perl-rs 05/15] pve-rs: tfa: clippy: borrowed expression impls the required traits

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 9381ef0..7588d6d 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -1048,7 +1048,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for 
UserAccess {
 
 fn remove(, userid: ) -> Result {
 let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file() {
+match std::fs::remove_file(path) {
 Ok(()) => Ok(true),
 Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(false),
 Err(err) => Err(err.into()),
-- 
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 proxmox-perl-rs 12/15] pmg-rs: tfa: clippy: the borrowed expression implements the required traits

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/tfa.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index 928b50b..af69721 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -178,7 +178,7 @@ mod export {
 #[try_from_ref] this: ,
 ) -> Result<(Option, Option), Error> {
 Ok(match this.inner.lock().unwrap().webauthn.clone() {
-Some(config) => (Some(hex::encode(())), 
Some(config.into())),
+Some(config) => (Some(hex::encode(config.digest())), 
Some(config.into())),
 None => (None, None),
 })
 }
@@ -644,7 +644,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for UserAccess 
{
 
 fn remove(, userid: ) -> Result {
 let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file() {
+match std::fs::remove_file(path) {
 Ok(()) => Ok(true),
 Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(false),
 Err(err) => Err(err.into()),
-- 
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 proxmox-perl-rs 06/15] pve-rs: tfa: clippy: accessing first element with `.get(0)`

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 7588d6d..7ead18c 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -831,7 +831,7 @@ fn generate_legacy_config(out:  perlmod::Hash, config: 
) {
 let users = Hash::new();
 
 for (user, data) in  {
-if let Some(u2f) = data.u2f.get(0) {
+if let Some(u2f) = data.u2f.first() {
 let data = Hash::new();
 data.insert(
 "publicKey",
@@ -850,7 +850,7 @@ fn generate_legacy_config(out:  perlmod::Hash, config: 
) {
 continue;
 }
 
-if let Some(totp) = data.totp.get(0) {
+if let Some(totp) = data.totp.first() {
 let totp = 
 let config = Hash::new();
 config.insert("digits", 
Value::new_int(isize::from(totp.digits(;
@@ -873,7 +873,7 @@ fn generate_legacy_config(out:  perlmod::Hash, config: 
) {
 continue;
 }
 
-if let Some(entry) = data.yubico.get(0) {
+if let Some(entry) = data.yubico.first() {
 let mut keys = entry.entry.clone();
 
 for entry in data.yubico.iter().skip(1) {
-- 
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 proxmox-perl-rs 04/15] pve-rs: tfa: clippy: question mark operator is useless here

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 6650151..9381ef0 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -490,11 +490,11 @@ mod export {
 #[export]
 fn api_unlock_tfa(#[raw] raw_this: Value, userid: ) -> Result {
 let this:  = (_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
  this.inner.lock().unwrap(),
 ::new(_this)?,
 userid,
-)?)
+)
 }
 
 #[derive(serde::Serialize)]
-- 
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 proxmox-perl-rs 11/15] pmg-rs: tfa: clippy: this function has too many arguments

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pmg-rs/src/tfa.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index a97d171..928b50b 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -361,6 +361,7 @@ mod export {
 methods::list_tfa(().unwrap(), authid, 
top_level_allowed)
 }
 
+#[allow(clippy::too_many_arguments)]
 #[export]
 fn api_add_tfa_entry(
 #[raw] raw_this: Value,
-- 
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 proxmox-perl-rs 07/15] pve-rs: tfa: clippy: redundant slicing of the whole range

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 7ead18c..1054169 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -802,7 +802,7 @@ fn usize_from_perl(value: JsonValue) -> Option {
 fn trim_ascii_whitespace_start(data: &[u8]) -> &[u8] {
 match data.iter().position(|| !c.is_ascii_whitespace()) {
 Some(from) => [from..],
-None => [..],
+None => data,
 }
 }
 
-- 
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 proxmox-perl-rs 03/15] pve-rs: tfa: clippy: this function has too many arguments

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 798cdad..6650151 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -409,6 +409,7 @@ mod export {
 methods::list_tfa(().unwrap(), authid, 
top_level_allowed)
 }
 
+#[allow(clippy::too_many_arguments)]
 #[export]
 fn api_add_tfa_entry(
 #[raw] raw_this: Value,
-- 
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 proxmox-perl-rs 01/15] pve-rs: apt: clippy: the borrowed expression implements the required traits

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 common/src/apt/repositories.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/src/apt/repositories.rs b/common/src/apt/repositories.rs
index e710819..6e0a196 100644
--- a/common/src/apt/repositories.rs
+++ b/common/src/apt/repositories.rs
@@ -42,7 +42,7 @@ pub mod export {
 #[export]
 pub fn repositories(product: ) -> Result {
 let (files, errors, digest) = 
proxmox_apt::repositories::repositories()?;
-let digest = hex::encode();
+let digest = hex::encode(digest);
 
 let suite = proxmox_apt::repositories::get_current_release_codename()?;
 
-- 
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 proxmox-perl-rs 02/15] pve-rs: tfa: clippy: unnecessary `pub(self)`

2024-06-20 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 pve-rs/src/tfa.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 2b61344..798cdad 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -20,7 +20,7 @@ use nix::errno::Errno;
 use nix::sys::stat::Mode;
 use serde_json::Value as JsonValue;
 
-pub(self) use proxmox_tfa::api::{
+use proxmox_tfa::api::{
 RecoveryState, TfaChallenge, TfaConfig, TfaResponse, TfaUserData, 
U2fConfig,
 UserChallengeAccess, WebauthnConfig,
 };
-- 
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] applied: [PATCH proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml

2024-06-20 Thread Lukas Wagner


On  2024-06-20 12:21, Fabian Grünbichler wrote:
> with a follow-up to adapt the Makefiles - please test builds when
> touching the build system ;)
> 

Sorry, only did a `cargo build` - :S My bad

-- 
- Lukas


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


Re: [pve-devel] [PATCH proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml

2024-06-20 Thread Lukas Wagner
Whoops, seems like I was on stable rust (1.78) and the warning only appears 
there
but not on 1.77 (packaged rust) - but we can apply that already anyway,
saves us the trouble once we are on 1.78 :)

On  2024-06-20 10:59, Lukas Wagner wrote:
> Fixes the following new warning that appeared after switching
> to rust 1.77:
> 
> warning: `proxmox-perl-rs/pve-rs/.cargo/config` is deprecated in
> favor of `config.toml`
> 
> Signed-off-by: Lukas Wagner 
> ---
>  pmg-rs/.cargo/{config => config.toml} | 0
>  pve-rs/.cargo/{config => config.toml} | 0
>  2 files changed, 0 insertions(+), 0 deletions(-)
>  rename pmg-rs/.cargo/{config => config.toml} (100%)
>  rename pve-rs/.cargo/{config => config.toml} (100%)
> 
> diff --git a/pmg-rs/.cargo/config b/pmg-rs/.cargo/config.toml
> similarity index 100%
> rename from pmg-rs/.cargo/config
> rename to pmg-rs/.cargo/config.toml
> diff --git a/pve-rs/.cargo/config b/pve-rs/.cargo/config.toml
> similarity index 100%
> rename from pve-rs/.cargo/config
> rename to pve-rs/.cargo/config.toml

-- 
- Lukas


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



[pve-devel] [PATCH proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml

2024-06-20 Thread Lukas Wagner
Fixes the following new warning that appeared after switching
to rust 1.77:

warning: `proxmox-perl-rs/pve-rs/.cargo/config` is deprecated in
favor of `config.toml`

Signed-off-by: Lukas Wagner 
---
 pmg-rs/.cargo/{config => config.toml} | 0
 pve-rs/.cargo/{config => config.toml} | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename pmg-rs/.cargo/{config => config.toml} (100%)
 rename pve-rs/.cargo/{config => config.toml} (100%)

diff --git a/pmg-rs/.cargo/config b/pmg-rs/.cargo/config.toml
similarity index 100%
rename from pmg-rs/.cargo/config
rename to pmg-rs/.cargo/config.toml
diff --git a/pve-rs/.cargo/config b/pve-rs/.cargo/config.toml
similarity index 100%
rename from pve-rs/.cargo/config
rename to pve-rs/.cargo/config.toml
-- 
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 19/19] notifications: backport some rephrased parts from PBS docs

2024-06-10 Thread 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 
---
 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
+ema

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

2024-06-10 Thread 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 
Tested-by: Maximiliano Sandoval 
---
 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: '{!show

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

2024-06-10 Thread Lukas Wagner
This should make the code more cohesive and easier to follow.

No functional changes.

Signed-off-by: Lukas Wagner 
Tested-by: Maximiliano Sandoval 
---
 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: {
+ 

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

2024-06-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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 06/19] vzdump: apt: notification: do not include domain in 'hostname' field

2024-06-10 Thread 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 
---
 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] [PATCH manager v7 02/19] api: jobs: vzdump: pass job 'job-id' parameter

2024-06-10 Thread 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 
---
 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] [PATCH pve-guest-common v7 01/19] vzdump: common: allow 'job-id' as a parameter without being in schema

2024-06-10 Thread 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 
---
 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 widget-toolkit v7 14/19] notification: matcher: move match-severity fields to panel

2024-06-10 Thread 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 
Tested-by: Maximiliano Sandoval 
---
 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

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

2024-06-10 Thread 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 
Tested-by: Maximiliano Sandoval 
---
 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 manager v7 08/19] api: notification: add API for getting known metadata fields/values

2024-06-10 Thread 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 
---
 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 => $

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

2024-06-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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 18/19] notifications: fix typo in 'notification'

2024-06-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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] [PATCH docs v7 15/19] notification: clarify that 'hostname' does not include the domain

2024-06-10 Thread 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 
---
 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] [PATCH docs v7 17/19] notifications: match-field 'exact'-mode can now match multiple values

2024-06-10 Thread Lukas Wagner
Signed-off-by: Lukas Wagner 
---
 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 manager v7 10/19] d/control: bump proxmox-widget-toolkit dependency to 4.1.4

2024-06-10 Thread 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 
---
 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 many v7 00/19] notifications: notification metadata matching improvements

2024-06-10 Thread 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
  notificatio

  1   2   3   4   5   6   7   8   9   10   >