* [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations
@ 2024-04-19 14:17 Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 01/20] notify: switch to file-based templating system Lukas Wagner
` (20 more replies)
0 siblings, 21 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
The notification system uses handlebar templates to render the subject
and the body of notifications. Previously, the template strings were
defined inline at the call site. This patch series extracts the templates
into template files and installs them at
/usr/share/pve-manager/templates/default
where they stored as <template-name>-{body,subject}.{txt,html}.hbs
The 'default' part in the path is a preparation for translated
notifications and/or overridable notification templates.
Future work could provide notifications translated to e.g. German
in `templates/de` or similar. This will be a first for having
translated strings on the backend-side, so there is need for further
research.
The patches for `proxmox` also include some preparatory patches for
the upcoming integration of the notification system into PBS. They
are not needed for PVE, but I included them here since Folke and I
tested the PVE changes with them applied. IMO they should just be
applied with the same version bump.
The list of optional, preparatory patches is:
notify: give each notification a unique ID
notify: api: add get_targets
notify: derive `api` for Deleteable*Property
notify: derive Deserialize/Serialize for Notification struct
notify: pbs context: include nodename in default sendmail author
notify: renderer: add relative-percentage helper from PBS
Folke kindly did some off-list testing before this was posted, hence
his T-bs were included.
Bumps/dependencies:
- proxmox_notify
- libproxmox-rs-perl/libpve-rs-perl (needs bumped proxmox_notify)
- libpve-notify-perl (needs bumped libproxmox-rs-perl/libpve-rs-perl)
- pve-ha-manager (needs bumped libpve-notify-perl)
- pve-manager (needs bumped libpve-notify-perl)
- proxmox-mail-forward (optional. should not be affected by any changes, but I think
it should be also be bumped for any larger proxmox_notify changes to avoid any
weird hidden regressions due to mismatches of proxmox_notify
Changes since v1:
- Incorporated feedback from @Fiona and @Fabian - thanks!
- most noteworthy: Change template path from /usr/share/proxmox-ve
to /usr/share/
- apart from that mostly just cosmetics/style
proxmox:
Lukas Wagner (12):
notify: switch to file-based templating system
notify: make api methods take config struct ownership
notify: convert Option<Vec<T>> -> Vec<T> in config structs
notify: don't make tests require pve-context
notify: make the `mail-forwarder` feature depend on proxmox-sys
notify: cargo.toml: add spaces before curly braces
notify: give each notification a unique ID
notify: api: add get_targets
notify: derive `api` for Deleteable*Property
notify: derive Deserialize/Serialize for Notification struct
notify: pbs context: include nodename in default sendmail author
notify: renderer: add relative-percentage helper from PBS
proxmox-notify/Cargo.toml | 7 +-
proxmox-notify/examples/render.rs | 63 ------
proxmox-notify/src/api/gotify.rs | 48 ++---
proxmox-notify/src/api/matcher.rs | 59 +++--
proxmox-notify/src/api/mod.rs | 113 ++++++++--
proxmox-notify/src/api/sendmail.rs | 60 +++---
proxmox-notify/src/api/smtp.rs | 122 +++++------
proxmox-notify/src/context/mod.rs | 10 +-
proxmox-notify/src/context/pbs.rs | 18 +-
proxmox-notify/src/context/pve.rs | 15 ++
proxmox-notify/src/context/test.rs | 9 +
proxmox-notify/src/endpoints/common/mail.rs | 20 +-
proxmox-notify/src/endpoints/gotify.rs | 12 +-
proxmox-notify/src/endpoints/sendmail.rs | 34 +--
proxmox-notify/src/endpoints/smtp.rs | 38 ++--
proxmox-notify/src/lib.rs | 59 ++---
proxmox-notify/src/matcher.rs | 71 +++---
proxmox-notify/src/renderer/html.rs | 14 --
proxmox-notify/src/renderer/mod.rs | 226 ++++++++------------
proxmox-notify/src/renderer/plaintext.rs | 39 ----
20 files changed, 506 insertions(+), 531 deletions(-)
delete mode 100644 proxmox-notify/examples/render.rs
proxmox-perl-rs:
Lukas Wagner (3):
notify: use file based notification templates
notify: don't pass config structs by reference
notify: adapt to Option<Vec<T>> to Vec<T> changes in proxmox_notify
common/src/notify.rs | 48 +++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 25 deletions(-)
pve-cluster:
Lukas Wagner (1):
notify: use named template instead of passing template strings
src/PVE/Notify.pm | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
pve-ha-manager:
Lukas Wagner (1):
env: notify: use named templates instead of passing template strings
debian/pve-ha-manager.install | 3 +++
src/Makefile | 1 +
src/PVE/HA/Env/PVE2.pm | 4 ++--
src/PVE/HA/NodeStatus.pm | 20 +------------------
src/PVE/HA/Sim/Env.pm | 3 ++-
src/templates/Makefile | 10 ++++++++++
src/templates/default/fencing-body.html.hbs | 14 +++++++++++++
src/templates/default/fencing-body.txt.hbs | 11 ++++++++++
src/templates/default/fencing-subject.txt.hbs | 1 +
9 files changed, 45 insertions(+), 22 deletions(-)
create mode 100644 src/templates/Makefile
create mode 100644 src/templates/default/fencing-body.html.hbs
create mode 100644 src/templates/default/fencing-body.txt.hbs
create mode 100644 src/templates/default/fencing-subject.txt.hbs
pve-manager:
Lukas Wagner (3):
gitignore: ignore any test artifacts
tests: remove vzdump_notification test
notifications: use named templates instead of in-code templates
.gitignore | 2 +
Makefile | 2 +-
PVE/API2/APT.pm | 9 +-
PVE/API2/Replication.pm | 20 +---
PVE/VZDump.pm | 20 +---
templates/Makefile | 24 +++++
.../default/package-updates-body.html.hbs | 6 ++
.../default/package-updates-body.txt.hbs | 3 +
.../default/package-updates-subject.txt.hbs | 1 +
templates/default/replication-body.html.hbs | 18 ++++
templates/default/replication-body.txt.hbs | 12 +++
templates/default/replication-subject.txt.hbs | 1 +
templates/default/test-body.html.hbs | 1 +
templates/default/test-body.txt.hbs | 1 +
templates/default/test-subject.txt.hbs | 1 +
templates/default/vzdump-body.html.hbs | 11 ++
templates/default/vzdump-body.txt.hbs | 10 ++
templates/default/vzdump-subject.txt.hbs | 1 +
test/Makefile | 6 +-
test/vzdump_notification_test.pl | 101 ------------------
20 files changed, 98 insertions(+), 152 deletions(-)
create mode 100644 templates/Makefile
create mode 100644 templates/default/package-updates-body.html.hbs
create mode 100644 templates/default/package-updates-body.txt.hbs
create mode 100644 templates/default/package-updates-subject.txt.hbs
create mode 100644 templates/default/replication-body.html.hbs
create mode 100644 templates/default/replication-body.txt.hbs
create mode 100644 templates/default/replication-subject.txt.hbs
create mode 100644 templates/default/test-body.html.hbs
create mode 100644 templates/default/test-body.txt.hbs
create mode 100644 templates/default/test-subject.txt.hbs
create mode 100644 templates/default/vzdump-body.html.hbs
create mode 100644 templates/default/vzdump-body.txt.hbs
create mode 100644 templates/default/vzdump-subject.txt.hbs
delete mode 100755 test/vzdump_notification_test.pl
Summary over all repositories:
51 files changed, 684 insertions(+), 747 deletions(-)
--
Generated by git-murpp 0.7.1
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 01/20] notify: switch to file-based templating system
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 02/20] notify: make api methods take config struct ownership Lukas Wagner
` (19 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Instead of passing the template strings for subject and body when
constructing a notification, we pass only the name of a template.
When rendering the template, the name of the template is used to find
corresponding template files. For PVE, they are located at
/usr/share/proxmox-ve/templates/default. The `default` part is
the 'template namespace', which is a preparation for user-customizable
and/or translatable notifications.
Previously, the same template string was used to render HTML and
plaintext notifications. This was achieved by providing some template
helpers that 'abstract away' HTML/plaintext formatting. However,
in hindsight this turned out to be pretty finicky. Since the
current changes lay the foundations for user-customizable notification
templates, I ripped these abstractions out. Now there are simply two
templates, one for plaintext, one for HTML.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/examples/render.rs | 63 --------
proxmox-notify/src/context/mod.rs | 10 +-
proxmox-notify/src/context/pbs.rs | 16 ++
proxmox-notify/src/context/pve.rs | 15 ++
proxmox-notify/src/context/test.rs | 9 ++
proxmox-notify/src/endpoints/gotify.rs | 9 +-
proxmox-notify/src/endpoints/sendmail.rs | 13 +-
proxmox-notify/src/endpoints/smtp.rs | 11 +-
proxmox-notify/src/lib.rs | 27 ++--
proxmox-notify/src/matcher.rs | 24 +--
proxmox-notify/src/renderer/html.rs | 14 --
proxmox-notify/src/renderer/mod.rs | 197 +++++++----------------
proxmox-notify/src/renderer/plaintext.rs | 39 -----
13 files changed, 137 insertions(+), 310 deletions(-)
delete mode 100644 proxmox-notify/examples/render.rs
diff --git a/proxmox-notify/examples/render.rs b/proxmox-notify/examples/render.rs
deleted file mode 100644
index d705fd0..0000000
--- a/proxmox-notify/examples/render.rs
+++ /dev/null
@@ -1,63 +0,0 @@
-use proxmox_notify::renderer::{render_template, TemplateRenderer};
-use proxmox_notify::Error;
-
-use serde_json::json;
-
-const TEMPLATE: &str = r#"
-{{ heading-1 "Backup Report"}}
-A backup job on host {{host}} was run.
-
-{{ heading-2 "Guests"}}
-{{ table table }}
-The total size of all backups is {{human-bytes total-size}}.
-
-The backup job took {{duration total-time}}.
-
-{{ heading-2 "Logs"}}
-{{ verbatim-monospaced logs}}
-
-{{ heading-2 "Objects"}}
-{{ object table }}
-"#;
-
-fn main() -> Result<(), Error> {
- let properties = json!({
- "host": "pali",
- "logs": "100: starting backup\n100: backup failed",
- "total-size": 1024 * 1024 + 2048 * 1024,
- "total-time": 100,
- "table": {
- "schema": {
- "columns": [
- {
- "label": "VMID",
- "id": "vmid"
- },
- {
- "label": "Size",
- "id": "size",
- "renderer": "human-bytes"
- }
- ],
- },
- "data" : [
- {
- "vmid": 1001,
- "size": "1048576"
- },
- {
- "vmid": 1002,
- "size": 2048 * 1024,
- }
- ]
- }
- });
-
- let output = render_template(TemplateRenderer::Html, TEMPLATE, &properties)?;
- println!("{output}");
-
- let output = render_template(TemplateRenderer::Plaintext, TEMPLATE, &properties)?;
- println!("{output}");
-
- Ok(())
-}
diff --git a/proxmox-notify/src/context/mod.rs b/proxmox-notify/src/context/mod.rs
index cc68603..c0a5a13 100644
--- a/proxmox-notify/src/context/mod.rs
+++ b/proxmox-notify/src/context/mod.rs
@@ -1,6 +1,8 @@
use std::fmt::Debug;
use std::sync::Mutex;
+use crate::Error;
+
#[cfg(any(feature = "pve-context", feature = "pbs-context"))]
pub mod common;
#[cfg(feature = "pbs-context")]
@@ -20,8 +22,14 @@ pub trait Context: Send + Sync + Debug {
fn default_sendmail_from(&self) -> String;
/// Proxy configuration for the current node
fn http_proxy_config(&self) -> Option<String>;
- // Return default config for built-in targets/matchers.
+ /// Return default config for built-in targets/matchers.
fn default_config(&self) -> &'static str;
+ /// Lookup a template in a certain (optional) namespace
+ fn lookup_template(
+ &self,
+ filename: &str,
+ namespace: Option<&str>,
+ ) -> Result<Option<String>, Error>;
}
#[cfg(not(test))]
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
index 5b97af7..70e993f 100644
--- a/proxmox-notify/src/context/pbs.rs
+++ b/proxmox-notify/src/context/pbs.rs
@@ -1,9 +1,11 @@
use serde::Deserialize;
+use std::path::Path;
use proxmox_schema::{ObjectSchema, Schema, StringSchema};
use proxmox_section_config::{SectionConfig, SectionConfigPlugin};
use crate::context::{common, Context};
+use crate::Error;
const PBS_USER_CFG_FILENAME: &str = "/etc/proxmox-backup/user.cfg";
const PBS_NODE_CFG_FILENAME: &str = "/etc/proxmox-backup/node.cfg";
@@ -98,6 +100,20 @@ impl Context for PBSContext {
fn default_config(&self) -> &'static str {
return DEFAULT_CONFIG;
}
+
+ fn lookup_template(
+ &self,
+ filename: &str,
+ namespace: Option<&str>,
+ ) -> Result<Option<String>, Error> {
+ let path = Path::new("/usr/share/proxmox-backup/templates")
+ .join(namespace.unwrap_or("default"))
+ .join(filename);
+
+ let template_string = proxmox_sys::fs::file_read_optional_string(path)
+ .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
+ Ok(template_string)
+ }
}
#[cfg(test)]
diff --git a/proxmox-notify/src/context/pve.rs b/proxmox-notify/src/context/pve.rs
index 39e0e4a..647f7c8 100644
--- a/proxmox-notify/src/context/pve.rs
+++ b/proxmox-notify/src/context/pve.rs
@@ -1,4 +1,6 @@
use crate::context::{common, Context};
+use crate::Error;
+use std::path::Path;
fn lookup_mail_address(content: &str, user: &str) -> Option<String> {
common::normalize_for_return(content.lines().find_map(|line| {
@@ -51,6 +53,19 @@ impl Context for PVEContext {
fn default_config(&self) -> &'static str {
return DEFAULT_CONFIG;
}
+
+ fn lookup_template(
+ &self,
+ filename: &str,
+ namespace: Option<&str>,
+ ) -> Result<Option<String>, Error> {
+ let path = Path::new("/usr/share/pve-manager/templates")
+ .join(namespace.unwrap_or("default"))
+ .join(filename);
+ let template_string = proxmox_sys::fs::file_read_optional_string(path)
+ .map_err(|err| Error::Generic(format!("could not load template: {err}")))?;
+ Ok(template_string)
+ }
}
pub static PVE_CONTEXT: PVEContext = PVEContext;
diff --git a/proxmox-notify/src/context/test.rs b/proxmox-notify/src/context/test.rs
index 61f794a..5df25d0 100644
--- a/proxmox-notify/src/context/test.rs
+++ b/proxmox-notify/src/context/test.rs
@@ -1,4 +1,5 @@
use crate::context::Context;
+use crate::Error;
#[derive(Debug)]
pub struct TestContext;
@@ -23,4 +24,12 @@ impl Context for TestContext {
fn default_config(&self) -> &'static str {
""
}
+
+ fn lookup_template(
+ &self,
+ _filename: &str,
+ _namespace: Option<&str>,
+ ) -> Result<Option<String>, Error> {
+ Ok(Some(String::new()))
+ }
}
diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs
index 20c83bf..afdfabc 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -9,7 +9,7 @@ use proxmox_schema::api_types::COMMENT_SCHEMA;
use proxmox_schema::{api, Updater};
use crate::context::context;
-use crate::renderer::TemplateRenderer;
+use crate::renderer::TemplateType;
use crate::schema::ENTITY_NAME_SCHEMA;
use crate::{renderer, Content, Endpoint, Error, Notification, Origin, Severity};
@@ -92,14 +92,13 @@ impl Endpoint for GotifyEndpoint {
fn send(&self, notification: &Notification) -> Result<(), Error> {
let (title, message) = match ¬ification.content {
Content::Template {
- title_template,
- body_template,
+ template_name,
data,
} => {
let rendered_title =
- renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+ renderer::render_template(TemplateType::Subject, template_name, data)?;
let rendered_message =
- renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+ renderer::render_template(TemplateType::PlaintextBody, template_name, data)?;
(rendered_title, rendered_message)
}
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index 4fc92b4..6178b5e 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -3,9 +3,9 @@ use serde::{Deserialize, Serialize};
use proxmox_schema::api_types::COMMENT_SCHEMA;
use proxmox_schema::{api, Updater};
-use crate::context::context;
+use crate::context;
use crate::endpoints::common::mail;
-use crate::renderer::TemplateRenderer;
+use crate::renderer::TemplateType;
use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA};
use crate::{renderer, Content, Endpoint, Error, Notification, Origin};
@@ -103,16 +103,15 @@ impl Endpoint for SendmailEndpoint {
match ¬ification.content {
Content::Template {
- title_template,
- body_template,
+ template_name,
data,
} => {
let subject =
- renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+ renderer::render_template(TemplateType::Subject, template_name, data)?;
let html_part =
- renderer::render_template(TemplateRenderer::Html, body_template, data)?;
+ renderer::render_template(TemplateType::HtmlBody, template_name, data)?;
let text_part =
- renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+ renderer::render_template(TemplateType::PlaintextBody, template_name, data)?;
let author = self
.config
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index 5470257..f0c836a 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -11,7 +11,7 @@ use proxmox_schema::{api, Updater};
use crate::context::context;
use crate::endpoints::common::mail;
-use crate::renderer::TemplateRenderer;
+use crate::renderer::TemplateType;
use crate::schema::{EMAIL_SCHEMA, ENTITY_NAME_SCHEMA, USER_SCHEMA};
use crate::{renderer, Content, Endpoint, Error, Notification, Origin};
@@ -202,16 +202,15 @@ impl Endpoint for SmtpEndpoint {
let mut email = match ¬ification.content {
Content::Template {
- title_template,
- body_template,
+ template_name,
data,
} => {
let subject =
- renderer::render_template(TemplateRenderer::Plaintext, title_template, data)?;
+ renderer::render_template(TemplateType::Subject, template_name, data)?;
let html_part =
- renderer::render_template(TemplateRenderer::Html, body_template, data)?;
+ renderer::render_template(TemplateType::HtmlBody, template_name, data)?;
let text_part =
- renderer::render_template(TemplateRenderer::Plaintext, body_template, data)?;
+ renderer::render_template(TemplateType::PlaintextBody, template_name, data)?;
email_builder = email_builder.subject(subject);
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index ee1e445..9f8683f 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -162,10 +162,8 @@ pub trait Endpoint {
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,
+ /// Name of the used template
+ template_name: String,
/// Data that can be used for template rendering.
data: Value,
},
@@ -203,10 +201,9 @@ pub struct Notification {
}
impl Notification {
- pub fn new_templated<S: AsRef<str>>(
+ pub fn from_template<S: AsRef<str>>(
severity: Severity,
- title: S,
- body: S,
+ template_name: S,
template_data: Value,
fields: HashMap<String, String>,
) -> Self {
@@ -217,8 +214,7 @@ impl Notification {
timestamp: proxmox_time::epoch_i64(),
},
content: Content::Template {
- title_template: title.as_ref().to_string(),
- body_template: body.as_ref().to_string(),
+ template_name: template_name.as_ref().to_string(),
data: template_data,
},
}
@@ -549,8 +545,7 @@ impl Bus {
timestamp: proxmox_time::epoch_i64(),
},
content: Content::Template {
- title_template: "Test notification".into(),
- body_template: "This is a test of the notification target '{{ target }}'".into(),
+ template_name: "test".to_string(),
data: json!({ "target": target }),
},
};
@@ -623,10 +618,9 @@ mod tests {
bus.add_matcher(matcher);
// Send directly to endpoint
- bus.send(&Notification::new_templated(
+ bus.send(&Notification::from_template(
Severity::Info,
- "Title",
- "Body",
+ "test",
Default::default(),
Default::default(),
));
@@ -661,10 +655,9 @@ mod tests {
});
let send_with_severity = |severity| {
- let notification = Notification::new_templated(
+ let notification = Notification::from_template(
severity,
- "Title",
- "Body",
+ "test",
Default::default(),
Default::default(),
);
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index 0f86445..d6051ac 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -456,7 +456,7 @@ mod tests {
fields.insert("foo".into(), "bar".into());
let notification =
- Notification::new_templated(Severity::Notice, "test", "test", Value::Null, fields);
+ Notification::from_template(Severity::Notice, "test", Value::Null, fields);
let matcher: FieldMatcher = "exact:foo=bar".parse().unwrap();
assert!(matcher.matches(¬ification).unwrap());
@@ -474,14 +474,14 @@ mod tests {
fields.insert("foo".into(), "test".into());
let notification =
- Notification::new_templated(Severity::Notice, "test", "test", Value::Null, fields);
+ Notification::from_template(Severity::Notice, "test", Value::Null, fields);
assert!(matcher.matches(¬ification).unwrap());
let mut fields = HashMap::new();
fields.insert("foo".into(), "notthere".into());
let notification =
- Notification::new_templated(Severity::Notice, "test", "test", Value::Null, fields);
+ Notification::from_template(Severity::Notice, "test", Value::Null, fields);
assert!(!matcher.matches(¬ification).unwrap());
assert!("regex:'3=b.*".parse::<FieldMatcher>().is_err());
@@ -489,13 +489,8 @@ mod tests {
}
#[test]
fn test_severities() {
- let notification = Notification::new_templated(
- Severity::Notice,
- "test",
- "test",
- Value::Null,
- Default::default(),
- );
+ let notification =
+ Notification::from_template(Severity::Notice, "test", Value::Null, Default::default());
let matcher: SeverityMatcher = "info,notice,warning,error".parse().unwrap();
assert!(matcher.matches(¬ification).unwrap());
@@ -503,13 +498,8 @@ mod tests {
#[test]
fn test_empty_matcher_matches_always() {
- let notification = Notification::new_templated(
- Severity::Notice,
- "test",
- "test",
- Value::Null,
- Default::default(),
- );
+ let notification =
+ Notification::from_template(Severity::Notice, "test", Value::Null, Default::default());
for mode in [MatchModeOperator::All, MatchModeOperator::Any] {
let config = MatcherConfig {
diff --git a/proxmox-notify/src/renderer/html.rs b/proxmox-notify/src/renderer/html.rs
index f794902..77545f7 100644
--- a/proxmox-notify/src/renderer/html.rs
+++ b/proxmox-notify/src/renderer/html.rs
@@ -5,7 +5,6 @@ use handlebars::{
use serde_json::Value;
use super::{table::Table, value_to_string};
-use crate::define_helper_with_prefix_and_postfix;
use crate::renderer::BlockRenderFunctions;
fn render_html_table(
@@ -79,22 +78,9 @@ fn render_object(
Ok(())
}
-define_helper_with_prefix_and_postfix!(verbatim_monospaced, "<pre>", "</pre>");
-define_helper_with_prefix_and_postfix!(heading_1, "<h1 style=\"font-size: 1.2em\">", "</h1>");
-define_helper_with_prefix_and_postfix!(heading_2, "<h2 style=\"font-size: 1em\">", "</h2>");
-define_helper_with_prefix_and_postfix!(
- verbatim,
- "<pre style=\"font-family: sans-serif\">",
- "</pre>"
-);
-
pub(super) fn block_render_functions() -> BlockRenderFunctions {
BlockRenderFunctions {
table: Box::new(render_html_table),
- verbatim_monospaced: Box::new(verbatim_monospaced),
object: Box::new(render_object),
- heading_1: Box::new(heading_1),
- heading_2: Box::new(heading_2),
- verbatim: Box::new(verbatim),
}
}
diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs
index e9f36e6..a51ece6 100644
--- a/proxmox-notify/src/renderer/mod.rs
+++ b/proxmox-notify/src/renderer/mod.rs
@@ -12,7 +12,7 @@ use serde_json::Value;
use proxmox_human_byte::HumanByte;
use proxmox_time::TimeSpan;
-use crate::Error;
+use crate::{context, Error};
mod html;
mod plaintext;
@@ -165,41 +165,47 @@ impl ValueRenderFunction {
}
}
-/// Available renderers for notification templates.
+/// Available template types
#[derive(Copy, Clone)]
-pub enum TemplateRenderer {
- /// Render to HTML code
- Html,
- /// Render to plain text
- Plaintext,
+pub enum TemplateType {
+ /// HTML body template
+ HtmlBody,
+ /// Plaintext body template
+ PlaintextBody,
+ /// Plaintext body template
+ Subject,
}
-impl TemplateRenderer {
- fn prefix(&self) -> &str {
+impl TemplateType {
+ fn file_suffix(&self) -> &'static str {
match self {
- TemplateRenderer::Html => "<html>\n<body>\n",
- TemplateRenderer::Plaintext => "",
+ TemplateType::HtmlBody => "body.html.hbs",
+ TemplateType::PlaintextBody => "body.txt.hbs",
+ TemplateType::Subject => "subject.txt.hbs",
}
}
- fn postfix(&self) -> &str {
- match self {
- TemplateRenderer::Html => "\n</body>\n</html>",
- TemplateRenderer::Plaintext => "",
+ fn postprocess(&self, mut rendered: String) -> String {
+ if let Self::Subject = self {
+ rendered = rendered.replace('\n', " ");
}
+
+ rendered
}
fn block_render_fns(&self) -> BlockRenderFunctions {
match self {
- TemplateRenderer::Html => html::block_render_functions(),
- TemplateRenderer::Plaintext => plaintext::block_render_functions(),
+ TemplateType::HtmlBody => html::block_render_functions(),
+ TemplateType::Subject => plaintext::block_render_functions(),
+ TemplateType::PlaintextBody => plaintext::block_render_functions(),
}
}
fn escape_fn(&self) -> fn(&str) -> String {
match self {
- TemplateRenderer::Html => handlebars::html_escape,
- TemplateRenderer::Plaintext => handlebars::no_escape,
+ TemplateType::PlaintextBody => handlebars::no_escape,
+ TemplateType::Subject => handlebars::no_escape,
+ TemplateType::HtmlBody => handlebars::html_escape,
}
}
}
@@ -208,28 +214,20 @@ type HelperFn = dyn HelperDef + Send + Sync;
struct BlockRenderFunctions {
table: Box<HelperFn>,
- verbatim_monospaced: Box<HelperFn>,
object: Box<HelperFn>,
- heading_1: Box<HelperFn>,
- heading_2: Box<HelperFn>,
- verbatim: Box<HelperFn>,
}
impl BlockRenderFunctions {
fn register_helpers(self, handlebars: &mut Handlebars) {
handlebars.register_helper("table", self.table);
- handlebars.register_helper("verbatim", self.verbatim);
- handlebars.register_helper("verbatim-monospaced", self.verbatim_monospaced);
handlebars.register_helper("object", self.object);
- handlebars.register_helper("heading-1", self.heading_1);
- handlebars.register_helper("heading-2", self.heading_2);
}
}
fn render_template_impl(
template: &str,
data: &Value,
- renderer: TemplateRenderer,
+ renderer: TemplateType,
) -> Result<String, Error> {
let mut handlebars = Handlebars::new();
handlebars.register_escape_fn(renderer.escape_fn());
@@ -248,61 +246,45 @@ fn render_template_impl(
/// Render a template string.
///
-/// The output format can be chosen via the `renderer` parameter (see [TemplateRenderer]
+/// The output format can be chosen via the `renderer` parameter (see [TemplateType]
/// for available options).
pub fn render_template(
- renderer: TemplateRenderer,
+ mut ty: TemplateType,
template: &str,
data: &Value,
) -> Result<String, Error> {
- let mut rendered_template = String::from(renderer.prefix());
+ let filename = format!("{template}-{suffix}", suffix = ty.file_suffix());
+
+ let template_string = context::context().lookup_template(&filename, None)?;
+
+ let (template_string, fallback) = match (template_string, ty) {
+ (None, TemplateType::HtmlBody) => {
+ ty = TemplateType::PlaintextBody;
+ let plaintext_filename = format!("{template}-{suffix}", suffix = ty.file_suffix());
+ log::info!("html template '{filename}' not found, falling back to plain text template '{plaintext_filename}'");
+ (
+ context::context().lookup_template(&plaintext_filename, None)?,
+ true,
+ )
+ }
+ (template_string, _) => (template_string, false),
+ };
- rendered_template.push_str(&render_template_impl(template, data, renderer)?);
- rendered_template.push_str(renderer.postfix());
+ let template_string = template_string.ok_or(Error::Generic(format!(
+ "could not load template '{template}'"
+ )))?;
- Ok(rendered_template)
-}
+ let mut rendered = render_template_impl(&template_string, data, ty)?;
+ rendered = ty.postprocess(rendered);
-#[macro_export]
-macro_rules! define_helper_with_prefix_and_postfix {
- ($name:ident, $pre:expr, $post:expr) => {
- fn $name<'reg, 'rc>(
- h: &Helper<'reg, 'rc>,
- handlebars: &'reg Handlebars,
- context: &'rc Context,
- render_context: &mut RenderContext<'reg, 'rc>,
- out: &mut dyn Output,
- ) -> HelperResult {
- use handlebars::Renderable;
-
- let block_text = h.template();
- let param = h.param(0);
-
- out.write($pre)?;
- match (param, block_text) {
- (None, Some(block_text)) => {
- block_text.render(handlebars, context, render_context, out)
- }
- (Some(param), None) => {
- let value = param.value();
- let text = value.as_str().ok_or_else(|| {
- HandlebarsRenderError::new(format!("value {value} is not a string"))
- })?;
+ if fallback {
+ rendered = format!(
+ "<html><body><pre>{}</pre></body></html>",
+ handlebars::html_escape(&rendered)
+ );
+ }
- out.write(text)?;
- Ok(())
- }
- (Some(_), Some(_)) => Err(HandlebarsRenderError::new(
- "Cannot use parameter and template at the same time",
- )),
- (None, None) => Err(HandlebarsRenderError::new(
- "Neither parameter nor template was provided",
- )),
- }?;
- out.write($post)?;
- Ok(())
- }
- };
+ Ok(rendered)
}
#[cfg(test)]
@@ -310,73 +292,6 @@ mod tests {
use super::*;
use serde_json::json;
- #[test]
- fn test_render_template() -> Result<(), Error> {
- let data = json!({
- "dur": 12345,
- "size": 1024 * 15,
-
- "table": {
- "schema": {
- "columns": [
- {
- "id": "col1",
- "label": "Column 1"
- },
- {
- "id": "col2",
- "label": "Column 2"
- }
- ]
- },
- "data": [
- {
- "col1": "val1",
- "col2": "val2"
- },
- {
- "col1": "val3",
- "col2": "val4"
- },
- ]
- }
-
- });
-
- let template = r#"
-{{heading-1 "Hello World"}}
-
-{{heading-2 "Hello World"}}
-
-{{human-bytes size}}
-{{duration dur}}
-
-{{table table}}"#;
-
- let expected_plaintext = r#"
-Hello World
-===========
-
-Hello World
------------
-
-15 KiB
-3h 25min 45s
-
-Column 1 Column 2
-val1 val2
-val3 val4
-"#;
-
- let rendered_plaintext = render_template(TemplateRenderer::Plaintext, template, &data)?;
-
- // Let's not bother about testing the HTML output, too fragile.
-
- assert_eq!(rendered_plaintext, expected_plaintext);
-
- Ok(())
- }
-
#[test]
fn test_helpers() {
assert_eq!(value_to_byte_size(&json!(1024)), Some("1 KiB".to_string()));
diff --git a/proxmox-notify/src/renderer/plaintext.rs b/proxmox-notify/src/renderer/plaintext.rs
index 437e412..59e917a 100644
--- a/proxmox-notify/src/renderer/plaintext.rs
+++ b/proxmox-notify/src/renderer/plaintext.rs
@@ -7,7 +7,6 @@ use handlebars::{
use serde_json::Value;
use super::{table::Table, value_to_string};
-use crate::define_helper_with_prefix_and_postfix;
use crate::renderer::BlockRenderFunctions;
fn optimal_column_widths(table: &Table) -> HashMap<&str, usize> {
@@ -76,40 +75,6 @@ fn render_plaintext_table(
Ok(())
}
-macro_rules! define_underlining_heading_fn {
- ($name:ident, $underline:expr) => {
- fn $name<'reg, 'rc>(
- h: &Helper<'reg, 'rc>,
- _handlebars: &'reg Handlebars,
- _context: &'rc Context,
- _render_context: &mut RenderContext<'reg, 'rc>,
- out: &mut dyn Output,
- ) -> HelperResult {
- let param = h
- .param(0)
- .ok_or_else(|| HandlebarsRenderError::new("No parameter provided"))?;
-
- let value = param.value();
- let text = value.as_str().ok_or_else(|| {
- HandlebarsRenderError::new(format!("value {value} is not a string"))
- })?;
-
- out.write(text)?;
- out.write("\n")?;
-
- for _ in 0..text.len() {
- out.write($underline)?;
- }
- Ok(())
- }
- };
-}
-
-define_helper_with_prefix_and_postfix!(verbatim_monospaced, "", "");
-define_underlining_heading_fn!(heading_1, "=");
-define_underlining_heading_fn!(heading_2, "-");
-define_helper_with_prefix_and_postfix!(verbatim, "", "");
-
fn render_object(
h: &Helper,
_: &Handlebars,
@@ -133,10 +98,6 @@ fn render_object(
pub(super) fn block_render_functions() -> BlockRenderFunctions {
BlockRenderFunctions {
table: Box::new(render_plaintext_table),
- verbatim_monospaced: Box::new(verbatim_monospaced),
- verbatim: Box::new(verbatim),
object: Box::new(render_object),
- heading_1: Box::new(heading_1),
- heading_2: Box::new(heading_2),
}
}
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 02/20] notify: make api methods take config struct ownership
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 01/20] notify: switch to file-based templating system Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 03/20] notify: convert Option<Vec<T>> -> Vec<T> in config structs Lukas Wagner
` (18 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
This saves us from some of the awkward cloning steps when updating.
Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/api/gotify.rs | 46 +++++++++---------
proxmox-notify/src/api/matcher.rs | 38 +++++++--------
proxmox-notify/src/api/mod.rs | 14 +++---
proxmox-notify/src/api/sendmail.rs | 40 +++++++--------
proxmox-notify/src/api/smtp.rs | 78 +++++++++++++++---------------
5 files changed, 108 insertions(+), 108 deletions(-)
diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index a93a024..15d94cb 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -41,8 +41,8 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<GotifyConfig, HttpErr
/// Panics if the names of the private config and the public config do not match.
pub fn add_endpoint(
config: &mut Config,
- endpoint_config: &GotifyConfig,
- private_endpoint_config: &GotifyPrivateConfig,
+ endpoint_config: GotifyConfig,
+ private_endpoint_config: GotifyPrivateConfig,
) -> Result<(), HttpError> {
if endpoint_config.name != private_endpoint_config.name {
// Programming error by the user of the crate, thus we panic
@@ -51,11 +51,11 @@ pub fn add_endpoint(
super::ensure_unique(config, &endpoint_config.name)?;
- set_private_config_entry(config, private_endpoint_config)?;
+ set_private_config_entry(config, &private_endpoint_config)?;
config
.config
- .set_data(&endpoint_config.name, GOTIFY_TYPENAME, endpoint_config)
+ .set_data(&endpoint_config.name, GOTIFY_TYPENAME, &endpoint_config)
.map_err(|e| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -75,8 +75,8 @@ pub fn add_endpoint(
pub fn update_endpoint(
config: &mut Config,
name: &str,
- endpoint_config_updater: &GotifyConfigUpdater,
- private_endpoint_config_updater: &GotifyPrivateConfigUpdater,
+ endpoint_config_updater: GotifyConfigUpdater,
+ private_endpoint_config_updater: GotifyPrivateConfigUpdater,
delete: Option<&[DeleteableGotifyProperty]>,
digest: Option<&[u8]>,
) -> Result<(), HttpError> {
@@ -93,11 +93,11 @@ pub fn update_endpoint(
}
}
- if let Some(server) = &endpoint_config_updater.server {
- endpoint.server = server.into();
+ if let Some(server) = endpoint_config_updater.server {
+ endpoint.server = server;
}
- if let Some(token) = &private_endpoint_config_updater.token {
+ if let Some(token) = private_endpoint_config_updater.token {
set_private_config_entry(
config,
&GotifyPrivateConfig {
@@ -107,12 +107,12 @@ pub fn update_endpoint(
)?;
}
- if let Some(comment) = &endpoint_config_updater.comment {
- endpoint.comment = Some(comment.into());
+ if let Some(comment) = endpoint_config_updater.comment {
+ endpoint.comment = Some(comment)
}
- if let Some(disable) = &endpoint_config_updater.disable {
- endpoint.disable = Some(*disable);
+ if let Some(disable) = endpoint_config_updater.disable {
+ endpoint.disable = Some(disable);
}
config
@@ -173,13 +173,13 @@ mod tests {
pub fn add_default_gotify_endpoint(config: &mut Config) -> Result<(), HttpError> {
add_endpoint(
config,
- &GotifyConfig {
+ GotifyConfig {
name: "gotify-endpoint".into(),
server: "localhost".into(),
comment: Some("comment".into()),
..Default::default()
},
- &GotifyPrivateConfig {
+ GotifyPrivateConfig {
name: "gotify-endpoint".into(),
token: "supersecrettoken".into(),
},
@@ -196,8 +196,8 @@ mod tests {
assert!(update_endpoint(
&mut config,
"test",
- &Default::default(),
- &Default::default(),
+ Default::default(),
+ Default::default(),
None,
None
)
@@ -214,8 +214,8 @@ mod tests {
assert!(update_endpoint(
&mut config,
"gotify-endpoint",
- &Default::default(),
- &Default::default(),
+ Default::default(),
+ Default::default(),
None,
Some(&[0; 32])
)
@@ -234,12 +234,12 @@ mod tests {
update_endpoint(
&mut config,
"gotify-endpoint",
- &GotifyConfigUpdater {
+ GotifyConfigUpdater {
server: Some("newhost".into()),
comment: Some("newcomment".into()),
..Default::default()
},
- &GotifyPrivateConfigUpdater {
+ GotifyPrivateConfigUpdater {
token: Some("changedtoken".into()),
},
None,
@@ -263,8 +263,8 @@ mod tests {
update_endpoint(
&mut config,
"gotify-endpoint",
- &Default::default(),
- &Default::default(),
+ Default::default(),
+ Default::default(),
Some(&[DeleteableGotifyProperty::Comment]),
None,
)?;
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
index ca01bc9..f0eabd9 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -36,7 +36,7 @@ pub fn get_matcher(config: &Config, name: &str) -> Result<MatcherConfig, HttpErr
/// 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> {
+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() {
@@ -45,7 +45,7 @@ pub fn add_matcher(config: &mut Config, matcher_config: &MatcherConfig) -> Resul
config
.config
- .set_data(&matcher_config.name, MATCHER_TYPENAME, matcher_config)
+ .set_data(&matcher_config.name, MATCHER_TYPENAME, &matcher_config)
.map_err(|e| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -67,7 +67,7 @@ pub fn add_matcher(config: &mut Config, matcher_config: &MatcherConfig) -> Resul
pub fn update_matcher(
config: &mut Config,
name: &str,
- matcher_updater: &MatcherConfigUpdater,
+ matcher_updater: MatcherConfigUpdater,
delete: Option<&[DeleteableMatcherProperty]>,
digest: Option<&[u8]>,
) -> Result<(), HttpError> {
@@ -90,16 +90,16 @@ pub fn update_matcher(
}
}
- if let Some(match_severity) = &matcher_updater.match_severity {
- matcher.match_severity = Some(match_severity.clone());
+ if let Some(match_severity) = matcher_updater.match_severity {
+ matcher.match_severity = Some(match_severity);
}
- if let Some(match_field) = &matcher_updater.match_field {
- matcher.match_field = Some(match_field.clone());
+ if let Some(match_field) = matcher_updater.match_field {
+ matcher.match_field = Some(match_field);
}
- if let Some(match_calendar) = &matcher_updater.match_calendar {
- matcher.match_calendar = Some(match_calendar.clone());
+ if let Some(match_calendar) = matcher_updater.match_calendar {
+ matcher.match_calendar = Some(match_calendar);
}
if let Some(mode) = matcher_updater.mode {
@@ -110,17 +110,17 @@ pub fn update_matcher(
matcher.invert_match = Some(invert_match);
}
- if let Some(comment) = &matcher_updater.comment {
- matcher.comment = Some(comment.into());
+ if let Some(comment) = matcher_updater.comment {
+ matcher.comment = Some(comment);
}
- if let Some(disable) = &matcher_updater.disable {
- matcher.disable = Some(*disable);
+ if let Some(disable) = matcher_updater.disable {
+ matcher.disable = Some(disable);
}
- if let Some(target) = &matcher_updater.target {
+ if let Some(target) = matcher_updater.target {
super::ensure_endpoints_exist(config, target.as_slice())?;
- matcher.target = Some(target.clone());
+ matcher.target = Some(target);
}
config
@@ -178,7 +178,7 @@ matcher: matcher2
#[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());
+ assert!(update_matcher(&mut config, "test", Default::default(), None, None).is_err());
Ok(())
}
@@ -188,7 +188,7 @@ matcher: matcher2
assert!(update_matcher(
&mut config,
"matcher1",
- &Default::default(),
+ Default::default(),
None,
Some(&[0u8; 32])
)
@@ -206,7 +206,7 @@ matcher: matcher2
update_matcher(
&mut config,
"matcher1",
- &MatcherConfigUpdater {
+ MatcherConfigUpdater {
mode: Some(MatchModeOperator::Any),
match_field: None,
match_severity: None,
@@ -230,7 +230,7 @@ matcher: matcher2
update_matcher(
&mut config,
"matcher1",
- &Default::default(),
+ Default::default(),
Some(&[
DeleteableMatcherProperty::InvertMatch,
DeleteableMatcherProperty::Mode,
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index 7cc2593..bb0371d 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -173,13 +173,13 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet<String> {
#[allow(unused)]
fn set_private_config_entry<T: Serialize>(
config: &mut Config,
- private_config: &T,
+ private_config: T,
typename: &str,
name: &str,
) -> Result<(), HttpError> {
config
.private_config
- .set_data(name, typename, private_config)
+ .set_data(name, typename, &private_config)
.map_err(|e| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -217,7 +217,7 @@ mod tests {
sendmail::add_endpoint(
&mut config,
- &SendmailConfig {
+ SendmailConfig {
name: "sendmail".to_string(),
mailto: Some(vec!["foo@example.com".to_string()]),
..Default::default()
@@ -226,7 +226,7 @@ mod tests {
sendmail::add_endpoint(
&mut config,
- &SendmailConfig {
+ SendmailConfig {
name: "builtin".to_string(),
mailto: Some(vec!["foo@example.com".to_string()]),
origin: Some(Origin::Builtin),
@@ -236,12 +236,12 @@ mod tests {
gotify::add_endpoint(
&mut config,
- &GotifyConfig {
+ GotifyConfig {
name: "gotify".to_string(),
server: "localhost".to_string(),
..Default::default()
},
- &GotifyPrivateConfig {
+ GotifyPrivateConfig {
name: "gotify".to_string(),
token: "foo".to_string(),
},
@@ -249,7 +249,7 @@ mod tests {
matcher::add_matcher(
&mut config,
- &MatcherConfig {
+ MatcherConfig {
name: "matcher".to_string(),
target: Some(vec![
"sendmail".to_string(),
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index e911505..3285ac7 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -37,7 +37,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SendmailConfig, HttpE
/// - an entity with the same name already exists (`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> {
+pub fn add_endpoint(config: &mut Config, endpoint: SendmailConfig) -> Result<(), HttpError> {
super::ensure_unique(config, &endpoint.name)?;
if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
@@ -49,7 +49,7 @@ pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<()
config
.config
- .set_data(&endpoint.name, SENDMAIL_TYPENAME, endpoint)
+ .set_data(&endpoint.name, SENDMAIL_TYPENAME, &endpoint)
.map_err(|e| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -69,7 +69,7 @@ pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<()
pub fn update_endpoint(
config: &mut Config,
name: &str,
- updater: &SendmailConfigUpdater,
+ updater: SendmailConfigUpdater,
delete: Option<&[DeleteableSendmailProperty]>,
digest: Option<&[u8]>,
) -> Result<(), HttpError> {
@@ -90,28 +90,28 @@ pub fn update_endpoint(
}
}
- if let Some(mailto) = &updater.mailto {
- endpoint.mailto = Some(mailto.iter().map(String::from).collect());
+ if let Some(mailto) = updater.mailto {
+ endpoint.mailto = Some(mailto);
}
- if let Some(mailto_user) = &updater.mailto_user {
- endpoint.mailto_user = Some(mailto_user.iter().map(String::from).collect());
+ if let Some(mailto_user) = updater.mailto_user {
+ endpoint.mailto_user = Some(mailto_user);
}
- if let Some(from_address) = &updater.from_address {
- endpoint.from_address = Some(from_address.into());
+ if let Some(from_address) = updater.from_address {
+ endpoint.from_address = Some(from_address);
}
- if let Some(author) = &updater.author {
- endpoint.author = Some(author.into());
+ if let Some(author) = updater.author {
+ endpoint.author = Some(author);
}
- if let Some(comment) = &updater.comment {
- endpoint.comment = Some(comment.into());
+ if let Some(comment) = updater.comment {
+ endpoint.comment = Some(comment);
}
- if let Some(disable) = &updater.disable {
- endpoint.disable = Some(*disable);
+ if let Some(disable) = updater.disable {
+ endpoint.disable = Some(disable);
}
if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
@@ -162,7 +162,7 @@ pub mod tests {
) -> Result<(), HttpError> {
add_endpoint(
config,
- &SendmailConfig {
+ SendmailConfig {
name: name.into(),
mailto: Some(vec!["user1@example.com".into()]),
mailto_user: None,
@@ -193,7 +193,7 @@ pub mod tests {
fn test_update_not_existing_returns_error() -> Result<(), HttpError> {
let mut config = empty_config();
- assert!(update_endpoint(&mut config, "test", &Default::default(), None, None,).is_err());
+ assert!(update_endpoint(&mut config, "test", Default::default(), None, None,).is_err());
Ok(())
}
@@ -206,7 +206,7 @@ pub mod tests {
assert!(update_endpoint(
&mut config,
"sendmail-endpoint",
- &SendmailConfigUpdater {
+ SendmailConfigUpdater {
mailto: Some(vec!["user2@example.com".into(), "user3@example.com".into()]),
mailto_user: None,
from_address: Some("root@example.com".into()),
@@ -232,7 +232,7 @@ pub mod tests {
update_endpoint(
&mut config,
"sendmail-endpoint",
- &SendmailConfigUpdater {
+ SendmailConfigUpdater {
mailto: Some(vec!["user2@example.com".into(), "user3@example.com".into()]),
mailto_user: Some(vec!["root@pam".into()]),
from_address: Some("root@example.com".into()),
@@ -262,7 +262,7 @@ pub mod tests {
update_endpoint(
&mut config,
"sendmail-endpoint",
- &Default::default(),
+ Default::default(),
Some(&[
DeleteableSendmailProperty::FromAddress,
DeleteableSendmailProperty::Author,
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 6bd0c4b..16d103c 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -40,8 +40,8 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SmtpConfig, HttpError
/// - mailto *and* mailto_user are both set to `None`
pub fn add_endpoint(
config: &mut Config,
- endpoint_config: &SmtpConfig,
- private_endpoint_config: &SmtpPrivateConfig,
+ endpoint_config: SmtpConfig,
+ private_endpoint_config: SmtpPrivateConfig,
) -> Result<(), HttpError> {
if endpoint_config.name != private_endpoint_config.name {
// Programming error by the user of the crate, thus we panic
@@ -66,7 +66,7 @@ pub fn add_endpoint(
config
.config
- .set_data(&endpoint_config.name, SMTP_TYPENAME, endpoint_config)
+ .set_data(&endpoint_config.name, SMTP_TYPENAME, &endpoint_config)
.map_err(|e| {
http_err!(
INTERNAL_SERVER_ERROR,
@@ -86,8 +86,8 @@ pub fn add_endpoint(
pub fn update_endpoint(
config: &mut Config,
name: &str,
- updater: &SmtpConfigUpdater,
- private_endpoint_config_updater: &SmtpPrivateConfigUpdater,
+ updater: SmtpConfigUpdater,
+ private_endpoint_config_updater: SmtpPrivateConfigUpdater,
delete: Option<&[DeleteableSmtpProperty]>,
digest: Option<&[u8]>,
) -> Result<(), HttpError> {
@@ -105,7 +105,7 @@ pub fn update_endpoint(
DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user = None,
DeleteableSmtpProperty::Password => super::set_private_config_entry(
config,
- &SmtpPrivateConfig {
+ SmtpPrivateConfig {
name: name.to_string(),
password: None,
},
@@ -118,49 +118,49 @@ pub fn update_endpoint(
}
}
- if let Some(mailto) = &updater.mailto {
- endpoint.mailto = Some(mailto.iter().map(String::from).collect());
+ if let Some(mailto) = updater.mailto {
+ endpoint.mailto = Some(mailto);
}
- if let Some(mailto_user) = &updater.mailto_user {
- endpoint.mailto_user = Some(mailto_user.iter().map(String::from).collect());
+ if let Some(mailto_user) = updater.mailto_user {
+ endpoint.mailto_user = Some(mailto_user);
}
- if let Some(from_address) = &updater.from_address {
- endpoint.from_address = from_address.into();
+ if let Some(from_address) = updater.from_address {
+ endpoint.from_address = from_address;
}
- if let Some(server) = &updater.server {
- endpoint.server = server.into();
+ if let Some(server) = updater.server {
+ endpoint.server = server;
}
- if let Some(port) = &updater.port {
- endpoint.port = Some(*port);
+ if let Some(port) = updater.port {
+ endpoint.port = Some(port);
}
- if let Some(username) = &updater.username {
- endpoint.username = Some(username.into());
+ if let Some(username) = updater.username {
+ endpoint.username = Some(username);
}
- if let Some(mode) = &updater.mode {
- endpoint.mode = Some(*mode);
+ if let Some(mode) = updater.mode {
+ endpoint.mode = Some(mode);
}
- if let Some(password) = &private_endpoint_config_updater.password {
+ if let Some(password) = private_endpoint_config_updater.password {
super::set_private_config_entry(
config,
- &SmtpPrivateConfig {
+ SmtpPrivateConfig {
name: name.into(),
- password: Some(password.into()),
+ password: Some(password),
},
SMTP_TYPENAME,
name,
)?;
}
- if let Some(author) = &updater.author {
- endpoint.author = Some(author.into());
+ if let Some(author) = updater.author {
+ endpoint.author = Some(author);
}
- if let Some(comment) = &updater.comment {
- endpoint.comment = Some(comment.into());
+ if let Some(comment) = updater.comment {
+ endpoint.comment = Some(comment);
}
- if let Some(disable) = &updater.disable {
- endpoint.disable = Some(*disable);
+ if let Some(disable) = updater.disable {
+ endpoint.disable = Some(disable);
}
if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
@@ -209,7 +209,7 @@ pub mod tests {
pub fn add_smtp_endpoint_for_test(config: &mut Config, name: &str) -> Result<(), HttpError> {
add_endpoint(
config,
- &SmtpConfig {
+ SmtpConfig {
name: name.into(),
mailto: Some(vec!["user1@example.com".into()]),
mailto_user: None,
@@ -222,7 +222,7 @@ pub mod tests {
username: Some("username".into()),
..Default::default()
},
- &SmtpPrivateConfig {
+ SmtpPrivateConfig {
name: name.into(),
password: Some("password".into()),
},
@@ -252,8 +252,8 @@ pub mod tests {
assert!(update_endpoint(
&mut config,
"test",
- &Default::default(),
- &Default::default(),
+ Default::default(),
+ Default::default(),
None,
None,
)
@@ -270,8 +270,8 @@ pub mod tests {
assert!(update_endpoint(
&mut config,
"sendmail-endpoint",
- &Default::default(),
- &Default::default(),
+ Default::default(),
+ Default::default(),
None,
Some(&[0; 32]),
)
@@ -290,7 +290,7 @@ pub mod tests {
update_endpoint(
&mut config,
"smtp-endpoint",
- &SmtpConfigUpdater {
+ SmtpConfigUpdater {
mailto: Some(vec!["user2@example.com".into(), "user3@example.com".into()]),
mailto_user: Some(vec!["root@pam".into()]),
from_address: Some("root@example.com".into()),
@@ -302,7 +302,7 @@ pub mod tests {
username: Some("newusername".into()),
..Default::default()
},
- &Default::default(),
+ Default::default(),
None,
Some(&digest),
)?;
@@ -325,8 +325,8 @@ pub mod tests {
update_endpoint(
&mut config,
"smtp-endpoint",
- &Default::default(),
- &Default::default(),
+ Default::default(),
+ Default::default(),
Some(&[
DeleteableSmtpProperty::Author,
DeleteableSmtpProperty::MailtoUser,
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 03/20] notify: convert Option<Vec<T>> -> Vec<T> in config structs
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 01/20] notify: switch to file-based templating system Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 02/20] notify: make api methods take config struct ownership Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 04/20] notify: don't make tests require pve-context Lukas Wagner
` (17 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/api/matcher.rs | 27 +++++++--------
proxmox-notify/src/api/mod.rs | 22 +++++-------
proxmox-notify/src/api/sendmail.rs | 22 ++++++------
proxmox-notify/src/api/smtp.rs | 24 ++++++-------
proxmox-notify/src/endpoints/common/mail.rs | 20 ++++-------
proxmox-notify/src/endpoints/sendmail.rs | 14 ++++----
proxmox-notify/src/endpoints/smtp.rs | 18 +++++-----
proxmox-notify/src/lib.rs | 10 +++---
proxmox-notify/src/matcher.rs | 38 ++++++++++++---------
9 files changed, 96 insertions(+), 99 deletions(-)
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
index f0eabd9..63ec73d 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -38,10 +38,7 @@ pub fn get_matcher(config: &Config, name: &str) -> Result<MatcherConfig, HttpErr
/// - 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)?;
- }
+ super::ensure_endpoints_exist(config, &matcher_config.target)?;
config
.config
@@ -78,10 +75,10 @@ pub fn update_matcher(
if let Some(delete) = delete {
for deleteable_property in delete {
match deleteable_property {
- DeleteableMatcherProperty::MatchSeverity => matcher.match_severity = None,
- DeleteableMatcherProperty::MatchField => matcher.match_field = None,
- DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar = None,
- DeleteableMatcherProperty::Target => matcher.target = None,
+ DeleteableMatcherProperty::MatchSeverity => matcher.match_severity.clear(),
+ DeleteableMatcherProperty::MatchField => matcher.match_field.clear(),
+ DeleteableMatcherProperty::MatchCalendar => matcher.match_calendar.clear(),
+ DeleteableMatcherProperty::Target => matcher.target.clear(),
DeleteableMatcherProperty::Mode => matcher.mode = None,
DeleteableMatcherProperty::InvertMatch => matcher.invert_match = None,
DeleteableMatcherProperty::Comment => matcher.comment = None,
@@ -91,15 +88,15 @@ pub fn update_matcher(
}
if let Some(match_severity) = matcher_updater.match_severity {
- matcher.match_severity = Some(match_severity);
+ matcher.match_severity = match_severity;
}
if let Some(match_field) = matcher_updater.match_field {
- matcher.match_field = Some(match_field);
+ matcher.match_field = match_field;
}
if let Some(match_calendar) = matcher_updater.match_calendar {
- matcher.match_calendar = Some(match_calendar);
+ matcher.match_calendar = match_calendar;
}
if let Some(mode) = matcher_updater.mode {
@@ -120,7 +117,7 @@ pub fn update_matcher(
if let Some(target) = matcher_updater.target {
super::ensure_endpoints_exist(config, target.as_slice())?;
- matcher.target = Some(target);
+ matcher.target = target;
}
config
@@ -244,9 +241,9 @@ matcher: matcher2
let matcher = get_matcher(&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.match_severity.is_empty());
+ assert!(matcher.match_field.is_empty());
+ assert!(matcher.target.is_empty());
assert!(matcher.mode.is_none());
assert_eq!(matcher.comment, None);
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index bb0371d..a2cf29e 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -102,10 +102,8 @@ fn get_referrers(config: &Config, entity: &str) -> Result<HashSet<String>, HttpE
let mut referrers = HashSet::new();
for matcher in matcher::get_matchers(config)? {
- if let Some(targets) = matcher.target {
- if targets.iter().any(|target| target == entity) {
- referrers.insert(matcher.name.clone());
- }
+ if matcher.target.iter().any(|target| target == entity) {
+ referrers.insert(matcher.name.clone());
}
}
@@ -149,11 +147,9 @@ fn get_referenced_entities(config: &Config, entity: &str) -> HashSet<String> {
let mut new = HashSet::new();
for entity in entities {
- if let Ok(group) = matcher::get_matcher(config, entity) {
- if let Some(targets) = group.target {
- for target in targets {
- new.insert(target.clone());
- }
+ if let Ok(matcher) = matcher::get_matcher(config, entity) {
+ for target in matcher.target {
+ new.insert(target.clone());
}
}
}
@@ -219,7 +215,7 @@ mod tests {
&mut config,
SendmailConfig {
name: "sendmail".to_string(),
- mailto: Some(vec!["foo@example.com".to_string()]),
+ mailto: vec!["foo@example.com".to_string()],
..Default::default()
},
)?;
@@ -228,7 +224,7 @@ mod tests {
&mut config,
SendmailConfig {
name: "builtin".to_string(),
- mailto: Some(vec!["foo@example.com".to_string()]),
+ mailto: vec!["foo@example.com".to_string()],
origin: Some(Origin::Builtin),
..Default::default()
},
@@ -251,11 +247,11 @@ mod tests {
&mut config,
MatcherConfig {
name: "matcher".to_string(),
- target: Some(vec![
+ target: vec![
"sendmail".to_string(),
"gotify".to_string(),
"builtin".to_string(),
- ]),
+ ],
..Default::default()
},
)?;
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index 3285ac7..c20a3e5 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -40,7 +40,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SendmailConfig, HttpE
pub fn add_endpoint(config: &mut Config, endpoint: SendmailConfig) -> Result<(), HttpError> {
super::ensure_unique(config, &endpoint.name)?;
- if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
+ if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -83,19 +83,19 @@ pub fn update_endpoint(
DeleteableSendmailProperty::FromAddress => endpoint.from_address = None,
DeleteableSendmailProperty::Author => endpoint.author = None,
DeleteableSendmailProperty::Comment => endpoint.comment = None,
- DeleteableSendmailProperty::Mailto => endpoint.mailto = None,
- DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user = None,
+ DeleteableSendmailProperty::Mailto => endpoint.mailto.clear(),
+ DeleteableSendmailProperty::MailtoUser => endpoint.mailto_user.clear(),
DeleteableSendmailProperty::Disable => endpoint.disable = None,
}
}
}
if let Some(mailto) = updater.mailto {
- endpoint.mailto = Some(mailto);
+ endpoint.mailto = mailto;
}
if let Some(mailto_user) = updater.mailto_user {
- endpoint.mailto_user = Some(mailto_user);
+ endpoint.mailto_user = mailto_user;
}
if let Some(from_address) = updater.from_address {
@@ -114,7 +114,7 @@ pub fn update_endpoint(
endpoint.disable = Some(disable);
}
- if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
+ if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -164,8 +164,8 @@ pub mod tests {
config,
SendmailConfig {
name: name.into(),
- mailto: Some(vec!["user1@example.com".into()]),
- mailto_user: None,
+ mailto: vec!["user1@example.com".into()],
+ mailto_user: vec![],
from_address: Some("from@example.com".into()),
author: Some("root".into()),
comment: Some("Comment".into()),
@@ -248,12 +248,12 @@ pub mod tests {
assert_eq!(
endpoint.mailto,
- Some(vec![
+ vec![
"user2@example.com".to_string(),
"user3@example.com".to_string()
- ])
+ ]
);
- assert_eq!(endpoint.mailto_user, Some(vec!["root@pam".to_string(),]));
+ assert_eq!(endpoint.mailto_user, vec!["root@pam".to_string(),]);
assert_eq!(endpoint.from_address, Some("root@example.com".to_string()));
assert_eq!(endpoint.author, Some("newauthor".to_string()));
assert_eq!(endpoint.comment, Some("new comment".to_string()));
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 16d103c..7a58677 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -50,7 +50,7 @@ pub fn add_endpoint(
super::ensure_unique(config, &endpoint_config.name)?;
- if endpoint_config.mailto.is_none() && endpoint_config.mailto_user.is_none() {
+ if endpoint_config.mailto.is_empty() && endpoint_config.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -101,8 +101,8 @@ pub fn update_endpoint(
DeleteableSmtpProperty::Author => endpoint.author = None,
DeleteableSmtpProperty::Comment => endpoint.comment = None,
DeleteableSmtpProperty::Disable => endpoint.disable = None,
- DeleteableSmtpProperty::Mailto => endpoint.mailto = None,
- DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user = None,
+ DeleteableSmtpProperty::Mailto => endpoint.mailto.clear(),
+ DeleteableSmtpProperty::MailtoUser => endpoint.mailto_user.clear(),
DeleteableSmtpProperty::Password => super::set_private_config_entry(
config,
SmtpPrivateConfig {
@@ -119,10 +119,10 @@ pub fn update_endpoint(
}
if let Some(mailto) = updater.mailto {
- endpoint.mailto = Some(mailto);
+ endpoint.mailto = mailto;
}
if let Some(mailto_user) = updater.mailto_user {
- endpoint.mailto_user = Some(mailto_user);
+ endpoint.mailto_user = mailto_user;
}
if let Some(from_address) = updater.from_address {
endpoint.from_address = from_address;
@@ -163,7 +163,7 @@ pub fn update_endpoint(
endpoint.disable = Some(disable);
}
- if endpoint.mailto.is_none() && endpoint.mailto_user.is_none() {
+ if endpoint.mailto.is_empty() && endpoint.mailto_user.is_empty() {
http_bail!(
BAD_REQUEST,
"must at least provide one recipient, either in mailto or in mailto-user"
@@ -211,8 +211,8 @@ pub mod tests {
config,
SmtpConfig {
name: name.into(),
- mailto: Some(vec!["user1@example.com".into()]),
- mailto_user: None,
+ mailto: vec!["user1@example.com".into()],
+ mailto_user: vec![],
from_address: "from@example.com".into(),
author: Some("root".into()),
comment: Some("Comment".into()),
@@ -311,12 +311,12 @@ pub mod tests {
assert_eq!(
endpoint.mailto,
- Some(vec![
+ vec![
"user2@example.com".to_string(),
"user3@example.com".to_string()
- ])
+ ]
);
- assert_eq!(endpoint.mailto_user, Some(vec!["root@pam".to_string(),]));
+ assert_eq!(endpoint.mailto_user, vec!["root@pam".to_string(),]);
assert_eq!(endpoint.from_address, "root@example.com".to_string());
assert_eq!(endpoint.author, Some("newauthor".to_string()));
assert_eq!(endpoint.comment, Some("new comment".to_string()));
@@ -343,7 +343,7 @@ pub mod tests {
assert_eq!(endpoint.comment, None);
assert_eq!(endpoint.port, None);
assert_eq!(endpoint.username, None);
- assert_eq!(endpoint.mailto_user, None);
+ assert!(endpoint.mailto_user.is_empty());
Ok(())
}
diff --git a/proxmox-notify/src/endpoints/common/mail.rs b/proxmox-notify/src/endpoints/common/mail.rs
index 0929d7c..b1c4823 100644
--- a/proxmox-notify/src/endpoints/common/mail.rs
+++ b/proxmox-notify/src/endpoints/common/mail.rs
@@ -2,22 +2,16 @@ use std::collections::HashSet;
use crate::context;
-pub(crate) fn get_recipients(
- email_addrs: Option<&[String]>,
- users: Option<&[String]>,
-) -> HashSet<String> {
+pub(crate) fn get_recipients(email_addrs: &[String], users: &[String]) -> HashSet<String> {
let mut recipients = HashSet::new();
- if let Some(mailto_addrs) = email_addrs {
- for addr in mailto_addrs {
- recipients.insert(addr.clone());
- }
+ for addr in email_addrs {
+ recipients.insert(addr.clone());
}
- if let Some(users) = users {
- for user in users {
- if let Some(addr) = context::context().lookup_email_for_user(user) {
- recipients.insert(addr);
- }
+
+ for user in users {
+ if let Some(addr) = context::context().lookup_email_for_user(user) {
+ recipients.insert(addr);
}
}
recipients
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index 6178b5e..fa04002 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -44,11 +44,13 @@ pub struct SendmailConfig {
#[updater(skip)]
pub name: String,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto: Vec<String>,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto_user: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto_user: Vec<String>,
/// `From` address for the mail
#[serde(skip_serializing_if = "Option::is_none")]
pub from_address: Option<String>,
@@ -90,8 +92,8 @@ pub struct SendmailEndpoint {
impl Endpoint for SendmailEndpoint {
fn send(&self, notification: &Notification) -> Result<(), Error> {
let recipients = mail::get_recipients(
- self.config.mailto.as_deref(),
- self.config.mailto_user.as_deref(),
+ self.config.mailto.as_slice(),
+ self.config.mailto_user.as_slice(),
);
let recipients_str: Vec<&str> = recipients.iter().map(String::as_str).collect();
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index f0c836a..ded5baf 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -44,8 +44,8 @@ pub enum SmtpMode {
},
mailto: {
type: Array,
- items: {
- schema: EMAIL_SCHEMA,
+ items: {
+ schema: EMAIL_SCHEMA,
},
optional: true,
},
@@ -80,11 +80,13 @@ pub struct SmtpConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub username: Option<String>,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto: Vec<String>,
/// Mail recipients
- #[serde(skip_serializing_if = "Option::is_none")]
- pub mailto_user: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub mailto_user: Vec<String>,
/// `From` address for the mail
pub from_address: String,
/// Author of the mail
@@ -177,8 +179,8 @@ impl Endpoint for SmtpEndpoint {
let transport = transport_builder.build();
let recipients = mail::get_recipients(
- self.config.mailto.as_deref(),
- self.config.mailto_user.as_deref(),
+ self.config.mailto.as_slice(),
+ self.config.mailto_user.as_slice(),
);
let mail_from = self.config.from_address.clone();
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 9f8683f..35dcb17 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -611,7 +611,7 @@ mod tests {
bus.add_endpoint(Box::new(mock.clone()));
let matcher = MatcherConfig {
- target: Some(vec!["endpoint".into()]),
+ target: vec!["endpoint".into()],
..Default::default()
};
@@ -642,15 +642,15 @@ mod tests {
bus.add_matcher(MatcherConfig {
name: "matcher1".into(),
- match_severity: Some(vec!["warning,error".parse()?]),
- target: Some(vec!["mock1".into()]),
+ match_severity: vec!["warning,error".parse()?],
+ target: vec!["mock1".into()],
..Default::default()
});
bus.add_matcher(MatcherConfig {
name: "matcher2".into(),
- match_severity: Some(vec!["error".parse()?]),
- target: Some(vec!["mock2".into()]),
+ match_severity: vec!["error".parse()?],
+ target: vec!["mock2".into()],
..Default::default()
});
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index d6051ac..b387fef 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -113,17 +113,19 @@ pub struct MatcherConfig {
pub name: String,
/// List of matched metadata fields
- #[serde(skip_serializing_if = "Option::is_none")]
- pub match_field: Option<Vec<FieldMatcher>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub match_field: Vec<FieldMatcher>,
/// List of matched severity levels
- #[serde(skip_serializing_if = "Option::is_none")]
- pub match_severity: Option<Vec<SeverityMatcher>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub match_severity: Vec<SeverityMatcher>,
/// List of matched severity levels
- #[serde(skip_serializing_if = "Option::is_none")]
- pub match_calendar: Option<Vec<CalendarMatcher>>,
-
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub match_calendar: Vec<CalendarMatcher>,
/// Decide if 'all' or 'any' match statements must match
#[serde(skip_serializing_if = "Option::is_none")]
pub mode: Option<MatchModeOperator>,
@@ -133,8 +135,9 @@ pub struct MatcherConfig {
pub invert_match: Option<bool>,
/// Targets to notify
- #[serde(skip_serializing_if = "Option::is_none")]
- pub target: Option<Vec<String>>,
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub target: Vec<String>,
/// Comment
#[serde(skip_serializing_if = "Option::is_none")]
@@ -284,29 +287,32 @@ impl MatcherConfig {
// 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() {
+ if !self.match_severity.is_empty() {
no_matchers = false;
is_match = mode.apply(
is_match,
- self.check_matches(notification, severity_matchers)?,
+ self.check_matches(notification, &self.match_severity)?,
);
}
- if let Some(field_matchers) = self.match_field.as_deref() {
+ if !self.match_field.is_empty() {
no_matchers = false;
- is_match = mode.apply(is_match, self.check_matches(notification, field_matchers)?);
+ is_match = mode.apply(
+ is_match,
+ self.check_matches(notification, &self.match_field)?,
+ );
}
- if let Some(calendar_matchers) = self.match_calendar.as_deref() {
+ if !self.match_calendar.is_empty() {
no_matchers = false;
is_match = mode.apply(
is_match,
- self.check_matches(notification, calendar_matchers)?,
+ self.check_matches(notification, &self.match_calendar)?,
);
}
let invert_match = self.invert_match.unwrap_or_default();
Ok(if is_match != invert_match || no_matchers {
- Some(self.target.as_deref().unwrap_or_default())
+ Some(&self.target)
} else {
None
})
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 04/20] notify: don't make tests require pve-context
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (2 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 03/20] notify: convert Option<Vec<T>> -> Vec<T> in config structs Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 05/20] notify: make the `mail-forwarder` feature depend on proxmox-sys Lukas Wagner
` (16 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Tests now have their own context, so requiring pve-context is not
necessary any more.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/api/gotify.rs | 2 +-
proxmox-notify/src/api/matcher.rs | 2 +-
proxmox-notify/src/api/sendmail.rs | 2 +-
proxmox-notify/src/api/smtp.rs | 24 ++++++++++++------------
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index 15d94cb..92151f5 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -165,7 +165,7 @@ fn remove_private_config_entry(config: &mut Config, name: &str) -> Result<(), Ht
Ok(())
}
-#[cfg(all(feature = "pve-context", test))]
+#[cfg(test)]
mod tests {
use super::*;
use crate::api::test_helpers::empty_config;
diff --git a/proxmox-notify/src/api/matcher.rs b/proxmox-notify/src/api/matcher.rs
index 63ec73d..fa11633 100644
--- a/proxmox-notify/src/api/matcher.rs
+++ b/proxmox-notify/src/api/matcher.rs
@@ -148,7 +148,7 @@ pub fn delete_matcher(config: &mut Config, name: &str) -> Result<(), HttpError>
Ok(())
}
-#[cfg(all(test, feature = "sendmail", feature = "pve-context"))]
+#[cfg(all(test, feature = "sendmail"))]
mod tests {
use super::*;
use crate::matcher::MatchModeOperator;
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index c20a3e5..47588af 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -151,7 +151,7 @@ pub fn delete_endpoint(config: &mut Config, name: &str) -> Result<(), HttpError>
Ok(())
}
-#[cfg(all(feature = "pve-context", test))]
+#[cfg(test)]
pub mod tests {
use super::*;
use crate::api::test_helpers::*;
diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
index 7a58677..1b4700e 100644
--- a/proxmox-notify/src/api/smtp.rs
+++ b/proxmox-notify/src/api/smtp.rs
@@ -200,7 +200,7 @@ pub fn delete_endpoint(config: &mut Config, name: &str) -> Result<(), HttpError>
Ok(())
}
-#[cfg(all(feature = "pve-context", test))]
+#[cfg(test)]
pub mod tests {
use super::*;
use crate::api::test_helpers::*;
@@ -348,15 +348,15 @@ pub mod tests {
Ok(())
}
- // #[test]
- // fn test_delete() -> Result<(), HttpError> {
- // let mut config = empty_config();
- // add_smtp_endpoint_for_test(&mut config, "smtp-endpoint")?;
- //
- // delete_endpoint(&mut config, "smtp-endpoint")?;
- // assert!(delete_endpoint(&mut config, "smtp-endpoint").is_err());
- // assert_eq!(get_endpoints(&config)?.len(), 0);
- //
- // Ok(())
- // }
+ #[test]
+ fn test_delete() -> Result<(), HttpError> {
+ let mut config = empty_config();
+ add_smtp_endpoint_for_test(&mut config, "smtp-endpoint")?;
+
+ delete_endpoint(&mut config, "smtp-endpoint")?;
+ assert!(delete_endpoint(&mut config, "smtp-endpoint").is_err());
+ assert_eq!(get_endpoints(&config)?.len(), 0);
+
+ Ok(())
+ }
}
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 05/20] notify: make the `mail-forwarder` feature depend on proxmox-sys
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (3 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 04/20] notify: don't make tests require pve-context Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 06/20] notify: cargo.toml: add spaces before curly braces Lukas Wagner
` (15 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
It uses proxmox_sys::nodename - the dep is needed, otherwise the code
does not compile in some feature flag permutations.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/Cargo.toml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 52a466e..3e8d253 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -31,7 +31,7 @@ proxmox-time.workspace = true
[features]
default = ["sendmail", "gotify", "smtp"]
-mail-forwarder = ["dep:mail-parser"]
+mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys"]
sendmail = ["dep:proxmox-sys"]
gotify = ["dep:proxmox-http"]
pve-context = ["dep:proxmox-sys"]
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 06/20] notify: cargo.toml: add spaces before curly braces
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (4 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 05/20] notify: make the `mail-forwarder` feature depend on proxmox-sys Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 07/20] notify: give each notification a unique ID Lukas Wagner
` (14 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/Cargo.toml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 3e8d253..185b50a 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -17,13 +17,13 @@ log.workspace = true
mail-parser = { workspace = true, optional = true }
openssl.workspace = true
regex.workspace = true
-serde = { workspace = true, features = ["derive"]}
+serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
proxmox-http = { workspace = true, features = ["client-sync"], optional = true }
proxmox-http-error.workspace = true
proxmox-human-byte.workspace = true
-proxmox-schema = { workspace = true, features = ["api-macro", "api-types"]}
+proxmox-schema = { workspace = true, features = ["api-macro", "api-types"] }
proxmox-section-config = { workspace = true }
proxmox-serde.workspace = true
proxmox-sys = { workspace = true, optional = true }
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 07/20] notify: give each notification a unique ID
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (5 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 06/20] notify: cargo.toml: add spaces before curly braces Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 08/20] notify: api: add get_targets Lukas Wagner
` (13 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
We need this for queuing notifications on PBS from the unprivileged
proxy process.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/Cargo.toml | 1 +
proxmox-notify/src/lib.rs | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 185b50a..797b1ac 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -28,6 +28,7 @@ proxmox-section-config = { workspace = true }
proxmox-serde.workspace = true
proxmox-sys = { workspace = true, optional = true }
proxmox-time.workspace = true
+proxmox-uuid = { workspace = true, features = ["serde"] }
[features]
default = ["sendmail", "gotify", "smtp"]
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 35dcb17..91c0b61 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -10,6 +10,7 @@ use serde_json::Value;
use proxmox_schema::api;
use proxmox_section_config::SectionConfigData;
+use proxmox_uuid::Uuid;
pub mod matcher;
use crate::config::CONFIG;
@@ -198,6 +199,8 @@ pub struct Notification {
content: Content,
/// Metadata
metadata: Metadata,
+ /// Unique ID
+ id: Uuid,
}
impl Notification {
@@ -217,6 +220,7 @@ impl Notification {
template_name: template_name.as_ref().to_string(),
data: template_data,
},
+ id: Uuid::generate(),
}
}
#[cfg(feature = "mail-forwarder")]
@@ -246,8 +250,14 @@ impl Notification {
additional_fields,
timestamp: proxmox_time::epoch_i64(),
},
+ id: Uuid::generate(),
})
}
+
+ /// Return the unique ID of this notification.
+ pub fn id(&self) -> &Uuid {
+ &self.id
+ }
}
/// Notification configuration
@@ -548,6 +558,7 @@ impl Bus {
template_name: "test".to_string(),
data: json!({ "target": target }),
},
+ id: Uuid::generate(),
};
if let Some(endpoint) = self.endpoints.get(target) {
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 08/20] notify: api: add get_targets
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (6 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 07/20] notify: give each notification a unique ID Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 09/20] notify: derive `api` for Deleteable*Property Lukas Wagner
` (12 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
This method allows us to get a list of all notification targets.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/api/mod.rs | 77 +++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index a2cf29e..a7f6261 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -3,6 +3,7 @@ use std::collections::HashSet;
use serde::{Deserialize, Serialize};
use proxmox_http_error::HttpError;
+use proxmox_schema::api;
use crate::{Config, Origin};
@@ -39,6 +40,82 @@ macro_rules! http_bail {
pub use http_bail;
pub use http_err;
+#[api]
+#[derive(Clone, Debug, Copy, Serialize, Deserialize, PartialEq, Eq, PartialOrd)]
+#[serde(rename_all = "kebab-case")]
+/// Type of the endpoint.
+pub enum EndpointType {
+ /// Sendmail endpoint
+ #[cfg(feature = "sendmail")]
+ Sendmail,
+ /// SMTP endpoint
+ #[cfg(feature = "smtp")]
+ Smtp,
+ /// Gotify endpoint
+ #[cfg(feature = "gotify")]
+ Gotify,
+}
+
+#[api]
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd)]
+#[serde(rename_all = "kebab-case")]
+/// Target information
+pub struct Target {
+ /// Name of the endpoint
+ name: String,
+ /// Origin of the endpoint
+ origin: Origin,
+ /// Type of the endpoint
+ #[serde(rename = "type")]
+ endpoint_type: EndpointType,
+ /// Target is disabled
+ #[serde(skip_serializing_if = "Option::is_none")]
+ disable: Option<bool>,
+ /// Comment
+ #[serde(skip_serializing_if = "Option::is_none")]
+ comment: Option<String>,
+}
+
+/// Get a list of all notification targets.
+pub fn get_targets(config: &Config) -> Result<Vec<Target>, HttpError> {
+ let mut targets = Vec::new();
+
+ #[cfg(feature = "gotify")]
+ for endpoint in gotify::get_endpoints(config)? {
+ targets.push(Target {
+ name: endpoint.name,
+ origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+ endpoint_type: EndpointType::Gotify,
+ disable: endpoint.disable,
+ comment: endpoint.comment,
+ })
+ }
+
+ #[cfg(feature = "sendmail")]
+ for endpoint in sendmail::get_endpoints(config)? {
+ targets.push(Target {
+ name: endpoint.name,
+ origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+ endpoint_type: EndpointType::Sendmail,
+ disable: endpoint.disable,
+ comment: endpoint.comment,
+ })
+ }
+
+ #[cfg(feature = "smtp")]
+ for endpoint in smtp::get_endpoints(config)? {
+ targets.push(Target {
+ name: endpoint.name,
+ origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+ endpoint_type: EndpointType::Smtp,
+ disable: endpoint.disable,
+ comment: endpoint.comment,
+ })
+ }
+
+ Ok(targets)
+}
+
fn verify_digest(config: &Config, digest: Option<&[u8]>) -> Result<(), HttpError> {
if let Some(digest) = digest {
if config.digest != *digest {
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 09/20] notify: derive `api` for Deleteable*Property
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (7 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 08/20] notify: api: add get_targets Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 10/20] notify: derive Deserialize/Serialize for Notification struct Lukas Wagner
` (11 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
The API endpoints in Proxmox Backup Server require ApiType to be
implemented for any deserialized parameter.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/endpoints/gotify.rs | 3 +++
proxmox-notify/src/endpoints/sendmail.rs | 7 +++++++
proxmox-notify/src/endpoints/smtp.rs | 9 +++++++++
proxmox-notify/src/matcher.rs | 9 +++++++++
4 files changed, 28 insertions(+)
diff --git a/proxmox-notify/src/endpoints/gotify.rs b/proxmox-notify/src/endpoints/gotify.rs
index afdfabc..ee8ca51 100644
--- a/proxmox-notify/src/endpoints/gotify.rs
+++ b/proxmox-notify/src/endpoints/gotify.rs
@@ -81,10 +81,13 @@ pub struct GotifyEndpoint {
pub private_config: GotifyPrivateConfig,
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableGotifyProperty {
+ /// Delete `comment`
Comment,
+ /// Delete `disable`
Disable,
}
diff --git a/proxmox-notify/src/endpoints/sendmail.rs b/proxmox-notify/src/endpoints/sendmail.rs
index fa04002..47901ef 100644
--- a/proxmox-notify/src/endpoints/sendmail.rs
+++ b/proxmox-notify/src/endpoints/sendmail.rs
@@ -73,14 +73,21 @@ pub struct SendmailConfig {
pub origin: Option<Origin>,
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableSendmailProperty {
+ /// Delete `author`
Author,
+ /// Delete `comment`
Comment,
+ /// Delete `disable`
Disable,
+ /// Delete `from-address`
FromAddress,
+ /// Delete `mailto`
Mailto,
+ /// Delete `mailto-user`
MailtoUser,
}
diff --git a/proxmox-notify/src/endpoints/smtp.rs b/proxmox-notify/src/endpoints/smtp.rs
index ded5baf..f04583a 100644
--- a/proxmox-notify/src/endpoints/smtp.rs
+++ b/proxmox-notify/src/endpoints/smtp.rs
@@ -104,16 +104,25 @@ pub struct SmtpConfig {
pub origin: Option<Origin>,
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableSmtpProperty {
+ /// Delete `author`
Author,
+ /// Delete `comment`
Comment,
+ /// Delete `disable`
Disable,
+ /// Delete `mailto`
Mailto,
+ /// Delete `mailto-user`
MailtoUser,
+ /// Delete `password`
Password,
+ /// Delete `port`
Port,
+ /// Delete `username`
Username,
}
diff --git a/proxmox-notify/src/matcher.rs b/proxmox-notify/src/matcher.rs
index b387fef..2d30378 100644
--- a/proxmox-notify/src/matcher.rs
+++ b/proxmox-notify/src/matcher.rs
@@ -412,16 +412,25 @@ impl FromStr for CalendarMatcher {
}
}
+#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum DeleteableMatcherProperty {
+ /// Delete `comment`
Comment,
+ /// Delete `disable`
Disable,
+ /// Delete `invert-match`
InvertMatch,
+ /// Delete `match-calendar`
MatchCalendar,
+ /// Delete `match-field`
MatchField,
+ /// Delete `match-severity`
MatchSeverity,
+ /// Delete `mode`
Mode,
+ /// Delete `target`
Target,
}
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 10/20] notify: derive Deserialize/Serialize for Notification struct
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (8 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 09/20] notify: derive `api` for Deleteable*Property Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 11/20] notify: pbs context: include nodename in default sendmail author Lukas Wagner
` (10 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/lib.rs | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 91c0b61..292396b 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -159,9 +159,11 @@ pub trait Endpoint {
fn disabled(&self) -> bool;
}
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
pub enum Content {
/// Title and body will be rendered as a template
+ #[serde(rename_all = "kebab-case")]
Template {
/// Name of the used template
template_name: String,
@@ -169,6 +171,7 @@ pub enum Content {
data: Value,
},
#[cfg(feature = "mail-forwarder")]
+ #[serde(rename_all = "kebab-case")]
ForwardedMail {
/// Raw mail contents
raw: Vec<u8>,
@@ -182,7 +185,8 @@ pub enum Content {
},
}
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
pub struct Metadata {
/// Notification severity
severity: Severity,
@@ -192,7 +196,8 @@ pub struct Metadata {
additional_fields: HashMap<String, String>,
}
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
/// Notification which can be sent
pub struct Notification {
/// Notification content
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 11/20] notify: pbs context: include nodename in default sendmail author
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (9 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 10/20] notify: derive Deserialize/Serialize for Notification struct Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 12/20] notify: renderer: add relative-percentage helper from PBS Lukas Wagner
` (9 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
The old notification stack in proxmox-backup includes the nodename, so
we include it here as well.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/context/pbs.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-notify/src/context/pbs.rs b/proxmox-notify/src/context/pbs.rs
index 70e993f..299f685 100644
--- a/proxmox-notify/src/context/pbs.rs
+++ b/proxmox-notify/src/context/pbs.rs
@@ -82,7 +82,7 @@ impl Context for PBSContext {
}
fn default_sendmail_author(&self) -> String {
- "Proxmox Backup Server".into()
+ format!("Proxmox Backup Server - {}", proxmox_sys::nodename())
}
fn default_sendmail_from(&self) -> String {
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox v2 12/20] notify: renderer: add relative-percentage helper from PBS
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (10 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 11/20] notify: pbs context: include nodename in default sendmail author Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 13/20] notify: use file based notification templates Lukas Wagner
` (8 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
proxmox-notify/src/renderer/mod.rs | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/proxmox-notify/src/renderer/mod.rs b/proxmox-notify/src/renderer/mod.rs
index a51ece6..ddb241d 100644
--- a/proxmox-notify/src/renderer/mod.rs
+++ b/proxmox-notify/src/renderer/mod.rs
@@ -73,6 +73,30 @@ fn value_to_timestamp(val: &Value) -> Option<String> {
proxmox_time::strftime_local("%F %H:%M:%S", timestamp).ok()
}
+fn handlebars_relative_percentage_helper(
+ h: &Helper,
+ _: &Handlebars,
+ _: &Context,
+ _rc: &mut RenderContext,
+ out: &mut dyn Output,
+) -> HelperResult {
+ let param0 = h
+ .param(0)
+ .and_then(|v| v.value().as_f64())
+ .ok_or_else(|| HandlebarsRenderError::new("relative-percentage: param0 not found"))?;
+ let param1 = h
+ .param(1)
+ .and_then(|v| v.value().as_f64())
+ .ok_or_else(|| HandlebarsRenderError::new("relative-percentage: param1 not found"))?;
+
+ if param1 == 0.0 {
+ out.write("-")?;
+ } else {
+ out.write(&format!("{:.2}%", (param0 * 100.0) / param1))?;
+ }
+ Ok(())
+}
+
/// Available render functions for `serde_json::Values``
///
/// May be used as a handlebars helper, e.g.
@@ -237,6 +261,11 @@ fn render_template_impl(
ValueRenderFunction::register_helpers(&mut handlebars);
+ handlebars.register_helper(
+ "relative-percentage",
+ Box::new(handlebars_relative_percentage_helper),
+ );
+
let rendered_template = handlebars
.render_template(template, data)
.map_err(|err| Error::RenderError(err.into()))?;
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox-perl-rs v2 13/20] notify: use file based notification templates
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (11 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 12/20] notify: renderer: add relative-percentage helper from PBS Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 14/20] notify: don't pass config structs by reference Lukas Wagner
` (7 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Instead of passing literal template strings to the notification
system, we now only pass an identifier. This identifier will be used
load the template files from a product-specific directory.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
common/src/notify.rs | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 8f9f38f..d965417 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -94,16 +94,14 @@ mod export {
fn send(
#[try_from_ref] this: &NotificationConfig,
severity: Severity,
- title: String,
- body: String,
+ template_name: String,
template_data: Option<JSONValue>,
fields: Option<HashMap<String, String>>,
) -> Result<(), HttpError> {
let config = this.config.lock().unwrap();
- let notification = Notification::new_templated(
+ let notification = Notification::from_template(
severity,
- title,
- body,
+ template_name,
template_data.unwrap_or_default(),
fields.unwrap_or_default(),
);
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox-perl-rs v2 14/20] notify: don't pass config structs by reference
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (12 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 13/20] notify: use file based notification templates Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 15/20] notify: adapt to Option<Vec<T>> to Vec<T> changes in proxmox_notify Lukas Wagner
` (6 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
proxmox_notify's api functions have been changed so that they take
ownership of config structs.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
common/src/notify.rs | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index d965417..00a6056 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -151,7 +151,7 @@ mod export {
api::sendmail::add_endpoint(
&mut config,
- &SendmailConfig {
+ SendmailConfig {
name,
mailto,
mailto_user,
@@ -185,7 +185,7 @@ mod export {
api::sendmail::update_endpoint(
&mut config,
name,
- &SendmailConfigUpdater {
+ SendmailConfigUpdater {
mailto,
mailto_user,
from_address,
@@ -236,7 +236,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::gotify::add_endpoint(
&mut config,
- &GotifyConfig {
+ GotifyConfig {
name: name.clone(),
server,
comment,
@@ -244,7 +244,7 @@ mod export {
filter: None,
origin: None,
},
- &GotifyPrivateConfig { name, token },
+ GotifyPrivateConfig { name, token },
)
}
@@ -266,12 +266,12 @@ mod export {
api::gotify::update_endpoint(
&mut config,
name,
- &GotifyConfigUpdater {
+ GotifyConfigUpdater {
server,
comment,
disable,
},
- &GotifyPrivateConfigUpdater { token },
+ GotifyPrivateConfigUpdater { token },
delete.as_deref(),
digest.as_deref(),
)
@@ -323,7 +323,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::smtp::add_endpoint(
&mut config,
- &SmtpConfig {
+ SmtpConfig {
name: name.clone(),
server,
port,
@@ -337,7 +337,7 @@ mod export {
disable,
origin: None,
},
- &SmtpPrivateConfig { name, password },
+ SmtpPrivateConfig { name, password },
)
}
@@ -366,7 +366,7 @@ mod export {
api::smtp::update_endpoint(
&mut config,
name,
- &SmtpConfigUpdater {
+ SmtpConfigUpdater {
server,
port,
mode,
@@ -378,7 +378,7 @@ mod export {
comment,
disable,
},
- &SmtpPrivateConfigUpdater { password },
+ SmtpPrivateConfigUpdater { password },
delete.as_deref(),
digest.as_deref(),
)
@@ -427,7 +427,7 @@ mod export {
let mut config = this.config.lock().unwrap();
api::matcher::add_matcher(
&mut config,
- &MatcherConfig {
+ MatcherConfig {
name,
match_severity,
match_field,
@@ -464,7 +464,7 @@ mod export {
api::matcher::update_matcher(
&mut config,
name,
- &MatcherConfigUpdater {
+ MatcherConfigUpdater {
match_severity,
match_field,
match_calendar,
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH proxmox-perl-rs v2 15/20] notify: adapt to Option<Vec<T>> to Vec<T> changes in proxmox_notify
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (13 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 14/20] notify: don't pass config structs by reference Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH cluster v2 16/20] notify: use named template instead of passing template strings Lukas Wagner
` (5 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
common/src/notify.rs | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index 00a6056..e1b006b 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -153,8 +153,8 @@ mod export {
&mut config,
SendmailConfig {
name,
- mailto,
- mailto_user,
+ mailto: mailto.unwrap_or_default(),
+ mailto_user: mailto_user.unwrap_or_default(),
from_address,
author,
comment,
@@ -329,8 +329,8 @@ mod export {
port,
mode,
username,
- mailto,
- mailto_user,
+ mailto: mailto.unwrap_or_default(),
+ mailto_user: mailto_user.unwrap_or_default(),
from_address,
author,
comment,
@@ -429,10 +429,10 @@ mod export {
&mut config,
MatcherConfig {
name,
- match_severity,
- match_field,
- match_calendar,
- target,
+ match_severity: match_severity.unwrap_or_default(),
+ match_field: match_field.unwrap_or_default(),
+ match_calendar: match_calendar.unwrap_or_default(),
+ target: target.unwrap_or_default(),
mode,
invert_match,
comment,
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH cluster v2 16/20] notify: use named template instead of passing template strings
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (14 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 15/20] notify: adapt to Option<Vec<T>> to Vec<T> changes in proxmox_notify Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH pve-ha-manager v2 17/20] env: notify: use named templates " Lukas Wagner
` (4 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
The notification system will now load template files from a defined
location. The template to use is now passed to proxmox_notify, instead
of separate template strings for subject/body.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
src/PVE/Notify.pm | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/PVE/Notify.pm b/src/PVE/Notify.pm
index 872eb25..c514111 100644
--- a/src/PVE/Notify.pm
+++ b/src/PVE/Notify.pm
@@ -58,17 +58,16 @@ sub write_config {
}
my $send_notification = sub {
- my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+ my ($severity, $template_name, $template_data, $fields, $config) = @_;
$config = read_config() if !defined($config);
- $config->send($severity, $title, $message, $template_data, $fields);
+ $config->send($severity, $template_name, $template_data, $fields);
};
sub notify {
- my ($severity, $title, $message, $template_data, $fields, $config) = @_;
+ my ($severity, $template_name, $template_data, $fields, $config) = @_;
$send_notification->(
$severity,
- $title,
- $message,
+ $template_name,
$template_data,
$fields,
$config
@@ -76,11 +75,10 @@ sub notify {
}
sub info {
- my ($title, $message, $template_data, $fields, $config) = @_;
+ my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'info',
- $title,
- $message,
+ $template_name,
$template_data,
$fields,
$config
@@ -88,11 +86,10 @@ sub info {
}
sub notice {
- my ($title, $message, $template_data, $fields, $config) = @_;
+ my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'notice',
- $title,
- $message,
+ $template_name,
$template_data,
$fields,
$config
@@ -100,11 +97,10 @@ sub notice {
}
sub warning {
- my ($title, $message, $template_data, $fields, $config) = @_;
+ my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'warning',
- $title,
- $message,
+ $template_name,
$template_data,
$fields,
$config
@@ -112,11 +108,10 @@ sub warning {
}
sub error {
- my ($title, $message, $template_data, $fields, $config) = @_;
+ my ($template_name, $template_data, $fields, $config) = @_;
$send_notification->(
'error',
- $title,
- $message,
+ $template_name,
$template_data,
$fields,
$config
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH pve-ha-manager v2 17/20] env: notify: use named templates instead of passing template strings
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (15 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH cluster v2 16/20] notify: use named template instead of passing template strings Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 18/20] gitignore: ignore any test artifacts Lukas Wagner
` (3 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
debian/pve-ha-manager.install | 3 +++
src/Makefile | 1 +
src/PVE/HA/Env/PVE2.pm | 4 ++--
src/PVE/HA/NodeStatus.pm | 20 +------------------
src/PVE/HA/Sim/Env.pm | 3 ++-
src/templates/Makefile | 10 ++++++++++
src/templates/default/fencing-body.html.hbs | 14 +++++++++++++
src/templates/default/fencing-body.txt.hbs | 11 ++++++++++
src/templates/default/fencing-subject.txt.hbs | 1 +
9 files changed, 45 insertions(+), 22 deletions(-)
create mode 100644 src/templates/Makefile
create mode 100644 src/templates/default/fencing-body.html.hbs
create mode 100644 src/templates/default/fencing-body.txt.hbs
create mode 100644 src/templates/default/fencing-subject.txt.hbs
diff --git a/debian/pve-ha-manager.install b/debian/pve-ha-manager.install
index a7598a9..0ffbd8d 100644
--- a/debian/pve-ha-manager.install
+++ b/debian/pve-ha-manager.install
@@ -38,3 +38,6 @@
/usr/share/perl5/PVE/HA/Usage/Static.pm
/usr/share/perl5/PVE/Service/pve_ha_crm.pm
/usr/share/perl5/PVE/Service/pve_ha_lrm.pm
+/usr/share/pve-manager/templates/default/fencing-body.html.hbs
+/usr/share/pve-manager/templates/default/fencing-body.txt.hbs
+/usr/share/pve-manager/templates/default/fencing-subject.txt.hbs
diff --git a/src/Makefile b/src/Makefile
index 87bb0de..56bd360 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -73,6 +73,7 @@ install: watchdog-mux pve-ha-crm pve-ha-lrm ha-manager.1 pve-ha-crm.8 pve-ha-lrm
install -d $(DESTDIR)/$(MAN1DIR)
install -m 0644 ha-manager.1 $(DESTDIR)/$(MAN1DIR)
gzip -9 $(DESTDIR)/$(MAN1DIR)/ha-manager.1
+ $(MAKE) -C templates $@
.PHONY: test
test:
diff --git a/src/PVE/HA/Env/PVE2.pm b/src/PVE/HA/Env/PVE2.pm
index fcb60a9..cb73bcf 100644
--- a/src/PVE/HA/Env/PVE2.pm
+++ b/src/PVE/HA/Env/PVE2.pm
@@ -221,10 +221,10 @@ sub log {
}
sub send_notification {
- my ($self, $subject, $text, $template_data, $metadata_fields) = @_;
+ my ($self, $template_name, $template_data, $metadata_fields) = @_;
eval {
- PVE::Notify::error($subject, $text, $template_data, $metadata_fields);
+ PVE::Notify::error($template_name, $template_data, $metadata_fields);
};
$self->log("warning", "could not notify: $@") if $@;
diff --git a/src/PVE/HA/NodeStatus.pm b/src/PVE/HA/NodeStatus.pm
index e053c55..9e6d898 100644
--- a/src/PVE/HA/NodeStatus.pm
+++ b/src/PVE/HA/NodeStatus.pm
@@ -188,23 +188,6 @@ sub update {
}
}
-my $body_template = <<EOT;
-{{#verbatim}}
-The node '{{node}}' failed and needs manual intervention.
-
-The PVE HA manager tries to fence it and recover the configured HA resources to
-a healthy node if possible.
-
-Current fence status: {{subject-prefix}}
-{{subject}}
-{{/verbatim}}
-
-{{heading-2 "Overall Cluster status:"}}
-{{object status-data}}
-EOT
-
-my $subject_template = "{{subject-prefix}}: {{subject}}";
-
# assembles a commont text for fence emails
my $send_fence_state_email = sub {
my ($self, $subject_prefix, $subject, $node) = @_;
@@ -228,8 +211,7 @@ my $send_fence_state_email = sub {
};
$haenv->send_notification(
- $subject_template,
- $body_template,
+ "fencing",
$template_data,
$metadata_fields,
);
diff --git a/src/PVE/HA/Sim/Env.pm b/src/PVE/HA/Sim/Env.pm
index d3aea8d..0f77065 100644
--- a/src/PVE/HA/Sim/Env.pm
+++ b/src/PVE/HA/Sim/Env.pm
@@ -289,11 +289,12 @@ sub log {
}
sub send_notification {
- my ($self, $subject, $text, $properties) = @_;
+ my ($self, $template_name, $properties) = @_;
# The template for the subject is "{{subject-prefix}}: {{subject}}"
# We have to perform poor-man's template rendering to pass the test cases.
+ my $subject = "{{subject-prefix}}: {{subject}}";
$subject = $subject =~ s/\{\{subject-prefix}}/$properties->{"subject-prefix"}/r;
$subject = $subject =~ s/\{\{subject}}/$properties->{"subject"}/r;
diff --git a/src/templates/Makefile b/src/templates/Makefile
new file mode 100644
index 0000000..396759f
--- /dev/null
+++ b/src/templates/Makefile
@@ -0,0 +1,10 @@
+NOTIFICATION_TEMPLATES= \
+ default/fencing-subject.txt.hbs \
+ default/fencing-body.txt.hbs \
+ default/fencing-body.html.hbs \
+
+.PHONY: install
+install:
+ install -dm 0755 $(DESTDIR)/usr/share/pve-manager/templates/default
+ $(foreach i,$(NOTIFICATION_TEMPLATES), \
+ install -m644 $(i) $(DESTDIR)/usr/share/pve-manager/templates/$(i) ;)
diff --git a/src/templates/default/fencing-body.html.hbs b/src/templates/default/fencing-body.html.hbs
new file mode 100644
index 0000000..1420348
--- /dev/null
+++ b/src/templates/default/fencing-body.html.hbs
@@ -0,0 +1,14 @@
+<html>
+ <body>
+ The node '{{node}}' failed and needs manual intervention.<br/><br/>
+
+ The PVE HA manager tries to fence it and recover the configured HA resources to
+ a healthy node if possible.<br/><br/>
+
+ Current fence status: {{subject-prefix}}<br/>
+ {{subject}}<br/>
+
+ <h2 style="font-size: 1em">Overall Cluster status:</h2>
+ {{object status-data}}
+ </body>
+</html>
diff --git a/src/templates/default/fencing-body.txt.hbs b/src/templates/default/fencing-body.txt.hbs
new file mode 100644
index 0000000..e46a1fd
--- /dev/null
+++ b/src/templates/default/fencing-body.txt.hbs
@@ -0,0 +1,11 @@
+The node '{{node}}' failed and needs manual intervention.
+
+The PVE HA manager tries to fence it and recover the configured HA resources to
+a healthy node if possible.
+
+Current fence status: {{subject-prefix}}
+{{subject}}
+
+Overall Cluster status:
+-----------------------
+{{object status-data}}
diff --git a/src/templates/default/fencing-subject.txt.hbs b/src/templates/default/fencing-subject.txt.hbs
new file mode 100644
index 0000000..43651f9
--- /dev/null
+++ b/src/templates/default/fencing-subject.txt.hbs
@@ -0,0 +1 @@
+{{subject-prefix}}: {{subject}}
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH manager v2 18/20] gitignore: ignore any test artifacts
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (16 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH pve-ha-manager v2 17/20] env: notify: use named templates " Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 19/20] tests: remove vzdump_notification test Lukas Wagner
` (2 subsequent siblings)
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
.gitignore | 2 ++
1 file changed, 2 insertions(+)
diff --git a/.gitignore b/.gitignore
index e8d1eb27..481ae1e0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,3 +9,5 @@ dest/
/www/mobile/pvemanager-mobile.js
/www/touch/touch-[0-9]*/
/pve-manager-[0-9]*/
+/test/.mocked_*
+/test/*.tmp
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH manager v2 19/20] tests: remove vzdump_notification test
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (17 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 18/20] gitignore: ignore any test artifacts Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 20/20] notifications: use named templates instead of in-code templates Lukas Wagner
2024-04-23 22:31 ` [pve-devel] partially-applied-series: [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Thomas Lamprecht
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
With the upcoming changes in how we send notifications, this one
really becomes pretty annoying to keep working. The location where
templates are looked up are defined in the proxmox_notify crate, so
there is no easy way to mock this for testing.
The test itself seemed not super valuable, mainly testing if
the backup logs are shortened if they ware too long - so they are just
removed.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
test/Makefile | 6 +-
test/vzdump_notification_test.pl | 101 -------------------------------
2 files changed, 1 insertion(+), 106 deletions(-)
delete mode 100755 test/vzdump_notification_test.pl
diff --git a/test/Makefile b/test/Makefile
index 62d75050..743804c8 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,7 +5,7 @@ all:
export PERLLIB=..
.PHONY: check
-check: test-replication test-balloon test-vzdump-notification test-vzdump test-osd
+check: test-replication test-balloon test-vzdump test-osd
.PHONY: test-balloon
test-balloon:
@@ -17,10 +17,6 @@ test-replication: replication1.t replication2.t replication3.t replication4.t re
replication%.t: replication_test%.pl
./$<
-.PHONY: test-vzdump-notification
-test-vzdump-notification:
- ./vzdump_notification_test.pl
-
.PHONY: test-vzdump
test-vzdump: test-vzdump-guest-included test-vzdump-new
diff --git a/test/vzdump_notification_test.pl b/test/vzdump_notification_test.pl
deleted file mode 100755
index 631606bb..00000000
--- a/test/vzdump_notification_test.pl
+++ /dev/null
@@ -1,101 +0,0 @@
-#!/usr/bin/perl
-
-use strict;
-use warnings;
-
-use lib '..';
-
-use Test::More tests => 3;
-use Test::MockModule;
-
-use PVE::VZDump;
-
-my $STATUS = qr/.*status.*/;
-my $NO_LOGFILE = qr/.*Could not open log file.*/;
-my $LOG_TOO_LONG = qr/.*Log output was too long.*/;
-my $TEST_FILE_PATH = '/tmp/mail_test';
-my $TEST_FILE_WRONG_PATH = '/tmp/mail_test_wrong';
-
-sub prepare_mail_with_status {
- open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
- print TEST_FILE "start of log file\n";
- print TEST_FILE "status: 0\% this should not be in the mail\n";
- print TEST_FILE "status: 55\% this should not be in the mail\n";
- print TEST_FILE "status: 100\% this should not be in the mail\n";
- print TEST_FILE "end of log file\n";
- close(TEST_FILE);
-}
-
-sub prepare_long_mail {
- open(TEST_FILE, '>', $TEST_FILE_PATH); # Removes previous content
- # 0.5 MB * 2 parts + the overview tables gives more than 1 MB mail
- print TEST_FILE "a" x (1024*1024);
- close(TEST_FILE);
-}
-
-my $result_text;
-my $result_properties;
-
-my $mock_notification_module = Test::MockModule->new('PVE::Notify');
-my $mocked_notify = sub {
- my ($severity, $title, $text, $properties, $metadata) = @_;
-
- $result_text = $text;
- $result_properties = $properties;
-};
-my $mocked_notify_short = sub {
- my (@params) = @_;
- return $mocked_notify->('<some severity>', @params);
-};
-
-$mock_notification_module->mock(
- 'notify' => $mocked_notify,
- 'info' => $mocked_notify_short,
- 'notice' => $mocked_notify_short,
- 'warning' => $mocked_notify_short,
- 'error' => $mocked_notify_short,
-);
-$mock_notification_module->mock('cfs_read_file', sub {
- my $path = shift;
-
- if ($path eq 'datacenter.cfg') {
- return {};
- } elsif ($path eq 'notifications.cfg' || $path eq 'priv/notifications.cfg') {
- return '';
- } else {
- die "unexpected cfs_read_file\n";
- }
-});
-
-my $MAILTO = ['test_address@proxmox.com'];
-my $SELF = {
- opts => { mailto => $MAILTO },
- cmdline => 'test_command_on_cli',
-};
-
-my $task = { state => 'ok', vmid => '100', };
-my $tasklist;
-sub prepare_test {
- $result_text = undef;
- $task->{tmplog} = shift;
- $tasklist = [ $task ];
-}
-
-{
- prepare_test($TEST_FILE_WRONG_PATH);
- PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
- like($result_properties->{logs}, $NO_LOGFILE, "Missing logfile is detected");
-}
-{
- prepare_test($TEST_FILE_PATH);
- prepare_mail_with_status();
- PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
- unlike($result_properties->{"status-text"}, $STATUS, "Status are not in text part of mails");
-}
-{
- prepare_test($TEST_FILE_PATH);
- prepare_long_mail();
- PVE::VZDump::send_notification($SELF, $tasklist, 0, undef, undef, undef);
- like($result_properties->{logs}, $LOG_TOO_LONG, "Text part of mails gets shortened");
-}
-unlink $TEST_FILE_PATH;
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] [PATCH manager v2 20/20] notifications: use named templates instead of in-code templates
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (18 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 19/20] tests: remove vzdump_notification test Lukas Wagner
@ 2024-04-19 14:17 ` Lukas Wagner
2024-04-23 22:31 ` [pve-devel] partially-applied-series: [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Thomas Lamprecht
20 siblings, 0 replies; 22+ messages in thread
From: Lukas Wagner @ 2024-04-19 14:17 UTC (permalink / raw)
To: pve-devel
This commit adapts notification sending for
- package update
- replication
- backups
to use named templates (installed in /usr/share/pve-manager/templates)
instead of passing template strings defined in code to the
notification stack.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Folke Gleumes <f.gleumes@proxmox.com>
Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
---
Makefile | 2 +-
PVE/API2/APT.pm | 9 +------
PVE/API2/Replication.pm | 20 +---------------
PVE/VZDump.pm | 20 ++--------------
templates/Makefile | 24 +++++++++++++++++++
.../default/package-updates-body.html.hbs | 6 +++++
.../default/package-updates-body.txt.hbs | 3 +++
.../default/package-updates-subject.txt.hbs | 1 +
templates/default/replication-body.html.hbs | 18 ++++++++++++++
templates/default/replication-body.txt.hbs | 12 ++++++++++
templates/default/replication-subject.txt.hbs | 1 +
templates/default/test-body.html.hbs | 1 +
templates/default/test-body.txt.hbs | 1 +
templates/default/test-subject.txt.hbs | 1 +
templates/default/vzdump-body.html.hbs | 11 +++++++++
templates/default/vzdump-body.txt.hbs | 10 ++++++++
templates/default/vzdump-subject.txt.hbs | 1 +
17 files changed, 95 insertions(+), 46 deletions(-)
create mode 100644 templates/Makefile
create mode 100644 templates/default/package-updates-body.html.hbs
create mode 100644 templates/default/package-updates-body.txt.hbs
create mode 100644 templates/default/package-updates-subject.txt.hbs
create mode 100644 templates/default/replication-body.html.hbs
create mode 100644 templates/default/replication-body.txt.hbs
create mode 100644 templates/default/replication-subject.txt.hbs
create mode 100644 templates/default/test-body.html.hbs
create mode 100644 templates/default/test-body.txt.hbs
create mode 100644 templates/default/test-subject.txt.hbs
create mode 100644 templates/default/vzdump-body.html.hbs
create mode 100644 templates/default/vzdump-body.txt.hbs
create mode 100644 templates/default/vzdump-subject.txt.hbs
diff --git a/Makefile b/Makefile
index 28295395..337682b3 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,7 @@ DSC=$(PACKAGE)_$(DEB_VERSION).dsc
DEB=$(PACKAGE)_$(DEB_VERSION)_$(DEB_HOST_ARCH).deb
DESTDIR=
-SUBDIRS = aplinfo PVE bin www services configs network-hooks test
+SUBDIRS = aplinfo PVE bin www services configs network-hooks test templates
all: $(SUBDIRS)
set -e && for i in $(SUBDIRS); do $(MAKE) -C $$i; done
diff --git a/PVE/API2/APT.pm b/PVE/API2/APT.pm
index 19f0baca..d0e7c544 100644
--- a/PVE/API2/APT.pm
+++ b/PVE/API2/APT.pm
@@ -238,12 +238,6 @@ __PACKAGE__->register_method({
return $pkglist;
}});
-my $updates_available_subject_template = "New software packages available ({{hostname}})";
-my $updates_available_body_template = <<EOT;
-The following updates are available:
-{{table updates}}
-EOT
-
__PACKAGE__->register_method({
name => 'update_database',
path => 'update',
@@ -358,8 +352,7 @@ __PACKAGE__->register_method({
};
PVE::Notify::info(
- $updates_available_subject_template,
- $updates_available_body_template,
+ "package-updates",
$template_data,
$metadata_fields,
);
diff --git a/PVE/API2/Replication.pm b/PVE/API2/Replication.pm
index 0dc944c9..d84ac1ab 100644
--- a/PVE/API2/Replication.pm
+++ b/PVE/API2/Replication.pm
@@ -92,23 +92,6 @@ my sub _should_mail_at_failcount {
return $i * 48 == $fail_count;
};
-my $replication_error_subject_template = "Replication Job: '{{job-id}}' failed";
-my $replication_error_body_template = <<EOT;
-{{#verbatim}}
-Replication job '{{job-id}}' with target '{{job-target}}' and schedule '{{job-schedule}}' failed!
-
-Last successful sync: {{timestamp last-sync}}
-Next sync try: {{timestamp next-sync}}
-Failure count: {{failure-count}}
-
-{{#if (eq failure-count 3)}}
-Note: The system will now reduce the frequency of error reports, as the job
-appears to be stuck.
-{{/if}}
-Error:
-{{verbatim-monospaced error}}
-{{/verbatim}}
-EOT
my sub _handle_job_err {
my ($job, $err, $mail) = @_;
@@ -146,8 +129,7 @@ my sub _handle_job_err {
eval {
PVE::Notify::error(
- $replication_error_subject_template,
- $replication_error_body_template,
+ "replication",
$template_data,
$metadata_fields
);
diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 02244cd7..73bb2ba5 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -454,20 +454,6 @@ my sub get_hostname {
return $hostname;
}
-my $subject_template = "vzdump backup status ({{hostname}}): {{status-text}}";
-
-my $body_template = <<EOT;
-{{error-message}}
-{{heading-1 "Details"}}
-{{table guest-table}}
-{{#verbatim}}
-Total running time: {{duration total-time}}
-Total size: {{human-bytes total-size}}
-{{/verbatim}}
-{{heading-1 "Logs"}}
-{{verbatim-monospaced logs}}
-EOT
-
use constant MAX_LOG_SIZE => 1024*1024;
sub send_notification {
@@ -565,8 +551,7 @@ sub send_notification {
PVE::Notify::notify(
$severity,
- $subject_template,
- $body_template,
+ "vzdump",
$notification_props,
$fields,
$notification_config
@@ -577,8 +562,7 @@ sub send_notification {
# no email addresses were configured.
PVE::Notify::notify(
$severity,
- $subject_template,
- $body_template,
+ "vzdump",
$notification_props,
$fields,
);
diff --git a/templates/Makefile b/templates/Makefile
new file mode 100644
index 00000000..236988c5
--- /dev/null
+++ b/templates/Makefile
@@ -0,0 +1,24 @@
+NOTIFICATION_TEMPLATES= \
+ default/test-subject.txt.hbs \
+ default/test-body.txt.hbs \
+ default/test-body.html.hbs \
+ default/vzdump-subject.txt.hbs \
+ default/vzdump-body.txt.hbs \
+ default/vzdump-body.html.hbs \
+ default/replication-subject.txt.hbs \
+ default/replication-body.txt.hbs \
+ default/replication-body.html.hbs \
+ default/package-updates-subject.txt.hbs \
+ default/package-updates-body.txt.hbs \
+ default/package-updates-body.html.hbs \
+
+all:
+
+.PHONY: install
+install:
+ install -dm 0755 $(DESTDIR)/usr/share/pve-manager/templates/default
+ $(foreach i,$(NOTIFICATION_TEMPLATES), \
+ install -m644 $(i) $(DESTDIR)/usr/share/pve-manager/templates/$(i) ;)
+
+
+clean:
diff --git a/templates/default/package-updates-body.html.hbs b/templates/default/package-updates-body.html.hbs
new file mode 100644
index 00000000..d058e114
--- /dev/null
+++ b/templates/default/package-updates-body.html.hbs
@@ -0,0 +1,6 @@
+<html>
+ <body>
+ The following updates are available:
+ {{table updates}}
+ </body>
+</html>
diff --git a/templates/default/package-updates-body.txt.hbs b/templates/default/package-updates-body.txt.hbs
new file mode 100644
index 00000000..14bdbf9e
--- /dev/null
+++ b/templates/default/package-updates-body.txt.hbs
@@ -0,0 +1,3 @@
+The following updates are available:
+
+{{table updates}}
diff --git a/templates/default/package-updates-subject.txt.hbs b/templates/default/package-updates-subject.txt.hbs
new file mode 100644
index 00000000..556a67b8
--- /dev/null
+++ b/templates/default/package-updates-subject.txt.hbs
@@ -0,0 +1 @@
+New software packages available ({{hostname}})
diff --git a/templates/default/replication-body.html.hbs b/templates/default/replication-body.html.hbs
new file mode 100644
index 00000000..d1dea6a1
--- /dev/null
+++ b/templates/default/replication-body.html.hbs
@@ -0,0 +1,18 @@
+<html>
+ <body>
+ Replication job '{{job-id}}' with target '{{job-target}}' and schedule '{{job-schedule}}' failed!<br/><br/>
+
+ Last successful sync: {{timestamp last-sync}}<br/>
+ Next sync try: {{timestamp next-sync}}<br/>
+ Failure count: {{failure-count}}<br/>
+
+ {{#if (eq failure-count 3)}}
+ Note: The system will now reduce the frequency of error reports, as the job appears to be stuck.
+ {{/if}}
+ <br/>
+ Error:<br/>
+ <pre>
+{{error}}
+ </pre>
+ </body>
+</html>
diff --git a/templates/default/replication-body.txt.hbs b/templates/default/replication-body.txt.hbs
new file mode 100644
index 00000000..a9273fef
--- /dev/null
+++ b/templates/default/replication-body.txt.hbs
@@ -0,0 +1,12 @@
+Replication job '{{job-id}}' with target '{{job-target}}' and schedule '{{job-schedule}}' failed!
+
+Last successful sync: {{timestamp last-sync}}
+Next sync try: {{timestamp next-sync}}
+Failure count: {{failure-count}}
+
+{{#if (eq failure-count 3)}}
+Note: The system will now reduce the frequency of error reports, as the job
+appears to be stuck.
+{{/if}}
+Error:
+{{ error }}
diff --git a/templates/default/replication-subject.txt.hbs b/templates/default/replication-subject.txt.hbs
new file mode 100644
index 00000000..9dbaafcd
--- /dev/null
+++ b/templates/default/replication-subject.txt.hbs
@@ -0,0 +1 @@
+Replication Job: '{{job-id}}' failed
diff --git a/templates/default/test-body.html.hbs b/templates/default/test-body.html.hbs
new file mode 100644
index 00000000..26a43dde
--- /dev/null
+++ b/templates/default/test-body.html.hbs
@@ -0,0 +1 @@
+This is a test of the notification target '{{ target }}'.
diff --git a/templates/default/test-body.txt.hbs b/templates/default/test-body.txt.hbs
new file mode 100644
index 00000000..26a43dde
--- /dev/null
+++ b/templates/default/test-body.txt.hbs
@@ -0,0 +1 @@
+This is a test of the notification target '{{ target }}'.
diff --git a/templates/default/test-subject.txt.hbs b/templates/default/test-subject.txt.hbs
new file mode 100644
index 00000000..cb8e1320
--- /dev/null
+++ b/templates/default/test-subject.txt.hbs
@@ -0,0 +1 @@
+Test notification
diff --git a/templates/default/vzdump-body.html.hbs b/templates/default/vzdump-body.html.hbs
new file mode 100644
index 00000000..b6730cbb
--- /dev/null
+++ b/templates/default/vzdump-body.html.hbs
@@ -0,0 +1,11 @@
+<html>
+ <body>
+ {{error-message}}
+ <h1 style="font-size: 1.2em">Details</h1>
+ {{table guest-table}}
+ Total running time: {{duration total-time}}<br/>
+ Total size: {{human-bytes total-size}}<br/>
+ <h1 style="font-size: 1.2em">Logs</h1>
+ <pre>{{logs}}</pre>
+ </body>
+</html>
diff --git a/templates/default/vzdump-body.txt.hbs b/templates/default/vzdump-body.txt.hbs
new file mode 100644
index 00000000..90f5b0a4
--- /dev/null
+++ b/templates/default/vzdump-body.txt.hbs
@@ -0,0 +1,10 @@
+{{error-message}}
+Details
+=======
+{{table guest-table}}
+Total running time: {{duration total-time}}
+Total size: {{human-bytes total-size}}
+
+Logs
+====
+{{logs}}
diff --git a/templates/default/vzdump-subject.txt.hbs b/templates/default/vzdump-subject.txt.hbs
new file mode 100644
index 00000000..98a3d9aa
--- /dev/null
+++ b/templates/default/vzdump-subject.txt.hbs
@@ -0,0 +1 @@
+vzdump backup status ({{hostname}}): {{status-text}}
--
2.39.2
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
* [pve-devel] partially-applied-series: [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
` (19 preceding siblings ...)
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 20/20] notifications: use named templates instead of in-code templates Lukas Wagner
@ 2024-04-23 22:31 ` Thomas Lamprecht
20 siblings, 0 replies; 22+ messages in thread
From: Thomas Lamprecht @ 2024-04-23 22:31 UTC (permalink / raw)
To: Proxmox VE development discussion, Lukas Wagner
Am 19/04/2024 um 16:17 schrieb Lukas Wagner:
> proxmox:
>
> Lukas Wagner (12):
> notify: switch to file-based templating system
> notify: make api methods take config struct ownership
> notify: convert Option<Vec<T>> -> Vec<T> in config structs
> notify: don't make tests require pve-context
> notify: make the `mail-forwarder` feature depend on proxmox-sys
> notify: cargo.toml: add spaces before curly braces
> notify: give each notification a unique ID
> notify: api: add get_targets
> notify: derive `api` for Deleteable*Property
> notify: derive Deserialize/Serialize for Notification struct
> notify: pbs context: include nodename in default sendmail author
> notify: renderer: add relative-percentage helper from PBS
>
> proxmox-notify/Cargo.toml | 7 +-
> proxmox-notify/examples/render.rs | 63 ------
> proxmox-notify/src/api/gotify.rs | 48 ++---
> proxmox-notify/src/api/matcher.rs | 59 +++--
> proxmox-notify/src/api/mod.rs | 113 ++++++++--
> proxmox-notify/src/api/sendmail.rs | 60 +++---
> proxmox-notify/src/api/smtp.rs | 122 +++++------
> proxmox-notify/src/context/mod.rs | 10 +-
> proxmox-notify/src/context/pbs.rs | 18 +-
> proxmox-notify/src/context/pve.rs | 15 ++
> proxmox-notify/src/context/test.rs | 9 +
> proxmox-notify/src/endpoints/common/mail.rs | 20 +-
> proxmox-notify/src/endpoints/gotify.rs | 12 +-
> proxmox-notify/src/endpoints/sendmail.rs | 34 +--
> proxmox-notify/src/endpoints/smtp.rs | 38 ++--
> proxmox-notify/src/lib.rs | 59 ++---
> proxmox-notify/src/matcher.rs | 71 +++---
> proxmox-notify/src/renderer/html.rs | 14 --
> proxmox-notify/src/renderer/mod.rs | 226 ++++++++------------
> proxmox-notify/src/renderer/plaintext.rs | 39 ----
> 20 files changed, 506 insertions(+), 531 deletions(-)
> delete mode 100644 proxmox-notify/examples/render.rs
>
>
applied above, i.e., the proxmox/proxmox-notify ones.
As talked off-list, for the PVE side I'd like to wait out the next one or two
weeks until the dust of the release settles. Until then, the libpve-rs should
stay on the proxmox-notify 0.3 release, while a hot-fix would be slightly more
work, it's still doable – so I see no real practical issue in having this
divergence between PVE and PBS (where this is all completely new anyway).
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2024-04-23 22:31 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-19 14:17 [pve-devel] [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 01/20] notify: switch to file-based templating system Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 02/20] notify: make api methods take config struct ownership Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 03/20] notify: convert Option<Vec<T>> -> Vec<T> in config structs Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 04/20] notify: don't make tests require pve-context Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 05/20] notify: make the `mail-forwarder` feature depend on proxmox-sys Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 06/20] notify: cargo.toml: add spaces before curly braces Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 07/20] notify: give each notification a unique ID Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 08/20] notify: api: add get_targets Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 09/20] notify: derive `api` for Deleteable*Property Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 10/20] notify: derive Deserialize/Serialize for Notification struct Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 11/20] notify: pbs context: include nodename in default sendmail author Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox v2 12/20] notify: renderer: add relative-percentage helper from PBS Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 13/20] notify: use file based notification templates Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 14/20] notify: don't pass config structs by reference Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH proxmox-perl-rs v2 15/20] notify: adapt to Option<Vec<T>> to Vec<T> changes in proxmox_notify Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH cluster v2 16/20] notify: use named template instead of passing template strings Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH pve-ha-manager v2 17/20] env: notify: use named templates " Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 18/20] gitignore: ignore any test artifacts Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 19/20] tests: remove vzdump_notification test Lukas Wagner
2024-04-19 14:17 ` [pve-devel] [PATCH manager v2 20/20] notifications: use named templates instead of in-code templates Lukas Wagner
2024-04-23 22:31 ` [pve-devel] partially-applied-series: [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations Thomas Lamprecht
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox