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 v4 proxmox 23/69] notify: ensure that filter/group/endpoint names are unique
Date: Thu, 20 Jul 2023 16:31:50 +0200	[thread overview]
Message-ID: <20230720143236.652292-24-l.wagner@proxmox.com> (raw)
In-Reply-To: <20230720143236.652292-1-l.wagner@proxmox.com>

Otherwise, a filter with the same name as an already existing
endpoint or group can overwrite it.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 proxmox-notify/src/api/filter.rs   |  7 +----
 proxmox-notify/src/api/gotify.rs   | 10 +------
 proxmox-notify/src/api/group.rs    | 24 ++-------------
 proxmox-notify/src/api/mod.rs      | 47 ++++++++++++++++++++++++++++--
 proxmox-notify/src/api/sendmail.rs |  7 +----
 5 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/proxmox-notify/src/api/filter.rs b/proxmox-notify/src/api/filter.rs
index 824f802d..b5b62849 100644
--- a/proxmox-notify/src/api/filter.rs
+++ b/proxmox-notify/src/api/filter.rs
@@ -31,12 +31,7 @@ pub fn get_filter(config: &Config, name: &str) -> Result<FilterConfig, ApiError>
 /// Returns an `ApiError` if a filter with the same name already exists or
 /// if the filter could not be saved.
 pub fn add_filter(config: &mut Config, filter_config: &FilterConfig) -> Result<(), ApiError> {
-    if get_filter(config, &filter_config.name).is_ok() {
-        return Err(ApiError::bad_request(
-            format!("filter '{}' already exists", filter_config.name),
-            None,
-        ));
-    }
+    super::ensure_unique(config, &filter_config.name)?;
 
     config
         .config
diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs
index 5c4db4be..521dd167 100644
--- a/proxmox-notify/src/api/gotify.rs
+++ b/proxmox-notify/src/api/gotify.rs
@@ -43,15 +43,7 @@ pub fn add_endpoint(
         panic!("name for endpoint config and private config must be identical");
     }
 
-    if super::endpoint_exists(config, &endpoint_config.name) {
-        return Err(ApiError::bad_request(
-            format!(
-                "endpoint with name '{}' already exists!",
-                endpoint_config.name
-            ),
-            None,
-        ));
-    }
+    super::ensure_unique(config, &endpoint_config.name)?;
 
     if let Some(filter) = &endpoint_config.filter {
         // Check if filter exists
diff --git a/proxmox-notify/src/api/group.rs b/proxmox-notify/src/api/group.rs
index fe3de12f..d6b8f38d 100644
--- a/proxmox-notify/src/api/group.rs
+++ b/proxmox-notify/src/api/group.rs
@@ -31,12 +31,7 @@ pub fn get_group(config: &Config, name: &str) -> Result<GroupConfig, ApiError> {
 /// Returns an `ApiError` if a group with the same name already exists, or
 /// if the group could not be saved
 pub fn add_group(config: &mut Config, group_config: &GroupConfig) -> Result<(), ApiError> {
-    if get_group(config, &group_config.name).is_ok() {
-        return Err(ApiError::bad_request(
-            format!("group '{}' already exists", group_config.name),
-            None,
-        ));
-    }
+    super::ensure_unique(config, &group_config.name)?;
 
     if group_config.endpoint.is_empty() {
         return Err(ApiError::bad_request(
@@ -50,7 +45,7 @@ pub fn add_group(config: &mut Config, group_config: &GroupConfig) -> Result<(),
         super::filter::get_filter(config, filter)?;
     }
 
-    check_if_endpoints_exist(config, &group_config.endpoint)?;
+    super::ensure_endpoints_exist(config, &group_config.endpoint)?;
 
     config
         .config
@@ -91,7 +86,7 @@ pub fn update_group(
     }
 
     if let Some(endpoints) = &updater.endpoint {
-        check_if_endpoints_exist(config, endpoints)?;
+        super::ensure_endpoints_exist(config, endpoints)?;
         if endpoints.is_empty() {
             return Err(ApiError::bad_request(
                 "group must contain at least one endpoint",
@@ -138,19 +133,6 @@ pub fn delete_group(config: &mut Config, name: &str) -> Result<(), ApiError> {
     Ok(())
 }
 
-fn check_if_endpoints_exist(config: &Config, endpoints: &[String]) -> Result<(), ApiError> {
-    for endpoint in endpoints {
-        if !super::endpoint_exists(config, endpoint) {
-            return Err(ApiError::not_found(
-                format!("endoint '{endpoint}' does not exist"),
-                None,
-            ));
-        }
-    }
-
-    Ok(())
-}
-
 // groups cannot be empty, so only  build the tests if we have the
 // sendmail endpoint available
 #[cfg(all(test, feature = "sendmail"))]
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index e064b607..9a59719b 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -87,7 +87,7 @@ fn verify_digest(config: &Config, digest: Option<&[u8]>) -> Result<(), ApiError>
     Ok(())
 }
 
-fn endpoint_exists(config: &Config, name: &str) -> bool {
+fn ensure_endpoint_exists(config: &Config, name: &str) -> Result<(), ApiError> {
     let mut exists = false;
 
     #[cfg(feature = "sendmail")]
@@ -99,7 +99,33 @@ fn endpoint_exists(config: &Config, name: &str) -> bool {
         exists = exists || gotify::get_endpoint(config, name).is_ok();
     }
 
-    exists
+    if !exists {
+        Err(ApiError::not_found(
+            format!("endpoint '{name}' does not exist"),
+            None,
+        ))
+    } else {
+        Ok(())
+    }
+}
+
+fn ensure_endpoints_exist<T: AsRef<str>>(config: &Config, endpoints: &[T]) -> Result<(), ApiError> {
+    for endpoint in endpoints {
+        ensure_endpoint_exists(config, endpoint.as_ref())?;
+    }
+
+    Ok(())
+}
+
+fn ensure_unique(config: &Config, entity: &str) -> Result<(), ApiError> {
+    if config.config.sections.contains_key(entity) {
+        return Err(ApiError::bad_request(
+            format!("Cannot create '{entity}', an entity with the same name already exists"),
+            None,
+        ));
+    }
+
+    Ok(())
 }
 
 fn get_referrers(config: &Config, entity: &str) -> Result<HashSet<String>, ApiError> {
@@ -324,4 +350,21 @@ mod tests {
         assert!(ensure_unused(&config, "sendmail").is_err());
         assert!(ensure_unused(&config, "group").is_ok());
     }
+
+    #[test]
+    fn test_ensure_unique() {
+        let config = prepare_config().unwrap();
+
+        assert!(ensure_unique(&config, "sendmail").is_err());
+        assert!(ensure_unique(&config, "group").is_err());
+        assert!(ensure_unique(&config, "new").is_ok());
+    }
+
+    #[test]
+    fn test_ensure_endpoints_exist() {
+        let config = prepare_config().unwrap();
+
+        assert!(ensure_endpoints_exist(&config, &vec!["sendmail", "gotify"]).is_ok());
+        assert!(ensure_endpoints_exist(&config, &vec!["group", "filter"]).is_err());
+    }
 }
diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs
index bf225f29..59a57b47 100644
--- a/proxmox-notify/src/api/sendmail.rs
+++ b/proxmox-notify/src/api/sendmail.rs
@@ -33,12 +33,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result<SendmailConfig, ApiEr
 /// Returns an `ApiError` if an endpoint with the same name already exists,
 /// or if the endpoint could not be saved.
 pub fn add_endpoint(config: &mut Config, endpoint: &SendmailConfig) -> Result<(), ApiError> {
-    if super::endpoint_exists(config, &endpoint.name) {
-        return Err(ApiError::bad_request(
-            format!("endpoint with name '{}' already exists!", &endpoint.name),
-            None,
-        ));
-    }
+    super::ensure_unique(config, &endpoint.name)?;
 
     if let Some(filter) = &endpoint.filter {
         // Check if filter exists
-- 
2.39.2





  parent reply	other threads:[~2023-07-20 14:33 UTC|newest]

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-20 14:31 [pve-devel] [PATCH v4 many 00/69] fix #4156: introduce new notification system Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 01/69] section-config: derive Clone for SectionConfigData Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 02/69] schema: add schema/format for comments Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 03/69] add proxmox-notify crate Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 04/69] notify: preparation for the first endpoint plugin Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 05/69] notify: preparation for the API Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 06/69] notify: api: add API for sending notifications/testing endpoints Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 07/69] notify: add sendmail plugin Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 08/69] notify: api: add API for sendmail endpoints Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 09/69] notify: add gotify endpoint Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 10/69] notify: api: add API for gotify endpoints Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 11/69] notify: add notification groups Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 12/69] notify: api: add API for groups Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 13/69] notify: add notification filter mechanism Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 14/69] notify: api: add API for filters Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 15/69] notify: add template rendering Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 16/69] notify: add example for " Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 17/69] notify: add context Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 18/69] notify: sendmail: allow users as recipients Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 19/69] notify: sendmail: query default author/mailfrom from context Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 20/69] notify: gotify: add proxy support Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 21/69] notify: api: allow to query entities referenced by filter/target Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 22/69] notify: on deletion, check if a filter/endp. is still used by anything Lukas Wagner
2023-07-20 14:31 ` Lukas Wagner [this message]
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 24/69] notify: additional logging when sending a notification Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox 25/69] notify: add debian packaging Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 26/69] add PVE::RS::Notify module Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 27/69] notify: add api for sending notifications/testing endpoints Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 28/69] notify: add api for notification groups Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 29/69] notify: add api for sendmail endpoints Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 30/69] notify: add api for gotify endpoints Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 31/69] notify: add api for notification filters Lukas Wagner
2023-07-20 14:31 ` [pve-devel] [PATCH v4 proxmox-perl-rs 32/69] notify: sendmail: support the `mailto-user` parameter Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-perl-rs 33/69] notify: implement context for getting default author/mailfrom Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-perl-rs 34/69] notify: add context for getting http_proxy from datacenter.cfg Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-perl-rs 35/69] notify: add wrapper for `get_referenced_entities` Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-cluster 36/69] cluster files: add notifications.cfg Lukas Wagner
2023-07-24 13:02   ` [pve-devel] partially-applied: " Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-cluster 37/69] datacenter: add APT/fencing/replication notification configuration Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-cluster 38/69] add libpve-notify-perl package Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-guest-common 39/69] vzdump: add config options for new notification backend Lukas Wagner
2023-07-24 13:30   ` [pve-devel] partially-applied: " Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-common 40/69] JSONSchema: increase maxLength of config-digest to 64 Lukas Wagner
2023-07-24  9:56   ` [pve-devel] applied: " Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-ha-manager 41/69] manager: send notifications via new notification module Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 42/69] test: fix names of .PHONY targets Lukas Wagner
2023-07-24 13:59   ` Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 43/69] d/control: add dependency to `libpve-notify-perl` Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 44/69] vzdump: send notifications via new notification module Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 45/69] test: rename mail_test.pl to vzdump_notification_test.pl Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 46/69] api: apt: send notification via new notification module Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 47/69] api: replication: send notifications " Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 48/69] api: prepare api handler module for notification config Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 49/69] api: notification: add api routes for groups Lukas Wagner
2023-07-24 13:54   ` Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 50/69] api: notification: add api routes for sendmail endpoints Lukas Wagner
2023-07-24 13:55   ` Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 51/69] api: notification: add api routes for gotify endpoints Lukas Wagner
2023-07-24 13:55   ` Wolfgang Bumiller
2023-07-25  9:02   ` Thomas Lamprecht
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 52/69] api: notification: add api routes for filters Lukas Wagner
2023-07-24 13:56   ` Wolfgang Bumiller
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 53/69] api: notification: allow fetching notification targets Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 54/69] api: notification: allow to test targets Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 55/69] api: notification: disallow removing targets if they are used Lukas Wagner
2023-07-24 13:50   ` Wolfgang Bumiller
2023-07-24 14:09     ` Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 56/69] ui: backup: allow to select notification target for jobs Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 57/69] ui: backup: adapt backup job details to new notification params Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 58/69] ui: backup: allow to set notification-target for one-off backups Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 59/69] ui: allow to configure notification event -> target mapping Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 60/69] ui: add notification target configuration panel Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 61/69] ui: perm path: add ACL paths for notifications, usb and pci mappings Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 62/69] ui: perm path: increase width of the perm path selector combobox Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-manager 63/69] ui: dc: remove notify key from datacenter option view Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-widget-toolkit 64/69] notification: add gui for sendmail notification endpoints Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-widget-toolkit 65/69] notification: add gui for gotify " Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-widget-toolkit 66/69] notification: add gui for notification groups Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-widget-toolkit 67/69] notification: allow to select filter for notification targets Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 proxmox-widget-toolkit 68/69] notification: add ui for managing notification filters Lukas Wagner
2023-07-20 14:32 ` [pve-devel] [PATCH v4 pve-docs 69/69] add documentation for the new notification system Lukas Wagner
2023-07-24  9:07 ` [pve-devel] partially-applied: [PATCH v4 many 00/69] fix #4156: introduce " Wolfgang Bumiller
2023-07-24  9:20 ` [pve-devel] " Thomas Lamprecht
2023-07-24  9:23 ` [pve-devel] partially-applied: " Wolfgang Bumiller

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=20230720143236.652292-24-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