From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id C20C6E113 for ; Mon, 17 Jul 2023 18:07:31 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A502611770 for ; Mon, 17 Jul 2023 18:07:01 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 17 Jul 2023 18:06:57 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id B483942B24 for ; Mon, 17 Jul 2023 18:06:57 +0200 (CEST) References: <20230717150051.710464-1-l.wagner@proxmox.com> <20230717150051.710464-3-l.wagner@proxmox.com> User-agent: mu4e 1.8.13; emacs 28.2 From: Maximiliano Sandoval To: Proxmox VE development discussion Date: Mon, 17 Jul 2023 17:48:45 +0200 In-reply-to: <20230717150051.710464-3-l.wagner@proxmox.com> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.001 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH v3 proxmox 02/66] notify: preparation for the first endpoint plugin X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Jul 2023 16:07:31 -0000 Lukas Wagner writes: > Signed-off-by: Lukas Wagner > --- > Cargo.toml | 1 + > proxmox-notify/Cargo.toml | 9 + > proxmox-notify/src/config.rs | 51 +++++ > proxmox-notify/src/endpoints/mod.rs | 0 > proxmox-notify/src/lib.rs | 311 ++++++++++++++++++++++++++++ > proxmox-notify/src/schema.rs | 43 ++++ > 6 files changed, 415 insertions(+) > create mode 100644 proxmox-notify/src/config.rs > create mode 100644 proxmox-notify/src/endpoints/mod.rs > create mode 100644 proxmox-notify/src/schema.rs > > diff --git a/Cargo.toml b/Cargo.toml > index 317593f0..ef8a050a 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -93,6 +93,7 @@ proxmox-lang =3D { version =3D "1.1", path =3D "proxmox= -lang" } > proxmox-rest-server =3D { version =3D "0.4.0", path =3D "proxmox-rest-se= rver" } > proxmox-router =3D { version =3D "1.3.1", path =3D "proxmox-router" } > proxmox-schema =3D { version =3D "1.3.7", path =3D "proxmox-schema" } > +proxmox-section-config =3D { version =3D "1.0.2", path =3D "proxmox-sect= ion-config" } > proxmox-serde =3D { version =3D "0.1.1", path =3D "proxmox-serde", featu= res =3D [ "serde_json" ] } > proxmox-sortable-macro =3D { version =3D "0.1.2", path =3D "proxmox-sort= able-macro" } > proxmox-sys =3D { version =3D "0.5.0", path =3D "proxmox-sys" } > diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml > index 2e69d5b0..37d175f0 100644 > --- a/proxmox-notify/Cargo.toml > +++ b/proxmox-notify/Cargo.toml > @@ -8,3 +8,12 @@ repository.workspace =3D true > exclude.workspace =3D true > > [dependencies] > +lazy_static.workspace =3D true > +log.workspace =3D true > +openssl.workspace =3D true > +proxmox-schema =3D { workspace =3D true, features =3D ["api-macro"]} > +proxmox-section-config =3D { workspace =3D true } > +proxmox-sys.workspace =3D true > +regex.workspace =3D true > +serde.workspace =3D true > +serde_json.workspace =3D true > diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs > new file mode 100644 > index 00000000..362ca0fc > --- /dev/null > +++ b/proxmox-notify/src/config.rs > @@ -0,0 +1,51 @@ > +use lazy_static::lazy_static; > +use proxmox_schema::{ApiType, ObjectSchema}; > +use proxmox_section_config::{SectionConfig, SectionConfigData, SectionCo= nfigPlugin}; > + > +use crate::schema::BACKEND_NAME_SCHEMA; > +use crate::Error; > + > +lazy_static! { Ideally this uses once_cell::sync::Lazy. > + pub static ref CONFIG: SectionConfig =3D config_init(); > + pub static ref PRIVATE_CONFIG: SectionConfig =3D private_config_init= (); > +} > + > +fn config_init() -> SectionConfig { > + let mut config =3D SectionConfig::new(&BACKEND_NAME_SCHEMA); unneeded mut keyword > + > + config > +} > + > +fn private_config_init() -> SectionConfig { > + let mut config =3D SectionConfig::new(&BACKEND_NAME_SCHEMA); Ditto > + > + config > +} > + > +pub fn config(raw_config: &str) -> Result<(SectionConfigData, [u8; 32]),= Error> { > + let digest =3D openssl::sha::sha256(raw_config.as_bytes()); > + let data =3D CONFIG > + .parse("notifications.cfg", raw_config) > + .map_err(|err| Error::ConfigDeserialization(err.into()))?; > + Ok((data, digest)) > +} > + > +pub fn private_config(raw_config: &str) -> Result<(SectionConfigData, [u= 8; 32]), Error> { > + let digest =3D openssl::sha::sha256(raw_config.as_bytes()); > + let data =3D PRIVATE_CONFIG > + .parse("priv/notifications.cfg", raw_config) > + .map_err(|err| Error::ConfigDeserialization(err.into()))?; > + Ok((data, digest)) > +} > + > +pub fn write(config: &SectionConfigData) -> Result { > + CONFIG > + .write("notifications.cfg", config) > + .map_err(|err| Error::ConfigSerialization(err.into())) > +} > + > +pub fn write_private(config: &SectionConfigData) -> Result { > + PRIVATE_CONFIG > + .write("priv/notifications.cfg", config) > + .map_err(|err| Error::ConfigSerialization(err.into())) > +} > diff --git a/proxmox-notify/src/endpoints/mod.rs b/proxmox-notify/src/end= points/mod.rs > new file mode 100644 > index 00000000..e69de29b > diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs > index e69de29b..7b90ee15 100644 > --- a/proxmox-notify/src/lib.rs > +++ b/proxmox-notify/src/lib.rs > @@ -0,0 +1,311 @@ > +use std::collections::HashMap; > +use std::fmt::Display; > + > +use proxmox_schema::api; > +use proxmox_section_config::SectionConfigData; > +use serde::{Deserialize, Serialize}; > +use serde_json::json; > +use serde_json::Value; > + > +use std::error::Error as StdError; > + > +mod config; > +pub mod endpoints; > +pub mod schema; > + > +#[derive(Debug)] > +pub enum Error { > + ConfigSerialization(Box), > + ConfigDeserialization(Box), > + NotifyFailed(String, Box), > + TargetDoesNotExist(String), > +} > + > +impl Display for Error { > + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { > + match self { > + Error::ConfigSerialization(err) =3D> { > + write!(f, "could not serialize configuration: {err}") > + } > + Error::ConfigDeserialization(err) =3D> { > + write!(f, "could not deserialize configuration: {err}") > + } > + Error::NotifyFailed(endpoint, err) =3D> { > + write!(f, "could not notify via endpoint(s): {endpoint}:= {err}") > + } > + Error::TargetDoesNotExist(target) =3D> { > + write!(f, "notification target '{target}' does not exist= ") > + } > + } > + } > +} > + > +impl StdError for Error { > + fn source(&self) -> Option<&(dyn StdError + 'static)> { > + match self { > + Error::ConfigSerialization(err) =3D> Some(&**err), Does this really need the double deref? > + Error::ConfigDeserialization(err) =3D> Some(&**err), > + Error::NotifyFailed(_, err) =3D> Some(&**err), > + Error::TargetDoesNotExist(_) =3D> None, > + } > + } > +} > + > +#[api()] > +#[derive(Clone, Debug, Copy, Serialize, Deserialize, PartialEq, Eq, Part= ialOrd)] > +#[serde(rename_all =3D "kebab-case")] > +/// Severity of a notification > +pub enum Severity { > + /// General information > + Info, > + /// A noteworthy event > + Notice, > + /// Warning > + Warning, > + /// Error > + Error, > +} > + > +/// Notification endpoint trait, implemented by all endpoint plugins > +pub trait Endpoint { > + /// Send a documentation > + fn send(&self, notification: &Notification) -> Result<(), Error>; > + > + /// The name/identifier for this endpoint > + fn name(&self) -> &str; > +} > + > +#[derive(Debug, Clone)] > +/// Notification which can be sent > +pub struct Notification { > + /// Notification severity > + pub severity: Severity, > + /// The title of the notification > + pub title: String, > + /// Notification text > + pub body: String, > + /// Additional metadata for the notification > + pub properties: Option, > +} > + > +/// Notification configuration > +pub struct Config { Might be good to derive Debug here. > + config: SectionConfigData, > + private_config: SectionConfigData, > + digest: [u8; 32], > + private_digest: [u8; 32], > +} > + > +impl Clone for Config { You can just derive Clone on Config, in general if a Copy type does the cor= rect thing they will be copied when clone() is called. > + fn clone(&self) -> Self { > + Self { > + config: SectionConfigData { > + sections: self.config.sections.clone(), > + order: self.config.order.clone(), > + }, > + private_config: SectionConfigData { > + sections: self.private_config.sections.clone(), > + order: self.private_config.order.clone(), > + }, > + digest: self.digest, > + private_digest: self.private_digest, > + } > + } > +} > + > +impl Config { > + /// Parse raw config > + pub fn new(raw_config: &str, raw_private_config: &str) -> Result { > + let (config, digest) =3D config::config(raw_config)?; > + let (private_config, private_digest) =3D config::private_config(= raw_private_config)?; > + > + Ok(Self { > + config, > + digest, > + private_config, > + private_digest, > + }) > + } > + > + /// Serialize config > + pub fn write(&self) -> Result<(String, String), Error> { > + Ok(( > + config::write(&self.config)?, > + config::write_private(&self.private_config)?, > + )) > + } > + > + /// Returns the SHA256 digest of the configuration. > + /// The digest is only computed once when the configuration deserial= ized. > + pub fn digest(&self) -> &[u8; 32] { > + &self.digest > + } > +} > + > +/// Notification bus - distributes notifications to all registered endpo= ints > +// The reason for the split between `Config` and this struct is to make = testing with mocked > +// endpoints a bit easier. > +#[derive(Default)] > +pub struct Bus { > + endpoints: HashMap>, > +} > + > +#[allow(unused_macros)] > +macro_rules! parse_endpoints_with_private_config { > + ($config:ident, $public_config:ty, $private_config:ty, $endpoint_typ= e:ident, $type_name:expr) =3D> { > + (|| -> Result>, Error> { > + let mut endpoints: Vec> =3D Vec::new(); > + > + let configs: Vec<$public_config> =3D $config > + .config > + .convert_to_typed_array($type_name) > + .map_err(|err| Error::ConfigDeserialization(err.into()))= ?; > + > + let private_configs: Vec<$private_config> =3D $config > + .private_config > + .convert_to_typed_array($type_name) > + .map_err(|err| Error::ConfigDeserialization(err.into()))= ?; > + > + for config in configs { > + if let Some(private_config) =3D private_configs.iter().f= ind(|p| p.name =3D=3D config.name) > + { > + endpoints.push(Box::new($endpoint_type { > + config, > + private_config: private_config.clone(), > + })); > + } else { > + log::error!( > + "Could not instantiate endpoint '{name}': privat= e config does not exist", > + name =3D config.name > + ); > + } > + } > + > + Ok(endpoints) > + })() > + }; > +} > + > +#[allow(unused_macros)] > +macro_rules! parse_endpoints_without_private_config { > + ($config:ident, $public_config:ty, $endpoint_type:ident, $type_name:= expr) =3D> { > + (|| -> Result>, Error> { > + let mut endpoints: Vec> =3D Vec::new(); > + > + let configs: Vec<$public_config> =3D $config > + .config > + .convert_to_typed_array($type_name) > + .map_err(|err| Error::ConfigDeserialization(err.into()))= ?; > + > + for config in configs { > + endpoints.push(Box::new($endpoint_type { config })); > + } > + > + Ok(endpoints) > + })() > + }; > +} > + > +impl Bus { > + /// Instantiate notification bus from a given configuration. > + pub fn from_config(config: &Config) -> Result { We are not using the config here? underscore config -> `_config`. > + let mut endpoints =3D HashMap::new(); Remove this mut. > + > + Ok(Bus { endpoints }) > + } > + > + #[cfg(test)] > + pub fn add_endpoint(&mut self, endpoint: Box) { > + self.endpoints.insert(endpoint.name().to_string(), endpoint); > + } > + > + pub fn send(&self, target: &str, notification: &Notification) -> Res= ult<(), Error> { > + log::info!( > + "sending notification with title '{title}'", > + title =3D notification.title > + ); > + > + let endpoint =3D self > + .endpoints > + .get(target) > + .ok_or(Error::TargetDoesNotExist(target.into()))?; Clippy: Use ok_or_else here =F0=9F=93=8E > + > + endpoint.send(notification).unwrap_or_else(|e| { > + log::error!( > + "could not notfiy via endpoint `{name}`: {e}", typo notfiy. > + name =3D endpoint.name() > + ) > + }); > + > + Ok(()) > + } > + > + pub fn test_target(&self, target: &str) -> Result<(), Error> { > + let endpoint =3D self > + .endpoints > + .get(target) > + .ok_or(Error::TargetDoesNotExist(target.into()))?; ok_or_else =F0=9F=93=8E > + > + endpoint.send(&Notification { > + severity: Severity::Info, > + title: "Test notification".into(), > + body: "This is a test of the notification target '{{ target = }}'".into(), > + properties: Some(json!({ "target": target })), > + })?; > + > + Ok(()) > + } > +} > + > +#[cfg(test)] > +mod tests { > + use std::{cell::RefCell, rc::Rc}; > + > + use super::*; > + > + #[derive(Default, Clone)] > + struct MockEndpoint { > + messages: Rc>>, > + } > + > + impl Endpoint for MockEndpoint { > + fn send(&self, message: &Notification) -> Result<(), Error> { > + self.messages.borrow_mut().push(message.clone()); > + > + Ok(()) > + } > + > + fn name(&self) -> &str { nit, maybe this trait method should return a static str instead. > + "mock-endpoint" > + } > + } > + > + impl MockEndpoint { > + fn messages(&self) -> Vec { > + self.messages.borrow().clone() > + } > + } > + > + #[test] > + fn test_add_mock_endpoint() -> Result<(), Error> { > + let mock =3D MockEndpoint::default(); > + > + let mut bus =3D Bus::default(); > + > + bus.add_endpoint(Box::new(mock.clone())); > + > + bus.send( > + "mock-endpoint", > + &Notification { > + title: "Title".into(), > + body: "Body".into(), > + severity: Severity::Info, > + properties: Default::default(), > + }, > + )?; > + let messages =3D mock.messages(); > + assert_eq!(messages.len(), 1); > + > + Ok(()) > + } > +} > diff --git a/proxmox-notify/src/schema.rs b/proxmox-notify/src/schema.rs > new file mode 100644 > index 00000000..68f11959 > --- /dev/null > +++ b/proxmox-notify/src/schema.rs > @@ -0,0 +1,43 @@ > +use proxmox_schema::{const_regex, ApiStringFormat, Schema, StringSchema}; > + > +// Copied from PBS > +macro_rules! proxmox_safe_id_regex_str { > + () =3D> { > + r"(?:[A-Za-z0-9_][A-Za-z0-9._\-]*)" > + }; > +} > + > +const_regex! { > + pub SINGLE_LINE_COMMENT_REGEX =3D r"^[[:^cntrl:]]*$"; > + pub PROXMOX_SAFE_ID_REGEX =3D concat!(r"^", proxmox_safe_id_regex_st= r!(), r"$"); > +} > + > +const SINGLE_LINE_COMMENT_FORMAT: ApiStringFormat =3D > + ApiStringFormat::Pattern(&SINGLE_LINE_COMMENT_REGEX); > + > +pub const COMMENT_SCHEMA: Schema =3D StringSchema::new("Comment.") > + .format(&SINGLE_LINE_COMMENT_FORMAT) > + .max_length(128) > + .schema(); > + > +pub const EMAIL_SCHEMA: Schema =3D StringSchema::new("E-Mail Address.") > + .format(&SINGLE_LINE_COMMENT_FORMAT) > + .min_length(2) > + .max_length(64) > + .schema(); > + > +pub const PROXMOX_SAFE_ID_FORMAT: ApiStringFormat =3D > + ApiStringFormat::Pattern(&PROXMOX_SAFE_ID_REGEX); > + > +pub const BACKEND_NAME_SCHEMA: Schema =3D StringSchema::new("Notificatio= n backend name.") > + .format(&PROXMOX_SAFE_ID_FORMAT) > + .min_length(3) > + .max_length(32) > + .schema(); > + > +pub const ENTITY_NAME_SCHEMA: Schema =3D > + StringSchema::new("Name schema for endpoints, filters and groups") > + .format(&PROXMOX_SAFE_ID_FORMAT) > + .min_length(2) > + .max_length(32) > + .schema(); nit, maybe these could go on the top.