all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v2 proxmox 08/42] notify: add notification channels
Date: Wed, 24 May 2023 15:56:15 +0200	[thread overview]
Message-ID: <20230524135649.934881-9-l.wagner@proxmox.com> (raw)
In-Reply-To: <20230524135649.934881-1-l.wagner@proxmox.com>

A notification channel is basically a 'group' of endpoints.
When notifying, a notification is now sent to a 'channel',
and forwared to all included endpoints.

To illustrate why the channel concept is useful, consider a backup job.
The plan is to provide a new option there where the user can choose a
notification channel that should be used for any notifications.
The channel decouples the job configuration from any
endpoints present in the system.
I expected this to be nicer than:
  - notifying via *all* endpoints. If this is not desired, the user
    would be forced to configure notification filtering (to be
    introduced in a later patch). The filtering approach is a bit
    cumbersome, since it requires the filter to be adapted for each and
    every new backup job.
  - adding the endpoints directly to the job configuration. This would
    mean that new/removed endpoints have to be added/removed from *all*
    affected backup job configurations.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-notify/src/channel.rs |  53 ++++++++++++++
 proxmox-notify/src/config.rs  |   9 +++
 proxmox-notify/src/lib.rs     | 133 ++++++++++++++++++++++++++++++----
 3 files changed, 179 insertions(+), 16 deletions(-)
 create mode 100644 proxmox-notify/src/channel.rs

diff --git a/proxmox-notify/src/channel.rs b/proxmox-notify/src/channel.rs
new file mode 100644
index 00000000..dc9edf98
--- /dev/null
+++ b/proxmox-notify/src/channel.rs
@@ -0,0 +1,53 @@
+use crate::schema::COMMENT_SCHEMA;
+use proxmox_schema::{api, Updater};
+use serde::{Deserialize, Serialize};
+
+pub(crate) const CHANNEL_TYPENAME: &str = "channel";
+
+#[api(
+    properties: {
+        "endpoint": {
+            optional: true,
+            type: Array,
+            items: {
+                description: "Name of the included endpoint(s)",
+                type: String,
+            },
+        },
+        comment: {
+            optional: true,
+            schema: COMMENT_SCHEMA,
+        },
+    },
+)]
+#[derive(Debug, Serialize, Deserialize, Updater)]
+#[serde(rename_all = "kebab-case")]
+/// Config for notification channels
+pub struct ChannelConfig {
+    /// Name of the channel
+    #[updater(skip)]
+    pub name: String,
+    /// Endpoints for this channel
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub endpoint: Option<Vec<String>>,
+    /// Comment
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub comment: Option<String>,
+}
+
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum DeleteableChannelProperty {
+    Endpoint,
+    Comment,
+}
+
+impl ChannelConfig {
+    pub fn should_notify_via_endpoint(&self, endpoint: &str) -> bool {
+        if let Some(endpoints) = &self.endpoint {
+            endpoints.iter().any(|e| *e == endpoint)
+        } else {
+            false
+        }
+    }
+}
diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
index 362ca0fc..3064065b 100644
--- a/proxmox-notify/src/config.rs
+++ b/proxmox-notify/src/config.rs
@@ -2,6 +2,7 @@ use lazy_static::lazy_static;
 use proxmox_schema::{ApiType, ObjectSchema};
 use proxmox_section_config::{SectionConfig, SectionConfigData, SectionConfigPlugin};
 
+use crate::channel::{ChannelConfig, CHANNEL_TYPENAME};
 use crate::schema::BACKEND_NAME_SCHEMA;
 use crate::Error;
 
@@ -13,6 +14,14 @@ lazy_static! {
 fn config_init() -> SectionConfig {
     let mut config = SectionConfig::new(&BACKEND_NAME_SCHEMA);
 
+    const CHANNEL_SCHEMA: &ObjectSchema = ChannelConfig::API_SCHEMA.unwrap_object_schema();
+
+    config.register_plugin(SectionConfigPlugin::new(
+        CHANNEL_TYPENAME.to_string(),
+        Some(String::from("name")),
+        CHANNEL_SCHEMA,
+    ));
+
     config
 }
 
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index f2d0e16c..3b881e26 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -1,5 +1,6 @@
 use std::fmt::Display;
 
+use channel::{ChannelConfig, CHANNEL_TYPENAME};
 use proxmox_schema::api;
 use proxmox_section_config::SectionConfigData;
 use serde::{Deserialize, Serialize};
@@ -9,6 +10,7 @@ use serde_json::Value;
 use std::error::Error as StdError;
 
 pub mod api;
+pub mod channel;
 mod config;
 pub mod endpoints;
 pub mod schema;
@@ -19,6 +21,7 @@ pub enum Error {
     ConfigDeserialization(Box<dyn StdError + Send + Sync + 'static>),
     NotifyFailed(String, Box<dyn StdError + Send + Sync + 'static>),
     EndpointDoesNotExist(String),
+    ChannelDoesNotExist(String),
 }
 
 impl Display for Error {
@@ -36,6 +39,9 @@ impl Display for Error {
             Error::EndpointDoesNotExist(endpoint) => {
                 write!(f, "endpoint '{endpoint}' does not exist")
             }
+            Error::ChannelDoesNotExist(channel) => {
+                write!(f, "channel '{channel}' does not exist")
+            }
         }
     }
 }
@@ -47,6 +53,7 @@ impl StdError for Error {
             Error::ConfigDeserialization(err) => Some(&**err),
             Error::NotifyFailed(_, err) => Some(&**err),
             Error::EndpointDoesNotExist(_) => None,
+            Error::ChannelDoesNotExist(_) => None,
         }
     }
 }
