* [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
@ 2024-07-12 11:27 Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets Lukas Wagner
` (11 more replies)
0 siblings, 12 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
Sending as an RFC because I don't want this merged yet; that being
said, the feature should be mostly finished at this point, I'd
appreciate any reviews and feedback.
This series adds support for webhook notification targets to PVE
and PBS.
A webhook is a HTTP API route provided by a third-party service that
can be used to inform the third-party about an event. In our case,
we can easily interact with various third-party notification/messaging
systems and send PVE/PBS notifications via this service.
The changes were tested against ntfy.sh, Discord and Slack.
The configuration of webhook targets allows one to configure:
- The URL
- The HTTP method (GET/POST/PUT)
- HTTP Headers
- Body
One can use handlebar templating to inject notification text and metadata
in the url, headers and body.
One challenge is the handling of sensitve tokens and other secrets.
Since the endpoint is completely generic, we cannot know in advance
whether the body/header/url contains sensitive values.
Thus we add 'secrets' which are stored in the protected config only
accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
secrets are accessible in URLs/headers/body via templating:
Url: https://example.com/{{ secrets.token }}
Secrets can only be set and updated, but never retrieved via the API.
In the UI, secrets are handled like other secret tokens/passwords.
Bumps for PVE:
- libpve-rs-perl needs proxmox-notify bumped
- pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
- proxmox-mail-forward needs proxmox-notify bumped
Bumps for PBS:
- proxmox-backup needs proxmox-notify bumped
- proxmox-mail-forward needs proxmox-notify bumped
Changes v1 -> v2:
- Rebase proxmox-notify changes
proxmox:
Lukas Wagner (2):
notify: implement webhook targets
notify: add api for webhook targets
proxmox-notify/Cargo.toml | 9 +-
proxmox-notify/src/api/mod.rs | 20 +
proxmox-notify/src/api/webhook.rs | 406 +++++++++++++++++++
proxmox-notify/src/config.rs | 23 ++
proxmox-notify/src/endpoints/mod.rs | 2 +
proxmox-notify/src/endpoints/webhook.rs | 509 ++++++++++++++++++++++++
proxmox-notify/src/lib.rs | 17 +
7 files changed, 983 insertions(+), 3 deletions(-)
create mode 100644 proxmox-notify/src/api/webhook.rs
create mode 100644 proxmox-notify/src/endpoints/webhook.rs
proxmox-perl-rs:
Lukas Wagner (2):
common: notify: add bindings for webhook API routes
common: notify: add bindings for get_targets
common/src/notify.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
proxmox-widget-toolkit:
Lukas Wagner (1):
notification: add UI for adding/updating webhook targets
src/Makefile | 1 +
src/Schema.js | 5 +
src/panel/WebhookEditPanel.js | 417 ++++++++++++++++++++++++++++++++++
3 files changed, 423 insertions(+)
create mode 100644 src/panel/WebhookEditPanel.js
pve-manager:
Lukas Wagner (2):
api: notifications: use get_targets impl from proxmox-notify
api: add routes for webhook notification endpoints
PVE/API2/Cluster/Notifications.pm | 297 ++++++++++++++++++++++++++----
1 file changed, 263 insertions(+), 34 deletions(-)
pve-docs:
Lukas Wagner (1):
notification: add documentation for webhook target endpoints.
notifications.adoc | 93 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
proxmox-backup:
Lukas Wagner (3):
api: notification: add API routes for webhook targets
ui: utils: enable webhook edit window
docs: notification: add webhook endpoint documentation
docs/notifications.rst | 100 +++++++++++++
src/api2/config/notifications/mod.rs | 2 +
src/api2/config/notifications/webhook.rs | 175 +++++++++++++++++++++++
www/Utils.js | 5 +
4 files changed, 282 insertions(+)
create mode 100644 src/api2/config/notifications/webhook.rs
proxmox-mail-forward:
Lukas Wagner (1):
bump proxmox-notify dependency
Cargo.toml | 2 +-
debian/control | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
Summary over all repositories:
19 files changed, 2121 insertions(+), 42 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] 26+ messages in thread
* [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-17 15:35 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 02/12] notify: add api for " Lukas Wagner
` (10 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
This target type allows users to perform HTTP requests to arbitrary
third party (notification) services, for instance
ntfy.sh/Discord/Slack.
The configuration for these endpoints allows one to freely configure
the URL, HTTP Method, headers and body. The URL, header values and
body support handlebars templating to inject notification text,
metadata and secrets. Secrets are stored in the protected
configuration file (e.g. /etc/pve/priv/notification.cfg) as key value
pairs, allowing users to protect sensitive tokens/passwords.
Secrets are accessible in handlebar templating via the secrets.*
namespace, e.g. if there is a secret named 'token', a body
could contain '{{ secrets.token }}' to inject the token into the
payload.
A couple of handlebars helpers are also provided:
- url-encoding (useful for templating in URLs)
- escape (escape any control characters in strings)
- json (print a property as json)
In the configuration, the body, header values and secret values
are stored in base64 encoding so that we can store any string we want.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/Cargo.toml | 9 +-
proxmox-notify/src/config.rs | 23 ++
proxmox-notify/src/endpoints/mod.rs | 2 +
proxmox-notify/src/endpoints/webhook.rs | 509 ++++++++++++++++++++++++
proxmox-notify/src/lib.rs | 17 +
5 files changed, 557 insertions(+), 3 deletions(-)
create mode 100644 proxmox-notify/src/endpoints/webhook.rs
diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
index 7801814d..484aff19 100644
--- a/proxmox-notify/Cargo.toml
+++ b/proxmox-notify/Cargo.toml
@@ -9,13 +9,15 @@ exclude.workspace = true
[dependencies]
anyhow.workspace = true
-base64.workspace = true
+base64 = { workspace = true, optional = true }
const_format.workspace = true
handlebars = { workspace = true }
+http = { workspace = true, optional = true }
lettre = { workspace = true, optional = true }
log.workspace = true
mail-parser = { workspace = true, optional = true }
openssl.workspace = true
+percent-encoding = { workspace = true, optional = true }
regex.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
@@ -31,10 +33,11 @@ proxmox-time.workspace = true
proxmox-uuid = { workspace = true, features = ["serde"] }
[features]
-default = ["sendmail", "gotify", "smtp"]
+default = ["sendmail", "gotify", "smtp", "webhook"]
mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys"]
-sendmail = ["dep:proxmox-sys"]
+sendmail = ["dep:proxmox-sys", "dep:base64"]
gotify = ["dep:proxmox-http"]
pve-context = ["dep:proxmox-sys"]
pbs-context = ["dep:proxmox-sys"]
smtp = ["dep:lettre"]
+webhook = ["dep:base64", "dep:http", "dep:percent-encoding", "dep:proxmox-http"]
diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
index 789c4a7d..4d0b53f7 100644
--- a/proxmox-notify/src/config.rs
+++ b/proxmox-notify/src/config.rs
@@ -57,6 +57,17 @@ fn config_init() -> SectionConfig {
GOTIFY_SCHEMA,
));
}
+ #[cfg(feature = "webhook")]
+ {
+ use crate::endpoints::webhook::{WebhookConfig, WEBHOOK_TYPENAME};
+
+ const WEBHOOK_SCHEMA: &ObjectSchema = WebhookConfig::API_SCHEMA.unwrap_object_schema();
+ config.register_plugin(SectionConfigPlugin::new(
+ WEBHOOK_TYPENAME.to_string(),
+ Some(String::from("name")),
+ WEBHOOK_SCHEMA,
+ ));
+ }
const MATCHER_SCHEMA: &ObjectSchema = MatcherConfig::API_SCHEMA.unwrap_object_schema();
config.register_plugin(SectionConfigPlugin::new(
@@ -110,6 +121,18 @@ fn private_config_init() -> SectionConfig {
));
}
+ #[cfg(feature = "webhook")]
+ {
+ use crate::endpoints::webhook::{WebhookPrivateConfig, WEBHOOK_TYPENAME};
+
+ const WEBHOOK_SCHEMA: &ObjectSchema =
+ WebhookPrivateConfig::API_SCHEMA.unwrap_object_schema();
+ config.register_plugin(SectionConfigPlugin::new(
+ WEBHOOK_TYPENAME.to_string(),
+ Some(String::from("name")),
+ WEBHOOK_SCHEMA,
+ ));
+ }
config
}
diff --git a/proxmox-notify/src/endpoints/mod.rs b/proxmox-notify/src/endpoints/mod.rs
index 97f79fcc..f20bee21 100644
--- a/proxmox-notify/src/endpoints/mod.rs
+++ b/proxmox-notify/src/endpoints/mod.rs
@@ -4,5 +4,7 @@ pub mod gotify;
pub mod sendmail;
#[cfg(feature = "smtp")]
pub mod smtp;
+#[cfg(feature = "webhook")]
+pub mod webhook;
mod common;
diff --git a/proxmox-notify/src/endpoints/webhook.rs b/proxmox-notify/src/endpoints/webhook.rs
new file mode 100644
index 00000000..7e976f6b
--- /dev/null
+++ b/proxmox-notify/src/endpoints/webhook.rs
@@ -0,0 +1,509 @@
+use handlebars::{
+ Context as HandlebarsContext, Handlebars, Helper, HelperResult, Output, RenderContext,
+ RenderError as HandlebarsRenderError,
+};
+use http::Request;
+use percent_encoding::AsciiSet;
+use proxmox_schema::property_string::PropertyString;
+use serde::{Deserialize, Serialize};
+use serde_json::{json, Map, Value};
+
+use proxmox_http::client::sync::Client;
+use proxmox_http::{HttpClient, HttpOptions, ProxyConfig};
+use proxmox_schema::api_types::COMMENT_SCHEMA;
+use proxmox_schema::{api, ApiStringFormat, ApiType, Schema, StringSchema, Updater};
+
+use crate::context::context;
+use crate::renderer::TemplateType;
+use crate::schema::ENTITY_NAME_SCHEMA;
+use crate::{renderer, Content, Endpoint, Error, Notification, Origin};
+
+pub(crate) const WEBHOOK_TYPENAME: &str = "webhook";
+
+#[api]
+#[derive(Serialize, Deserialize, Clone, Copy, Default)]
+#[serde(rename_all = "kebab-case")]
+/// HTTP Method to use
+pub enum HttpMethod {
+ /// HTTP POST
+ #[default]
+ Post,
+ /// HTTP PUT
+ Put,
+ /// HTTP GET
+ Get,
+}
+
+// We only ever need a &str, so we rather implement this
+// instead of Display.
+impl From<HttpMethod> for &str {
+ fn from(value: HttpMethod) -> Self {
+ match value {
+ HttpMethod::Post => "POST",
+ HttpMethod::Put => "PUT",
+ HttpMethod::Get => "GET",
+ }
+ }
+}
+
+#[api(
+ properties: {
+ name: {
+ schema: ENTITY_NAME_SCHEMA,
+ },
+ comment: {
+ optional: true,
+ schema: COMMENT_SCHEMA,
+ },
+ header: {
+ type: Array,
+ items: {
+ schema: KEY_AND_BASE64_VALUE_SCHEMA,
+ },
+ optional: true,
+ },
+ secret: {
+ type: Array,
+ items: {
+ schema: KEY_AND_BASE64_VALUE_SCHEMA,
+ },
+ optional: true,
+ },
+ }
+)]
+#[derive(Serialize, Deserialize, Updater, Default, Clone)]
+#[serde(rename_all = "kebab-case")]
+/// Config for Webhook notification endpoints
+pub struct WebhookConfig {
+ /// Name of the endpoint.
+ #[updater(skip)]
+ pub name: String,
+
+ pub method: HttpMethod,
+
+ /// Webhook URL.
+ pub url: String,
+ /// Array of HTTP headers. Each entry is a property string with a name and a value.
+ /// The value property contains the header in base64 encoding.
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub header: Vec<PropertyString<KeyAndBase64Val>>,
+ /// Body.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub body: Option<String>,
+
+ /// Comment.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub comment: Option<String>,
+ /// Disable this target.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub disable: Option<bool>,
+ /// Origin of this config entry.
+ #[serde(skip_serializing_if = "Option::is_none")]
+ #[updater(skip)]
+ pub origin: Option<Origin>,
+ /// Array of secrets. Each entry is a property string with a name and an optional value.
+ /// The value property contains the secret in base64 encoding.
+ /// For any API endpoints returning the endpoint config,
+ /// only the secret name but not the value will be returned.
+ /// When updating the config, also send all secrest that you want
+ /// to keep, setting only the name but not the value.
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ pub secret: Vec<PropertyString<KeyAndBase64Val>>,
+}
+
+#[api(
+ properties: {
+ name: {
+ schema: ENTITY_NAME_SCHEMA,
+ },
+ secret: {
+ type: Array,
+ items: {
+ schema: KEY_AND_BASE64_VALUE_SCHEMA,
+ },
+ optional: true,
+ },
+ }
+)]
+#[derive(Serialize, Deserialize, Clone, Updater, Default)]
+#[serde(rename_all = "kebab-case")]
+/// Private configuration for Webhook notification endpoints.
+/// This config will be saved to a separate configuration file with stricter
+/// permissions (root:root 0600)
+pub struct WebhookPrivateConfig {
+ /// Name of the endpoint
+ #[updater(skip)]
+ pub name: String,
+
+ #[serde(default, skip_serializing_if = "Vec::is_empty")]
+ #[updater(serde(skip_serializing_if = "Option::is_none"))]
+ /// Array of secrets. Each entry is a property string with a name,
+ /// and a value property. The value property contains the secret
+ /// in base64 encoding.
+ pub secret: Vec<PropertyString<KeyAndBase64Val>>,
+}
+
+/// A Webhook notification endpoint.
+pub struct WebhookEndpoint {
+ pub config: WebhookConfig,
+ pub private_config: WebhookPrivateConfig,
+}
+
+#[api]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum DeleteableWebhookProperty {
+ /// Delete `comment`
+ Comment,
+ /// Delete `disable`
+ Disable,
+ /// Delete `header`
+ Header,
+ /// Delete `body`
+ Body,
+ /// Delete `secret`
+ Secret,
+}
+
+#[api]
+#[derive(Serialize, Deserialize, Debug, Default, Clone)]
+/// Datatype used to represent key-value pairs, the value
+/// being encoded in base64.
+pub struct KeyAndBase64Val {
+ /// Name
+ pub(crate) name: String,
+ /// Base64 encoded value
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub(crate) value: Option<String>,
+}
+
+impl KeyAndBase64Val {
+ #[cfg(test)]
+ pub(crate) fn new_with_plain_value(name: &str, value: &str) -> Self {
+ let value = base64::encode(value);
+
+ Self {
+ name: name.into(),
+ value: Some(value),
+ }
+ }
+
+ pub(crate) fn decode_value(&self) -> Result<String, Error> {
+ let value = self.value.as_deref().unwrap_or_default();
+ let bytes = base64::decode(value).map_err(|_| {
+ Error::Generic(format!(
+ "could not decode base64 value with name '{}'",
+ self.name
+ ))
+ })?;
+ let value = String::from_utf8(bytes).map_err(|_| {
+ Error::Generic(format!(
+ "could not decode UTF8 string from base64, name={}",
+ self.name
+ ))
+ })?;
+
+ Ok(value)
+ }
+}
+
+pub const KEY_AND_BASE64_VALUE_SCHEMA: Schema =
+ StringSchema::new("String schema for pairs of keys and base64 encoded values")
+ .format(&ApiStringFormat::PropertyString(
+ &KeyAndBase64Val::API_SCHEMA,
+ ))
+ .schema();
+
+impl Endpoint for WebhookEndpoint {
+ fn send(&self, notification: &Notification) -> Result<(), Error> {
+ let request = self.build_request(notification)?;
+
+ self.create_client()?
+ .request(request)
+ .map_err(|err| Error::NotifyFailed(self.name().to_string(), err.into()))?;
+
+ Ok(())
+ }
+
+ fn name(&self) -> &str {
+ &self.config.name
+ }
+
+ /// Check if the endpoint is disabled
+ fn disabled(&self) -> bool {
+ self.config.disable.unwrap_or_default()
+ }
+}
+
+impl WebhookEndpoint {
+ fn create_client(&self) -> Result<Client, Error> {
+ let proxy_config = context()
+ .http_proxy_config()
+ .map(|url| ProxyConfig::parse_proxy_url(&url))
+ .transpose()
+ .map_err(|err| Error::NotifyFailed(self.name().to_string(), err.into()))?;
+
+ let options = HttpOptions {
+ proxy_config,
+ ..Default::default()
+ };
+
+ Ok(Client::new(options))
+ }
+
+ fn build_request(&self, notification: &Notification) -> Result<Request<String>, Error> {
+ let (title, message) = match ¬ification.content {
+ Content::Template {
+ template_name,
+ data,
+ } => {
+ let rendered_title =
+ renderer::render_template(TemplateType::Subject, template_name, data)?;
+ let rendered_message =
+ renderer::render_template(TemplateType::PlaintextBody, template_name, data)?;
+
+ (rendered_title, rendered_message)
+ }
+ #[cfg(feature = "mail-forwarder")]
+ Content::ForwardedMail { title, body, .. } => (title.clone(), body.clone()),
+ };
+
+ let mut fields = Map::new();
+
+ for (field_name, field_value) in ¬ification.metadata.additional_fields {
+ fields.insert(field_name.clone(), Value::String(field_value.to_string()));
+ }
+
+ let mut secrets = Map::new();
+
+ for secret in &self.private_config.secret {
+ let value = secret.decode_value()?;
+ secrets.insert(secret.name.clone(), Value::String(value));
+ }
+
+ let data = json!({
+ "title": &title,
+ "message": &message,
+ "severity": notification.metadata.severity,
+ "timestamp": notification.metadata.timestamp,
+ "fields": fields,
+ "secrets": secrets,
+ });
+
+ let handlebars = setup_handlebars();
+ let body_template = self.base_64_decode(self.config.body.as_deref().unwrap_or_default())?;
+
+ let body = handlebars
+ .render_template(&body_template, &data)
+ .map_err(|err| {
+ // TODO: Cleanup error types, they have become a bit messy.
+ // No user of the notify crate distinguish between the error types any way, so
+ // we can refactor without any issues....
+ Error::Generic(format!("failed to render webhook body: {err}"))
+ })?;
+
+ let url = handlebars
+ .render_template(&self.config.url, &data)
+ .map_err(|err| Error::Generic(format!("failed to render webhook url: {err}")))?;
+
+ let method: &str = self.config.method.into();
+ let mut builder = http::Request::builder().uri(url).method(method);
+
+ for header in &self.config.header {
+ let value = header.decode_value()?;
+
+ let value = handlebars.render_template(&value, &data).map_err(|err| {
+ Error::Generic(format!(
+ "failed to render header value template: {value}: {err}"
+ ))
+ })?;
+
+ builder = builder.header(header.name.clone(), value);
+ }
+
+ let request = builder
+ .body(body)
+ .map_err(|err| Error::Generic(format!("failed to build http request: {err}")))?;
+
+ Ok(request)
+ }
+
+ fn base_64_decode(&self, s: &str) -> Result<String, Error> {
+ // Also here, TODO: revisit Error variants for the *whole* crate.
+ let s = base64::decode(s)
+ .map_err(|err| Error::Generic(format!("could not decode base64 value: {err}")))?;
+
+ String::from_utf8(s).map_err(|err| {
+ Error::Generic(format!(
+ "base64 encoded value did not contain valid utf8: {err}"
+ ))
+ })
+ }
+}
+
+fn setup_handlebars() -> Handlebars<'static> {
+ let mut handlebars = Handlebars::new();
+
+ handlebars.register_helper("url-encode", Box::new(handlebars_percent_encode));
+ handlebars.register_helper("json", Box::new(handlebars_json));
+ handlebars.register_helper("escape", Box::new(handlebars_escape));
+
+ // There is no escape.
+ handlebars.register_escape_fn(handlebars::no_escape);
+
+ handlebars
+}
+
+fn handlebars_percent_encode(
+ h: &Helper,
+ _: &Handlebars,
+ _: &HandlebarsContext,
+ _rc: &mut RenderContext,
+ out: &mut dyn Output,
+) -> HelperResult {
+ let param0 = h
+ .param(0)
+ .and_then(|v| v.value().as_str())
+ .ok_or_else(|| HandlebarsRenderError::new("url-encode: missing parameter"))?;
+
+ // See https://developer.mozilla.org/en-US/docs/Glossary/Percent-encoding
+ const FRAGMENT: &AsciiSet = &percent_encoding::CONTROLS
+ .add(b':')
+ .add(b'/')
+ .add(b'?')
+ .add(b'#')
+ .add(b'[')
+ .add(b']')
+ .add(b'@')
+ .add(b'!')
+ .add(b'$')
+ .add(b'&')
+ .add(b'\'')
+ .add(b'(')
+ .add(b')')
+ .add(b'*')
+ .add(b'+')
+ .add(b',')
+ .add(b';')
+ .add(b'=')
+ .add(b'%')
+ .add(b' ');
+ let a = percent_encoding::utf8_percent_encode(param0, FRAGMENT);
+
+ out.write(&a.to_string())?;
+
+ Ok(())
+}
+
+fn handlebars_json(
+ h: &Helper,
+ _: &Handlebars,
+ _: &HandlebarsContext,
+ _rc: &mut RenderContext,
+ out: &mut dyn Output,
+) -> HelperResult {
+ let param0 = h
+ .param(0)
+ .map(|v| v.value())
+ .ok_or_else(|| HandlebarsRenderError::new("json: missing parameter"))?;
+
+ let json = serde_json::to_string(param0)?;
+ out.write(&json)?;
+
+ Ok(())
+}
+
+fn handlebars_escape(
+ h: &Helper,
+ _: &Handlebars,
+ _: &HandlebarsContext,
+ _rc: &mut RenderContext,
+ out: &mut dyn Output,
+) -> HelperResult {
+ let text = h
+ .param(0)
+ .and_then(|v| v.value().as_str())
+ .ok_or_else(|| HandlebarsRenderError::new("escape: missing text parameter"))?;
+
+ let val = Value::String(text.to_string());
+ let json = serde_json::to_string(&val)?;
+ out.write(&json[1..json.len() - 1])?;
+
+ Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+ use std::collections::HashMap;
+
+ use super::*;
+ use crate::Severity;
+
+
+
+ #[test]
+ fn test_build_request() -> Result<(), Error> {
+ let data = HashMap::from_iter([
+ ("hello".into(), "hello world".into()),
+ ("test".into(), "escaped\nstring".into()),
+ ]);
+
+ let body_template = r#"
+{{ fields.test }}
+{{ escape fields.test }}
+
+{{ json fields }}
+{{ json fields.hello }}
+
+{{ url-encode fields.hello }}
+
+{{ json severity }}
+
+"#;
+
+ let expected_body = r#"
+escaped
+string
+escaped\nstring
+
+{"hello":"hello world","test":"escaped\nstring"}
+"hello world"
+
+hello%20world
+
+"info"
+
+"#;
+
+ let endpoint = WebhookEndpoint {
+ config: WebhookConfig {
+ name: "test".into(),
+ method: HttpMethod::Post,
+ url: "http://localhost/{{ url-encode fields.hello }}".into(),
+ header: vec![
+ KeyAndBase64Val::new_with_plain_value("X-Severity", "{{ severity }}").into(),
+ ],
+ body: Some(base64::encode(body_template)),
+ ..Default::default()
+ },
+ private_config: WebhookPrivateConfig {
+ name: "test".into(),
+ ..Default::default()
+ },
+ };
+
+ let notification = Notification::from_template(Severity::Info, "foo", json!({}), data);
+
+ let request = endpoint.build_request(¬ification)?;
+
+ assert_eq!(request.uri(), "http://localhost/hello%20world");
+ assert_eq!(request.body(), expected_body);
+ assert_eq!(request.method(), "POST");
+
+ assert_eq!(request.headers().get("X-Severity").unwrap(), "info");
+
+ Ok(())
+ }
+}
diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
index 910dfa06..ebaba119 100644
--- a/proxmox-notify/src/lib.rs
+++ b/proxmox-notify/src/lib.rs
@@ -499,6 +499,23 @@ impl Bus {
);
}
+ #[cfg(feature = "webhook")]
+ {
+ use endpoints::webhook::WEBHOOK_TYPENAME;
+ use endpoints::webhook::{WebhookConfig, WebhookEndpoint, WebhookPrivateConfig};
+ endpoints.extend(
+ parse_endpoints_with_private_config!(
+ config,
+ WebhookConfig,
+ WebhookPrivateConfig,
+ WebhookEndpoint,
+ WEBHOOK_TYPENAME
+ )?
+ .into_iter()
+ .map(|e| (e.name().into(), e)),
+ );
+ }
+
let matchers = config
.config
.convert_to_typed_array(MATCHER_TYPENAME)
--
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] 26+ messages in thread
* [pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-17 15:35 ` Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes Lukas Wagner
` (9 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
All in all pretty similar to other endpoint APIs.
One thing worth noting is how secrets are handled. We never ever
return the values of previously stored secrets in get_endpoint(s)
calls, but only a list of the names of all secrets. This is needed
to build the UI, where we display all secrets that were set before in
a table.
For update calls, one is supposed to send all secrets that should be
kept and updated. If the value should be updated, the name and value
is expected, and if the current value should preseved, only the name
is sent. If a secret's name is not present in the updater, it will be
dropped. If 'secret' is present in the 'delete' array, all secrets
will be dropped, apart from those which are also set/preserved in the
same update call.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
proxmox-notify/src/api/mod.rs | 20 ++
proxmox-notify/src/api/webhook.rs | 406 ++++++++++++++++++++++++++++++
2 files changed, 426 insertions(+)
create mode 100644 proxmox-notify/src/api/webhook.rs
diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
index a7f6261c..7f823bc7 100644
--- a/proxmox-notify/src/api/mod.rs
+++ b/proxmox-notify/src/api/mod.rs
@@ -15,6 +15,8 @@ pub mod matcher;
pub mod sendmail;
#[cfg(feature = "smtp")]
pub mod smtp;
+#[cfg(feature = "webhook")]
+pub mod webhook;
// We have our own, local versions of http_err and http_bail, because
// we don't want to wrap the error in anyhow::Error. If we were to do that,
@@ -54,6 +56,9 @@ pub enum EndpointType {
/// Gotify endpoint
#[cfg(feature = "gotify")]
Gotify,
+ /// Webhook endpoint
+ #[cfg(feature = "webhook")]
+ Webhook,
}
#[api]
@@ -113,6 +118,17 @@ pub fn get_targets(config: &Config) -> Result<Vec<Target>, HttpError> {
})
}
+ #[cfg(feature = "webhook")]
+ for endpoint in webhook::get_endpoints(config)? {
+ targets.push(Target {
+ name: endpoint.name,
+ origin: endpoint.origin.unwrap_or(Origin::UserCreated),
+ endpoint_type: EndpointType::Webhook,
+ disable: endpoint.disable,
+ comment: endpoint.comment,
+ })
+ }
+
Ok(targets)
}
@@ -145,6 +161,10 @@ fn ensure_endpoint_exists(#[allow(unused)] config: &Config, name: &str) -> Resul
{
exists = exists || smtp::get_endpoint(config, name).is_ok();
}
+ #[cfg(feature = "webhook")]
+ {
+ exists = exists || webhook::get_endpoint(config, name).is_ok();
+ }
if !exists {
http_bail!(NOT_FOUND, "endpoint '{name}' does not exist")
diff --git a/proxmox-notify/src/api/webhook.rs b/proxmox-notify/src/api/webhook.rs
new file mode 100644
index 00000000..b7f17c55
--- /dev/null
+++ b/proxmox-notify/src/api/webhook.rs
@@ -0,0 +1,406 @@
+use proxmox_http_error::HttpError;
+use proxmox_schema::property_string::PropertyString;
+
+use crate::api::http_err;
+use crate::endpoints::webhook::{
+ DeleteableWebhookProperty, KeyAndBase64Val, WebhookConfig, WebhookConfigUpdater,
+ WebhookPrivateConfig, WEBHOOK_TYPENAME,
+};
+use crate::{http_bail, Config};
+
+use super::remove_private_config_entry;
+use super::set_private_config_entry;
+
+/// Get a list of all webhook endpoints.
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns a list of all webhook endpoints or a `HttpError` if the config is
+/// erroneous (`500 Internal server error`).
+pub fn get_endpoints(config: &Config) -> Result<Vec<WebhookConfig>, HttpError> {
+ let mut endpoints: Vec<WebhookConfig> = config
+ .config
+ .convert_to_typed_array(WEBHOOK_TYPENAME)
+ .map_err(|e| http_err!(NOT_FOUND, "Could not fetch endpoints: {e}"))?;
+
+ for endpoint in &mut endpoints {
+ let priv_config: WebhookPrivateConfig = config
+ .private_config
+ .lookup(WEBHOOK_TYPENAME, &endpoint.name)
+ .unwrap_or_default();
+
+ let mut secret_names = Vec::new();
+ for secret in priv_config.secret {
+ secret_names.push(
+ KeyAndBase64Val {
+ name: secret.name.clone(),
+ value: None,
+ }
+ .into(),
+ )
+ }
+
+ endpoint.secret = secret_names;
+ }
+
+ Ok(endpoints)
+}
+
+/// Get webhook endpoint with given `name`
+///
+/// The caller is responsible for any needed permission checks.
+/// Returns the endpoint or a `HttpError` if the endpoint was not found (`404 Not found`).
+pub fn get_endpoint(config: &Config, name: &str) -> Result<WebhookConfig, HttpError> {
+ let mut endpoint: WebhookConfig = config
+ .config
+ .lookup(WEBHOOK_TYPENAME, name)
+ .map_err(|_| http_err!(NOT_FOUND, "endpoint '{name}' not found"))?;
+
+ let priv_config: Option<WebhookPrivateConfig> = config
+ .private_config
+ .lookup(WEBHOOK_TYPENAME, &endpoint.name)
+ .ok();
+
+ let mut secret_names = Vec::new();
+ if let Some(priv_config) = priv_config {
+ for secret in &priv_config.secret {
+ secret_names.push(
+ KeyAndBase64Val {
+ name: secret.name.clone(),
+ value: None,
+ }
+ .into(),
+ );
+ }
+ }
+
+ endpoint.secret = secret_names;
+
+ Ok(endpoint)
+}
+
+/// Add a new webhook endpoint.
+///
+/// The caller is responsible for any needed permission checks.
+/// The caller also responsible for locking the configuration files.
+/// Returns a `HttpError` if:
+/// - an entity with the same name already exists (`400 Bad request`)
+/// - the configuration could not be saved (`500 Internal server error`)
+pub fn add_endpoint(
+ config: &mut Config,
+ mut endpoint_config: WebhookConfig,
+) -> Result<(), HttpError> {
+ super::ensure_unique(config, &endpoint_config.name)?;
+
+ let secrets = std::mem::take(&mut endpoint_config.secret);
+
+ set_private_config_entry(
+ config,
+ &WebhookPrivateConfig {
+ name: endpoint_config.name.clone(),
+ secret: secrets,
+ },
+ WEBHOOK_TYPENAME,
+ &endpoint_config.name,
+ )?;
+
+ config
+ .config
+ .set_data(&endpoint_config.name, WEBHOOK_TYPENAME, &endpoint_config)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not save endpoint '{}': {e}",
+ endpoint_config.name
+ )
+ })
+}
+
+/// Update existing webhook endpoint
+///
+/// The caller is responsible for any needed permission checks.
+/// The caller also responsible for locking the configuration files.
+/// Returns a `HttpError` if:
+/// - an entity with the same name already exists (`400 Bad request`)
+/// - the configuration could not be saved (`500 Internal server error`)
+pub fn update_endpoint(
+ config: &mut Config,
+ name: &str,
+ config_updater: WebhookConfigUpdater,
+ delete: Option<&[DeleteableWebhookProperty]>,
+ digest: Option<&[u8]>,
+) -> Result<(), HttpError> {
+ super::verify_digest(config, digest)?;
+
+ let mut endpoint = get_endpoint(config, name)?;
+ endpoint.secret.clear();
+
+ let old_secrets = config
+ .private_config
+ .lookup::<WebhookPrivateConfig>(WEBHOOK_TYPENAME, name)
+ .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "could not read secret config: {err}"))?
+ .secret;
+
+ if let Some(delete) = delete {
+ for deleteable_property in delete {
+ match deleteable_property {
+ DeleteableWebhookProperty::Comment => endpoint.comment = None,
+ DeleteableWebhookProperty::Disable => endpoint.disable = None,
+ DeleteableWebhookProperty::Header => endpoint.header = Vec::new(),
+ DeleteableWebhookProperty::Body => endpoint.body = None,
+ DeleteableWebhookProperty::Secret => {
+ set_private_config_entry(
+ config,
+ &WebhookPrivateConfig {
+ name: name.into(),
+ secret: Vec::new(),
+ },
+ WEBHOOK_TYPENAME,
+ name,
+ )?;
+ }
+ }
+ }
+ }
+
+ // Destructuring makes sure we don't forget any members
+ let WebhookConfigUpdater {
+ url,
+ body,
+ header,
+ method,
+ disable,
+ comment,
+ secret,
+ } = config_updater;
+
+ if let Some(url) = url {
+ endpoint.url = url;
+ }
+
+ if let Some(body) = body {
+ endpoint.body = Some(body);
+ }
+
+ if let Some(header) = header {
+ endpoint.header = header;
+ }
+
+ if let Some(method) = method {
+ endpoint.method = method;
+ }
+
+ if let Some(disable) = disable {
+ endpoint.disable = Some(disable);
+ }
+
+ if let Some(comment) = comment {
+ endpoint.comment = Some(comment);
+ }
+
+ if let Some(secret) = secret {
+ let mut new_secrets: Vec<PropertyString<KeyAndBase64Val>> = Vec::new();
+
+ for new_secret in &secret {
+ let a = if new_secret.value.is_some() {
+ // Make sure it is valid base64 encoded data
+ let _ = new_secret.decode_value().map_err(|_| {
+ http_err!(
+ BAD_REQUEST,
+ "secret '{}' does not have valid base64 encoded data",
+ new_secret.name
+ )
+ })?;
+ new_secret.clone()
+ } else if let Some(old_secret) = old_secrets.iter().find(|v| v.name == new_secret.name)
+ {
+ old_secret.clone()
+ } else {
+ http_bail!(BAD_REQUEST, "secret '{}' not known", new_secret.name);
+ };
+
+ if new_secrets.iter().any(|s| s.name == a.name) {
+ http_bail!(BAD_REQUEST, "secret '{}' defined multiple times", a.name)
+ }
+
+ new_secrets.push(a);
+ }
+
+ set_private_config_entry(
+ config,
+ &WebhookPrivateConfig {
+ name: name.into(),
+ secret: new_secrets,
+ },
+ WEBHOOK_TYPENAME,
+ name,
+ )?;
+ }
+
+ config
+ .config
+ .set_data(name, WEBHOOK_TYPENAME, &endpoint)
+ .map_err(|e| {
+ http_err!(
+ INTERNAL_SERVER_ERROR,
+ "could not save endpoint '{name}': {e}"
+ )
+ })
+}
+
+/// Delete existing webhook endpoint
+///
+/// The caller is responsible for any needed permission checks.
+/// The caller also responsible for locking the configuration files.
+/// Returns a `HttpError` if:
+/// - the entity does not exist (`404 Not found`)
+/// - the endpoint is still referenced by another entity (`400 Bad request`)
+pub fn delete_endpoint(config: &mut Config, name: &str) -> Result<(), HttpError> {
+ // Check if the endpoint exists
+ let _ = get_endpoint(config, name)?;
+ super::ensure_safe_to_delete(config, name)?;
+
+ remove_private_config_entry(config, name)?;
+ config.config.sections.remove(name);
+
+ Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::{api::test_helpers::empty_config, endpoints::webhook::HttpMethod};
+
+ use base64::encode;
+
+ pub fn add_default_webhook_endpoint(config: &mut Config) -> Result<(), HttpError> {
+ add_endpoint(
+ config,
+ WebhookConfig {
+ name: "webhook-endpoint".into(),
+ method: HttpMethod::Post,
+ url: "http://example.com/webhook".into(),
+ header: vec![KeyAndBase64Val::new_with_plain_value(
+ "Content-Type",
+ "application/json",
+ )
+ .into()],
+ body: Some(encode("this is the body")),
+ comment: Some("comment".into()),
+ disable: Some(false),
+ secret: vec![KeyAndBase64Val::new_with_plain_value("token", "secret").into()],
+ ..Default::default()
+ },
+ )?;
+
+ assert!(get_endpoint(config, "webhook-endpoint").is_ok());
+ Ok(())
+ }
+
+ #[test]
+ 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());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_update_invalid_digest_returns_error() -> Result<(), HttpError> {
+ let mut config = empty_config();
+ add_default_webhook_endpoint(&mut config)?;
+
+ assert!(update_endpoint(
+ &mut config,
+ "webhook-endpoint",
+ Default::default(),
+ None,
+ Some(&[0; 32])
+ )
+ .is_err());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_update() -> Result<(), HttpError> {
+ let mut config = empty_config();
+ add_default_webhook_endpoint(&mut config)?;
+
+ let digest = config.digest;
+
+ update_endpoint(
+ &mut config,
+ "webhook-endpoint",
+ WebhookConfigUpdater {
+ url: Some("http://new.example.com/webhook".into()),
+ comment: Some("newcomment".into()),
+ method: Some(HttpMethod::Put),
+ // Keep the old token and set a new one
+ secret: Some(vec![
+ KeyAndBase64Val::new_with_plain_value("token2", "newsecret").into(),
+ KeyAndBase64Val {
+ name: "token".into(),
+ value: None,
+ }
+ .into(),
+ ]),
+ ..Default::default()
+ },
+ None,
+ Some(&digest),
+ )?;
+
+ let endpoint = get_endpoint(&config, "webhook-endpoint")?;
+
+ assert_eq!(endpoint.url, "http://new.example.com/webhook".to_string());
+ assert_eq!(endpoint.comment, Some("newcomment".to_string()));
+ assert!(matches!(endpoint.method, HttpMethod::Put));
+
+ let secrets = config
+ .private_config
+ .lookup::<WebhookPrivateConfig>(WEBHOOK_TYPENAME, "webhook-endpoint")
+ .unwrap()
+ .secret;
+
+ assert_eq!(secrets[1].name, "token".to_string());
+ assert_eq!(secrets[1].value, Some(encode("secret")));
+ assert_eq!(secrets[0].name, "token2".to_string());
+ assert_eq!(secrets[0].value, Some(encode("newsecret")));
+
+ // Test property deletion
+ update_endpoint(
+ &mut config,
+ "webhook-endpoint",
+ Default::default(),
+ Some(&[DeleteableWebhookProperty::Comment, DeleteableWebhookProperty::Secret]),
+ None,
+ )?;
+
+ let endpoint = get_endpoint(&config, "webhook-endpoint")?;
+ assert_eq!(endpoint.comment, None);
+
+ let secrets = config
+ .private_config
+ .lookup::<WebhookPrivateConfig>(WEBHOOK_TYPENAME, "webhook-endpoint")
+ .unwrap()
+ .secret;
+
+ assert!(secrets.is_empty());
+
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_gotify_endpoint_delete() -> Result<(), HttpError> {
+ let mut config = empty_config();
+ add_default_webhook_endpoint(&mut config)?;
+
+ delete_endpoint(&mut config, "webhook-endpoint")?;
+ assert!(delete_endpoint(&mut config, "webhook-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] 26+ messages in thread
* [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 02/12] notify: add api for " Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-17 15:35 ` Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets Lukas Wagner
` (8 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
common/src/notify.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index e1b006b..fe192d5 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -19,6 +19,9 @@ mod export {
DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig,
SmtpPrivateConfigUpdater,
};
+ use proxmox_notify::endpoints::webhook::{
+ DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
+ };
use proxmox_notify::matcher::{
CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
MatcherConfigUpdater, SeverityMatcher,
@@ -393,6 +396,66 @@ mod export {
api::smtp::delete_endpoint(&mut config, name)
}
+ #[export(serialize_error)]
+ fn get_webhook_endpoints(
+ #[try_from_ref] this: &NotificationConfig,
+ ) -> Result<Vec<WebhookConfig>, HttpError> {
+ let config = this.config.lock().unwrap();
+ api::webhook::get_endpoints(&config)
+ }
+
+ #[export(serialize_error)]
+ fn get_webhook_endpoint(
+ #[try_from_ref] this: &NotificationConfig,
+ id: &str,
+ ) -> Result<WebhookConfig, HttpError> {
+ let config = this.config.lock().unwrap();
+ api::webhook::get_endpoint(&config, id)
+ }
+
+ #[export(serialize_error)]
+ #[allow(clippy::too_many_arguments)]
+ fn add_webhook_endpoint(
+ #[try_from_ref] this: &NotificationConfig,
+ endpoint_config: WebhookConfig,
+ ) -> Result<(), HttpError> {
+ let mut config = this.config.lock().unwrap();
+ api::webhook::add_endpoint(
+ &mut config,
+ endpoint_config,
+ )
+ }
+
+ #[export(serialize_error)]
+ #[allow(clippy::too_many_arguments)]
+ fn update_webhook_endpoint(
+ #[try_from_ref] this: &NotificationConfig,
+ name: &str,
+ config_updater: WebhookConfigUpdater,
+ delete: Option<Vec<DeleteableWebhookProperty>>,
+ digest: Option<&str>,
+ ) -> Result<(), HttpError> {
+ let mut config = this.config.lock().unwrap();
+ let digest = decode_digest(digest)?;
+
+ api::webhook::update_endpoint(
+ &mut config,
+ name,
+ config_updater,
+ delete.as_deref(),
+ digest.as_deref(),
+ )
+ }
+
+ #[export(serialize_error)]
+ fn delete_webhook_endpoint(
+ #[try_from_ref] this: &NotificationConfig,
+ name: &str,
+ ) -> Result<(), HttpError> {
+ let mut config = this.config.lock().unwrap();
+ api::webhook::delete_endpoint(&mut config, name)
+ }
+
#[export(serialize_error)]
fn get_matchers(
#[try_from_ref] this: &NotificationConfig,
--
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] 26+ messages in thread
* [pve-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (2 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-17 15:36 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH widget-toolkit v2 05/12] notification: add UI for adding/updating webhook targets Lukas Wagner
` (7 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
This allows us to drop the impl of that function on the perl side.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
common/src/notify.rs | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/common/src/notify.rs b/common/src/notify.rs
index fe192d5..0f8a35d 100644
--- a/common/src/notify.rs
+++ b/common/src/notify.rs
@@ -27,6 +27,7 @@ mod export {
MatcherConfigUpdater, SeverityMatcher,
};
use proxmox_notify::{api, Config, Notification, Severity};
+ use proxmox_notify::api::Target;
pub struct NotificationConfig {
config: Mutex<Config>,
@@ -112,6 +113,14 @@ mod export {
api::common::send(&config, ¬ification)
}
+ #[export(serialize_error)]
+ fn get_targets(
+ #[try_from_ref] this: &NotificationConfig,
+ ) -> Result<Vec<Target>, HttpError> {
+ let config = this.config.lock().unwrap();
+ api::get_targets(&config)
+ }
+
#[export(serialize_error)]
fn test_target(
#[try_from_ref] this: &NotificationConfig,
--
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] 26+ messages in thread
* [pve-devel] [PATCH widget-toolkit v2 05/12] notification: add UI for adding/updating webhook targets
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (3 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 06/12] api: notifications: use get_targets impl from proxmox-notify Lukas Wagner
` (6 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
The widgets for editing the headers/secrets were adapted from
the 'Tag Edit' dialog from PVE's datacenter options.
Apart from that, the new dialog is rather standard. I've decided
to put the http method and url in a single row, mostly to
save space and also to make it analogous to how an actual http request
is structured (VERB URL, followed by headers, followed by the body).
The secrets are a mechanism to store tokens/passwords in the
protected notification config. Secrets are accessible via
templating in the URL, headers and body via {{ secrets.NAME }}.
Secrets can only be set/updated, but not retrieved/displayed.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/Makefile | 1 +
src/Schema.js | 5 +
src/panel/WebhookEditPanel.js | 417 ++++++++++++++++++++++++++++++++++
3 files changed, 423 insertions(+)
create mode 100644 src/panel/WebhookEditPanel.js
diff --git a/src/Makefile b/src/Makefile
index 0478251..cfaffd7 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -78,6 +78,7 @@ JSSRC= \
panel/StatusView.js \
panel/TfaView.js \
panel/NotesView.js \
+ panel/WebhookEditPanel.js \
window/Edit.js \
window/PasswordEdit.js \
window/SafeDestroy.js \
diff --git a/src/Schema.js b/src/Schema.js
index 42541e0..cd1c306 100644
--- a/src/Schema.js
+++ b/src/Schema.js
@@ -65,6 +65,11 @@ Ext.define('Proxmox.Schema', { // a singleton
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
+ webhook: {
+ name: 'Webhook',
+ ipanel: 'pmxWebhookEditPanel',
+ iconCls: 'fa-bell-o',
+ },
},
// to add or change existing for product specific ones
diff --git a/src/panel/WebhookEditPanel.js b/src/panel/WebhookEditPanel.js
new file mode 100644
index 0000000..dfc7f3f
--- /dev/null
+++ b/src/panel/WebhookEditPanel.js
@@ -0,0 +1,417 @@
+Ext.define('Proxmox.panel.WebhookEditPanel', {
+ extend: 'Proxmox.panel.InputPanel',
+ xtype: 'pmxWebhookEditPanel',
+ mixins: ['Proxmox.Mixin.CBind'],
+ onlineHelp: 'notification_targets_webhook',
+
+ type: 'webhook',
+
+ columnT: [
+
+ ],
+
+ column1: [
+ {
+ xtype: 'pmxDisplayEditField',
+ name: 'name',
+ cbind: {
+ value: '{name}',
+ editable: '{isCreate}',
+ },
+ fieldLabel: gettext('Endpoint Name'),
+ allowBlank: false,
+ },
+ ],
+
+ column2: [
+ {
+ xtype: 'proxmoxcheckbox',
+ name: 'enable',
+ fieldLabel: gettext('Enable'),
+ allowBlank: false,
+ checked: true,
+ },
+ ],
+
+ columnB: [
+ {
+ layout: 'hbox',
+ border: false,
+ margin: '0 0 5 0',
+ items: [
+ {
+ xtype: 'displayfield',
+ value: gettext('Method/URL:'),
+ width: 125,
+ },
+ {
+ xtype: 'proxmoxKVComboBox',
+ name: 'method',
+ //fieldLabel: gettext('Method'),
+ editable: false,
+ value: 'post',
+ comboItems: [
+ ['post', 'POST'],
+ ['put', 'PUT'],
+ ['get', 'GET'],
+ ],
+ width: 80,
+ margin: '0 5 0 0',
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ //fieldLabel: gettext('URL'),
+ name: 'url',
+ allowBlank: false,
+ flex: 4,
+ },
+ ],
+ },
+ {
+ xtype: 'pmxWebhookKeyValueList',
+ name: 'header',
+ fieldLabel: gettext('Headers'),
+ maskValues: false,
+ cbind: {
+ isCreate: '{isCreate}',
+ },
+ },
+ {
+ xtype: 'textarea',
+ fieldLabel: gettext('Body'),
+ name: 'body',
+ allowBlank: true,
+ minHeight: '150',
+ fieldStyle: {
+ 'font-family': 'monospace',
+ },
+ margin: '15 0 0 0',
+ },
+ {
+ xtype: 'pmxWebhookKeyValueList',
+ name: 'secret',
+ fieldLabel: gettext('Secrets'),
+ maskValues: true,
+ cbind: {
+ isCreate: '{isCreate}',
+ },
+ },
+ {
+ xtype: 'proxmoxtextfield',
+ name: 'comment',
+ fieldLabel: gettext('Comment'),
+ cbind: {
+ deleteEmpty: '{!isCreate}',
+ },
+ },
+ ],
+
+ onSetValues: (values) => {
+ values.enable = !values.disable;
+
+ if (values.body) {
+ values.body = atob(values.body);
+ }
+
+ delete values.disable;
+ return values;
+ },
+
+ onGetValues: function(values) {
+ let me = this;
+
+ if (values.enable) {
+ if (!me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { 'delete': 'disable' });
+ }
+ } else {
+ values.disable = 1;
+ }
+
+ if (values.body) {
+ values.body = btoa(values.body);
+ } else {
+ delete values.body;
+ if (!me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { 'delete': 'body' });
+ }
+ }
+
+ if (Ext.isArray(values.header) && !values.header.length) {
+ delete values.header;
+ if (!me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { 'delete': 'header' });
+ }
+ }
+
+ if (Ext.isArray(values.secret) && !values.secret.length) {
+ delete values.secret;
+ if (!me.isCreate) {
+ Proxmox.Utils.assemble_field_data(values, { 'delete': 'secret' });
+ }
+ }
+ delete values.enable;
+
+ return values;
+ },
+});
+
+Ext.define('Proxmox.form.WebhookKeyValueList', {
+ extend: 'Ext.container.Container',
+ alias: 'widget.pmxWebhookKeyValueList',
+
+ mixins: [
+ 'Ext.form.field.Field',
+ ],
+
+ // override for column header
+ fieldTitle: gettext('Item'),
+
+ // will be applied to the textfields
+ maskRe: undefined,
+
+ allowBlank: true,
+ selectAll: false,
+ isFormField: true,
+ deleteEmpty: false,
+ config: {
+ deleteEmpty: false,
+ maskValues: false,
+ },
+
+ setValue: function(list) {
+ let me = this;
+
+ list = Ext.isArray(list) ? list : (list ?? '').split(';').filter(t => t !== '');
+
+ let store = me.lookup('grid').getStore();
+ if (list.length > 0) {
+ store.setData(list.map(item => {
+ let properties = Proxmox.Utils.parsePropertyString(item);
+
+ let value = me.maskValues ? '' : atob(properties.value);
+
+ let obj = {
+ headerName: properties.name,
+ headerValue: value,
+ };
+
+ if (!me.isCreate) {
+ obj.emptyText = gettext('Unchanged');
+ }
+
+ return obj;
+ }));
+ } else {
+ store.removeAll();
+ }
+ me.checkChange();
+ return me;
+ },
+
+ getValue: function() {
+ let me = this;
+ let values = [];
+ me.lookup('grid').getStore().each((rec) => {
+ if (rec.data.headerName) {
+ let obj = {
+ name: rec.data.headerName,
+ value: btoa(rec.data.headerValue),
+ };
+
+ values.push(Proxmox.Utils.printPropertyString(obj));
+ }
+ });
+
+ return values;
+ },
+
+ getErrors: function(value) {
+ let me = this;
+ let empty = false;
+
+ me.lookup('grid').getStore().each((rec) => {
+ if (!rec.data.headerName) {
+ empty = true;
+ }
+
+ if (!rec.data.headerValue && rec.data.newValue) {
+ empty = true;
+ }
+ });
+ if (empty) {
+ return [gettext('Name/value must not be empty.')];
+ }
+ return [];
+ },
+
+ // override framework function to implement deleteEmpty behaviour
+ getSubmitData: function() {
+ let me = this,
+ data = null,
+ val;
+ if (!me.disabled && me.submitValue) {
+ val = me.getValue();
+ if (val !== null && val !== '') {
+ data = {};
+ data[me.getName()] = val;
+ } else if (me.getDeleteEmpty()) {
+ data = {};
+ data.delete = me.getName();
+ }
+ }
+ return data;
+ },
+
+ controller: {
+ xclass: 'Ext.app.ViewController',
+
+ addLine: function() {
+ let me = this;
+ me.lookup('grid').getStore().add({
+ headerName: '',
+ headerValue: '',
+ emptyText: '',
+ newValue: true,
+ });
+ },
+
+ removeSelection: function(field) {
+ let me = this;
+ let view = me.getView();
+ let grid = me.lookup('grid');
+
+ let record = field.getWidgetRecord();
+ if (record === undefined) {
+ // this is sometimes called before a record/column is initialized
+ return;
+ }
+
+ grid.getStore().remove(record);
+ view.checkChange();
+ view.validate();
+ },
+
+ itemChange: function(field, newValue) {
+ let rec = field.getWidgetRecord();
+ if (!rec) {
+ return;
+ }
+
+ let column = field.getWidgetColumn();
+ rec.set(column.dataIndex, newValue);
+ let list = field.up('pmxWebhookKeyValueList');
+ list.checkChange();
+ list.validate();
+ },
+
+ control: {
+ 'grid button': {
+ click: 'removeSelection',
+ },
+ },
+ },
+
+ margin: '10 0 5 0',
+
+ items: [
+ {
+ layout: 'hbox',
+ border: false,
+ items: [
+ {
+ xtype: 'displayfield',
+ width: 125,
+ },
+ {
+ xtype: 'button',
+ text: gettext('Add'),
+ iconCls: 'fa fa-plus-circle',
+ handler: 'addLine',
+ margin: '0 5 5 0',
+ },
+ ],
+ },
+ {
+ xtype: 'grid',
+ reference: 'grid',
+ minHeight: 100,
+ maxHeight: 100,
+ scrollable: 'vertical',
+ margin: '0 0 0 125',
+
+ viewConfig: {
+ deferEmptyText: false,
+ },
+
+ store: {
+ listeners: {
+ update: function() {
+ this.commitChanges();
+ },
+ },
+ },
+ },
+ ],
+
+ initComponent: function() {
+ let me = this;
+
+ for (const [key, value] of Object.entries(me.gridConfig ?? {})) {
+ me.items[1][key] = value;
+ }
+
+ me.items[0].items[0].value = me.fieldLabel + ':';
+
+ me.items[1].columns = [
+ {
+ header: me.fieldTtitle,
+ dataIndex: 'headerName',
+ xtype: 'widgetcolumn',
+ widget: {
+ xtype: 'textfield',
+ isFormField: false,
+ maskRe: me.maskRe,
+ allowBlank: false,
+ queryMode: 'local',
+ listeners: {
+ change: 'itemChange',
+ },
+ },
+ flex: 1,
+ },
+ {
+ header: me.fieldTtitle,
+ dataIndex: 'headerValue',
+ xtype: 'widgetcolumn',
+ widget: {
+ xtype: 'proxmoxtextfield',
+ inputType: me.maskValues ? 'password' : 'text',
+ isFormField: false,
+ maskRe: me.maskRe,
+ queryMode: 'local',
+ listeners: {
+ change: 'itemChange',
+ },
+ allowBlank: !me.isCreate && me.maskValues,
+
+ bind: {
+ emptyText: '{record.emptyText}',
+ },
+ },
+ flex: 1,
+ },
+ {
+ xtype: 'widgetcolumn',
+ width: 40,
+ widget: {
+ xtype: 'button',
+ iconCls: 'fa fa-trash-o',
+ },
+ },
+ ];
+
+ me.callParent();
+ me.initField();
+ },
+});
--
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] 26+ messages in thread
* [pve-devel] [PATCH manager v2 06/12] api: notifications: use get_targets impl from proxmox-notify
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (4 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH widget-toolkit v2 05/12] notification: add UI for adding/updating webhook targets Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints Lukas Wagner
` (5 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
The get_targets API endpoint is now implemented in Rust.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 34 +------------------------------
1 file changed, 1 insertion(+), 33 deletions(-)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 68fdda2a..10b611c9 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -170,39 +170,7 @@ __PACKAGE__->register_method ({
my $config = PVE::Notify::read_config();
my $targets = eval {
- my $result = [];
-
- for my $target (@{$config->get_sendmail_endpoints()}) {
- push @$result, {
- name => $target->{name},
- comment => $target->{comment},
- type => 'sendmail',
- disable => $target->{disable},
- origin => $target->{origin},
- };
- }
-
- for my $target (@{$config->get_gotify_endpoints()}) {
- push @$result, {
- name => $target->{name},
- comment => $target->{comment},
- type => 'gotify',
- disable => $target->{disable},
- origin => $target->{origin},
- };
- }
-
- for my $target (@{$config->get_smtp_endpoints()}) {
- push @$result, {
- name => $target->{name},
- comment => $target->{comment},
- type => 'smtp',
- disable => $target->{disable},
- origin => $target->{origin},
- };
- }
-
- $result
+ $config->get_targets();
};
raise_api_error($@) if $@;
--
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] 26+ messages in thread
* [pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (5 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 06/12] api: notifications: use get_targets impl from proxmox-notify Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-17 15:36 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets Lukas Wagner
` (4 subsequent siblings)
11 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
These just call the API implementation via the perl-rs bindings.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
PVE/API2/Cluster/Notifications.pm | 263 +++++++++++++++++++++++++++++-
1 file changed, 262 insertions(+), 1 deletion(-)
diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
index 10b611c9..eae2d436 100644
--- a/PVE/API2/Cluster/Notifications.pm
+++ b/PVE/API2/Cluster/Notifications.pm
@@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
{ name => 'gotify' },
{ name => 'sendmail' },
{ name => 'smtp' },
+ { name => 'webhook' },
];
return $result;
@@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
'type' => {
description => 'Type of the target.',
type => 'string',
- enum => [qw(sendmail gotify smtp)],
+ enum => [qw(sendmail gotify smtp webhook)],
},
'comment' => {
description => 'Comment',
@@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
}
});
+my $webhook_properties = {
+ name => {
+ description => 'The name of the endpoint.',
+ type => 'string',
+ format => 'pve-configid',
+ },
+ url => {
+ description => 'Server URL',
+ type => 'string',
+ },
+ method => {
+ description => 'HTTP method',
+ type => 'string',
+ enum => [qw(post put get)],
+ },
+ header => {
+ description => 'HTTP headers to set. These have to be formatted as'
+ . ' a property string in the format name=<name>,value=<base64 of value>',
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ optional => 1,
+ },
+ body => {
+ description => 'HTTP body, base64 encoded',
+ type => 'string',
+ optional => 1,
+ },
+ secret => {
+ description => 'Secrets to set. These have to be formatted as'
+ . ' a property string in the format name=<name>,value=<base64 of value>',
+ type => 'array',
+ items => {
+ type => 'string',
+ },
+ optional => 1,
+ },
+ comment => {
+ description => 'Comment',
+ type => 'string',
+ optional => 1,
+ },
+ disable => {
+ description => 'Disable this target',
+ type => 'boolean',
+ optional => 1,
+ default => 0,
+ },
+};
+
+__PACKAGE__->register_method ({
+ name => 'get_webhook_endpoints',
+ path => 'endpoints/webhook',
+ method => 'GET',
+ description => 'Returns a list of all webhook endpoints',
+ protected => 1,
+ permissions => {
+ check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {},
+ },
+ returns => {
+ type => 'array',
+ items => {
+ type => 'object',
+ properties => {
+ %$webhook_properties,
+ 'origin' => {
+ description => 'Show if this entry was created by a user or was built-in',
+ type => 'string',
+ enum => [qw(user-created builtin modified-builtin)],
+ },
+ },
+ },
+ links => [ { rel => 'child', href => '{name}' } ],
+ },
+ code => sub {
+ my $config = PVE::Notify::read_config();
+ my $rpcenv = PVE::RPCEnvironment::get();
+
+ my $entities = eval {
+ $config->get_webhook_endpoints();
+ };
+ raise_api_error($@) if $@;
+
+ return $entities;
+ }
+});
+
+__PACKAGE__->register_method ({
+ name => 'get_webhook_endpoint',
+ path => 'endpoints/webhook/{name}',
+ method => 'GET',
+ description => 'Return a specific webhook endpoint',
+ protected => 1,
+ permissions => {
+ check => ['or',
+ ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ ['perm', '/mapping/notifications', ['Mapping.Audit']],
+ ],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ name => {
+ type => 'string',
+ format => 'pve-configid',
+ description => 'Name of the endpoint.'
+ },
+ }
+ },
+ returns => {
+ type => 'object',
+ properties => {
+ %$webhook_properties,
+ digest => get_standard_option('pve-config-digest'),
+ }
+ },
+ code => sub {
+ my ($param) = @_;
+ my $name = extract_param($param, 'name');
+
+ my $config = PVE::Notify::read_config();
+ my $endpoint = eval {
+ $config->get_webhook_endpoint($name)
+ };
+
+ raise_api_error($@) if $@;
+ $endpoint->{digest} = $config->digest();
+
+ return $endpoint;
+ }
+});
+
+__PACKAGE__->register_method ({
+ name => 'create_webhook_endpoint',
+ path => 'endpoints/webhook',
+ protected => 1,
+ method => 'POST',
+ description => 'Create a new webhook endpoint',
+ permissions => {
+ check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => $webhook_properties,
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+ eval {
+ PVE::Notify::lock_config(sub {
+ my $config = PVE::Notify::read_config();
+
+ $config->add_webhook_endpoint(
+ $param,
+ );
+
+ PVE::Notify::write_config($config);
+ });
+ };
+
+ raise_api_error($@) if $@;
+ return;
+ }
+});
+
+__PACKAGE__->register_method ({
+ name => 'update_webhook_endpoint',
+ path => 'endpoints/webhook/{name}',
+ protected => 1,
+ method => 'PUT',
+ description => 'Update existing webhook endpoint',
+ permissions => {
+ check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ %{ make_properties_optional($webhook_properties) },
+ delete => {
+ type => 'array',
+ items => {
+ type => 'string',
+ format => 'pve-configid',
+ },
+ optional => 1,
+ description => 'A list of settings you want to delete.',
+ },
+ digest => get_standard_option('pve-config-digest'),
+ }
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+
+ my $name = extract_param($param, 'name');
+ my $delete = extract_param($param, 'delete');
+ my $digest = extract_param($param, 'digest');
+
+ eval {
+ PVE::Notify::lock_config(sub {
+ my $config = PVE::Notify::read_config();
+
+ $config->update_webhook_endpoint(
+ $name,
+ $param, # Config updater
+ $delete,
+ $digest,
+ );
+
+ PVE::Notify::write_config($config);
+ });
+ };
+
+ raise_api_error($@) if $@;
+ return;
+ }
+});
+
+__PACKAGE__->register_method ({
+ name => 'delete_webhook_endpoint',
+ protected => 1,
+ path => 'endpoints/webhook/{name}',
+ method => 'DELETE',
+ description => 'Remove webhook endpoint',
+ permissions => {
+ check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
+ },
+ parameters => {
+ additionalProperties => 0,
+ properties => {
+ name => {
+ type => 'string',
+ format => 'pve-configid',
+ },
+ }
+ },
+ returns => { type => 'null' },
+ code => sub {
+ my ($param) = @_;
+ my $name = extract_param($param, 'name');
+
+ eval {
+ PVE::Notify::lock_config(sub {
+ my $config = PVE::Notify::read_config();
+ $config->delete_webhook_endpoint($name);
+ PVE::Notify::write_config($config);
+ });
+ };
+
+ raise_api_error($@) if $@;
+ return;
+ }
+});
+
my $matcher_properties = {
name => {
description => 'Name of the matcher.',
--
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] 26+ messages in thread
* [pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (6 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window Lukas Wagner
` (3 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
Copied and adapted from the Gotify ones.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
src/api2/config/notifications/mod.rs | 2 +
src/api2/config/notifications/webhook.rs | 175 +++++++++++++++++++++++
2 files changed, 177 insertions(+)
create mode 100644 src/api2/config/notifications/webhook.rs
diff --git a/src/api2/config/notifications/mod.rs b/src/api2/config/notifications/mod.rs
index dfe82ed0..81ca9800 100644
--- a/src/api2/config/notifications/mod.rs
+++ b/src/api2/config/notifications/mod.rs
@@ -22,6 +22,7 @@ pub mod matchers;
pub mod sendmail;
pub mod smtp;
pub mod targets;
+pub mod webhook;
#[sortable]
const SUBDIRS: SubdirMap = &sorted!([
@@ -41,6 +42,7 @@ const ENDPOINT_SUBDIRS: SubdirMap = &sorted!([
("gotify", &gotify::ROUTER),
("sendmail", &sendmail::ROUTER),
("smtp", &smtp::ROUTER),
+ ("webhook", &webhook::ROUTER),
]);
const ENDPOINT_ROUTER: Router = Router::new()
diff --git a/src/api2/config/notifications/webhook.rs b/src/api2/config/notifications/webhook.rs
new file mode 100644
index 00000000..4a040024
--- /dev/null
+++ b/src/api2/config/notifications/webhook.rs
@@ -0,0 +1,175 @@
+use anyhow::Error;
+use serde_json::Value;
+
+use proxmox_notify::endpoints::webhook::{
+ DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
+};
+use proxmox_notify::schema::ENTITY_NAME_SCHEMA;
+use proxmox_router::{Permission, Router, RpcEnvironment};
+use proxmox_schema::api;
+
+use pbs_api_types::{PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA};
+
+#[api(
+ protected: true,
+ input: {
+ properties: {},
+ },
+ returns: {
+ description: "List of webhook endpoints.",
+ type: Array,
+ items: { type: WebhookConfig },
+ },
+ access: {
+ permission: &Permission::Privilege(&["system", "notifications"], PRIV_SYS_AUDIT, false),
+ },
+)]
+/// List all webhook endpoints.
+pub fn list_endpoints(
+ _param: Value,
+ _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<Vec<WebhookConfig>, Error> {
+ let config = pbs_config::notifications::config()?;
+
+ let endpoints = proxmox_notify::api::webhook::get_endpoints(&config)?;
+
+ Ok(endpoints)
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ name: {
+ schema: ENTITY_NAME_SCHEMA,
+ }
+ },
+ },
+ returns: { type: WebhookConfig },
+ access: {
+ permission: &Permission::Privilege(&["system", "notifications"], PRIV_SYS_AUDIT, false),
+ },
+)]
+/// Get a webhook endpoint.
+pub fn get_endpoint(name: String, rpcenv: &mut dyn RpcEnvironment) -> Result<WebhookConfig, Error> {
+ let config = pbs_config::notifications::config()?;
+ let endpoint = proxmox_notify::api::webhook::get_endpoint(&config, &name)?;
+
+ rpcenv["digest"] = hex::encode(config.digest()).into();
+
+ Ok(endpoint)
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ endpoint: {
+ type: WebhookConfig,
+ flatten: true,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Privilege(&["system", "notifications"], PRIV_SYS_MODIFY, false),
+ },
+)]
+/// Add a new webhook endpoint.
+pub fn add_endpoint(
+ endpoint: WebhookConfig,
+ _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+ let _lock = pbs_config::notifications::lock_config()?;
+ let mut config = pbs_config::notifications::config()?;
+
+ proxmox_notify::api::webhook::add_endpoint(&mut config, endpoint)?;
+
+ pbs_config::notifications::save_config(config)?;
+ Ok(())
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ name: {
+ schema: ENTITY_NAME_SCHEMA,
+ },
+ updater: {
+ type: WebhookConfigUpdater,
+ flatten: true,
+ },
+ delete: {
+ description: "List of properties to delete.",
+ type: Array,
+ optional: true,
+ items: {
+ type: DeleteableWebhookProperty,
+ }
+ },
+ digest: {
+ optional: true,
+ schema: PROXMOX_CONFIG_DIGEST_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Privilege(&["system", "notifications"], PRIV_SYS_MODIFY, false),
+ },
+)]
+/// Update webhook endpoint.
+pub fn update_endpoint(
+ name: String,
+ updater: WebhookConfigUpdater,
+ delete: Option<Vec<DeleteableWebhookProperty>>,
+ digest: Option<String>,
+ _rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+ let _lock = pbs_config::notifications::lock_config()?;
+ let mut config = pbs_config::notifications::config()?;
+ let digest = digest.map(hex::decode).transpose()?;
+
+ proxmox_notify::api::webhook::update_endpoint(
+ &mut config,
+ &name,
+ updater,
+ delete.as_deref(),
+ digest.as_deref(),
+ )?;
+
+ pbs_config::notifications::save_config(config)?;
+ Ok(())
+}
+
+#[api(
+ protected: true,
+ input: {
+ properties: {
+ name: {
+ schema: ENTITY_NAME_SCHEMA,
+ }
+ },
+ },
+ access: {
+ permission: &Permission::Privilege(&["system", "notifications"], PRIV_SYS_MODIFY, false),
+ },
+)]
+/// Delete webhook endpoint.
+pub fn delete_endpoint(name: String, _rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
+ let _lock = pbs_config::notifications::lock_config()?;
+ let mut config = pbs_config::notifications::config()?;
+ proxmox_notify::api::webhook::delete_endpoint(&mut config, &name)?;
+
+ pbs_config::notifications::save_config(config)?;
+ Ok(())
+}
+
+const ITEM_ROUTER: Router = Router::new()
+ .get(&API_METHOD_GET_ENDPOINT)
+ .put(&API_METHOD_UPDATE_ENDPOINT)
+ .delete(&API_METHOD_DELETE_ENDPOINT);
+
+pub const ROUTER: Router = Router::new()
+ .get(&API_METHOD_LIST_ENDPOINTS)
+ .post(&API_METHOD_ADD_ENDPOINT)
+ .match_all("name", &ITEM_ROUTER);
--
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] 26+ messages in thread
* [pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (7 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency Lukas Wagner
` (2 subsequent siblings)
11 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
This allows users to add/edit new webhook targets.
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
www/Utils.js | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/www/Utils.js b/www/Utils.js
index 4853be36..b715972f 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -482,6 +482,11 @@ Ext.define('PBS.Utils', {
ipanel: 'pmxGotifyEditPanel',
iconCls: 'fa-bell-o',
},
+ webhook: {
+ name: 'Webhook',
+ ipanel: 'pmxWebhookEditPanel',
+ iconCls: 'fa-bell-o',
+ },
};
},
--
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] 26+ messages in thread
* [pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (8 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window Lukas Wagner
@ 2024-07-12 11:27 ` Lukas Wagner
2024-07-17 15:34 ` [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Max Carrara
2024-07-22 12:10 ` Stefan Hanreich
11 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-12 11:27 UTC (permalink / raw)
To: pve-devel, pbs-devel
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
Cargo.toml | 2 +-
debian/control | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index f39d118..49ca079 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -20,4 +20,4 @@ nix = "0.26"
syslog = "6.0"
proxmox-sys = "0.5.3"
-proxmox-notify = {version = "0.4", features = ["mail-forwarder", "pve-context", "pbs-context"] }
+proxmox-notify = {version = "0.5", features = ["mail-forwarder", "pve-context", "pbs-context"] }
diff --git a/debian/control b/debian/control
index 7329a24..0ab74a9 100644
--- a/debian/control
+++ b/debian/control
@@ -6,10 +6,10 @@ Build-Depends: cargo:native,
librust-anyhow-1+default-dev,
librust-log-0.4+default-dev (>= 0.4.17-~~),
librust-nix-0.26+default-dev,
- librust-proxmox-notify-0.4+default-dev,
- librust-proxmox-notify-0.4+mail-forwarder-dev,
- librust-proxmox-notify-0.4+pbs-context-dev,
- librust-proxmox-notify-0.4+pve-context-dev,
+ librust-proxmox-notify-0.5+default-dev,
+ librust-proxmox-notify-0.5+mail-forwarder-dev,
+ librust-proxmox-notify-0.5+pbs-context-dev,
+ librust-proxmox-notify-0.5+pve-context-dev,
librust-proxmox-sys-0.5+default-dev (>= 0.5.3-~~),
librust-syslog-6+default-dev,
libstd-rust-dev,
--
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] 26+ messages in thread
* Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (9 preceding siblings ...)
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency Lukas Wagner
@ 2024-07-17 15:34 ` Max Carrara
2024-07-22 7:50 ` Lukas Wagner
2024-07-22 12:10 ` Stefan Hanreich
11 siblings, 1 reply; 26+ messages in thread
From: Max Carrara @ 2024-07-17 15:34 UTC (permalink / raw)
To: Proxmox VE development discussion, pbs-devel
On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> Sending as an RFC because I don't want this merged yet; that being
> said, the feature should be mostly finished at this point, I'd
> appreciate any reviews and feedback.
>
> This series adds support for webhook notification targets to PVE
> and PBS.
>
> A webhook is a HTTP API route provided by a third-party service that
> can be used to inform the third-party about an event. In our case,
> we can easily interact with various third-party notification/messaging
> systems and send PVE/PBS notifications via this service.
> The changes were tested against ntfy.sh, Discord and Slack.
>
> The configuration of webhook targets allows one to configure:
> - The URL
> - The HTTP method (GET/POST/PUT)
> - HTTP Headers
> - Body
>
> One can use handlebar templating to inject notification text and metadata
> in the url, headers and body.
>
> One challenge is the handling of sensitve tokens and other secrets.
> Since the endpoint is completely generic, we cannot know in advance
> whether the body/header/url contains sensitive values.
> Thus we add 'secrets' which are stored in the protected config only
> accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
> secrets are accessible in URLs/headers/body via templating:
>
> Url: https://example.com/{{ secrets.token }}
>
> Secrets can only be set and updated, but never retrieved via the API.
> In the UI, secrets are handled like other secret tokens/passwords.
>
> Bumps for PVE:
> - libpve-rs-perl needs proxmox-notify bumped
> - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
> - proxmox-mail-forward needs proxmox-notify bumped
>
> Bumps for PBS:
> - proxmox-backup needs proxmox-notify bumped
> - proxmox-mail-forward needs proxmox-notify bumped
Since this is an RFC, I mainly just did some proofreading; I haven't
really spotted anything out of the ordinary, apart from a few *very
small* things I commented on inline.
I like the overall idea of adding webhooks, so this looks pretty solid
to me. At first I thought that this might be a bit of a niche use case,
but I feel like it might actually be quite interesting for orgs that are
e.g. on Slack: You could e.g. just "route" all notifications via a
webhook to Slack, and Slack then sends a push notification to one's
phone. The same can obviously done with other applications / services as
well. So, pretty cool stuff :)
Not sure if this has been discussed somewhere already (off list etc.),
but could you elaborate on why you don't want this merged yet? The
patches look pretty solid to me, IMHO. Then again, I haven't really
tested them yet due to all the required package bumps, so take this with
a grain of salt.
If you want to have this RFC tested, I can of course give it a shot - do
let me know if that's the case :)
>
>
> Changes v1 -> v2:
> - Rebase proxmox-notify changes
>
> proxmox:
>
> Lukas Wagner (2):
> notify: implement webhook targets
> notify: add api for webhook targets
>
> proxmox-notify/Cargo.toml | 9 +-
> proxmox-notify/src/api/mod.rs | 20 +
> proxmox-notify/src/api/webhook.rs | 406 +++++++++++++++++++
> proxmox-notify/src/config.rs | 23 ++
> proxmox-notify/src/endpoints/mod.rs | 2 +
> proxmox-notify/src/endpoints/webhook.rs | 509 ++++++++++++++++++++++++
> proxmox-notify/src/lib.rs | 17 +
> 7 files changed, 983 insertions(+), 3 deletions(-)
> create mode 100644 proxmox-notify/src/api/webhook.rs
> create mode 100644 proxmox-notify/src/endpoints/webhook.rs
>
>
> proxmox-perl-rs:
>
> Lukas Wagner (2):
> common: notify: add bindings for webhook API routes
> common: notify: add bindings for get_targets
>
> common/src/notify.rs | 72 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
>
> proxmox-widget-toolkit:
>
> Lukas Wagner (1):
> notification: add UI for adding/updating webhook targets
>
> src/Makefile | 1 +
> src/Schema.js | 5 +
> src/panel/WebhookEditPanel.js | 417 ++++++++++++++++++++++++++++++++++
> 3 files changed, 423 insertions(+)
> create mode 100644 src/panel/WebhookEditPanel.js
>
>
> pve-manager:
>
> Lukas Wagner (2):
> api: notifications: use get_targets impl from proxmox-notify
> api: add routes for webhook notification endpoints
>
> PVE/API2/Cluster/Notifications.pm | 297 ++++++++++++++++++++++++++----
> 1 file changed, 263 insertions(+), 34 deletions(-)
>
>
> pve-docs:
>
> Lukas Wagner (1):
> notification: add documentation for webhook target endpoints.
>
> notifications.adoc | 93 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
>
> proxmox-backup:
>
> Lukas Wagner (3):
> api: notification: add API routes for webhook targets
> ui: utils: enable webhook edit window
> docs: notification: add webhook endpoint documentation
>
> docs/notifications.rst | 100 +++++++++++++
> src/api2/config/notifications/mod.rs | 2 +
> src/api2/config/notifications/webhook.rs | 175 +++++++++++++++++++++++
> www/Utils.js | 5 +
> 4 files changed, 282 insertions(+)
> create mode 100644 src/api2/config/notifications/webhook.rs
>
>
> proxmox-mail-forward:
>
> Lukas Wagner (1):
> bump proxmox-notify dependency
>
> Cargo.toml | 2 +-
> debian/control | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
>
> Summary over all repositories:
> 19 files changed, 2121 insertions(+), 42 deletions(-)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets Lukas Wagner
@ 2024-07-17 15:35 ` Max Carrara
2024-07-22 7:30 ` Lukas Wagner
0 siblings, 1 reply; 26+ messages in thread
From: Max Carrara @ 2024-07-17 15:35 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, pve-devel
On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> This target type allows users to perform HTTP requests to arbitrary
> third party (notification) services, for instance
> ntfy.sh/Discord/Slack.
>
> The configuration for these endpoints allows one to freely configure
> the URL, HTTP Method, headers and body. The URL, header values and
> body support handlebars templating to inject notification text,
> metadata and secrets. Secrets are stored in the protected
> configuration file (e.g. /etc/pve/priv/notification.cfg) as key value
> pairs, allowing users to protect sensitive tokens/passwords.
> Secrets are accessible in handlebar templating via the secrets.*
> namespace, e.g. if there is a secret named 'token', a body
> could contain '{{ secrets.token }}' to inject the token into the
> payload.
>
> A couple of handlebars helpers are also provided:
> - url-encoding (useful for templating in URLs)
> - escape (escape any control characters in strings)
> - json (print a property as json)
>
> In the configuration, the body, header values and secret values
> are stored in base64 encoding so that we can store any string we want.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> proxmox-notify/Cargo.toml | 9 +-
> proxmox-notify/src/config.rs | 23 ++
> proxmox-notify/src/endpoints/mod.rs | 2 +
> proxmox-notify/src/endpoints/webhook.rs | 509 ++++++++++++++++++++++++
> proxmox-notify/src/lib.rs | 17 +
> 5 files changed, 557 insertions(+), 3 deletions(-)
> create mode 100644 proxmox-notify/src/endpoints/webhook.rs
>
> diff --git a/proxmox-notify/Cargo.toml b/proxmox-notify/Cargo.toml
> index 7801814d..484aff19 100644
> --- a/proxmox-notify/Cargo.toml
> +++ b/proxmox-notify/Cargo.toml
> @@ -9,13 +9,15 @@ exclude.workspace = true
>
> [dependencies]
> anyhow.workspace = true
> -base64.workspace = true
> +base64 = { workspace = true, optional = true }
> const_format.workspace = true
> handlebars = { workspace = true }
> +http = { workspace = true, optional = true }
> lettre = { workspace = true, optional = true }
> log.workspace = true
> mail-parser = { workspace = true, optional = true }
> openssl.workspace = true
> +percent-encoding = { workspace = true, optional = true }
> regex.workspace = true
> serde = { workspace = true, features = ["derive"] }
> serde_json.workspace = true
> @@ -31,10 +33,11 @@ proxmox-time.workspace = true
> proxmox-uuid = { workspace = true, features = ["serde"] }
>
> [features]
> -default = ["sendmail", "gotify", "smtp"]
> +default = ["sendmail", "gotify", "smtp", "webhook"]
> mail-forwarder = ["dep:mail-parser", "dep:proxmox-sys"]
> -sendmail = ["dep:proxmox-sys"]
> +sendmail = ["dep:proxmox-sys", "dep:base64"]
> gotify = ["dep:proxmox-http"]
> pve-context = ["dep:proxmox-sys"]
> pbs-context = ["dep:proxmox-sys"]
> smtp = ["dep:lettre"]
> +webhook = ["dep:base64", "dep:http", "dep:percent-encoding", "dep:proxmox-http"]
> diff --git a/proxmox-notify/src/config.rs b/proxmox-notify/src/config.rs
> index 789c4a7d..4d0b53f7 100644
> --- a/proxmox-notify/src/config.rs
> +++ b/proxmox-notify/src/config.rs
> @@ -57,6 +57,17 @@ fn config_init() -> SectionConfig {
> GOTIFY_SCHEMA,
> ));
> }
> + #[cfg(feature = "webhook")]
> + {
> + use crate::endpoints::webhook::{WebhookConfig, WEBHOOK_TYPENAME};
> +
> + const WEBHOOK_SCHEMA: &ObjectSchema = WebhookConfig::API_SCHEMA.unwrap_object_schema();
> + config.register_plugin(SectionConfigPlugin::new(
> + WEBHOOK_TYPENAME.to_string(),
> + Some(String::from("name")),
> + WEBHOOK_SCHEMA,
> + ));
> + }
>
> const MATCHER_SCHEMA: &ObjectSchema = MatcherConfig::API_SCHEMA.unwrap_object_schema();
> config.register_plugin(SectionConfigPlugin::new(
> @@ -110,6 +121,18 @@ fn private_config_init() -> SectionConfig {
> ));
> }
>
> + #[cfg(feature = "webhook")]
> + {
> + use crate::endpoints::webhook::{WebhookPrivateConfig, WEBHOOK_TYPENAME};
> +
> + const WEBHOOK_SCHEMA: &ObjectSchema =
> + WebhookPrivateConfig::API_SCHEMA.unwrap_object_schema();
> + config.register_plugin(SectionConfigPlugin::new(
> + WEBHOOK_TYPENAME.to_string(),
> + Some(String::from("name")),
> + WEBHOOK_SCHEMA,
> + ));
> + }
> config
> }
>
> diff --git a/proxmox-notify/src/endpoints/mod.rs b/proxmox-notify/src/endpoints/mod.rs
> index 97f79fcc..f20bee21 100644
> --- a/proxmox-notify/src/endpoints/mod.rs
> +++ b/proxmox-notify/src/endpoints/mod.rs
> @@ -4,5 +4,7 @@ pub mod gotify;
> pub mod sendmail;
> #[cfg(feature = "smtp")]
> pub mod smtp;
> +#[cfg(feature = "webhook")]
> +pub mod webhook;
>
> mod common;
> diff --git a/proxmox-notify/src/endpoints/webhook.rs b/proxmox-notify/src/endpoints/webhook.rs
> new file mode 100644
> index 00000000..7e976f6b
> --- /dev/null
> +++ b/proxmox-notify/src/endpoints/webhook.rs
> @@ -0,0 +1,509 @@
> +use handlebars::{
> + Context as HandlebarsContext, Handlebars, Helper, HelperResult, Output, RenderContext,
> + RenderError as HandlebarsRenderError,
> +};
> +use http::Request;
> +use percent_encoding::AsciiSet;
> +use proxmox_schema::property_string::PropertyString;
> +use serde::{Deserialize, Serialize};
> +use serde_json::{json, Map, Value};
> +
> +use proxmox_http::client::sync::Client;
> +use proxmox_http::{HttpClient, HttpOptions, ProxyConfig};
> +use proxmox_schema::api_types::COMMENT_SCHEMA;
> +use proxmox_schema::{api, ApiStringFormat, ApiType, Schema, StringSchema, Updater};
> +
> +use crate::context::context;
> +use crate::renderer::TemplateType;
> +use crate::schema::ENTITY_NAME_SCHEMA;
> +use crate::{renderer, Content, Endpoint, Error, Notification, Origin};
> +
> +pub(crate) const WEBHOOK_TYPENAME: &str = "webhook";
> +
> +#[api]
> +#[derive(Serialize, Deserialize, Clone, Copy, Default)]
> +#[serde(rename_all = "kebab-case")]
> +/// HTTP Method to use
> +pub enum HttpMethod {
> + /// HTTP POST
> + #[default]
> + Post,
> + /// HTTP PUT
> + Put,
> + /// HTTP GET
> + Get,
> +}
> +
> +// We only ever need a &str, so we rather implement this
> +// instead of Display.
> +impl From<HttpMethod> for &str {
> + fn from(value: HttpMethod) -> Self {
> + match value {
> + HttpMethod::Post => "POST",
> + HttpMethod::Put => "PUT",
> + HttpMethod::Get => "GET",
> + }
> + }
> +}
> +
> +#[api(
> + properties: {
> + name: {
> + schema: ENTITY_NAME_SCHEMA,
> + },
> + comment: {
> + optional: true,
> + schema: COMMENT_SCHEMA,
> + },
> + header: {
> + type: Array,
> + items: {
> + schema: KEY_AND_BASE64_VALUE_SCHEMA,
> + },
> + optional: true,
> + },
> + secret: {
> + type: Array,
> + items: {
> + schema: KEY_AND_BASE64_VALUE_SCHEMA,
> + },
> + optional: true,
> + },
> + }
> +)]
> +#[derive(Serialize, Deserialize, Updater, Default, Clone)]
> +#[serde(rename_all = "kebab-case")]
> +/// Config for Webhook notification endpoints
> +pub struct WebhookConfig {
> + /// Name of the endpoint.
> + #[updater(skip)]
> + pub name: String,
> +
> + pub method: HttpMethod,
> +
> + /// Webhook URL.
> + pub url: String,
> + /// Array of HTTP headers. Each entry is a property string with a name and a value.
> + /// The value property contains the header in base64 encoding.
> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
> + pub header: Vec<PropertyString<KeyAndBase64Val>>,
> + /// Body.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub body: Option<String>,
> +
> + /// Comment.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub comment: Option<String>,
> + /// Disable this target.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub disable: Option<bool>,
> + /// Origin of this config entry.
> + #[serde(skip_serializing_if = "Option::is_none")]
> + #[updater(skip)]
> + pub origin: Option<Origin>,
> + /// Array of secrets. Each entry is a property string with a name and an optional value.
> + /// The value property contains the secret in base64 encoding.
> + /// For any API endpoints returning the endpoint config,
> + /// only the secret name but not the value will be returned.
> + /// When updating the config, also send all secrest that you want
> + /// to keep, setting only the name but not the value.
> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
> + pub secret: Vec<PropertyString<KeyAndBase64Val>>,
> +}
> +
> +#[api(
> + properties: {
> + name: {
> + schema: ENTITY_NAME_SCHEMA,
> + },
> + secret: {
> + type: Array,
> + items: {
> + schema: KEY_AND_BASE64_VALUE_SCHEMA,
> + },
> + optional: true,
> + },
> + }
> +)]
> +#[derive(Serialize, Deserialize, Clone, Updater, Default)]
> +#[serde(rename_all = "kebab-case")]
> +/// Private configuration for Webhook notification endpoints.
> +/// This config will be saved to a separate configuration file with stricter
> +/// permissions (root:root 0600)
> +pub struct WebhookPrivateConfig {
> + /// Name of the endpoint
> + #[updater(skip)]
> + pub name: String,
> +
> + #[serde(default, skip_serializing_if = "Vec::is_empty")]
> + #[updater(serde(skip_serializing_if = "Option::is_none"))]
> + /// Array of secrets. Each entry is a property string with a name,
> + /// and a value property. The value property contains the secret
> + /// in base64 encoding.
> + pub secret: Vec<PropertyString<KeyAndBase64Val>>,
> +}
> +
> +/// A Webhook notification endpoint.
> +pub struct WebhookEndpoint {
> + pub config: WebhookConfig,
> + pub private_config: WebhookPrivateConfig,
> +}
> +
> +#[api]
> +#[derive(Serialize, Deserialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub enum DeleteableWebhookProperty {
> + /// Delete `comment`
> + Comment,
> + /// Delete `disable`
> + Disable,
> + /// Delete `header`
> + Header,
> + /// Delete `body`
> + Body,
> + /// Delete `secret`
> + Secret,
> +}
> +
> +#[api]
> +#[derive(Serialize, Deserialize, Debug, Default, Clone)]
> +/// Datatype used to represent key-value pairs, the value
> +/// being encoded in base64.
> +pub struct KeyAndBase64Val {
> + /// Name
> + pub(crate) name: String,
> + /// Base64 encoded value
> + #[serde(skip_serializing_if = "Option::is_none")]
> + pub(crate) value: Option<String>,
> +}
> +
> +impl KeyAndBase64Val {
> + #[cfg(test)]
> + pub(crate) fn new_with_plain_value(name: &str, value: &str) -> Self {
> + let value = base64::encode(value);
> +
> + Self {
> + name: name.into(),
> + value: Some(value),
> + }
> + }
> +
> + pub(crate) fn decode_value(&self) -> Result<String, Error> {
> + let value = self.value.as_deref().unwrap_or_default();
> + let bytes = base64::decode(value).map_err(|_| {
> + Error::Generic(format!(
> + "could not decode base64 value with name '{}'",
> + self.name
> + ))
> + })?;
> + let value = String::from_utf8(bytes).map_err(|_| {
> + Error::Generic(format!(
> + "could not decode UTF8 string from base64, name={}",
> + self.name
> + ))
> + })?;
> +
> + Ok(value)
> + }
> +}
> +
> +pub const KEY_AND_BASE64_VALUE_SCHEMA: Schema =
> + StringSchema::new("String schema for pairs of keys and base64 encoded values")
> + .format(&ApiStringFormat::PropertyString(
> + &KeyAndBase64Val::API_SCHEMA,
> + ))
> + .schema();
> +
> +impl Endpoint for WebhookEndpoint {
> + fn send(&self, notification: &Notification) -> Result<(), Error> {
> + let request = self.build_request(notification)?;
> +
> + self.create_client()?
> + .request(request)
> + .map_err(|err| Error::NotifyFailed(self.name().to_string(), err.into()))?;
> +
> + Ok(())
> + }
> +
> + fn name(&self) -> &str {
> + &self.config.name
> + }
> +
> + /// Check if the endpoint is disabled
> + fn disabled(&self) -> bool {
> + self.config.disable.unwrap_or_default()
> + }
> +}
> +
> +impl WebhookEndpoint {
> + fn create_client(&self) -> Result<Client, Error> {
> + let proxy_config = context()
> + .http_proxy_config()
> + .map(|url| ProxyConfig::parse_proxy_url(&url))
> + .transpose()
> + .map_err(|err| Error::NotifyFailed(self.name().to_string(), err.into()))?;
> +
> + let options = HttpOptions {
> + proxy_config,
> + ..Default::default()
> + };
> +
> + Ok(Client::new(options))
> + }
> +
> + fn build_request(&self, notification: &Notification) -> Result<Request<String>, Error> {
> + let (title, message) = match ¬ification.content {
> + Content::Template {
> + template_name,
> + data,
> + } => {
> + let rendered_title =
> + renderer::render_template(TemplateType::Subject, template_name, data)?;
> + let rendered_message =
> + renderer::render_template(TemplateType::PlaintextBody, template_name, data)?;
> +
> + (rendered_title, rendered_message)
> + }
> + #[cfg(feature = "mail-forwarder")]
> + Content::ForwardedMail { title, body, .. } => (title.clone(), body.clone()),
> + };
> +
> + let mut fields = Map::new();
> +
> + for (field_name, field_value) in ¬ification.metadata.additional_fields {
> + fields.insert(field_name.clone(), Value::String(field_value.to_string()));
> + }
> +
> + let mut secrets = Map::new();
> +
> + for secret in &self.private_config.secret {
> + let value = secret.decode_value()?;
> + secrets.insert(secret.name.clone(), Value::String(value));
> + }
> +
> + let data = json!({
> + "title": &title,
> + "message": &message,
> + "severity": notification.metadata.severity,
> + "timestamp": notification.metadata.timestamp,
> + "fields": fields,
> + "secrets": secrets,
> + });
> +
> + let handlebars = setup_handlebars();
> + let body_template = self.base_64_decode(self.config.body.as_deref().unwrap_or_default())?;
> +
> + let body = handlebars
> + .render_template(&body_template, &data)
> + .map_err(|err| {
> + // TODO: Cleanup error types, they have become a bit messy.
> + // No user of the notify crate distinguish between the error types any way, so
> + // we can refactor without any issues....
> + Error::Generic(format!("failed to render webhook body: {err}"))
I'm curious, how would you clean up the error types in particular?
> + })?;
> +
> + let url = handlebars
> + .render_template(&self.config.url, &data)
> + .map_err(|err| Error::Generic(format!("failed to render webhook url: {err}")))?;
> +
> + let method: &str = self.config.method.into();
> + let mut builder = http::Request::builder().uri(url).method(method);
> +
> + for header in &self.config.header {
> + let value = header.decode_value()?;
> +
> + let value = handlebars.render_template(&value, &data).map_err(|err| {
> + Error::Generic(format!(
> + "failed to render header value template: {value}: {err}"
> + ))
> + })?;
> +
> + builder = builder.header(header.name.clone(), value);
> + }
> +
> + let request = builder
> + .body(body)
> + .map_err(|err| Error::Generic(format!("failed to build http request: {err}")))?;
> +
> + Ok(request)
> + }
> +
> + fn base_64_decode(&self, s: &str) -> Result<String, Error> {
> + // Also here, TODO: revisit Error variants for the *whole* crate.
> + let s = base64::decode(s)
> + .map_err(|err| Error::Generic(format!("could not decode base64 value: {err}")))?;
> +
> + String::from_utf8(s).map_err(|err| {
> + Error::Generic(format!(
> + "base64 encoded value did not contain valid utf8: {err}"
> + ))
> + })
> + }
> +}
> +
> +fn setup_handlebars() -> Handlebars<'static> {
> + let mut handlebars = Handlebars::new();
> +
> + handlebars.register_helper("url-encode", Box::new(handlebars_percent_encode));
> + handlebars.register_helper("json", Box::new(handlebars_json));
> + handlebars.register_helper("escape", Box::new(handlebars_escape));
> +
> + // There is no escape.
> + handlebars.register_escape_fn(handlebars::no_escape);
> +
> + handlebars
> +}
> +
> +fn handlebars_percent_encode(
> + h: &Helper,
> + _: &Handlebars,
> + _: &HandlebarsContext,
> + _rc: &mut RenderContext,
> + out: &mut dyn Output,
> +) -> HelperResult {
> + let param0 = h
> + .param(0)
> + .and_then(|v| v.value().as_str())
> + .ok_or_else(|| HandlebarsRenderError::new("url-encode: missing parameter"))?;
> +
> + // See https://developer.mozilla.org/en-US/docs/Glossary/Percent-encoding
> + const FRAGMENT: &AsciiSet = &percent_encoding::CONTROLS
> + .add(b':')
> + .add(b'/')
> + .add(b'?')
> + .add(b'#')
> + .add(b'[')
> + .add(b']')
> + .add(b'@')
> + .add(b'!')
> + .add(b'$')
> + .add(b'&')
> + .add(b'\'')
> + .add(b'(')
> + .add(b')')
> + .add(b'*')
> + .add(b'+')
> + .add(b',')
> + .add(b';')
> + .add(b'=')
> + .add(b'%')
> + .add(b' ');
> + let a = percent_encoding::utf8_percent_encode(param0, FRAGMENT);
> +
> + out.write(&a.to_string())?;
> +
> + Ok(())
> +}
> +
> +fn handlebars_json(
> + h: &Helper,
> + _: &Handlebars,
> + _: &HandlebarsContext,
> + _rc: &mut RenderContext,
> + out: &mut dyn Output,
> +) -> HelperResult {
> + let param0 = h
> + .param(0)
> + .map(|v| v.value())
> + .ok_or_else(|| HandlebarsRenderError::new("json: missing parameter"))?;
> +
> + let json = serde_json::to_string(param0)?;
> + out.write(&json)?;
> +
> + Ok(())
> +}
> +
> +fn handlebars_escape(
> + h: &Helper,
> + _: &Handlebars,
> + _: &HandlebarsContext,
> + _rc: &mut RenderContext,
> + out: &mut dyn Output,
> +) -> HelperResult {
> + let text = h
> + .param(0)
> + .and_then(|v| v.value().as_str())
> + .ok_or_else(|| HandlebarsRenderError::new("escape: missing text parameter"))?;
> +
> + let val = Value::String(text.to_string());
> + let json = serde_json::to_string(&val)?;
> + out.write(&json[1..json.len() - 1])?;
> +
> + Ok(())
> +}
> +
> +#[cfg(test)]
> +mod tests {
> + use std::collections::HashMap;
> +
> + use super::*;
> + use crate::Severity;
> +
> +
> +
> + #[test]
> + fn test_build_request() -> Result<(), Error> {
> + let data = HashMap::from_iter([
> + ("hello".into(), "hello world".into()),
> + ("test".into(), "escaped\nstring".into()),
> + ]);
> +
> + let body_template = r#"
> +{{ fields.test }}
> +{{ escape fields.test }}
> +
> +{{ json fields }}
> +{{ json fields.hello }}
> +
> +{{ url-encode fields.hello }}
> +
> +{{ json severity }}
> +
> +"#;
> +
> + let expected_body = r#"
> +escaped
> +string
> +escaped\nstring
> +
> +{"hello":"hello world","test":"escaped\nstring"}
> +"hello world"
> +
> +hello%20world
> +
> +"info"
> +
> +"#;
> +
> + let endpoint = WebhookEndpoint {
> + config: WebhookConfig {
> + name: "test".into(),
> + method: HttpMethod::Post,
> + url: "http://localhost/{{ url-encode fields.hello }}".into(),
> + header: vec![
> + KeyAndBase64Val::new_with_plain_value("X-Severity", "{{ severity }}").into(),
> + ],
> + body: Some(base64::encode(body_template)),
> + ..Default::default()
> + },
> + private_config: WebhookPrivateConfig {
> + name: "test".into(),
> + ..Default::default()
> + },
> + };
> +
> + let notification = Notification::from_template(Severity::Info, "foo", json!({}), data);
> +
> + let request = endpoint.build_request(¬ification)?;
> +
> + assert_eq!(request.uri(), "http://localhost/hello%20world");
> + assert_eq!(request.body(), expected_body);
> + assert_eq!(request.method(), "POST");
> +
> + assert_eq!(request.headers().get("X-Severity").unwrap(), "info");
> +
> + Ok(())
> + }
> +}
> diff --git a/proxmox-notify/src/lib.rs b/proxmox-notify/src/lib.rs
> index 910dfa06..ebaba119 100644
> --- a/proxmox-notify/src/lib.rs
> +++ b/proxmox-notify/src/lib.rs
> @@ -499,6 +499,23 @@ impl Bus {
> );
> }
>
> + #[cfg(feature = "webhook")]
> + {
> + use endpoints::webhook::WEBHOOK_TYPENAME;
> + use endpoints::webhook::{WebhookConfig, WebhookEndpoint, WebhookPrivateConfig};
> + endpoints.extend(
> + parse_endpoints_with_private_config!(
> + config,
> + WebhookConfig,
> + WebhookPrivateConfig,
> + WebhookEndpoint,
> + WEBHOOK_TYPENAME
> + )?
> + .into_iter()
> + .map(|e| (e.name().into(), e)),
> + );
> + }
> +
> let matchers = config
> .config
> .convert_to_typed_array(MATCHER_TYPENAME)
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 02/12] notify: add api for " Lukas Wagner
@ 2024-07-17 15:35 ` Max Carrara
2024-07-22 7:32 ` Lukas Wagner
0 siblings, 1 reply; 26+ messages in thread
From: Max Carrara @ 2024-07-17 15:35 UTC (permalink / raw)
To: Proxmox VE development discussion, pbs-devel
On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> All in all pretty similar to other endpoint APIs.
> One thing worth noting is how secrets are handled. We never ever
> return the values of previously stored secrets in get_endpoint(s)
> calls, but only a list of the names of all secrets. This is needed
> to build the UI, where we display all secrets that were set before in
> a table.
>
> For update calls, one is supposed to send all secrets that should be
> kept and updated. If the value should be updated, the name and value
> is expected, and if the current value should preseved, only the name
> is sent. If a secret's name is not present in the updater, it will be
> dropped. If 'secret' is present in the 'delete' array, all secrets
> will be dropped, apart from those which are also set/preserved in the
> same update call.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> proxmox-notify/src/api/mod.rs | 20 ++
> proxmox-notify/src/api/webhook.rs | 406 ++++++++++++++++++++++++++++++
> 2 files changed, 426 insertions(+)
> create mode 100644 proxmox-notify/src/api/webhook.rs
>
> diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs
> index a7f6261c..7f823bc7 100644
> --- a/proxmox-notify/src/api/mod.rs
> +++ b/proxmox-notify/src/api/mod.rs
> @@ -15,6 +15,8 @@ pub mod matcher;
> pub mod sendmail;
> #[cfg(feature = "smtp")]
> pub mod smtp;
> +#[cfg(feature = "webhook")]
> +pub mod webhook;
>
> // We have our own, local versions of http_err and http_bail, because
> // we don't want to wrap the error in anyhow::Error. If we were to do that,
> @@ -54,6 +56,9 @@ pub enum EndpointType {
> /// Gotify endpoint
> #[cfg(feature = "gotify")]
> Gotify,
> + /// Webhook endpoint
> + #[cfg(feature = "webhook")]
> + Webhook,
> }
>
> #[api]
> @@ -113,6 +118,17 @@ pub fn get_targets(config: &Config) -> Result<Vec<Target>, HttpError> {
> })
> }
>
> + #[cfg(feature = "webhook")]
> + for endpoint in webhook::get_endpoints(config)? {
> + targets.push(Target {
> + name: endpoint.name,
> + origin: endpoint.origin.unwrap_or(Origin::UserCreated),
> + endpoint_type: EndpointType::Webhook,
> + disable: endpoint.disable,
> + comment: endpoint.comment,
> + })
> + }
> +
> Ok(targets)
> }
>
> @@ -145,6 +161,10 @@ fn ensure_endpoint_exists(#[allow(unused)] config: &Config, name: &str) -> Resul
> {
> exists = exists || smtp::get_endpoint(config, name).is_ok();
> }
> + #[cfg(feature = "webhook")]
> + {
> + exists = exists || webhook::get_endpoint(config, name).is_ok();
> + }
>
> if !exists {
> http_bail!(NOT_FOUND, "endpoint '{name}' does not exist")
> diff --git a/proxmox-notify/src/api/webhook.rs b/proxmox-notify/src/api/webhook.rs
> new file mode 100644
> index 00000000..b7f17c55
> --- /dev/null
> +++ b/proxmox-notify/src/api/webhook.rs
> @@ -0,0 +1,406 @@
> +use proxmox_http_error::HttpError;
> +use proxmox_schema::property_string::PropertyString;
> +
> +use crate::api::http_err;
> +use crate::endpoints::webhook::{
> + DeleteableWebhookProperty, KeyAndBase64Val, WebhookConfig, WebhookConfigUpdater,
> + WebhookPrivateConfig, WEBHOOK_TYPENAME,
> +};
> +use crate::{http_bail, Config};
> +
> +use super::remove_private_config_entry;
> +use super::set_private_config_entry;
> +
> +/// Get a list of all webhook endpoints.
> +///
> +/// The caller is responsible for any needed permission checks.
> +/// Returns a list of all webhook endpoints or a `HttpError` if the config is
> +/// erroneous (`500 Internal server error`).
> +pub fn get_endpoints(config: &Config) -> Result<Vec<WebhookConfig>, HttpError> {
> + let mut endpoints: Vec<WebhookConfig> = config
> + .config
> + .convert_to_typed_array(WEBHOOK_TYPENAME)
> + .map_err(|e| http_err!(NOT_FOUND, "Could not fetch endpoints: {e}"))?;
> +
> + for endpoint in &mut endpoints {
> + let priv_config: WebhookPrivateConfig = config
> + .private_config
> + .lookup(WEBHOOK_TYPENAME, &endpoint.name)
> + .unwrap_or_default();
> +
> + let mut secret_names = Vec::new();
> + for secret in priv_config.secret {
> + secret_names.push(
> + KeyAndBase64Val {
> + name: secret.name.clone(),
> + value: None,
> + }
> + .into(),
> + )
> + }
> +
> + endpoint.secret = secret_names;
> + }
> +
> + Ok(endpoints)
> +}
> +
> +/// Get webhook endpoint with given `name`
> +///
> +/// The caller is responsible for any needed permission checks.
> +/// Returns the endpoint or a `HttpError` if the endpoint was not found (`404 Not found`).
> +pub fn get_endpoint(config: &Config, name: &str) -> Result<WebhookConfig, HttpError> {
> + let mut endpoint: WebhookConfig = config
> + .config
> + .lookup(WEBHOOK_TYPENAME, name)
> + .map_err(|_| http_err!(NOT_FOUND, "endpoint '{name}' not found"))?;
> +
> + let priv_config: Option<WebhookPrivateConfig> = config
> + .private_config
> + .lookup(WEBHOOK_TYPENAME, &endpoint.name)
> + .ok();
> +
> + let mut secret_names = Vec::new();
> + if let Some(priv_config) = priv_config {
> + for secret in &priv_config.secret {
> + secret_names.push(
> + KeyAndBase64Val {
> + name: secret.name.clone(),
> + value: None,
> + }
> + .into(),
> + );
> + }
> + }
> +
> + endpoint.secret = secret_names;
> +
> + Ok(endpoint)
> +}
> +
> +/// Add a new webhook endpoint.
> +///
> +/// The caller is responsible for any needed permission checks.
> +/// The caller also responsible for locking the configuration files.
> +/// Returns a `HttpError` if:
> +/// - an entity with the same name already exists (`400 Bad request`)
> +/// - the configuration could not be saved (`500 Internal server error`)
> +pub fn add_endpoint(
> + config: &mut Config,
> + mut endpoint_config: WebhookConfig,
> +) -> Result<(), HttpError> {
> + super::ensure_unique(config, &endpoint_config.name)?;
> +
> + let secrets = std::mem::take(&mut endpoint_config.secret);
> +
> + set_private_config_entry(
> + config,
> + &WebhookPrivateConfig {
> + name: endpoint_config.name.clone(),
> + secret: secrets,
> + },
> + WEBHOOK_TYPENAME,
> + &endpoint_config.name,
> + )?;
> +
> + config
> + .config
> + .set_data(&endpoint_config.name, WEBHOOK_TYPENAME, &endpoint_config)
> + .map_err(|e| {
> + http_err!(
> + INTERNAL_SERVER_ERROR,
> + "could not save endpoint '{}': {e}",
> + endpoint_config.name
> + )
> + })
> +}
> +
> +/// Update existing webhook endpoint
> +///
> +/// The caller is responsible for any needed permission checks.
> +/// The caller also responsible for locking the configuration files.
> +/// Returns a `HttpError` if:
> +/// - an entity with the same name already exists (`400 Bad request`)
> +/// - the configuration could not be saved (`500 Internal server error`)
> +pub fn update_endpoint(
> + config: &mut Config,
> + name: &str,
> + config_updater: WebhookConfigUpdater,
> + delete: Option<&[DeleteableWebhookProperty]>,
> + digest: Option<&[u8]>,
> +) -> Result<(), HttpError> {
> + super::verify_digest(config, digest)?;
> +
> + let mut endpoint = get_endpoint(config, name)?;
> + endpoint.secret.clear();
> +
> + let old_secrets = config
> + .private_config
> + .lookup::<WebhookPrivateConfig>(WEBHOOK_TYPENAME, name)
> + .map_err(|err| http_err!(INTERNAL_SERVER_ERROR, "could not read secret config: {err}"))?
> + .secret;
> +
> + if let Some(delete) = delete {
> + for deleteable_property in delete {
> + match deleteable_property {
> + DeleteableWebhookProperty::Comment => endpoint.comment = None,
> + DeleteableWebhookProperty::Disable => endpoint.disable = None,
> + DeleteableWebhookProperty::Header => endpoint.header = Vec::new(),
> + DeleteableWebhookProperty::Body => endpoint.body = None,
> + DeleteableWebhookProperty::Secret => {
> + set_private_config_entry(
> + config,
> + &WebhookPrivateConfig {
> + name: name.into(),
> + secret: Vec::new(),
> + },
> + WEBHOOK_TYPENAME,
> + name,
> + )?;
> + }
> + }
> + }
> + }
> +
> + // Destructuring makes sure we don't forget any members
> + let WebhookConfigUpdater {
> + url,
> + body,
> + header,
> + method,
> + disable,
> + comment,
> + secret,
> + } = config_updater;
I didn't know that that kind of destructuring was a thing. Neat!
> +
> + if let Some(url) = url {
> + endpoint.url = url;
> + }
> +
> + if let Some(body) = body {
> + endpoint.body = Some(body);
> + }
> +
> + if let Some(header) = header {
> + endpoint.header = header;
> + }
> +
> + if let Some(method) = method {
> + endpoint.method = method;
> + }
> +
> + if let Some(disable) = disable {
> + endpoint.disable = Some(disable);
> + }
> +
> + if let Some(comment) = comment {
> + endpoint.comment = Some(comment);
> + }
> +
> + if let Some(secret) = secret {
> + let mut new_secrets: Vec<PropertyString<KeyAndBase64Val>> = Vec::new();
> +
> + for new_secret in &secret {
> + let a = if new_secret.value.is_some() {
> + // Make sure it is valid base64 encoded data
> + let _ = new_secret.decode_value().map_err(|_| {
> + http_err!(
> + BAD_REQUEST,
> + "secret '{}' does not have valid base64 encoded data",
> + new_secret.name
> + )
> + })?;
> + new_secret.clone()
> + } else if let Some(old_secret) = old_secrets.iter().find(|v| v.name == new_secret.name)
> + {
> + old_secret.clone()
> + } else {
> + http_bail!(BAD_REQUEST, "secret '{}' not known", new_secret.name);
> + };
> +
> + if new_secrets.iter().any(|s| s.name == a.name) {
> + http_bail!(BAD_REQUEST, "secret '{}' defined multiple times", a.name)
> + }
> +
> + new_secrets.push(a);
> + }
> +
> + set_private_config_entry(
> + config,
> + &WebhookPrivateConfig {
> + name: name.into(),
> + secret: new_secrets,
> + },
> + WEBHOOK_TYPENAME,
> + name,
> + )?;
> + }
> +
> + config
> + .config
> + .set_data(name, WEBHOOK_TYPENAME, &endpoint)
> + .map_err(|e| {
> + http_err!(
> + INTERNAL_SERVER_ERROR,
> + "could not save endpoint '{name}': {e}"
> + )
> + })
> +}
> +
> +/// Delete existing webhook endpoint
> +///
> +/// The caller is responsible for any needed permission checks.
> +/// The caller also responsible for locking the configuration files.
> +/// Returns a `HttpError` if:
> +/// - the entity does not exist (`404 Not found`)
> +/// - the endpoint is still referenced by another entity (`400 Bad request`)
> +pub fn delete_endpoint(config: &mut Config, name: &str) -> Result<(), HttpError> {
> + // Check if the endpoint exists
> + let _ = get_endpoint(config, name)?;
> + super::ensure_safe_to_delete(config, name)?;
> +
> + remove_private_config_entry(config, name)?;
> + config.config.sections.remove(name);
> +
> + Ok(())
> +}
> +
> +#[cfg(test)]
> +mod tests {
> + use super::*;
> + use crate::{api::test_helpers::empty_config, endpoints::webhook::HttpMethod};
> +
> + use base64::encode;
> +
> + pub fn add_default_webhook_endpoint(config: &mut Config) -> Result<(), HttpError> {
> + add_endpoint(
> + config,
> + WebhookConfig {
> + name: "webhook-endpoint".into(),
> + method: HttpMethod::Post,
> + url: "http://example.com/webhook".into(),
> + header: vec![KeyAndBase64Val::new_with_plain_value(
> + "Content-Type",
> + "application/json",
> + )
> + .into()],
> + body: Some(encode("this is the body")),
> + comment: Some("comment".into()),
> + disable: Some(false),
> + secret: vec![KeyAndBase64Val::new_with_plain_value("token", "secret").into()],
> + ..Default::default()
> + },
> + )?;
> +
> + assert!(get_endpoint(config, "webhook-endpoint").is_ok());
> + Ok(())
> + }
> +
> + #[test]
> + 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());
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_update_invalid_digest_returns_error() -> Result<(), HttpError> {
> + let mut config = empty_config();
> + add_default_webhook_endpoint(&mut config)?;
> +
> + assert!(update_endpoint(
> + &mut config,
> + "webhook-endpoint",
> + Default::default(),
> + None,
> + Some(&[0; 32])
> + )
> + .is_err());
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_update() -> Result<(), HttpError> {
> + let mut config = empty_config();
> + add_default_webhook_endpoint(&mut config)?;
> +
> + let digest = config.digest;
> +
> + update_endpoint(
> + &mut config,
> + "webhook-endpoint",
> + WebhookConfigUpdater {
> + url: Some("http://new.example.com/webhook".into()),
> + comment: Some("newcomment".into()),
> + method: Some(HttpMethod::Put),
> + // Keep the old token and set a new one
> + secret: Some(vec![
> + KeyAndBase64Val::new_with_plain_value("token2", "newsecret").into(),
> + KeyAndBase64Val {
> + name: "token".into(),
> + value: None,
> + }
> + .into(),
> + ]),
> + ..Default::default()
> + },
> + None,
> + Some(&digest),
> + )?;
> +
> + let endpoint = get_endpoint(&config, "webhook-endpoint")?;
> +
> + assert_eq!(endpoint.url, "http://new.example.com/webhook".to_string());
> + assert_eq!(endpoint.comment, Some("newcomment".to_string()));
> + assert!(matches!(endpoint.method, HttpMethod::Put));
> +
> + let secrets = config
> + .private_config
> + .lookup::<WebhookPrivateConfig>(WEBHOOK_TYPENAME, "webhook-endpoint")
> + .unwrap()
> + .secret;
> +
> + assert_eq!(secrets[1].name, "token".to_string());
> + assert_eq!(secrets[1].value, Some(encode("secret")));
> + assert_eq!(secrets[0].name, "token2".to_string());
> + assert_eq!(secrets[0].value, Some(encode("newsecret")));
> +
> + // Test property deletion
> + update_endpoint(
> + &mut config,
> + "webhook-endpoint",
> + Default::default(),
> + Some(&[DeleteableWebhookProperty::Comment, DeleteableWebhookProperty::Secret]),
You missed a `cargo fmt` here ;)
> + None,
> + )?;
> +
> + let endpoint = get_endpoint(&config, "webhook-endpoint")?;
> + assert_eq!(endpoint.comment, None);
> +
> + let secrets = config
> + .private_config
> + .lookup::<WebhookPrivateConfig>(WEBHOOK_TYPENAME, "webhook-endpoint")
> + .unwrap()
> + .secret;
> +
> + assert!(secrets.is_empty());
> +
> +
> + Ok(())
> + }
> +
> + #[test]
> + fn test_gotify_endpoint_delete() -> Result<(), HttpError> {
> + let mut config = empty_config();
> + add_default_webhook_endpoint(&mut config)?;
> +
> + delete_endpoint(&mut config, "webhook-endpoint")?;
> + assert!(delete_endpoint(&mut config, "webhook-endpoint").is_err());
> + assert_eq!(get_endpoints(&config)?.len(), 0);
> +
> + Ok(())
> + }
> +}
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes Lukas Wagner
@ 2024-07-17 15:35 ` Max Carrara
0 siblings, 0 replies; 26+ messages in thread
From: Max Carrara @ 2024-07-17 15:35 UTC (permalink / raw)
To: Proxmox VE development discussion, pbs-devel
Missed a `cargo fmt` here as well ;)
On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> common/src/notify.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/common/src/notify.rs b/common/src/notify.rs
> index e1b006b..fe192d5 100644
> --- a/common/src/notify.rs
> +++ b/common/src/notify.rs
> @@ -19,6 +19,9 @@ mod export {
> DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig,
> SmtpPrivateConfigUpdater,
> };
> + use proxmox_notify::endpoints::webhook::{
> + DeleteableWebhookProperty, WebhookConfig, WebhookConfigUpdater,
> + };
> use proxmox_notify::matcher::{
> CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
> MatcherConfigUpdater, SeverityMatcher,
> @@ -393,6 +396,66 @@ mod export {
> api::smtp::delete_endpoint(&mut config, name)
> }
>
> + #[export(serialize_error)]
> + fn get_webhook_endpoints(
> + #[try_from_ref] this: &NotificationConfig,
> + ) -> Result<Vec<WebhookConfig>, HttpError> {
> + let config = this.config.lock().unwrap();
> + api::webhook::get_endpoints(&config)
> + }
> +
> + #[export(serialize_error)]
> + fn get_webhook_endpoint(
> + #[try_from_ref] this: &NotificationConfig,
> + id: &str,
> + ) -> Result<WebhookConfig, HttpError> {
> + let config = this.config.lock().unwrap();
> + api::webhook::get_endpoint(&config, id)
> + }
> +
> + #[export(serialize_error)]
> + #[allow(clippy::too_many_arguments)]
> + fn add_webhook_endpoint(
> + #[try_from_ref] this: &NotificationConfig,
> + endpoint_config: WebhookConfig,
> + ) -> Result<(), HttpError> {
> + let mut config = this.config.lock().unwrap();
> + api::webhook::add_endpoint(
> + &mut config,
> + endpoint_config,
> + )
> + }
> +
> + #[export(serialize_error)]
> + #[allow(clippy::too_many_arguments)]
> + fn update_webhook_endpoint(
> + #[try_from_ref] this: &NotificationConfig,
> + name: &str,
> + config_updater: WebhookConfigUpdater,
> + delete: Option<Vec<DeleteableWebhookProperty>>,
> + digest: Option<&str>,
> + ) -> Result<(), HttpError> {
> + let mut config = this.config.lock().unwrap();
> + let digest = decode_digest(digest)?;
> +
> + api::webhook::update_endpoint(
> + &mut config,
> + name,
> + config_updater,
> + delete.as_deref(),
> + digest.as_deref(),
> + )
> + }
> +
> + #[export(serialize_error)]
> + fn delete_webhook_endpoint(
> + #[try_from_ref] this: &NotificationConfig,
> + name: &str,
> + ) -> Result<(), HttpError> {
> + let mut config = this.config.lock().unwrap();
> + api::webhook::delete_endpoint(&mut config, name)
> + }
> +
> #[export(serialize_error)]
> fn get_matchers(
> #[try_from_ref] this: &NotificationConfig,
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets Lukas Wagner
@ 2024-07-17 15:36 ` Max Carrara
0 siblings, 0 replies; 26+ messages in thread
From: Max Carrara @ 2024-07-17 15:36 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, pve-devel
And missed `cargo fmt` here too ;)
On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> This allows us to drop the impl of that function on the perl side.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> common/src/notify.rs | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/common/src/notify.rs b/common/src/notify.rs
> index fe192d5..0f8a35d 100644
> --- a/common/src/notify.rs
> +++ b/common/src/notify.rs
> @@ -27,6 +27,7 @@ mod export {
> MatcherConfigUpdater, SeverityMatcher,
> };
> use proxmox_notify::{api, Config, Notification, Severity};
> + use proxmox_notify::api::Target;
>
> pub struct NotificationConfig {
> config: Mutex<Config>,
> @@ -112,6 +113,14 @@ mod export {
> api::common::send(&config, ¬ification)
> }
>
> + #[export(serialize_error)]
> + fn get_targets(
> + #[try_from_ref] this: &NotificationConfig,
> + ) -> Result<Vec<Target>, HttpError> {
> + let config = this.config.lock().unwrap();
> + api::get_targets(&config)
> + }
> +
> #[export(serialize_error)]
> fn test_target(
> #[try_from_ref] this: &NotificationConfig,
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints Lukas Wagner
@ 2024-07-17 15:36 ` Max Carrara
2024-07-22 7:37 ` Lukas Wagner
0 siblings, 1 reply; 26+ messages in thread
From: Max Carrara @ 2024-07-17 15:36 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, pve-devel
On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> These just call the API implementation via the perl-rs bindings.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
> PVE/API2/Cluster/Notifications.pm | 263 +++++++++++++++++++++++++++++-
> 1 file changed, 262 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
> index 10b611c9..eae2d436 100644
> --- a/PVE/API2/Cluster/Notifications.pm
> +++ b/PVE/API2/Cluster/Notifications.pm
> @@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
> { name => 'gotify' },
> { name => 'sendmail' },
> { name => 'smtp' },
> + { name => 'webhook' },
> ];
>
> return $result;
> @@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
> 'type' => {
> description => 'Type of the target.',
> type => 'string',
> - enum => [qw(sendmail gotify smtp)],
> + enum => [qw(sendmail gotify smtp webhook)],
> },
> 'comment' => {
> description => 'Comment',
> @@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
> }
> });
>
> +my $webhook_properties = {
> + name => {
> + description => 'The name of the endpoint.',
> + type => 'string',
> + format => 'pve-configid',
> + },
> + url => {
> + description => 'Server URL',
> + type => 'string',
> + },
> + method => {
> + description => 'HTTP method',
> + type => 'string',
> + enum => [qw(post put get)],
> + },
> + header => {
> + description => 'HTTP headers to set. These have to be formatted as'
> + . ' a property string in the format name=<name>,value=<base64 of value>',
> + type => 'array',
> + items => {
> + type => 'string',
> + },
> + optional => 1,
> + },
> + body => {
> + description => 'HTTP body, base64 encoded',
> + type => 'string',
> + optional => 1,
> + },
> + secret => {
> + description => 'Secrets to set. These have to be formatted as'
> + . ' a property string in the format name=<name>,value=<base64 of value>',
> + type => 'array',
> + items => {
> + type => 'string',
> + },
> + optional => 1,
> + },
> + comment => {
> + description => 'Comment',
> + type => 'string',
> + optional => 1,
> + },
> + disable => {
> + description => 'Disable this target',
> + type => 'boolean',
> + optional => 1,
> + default => 0,
> + },
> +};
> +
> +__PACKAGE__->register_method ({
> + name => 'get_webhook_endpoints',
> + path => 'endpoints/webhook',
> + method => 'GET',
> + description => 'Returns a list of all webhook endpoints',
> + protected => 1,
> + permissions => {
> + check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {},
> + },
> + returns => {
> + type => 'array',
> + items => {
> + type => 'object',
> + properties => {
> + %$webhook_properties,
Would prefer `$webhook_properties->%*` here (postfix dereferencing) -
even though not explicitly stated in our style guide, we use that kind
of syntax for calling subroutines behind a reference, e.g.
`$foo->($arg)` instead of `&$foo($arg)`.
> + 'origin' => {
> + description => 'Show if this entry was created by a user or was built-in',
> + type => 'string',
> + enum => [qw(user-created builtin modified-builtin)],
> + },
> + },
> + },
> + links => [ { rel => 'child', href => '{name}' } ],
> + },
> + code => sub {
> + my $config = PVE::Notify::read_config();
> + my $rpcenv = PVE::RPCEnvironment::get();
> +
> + my $entities = eval {
> + $config->get_webhook_endpoints();
> + };
> + raise_api_error($@) if $@;
> +
> + return $entities;
> + }
> +});
> +
> +__PACKAGE__->register_method ({
> + name => 'get_webhook_endpoint',
> + path => 'endpoints/webhook/{name}',
> + method => 'GET',
> + description => 'Return a specific webhook endpoint',
> + protected => 1,
> + permissions => {
> + check => ['or',
> + ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + ['perm', '/mapping/notifications', ['Mapping.Audit']],
> + ],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + name => {
> + type => 'string',
> + format => 'pve-configid',
> + description => 'Name of the endpoint.'
> + },
> + }
> + },
> + returns => {
> + type => 'object',
> + properties => {
> + %$webhook_properties,
Same here :)
> + digest => get_standard_option('pve-config-digest'),
> + }
> + },
> + code => sub {
> + my ($param) = @_;
> + my $name = extract_param($param, 'name');
> +
> + my $config = PVE::Notify::read_config();
> + my $endpoint = eval {
> + $config->get_webhook_endpoint($name)
> + };
> +
> + raise_api_error($@) if $@;
> + $endpoint->{digest} = $config->digest();
> +
> + return $endpoint;
> + }
> +});
> +
> +__PACKAGE__->register_method ({
> + name => 'create_webhook_endpoint',
> + path => 'endpoints/webhook',
> + protected => 1,
> + method => 'POST',
> + description => 'Create a new webhook endpoint',
> + permissions => {
> + check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => $webhook_properties,
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> + eval {
> + PVE::Notify::lock_config(sub {
> + my $config = PVE::Notify::read_config();
> +
> + $config->add_webhook_endpoint(
> + $param,
> + );
> +
> + PVE::Notify::write_config($config);
> + });
> + };
> +
> + raise_api_error($@) if $@;
> + return;
> + }
> +});
> +
> +__PACKAGE__->register_method ({
> + name => 'update_webhook_endpoint',
> + path => 'endpoints/webhook/{name}',
> + protected => 1,
> + method => 'PUT',
> + description => 'Update existing webhook endpoint',
> + permissions => {
> + check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + %{ make_properties_optional($webhook_properties) },
> + delete => {
> + type => 'array',
> + items => {
> + type => 'string',
> + format => 'pve-configid',
> + },
> + optional => 1,
> + description => 'A list of settings you want to delete.',
> + },
> + digest => get_standard_option('pve-config-digest'),
> + }
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> +
> + my $name = extract_param($param, 'name');
> + my $delete = extract_param($param, 'delete');
> + my $digest = extract_param($param, 'digest');
> +
> + eval {
> + PVE::Notify::lock_config(sub {
> + my $config = PVE::Notify::read_config();
> +
> + $config->update_webhook_endpoint(
> + $name,
> + $param, # Config updater
> + $delete,
> + $digest,
> + );
> +
> + PVE::Notify::write_config($config);
> + });
> + };
> +
> + raise_api_error($@) if $@;
> + return;
> + }
> +});
> +
> +__PACKAGE__->register_method ({
> + name => 'delete_webhook_endpoint',
> + protected => 1,
> + path => 'endpoints/webhook/{name}',
> + method => 'DELETE',
> + description => 'Remove webhook endpoint',
> + permissions => {
> + check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
> + },
> + parameters => {
> + additionalProperties => 0,
> + properties => {
> + name => {
> + type => 'string',
> + format => 'pve-configid',
> + },
> + }
> + },
> + returns => { type => 'null' },
> + code => sub {
> + my ($param) = @_;
> + my $name = extract_param($param, 'name');
> +
> + eval {
> + PVE::Notify::lock_config(sub {
> + my $config = PVE::Notify::read_config();
> + $config->delete_webhook_endpoint($name);
> + PVE::Notify::write_config($config);
> + });
> + };
> +
> + raise_api_error($@) if $@;
> + return;
> + }
> +});
> +
> my $matcher_properties = {
> name => {
> description => 'Name of the matcher.',
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets
2024-07-17 15:35 ` [pve-devel] [pbs-devel] " Max Carrara
@ 2024-07-22 7:30 ` Lukas Wagner
2024-07-22 9:41 ` Max Carrara
0 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-22 7:30 UTC (permalink / raw)
To: Proxmox VE development discussion, Max Carrara,
Proxmox Backup Server development discussion
On 2024-07-17 17:35, Max Carrara wrote:
>> + let handlebars = setup_handlebars();
>> + let body_template = self.base_64_decode(self.config.body.as_deref().unwrap_or_default())?;
>> +
>> + let body = handlebars
>> + .render_template(&body_template, &data)
>> + .map_err(|err| {
>> + // TODO: Cleanup error types, they have become a bit messy.
>> + // No user of the notify crate distinguish between the error types any way, so
>> + // we can refactor without any issues....
>> + Error::Generic(format!("failed to render webhook body: {err}"))
>
> I'm curious, how would you clean up the error types in particular?
>
Right now, error handling is a bit messy... Some endpoints primarily use
the `NotifyFailed` variant, which wraps another error, while in some places
where I need a leaf error type that does not wrap any error I use the
`Generic` variant, which only stores a string.
I could have used the `NotifyFailed` variant here, but that one would
not have allowed me to add additional context ("failed to render webhook ..."),
unless wrapping another `Generic` variant...
I don't have yet made any detailed plans on how to clean that up though.
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets
2024-07-17 15:35 ` Max Carrara
@ 2024-07-22 7:32 ` Lukas Wagner
0 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-22 7:32 UTC (permalink / raw)
To: Proxmox VE development discussion, Max Carrara, pbs-devel
On 2024-07-17 17:35, Max Carrara wrote:
>> +
>> + assert_eq!(secrets[1].name, "token".to_string());
>> + assert_eq!(secrets[1].value, Some(encode("secret")));
>> + assert_eq!(secrets[0].name, "token2".to_string());
>> + assert_eq!(secrets[0].value, Some(encode("newsecret")));
>> +
>> + // Test property deletion
>> + update_endpoint(
>> + &mut config,
>> + "webhook-endpoint",
>> + Default::default(),
>> + Some(&[DeleteableWebhookProperty::Comment, DeleteableWebhookProperty::Secret]),
>
> You missed a `cargo fmt` here ;)
>
Right... Thanks! Was too used to RustRover autoformatting everything on save, which
I have not set up in my current nvim setup yet.
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints
2024-07-17 15:36 ` [pve-devel] [pbs-devel] " Max Carrara
@ 2024-07-22 7:37 ` Lukas Wagner
2024-07-22 9:50 ` Max Carrara
0 siblings, 1 reply; 26+ messages in thread
From: Lukas Wagner @ 2024-07-22 7:37 UTC (permalink / raw)
To: Proxmox VE development discussion, Max Carrara,
Proxmox Backup Server development discussion
On 2024-07-17 17:36, Max Carrara wrote:
> On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
>> These just call the API implementation via the perl-rs bindings.
>>
>> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>> ---
>> PVE/API2/Cluster/Notifications.pm | 263 +++++++++++++++++++++++++++++-
>> 1 file changed, 262 insertions(+), 1 deletion(-)
>>
>> diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
>> index 10b611c9..eae2d436 100644
>> --- a/PVE/API2/Cluster/Notifications.pm
>> +++ b/PVE/API2/Cluster/Notifications.pm
>> @@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
>> { name => 'gotify' },
>> { name => 'sendmail' },
>> { name => 'smtp' },
>> + { name => 'webhook' },
>> ];
>>
>> return $result;
>> @@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
>> 'type' => {
>> description => 'Type of the target.',
>> type => 'string',
>> - enum => [qw(sendmail gotify smtp)],
>> + enum => [qw(sendmail gotify smtp webhook)],
>> },
>> 'comment' => {
>> description => 'Comment',
>> @@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
>> }
>> });
>>
>> +my $webhook_properties = {
>> + name => {
>> + description => 'The name of the endpoint.',
>> + type => 'string',
>> + format => 'pve-configid',
>> + },
>> + url => {
>> + description => 'Server URL',
>> + type => 'string',
>> + },
>> + method => {
>> + description => 'HTTP method',
>> + type => 'string',
>> + enum => [qw(post put get)],
>> + },
>> + header => {
>> + description => 'HTTP headers to set. These have to be formatted as'
>> + . ' a property string in the format name=<name>,value=<base64 of value>',
>> + type => 'array',
>> + items => {
>> + type => 'string',
>> + },
>> + optional => 1,
>> + },
>> + body => {
>> + description => 'HTTP body, base64 encoded',
>> + type => 'string',
>> + optional => 1,
>> + },
>> + secret => {
>> + description => 'Secrets to set. These have to be formatted as'
>> + . ' a property string in the format name=<name>,value=<base64 of value>',
>> + type => 'array',
>> + items => {
>> + type => 'string',
>> + },
>> + optional => 1,
>> + },
>> + comment => {
>> + description => 'Comment',
>> + type => 'string',
>> + optional => 1,
>> + },
>> + disable => {
>> + description => 'Disable this target',
>> + type => 'boolean',
>> + optional => 1,
>> + default => 0,
>> + },
>> +};
>> +
>> +__PACKAGE__->register_method ({
>> + name => 'get_webhook_endpoints',
>> + path => 'endpoints/webhook',
>> + method => 'GET',
>> + description => 'Returns a list of all webhook endpoints',
>> + protected => 1,
>> + permissions => {
>> + check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
>> + check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
>> + },
>> + parameters => {
>> + additionalProperties => 0,
>> + properties => {},
>> + },
>> + returns => {
>> + type => 'array',
>> + items => {
>> + type => 'object',
>> + properties => {
>> + %$webhook_properties,
>
> Would prefer `$webhook_properties->%*` here (postfix dereferencing) -
> even though not explicitly stated in our style guide, we use that kind
> of syntax for calling subroutines behind a reference, e.g.
> `$foo->($arg)` instead of `&$foo($arg)`.
>
I kinda prefer the brevity of the prefix variant in this case. Are there
any pitfalls/problems with the prefix that I'm not aware of? If not, I'd prefer
to keep this as is, I used the syntax in many other spots in this file ;)
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
2024-07-17 15:34 ` [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Max Carrara
@ 2024-07-22 7:50 ` Lukas Wagner
0 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-22 7:50 UTC (permalink / raw)
To: Proxmox VE development discussion, Max Carrara, pbs-devel
On 2024-07-17 17:34, Max Carrara wrote:
> On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
>> Sending as an RFC because I don't want this merged yet; that being
>> said, the feature should be mostly finished at this point, I'd
>> appreciate any reviews and feedback.
>>
>> This series adds support for webhook notification targets to PVE
>> and PBS.
>>
>> A webhook is a HTTP API route provided by a third-party service that
>> can be used to inform the third-party about an event. In our case,
>> we can easily interact with various third-party notification/messaging
>> systems and send PVE/PBS notifications via this service.
>> The changes were tested against ntfy.sh, Discord and Slack.
>>
>> The configuration of webhook targets allows one to configure:
>> - The URL
>> - The HTTP method (GET/POST/PUT)
>> - HTTP Headers
>> - Body
>>
>> One can use handlebar templating to inject notification text and metadata
>> in the url, headers and body.
>>
>> One challenge is the handling of sensitve tokens and other secrets.
>> Since the endpoint is completely generic, we cannot know in advance
>> whether the body/header/url contains sensitive values.
>> Thus we add 'secrets' which are stored in the protected config only
>> accessible by root (e.g. /etc/pve/priv/notifications.cfg). These
>> secrets are accessible in URLs/headers/body via templating:
>>
>> Url: https://example.com/{{ secrets.token }}
>>
>> Secrets can only be set and updated, but never retrieved via the API.
>> In the UI, secrets are handled like other secret tokens/passwords.
>>
>> Bumps for PVE:
>> - libpve-rs-perl needs proxmox-notify bumped
>> - pve-manager needs bumped proxmox-widget-toolkit and libpve-rs-perl bumped
>> - proxmox-mail-forward needs proxmox-notify bumped
>>
>> Bumps for PBS:
>> - proxmox-backup needs proxmox-notify bumped
>> - proxmox-mail-forward needs proxmox-notify bumped
>
> Since this is an RFC, I mainly just did some proofreading; I haven't
> really spotted anything out of the ordinary, apart from a few *very
> small* things I commented on inline.
>
> I like the overall idea of adding webhooks, so this looks pretty solid
> to me. At first I thought that this might be a bit of a niche use case,
> but I feel like it might actually be quite interesting for orgs that are
> e.g. on Slack: You could e.g. just "route" all notifications via a
> webhook to Slack, and Slack then sends a push notification to one's
> phone. The same can obviously done with other applications / services as
> well. So, pretty cool stuff :)
>
> Not sure if this has been discussed somewhere already (off list etc.),
> but could you elaborate on why you don't want this merged yet? The
> patches look pretty solid to me, IMHO. Then again, I haven't really
> tested them yet due to all the required package bumps, so take this with
> a grain of salt.
>
> If you want to have this RFC tested, I can of course give it a shot - do
> let me know if that's the case :)
>
I posted this as an RFC because while I consider this as mostly finished,
it did not yet go through my own rigorous self-review/testing.
I had to switch to some other task and wanted to get this version out to get some
general feedback.
There are no changes planned unless I or somebody else discovers any issues,
so I'd very much welcome any testing :)
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets
2024-07-22 7:30 ` Lukas Wagner
@ 2024-07-22 9:41 ` Max Carrara
0 siblings, 0 replies; 26+ messages in thread
From: Max Carrara @ 2024-07-22 9:41 UTC (permalink / raw)
To: Lukas Wagner, Proxmox VE development discussion,
Proxmox Backup Server development discussion
On Mon Jul 22, 2024 at 9:30 AM CEST, Lukas Wagner wrote:
>
>
> On 2024-07-17 17:35, Max Carrara wrote:
> >> + let handlebars = setup_handlebars();
> >> + let body_template = self.base_64_decode(self.config.body.as_deref().unwrap_or_default())?;
> >> +
> >> + let body = handlebars
> >> + .render_template(&body_template, &data)
> >> + .map_err(|err| {
> >> + // TODO: Cleanup error types, they have become a bit messy.
> >> + // No user of the notify crate distinguish between the error types any way, so
> >> + // we can refactor without any issues....
> >> + Error::Generic(format!("failed to render webhook body: {err}"))
> >
> > I'm curious, how would you clean up the error types in particular?
> >
>
> Right now, error handling is a bit messy... Some endpoints primarily use
> the `NotifyFailed` variant, which wraps another error, while in some places
> where I need a leaf error type that does not wrap any error I use the
> `Generic` variant, which only stores a string.
> I could have used the `NotifyFailed` variant here, but that one would
> not have allowed me to add additional context ("failed to render webhook ..."),
> unless wrapping another `Generic` variant...
>
> I don't have yet made any detailed plans on how to clean that up though.
Hmm, I see - thanks for elaborating! I'm not really sure how to clean
them up either, but if I can think of something, I'll let you know.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints
2024-07-22 7:37 ` Lukas Wagner
@ 2024-07-22 9:50 ` Max Carrara
2024-07-22 13:56 ` Thomas Lamprecht
0 siblings, 1 reply; 26+ messages in thread
From: Max Carrara @ 2024-07-22 9:50 UTC (permalink / raw)
To: Lukas Wagner, Proxmox VE development discussion,
Proxmox Backup Server development discussion
On Mon Jul 22, 2024 at 9:37 AM CEST, Lukas Wagner wrote:
>
>
> On 2024-07-17 17:36, Max Carrara wrote:
> > On Fri Jul 12, 2024 at 1:27 PM CEST, Lukas Wagner wrote:
> >> These just call the API implementation via the perl-rs bindings.
> >>
> >> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> >> ---
> >> PVE/API2/Cluster/Notifications.pm | 263 +++++++++++++++++++++++++++++-
> >> 1 file changed, 262 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/PVE/API2/Cluster/Notifications.pm b/PVE/API2/Cluster/Notifications.pm
> >> index 10b611c9..eae2d436 100644
> >> --- a/PVE/API2/Cluster/Notifications.pm
> >> +++ b/PVE/API2/Cluster/Notifications.pm
> >> @@ -108,6 +108,7 @@ __PACKAGE__->register_method ({
> >> { name => 'gotify' },
> >> { name => 'sendmail' },
> >> { name => 'smtp' },
> >> + { name => 'webhook' },
> >> ];
> >>
> >> return $result;
> >> @@ -144,7 +145,7 @@ __PACKAGE__->register_method ({
> >> 'type' => {
> >> description => 'Type of the target.',
> >> type => 'string',
> >> - enum => [qw(sendmail gotify smtp)],
> >> + enum => [qw(sendmail gotify smtp webhook)],
> >> },
> >> 'comment' => {
> >> description => 'Comment',
> >> @@ -1094,6 +1095,266 @@ __PACKAGE__->register_method ({
> >> }
> >> });
> >>
> >> +my $webhook_properties = {
> >> + name => {
> >> + description => 'The name of the endpoint.',
> >> + type => 'string',
> >> + format => 'pve-configid',
> >> + },
> >> + url => {
> >> + description => 'Server URL',
> >> + type => 'string',
> >> + },
> >> + method => {
> >> + description => 'HTTP method',
> >> + type => 'string',
> >> + enum => [qw(post put get)],
> >> + },
> >> + header => {
> >> + description => 'HTTP headers to set. These have to be formatted as'
> >> + . ' a property string in the format name=<name>,value=<base64 of value>',
> >> + type => 'array',
> >> + items => {
> >> + type => 'string',
> >> + },
> >> + optional => 1,
> >> + },
> >> + body => {
> >> + description => 'HTTP body, base64 encoded',
> >> + type => 'string',
> >> + optional => 1,
> >> + },
> >> + secret => {
> >> + description => 'Secrets to set. These have to be formatted as'
> >> + . ' a property string in the format name=<name>,value=<base64 of value>',
> >> + type => 'array',
> >> + items => {
> >> + type => 'string',
> >> + },
> >> + optional => 1,
> >> + },
> >> + comment => {
> >> + description => 'Comment',
> >> + type => 'string',
> >> + optional => 1,
> >> + },
> >> + disable => {
> >> + description => 'Disable this target',
> >> + type => 'boolean',
> >> + optional => 1,
> >> + default => 0,
> >> + },
> >> +};
> >> +
> >> +__PACKAGE__->register_method ({
> >> + name => 'get_webhook_endpoints',
> >> + path => 'endpoints/webhook',
> >> + method => 'GET',
> >> + description => 'Returns a list of all webhook endpoints',
> >> + protected => 1,
> >> + permissions => {
> >> + check => ['perm', '/mapping/notifications', ['Mapping.Modify']],
> >> + check => ['perm', '/mapping/notifications', ['Mapping.Audit']],
> >> + },
> >> + parameters => {
> >> + additionalProperties => 0,
> >> + properties => {},
> >> + },
> >> + returns => {
> >> + type => 'array',
> >> + items => {
> >> + type => 'object',
> >> + properties => {
> >> + %$webhook_properties,
> >
> > Would prefer `$webhook_properties->%*` here (postfix dereferencing) -
> > even though not explicitly stated in our style guide, we use that kind
> > of syntax for calling subroutines behind a reference, e.g.
> > `$foo->($arg)` instead of `&$foo($arg)`.
> >
>
> I kinda prefer the brevity of the prefix variant in this case. Are there
> any pitfalls/problems with the prefix that I'm not aware of? If not, I'd prefer
> to keep this as is, I used the syntax in many other spots in this file ;)
I personally have no hard feelings if you keep it tbh. Postfix
dereference is mainly useful if you have e.g. a nested hash (or rather,
makes more sense) because of how the code is usually read. For example,
%$foo->{bar}->{baz}
vs
$foo->{bar}->{baz}->%*
I'd argue that the main benefit is that it's easier to read for people
who aren't as familiar with Perl, but before this gets too bikesheddy,
I'm personally fine if you keep it as-is for simple cases like the above
:P
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
` (10 preceding siblings ...)
2024-07-17 15:34 ` [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Max Carrara
@ 2024-07-22 12:10 ` Stefan Hanreich
2024-07-22 12:29 ` Lukas Wagner
11 siblings, 1 reply; 26+ messages in thread
From: Stefan Hanreich @ 2024-07-22 12:10 UTC (permalink / raw)
To: Proxmox VE development discussion, Lukas Wagner, pbs-devel
Hi, I tested the following things via the 'Test' option in the
Notifications configuration panel in PBS and PVE:
* Setting a custom header
* Using a secret in body / URL
* POST / PUT / GET methods
I triggered notifications in PBS for the following things:
* GC
* Sync
I triggered notifications in PVE for the following things:
* Backup
using the following template:
```
this is {{ secrets.test }}, can has webhook?
{{ title }}
{{ message }}
{{ severity }}
{{ timestamp }}
{{ json fields }}
{{ json secrets }}
{{ url-encode secrets.encode }}
{{ escape secrets.escape }}
```
I used dummyhttp [1] for debugging and displaying the HTTP requests.
I got the following JS error when trying to save a secret/header/body
template with the content: `qweqwe ßẞuuuu`
Uncaught DOMException: String contains an invalid character
getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11924
each ExtJS
getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11920
checkChange ExtJS
itemChange https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:12009
ExtJS 22
proxmoxlib.js:11924
Seems like the issue is with btoa and uppercase scharfes S?
When using Umlauts, I at least get an error message, but cannot save.
Other than that everything worked fine - consider this:
Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints
2024-07-22 12:10 ` Stefan Hanreich
@ 2024-07-22 12:29 ` Lukas Wagner
0 siblings, 0 replies; 26+ messages in thread
From: Lukas Wagner @ 2024-07-22 12:29 UTC (permalink / raw)
To: Proxmox VE development discussion, Stefan Hanreich, pbs-devel
On 2024-07-22 14:10, Stefan Hanreich wrote:
> I got the following JS error when trying to save a secret/header/body
> template with the content: `qweqwe ßẞuuuu`
>
> Uncaught DOMException: String contains an invalid character
> getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11924
> each ExtJS
> getValue https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:11920
> checkChange ExtJS
> itemChange https://10.101.110.1:8007/widgettoolkit/proxmoxlib.js:12009
> ExtJS 22
> proxmoxlib.js:11924
>
>
> Seems like the issue is with btoa and uppercase scharfes S?
> When using Umlauts, I at least get an error message, but cannot save.
>
>
> Other than that everything worked fine - consider this:
> Tested-By: Stefan Hanreich <s.hanreich@proxmox.com>
>
Thanks a lot for testing, Stefan.
Indeed it looks like btoa has problems with unicode characters.
Luckily, MDN [1] has got our back with that one. I'll use that workaround
in an upcoming v3.
[1] https://developer.mozilla.org/en-US/docs/Glossary/Base64#the_unicode_problem
--
- Lukas
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [pve-devel] [pbs-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints
2024-07-22 9:50 ` Max Carrara
@ 2024-07-22 13:56 ` Thomas Lamprecht
0 siblings, 0 replies; 26+ messages in thread
From: Thomas Lamprecht @ 2024-07-22 13:56 UTC (permalink / raw)
To: Proxmox VE development discussion, Max Carrara, Lukas Wagner,
Proxmox Backup Server development discussion
just chiming in on the perl dereference style
Am 22/07/2024 um 11:50 schrieb Max Carrara:
>>>> + %$webhook_properties,
>>> Would prefer `$webhook_properties->%*` here (postfix dereferencing) -
>>> even though not explicitly stated in our style guide, we use that kind
>>> of syntax for calling subroutines behind a reference, e.g.
>>> `$foo->($arg)` instead of `&$foo($arg)`.
>>>
>> I kinda prefer the brevity of the prefix variant in this case. Are there
>> any pitfalls/problems with the prefix that I'm not aware of? If not, I'd prefer
>> to keep this as is, I used the syntax in many other spots in this file 😉
> I personally have no hard feelings if you keep it tbh. Postfix
> dereference is mainly useful if you have e.g. a nested hash (or rather,
> makes more sense) because of how the code is usually read. For example,
>
> %$foo->{bar}->{baz}
IIRC above cannot work, and even if, it still might benefit from being
written as `%{$foo->{bar}->{baz}}`
>
> vs
>
> $foo->{bar}->{baz}->%*
>
> I'd argue that the main benefit is that it's easier to read for people
> who aren't as familiar with Perl, but before this gets too bikesheddy,
> I'm personally fine if you keep it as-is for simple cases like the above
> :P
It can often be a bit easier to read, as you can go from left to right
without having to backtrack to check what the dereferencing actually
affects, though you can get used to both, so of course it can be a bit
subjective.
For variables that are dereferenced as a whole, i.e. not a hash or array
sub-element of them, it's definitely fine to use the `%$foo` style, as it
can't really be confusing, and we already use that all over the place.
For dereferencing a sub-element, I'd slightly prefer the newer variant,
i.e. `$foo->{bar}->%*` for a hash or `$foo->{bar}->@*` for a list.
You could also convert to this variant when touching lines anyway, but I
do not think this is such a big style issue to make that a must or even
do such transformations for their own sake.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-07-22 13:55 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-12 11:27 [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 01/12] notify: implement webhook targets Lukas Wagner
2024-07-17 15:35 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-22 7:30 ` Lukas Wagner
2024-07-22 9:41 ` Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox v2 02/12] notify: add api for " Lukas Wagner
2024-07-17 15:35 ` Max Carrara
2024-07-22 7:32 ` Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 03/12] common: notify: add bindings for webhook API routes Lukas Wagner
2024-07-17 15:35 ` Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-perl-rs v2 04/12] common: notify: add bindings for get_targets Lukas Wagner
2024-07-17 15:36 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-12 11:27 ` [pve-devel] [PATCH widget-toolkit v2 05/12] notification: add UI for adding/updating webhook targets Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 06/12] api: notifications: use get_targets impl from proxmox-notify Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH manager v2 07/12] api: add routes for webhook notification endpoints Lukas Wagner
2024-07-17 15:36 ` [pve-devel] [pbs-devel] " Max Carrara
2024-07-22 7:37 ` Lukas Wagner
2024-07-22 9:50 ` Max Carrara
2024-07-22 13:56 ` Thomas Lamprecht
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 09/12] api: notification: add API routes for webhook targets Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-backup v2 10/12] ui: utils: enable webhook edit window Lukas Wagner
2024-07-12 11:27 ` [pve-devel] [PATCH proxmox-mail-forward v2 12/12] bump proxmox-notify dependency Lukas Wagner
2024-07-17 15:34 ` [pve-devel] [RFC many v2 00/12] notifications: add support for webhook endpoints Max Carrara
2024-07-22 7:50 ` Lukas Wagner
2024-07-22 12:10 ` Stefan Hanreich
2024-07-22 12:29 ` Lukas Wagner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox