Re: [pve-devel] [PATCH pve-manager 2/3] Fix #5708: Add CPU raw counters
On 2024-09-24 14:25, Daniel Kral wrote:
> On 9/17/24 07:50, Sascha Westermann via pve-devel wrote:
>> Add a map containing raw values from /proc/stat and "uptime_ticks" which
>> can be used in combination with cpuinfo.user_hz to calculate CPU usage
>> from two samples. "uptime_ticks" is only defined at the top level, as
>> /proc/stat is read once, so that core-specific raw values match this
>> value.
>>
>> Signed-off-by: Sascha Westermann
>> ---
>> PVE/API2/Nodes.pm | 32
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
>> index 9920e977..1943ec56 100644
>> --- a/PVE/API2/Nodes.pm
>> +++ b/PVE/API2/Nodes.pm
>> @@ -5,6 +5,7 @@ use warnings;
>>
>> use Digest::MD5;
>> use Digest::SHA;
>> +use IO::File;
>> use Filesys::Df;
>> use HTTP::Status qw(:constants);
>> use JSON;
>> @@ -466,6 +467,37 @@ __PACKAGE__->register_method({
>
> note: the same route also gets called when using the WebGUI and a set of the
> values that get returned are displayed on the "Node > Status" page. What I
> have seen, the added data size is very negligible.
>
>> $res->{cpu} = $stat->{cpu};
>> $res->{wait} = $stat->{wait};
>>
>> + if (my $fh = IO::File->new ("/proc/stat", "r")) {
>
> nit: Minor note, but there shouldn't be a space between the function's name
> and its parameter list [0].
>
>> + my ($uptime_ticks) = PVE::ProcFSTools::read_proc_uptime(1);
>> + while (defined (my $line = <$fh>)) {
>> + if ($line =~
>> m|^cpu\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)(?:\s+(\d+)\s+(\d+))?|)
>> {
>> + $res->{cpustat}->{user} = int($1);
>> + $res->{cpustat}->{nice} = int($2);
>> + $res->{cpustat}->{system} = int($3);
>> + $res->{cpustat}->{idle} = int($4);
>> + $res->{cpustat}->{iowait} = int($5);
>> + $res->{cpustat}->{irq} = int($6);
>> + $res->{cpustat}->{softirq} = int($7);
>> + $res->{cpustat}->{steal} = int($8);
>> + $res->{cpustat}->{guest} = int($9);
>> + $res->{cpustat}->{guest_nice} = int($10);
>> + $res->{cpustat}->{uptime_ticks} = $uptime_ticks;
>
> nit: I think this could be placed rather nicely at `$res->{uptime_ticks}`,
> like `$res->{uptime}`, to make `cpustat` a little more consistent with
> `PVE::ProcFSTools::read_proc_stat()` and
>
>> + } elsif ($line =~
>> m|^cpu(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)(?:\s+(\d+)\s+(\d+))?|)
>> {
>> + $res->{cpustat}->{"cpu" . $1}->{user} = int($2);
>> + $res->{cpustat}->{"cpu" . $1}->{nice} = int($3);
>> + $res->{cpustat}->{"cpu" . $1}->{system} = int($4);
>> + $res->{cpustat}->{"cpu" . $1}->{idle} = int($5);
>> + $res->{cpustat}->{"cpu" . $1}->{iowait} = int($6);
>> + $res->{cpustat}->{"cpu" . $1}->{irq} = int($7);
>> + $res->{cpustat}->{"cpu" . $1}->{softirq} = int($8);
>> + $res->{cpustat}->{"cpu" . $1}->{steal} = int($9);
>> + $res->{cpustat}->{"cpu" . $1}->{guest} = int($10);
>> + $res->{cpustat}->{"cpu" . $1}->{guest_nice} = int($11);
>> + }
>> + }
>> + $fh->close;
>> + }
>
> Is there something that is holding us back to move this directly into
> `PVE::ProcFSTools::read_proc_stat()`?
>
> As far as I can tell, the output of `PVE::ProcFSTools::read_proc_stat()` is
> used at these locations:
>
> - the PVE `/nodes/{node}/status` API endpoint of course, which only uses the
> values of `cpu` and `wait` at the moment
> - `PMG::API2::Nodes`: also only uses the values of `cpu` and `wait`
> - the PMG `/nodes/{node}/status` API endpoint, which also only uses the
> values of `cpu` and `wait`
> - `PVE::Service::pvestatd::update_node_status`: retrieve the current node
> status and then update them for rrd via `broadcast_rrd` (uses only the values
> of `cpu` and `wait` selectively) and external metric servers
>
> The first three and a half (speaking of `broadcast_rrd` in the latter) look
> fine to me, but we should take a closer look how external metric servers will
> handle the added data, especially for existing queries/dashboards. It could
> also be a name collision, as 'cpustat' is also used for the data that gets
> sent to the metric servers.
Just as a side-note from me, since I recently went down this rabbithole as well:
In the long-term I might be good to add a layer of abstraction between the
cpustat hash produced by
read_proc_stat (and similar functions) and the metric server integration.
Right now, the metric server implementation will add fields for every single
member of the hash to a datapoint which is then sent to the external metric
server. As a result, the cpustat
hash is essentially a public interface at the moment. If we ever change the
format of the hash, we
risk breaking custom monitoring dashboards in our user's set
Re: [pve-devel] partially-applied: [PATCH many v9 00/13] notifications: notification metadata matching improvements
On 2024-07-22 19:36, Thomas Lamprecht wrote: >> Lukas Wagner (5): >> api: jobs: vzdump: pass job 'job-id' parameter >> ui: dc: backup: allow to set custom job id in advanced settings >> api: notification: add API for getting known metadata fields/values >> ui: utils: add overrides for translatable notification fields/values >> d/control: bump proxmox-widget-toolkit dependency to 4.1.4 >> >> PVE/API2/Backup.pm | 2 +- >> PVE/API2/Cluster/Notifications.pm | 139 >> PVE/API2/VZDump.pm | 13 +- >> PVE/Jobs/VZDump.pm | 4 +- >> PVE/VZDump.pm | 6 +- >> debian/control | 2 +- >> www/manager6/Utils.js | 11 ++ >> www/manager6/dc/Backup.js | 4 - >> www/manager6/panel/BackupAdvancedOptions.js | 23 >> 9 files changed, 192 insertions(+), 12 deletions(-) >> > > applied above for now, we probably should bump manager soonish and then can > apply below. > pve-manager has been bumped in the meanwhile, I guess we could now merge the remaining patches for pve-docs and proxmox-widget-toolkit? They still apply cleanly and a quick test also showed that everything still works as expected. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs] pve-network: correct language errors
On 2024-08-13 12:23, Alexander Zeidler wrote: > Signed-off-by: Alexander Zeidler > --- > pve-network.adoc | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/pve-network.adoc b/pve-network.adoc > index acdcf39..434430d 100644 > --- a/pve-network.adoc > +++ b/pve-network.adoc > @@ -447,7 +447,7 @@ slaves in the single logical bonded interface such that > different > network-peers use different MAC addresses for their network packet > traffic. > > -If your switch support the LACP (IEEE 802.3ad) protocol then we recommend > using > +If your switch supports the LACP (IEEE 802.3ad) protocol then we recommend > using ^ There should be comma here: If ..., then ... > the corresponding bonding mode (802.3ad). Otherwise you should generally use > the > active-backup mode. > > @@ -490,7 +490,7 @@ iface vmbr0 inet static > > > [thumbnail="default-network-setup-bond.svg"] > -Another possibility it to use the bond directly as bridge port. > +Another possibility is to use the bond directly as bridge port. > This can be used to make the guest network fault-tolerant. > > .Example: Use a bond as bridge port Apart from this, this looks good to me: Reviewed-by: Lukas Wagner -- - 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] update to proxmox-log 0.2
On 2024-07-25 12:56, Fabian Grünbichler wrote: > Signed-off-by: Fabian Grünbichler > --- > common/src/logger.rs | 4 +--- > pmg-rs/Cargo.toml | 2 +- > pmg-rs/debian/control | 2 +- > pve-rs/Cargo.toml | 2 +- > pve-rs/debian/control | 2 +- > 5 files changed, 5 insertions(+), 7 deletions(-) > Did the same changes to get pve-rs compiling again in my local metrics branch, so consider this: Reviewed-by: Lukas Wagner -- - 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 v2 11/15] pmg-rs: tfa: clippy: the borrowed expression implements the required traits
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: &Tfa,
) -> 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.into())),
None => (None, None),
})
}
@@ -644,7 +644,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for UserAccess
{
fn remove(&self, userid: &str) -> Result {
let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file(&path) {
+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 v2 12/15] pmg-rs: tfa: clippy: useless conversion to the same type
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: &Tfa,
) -> 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 v2 13/15] pmg-rs: acme: clippy: reference is immediately deref'd by the compiler
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(&data, 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 v2 02/15] pve-rs: tfa: clippy: this function has too many arguments
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(&this.inner.lock().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 v2 14/15] pmg-rs: acme: simplify acount config saving
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(&account_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(&self.to_account_data()?)?;
+let create_options =
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(&account_path, &data, 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(&mut self, out: T) -> Result<(), Error> {
-let data = self.to_account_data()?;
-
-Ok(serde_json::to_writer_pretty(out, &data)?)
-}
-
pub fn update_account(&mut self, data: &T) -> 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(&tmp_path)
-.map_err(|err| format_err!("failed to open {:?} for writing: {}",
tmp_path, err))?;
-self.write_to(&mut 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(&tmp_path, account_path).map_err(|err| {
-format_err!(
-"failed to rotate temp file into place ({:?} -> {:?}): {}",
-&tmp_path,
-account_path,
-err
-)
-})?;
-drop(file);
+let data = serde_json::to_vec(&self.to_account_data()?)?;
+let create_options =
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(account_path, &data, 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 v2 15/15] tree-wide: run cargo fmt
Signed-off-by: Lukas Wagner
---
New in v2
common/src/subscription.rs | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/common/src/subscription.rs b/common/src/subscription.rs
index d4c7227..594c778 100644
--- a/common/src/subscription.rs
+++ b/common/src/subscription.rs
@@ -5,9 +5,9 @@ mod export {
use proxmox_subscription::SubscriptionInfo;
use proxmox_sys::fs::CreateOptions;
-use proxmox_http::ProxyConfig;
-use proxmox_http::HttpOptions;
use proxmox_http::client::sync::Client;
+use proxmox_http::HttpOptions;
+use proxmox_http::ProxyConfig;
#[export]
fn read_subscription(path: String) -> Result,
Error> {
@@ -69,7 +69,11 @@ mod export {
Some(url) => Some(ProxyConfig::parse_proxy_url(&url)?),
None => None,
};
-let options = HttpOptions { proxy_config, user_agent: Some(user_agent)
, ..Default::default() };
+let options = HttpOptions {
+proxy_config,
+user_agent: Some(user_agent),
+..Default::default()
+};
let client = Client::new(options);
proxmox_subscription::check::check_subscription(key, server_id,
product_url, client)
--
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 10/15] pmg-rs: tfa: clippy: this function has too many arguments
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(&this.inner.lock().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 v2 09/15] pmg-rs: tfa: clippy: question mark operator is useless here
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: &str) -> Result {
let this: &Tfa = (&raw_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
&mut this.inner.lock().unwrap(),
&UserAccess::new(&raw_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 v2 07/15] pve-rs: tfa: clippy: stripping a prefix manually
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(&key[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 },
&key[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 v2 05/15] pve-rs: tfa: clippy: accessing first element with `.get(0)`
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: &mut perlmod::Hash, config:
&TfaConfig) {
let users = Hash::new();
for (user, data) in &config.users {
-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: &mut perlmod::Hash, config:
&TfaConfig) {
continue;
}
-if let Some(totp) = data.totp.get(0) {
+if let Some(totp) = data.totp.first() {
let totp = &totp.entry;
let config = Hash::new();
config.insert("digits",
Value::new_int(isize::from(totp.digits(;
@@ -873,7 +873,7 @@ fn generate_legacy_config(out: &mut perlmod::Hash, config:
&TfaConfig) {
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 v2 06/15] pve-rs: tfa: clippy: redundant slicing of the whole range
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| !c.is_ascii_whitespace()) {
Some(from) => &data[from..],
-None => &data[..],
+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 v2 08/15] pmg-rs: tfa: clippy: unnecessary `pub(self)`
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 v2 04/15] pve-rs: tfa: clippy: borrowed expression impls the required traits
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(&self, userid: &str) -> Result {
let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file(&path) {
+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 v2 01/15] pve-rs: tfa: clippy: unnecessary `pub(self)`
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
[pve-devel] [PATCH proxmox-perl-rs v2 03/15] pve-rs: tfa: clippy: question mark operator is useless here
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: &str) -> Result {
let this: &Tfa = (&raw_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
&mut this.inner.lock().unwrap(),
&UserAccess::new(&raw_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
Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
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
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
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
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(
>> +&mut 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
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(&body_template, &data)
>> +.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
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:
[pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets
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: &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:
&Config, name: &str) -> 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: &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 &mut endpoints {
+let priv_config: WebhookPrivateConfig = config
+.private_config
+.lookup(WEBHOOK_TYPENAME, &endpoint.name)
+.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: &Config, name: &str) -> 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
+
[pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency
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
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
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: &ObjectSchema =
WebhookConfig::API_SCHEMA.unwrap_object_schema();
+config.register_plugin(SectionConfigPlugin::new(
+WEBHOOK_TYPENAME.to_string(),
+Some(String::from("name")),
+WEBHOOK_SCHEMA,
+));
+}
const MATCHER_SCHEMA: &ObjectSchema =
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: &ObjectSchema =
+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-no
[pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints
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 =>
[pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets
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 = &sorted!([
@@ -41,6 +42,7 @@ const ENDPOINT_SUBDIRS: SubdirMap = &sorted!([
("gotify", &gotify::ROUTER),
("sendmail", &sendmail::ROUTER),
("smtp", &smtp::ROUTER),
+("webhook", &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: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_AUDIT, false),
+},
+)]
+/// List all webhook endpoints.
+pub fn list_endpoints(
+_param: Value,
+_rpcenv: &mut dyn RpcEnvironment,
+) -> Result, Error> {
+let config = pbs_config::notifications::config()?;
+
+let endpoints = proxmox_notify::api::webhook::get_endpoints(&config)?;
+
+Ok(endpoints)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+name: {
+schema: ENTITY_NAME_SCHEMA,
+}
+},
+},
+returns: { type: WebhookConfig },
+access: {
+permission: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_AUDIT, false),
+},
+)]
+/// Get a webhook endpoint.
+pub fn get_endpoint(name: String, rpcenv: &mut dyn RpcEnvironment) ->
Result {
+let config = pbs_config::notifications::config()?;
+let endpoint = proxmox_notify::api::webhook::get_endpoint(&config, &name)?;
+
+rpcenv["digest"] = hex::encode(config.digest()).into();
+
+Ok(endpoint)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+endpoint: {
+type: WebhookConfig,
+flatten: true,
+},
+},
+},
+access: {
+permission: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Add a new webhook endpoint.
+pub fn add_endpoint(
+endpoint: WebhookConfig,
+_rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config()?;
+
+proxmox_notify::api::webhook::add_endpoint(&mut 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: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Update webhook endpoint.
+pub fn update_endpoint(
+name: String,
+updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option,
+_rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config
[pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window
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
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
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(&config, ¬ification)
}
+#[export(serialize_error)]
+fn get_targets(
+#[try_from_ref] this: &NotificationConfig,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::get_targets(&config)
+}
+
#[export(serialize_error)]
fn test_target(
#[try_from_ref] this: &NotificationConfig,
--
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
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(&mut config, name)
}
+#[export(serialize_error)]
+fn get_webhook_endpoints(
+#[try_from_ref] this: &NotificationConfig,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoints(&config)
+}
+
+#[export(serialize_error)]
+fn get_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+id: &str,
+) -> Result {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoint(&config, id)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn add_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+endpoint_config: WebhookConfig,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::add_endpoint(
+&mut config,
+endpoint_config,
+)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn update_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+name: &str,
+config_updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option<&str>,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+let digest = decode_digest(digest)?;
+
+api::webhook::update_endpoint(
+&mut config,
+name,
+config_updater,
+delete.as_deref(),
+digest.as_deref(),
+)
+}
+
+#[export(serialize_error)]
+fn delete_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+name: &str,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::delete_endpoint(&mut config, name)
+}
+
#[export(serialize_error)]
fn get_matchers(
#[try_from_ref] this: &NotificationConfig,
--
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.
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
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
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:
[pve-devel] [PATCH manager 07/12] api: add routes for webhook notification endpoints
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 =>
[pve-devel] [PATCH proxmox-mail-forward 12/12] bump proxmox-notify dependency
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
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
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(&config, ¬ification)
}
+#[export(serialize_error)]
+fn get_targets(
+#[try_from_ref] this: &NotificationConfig,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::get_targets(&config)
+}
+
#[export(serialize_error)]
fn test_target(
#[try_from_ref] this: &NotificationConfig,
--
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
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(&mut config, name)
}
+#[export(serialize_error)]
+fn get_webhook_endpoints(
+#[try_from_ref] this: &NotificationConfig,
+) -> Result, HttpError> {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoints(&config)
+}
+
+#[export(serialize_error)]
+fn get_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+id: &str,
+) -> Result {
+let config = this.config.lock().unwrap();
+api::webhook::get_endpoint(&config, id)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn add_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+endpoint_config: WebhookConfig,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::add_endpoint(
+&mut config,
+endpoint_config,
+)
+}
+
+#[export(serialize_error)]
+#[allow(clippy::too_many_arguments)]
+fn update_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+name: &str,
+config_updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option<&str>,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+let digest = decode_digest(digest)?;
+
+api::webhook::update_endpoint(
+&mut config,
+name,
+config_updater,
+delete.as_deref(),
+digest.as_deref(),
+)
+}
+
+#[export(serialize_error)]
+fn delete_webhook_endpoint(
+#[try_from_ref] this: &NotificationConfig,
+name: &str,
+) -> Result<(), HttpError> {
+let mut config = this.config.lock().unwrap();
+api::webhook::delete_endpoint(&mut config, name)
+}
+
#[export(serialize_error)]
fn get_matchers(
#[try_from_ref] this: &NotificationConfig,
--
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
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: &ObjectSchema =
WebhookConfig::API_SCHEMA.unwrap_object_schema();
+config.register_plugin(SectionConfigPlugin::new(
+WEBHOOK_TYPENAME.to_string(),
+Some(String::from("name")),
+WEBHOOK_SCHEMA,
+));
+}
const MATCHER_SCHEMA: &ObjectSchema =
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: &ObjectSchema =
+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 Handl
[pve-devel] [PATCH proxmox 02/12] notify: add api for webhook targets
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: &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:
&Config, name: &str) -> 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: &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 &mut endpoints {
+let priv_config: WebhookPrivateConfig = config
+.private_config
+.lookup(WEBHOOK_TYPENAME, &endpoint.name)
+.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: &Config, name: &str) -> 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
+
[pve-devel] [PATCH manager 06/12] api: notifications: use get_targets impl from proxmox-notify
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
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 = &sorted!([
@@ -41,6 +42,7 @@ const ENDPOINT_SUBDIRS: SubdirMap = &sorted!([
("gotify", &gotify::ROUTER),
("sendmail", &sendmail::ROUTER),
("smtp", &smtp::ROUTER),
+("webhook", &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: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_AUDIT, false),
+},
+)]
+/// List all webhook endpoints.
+pub fn list_endpoints(
+_param: Value,
+_rpcenv: &mut dyn RpcEnvironment,
+) -> Result, Error> {
+let config = pbs_config::notifications::config()?;
+
+let endpoints = proxmox_notify::api::webhook::get_endpoints(&config)?;
+
+Ok(endpoints)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+name: {
+schema: ENTITY_NAME_SCHEMA,
+}
+},
+},
+returns: { type: WebhookConfig },
+access: {
+permission: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_AUDIT, false),
+},
+)]
+/// Get a webhook endpoint.
+pub fn get_endpoint(name: String, rpcenv: &mut dyn RpcEnvironment) ->
Result {
+let config = pbs_config::notifications::config()?;
+let endpoint = proxmox_notify::api::webhook::get_endpoint(&config, &name)?;
+
+rpcenv["digest"] = hex::encode(config.digest()).into();
+
+Ok(endpoint)
+}
+
+#[api(
+protected: true,
+input: {
+properties: {
+endpoint: {
+type: WebhookConfig,
+flatten: true,
+},
+},
+},
+access: {
+permission: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Add a new webhook endpoint.
+pub fn add_endpoint(
+endpoint: WebhookConfig,
+_rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config()?;
+
+proxmox_notify::api::webhook::add_endpoint(&mut 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: &Permission::Privilege(&["system", "notifications"],
PRIV_SYS_MODIFY, false),
+},
+)]
+/// Update webhook endpoint.
+pub fn update_endpoint(
+name: String,
+updater: WebhookConfigUpdater,
+delete: Option>,
+digest: Option,
+_rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+let _lock = pbs_config::notifications::lock_config()?;
+let mut config = pbs_config::notifications::config
[pve-devel] [RFC many 00/12] notifications: add support for webhook endpoints
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
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,
+ mult
[pve-devel] [PATCH widget-toolkit v9 08/13] notification: matcher: move match-field formulas to local viewModel
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,
+
[pve-devel] [PATCH docs v9 11/13] notifications: describe new notification metadata fields
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
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
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 => $backu
[pve-devel] [PATCH many v9 00/13] notifications: notification metadata matching improvements
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 f
[pve-devel] [PATCH manager v9 02/13] api: jobs: vzdump: pass job 'job-id' parameter
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
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
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',
[pve-devel] [PATCH pve-guest-common v9 01/13] vzdump: common: allow 'job-id' as a parameter without being in schema
'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
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 =
[pve-devel] [PATCH manager v9 03/13] ui: dc: backup: allow to set custom job id in advanced settings
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
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
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
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
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
[pve-devel] [PATCH widget-toolkit v8 07/13] notification: matcher: match-field: show known fields/values
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',
[pve-devel] [PATCH widget-toolkit v8 10/13] notification: matcher: move match-severity fields to panel
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:
[pve-devel] [PATCH widget-toolkit v8 08/13] notification: matcher: move match-field formulas to local viewModel
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:
[pve-devel] [PATCH docs v8 11/13] notifications: describe new notification metadata fields
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
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
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
[pve-devel] [PATCH manager v8 06/13] d/control: bump proxmox-widget-toolkit dependency to 4.1.4
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
'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
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
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
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
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
[pve-devel] [PATCH manager v8 02/13] api: jobs: vzdump: pass job 'job-id' parameter
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
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
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
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: &str,
author: &str,
) -> 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: &[&str],
+mailfrom: &str,
+author: &str,
+subject: &str,
+text: Option<&str>,
+html: Option<&str>,
+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: {}", &recipients);
-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
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(
&recipients_str,
&subject,
-Some(&text_part),
-Some(&html_part),
+&text_part,
+&html_part,
&mailfrom,
&author,
)
@@ -172,8 +172,8 @@ impl Endpoint for SendmailEndpoint {
fn sendmail(
mailto: &[&str],
subject: &str,
-text: Option<&str>,
-html: Option<&str>,
+text: &str,
+html: &str,
mailfrom: &str,
author: &str,
) -> Result<(), Error> {
@@ -229,26 +229,20 @@ fn format_mail(
mailfrom: &str,
author: &str,
subject: &str,
-text: Option<&str>,
-html: Option<&str>,
+text: &str,
+html: &str,
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",
-
[pve-devel] [PATCH proxmox 1/6] notify: copy sendmail/forward fn's from proxmox_sys
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(
&recipients_str,
&subject,
Some(&text_part),
@@ -145,7 +148,7 @@ impl Endpoint for SendmailEndpoint {
}
#[cfg(feature = "mail-forwarder")]
Content::ForwardedMail { raw, uid, .. } => {
-proxmox_sys::email::forward(&recipients_str, &mailfrom, raw,
*uid)
+forward(&recipients_str, &mailfrom, 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: &[&str],
+subject: &str,
+text: Option<&str>,
+html: Option<&str>,
+mailfrom: Option<&str>,
+author: Option<&str>,
+) -> 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: {}", &recipients);
+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 i
[pve-devel] [PATCH proxmox 2/6] sys: mark email fn's as deprecated
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: &[&str],
subject: &str,
@@ -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: &[&str],
mailfrom: &str,
@@ -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
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: {}", &recipients);
+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
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 {
&subject,
Some(&text_part),
Some(&html_part),
-Some(&mailfrom),
-Some(&author),
+&mailfrom,
+&author,
)
.map_err(|err| Error::NotifyFailed(self.config.name.clone(),
err.into()))
}
@@ -174,8 +174,8 @@ fn sendmail(
subject: &str,
text: Option<&str>,
html: Option<&str>,
-mailfrom: Option<&str>,
-author: Option<&str>,
+mailfrom: &str,
+author: &str,
) -> 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
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(&account_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(&self.to_account_data()?)?;
+let create_options =
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(&account_path, &data, 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(&mut self, out: T) -> Result<(), Error> {
-let data = self.to_account_data()?;
-
-Ok(serde_json::to_writer_pretty(out, &data)?)
-}
-
pub fn update_account(&mut self, data: &T) -> 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(&tmp_path)
-.map_err(|err| format_err!("failed to open {:?} for writing: {}",
tmp_path, err))?;
-self.write_to(&mut 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(&tmp_path, account_path).map_err(|err| {
-format_err!(
-"failed to rotate temp file into place ({:?} -> {:?}): {}",
-&tmp_path,
-account_path,
-err
-)
-})?;
-drop(file);
+let data = serde_json::to_vec(&self.to_account_data()?)?;
+let create_options =
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(account_path, &data, 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
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: &Tfa,
) -> 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)`
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
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: &str) -> Result {
let this: &Tfa = (&raw_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
&mut this.inner.lock().unwrap(),
&UserAccess::new(&raw_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
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(&data, 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
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(&key[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 },
&key[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
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(&self, userid: &str) -> Result {
let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file(&path) {
+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
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: &Tfa,
) -> 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.into())),
None => (None, None),
})
}
@@ -644,7 +644,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for UserAccess
{
fn remove(&self, userid: &str) -> Result {
let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file(&path) {
+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)`
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: &mut perlmod::Hash, config:
&TfaConfig) {
let users = Hash::new();
for (user, data) in &config.users {
-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: &mut perlmod::Hash, config:
&TfaConfig) {
continue;
}
-if let Some(totp) = data.totp.get(0) {
+if let Some(totp) = data.totp.first() {
let totp = &totp.entry;
let config = Hash::new();
config.insert("digits",
Value::new_int(isize::from(totp.digits(;
@@ -873,7 +873,7 @@ fn generate_legacy_config(out: &mut perlmod::Hash, config:
&TfaConfig) {
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
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: &str) -> Result {
let this: &Tfa = (&raw_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
&mut this.inner.lock().unwrap(),
&UserAccess::new(&raw_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
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(&this.inner.lock().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
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| !c.is_ascii_whitespace()) {
Some(from) => &data[from..],
-None => &data[..],
+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
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(&this.inner.lock().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
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: &str) -> Result {
let (files, errors, digest) =
proxmox_apt::repositories::repositories()?;
-let digest = hex::encode(&digest);
+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)`
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
