Re: [PATCH proxmox-perl-rs 1/1] notify (smtp): add oauth2 parameters to bindings

["Lukas Wagner" <l.wagner@proxmox.com>]

On Fri Feb 13, 2026 at 5:04 PM CET, Arthur Bied-Charreton wrote:
> Update SMTP bindings to take the Smtp(Private)?Config structs directly,
> and additionally the OAuth2 refresh token.

The commit message should probably go into more detail on *why* we now
use the struct itself instead of enumerating all parameters in the
function signature.

The addition of 'trigger_state_refresh' should probably its own commit.
The change of parameters for the existing functions is a breaking
change, so I think it would be nice to have these separated from the
addition of this new function.

>
> Signed-off-by: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
> ---
>  common/src/bindings/notify.rs | 82 ++++++++++++-----------------------
>  1 file changed, 28 insertions(+), 54 deletions(-)
>
> diff --git a/common/src/bindings/notify.rs b/common/src/bindings/notify.rs
> index 409270a..137cc79 100644
> --- a/common/src/bindings/notify.rs
> +++ b/common/src/bindings/notify.rs
> @@ -12,7 +12,7 @@ pub mod proxmox_rs_notify {
>      use std::collections::HashMap;
>      use std::sync::Mutex;
>  
> -    use anyhow::{Error, bail};
> +    use anyhow::{bail, Error};
>      use serde_json::Value as JSONValue;
>  
>      use perlmod::Value;
> @@ -26,7 +26,7 @@ pub mod proxmox_rs_notify {
>          DeleteableSendmailProperty, SendmailConfig, SendmailConfigUpdater,
>      };
>      use proxmox_notify::endpoints::smtp::{
> -        DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpMode, SmtpPrivateConfig,
> +        DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
>          SmtpPrivateConfigUpdater,
>      };
>      use proxmox_notify::endpoints::webhook::{
> @@ -36,7 +36,7 @@ pub mod proxmox_rs_notify {
>          CalendarMatcher, DeleteableMatcherProperty, FieldMatcher, MatchModeOperator, MatcherConfig,
>          MatcherConfigUpdater, SeverityMatcher,
>      };
> -    use proxmox_notify::{Config, Notification, Severity, api};
> +    use proxmox_notify::{api, Config, Notification, Severity};
>  
>      /// A notification catalog instance.
>      ///
> @@ -141,6 +141,19 @@ pub mod proxmox_rs_notify {
>          api::common::send(&config, &notification)
>      }
>  
> +    /// Method: Refresh the state for all endpoints.
> +    ///
> +    /// This iterates through all configured targets, refreshing their state if needed.
> +    ///
> +    /// See [`api::common::refresh_targets`]
> +    #[export(serialize_error)]
> +    pub fn trigger_state_refresh(
> +        #[try_from_ref] this: &NotificationConfig,
> +    ) -> Result<(), HttpError> {
> +        let config = this.config.lock().unwrap();
> +        api::common::trigger_state_refresh(&config)
> +    }
> +
>      /// Method: Get a list of all notification targets.
>      ///
>      /// See [`api::get_targets`].
> @@ -390,37 +403,16 @@ pub mod proxmox_rs_notify {
>      #[allow(clippy::too_many_arguments)]
>      pub fn add_smtp_endpoint(
>          #[try_from_ref] this: &NotificationConfig,
> -        name: String,
> -        server: String,
> -        port: Option<u16>,
> -        mode: Option<SmtpMode>,
> -        username: Option<String>,
> -        password: Option<String>,
> -        mailto: Option<Vec<String>>,
> -        mailto_user: Option<Vec<String>>,
> -        from_address: String,
> -        author: Option<String>,
> -        comment: Option<String>,
> -        disable: Option<bool>,
> +        smtp_config: SmtpConfig,
> +        smtp_private_config: SmtpPrivateConfig,
> +        oauth2_refresh_token: Option<String>,
>      ) -> Result<(), HttpError> {
>          let mut config = this.config.lock().unwrap();
>          api::smtp::add_endpoint(
>              &mut config,
> -            SmtpConfig {
> -                name: name.clone(),
> -                server,
> -                port,
> -                mode,
> -                username,
> -                mailto: mailto.unwrap_or_default(),
> -                mailto_user: mailto_user.unwrap_or_default(),
> -                from_address,
> -                author,
> -                comment,
> -                disable,
> -                origin: None,
> -            },
> -            SmtpPrivateConfig { name, password },
> +            smtp_config,
> +            smtp_private_config,
> +            oauth2_refresh_token,
>          )
>      }
>  
> @@ -432,17 +424,9 @@ pub mod proxmox_rs_notify {
>      pub fn update_smtp_endpoint(
>          #[try_from_ref] this: &NotificationConfig,
>          name: &str,
> -        server: Option<String>,
> -        port: Option<u16>,
> -        mode: Option<SmtpMode>,
> -        username: Option<String>,
> -        password: Option<String>,
> -        mailto: Option<Vec<String>>,
> -        mailto_user: Option<Vec<String>>,
> -        from_address: Option<String>,
> -        author: Option<String>,
> -        comment: Option<String>,
> -        disable: Option<bool>,
> +        smtp_config_updater: SmtpConfigUpdater,
> +        smtp_private_config_updater: SmtpPrivateConfigUpdater,
> +        oauth2_refresh_token: Option<String>,
>          delete: Option<Vec<DeleteableSmtpProperty>>,
>          digest: Option<&str>,
>      ) -> Result<(), HttpError> {
> @@ -452,19 +436,9 @@ pub mod proxmox_rs_notify {
>          api::smtp::update_endpoint(
>              &mut config,
>              name,
> -            SmtpConfigUpdater {
> -                server,
> -                port,
> -                mode,
> -                username,
> -                mailto,
> -                mailto_user,
> -                from_address,
> -                author,
> -                comment,
> -                disable,
> -            },
> -            SmtpPrivateConfigUpdater { password },
> +            smtp_config_updater,
> +            smtp_private_config_updater,
> +            oauth2_refresh_token,
>              delete.as_deref(),
>              digest.as_deref(),
>          )

I wondered (and we shortly discussed off-list) whether it would make
sense to keep the existing functions as is and then add two *new*
functions which offer the 'new' struct/hash-based API.

It would have the benefit of being a non-breaking change which would
ease life for maintainers a bit, but then on the other hand it's also a
bit weird since we'd want to remove the old functions pretty soon anyway
-- we have only one caller to worry about anyway.

When removing the old variants we'd have to bump the version contraints
anyway, so in reality, it does not help that much. So after some
thought, I think the current approach is fine.