Re: [PATCH proxmox v2 05/16] notify: smtp: Add state handling logic

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

On Wed Mar 25, 2026 at 2:14 PM CET, Arthur Bied-Charreton wrote:
> Create new state file in add_endpoint, and create/update existing one in
> update_endpoint.
>
> Add trigger_state_refresh to the Endpoint trait, with no-op default
> implementation. Override trigger_state_refresh in SmtpEndpoint's
> Endpoint impl to trigger an OAuth2 token exchange, in order to rotate
> an existing token, or extend its lifetime.
>
> Since trigger_state_refresh is called in pveupdate, it may be called
> multiple times in quick succession by the different nodes in the
> cluster. In order to avoid unnecessary churn on the state files, the
> last_refreshed field is used to check if the state has been refreshed
> shortly before, and skip the update if it is the case.
>

[...]


> diff --git a/proxmox-notify/src/api/smtp.rs b/proxmox-notify/src/api/smtp.rs
> index 4231cdae..1b25e5d3 100644
> --- a/proxmox-notify/src/api/smtp.rs
> +++ b/proxmox-notify/src/api/smtp.rs
> @@ -1,8 +1,9 @@
>  use proxmox_http_error::HttpError;
>  
>  use crate::api::{http_bail, http_err};
> +use crate::context::context;
>  use crate::endpoints::smtp::{
> -    DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
> +    self, DeleteableSmtpProperty, SmtpConfig, SmtpConfigUpdater, SmtpPrivateConfig,
>      SmtpPrivateConfigUpdater, SMTP_TYPENAME,
>  };
>  use crate::Config;
> @@ -69,6 +70,20 @@ pub fn add_endpoint(
>          &endpoint_config.name,
>      )?;
>  
> +    if oauth2_refresh_token.is_some() {
> +        let oauth_state = smtp::State::from(oauth2_refresh_token);

If you changes to a State::new as proposed two patches prior,
this could just be a 

if let Some(token) = oauth2_refresh_token {
    let oauth_state = smtp::state::new(token, proxmox_time::epoch_i64());
}

> +
> +        context()
> +            .save_oauth_state(&endpoint_config.name, Some(oauth_state))
> +            .map_err(|e| {
> +                http_err!(
> +                    INTERNAL_SERVER_ERROR,
> +                    "could not create state file for '{}': {e}",
> +                    &endpoint_config.name
> +                )
> +            })?;
> +    }
> +

with this fixed:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>