@@ -142,6 +149,7 @@ impl Config {
 #[derive(Default)]
 pub struct Bus {
     endpoints: Vec<Box<dyn Endpoint>>,
+    channels: Vec<ChannelConfig>,
 }
 
 #[allow(unused_macros)]
@@ -204,7 +212,15 @@ impl Bus {
     pub fn from_config(config: &Config) -> Result<Self, Error> {
         let mut endpoints = Vec::new();
 
-        Ok(Bus { endpoints })
+        let channels = config
+            .config
+            .convert_to_typed_array(CHANNEL_TYPENAME)
+            .map_err(|err| Error::ConfigDeserialization(err.into()))?;
+
+        Ok(Bus {
+            endpoints,
+            channels,
+        })
     }
 
     #[cfg(test)]
@@ -212,20 +228,43 @@ impl Bus {
         self.endpoints.push(endpoint);
     }
 
-    /// Send a notification to all registered endpoints
-    pub fn send(&self, notification: &Notification) -> Result<(), Error> {
-        log::info!(
-            "sending notification with title '{title}'",
+    #[cfg(test)]
+    pub fn add_channel(&mut self, channel: ChannelConfig) {
+        self.channels.push(channel);
+    }
+
+    pub fn send(&self, channel: &str, notification: &Notification) -> Result<(), Error> {
+        log::debug!(
+            "sending notification with title `{title}`",
             title = notification.title
         );
 
+        // TODO: Maybe fallback to some default channel (e.g. send to all endpoints) in case
+        // the channel does not exist? Just to ensure that notifications are *never* swallowed...
+        let channel = self
+            .channels
+            .iter()
+            .find(|c| c.name == channel)
+            .ok_or(Error::ChannelDoesNotExist(channel.into()))?;
+
         for endpoint in &self.endpoints {
-            endpoint.send(notification).unwrap_or_else(|e| {
+            if !channel.should_notify_via_endpoint(endpoint.name()) {
+                log::debug!(
+                    "channel '{channel}' does not notify via endpoint '{endpoint}', skipping",
+                    channel = channel.name,
+                    endpoint = endpoint.name()
+                );
+                continue;
+            }
+
+            if let Err(e) = endpoint.send(notification) {
                 log::error!(
                     "could not notfiy via endpoint `{name}`: {e}",
                     name = endpoint.name()
-                )
-            })
+                );
+            } else {
+                log::info!("notified via endpoint `{name}`", name = endpoint.name());
+            }
         }
 
         Ok(())
@@ -257,6 +296,7 @@ mod tests {
 
     #[derive(Default, Clone)]
     struct MockEndpoint {
+        name: &'static str,
         messages: Rc<RefCell<Vec<Notification>>>,
     }
 
@@ -268,11 +308,18 @@ mod tests {
         }
 
         fn name(&self) -> &str {
-            "mock-endpoint"
+            self.name
         }
     }
 
     impl MockEndpoint {
+        fn new(name: &'static str, filter: Option<String>) -> Self {
+            Self {
+                name,
+                ..Default::default()
+            }
+        }
+
         fn messages(&self) -> Vec<Notification> {
             self.messages.borrow().clone()
         }
@@ -283,18 +330,72 @@ mod tests {
         let mock = MockEndpoint::default();
 
         let mut bus = Bus::default();
-
         bus.add_endpoint(Box::new(mock.clone()));
+        bus.add_channel(ChannelConfig {
+            name: "channel".to_string(),
+            endpoint: Some(vec!["".into()]),
+            comment: None,
+        });
+
+        bus.send(
+            "channel",
+            &Notification {
+                title: "Title".into(),
+                body: "Body".into(),
+                severity: Severity::Info,
+                properties: Default::default(),
+            },
+        )?;
 
-        bus.send(&Notification {
-            title: "Title".into(),
-            body: "Body".into(),
-            severity: Severity::Info,
-            properties: Default::default(),
-        })?;
         let messages = mock.messages();
         assert_eq!(messages.len(), 1);
 
         Ok(())
     }
+
+    #[test]
+    fn test_channels() -> Result<(), Error> {
+        let endpoint1 = MockEndpoint::new("mock1", None);
+        let endpoint2 = MockEndpoint::new("mock2", None);
+
+        let mut bus = Bus::default();
+
+        bus.add_channel(ChannelConfig {
+            name: "channel1".to_string(),
+            endpoint: Some(vec!["mock1".into()]),
+            comment: None,
+        });
+
+        bus.add_channel(ChannelConfig {
+            name: "channel2".to_string(),
+            endpoint: Some(vec!["mock2".into()]),
+            comment: None,
+        });
+
+        bus.add_endpoint(Box::new(endpoint1.clone()));
+        bus.add_endpoint(Box::new(endpoint2.clone()));
+
+        let send_to_channel = |channel| {
+            bus.send(
+                channel,
+                &Notification {
+                    title: "Title".into(),
+                    body: "Body".into(),
+                    severity: Severity::Info,
+                    properties: Default::default(),
+                },
+            )
+            .unwrap();
+        };
+
+        send_to_channel("channel1");
+        assert_eq!(endpoint1.messages().len(), 1);
+        assert_eq!(endpoint2.messages().len(), 0);
+
+        send_to_channel("channel2");
+        assert_eq!(endpoint1.messages().len(), 1);
+        assert_eq!(endpoint2.messages().len(), 1);
+
+        Ok(())
+    }
 }
-- 
2.30.2





  parent reply	other threads:[~2023-05-24 13:58 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-24 13:56 [pve-devel] [PATCH v2 cluster/guest-common/manager/ha-manager/proxmox{, -perl-rs} 00/42] fix #4156: introduce new notification module Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 01/42] add `proxmox-human-byte` crate Lukas Wagner
2023-06-26 11:58   ` Wolfgang Bumiller
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 02/42] human-byte: move tests to their own sub-module Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 03/42] add proxmox-notify crate Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 04/42] notify: add debian packaging Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 05/42] notify: preparation for the first endpoint plugin Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 06/42] notify: preparation for the API Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 07/42] notify: api: add API for sending notifications/testing endpoints Lukas Wagner
2023-05-24 13:56 ` Lukas Wagner [this message]
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 09/42] notify: api: add API for channels Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 10/42] notify: add sendmail plugin Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 11/42] notify: api: add API for sendmail endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 12/42] notify: add gotify endpoint Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 13/42] notify: api: add API for gotify endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 14/42] notify: add notification filter mechanism Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 15/42] notify: api: add API for filters Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 16/42] notify: add template rendering Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox 17/42] notify: add example for " Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 18/42] log: set default log level to 'info', add product specific logging env var Lukas Wagner
2023-06-05  7:27   ` Wolfgang Bumiller
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 19/42] add PVE::RS::Notify module Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 20/42] notify: add api for sending notifications/testing endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 21/42] notify: add api for notification channels Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 22/42] notify: add api for sendmail endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 23/42] notify: add api for gotify endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 proxmox-perl-rs 24/42] notify: add api for notification filters Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-cluster 25/42] cluster files: add notifications.cfg Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-guest-common 26/42] vzdump: add config options for new notification backend Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 27/42] test: fix names of .PHONY targets Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 28/42] add PVE::Notify module Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 29/42] vzdump: send notifications via new notification module Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 30/42] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 31/42] api: apt: send notification via new notification module Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 32/42] api: replication: send notifications " Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 33/42] ui: backup: allow to select notification channel for notifications Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 34/42] ui: backup: adapt backup job details to new notification params Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 35/42] ui: backup: allow to set notification-{channel, mode} for one-off backups Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 36/42] api: prepare api handler module for notification config Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 37/42] api: add api routes for notification channels Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 38/42] api: add api routes for sendmail endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 39/42] api: add api routes for gotify endpoints Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 40/42] api: add api routes for notification filters Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-manager 41/42] ui: backup: disable notification mode selector for now Lukas Wagner
2023-05-24 13:56 ` [pve-devel] [PATCH v2 pve-ha-manager 42/42] manager: send notifications via new notification module Lukas Wagner
2023-05-26  8:31 ` [pve-devel] [PATCH v2 cluster/guest-common/manager/ha-manager/proxmox{, -perl-rs} 00/42] fix #4156: introduce " Lukas Wagner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230524135649.934881-9-l.wagner@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal