* [pve-devel] [PATCH proxmox 01/27] notify: introduce Error::Generic
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 02/27] notify: factor out notification content into its own type Lukas Wagner
` (26 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
... as leaf error-type for anything for which we do not necessarily
want a separate enum variant.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/src/lib.rs | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 7500778..f7d480c 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -25,13 +25,22 @@ mod config;
#[derive(Debug)]
pub enum Error {
+ /// There was an error serializing the config
ConfigSerialization(Box<dyn StdError + Send + Sync>),
+ /// There was an error deserializing the config
ConfigDeserialization(Box<dyn StdError + Send + Sync>),
+ /// An endpoint failed to send a notification
NotifyFailed(String, Box<dyn StdError + Send + Sync>),
+ /// A target does not exist
TargetDoesNotExist(String),
+ /// Testing one or more notification targets failed
TargetTestFailed(Vec<Box<dyn StdError + Send + Sync>>),
+ /// A filter could not be applied
FilterFailed(String),
+ /// The notification's template string could not be rendered
RenderError(Box<dyn StdError + Send + Sync>),
+ /// Generic error for anything else
+ Generic(String),
}
impl Display for Error {
@@ -60,6 +69,7 @@ impl Display for Error {
write!(f, "could not apply filter: {message}")
}
Error::RenderError(err) => write!(f, "could not render notification template: {err}"),
+ Error::Generic(message) => f.write_str(message),
}
}
}
@@ -74,6 +84,7 @@ impl StdError for Error {
Error::TargetTestFailed(errs) => Some(&*errs[0]),
Error::FilterFailed(_) => None,
Error::RenderError(err) => Some(&**err),
+ Error::Generic(_) => None,
}
}
}
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox 02/27] notify: factor out notification content into its own type
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 01/27] notify: introduce Error::Generic Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 03/27] notify: replace filters and groups with matcher-based system Lukas Wagner
` (25 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This will be useful later for system mail forwarding, where
the content of the mail should be forwarded unchanged.
This moves notification properties into this new type and calls them
'data'. They will exclusively used for template rendering.
`Notification` will receive a separate field for metadata, which
will be useful for notification filtering. This decouples
template rendering and filtering, which enables us to be very precise
about which metadata fields we allow to be used in filters.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/examples/render.rs | 4 +-
proxmox-notify/src/endpoints/gotify.rs | 26 +++++---
proxmox-notify/src/endpoints/sendmail.rs | 62 +++++++++---------
proxmox-notify/src/filter.rs | 10 +--
proxmox-notify/src/lib.rs | 81 ++++++++++++++----------
proxmox-notify/src/renderer/mod.rs | 15 ++---
6 files changed, 109 insertions(+), 89 deletions(-)
diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs
index c0a6f27..d705fd0 100644
--- a/proxmox-notify/examples/render.rs
+++ b/proxmox-notify/examples/render.rs
@@ -53,10 +53,10 @@ fn main() -> Result<(), Error> {
}
});
- let output = render_template(TemplateRenderer::Html, TEMPLATE, Some(&properties))?;
+ let output = render_template(TemplateRenderer::Html, TEMPLATE, &properties)?;
println!("{output}");
- let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, Some(&properties))?;
+ let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, &properties)?;
println!("{output}");
Ok(())
diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs
index 83df41f..af86f9c 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater};
use crate::context::context;
use crate::renderer::TemplateRenderer;
use crate::schema::ENTITY_NAME_SCHEMA;
-use crate::{renderer, Endpoint, Error, Notification, Severity};
+use crate::{renderer, Content, Endpoint, Error, Notification, Severity};
fn severity_to_priority(level: Severity) -> u32 {
match level {
@@ -85,15 +85,21 @@ pub enum DeleteableGotifyProperty {
impl Endpoint for GotifyEndpoint {
fn send(&self, notification: &Notification) -> Result<(), Error> {
- let properties = notification.properties.as_ref();
-
- let title = renderer::render_template(
- TemplateRenderer::Plaintext,
- ¬ification.title,
- properties,
- )?;
- let message =
- renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?;
+
+ let (title, message) = match ¬ification.content {
+ Content::Template {
+ title_template,
+ body_template,
+ data
+ } => {
+ let rendered_title =
+ renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+ let rendered_message =
+ renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+
+ (rendered_title, rendered_message)
+ }
+ };
// We don't have a TemplateRenderer::Markdown yet, so simply put everything
// in code tags. Otherwise tables etc. are not formatted properly
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index 26e2a17..c540925 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -8,7 +8,7 @@ use proxmox_schema::{api, Updater};
use crate::context::context;
use crate::renderer::TemplateRenderer;
use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA};
-use crate::{renderer, Endpoint, Error, Notification};
+use crate::{renderer, Content, Endpoint, Error, Notification};
pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail";
@@ -102,41 +102,43 @@ impl Endpoint for SendmailEndpoint {
}
}
- let properties = notification.properties.as_ref();
-
- let subject = renderer::render_template(
- TemplateRenderer::Plaintext,
- ¬ification.title,
- properties,
- )?;
- let html_part =
- renderer::render_template(TemplateRenderer::Html, ¬ification.body, properties)?;
- let text_part =
- renderer::render_template(TemplateRenderer::Plaintext, ¬ification.body, properties)?;
-
- let author = self
- .config
- .author
- .clone()
- .unwrap_or_else(|| context().default_sendmail_author());
-
+ let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
let mailfrom = self
.config
.from_address
.clone()
.unwrap_or_else(|| context().default_sendmail_from());
- let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
-
- proxmox_sys::email::sendmail(
- &recipients_str,
- &subject,
- Some(&text_part),
- Some(&html_part),
- Some(&mailfrom),
- Some(&author),
- )
- .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+ match ¬ification.content {
+ Content::Template {
+ title_template,
+ body_template,
+ data,
+ } => {
+ let subject =
+ renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+ let html_part =
+ renderer::render_template(TemplateRenderer::Html, body_template, data)?;
+ let text_part =
+ renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+
+ let author = self
+ .config
+ .author
+ .clone()
+ .unwrap_or_else(|| context().default_sendmail_author());
+
+ proxmox_sys::email::sendmail(
+ &recipients_str,
+ &subject,
+ Some(&text_part),
+ Some(&html_part),
+ Some(&mailfrom),
+ Some(&author),
+ )
+ .map_err(|err| Error::NotifyFailed(self.config.name.clone(), err.into()))
+ }
+ }
}
fn name(&self) -> &str {
diff --git a/proxmox-notify/src/filter.rs b/proxmox-notify/src/filter.rs
index 748ec4e..e014a59 100644
--- a/proxmox-notify/src/filter.rs
+++ b/proxmox-notify/src/filter.rs
@@ -160,7 +160,7 @@ impl<'a> FilterMatcher<'a> {
#[cfg(test)]
mod tests {
use super::*;
- use crate::config;
+ use crate::{config, Content};
fn parse_filters(config: &str) -> Result<Vec<FilterConfig>, Error> {
let (config, _) = config::config(config)?;
@@ -169,10 +169,12 @@ mod tests {
fn empty_notification_with_severity(severity: Severity) -> Notification {
Notification {
- title: String::new(),
- body: String::new(),
+ content: Content::Template {
+ title_template: String::new(),
+ body_template: String::new(),
+ data: Default::default(),
+ },
severity,
- properties: Default::default(),
}
}
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index f7d480c..d40d017 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -116,17 +116,44 @@ pub trait Endpoint {
fn filter(&self) -> Option<&str>;
}
+#[derive(Debug, Clone)]
+pub enum Content {
+ /// Title and body will be rendered as a template
+ Template {
+ /// Template for the notification title.
+ title_template: String,
+ /// Template for the notification body.
+ body_template: String,
+ /// Data that can be used for template rendering.
+ data: Value,
+ },
+}
+
#[derive(Debug, Clone)]
/// Notification which can be sent
pub struct Notification {
/// Notification severity
- pub severity: Severity,
- /// The title of the notification
- pub title: String,
- /// Notification text
- pub body: String,
- /// Additional metadata for the notification
- pub properties: Option<Value>,
+ severity: Severity,
+ /// Notification content
+ content: Content,
+}
+
+impl Notification {
+ pub fn new_templated<S: AsRef<str>>(
+ severity: Severity,
+ title: S,
+ body: S,
+ properties: Value,
+ ) -> Self {
+ Self {
+ severity,
+ content: Content::Template {
+ title_template: title.as_ref().to_string(),
+ body_template: body.as_ref().to_string(),
+ data: properties,
+ },
+ }
+ }
}
/// Notification configuration
@@ -384,9 +411,11 @@ impl Bus {
pub fn test_target(&self, target: &str) -> Result<(), Error> {
let notification = Notification {
severity: Severity::Info,
- title: "Test notification".into(),
- body: "This is a test of the notification target '{{ target }}'".into(),
- properties: Some(json!({ "target": target })),
+ content: Content::Template {
+ title_template: "Test notification".into(),
+ body_template: "This is a test of the notification target '{{ target }}'".into(),
+ data: json!({ "target": target }),
+ },
};
let mut errors: Vec<Box<dyn StdError + Send + Sync>> = Vec::new();
@@ -473,12 +502,7 @@ mod tests {
// Send directly to endpoint
bus.send(
"endpoint",
- &Notification {
- title: "Title".into(),
- body: "Body".into(),
- severity: Severity::Info,
- properties: Default::default(),
- },
+ &Notification::new_templated(Severity::Info, "Title", "Body", Default::default()),
);
let messages = mock.messages();
assert_eq!(messages.len(), 1);
@@ -511,15 +535,9 @@ mod tests {
bus.add_endpoint(Box::new(endpoint2.clone()));
let send_to_group = |channel| {
- bus.send(
- channel,
- &Notification {
- title: "Title".into(),
- body: "Body".into(),
- severity: Severity::Info,
- properties: Default::default(),
- },
- )
+ let notification =
+ Notification::new_templated(Severity::Info, "Title", "Body", Default::default());
+ bus.send(channel, ¬ification)
};
send_to_group("group1");
@@ -579,15 +597,10 @@ mod tests {
});
let send_with_severity = |severity| {
- bus.send(
- "channel1",
- &Notification {
- title: "Title".into(),
- body: "Body".into(),
- severity,
- properties: Default::default(),
- },
- );
+ let notification =
+ Notification::new_templated(severity, "Title", "Body", Default::default());
+
+ bus.send("channel1", ¬ification);
};
send_with_severity(Severity::Info);
diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs
index 24f14aa..e9f36e6 100644
--- a/proxmox-notify/src/renderer/mod.rs
+++ b/proxmox-notify/src/renderer/mod.rs
@@ -228,11 +228,9 @@ impl BlockRenderFunctions {
fn render_template_impl(
template: &str,
- properties: Option<&Value>,
+ data: &Value,
renderer: TemplateRenderer,
) -> Result<String, Error> {
- let properties = properties.unwrap_or(&Value::Null);
-
let mut handlebars = Handlebars::new();
handlebars.register_escape_fn(renderer.escape_fn());
@@ -242,7 +240,7 @@ fn render_template_impl(
ValueRenderFunction::register_helpers(&mut handlebars);
let rendered_template = handlebars
- .render_template(template, properties)
+ .render_template(template, data)
.map_err(|err| Error::RenderError(err.into()))?;
Ok(rendered_template)
@@ -255,11 +253,11 @@ fn render_template_impl(
pub fn render_template(
renderer: TemplateRenderer,
template: &str,
- properties: Option<&Value>,
+ data: &Value,
) -> Result<String, Error> {
let mut rendered_template = String::from(renderer.prefix());
- rendered_template.push_str(&render_template_impl(template, properties, renderer)?);
+ rendered_template.push_str(&render_template_impl(template, data, renderer)?);
rendered_template.push_str(renderer.postfix());
Ok(rendered_template)
@@ -314,7 +312,7 @@ mod tests {
#[test]
fn test_render_template() -> Result<(), Error> {
- let properties = json!({
+ let data = json!({
"dur": 12345,
"size": 1024 * 15,
@@ -370,8 +368,7 @@ val1 val2
val3 val4
"#;
- let rendered_plaintext =
- render_template(TemplateRenderer::Plaintext, template, Some(&properties))?;
+ let rendered_plaintext = render_template(TemplateRenderer::Plaintext, template, &data)?;
// Let's not bother about testing the HTML output, too fragile.
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox 03/27] notify: replace filters and groups with matcher-based system
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 01/27] notify: introduce Error::Generic Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 02/27] notify: factor out notification content into its own type Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 04/27] notify: add calendar matcher Lukas Wagner
` (24 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This shifts notification routing into the matcher-system. Every
notification has associated metadata (key-value fields, severity -
to be extended) that can be match with match directives in
notification matchers. Right now, there are 2 matching directives,
match-field and match-severity. The first one allows one to do a
regex match/exact match on a metadata field, the other one allows one
to match one or more severites.
Every matcher also allows 'target' directives, these decide which
target(s) will be notified if a matcher matches a notification.
Since routing now happens in matchers, the API for sending is
simplified, since we do not need to specify a target any more.
The API routes for filters and groups have been removed completely.
The parser for the configuration file will still accept filter/group
entries, but will delete them once the config is saved again. This is
needed to allow a smooth transition from the old system to the new
system, since the old system was already available on pvetest.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
Sorry for the large commit, many of these changes interact with each
other and it would have been significantly more effort to keep
everything nice, tidy and compileable after splitting this apart.
I wantend to get these changes out ASAP.
proxmox-notify/Cargo.toml | 2 +
proxmox-notify/src/api/common.rs | 6 +-
proxmox-notify/src/api/filter.rs | 231 -------------
proxmox-notify/src/api/gotify.rs | 16 -
proxmox-notify/src/api/group.rs | 259 ---------------
proxmox-notify/src/api/matcher.rs | 254 +++++++++++++++
proxmox-notify/src/api/mod.rs | 115 ++-----
proxmox-notify/src/api/sendmail.rs | 15 -
proxmox-notify/src/config.rs | 34 +-
proxmox-notify/src/endpoints/gotify.rs | 19 +-
proxmox-notify/src/endpoints/sendmail.rs | 14 +-
proxmox-notify/src/filter.rs | 195 +----------
proxmox-notify/src/group.rs | 40 +--
proxmox-notify/src/lib.rs | 317 +++++++-----------
proxmox-notify/src/matcher.rs | 395 +++++++++++++++++++++++
proxmox-notify/src/schema.rs | 11 +-
16 files changed, 848 insertions(+), 1075 deletions(-)
delete mode 100644 proxmox-notify/src/api/filter.rs
delete mode 100644 proxmox-notify/src/api/group.rs
create mode 100644 proxmox-notify/src/api/matcher.rs
create mode 100644 proxmox-notify/src/matcher.rs
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 1541b8b..4812896 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -8,6 +8,7 @@ repository.workspace = true
exclude.workspace = true
[dependencies]
+anyhow.workspace = true
handlebars = { workspace = true }
lazy_static.workspace = true
log.workspace = true
@@ -16,6 +17,7 @@ openssl.workspace = true
proxmox-http = { workspace = true, features = ["client-sync"], optional = true }
proxmox-http-error.workspace = true
proxmox-human-byte.workspace = true
+proxmox-serde.workspace = true
proxmox-schema = { workspace = true, features = ["api-macro", "api-types"]}
proxmox-section-config = { workspace = true }
proxmox-sys = { workspace = true, optional = true }
diff --git a/proxmox-notify/src/api/common.rs b/proxmox-notify/src/api/common.rs
index d17f4db..fa2356e 100644
--- a/proxmox-notify/src/api/common.rs
+++ b/proxmox-notify/src/api/common.rs
@@ -7,7 +7,7 @@ use crate::{Bus, Config, Notification};
///
/// The caller is responsible for any needed permission checks.
/// Returns an `anyhow::Error` in case of an error.
-pub fn send(config: &Config, channel: &str, notification: &Notification) -> Result<(), HttpError> {
+pub fn send(config: &Config, notification: &Notification) -> Result<(), HttpError> {
let bus = Bus::from_config(config).map_err(|err| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -15,7 +15,7 @@ pub fn send(config: &Config, channel: &str, notification: &Notification) -> Resu
)
})?;
- bus.send(channel, notification);
+ bus.send(notification);
Ok(())
}
@@ -50,5 +50,5 @@ pub fn test_target(config: &Config, endpoint: &str) -> Result<(), HttpError> {
/// If the entity does not exist, the result will only contain the entity.
pub fn get_referenced_entities(config: &Config, entity: &str) -> Result<Vec<String>, HttpError> {
let entities = super::get_referenced_entities(config, entity);
- Ok(Vec::from_iter(entities.into_iter()))
+ Ok(Vec::from_iter(entities))
}
diff --git a/proxmox-notify/src/api/filter.rs b/proxmox-notify/src/api/filter.rs
deleted file mode 100644
index b8682f4..0000000
--- a/proxmox-notify/src/api/filter.rs
+++ /dev/null
@@ -1,231 +0,0 @@
-use proxmox_http_error::HttpError;
-
-use crate::api::http_err;
-use crate::filter::{DeleteableFilterProperty, FilterConfig, FilterConfigUpdater, FILTER_TYPENAME};
-use crate::Config;
-
-/// Get a list of all filters
-///
-/// The caller is responsible for any needed permission checks.
-/// Returns a list of all filters or a `HttpError` if the config is
-/// (`500 Internal server error`).
-pub fn get_filters(config: &Config) -> Result<Vec<FilterConfig>, HttpError> {
- config
- .config
- .convert_to_typed_array(FILTER_TYPENAME)
- .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "Could not fetch filters: {e}"))
-}
-
-/// Get filter with given `name`
-///
-/// The caller is responsible for any needed permission checks.
-/// Returns the endpoint or a `HttpError` if the filter was not found (`404 Not found`).
-pub fn get_filter(config: &Config, name: &str) -> Result<FilterConfig, HttpError> {
- config
- .config
- .lookup(FILTER_TYPENAME, name)
- .map_err(|_| http_err!(NOT_FOUND, "filter '{name}' not found"))
-}
-
-/// Add new notification filter.
-///
-/// The caller is responsible for any needed permission checks.
-/// The caller also responsible for locking the configuration files.
-/// Returns a `HttpError` if:
-/// - an entity with the same name already exists (`400 Bad request`)
-/// - the configuration could not be saved (`500 Internal server error`)
-pub fn add_filter(config: &mut Config, filter_config: &FilterConfig) -> Result<(), HttpError> {
- super::ensure_unique(config, &filter_config.name)?;
-
- config
- .config
- .set_data(&filter_config.name, FILTER_TYPENAME, filter_config)
- .map_err(|e| {
- http_err!(
- INTERNAL_SERVER_ERROR,
- "could not save filter '{}': {e}",
- filter_config.name
- )
- })?;
-
- Ok(())
-}
-
-/// Update existing notification filter
-///
-/// The caller is responsible for any needed permission checks.
-/// The caller also responsible for locking the configuration files.
-/// Returns a `HttpError` if:
-/// - the configuration could not be saved (`500 Internal server error`)
-/// - an invalid digest was passed (`400 Bad request`)
-pub fn update_filter(
- config: &mut Config,
- name: &str,
- filter_updater: &FilterConfigUpdater,
- delete: Option<&[DeleteableFilterProperty]>,
- digest: Option<&[u8]>,
-) -> Result<(), HttpError> {
- super::verify_digest(config, digest)?;
-
- let mut filter = get_filter(config, name)?;
-
- if let Some(delete) = delete {
- for deleteable_property in delete {
- match deleteable_property {
- DeleteableFilterProperty::MinSeverity => filter.min_severity = None,
- DeleteableFilterProperty::Mode => filter.mode = None,
- DeleteableFilterProperty::InvertMatch => filter.invert_match = None,
- DeleteableFilterProperty::Comment => filter.comment = None,
- }
- }
- }
-
- if let Some(min_severity) = filter_updater.min_severity {
- filter.min_severity = Some(min_severity);
- }
-
- if let Some(mode) = filter_updater.mode {
- filter.mode = Some(mode);
- }
-
- if let Some(invert_match) = filter_updater.invert_match {
- filter.invert_match = Some(invert_match);
- }
-
- if let Some(comment) = &filter_updater.comment {
- filter.comment = Some(comment.into());
- }
-
- config
- .config
- .set_data(name, FILTER_TYPENAME, &filter)
- .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "could not save filter '{name}': {e}"))?;
-
- Ok(())
-}
-
-/// Delete existing filter
-///
-/// The caller is responsible for any needed permission checks.
-/// The caller also responsible for locking the configuration files.
-/// Returns a `HttpError` if:
-/// - the entity does not exist (`404 Not found`)
-/// - the filter is still referenced by another entity (`400 Bad request`)
-pub fn delete_filter(config: &mut Config, name: &str) -> Result<(), HttpError> {
- // Check if the filter exists
- let _ = get_filter(config, name)?;
- super::ensure_unused(config, name)?;
-
- config.config.sections.remove(name);
-
- Ok(())
-}
-
-#[cfg(test)]
-mod tests {
- use super::*;
- use crate::filter::FilterModeOperator;
- use crate::Severity;
-
- fn empty_config() -> Config {
- Config::new("", "").unwrap()
- }
-
- fn config_with_two_filters() -> Config {
- Config::new(
- "
-filter: filter1
- min-severity info
-
-filter: filter2
- min-severity warning
-",
- "",
- )
- .unwrap()
- }
-
- #[test]
- fn test_update_not_existing_returns_error() -> Result<(), HttpError> {
- let mut config = empty_config();
- assert!(update_filter(&mut config, "test", &Default::default(), None, None).is_err());
- Ok(())
- }
-
- #[test]
- fn test_update_invalid_digest_returns_error() -> Result<(), HttpError> {
- let mut config = config_with_two_filters();
- assert!(update_filter(
- &mut config,
- "filter1",
- &Default::default(),
- None,
- Some(&[0u8; 32])
- )
- .is_err());
-
- Ok(())
- }
-
- #[test]
- fn test_filter_update() -> Result<(), HttpError> {
- let mut config = config_with_two_filters();
-
- let digest = config.digest;
-
- update_filter(
- &mut config,
- "filter1",
- &FilterConfigUpdater {
- min_severity: Some(Severity::Error),
- mode: Some(FilterModeOperator::Or),
- invert_match: Some(true),
- comment: Some("new comment".into()),
- },
- None,
- Some(&digest),
- )?;
-
- let filter = get_filter(&config, "filter1")?;
-
- assert!(matches!(filter.mode, Some(FilterModeOperator::Or)));
- assert!(matches!(filter.min_severity, Some(Severity::Error)));
- assert_eq!(filter.invert_match, Some(true));
- assert_eq!(filter.comment, Some("new comment".into()));
-
- // Test property deletion
- update_filter(
- &mut config,
- "filter1",
- &Default::default(),
- Some(&[
- DeleteableFilterProperty::InvertMatch,
- DeleteableFilterProperty::Mode,
- DeleteableFilterProperty::InvertMatch,
- DeleteableFilterProperty::MinSeverity,
- DeleteableFilterProperty::Comment,
- ]),
- Some(&digest),
- )?;
-
- let filter = get_filter(&config, "filter1")?;
-
- assert_eq!(filter.invert_match, None);
- assert_eq!(filter.min_severity, None);
- assert!(matches!(filter.mode, None));
- assert_eq!(filter.comment, None);
-
- Ok(())
- }
-
- #[test]
- fn test_filter_delete() -> Result<(), HttpError> {
- let mut config = config_with_two_filters();
-
- delete_filter(&mut config, "filter1")?;
- assert!(delete_filter(&mut config, "filter1").is_err());
- assert_eq!(get_filters(&config)?.len(), 1);
-
- Ok(())
- }
-}
diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index 0ec48fd..22d3d2e 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -36,7 +36,6 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<GotifyConfig, HttpErr
/// The caller also responsible for locking the configuration files.
/// Returns a `HttpError` if:
/// - an entity with the same name already exists (`400 Bad request`)
-/// - a referenced filter does not exist (`400 Bad request`)
/// - the configuration could not be saved (`500 Internal server error`)
///
/// Panics if the names of the private config and the public config do not match.
@@ -52,11 +51,6 @@ pub fn add_endpoint(
super::ensure_unique(config, &endpoint_config.name)?;
- if let Some(filter) = &endpoint_config.filter {
- // Check if filter exists
- super::filter::get_filter(config, filter)?;
- }
-
set_private_config_entry(config, private_endpoint_config)?;
config
@@ -77,7 +71,6 @@ pub fn add_endpoint(
/// The caller also responsible for locking the configuration files.
/// Returns a `HttpError` if:
/// - an entity with the same name already exists (`400 Bad request`)
-/// - a referenced filter does not exist (`400 Bad request`)
/// - the configuration could not be saved (`500 Internal server error`)
pub fn update_endpoint(
config: &mut Config,
@@ -95,7 +88,6 @@ pub fn update_endpoint(
for deleteable_property in delete {
match deleteable_property {
DeleteableGotifyProperty::Comment => endpoint.comment = None,
- DeleteableGotifyProperty::Filter => endpoint.filter = None,
}
}
}
@@ -118,13 +110,6 @@ pub fn update_endpoint(
endpoint.comment = Some(comment.into());
}
- if let Some(filter) = &endpoint_config_updater.filter {
- // Check if filter exists
- let _ = super::filter::get_filter(config, filter)?;
-
- endpoint.filter = Some(filter.into());
- }
-
config
.config
.set_data(name, GOTIFY_TYPENAME, &endpoint)
@@ -247,7 +232,6 @@ mod tests {
&GotifyConfigUpdater {
server: Some("newhost".into()),
comment: Some("newcomment".into()),
- filter: None,
},
&GotifyPrivateConfigUpdater {
token: Some("changedtoken".into()),
diff --git a/proxmox-notify/src/api/group.rs b/proxmox-notify/src/api/group.rs
deleted file mode 100644
index 6fc71ea..0000000
--- a/proxmox-notify/src/api/group.rs
+++ /dev/null
@@ -1,259 +0,0 @@
-use proxmox_http_error::HttpError;
-
-use crate::api::{http_bail, http_err};
-use crate::group::{DeleteableGroupProperty, GroupConfig, GroupConfigUpdater, GROUP_TYPENAME};
-use crate::Config;
-
-/// Get all notification groups
-///
-/// The caller is responsible for any needed permission checks.
-/// Returns a list of all groups or a `HttpError` if the config is
-/// erroneous (`500 Internal server error`).
-pub fn get_groups(config: &Config) -> Result<Vec<GroupConfig>, HttpError> {
- config
- .config
- .convert_to_typed_array(GROUP_TYPENAME)
- .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "Could not fetch groups: {e}"))
-}
-
-/// Get group with given `name`
-///
-/// The caller is responsible for any needed permission checks.
-/// Returns the endpoint or an `HttpError` if the group was not found (`404 Not found`).
-pub fn get_group(config: &Config, name: &str) -> Result<GroupConfig, HttpError> {
- config
- .config
- .lookup(GROUP_TYPENAME, name)
- .map_err(|_| http_err!(NOT_FOUND, "group '{name}' not found"))
-}
-
-/// Add a new group.
-///
-/// The caller is responsible for any needed permission checks.
-/// The caller also responsible for locking the configuration files.
-/// Returns a `HttpError` if:
-/// - an entity with the same name already exists (`400 Bad request`)
-/// - a referenced filter does not exist (`400 Bad request`)
-/// - no endpoints were passed (`400 Bad request`)
-/// - referenced endpoints do not exist (`404 Not found`)
-/// - the configuration could not be saved (`500 Internal server error`)
-pub fn add_group(config: &mut Config, group_config: &GroupConfig) -> Result<(), HttpError> {
- super::ensure_unique(config, &group_config.name)?;
-
- if group_config.endpoint.is_empty() {
- http_bail!(BAD_REQUEST, "group must contain at least one endpoint",);
- }
-
- if let Some(filter) = &group_config.filter {
- // Check if filter exists
- super::filter::get_filter(config, filter)?;
- }
-
- super::ensure_endpoints_exist(config, &group_config.endpoint)?;
-
- config
- .config
- .set_data(&group_config.name, GROUP_TYPENAME, group_config)
- .map_err(|e| {
- http_err!(
- INTERNAL_SERVER_ERROR,
- "could not save group '{}': {e}",
- group_config.name
- )
- })
-}
-
-/// Update existing group
-///
-/// The caller is responsible for any needed permission checks.
-/// The caller also responsible for locking the configuration files.
-/// Returns a `HttpError` if:
-/// - a referenced filter does not exist (`400 Bad request`)
-/// - an invalid digest was passed (`400 Bad request`)
-/// - no endpoints were passed (`400 Bad request`)
-/// - referenced endpoints do not exist (`404 Not found`)
-/// - the configuration could not be saved (`500 Internal server error`)
-pub fn update_group(
- config: &mut Config,
- name: &str,
- updater: &GroupConfigUpdater,
- delete: Option<&[DeleteableGroupProperty]>,
- digest: Option<&[u8]>,
-) -> Result<(), HttpError> {
- super::verify_digest(config, digest)?;
-
- let mut group = get_group(config, name)?;
-
- if let Some(delete) = delete {
- for deleteable_property in delete {
- match deleteable_property {
- DeleteableGroupProperty::Comment => group.comment = None,
- DeleteableGroupProperty::Filter => group.filter = None,
- }
- }
- }
-
- if let Some(endpoints) = &updater.endpoint {
- super::ensure_endpoints_exist(config, endpoints)?;
- if endpoints.is_empty() {
- http_bail!(BAD_REQUEST, "group must contain at least one endpoint",);
- }
- group.endpoint = endpoints.iter().map(Into::into).collect()
- }
-
- if let Some(comment) = &updater.comment {
- group.comment = Some(comment.into());
- }
-
- if let Some(filter) = &updater.filter {
- // Check if filter exists
- let _ = super::filter::get_filter(config, filter)?;
- group.filter = Some(filter.into());
- }
-
- config
- .config
- .set_data(name, GROUP_TYPENAME, &group)
- .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "could not save group '{name}': {e}"))
-}
-
-/// Delete existing group
-///
-/// The caller is responsible for any needed permission checks.
-/// The caller also responsible for locking the configuration files.
-/// Returns a `HttpError` if the group does not exist (`404 Not found`).
-pub fn delete_group(config: &mut Config, name: &str) -> Result<(), HttpError> {
- // Check if the group exists
- let _ = get_group(config, name)?;
-
- config.config.sections.remove(name);
-
- Ok(())
-}
-
-// groups cannot be empty, so only build the tests if we have the
-// sendmail endpoint available
-#[cfg(all(test, feature = "sendmail"))]
-mod tests {
- use super::*;
- use crate::api::sendmail::tests::add_sendmail_endpoint_for_test;
- use crate::api::test_helpers::*;
-
- fn add_default_group(config: &mut Config) -> Result<(), HttpError> {
- add_sendmail_endpoint_for_test(config, "test")?;
-
- add_group(
- config,
- &GroupConfig {
- name: "group1".into(),
- endpoint: vec!["test".to_string()],
- comment: None,
- filter: None,
- },
- )?;
-
- Ok(())
- }
-
- #[test]
- fn test_add_group_fails_if_endpoint_does_not_exist() {
- let mut config = empty_config();
- assert!(add_group(
- &mut config,
- &GroupConfig {
- name: "group1".into(),
- endpoint: vec!["foo".into()],
- comment: None,
- filter: None,
- },
- )
- .is_err());
- }
-
- #[test]
- fn test_add_group() -> Result<(), HttpError> {
- let mut config = empty_config();
- assert!(add_default_group(&mut config).is_ok());
- Ok(())
- }
-
- #[test]
- fn test_update_group_fails_if_endpoint_does_not_exist() -> Result<(), HttpError> {
- let mut config = empty_config();
- add_default_group(&mut config)?;
-
- assert!(update_group(
- &mut config,
- "group1",
- &GroupConfigUpdater {
- endpoint: Some(vec!["foo".into()]),
- ..Default::default()
- },
- None,
- None
- )
- .is_err());
- Ok(())
- }
-
- #[test]
- fn test_update_group_fails_if_digest_invalid() -> Result<(), HttpError> {
- let mut config = empty_config();
- add_default_group(&mut config)?;
-
- assert!(update_group(
- &mut config,
- "group1",
- &Default::default(),
- None,
- Some(&[0u8; 32])
- )
- .is_err());
- Ok(())
- }
-
- #[test]
- fn test_update_group() -> Result<(), HttpError> {
- let mut config = empty_config();
- add_default_group(&mut config)?;
-
- assert!(update_group(
- &mut config,
- "group1",
- &GroupConfigUpdater {
- endpoint: None,
- comment: Some("newcomment".into()),
- filter: None
- },
- None,
- None,
- )
- .is_ok());
- let group = get_group(&config, "group1")?;
- assert_eq!(group.comment, Some("newcomment".into()));
-
- assert!(update_group(
- &mut config,
- "group1",
- &Default::default(),
- Some(&[DeleteableGroupProperty::Comment]),
- None
- )
- .is_ok());
- let group = get_group(&config, "group1")?;
- assert_eq!(group.comment, None);
-
- Ok(())
- }
-
- #[test]
- fn test_group_delete() -> Result<(), HttpError> {
- let mut config = empty_config();
- add_default_group(&mut config)?;
-
- assert!(delete_group(&mut config, "group1").is_ok());
- assert!(delete_group(&mut config, "group1").is_err());
-
- Ok(())
- }
-}
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
new file mode 100644
index 0000000..e37b74f
--- /dev/null
+++ b/proxmox-notify/src/api/matcher.rs
@@ -0,0 +1,254 @@
+use proxmox_http_error::HttpError;
+
+use crate::api::http_err;
+use crate::matcher::{
+ DeleteableMatcherProperty, MatcherConfig, MatcherConfigUpdater, MATCHER_TYPENAME,
+};
+use crate::Config;
+
+/// Get a list of all matchers
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns a list of all matchers or a `HttpError` if the config is
+/// (`500 Internal server error`).
+pub fn get_matchers(config: &Config) -> Result<Vec<MatcherConfig>, HttpError> {
+ config
+ .config
+ .convert_to_typed_array(MATCHER_TYPENAME)
+ .map_err(|e| http_err!(INTERNAL_SERVER_ERROR, "Could not fetch matchers: {e}"))
+}
+
+/// Get matcher with given `name`
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns the endpoint or a `HttpError` if the matcher was not found (`404 Not found`).
+pub fn get_matcher(config: &Config, name: &str) -> Result<MatcherConfig, HttpError> {
+ config
+ .config
+ .lookup(MATCHER_TYPENAME, name)
+ .map_err(|_| http_err!(NOT_FOUND, "matcher '{name}' not found"))
+}
+
+/// Add new notification matcher.
+///
+/// The caller is responsible for any needed permission checks.
+/// The caller also responsible for locking the configuration files.
+/// Returns a `HttpError` if:
+/// - an entity with the same name already exists (`400 Bad request`)
+/// - the configuration could not be saved (`500 Internal server error`)
+pub fn add_matcher(config: &mut Config, matcher_config: &MatcherConfig) -> Result<(), HttpError> {
+ super::ensure_unique(config, &matcher_config.name)?;
+
+ if let Some(targets) = matcher_config.target.as_deref() {
+ super::ensure_endpoints_exist(config, targets)?;
+ }
+
+ config
+ .config
+ .set_data(&matcher_config.name, MATCHER_TYPENAME, matcher_config)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not save matcher '{}': {e}",
+ matcher_config.name
+ )
+ })?;
+
+ Ok(())
+}
+
+/// Update existing notification matcher
+///
+/// The caller is responsible for any needed permission checks.
+/// The caller also responsible for locking the configuration files.
+/// Returns a `HttpError` if:
+/// - the configuration could not be saved (`500 Internal server error`)
+/// - an invalid digest was passed (`400 Bad request`)
+pub fn update_matcher(
+ config: &mut Config,
+ name: &str,
+ matcher_updater: &MatcherConfigUpdater,
+ delete: Option<&[DeleteableMatcherProperty]>,
+ digest: Option<&[u8]>,
+) -> Result<(), HttpError> {
+ super::verify_digest(config, digest)?;
+
+ let mut matcher = get_matcher(config, name)?;
+
+ 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::Target => matcher.target = None,
+ DeleteableMatcherProperty::Mode => matcher.mode = None,
+ DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None,
+ DeleteableMatcherProperty::Comment => matcher.comment = None,
+ }
+ }
+ }
+
+ if let Some(match_severity) = &matcher_updater.match_severity {
+ matcher.match_severity = Some(match_severity.clone());
+ }
+
+ if let Some(match_field) = &matcher_updater.match_field {
+ matcher.match_field = Some(match_field.clone());
+ }
+
+ if let Some(mode) = matcher_updater.mode {
+ matcher.mode = Some(mode);
+ }
+
+ if let Some(invert_match) = matcher_updater.invert_match {
+ matcher.invert_match = Some(invert_match);
+ }
+
+ if let Some(comment) = &matcher_updater.comment {
+ matcher.comment = Some(comment.into());
+ }
+
+ if let Some(target) = &matcher_updater.target {
+ super::ensure_endpoints_exist(config, target.as_slice())?;
+ matcher.target = Some(target.clone());
+ }
+
+ config
+ .config
+ .set_data(name, MATCHER_TYPENAME, &matcher)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not save matcher '{name}': {e}"
+ )
+ })?;
+
+ Ok(())
+}
+
+/// Delete existing matcher
+///
+/// The caller is responsible for any needed permission checks.
+/// The caller also responsible for locking the configuration files.
+/// Returns a `HttpError` if:
+/// - the entity does not exist (`404 Not found`)
+pub fn delete_matcher(config: &mut Config, name: &str) -> Result<(), HttpError> {
+ // Check if the matcher exists
+ let _ = get_matcher(config, name)?;
+
+ config.config.sections.remove(name);
+
+ Ok(())
+}
+
+#[cfg(all(test, feature = "sendmail"))]
+mod tests {
+ use super::*;
+ use crate::matcher::MatchModeOperator;
+
+ fn empty_config() -> Config {
+ Config::new("", "").unwrap()
+ }
+
+ fn config_with_two_matchers() -> Config {
+ Config::new(
+ "
+sendmail: foo
+ mailto test@example.com
+
+matcher: matcher1
+
+matcher: matcher2
+",
+ "",
+ )
+ .unwrap()
+ }
+
+ #[test]
+ fn test_update_not_existing_returns_error() -> Result<(), HttpError> {
+ let mut config = empty_config();
+ assert!(update_matcher(&mut config, "test", &Default::default(), None, None).is_err());
+ Ok(())
+ }
+
+ #[test]
+ fn test_update_invalid_digest_returns_error() -> Result<(), HttpError> {
+ let mut config = config_with_two_matchers();
+ assert!(update_matcher(
+ &mut config,
+ "matcher1",
+ &Default::default(),
+ None,
+ Some(&[0u8; 32])
+ )
+ .is_err());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_matcher_update() -> Result<(), HttpError> {
+ let mut config = config_with_two_matchers();
+
+ let digest = config.digest;
+
+ update_matcher(
+ &mut config,
+ "matcher1",
+ &MatcherConfigUpdater {
+ mode: Some(MatchModeOperator::Any),
+ match_field: None,
+ match_severity: None,
+ invert_match: Some(true),
+ target: Some(vec!["foo".into()]),
+ comment: Some("new comment".into()),
+ },
+ None,
+ Some(&digest),
+ )?;
+
+ let matcher = get_matcher(&config, "matcher1")?;
+
+ assert!(matches!(matcher.mode, Some(MatchModeOperator::Any)));
+ assert_eq!(matcher.invert_match, Some(true));
+ assert_eq!(matcher.comment, Some("new comment".into()));
+
+ // Test property deletion
+ update_matcher(
+ &mut config,
+ "matcher1",
+ &Default::default(),
+ Some(&[
+ DeleteableMatcherProperty::InvertMatch,
+ DeleteableMatcherProperty::Mode,
+ DeleteableMatcherProperty::MatchField,
+ DeleteableMatcherProperty::Target,
+ DeleteableMatcherProperty::Comment,
+ ]),
+ Some(&digest),
+ )?;
+
+ let matcher = get_matcher(&config, "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.mode.is_none());
+ assert_eq!(matcher.comment, None);
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_matcher_delete() -> Result<(), HttpError> {
+ let mut config = config_with_two_matchers();
+
+ delete_matcher(&mut config, "matcher1")?;
+ assert!(delete_matcher(&mut config, "matcher1").is_err());
+ assert_eq!(get_matchers(&config)?.len(), 1);
+
+ Ok(())
+ }
+}
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index 8dc9b4e..8042157 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -5,10 +5,9 @@ use proxmox_http_error::HttpError;
use crate::Config;
pub mod common;
-pub mod filter;
#[cfg(feature = "gotify")]
pub mod gotify;
-pub mod group;
+pub mod matcher;
#[cfg(feature = "sendmail")]
pub mod sendmail;
@@ -94,36 +93,13 @@ fn ensure_unique(config: &Config, entity: &str) -> Result<(), HttpError> {
fn get_referrers(config: &Config, entity: &str) -> Result<HashSet<String>, HttpError> {
let mut referrers = HashSet::new();
- for group in group::get_groups(config)? {
- if group.endpoint.iter().any(|endpoint| endpoint == entity) {
- referrers.insert(group.name.clone());
- }
-
- if let Some(filter) = group.filter {
- if filter == entity {
- referrers.insert(group.name);
- }
- }
- }
-
- #[cfg(feature = "sendmail")]
- for endpoint in sendmail::get_endpoints(config)? {
- if let Some(filter) = endpoint.filter {
- if filter == entity {
- referrers.insert(endpoint.name);
+ 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());
}
}
}
-
- #[cfg(feature = "gotify")]
- for endpoint in gotify::get_endpoints(config)? {
- if let Some(filter) = endpoint.filter {
- if filter == entity {
- referrers.insert(endpoint.name);
- }
- }
- }
-
Ok(referrers)
}
@@ -151,23 +127,11 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet<String> {
let mut new = HashSet::new();
for entity in entities {
- if let Ok(group) = group::get_group(config, entity) {
- for target in group.endpoint {
- new.insert(target.clone());
- }
- }
-
- #[cfg(feature = "sendmail")]
- if let Ok(target) = sendmail::get_endpoint(config, entity) {
- if let Some(filter) = target.filter {
- new.insert(filter.clone());
- }
- }
-
- #[cfg(feature = "gotify")]
- if let Ok(target) = gotify::get_endpoint(config, entity) {
- if let Some(filter) = target.filter {
- new.insert(filter.clone());
+ if let Ok(group) = matcher::get_matcher(config, entity) {
+ if let Some(targets) = group.target {
+ for target in targets {
+ new.insert(target.clone());
+ }
}
}
}
@@ -205,11 +169,12 @@ mod tests {
fn prepare_config() -> Result<Config, HttpError> {
let mut config = super::test_helpers::empty_config();
- filter::add_filter(
+ matcher::add_matcher(
&mut config,
- &FilterConfig {
- name: "filter".to_string(),
- ..Default::default()
+ &MatcherConfig {
+ name: "matcher".to_string(),
+ target: Some(vec!["sendmail".to_string(), "gotify".to_string()])
+ ..Default::default(),
},
)?;
@@ -218,7 +183,6 @@ mod tests {
&SendmailConfig {
name: "sendmail".to_string(),
mailto: Some(vec!["foo@example.com".to_string()]),
- filter: Some("filter".to_string()),
..Default::default()
},
)?;
@@ -228,7 +192,6 @@ mod tests {
&GotifyConfig {
name: "gotify".to_string(),
server: "localhost".to_string(),
- filter: Some("filter".to_string()),
..Default::default()
},
&GotifyPrivateConfig {
@@ -237,16 +200,6 @@ mod tests {
},
)?;
- group::add_group(
- &mut config,
- &GroupConfig {
- name: "group".to_string(),
- endpoint: vec!["gotify".to_string(), "sendmail".to_string()],
- filter: Some("filter".to_string()),
- ..Default::default()
- },
- )?;
-
Ok(config)
}
@@ -255,24 +208,11 @@ mod tests {
let config = prepare_config().unwrap();
assert_eq!(
- get_referenced_entities(&config, "filter"),
- HashSet::from(["filter".to_string()])
- );
- assert_eq!(
- get_referenced_entities(&config, "sendmail"),
- HashSet::from(["filter".to_string(), "sendmail".to_string()])
- );
- assert_eq!(
- get_referenced_entities(&config, "gotify"),
- HashSet::from(["filter".to_string(), "gotify".to_string()])
- );
- assert_eq!(
- get_referenced_entities(&config, "group"),
+ get_referenced_entities(&config, "matcher"),
HashSet::from([
- "filter".to_string(),
- "gotify".to_string(),
+ "matcher".to_string(),
"sendmail".to_string(),
- "group".to_string()
+ "gotify".to_string()
])
);
}
@@ -281,27 +221,16 @@ mod tests {
fn test_get_referrers_for_entity() -> Result<(), HttpError> {
let config = prepare_config().unwrap();
- assert_eq!(
- get_referrers(&config, "filter")?,
- HashSet::from([
- "gotify".to_string(),
- "sendmail".to_string(),
- "group".to_string()
- ])
- );
-
assert_eq!(
get_referrers(&config, "sendmail")?,
- HashSet::from(["group".to_string()])
+ HashSet::from(["matcher".to_string()])
);
assert_eq!(
get_referrers(&config, "gotify")?,
- HashSet::from(["group".to_string()])
+ HashSet::from(["matcher".to_string()])
);
- assert!(get_referrers(&config, "group")?.is_empty(),);
-
Ok(())
}
@@ -309,10 +238,9 @@ mod tests {
fn test_ensure_unused() {
let config = prepare_config().unwrap();
- assert!(ensure_unused(&config, "filter").is_err());
assert!(ensure_unused(&config, "gotify").is_err());
assert!(ensure_unused(&config, "sendmail").is_err());
- assert!(ensure_unused(&config, "group").is_ok());
+ assert!(ensure_unused(&config, "matcher").is_ok());
}
#[test]
@@ -329,6 +257,5 @@ mod tests {
let config = prepare_config().unwrap();
assert!(ensure_endpoints_exist(&config, &vec!["sendmail", "gotify"]).is_ok());
- assert!(ensure_endpoints_exist(&config, &vec!["group", "filter"]).is_err());
}
}
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index ac8737c..dbd9559 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -35,17 +35,11 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SendmailConfig, HttpE
/// The caller also responsible for locking the configuration files.
/// Returns a `HttpError` if:
/// - an entity with the same name already exists (`400 Bad request`)
-/// - a referenced filter does not exist (`400 Bad request`)
/// - the configuration could not be saved (`500 Internal server error`)
/// - mailto *and* mailto_user are both set to `None`
pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<(), HttpError> {
super::ensure_unique(config, &endpoint.name)?;
- if let Some(filter) = &endpoint.filter {
- // Check if filter exists
- super::filter::get_filter(config, filter)?;
- }
-
if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
http_bail!(
BAD_REQUEST,
@@ -70,7 +64,6 @@ pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<()
/// The caller is responsible for any needed permission checks.
/// The caller also responsible for locking the configuration files.
/// Returns a `HttpError` if:
-/// - a referenced filter does not exist (`400 Bad request`)
/// - the configuration could not be saved (`500 Internal server error`)
/// - mailto *and* mailto_user are both set to `None`
pub fn update_endpoint(
@@ -90,7 +83,6 @@ pub fn update_endpoint(
DeleteableSendmailProperty::FromAddress => endpoint.from_address = None,
DeleteableSendmailProperty::Author => endpoint.author = None,
DeleteableSendmailProperty::Comment => endpoint.comment = None,
- DeleteableSendmailProperty::Filter => endpoint.filter = None,
DeleteableSendmailProperty::Mailto => endpoint.mailto = None,
DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user = None,
}
@@ -117,11 +109,6 @@ pub fn update_endpoint(
endpoint.comment = Some(comment.into());
}
- if let Some(filter) = &updater.filter {
- let _ = super::filter::get_filter(config, filter)?;
- endpoint.filter = Some(filter.into());
- }
-
if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
http_bail!(
BAD_REQUEST,
@@ -221,7 +208,6 @@ pub mod tests {
from_address: Some("root@example.com".into()),
author: Some("newauthor".into()),
comment: Some("new comment".into()),
- filter: None,
},
None,
Some(&[0; 32]),
@@ -247,7 +233,6 @@ pub mod tests {
from_address: Some("root@example.com".into()),
author: Some("newauthor".into()),
comment: Some("new comment".into()),
- filter: None,
},
None,
Some(&digest),
diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
index cdbf42a..a86995e 100644
--- a/proxmox-notify/src/config.rs
+++ b/proxmox-notify/src/config.rs
@@ -5,6 +5,7 @@ use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlug
use crate::filter::{FilterConfig, FILTER_TYPENAME};
use crate::group::{GroupConfig, GROUP_TYPENAME};
+use crate::matcher::{MatcherConfig, MATCHER_TYPENAME};
use crate::schema::BACKEND_NAME_SCHEMA;
use crate::Error;
@@ -39,8 +40,14 @@ fn config_init() -> SectionConfig {
));
}
- const GROUP_SCHEMA: &ObjectSchema = GroupConfig::API_SCHEMA.unwrap_object_schema();
+ const MATCHER_SCHEMA: &ObjectSchema = MatcherConfig::API_SCHEMA.unwrap_object_schema();
+ config.register_plugin(SectionConfigPlugin::new(
+ MATCHER_TYPENAME.to_string(),
+ Some(String::from("name")),
+ MATCHER_SCHEMA,
+ ));
+ const GROUP_SCHEMA: &ObjectSchema = GroupConfig::API_SCHEMA.unwrap_object_schema();
config.register_plugin(SectionConfigPlugin::new(
GROUP_TYPENAME.to_string(),
Some(String::from("name")),
@@ -78,9 +85,32 @@ fn private_config_init() -> SectionConfig {
pub fn config(raw_config: &str) -> Result<(SectionConfigData, [u8; 32]), Error> {
let digest = openssl::sha::sha256(raw_config.as_bytes());
- let data = CONFIG
+ let mut data = CONFIG
.parse("notifications.cfg", raw_config)
.map_err(|err| Error::ConfigDeserialization(err.into()))?;
+
+ // TODO: Remove this once this has been in production for a while.
+ // 'group' and 'filter' sections are remnants of the 'old'
+ // notification routing approach that already hit pvetest...
+ // This mechanism cleans out left-over entries.
+ let entries: Vec<GroupConfig> = data.convert_to_typed_array("group").unwrap_or_default();
+ if !entries.is_empty() {
+ log::warn!("clearing left-over 'group' entries from notifications.cfg");
+ }
+
+ for entry in entries {
+ data.sections.remove(&entry.name);
+ }
+
+ let entries: Vec<FilterConfig> = data.convert_to_typed_array("filter").unwrap_or_default();
+ if !entries.is_empty() {
+ log::warn!("clearing left-over 'filter' entries from notifications.cfg");
+ }
+
+ for entry in entries {
+ data.sections.remove(&entry.name);
+ }
+
Ok((data, digest))
}
diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs
index af86f9c..1c307a4 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -33,10 +33,6 @@ pub(crate) const GOTIFY_TYPENAME: &str = "gotify";
optional: true,
schema: COMMENT_SCHEMA,
},
- filter: {
- optional: true,
- schema: ENTITY_NAME_SCHEMA,
- },
}
)]
#[derive(Serialize, Deserialize, Updater, Default)]
@@ -51,8 +47,9 @@ pub struct GotifyConfig {
/// Comment
#[serde(skip_serializing_if = "Option::is_none")]
pub comment: Option<String>,
- /// Filter to apply
- #[serde(skip_serializing_if = "Option::is_none")]
+ /// Deprecated.
+ #[serde(skip_serializing)]
+ #[updater(skip)]
pub filter: Option<String>,
}
@@ -80,17 +77,15 @@ pub struct GotifyEndpoint {
#[serde(rename_all = "kebab-case")]
pub enum DeleteableGotifyProperty {
Comment,
- Filter,
}
impl Endpoint for GotifyEndpoint {
fn send(&self, notification: &Notification) -> Result<(), Error> {
-
let (title, message) = match ¬ification.content {
Content::Template {
title_template,
body_template,
- data
+ data,
} => {
let rendered_title =
renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
@@ -108,7 +103,7 @@ impl Endpoint for GotifyEndpoint {
let body = json!({
"title": &title,
"message": &message,
- "priority": severity_to_priority(notification.severity),
+ "priority": severity_to_priority(notification.metadata.severity),
"extras": {
"client::display": {
"contentType": "text/markdown"
@@ -152,8 +147,4 @@ impl Endpoint for GotifyEndpoint {
fn name(&self) -> &str {
&self.config.name
}
-
- fn filter(&self) -> Option<&str> {
- self.config.filter.as_deref()
- }
}
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index c540925..a601744 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -35,10 +35,6 @@ pub(crate) const SENDMAIL_TYPENAME: &str = "sendmail";
optional: true,
schema: COMMENT_SCHEMA,
},
- filter: {
- optional: true,
- schema: ENTITY_NAME_SCHEMA,
- },
},
)]
#[derive(Debug, Serialize, Deserialize, Updater, Default)]
@@ -63,8 +59,9 @@ pub struct SendmailConfig {
/// Comment
#[serde(skip_serializing_if = "Option::is_none")]
pub comment: Option<String>,
- /// Filter to apply
- #[serde(skip_serializing_if = "Option::is_none")]
+ /// Deprecated.
+ #[serde(skip_serializing)]
+ #[updater(skip)]
pub filter: Option<String>,
}
@@ -74,7 +71,6 @@ pub enum DeleteableSendmailProperty {
FromAddress,
Author,
Comment,
- Filter,
Mailto,
MailtoUser,
}
@@ -144,8 +140,4 @@ impl Endpoint for SendmailEndpoint {
fn name(&self) -> &str {
&self.config.name
}
-
- fn filter(&self) -> Option<&str> {
- self.config.filter.as_deref()
- }
}
diff --git a/proxmox-notify/src/filter.rs b/proxmox-notify/src/filter.rs
index e014a59..c9b152b 100644
--- a/proxmox-notify/src/filter.rs
+++ b/proxmox-notify/src/filter.rs
@@ -1,202 +1,23 @@
-use std::collections::{HashMap, HashSet};
-
use serde::{Deserialize, Serialize};
-use proxmox_schema::api_types::COMMENT_SCHEMA;
-use proxmox_schema::{api, Updater};
+use proxmox_schema::api;
use crate::schema::ENTITY_NAME_SCHEMA;
-use crate::{Error, Notification, Severity};
-
-pub const FILTER_TYPENAME: &str = "filter";
-
-#[api]
-#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy)]
-#[serde(rename_all = "kebab-case")]
-pub enum FilterModeOperator {
- /// All filter properties have to match (AND)
- #[default]
- And,
- /// At least one filter property has to match (OR)
- Or,
-}
-impl FilterModeOperator {
- /// Apply the mode operator to two bools, lhs and rhs
- fn apply(&self, lhs: bool, rhs: bool) -> bool {
- match self {
- FilterModeOperator::And => lhs && rhs,
- FilterModeOperator::Or => lhs || rhs,
- }
- }
-
- fn neutral_element(&self) -> bool {
- match self {
- FilterModeOperator::And => true,
- FilterModeOperator::Or => false,
- }
- }
-}
+pub(crate) const FILTER_TYPENAME: &str = "filter";
#[api(
properties: {
name: {
schema: ENTITY_NAME_SCHEMA,
},
- comment: {
- optional: true,
- schema: COMMENT_SCHEMA,
- },
- })]
-#[derive(Debug, Serialize, Deserialize, Updater, Default)]
+ },
+ additional_properties: true,
+)]
+#[derive(Debug, Serialize, Deserialize, Default)]
#[serde(rename_all = "kebab-case")]
-/// Config for Sendmail notification endpoints
+/// Config for the old filter system - can be removed at some point.
pub struct FilterConfig {
- /// Name of the filter
- #[updater(skip)]
+ /// Name of the group
pub name: String,
-
- /// Minimum severity to match
- #[serde(skip_serializing_if = "Option::is_none")]
- pub min_severity: Option<Severity>,
-
- /// Choose between 'and' and 'or' for when multiple properties are specified
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mode: Option<FilterModeOperator>,
-
- /// Invert match of the whole filter
- #[serde(skip_serializing_if = "Option::is_none")]
- pub invert_match: Option<bool>,
-
- /// Comment
- #[serde(skip_serializing_if = "Option::is_none")]
- pub comment: Option<String>,
-}
-
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-pub enum DeleteableFilterProperty {
- MinSeverity,
- Mode,
- InvertMatch,
- Comment,
-}
-
-/// A caching, lazily-evaluating notification filter. Parameterized with the notification itself,
-/// since there are usually multiple filters to check for a single notification that is to be sent.
-pub(crate) struct FilterMatcher<'a> {
- filters: HashMap<&'a str, &'a FilterConfig>,
- cached_results: HashMap<&'a str, bool>,
- notification: &'a Notification,
-}
-
-impl<'a> FilterMatcher<'a> {
- pub(crate) fn new(filters: &'a [FilterConfig], notification: &'a Notification) -> Self {
- let filters = filters.iter().map(|f| (f.name.as_str(), f)).collect();
-
- Self {
- filters,
- cached_results: Default::default(),
- notification,
- }
- }
-
- /// Check if the notification that was used to instantiate Self matches a given filter
- pub(crate) fn check_filter_match(&mut self, filter_name: &str) -> Result<bool, Error> {
- let mut visited = HashSet::new();
-
- self.do_check_filter(filter_name, &mut visited)
- }
-
- fn do_check_filter(
- &mut self,
- filter_name: &str,
- visited: &mut HashSet<String>,
- ) -> Result<bool, Error> {
- if visited.contains(filter_name) {
- return Err(Error::FilterFailed(format!(
- "recursive filter definition: {filter_name}"
- )));
- }
-
- if let Some(is_match) = self.cached_results.get(filter_name) {
- return Ok(*is_match);
- }
-
- visited.insert(filter_name.into());
-
- let filter_config =
- self.filters.get(filter_name).copied().ok_or_else(|| {
- Error::FilterFailed(format!("filter '{filter_name}' does not exist"))
- })?;
-
- let invert_match = filter_config.invert_match.unwrap_or_default();
-
- let mode_operator = filter_config.mode.unwrap_or_default();
-
- let mut notification_matches = mode_operator.neutral_element();
-
- notification_matches = mode_operator.apply(
- notification_matches,
- self.check_severity_match(filter_config, mode_operator),
- );
-
- Ok(notification_matches != invert_match)
- }
-
- fn check_severity_match(
- &self,
- filter_config: &FilterConfig,
- mode_operator: FilterModeOperator,
- ) -> bool {
- if let Some(min_severity) = filter_config.min_severity {
- self.notification.severity >= min_severity
- } else {
- mode_operator.neutral_element()
- }
- }
-}
-
-#[cfg(test)]
-mod tests {
- use super::*;
- use crate::{config, Content};
-
- fn parse_filters(config: &str) -> Result<Vec<FilterConfig>, Error> {
- let (config, _) = config::config(config)?;
- Ok(config.convert_to_typed_array("filter").unwrap())
- }
-
- fn empty_notification_with_severity(severity: Severity) -> Notification {
- Notification {
- content: Content::Template {
- title_template: String::new(),
- body_template: String::new(),
- data: Default::default(),
- },
- severity,
- }
- }
-
- #[test]
- fn test_trivial_severity_filters() -> Result<(), Error> {
- let config = "
-filter: test
- min-severity warning
-";
-
- let filters = parse_filters(config)?;
-
- let is_match = |severity| {
- let notification = empty_notification_with_severity(severity);
- let mut results = FilterMatcher::new(&filters, ¬ification);
- results.check_filter_match("test")
- };
-
- assert!(is_match(Severity::Warning)?);
- assert!(!is_match(Severity::Notice)?);
- assert!(is_match(Severity::Error)?);
-
- Ok(())
- }
}
diff --git a/proxmox-notify/src/group.rs b/proxmox-notify/src/group.rs
index 713e52e..46458db 100644
--- a/proxmox-notify/src/group.rs
+++ b/proxmox-notify/src/group.rs
@@ -1,7 +1,6 @@
use serde::{Deserialize, Serialize};
-use proxmox_schema::api_types::COMMENT_SCHEMA;
-use proxmox_schema::{api, Updater};
+use proxmox_schema::api;
use crate::schema::ENTITY_NAME_SCHEMA;
@@ -9,43 +8,16 @@ pub(crate) const GROUP_TYPENAME: &str = "group";
#[api(
properties: {
- "endpoint": {
- type: Array,
- items: {
- description: "Name of the included endpoint(s)",
- type: String,
- },
- },
- comment: {
- optional: true,
- schema: COMMENT_SCHEMA,
- },
- filter: {
- optional: true,
+ name: {
schema: ENTITY_NAME_SCHEMA,
},
},
+ additional_properties: true,
)]
-#[derive(Debug, Serialize, Deserialize, Updater, Default)]
+#[derive(Debug, Serialize, Deserialize, Default)]
#[serde(rename_all = "kebab-case")]
-/// Config for notification channels
+/// Config for the old target groups - can be removed at some point.
pub struct GroupConfig {
- /// Name of the channel
- #[updater(skip)]
+ /// Name of the group
pub name: String,
- /// Endpoints for this channel
- pub endpoint: Vec<String>,
- /// Comment
- #[serde(skip_serializing_if = "Option::is_none")]
- pub comment: Option<String>,
- /// Filter to apply
- #[serde(skip_serializing_if = "Option::is_none")]
- pub filter: Option<String>,
-}
-
-#[derive(Serialize, Deserialize)]
-#[serde(rename_all = "kebab-case")]
-pub enum DeleteableGroupProperty {
- Comment,
- Filter,
}
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index d40d017..1f95ae0 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -1,6 +1,7 @@
use std::collections::HashMap;
use std::error::Error as StdError;
use std::fmt::Display;
+use std::str::FromStr;
use serde::{Deserialize, Serialize};
use serde_json::json;
@@ -9,15 +10,14 @@ use serde_json::Value;
use proxmox_schema::api;
use proxmox_section_config::SectionConfigData;
-pub mod filter;
-use filter::{FilterConfig, FilterMatcher, FILTER_TYPENAME};
-
-pub mod group;
-use group::{GroupConfig, GROUP_TYPENAME};
+pub mod matcher;
+use matcher::{MatcherConfig, MATCHER_TYPENAME};
pub mod api;
pub mod context;
pub mod endpoints;
+pub mod filter;
+pub mod group;
pub mod renderer;
pub mod schema;
@@ -104,6 +104,30 @@ pub enum Severity {
Error,
}
+impl Display for Severity {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::result::Result<(), std::fmt::Error> {
+ match self {
+ Severity::Info => f.write_str("info"),
+ Severity::Notice => f.write_str("notice"),
+ Severity::Warning => f.write_str("warning"),
+ Severity::Error => f.write_str("error"),
+ }
+ }
+}
+
+impl FromStr for Severity {
+ type Err = Error;
+ fn from_str(s: &str) -> Result<Self, Error> {
+ match s {
+ "info" => Ok(Self::Info),
+ "notice" => Ok(Self::Notice),
+ "warning" => Ok(Self::Warning),
+ "error" => Ok(Self::Error),
+ _ => Err(Error::Generic(format!("invalid severity {s}"))),
+ }
+ }
+}
+
/// Notification endpoint trait, implemented by all endpoint plugins
pub trait Endpoint {
/// Send a documentation
@@ -111,9 +135,6 @@ pub trait Endpoint {
/// The name/identifier for this endpoint
fn name(&self) -> &str;
-
- /// The name of the filter to use
- fn filter(&self) -> Option<&str>;
}
#[derive(Debug, Clone)]
@@ -130,12 +151,20 @@ pub enum Content {
}
#[derive(Debug, Clone)]
-/// Notification which can be sent
-pub struct Notification {
+pub struct Metadata {
/// Notification severity
severity: Severity,
+ /// Additional fields for additional key-value metadata
+ additional_fields: HashMap<String, String>,
+}
+
+#[derive(Debug, Clone)]
+/// Notification which can be sent
+pub struct Notification {
/// Notification content
content: Content,
+ /// Metadata
+ metadata: Metadata,
}
impl Notification {
@@ -143,14 +172,18 @@ impl Notification {
severity: Severity,
title: S,
body: S,
- properties: Value,
+ template_data: Value,
+ fields: HashMap<String, String>,
) -> Self {
Self {
- severity,
+ metadata: Metadata {
+ severity,
+ additional_fields: fields,
+ },
content: Content::Template {
title_template: title.as_ref().to_string(),
body_template: body.as_ref().to_string(),
- data: properties,
+ data: template_data,
},
}
}
@@ -198,8 +231,7 @@ impl Config {
#[derive(Default)]
pub struct Bus {
endpoints: HashMap<String, Box<dyn Endpoint>>,
- groups: HashMap<String, GroupConfig>,
- filters: Vec<FilterConfig>,
+ matchers: Vec<MatcherConfig>,
}
#[allow(unused_macros)]
@@ -304,23 +336,14 @@ impl Bus {
);
}
- let groups: HashMap<String, GroupConfig> = config
+ let matchers = config
.config
- .convert_to_typed_array(GROUP_TYPENAME)
- .map_err(|err| Error::ConfigDeserialization(err.into()))?
- .into_iter()
- .map(|group: GroupConfig| (group.name.clone(), group))
- .collect();
-
- let filters = config
- .config
- .convert_to_typed_array(FILTER_TYPENAME)
+ .convert_to_typed_array(MATCHER_TYPENAME)
.map_err(|err| Error::ConfigDeserialization(err.into()))?;
Ok(Bus {
endpoints,
- groups,
- filters,
+ matchers,
})
}
@@ -330,77 +353,33 @@ impl Bus {
}
#[cfg(test)]
- pub fn add_group(&mut self, group: GroupConfig) {
- self.groups.insert(group.name.clone(), group);
- }
-
- #[cfg(test)]
- pub fn add_filter(&mut self, filter: FilterConfig) {
- self.filters.push(filter)
+ pub fn add_matcher(&mut self, filter: MatcherConfig) {
+ self.matchers.push(filter)
}
- /// Send a notification to a given target (endpoint or group).
+ /// Send a notification. Notification matchers will determine which targets will receive
+ /// the notification.
///
/// Any errors will not be returned but only logged.
- pub fn send(&self, endpoint_or_group: &str, notification: &Notification) {
- let mut filter_matcher = FilterMatcher::new(&self.filters, notification);
-
- if let Some(group) = self.groups.get(endpoint_or_group) {
- if !Bus::check_filter(&mut filter_matcher, group.filter.as_deref()) {
- log::info!("skipped target '{endpoint_or_group}', filter did not match");
- return;
- }
-
- log::info!("target '{endpoint_or_group}' is a group, notifying all members...");
-
- for endpoint in &group.endpoint {
- self.send_via_single_endpoint(endpoint, notification, &mut filter_matcher);
- }
- } else {
- self.send_via_single_endpoint(endpoint_or_group, notification, &mut filter_matcher);
- }
- }
+ pub fn send(&self, notification: &Notification) {
+ let targets = matcher::check_matches(self.matchers.as_slice(), notification);
- fn check_filter(filter_matcher: &mut FilterMatcher, filter: Option<&str>) -> bool {
- if let Some(filter) = filter {
- match filter_matcher.check_filter_match(filter) {
- // If the filter does not match, do nothing
- Ok(r) => r,
- Err(err) => {
- // If there is an error, only log it and still send
- log::error!("could not apply filter '{filter}': {err}");
- true
- }
- }
- } else {
- true
- }
- }
-
- fn send_via_single_endpoint(
- &self,
- endpoint: &str,
- notification: &Notification,
- filter_matcher: &mut FilterMatcher,
- ) {
- if let Some(endpoint) = self.endpoints.get(endpoint) {
- let name = endpoint.name();
- if !Bus::check_filter(filter_matcher, endpoint.filter()) {
- log::info!("skipped target '{name}', filter did not match");
- return;
- }
+ for target in targets {
+ if let Some(endpoint) = self.endpoints.get(target) {
+ let name = endpoint.name();
- match endpoint.send(notification) {
- Ok(_) => {
- log::info!("notified via target `{name}`");
- }
- Err(e) => {
- // Only log on errors, do not propagate fail to the caller.
- log::error!("could not notify via target `{name}`: {e}");
+ match endpoint.send(notification) {
+ Ok(_) => {
+ log::info!("notified via target `{name}`");
+ }
+ Err(e) => {
+ // Only log on errors, do not propagate fail to the caller.
+ log::error!("could not notify via target `{name}`: {e}");
+ }
}
+ } else {
+ log::error!("could not notify via target '{target}', it does not exist");
}
- } else {
- log::error!("could not notify via target '{endpoint}', it does not exist");
}
}
@@ -410,7 +389,11 @@ impl Bus {
/// any errors to the caller.
pub fn test_target(&self, target: &str) -> Result<(), Error> {
let notification = Notification {
- severity: Severity::Info,
+ metadata: Metadata {
+ severity: Severity::Info,
+ // TODO: what fields would make sense for test notifications?
+ additional_fields: Default::default(),
+ },
content: Content::Template {
title_template: "Test notification".into(),
body_template: "This is a test of the notification target '{{ target }}'".into(),
@@ -418,29 +401,10 @@ impl Bus {
},
};
- let mut errors: Vec<Box<dyn StdError + Send + Sync>> = Vec::new();
-
- let mut my_send = |target: &str| -> Result<(), Error> {
- if let Some(endpoint) = self.endpoints.get(target) {
- if let Err(e) = endpoint.send(¬ification) {
- errors.push(Box::new(e));
- }
- } else {
- return Err(Error::TargetDoesNotExist(target.to_string()));
- }
- Ok(())
- };
-
- if let Some(group) = self.groups.get(target) {
- for endpoint_name in &group.endpoint {
- my_send(endpoint_name)?;
- }
+ if let Some(endpoint) = self.endpoints.get(target) {
+ endpoint.send(¬ification)?;
} else {
- my_send(target)?;
- }
-
- if !errors.is_empty() {
- return Err(Error::TargetTestFailed(errors));
+ return Err(Error::TargetDoesNotExist(target.to_string()));
}
Ok(())
@@ -459,7 +423,6 @@ mod tests {
// Needs to be an Rc so that we can clone MockEndpoint before
// passing it to Bus, while still retaining a handle to the Vec
messages: Rc<RefCell<Vec<Notification>>>,
- filter: Option<String>,
}
impl Endpoint for MockEndpoint {
@@ -472,17 +435,12 @@ mod tests {
fn name(&self) -> &str {
self.name
}
-
- fn filter(&self) -> Option<&str> {
- self.filter.as_deref()
- }
}
impl MockEndpoint {
- fn new(name: &'static str, filter: Option<String>) -> Self {
+ fn new(name: &'static str) -> Self {
Self {
name,
- filter,
..Default::default()
}
}
@@ -494,113 +452,66 @@ mod tests {
#[test]
fn test_add_mock_endpoint() -> Result<(), Error> {
- let mock = MockEndpoint::new("endpoint", None);
+ let mock = MockEndpoint::new("endpoint");
let mut bus = Bus::default();
bus.add_endpoint(Box::new(mock.clone()));
- // Send directly to endpoint
- bus.send(
- "endpoint",
- &Notification::new_templated(Severity::Info, "Title", "Body", Default::default()),
- );
- let messages = mock.messages();
- assert_eq!(messages.len(), 1);
-
- Ok(())
- }
-
- #[test]
- fn test_groups() -> Result<(), Error> {
- let endpoint1 = MockEndpoint::new("mock1", None);
- let endpoint2 = MockEndpoint::new("mock2", None);
-
- let mut bus = Bus::default();
-
- bus.add_group(GroupConfig {
- name: "group1".to_string(),
- endpoint: vec!["mock1".into()],
- comment: None,
- filter: None,
- });
-
- bus.add_group(GroupConfig {
- name: "group2".to_string(),
- endpoint: vec!["mock2".into()],
- comment: None,
- filter: None,
- });
-
- bus.add_endpoint(Box::new(endpoint1.clone()));
- bus.add_endpoint(Box::new(endpoint2.clone()));
-
- let send_to_group = |channel| {
- let notification =
- Notification::new_templated(Severity::Info, "Title", "Body", Default::default());
- bus.send(channel, ¬ification)
+ let matcher = MatcherConfig {
+ target: Some(vec!["endpoint".into()]),
+ ..Default::default()
};
- send_to_group("group1");
- assert_eq!(endpoint1.messages().len(), 1);
- assert_eq!(endpoint2.messages().len(), 0);
+ bus.add_matcher(matcher);
- send_to_group("group2");
- assert_eq!(endpoint1.messages().len(), 1);
- assert_eq!(endpoint2.messages().len(), 1);
+ // Send directly to endpoint
+ bus.send(&Notification::new_templated(
+ Severity::Info,
+ "Title",
+ "Body",
+ Default::default(),
+ Default::default(),
+ ));
+ let messages = mock.messages();
+ assert_eq!(messages.len(), 1);
Ok(())
}
#[test]
- fn test_severity_ordering() {
- // Not intended to be exhaustive, just a quick
- // sanity check ;)
-
- assert!(Severity::Info < Severity::Notice);
- assert!(Severity::Info < Severity::Warning);
- assert!(Severity::Info < Severity::Error);
- assert!(Severity::Error > Severity::Warning);
- assert!(Severity::Warning > Severity::Notice);
- }
-
- #[test]
- fn test_multiple_endpoints_with_different_filters() -> Result<(), Error> {
- let endpoint1 = MockEndpoint::new("mock1", Some("filter1".into()));
- let endpoint2 = MockEndpoint::new("mock2", Some("filter2".into()));
+ fn test_multiple_endpoints_with_different_matchers() -> Result<(), Error> {
+ let endpoint1 = MockEndpoint::new("mock1");
+ let endpoint2 = MockEndpoint::new("mock2");
let mut bus = Bus::default();
bus.add_endpoint(Box::new(endpoint1.clone()));
bus.add_endpoint(Box::new(endpoint2.clone()));
- bus.add_group(GroupConfig {
- name: "channel1".to_string(),
- endpoint: vec!["mock1".into(), "mock2".into()],
- comment: None,
- filter: None,
+ bus.add_matcher(MatcherConfig {
+ name: "matcher1".into(),
+ match_severity: Some(vec!["warning,error".parse()?]),
+ target: Some(vec!["mock1".into()]),
+ ..Default::default()
});
- bus.add_filter(FilterConfig {
- name: "filter1".into(),
- min_severity: Some(Severity::Warning),
- mode: None,
- invert_match: None,
- comment: None,
- });
-
- bus.add_filter(FilterConfig {
- name: "filter2".into(),
- min_severity: Some(Severity::Error),
- mode: None,
- invert_match: None,
- comment: None,
+ bus.add_matcher(MatcherConfig {
+ name: "matcher2".into(),
+ match_severity: Some(vec!["error".parse()?]),
+ target: Some(vec!["mock2".into()]),
+ ..Default::default()
});
let send_with_severity = |severity| {
- let notification =
- Notification::new_templated(severity, "Title", "Body", Default::default());
+ let notification = Notification::new_templated(
+ severity,
+ "Title",
+ "Body",
+ Default::default(),
+ Default::default(),
+ );
- bus.send("channel1", ¬ification);
+ bus.send(¬ification);
};
send_with_severity(Severity::Info);
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
new file mode 100644
index 0000000..c24726d
--- /dev/null
+++ b/proxmox-notify/src/matcher.rs
@@ -0,0 +1,395 @@
+use regex::Regex;
+use std::collections::HashSet;
+use std::fmt;
+use std::fmt::Debug;
+use std::str::FromStr;
+
+use serde::{Deserialize, Serialize};
+
+use proxmox_schema::api_types::COMMENT_SCHEMA;
+use proxmox_schema::{
+ api, const_regex, ApiStringFormat, Schema, StringSchema, Updater, SAFE_ID_REGEX_STR,
+};
+
+use crate::schema::ENTITY_NAME_SCHEMA;
+use crate::{Error, Notification, Severity};
+
+pub const MATCHER_TYPENAME: &str = "matcher";
+
+#[api]
+#[derive(Debug, Serialize, Deserialize, Default, Clone, Copy)]
+#[serde(rename_all = "kebab-case")]
+pub enum MatchModeOperator {
+ /// All match statements have to match (AND)
+ #[default]
+ All,
+ /// At least one filter property has to match (OR)
+ Any,
+}
+
+impl MatchModeOperator {
+ /// Apply the mode operator to two bools, lhs and rhs
+ fn apply(&self, lhs: bool, rhs: bool) -> bool {
+ match self {
+ MatchModeOperator::All => lhs && rhs,
+ MatchModeOperator::Any => lhs || rhs,
+ }
+ }
+
+ // https://en.wikipedia.org/wiki/Identity_element
+ fn neutral_element(&self) -> bool {
+ match self {
+ MatchModeOperator::All => true,
+ MatchModeOperator::Any => false,
+ }
+ }
+}
+
+const_regex! {
+ pub MATCH_FIELD_ENTRY_REGEX = concat!(r"^(?:(exact|regex):)?(", SAFE_ID_REGEX_STR!(), r")=(.*)$");
+}
+
+pub const MATCH_FIELD_ENTRY_FORMAT: ApiStringFormat =
+ ApiStringFormat::VerifyFn(verify_field_matcher);
+
+fn verify_field_matcher(s: &str) -> Result<(), anyhow::Error> {
+ let _: FieldMatcher = s.parse()?;
+ Ok(())
+}
+
+pub const MATCH_FIELD_ENTRY_SCHEMA: Schema = StringSchema::new("Match metadata field.")
+ .format(&MATCH_FIELD_ENTRY_FORMAT)
+ .min_length(1)
+ .max_length(1024)
+ .schema();
+
+#[api(
+ properties: {
+ name: {
+ schema: ENTITY_NAME_SCHEMA,
+ },
+ comment: {
+ optional: true,
+ schema: COMMENT_SCHEMA,
+ },
+ "match-field": {
+ type: Array,
+ items: {
+ description: "Fields to match",
+ type: String
+ },
+ optional: true,
+ },
+ "match-severity": {
+ type: Array,
+ items: {
+ description: "Severity level to match.",
+ type: String
+ },
+ optional: true,
+ },
+ "target": {
+ type: Array,
+ items: {
+ schema: ENTITY_NAME_SCHEMA,
+ },
+ optional: true,
+ },
+ })]
+#[derive(Debug, Serialize, Deserialize, Updater, Default)]
+#[serde(rename_all = "kebab-case")]
+/// Config for Sendmail notification endpoints
+pub struct MatcherConfig {
+ /// Name of the matcher
+ #[updater(skip)]
+ pub name: String,
+
+ /// List of matched metadata fields
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub match_field: Option<Vec<FieldMatcher>>,
+
+ /// List of matched severity levels
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub match_severity: Option<Vec<SeverityMatcher>>,
+
+ /// Decide if 'all' or 'any' match statements must match
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub mode: Option<MatchModeOperator>,
+
+ /// Invert match of the whole filter
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub invert_match: Option<bool>,
+
+ /// Targets to notify
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub target: Option<Vec<String>>,
+
+ /// Comment
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub comment: Option<String>,
+}
+
+#[derive(Clone, Debug)]
+pub enum FieldMatcher {
+ Exact {
+ field: String,
+ matched_value: String,
+ },
+ Regex {
+ field: String,
+ matched_regex: Regex,
+ },
+}
+
+proxmox_serde::forward_deserialize_to_from_str!(FieldMatcher);
+proxmox_serde::forward_serialize_to_display!(FieldMatcher);
+
+impl FieldMatcher {
+ fn matches(&self, notification: &Notification) -> bool {
+ match self {
+ FieldMatcher::Exact {
+ field,
+ matched_value,
+ } => {
+ let value = notification.metadata.additional_fields.get(field);
+
+ if let Some(value) = value {
+ matched_value == value
+ } else {
+ // Metadata field does not exist, so we do not match
+ false
+ }
+ }
+ FieldMatcher::Regex {
+ field,
+ matched_regex,
+ } => {
+ let value = notification.metadata.additional_fields.get(field);
+
+ if let Some(value) = value {
+ matched_regex.is_match(value)
+ } else {
+ // Metadata field does not exist, so we do not match
+ false
+ }
+ }
+ }
+ }
+}
+
+impl fmt::Display for FieldMatcher {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ // Attention, Display is used to implement Serialize, do not
+ // change the format.
+
+ match self {
+ FieldMatcher::Exact {
+ field,
+ matched_value,
+ } => {
+ write!(f, "exact:{field}={matched_value}")
+ }
+ FieldMatcher::Regex {
+ field,
+ matched_regex,
+ } => {
+ let re = matched_regex.as_str();
+ write!(f, "regex:{field}={re}")
+ }
+ }
+ }
+}
+
+impl FromStr for FieldMatcher {
+ type Err = Error;
+ fn from_str(s: &str) -> Result<Self, Error> {
+ if !MATCH_FIELD_ENTRY_REGEX.is_match(s) {
+ return Err(Error::FilterFailed(format!(
+ "invalid match-field statement: {s}"
+ )));
+ }
+
+ if let Some(remaining) = s.strip_prefix("regex:") {
+ match remaining.split_once('=') {
+ None => Err(Error::FilterFailed(format!(
+ "invalid match-field statement: {s}"
+ ))),
+ Some((field, expected_value_regex)) => {
+ let regex = Regex::new(expected_value_regex)
+ .map_err(|err| Error::FilterFailed(format!("invalid regex: {err}")))?;
+
+ Ok(Self::Regex {
+ field: field.into(),
+ matched_regex: regex,
+ })
+ }
+ }
+ } else if let Some(remaining) = s.strip_prefix("exact:") {
+ match remaining.split_once('=') {
+ None => Err(Error::FilterFailed(format!(
+ "invalid match-field statement: {s}"
+ ))),
+ Some((field, expected_value)) => Ok(Self::Exact {
+ field: field.into(),
+ matched_value: expected_value.into(),
+ }),
+ }
+ } else {
+ Err(Error::FilterFailed(format!(
+ "invalid match-field statement: {s}"
+ )))
+ }
+ }
+}
+
+impl MatcherConfig {
+ pub fn matches(&self, notification: &Notification) -> Result<Option<&[String]>, Error> {
+ let mode = self.mode.unwrap_or_default();
+
+ let mut is_match = mode.neutral_element();
+ is_match = mode.apply(is_match, self.check_severity_match(notification));
+ is_match = mode.apply(is_match, self.check_field_match(notification)?);
+
+ let invert_match = self.invert_match.unwrap_or_default();
+
+ Ok(if is_match != invert_match {
+ Some(self.target.as_deref().unwrap_or_default())
+ } else {
+ None
+ })
+ }
+
+ fn check_field_match(&self, notification: &Notification) -> Result<bool, Error> {
+ let mode = self.mode.unwrap_or_default();
+ let mut is_match = mode.neutral_element();
+
+ if let Some(match_field) = self.match_field.as_deref() {
+ for field_matcher in match_field {
+ // let field_matcher: FieldMatcher = match_stmt.parse()?;
+ is_match = mode.apply(is_match, field_matcher.matches(notification));
+ }
+ }
+
+ Ok(is_match)
+ }
+
+ fn check_severity_match(&self, notification: &Notification) -> bool {
+ let mode = self.mode.unwrap_or_default();
+ let mut is_match = mode.neutral_element();
+
+ if let Some(matchers) = self.match_severity.as_ref() {
+ for severity_matcher in matchers {
+ is_match = mode.apply(is_match, severity_matcher.matches(notification));
+ }
+ }
+
+ is_match
+ }
+}
+#[derive(Clone, Debug)]
+pub struct SeverityMatcher {
+ severities: Vec<Severity>,
+}
+
+proxmox_serde::forward_deserialize_to_from_str!(SeverityMatcher);
+proxmox_serde::forward_serialize_to_display!(SeverityMatcher);
+
+impl SeverityMatcher {
+ fn matches(&self, notification: &Notification) -> bool {
+ self.severities.contains(¬ification.metadata.severity)
+ }
+}
+
+impl fmt::Display for SeverityMatcher {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ let severities: Vec<String> = self.severities.iter().map(|s| format!("{s}")).collect();
+ f.write_str(&severities.join(","))
+ }
+}
+
+impl FromStr for SeverityMatcher {
+ type Err = Error;
+ fn from_str(s: &str) -> Result<Self, Error> {
+ let mut severities = Vec::new();
+
+ for element in s.split(',') {
+ let element = element.trim();
+ let severity: Severity = element.parse()?;
+
+ severities.push(severity)
+ }
+
+ Ok(Self { severities })
+ }
+}
+
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum DeleteableMatcherProperty {
+ MatchSeverity,
+ MatchField,
+ Target,
+ Mode,
+ InvertMatch,
+ Comment,
+}
+
+pub fn check_matches<'a>(
+ matchers: &'a [MatcherConfig],
+ notification: &Notification,
+) -> HashSet<&'a str> {
+ let mut targets = HashSet::new();
+
+ for matcher in matchers {
+ match matcher.matches(notification) {
+ Ok(t) => {
+ let t = t.unwrap_or_default();
+ targets.extend(t.iter().map(|s| s.as_str()));
+ }
+ Err(err) => log::error!("matcher '{matcher}' failed: {err}", matcher = matcher.name),
+ }
+ }
+
+ targets
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use serde_json::Value;
+ use std::collections::HashMap;
+
+ #[test]
+ fn test_matching() {
+ let mut fields = HashMap::new();
+ fields.insert("foo".into(), "bar".into());
+
+ let notification =
+ Notification::new_templated(Severity::Notice, "test", "test", Value::Null, fields);
+
+ let matcher: FieldMatcher = "exact:foo=bar".parse().unwrap();
+ assert!(matcher.matches(¬ification));
+
+ let matcher: FieldMatcher = "regex:foo=b.*".parse().unwrap();
+ assert!(matcher.matches(¬ification));
+
+ let matcher: FieldMatcher = "regex:notthere=b.*".parse().unwrap();
+ assert!(!matcher.matches(¬ification));
+
+ assert!("regex:'3=b.*".parse::<FieldMatcher>().is_err());
+ assert!("invalid:'bar=b.*".parse::<FieldMatcher>().is_err());
+ }
+ #[test]
+ fn test_severities() {
+ let notification = Notification::new_templated(
+ Severity::Notice,
+ "test",
+ "test",
+ Value::Null,
+ Default::default(),
+ );
+
+ let matcher: SeverityMatcher = "info,notice,warning,error".parse().unwrap();
+ assert!(matcher.matches(¬ification));
+ }
+}
diff --git a/proxmox-notify/src/schema.rs b/proxmox-notify/src/schema.rs
index fc6c46c..93347a5 100644
--- a/proxmox-notify/src/schema.rs
+++ b/proxmox-notify/src/schema.rs
@@ -19,9 +19,8 @@ pub const BACKEND_NAME_SCHEMA: Schema = StringSchema::new("Notification backend
.max_length(32)
.schema();
-pub const ENTITY_NAME_SCHEMA: Schema =
- StringSchema::new("Name schema for endpoints, filters and groups")
- .format(&SAFE_ID_FORMAT)
- .min_length(2)
- .max_length(32)
- .schema();
+pub const ENTITY_NAME_SCHEMA: Schema = StringSchema::new("Name schema for targets and matchers")
+ .format(&SAFE_ID_FORMAT)
+ .min_length(2)
+ .max_length(32)
+ .schema();
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox 04/27] notify: add calendar matcher
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (2 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 03/27] notify: replace filters and groups with matcher-based system Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 05/27] notify: matcher: introduce common trait for match directives Lukas Wagner
` (23 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This allows matching by a notification's timestamp:
matcher: foo
match-calendar mon..fri 8-12
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/src/api/matcher.rs | 6 +++
proxmox-notify/src/lib.rs | 4 ++
proxmox-notify/src/matcher.rs | 65 +++++++++++++++++++++++++++++++
3 files changed, 75 insertions(+)
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
index e37b74f..0592b14 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -80,6 +80,7 @@ pub fn update_matcher(
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::Mode => matcher.mode = None,
DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None,
@@ -96,6 +97,10 @@ pub fn update_matcher(
matcher.match_field = Some(match_field.clone());
}
+ if let Some(match_calendar) = &matcher_updater.match_calendar {
+ matcher.match_calendar = Some(match_calendar.clone());
+ }
+
if let Some(mode) = matcher_updater.mode {
matcher.mode = Some(mode);
}
@@ -200,6 +205,7 @@ matcher: matcher2
mode: Some(MatchModeOperator::Any),
match_field: None,
match_severity: None,
+ match_calendar: None,
invert_match: Some(true),
target: Some(vec!["foo".into()]),
comment: Some("new comment".into()),
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 1f95ae0..9997cef 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -154,6 +154,8 @@ pub enum Content {
pub struct Metadata {
/// Notification severity
severity: Severity,
+ /// Timestamp of the notification as a UNIX epoch
+ timestamp: i64,
/// Additional fields for additional key-value metadata
additional_fields: HashMap<String, String>,
}
@@ -179,6 +181,7 @@ impl Notification {
metadata: Metadata {
severity,
additional_fields: fields,
+ timestamp: proxmox_time::epoch_i64(),
},
content: Content::Template {
title_template: title.as_ref().to_string(),
@@ -393,6 +396,7 @@ impl Bus {
severity: Severity::Info,
// TODO: what fields would make sense for test notifications?
additional_fields: Default::default(),
+ timestamp: proxmox_time::epoch_i64(),
},
content: Content::Template {
title_template: "Test notification".into(),
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index c24726d..b03d11d 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -10,6 +10,7 @@ use proxmox_schema::api_types::COMMENT_SCHEMA;
use proxmox_schema::{
api, const_regex, ApiStringFormat, Schema, StringSchema, Updater, SAFE_ID_REGEX_STR,
};
+use proxmox_time::{parse_daily_duration, DailyDuration};
use crate::schema::ENTITY_NAME_SCHEMA;
use crate::{Error, Notification, Severity};
@@ -88,6 +89,14 @@ pub const MATCH_FIELD_ENTRY_SCHEMA: Schema = StringSchema::new("Match metadata f
},
optional: true,
},
+ "match-calendar": {
+ type: Array,
+ items: {
+ description: "Time stamps to match",
+ type: String
+ },
+ optional: true,
+ },
"target": {
type: Array,
items: {
@@ -112,6 +121,10 @@ pub struct MatcherConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub match_severity: Option<Vec<SeverityMatcher>>,
+ /// List of matched severity levels
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub match_calendar: Option<Vec<CalendarMatcher>>,
+
/// Decide if 'all' or 'any' match statements must match
#[serde(skip_serializing_if = "Option::is_none")]
pub mode: Option<MatchModeOperator>,
@@ -249,6 +262,7 @@ impl MatcherConfig {
let mut is_match = mode.neutral_element();
is_match = mode.apply(is_match, self.check_severity_match(notification));
is_match = mode.apply(is_match, self.check_field_match(notification)?);
+ is_match = mode.apply(is_match, self.check_calendar_match(notification)?);
let invert_match = self.invert_match.unwrap_or_default();
@@ -285,6 +299,19 @@ impl MatcherConfig {
is_match
}
+
+ fn check_calendar_match(&self, notification: &Notification) -> Result<bool, Error> {
+ let mode = self.mode.unwrap_or_default();
+ let mut is_match = mode.neutral_element();
+
+ if let Some(matchers) = self.match_calendar.as_ref() {
+ for matcher in matchers {
+ is_match = mode.apply(is_match, matcher.matches(notification)?);
+ }
+ }
+
+ Ok(is_match)
+ }
}
#[derive(Clone, Debug)]
pub struct SeverityMatcher {
@@ -323,11 +350,49 @@ impl FromStr for SeverityMatcher {
}
}
+/// Match timestamp of the notification.
+#[derive(Clone, Debug)]
+pub struct CalendarMatcher {
+ schedule: DailyDuration,
+ original: String,
+}
+
+proxmox_serde::forward_deserialize_to_from_str!(CalendarMatcher);
+proxmox_serde::forward_serialize_to_display!(CalendarMatcher);
+
+impl CalendarMatcher {
+ fn matches(&self, notification: &Notification) -> Result<bool, Error> {
+ self.schedule
+ .time_match(notification.metadata.timestamp, false)
+ .map_err(|err| Error::Generic(format!("could not match timestamp: {err}")))
+ }
+}
+
+impl fmt::Display for CalendarMatcher {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ f.write_str(&self.original)
+ }
+}
+
+impl FromStr for CalendarMatcher {
+ type Err = Error;
+ fn from_str(s: &str) -> Result<Self, Error> {
+ let schedule = parse_daily_duration(s)
+ .map_err(|e| Error::Generic(format!("could not parse schedule: {e}")))?;
+
+ Ok(Self {
+ schedule,
+ original: s.to_string(),
+ })
+ }
+}
+
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableMatcherProperty {
MatchSeverity,
MatchField,
+ MatchCalendar,
Target,
Mode,
InvertMatch,
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox 05/27] notify: matcher: introduce common trait for match directives
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (3 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 04/27] notify: add calendar matcher Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 06/27] notify: let a matcher always match if it has no matching directives Lukas Wagner
` (22 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This allows us to make the match-checking code a bit shorter.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/src/matcher.rs | 92 +++++++++++++++++------------------
1 file changed, 45 insertions(+), 47 deletions(-)
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index b03d11d..e299fd0 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -142,6 +142,11 @@ pub struct MatcherConfig {
pub comment: Option<String>,
}
+trait MatchDirective {
+ fn matches(&self, notification: &Notification) -> Result<bool, Error>;
+}
+
+/// Check if the notification metadata fields match
#[derive(Clone, Debug)]
pub enum FieldMatcher {
Exact {
@@ -157,9 +162,9 @@ pub enum FieldMatcher {
proxmox_serde::forward_deserialize_to_from_str!(FieldMatcher);
proxmox_serde::forward_serialize_to_display!(FieldMatcher);
-impl FieldMatcher {
- fn matches(&self, notification: &Notification) -> bool {
- match self {
+impl MatchDirective for FieldMatcher {
+ fn matches(&self, notification: &Notification) -> Result<bool, Error> {
+ Ok(match self {
FieldMatcher::Exact {
field,
matched_value,
@@ -186,7 +191,7 @@ impl FieldMatcher {
false
}
}
- }
+ })
}
}
@@ -260,9 +265,22 @@ impl MatcherConfig {
let mode = self.mode.unwrap_or_default();
let mut is_match = mode.neutral_element();
- is_match = mode.apply(is_match, self.check_severity_match(notification));
- is_match = mode.apply(is_match, self.check_field_match(notification)?);
- is_match = mode.apply(is_match, self.check_calendar_match(notification)?);
+
+ if let Some(severity_matchers) = self.match_severity.as_deref() {
+ is_match = mode.apply(
+ is_match,
+ self.check_matches(notification, severity_matchers)?,
+ );
+ }
+ if let Some(field_matchers) = self.match_field.as_deref() {
+ is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?);
+ }
+ if let Some(calendar_matchers) = self.match_calendar.as_deref() {
+ is_match = mode.apply(
+ is_match,
+ self.check_matches(notification, calendar_matchers)?,
+ );
+ }
let invert_match = self.invert_match.unwrap_or_default();
@@ -273,46 +291,24 @@ impl MatcherConfig {
})
}
- fn check_field_match(&self, notification: &Notification) -> Result<bool, Error> {
- let mode = self.mode.unwrap_or_default();
- let mut is_match = mode.neutral_element();
-
- if let Some(match_field) = self.match_field.as_deref() {
- for field_matcher in match_field {
- // let field_matcher: FieldMatcher = match_stmt.parse()?;
- is_match = mode.apply(is_match, field_matcher.matches(notification));
- }
- }
-
- Ok(is_match)
- }
-
- fn check_severity_match(&self, notification: &Notification) -> bool {
+ /// Check if given `MatchDirectives` match a notification.
+ fn check_matches(
+ &self,
+ notification: &Notification,
+ matchers: &[impl MatchDirective],
+ ) -> Result<bool, Error> {
let mode = self.mode.unwrap_or_default();
let mut is_match = mode.neutral_element();
- if let Some(matchers) = self.match_severity.as_ref() {
- for severity_matcher in matchers {
- is_match = mode.apply(is_match, severity_matcher.matches(notification));
- }
- }
-
- is_match
- }
-
- fn check_calendar_match(&self, notification: &Notification) -> Result<bool, Error> {
- let mode = self.mode.unwrap_or_default();
- let mut is_match = mode.neutral_element();
-
- if let Some(matchers) = self.match_calendar.as_ref() {
- for matcher in matchers {
- is_match = mode.apply(is_match, matcher.matches(notification)?);
- }
+ for field_matcher in matchers {
+ is_match = mode.apply(is_match, field_matcher.matches(notification)?);
}
Ok(is_match)
}
}
+
+/// Match severity of the notification.
#[derive(Clone, Debug)]
pub struct SeverityMatcher {
severities: Vec<Severity>,
@@ -321,9 +317,11 @@ pub struct SeverityMatcher {
proxmox_serde::forward_deserialize_to_from_str!(SeverityMatcher);
proxmox_serde::forward_serialize_to_display!(SeverityMatcher);
-impl SeverityMatcher {
- fn matches(&self, notification: &Notification) -> bool {
- self.severities.contains(¬ification.metadata.severity)
+/// Common trait implemented by all matching directives
+impl MatchDirective for SeverityMatcher {
+ /// Check if this directive matches a given notification
+ fn matches(&self, notification: &Notification) -> Result<bool, Error> {
+ Ok(self.severities.contains(¬ification.metadata.severity))
}
}
@@ -360,7 +358,7 @@ pub struct CalendarMatcher {
proxmox_serde::forward_deserialize_to_from_str!(CalendarMatcher);
proxmox_serde::forward_serialize_to_display!(CalendarMatcher);
-impl CalendarMatcher {
+impl MatchDirective for CalendarMatcher {
fn matches(&self, notification: &Notification) -> Result<bool, Error> {
self.schedule
.time_match(notification.metadata.timestamp, false)
@@ -433,13 +431,13 @@ mod tests {
Notification::new_templated(Severity::Notice, "test", "test", Value::Null, fields);
let matcher: FieldMatcher = "exact:foo=bar".parse().unwrap();
- assert!(matcher.matches(¬ification));
+ assert!(matcher.matches(¬ification).unwrap());
let matcher: FieldMatcher = "regex:foo=b.*".parse().unwrap();
- assert!(matcher.matches(¬ification));
+ assert!(matcher.matches(¬ification).unwrap());
let matcher: FieldMatcher = "regex:notthere=b.*".parse().unwrap();
- assert!(!matcher.matches(¬ification));
+ assert!(!matcher.matches(¬ification).unwrap());
assert!("regex:'3=b.*".parse::<FieldMatcher>().is_err());
assert!("invalid:'bar=b.*".parse::<FieldMatcher>().is_err());
@@ -455,6 +453,6 @@ mod tests {
);
let matcher: SeverityMatcher = "info,notice,warning,error".parse().unwrap();
- assert!(matcher.matches(¬ification));
+ assert!(matcher.matches(¬ification).unwrap());
}
}
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox 06/27] notify: let a matcher always match if it has no matching directives
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (4 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 05/27] notify: matcher: introduce common trait for match directives Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-perl-rs 07/27] notify: adapt to new matcher-based notification routing Lukas Wagner
` (21 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This should be a bit more intuitive to users than the current
behavior, which is 'always match' for mode==all and 'never match' for
mode==any. The current behavior originates in the neutral element of
the underlying logical operation (and, or).
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/src/matcher.rs | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index e299fd0..553ca87 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -265,17 +265,22 @@ impl MatcherConfig {
let mode = self.mode.unwrap_or_default();
let mut is_match = mode.neutral_element();
+ // If there are no matching directives, the matcher will always match
+ let mut no_matchers = true;
if let Some(severity_matchers) = self.match_severity.as_deref() {
+ no_matchers = false;
is_match = mode.apply(
is_match,
self.check_matches(notification, severity_matchers)?,
);
}
if let Some(field_matchers) = self.match_field.as_deref() {
+ no_matchers = false;
is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?);
}
if let Some(calendar_matchers) = self.match_calendar.as_deref() {
+ no_matchers = false;
is_match = mode.apply(
is_match,
self.check_matches(notification, calendar_matchers)?,
@@ -284,7 +289,7 @@ impl MatcherConfig {
let invert_match = self.invert_match.unwrap_or_default();
- Ok(if is_match != invert_match {
+ Ok(if is_match != invert_match || no_matchers {
Some(self.target.as_deref().unwrap_or_default())
} else {
None
@@ -455,4 +460,25 @@ mod tests {
let matcher: SeverityMatcher = "info,notice,warning,error".parse().unwrap();
assert!(matcher.matches(¬ification).unwrap());
}
+
+ #[test]
+ fn test_empty_matcher_matches_always() {
+ let notification = Notification::new_templated(
+ Severity::Notice,
+ "test",
+ "test",
+ Value::Null,
+ Default::default(),
+ );
+
+ for mode in [MatchModeOperator::All, MatchModeOperator::Any] {
+ let config = MatcherConfig {
+ name: "matcher".to_string(),
+ mode: Some(mode),
+ ..Default::default()
+ };
+
+ assert!(config.matches(¬ification).unwrap().is_some())
+ }
+ }
}
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-perl-rs 07/27] notify: adapt to new matcher-based notification routing
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (5 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox 06/27] notify: let a matcher always match if it has no matching directives Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-cluster 08/27] notify: adapt to matcher based notification system Lukas Wagner
` (20 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
common/src/notify.rs | 167 +++++++++++++------------------------------
1 file changed, 50 insertions(+), 117 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 9f44225..4fbd705 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -1,10 +1,12 @@
#[perlmod::package(name = "Proxmox::RS::Notify")]
mod export {
+ use std::collections::HashMap;
+ use std::sync::Mutex;
+
use anyhow::{bail, Error};
- use perlmod::Value;
use serde_json::Value as JSONValue;
- use std::sync::Mutex;
+ use perlmod::Value;
use proxmox_http_error::HttpError;
use proxmox_notify::endpoints::gotify::{
DeleteableGotifyProperty, GotifyConfig, GotifyConfigUpdater, GotifyPrivateConfig,
@@ -13,10 +15,10 @@ mod export {
use proxmox_notify::endpoints::sendmail::{
DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater,
};
- use proxmox_notify::filter::{
- DeleteableFilterProperty, FilterConfig, FilterConfigUpdater, FilterModeOperator,
+ use proxmox_notify::matcher::{
+ CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
+ MatcherConfigUpdater, SeverityMatcher,
};
- use proxmox_notify::group::{DeleteableGroupProperty, GroupConfig, GroupConfigUpdater};
use proxmox_notify::{api, Config, Notification, Severity};
pub struct NotificationConfig {
@@ -87,22 +89,22 @@ mod export {
#[export(serialize_error)]
fn send(
#[try_from_ref] this: &NotificationConfig,
- channel: &str,
severity: Severity,
title: String,
body: String,
- properties: Option<JSONValue>,
+ template_data: Option<JSONValue>,
+ fields: Option<HashMap<String, String>>,
) -> Result<(), HttpError> {
let config = this.config.lock().unwrap();
-
- let notification = Notification {
+ let notification = Notification::new_templated(
severity,
title,
body,
- properties,
- };
+ template_data.unwrap_or_default(),
+ fields.unwrap_or_default(),
+ );
- api::common::send(&config, channel, ¬ification)
+ api::common::send(&config, ¬ification)
}
#[export(serialize_error)]
@@ -114,78 +116,6 @@ mod export {
api::common::test_target(&config, target)
}
- #[export(serialize_error)]
- fn get_groups(
- #[try_from_ref] this: &NotificationConfig,
- ) -> Result<Vec<GroupConfig>, HttpError> {
- let config = this.config.lock().unwrap();
- api::group::get_groups(&config)
- }
-
- #[export(serialize_error)]
- fn get_group(
- #[try_from_ref] this: &NotificationConfig,
- id: &str,
- ) -> Result<GroupConfig, HttpError> {
- let config = this.config.lock().unwrap();
- api::group::get_group(&config, id)
- }
-
- #[export(serialize_error)]
- fn add_group(
- #[try_from_ref] this: &NotificationConfig,
- name: String,
- endpoints: Vec<String>,
- comment: Option<String>,
- filter: Option<String>,
- ) -> Result<(), HttpError> {
- let mut config = this.config.lock().unwrap();
- api::group::add_group(
- &mut config,
- &GroupConfig {
- name,
- endpoint: endpoints,
- comment,
- filter,
- },
- )
- }
-
- #[export(serialize_error)]
- fn update_group(
- #[try_from_ref] this: &NotificationConfig,
- name: &str,
- endpoints: Option<Vec<String>>,
- comment: Option<String>,
- filter: Option<String>,
- delete: Option<Vec<DeleteableGroupProperty>>,
- digest: Option<&str>,
- ) -> Result<(), HttpError> {
- let mut config = this.config.lock().unwrap();
- let digest = decode_digest(digest)?;
-
- api::group::update_group(
- &mut config,
- name,
- &GroupConfigUpdater {
- endpoint: endpoints,
- comment,
- filter,
- },
- delete.as_deref(),
- digest.as_deref(),
- )
- }
-
- #[export(serialize_error)]
- fn delete_group(
- #[try_from_ref] this: &NotificationConfig,
- name: &str,
- ) -> Result<(), HttpError> {
- let mut config = this.config.lock().unwrap();
- api::group::delete_group(&mut config, name)
- }
-
#[export(serialize_error)]
fn get_sendmail_endpoints(
#[try_from_ref] this: &NotificationConfig,
@@ -213,7 +143,6 @@ mod export {
from_address: Option<String>,
author: Option<String>,
comment: Option<String>,
- filter: Option<String>,
) -> Result<(), HttpError> {
let mut config = this.config.lock().unwrap();
@@ -226,7 +155,7 @@ mod export {
from_address,
author,
comment,
- filter,
+ filter: None,
},
)
}
@@ -241,7 +170,6 @@ mod export {
from_address: Option<String>,
author: Option<String>,
comment: Option<String>,
- filter: Option<String>,
delete: Option<Vec<DeleteableSendmailProperty>>,
digest: Option<&str>,
) -> Result<(), HttpError> {
@@ -257,7 +185,6 @@ mod export {
from_address,
author,
comment,
- filter,
},
delete.as_deref(),
digest.as_deref(),
@@ -297,7 +224,6 @@ mod export {
server: String,
token: String,
comment: Option<String>,
- filter: Option<String>,
) -> Result<(), HttpError> {
let mut config = this.config.lock().unwrap();
api::gotify::add_endpoint(
@@ -306,7 +232,7 @@ mod export {
name: name.clone(),
server,
comment,
- filter,
+ filter: None,
},
&GotifyPrivateConfig { name, token },
)
@@ -320,7 +246,6 @@ mod export {
server: Option<String>,
token: Option<String>,
comment: Option<String>,
- filter: Option<String>,
delete: Option<Vec<DeleteableGotifyProperty>>,
digest: Option<&str>,
) -> Result<(), HttpError> {
@@ -330,11 +255,7 @@ mod export {
api::gotify::update_endpoint(
&mut config,
name,
- &GotifyConfigUpdater {
- server,
- comment,
- filter,
- },
+ &GotifyConfigUpdater { server, comment },
&GotifyPrivateConfigUpdater { token },
delete.as_deref(),
digest.as_deref(),
@@ -351,38 +272,44 @@ mod export {
}
#[export(serialize_error)]
- fn get_filters(
+ fn get_matchers(
#[try_from_ref] this: &NotificationConfig,
- ) -> Result<Vec<FilterConfig>, HttpError> {
+ ) -> Result<Vec<MatcherConfig>, HttpError> {
let config = this.config.lock().unwrap();
- api::filter::get_filters(&config)
+ api::matcher::get_matchers(&config)
}
#[export(serialize_error)]
- fn get_filter(
+ fn get_matcher(
#[try_from_ref] this: &NotificationConfig,
id: &str,
- ) -> Result<FilterConfig, HttpError> {
+ ) -> Result<MatcherConfig, HttpError> {
let config = this.config.lock().unwrap();
- api::filter::get_filter(&config, id)
+ api::matcher::get_matcher(&config, id)
}
#[export(serialize_error)]
#[allow(clippy::too_many_arguments)]
- fn add_filter(
+ fn add_matcher(
#[try_from_ref] this: &NotificationConfig,
name: String,
- min_severity: Option<Severity>,
- mode: Option<FilterModeOperator>,
+ target: Option<Vec<String>>,
+ match_severity: Option<Vec<SeverityMatcher>>,
+ match_field: Option<Vec<FieldMatcher>>,
+ match_calendar: Option<Vec<CalendarMatcher>>,
+ mode: Option<MatchModeOperator>,
invert_match: Option<bool>,
comment: Option<String>,
) -> Result<(), HttpError> {
let mut config = this.config.lock().unwrap();
- api::filter::add_filter(
+ api::matcher::add_matcher(
&mut config,
- &FilterConfig {
+ &MatcherConfig {
name,
- min_severity,
+ match_severity,
+ match_field,
+ match_calendar,
+ target,
mode,
invert_match,
comment,
@@ -392,24 +319,30 @@ mod export {
#[export(serialize_error)]
#[allow(clippy::too_many_arguments)]
- fn update_filter(
+ fn update_matcher(
#[try_from_ref] this: &NotificationConfig,
name: &str,
- min_severity: Option<Severity>,
- mode: Option<FilterModeOperator>,
+ target: Option<Vec<String>>,
+ match_severity: Option<Vec<SeverityMatcher>>,
+ match_field: Option<Vec<FieldMatcher>>,
+ match_calendar: Option<Vec<CalendarMatcher>>,
+ mode: Option<MatchModeOperator>,
invert_match: Option<bool>,
comment: Option<String>,
- delete: Option<Vec<DeleteableFilterProperty>>,
+ delete: Option<Vec<DeleteableMatcherProperty>>,
digest: Option<&str>,
) -> Result<(), HttpError> {
let mut config = this.config.lock().unwrap();
let digest = decode_digest(digest)?;
- api::filter::update_filter(
+ api::matcher::update_matcher(
&mut config,
name,
- &FilterConfigUpdater {
- min_severity,
+ &MatcherConfigUpdater {
+ match_severity,
+ match_field,
+ match_calendar,
+ target,
mode,
invert_match,
comment,
@@ -420,12 +353,12 @@ mod export {
}
#[export(serialize_error)]
- fn delete_filter(
+ fn delete_matcher(
#[try_from_ref] this: &NotificationConfig,
name: &str,
) -> Result<(), HttpError> {
let mut config = this.config.lock().unwrap();
- api::filter::delete_filter(&mut config, name)
+ api::matcher::delete_matcher(&mut config, name)
}
#[export]
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-cluster 08/27] notify: adapt to matcher based notification system
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (6 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-perl-rs 07/27] notify: adapt to new matcher-based notification routing Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-guest-common 09/27] vzdump: deprecate mailto/mailnotification/notification-{target, policy} Lukas Wagner
` (19 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This commit removes the target paramters from all notify calls. Also,
the default 'mail-to-root' target is not added automatically any more
- this target will be added by an dpkg hook in the future.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/PVE/Notify.pm | 101 +++++++++++++++++++++-------------------------
1 file changed, 47 insertions(+), 54 deletions(-)
diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
index 419bf6d..872eb25 100644
--- a/src/PVE/Notify.pm
+++ b/src/PVE/Notify.pm
@@ -18,8 +18,6 @@ cfs_register_file(
\&write_notification_config,
);
-my $mail_to_root_target = 'mail-to-root';
-
sub parse_notification_config {
my ($filename, $raw) = @_;
@@ -48,86 +46,81 @@ sub read_config {
my $notification_config = Proxmox::RS::Notify->parse_config($config, $priv_config);
- eval {
- # This target should always be available...
- $notification_config->add_sendmail_endpoint(
- $mail_to_root_target,
- undef,
- ['root@pam'],
- undef,
- undef,
- 'Send mail to root@pam\'s email address'
- );
- };
-
return $notification_config;
}
sub write_config {
my ($notification_config) = @_;
- eval {
- # ... but don't persist it to the config.
- # Rationale: If it is in the config, the user might think
- # that it can be changed by editing the configuration there.
- # However, since we always add it in `read_config`, any changes
- # will be implicitly overridden by the default.
-
- # If users want's to change the configuration, they are supposed to
- # create a new sendmail endpoint.
- $notification_config->delete_sendmail_endpoint($mail_to_root_target);
- };
-
my ($config, $priv_config) = $notification_config->write_config();
cfs_write_file('notifications.cfg', $config, 1);
cfs_write_file('priv/notifications.cfg', $priv_config, 1);
}
-sub default_target {
- return $mail_to_root_target;
-}
-
my $send_notification = sub {
- my ($target, $severity, $title, $message, $properties, $config) = @_;
+ my ($severity, $title, $message, $template_data, $fields, $config) = @_;
$config = read_config() if !defined($config);
- my ($module, $file, $line) = caller(1);
-
- # Augment properties with the source code location of the notify call
- my $props_with_source = {
- %$properties,
- source => {
- module => $module,
- file => $file,
- line => $line,
- }
- };
-
- $config->send($target, $severity, $title, $message, $props_with_source);
+ $config->send($severity, $title, $message, $template_data, $fields);
};
sub notify {
- my ($target, $severity, $title, $message, $properties, $config) = @_;
- $send_notification->($target, $severity, $title, $message, $properties, $config);
+ my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+ $send_notification->(
+ $severity,
+ $title,
+ $message,
+ $template_data,
+ $fields,
+ $config
+ );
}
sub info {
- my ($target, $title, $message, $properties, $config) = @_;
- $send_notification->($target, 'info', $title, $message, $properties, $config);
+ my ($title, $message, $template_data, $fields, $config) = @_;
+ $send_notification->(
+ 'info',
+ $title,
+ $message,
+ $template_data,
+ $fields,
+ $config
+ );
}
sub notice {
- my ($target, $title, $message, $properties, $config) = @_;
- $send_notification->($target, 'notice', $title, $message, $properties, $config);
+ my ($title, $message, $template_data, $fields, $config) = @_;
+ $send_notification->(
+ 'notice',
+ $title,
+ $message,
+ $template_data,
+ $fields,
+ $config
+ );
}
sub warning {
- my ($target, $title, $message, $properties, $config) = @_;
- $send_notification->($target, 'warning', $title, $message, $properties, $config);
+ my ($title, $message, $template_data, $fields, $config) = @_;
+ $send_notification->(
+ 'warning',
+ $title,
+ $message,
+ $template_data,
+ $fields,
+ $config
+ );
}
sub error {
- my ($target, $title, $message, $properties, $config) = @_;
- $send_notification->($target, 'error', $title, $message, $properties, $config);
+ my ($title, $message, $template_data, $fields, $config) = @_;
+ $send_notification->(
+ 'error',
+ $title,
+ $message,
+ $template_data,
+ $fields,
+ $config
+ );
}
sub check_may_use_target {
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-guest-common 09/27] vzdump: deprecate mailto/mailnotification/notification-{target, policy}
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (7 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-cluster 08/27] notify: adapt to matcher based notification system Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-ha-manager 10/27] env: switch to matcher-based notification system Lukas Wagner
` (18 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
The first two will be migrated to the notification system, the second
were part for the first attempt for the new notification system.
The first attempt only ever hit pvetest, so we simply tell the user
to not use the two params.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/PVE/VZDump/Common.pm | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
index be605af..b93ad86 100644
--- a/src/PVE/VZDump/Common.pm
+++ b/src/PVE/VZDump/Common.pm
@@ -175,21 +175,22 @@ my $confdesc = {
mailto => {
type => 'string',
format => 'email-or-username-list',
- description => "Comma-separated list of email addresses or users that should" .
- " receive email notifications. Has no effect if the 'notification-target' option " .
- " is set at the same time.",
+ description => "Deprecated: Use notification targets/matchers instead." .
+ " Comma-separated list of email addresses or users that should" .
+ " receive email notifications.",
optional => 1,
},
mailnotification => {
type => 'string',
- description => "Deprecated: use 'notification-policy' instead.",
+ description => "Deprecated: use notification targets/matchers instead." .
+ " Specify when to send a notification mail",
optional => 1,
enum => [ 'always', 'failure' ],
default => 'always',
},
'notification-policy' => {
type => 'string',
- description => "Specify when to send a notification",
+ description => "Deprecated: Do not use",
optional => 1,
enum => [ 'always', 'failure', 'never'],
default => 'always',
@@ -197,10 +198,7 @@ my $confdesc = {
'notification-target' => {
type => 'string',
format => 'pve-configid',
- description => "Determine the target to which notifications should be sent." .
- " Can either be a notification endpoint or a notification group." .
- " This option takes precedence over 'mailto', meaning that if both are " .
- " set, the 'mailto' option will be ignored.",
+ description => "Deprecated: Do not use",
optional => 1,
},
tmpdir => {
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-ha-manager 10/27] env: switch to matcher-based notification system
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (8 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-guest-common 09/27] vzdump: deprecate mailto/mailnotification/notification-{target, policy} Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 11/27] api: notification: remove notification groups Lukas Wagner
` (17 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/PVE/HA/Env/PVE2.pm | 10 ++--------
src/PVE/HA/NodeStatus.pm | 11 +++++++++--
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index ea9e6e4..fcb60a9 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -221,16 +221,10 @@ sub log {
}
sub send_notification {
- my ($self, $subject, $text, $properties) = @_;
+ my ($self, $subject, $text, $template_data, $metadata_fields) = @_;
eval {
- my $dc_config = PVE::Cluster::cfs_read_file('datacenter.cfg');
- my $target = $dc_config->{notify}->{'target-fencing'} // PVE::Notify::default_target();
- my $notify = $dc_config->{notify}->{fencing} // 'always';
-
- if ($notify eq 'always') {
- PVE::Notify::error($target, $subject, $text, $properties);
- }
+ PVE::Notify::error($subject, $text, $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 b264a36..e053c55 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -212,7 +212,7 @@ my $send_fence_state_email = sub {
my $haenv = $self->{haenv};
my $status = $haenv->read_manager_status();
- my $notification_properties = {
+ my $template_data = {
"status-data" => {
manager_status => $status,
node_status => $self->{status}
@@ -222,11 +222,18 @@ my $send_fence_state_email = sub {
"subject" => $subject,
};
+ my $metadata_fields = {
+ type => 'fencing',
+ hostname => $node,
+ };
+
$haenv->send_notification(
$subject_template,
$body_template,
- $notification_properties
+ $template_data,
+ $metadata_fields,
);
+
};
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 11/27] api: notification: remove notification groups
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (9 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-ha-manager 10/27] env: switch to matcher-based notification system Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 12/27] api: notification: add new matcher-based notification API Lukas Wagner
` (16 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 267 +-----------------------------
1 file changed, 4 insertions(+), 263 deletions(-)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index ec666903..b34802c8 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -121,7 +121,6 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'endpoints' },
{ name => 'filters' },
- { name => 'groups' },
{ name => 'targets' },
];
@@ -161,8 +160,7 @@ __PACKAGE__->register_method ({
name => 'get_all_targets',
path => 'targets',
method => 'GET',
- description => 'Returns a list of all entities that can be used as notification targets' .
- ' (endpoints and groups).',
+ description => 'Returns a list of all entities that can be used as notification targets.',
permissions => {
description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
. " 'Mapping.Audit' permissions on '/mapping/notification/<name>'."
@@ -180,14 +178,14 @@ __PACKAGE__->register_method ({
type => 'object',
properties => {
name => {
- description => 'Name of the endpoint/group.',
+ description => 'Name of the target.',
type => 'string',
format => 'pve-configid',
},
'type' => {
- description => 'Type of the endpoint or group.',
+ description => 'Type of the target.',
type => 'string',
- enum => [qw(sendmail gotify group)],
+ enum => [qw(sendmail gotify)],
},
'comment' => {
description => 'Comment',
@@ -221,14 +219,6 @@ __PACKAGE__->register_method ({
};
}
- for my $target (@{$config->get_groups()}) {
- push @$result, {
- name => $target->{name},
- comment => $target->{comment},
- type => 'group',
- };
- }
-
$result
};
@@ -290,255 +280,6 @@ __PACKAGE__->register_method ({
}
});
-my $group_properties = {
- name => {
- description => 'Name of the group.',
- type => 'string',
- format => 'pve-configid',
- },
- 'endpoint' => {
- type => 'array',
- items => {
- type => 'string',
- format => 'pve-configid',
- },
- description => 'List of included endpoints',
- },
- 'comment' => {
- description => 'Comment',
- type => 'string',
- optional => 1,
- },
- filter => {
- description => 'Name of the filter that should be applied.',
- type => 'string',
- format => 'pve-configid',
- optional => 1,
- },
-};
-
-__PACKAGE__->register_method ({
- name => 'get_groups',
- path => 'groups',
- method => 'GET',
- description => 'Returns a list of all groups',
- protected => 1,
- permissions => {
- description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
- . " 'Mapping.Audit' permissions on '/mapping/notification/<name>'.",
- user => 'all',
- },
- parameters => {
- additionalProperties => 0,
- properties => {},
- },
- returns => {
- type => 'array',
- items => {
- type => 'object',
- properties => $group_properties,
- },
- links => [ { rel => 'child', href => '{name}' } ],
- },
- code => sub {
- my $config = PVE::Notify::read_config();
- my $rpcenv = PVE::RPCEnvironment::get();
-
- my $entities = eval {
- $config->get_groups();
- };
- raise_api_error($@) if $@;
-
- return filter_entities_by_privs($rpcenv, $entities);
- }
-});
-
-__PACKAGE__->register_method ({
- name => 'get_group',
- path => 'groups/{name}',
- method => 'GET',
- description => 'Return a specific group',
- protected => 1,
- permissions => {
- check => ['or',
- ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
- ['perm', '/mapping/notification/{name}', ['Mapping.Audit']],
- ],
- },
- parameters => {
- additionalProperties => 0,
- properties => {
- name => {
- type => 'string',
- format => 'pve-configid',
- },
- }
- },
- returns => {
- type => 'object',
- properties => {
- %$group_properties,
- digest => get_standard_option('pve-config-digest'),
- },
- },
- code => sub {
- my ($param) = @_;
- my $name = extract_param($param, 'name');
-
- my $config = PVE::Notify::read_config();
-
- my $group = eval {
- $config->get_group($name)
- };
-
- raise_api_error($@) if $@;
- $group->{digest} = $config->digest();
-
- return $group;
- }
-});
-
-__PACKAGE__->register_method ({
- name => 'create_group',
- path => 'groups',
- protected => 1,
- method => 'POST',
- description => 'Create a new group',
- permissions => {
- check => ['perm', '/mapping/notification', ['Mapping.Modify']],
- },
- parameters => {
- additionalProperties => 0,
- properties => $group_properties,
- },
- returns => { type => 'null' },
- code => sub {
- my ($param) = @_;
-
- my $name = extract_param($param, 'name');
- my $endpoint = extract_param($param, 'endpoint');
- my $comment = extract_param($param, 'comment');
- my $filter = extract_param($param, 'filter');
-
- eval {
- PVE::Notify::lock_config(sub {
- my $config = PVE::Notify::read_config();
-
- $config->add_group(
- $name,
- $endpoint,
- $comment,
- $filter,
- );
-
- PVE::Notify::write_config($config);
- });
- };
-
- raise_api_error($@) if $@;
- return;
- }
-});
-
-__PACKAGE__->register_method ({
- name => 'update_group',
- path => 'groups/{name}',
- protected => 1,
- method => 'PUT',
- description => 'Update existing group',
- permissions => {
- check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
- },
- parameters => {
- additionalProperties => 0,
- properties => {
- %{ make_properties_optional($group_properties) },
- delete => {
- type => 'array',
- items => {
- type => 'string',
- format => 'pve-configid',
- },
- optional => 1,
- description => 'A list of settings you want to delete.',
- },
- digest => get_standard_option('pve-config-digest'),
- },
- },
- returns => { type => 'null' },
- code => sub {
- my ($param) = @_;
-
- my $name = extract_param($param, 'name');
- my $endpoint = extract_param($param, 'endpoint');
- my $comment = extract_param($param, 'comment');
- my $filter = extract_param($param, 'filter');
- my $digest = extract_param($param, 'digest');
- my $delete = extract_param($param, 'delete');
-
- eval {
- PVE::Notify::lock_config(sub {
- my $config = PVE::Notify::read_config();
-
- $config->update_group(
- $name,
- $endpoint,
- $comment,
- $filter,
- $delete,
- $digest,
- );
-
- PVE::Notify::write_config($config);
- });
- };
-
- raise_api_error($@) if $@;
- return;
- }
-});
-
-__PACKAGE__->register_method ({
- name => 'delete_group',
- protected => 1,
- path => 'groups/{name}',
- method => 'DELETE',
- description => 'Remove group',
- permissions => {
- check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
- },
- parameters => {
- additionalProperties => 0,
- properties => {
- name => {
- type => 'string',
- format => 'pve-configid',
- },
- }
- },
- returns => { type => 'null' },
- code => sub {
- my ($param) = @_;
- my $name = extract_param($param, 'name');
-
- my $used_by = target_used_by($name);
- if ($used_by) {
- raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"});
- }
-
- eval {
- PVE::Notify::lock_config(sub {
- my $config = PVE::Notify::read_config();
- $config->delete_group($name);
- PVE::Notify::write_config($config);
- });
- };
-
- raise_api_error($@) if $@;
- return;
- }
-});
-
my $sendmail_properties = {
name => {
description => 'The name of the endpoint.',
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 12/27] api: notification: add new matcher-based notification API
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (10 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 11/27] api: notification: remove notification groups Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 13/27] ui: dc: remove unneeded notification events panel Lukas Wagner
` (15 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This renames filters -> matchers and adds new configuration options
needed by matchers (e.g. match-field, match-calendar, etc.)
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 195 ++++++++++++++----------------
1 file changed, 88 insertions(+), 107 deletions(-)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index b34802c8..8f716f26 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -68,37 +68,12 @@ sub filter_entities_by_privs {
"/mapping/notification/$_->{name}",
$can_see_mapping_privs,
1
- ) || $_->{name} eq PVE::Notify::default_target();
+ );
} @$entities];
return $filtered;
}
-sub target_used_by {
- my ($target) = @_;
-
- my $used_by = [];
-
- # Check keys in datacenter.cfg
- my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
- for my $key (qw(target-package-updates target-replication target-fencing)) {
- if ($dc_conf->{notify} && $dc_conf->{notify}->{$key} eq $target) {
- push @$used_by, $key;
- }
- }
-
- # Check backup jobs
- my $jobs_conf = PVE::Cluster::cfs_read_file('jobs.cfg');
- for my $key (keys %{$jobs_conf->{ids}}) {
- my $job = $jobs_conf->{ids}->{$key};
- if ($job->{'notification-target'} eq $target) {
- push @$used_by, $key;
- }
- }
-
- return join(', ', @$used_by);
-}
-
__PACKAGE__->register_method ({
name => 'index',
path => '',
@@ -120,7 +95,7 @@ __PACKAGE__->register_method ({
code => sub {
my $result = [
{ name => 'endpoints' },
- { name => 'filters' },
+ { name => 'matchers' },
{ name => 'targets' },
];
@@ -259,15 +234,11 @@ __PACKAGE__->register_method ({
my $privs = ['Mapping.Modify', 'Mapping.Use', 'Mapping.Audit'];
- if ($name ne PVE::Notify::default_target()) {
- # Due to backwards compatibility reasons the 'mail-to-root'
- # target must be accessible for any user
- $rpcenv->check_any(
- $authuser,
- "/mapping/notification/$name",
- $privs,
- );
- }
+ $rpcenv->check_any(
+ $authuser,
+ "/mapping/notification/$name",
+ $privs,
+ );
eval {
my $config = PVE::Notify::read_config();
@@ -319,12 +290,6 @@ my $sendmail_properties = {
type => 'string',
optional => 1,
},
- filter => {
- description => 'Name of the filter that should be applied.',
- type => 'string',
- format => 'pve-configid',
- optional => 1,
- },
};
__PACKAGE__->register_method ({
@@ -431,7 +396,6 @@ __PACKAGE__->register_method ({
my $from_address = extract_param($param, 'from-address');
my $author = extract_param($param, 'author');
my $comment = extract_param($param, 'comment');
- my $filter = extract_param($param, 'filter');
eval {
PVE::Notify::lock_config(sub {
@@ -444,7 +408,6 @@ __PACKAGE__->register_method ({
$from_address,
$author,
$comment,
- $filter
);
PVE::Notify::write_config($config);
@@ -492,7 +455,6 @@ __PACKAGE__->register_method ({
my $from_address = extract_param($param, 'from-address');
my $author = extract_param($param, 'author');
my $comment = extract_param($param, 'comment');
- my $filter = extract_param($param, 'filter');
my $delete = extract_param($param, 'delete');
my $digest = extract_param($param, 'digest');
@@ -508,7 +470,6 @@ __PACKAGE__->register_method ({
$from_address,
$author,
$comment,
- $filter,
$delete,
$digest,
);
@@ -545,11 +506,6 @@ __PACKAGE__->register_method ({
my ($param) = @_;
my $name = extract_param($param, 'name');
- my $used_by = target_used_by($name);
- if ($used_by) {
- raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"});
- }
-
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
@@ -582,12 +538,6 @@ my $gotify_properties = {
type => 'string',
optional => 1,
},
- 'filter' => {
- description => 'Name of the filter that should be applied.',
- type => 'string',
- format => 'pve-configid',
- optional => 1,
- }
};
__PACKAGE__->register_method ({
@@ -692,7 +642,6 @@ __PACKAGE__->register_method ({
my $server = extract_param($param, 'server');
my $token = extract_param($param, 'token');
my $comment = extract_param($param, 'comment');
- my $filter = extract_param($param, 'filter');
eval {
PVE::Notify::lock_config(sub {
@@ -703,7 +652,6 @@ __PACKAGE__->register_method ({
$server,
$token,
$comment,
- $filter
);
PVE::Notify::write_config($config);
@@ -748,7 +696,6 @@ __PACKAGE__->register_method ({
my $server = extract_param($param, 'server');
my $token = extract_param($param, 'token');
my $comment = extract_param($param, 'comment');
- my $filter = extract_param($param, 'filter');
my $delete = extract_param($param, 'delete');
my $digest = extract_param($param, 'digest');
@@ -762,7 +709,6 @@ __PACKAGE__->register_method ({
$server,
$token,
$comment,
- $filter,
$delete,
$digest,
);
@@ -799,11 +745,6 @@ __PACKAGE__->register_method ({
my ($param) = @_;
my $name = extract_param($param, 'name');
- my $used_by = target_used_by($name);
- if ($used_by) {
- raise_param_exc({'name' => "Cannot remove $name, used by: $used_by"});
- }
-
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
@@ -817,28 +758,56 @@ __PACKAGE__->register_method ({
}
});
-my $filter_properties = {
+my $matcher_properties = {
name => {
- description => 'Name of the endpoint.',
+ description => 'Name of the matcher.',
type => 'string',
format => 'pve-configid',
},
- 'min-severity' => {
- type => 'string',
- description => 'Minimum severity to match',
+ 'match-field' => {
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ optional => 1,
+ description => 'Metadata fields to match (regex or exact match).'
+ . ' Must be in the form (regex|exact):<field>=<value>',
+ },
+ 'match-severity' => {
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ optional => 1,
+ description => 'Notification severities to match',
+ },
+ 'match-calendar' => {
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ optional => 1,
+ description => 'Match notification timestamp',
+ },
+ 'target' => {
+ type => 'array',
+ items => {
+ type => 'string',
+ format => 'pve-configid',
+ },
optional => 1,
- enum => [qw(info notice warning error)],
+ description => 'Targets to notify on match',
},
mode => {
type => 'string',
- description => "Choose between 'and' and 'or' for when multiple properties are specified",
+ description => "Choose between 'all' and 'any' for when multiple properties are specified",
optional => 1,
- enum => [qw(and or)],
- default => 'and',
+ enum => [qw(all any)],
+ default => 'all',
},
'invert-match' => {
type => 'boolean',
- description => 'Invert match of the whole filter',
+ description => 'Invert match of the whole matcher',
optional => 1,
},
'comment' => {
@@ -849,10 +818,10 @@ my $filter_properties = {
};
__PACKAGE__->register_method ({
- name => 'get_filters',
- path => 'filters',
+ name => 'get_matchers',
+ path => 'matchers',
method => 'GET',
- description => 'Returns a list of all filters',
+ description => 'Returns a list of all matchers',
protected => 1,
permissions => {
description => "Only lists entries where you have 'Mapping.Modify', 'Mapping.Use' or"
@@ -867,7 +836,7 @@ __PACKAGE__->register_method ({
type => 'array',
items => {
type => 'object',
- properties => $filter_properties,
+ properties => $matcher_properties,
},
links => [ { rel => 'child', href => '{name}' } ],
},
@@ -876,7 +845,7 @@ __PACKAGE__->register_method ({
my $rpcenv = PVE::RPCEnvironment::get();
my $entities = eval {
- $config->get_filters();
+ $config->get_matchers();
};
raise_api_error($@) if $@;
@@ -885,10 +854,10 @@ __PACKAGE__->register_method ({
});
__PACKAGE__->register_method ({
- name => 'get_filter',
- path => 'filters/{name}',
+ name => 'get_matcher',
+ path => 'matchers/{name}',
method => 'GET',
- description => 'Return a specific filter',
+ description => 'Return a specific matcher',
protected => 1,
permissions => {
check => ['or',
@@ -908,7 +877,7 @@ __PACKAGE__->register_method ({
returns => {
type => 'object',
properties => {
- %$filter_properties,
+ %$matcher_properties,
digest => get_standard_option('pve-config-digest'),
},
},
@@ -918,37 +887,40 @@ __PACKAGE__->register_method ({
my $config = PVE::Notify::read_config();
- my $filter = eval {
- $config->get_filter($name)
+ my $matcher = eval {
+ $config->get_matcher($name)
};
raise_api_error($@) if $@;
- $filter->{digest} = $config->digest();
+ $matcher->{digest} = $config->digest();
- return $filter;
+ return $matcher;
}
});
__PACKAGE__->register_method ({
- name => 'create_filter',
- path => 'filters',
+ name => 'create_matcher',
+ path => 'matchers',
protected => 1,
method => 'POST',
- description => 'Create a new filter',
+ description => 'Create a new matcher',
protected => 1,
permissions => {
check => ['perm', '/mapping/notification', ['Mapping.Modify']],
},
parameters => {
additionalProperties => 0,
- properties => $filter_properties,
+ properties => $matcher_properties,
},
returns => { type => 'null' },
code => sub {
my ($param) = @_;
my $name = extract_param($param, 'name');
- my $min_severity = extract_param($param, 'min-severity');
+ my $match_severity = extract_param($param, 'match-severity');
+ my $match_field = extract_param($param, 'match-field');
+ my $match_calendar = extract_param($param, 'match-calendar');
+ my $target = extract_param($param, 'target');
my $mode = extract_param($param, 'mode');
my $invert_match = extract_param($param, 'invert-match');
my $comment = extract_param($param, 'comment');
@@ -957,9 +929,12 @@ __PACKAGE__->register_method ({
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
- $config->add_filter(
+ $config->add_matcher(
$name,
- $min_severity,
+ $target,
+ $match_severity,
+ $match_field,
+ $match_calendar,
$mode,
$invert_match,
$comment,
@@ -975,18 +950,18 @@ __PACKAGE__->register_method ({
});
__PACKAGE__->register_method ({
- name => 'update_filter',
- path => 'filters/{name}',
+ name => 'update_matcher',
+ path => 'matchers/{name}',
protected => 1,
method => 'PUT',
- description => 'Update existing filter',
+ description => 'Update existing matcher',
permissions => {
check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
},
parameters => {
additionalProperties => 0,
properties => {
- %{ make_properties_optional($filter_properties) },
+ %{ make_properties_optional($matcher_properties) },
delete => {
type => 'array',
items => {
@@ -1004,7 +979,10 @@ __PACKAGE__->register_method ({
my ($param) = @_;
my $name = extract_param($param, 'name');
- my $min_severity = extract_param($param, 'min-severity');
+ my $match_severity = extract_param($param, 'match-severity');
+ my $match_field = extract_param($param, 'match-field');
+ my $match_calendar = extract_param($param, 'match-calendar');
+ my $target = extract_param($param, 'target');
my $mode = extract_param($param, 'mode');
my $invert_match = extract_param($param, 'invert-match');
my $comment = extract_param($param, 'comment');
@@ -1015,9 +993,12 @@ __PACKAGE__->register_method ({
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
- $config->update_filter(
+ $config->update_matcher(
$name,
- $min_severity,
+ $target,
+ $match_severity,
+ $match_field,
+ $match_calendar,
$mode,
$invert_match,
$comment,
@@ -1035,11 +1016,11 @@ __PACKAGE__->register_method ({
});
__PACKAGE__->register_method ({
- name => 'delete_filter',
+ name => 'delete_matcher',
protected => 1,
- path => 'filters/{name}',
+ path => 'matchers/{name}',
method => 'DELETE',
- description => 'Remove filter',
+ description => 'Remove matcher',
permissions => {
check => ['perm', '/mapping/notification/{name}', ['Mapping.Modify']],
},
@@ -1060,7 +1041,7 @@ __PACKAGE__->register_method ({
eval {
PVE::Notify::lock_config(sub {
my $config = PVE::Notify::read_config();
- $config->delete_filter($name);
+ $config->delete_matcher($name);
PVE::Notify::write_config($config);
});
};
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 13/27] ui: dc: remove unneeded notification events panel
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (11 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 12/27] api: notification: add new matcher-based notification API Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 14/27] vzdump: adapt to new matcher based notification system Lukas Wagner
` (14 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
The notification event settings are replaced by notification matchers,
which will combine the notification routing and filtering into a
single concept.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
www/manager6/Makefile | 4 -
www/manager6/dc/Config.js | 17 +-
www/manager6/dc/NotificationEvents.js | 276 --------------------------
3 files changed, 2 insertions(+), 295 deletions(-)
delete mode 100644 www/manager6/dc/NotificationEvents.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 57e1b48f..18baa024 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -159,7 +159,6 @@ JSSRC= \
dc/Health.js \
dc/Log.js \
dc/NodeView.js \
- dc/NotificationEvents.js \
dc/OptionView.js \
dc/PermissionView.js \
dc/PoolEdit.js \
@@ -346,6 +345,3 @@ install: pvemanagerlib.js
.PHONY: clean
clean:
rm -rf pvemanagerlib.js OnlineHelpInfo.js .lint-incremental
-
-
-
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 7d01da5f..0dea1c67 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -319,18 +319,6 @@ Ext.define('PVE.dc.Config', {
// this is being reworked, but we need to release newer manager versions already..
let notification_enabled = false;
- if (notification_enabled && caps.dc['Sys.Audit']) {
- me.items.push(
- {
- xtype: 'pveNotificationEvents',
- title: gettext('Notifications'),
- onlineHelp: 'notification_events',
- iconCls: 'fa fa-bell-o',
- itemId: 'notifications',
- },
- );
- }
-
if (notification_enabled && (
caps.mapping['Mapping.Audit'] ||
caps.mapping['Mapping.Use'] ||
@@ -340,12 +328,11 @@ Ext.define('PVE.dc.Config', {
me.items.push(
{
xtype: 'pmxNotificationConfigView',
- title: gettext('Notification Targets'),
+ title: gettext('Notifications'),
onlineHelp: 'notification_targets',
itemId: 'notification-targets',
- iconCls: 'fa fa-dot-circle-o',
+ iconCls: 'fa fa-bell-o',
baseUrl: '/cluster/notifications',
- groups: ['notifications'],
},
);
}
diff --git a/www/manager6/dc/NotificationEvents.js b/www/manager6/dc/NotificationEvents.js
deleted file mode 100644
index 18816290..00000000
--- a/www/manager6/dc/NotificationEvents.js
+++ /dev/null
@@ -1,276 +0,0 @@
-Ext.define('PVE.dc.NotificationEventsPolicySelector', {
- alias: ['widget.pveNotificationEventsPolicySelector'],
- extend: 'Proxmox.form.KVComboBox',
- deleteEmpty: false,
- value: '__default__',
-
- config: {
- warningRef: null,
- warnIfValIs: null,
- },
-
- listeners: {
- change: function(field, newValue) {
- let me = this;
- if (!me.warningRef && !me.warnIfValIs) {
- return;
- }
-
- let warningField = field.nextSibling(
- `displayfield[reference=${me.warningRef}]`,
- );
- warningField.setVisible(newValue === me.warnIfValIs);
- },
- },
-});
-
-Ext.define('PVE.dc.NotificationEventDisabledWarning', {
- alias: ['widget.pveNotificationEventDisabledWarning'],
- extend: 'Ext.form.field.Display',
- userCls: 'pmx-hint',
- hidden: true,
- value: gettext('Disabling notifications is not recommended for production systems!'),
-});
-
-Ext.define('PVE.dc.NotificationEventsTargetSelector', {
- alias: ['widget.pveNotificationEventsTargetSelector'],
- extend: 'PVE.form.NotificationTargetSelector',
- fieldLabel: gettext('Notification Target'),
- allowBlank: true,
- editable: true,
- autoSelect: false,
- deleteEmpty: false,
- emptyText: `${Proxmox.Utils.defaultText} (mail-to-root)`,
-});
-
-Ext.define('PVE.dc.NotificationEvents', {
- extend: 'Proxmox.grid.ObjectGrid',
- alias: ['widget.pveNotificationEvents'],
-
- // Taken from OptionView.js, but adapted slightly.
- // The modified version allows us to have multiple rows in the ObjectGrid
- // for the same underlying property (notify).
- // Every setting is eventually stored as a property string in the
- // notify key of datacenter.cfg.
- // When updating 'notify', all properties that were already set
- // also have to be submitted, even if they were not modified.
- // This means that we need to save the old value somewhere.
- addInputPanelRow: function(name, propertyName, text, opts) {
- let me = this;
-
- opts = opts || {};
- me.rows = me.rows || {};
-
- me.rows[name] = {
- required: true,
- defaultValue: opts.defaultValue,
- header: text,
- renderer: opts.renderer,
- name: propertyName,
- editor: {
- xtype: 'proxmoxWindowEdit',
- width: opts.width || 400,
- subject: text,
- onlineHelp: opts.onlineHelp,
- fieldDefaults: {
- labelWidth: opts.labelWidth || 150,
- },
- setValues: function(values) {
- let value = values[propertyName];
-
- if (opts.parseBeforeSet) {
- value = PVE.Parser.parsePropertyString(value);
- }
-
- Ext.Array.each(this.query('inputpanel'), function(panel) {
- panel.setValues(value);
-
- // Save the original value
- panel.originalValue = {
- ...value,
- };
- });
- },
- url: opts.url,
- items: [{
- xtype: 'inputpanel',
- onGetValues: function(values) {
- let fields = this.config.items.map(field => field.name).filter(n => n);
-
- // Restore old, unchanged values
- for (const [key, value] of Object.entries(this.originalValue)) {
- if (!fields.includes(key)) {
- values[key] = value;
- }
- }
-
- let value = {};
- if (Object.keys(values).length > 0) {
- value[propertyName] = PVE.Parser.printPropertyString(values);
- } else {
- Proxmox.Utils.assemble_field_data(value, { 'delete': propertyName });
- }
-
- return value;
- },
- items: opts.items,
- }],
- },
- };
- },
-
- initComponent: function() {
- let me = this;
-
- // Helper function for rendering the property
- // Needed since the actual value is always stored in the 'notify' property
- let render_value = (store, target_key, mode_key, default_val) => {
- let value = store.getById('notify')?.get('value') ?? {};
- let target = value[target_key] ?? 'mail-to-root';
- let template;
-
- switch (value[mode_key]) {
- case 'always':
- template = gettext('Always, notify via target \'{0}\'');
- break;
- case 'never':
- template = gettext('Never');
- break;
- case 'auto':
- template = gettext('Automatically, notify via target \'{0}\'');
- break;
- default:
- template = gettext('{1} ({2}), notify via target \'{0}\'');
- break;
- }
-
- return Ext.String.format(template, target, Proxmox.Utils.defaultText, default_val);
- };
-
- me.addInputPanelRow('fencing', 'notify', gettext('Node Fencing'), {
- renderer: (value, metaData, record, rowIndex, colIndex, store) =>
- render_value(store, 'target-fencing', 'fencing', gettext('Always')),
- url: "/api2/extjs/cluster/options",
- items: [
- {
- xtype: 'pveNotificationEventsPolicySelector',
- name: 'fencing',
- fieldLabel: gettext('Notify'),
- comboItems: [
- ['__default__', `${Proxmox.Utils.defaultText} (${gettext('Always')})`],
- ['always', gettext('Always')],
- ['never', gettext('Never')],
- ],
- warningRef: 'warning',
- warnIfValIs: 'never',
- },
- {
- xtype: 'pveNotificationEventsTargetSelector',
- name: 'target-fencing',
- },
- {
- xtype: 'pveNotificationEventDisabledWarning',
- reference: 'warning',
- },
- ],
- });
-
- me.addInputPanelRow('replication', 'notify', gettext('Replication'), {
- renderer: (value, metaData, record, rowIndex, colIndex, store) =>
- render_value(store, 'target-replication', 'replication', gettext('Always')),
- url: "/api2/extjs/cluster/options",
- items: [
- {
- xtype: 'pveNotificationEventsPolicySelector',
- name: 'replication',
- fieldLabel: gettext('Notify'),
- comboItems: [
- ['__default__', `${Proxmox.Utils.defaultText} (${gettext('Always')})`],
- ['always', gettext('Always')],
- ['never', gettext('Never')],
- ],
- warningRef: 'warning',
- warnIfValIs: 'never',
- },
- {
- xtype: 'pveNotificationEventsTargetSelector',
- name: 'target-replication',
- },
- {
- xtype: 'pveNotificationEventDisabledWarning',
- reference: 'warning',
- },
- ],
- });
-
- me.addInputPanelRow('updates', 'notify', gettext('Package Updates'), {
- renderer: (value, metaData, record, rowIndex, colIndex, store) =>
- render_value(
- store,
- 'target-package-updates',
- 'package-updates',
- gettext('Automatically'),
- ),
- url: "/api2/extjs/cluster/options",
- items: [
- {
- xtype: 'pveNotificationEventsPolicySelector',
- name: 'package-updates',
- fieldLabel: gettext('Notify'),
- comboItems: [
- [
- '__default__',
- `${Proxmox.Utils.defaultText} (${gettext('Automatically')})`,
- ],
- ['auto', gettext('Automatically')],
- ['always', gettext('Always')],
- ['never', gettext('Never')],
- ],
- warningRef: 'warning',
- warnIfValIs: 'never',
- },
- {
- xtype: 'pveNotificationEventsTargetSelector',
- name: 'target-package-updates',
- },
- {
- xtype: 'pveNotificationEventDisabledWarning',
- reference: 'warning',
- },
- ],
- });
-
- // Hack: Also load the notify property to make it accessible
- // for our render functions.
- me.rows.notify = {
- visible: false,
- };
-
- me.selModel = Ext.create('Ext.selection.RowModel', {});
-
- Ext.apply(me, {
- tbar: [{
- text: gettext('Edit'),
- xtype: 'proxmoxButton',
- disabled: true,
- handler: () => me.run_editor(),
- selModel: me.selModel,
- }],
- url: "/api2/json/cluster/options",
- editorConfig: {
- url: "/api2/extjs/cluster/options",
- },
- interval: 5000,
- cwidth1: 200,
- listeners: {
- itemdblclick: me.run_editor,
- },
- });
-
- me.callParent();
-
- me.on('activate', me.rstore.startUpdate);
- me.on('destroy', me.rstore.stopUpdate);
- me.on('deactivate', me.rstore.stopUpdate);
- },
-});
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 14/27] vzdump: adapt to new matcher based notification system
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (12 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 13/27] ui: dc: remove unneeded notification events panel Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 15/27] api: apt: adapt to matcher-based notifications Lukas Wagner
` (13 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
To ease the migration from old-style mailto/mailnotification paramters
for backup jobs, the code will add a ephemeral sendmail endpoint and
a matcher.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/VZDump.pm | 8 +-------
PVE/VZDump.pm | 40 +++++++++++++++++++---------------------
2 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
index 3886772e..f66fc740 100644
--- a/PVE/API2/VZDump.pm
+++ b/PVE/API2/VZDump.pm
@@ -44,9 +44,7 @@ __PACKAGE__->register_method ({
."'Datastore.AllocateSpace' on the backup storage. The 'tmpdir', 'dumpdir' and "
."'script' parameters are restricted to the 'root\@pam' user. The 'maxfiles' and "
."'prune-backups' settings require 'Datastore.Allocate' on the backup storage. The "
- ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. "
- ."If 'notification-target' is set, then the 'Mapping.Use' permission is needed on "
- ."'/mapping/notification/<target>'.",
+ ."'bwlimit', 'performance' and 'ionice' parameters require 'Sys.Modify' on '/'. ",
user => 'all',
},
protected => 1,
@@ -115,10 +113,6 @@ __PACKAGE__->register_method ({
$rpcenv->check($user, "/storage/$storeid", [ 'Datastore.AllocateSpace' ]);
}
- if (my $target = $param->{'notification-target'}) {
- PVE::Notify::check_may_use_target($target, $rpcenv);
- }
-
my $worker = sub {
my $upid = shift;
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 454ab494..b0574d41 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -452,20 +452,18 @@ sub send_notification {
my $opts = $self->{opts};
my $mailto = $opts->{mailto};
my $cmdline = $self->{cmdline};
- my $target = $opts->{"notification-target"};
- # Fall back to 'mailnotification' if 'notification-policy' is not set.
- # If both are set, 'notification-policy' takes precedence
- my $policy = $opts->{"notification-policy"} // $opts->{mailnotification} // 'always';
+ # Old-style notification policy. This parameter will influce
+ # if an ad-hoc notification target/matcher will be created.
+ my $policy = $opts->{"notification-policy"} //
+ $opts->{mailnotification} //
+ 'always';
- return if ($policy eq 'never');
sanitize_task_list($tasklist);
my $error_count = count_failed_tasks($tasklist);
my $failed = ($error_count || $err);
- return if (!$failed && ($policy eq 'failure'));
-
my $status_text = $failed ? 'backup failed' : 'backup successful';
if ($err) {
@@ -489,8 +487,10 @@ sub send_notification {
"See Task History for details!\n";
};
+ my $hostname = get_hostname();
+
my $notification_props = {
- "hostname" => get_hostname(),
+ "hostname" => $hostname,
"error-message" => $err,
"guest-table" => build_guest_table($tasklist),
"logs" => $text_log_part,
@@ -498,9 +498,16 @@ sub send_notification {
"total-time" => $total_time,
};
+ my $fields = {
+ type => "vzdump",
+ hostname => $hostname,
+ };
+
my $notification_config = PVE::Notify::read_config();
- if ($mailto && scalar(@$mailto)) {
+ my $legacy_sendmail = $policy eq "always" || ($policy eq "failure" && $failed);
+
+ if ($mailto && scalar(@$mailto) && $legacy_sendmail) {
# <, >, @ are not allowed in endpoint names, but that is only
# verified once the config is serialized. That means that
# we can rely on that fact that no other endpoint with this name exists.
@@ -514,29 +521,20 @@ sub send_notification {
my $endpoints = [$endpoint_name];
- # Create an anonymous group containing the sendmail endpoint and the
- # $target endpoint, if specified
- if ($target) {
- push @$endpoints, $target;
- }
-
- $target = "<group-$endpoint_name>";
- $notification_config->add_group(
- $target,
+ $notification_config->add_matcher(
+ "<matcher-$endpoint_name>",
$endpoints,
);
}
- return if (!$target);
-
my $severity = $failed ? "error" : "info";
PVE::Notify::notify(
- $target,
$severity,
$subject_template,
$body_template,
$notification_props,
+ $fields,
$notification_config
);
};
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 15/27] api: apt: adapt to matcher-based notifications
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (13 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 14/27] vzdump: adapt to new matcher based notification system Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 16/27] api: replication: adapt to matcher-based notification system Lukas Wagner
` (12 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/APT.pm | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index a213fc59..da75a4dc 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -286,8 +286,6 @@ __PACKAGE__->register_method({
description => "This is used to resynchronize the package index files from their sources (apt-get update).",
permissions => {
check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
- description => "If 'notify: target-package-updates' is set, then the user must have the "
- . "'Mapping.Use' permission on '/mapping/notification/<target>'",
},
protected => 1,
proxyto => 'node',
@@ -297,7 +295,7 @@ __PACKAGE__->register_method({
node => get_standard_option('pve-node'),
notify => {
type => 'boolean',
- description => "Send notification mail about new packages (to email address specified for user 'root\@pam').",
+ description => "Send notification about new packages.",
optional => 1,
default => 0,
},
@@ -317,16 +315,6 @@ __PACKAGE__->register_method({
my $rpcenv = PVE::RPCEnvironment::get();
my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
- my $target = $dcconf->{notify}->{'target-package-updates'} //
- PVE::Notify::default_target();
-
- if ($param->{notify} && $target ne PVE::Notify::default_target()) {
- # If we notify via anything other than the default target (mail to root),
- # then the user must have the proper permissions for the target.
- # The mail-to-root target does not require these, as otherwise
- # we would break compatibility.
- PVE::Notify::check_may_use_target($target, $rpcenv);
- }
my $authuser = $rpcenv->get_user();
@@ -392,16 +380,23 @@ __PACKAGE__->register_method({
return if !$count;
- my $properties = {
+ my $template_data = {
updates => $updates_table,
hostname => $hostname,
};
+ # Additional metadata fields that can be used in notification
+ # matchers.
+ my $metadata_fields = {
+ type => 'package-updates',
+ hostname => $hostname,
+ };
+
PVE::Notify::info(
- $target,
$updates_available_subject_template,
$updates_available_body_template,
- $properties,
+ $template_data,
+ $metadata_fields,
);
foreach my $pi (@$pkglist) {
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 16/27] api: replication: adapt to matcher-based notification system
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (14 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 15/27] api: apt: adapt to matcher-based notifications Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 17/27] debian: postinst: create notifications.cfg if it does not exist Lukas Wagner
` (11 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/Replication.pm | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index d61518ba..0dc944c9 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -129,7 +129,7 @@ my sub _handle_job_err {
# The replication job is run every 15 mins if no schedule is set.
my $schedule = $job->{schedule} // '*/15';
- my $properties = {
+ my $template_data = {
"failure-count" => $fail_count,
"last-sync" => $jobstate->{last_sync},
"next-sync" => $next_sync,
@@ -139,19 +139,18 @@ my sub _handle_job_err {
"error" => $err,
};
+ my $metadata_fields = {
+ # TODO: Add job-id?
+ type => "replication",
+ };
+
eval {
- my $dcconf = PVE::Cluster::cfs_read_file('datacenter.cfg');
- my $target = $dcconf->{notify}->{'target-replication'} // PVE::Notify::default_target();
- my $notify = $dcconf->{notify}->{'replication'} // 'always';
-
- if ($notify eq 'always') {
- PVE::Notify::error(
- $target,
- $replication_error_subject_template,
- $replication_error_body_template,
- $properties
- );
- }
+ PVE::Notify::error(
+ $replication_error_subject_template,
+ $replication_error_body_template,
+ $template_data,
+ $metadata_fields
+ );
};
warn ": $@" if $@;
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 17/27] debian: postinst: create notifications.cfg if it does not exist
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (15 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 16/27] api: replication: adapt to matcher-based notification system Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 18/27] test: fix vzdump notification test Lukas Wagner
` (10 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
We only warn on failure so that the postinst script does not fail
in case pmxcfs is not running.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
debian/postinst | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/debian/postinst b/debian/postinst
index 4c9a1f25..7dad2b1a 100755
--- a/debian/postinst
+++ b/debian/postinst
@@ -93,6 +93,32 @@ migrate_apt_auth_conf() {
fi
}
+write_notification_cfg() {
+ # Create default config:
+ # A sendmail-target that sends to root@pam, and a
+ # matcher that sends all notifications to this target
+ cat >> /etc/pve/notifications.cfg <<EOF
+sendmail: default-target
+ mailto-user root@pam
+ comment Send mails to root@pam's email address
+
+matcher: default-matcher
+ target default-target
+ comment Send all notifications to 'default-target'
+EOF
+}
+
+create_default_notification_cfg() {
+ if ! test -f /etc/pve/notifications.cfg ; then
+ echo "Creating default 'notifications.cfg' file"
+
+ # Only warn in case we cannot write to pmxcfs
+ if ! write_notification_cfg ; then
+ echo "Could not create default 'notifications.cfg' file"
+ fi
+ fi
+}
+
case "$1" in
triggered)
# We don't print a status message here, as dpkg already said
@@ -167,6 +193,8 @@ case "$1" in
set_lvm_conf
+ create_default_notification_cfg
+
if test ! -e /proxmox_install_mode; then
# modeled after code generated by dh_start
for unit in ${UNITS}; do
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 18/27] test: fix vzdump notification test
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (16 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 17/27] debian: postinst: create notifications.cfg if it does not exist Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 19/27] ui: vzdump: remove left-overs from target/policy based notifications Lukas Wagner
` (9 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
The signature of the PVE::Notify functions have changed, this commit
adapts the mocked functions so that the tests work again.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
test/vzdump_notification_test.pl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/test/vzdump_notification_test.pl b/test/vzdump_notification_test.pl
index 21c31651..631606bb 100755
--- a/test/vzdump_notification_test.pl
+++ b/test/vzdump_notification_test.pl
@@ -38,14 +38,14 @@ my $result_properties;
my $mock_notification_module = Test::MockModule->new('PVE::Notify');
my $mocked_notify = sub {
- my ($channel, $severity, $title, $text, $properties) = @_;
+ my ($severity, $title, $text, $properties, $metadata) = @_;
$result_text = $text;
$result_properties = $properties;
};
my $mocked_notify_short = sub {
- my ($channel, @rest) = @_;
- return $mocked_notify->($channel, '<some severity>', @rest);
+ my (@params) = @_;
+ return $mocked_notify->('<some severity>', @params);
};
$mock_notification_module->mock(
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 19/27] ui: vzdump: remove left-overs from target/policy based notifications
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (17 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 18/27] test: fix vzdump notification test Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 20/27] ui: dc: config: show notification panel again Lukas Wagner
` (8 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
www/manager6/dc/Backup.js | 81 ++++---------------
.../form/NotificationPolicySelector.js | 1 -
www/manager6/window/Backup.js | 35 +-------
3 files changed, 15 insertions(+), 102 deletions(-)
diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
index 0c8d2d4f..e1c76a1d 100644
--- a/www/manager6/dc/Backup.js
+++ b/www/manager6/dc/Backup.js
@@ -36,29 +36,11 @@ Ext.define('PVE.dc.BackupEdit', {
delete values.node;
}
- if (!isCreate) {
- // 'mailnotification' is deprecated in favor of 'notification-policy'
- // -> Migration to the new parameter happens in init, so we are
- // safe to remove the old parameter here.
- Proxmox.Utils.assemble_field_data(values, { 'delete': 'mailnotification' });
-
- // If sending notifications via mail, remove the current value of
- // 'notification-target'
- if (values['notification-mode'] === "mailto") {
- Proxmox.Utils.assemble_field_data(
- values,
- { 'delete': 'notification-target' },
- );
- } else {
- // and vice versa...
- Proxmox.Utils.assemble_field_data(
- values,
- { 'delete': 'mailto' },
- );
- }
- }
-
- delete values['notification-mode'];
+ // Get rid of new-old parameters for notification settings.
+ // These should only be set for those selected few who ran
+ // pve-manager from pvetest.
+ Proxmox.Utils.assemble_field_data(values, { 'delete': 'notification-policy' });
+ 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);
@@ -170,20 +152,14 @@ Ext.define('PVE.dc.BackupEdit', {
success: function(response, _options) {
let data = response.result.data;
- // 'mailnotification' is deprecated. Let's automatically
- // migrate to the compatible 'notification-policy' parameter
- if (data.mailnotification) {
- if (!data["notification-policy"]) {
- data["notification-policy"] = data.mailnotification;
- }
-
- delete data.mailnotification;
- }
-
- if (data['notification-target']) {
- data['notification-mode'] = 'notification-target';
- } else if (data.mailto) {
- data['notification-mode'] = 'mailto';
+ // Migrate 'new'-old notification-policy back to
+ // old-old mailnotification. Only should affect
+ // users who used pve-manager from pvetest.
+ // This was a remnant of notifications before the
+ // overhaul.
+ let policy = data['notification-policy'];
+ if (policy === 'always' || policy === 'failure') {
+ data.mailnotification = policy;
}
if (data.exclude) {
@@ -228,7 +204,6 @@ Ext.define('PVE.dc.BackupEdit', {
viewModel: {
data: {
selMode: 'include',
- notificationMode: 'notification-target',
},
formulas: {
@@ -327,44 +302,16 @@ Ext.define('PVE.dc.BackupEdit', {
{
xtype: 'pveEmailNotificationSelector',
fieldLabel: gettext('Notify'),
- name: 'notification-policy',
+ name: 'mailnotification',
cbind: {
value: (get) => get('isCreate') ? 'always' : '',
deleteEmpty: '{!isCreate}',
},
},
- {
- xtype: 'pveNotificationModeSelector',
- fieldLabel: gettext('Notify via'),
- name: 'notification-mode',
- bind: {
- value: '{notificationMode}',
- },
- },
- {
- xtype: 'pveNotificationTargetSelector',
- fieldLabel: gettext('Notification Target'),
- name: 'notification-target',
- allowBlank: true,
- editable: true,
- autoSelect: false,
- bind: {
- hidden: '{mailNotificationSelected}',
- disabled: '{mailNotificationSelected}',
- },
- cbind: {
- deleteEmpty: '{!isCreate}',
- },
- },
{
xtype: 'textfield',
fieldLabel: gettext('Send email to'),
name: 'mailto',
- hidden: true,
- bind: {
- hidden: '{!mailNotificationSelected}',
- disabled: '{!mailNotificationSelected}',
- },
},
{
xtype: 'pveBackupCompressionSelector',
diff --git a/www/manager6/form/NotificationPolicySelector.js b/www/manager6/form/NotificationPolicySelector.js
index 68087275..f318ea18 100644
--- a/www/manager6/form/NotificationPolicySelector.js
+++ b/www/manager6/form/NotificationPolicySelector.js
@@ -4,6 +4,5 @@ Ext.define('PVE.form.EmailNotificationSelector', {
comboItems: [
['always', gettext('Notify always')],
['failure', gettext('On failure only')],
- ['never', gettext('Notify never')],
],
});
diff --git a/www/manager6/window/Backup.js b/www/manager6/window/Backup.js
index 8e6fa77e..8d8c9ff0 100644
--- a/www/manager6/window/Backup.js
+++ b/www/manager6/window/Backup.js
@@ -30,32 +30,12 @@ Ext.define('PVE.window.Backup', {
name: 'mode',
});
- let notificationTargetSelector = Ext.create('PVE.form.NotificationTargetSelector', {
- fieldLabel: gettext('Notification target'),
- name: 'notification-target',
- emptyText: Proxmox.Utils.noneText,
- hidden: true,
- });
-
let mailtoField = Ext.create('Ext.form.field.Text', {
fieldLabel: gettext('Send email to'),
name: 'mailto',
emptyText: Proxmox.Utils.noneText,
});
- let notificationModeSelector = Ext.create('PVE.form.NotificationModeSelector', {
- fieldLabel: gettext('Notify via'),
- value: 'mailto',
- name: 'notification-mode',
- listeners: {
- change: function(f, v) {
- let mailSelected = v === 'mailto';
- notificationTargetSelector.setHidden(mailSelected);
- mailtoField.setHidden(!mailSelected);
- },
- },
- });
-
const keepNames = [
['keep-last', gettext('Keep Last')],
['keep-hourly', gettext('Keep Hourly')],
@@ -127,12 +107,6 @@ Ext.define('PVE.window.Backup', {
success: function(response, opts) {
const data = response.result.data;
- if (!initialDefaults && data['notification-mode'] !== undefined) {
- notificationModeSelector.setValue(data['notification-mode']);
- }
- if (!initialDefaults && data['notification-channel'] !== undefined) {
- notificationTargetSelector.setValue(data['notification-channel']);
- }
if (!initialDefaults && data.mailto !== undefined) {
mailtoField.setValue(data.mailto);
}
@@ -202,8 +176,6 @@ Ext.define('PVE.window.Backup', {
],
column2: [
compressionSelector,
- notificationModeSelector,
- notificationTargetSelector,
mailtoField,
removeCheckbox,
],
@@ -280,15 +252,10 @@ Ext.define('PVE.window.Backup', {
remove: values.remove,
};
- if (values.mailto && values['notification-mode'] === 'mailto') {
+ if (values.mailto) {
params.mailto = values.mailto;
}
- if (values['notification-target'] &&
- values['notification-mode'] === 'notification-target') {
- params['notification-target'] = values['notification-target'];
- }
-
if (values.compress) {
params.compress = values.compress;
}
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH pve-manager 20/27] ui: dc: config: show notification panel again
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (18 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 19/27] ui: vzdump: remove left-overs from target/policy based notifications Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 21/27] notification ui: add target selector for matcher Lukas Wagner
` (7 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Rework should be done now.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
www/manager6/dc/Config.js | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/www/manager6/dc/Config.js b/www/manager6/dc/Config.js
index 0dea1c67..74a84e91 100644
--- a/www/manager6/dc/Config.js
+++ b/www/manager6/dc/Config.js
@@ -317,14 +317,9 @@ Ext.define('PVE.dc.Config', {
);
}
- // this is being reworked, but we need to release newer manager versions already..
- let notification_enabled = false;
- if (notification_enabled && (
- caps.mapping['Mapping.Audit'] ||
- caps.mapping['Mapping.Use'] ||
- caps.mapping['Mapping.Modify']
- )
- ) {
+ if (caps.mapping['Mapping.Audit'] ||
+ caps.mapping['Mapping.Use'] ||
+ caps.mapping['Mapping.Modify']) {
me.items.push(
{
xtype: 'pmxNotificationConfigView',
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-widget-toolkit 21/27] notification ui: add target selector for matcher
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (19 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH pve-manager 20/27] ui: dc: config: show notification panel again Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 22/27] notification ui: remove filter setting for targets Lukas Wagner
` (6 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/window/NotificationFilterEdit.js | 145 +++++++++++++++++++++++++++
1 file changed, 145 insertions(+)
diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationFilterEdit.js
index 703a9e2..bcde4fa 100644
--- a/src/window/NotificationFilterEdit.js
+++ b/src/window/NotificationFilterEdit.js
@@ -49,6 +49,11 @@ Ext.define('Proxmox.panel.NotificationFilterEditPanel', {
deleteDefaultValue: '{!isCreate}',
},
},
+ {
+ xtype: 'pmxNotificationTargetSelector',
+ name: 'target',
+ allowBlank: false,
+ },
{
xtype: 'proxmoxtextfield',
name: 'comment',
@@ -107,3 +112,143 @@ Ext.define('Proxmox.window.NotificationFilterEdit', {
}
},
});
+
+Ext.define('Proxmox.form.NotificationTargetSelector', {
+ extend: 'Ext.grid.Panel',
+ alias: 'widget.pmxNotificationTargetSelector',
+
+ mixins: {
+ field: 'Ext.form.field.Field',
+ },
+
+ padding: '0 0 10 0',
+
+ allowBlank: true,
+ selectAll: false,
+ isFormField: true,
+
+ store: {
+ autoLoad: true,
+ model: 'proxmox-notification-endpoints',
+ sorters: 'name',
+ },
+
+ columns: [
+ {
+ header: gettext('Target Name'),
+ dataIndex: 'name',
+ flex: 1,
+ },
+ {
+ header: gettext('Type'),
+ dataIndex: 'type',
+ flex: 1,
+ },
+ {
+ header: gettext('Comment'),
+ dataIndex: 'comment',
+ flex: 3,
+ },
+ ],
+
+ selModel: {
+ selType: 'checkboxmodel',
+ mode: 'SIMPLE',
+ },
+
+ checkChangeEvents: [
+ 'selectionchange',
+ 'change',
+ ],
+
+ listeners: {
+ selectionchange: function() {
+ // to trigger validity and error checks
+ this.checkChange();
+ },
+ },
+
+ getSubmitData: function() {
+ let me = this;
+ let res = {};
+ res[me.name] = me.getValue();
+ return res;
+ },
+
+ getValue: function() {
+ let me = this;
+ if (me.savedValue !== undefined) {
+ return me.savedValue;
+ }
+ let sm = me.getSelectionModel();
+ return (sm.getSelection() ?? []).map(item => item.data.name);
+ },
+
+ setValueSelection: function(value) {
+ let me = this;
+
+ let store = me.getStore();
+
+ let notFound = [];
+ let selection = value.map(item => {
+ let found = store.findRecord('name', item, 0, false, true, true);
+ if (!found) {
+ notFound.push(item);
+ }
+ return found;
+ }).filter(r => r);
+
+ for (const name of notFound) {
+ let rec = store.add({
+ name,
+ type: '-',
+ comment: gettext('Included target does not exist!'),
+ });
+ selection.push(rec[0]);
+ }
+
+ let sm = me.getSelectionModel();
+ if (selection.length) {
+ sm.select(selection);
+ } else {
+ sm.deselectAll();
+ }
+ // to correctly trigger invalid class
+ me.getErrors();
+ },
+
+ setValue: function(value) {
+ let me = this;
+
+ let store = me.getStore();
+ if (!store.isLoaded()) {
+ me.savedValue = value;
+ store.on('load', function() {
+ me.setValueSelection(value);
+ delete me.savedValue;
+ }, { single: true });
+ } else {
+ me.setValueSelection(value);
+ }
+ return me.mixins.field.setValue.call(me, value);
+ },
+
+ getErrors: function(value) {
+ let me = this;
+ if (!me.isDisabled() && me.allowBlank === false &&
+ me.getSelectionModel().getCount() === 0) {
+ me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+ return [gettext('No target selected')];
+ }
+
+ me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
+ return [];
+ },
+
+ initComponent: function() {
+ let me = this;
+ me.callParent();
+ me.initField();
+ },
+
+});
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-widget-toolkit 22/27] notification ui: remove filter setting for targets
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (20 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 21/27] notification ui: add target selector for matcher Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 23/27] notification ui: remove notification groups Lukas Wagner
` (5 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/Makefile | 1 -
src/form/NotificationFilterSelector.js | 58 -------------------------
src/panel/GotifyEditPanel.js | 9 ----
src/panel/NotificationGroupEditPanel.js | 9 ----
src/panel/SendmailEditPanel.js | 9 ----
5 files changed, 86 deletions(-)
delete mode 100644 src/form/NotificationFilterSelector.js
diff --git a/src/Makefile b/src/Makefile
index 21fbe76..85ecea4 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -44,7 +44,6 @@ JSSRC= \
form/RoleSelector.js \
form/DiskSelector.js \
form/MultiDiskSelector.js \
- form/NotificationFilterSelector.js \
form/TaskTypeSelector.js \
form/ACME.js \
form/UserSelector.js \
diff --git a/src/form/NotificationFilterSelector.js b/src/form/NotificationFilterSelector.js
deleted file mode 100644
index d2ab8be..0000000
--- a/src/form/NotificationFilterSelector.js
+++ /dev/null
@@ -1,58 +0,0 @@
-Ext.define('Proxmox.form.NotificationFilterSelector', {
- extend: 'Proxmox.form.ComboGrid',
- alias: ['widget.pmxNotificationFilterSelector'],
-
- // set default value to empty array, else it inits it with
- // null and after the store load it is an empty array,
- // triggering dirtychange
- value: [],
- valueField: 'name',
- displayField: 'name',
- deleteEmpty: true,
- skipEmptyText: true,
- allowBlank: true,
- editable: false,
- autoSelect: false,
-
- listConfig: {
- columns: [
- {
- header: gettext('Filter'),
- dataIndex: 'name',
- sortable: true,
- hideable: false,
- flex: 1,
- },
- {
- header: gettext('Comment'),
- dataIndex: 'comment',
- sortable: true,
- hideable: false,
- flex: 2,
- },
- ],
- },
-
- initComponent: function() {
- let me = this;
-
- Ext.apply(me, {
- store: {
- fields: ['name', 'comment'],
- proxy: {
- type: 'proxmox',
- url: `/api2/json/${me.baseUrl}/filters`,
- },
- sorters: [
- {
- property: 'name',
- direction: 'ASC',
- },
- ],
- autoLoad: true,
- },
- });
-
- me.callParent();
- },
-});
diff --git a/src/panel/GotifyEditPanel.js b/src/panel/GotifyEditPanel.js
index 3ddcc4d..5d814e5 100644
--- a/src/panel/GotifyEditPanel.js
+++ b/src/panel/GotifyEditPanel.js
@@ -32,15 +32,6 @@ Ext.define('Proxmox.panel.GotifyEditPanel', {
allowBlank: '{!isCreate}',
},
},
- {
- xtype: 'pmxNotificationFilterSelector',
- name: 'filter',
- fieldLabel: gettext('Filter'),
- cbind: {
- deleteEmpty: '{!isCreate}',
- baseUrl: '{baseUrl}',
- },
- },
{
xtype: 'proxmoxtextfield',
name: 'comment',
diff --git a/src/panel/NotificationGroupEditPanel.js b/src/panel/NotificationGroupEditPanel.js
index aa76810..910d15a 100644
--- a/src/panel/NotificationGroupEditPanel.js
+++ b/src/panel/NotificationGroupEditPanel.js
@@ -21,15 +21,6 @@ Ext.define('Proxmox.panel.NotificationGroupEditPanel', {
name: 'endpoint',
allowBlank: false,
},
- {
- xtype: 'pmxNotificationFilterSelector',
- name: 'filter',
- fieldLabel: gettext('Filter'),
- cbind: {
- deleteEmpty: '{!isCreate}',
- baseUrl: '{baseUrl}',
- },
- },
{
xtype: 'proxmoxtextfield',
name: 'comment',
diff --git a/src/panel/SendmailEditPanel.js b/src/panel/SendmailEditPanel.js
index ace6129..16abebc 100644
--- a/src/panel/SendmailEditPanel.js
+++ b/src/panel/SendmailEditPanel.js
@@ -86,15 +86,6 @@ Ext.define('Proxmox.panel.SendmailEditPanel', {
return this.up('pmxSendmailEditPanel').mailValidator();
},
},
- {
- xtype: 'pmxNotificationFilterSelector',
- name: 'filter',
- fieldLabel: gettext('Filter'),
- cbind: {
- deleteEmpty: '{!isCreate}',
- baseUrl: '{baseUrl}',
- },
- },
{
xtype: 'proxmoxtextfield',
name: 'comment',
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-widget-toolkit 23/27] notification ui: remove notification groups
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (21 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 22/27] notification ui: remove filter setting for targets Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 24/27] notification ui: rename filter to matcher Lukas Wagner
` (4 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/Makefile | 1 -
src/Schema.js | 5 -
src/panel/NotificationConfigView.js | 4 -
src/panel/NotificationGroupEditPanel.js | 174 ------------------------
4 files changed, 184 deletions(-)
delete mode 100644 src/panel/NotificationGroupEditPanel.js
diff --git a/src/Makefile b/src/Makefile
index 85ecea4..e07f17c 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -61,7 +61,6 @@ JSSRC= \
panel/LogView.js \
panel/NodeInfoRepoStatus.js \
panel/NotificationConfigView.js \
- panel/NotificationGroupEditPanel.js \
panel/JournalView.js \
panel/PermissionView.js \
panel/PruneKeepPanel.js \
diff --git a/src/Schema.js b/src/Schema.js
index a7ffdf8..37ecd88 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -48,11 +48,6 @@ Ext.define('Proxmox.Schema', { // a singleton
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
- group: {
- name: gettext('Notification Group'),
- ipanel: 'pmxNotificationGroupEditPanel',
- iconCls: 'fa-bell-o',
- },
},
pxarFileTypes: {
diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js
index ff9c512..ba98395 100644
--- a/src/panel/NotificationConfigView.js
+++ b/src/panel/NotificationConfigView.js
@@ -191,10 +191,6 @@ Ext.define('Proxmox.panel.NotificationEndpointView', {
callback: 'reload',
enableFn: rec => rec.data.name !== 'mail-to-root',
getUrl: function(rec) {
- if (rec.data.type === 'group') {
- return `${me.baseUrl}/groups/${rec.getId()}`;
- }
-
return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`;
},
},
diff --git a/src/panel/NotificationGroupEditPanel.js b/src/panel/NotificationGroupEditPanel.js
deleted file mode 100644
index 910d15a..0000000
--- a/src/panel/NotificationGroupEditPanel.js
+++ /dev/null
@@ -1,174 +0,0 @@
-Ext.define('Proxmox.panel.NotificationGroupEditPanel', {
- extend: 'Proxmox.panel.InputPanel',
- xtype: 'pmxNotificationGroupEditPanel',
- mixins: ['Proxmox.Mixin.CBind'],
-
- type: 'group',
-
- items: [
- {
- xtype: 'pmxDisplayEditField',
- name: 'name',
- cbind: {
- value: '{name}',
- editable: '{isCreate}',
- },
- fieldLabel: gettext('Group Name'),
- allowBlank: false,
- },
- {
- xtype: 'pmxNotificationEndpointSelector',
- name: 'endpoint',
- allowBlank: false,
- },
- {
- xtype: 'proxmoxtextfield',
- name: 'comment',
- fieldLabel: gettext('Comment'),
- cbind: {
- deleteEmpty: '{!isCreate}',
- },
- },
- ],
-});
-
-Ext.define('Proxmox.form.NotificationEndpointSelector', {
- extend: 'Ext.grid.Panel',
- alias: 'widget.pmxNotificationEndpointSelector',
-
- mixins: {
- field: 'Ext.form.field.Field',
- },
-
- padding: '0 0 10 0',
-
- allowBlank: true,
- selectAll: false,
- isFormField: true,
-
- store: {
- autoLoad: true,
- model: 'proxmox-notification-endpoints',
- sorters: 'name',
- filters: item => item.data.type !== 'group',
- },
-
- columns: [
- {
- header: gettext('Endpoint Name'),
- dataIndex: 'name',
- flex: 1,
- },
- {
- header: gettext('Type'),
- dataIndex: 'type',
- flex: 1,
- },
- {
- header: gettext('Comment'),
- dataIndex: 'comment',
- flex: 3,
- },
- ],
-
- selModel: {
- selType: 'checkboxmodel',
- mode: 'SIMPLE',
- },
-
- checkChangeEvents: [
- 'selectionchange',
- 'change',
- ],
-
- listeners: {
- selectionchange: function() {
- // to trigger validity and error checks
- this.checkChange();
- },
- },
-
- getSubmitData: function() {
- let me = this;
- let res = {};
- res[me.name] = me.getValue();
- return res;
- },
-
- getValue: function() {
- let me = this;
- if (me.savedValue !== undefined) {
- return me.savedValue;
- }
- let sm = me.getSelectionModel();
- return (sm.getSelection() ?? []).map(item => item.data.name);
- },
-
- setValueSelection: function(value) {
- let me = this;
-
- let store = me.getStore();
-
- let notFound = [];
- let selection = value.map(item => {
- let found = store.findRecord('name', item, 0, false, true, true);
- if (!found) {
- notFound.push(item);
- }
- return found;
- }).filter(r => r);
-
- for (const name of notFound) {
- let rec = store.add({
- name,
- type: '-',
- comment: gettext('Included endpoint does not exist!'),
- });
- selection.push(rec[0]);
- }
-
- let sm = me.getSelectionModel();
- if (selection.length) {
- sm.select(selection);
- } else {
- sm.deselectAll();
- }
- // to correctly trigger invalid class
- me.getErrors();
- },
-
- setValue: function(value) {
- let me = this;
-
- let store = me.getStore();
- if (!store.isLoaded()) {
- me.savedValue = value;
- store.on('load', function() {
- me.setValueSelection(value);
- delete me.savedValue;
- }, { single: true });
- } else {
- me.setValueSelection(value);
- }
- return me.mixins.field.setValue.call(me, value);
- },
-
- getErrors: function(value) {
- let me = this;
- if (!me.isDisabled() && me.allowBlank === false &&
- me.getSelectionModel().getCount() === 0) {
- me.addBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
- return [gettext('No endpoint selected')];
- }
-
- me.removeBodyCls(['x-form-trigger-wrap-default', 'x-form-trigger-wrap-invalid']);
- return [];
- },
-
- initComponent: function() {
- let me = this;
- me.callParent();
- me.initField();
- },
-
-});
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-widget-toolkit 24/27] notification ui: rename filter to matcher
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (22 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 23/27] notification ui: remove notification groups Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing Lukas Wagner
` (3 subsequent siblings)
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/Makefile | 2 +-
src/data/model/NotificationConfig.js | 2 +-
src/panel/NotificationConfigView.js | 26 +++++++++----------
...lterEdit.js => NotificationMatcherEdit.js} | 14 +++++-----
4 files changed, 22 insertions(+), 22 deletions(-)
rename src/window/{NotificationFilterEdit.js => NotificationMatcherEdit.js} (92%)
diff --git a/src/Makefile b/src/Makefile
index e07f17c..c6d31c3 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -88,7 +88,7 @@ JSSRC= \
window/ACMEPluginEdit.js \
window/ACMEDomains.js \
window/EndpointEditBase.js \
- window/NotificationFilterEdit.js \
+ window/NotificationMatcherEdit.js \
window/FileBrowser.js \
window/AuthEditBase.js \
window/AuthEditOpenId.js \
diff --git a/src/data/model/NotificationConfig.js b/src/data/model/NotificationConfig.js
index bb4ef85..f447db4 100644
--- a/src/data/model/NotificationConfig.js
+++ b/src/data/model/NotificationConfig.js
@@ -7,7 +7,7 @@ Ext.define('proxmox-notification-endpoints', {
idProperty: 'name',
});
-Ext.define('proxmox-notification-filters', {
+Ext.define('proxmox-notification-matchers', {
extend: 'Ext.data.Model',
fields: ['name', 'comment'],
proxy: {
diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js
index ba98395..ecf764d 100644
--- a/src/panel/NotificationConfigView.js
+++ b/src/panel/NotificationConfigView.js
@@ -21,7 +21,7 @@ Ext.define('Proxmox.panel.NotificationConfigView', {
border: false,
collapsible: true,
animCollapse: false,
- xtype: 'pmxNotificationFilterView',
+ xtype: 'pmxNotificationMatcherView',
cbind: {
baseUrl: '{baseUrl}',
},
@@ -209,21 +209,21 @@ Ext.define('Proxmox.panel.NotificationEndpointView', {
},
});
-Ext.define('Proxmox.panel.NotificationFilterView', {
+Ext.define('Proxmox.panel.NotificationMatcherView', {
extend: 'Ext.grid.Panel',
- alias: 'widget.pmxNotificationFilterView',
+ alias: 'widget.pmxNotificationMatcherView',
- title: gettext('Notification Filters'),
+ title: gettext('Notification Matchers'),
controller: {
xclass: 'Ext.app.ViewController',
- openEditWindow: function(filter) {
+ openEditWindow: function(matcher) {
let me = this;
- Ext.create('Proxmox.window.NotificationFilterEdit', {
+ Ext.create('Proxmox.window.NotificationMatcherEdit', {
baseUrl: me.getView().baseUrl,
- name: filter,
+ name: matcher,
autoShow: true,
listeners: {
destroy: () => me.reload(),
@@ -253,12 +253,12 @@ Ext.define('Proxmox.panel.NotificationFilterView', {
activate: 'reload',
},
- emptyText: gettext('No notification filters configured'),
+ emptyText: gettext('No notification matchers configured'),
columns: [
{
dataIndex: 'name',
- text: gettext('Filter Name'),
+ text: gettext('Matcher Name'),
renderer: Ext.String.htmlEncode,
flex: 1,
},
@@ -276,8 +276,8 @@ Ext.define('Proxmox.panel.NotificationFilterView', {
autoDestroyRstore: true,
rstore: {
type: 'update',
- storeid: 'proxmox-notification-filters',
- model: 'proxmox-notification-filters',
+ storeid: 'proxmox-notification-matchers',
+ model: 'proxmox-notification-matchers',
autoStart: true,
},
sorters: 'name',
@@ -307,12 +307,12 @@ Ext.define('Proxmox.panel.NotificationFilterView', {
{
xtype: 'proxmoxStdRemoveButton',
callback: 'reload',
- baseurl: `${me.baseUrl}/filters`,
+ baseurl: `${me.baseUrl}/matchers`,
},
],
});
me.callParent();
- me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/filters`);
+ me.store.rstore.proxy.setUrl(`/api2/json/${me.baseUrl}/matchers`);
},
});
diff --git a/src/window/NotificationFilterEdit.js b/src/window/NotificationMatcherEdit.js
similarity index 92%
rename from src/window/NotificationFilterEdit.js
rename to src/window/NotificationMatcherEdit.js
index bcde4fa..a014f3e 100644
--- a/src/window/NotificationFilterEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -1,6 +1,6 @@
-Ext.define('Proxmox.panel.NotificationFilterEditPanel', {
+Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
extend: 'Proxmox.panel.InputPanel',
- xtype: 'pmxNotificationFilterEditPanel',
+ xtype: 'pmxNotificationMatcherEditPanel',
mixins: ['Proxmox.Mixin.CBind'],
items: [
@@ -11,7 +11,7 @@ Ext.define('Proxmox.panel.NotificationFilterEditPanel', {
value: '{name}',
editable: '{isCreate}',
},
- fieldLabel: gettext('Filter Name'),
+ fieldLabel: gettext('Matcher Name'),
allowBlank: false,
},
{
@@ -65,7 +65,7 @@ Ext.define('Proxmox.panel.NotificationFilterEditPanel', {
],
});
-Ext.define('Proxmox.window.NotificationFilterEdit', {
+Ext.define('Proxmox.window.NotificationMatcherEdit', {
extend: 'Proxmox.window.Edit',
isAdd: true,
@@ -85,7 +85,7 @@ Ext.define('Proxmox.window.NotificationFilterEdit', {
throw "baseUrl not set";
}
- me.url = `/api2/extjs${me.baseUrl}/filters`;
+ me.url = `/api2/extjs${me.baseUrl}/matchers`;
if (me.isCreate) {
me.method = 'POST';
@@ -94,12 +94,12 @@ Ext.define('Proxmox.window.NotificationFilterEdit', {
me.method = 'PUT';
}
- me.subject = gettext('Notification Filter');
+ me.subject = gettext('Notification Matcher');
Ext.apply(me, {
items: [{
name: me.name,
- xtype: 'pmxNotificationFilterEditPanel',
+ xtype: 'pmxNotificationMatcherEditPanel',
isCreate: me.isCreate,
baseUrl: me.baseUrl,
}],
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (23 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 24/27] notification ui: rename filter to matcher Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-13 15:13 ` Dominik Csapak
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 26/27] notification ui: unprotected mailto-root target Lukas Wagner
` (2 subsequent siblings)
27 siblings, 1 reply; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
This modifies the old filter edit window in the following ways:
- Split content into multiple panels
- Name and comment in the first tab
- Match rules in a tree-structure in the second tab
- Targets to notify in the third tab
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Notes:
The code binding the match rule tree structure to the editable fields
could definitely be a bit cleaner. I think this is the first time that
we have used such a pattern, so there there was much experimentation
needed to get this working.
I plan to revisit it and clean up a bit later, I wanted to get
the notification system changes on the list ASAP.
src/window/NotificationMatcherEdit.js | 867 ++++++++++++++++++++++++--
1 file changed, 820 insertions(+), 47 deletions(-)
diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index a014f3e..c6f0726 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -1,6 +1,6 @@
-Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
+Ext.define('Proxmox.panel.NotificationMatcherGeneralPanel', {
extend: 'Proxmox.panel.InputPanel',
- xtype: 'pmxNotificationMatcherEditPanel',
+ xtype: 'pmxNotificationMatcherGeneralPanel',
mixins: ['Proxmox.Mixin.CBind'],
items: [
@@ -15,53 +15,27 @@ Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
allowBlank: false,
},
{
- xtype: 'proxmoxKVComboBox',
- name: 'min-severity',
- fieldLabel: gettext('Minimum Severity'),
- value: null,
+ xtype: 'proxmoxtextfield',
+ name: 'comment',
+ fieldLabel: gettext('Comment'),
cbind: {
deleteEmpty: '{!isCreate}',
},
- comboItems: [
- ['info', 'info'],
- ['notice', 'notice'],
- ['warning', 'warning'],
- ['error', 'error'],
- ],
- triggers: {
- clear: {
- cls: 'pmx-clear-trigger',
- weight: -1,
- hidden: false,
- handler: function() {
- this.setValue('');
- },
- },
- },
- },
- {
- xtype: 'proxmoxcheckbox',
- fieldLabel: gettext('Invert match'),
- name: 'invert-match',
- uncheckedValue: 0,
- defaultValue: 0,
- cbind: {
- deleteDefaultValue: '{!isCreate}',
- },
},
+ ],
+});
+
+Ext.define('Proxmox.panel.NotificationMatcherTargetPanel', {
+ extend: 'Proxmox.panel.InputPanel',
+ xtype: 'pmxNotificationMatcherTargetPanel',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ items: [
{
xtype: 'pmxNotificationTargetSelector',
name: 'target',
allowBlank: false,
},
- {
- xtype: 'proxmoxtextfield',
- name: 'comment',
- fieldLabel: gettext('Comment'),
- cbind: {
- deleteEmpty: '{!isCreate}',
- },
- },
],
});
@@ -74,7 +48,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
labelWidth: 120,
},
- width: 500,
+ width: 700,
initComponent: function() {
let me = this;
@@ -97,12 +71,38 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
me.subject = gettext('Notification Matcher');
Ext.apply(me, {
- items: [{
- name: me.name,
- xtype: 'pmxNotificationMatcherEditPanel',
- isCreate: me.isCreate,
- baseUrl: me.baseUrl,
- }],
+ bodyPadding: 0,
+ items: [
+ {
+ xtype: 'tabpanel',
+ region: 'center',
+ layout: 'fit',
+ bodyPadding: 10,
+ items: [
+ {
+ name: me.name,
+ title: gettext('General'),
+ xtype: 'pmxNotificationMatcherGeneralPanel',
+ isCreate: me.isCreate,
+ baseUrl: me.baseUrl,
+ },
+ {
+ name: me.name,
+ title: gettext('Match Rules'),
+ xtype: 'pmxNotificationMatchRulesEditPanel',
+ isCreate: me.isCreate,
+ baseUrl: me.baseUrl,
+ },
+ {
+ name: me.name,
+ title: gettext('Targets to notify'),
+ xtype: 'pmxNotificationMatcherTargetPanel',
+ isCreate: me.isCreate,
+ baseUrl: me.baseUrl,
+ },
+ ],
+ },
+ ],
});
me.callParent();
@@ -252,3 +252,776 @@ Ext.define('Proxmox.form.NotificationTargetSelector', {
},
});
+
+Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
+ extend: 'Proxmox.panel.InputPanel',
+ xtype: 'pmxNotificationMatchRulesEditPanel',
+ mixins: ['Proxmox.Mixin.CBind'],
+
+ viewModel: {
+ data: {
+ selectedRecord: null,
+ matchFieldType: 'exact',
+ matchFieldField: '',
+ matchFieldValue: '',
+ rootMode: 'all',
+ },
+
+ formulas: {
+ nodeType: {
+ get: function(get) {
+ let record = get('selectedRecord');
+ return record?.get('type');
+ },
+ set: function(value) {
+ let me = this;
+ let record = me.get('selectedRecord');
+
+ let data;
+
+ switch (value) {
+ case 'match-severity':
+ data = {
+ value: ['info', 'notice', 'warning', 'error'],
+ };
+ break;
+ case 'match-field':
+ data = {
+ type: 'exact',
+ field: '',
+ value: '',
+ };
+ break;
+ case 'match-calendar':
+ data = {
+ value: '',
+ };
+ break;
+ }
+
+ let node = {
+ type: value,
+ data,
+ };
+ record.set(node);
+ },
+ },
+ showMatchingMode: function(get) {
+ let record = get('selectedRecord');
+ if (!record) {
+ return false;
+ }
+ return record.isRoot();
+ },
+ showMatcherType: function(get) {
+ let record = get('selectedRecord');
+ if (!record) {
+ return false;
+ }
+ return !record.isRoot();
+ },
+ typeIsMatchField: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-field';
+ },
+ },
+ typeIsMatchSeverity: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ return record?.get('type') === 'match-severity';
+ },
+ },
+ typeIsMatchCalendar: {
+ bind: {
+ bindTo: '{selectedRecord}',
+ deep: true,
+ },
+ get: function(record) {
+ 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');
+ record.set({
+ data: {
+ ...currentData,
+ type: value,
+ },
+ });
+ },
+ 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,
+ },
+ });
+ },
+ 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 currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value: value,
+ },
+ });
+ },
+ get: function(record) {
+ 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}',
+ deep: true,
+ },
+ set: function(value) {
+ let me = this;
+ let record = me.get('selectedRecord');
+ let currentData = record.get('data');
+ record.set({
+ data: {
+ ...currentData,
+ value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.value;
+ },
+ },
+ invertMatch: {
+ 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,
+ invert: value,
+ },
+ });
+ },
+ get: function(record) {
+ return record?.get('data')?.invert;
+ },
+ },
+ },
+ },
+
+ column1: [
+ {
+ xtype: 'pmxNotificationMatchRuleTree',
+ cbind: {
+ isCreate: '{isCreate}',
+ },
+ },
+ ],
+ column2: [
+ {
+ xtype: 'pmxNotificationMatchRuleSettings',
+ },
+
+ ],
+
+ onGetValues: function(values) {
+ let me = this;
+
+ let deleteArrayIfEmtpy = (field) => {
+ if (Ext.isArray(values[field])) {
+ if (values[field].length === 0) {
+ delete values[field];
+ if (!me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { 'delete': field });
+ }
+ }
+ }
+ };
+ deleteArrayIfEmtpy('match-field');
+ deleteArrayIfEmtpy('match-severity');
+ deleteArrayIfEmtpy('match-calendar');
+
+ return values;
+ },
+});
+
+Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
+ extend: 'Ext.panel.Panel',
+ xtype: 'pmxNotificationMatchRuleTree',
+ mixins: ['Proxmox.Mixin.CBind'],
+ border: false,
+
+ getNodeTextAndIcon: function(type, data) {
+ let text;
+ let iconCls;
+
+ switch (type) {
+ case 'match-severity': {
+ let v = data.value.join(', ');
+ text = Ext.String.format(gettext("Match severity: {0}"), v);
+ iconCls = 'fa fa-exclamation';
+ } break;
+ case 'match-field': {
+ let field = data.field;
+ let value = data.value;
+ text = Ext.String.format(gettext("Match field: {0}={1}"), field, value);
+ iconCls = 'fa fa-cube';
+ } break;
+ case 'match-calendar': {
+ let v = data.value;
+ text = Ext.String.format(gettext("Match calendar: {0}"), v);
+ iconCls = 'fa fa-calendar-o';
+ } break;
+ case 'mode':
+ if (data.value === 'all') {
+ text = gettext("All");
+ } else if (data.value === 'any') {
+ text = gettext("Any");
+ }
+ if (data.invert) {
+ text = `!${text}`;
+ }
+ iconCls = 'fa fa-filter';
+
+ break;
+ }
+
+ return [text, iconCls];
+ },
+
+ initComponent: function() {
+ let me = this;
+
+ let treeStore = Ext.create('Ext.data.TreeStore', {
+ root: {
+ expanded: true,
+ expandable: false,
+ text: '',
+ type: 'mode',
+ data: {
+ value: 'all',
+ invert: false,
+ },
+ children: [],
+ iconCls: 'fa fa-filter',
+ },
+ });
+
+ let realMatchFields = Ext.create({
+ xtype: 'hiddenfield',
+ setValue: function(value) {
+ this.value = value;
+ this.checkChange();
+ },
+ getValue: function() {
+ return this.value;
+ },
+ getSubmitValue: function() {
+ let value = this.value;
+ if (!value) {
+ value = [];
+ }
+ return value;
+ },
+ name: 'match-field',
+ });
+
+ let realMatchSeverity = Ext.create({
+ xtype: 'hiddenfield',
+ setValue: function(value) {
+ this.value = value;
+ this.checkChange();
+ },
+ getValue: function() {
+ return this.value;
+ },
+ getSubmitValue: function() {
+ let value = this.value;
+ if (!value) {
+ value = [];
+ }
+ return value;
+ },
+ name: 'match-severity',
+ });
+
+ let realMode = Ext.create({
+ xtype: 'hiddenfield',
+ name: 'mode',
+ setValue: function(value) {
+ this.value = value;
+ this.checkChange();
+ },
+ getValue: function() {
+ return this.value;
+ },
+ getSubmitValue: function() {
+ let value = this.value;
+ return value;
+ },
+ });
+
+ let realMatchCalendar = Ext.create({
+ xtype: 'hiddenfield',
+ name: 'match-calendar',
+
+ setValue: function(value) {
+ this.value = value;
+ this.checkChange();
+ },
+ getValue: function() {
+ return this.value;
+ },
+ getSubmitValue: function() {
+ let value = this.value;
+ return value;
+ },
+ });
+
+ let realInvertMatch = Ext.create({
+ xtype: 'proxmoxcheckbox',
+ name: 'invert-match',
+ hidden: true,
+ deleteEmpty: !me.isCreate,
+ });
+
+ let storeChanged = function(store) {
+ store.suspendEvent('datachanged');
+
+ let matchFieldStmts = [];
+ let matchSeverityStmts = [];
+ let matchCalendarStmts = [];
+ let modeStmt = 'all';
+ let invertMatchStmt = false;
+
+ store.each(function(model) {
+ let type = model.get('type');
+ let data = model.get('data');
+
+ switch (type) {
+ case 'match-field':
+ matchFieldStmts.push(`${data.type}:${data.field}=${data.value}`);
+ break;
+ case 'match-severity':
+ matchSeverityStmts.push(data.value.join(','));
+ break;
+ case 'match-calendar':
+ matchCalendarStmts.push(data.value);
+ break;
+ case 'mode':
+ modeStmt = data.value;
+ invertMatchStmt = data.invert;
+ break;
+ }
+
+ let [text, iconCls] = me.getNodeTextAndIcon(type, data);
+ model.set({
+ text,
+ iconCls,
+ });
+ });
+
+ realMatchFields.suspendEvent('change');
+ realMatchFields.setValue(matchFieldStmts);
+ realMatchFields.resumeEvent('change');
+
+ realMatchCalendar.suspendEvent('change');
+ realMatchCalendar.setValue(matchCalendarStmts);
+ realMatchCalendar.resumeEvent('change');
+
+ realMode.suspendEvent('change');
+ realMode.setValue(modeStmt);
+ realMode.resumeEvent('change');
+
+ realInvertMatch.suspendEvent('change');
+ realInvertMatch.setValue(invertMatchStmt);
+ realInvertMatch.resumeEvent('change');
+
+ realMatchSeverity.suspendEvent('change');
+ realMatchSeverity.setValue(matchSeverityStmts);
+ realMatchSeverity.resumeEvent('change');
+
+ store.resumeEvent('datachanged');
+ };
+
+ realMatchFields.addListener('change', function(field, value) {
+ let parseMatchField = function(filter) {
+ let [, type, matchedField, matchedValue] =
+ filter.match(/^(?:(regex|exact):)?([A-Za-z0-9_][A-Za-z0-9._-]*)=(.+)$/);
+ if (type === undefined) {
+ type = "exact";
+ }
+ return {
+ type: 'match-field',
+ data: {
+ type,
+ field: matchedField,
+ value: matchedValue,
+ },
+ leaf: true,
+ };
+ };
+
+ for (let node of treeStore.queryBy(
+ record => record.get('type') === 'match-field',
+ ).getRange()) {
+ node.remove(true);
+ }
+
+ let records = value.map(parseMatchField);
+
+ let rootNode = treeStore.getRootNode();
+
+ for (let record of records) {
+ rootNode.appendChild(record);
+ }
+ });
+
+ realMatchSeverity.addListener('change', function(field, value) {
+ let parseSeverity = function(severities) {
+ return {
+ type: 'match-severity',
+ data: {
+ value: severities.split(','),
+ },
+ leaf: true,
+ };
+ };
+
+ for (let node of treeStore.queryBy(
+ record => record.get('type') === 'match-severity').getRange()) {
+ node.remove(true);
+ }
+
+ let records = value.map(parseSeverity);
+ let rootNode = treeStore.getRootNode();
+
+ for (let record of records) {
+ rootNode.appendChild(record);
+ }
+ });
+
+ realMatchCalendar.addListener('change', function(field, value) {
+ let parseCalendar = function(timespan) {
+ return {
+ type: 'match-calendar',
+ data: {
+ value: timespan,
+ },
+ leaf: true,
+ };
+ };
+
+ for (let node of treeStore.queryBy(
+ record => record.get('type') === 'match-calendar').getRange()) {
+ node.remove(true);
+ }
+
+ let records = value.map(parseCalendar);
+ let rootNode = treeStore.getRootNode();
+
+ for (let record of records) {
+ rootNode.appendChild(record);
+ }
+ });
+
+ realMode.addListener('change', function(field, value) {
+ let data = treeStore.getRootNode().get('data');
+ treeStore.getRootNode().set('data', {
+ ...data,
+ value,
+ });
+ });
+
+ realInvertMatch.addListener('change', function(field, value) {
+ let data = treeStore.getRootNode().get('data');
+ treeStore.getRootNode().set('data', {
+ ...data,
+ invert: value,
+ });
+ });
+
+ treeStore.addListener('datachanged', storeChanged);
+
+ let treePanel = Ext.create({
+ xtype: 'treepanel',
+ store: treeStore,
+ minHeight: 300,
+ maxHeight: 300,
+ scrollable: true,
+
+ bind: {
+ selection: '{selectedRecord}',
+ },
+ });
+
+ let addNode = function() {
+ let node = {
+ type: 'match-field',
+ data: {
+ type: 'exact',
+ field: '',
+ value: '',
+ },
+ leaf: true,
+ };
+ treeStore.getRootNode().appendChild(node);
+ treePanel.setSelection(treeStore.getRootNode().lastChild);
+ };
+
+ let deleteNode = function() {
+ let selection = treePanel.getSelection();
+ for (let selected of selection) {
+ if (!selected.isRoot()) {
+ selected.remove(true);
+ }
+ }
+ };
+
+ Ext.apply(me, {
+ items: [
+ realMatchFields,
+ realMode,
+ realMatchSeverity,
+ realInvertMatch,
+ realMatchCalendar,
+ treePanel,
+ {
+ xtype: 'button',
+ margin: '5 5 5 0',
+ text: gettext('Add'),
+ iconCls: 'fa fa-plus-circle',
+ handler: addNode,
+ },
+ {
+ xtype: 'button',
+ margin: '5 5 5 0',
+ text: gettext('Remove'),
+ iconCls: 'fa fa-minus-circle',
+ handler: deleteNode,
+ },
+ ],
+ });
+ me.callParent();
+ },
+});
+
+Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
+ extend: 'Ext.panel.Panel',
+ xtype: 'pmxNotificationMatchRuleSettings',
+ border: false,
+
+ items: [
+ {
+ xtype: 'proxmoxKVComboBox',
+ name: 'mode',
+ fieldLabel: gettext('Match if'),
+ allowBlank: false,
+ isFormField: false,
+
+ comboItems: [
+ ['all', gettext('All rules match')],
+ ['any', gettext('Any rule matches')],
+ ],
+ bind: {
+ hidden: '{!showMatchingMode}',
+ disabled: '{!showMatchingMode}',
+ value: '{rootMode}',
+ },
+ },
+ {
+ xtype: 'proxmoxcheckbox',
+ fieldLabel: gettext('Invert match'),
+ isFormField: false,
+ uncheckedValue: 0,
+ defaultValue: 0,
+ bind: {
+ hidden: '{!showMatchingMode}',
+ disabled: '{!showMatchingMode}',
+ value: '{invertMatch}',
+ },
+
+ },
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Node type'),
+ isFormField: false,
+ allowBlank: false,
+
+ bind: {
+ value: '{nodeType}',
+ hidden: '{!showMatcherType}',
+ disabled: '{!showMatcherType}',
+ },
+
+ comboItems: [
+ ['match-field', gettext('Match Field')],
+ ['match-severity', gettext('Match Severity')],
+ ['match-calendar', gettext('Match Calendar')],
+ ],
+ },
+ {
+ fieldLabel: 'Match Type',
+ xtype: 'proxmoxKVComboBox',
+ reference: 'type',
+ isFormField: false,
+ allowBlank: false,
+ submitValue: false,
+
+ bind: {
+ hidden: '{!typeIsMatchField}',
+ disabled: '{!typeIsMatchField}',
+ value: '{matchFieldType}',
+ },
+
+ comboItems: [
+ ['exact', gettext('Exact')],
+ ['regex', gettext('Regex')],
+ ],
+ },
+ {
+ fieldLabel: gettext('Field'),
+ xtype: 'textfield',
+ isFormField: false,
+ submitValue: false,
+ bind: {
+ hidden: '{!typeIsMatchField}',
+ disabled: '{!typeIsMatchField}',
+ value: '{matchFieldField}',
+ },
+ },
+ {
+ fieldLabel: gettext('Value'),
+ xtype: 'textfield',
+ isFormField: false,
+ submitValue: false,
+ allowBlank: false,
+ bind: {
+ hidden: '{!typeIsMatchField}',
+ disabled: '{!typeIsMatchField}',
+ value: '{matchFieldValue}',
+ },
+ },
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Severities to match'),
+ isFormField: false,
+ allowBlank: true,
+ multiSelect: true,
+
+ bind: {
+ value: '{matchSeverityValue}',
+ hidden: '{!typeIsMatchSeverity}',
+ disabled: '{!typeIsMatchSeverity}',
+ },
+
+ comboItems: [
+ ['info', gettext('Info')],
+ ['notice', gettext('Notice')],
+ ['warning', gettext('Warning')],
+ ['error', gettext('Error')],
+ ],
+ },
+ {
+ xtype: 'proxmoxKVComboBox',
+ fieldLabel: gettext('Timespan to match'),
+ isFormField: false,
+ allowBlank: false,
+ editable: true,
+ displayField: 'key',
+
+ bind: {
+ value: '{matchCalendarValue}',
+ hidden: '{!typeIsMatchCalendar}',
+ disabled: '{!typeIsMatchCalender}',
+ },
+
+ comboItems: [
+ ['mon 8-12', ''],
+ ['tue..fri,sun 0:00-23:59', ''],
+ ],
+ },
+ ],
+});
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing Lukas Wagner
@ 2023-11-13 15:13 ` Dominik Csapak
0 siblings, 0 replies; 32+ messages in thread
From: Dominik Csapak @ 2023-11-13 15:13 UTC (permalink / raw)
To: Proxmox VE development discussion, Lukas Wagner
in generaly my biggest issue with this patch is that
the various functions/formulas/handler are way to far
away from the place they live/get used/etc.
in generaly i'd try to use the 'controller+viewmodel' consistently
so have a controller (where the logic (methods+handlers) live, the
view where the structure of the layout lives
(and if necessary the viewmodel where the data+formulas live)
otherwise not a very deep review, but some comments inline nonetheless:
On 11/7/23 11:18, Lukas Wagner wrote:
> This modifies the old filter edit window in the following ways:
> - Split content into multiple panels
> - Name and comment in the first tab
> - Match rules in a tree-structure in the second tab
> - Targets to notify in the third tab
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>
> Notes:
> The code binding the match rule tree structure to the editable fields
> could definitely be a bit cleaner. I think this is the first time that
> we have used such a pattern, so there there was much experimentation
> needed to get this working.
> I plan to revisit it and clean up a bit later, I wanted to get
> the notification system changes on the list ASAP.
>
> src/window/NotificationMatcherEdit.js | 867 ++++++++++++++++++++++++--
> 1 file changed, 820 insertions(+), 47 deletions(-)
>
> diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
> index a014f3e..c6f0726 100644
> --- a/src/window/NotificationMatcherEdit.js
> +++ b/src/window/NotificationMatcherEdit.js
> @@ -1,6 +1,6 @@
> -Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
> +Ext.define('Proxmox.panel.NotificationMatcherGeneralPanel', {
> extend: 'Proxmox.panel.InputPanel',
> - xtype: 'pmxNotificationMatcherEditPanel',
> + xtype: 'pmxNotificationMatcherGeneralPanel',
> mixins: ['Proxmox.Mixin.CBind'],
>
> items: [
> @@ -15,53 +15,27 @@ Ext.define('Proxmox.panel.NotificationMatcherEditPanel', {
> allowBlank: false,
> },
> {
> - xtype: 'proxmoxKVComboBox',
> - name: 'min-severity',
> - fieldLabel: gettext('Minimum Severity'),
> - value: null,
> + xtype: 'proxmoxtextfield',
> + name: 'comment',
> + fieldLabel: gettext('Comment'),
> cbind: {
> deleteEmpty: '{!isCreate}',
> },
> - comboItems: [
> - ['info', 'info'],
> - ['notice', 'notice'],
> - ['warning', 'warning'],
> - ['error', 'error'],
> - ],
> - triggers: {
> - clear: {
> - cls: 'pmx-clear-trigger',
> - weight: -1,
> - hidden: false,
> - handler: function() {
> - this.setValue('');
> - },
> - },
> - },
> - },
> - {
> - xtype: 'proxmoxcheckbox',
> - fieldLabel: gettext('Invert match'),
> - name: 'invert-match',
> - uncheckedValue: 0,
> - defaultValue: 0,
> - cbind: {
> - deleteDefaultValue: '{!isCreate}',
> - },
> },
> + ],
> +});
> +
> +Ext.define('Proxmox.panel.NotificationMatcherTargetPanel', {
> + extend: 'Proxmox.panel.InputPanel',
> + xtype: 'pmxNotificationMatcherTargetPanel',
> + mixins: ['Proxmox.Mixin.CBind'],
> +
> + items: [
> {
> xtype: 'pmxNotificationTargetSelector',
> name: 'target',
> allowBlank: false,
> },
> - {
> - xtype: 'proxmoxtextfield',
> - name: 'comment',
> - fieldLabel: gettext('Comment'),
> - cbind: {
> - deleteEmpty: '{!isCreate}',
> - },
> - },
> ],
> });
>
> @@ -74,7 +48,7 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
> labelWidth: 120,
> },
>
> - width: 500,
> + width: 700,
>
> initComponent: function() {
> let me = this;
> @@ -97,12 +71,38 @@ Ext.define('Proxmox.window.NotificationMatcherEdit', {
> me.subject = gettext('Notification Matcher');
>
> Ext.apply(me, {
> - items: [{
> - name: me.name,
> - xtype: 'pmxNotificationMatcherEditPanel',
> - isCreate: me.isCreate,
> - baseUrl: me.baseUrl,
> - }],
> + bodyPadding: 0,
> + items: [
> + {
> + xtype: 'tabpanel',
> + region: 'center',
> + layout: 'fit',
> + bodyPadding: 10,
> + items: [
> + {
> + name: me.name,
> + title: gettext('General'),
> + xtype: 'pmxNotificationMatcherGeneralPanel',
> + isCreate: me.isCreate,
> + baseUrl: me.baseUrl,
> + },
> + {
> + name: me.name,
> + title: gettext('Match Rules'),
> + xtype: 'pmxNotificationMatchRulesEditPanel',
> + isCreate: me.isCreate,
> + baseUrl: me.baseUrl,
> + },
> + {
> + name: me.name,
> + title: gettext('Targets to notify'),
> + xtype: 'pmxNotificationMatcherTargetPanel',
> + isCreate: me.isCreate,
> + baseUrl: me.baseUrl,
> + },
> + ],
> + },
> + ],
> });
>
> me.callParent();
> @@ -252,3 +252,776 @@ Ext.define('Proxmox.form.NotificationTargetSelector', {
> },
>
> });
> +
> +Ext.define('Proxmox.panel.NotificationRulesEditPanel', {
> + extend: 'Proxmox.panel.InputPanel',
> + xtype: 'pmxNotificationMatchRulesEditPanel',
> + mixins: ['Proxmox.Mixin.CBind'],
> +
> + viewModel: {
> + data: {
> + selectedRecord: null,
> + matchFieldType: 'exact',
> + matchFieldField: '',
> + matchFieldValue: '',
> + rootMode: 'all',
> + },
> +
> + formulas: {
i'm not really a big fan of this, basically i have two
complaints:
* there are too many formulas here
* they are very far away from where they are used
(logically (a different component) as well as literally far away in the code)
you can inherit the viewmodels too, so you could probably move some formulas
in the child component where they are used?
also there is room for some improvement there:
(see further down)
> + nodeType: {
> + get: function(get) {
> + let record = get('selectedRecord');
> + return record?.get('type');
> + },
> + set: function(value) {
> + let me = this;
> + let record = me.get('selectedRecord');
> +
> + let data;
> +
> + switch (value) {
> + case 'match-severity':
> + data = {
> + value: ['info', 'notice', 'warning', 'error'],
> + };
> + break;
> + case 'match-field':
> + data = {
> + type: 'exact',
> + field: '',
> + value: '',
> + };
> + break;
> + case 'match-calendar':
> + data = {
> + value: '',
> + };
> + break;
> + }
> +
> + let node = {
> + type: value,
> + data,
> + };
> + record.set(node);
> + },
> + },
> + showMatchingMode: function(get) {
> + let record = get('selectedRecord');
> + if (!record) {
> + return false;
> + }
> + return record.isRoot();
> + },
> + showMatcherType: function(get) {
> + let record = get('selectedRecord');
> + if (!record) {
> + return false;
> + }
> + return !record.isRoot();
> + },
these two are inverts of each other, so with the '!' operator
there is no need for both
> + typeIsMatchField: {
> + bind: {
> + bindTo: '{selectedRecord}',
> + deep: true,
> + },
> + get: function(record) {
> + return record?.get('type') === 'match-field';
> + },
> + },
> + typeIsMatchSeverity: {
> + bind: {
> + bindTo: '{selectedRecord}',
> + deep: true,
> + },
> + get: function(record) {
> + return record?.get('type') === 'match-severity';
> + },
> + },
> + typeIsMatchCalendar: {
> + bind: {
> + bindTo: '{selectedRecord}',
> + deep: true,
> + },
> + get: function(record) {
> + 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');
> + record.set({
> + data: {
> + ...currentData,
> + type: value,
> + },
> + });
> + },
> + 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,
> + },
> + });
> + },
> + 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 currentData = record.get('data');
> + record.set({
> + data: {
> + ...currentData,
> + value: value,
> + },
> + });
> + },
> + get: function(record) {
> + 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}',
> + deep: true,
> + },
> + set: function(value) {
> + let me = this;
> + let record = me.get('selectedRecord');
> + let currentData = record.get('data');
> + record.set({
> + data: {
> + ...currentData,
> + value,
> + },
> + });
> + },
> + get: function(record) {
> + return record?.get('data')?.value;
> + },
> + },
> + invertMatch: {
> + 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,
> + invert: value,
> + },
> + });
> + },
> + get: function(record) {
> + return record?.get('data')?.invert;
> + },
> + },
for all the 'matchXY' you could probably simply use a change handler
instead of writing out the setter for each manually
(the get can be ok, but for those i guess you wouldn't need a formula
but coud use '{selectedRecord.data.value}' or '{selectedRecord.data.field}', no?
even if they are really needed, they are basically all the same and could
be generated quite easily IMHO. not only would it make much shorter,
but would catch copy & paste errors
> + },
> + },
> +
> + column1: [
> + {
> + xtype: 'pmxNotificationMatchRuleTree',
> + cbind: {
> + isCreate: '{isCreate}',
> + },
> + },
> + ],
> + column2: [
> + {
> + xtype: 'pmxNotificationMatchRuleSettings',
> + },
> +
> + ],
> +
> + onGetValues: function(values) {
> + let me = this;
> +
> + let deleteArrayIfEmtpy = (field) => {
> + if (Ext.isArray(values[field])) {
> + if (values[field].length === 0) {
> + delete values[field];
> + if (!me.isCreate) {
> + Proxmox.Utils.assemble_field_data(values, { 'delete': field });
> + }
> + }
> + }
> + };
> + deleteArrayIfEmtpy('match-field');
> + deleteArrayIfEmtpy('match-severity');
> + deleteArrayIfEmtpy('match-calendar');
> +
> + return values;
> + },
> +});
> +
> +Ext.define('Proxmox.panel.NotificationMatchRuleTree', {
> + extend: 'Ext.panel.Panel',
> + xtype: 'pmxNotificationMatchRuleTree',
> + mixins: ['Proxmox.Mixin.CBind'],
> + border: false,
> +
> + getNodeTextAndIcon: function(type, data) {
> + let text;
> + let iconCls;
> +
> + switch (type) {
> + case 'match-severity': {
> + let v = data.value.join(', ');
> + text = Ext.String.format(gettext("Match severity: {0}"), v);
> + iconCls = 'fa fa-exclamation';
> + } break;
> + case 'match-field': {
> + let field = data.field;
> + let value = data.value;
> + text = Ext.String.format(gettext("Match field: {0}={1}"), field, value);
> + iconCls = 'fa fa-cube';
> + } break;
> + case 'match-calendar': {
> + let v = data.value;
> + text = Ext.String.format(gettext("Match calendar: {0}"), v);
> + iconCls = 'fa fa-calendar-o';
> + } break;
> + case 'mode':
> + if (data.value === 'all') {
> + text = gettext("All");
> + } else if (data.value === 'any') {
> + text = gettext("Any");
> + }
> + if (data.invert) {
> + text = `!${text}`;
> + }
> + iconCls = 'fa fa-filter';
> +
> + break;
> + }
> +
> + return [text, iconCls];
> + },
> +
> + initComponent: function() {
> + let me = this;
> +
> + let treeStore = Ext.create('Ext.data.TreeStore', {
> + root: {
> + expanded: true,
> + expandable: false,
> + text: '',
> + type: 'mode',
> + data: {
> + value: 'all',
> + invert: false,
> + },
> + children: [],
> + iconCls: 'fa fa-filter',
> + },
> + });
> +
> + let realMatchFields = Ext.create({
> + xtype: 'hiddenfield',
> + setValue: function(value) {
> + this.value = value;
> + this.checkChange();
> + },
> + getValue: function() {
> + return this.value;
> + },
> + getSubmitValue: function() {
> + let value = this.value;
> + if (!value) {
> + value = [];
> + }
> + return value;
> + },
> + name: 'match-field',
> + });
> +
> + let realMatchSeverity = Ext.create({
> + xtype: 'hiddenfield',
> + setValue: function(value) {
> + this.value = value;
> + this.checkChange();
> + },
> + getValue: function() {
> + return this.value;
> + },
> + getSubmitValue: function() {
> + let value = this.value;
> + if (!value) {
> + value = [];
> + }
> + return value;
> + },
> + name: 'match-severity',
> + });
> +
> + let realMode = Ext.create({
> + xtype: 'hiddenfield',
> + name: 'mode',
> + setValue: function(value) {
> + this.value = value;
> + this.checkChange();
> + },
> + getValue: function() {
> + return this.value;
> + },
> + getSubmitValue: function() {
> + let value = this.value;
> + return value;
> + },
> + });
> +
> + let realMatchCalendar = Ext.create({
> + xtype: 'hiddenfield',
> + name: 'match-calendar',
> +
> + setValue: function(value) {
> + this.value = value;
> + this.checkChange();
> + },
> + getValue: function() {
> + return this.value;
> + },
> + getSubmitValue: function() {
> + let value = this.value;
> + return value;
> + },
> + });
are these really all necessary? especially the lines
getSubmitValue: function{
let value = this.value;
return value;
}
looks weird. submitValue does basically the same already?
(+ processrawvalue, which does nothing by defaut, +getRawvalue)
if not, a
getSubmitValue: function() {
return this.value;
}
would be shorter (return this.value || []; for the array case)
IMHO having such fields defined inline can be ok, but
not if there are many of those, then i'd rather prefer
having the functionality split out into a class (or helper)
and reuse that
> +
> + let realInvertMatch = Ext.create({
> + xtype: 'proxmoxcheckbox',
> + name: 'invert-match',
> + hidden: true,
> + deleteEmpty: !me.isCreate,
> + });
> +
> + let storeChanged = function(store) {
> + store.suspendEvent('datachanged');
> +
> + let matchFieldStmts = [];
> + let matchSeverityStmts = [];
> + let matchCalendarStmts = [];
> + let modeStmt = 'all';
> + let invertMatchStmt = false;
> +
> + store.each(function(model) {
> + let type = model.get('type');
> + let data = model.get('data');
> +
> + switch (type) {
> + case 'match-field':
> + matchFieldStmts.push(`${data.type}:${data.field}=${data.value}`);
> + break;
> + case 'match-severity':
> + matchSeverityStmts.push(data.value.join(','));
> + break;
> + case 'match-calendar':
> + matchCalendarStmts.push(data.value);
> + break;
> + case 'mode':
> + modeStmt = data.value;
> + invertMatchStmt = data.invert;
> + break;
> + }
> +
> + let [text, iconCls] = me.getNodeTextAndIcon(type, data);
> + model.set({
> + text,
> + iconCls,
> + });
> + });
> +
> + realMatchFields.suspendEvent('change');
> + realMatchFields.setValue(matchFieldStmts);
> + realMatchFields.resumeEvent('change');
> +
> + realMatchCalendar.suspendEvent('change');
> + realMatchCalendar.setValue(matchCalendarStmts);
> + realMatchCalendar.resumeEvent('change');
> +
> + realMode.suspendEvent('change');
> + realMode.setValue(modeStmt);
> + realMode.resumeEvent('change');
> +
> + realInvertMatch.suspendEvent('change');
> + realInvertMatch.setValue(invertMatchStmt);
> + realInvertMatch.resumeEvent('change');
> +
> + realMatchSeverity.suspendEvent('change');
> + realMatchSeverity.setValue(matchSeverityStmts);
> + realMatchSeverity.resumeEvent('change');
wouldn't it be much easier and shorter to do:
[realmatchfields,realmatchcalender,...].forEach(field => field.suspendEvent(...));
(and same for resume? or is the separation here necessary?
> +
> + store.resumeEvent('datachanged');
> + };
> +
> + realMatchFields.addListener('change', function(field, value) {
> + let parseMatchField = function(filter) {
> + let [, type, matchedField, matchedValue] =
> + filter.match(/^(?:(regex|exact):)?([A-Za-z0-9_][A-Za-z0-9._-]*)=(.+)$/);
> + if (type === undefined) {
> + type = "exact";
> + }
> + return {
> + type: 'match-field',
> + data: {
> + type,
> + field: matchedField,
> + value: matchedValue,
> + },
> + leaf: true,
> + };
> + };
> +
> + for (let node of treeStore.queryBy(
> + record => record.get('type') === 'match-field',
> + ).getRange()) {
> + node.remove(true);
> + }
> +
> + let records = value.map(parseMatchField);
> +
> + let rootNode = treeStore.getRootNode();
> +
> + for (let record of records) {
> + rootNode.appendChild(record);
> + }
> + });
> +
> + realMatchSeverity.addListener('change', function(field, value) {
> + let parseSeverity = function(severities) {
> + return {
> + type: 'match-severity',
> + data: {
> + value: severities.split(','),
> + },
> + leaf: true,
> + };
> + };
> +
> + for (let node of treeStore.queryBy(
> + record => record.get('type') === 'match-severity').getRange()) {
> + node.remove(true);
> + }
> +
> + let records = value.map(parseSeverity);
> + let rootNode = treeStore.getRootNode();
> +
> + for (let record of records) {
> + rootNode.appendChild(record);
> + }
> + });
> +
> + realMatchCalendar.addListener('change', function(field, value) {
> + let parseCalendar = function(timespan) {
> + return {
> + type: 'match-calendar',
> + data: {
> + value: timespan,
> + },
> + leaf: true,
> + };
> + };
> +
> + for (let node of treeStore.queryBy(
> + record => record.get('type') === 'match-calendar').getRange()) {
> + node.remove(true);
> + }
> +
> + let records = value.map(parseCalendar);
> + let rootNode = treeStore.getRootNode();
> +
> + for (let record of records) {
> + rootNode.appendChild(record);
> + }
> + });
> +
> + realMode.addListener('change', function(field, value) {
> + let data = treeStore.getRootNode().get('data');
> + treeStore.getRootNode().set('data', {
> + ...data,
> + value,
> + });
> + });
> +
> + realInvertMatch.addListener('change', function(field, value) {
> + let data = treeStore.getRootNode().get('data');
> + treeStore.getRootNode().set('data', {
> + ...data,
> + invert: value,
> + });
> + });
i think you should either define them inline with the fields,
or go all in to the controller method, and define them in a controller
and the fields declarative
> +
> + treeStore.addListener('datachanged', storeChanged);
> +
> + let treePanel = Ext.create({
> + xtype: 'treepanel',
> + store: treeStore,
> + minHeight: 300,
> + maxHeight: 300,
> + scrollable: true,
> +
> + bind: {
> + selection: '{selectedRecord}',
> + },
> + });
> +
> + let addNode = function() {
> + let node = {
> + type: 'match-field',
> + data: {
> + type: 'exact',
> + field: '',
> + value: '',
> + },
> + leaf: true,
> + };
> + treeStore.getRootNode().appendChild(node);
> + treePanel.setSelection(treeStore.getRootNode().lastChild);
> + };
> +
> + let deleteNode = function() {
> + let selection = treePanel.getSelection();
> + for (let selected of selection) {
> + if (!selected.isRoot()) {
> + selected.remove(true);
> + }
> + }
> + };
> +
> + Ext.apply(me, {
> + items: [
> + realMatchFields,
> + realMode,
> + realMatchSeverity,
> + realInvertMatch,
> + realMatchCalendar,
> + treePanel,
> + {
> + xtype: 'button',
> + margin: '5 5 5 0',
> + text: gettext('Add'),
> + iconCls: 'fa fa-plus-circle',
> + handler: addNode,
> + },
> + {
> + xtype: 'button',
> + margin: '5 5 5 0',
> + text: gettext('Remove'),
> + iconCls: 'fa fa-minus-circle',
> + handler: deleteNode,
> + },
> + ],
> + });
> + me.callParent();
> + },
> +});
> +
> +Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
> + extend: 'Ext.panel.Panel',
> + xtype: 'pmxNotificationMatchRuleSettings',
> + border: false,
> +
> + items: [
> + {
> + xtype: 'proxmoxKVComboBox',
> + name: 'mode',
> + fieldLabel: gettext('Match if'),
> + allowBlank: false,
> + isFormField: false,
> +
> + comboItems: [
> + ['all', gettext('All rules match')],
> + ['any', gettext('Any rule matches')],
> + ],
> + bind: {
> + hidden: '{!showMatchingMode}',
> + disabled: '{!showMatchingMode}',
> + value: '{rootMode}',
> + },
> + },
> + {
> + xtype: 'proxmoxcheckbox',
> + fieldLabel: gettext('Invert match'),
> + isFormField: false,
> + uncheckedValue: 0,
> + defaultValue: 0,
> + bind: {
> + hidden: '{!showMatchingMode}',
> + disabled: '{!showMatchingMode}',
> + value: '{invertMatch}',
> + },
> +
> + },
> + {
> + xtype: 'proxmoxKVComboBox',
> + fieldLabel: gettext('Node type'),
> + isFormField: false,
> + allowBlank: false,
> +
> + bind: {
> + value: '{nodeType}',
> + hidden: '{!showMatcherType}',
> + disabled: '{!showMatcherType}',
> + },
> +
> + comboItems: [
> + ['match-field', gettext('Match Field')],
> + ['match-severity', gettext('Match Severity')],
> + ['match-calendar', gettext('Match Calendar')],
> + ],
> + },
> + {
> + fieldLabel: 'Match Type',
> + xtype: 'proxmoxKVComboBox',
> + reference: 'type',
> + isFormField: false,
> + allowBlank: false,
> + submitValue: false,
> +
> + bind: {
> + hidden: '{!typeIsMatchField}',
> + disabled: '{!typeIsMatchField}',
> + value: '{matchFieldType}',
> + },
> +
> + comboItems: [
> + ['exact', gettext('Exact')],
> + ['regex', gettext('Regex')],
> + ],
> + },
> + {
> + fieldLabel: gettext('Field'),
> + xtype: 'textfield',
> + isFormField: false,
> + submitValue: false,
> + bind: {
> + hidden: '{!typeIsMatchField}',
> + disabled: '{!typeIsMatchField}',
> + value: '{matchFieldField}',
> + },
> + },
> + {
> + fieldLabel: gettext('Value'),
> + xtype: 'textfield',
> + isFormField: false,
> + submitValue: false,
> + allowBlank: false,
> + bind: {
> + hidden: '{!typeIsMatchField}',
> + disabled: '{!typeIsMatchField}',
> + value: '{matchFieldValue}',
> + },
> + },
> + {
> + xtype: 'proxmoxKVComboBox',
> + fieldLabel: gettext('Severities to match'),
> + isFormField: false,
> + allowBlank: true,
> + multiSelect: true,
> +
> + bind: {
> + value: '{matchSeverityValue}',
> + hidden: '{!typeIsMatchSeverity}',
> + disabled: '{!typeIsMatchSeverity}',
> + },
> +
> + comboItems: [
> + ['info', gettext('Info')],
> + ['notice', gettext('Notice')],
> + ['warning', gettext('Warning')],
> + ['error', gettext('Error')],
> + ],
> + },
> + {
> + xtype: 'proxmoxKVComboBox',
> + fieldLabel: gettext('Timespan to match'),
> + isFormField: false,
> + allowBlank: false,
> + editable: true,
> + displayField: 'key',
> +
> + bind: {
> + value: '{matchCalendarValue}',
> + hidden: '{!typeIsMatchCalendar}',
> + disabled: '{!typeIsMatchCalender}',
> + },
> +
> + comboItems: [
> + ['mon 8-12', ''],
> + ['tue..fri,sun 0:00-23:59', ''],
> + ],
> + },
> + ],
> +});
wouldn't it make sense to have multiple panels depending on the type, so that the various
matchers are separated, and simply show only the one thats currently selected?
^ permalink raw reply [flat|nested] 32+ messages in thread
* [pve-devel] [PATCH proxmox-widget-toolkit 26/27] notification ui: unprotected mailto-root target
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (24 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 25/27] notification: matcher: add UI for matcher editing Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 27/27] noficiation: matcher edit: make 'field' an editable combobox Lukas Wagner
2023-11-13 14:34 ` [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Dominik Csapak
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
A default notification config will now be created in pve-manager's
postinst hook - which is not magic in any way and can be modified
and deleted as desired.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/panel/NotificationConfigView.js | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/panel/NotificationConfigView.js b/src/panel/NotificationConfigView.js
index ecf764d..6a9bc20 100644
--- a/src/panel/NotificationConfigView.js
+++ b/src/panel/NotificationConfigView.js
@@ -41,10 +41,6 @@ Ext.define('Proxmox.panel.NotificationEndpointView', {
openEditWindow: function(endpointType, endpoint) {
let me = this;
- if (endpoint === 'mail-to-root') {
- return;
- }
-
Ext.create('Proxmox.window.EndpointEditBase', {
baseUrl: me.getView().baseUrl,
type: endpointType,
@@ -183,13 +179,11 @@ Ext.define('Proxmox.panel.NotificationEndpointView', {
xtype: 'proxmoxButton',
text: gettext('Modify'),
handler: 'openEditForSelectedItem',
- enableFn: rec => rec.data.name !== 'mail-to-root',
disabled: true,
},
{
xtype: 'proxmoxStdRemoveButton',
callback: 'reload',
- enableFn: rec => rec.data.name !== 'mail-to-root',
getUrl: function(rec) {
return `${me.baseUrl}/endpoints/${rec.data.type}/${rec.getId()}`;
},
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* [pve-devel] [PATCH proxmox-widget-toolkit 27/27] noficiation: matcher edit: make 'field' an editable combobox
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (25 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 26/27] notification ui: unprotected mailto-root target Lukas Wagner
@ 2023-11-07 10:18 ` Lukas Wagner
2023-11-13 14:34 ` [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Dominik Csapak
27 siblings, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-07 10:18 UTC (permalink / raw)
To: pve-devel
For now with fixed options that are shared between most notification
events - later, once we have a notification registry, this should be
filled dynamically.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/window/NotificationMatcherEdit.js | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/window/NotificationMatcherEdit.js b/src/window/NotificationMatcherEdit.js
index c6f0726..fb55e17 100644
--- a/src/window/NotificationMatcherEdit.js
+++ b/src/window/NotificationMatcherEdit.js
@@ -963,14 +963,23 @@ Ext.define('Proxmox.panel.NotificationMatchRuleSettings', {
},
{
fieldLabel: gettext('Field'),
- xtype: 'textfield',
+ xtype: 'proxmoxKVComboBox',
isFormField: false,
submitValue: false,
+ allowBlank: false,
+ editable: true,
+ displayField: 'key',
bind: {
hidden: '{!typeIsMatchField}',
disabled: '{!typeIsMatchField}',
value: '{matchFieldField}',
},
+ // TODO: Once we have a 'notification registry', we should
+ // retrive those via an API call.
+ comboItems: [
+ ['type', ''],
+ ['hostname', ''],
+ ],
},
{
fieldLabel: gettext('Value'),
--
2.39.2
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters
2023-11-07 10:18 [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Lukas Wagner
` (26 preceding siblings ...)
2023-11-07 10:18 ` [pve-devel] [PATCH proxmox-widget-toolkit 27/27] noficiation: matcher edit: make 'field' an editable combobox Lukas Wagner
@ 2023-11-13 14:34 ` Dominik Csapak
2023-11-13 14:54 ` Thomas Lamprecht
2023-11-13 14:58 ` Lukas Wagner
27 siblings, 2 replies; 32+ messages in thread
From: Dominik Csapak @ 2023-11-13 14:34 UTC (permalink / raw)
To: Proxmox VE development discussion, Lukas Wagner
a few high level ui things
(i did not look too deeply in the code, but i'll send
probably some comments there too)
that probably was already there, but i find the all/any + invert combination
confusing (i had to think about it for a bit before getting a grasp on it)
i would propose we can write the four options out what they mean
and internally convert them to all/any + invert, e.g.
'all rule match'
'any rule match'
'at least one rule does not match' (all + invert)
'no rule matches' (any + invert)
(also is the change from and/or to all/any not a breaking change?,
did we expose this in the api yet ?)
second, we already have a very similar interface in the guest wizard for
the disks, and there we have the remove button inline,
i guess we should keep the style consistent
third, do we really need the tree? we basically have the
four options from above, and then a list of the matches
wouldn't it be enough to seperate them?
e.g. have a dropdown with the four options + a list instead of a tree?
also currently the match options are not really verified before
i can submit the form
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters
2023-11-13 14:34 ` [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Dominik Csapak
@ 2023-11-13 14:54 ` Thomas Lamprecht
2023-11-13 14:58 ` Lukas Wagner
1 sibling, 0 replies; 32+ messages in thread
From: Thomas Lamprecht @ 2023-11-13 14:54 UTC (permalink / raw)
To: Proxmox VE development discussion, Dominik Csapak, Lukas Wagner
Am 13/11/2023 um 15:34 schrieb Dominik Csapak:
> (also is the change from and/or to all/any not a breaking change?,
> did we expose this in the api yet ?)
there's lots of breaking change, it was applied a bit to early (reviewer
"fault", not dev one) but only ever exposed on pvetest, so Lukas got my
OK to break this, which for is most of the time fine for pvetest stuff
anyway, after all it's a testing ground and we should not make our life
harder than it is (configs get migrated best-effort wise).
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters
2023-11-13 14:34 ` [pve-devel] [PATCH many 00/27] overhaul notification system, use matchers instead of filters Dominik Csapak
2023-11-13 14:54 ` Thomas Lamprecht
@ 2023-11-13 14:58 ` Lukas Wagner
1 sibling, 0 replies; 32+ messages in thread
From: Lukas Wagner @ 2023-11-13 14:58 UTC (permalink / raw)
To: Dominik Csapak, Proxmox VE development discussion
Hi, thanks for your input.
On 11/13/23 15:34, Dominik Csapak wrote:
> a few high level ui things
> (i did not look too deeply in the code, but i'll send
> probably some comments there too)
>
Just as a warning, the tree code/data binding is definitely not as clean
as it could be right now, a cleanup patch will follow. I just wanted to
get these patches out ASAP ;)
> that probably was already there, but i find the all/any + invert
> combination
> confusing (i had to think about it for a bit before getting a grasp on it)
>
> i would propose we can write the four options out what they mean
> and internally convert them to all/any + invert, e.g.
>
> 'all rule match'
> 'any rule match'
> 'at least one rule does not match' (all + invert)
> 'no rule matches' (any + invert)
I was considering this as well and discussed both approaches with Aaron.
He was slightly in favor of the one I implemented, so I went with that.
But now that I think about it again I think I like this version better,
I'll send a followup for this (since this is is purely cosmetic).
>
> (also is the change from and/or to all/any not a breaking change?,
> did we expose this in the api yet ?)
Well, the switch-over from filters to matchers is breaking as a whole,
but it only ever hit pvetest, it is not a big problem.
(config parsing was adapted so that it will clean out 'old' entries)
The notification stuff was merged a bit too eagerly, this rework
attempts to fix some of the issues with the first approach.
>
> second, we already have a very similar interface in the guest wizard for
> the disks, and there we have the remove button inline,
> i guess we should keep the style consistent
Noted, i'll put it on my list for followups.
>
> third, do we really need the tree? we basically have the
> four options from above, and then a list of the matches
>
> wouldn't it be enough to seperate them?
The tree was a conscious decision, because at some point I want to
support 'sub-matchers' - where a matcher can reference another matcher.
that way users can compose arbitrary boolean formulas, e.g.:
All of
match ...
match ...
any of
match ...
match ...
For best UX, the sub-matchers shall be configurable within a single
dialog, and this is the reason why I went with a tree widget.
>
> e.g. have a dropdown with the four options + a list instead of a tree?
>
> also currently the match options are not really verified before
> i can submit the form
>
True, will fix this in a followup.
Thx again!
--
- Lukas
^ permalink raw reply [flat|nested] 32+ messages in thread