[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(_path)
-.map_err(|err| format_err!("failed to open {:?} for writing: {}",
account_path, err))?;
-self.write_to(file).map_err(|err| {
-format_err!(
-"failed to write acme account to {:?}: {}",
-account_path,
-err
-)
-})?;
+
+let data = serde_json::to_vec(_account_data()?)?;
+let create_options =
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(_path, , create_options,
true)
+.map_err(|err| format_err!("failed to replace ACME account config:
{err}"))?;
+
self.account_path = Some(account_path);
Ok(())
@@ -131,12 +122,6 @@ impl Inner {
})
}
-fn write_to( self, out: T) -> Result<(), Error> {
-let data = self.to_account_data()?;
-
-Ok(serde_json::to_writer_pretty(out, )?)
-}
-
pub fn update_account( self, data: ) -> Result<(),
Error> {
let account_path = self
.account_path
@@ -144,32 +129,11 @@ impl Inner {
.ok_or_else(|| format_err!("missing account path"))?;
self.client.update_account(data)?;
-let tmp_path = format!("{}.tmp", account_path);
-// FIXME: move proxmox::tools::replace_file & make_temp out into a
nice *little* crate...
-let mut file = OpenOptions::new()
-.write(true)
-.create(true)
-.mode(0o600)
-.open(_path)
-.map_err(|err| format_err!("failed to open {:?} for writing: {}",
tmp_path, err))?;
-self.write_to( file).map_err(|err| {
-format_err!("failed to write acme account to {:?}: {}", tmp_path,
err)
-})?;
-file.flush().map_err(|err| {
-format_err!("failed to flush acme account file {:?}: {}",
tmp_path, err)
-})?;
-
-// re-borrow since we needed `self` as mut earlier
-let account_path = self.account_path.as_deref().unwrap();
-std::fs::rename(_path, account_path).map_err(|err| {
-format_err!(
-"failed to rotate temp file into place ({:?} -> {:?}): {}",
-_path,
-account_path,
-err
-)
-})?;
-drop(file);
+let data = serde_json::to_vec(_account_data()?)?;
+let create_options =
CreateOptions::new().perm(Mode::from_bits_truncate(0o600));
+proxmox_sys::fs::replace_file(account_path, , create_options,
true)
+.map_err(|err| format_err!("failed to replace ACME account config:
{err}"))?;
+
Ok(())
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 13/15] pmg-rs: tfa: clippy: useless conversion to the same type
Signed-off-by: Lukas Wagner
---
pmg-rs/src/tfa.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index af69721..4e9ce8f 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -178,7 +178,7 @@ mod export {
#[try_from_ref] this: ,
) -> Result<(Option, Option), Error> {
Ok(match this.inner.lock().unwrap().webauthn.clone() {
-Some(config) => (Some(hex::encode(config.digest())),
Some(config.into())),
+Some(config) => (Some(hex::encode(config.digest())), Some(config)),
None => (None, None),
})
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 09/15] pmg-rs: tfa: clippy: unnecessary `pub(self)`
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: ) -> Result {
let this: = (_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
this.inner.lock().unwrap(),
::new(_this)?,
userid,
-)?)
+)
}
#[derive(serde::Serialize)]
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 14/15] pmg-rs: acme: clippy: reference is immediately deref'd by the compiler
Signed-off-by: Lukas Wagner
---
pmg-rs/src/acme.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pmg-rs/src/acme.rs b/pmg-rs/src/acme.rs
index 7ea78c6..e2e7327 100644
--- a/pmg-rs/src/acme.rs
+++ b/pmg-rs/src/acme.rs
@@ -403,7 +403,7 @@ pub mod export {
this.inner
.lock()
.unwrap()
-.revoke_certificate(, reason)?;
+.revoke_certificate(data, reason)?;
Ok(())
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 08/15] pve-rs: tfa: clippy: stripping a prefix manually
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 1054169..66dca3d 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -736,10 +736,10 @@ fn decode_old_oath_entry(
let key = unsafe { std::str::from_utf8_unchecked(key) };
// See PVE::OTP::oath_verify_otp
-let key = if key.starts_with("v2-0x") {
-hex::decode([5..]).map_err(|_| format_err!("bad v2 hex key in
oath entry"))?
-} else if key.starts_with("v2-") {
-base32::decode(base32::Alphabet::RFC4648 { padding: true },
[3..])
+let key = if let Some(key) = key.strip_prefix("v2-0x") {
+hex::decode(key).map_err(|_| format_err!("bad v2 hex key in oath
entry"))?
+} else if let Some(key) = key.strip_prefix("v2-") {
+base32::decode(base32::Alphabet::RFC4648 { padding: true }, key)
.ok_or_else(|| format_err!("bad v2 base32 key in oath entry"))?
} else if key.len() == 16 {
base32::decode(base32::Alphabet::RFC4648 { padding: true }, key)
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 05/15] pve-rs: tfa: clippy: borrowed expression impls the required traits
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 9381ef0..7588d6d 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -1048,7 +1048,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for
UserAccess {
fn remove(, userid: ) -> Result {
let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file() {
+match std::fs::remove_file(path) {
Ok(()) => Ok(true),
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(false),
Err(err) => Err(err.into()),
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 12/15] pmg-rs: tfa: clippy: the borrowed expression implements the required traits
Signed-off-by: Lukas Wagner
---
pmg-rs/src/tfa.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index 928b50b..af69721 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -178,7 +178,7 @@ mod export {
#[try_from_ref] this: ,
) -> Result<(Option, Option), Error> {
Ok(match this.inner.lock().unwrap().webauthn.clone() {
-Some(config) => (Some(hex::encode(())),
Some(config.into())),
+Some(config) => (Some(hex::encode(config.digest())),
Some(config.into())),
None => (None, None),
})
}
@@ -644,7 +644,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for UserAccess
{
fn remove(, userid: ) -> Result {
let path = challenge_data_path(userid, self.is_debug());
-match std::fs::remove_file() {
+match std::fs::remove_file(path) {
Ok(()) => Ok(true),
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(false),
Err(err) => Err(err.into()),
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 06/15] pve-rs: tfa: clippy: accessing first element with `.get(0)`
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 7588d6d..7ead18c 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -831,7 +831,7 @@ fn generate_legacy_config(out: perlmod::Hash, config:
) {
let users = Hash::new();
for (user, data) in {
-if let Some(u2f) = data.u2f.get(0) {
+if let Some(u2f) = data.u2f.first() {
let data = Hash::new();
data.insert(
"publicKey",
@@ -850,7 +850,7 @@ fn generate_legacy_config(out: perlmod::Hash, config:
) {
continue;
}
-if let Some(totp) = data.totp.get(0) {
+if let Some(totp) = data.totp.first() {
let totp =
let config = Hash::new();
config.insert("digits",
Value::new_int(isize::from(totp.digits(;
@@ -873,7 +873,7 @@ fn generate_legacy_config(out: perlmod::Hash, config:
) {
continue;
}
-if let Some(entry) = data.yubico.get(0) {
+if let Some(entry) = data.yubico.first() {
let mut keys = entry.entry.clone();
for entry in data.yubico.iter().skip(1) {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 04/15] pve-rs: tfa: clippy: question mark operator is useless here
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 6650151..9381ef0 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -490,11 +490,11 @@ mod export {
#[export]
fn api_unlock_tfa(#[raw] raw_this: Value, userid: ) -> Result {
let this: = (_this).try_into()?;
-Ok(methods::unlock_and_reset_tfa(
+methods::unlock_and_reset_tfa(
this.inner.lock().unwrap(),
::new(_this)?,
userid,
-)?)
+)
}
#[derive(serde::Serialize)]
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 11/15] pmg-rs: tfa: clippy: this function has too many arguments
Signed-off-by: Lukas Wagner
---
pmg-rs/src/tfa.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/pmg-rs/src/tfa.rs b/pmg-rs/src/tfa.rs
index a97d171..928b50b 100644
--- a/pmg-rs/src/tfa.rs
+++ b/pmg-rs/src/tfa.rs
@@ -361,6 +361,7 @@ mod export {
methods::list_tfa(().unwrap(), authid,
top_level_allowed)
}
+#[allow(clippy::too_many_arguments)]
#[export]
fn api_add_tfa_entry(
#[raw] raw_this: Value,
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 07/15] pve-rs: tfa: clippy: redundant slicing of the whole range
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 7ead18c..1054169 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -802,7 +802,7 @@ fn usize_from_perl(value: JsonValue) -> Option {
fn trim_ascii_whitespace_start(data: &[u8]) -> &[u8] {
match data.iter().position(|| !c.is_ascii_whitespace()) {
Some(from) => [from..],
-None => [..],
+None => data,
}
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 03/15] pve-rs: tfa: clippy: this function has too many arguments
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 1 +
1 file changed, 1 insertion(+)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 798cdad..6650151 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -409,6 +409,7 @@ mod export {
methods::list_tfa(().unwrap(), authid,
top_level_allowed)
}
+#[allow(clippy::too_many_arguments)]
#[export]
fn api_add_tfa_entry(
#[raw] raw_this: Value,
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 01/15] pve-rs: apt: clippy: the borrowed expression implements the required traits
Signed-off-by: Lukas Wagner
---
common/src/apt/repositories.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/src/apt/repositories.rs b/common/src/apt/repositories.rs
index e710819..6e0a196 100644
--- a/common/src/apt/repositories.rs
+++ b/common/src/apt/repositories.rs
@@ -42,7 +42,7 @@ pub mod export {
#[export]
pub fn repositories(product: ) -> Result {
let (files, errors, digest) =
proxmox_apt::repositories::repositories()?;
-let digest = hex::encode();
+let digest = hex::encode(digest);
let suite = proxmox_apt::repositories::get_current_release_codename()?;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs 02/15] pve-rs: tfa: clippy: unnecessary `pub(self)`
Signed-off-by: Lukas Wagner
---
pve-rs/src/tfa.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pve-rs/src/tfa.rs b/pve-rs/src/tfa.rs
index 2b61344..798cdad 100644
--- a/pve-rs/src/tfa.rs
+++ b/pve-rs/src/tfa.rs
@@ -20,7 +20,7 @@ use nix::errno::Errno;
use nix::sys::stat::Mode;
use serde_json::Value as JsonValue;
-pub(self) use proxmox_tfa::api::{
+use proxmox_tfa::api::{
RecoveryState, TfaChallenge, TfaConfig, TfaResponse, TfaUserData,
U2fConfig,
UserChallengeAccess, WebauthnConfig,
};
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] applied: [PATCH proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml
On 2024-06-20 12:21, Fabian Grünbichler wrote: > with a follow-up to adapt the Makefiles - please test builds when > touching the build system ;) > Sorry, only did a `cargo build` - :S My bad -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml
Whoops, seems like I was on stable rust (1.78) and the warning only appears
there
but not on 1.77 (packaged rust) - but we can apply that already anyway,
saves us the trouble once we are on 1.78 :)
On 2024-06-20 10:59, Lukas Wagner wrote:
> Fixes the following new warning that appeared after switching
> to rust 1.77:
>
> warning: `proxmox-perl-rs/pve-rs/.cargo/config` is deprecated in
> favor of `config.toml`
>
> Signed-off-by: Lukas Wagner
> ---
> pmg-rs/.cargo/{config => config.toml} | 0
> pve-rs/.cargo/{config => config.toml} | 0
> 2 files changed, 0 insertions(+), 0 deletions(-)
> rename pmg-rs/.cargo/{config => config.toml} (100%)
> rename pve-rs/.cargo/{config => config.toml} (100%)
>
> diff --git a/pmg-rs/.cargo/config b/pmg-rs/.cargo/config.toml
> similarity index 100%
> rename from pmg-rs/.cargo/config
> rename to pmg-rs/.cargo/config.toml
> diff --git a/pve-rs/.cargo/config b/pve-rs/.cargo/config.toml
> similarity index 100%
> rename from pve-rs/.cargo/config
> rename to pve-rs/.cargo/config.toml
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox-perl-rs] pve-rs: pmg-rs: move deprecated .cargo/config to .cargo/config.toml
Fixes the following new warning that appeared after switching
to rust 1.77:
warning: `proxmox-perl-rs/pve-rs/.cargo/config` is deprecated in
favor of `config.toml`
Signed-off-by: Lukas Wagner
---
pmg-rs/.cargo/{config => config.toml} | 0
pve-rs/.cargo/{config => config.toml} | 0
2 files changed, 0 insertions(+), 0 deletions(-)
rename pmg-rs/.cargo/{config => config.toml} (100%)
rename pve-rs/.cargo/{config => config.toml} (100%)
diff --git a/pmg-rs/.cargo/config b/pmg-rs/.cargo/config.toml
similarity index 100%
rename from pmg-rs/.cargo/config
rename to pmg-rs/.cargo/config.toml
diff --git a/pve-rs/.cargo/config b/pve-rs/.cargo/config.toml
similarity index 100%
rename from pve-rs/.cargo/config
rename to pve-rs/.cargo/config.toml
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 19/19] notifications: backport some rephrased parts from PBS docs
Most of the changes were done when adapting the PVE docs to
the new PBS notification system, so now we 'backport' those
improvements.
Signed-off-by: Lukas Wagner
---
notifications.adoc | 99 +++---
1 file changed, 67 insertions(+), 32 deletions(-)
diff --git a/notifications.adoc b/notifications.adoc
index 9c5228c..bdfebd0 100644
--- a/notifications.adoc
+++ b/notifications.adoc
@@ -9,37 +9,38 @@ Overview
[thumbnail="screenshot/gui-datacenter-notification-overview.png"]
-{pve} will send notifications if case of noteworthy events in the system.
-
-There are a number of different xref:notification_events[notification events],
-each with their own set of metadata fields that can be used in
-notification matchers.
-
-A xref:notification_matchers[notification matcher] determines
-_which_ notifications shall be sent _where_.
-A matcher has _match rules_, that can be used to
-match on certain notification properties (e.g. timestamp, severity,
-metadata fields).
-If a matcher matches a notification, the notification will be routed
-to a configurable set of notification targets.
-
-A xref:notification_targets[notification target] is an abstraction for a
-destination where a notification should be sent to - for instance,
-a Gotify server instance, or a set of email addresses.
-There are multiple types of notification targets, including
-`sendmail`, which uses the system's sendmail command to send emails,
-or `gotify`, which sends a notification to a Gotify instance.
+* {pve} emits xref:notification_events[Notification Events] in case of
+ storage replication failures, node fencing, finished/failed backups
+ and other events.
+ These events are handled by the notification system. A notification
+ event has metadata, for example a timestamp, a severity level,
+ a type, and other optional metadata fields.
+* xref:notification_matchers[Notification Matchers] route a notification
+ event to one or more notification targets. A matcher can have match
+ rules to selectively route based on the metadata of a notification event.
+* xref:notification_targets[Notification Targets] are a destination to
+ which a notification event is routed to by a matcher.
+ There are multiple types of target, mail-based (Sendmail and SMTP)
+ and Gotify.
+
+Backup jobs have a configurable xref:notification_mode[Notification Mode].
+It allows you to choose between the notification system and a legacy mode
+for sending notification emails. The legacy mode is equivalent to the
+way notifications were handled before {pve} 8.1.
The notification system can be configured in the GUI under
-Datacenter -> Notifications. The configuration is stored in
+Datacenter → Notifications. The configuration is stored in
`/etc/pve/notifications.cfg` and `/etc/pve/priv/notifications.cfg` -
the latter contains sensitive configuration options such as
-passwords or authentication tokens for notification targets.
+passwords or authentication tokens for notification targets and can
+only be read by `root`.
[[notification_targets]]
Notification Targets
+{pve} offers multiple types of notification targets.
+
[[notification_targets_sendmail]]
Sendmail
@@ -50,14 +51,19 @@ that handles the sending of email messages.
It is a command-line utility that allows users and applications to send emails
directly from the command line or from within scripts.
-The sendmail notification target uses the `sendmail` binary to send emails.
-
+The sendmail notification target uses the `sendmail` binary to send emails to a
+list of configured users or email addresses. If a user is selected as a
recipient,
+the email address configured in user's settings will be used.
+For the `root@pam` user, this is the email address entered during installation.
+A user's email address can be configured in
+`Datacenter → Permissions → Users`.
+If a user has no associated email address, no email will be sent.
NOTE: In standard {pve} installations, the `sendmail` binary is provided by
-Postfix. For this type of target to work correctly, it might be necessary to
-change Postfix's configuration so that it can correctly deliver emails.
-For cluster setups it is necessary to have a working Postfix configuration on
-every single cluster node.
+Postfix. It may be necessary to configure Postfix so that it can deliver
+mails correctly - for example by setting an external mail relay (smart host).
+In case of failed delivery, check the system logs for messages logged by
+the Postfix daemon.
The configuration for Sendmail target plugins has the following options:
@@ -90,6 +96,12 @@ SMTP
[thumbnail="screenshot/gui-datacenter-notification-smtp.png"]
SMTP notification targets can send emails directly to an SMTP mail relay.
+This target does not use the system's MTA to deliver emails.
+Similar to sendmail targets, if a user is selected as a recipient, the user's
configured
+ema
[pve-devel] [PATCH widget-toolkit v7 11/19] notification: matcher: match-field: show known fields/values
These changes introduce combogrid pickers for the 'field' and 'value'
form elements for 'match-field' match rules. The 'field' picker shows
a list of all known metadata fields, while the 'value' picker shows a
list of all known values, filtered depending on the current value of
'field'.
The list of known fields/values is retrieved from new API endpoints.
Some values are marked 'internal' by the backend. This means that the
'value' field was not user-created (counter example: backup job
IDs) and can therefore be used as a base for translations.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/data/model/NotificationConfig.js | 12 ++
src/window/NotificationMatcherEdit.js | 297 +-
2 files changed, 253 insertions(+), 56 deletions(-)
diff --git a/src/data/model/NotificationConfig.js
b/src/data/model/NotificationConfig.js
index e8ebf28..03cf317 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -15,3 +15,15 @@ Ext.define('proxmox-notification-matchers', {
},
idProperty: 'name',
});
+
+Ext.define('proxmox-notification-fields', {
+extend: 'Ext.data.Model',
+fields: ['name', 'description'],
+idProperty: 'name',
+});
+
+Ext.define('proxmox-notification-field-values', {
+extend: 'Ext.data.Model',
+fields: ['value', 'comment', 'field'],
+idProperty: 'value',
+});
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index e717ad7..be33efe 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -79,7 +79,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
labelWidth: 120,
},
-width: 700,
+width: 800,
initComponent: function() {
let me = this;
@@ -416,10 +416,22 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
let me = this;
let record = me.get('selectedRecord');
let currentData = record.get('data');
+
+ let newValue = [];
+
+ // Build equivalent regular expression if switching
+ // to 'regex' mode
+ if (value === 'regex') {
+ let regexVal = "^(";
+ regexVal += currentData.value.join('|') + ")$";
+ newValue.push(regexVal);
+ }
+
record.set({
data: {
...currentData,
type: value,
+ value: newValue,
},
});
},
@@ -441,6 +453,8 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
data: {
...currentData,
field: value,
+ // Reset value if field changes
+ value: [],
},
});
},
@@ -549,6 +563,9 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
column2: [
{
xtype: 'pmxNotificationMatchRuleSettings',
+ cbind: {
+ baseUrl: '{baseUrl}',
+ },
},
],
@@ -601,7 +618,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
let value = data.value;
text = Ext.String.format(gettext("Match field: {0}={1}"),
field, value);
iconCls = 'fa fa-square-o';
- if (!field || !value) {
+ if (!field || !value || (Ext.isArray(value) && !value.length)) {
iconCls += ' internal-error';
}
} break;
@@ -821,6 +838,11 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
if (type === undefined) {
type = "exact";
}
+
+ if (type === 'exact') {
+ matchedValue = matchedValue.split(',');
+ }
+
return {
type: 'match-field',
data: {
@@ -982,7 +1004,9 @@ Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
extend: 'Ext.panel.Panel',
xtype: 'pmxNotificationMatchRuleSettings',
+mixins: ['Proxmox.Mixin.CBind'],
border: false,
+layout: 'anchor',
items: [
{
@@ -1000,6 +1024,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings',
{
['notall', gettext('At least one rule does not match')],
['notany', gettext('No rule matches')],
],
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
bind: {
hidden: '{!showMatchingMode}',
disabled: '{!show
[pve-devel] [PATCH widget-toolkit v7 12/19] notification: matcher: move match-field formulas to local viewModel
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 189 +-
1 file changed, 95 insertions(+), 94 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index be33efe..559b405 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,15 +380,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchField: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-field';
- },
- },
typeIsMatchSeverity: {
bind: {
bindTo: '{selectedRecord}',
@@ -407,89 +398,13 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('type') === 'match-calendar';
},
},
- matchFieldType: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
-
- let newValue = [];
-
- // Build equivalent regular expression if switching
- // to 'regex' mode
- if (value === 'regex') {
- let regexVal = "^(";
- regexVal += currentData.value.join('|') + ")$";
- newValue.push(regexVal);
- }
-
- record.set({
- data: {
- ...currentData,
- type: value,
- value: newValue,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.type;
- },
- },
- matchFieldField: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
-
- record.set({
- data: {
- ...currentData,
- field: value,
- // Reset value if field changes
- value: [],
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.field;
- },
- },
- matchFieldValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
matchSeverityValue: {
bind: {
bindTo: '{selectedRecord}',
deep: true,
},
set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
+ let record = this.get('selectedRecord');
let currentData = record.get('data');
record.set({
data: {
@@ -1137,7 +1052,98 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
},
},
},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchField: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-field';
+ },
+ },
+ isRegex: function(get) {
+ return get('matchFieldType') === 'regex';
+ },
+ matchFieldType: {
+ bind: {
+
[pve-devel] [PATCH manager v7 09/19] ui: utils: add overrides for translatable notification fields/values
Signed-off-by: Lukas Wagner
---
www/manager6/Utils.js | 11 +++
1 file changed, 11 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 5b0d51eb..ea448bfb 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2060,6 +2060,17 @@ Ext.define('PVE.Utils', {
zfscreate: [gettext('ZFS Storage'), gettext('Create')],
zfsremove: ['ZFS Pool', gettext('Remove')],
});
+
+ Proxmox.Utils.overrideNotificationFieldName({
+ 'job-id': gettext('Job ID'),
+ });
+
+ Proxmox.Utils.overrideNotificationFieldValue({
+ 'package-updates': gettext('Package updates are available'),
+ 'vzdump': gettext('Backup notifications'),
+ 'replication': gettext('Replication job notifications'),
+ 'fencing': gettext('Node fencing notifications'),
+ });
},
});
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 06/19] vzdump: apt: notification: do not include domain in 'hostname' field
- The man page warns about the usage of `hostname -f`, since a host
may have multiple domains (or none at all)
- The fallback PVE::INotify::nodename() already only returned the
hostname without the domain part
- Fencing notifications didn't include the domain part anyway
This may result in soft-breakage for any users who have already relied
on the domain being present. If there is need for it, it could include
a fqdn metadata field.
The hostname property used for rendering the notification template
is unaffected for now.
Signed-off-by: Lukas Wagner
---
PVE/API2/APT.pm | 3 ++-
PVE/VZDump.pm | 8
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index ec7c21b2..47c50961 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -348,7 +348,8 @@ __PACKAGE__->register_method({
# matchers.
my $metadata_fields = {
type => 'package-updates',
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
PVE::Notify::info(
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 2167b289..db3a02a9 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -517,10 +517,9 @@ sub send_notification {
"See Task History for details!\n";
};
-my $hostname = get_hostname();
-
my $notification_props = {
- "hostname" => $hostname,
+ # Hostname, might include domain part
+ "hostname" => get_hostname(),
"error-message" => $err,
"guest-table" => build_guest_table($tasklist),
"logs" => $text_log_part,
@@ -531,7 +530,8 @@ sub send_notification {
my $fields = {
type => "vzdump",
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
# Add backup-job metadata field in case this is a backup job.
$fields->{'job-id'} = $job_id if $job_id;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 02/19] api: jobs: vzdump: pass job 'job-id' parameter
This allows us to access us the backup job id in the send_notification
function, where we can set it as metadata for the notification.
Signed-off-by: Lukas Wagner
---
PVE/API2/VZDump.pm | 8
PVE/Jobs/VZDump.pm | 4 +++-
PVE/VZDump.pm | 6 +++---
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 7f92e7ec..84dbc100 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -53,6 +53,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => PVE::VZDump::Common::json_config_properties({
+ 'job-id' => {
+ description => "The ID of the backup job. If set, the
'backup-job' metadata field"
+ . " of the backup notification will be set to this value.",
+ type => 'string',
+ format => 'pve-configid',
+ maxLength => 256,
+ optional => 1,
+ },
stdout => {
type => 'boolean',
description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2dad3f55 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
use base qw(PVE::VZDump::JobBase);
sub run {
-my ($class, $conf) = @_;
+my ($class, $conf, $job_id) = @_;
my $props = $class->properties();
# remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
delete $conf->{$opt} if !defined($props->{$opt});
}
+$conf->{'job-id'} = $job_id;
+
# Required as string parameters # FIXME why?! we could just check ref()
for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 5b7080f3..2167b289 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -483,6 +483,7 @@ sub send_notification {
my ($self, $tasklist, $total_time, $err, $detail_pre, $detail_post) = @_;
my $opts = $self->{opts};
+my $job_id = $opts->{'job-id'};
my $mailto = $opts->{mailto};
my $cmdline = $self->{cmdline};
my $policy = $opts->{mailnotification} // 'always';
@@ -529,12 +530,11 @@ sub send_notification {
};
my $fields = {
- # TODO: There is no straight-forward way yet to get the
- # backup job id here... (I think pvescheduler would need
- # to pass that to the vzdump call?)
type => "vzdump",
hostname => $hostname,
};
+# Add backup-job metadata field in case this is a backup job.
+$fields->{'job-id'} = $job_id if $job_id;
my $severity = $failed ? "error" : "info";
my $email_configured = $mailto && scalar(@$mailto);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-guest-common v7 01/19] vzdump: common: allow 'job-id' as a parameter without being in schema
'job-id' is passed when a backup as started as a job and will be
passed to the notification system as matchable metadata. It it
can be considered 'internal'.
Signed-off-by: Lukas Wagner
---
src/PVE/VZDump/Common.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index 1539444..e835b05 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -496,7 +496,7 @@ sub command_line {
foreach my $p (keys %$param) {
next if $p eq 'id' || $p eq 'vmid' || $p eq 'starttime' ||
- $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled';
+ $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled' || $p eq
'job-id';
my $v = $param->{$p};
my $pd = $confdesc->{$p} || die "no such vzdump option '$p'\n";
if ($p eq 'exclude-path') {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v7 14/19] notification: matcher: move match-severity fields to panel
Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 129 --
1 file changed, 80 insertions(+), 49 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index 50145e3..9ab443f 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,34 +380,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchSeverity: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-severity';
- },
- },
- matchSeverityValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let record = this.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
+
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -944,27 +917,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
},
},
{
- xtype: 'proxmoxKVComboBox',
- fieldLabel: gettext('Severities to match'),
- isFormField: false,
- allowBlank: true,
- multiSelect: true,
- field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
- bind: {
- value: '{matchSeverityValue}',
- hidden: '{!typeIsMatchSeverity}',
- disabled: '{!typeIsMatchSeverity}',
- },
-
- comboItems: [
- ['info', gettext('Info')],
- ['notice', gettext('Notice')],
- ['warning', gettext('Warning')],
- ['error', gettext('Error')],
- ['unknown', gettext('Unknown')],
- ],
+ xtype: 'pmxNotificationMatchSeveritySettings',
},
{
xtype: 'pmxNotificationMatchCalendarSettings',
@@ -1047,6 +1000,84 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
},
});
+Ext.define('Proxmox.panel.MatchSeveritySettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchSeveritySettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchSeverity}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchSeverity: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-severity';
+ },
+ },
+ matchSeverityValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let record = this.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Severities to match'),
+ isFormField: false,
+ allowBlank: true,
+ multiSelect: true,
+ field: 'value',
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
+ bind: {
+ value: '{matchSeverityValue}',
+ hidden: '{!typeIsMatchSeverity}',
+ disabled: '{!typeIsMatchSeverity}',
+ },
+
+ comboItems: [
+ ['info', gettext('Info')],
+ ['notice', gettext('Notice
[pve-devel] [PATCH widget-toolkit v7 13/19] notification: matcher: move match-calendar fields to panel
Also introduce a local viewModel that is linked to a parent viewModel,
allowing us to move the formulas to the panel.
This should make the code more cohesive and easier to follow.
No functional changes.
Signed-off-by: Lukas Wagner
Tested-by: Maximiliano Sandoval
---
src/window/NotificationMatcherEdit.js | 92 +--
1 file changed, 60 insertions(+), 32 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index 559b405..50145e3 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -389,15 +389,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('type') === 'match-severity';
},
},
- typeIsMatchCalendar: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-calendar';
- },
- },
matchSeverityValue: {
bind: {
bindTo: '{selectedRecord}',
@@ -417,26 +408,6 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
return record?.get('data')?.value;
},
},
- matchCalendarValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let me = this;
- let record = me.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -995,6 +966,58 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
['unknown', gettext('Unknown')],
],
},
+ {
+ xtype: 'pmxNotificationMatchCalendarSettings',
+ },
+],
+});
+
+Ext.define('Proxmox.panel.MatchCalendarSettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchCalendarSettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchCalendar}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchCalendar: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-calendar';
+ },
+ },
+
+ matchCalendarValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let me = this;
+ let record = me.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
{
xtype: 'proxmoxKVComboBox',
fieldLabel: gettext('Timespan to match'),
@@ -1003,11 +1026,8 @@
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
editable: true,
displayField: 'key',
field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
bind: {
value: '{matchCalendarValue}',
- hidden: '{!typeIsMatchCalendar}',
disabled: '{!typeIsMatchCalender}',
},
@@ -1017,6 +1037,14 @@
Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
],
},
],
+
+initComponent: function() {
+ let me = this;
+ Ext.apply(me.viewModel, {
+ parent: me.up('pmxNotificationMatchRulesEditPanel').getViewModel(),
+ });
+ me.callParent();
+},
});
Ext.define('Proxmox.panel.MatchFieldSettings', {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 08/19] api: notification: add API for getting known metadata fields/values
This new API route returns known notification metadata fields and
a list of known possible values. This will be used by the UI to
provide suggestions when adding/modifying match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Cluster/Notifications.pm | 139 ++
1 file changed, 139 insertions(+)
diff --git a/PVE/API2/Cluster/Notifications.pm
b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..2b202c28 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -79,12 +79,151 @@ __PACKAGE__->register_method ({
{ name => 'endpoints' },
{ name => 'matchers' },
{ name => 'targets' },
+ { name => 'matcher-fields' },
+ { name => 'matcher-field-values' },
];
return $result;
}
});
+__PACKAGE__->register_method ({
+name => 'get_matcher_fields',
+path => 'matcher-fields',
+method => 'GET',
+description => 'Returns known notification metadata fields',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 0,
+parameters => {
+ additionalProperties => 0,
+ properties => {},
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ name => {
+ description => 'Name of the field.',
+ type => 'string',
+ },
+ },
+ },
+ links => [ { rel => 'child', href => '{name}' } ],
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+
+ my $result = [
+ { name => 'type' },
+ { name => 'hostname' },
+ { name => 'job-id' },
+ ];
+
+ return $result;
+}
+});
+
+__PACKAGE__->register_method ({
+name => 'get_matcher_field_values',
+path => 'matcher-field-values',
+method => 'GET',
+description => 'Returns known notification metadata fields and their known
values',
+permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+},
+protected => 1,
+parameters => {
+ additionalProperties => 0,
+},
+returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ 'value' => {
+ description => 'Notification metadata value known by the
system.',
+ type => 'string'
+ },
+ 'comment' => {
+ description => 'Additional comment for this value.',
+ type => 'string',
+ optional => 1,
+ },
+ 'field' => {
+ description => 'Field this value belongs to.',
+ type => 'string',
+ },
+ },
+ },
+},
+code => sub {
+ # TODO: Adapt this API handler once we have a 'notification registry'
+ my $rpcenv = PVE::RPCEnvironment::get();
+ my $user = $rpcenv->get_user();
+
+ my $values = [
+ {
+ value => 'package-updates',
+ field => 'type',
+ },
+ {
+ value => 'fencing',
+ field => 'type',
+ },
+ {
+ value => 'replication',
+ field => 'type',
+ },
+ {
+ value => 'vzdump',
+ field => 'type',
+ },
+ {
+ value => 'system-mail',
+ field => 'type',
+ },
+ ];
+
+ # Here we need a manual permission check.
+ if ($rpcenv->check($user, "/", ["Sys.Audit"], 1)) {
+ for my $backup_job (@{PVE::API2::Backup->index({})}) {
+ push @$values, {
+ value => $backup_job->{id},
+ comment => $backup_job->{comment},
+ field => 'job-id'
+ };
+ }
+ }
+ # The API call returns only returns jobs for which the user
+ # has adequate permissions.
+ for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+ push @$values, {
+ value => $sync_job->{id},
+ comment => $sync_job->{comment},
+ field => 'job-id'
+ };
+ }
+
+ for my $node (@{PVE::Cluster::get_nodelist()}) {
+ push @$values, {
+ value => $
[pve-devel] [PATCH docs v7 16/19] notifications: describe new notification metadata fields
Signed-off-by: Lukas Wagner --- notifications.adoc | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index 57053c8..dec878a 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -289,19 +289,21 @@ Notification Events [width="100%",options="header"] |=== -| Event| `type`| Severity | Metadata fields (in addition to `type`) -| System updates available |`package-updates` | `info` | `hostname` -| Cluster node fenced |`fencing` | `error` | `hostname` -| Storage replication failed |`replication` | `error` | - -| Backup finished |`vzdump` | `info` (`error` on failure) | `hostname` -| Mail for root|`system-mail` | `unknown`| - +| Event| `type`| Severity | Metadata fields (in addition to `type`) +| System updates available |`package-updates` | `info` | `hostname` +| Cluster node fenced |`fencing` | `error` | `hostname` +| Storage replication job failed |`replication` | `error` | `hostname`, `job-id` +| Backup succeeded |`vzdump` | `info` | `hostname`, `job-id` (only for backup jobs) +| Backup failed|`vzdump` | `error` | `hostname`, `job-id` (only for backup jobs) +| Mail for root|`system-mail` | `unknown`| `hostname` |=== [width="100%",options="header"] |=== -| Field name | Description -| `type` | Type of the notifcation -| `hostname` | Hostname, without domain (e.g. `pve1`) +| Field name| Description +| `type`| Type of the notifcation +| `hostname`| Hostname, without domain (e.g. `pve1`) +| `job-id` | Job ID |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 18/19] notifications: fix typo in 'notification'
Signed-off-by: Lukas Wagner --- notifications.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.adoc b/notifications.adoc index 07f0b3e..9c5228c 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -295,7 +295,7 @@ Notification Events [width="100%",options="header"] |=== | Field name| Description -| `type`| Type of the notifcation +| `type`| Type of the notification | `hostname`| Hostname, without domain (e.g. `pve1`) | `job-id` | Job ID |=== -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 15/19] notification: clarify that 'hostname' does not include the domain
This was a bit inconsistent between the different notification types: - APT/VZDump included the domain part - fence notifications did not A decision has been made to unify this by removing the domain part from APT/VZDump notifications. Signed-off-by: Lukas Wagner --- notifications.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.adoc b/notifications.adoc index 46aff6a..57053c8 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -301,7 +301,7 @@ Notification Events |=== | Field name | Description | `type` | Type of the notifcation -| `hostname` | Hostname, including domain (e.g. `pve1.example.com`) +| `hostname` | Hostname, without domain (e.g. `pve1`) |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v7 17/19] notifications: match-field 'exact'-mode can now match multiple values
Signed-off-by: Lukas Wagner --- notifications.adoc | 18 ++ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/notifications.adoc b/notifications.adoc index dec878a..07f0b3e 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -221,11 +221,16 @@ configurable schedule. Field Matching Rules Notifications have a selection of metadata fields that can be matched. +When using `exact` as a matching mode, a `,` can be used as a separator. +The matching rule then matches if the metadata field has *any* of the specified +values. * `match-field exact:type=vzdump` Only match notifications about backups. +* `match-field exact:type=replication,fencing` Match `replication` and `fencing` notifications. * `match-field regex:hostname=^.+\.example\.com$` Match the hostname of the node. + If a matched metadata field does not exist, the notification will not be matched. For instance, a `match-field regex:hostname=.*` directive will only match @@ -267,18 +272,7 @@ matcher: backup-failures comment Send notifications about backup failures to one group of admins matcher: cluster-failures -match-field exact:type=replication -match-field exact:type=fencing -mode any -target cluster-admins -comment Send cluster-related notifications to other group of admins - - -The last matcher could also be rewritten using a field matcher with a regular -expression: - -matcher: cluster-failures -match-field regex:type=^(replication|fencing)$ +match-field exact:type=replication,fencing target cluster-admins comment Send cluster-related notifications to other group of admins -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 10/19] d/control: bump proxmox-widget-toolkit dependency to 4.1.4
We need "utils: add mechanism to add and override translatable notification event descriptions in the product specific UIs" otherwise there is an error in the browser console. Signed-off-by: Lukas Wagner --- debian/control | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 4988a7d2..e400ddc5 100644 --- a/debian/control +++ b/debian/control @@ -20,7 +20,7 @@ Build-Depends: debhelper-compat (= 13), libtemplate-perl, libtest-mockmodule-perl, lintian, - proxmox-widget-toolkit (>= 4.0.7), + proxmox-widget-toolkit (>= 4.1.4), pve-cluster, pve-container, pve-doc-generator (>= 8.0.5), -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH many v7 00/19] notifications: notification metadata matching improvements
This patch series attempts to improve the user experience when creating notification matchers. Some of the noteworthy changes: - Fixup inconsistent 'hostname' field. Some notification events sent the hostname including a domain, while other did not. This series unifies the behavior, now the field only includes the hostname without a domain. Also updated the docs to reflect this change. - Allow setting a custom backup job ID, similar how we handle it for sync/prune jobs in PBS (to allow recognizable names used in matchers) - Adding columns for backup job ID/replication job ID in the UI - New metadata fields: - job-id: Job ID for backup-jobs or replication-jobs - Add an API that enumerates known notification metadata fields/values - Suggest known fields/values in match rule window - Some code clean up for match rule edit window - Extended the 'exact' match-field mode - it now allows setting multiple allowed values, separated via ',': e.g. `match-field exact:type=replication,fencing Originally, I created a separate 'list' match type for this, but since the semantics for a list with one value and 'exact' mode are identical, I decided to just extend 'exact'. This is a safe change since there are are no values where a ',' makes any sense (config IDs, hostnames) NOTE: Might need a versionened break, since the widget-toolkit-patches depend on new APIs provided by pve-manager. If the API is not present, creating matchers with 'match-field' does not work (cannot load lists of known values/fields) Inter-Dependencies: - the widget-toolkit dep in pve-manager needs to be bumped to at least 4.1.4 (we need "utils: add mechanism to add and override translatable notification event descriptions in the product specific UIs", otherwise the UI breaks with the pve-manager patches applied) --> already included a patch for this - widget-toolkit relies on a new API endpoint provided by pve-manager: --> we require a versioned break in widget-toolkit on pve-manager Changelog: - v7: incorporated some more feedback from @Fiona, thx! - Fixed error when switching from 'exact' to 'regex' if the text field was empty - rebased to latest master - 'backport' doc improvements from PBS - bumped widget-toolkit dep - v6: incorporate feedback from @Fiona, thx! - rename 'id' -> 'job-id' in VZDump API handler - consolidate 'replication-job'/'backup-job' to 'job-id' - Move 'job-id' setting to advanced tab in backup job edit. - Don't use 'internal' flag to mark translatable fields, since the only field where that's necessary is 'type' for now - so just add a hardcoded check - v5: - Rebased onto latest master, resolving some small conflict - v4: - widget-toolkit: break out changes for the utils module so that they can be applied ahead of time to ease dep bumping - don't show Job IDs in the backup/replication job columns - v3: - Drop already applied patches for `proxmox` - Rebase onto latest master - minor conflict resolution was needed - v2: - include 'type' metadata field for forwarded mails --> otherwise it's not possible to match them - include Maximilliano's T-b trailer in UI patches pve-guest-common: Lukas Wagner (1): vzdump: common: allow 'job-id' as a parameter without being in schema src/PVE/VZDump/Common.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) pve-manager: Lukas Wagner (9): api: jobs: vzdump: pass job 'job-id' parameter ui: dc: backup: send 'job-id' property when starting a backup job manually ui: dc: backup: allow to set custom job id in advanced settings api: replication: add 'job-id' to notification metadata vzdump: apt: notification: do not include domain in 'hostname' field api: replication: include 'hostname' field for notifications api: notification: add API for getting known metadata fields/values ui: utils: add overrides for translatable notification fields/values d/control: bump proxmox-widget-toolkit dependency to 4.1.4 PVE/API2/APT.pm | 3 +- PVE/API2/Cluster/Notifications.pm | 139 PVE/API2/Replication.pm | 4 +- PVE/API2/VZDump.pm | 8 ++ PVE/Jobs/VZDump.pm | 4 +- PVE/VZDump.pm | 14 +- debian/control | 2 +- www/manager6/Utils.js | 12 ++ www/manager6/dc/Backup.js | 7 +- www/manager6/panel/BackupAdvancedOptions.js | 23 10 files changed, 200 insertions(+), 16 deletions(-) proxmox-widget-toolkit: Lukas Wagner (4): notification: matcher: match-field: show known fields/values notification: matcher: move match-field formulas to local viewModel notification: matcher: move match-calendar fields to panel notificatio
[pve-devel] [PATCH manager v7 04/19] 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/Utils.js | 1 +
www/manager6/dc/Backup.js | 4
www/manager6/panel/BackupAdvancedOptions.js | 23 +
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index f5608944..5b0d51eb 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1963,6 +1963,7 @@ Ext.define('PVE.Utils', {
singleton: true,
constructor: function() {
var me = this;
+
Ext.apply(me, me.utilities);
Proxmox.Utils.override_task_descriptions({
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4b45b5c6..5975cb1c 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -45,10 +45,6 @@ Ext.define('PVE.dc.BackupEdit', {
Proxmox.Utils.assemble_field_data(values, { 'delete':
'notification-target' });
}
- if (!values.id && isCreate) {
- values.id = 'backup-' +
Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
- }
-
let selMode = values.selMode;
delete values.selMode;
diff --git a/www/manager6/panel/BackupAdvancedOptions.js
b/www/manager6/panel/BackupAdvancedOptions.js
index 1026c6f4..8860e42c 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -37,6 +37,10 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
return {};
}
+ if (!formValues.id && me.isCreate) {
+ formValues.id = 'backup-' +
Ext.data.identifier.Uuid.Global.generate().slice(0, 13);
+ }
+
let options = {};
if (!me.isCreate) {
@@ -105,6 +109,25 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
},
items: [
+ {
+ xtype: 'pveTwoColumnContainer',
+ startColumn: {
+ xtype: 'pmxDisplayEditField',
+ vtype: 'ConfigId',
+ fieldLabel: gettext('Job ID'),
+ emptyText: gettext('Autogenerate'),
+ name: 'id',
+ allowBlank: true,
+ cbind: {
+ editable: '{isCreate}',
+ },
+ },
+ endFlex: 2,
+ endColumn: {
+ xtype: 'displayfield',
+ value: gettext('Can be used in notification matchers to match
this job.'),
+ },
+ },
{
xtype: 'pveTwoColumnContainer',
startColumn: {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 07/19] api: replication: include 'hostname' field for notifications
The field contains the hostname of the host (without any domain part)
which sends the notification. This field can be used in match-field
match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 6208a1a2..e4a7180f 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -125,6 +125,8 @@ my sub _handle_job_err {
my $metadata_fields = {
type => "replication",
"job-id" => $job->{id},
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 05/19] api: replication: add 'job-id' to notification metadata
This allows users to create notification match rules for specific
replication jobs, if they so desire.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index d84ac1ab..6208a1a2 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -123,8 +123,8 @@ my sub _handle_job_err {
};
my $metadata_fields = {
- # TODO: Add job-id?
type => "replication",
+ "job-id" => $job->{id},
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v7 03/19] ui: dc: backup: send 'job-id' property when starting a backup job manually
Signed-off-by: Lukas Wagner
---
www/manager6/dc/Backup.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 4ba80b31..4b45b5c6 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -604,11 +604,12 @@ Ext.define('PVE.dc.BackupView', {
job = Ext.clone(job);
let jobNode = job.node;
+ job['job-id'] = job.id;
+ delete job.id;
// Remove properties related to scheduling
delete job.enabled;
delete job.starttime;
delete job.dow;
- delete job.id;
delete job.schedule;
delete job.type;
delete job.node;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v1 pve-manager 2/8] ceph: tools: update Ceph version regex
On 2024-04-30 17:28, Max Carrara wrote:
> Make the regex more maintainable declaring it as a variable, breaking it
> up and commenting it by using the x flag.
>
> Also remove the part that parses our Debian revision (e.g. -pve1) from
> the version, as we do not actually include that in our Ceph builds.
>
> The part of the regex that parses the build commit hash is made
> mandatory (remove '?' after its group).
>
> Signed-off-by: Max Carrara
> ---
> PVE/Ceph/Tools.pm | 13 -
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 087c4ef3..a00d23e1 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> @@ -68,7 +68,18 @@ sub get_local_version {
>
> return undef if !defined $ceph_version;
>
> -if ($ceph_version =~
> /^ceph.*\sv?(\d+(?:\.\d+)+(?:-pve\d+)?)\s+(?:\(([a-zA-Z0-9]+)\))?/) {
> +my $re_ceph_version = qr/
> + # Skip ahead to the version, which may optionally start with 'v'
> + ^ceph.*\sv?
> +
> + # Parse the version X.Y, X.Y.Z, etc.
> + ( \d+ (?:\.\d+)+ ) \s+
> +
> + # Parse the git commit hash between parentheses
> + (?: \( ([a-zA-Z0-9]+) \) )
> +/x;
TIL about the x modifier! Neat!
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v1 pve-manager 8/8] ui: ceph: osd: rework rendering of version field & show build commit
On 2024-04-30 17:28, Max Carrara wrote:
> The logic of the `render_version` function is split up in order to
> handle how the version is displayed depending on the type of the row.
>
> If the parsed version is `undefined` or the row marks the beginning of
> the tree, an empty string is now returned. This behaviour is
> equivalent to before, it just makes the overall logic easier.
>
> If the row belongs to a node, the build commit is now additionally
> displayed in parentheses next to the installed Ceph version:
>
> 18.2.2 (abcd1234)
>
> If the row belongs to an osd, the build commit is also additionally
> displayed in parentheses next to the installed Ceph version.
> Furthermore, should the build commit of the running osd differ from
> the one that's installed on the host, the new hash will also be shown
> in parentheses:
>
> 18.2.2 (abcd1234 -> 5678fedc)
>
> The icon displayed for running an osd with an outdated build is the
> same as for running an outdated version. The conditional display of
> cluster health-related icons remains the same otherwise.
>
> Signed-off-by: Max Carrara
> ---
> www/manager6/ceph/OSD.js | 55
> 1 file changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
> index d2caafa4..988962b1 100644
> --- a/www/manager6/ceph/OSD.js
> +++ b/www/manager6/ceph/OSD.js
> @@ -642,23 +642,58 @@ Ext.define('PVE.node.CephOsdTree', {
> },
>
> render_version: function(value, metadata, rec) {
> + if (value === undefined || rec.data.type === 'root') {
> + return '';
> + }
> +
> let vm = this.getViewModel();
> - let versions = vm.get('versions');
> let icon = "";
> - let version = value || "";
> - let maxversion = vm.get('maxversion');
> - if (value && PVE.Utils.compare_ceph_versions(value, maxversion) !==
> 0) {
> - let host_version = rec.parentNode?.data?.version ||
> versions[rec.data.host] || "";
> - if (rec.data.type === 'host' ||
> PVE.Utils.compare_ceph_versions(host_version, maxversion) !== 0) {
> + const maxversion = vm.get('maxversion');
> + const mixedversions = vm.get('mixedversions');
> +
> + if (rec.data.type === 'host') {
> + const bc = rec.data.buildcommit ?? '';
> +
> + if (PVE.Utils.compare_ceph_versions(maxversion, value) > 0) {
> icon = PVE.Utils.get_ceph_icon_html('HEALTH_UPGRADE');
> - } else {
> - icon = PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> + } else if (mixedversions) {
> + icon = PVE.Utils.get_ceph_icon_html('HEALTH_OK');
> }
> - } else if (value && vm.get('mixedversions')) {
> +
> + if (bc === '') {
> + return `${icon}${value}`;
> + }
> +
> + return `${icon}${value} (${bc.substring(0, 9)})`;
> + }
> +
> + const versionNode = rec.parentNode?.data?.version ?? '';
> +
> + const bc = PVE.Utils.parse_ceph_buildcommit(rec.data) ?? '';
> + const bcNode = rec.parentNode?.data?.buildcommit ?? '';
same as before, rather use `let` than `const` and maybe use a longer, more
clear name.
> +
> + let bcChanged = bc !== '' && bcNode !== '' && bc !== bcNode;
> +
> + if (PVE.Utils.compare_ceph_versions(maxversion, value) > 0) {
> + icon = PVE.Utils.get_ceph_icon_html('HEALTH_UPGRADE');
> + } else if (versionNode &&
> PVE.Utils.compare_ceph_versions(versionNode, value) > 0) {
> + icon = PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> + } else if (mixedversions && !bcChanged) {
> icon = PVE.Utils.get_ceph_icon_html('HEALTH_OK');
> }
>
> - return icon + version;
> + let bcPart = bc.substring(0, 9);
> + if (bcChanged) {
> + const arrow = '';
> + icon ||= PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> + bcPart = `${bc.substring(0, 9)}${arrow}${bcNode.substring(0,
> 9)}`;
> + }
> +
> + if (bcPart === '') {
> + return `${icon}${value}`;
> + }
> +
> + return `${icon}${value} (${bcPart})`;
> },
>
> render_osd_val: function(value, metaData, rec) {
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH v1 pve-manager 6/8] ui: ceph: services: parse and display build commit
On 2024-04-30 17:28, Max Carrara wrote:
> This commit adds `PVE.Utils.parse_ceph_buildcommit`, which can be used
> to get the full hash "eccf199d..." in parentheses from a string like
> the following:
>
> ceph version 17.2.7 (eccf199d63457659c09677399928203b7903c888) quincy
> (stable)
>
> This hash is displayed and taken into account when comparing monitor
> and manager versions in the client. Specifically, the shortened build
> commit is now displayed in parentheses next to the version for both
> monitors and managers like so:
>
> 18.2.2 (abcd1234)
>
> Should the build commit of the running service differ from the one
> that's installed on the host, the newer build commit will also be
> shown in parentheses:
>
> 18.2.2 (abcd1234 -> 5678fedc)
>
> The icon displayed for running a service with an outdated build is the
> same as for running an outdated version. The conditional display of
> cluster health-related icons remains the same otherwise.
>
> The Ceph summary view also displays the hash and will show a warning
> if a service is running with a build commit that doesn't match the one
> that's advertised by the host.
>
> Signed-off-by: Max Carrara
> ---
> www/manager6/Utils.js| 14 ++
> www/manager6/ceph/ServiceList.js | 28 ++--
> www/manager6/ceph/Services.js| 14 +-
> 3 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 74e46694..435c79d7 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -128,6 +128,20 @@ Ext.define('PVE.Utils', {
> return undefined;
> },
>
> +parse_ceph_buildcommit: function(service) {
> + if (service.ceph_version) {
> + // See PVE/Ceph/Tools.pm - get_local_version
> + const match = service.ceph_version.match(
> + /^ceph.*\sv?(?:\d+(?:\.\d+)+)\s+(?:\(([a-zA-Z0-9]+)\))/,
> + );
> + if (match) {
> + return match[1];
> + }
> + }
> +
> + return undefined;
> +},
> +
> compare_ceph_versions: function(a, b) {
style nit: new functions/vars should use camelCase, see our JS styleguide [1]
> let avers = [];
> let bvers = [];
> diff --git a/www/manager6/ceph/ServiceList.js
> b/www/manager6/ceph/ServiceList.js
> index 76710146..30f455ed 100644
> --- a/www/manager6/ceph/ServiceList.js
> +++ b/www/manager6/ceph/ServiceList.js
> @@ -102,21 +102,37 @@ Ext.define('PVE.node.CephServiceController', {
> if (value === undefined) {
> return '';
> }
> +
> + const bc = PVE.Utils.parse_ceph_buildcommit(rec.data) ?? '';
I'd prefer a longer variable name here, e.g. buildCommit - bc is a bit too
terse for my
taste :)
> +
> let view = this.getView();
> - let host = rec.data.host, nodev = [0];
> + const host = rec.data.host;
style nit: we only really use `const` for proper constants, see our JS
guidelines
> +
> + let versionNode = [0];
> + let bcNode = '';
> if (view.nodeversions[host] !== undefined) {
> - nodev = view.nodeversions[host].version.parts;
> + versionNode = view.nodeversions[host].version.parts;
> + bcNode = view.nodeversions[host].buildcommit;
> }
>
> + let bcChanged = bc !== '' && bcNode !== '' && bc !== bcNode;
> let icon = '';
> - if (PVE.Utils.compare_ceph_versions(view.maxversion, nodev) > 0) {
> + if (PVE.Utils.compare_ceph_versions(view.maxversion, versionNode) > 0) {
> icon = PVE.Utils.get_ceph_icon_html('HEALTH_UPGRADE');
> - } else if (PVE.Utils.compare_ceph_versions(nodev, value) > 0) {
> + } else if (PVE.Utils.compare_ceph_versions(versionNode, value) > 0) {
> icon = PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> - } else if (view.mixedversions) {
> + } else if (view.mixedversions && !bcChanged) {
> icon = PVE.Utils.get_ceph_icon_html('HEALTH_OK');
> }
> - return icon + value;
> +
> + let bcPart = bc.substring(0, 9);
> + if (bcChanged) {
> + const arrow = '';
> + icon ||= PVE.Utils.get_ceph_icon_html('HEALTH_OLD');
> + bcPart = `${bc.substring(0, 9)}${arrow}${bcNode.substring(0, 9)}`;
> + }
> +
> + return `${icon}${value} (${bcPart})`;
> },
>
> getMaxVersions: function(store, records, success) {
> diff --git a/www/manager6/ceph/Services.js b/www/manager6/ceph/Services.js
> index b9fc52c8..410b28bf 100644
> --- a/www/manager6/ceph/Services.js
> +++ b/www/manager6/ceph/Services.js
> @@ -155,6 +155,7 @@ Ext.define('PVE.ceph.Services', {
> title: metadata[type][id].name || name,
> host: host,
> version: PVE.Utils.parse_ceph_version(metadata[type][id]),
> + buildcommit:
> PVE.Utils.parse_ceph_buildcommit(metadata[type][id]),
> service: metadata[type][id].service,
> addr:
Re: [pve-devel] [PATCH v1 pve-manager 0/8] Ceph Build Commit in UI
On 2024-04-30 17:28, Max Carrara wrote: > Ceph Build Commit in UI - Version 1 > === > > This series adds Ceph's build commit to the UI and lets the user know if > a service is running an outdated build and therefore ought to be > restarted. > > The build commit is now displayed next to the version for all Ceph > services like so: > > 18.2.2 (abcd1234) > > Should a service run an outdated build, the new build commit is also > displayed: > > 18.2.2 (abcd1234 -> 5678fedc) > > (Icons are omitted here). > > See the individual patches for more in-depth information. > > Additionally, some of the code was also cleaned up and refactored a > little along the way. > > I'm not 100% sure if the design I've opted for here is the best, so it > would be great to get some opinions on this. The OSD tree/list view > especially can get a little noisy if there are a lot of outdated OSDs > running. > Neat, gave this a quick test and this seems to do what you describe in the cover letter. Some comments: - The 'outdated OSD' view in the top-level status widget uses a different icon to signal an outdated OSD than the version column in the OSD tree view - maybe it would make sense to use the same icon here? - The 'Detail' view for OSDs could also show the commit hash, right now it only shows e.g. 18.2.2 (reef) With the nits from the review fixed: Tested-by: Lukas Wagner 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 pve-ha-manager v3 5/8] env: notify: use named templates instead of passing template strings
Signed-off-by: Lukas Wagner
---
debian/pve-ha-manager.install | 3 +++
src/Makefile | 1 +
src/PVE/HA/Env/PVE2.pm| 4 ++--
src/PVE/HA/NodeStatus.pm | 20 +--
src/PVE/HA/Sim/Env.pm | 3 ++-
src/templates/Makefile| 10 ++
src/templates/default/fencing-body.html.hbs | 14 +
src/templates/default/fencing-body.txt.hbs| 11 ++
src/templates/default/fencing-subject.txt.hbs | 1 +
9 files changed, 45 insertions(+), 22 deletions(-)
create mode 100644 src/templates/Makefile
create mode 100644 src/templates/default/fencing-body.html.hbs
create mode 100644 src/templates/default/fencing-body.txt.hbs
create mode 100644 src/templates/default/fencing-subject.txt.hbs
diff --git a/debian/pve-ha-manager.install b/debian/pve-ha-manager.install
index a7598a9..0ffbd8d 100644
--- a/debian/pve-ha-manager.install
+++ b/debian/pve-ha-manager.install
@@ -38,3 +38,6 @@
/usr/share/perl5/PVE/HA/Usage/Static.pm
/usr/share/perl5/PVE/Service/pve_ha_crm.pm
/usr/share/perl5/PVE/Service/pve_ha_lrm.pm
+/usr/share/pve-manager/templates/default/fencing-body.html.hbs
+/usr/share/pve-manager/templates/default/fencing-body.txt.hbs
+/usr/share/pve-manager/templates/default/fencing-subject.txt.hbs
diff --git a/src/Makefile b/src/Makefile
index 87bb0de..56bd360 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -73,6 +73,7 @@ install: watchdog-mux pve-ha-crm pve-ha-lrm ha-manager.1
pve-ha-crm.8 pve-ha-lrm
install -d $(DESTDIR)/$(MAN1DIR)
install -m 0644 ha-manager.1 $(DESTDIR)/$(MAN1DIR)
gzip -9 $(DESTDIR)/$(MAN1DIR)/ha-manager.1
+ $(MAKE) -C templates $@
.PHONY: test
test:
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index fcb60a9..cb73bcf 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -221,10 +221,10 @@ sub log {
}
sub send_notification {
-my ($self, $subject, $text, $template_data, $metadata_fields) = @_;
+my ($self, $template_name, $template_data, $metadata_fields) = @_;
eval {
- PVE::Notify::error($subject, $text, $template_data, $metadata_fields);
+ PVE::Notify::error($template_name, $template_data, $metadata_fields);
};
$self->log("warning", "could not notify: $@") if $@;
diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
index e053c55..9e6d898 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -188,23 +188,6 @@ sub update {
}
}
-my $body_template = <send_notification(
- $subject_template,
- $body_template,
+ "fencing",
$template_data,
$metadata_fields,
);
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index d3aea8d..0f77065 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -289,11 +289,12 @@ sub log {
}
sub send_notification {
-my ($self, $subject, $text, $properties) = @_;
+my ($self, $template_name, $properties) = @_;
# The template for the subject is "{{subject-prefix}}: {{subject}}"
# We have to perform poor-man's template rendering to pass the test cases.
+my $subject = "{{subject-prefix}}: {{subject}}";
$subject = $subject =~
s/\{\{subject-prefix}}/$properties->{"subject-prefix"}/r;
$subject = $subject =~ s/\{\{subject}}/$properties->{"subject"}/r;
diff --git a/src/templates/Makefile b/src/templates/Makefile
new file mode 100644
index 000..396759f
--- /dev/null
+++ b/src/templates/Makefile
@@ -0,0 +1,10 @@
+NOTIFICATION_TEMPLATES=
\
+ default/fencing-subject.txt.hbs \
+ default/fencing-body.txt.hbs\
+ default/fencing-body.html.hbs \
+
+.PHONY: install
+install:
+ install -dm 0755 $(DESTDIR)/usr/share/pve-manager/templates/default
+ $(foreach i,$(NOTIFICATION_TEMPLATES), \
+ install -m644 $(i) $(DESTDIR)/usr/share/pve-manager/templates/$(i)
;)
diff --git a/src/templates/default/fencing-body.html.hbs
b/src/templates/default/fencing-body.html.hbs
new file mode 100644
index 000..1420348
--- /dev/null
+++ b/src/templates/default/fencing-body.html.hbs
@@ -0,0 +1,14 @@
+
+
+The node '{{node}}' failed and needs manual intervention.
+
+The PVE HA manager tries to fence it and recover the configured HA
resources to
+a healthy node if possible.
+
+Current fence status: {{subject-prefix}}
+{{subject}}
+
+Overall Cluster status:
+{{object status-data}}
+
+
diff --git a/src/templates/default/fencing-body.txt.hbs
b/src/templates/default/fencing-body.txt.hbs
new file mode 100644
index 000..e46a1fd
--- /dev/null
+++ b/src/templates/default/fencing-body.txt.hbs
@@ -0,0 +1,11 @@
+The
[pve-devel] [PATCH proxmox-perl-rs v3 2/8] notify: don't pass config structs by reference
proxmox_notify's api functions have been changed so that they take
ownership of config structs.
Signed-off-by: Lukas Wagner
---
common/src/notify.rs | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index d965417..00a6056 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -151,7 +151,7 @@ mod export {
api::sendmail::add_endpoint(
config,
- {
+SendmailConfig {
name,
mailto,
mailto_user,
@@ -185,7 +185,7 @@ mod export {
api::sendmail::update_endpoint(
config,
name,
- {
+SendmailConfigUpdater {
mailto,
mailto_user,
from_address,
@@ -236,7 +236,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::gotify::add_endpoint(
config,
- {
+GotifyConfig {
name: name.clone(),
server,
comment,
@@ -244,7 +244,7 @@ mod export {
filter: None,
origin: None,
},
- { name, token },
+GotifyPrivateConfig { name, token },
)
}
@@ -266,12 +266,12 @@ mod export {
api::gotify::update_endpoint(
config,
name,
- {
+GotifyConfigUpdater {
server,
comment,
disable,
},
- { token },
+GotifyPrivateConfigUpdater { token },
delete.as_deref(),
digest.as_deref(),
)
@@ -323,7 +323,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::smtp::add_endpoint(
config,
- {
+SmtpConfig {
name: name.clone(),
server,
port,
@@ -337,7 +337,7 @@ mod export {
disable,
origin: None,
},
- { name, password },
+SmtpPrivateConfig { name, password },
)
}
@@ -366,7 +366,7 @@ mod export {
api::smtp::update_endpoint(
config,
name,
- {
+SmtpConfigUpdater {
server,
port,
mode,
@@ -378,7 +378,7 @@ mod export {
comment,
disable,
},
- { password },
+SmtpPrivateConfigUpdater { password },
delete.as_deref(),
digest.as_deref(),
)
@@ -427,7 +427,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::matcher::add_matcher(
config,
- {
+MatcherConfig {
name,
match_severity,
match_field,
@@ -464,7 +464,7 @@ mod export {
api::matcher::update_matcher(
config,
name,
- {
+MatcherConfigUpdater {
match_severity,
match_field,
match_calendar,
--
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 v3 0/8] notifications: move template strings to template files
These changes adapts the PVE notification stack to the changes introduced
in proxmox-notify 0.4.
The notification system uses handlebar templates to render the subject
and the body of notifications. Previously, the template strings were
defined inline at the call site. This patch series extracts the templates
into template files and installs them at
/usr/share/pve-manager/templates/default
where they stored as -{body,subject}.{txt,html}.hbs
The 'default' part in the path is a preparation for translated
notifications and/or overridable notification templates.
Future work could provide notifications translated to e.g. German
in `templates/de` or similar. This will be a first for having
translated strings on the backend-side, so there is need for further
research.
Folke kindly did some off-list testing before this was posted, hence
his T-bs were included.
Bumps/dependencies:
- libproxmox-rs-perl needs to have its proxmox-notify dep updated to 0.4
and breaks old libpve-notify-perl (versioned break)
- libpve-notify-perl breaks old pve-manager and old pve-ha-manager (versioned
break)
The versioned breaks are necessary due to changed semantics in the API (passing
a template name instead of template strings) and due to changes in how
templates are rendered (separate templates for HTML/plain text, whereas before
both were rendered from the same template string, with some magic from
handlebar helpers)
Changes since v2:
- Dropped already applied patches for 'proxmox'
- Rebased, quick smoke-test to check if anything broke
in the meanwhile
Changes since v1:
- Incorporated feedback from @Fiona and @Fabian - thanks!
- most noteworthy: Change template path from /usr/share/proxmox-ve
to /usr/share/pve-manager
- apart from that mostly just cosmetics/style
proxmox-perl-rs:
Lukas Wagner (3):
notify: use file based notification templates
notify: don't pass config structs by reference
notify: adapt to Option> to Vec changes in proxmox_notify
common/src/notify.rs | 48 +---
1 file changed, 23 insertions(+), 25 deletions(-)
pve-cluster:
Lukas Wagner (1):
notify: use named template instead of passing template strings
src/PVE/Notify.pm | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
pve-ha-manager:
Lukas Wagner (1):
env: notify: use named templates instead of passing template strings
debian/pve-ha-manager.install | 3 +++
src/Makefile | 1 +
src/PVE/HA/Env/PVE2.pm| 4 ++--
src/PVE/HA/NodeStatus.pm | 20 +--
src/PVE/HA/Sim/Env.pm | 3 ++-
src/templates/Makefile| 10 ++
src/templates/default/fencing-body.html.hbs | 14 +
src/templates/default/fencing-body.txt.hbs| 11 ++
src/templates/default/fencing-subject.txt.hbs | 1 +
9 files changed, 45 insertions(+), 22 deletions(-)
create mode 100644 src/templates/Makefile
create mode 100644 src/templates/default/fencing-body.html.hbs
create mode 100644 src/templates/default/fencing-body.txt.hbs
create mode 100644 src/templates/default/fencing-subject.txt.hbs
pve-manager:
Lukas Wagner (3):
gitignore: ignore any test artifacts
tests: remove vzdump_notification test
notifications: use named templates instead of in-code templates
.gitignore| 2 +
Makefile | 2 +-
PVE/API2/APT.pm | 9 +-
PVE/API2/Replication.pm | 20 +---
PVE/VZDump.pm | 20 +---
templates/Makefile| 24 +
.../default/package-updates-body.html.hbs | 6 ++
.../default/package-updates-body.txt.hbs | 3 +
.../default/package-updates-subject.txt.hbs | 1 +
templates/default/replication-body.html.hbs | 18
templates/default/replication-body.txt.hbs| 12 +++
templates/default/replication-subject.txt.hbs | 1 +
templates/default/test-body.html.hbs | 1 +
templates/default/test-body.txt.hbs | 1 +
templates/default/test-subject.txt.hbs| 1 +
templates/default/vzdump-body.html.hbs| 11 ++
templates/default/vzdump-body.txt.hbs | 10 ++
templates/default/vzdump-subject.txt.hbs | 1 +
test/Makefile | 6 +-
test/vzdump_notification_test.pl | 101 --
20 files changed, 98 insertions(+), 152 deletions(-)
create mode 100644 templates/Makefile
create mode 100644 templates/default/package-updates-body.html.hbs
create mode 100644 templates/default/package-updates-body.txt.hbs
create mode 100644 templates/default/package-updates-subject.txt.hbs
create mode 100644 templates/default/replication-body.html.hbs
create mode 100644 templates/defa
[pve-devel] [PATCH proxmox-perl-rs v3 3/8] notify: adapt to Option> to Vec changes in proxmox_notify
Signed-off-by: Lukas Wagner
---
common/src/notify.rs | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 00a6056..e1b006b 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -153,8 +153,8 @@ mod export {
config,
SendmailConfig {
name,
-mailto,
-mailto_user,
+mailto: mailto.unwrap_or_default(),
+mailto_user: mailto_user.unwrap_or_default(),
from_address,
author,
comment,
@@ -329,8 +329,8 @@ mod export {
port,
mode,
username,
-mailto,
-mailto_user,
+mailto: mailto.unwrap_or_default(),
+mailto_user: mailto_user.unwrap_or_default(),
from_address,
author,
comment,
@@ -429,10 +429,10 @@ mod export {
config,
MatcherConfig {
name,
-match_severity,
-match_field,
-match_calendar,
-target,
+match_severity: match_severity.unwrap_or_default(),
+match_field: match_field.unwrap_or_default(),
+match_calendar: match_calendar.unwrap_or_default(),
+target: target.unwrap_or_default(),
mode,
invert_match,
comment,
--
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 v3 8/8] notifications: use named templates instead of in-code templates
This commit adapts notification sending for
- package update
- replication
- backups
to use named templates (installed in /usr/share/pve-manager/templates)
instead of passing template strings defined in code to the
notification stack.
Signed-off-by: Lukas Wagner
---
Makefile | 2 +-
PVE/API2/APT.pm | 9 +--
PVE/API2/Replication.pm | 20 +---
PVE/VZDump.pm | 20 ++--
templates/Makefile| 24 +++
.../default/package-updates-body.html.hbs | 6 +
.../default/package-updates-body.txt.hbs | 3 +++
.../default/package-updates-subject.txt.hbs | 1 +
templates/default/replication-body.html.hbs | 18 ++
templates/default/replication-body.txt.hbs| 12 ++
templates/default/replication-subject.txt.hbs | 1 +
templates/default/test-body.html.hbs | 1 +
templates/default/test-body.txt.hbs | 1 +
templates/default/test-subject.txt.hbs| 1 +
templates/default/vzdump-body.html.hbs| 11 +
templates/default/vzdump-body.txt.hbs | 10
templates/default/vzdump-subject.txt.hbs | 1 +
17 files changed, 95 insertions(+), 46 deletions(-)
create mode 100644 templates/Makefile
create mode 100644 templates/default/package-updates-body.html.hbs
create mode 100644 templates/default/package-updates-body.txt.hbs
create mode 100644 templates/default/package-updates-subject.txt.hbs
create mode 100644 templates/default/replication-body.html.hbs
create mode 100644 templates/default/replication-body.txt.hbs
create mode 100644 templates/default/replication-subject.txt.hbs
create mode 100644 templates/default/test-body.html.hbs
create mode 100644 templates/default/test-body.txt.hbs
create mode 100644 templates/default/test-subject.txt.hbs
create mode 100644 templates/default/vzdump-body.html.hbs
create mode 100644 templates/default/vzdump-body.txt.hbs
create mode 100644 templates/default/vzdump-subject.txt.hbs
diff --git a/Makefile b/Makefile
index 28295395..337682b3 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,7 @@ DSC=$(PACKAGE)_$(DEB_VERSION).dsc
DEB=$(PACKAGE)_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
DESTDIR=
-SUBDIRS = aplinfo PVE bin www services configs network-hooks test
+SUBDIRS = aplinfo PVE bin www services configs network-hooks test templates
all: $(SUBDIRS)
set -e && for i in $(SUBDIRS); do $(MAKE) -C $$i; done
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 19f0baca..d0e7c544 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -238,12 +238,6 @@ __PACKAGE__->register_method({
return $pkglist;
}});
-my $updates_available_subject_template = "New software packages available
({{hostname}})";
-my $updates_available_body_template = <register_method({
name => 'update_database',
path => 'update',
@@ -358,8 +352,7 @@ __PACKAGE__->register_method({
};
PVE::Notify::info(
- $updates_available_subject_template,
- $updates_available_body_template,
+ "package-updates",
$template_data,
$metadata_fields,
);
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 0dc944c9..d84ac1ab 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -92,23 +92,6 @@ my sub _should_mail_at_failcount {
return $i * 48 == $fail_count;
};
-my $replication_error_subject_template = "Replication Job: '{{job-id}}'
failed";
-my $replication_error_body_template = < 1024*1024;
sub send_notification {
@@ -588,8 +574,7 @@ sub send_notification {
PVE::Notify::notify(
$severity,
- $subject_template,
- $body_template,
+ "vzdump",
$notification_props,
$fields,
$notification_config
@@ -600,8 +585,7 @@ sub send_notification {
# no email addresses were configured.
PVE::Notify::notify(
$severity,
- $subject_template,
- $body_template,
+ "vzdump",
$notification_props,
$fields,
);
diff --git a/templates/Makefile b/templates/Makefile
new file mode 100644
index ..236988c5
--- /dev/null
+++ b/templates/Makefile
@@ -0,0 +1,24 @@
+NOTIFICATION_TEMPLATES=\
+ default/test-subject.txt.hbs\
+ default/test-body.txt.hbs \
+ default/test-body.html.hbs \
+ default/vzdump-subject.txt.hbs \
+ default/vzdump-body.txt.hbs \
+ default/vzdump-body.html.hbs\
+ d
[pve-devel] [PATCH manager v3 7/8] tests: remove vzdump_notification test
With the upcoming changes in how we send notifications, this one
really becomes pretty annoying to keep working. The location where
templates are looked up are defined in the proxmox_notify crate, so
there is no easy way to mock this for testing.
The test itself seemed not super valuable, mainly testing if
the backup logs are shortened if they ware too long - so they are just
removed.
Signed-off-by: Lukas Wagner
---
test/Makefile| 6 +-
test/vzdump_notification_test.pl | 101 ---
2 files changed, 1 insertion(+), 106 deletions(-)
delete mode 100755 test/vzdump_notification_test.pl
diff --git a/test/Makefile b/test/Makefile
index 62d75050..743804c8 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,7 +5,7 @@ all:
export PERLLIB=..
.PHONY: check
-check: test-replication test-balloon test-vzdump-notification test-vzdump
test-osd
+check: test-replication test-balloon test-vzdump test-osd
.PHONY: test-balloon
test-balloon:
@@ -17,10 +17,6 @@ test-replication: replication1.t replication2.t
replication3.t replication4.t re
replication%.t: replication_test%.pl
./$<
-.PHONY: test-vzdump-notification
-test-vzdump-notification:
- ./vzdump_notification_test.pl
-
.PHONY: test-vzdump
test-vzdump: test-vzdump-guest-included test-vzdump-new
diff --git a/test/vzdump_notification_test.pl b/test/vzdump_notification_test.pl
deleted file mode 100755
index 631606bb..
--- a/test/vzdump_notification_test.pl
+++ /dev/null
@@ -1,101 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-
-use lib '..';
-
-use Test::More tests => 3;
-use Test::MockModule;
-
-use PVE::VZDump;
-
-my $STATUS = qr/.*status.*/;
-my $NO_LOGFILE = qr/.*Could not open log file.*/;
-my $LOG_TOO_LONG = qr/.*Log output was too long.*/;
-my $TEST_FILE_PATH = '/tmp/mail_test';
-my $TEST_FILE_WRONG_PATH = '/tmp/mail_test_wrong';
-
-sub prepare_mail_with_status {
-open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
-print TEST_FILE "start of log file\n";
-print TEST_FILE "status: 0\% this should not be in the mail\n";
-print TEST_FILE "status: 55\% this should not be in the mail\n";
-print TEST_FILE "status: 100\% this should not be in the mail\n";
-print TEST_FILE "end of log file\n";
-close(TEST_FILE);
-}
-
-sub prepare_long_mail {
-open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
-# 0.5 MB * 2 parts + the overview tables gives more than 1 MB mail
-print TEST_FILE "a" x (1024*1024);
-close(TEST_FILE);
-}
-
-my $result_text;
-my $result_properties;
-
-my $mock_notification_module = Test::MockModule->new('PVE::Notify');
-my $mocked_notify = sub {
-my ($severity, $title, $text, $properties, $metadata) = @_;
-
-$result_text = $text;
-$result_properties = $properties;
-};
-my $mocked_notify_short = sub {
-my (@params) = @_;
-return $mocked_notify->('', @params);
-};
-
-$mock_notification_module->mock(
-'notify' => $mocked_notify,
-'info' => $mocked_notify_short,
-'notice' => $mocked_notify_short,
-'warning' => $mocked_notify_short,
-'error' => $mocked_notify_short,
-);
-$mock_notification_module->mock('cfs_read_file', sub {
-my $path = shift;
-
-if ($path eq 'datacenter.cfg') {
-return {};
-} elsif ($path eq 'notifications.cfg' || $path eq
'priv/notifications.cfg') {
-return '';
-} else {
- die "unexpected cfs_read_file\n";
-}
-});
-
-my $MAILTO = ['test_addr...@proxmox.com'];
-my $SELF = {
-opts => { mailto => $MAILTO },
-cmdline => 'test_command_on_cli',
-};
-
-my $task = { state => 'ok', vmid => '100', };
-my $tasklist;
-sub prepare_test {
-$result_text = undef;
-$task->{tmplog} = shift;
-$tasklist = [ $task ];
-}
-
-{
-prepare_test($TEST_FILE_WRONG_PATH);
-PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
-like($result_properties->{logs}, $NO_LOGFILE, "Missing logfile is
detected");
-}
-{
-prepare_test($TEST_FILE_PATH);
-prepare_mail_with_status();
-PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
-unlike($result_properties->{"status-text"}, $STATUS, "Status are not in
text part of mails");
-}
-{
-prepare_test($TEST_FILE_PATH);
-prepare_long_mail();
-PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
-like($result_properties->{logs}, $LOG_TOO_LONG, "Text part of mails gets
shortened");
-}
-unlink $TEST_FILE_PATH;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v3 6/8] gitignore: ignore any test artifacts
Signed-off-by: Lukas Wagner --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index e8d1eb27..481ae1e0 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ dest/ /www/mobile/pvemanager-mobile.js /www/touch/touch-[0-9]*/ /pve-manager-[0-9]*/ +/test/.mocked_* +/test/*.tmp -- 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 cluster v3 4/8] notify: use named template instead of passing template strings
The notification system will now load template files from a defined
location. The template to use is now passed to proxmox_notify, instead
of separate template strings for subject/body.
Signed-off-by: Lukas Wagner
---
src/PVE/Notify.pm | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
index 872eb25..c514111 100644
--- a/src/PVE/Notify.pm
+++ b/src/PVE/Notify.pm
@@ -58,17 +58,16 @@ sub write_config {
}
my $send_notification = sub {
-my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+my ($severity, $template_name, $template_data, $fields, $config) = @_;
$config = read_config() if !defined($config);
-$config->send($severity, $title, $message, $template_data, $fields);
+$config->send($severity, $template_name, $template_data, $fields);
};
sub notify {
-my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+my ($severity, $template_name, $template_data, $fields, $config) = @_;
$send_notification->(
$severity,
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -76,11 +75,10 @@ sub notify {
}
sub info {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'info',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -88,11 +86,10 @@ sub info {
}
sub notice {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'notice',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -100,11 +97,10 @@ sub notice {
}
sub warning {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'warning',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -112,11 +108,10 @@ sub warning {
}
sub error {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'error',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
--
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 v3 1/8] notify: use file based notification templates
Instead of passing literal template strings to the notification
system, we now only pass an identifier. This identifier will be used
load the template files from a product-specific directory.
Signed-off-by: Lukas Wagner
---
common/src/notify.rs | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 8f9f38f..d965417 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -94,16 +94,14 @@ mod export {
fn send(
#[try_from_ref] this: ,
severity: Severity,
-title: String,
-body: String,
+template_name: String,
template_data: Option,
fields: Option>,
) -> Result<(), HttpError> {
let config = this.config.lock().unwrap();
-let notification = Notification::new_templated(
+let notification = Notification::from_template(
severity,
-title,
-body,
+template_name,
template_data.unwrap_or_default(),
fields.unwrap_or_default(),
);
--
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 docs v6 15/16] notifications: describe new notification metadata fields
On 2024-04-22 11:10, Fiona Ebner wrote: >> +| 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) > > I think the "(only for backup jobs)" should not be there, or? > No, that should be there. The `job-id` field is set if the backup is started as a scheduled job and if that job is started by hand, but *not* if e.g. a single VM is backed up manually. Would not make sense for the latter, since that one has no associated job. -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v6 15/16] 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 57053c8..dec878a 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -289,19 +289,21 @@ Notification Events [width="100%",options="header"] |=== -| Event| `type`| Severity | Metadata fields (in addition to `type`) -| System updates available |`package-updates` | `info` | `hostname` -| Cluster node fenced |`fencing` | `error` | `hostname` -| Storage replication failed |`replication` | `error` | - -| Backup finished |`vzdump` | `info` (`error` on failure) | `hostname` -| Mail for root|`system-mail` | `unknown`| - +| Event| `type`| Severity | Metadata fields (in addition to `type`) +| System updates available |`package-updates` | `info` | `hostname` +| Cluster node fenced |`fencing` | `error` | `hostname` +| Storage replication job failed |`replication` | `error` | `hostname`, `job-id` +| Backup succeeded |`vzdump` | `info` | `hostname`, `job-id` (only for backup jobs) +| Backup failed|`vzdump` | `error` | `hostname`, `job-id` (only for backup jobs) +| Mail for root|`system-mail` | `unknown`| `hostname` |=== [width="100%",options="header"] |=== -| Field name | Description -| `type` | Type of the notifcation -| `hostname` | Hostname, without domain (e.g. `pve1`) +| Field name| Description +| `type`| Type of the notifcation +| `hostname`| Hostname, without domain (e.g. `pve1`) +| `job-id` | Job ID |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v6 16/16] 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 dec878a..07f0b3e 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -221,11 +221,16 @@ configurable schedule. Field Matching Rules Notifications have a selection of metadata fields that can be matched. +When using `exact` as a matching mode, a `,` can be used as a separator. +The matching rule then matches if the metadata field has *any* of the specified +values. * `match-field exact:type=vzdump` Only match notifications about backups. +* `match-field exact:type=replication,fencing` Match `replication` and `fencing` notifications. * `match-field regex:hostname=^.+\.example\.com$` Match the hostname of the node. + If a matched metadata field does not exist, the notification will not be matched. For instance, a `match-field regex:hostname=.*` directive will only match @@ -267,18 +272,7 @@ matcher: backup-failures comment Send notifications about backup failures to one group of admins matcher: cluster-failures -match-field exact:type=replication -match-field exact:type=fencing -mode any -target cluster-admins -comment Send cluster-related notifications to other group of admins - - -The last matcher could also be rewritten using a field matcher with a regular -expression: - -matcher: cluster-failures -match-field regex:type=^(replication|fencing)$ +match-field exact:type=replication,fencing target cluster-admins comment Send cluster-related notifications to other group of admins -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v6 14/16] notification: clarify that 'hostname' does not include the domain
This was a bit inconsistent between the different notification types: - APT/VZDump included the domain part - fence notifications did not A decision has been made to unify this by removing the domain part from APT/VZDump notifications. Signed-off-by: Lukas Wagner --- notifications.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notifications.adoc b/notifications.adoc index 46aff6a..57053c8 100644 --- a/notifications.adoc +++ b/notifications.adoc @@ -301,7 +301,7 @@ Notification Events |=== | Field name | Description | `type` | Type of the notifcation -| `hostname` | Hostname, including domain (e.g. `pve1.example.com`) +| `hostname` | Hostname, without domain (e.g. `pve1`) |=== System Mail Forwarding -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 07/16] api: replication: include 'hostname' field for notifications
The field contains the hostname of the host (without any domain part)
which sends the notification. This field can be used in match-field
match rules.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 5d75f8da..917804de 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -142,6 +142,8 @@ my sub _handle_job_err {
my $metadata_fields = {
type => "replication",
"job-id" => $job->{id},
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 04/16] 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/Utils.js | 1 +
www/manager6/panel/BackupAdvancedOptions.js | 15 +++
2 files changed, 16 insertions(+)
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 54aa8bac..5216198b 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1963,6 +1963,7 @@ Ext.define('PVE.Utils', {
singleton: true,
constructor: function() {
var me = this;
+
Ext.apply(me, me.utilities);
Proxmox.Utils.override_task_descriptions({
diff --git a/www/manager6/panel/BackupAdvancedOptions.js
b/www/manager6/panel/BackupAdvancedOptions.js
index 0ea585a8..144819e7 100644
--- a/www/manager6/panel/BackupAdvancedOptions.js
+++ b/www/manager6/panel/BackupAdvancedOptions.js
@@ -67,6 +67,17 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
},
column1: [
+ {
+ xtype: 'pmxDisplayEditField',
+ vtype: 'ConfigId',
+ fieldLabel: gettext('Job ID'),
+ emptyText: gettext('Autogenerate'),
+ name: 'id',
+ allowBlank: true,
+ cbind: {
+ editable: '{isCreate}',
+ },
+ },
{
xtype: 'pveBandwidthField',
name: 'bwlimit',
@@ -135,6 +146,10 @@ Ext.define('PVE.panel.BackupAdvancedOptions', {
],
column2: [
+ {
+ xtype: 'displayfield',
+ value: gettext('Can be used in notification matchers to match this
job.'),
+ },
{
xtype: 'displayfield',
value: gettext('Limit I/O bandwidth.'),
--
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 v6 05/16] api: replication: add 'job-id' to notification metadata
This allows users to create notification match rules for specific
replication jobs, if they so desire.
Signed-off-by: Lukas Wagner
---
PVE/API2/Replication.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 0dc944c9..5d75f8da 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -140,8 +140,8 @@ my sub _handle_job_err {
};
my $metadata_fields = {
- # TODO: Add job-id?
type => "replication",
+ "job-id" => $job->{id},
};
eval {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 09/16] 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 5216198b..89929d21 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -2060,6 +2060,17 @@ Ext.define('PVE.Utils', {
zfscreate: [gettext('ZFS Storage'), gettext('Create')],
zfsremove: ['ZFS Pool', gettext('Remove')],
});
+
+ Proxmox.Utils.overrideNotificationFieldName({
+ 'job-id': gettext('Job ID'),
+ });
+
+ Proxmox.Utils.overrideNotificationFieldValue({
+ 'package-updates': gettext('Package updates are available'),
+ 'vzdump': gettext('Backup notifications'),
+ 'replication': gettext('Replication job notifications'),
+ 'fencing': gettext('Node fencing notifications'),
+ });
},
});
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit v6 10/16] 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',
items: [
{
@@ -1000,6 +1024,8 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings',
{
['notall', gettext('At least one rule does not match')],
['notany', gettext('No rule matches')],
],
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
bind: {
hidden: '{!showMatchingMode}',
disabled: '{!show
[pve-devel] [PATCH widget-toolkit v6 13/16] 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 10b4af2..5666e12 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -380,34 +380,7 @@ Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
}
return !record.isRoot();
},
- typeIsMatchSeverity: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- get: function(record) {
- return record?.get('type') === 'match-severity';
- },
- },
- matchSeverityValue: {
- bind: {
- bindTo: '{selectedRecord}',
- deep: true,
- },
- set: function(value) {
- let record = this.get('selectedRecord');
- let currentData = record.get('data');
- record.set({
- data: {
- ...currentData,
- value: value,
- },
- });
- },
- get: function(record) {
- return record?.get('data')?.value;
- },
- },
+
rootMode: {
bind: {
bindTo: '{selectedRecord}',
@@ -944,27 +917,7 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
},
},
{
- xtype: 'proxmoxKVComboBox',
- fieldLabel: gettext('Severities to match'),
- isFormField: false,
- allowBlank: true,
- multiSelect: true,
- field: 'value',
- // Hide initially to avoid glitches when opening the window
- hidden: true,
- bind: {
- value: '{matchSeverityValue}',
- hidden: '{!typeIsMatchSeverity}',
- disabled: '{!typeIsMatchSeverity}',
- },
-
- comboItems: [
- ['info', gettext('Info')],
- ['notice', gettext('Notice')],
- ['warning', gettext('Warning')],
- ['error', gettext('Error')],
- ['unknown', gettext('Unknown')],
- ],
+ xtype: 'pmxNotificationMatchSeveritySettings',
},
{
xtype: 'pmxNotificationMatchCalendarSettings',
@@ -1047,6 +1000,84 @@ Ext.define('Proxmox.panel.MatchCalendarSettings', {
},
});
+Ext.define('Proxmox.panel.MatchSeveritySettings', {
+extend: 'Ext.panel.Panel',
+xtype: 'pmxNotificationMatchSeveritySettings',
+border: false,
+layout: 'anchor',
+// Hide initially to avoid glitches when opening the window
+hidden: true,
+bind: {
+ hidden: '{!typeIsMatchSeverity}',
+},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchSeverity: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-severity';
+ },
+ },
+ matchSeverityValue: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ set: function(value) {
+ let record = this.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ },
+},
+items: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Severities to match'),
+ isFormField: false,
+ allowBlank: true,
+ multiSelect: true,
+ field: 'value',
+ // Hide initially to avoid glitches when opening the window
+ hidden: true,
+ bind: {
+ value: '{matchSeverityValue}',
+ hidden: '{!typeIsMatchSeverity}',
+ disabled: '{!typeIsMatchSeverity}',
+ },
+
+ comboItems: [
+ ['info', gettext('Info')],
+ ['notice', gettext('Notice
[pve-devel] [PATCH widget-toolkit v6 11/16] 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 | 186 +-
1 file changed, 92 insertions(+), 94 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js
b/src/window/NotificationMatcherEdit.js
index be33efe..5429d1b 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,95 @@ Ext.define('Proxmox.panel.MatchFieldSettings', {
},
},
},
+viewModel: {
+ // parent is set in `initComponents`
+ formulas: {
+ typeIsMatchField: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-field';
+ },
+ },
+ isRegex: function(get) {
+ return get('matchFieldType') === 'regex';
+ },
+ matchFieldType: {
+ bind: {
+
[pve-devel] [PATCH manager v6 08/16] 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->{comment},
+ field => 'job-id'
+ };
+ }
+ }
+ # The API call returns only returns jobs for which the user
+ # has adequate permissions.
+ for my $sync_job (@{PVE::API2::ReplicationConfig->index({})}) {
+ push @$values, {
+ value => $sync_job->{id},
+ comment => $sync_job->{comment},
+ field => 'job-id'
+ };
+ }
+
+ for my $node (@{PVE::Cluster::get_nodelist()}) {
+ push @$values, {
+ value => $
[pve-devel] [PATCH widget-toolkit v6 12/16] 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 5429d1b..10b4af2 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/guest-common/manager/widget-toolkit v6 00/16] notifications: notification metadata matching improvements
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) 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) 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) - widget-toolkit relies on a new API endpoint provided by pve-manager - the changes in the backend for matching multiple values in "exact" mode have already been rolled out as of libpve-rs-perl 0.8.8 --> this means that libpve-notify-perl (which pulls in libpve-rs-perl) must depend on libpve-rs-perl 0.8.8 at minimum - otherwise the notification stack cannot understand the comma-separated array of matched values Changelog: - 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 (8): 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 PVE/API2/APT.pm | 3 +- PVE/API2/Cluster/Notifications.pm | 139 PVE/API2/Replication.pm | 4 +- PVE/API2/VZDump.pm | 8 ++ PVE/Jobs/VZDump.pm | 4 +- PVE/VZDump.pm | 14 +- www/manager6/Utils.js | 12 ++ www/manager6/dc/Backup.js | 3 +- www/manager6/panel/BackupAdvancedOptions.js | 15 +++ 9 files changed, 191 insertions(+), 11 deletions(-) proxmox-widget-toolkit: Lukas Wagner (4): notification: matcher: match-field: show known fields/values notification: matcher: move match-field formulas to local viewModel notification: matcher: move match-calendar fields to panel notification: matcher: move match-severity fields to panel src/data/model/NotificationConfig.js | 12 + src/window/Not
[pve-devel] [PATCH manager v6 06/16] vzdump: apt: notification: do not include domain in 'hostname' field
- The man page warns about the usage of `hostname -f`, since a host
may have multiple domains (or none at all)
- The fallback PVE::INotify::nodename() already only returned the
hostname without the domain part
- Fencing notifications didn't include the domain part anyway
This may result in soft-breakage for any users who have already relied
on the domain being present. If there is need for it, it could include
a fqdn metadata field.
The hostname property used for rendering the notification template
is unaffected for now.
Signed-off-by: Lukas Wagner
---
PVE/API2/APT.pm | 3 ++-
PVE/VZDump.pm | 8
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 19f0baca..c37abb0c 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -354,7 +354,8 @@ __PACKAGE__->register_method({
# matchers.
my $metadata_fields = {
type => 'package-updates',
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
PVE::Notify::info(
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index df5165bd..a860540b 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -508,10 +508,9 @@ sub send_notification {
"See Task History for details!\n";
};
-my $hostname = get_hostname();
-
my $notification_props = {
- "hostname" => $hostname,
+ # Hostname, might include domain part
+ "hostname" => get_hostname(),
"error-message" => $err,
"guest-table" => build_guest_table($tasklist),
"logs" => $text_log_part,
@@ -522,7 +521,8 @@ sub send_notification {
my $fields = {
type => "vzdump",
- hostname => $hostname,
+ # Hostname (without domain part)
+ hostname => PVE::INotify::nodename(),
};
# Add backup-job metadata field in case this is a backup job.
$fields->{'job-id'} = $job_id if $job_id;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 02/16] 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.
Signed-off-by: Lukas Wagner
---
PVE/API2/VZDump.pm | 8
PVE/Jobs/VZDump.pm | 4 +++-
PVE/VZDump.pm | 6 +++---
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index f66fc740..15236ea5 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -52,6 +52,14 @@ __PACKAGE__->register_method ({
parameters => {
additionalProperties => 0,
properties => PVE::VZDump::Common::json_config_properties({
+ 'job-id' => {
+ description => "The ID of the backup job. If set, the
'backup-job' metadata field"
+ . " of the backup notification will be set to this value.",
+ type => 'string',
+ format => 'pve-configid',
+ maxLength => 256,
+ optional => 1,
+ },
stdout => {
type => 'boolean',
description => "Write tar to stdout, not to a file.",
diff --git a/PVE/Jobs/VZDump.pm b/PVE/Jobs/VZDump.pm
index b8e57945..2dad3f55 100644
--- a/PVE/Jobs/VZDump.pm
+++ b/PVE/Jobs/VZDump.pm
@@ -12,7 +12,7 @@ use PVE::API2::VZDump;
use base qw(PVE::VZDump::JobBase);
sub run {
-my ($class, $conf) = @_;
+my ($class, $conf, $job_id) = @_;
my $props = $class->properties();
# remove all non vzdump related options
@@ -20,6 +20,8 @@ sub run {
delete $conf->{$opt} if !defined($props->{$opt});
}
+$conf->{'job-id'} = $job_id;
+
# Required as string parameters # FIXME why?! we could just check ref()
for my $key (keys $PVE::VZDump::Common::PROPERTY_STRINGS->%*) {
if ($conf->{$key} && ref($conf->{$key}) eq 'HASH') {
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 02244cd7..df5165bd 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -474,6 +474,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';
@@ -520,12 +521,11 @@ sub send_notification {
};
my $fields = {
- # TODO: There is no straight-forward way yet to get the
- # backup job id here... (I think pvescheduler would need
- # to pass that to the vzdump call?)
type => "vzdump",
hostname => $hostname,
};
+# Add backup-job metadata field in case this is a backup job.
+$fields->{'job-id'} = $job_id if $job_id;
my $severity = $failed ? "error" : "info";
my $email_configured = $mailto && scalar(@$mailto);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v6 03/16] ui: dc: backup: send 'job-id' property when starting a backup job manually
Signed-off-by: Lukas Wagner
---
www/manager6/dc/Backup.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 2619a77b..d68f553c 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -586,11 +586,12 @@ Ext.define('PVE.dc.BackupView', {
job = Ext.clone(job);
let jobNode = job.node;
+ job['job-id'] = job.id;
+ delete job.id;
// Remove properties related to scheduling
delete job.enabled;
delete job.starttime;
delete job.dow;
- delete job.id;
delete job.schedule;
delete job.type;
delete job.node;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-guest-common v6 01/16] 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 1539444..e835b05 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -496,7 +496,7 @@ sub command_line {
foreach my $p (keys %$param) {
next if $p eq 'id' || $p eq 'vmid' || $p eq 'starttime' ||
- $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled';
+ $p eq 'dow' || $p eq 'stdout' || $p eq 'enabled' || $p eq
'job-id';
my $v = $param->{$p};
my $pd = $confdesc->{$p} || die "no such vzdump option '$p'\n";
if ($p eq 'exclude-path') {
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH proxmox v2 01/20] notify: switch to file-based templating system
Instead of passing the template strings for subject and body when
constructing a notification, we pass only the name of a template.
When rendering the template, the name of the template is used to find
corresponding template files. For PVE, they are located at
/usr/share/proxmox-ve/templates/default. The `default` part is
the 'template namespace', which is a preparation for user-customizable
and/or translatable notifications.
Previously, the same template string was used to render HTML and
plaintext notifications. This was achieved by providing some template
helpers that 'abstract away' HTML/plaintext formatting. However,
in hindsight this turned out to be pretty finicky. Since the
current changes lay the foundations for user-customizable notification
templates, I ripped these abstractions out. Now there are simply two
templates, one for plaintext, one for HTML.
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
proxmox-notify/examples/render.rs| 63
proxmox-notify/src/context/mod.rs| 10 +-
proxmox-notify/src/context/pbs.rs| 16 ++
proxmox-notify/src/context/pve.rs| 15 ++
proxmox-notify/src/context/test.rs | 9 ++
proxmox-notify/src/endpoints/gotify.rs | 9 +-
proxmox-notify/src/endpoints/sendmail.rs | 13 +-
proxmox-notify/src/endpoints/smtp.rs | 11 +-
proxmox-notify/src/lib.rs| 27 ++--
proxmox-notify/src/matcher.rs| 24 +--
proxmox-notify/src/renderer/html.rs | 14 --
proxmox-notify/src/renderer/mod.rs | 197 +++
proxmox-notify/src/renderer/plaintext.rs | 39 -
13 files changed, 137 insertions(+), 310 deletions(-)
delete mode 100644 proxmox-notify/examples/render.rs
diff --git a/proxmox-notify/examples/render.rs
b/proxmox-notify/examples/render.rs
deleted file mode 100644
index d705fd0..000
--- a/proxmox-notify/examples/render.rs
+++ /dev/null
@@ -1,63 +0,0 @@
-use proxmox_notify::renderer::{render_template, TemplateRenderer};
-use proxmox_notify::Error;
-
-use serde_json::json;
-
-const TEMPLATE: = r#"
-{{ heading-1 "Backup Report"}}
-A backup job on host {{host}} was run.
-
-{{ heading-2 "Guests"}}
-{{ table table }}
-The total size of all backups is {{human-bytes total-size}}.
-
-The backup job took {{duration total-time}}.
-
-{{ heading-2 "Logs"}}
-{{ verbatim-monospaced logs}}
-
-{{ heading-2 "Objects"}}
-{{ object table }}
-"#;
-
-fn main() -> Result<(), Error> {
-let properties = json!({
-"host": "pali",
-"logs": "100: starting backup\n100: backup failed",
-"total-size": 1024 * 1024 + 2048 * 1024,
-"total-time": 100,
-"table": {
-"schema": {
-"columns": [
-{
-"label": "VMID",
-"id": "vmid"
-},
-{
-"label": "Size",
-"id": "size",
-"renderer": "human-bytes"
-}
-],
-},
-"data" : [
-{
-"vmid": 1001,
-"size": "1048576"
-},
-{
-"vmid": 1002,
-"size": 2048 * 1024,
-}
-]
-}
-});
-
-let output = render_template(TemplateRenderer::Html, TEMPLATE,
)?;
-println!("{output}");
-
-let output = render_template(TemplateRenderer::Plaintext, TEMPLATE,
)?;
-println!("{output}");
-
-Ok(())
-}
diff --git a/proxmox-notify/src/context/mod.rs
b/proxmox-notify/src/context/mod.rs
index cc68603..c0a5a13 100644
--- a/proxmox-notify/src/context/mod.rs
+++ b/proxmox-notify/src/context/mod.rs
@@ -1,6 +1,8 @@
use std::fmt::Debug;
use std::sync::Mutex;
+use crate::Error;
+
#[cfg(any(feature = "pve-context", feature = "pbs-context"))]
pub mod common;
#[cfg(feature = "pbs-context")]
@@ -20,8 +22,14 @@ pub trait Context: Send + Sync + Debug {
fn default_sendmail_from() -> String;
/// Proxy configuration for the current node
fn http_proxy_config() -> Option;
-// Return default config for built-in targets/matchers.
+/// Return default config for built-in targets/matchers.
fn default_config() -> &'static str;
+/// Lookup a template in a certain (optional) namespace
+fn lookup_template(
+,
+filename: ,
+namespace: Option<>,
+) -> Result, Error>;
}
#[cfg(not(test))]
diff --git a/proxmox-notify/src/context/pbs.rs
b/prox
[pve-devel] [PATCH proxmox v2 02/20] notify: make api methods take config struct ownership
This saves us from some of the awkward cloning steps when updating.
Suggested-by: Wolfgang Bumiller
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/api/gotify.rs | 46 +-
proxmox-notify/src/api/matcher.rs | 38 +++
proxmox-notify/src/api/mod.rs | 14 +++---
proxmox-notify/src/api/sendmail.rs | 40 +++
proxmox-notify/src/api/smtp.rs | 78 +++---
5 files changed, 108 insertions(+), 108 deletions(-)
diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index a93a024..15d94cb 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -41,8 +41,8 @@ pub fn get_endpoint(config: , name: ) ->
Result Result<(), HttpError> {
if endpoint_config.name != private_endpoint_config.name {
// Programming error by the user of the crate, thus we panic
@@ -51,11 +51,11 @@ pub fn add_endpoint(
super::ensure_unique(config, _config.name)?;
-set_private_config_entry(config, private_endpoint_config)?;
+set_private_config_entry(config, _endpoint_config)?;
config
.config
-.set_data(_config.name, GOTIFY_TYPENAME, endpoint_config)
+.set_data(_config.name, GOTIFY_TYPENAME, _config)
.map_err(|e| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -75,8 +75,8 @@ pub fn add_endpoint(
pub fn update_endpoint(
config: Config,
name: ,
-endpoint_config_updater: ,
-private_endpoint_config_updater: ,
+endpoint_config_updater: GotifyConfigUpdater,
+private_endpoint_config_updater: GotifyPrivateConfigUpdater,
delete: Option<&[DeleteableGotifyProperty]>,
digest: Option<&[u8]>,
) -> Result<(), HttpError> {
@@ -93,11 +93,11 @@ pub fn update_endpoint(
}
}
-if let Some(server) = _config_updater.server {
-endpoint.server = server.into();
+if let Some(server) = endpoint_config_updater.server {
+endpoint.server = server;
}
-if let Some(token) = _endpoint_config_updater.token {
+if let Some(token) = private_endpoint_config_updater.token {
set_private_config_entry(
config,
{
@@ -107,12 +107,12 @@ pub fn update_endpoint(
)?;
}
-if let Some(comment) = _config_updater.comment {
-endpoint.comment = Some(comment.into());
+if let Some(comment) = endpoint_config_updater.comment {
+endpoint.comment = Some(comment)
}
-if let Some(disable) = _config_updater.disable {
-endpoint.disable = Some(*disable);
+if let Some(disable) = endpoint_config_updater.disable {
+endpoint.disable = Some(disable);
}
config
@@ -173,13 +173,13 @@ mod tests {
pub fn add_default_gotify_endpoint(config: Config) -> Result<(),
HttpError> {
add_endpoint(
config,
- {
+GotifyConfig {
name: "gotify-endpoint".into(),
server: "localhost".into(),
comment: Some("comment".into()),
..Default::default()
},
- {
+GotifyPrivateConfig {
name: "gotify-endpoint".into(),
token: "supersecrettoken".into(),
},
@@ -196,8 +196,8 @@ mod tests {
assert!(update_endpoint(
config,
"test",
-::default(),
-::default(),
+Default::default(),
+Default::default(),
None,
None
)
@@ -214,8 +214,8 @@ mod tests {
assert!(update_endpoint(
config,
"gotify-endpoint",
-::default(),
-::default(),
+Default::default(),
+Default::default(),
None,
Some(&[0; 32])
)
@@ -234,12 +234,12 @@ mod tests {
update_endpoint(
config,
"gotify-endpoint",
- {
+GotifyConfigUpdater {
server: Some("newhost".into()),
comment: Some("newcomment".into()),
..Default::default()
},
- {
+GotifyPrivateConfigUpdater {
token: Some("changedtoken".into()),
},
None,
@@ -263,8 +263,8 @@ mod tests {
update_endpoint(
config,
"gotify-endpoint",
-::default(),
-::default(),
+Default::default(),
+Default::default(),
Some(&[DeleteableGotifyProperty::Comment]),
None,
)?;
diff --git a/proxmox-notify/src/api/matcher.rs
b/proxmox-notify/src/api/matcher.rs
index ca01bc9..f0eabd9 100644
---
[pve-devel] [PATCH proxmox v2 03/20] notify: convert Option> -> Vec in config structs
Suggested-by: Wolfgang Bumiller
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/api/matcher.rs | 27 +++
proxmox-notify/src/api/mod.rs | 22 +---
proxmox-notify/src/api/sendmail.rs | 22 ++--
proxmox-notify/src/api/smtp.rs | 24 ++---
proxmox-notify/src/endpoints/common/mail.rs | 20 ---
proxmox-notify/src/endpoints/sendmail.rs| 14
proxmox-notify/src/endpoints/smtp.rs| 18 +-
proxmox-notify/src/lib.rs | 10 +++---
proxmox-notify/src/matcher.rs | 38 -
9 files changed, 96 insertions(+), 99 deletions(-)
diff --git a/proxmox-notify/src/api/matcher.rs
b/proxmox-notify/src/api/matcher.rs
index f0eabd9..63ec73d 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -38,10 +38,7 @@ pub fn get_matcher(config: , name: ) ->
Result
Result<(), HttpError> {
super::ensure_unique(config, _config.name)?;
-
-if let Some(targets) = matcher_config.target.as_deref() {
-super::ensure_endpoints_exist(config, targets)?;
-}
+super::ensure_endpoints_exist(config, _config.target)?;
config
.config
@@ -78,10 +75,10 @@ pub fn update_matcher(
if let Some(delete) = delete {
for deleteable_property in delete {
match deleteable_property {
-DeleteableMatcherProperty::MatchSeverity =>
matcher.match_severity = None,
-DeleteableMatcherProperty::MatchField => matcher.match_field =
None,
-DeleteableMatcherProperty::MatchCalendar =>
matcher.match_calendar = None,
-DeleteableMatcherProperty::Target => matcher.target = None,
+DeleteableMatcherProperty::MatchSeverity =>
matcher.match_severity.clear(),
+DeleteableMatcherProperty::MatchField =>
matcher.match_field.clear(),
+DeleteableMatcherProperty::MatchCalendar =>
matcher.match_calendar.clear(),
+DeleteableMatcherProperty::Target => matcher.target.clear(),
DeleteableMatcherProperty::Mode => matcher.mode = None,
DeleteableMatcherProperty::InvertMatch => matcher.invert_match
= None,
DeleteableMatcherProperty::Comment => matcher.comment = None,
@@ -91,15 +88,15 @@ pub fn update_matcher(
}
if let Some(match_severity) = matcher_updater.match_severity {
-matcher.match_severity = Some(match_severity);
+matcher.match_severity = match_severity;
}
if let Some(match_field) = matcher_updater.match_field {
-matcher.match_field = Some(match_field);
+matcher.match_field = match_field;
}
if let Some(match_calendar) = matcher_updater.match_calendar {
-matcher.match_calendar = Some(match_calendar);
+matcher.match_calendar = match_calendar;
}
if let Some(mode) = matcher_updater.mode {
@@ -120,7 +117,7 @@ pub fn update_matcher(
if let Some(target) = matcher_updater.target {
super::ensure_endpoints_exist(config, target.as_slice())?;
-matcher.target = Some(target);
+matcher.target = target;
}
config
@@ -244,9 +241,9 @@ matcher: matcher2
let matcher = get_matcher(, "matcher1")?;
assert_eq!(matcher.invert_match, None);
-assert!(matcher.match_severity.is_none());
-assert!(matches!(matcher.match_field, None));
-assert_eq!(matcher.target, None);
+assert!(matcher.match_severity.is_empty());
+assert!(matcher.match_field.is_empty());
+assert!(matcher.target.is_empty());
assert!(matcher.mode.is_none());
assert_eq!(matcher.comment, None);
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index bb0371d..a2cf29e 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -102,10 +102,8 @@ fn get_referrers(config: , entity: ) ->
Result, HttpE
let mut referrers = HashSet::new();
for matcher in matcher::get_matchers(config)? {
-if let Some(targets) = matcher.target {
-if targets.iter().any(|target| target == entity) {
-referrers.insert(matcher.name.clone());
-}
+if matcher.target.iter().any(|target| target == entity) {
+referrers.insert(matcher.name.clone());
}
}
@@ -149,11 +147,9 @@ fn get_referenced_entities(config: , entity: )
-> HashSet {
let mut new = HashSet::new();
for entity in entities {
-if let Ok(group) = matcher::get_matcher(config, entity) {
-if let Some(targets) = group.target {
-for target in targets {
-new.insert(target.clone());
-}
+
[pve-devel] [PATCH proxmox-perl-rs v2 14/20] notify: don't pass config structs by reference
proxmox_notify's api functions have been changed so that they take
ownership of config structs.
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
common/src/notify.rs | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index d965417..00a6056 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -151,7 +151,7 @@ mod export {
api::sendmail::add_endpoint(
config,
- {
+SendmailConfig {
name,
mailto,
mailto_user,
@@ -185,7 +185,7 @@ mod export {
api::sendmail::update_endpoint(
config,
name,
- {
+SendmailConfigUpdater {
mailto,
mailto_user,
from_address,
@@ -236,7 +236,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::gotify::add_endpoint(
config,
- {
+GotifyConfig {
name: name.clone(),
server,
comment,
@@ -244,7 +244,7 @@ mod export {
filter: None,
origin: None,
},
- { name, token },
+GotifyPrivateConfig { name, token },
)
}
@@ -266,12 +266,12 @@ mod export {
api::gotify::update_endpoint(
config,
name,
- {
+GotifyConfigUpdater {
server,
comment,
disable,
},
- { token },
+GotifyPrivateConfigUpdater { token },
delete.as_deref(),
digest.as_deref(),
)
@@ -323,7 +323,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::smtp::add_endpoint(
config,
- {
+SmtpConfig {
name: name.clone(),
server,
port,
@@ -337,7 +337,7 @@ mod export {
disable,
origin: None,
},
- { name, password },
+SmtpPrivateConfig { name, password },
)
}
@@ -366,7 +366,7 @@ mod export {
api::smtp::update_endpoint(
config,
name,
- {
+SmtpConfigUpdater {
server,
port,
mode,
@@ -378,7 +378,7 @@ mod export {
comment,
disable,
},
- { password },
+SmtpPrivateConfigUpdater { password },
delete.as_deref(),
digest.as_deref(),
)
@@ -427,7 +427,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::matcher::add_matcher(
config,
- {
+MatcherConfig {
name,
match_severity,
match_field,
@@ -464,7 +464,7 @@ mod export {
api::matcher::update_matcher(
config,
name,
- {
+MatcherConfigUpdater {
match_severity,
match_field,
match_calendar,
--
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 v2 09/20] notify: derive `api` for Deleteable*Property
The API endpoints in Proxmox Backup Server require ApiType to be
implemented for any deserialized parameter.
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/endpoints/gotify.rs | 3 +++
proxmox-notify/src/endpoints/sendmail.rs | 7 +++
proxmox-notify/src/endpoints/smtp.rs | 9 +
proxmox-notify/src/matcher.rs| 9 +
4 files changed, 28 insertions(+)
diff --git a/proxmox-notify/src/endpoints/gotify.rs
b/proxmox-notify/src/endpoints/gotify.rs
index afdfabc..ee8ca51 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -81,10 +81,13 @@ pub struct GotifyEndpoint {
pub private_config: GotifyPrivateConfig,
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableGotifyProperty {
+/// Delete `comment`
Comment,
+/// Delete `disable`
Disable,
}
diff --git a/proxmox-notify/src/endpoints/sendmail.rs
b/proxmox-notify/src/endpoints/sendmail.rs
index fa04002..47901ef 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -73,14 +73,21 @@ pub struct SendmailConfig {
pub origin: Option,
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableSendmailProperty {
+/// Delete `author`
Author,
+/// Delete `comment`
Comment,
+/// Delete `disable`
Disable,
+/// Delete `from-address`
FromAddress,
+/// Delete `mailto`
Mailto,
+/// Delete `mailto-user`
MailtoUser,
}
diff --git a/proxmox-notify/src/endpoints/smtp.rs
b/proxmox-notify/src/endpoints/smtp.rs
index ded5baf..f04583a 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -104,16 +104,25 @@ pub struct SmtpConfig {
pub origin: Option,
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableSmtpProperty {
+/// Delete `author`
Author,
+/// Delete `comment`
Comment,
+/// Delete `disable`
Disable,
+/// Delete `mailto`
Mailto,
+/// Delete `mailto-user`
MailtoUser,
+/// Delete `password`
Password,
+/// Delete `port`
Port,
+/// Delete `username`
Username,
}
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index b387fef..2d30378 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -412,16 +412,25 @@ impl FromStr for CalendarMatcher {
}
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableMatcherProperty {
+/// Delete `comment`
Comment,
+/// Delete `disable`
Disable,
+/// Delete `invert-match`
InvertMatch,
+/// Delete `match-calendar`
MatchCalendar,
+/// Delete `match-field`
MatchField,
+/// Delete `match-severity`
MatchSeverity,
+/// Delete `mode`
Mode,
+/// Delete `target`
Target,
}
--
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/20] notify: use file based notification templates
Instead of passing literal template strings to the notification
system, we now only pass an identifier. This identifier will be used
load the template files from a product-specific directory.
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
common/src/notify.rs | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 8f9f38f..d965417 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -94,16 +94,14 @@ mod export {
fn send(
#[try_from_ref] this: ,
severity: Severity,
-title: String,
-body: String,
+template_name: String,
template_data: Option,
fields: Option>,
) -> Result<(), HttpError> {
let config = this.config.lock().unwrap();
-let notification = Notification::new_templated(
+let notification = Notification::from_template(
severity,
-title,
-body,
+template_name,
template_data.unwrap_or_default(),
fields.unwrap_or_default(),
);
--
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/20] notify: adapt to Option> to Vec changes in proxmox_notify
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
common/src/notify.rs | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 00a6056..e1b006b 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -153,8 +153,8 @@ mod export {
config,
SendmailConfig {
name,
-mailto,
-mailto_user,
+mailto: mailto.unwrap_or_default(),
+mailto_user: mailto_user.unwrap_or_default(),
from_address,
author,
comment,
@@ -329,8 +329,8 @@ mod export {
port,
mode,
username,
-mailto,
-mailto_user,
+mailto: mailto.unwrap_or_default(),
+mailto_user: mailto_user.unwrap_or_default(),
from_address,
author,
comment,
@@ -429,10 +429,10 @@ mod export {
config,
MatcherConfig {
name,
-match_severity,
-match_field,
-match_calendar,
-target,
+match_severity: match_severity.unwrap_or_default(),
+match_field: match_field.unwrap_or_default(),
+match_calendar: match_calendar.unwrap_or_default(),
+target: target.unwrap_or_default(),
mode,
invert_match,
comment,
--
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 v2 10/20] notify: derive Deserialize/Serialize for Notification struct
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/lib.rs | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 91c0b61..292396b 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -159,9 +159,11 @@ pub trait Endpoint {
fn disabled() -> bool;
}
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
pub enum Content {
/// Title and body will be rendered as a template
+#[serde(rename_all = "kebab-case")]
Template {
/// Name of the used template
template_name: String,
@@ -169,6 +171,7 @@ pub enum Content {
data: Value,
},
#[cfg(feature = "mail-forwarder")]
+#[serde(rename_all = "kebab-case")]
ForwardedMail {
/// Raw mail contents
raw: Vec,
@@ -182,7 +185,8 @@ pub enum Content {
},
}
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
pub struct Metadata {
/// Notification severity
severity: Severity,
@@ -192,7 +196,8 @@ pub struct Metadata {
additional_fields: HashMap,
}
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
/// Notification which can be sent
pub struct Notification {
/// Notification content
--
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 v2 11/20] notify: pbs context: include nodename in default sendmail author
The old notification stack in proxmox-backup includes the nodename, so
we include it here as well.
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/context/pbs.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-notify/src/context/pbs.rs
b/proxmox-notify/src/context/pbs.rs
index 70e993f..299f685 100644
--- a/proxmox-notify/src/context/pbs.rs
+++ b/proxmox-notify/src/context/pbs.rs
@@ -82,7 +82,7 @@ impl Context for PBSContext {
}
fn default_sendmail_author() -> String {
-"Proxmox Backup Server".into()
+format!("Proxmox Backup Server - {}", proxmox_sys::nodename())
}
fn default_sendmail_from() -> String {
--
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 v2 08/20] notify: api: add get_targets
This method allows us to get a list of all notification targets.
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/api/mod.rs | 77 +++
1 file changed, 77 insertions(+)
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index a2cf29e..a7f6261 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -3,6 +3,7 @@ use std::collections::HashSet;
use serde::{Deserialize, Serialize};
use proxmox_http_error::HttpError;
+use proxmox_schema::api;
use crate::{Config, Origin};
@@ -39,6 +40,82 @@ macro_rules! http_bail {
pub use http_bail;
pub use http_err;
+#[api]
+#[derive(Clone, Debug, Copy, Serialize, Deserialize, PartialEq, Eq,
PartialOrd)]
+#[serde(rename_all = "kebab-case")]
+/// Type of the endpoint.
+pub enum EndpointType {
+/// Sendmail endpoint
+#[cfg(feature = "sendmail")]
+Sendmail,
+/// SMTP endpoint
+#[cfg(feature = "smtp")]
+Smtp,
+/// Gotify endpoint
+#[cfg(feature = "gotify")]
+Gotify,
+}
+
+#[api]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd)]
+#[serde(rename_all = "kebab-case")]
+/// Target information
+pub struct Target {
+/// Name of the endpoint
+name: String,
+/// Origin of the endpoint
+origin: Origin,
+/// Type of the endpoint
+#[serde(rename = "type")]
+endpoint_type: EndpointType,
+/// Target is disabled
+#[serde(skip_serializing_if = "Option::is_none")]
+disable: Option,
+/// Comment
+#[serde(skip_serializing_if = "Option::is_none")]
+comment: Option,
+}
+
+/// Get a list of all notification targets.
+pub fn get_targets(config: ) -> Result, HttpError> {
+let mut targets = Vec::new();
+
+#[cfg(feature = "gotify")]
+for endpoint in gotify::get_endpoints(config)? {
+targets.push(Target {
+name: endpoint.name,
+origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+endpoint_type: EndpointType::Gotify,
+disable: endpoint.disable,
+comment: endpoint.comment,
+})
+}
+
+#[cfg(feature = "sendmail")]
+for endpoint in sendmail::get_endpoints(config)? {
+targets.push(Target {
+name: endpoint.name,
+origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+endpoint_type: EndpointType::Sendmail,
+disable: endpoint.disable,
+comment: endpoint.comment,
+})
+}
+
+#[cfg(feature = "smtp")]
+for endpoint in smtp::get_endpoints(config)? {
+targets.push(Target {
+name: endpoint.name,
+origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+endpoint_type: EndpointType::Smtp,
+disable: endpoint.disable,
+comment: endpoint.comment,
+})
+}
+
+Ok(targets)
+}
+
fn verify_digest(config: , digest: Option<&[u8]>) -> Result<(),
HttpError> {
if let Some(digest) = digest {
if config.digest != *digest {
--
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 v2 07/20] notify: give each notification a unique ID
We need this for queuing notifications on PBS from the unprivileged
proxy process.
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/Cargo.toml | 1 +
proxmox-notify/src/lib.rs | 11 +++
2 files changed, 12 insertions(+)
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 185b50a..797b1ac 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -28,6 +28,7 @@ proxmox-section-config = { workspace = true }
proxmox-serde.workspace = true
proxmox-sys = { workspace = true, optional = true }
proxmox-time.workspace = true
+proxmox-uuid = { workspace = true, features = ["serde"] }
[features]
default = ["sendmail", "gotify", "smtp"]
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 35dcb17..91c0b61 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -10,6 +10,7 @@ use serde_json::Value;
use proxmox_schema::api;
use proxmox_section_config::SectionConfigData;
+use proxmox_uuid::Uuid;
pub mod matcher;
use crate::config::CONFIG;
@@ -198,6 +199,8 @@ pub struct Notification {
content: Content,
/// Metadata
metadata: Metadata,
+/// Unique ID
+id: Uuid,
}
impl Notification {
@@ -217,6 +220,7 @@ impl Notification {
template_name: template_name.as_ref().to_string(),
data: template_data,
},
+id: Uuid::generate(),
}
}
#[cfg(feature = "mail-forwarder")]
@@ -246,8 +250,14 @@ impl Notification {
additional_fields,
timestamp: proxmox_time::epoch_i64(),
},
+id: Uuid::generate(),
})
}
+
+/// Return the unique ID of this notification.
+pub fn id() -> {
+
+}
}
/// Notification configuration
@@ -548,6 +558,7 @@ impl Bus {
template_name: "test".to_string(),
data: json!({ "target": target }),
},
+id: Uuid::generate(),
};
if let Some(endpoint) = self.endpoints.get(target) {
--
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 20/20] notifications: use named templates instead of in-code templates
This commit adapts notification sending for
- package update
- replication
- backups
to use named templates (installed in /usr/share/pve-manager/templates)
instead of passing template strings defined in code to the
notification stack.
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
Makefile | 2 +-
PVE/API2/APT.pm | 9 +--
PVE/API2/Replication.pm | 20 +---
PVE/VZDump.pm | 20 ++--
templates/Makefile| 24 +++
.../default/package-updates-body.html.hbs | 6 +
.../default/package-updates-body.txt.hbs | 3 +++
.../default/package-updates-subject.txt.hbs | 1 +
templates/default/replication-body.html.hbs | 18 ++
templates/default/replication-body.txt.hbs| 12 ++
templates/default/replication-subject.txt.hbs | 1 +
templates/default/test-body.html.hbs | 1 +
templates/default/test-body.txt.hbs | 1 +
templates/default/test-subject.txt.hbs| 1 +
templates/default/vzdump-body.html.hbs| 11 +
templates/default/vzdump-body.txt.hbs | 10
templates/default/vzdump-subject.txt.hbs | 1 +
17 files changed, 95 insertions(+), 46 deletions(-)
create mode 100644 templates/Makefile
create mode 100644 templates/default/package-updates-body.html.hbs
create mode 100644 templates/default/package-updates-body.txt.hbs
create mode 100644 templates/default/package-updates-subject.txt.hbs
create mode 100644 templates/default/replication-body.html.hbs
create mode 100644 templates/default/replication-body.txt.hbs
create mode 100644 templates/default/replication-subject.txt.hbs
create mode 100644 templates/default/test-body.html.hbs
create mode 100644 templates/default/test-body.txt.hbs
create mode 100644 templates/default/test-subject.txt.hbs
create mode 100644 templates/default/vzdump-body.html.hbs
create mode 100644 templates/default/vzdump-body.txt.hbs
create mode 100644 templates/default/vzdump-subject.txt.hbs
diff --git a/Makefile b/Makefile
index 28295395..337682b3 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,7 @@ DSC=$(PACKAGE)_$(DEB_VERSION).dsc
DEB=$(PACKAGE)_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
DESTDIR=
-SUBDIRS = aplinfo PVE bin www services configs network-hooks test
+SUBDIRS = aplinfo PVE bin www services configs network-hooks test templates
all: $(SUBDIRS)
set -e && for i in $(SUBDIRS); do $(MAKE) -C $$i; done
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 19f0baca..d0e7c544 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -238,12 +238,6 @@ __PACKAGE__->register_method({
return $pkglist;
}});
-my $updates_available_subject_template = "New software packages available
({{hostname}})";
-my $updates_available_body_template = <register_method({
name => 'update_database',
path => 'update',
@@ -358,8 +352,7 @@ __PACKAGE__->register_method({
};
PVE::Notify::info(
- $updates_available_subject_template,
- $updates_available_body_template,
+ "package-updates",
$template_data,
$metadata_fields,
);
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 0dc944c9..d84ac1ab 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -92,23 +92,6 @@ my sub _should_mail_at_failcount {
return $i * 48 == $fail_count;
};
-my $replication_error_subject_template = "Replication Job: '{{job-id}}'
failed";
-my $replication_error_body_template = < 1024*1024;
sub send_notification {
@@ -565,8 +551,7 @@ sub send_notification {
PVE::Notify::notify(
$severity,
- $subject_template,
- $body_template,
+ "vzdump",
$notification_props,
$fields,
$notification_config
@@ -577,8 +562,7 @@ sub send_notification {
# no email addresses were configured.
PVE::Notify::notify(
$severity,
- $subject_template,
- $body_template,
+ "vzdump",
$notification_props,
$fields,
);
diff --git a/templates/Makefile b/templates/Makefile
new file mode 100644
index ..236988c5
--- /dev/null
+++ b/templates/Makefile
@@ -0,0 +1,24 @@
+NOTIFICATION_TEMPLATES=\
+ default/test-subject.txt.hbs\
+ default/test-body.txt.hbs \
+ default/test-body.html.hbs \
+ default/vzdump-subject.txt.hbs \
+ default/vzdump-body.txt.hbs \
+ d
[pve-devel] [PATCH pve-ha-manager v2 17/20] env: notify: use named templates instead of passing template strings
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
debian/pve-ha-manager.install | 3 +++
src/Makefile | 1 +
src/PVE/HA/Env/PVE2.pm| 4 ++--
src/PVE/HA/NodeStatus.pm | 20 +--
src/PVE/HA/Sim/Env.pm | 3 ++-
src/templates/Makefile| 10 ++
src/templates/default/fencing-body.html.hbs | 14 +
src/templates/default/fencing-body.txt.hbs| 11 ++
src/templates/default/fencing-subject.txt.hbs | 1 +
9 files changed, 45 insertions(+), 22 deletions(-)
create mode 100644 src/templates/Makefile
create mode 100644 src/templates/default/fencing-body.html.hbs
create mode 100644 src/templates/default/fencing-body.txt.hbs
create mode 100644 src/templates/default/fencing-subject.txt.hbs
diff --git a/debian/pve-ha-manager.install b/debian/pve-ha-manager.install
index a7598a9..0ffbd8d 100644
--- a/debian/pve-ha-manager.install
+++ b/debian/pve-ha-manager.install
@@ -38,3 +38,6 @@
/usr/share/perl5/PVE/HA/Usage/Static.pm
/usr/share/perl5/PVE/Service/pve_ha_crm.pm
/usr/share/perl5/PVE/Service/pve_ha_lrm.pm
+/usr/share/pve-manager/templates/default/fencing-body.html.hbs
+/usr/share/pve-manager/templates/default/fencing-body.txt.hbs
+/usr/share/pve-manager/templates/default/fencing-subject.txt.hbs
diff --git a/src/Makefile b/src/Makefile
index 87bb0de..56bd360 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -73,6 +73,7 @@ install: watchdog-mux pve-ha-crm pve-ha-lrm ha-manager.1
pve-ha-crm.8 pve-ha-lrm
install -d $(DESTDIR)/$(MAN1DIR)
install -m 0644 ha-manager.1 $(DESTDIR)/$(MAN1DIR)
gzip -9 $(DESTDIR)/$(MAN1DIR)/ha-manager.1
+ $(MAKE) -C templates $@
.PHONY: test
test:
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index fcb60a9..cb73bcf 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -221,10 +221,10 @@ sub log {
}
sub send_notification {
-my ($self, $subject, $text, $template_data, $metadata_fields) = @_;
+my ($self, $template_name, $template_data, $metadata_fields) = @_;
eval {
- PVE::Notify::error($subject, $text, $template_data, $metadata_fields);
+ PVE::Notify::error($template_name, $template_data, $metadata_fields);
};
$self->log("warning", "could not notify: $@") if $@;
diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
index e053c55..9e6d898 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -188,23 +188,6 @@ sub update {
}
}
-my $body_template = <send_notification(
- $subject_template,
- $body_template,
+ "fencing",
$template_data,
$metadata_fields,
);
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index d3aea8d..0f77065 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -289,11 +289,12 @@ sub log {
}
sub send_notification {
-my ($self, $subject, $text, $properties) = @_;
+my ($self, $template_name, $properties) = @_;
# The template for the subject is "{{subject-prefix}}: {{subject}}"
# We have to perform poor-man's template rendering to pass the test cases.
+my $subject = "{{subject-prefix}}: {{subject}}";
$subject = $subject =~
s/\{\{subject-prefix}}/$properties->{"subject-prefix"}/r;
$subject = $subject =~ s/\{\{subject}}/$properties->{"subject"}/r;
diff --git a/src/templates/Makefile b/src/templates/Makefile
new file mode 100644
index 000..396759f
--- /dev/null
+++ b/src/templates/Makefile
@@ -0,0 +1,10 @@
+NOTIFICATION_TEMPLATES=
\
+ default/fencing-subject.txt.hbs \
+ default/fencing-body.txt.hbs\
+ default/fencing-body.html.hbs \
+
+.PHONY: install
+install:
+ install -dm 0755 $(DESTDIR)/usr/share/pve-manager/templates/default
+ $(foreach i,$(NOTIFICATION_TEMPLATES), \
+ install -m644 $(i) $(DESTDIR)/usr/share/pve-manager/templates/$(i)
;)
diff --git a/src/templates/default/fencing-body.html.hbs
b/src/templates/default/fencing-body.html.hbs
new file mode 100644
index 000..1420348
--- /dev/null
+++ b/src/templates/default/fencing-body.html.hbs
@@ -0,0 +1,14 @@
+
+
+The node '{{node}}' failed and needs manual intervention.
+
+The PVE HA manager tries to fence it and recover the configured HA
resources to
+a healthy node if possible.
+
+Current fence status: {{subject-prefix}}
+{{subject}}
+
+Overall Cluster status:
+{{object status-data}}
+
+
diff --git a/src/templates/default/fencing-body.txt.hbs
b/src/templates/default/fencing-body.txt.hbs
new file mode 100644
index 000..e46a1fd
--- /dev/null
+++ b/src/templates/d
[pve-devel] [PATCH manager v2 19/20] tests: remove vzdump_notification test
With the upcoming changes in how we send notifications, this one
really becomes pretty annoying to keep working. The location where
templates are looked up are defined in the proxmox_notify crate, so
there is no easy way to mock this for testing.
The test itself seemed not super valuable, mainly testing if
the backup logs are shortened if they ware too long - so they are just
removed.
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
test/Makefile| 6 +-
test/vzdump_notification_test.pl | 101 ---
2 files changed, 1 insertion(+), 106 deletions(-)
delete mode 100755 test/vzdump_notification_test.pl
diff --git a/test/Makefile b/test/Makefile
index 62d75050..743804c8 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,7 +5,7 @@ all:
export PERLLIB=..
.PHONY: check
-check: test-replication test-balloon test-vzdump-notification test-vzdump
test-osd
+check: test-replication test-balloon test-vzdump test-osd
.PHONY: test-balloon
test-balloon:
@@ -17,10 +17,6 @@ test-replication: replication1.t replication2.t
replication3.t replication4.t re
replication%.t: replication_test%.pl
./$<
-.PHONY: test-vzdump-notification
-test-vzdump-notification:
- ./vzdump_notification_test.pl
-
.PHONY: test-vzdump
test-vzdump: test-vzdump-guest-included test-vzdump-new
diff --git a/test/vzdump_notification_test.pl b/test/vzdump_notification_test.pl
deleted file mode 100755
index 631606bb..
--- a/test/vzdump_notification_test.pl
+++ /dev/null
@@ -1,101 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-
-use lib '..';
-
-use Test::More tests => 3;
-use Test::MockModule;
-
-use PVE::VZDump;
-
-my $STATUS = qr/.*status.*/;
-my $NO_LOGFILE = qr/.*Could not open log file.*/;
-my $LOG_TOO_LONG = qr/.*Log output was too long.*/;
-my $TEST_FILE_PATH = '/tmp/mail_test';
-my $TEST_FILE_WRONG_PATH = '/tmp/mail_test_wrong';
-
-sub prepare_mail_with_status {
-open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
-print TEST_FILE "start of log file\n";
-print TEST_FILE "status: 0\% this should not be in the mail\n";
-print TEST_FILE "status: 55\% this should not be in the mail\n";
-print TEST_FILE "status: 100\% this should not be in the mail\n";
-print TEST_FILE "end of log file\n";
-close(TEST_FILE);
-}
-
-sub prepare_long_mail {
-open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
-# 0.5 MB * 2 parts + the overview tables gives more than 1 MB mail
-print TEST_FILE "a" x (1024*1024);
-close(TEST_FILE);
-}
-
-my $result_text;
-my $result_properties;
-
-my $mock_notification_module = Test::MockModule->new('PVE::Notify');
-my $mocked_notify = sub {
-my ($severity, $title, $text, $properties, $metadata) = @_;
-
-$result_text = $text;
-$result_properties = $properties;
-};
-my $mocked_notify_short = sub {
-my (@params) = @_;
-return $mocked_notify->('', @params);
-};
-
-$mock_notification_module->mock(
-'notify' => $mocked_notify,
-'info' => $mocked_notify_short,
-'notice' => $mocked_notify_short,
-'warning' => $mocked_notify_short,
-'error' => $mocked_notify_short,
-);
-$mock_notification_module->mock('cfs_read_file', sub {
-my $path = shift;
-
-if ($path eq 'datacenter.cfg') {
-return {};
-} elsif ($path eq 'notifications.cfg' || $path eq
'priv/notifications.cfg') {
-return '';
-} else {
- die "unexpected cfs_read_file\n";
-}
-});
-
-my $MAILTO = ['test_addr...@proxmox.com'];
-my $SELF = {
-opts => { mailto => $MAILTO },
-cmdline => 'test_command_on_cli',
-};
-
-my $task = { state => 'ok', vmid => '100', };
-my $tasklist;
-sub prepare_test {
-$result_text = undef;
-$task->{tmplog} = shift;
-$tasklist = [ $task ];
-}
-
-{
-prepare_test($TEST_FILE_WRONG_PATH);
-PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
-like($result_properties->{logs}, $NO_LOGFILE, "Missing logfile is
detected");
-}
-{
-prepare_test($TEST_FILE_PATH);
-prepare_mail_with_status();
-PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
-unlike($result_properties->{"status-text"}, $STATUS, "Status are not in
text part of mails");
-}
-{
-prepare_test($TEST_FILE_PATH);
-prepare_long_mail();
-PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
-like($result_properties->{logs}, $LOG_TOO_LONG, "Text part of mails gets
shortened");
-}
-unlink $TEST_FILE_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 cluster v2 16/20] notify: use named template instead of passing template strings
The notification system will now load template files from a defined
location. The template to use is now passed to proxmox_notify, instead
of separate template strings for subject/body.
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
src/PVE/Notify.pm | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
index 872eb25..c514111 100644
--- a/src/PVE/Notify.pm
+++ b/src/PVE/Notify.pm
@@ -58,17 +58,16 @@ sub write_config {
}
my $send_notification = sub {
-my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+my ($severity, $template_name, $template_data, $fields, $config) = @_;
$config = read_config() if !defined($config);
-$config->send($severity, $title, $message, $template_data, $fields);
+$config->send($severity, $template_name, $template_data, $fields);
};
sub notify {
-my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+my ($severity, $template_name, $template_data, $fields, $config) = @_;
$send_notification->(
$severity,
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -76,11 +75,10 @@ sub notify {
}
sub info {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'info',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -88,11 +86,10 @@ sub info {
}
sub notice {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'notice',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -100,11 +97,10 @@ sub notice {
}
sub warning {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'warning',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
@@ -112,11 +108,10 @@ sub warning {
}
sub error {
-my ($title, $message, $template_data, $fields, $config) = @_;
+my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'error',
-$title,
-$message,
+$template_name,
$template_data,
$fields,
$config
--
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 v2 04/20] notify: don't make tests require pve-context
Tests now have their own context, so requiring pve-context is not
necessary any more.
Signed-off-by: Lukas Wagner
Tested-by: Folke Gleumes
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/api/gotify.rs | 2 +-
proxmox-notify/src/api/matcher.rs | 2 +-
proxmox-notify/src/api/sendmail.rs | 2 +-
proxmox-notify/src/api/smtp.rs | 24
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index 15d94cb..92151f5 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -165,7 +165,7 @@ fn remove_private_config_entry(config: Config, name:
) -> Result<(), Ht
Ok(())
}
-#[cfg(all(feature = "pve-context", test))]
+#[cfg(test)]
mod tests {
use super::*;
use crate::api::test_helpers::empty_config;
diff --git a/proxmox-notify/src/api/matcher.rs
b/proxmox-notify/src/api/matcher.rs
index 63ec73d..fa11633 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -148,7 +148,7 @@ pub fn delete_matcher(config: Config, name: ) ->
Result<(), HttpError>
Ok(())
}
-#[cfg(all(test, feature = "sendmail", feature = "pve-context"))]
+#[cfg(all(test, feature = "sendmail"))]
mod tests {
use super::*;
use crate::matcher::MatchModeOperator;
diff --git a/proxmox-notify/src/api/sendmail.rs
b/proxmox-notify/src/api/sendmail.rs
index c20a3e5..47588af 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -151,7 +151,7 @@ pub fn delete_endpoint(config: Config, name: ) ->
Result<(), HttpError>
Ok(())
}
-#[cfg(all(feature = "pve-context", test))]
+#[cfg(test)]
pub mod tests {
use super::*;
use crate::api::test_helpers::*;
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 7a58677..1b4700e 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -200,7 +200,7 @@ pub fn delete_endpoint(config: Config, name: ) ->
Result<(), HttpError>
Ok(())
}
-#[cfg(all(feature = "pve-context", test))]
+#[cfg(test)]
pub mod tests {
use super::*;
use crate::api::test_helpers::*;
@@ -348,15 +348,15 @@ pub mod tests {
Ok(())
}
-// #[test]
-// fn test_delete() -> Result<(), HttpError> {
-// let mut config = empty_config();
-// add_smtp_endpoint_for_test( config, "smtp-endpoint")?;
-//
-// delete_endpoint( config, "smtp-endpoint")?;
-// assert!(delete_endpoint( config, "smtp-endpoint").is_err());
-// assert_eq!(get_endpoints()?.len(), 0);
-//
-// Ok(())
-// }
+#[test]
+fn test_delete() -> Result<(), HttpError> {
+let mut config = empty_config();
+add_smtp_endpoint_for_test( config, "smtp-endpoint")?;
+
+delete_endpoint( config, "smtp-endpoint")?;
+assert!(delete_endpoint( config, "smtp-endpoint").is_err());
+assert_eq!(get_endpoints()?.len(), 0);
+
+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 manager v2 18/20] gitignore: ignore any test artifacts
Signed-off-by: Lukas Wagner Reviewed-by: Fiona Ebner --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index e8d1eb27..481ae1e0 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ dest/ /www/mobile/pvemanager-mobile.js /www/touch/touch-[0-9]*/ /pve-manager-[0-9]*/ +/test/.mocked_* +/test/*.tmp -- 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 v2 12/20] notify: renderer: add relative-percentage helper from PBS
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/src/renderer/mod.rs | 29 +
1 file changed, 29 insertions(+)
diff --git a/proxmox-notify/src/renderer/mod.rs
b/proxmox-notify/src/renderer/mod.rs
index a51ece6..ddb241d 100644
--- a/proxmox-notify/src/renderer/mod.rs
+++ b/proxmox-notify/src/renderer/mod.rs
@@ -73,6 +73,30 @@ fn value_to_timestamp(val: ) -> Option {
proxmox_time::strftime_local("%F %H:%M:%S", timestamp).ok()
}
+fn handlebars_relative_percentage_helper(
+h: ,
+_: ,
+_: ,
+_rc: RenderContext,
+out: dyn Output,
+) -> HelperResult {
+let param0 = h
+.param(0)
+.and_then(|v| v.value().as_f64())
+.ok_or_else(|| HandlebarsRenderError::new("relative-percentage: param0
not found"))?;
+let param1 = h
+.param(1)
+.and_then(|v| v.value().as_f64())
+.ok_or_else(|| HandlebarsRenderError::new("relative-percentage: param1
not found"))?;
+
+if param1 == 0.0 {
+out.write("-")?;
+} else {
+out.write(!("{:.2}%", (param0 * 100.0) / param1))?;
+}
+Ok(())
+}
+
/// Available render functions for `serde_json::Values``
///
/// May be used as a handlebars helper, e.g.
@@ -237,6 +261,11 @@ fn render_template_impl(
ValueRenderFunction::register_helpers( handlebars);
+handlebars.register_helper(
+"relative-percentage",
+Box::new(handlebars_relative_percentage_helper),
+);
+
let rendered_template = handlebars
.render_template(template, data)
.map_err(|err| Error::RenderError(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 v2 06/20] notify: cargo.toml: add spaces before curly braces
Signed-off-by: Lukas Wagner
Reviewed-by: Fiona Ebner
---
proxmox-notify/Cargo.toml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 3e8d253..185b50a 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -17,13 +17,13 @@ log.workspace = true
mail-parser = { workspace = true, optional = true }
openssl.workspace = true
regex.workspace = true
-serde = { workspace = true, features = ["derive"]}
+serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
proxmox-http = { workspace = true, features = ["client-sync"], optional = true
}
proxmox-http-error.workspace = true
proxmox-human-byte.workspace = true
-proxmox-schema = { workspace = true, features = ["api-macro", "api-types"]}
+proxmox-schema = { workspace = true, features = ["api-macro", "api-types"] }
proxmox-section-config = { workspace = true }
proxmox-serde.workspace = true
proxmox-sys = { workspace = true, optional = true }
--
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 v2 05/20] notify: make the `mail-forwarder` feature depend on proxmox-sys
It uses proxmox_sys::nodename - the dep is needed, otherwise the code does not compile in some feature flag permutations. Signed-off-by: Lukas Wagner Tested-by: Folke Gleumes Reviewed-by: Fiona Ebner --- proxmox-notify/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml index 52a466e..3e8d253 100644 --- a/proxmox-notify/Cargo.toml +++ b/proxmox-notify/Cargo.toml @@ -31,7 +31,7 @@ proxmox-time.workspace = true [features] default = ["sendmail", "gotify", "smtp"] -mail-forwarder = ["dep:mail-parser"] +mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys"] sendmail = ["dep:proxmox-sys"] gotify = ["dep:proxmox-http"] pve-context = ["dep:proxmox-sys"] -- 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 v2 00/20] notifications: move template strings to template files; PBS preparations
The notification system uses handlebar templates to render the subject
and the body of notifications. Previously, the template strings were
defined inline at the call site. This patch series extracts the templates
into template files and installs them at
/usr/share/pve-manager/templates/default
where they stored as -{body,subject}.{txt,html}.hbs
The 'default' part in the path is a preparation for translated
notifications and/or overridable notification templates.
Future work could provide notifications translated to e.g. German
in `templates/de` or similar. This will be a first for having
translated strings on the backend-side, so there is need for further
research.
The patches for `proxmox` also include some preparatory patches for
the upcoming integration of the notification system into PBS. They
are not needed for PVE, but I included them here since Folke and I
tested the PVE changes with them applied. IMO they should just be
applied with the same version bump.
The list of optional, preparatory patches is:
notify: give each notification a unique ID
notify: api: add get_targets
notify: derive `api` for Deleteable*Property
notify: derive Deserialize/Serialize for Notification struct
notify: pbs context: include nodename in default sendmail author
notify: renderer: add relative-percentage helper from PBS
Folke kindly did some off-list testing before this was posted, hence
his T-bs were included.
Bumps/dependencies:
- proxmox_notify
- libproxmox-rs-perl/libpve-rs-perl (needs bumped proxmox_notify)
- libpve-notify-perl (needs bumped libproxmox-rs-perl/libpve-rs-perl)
- pve-ha-manager (needs bumped libpve-notify-perl)
- pve-manager (needs bumped libpve-notify-perl)
- proxmox-mail-forward (optional. should not be affected by any changes,
but I think
it should be also be bumped for any larger proxmox_notify changes to
avoid any
weird hidden regressions due to mismatches of proxmox_notify
Changes since v1:
- Incorporated feedback from @Fiona and @Fabian - thanks!
- most noteworthy: Change template path from /usr/share/proxmox-ve
to /usr/share/
- apart from that mostly just cosmetics/style
proxmox:
Lukas Wagner (12):
notify: switch to file-based templating system
notify: make api methods take config struct ownership
notify: convert Option> -> Vec in config structs
notify: don't make tests require pve-context
notify: make the `mail-forwarder` feature depend on proxmox-sys
notify: cargo.toml: add spaces before curly braces
notify: give each notification a unique ID
notify: api: add get_targets
notify: derive `api` for Deleteable*Property
notify: derive Deserialize/Serialize for Notification struct
notify: pbs context: include nodename in default sendmail author
notify: renderer: add relative-percentage helper from PBS
proxmox-notify/Cargo.toml | 7 +-
proxmox-notify/examples/render.rs | 63 --
proxmox-notify/src/api/gotify.rs| 48 ++---
proxmox-notify/src/api/matcher.rs | 59 +++--
proxmox-notify/src/api/mod.rs | 113 --
proxmox-notify/src/api/sendmail.rs | 60 +++---
proxmox-notify/src/api/smtp.rs | 122 +--
proxmox-notify/src/context/mod.rs | 10 +-
proxmox-notify/src/context/pbs.rs | 18 +-
proxmox-notify/src/context/pve.rs | 15 ++
proxmox-notify/src/context/test.rs | 9 +
proxmox-notify/src/endpoints/common/mail.rs | 20 +-
proxmox-notify/src/endpoints/gotify.rs | 12 +-
proxmox-notify/src/endpoints/sendmail.rs| 34 +--
proxmox-notify/src/endpoints/smtp.rs| 38 ++--
proxmox-notify/src/lib.rs | 59 ++---
proxmox-notify/src/matcher.rs | 71 +++---
proxmox-notify/src/renderer/html.rs | 14 --
proxmox-notify/src/renderer/mod.rs | 226
proxmox-notify/src/renderer/plaintext.rs| 39
20 files changed, 506 insertions(+), 531 deletions(-)
delete mode 100644 proxmox-notify/examples/render.rs
proxmox-perl-rs:
Lukas Wagner (3):
notify: use file based notification templates
notify: don't pass config structs by reference
notify: adapt to Option> to Vec changes in proxmox_notify
common/src/notify.rs | 48 +---
1 file changed, 23 insertions(+), 25 deletions(-)
pve-cluster:
Lukas Wagner (1):
notify: use named template instead of passing template strings
src/PVE/Notify.pm | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
pve-ha-manager:
Lukas Wagner (1):
env: notify: use named templates instead of passing template strings
debian/pve-ha-manager.install | 3 +++
src/Makefile | 1 +
src/PVE/HA/Env/PVE2.pm| 4 ++--
src/PVE/HA/NodeStatus.pm
Re: [pve-devel] [PATCH manager v5 08/16] api: notification: add API for getting known metadata fields/values
On 2024-04-19 15:45, Fiona Ebner wrote:
> Am 15.04.24 um 10:26 schrieb Lukas Wagner:
>> +
>> +__PACKAGE__->register_method ({
>> +name => 'get_field_values',
>> +path => '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',
>> +optional => 1,
>> +},
>> +'internal' => {
>> +description => 'Set if "value" was generated by the system
>> and can'
>> + . ' safely be used as base for translations.',
>> +type => 'boolean',
>> +optional => 1,
>> +},
>
> And wouldn't it be nicer to return already grouped by field? Then maybe
> we also don't really need the dedicated fields API endpoint either as
> those are just the top-level of the result (with empty array when there
> are no values so we don't ever miss any fields).
>
The design of both endpoints was mostly driven by the intention
of keeping the ExtJS side as simple as possible.
Two comboboxes, each with their own api endpoint to fetch data from,
one setting a filter for the other one.
I tried using a single endpoint at first, but was quickly frustrated
by ExtJS and its documentation and settled for this approach as a consequence.
So I'd prefer to leave it as is :D
Regarding the 'internal' flag: Yes, you are right, right now we only need it
for 'type'.
I'll leave it out then and handle everything in the UI.
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v5 05/16] api: replication: add 'replication-job' to notification metadata
On 2024-04-19 15:11, Fiona Ebner wrote:
> Am 19.04.24 um 14:22 schrieb Lukas Wagner:
>>
>>
>> On 2024-04-19 14:02, Fiona Ebner wrote:
>>> Am 15.04.24 um 10:26 schrieb Lukas Wagner:
>>>> This allows users to create notification match rules for specific
>>>> replication jobs, if they so desire.
>>>>
>>>> Signed-off-by: Lukas Wagner
>>>> ---
>>>> PVE/API2/Replication.pm | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
>>>> index 0dc944c9..703640f5 100644
>>>> --- a/PVE/API2/Replication.pm
>>>> +++ b/PVE/API2/Replication.pm
>>>> @@ -140,8 +140,8 @@ my sub _handle_job_err {
>>>> };
>>>>
>>>> my $metadata_fields = {
>>>> - # TODO: Add job-id?
>>>>type => "replication",
>>>> + "replication-job" => $job->{id},
>>>> };
>>>>
>>>> eval {
>>>
>>> Not sure if we should use "replication-job" and "backup-job" for the
>>> metadata entries rather then just "job-id". The type is already
>>> something that can be matched, why re-do it implicitly with the field
>>> name? E.g. I want to see all jobs with -fiona- on the system, now I'd
>>> have to create a matcher rule for each job type.
>>
>> Had a 'job-id' field at first, but I *think* (can't be too sure after more
>> than
>> 4 months of not working on this) the reason why I changed it to this approach
>> were the replication job IDs, which look like '100-0' or similar.
>> Giving them and other job IDs a unique field made it a bit easier to
>> understand what is what when creating a matcher in the improved UI.
>>
>> For instance, if you just have 'job-id', the pre-filled combo box in the
>> match-field edit UI might contain these values
>>
>> - backup-gaasdgh7asdfg
>> - 100-0
>> - 101-0
>>
>> IMO it's a bit hard to understand that the last two are replication jobs.
>> The separate
>> job fields make this easier.
>
> We know that it either starts with "backup-" (or "realmsync-", should
> those get notifications), or is a replication job. So we could also just
> display something that indicates they are replication jobs (e.g.
> "replication-XYZ" or "XYZ (replication)"), until we turn replication
> jobs into proper jobs in the backend. Otherwise, each job type we add
> will just have a new matcher field for its ID.
Ok, fine with me. :)
--
- 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 07/19] notify: api: add get_targets
On 2024-04-19 10:34, Fiona Ebner wrote:
> Am 09.04.24 um 15:25 schrieb Lukas Wagner:
>> +/// Get a list of all notification targets.
>> +pub fn get_targets(config: ) -> Result, HttpError> {
>> +let mut targets = Vec::new();
>> +
>> +#[cfg(feature = "gotify")]
>> +for endpoint in gotify::get_endpoints(config)? {
>> +targets.push(Target {
>> +name: endpoint.name,
>> +origin: endpoint.origin.unwrap_or(Origin::UserCreated),
>> +endpoint_type: EndpointType::Gotify,
>> +disable: endpoint.disable,
>> +comment: endpoint.comment,
>> +})
>
> Would it make sense to have into() functions for
> {Gotify,Sendmail,Smtp}Config -> Target ?
That would indeed be a bit nicer - but I'll do that in a follow-up, since this
is
completely internal to proxmox-notify and is more 'style' than an actual issue
:)
Thanks!
--
- 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 09/19] notify: derive Deserialize/Serialize for Notification struct
On 2024-04-19 10:45, Fiona Ebner wrote:
> Nit: I always like a quick sentence for who needs it for such changes.
>
> Am 09.04.24 um 15:25 schrieb Lukas Wagner:
>> Signed-off-by: Lukas Wagner
>> ---
>> proxmox-notify/src/lib.rs | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
>> index 91c0b61..8d4dc63 100644
>> --- a/proxmox-notify/src/lib.rs
>> +++ b/proxmox-notify/src/lib.rs
>> @@ -159,11 +159,13 @@ pub trait Endpoint {
>> fn disabled() -> bool;
>> }
>>
>> -#[derive(Debug, Clone)]
>> +#[derive(Debug, Clone, Serialize, Deserialize)]
>> +#[serde(rename_all = "kebab-case")]
>> pub enum Content {
>> /// Title and body will be rendered as a template
>> Template {
>> /// Name of the used template
>> +#[serde(rename = "template-name")]
>
> So I guess this is here, because the rename_all above is not recursive?
> Should we put rename_all on top of Template and ForwardedMail (if that
> even works), so we are sure not to forget it for potential future fields?
>
Yup, rename_all is not recursive. Added a rename_all for Template and
ForwardedMail,
this makes more sense.
Thanks!
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v5 04/16] ui: dc: backup: allow to set custom job id in advanced settings
On 2024-04-19 12:31, Fiona Ebner wrote:
> Am 15.04.24 um 10:26 schrieb Lukas Wagner:
>> This might be useful if somebody wants to match on the new
>> 'backup-job' field in a notification match rule.
>>
>> Signed-off-by: Lukas Wagner
>> ---
>> www/manager6/Utils.js | 4
>> www/manager6/dc/Backup.js | 11 +++
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
>> index 287d651a..d4b5f3e6 100644
>> --- a/www/manager6/Utils.js
>> +++ b/www/manager6/Utils.js
>> @@ -1952,6 +1952,10 @@ Ext.define('PVE.Utils', {
>> singleton: true,
>> constructor: function() {
>> var me = this;
>> +
>> +// Same regex as 'pve-configid
>> +me.CONFIGID_RE = /^[A-Za-z][A-Za-z0-9_-]+$/;
>
> This already exists (with nice verification errors), by using
> vtype: 'ConfigId'
> for the field. It's defined in widget-toolkit, Toolkit.js
>
Fixed, thanks!
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v5 05/16] api: replication: add 'replication-job' to notification metadata
On 2024-04-19 14:02, Fiona Ebner wrote:
> Am 15.04.24 um 10:26 schrieb Lukas Wagner:
>> This allows users to create notification match rules for specific
>> replication jobs, if they so desire.
>>
>> Signed-off-by: Lukas Wagner
>> ---
>> PVE/API2/Replication.pm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
>> index 0dc944c9..703640f5 100644
>> --- a/PVE/API2/Replication.pm
>> +++ b/PVE/API2/Replication.pm
>> @@ -140,8 +140,8 @@ my sub _handle_job_err {
>> };
>>
>> my $metadata_fields = {
>> -# TODO: Add job-id?
>> type => "replication",
>> +"replication-job" => $job->{id},
>> };
>>
>> eval {
>
> Not sure if we should use "replication-job" and "backup-job" for the
> metadata entries rather then just "job-id". The type is already
> something that can be matched, why re-do it implicitly with the field
> name? E.g. I want to see all jobs with -fiona- on the system, now I'd
> have to create a matcher rule for each job type.
Had a 'job-id' field at first, but I *think* (can't be too sure after more than
4 months of not working on this) the reason why I changed it to this approach
were the replication job IDs, which look like '100-0' or similar.
Giving them and other job IDs a unique field made it a bit easier to
understand what is what when creating a matcher in the improved UI.
For instance, if you just have 'job-id', the pre-filled combo box in the
match-field edit UI might contain these values
- backup-gaasdgh7asdfg
- 100-0
- 101-0
IMO it's a bit hard to understand that the last two are replication jobs. The
separate
job fields make this easier.
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH many 00/19] notifications: move template strings to template files; PBS preparations
On 2024-04-19 13:22, Fabian Grünbichler wrote: > On April 19, 2024 12:09 pm, Fiona Ebner wrote: >> Am 09.04.24 um 15:25 schrieb Lukas Wagner: >>> Bumps/dependencies: >>> - proxmox_notify >>> - libproxmox-rs-perl/libpve-rs-perl (needs bumped proxmox_notify) >>> - libpve-notify-perl (needs bumped >>> libproxmox-rs-perl/libpve-rs-perl) >>> - pve-ha-manager (needs bumped libpve-notify-perl) >>> - pve-manager (needs bumped libpve-notify-perl) >>> - proxmox-mail-forward (optional. should not be affected by any >>> changes, but I think >>> it should be also be bumped for any larger proxmox_notify changes >>> to avoid any >>> weird hidden regressions due to mismatches of proxmox_notify >>> >> >> Versioned breaks required: >> - new pve-cluster will break old pve-manager and old pve-ha-manager >> - new libproxmox-rs-perl/libpve-rs-perl will break old pve-cluster >> >> Still not too sure about putting the templates in >> /usr/share/proxmox-ve/, but maybe @Thomas or @Fabian can chime in on that. > > without in-depth look at all the changes in this series, I think it > would make most sense for the package shipping (most?) templates to > "own" the dir where they are shipped. that seems to be pve-manager, so > maybe /usr/share/pve-manager would be an okay fit? > Okay, I think I'll just use pve-manager then. Thanks! -- - Lukas ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 19/19] notifications: use named templates instead of in-code templates
On 2024-04-19 11:59, Fiona Ebner wrote:
> Am 09.04.24 um 15:25 schrieb Lukas Wagner:
>> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
>> index 152eb3e5..2ea626f0 100644
>> --- a/PVE/VZDump.pm
>> +++ b/PVE/VZDump.pm
>
> The existing $subject_template and $body_template could be removed now
> like for the others
>
>> diff --git a/templates/Makefile b/templates/Makefile
>> new file mode 100644
>> index ..b0add81e
>> --- /dev/null
>> +++ b/templates/Makefile
>> @@ -0,0 +1,24 @@
>> +NOTIFICATION_TEMPLATES=
>> \
>> +default/test-subject.txt.hbs\
>> +default/test-body.txt.hbs \
>> +default/test-body.html.hbs \
>> +default/vzdump-subject.txt.hbs \
>> +default/vzdump-body.txt.hbs \
>> +default/vzdump-body.html.hbs\
>> +default/replication-subject.txt.hbs \
>> +default/replication-body.txt.hbs\
>> +default/replication-body.html.hbs \
>> +default/package-updates-subject.txt.hbs \
>> +default/package-updates-body.txt.hbs\
>> +default/package-updates-body.html.hbs \
>> +
>
> Nit: backslashes are not aligned
>
>> diff --git a/templates/default/replication-body.html.hbs
>> b/templates/default/replication-body.html.hbs
>> new file mode 100644
>> index ..d1dea6a1
>> --- /dev/null
>> +++ b/templates/default/replication-body.html.hbs
>> @@ -0,0 +1,18 @@
>> +
>> +
>> +Replication job '{{job-id}}' with target '{{job-target}}' and
>> schedule '{{job-schedule}}' failed!
>> +
>> +Last successful sync: {{timestamp last-sync}}
>> +Next sync try: {{timestamp next-sync}}
>> +Failure count: {{failure-count}}
>> +
>> +{{#if (eq failure-count 3)}}
>> +Note: The system will now reduce the frequency of error
>> reports, as the job appears to be stuck.
>> +{{/if}}
>> +
>> +Error:
>> +
>> +{{error}}
>
> Nit: is there a special reason for not indenting this?
Yes, since it's in a block, the first line of the logs would be displayed
indented otherwise.
--
- 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 01/19] notify: switch to file-based templating system
On 2024-04-19 10:57, Fiona Ebner wrote:
> Am 19.04.24 um 10:45 schrieb Lukas Wagner:
>>> Who adds the template files? I don't see a patch for proxmox-ve in this
>>> series. Does this series require some versioned breaks to some package?
>>
>> The pve-manager and pve-ha-manager (for fencing notifications) patches add
>> the templates.
>> I can't use `/usr/share/pve-manager` and `/usr/share/pve-ha-manager` because
>> proxmox_notify needs to have a single base directory from where to load
>> template files.
>> Maybe we should use some other base dir to avoid confusion with the
>> `proxmox-ve` metapackage?
>>>
> Ah, I see. Yes, maybe a directory named based on libpve-notify-perl
> would be better or proxmox-notify (but would need to be a bit careful
> with co-installed PBS and PVE to not create accidental conflicts).
mhmm. In PBS we install the templates in /usr/share/proxmox-backup/... (there
it makes sense,
because the templates are part of the .deb with the same name).
Using `proxmox-notify` would IMO be not so good. We might have similar
notification templates in
both products (e.g. package-updates) which are not necessarily 100% compatible.
Of course we could unify them, but I kinda prefer the flexibility of
having these separate.
A name based on libpve-notify implies that the templates belong to
PVE, which is good. However, that only shifts the problem:
pve-{ha-,}manager still install templates to a location which appears
to be owned by another package, this time libpve-notify...
Thinking about the options, and also about 'user experience' when
adding overridable templates, I think we could just keep on
using 'proxmox-ve' here.
>
>> In terms of versions:
>> pve-{ha}-manager needs to pull in a bumped libpve-notify-perl
>> libpve-notify-perl needs to pull in bumped libpve-rs-perl/libproxmox-rs-perl
>> libpve-rs-perl needs to pull in bumped librust-proxmox-notify
>>
>> I really wish the dep-chain was a bit easier, yet here we are.
>>
>
> But there also is a need for versioned breaks, right? Because installing
> new libpve-notify-perl (using new proxmox-perl-rs using new
> proxmox-notify) without also upgrading pve-manager and pve-ha-manager
> will be broken or am I missing something?
Ah yes, sorry, my packaging knowledge is not yet the best.
A versioned break will be necessary due to the API changes (passing a template
name
instead of a title/body) and due to the fact that the template
files must be present.
--
- 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 01/19] notify: switch to file-based templating system
On 2024-04-19 10:14, Fiona Ebner wrote:
> Am 09.04.24 um 15:25 schrieb Lukas Wagner:
>> Instead of passing the template strings for subject and body when
>> constructing a notification, we pass only the name of a template.
>> When rendering the template, the name of the template is used to find
>> corresponding template files. For PVE, they are located at
>> /usr/share/proxmox-ve/templates/default. The `default` part is
>> the 'template namespace', which is a preparation for user-customizable
>> and/or translatable notifications.
>>
>
> Is the plan to create different namespaces there ourselves or tell users
> they can put their custom templates there? In the latter case, I'm not
> sure /usr/share is the best place, rather than some place under /etc/
The idea would be to implement translations as other namespaces, e.g. `de` or
`fr`
instead of `default`.
For user-overridable templates we would extend the implementation to search for
a template
in another location first and then fall back to the templates provided by us in
'/usr/share/...`
I have not made up my mind yet on where these user-provided templates would be
located, either
in /usr/local/share/ or somewhere in /etc (/etc/pve would ensure that we
have the same templates
on all nodes, but I'm not sure if it is a good idea to put custom, user-created
files in there...)
Combining both ideas: assuming that we want to render a fencing notification
translated to German, assuming
that the user-override is in /usr/local/share:
- First try to load /usr/local/share/proxmox-ve/templates/de/fencing.txt.hbs
- If not found, try loading the shipped template at
/usr/share/proxmox-ve/templates/de/...
- If that one also does not exist, fall back to `default` namespace
- first user-location
- finally shipped template
>
> Who adds the template files? I don't see a patch for proxmox-ve in this
> series. Does this series require some versioned breaks to some package?
The pve-manager and pve-ha-manager (for fencing notifications) patches add the
templates.
I can't use `/usr/share/pve-manager` and `/usr/share/pve-ha-manager` because
proxmox_notify needs to have a single base directory from where to load
template files.
Maybe we should use some other base dir to avoid confusion with the
`proxmox-ve` metapackage?
In terms of versions:
pve-{ha}-manager needs to pull in a bumped libpve-notify-perl
libpve-notify-perl needs to pull in bumped libpve-rs-perl/libproxmox-rs-perl
libpve-rs-perl needs to pull in bumped librust-proxmox-notify
I really wish the dep-chain was a bit easier, yet here we are.
--
- Lukas
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
