[pbs-devel] [pve-devel] [PATCH proxmox v2 02/12] notify: add api for webhook targets
Max Carrara
m.carrara at proxmox.com
Wed Jul 17 17:35:28 CEST 2024
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 at 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(())
> + }
> +}
More information about the pbs-devel
mailing list