Re: [PATCH datacenter-manager v3 2/6] server: api: add support to optionally delete token from remote

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Shan Shaji <s.shaji@proxmox.com> writes:

> Previously, when removing a remote, the token was still present in the
> remote configuration. When users tried to add the remote again, they
> received an error because a token with the same name already existed.
> To support deleting the token from the remote, add an optional
> parameter to the API endpoint.
>
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>
> changes since v2: No changes. 
> changes since v1:
>  - nit: inlined the id argument using the format string.
>  - used `get` instead of `get_mut` inorder to access remote.
>  - removed unnecessary `&` operator use.
>
>  server/src/api/remotes.rs | 46 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs
> index 298ad13..82b8469 100644
> --- a/server/src/api/remotes.rs
> +++ b/server/src/api/remotes.rs
> @@ -27,6 +27,7 @@ use crate::api::remote_updates;
>  use crate::metric_collection;
>  use crate::{connection, pbs_client};
>  
> +use super::pbs;
>  use super::pve;
>  use super::rrd_common;
>  use super::rrd_common::DataPoint;
> @@ -292,16 +293,57 @@ pub fn update_remote(
>      input: {
>          properties: {
>              id: { schema: REMOTE_ID_SCHEMA },
> +            "delete-token": {
> +                type: bool,
> +                description: "Optional boolean value to delete the token from remote.",

Being optional and boolean are already in the schema, please use
something along the lines of:

- Whether to delete the API token on the remote
- If true, deletes the API token on the remote

> +                optional: true,
> +            }
>          },
>      },
>      access: {
>          permission: &Permission::Privilege(&["resource"], PRIV_RESOURCE_MODIFY, false),
>      },
>  )]
> -/// List all the remotes this instance is managing.
> -pub fn remove_remote(id: String) -> Result<(), Error> {
> +/// Remove a remote that this instance is managing.
> +pub async fn remove_remote(id: String, delete_token: Option<bool>) -> Result<(), Error> {
> +    let _lock = pdm_config::remotes::lock_config()?;
>      let (mut remotes, _) = pdm_config::remotes::config()?;
>  
> +    if delete_token.unwrap_or(false) {
> +        let remote = remotes
> +            .get(&id)
> +            .ok_or_else(|| http_err!(NOT_FOUND, "no such remote {id:?}"))?;
> +
> +        let user = remote.authid.user();
> +
> +        let short_delete_err = |err: proxmox_client::Error| {
> +            format_err!("error deleting token: {}", err.source().unwrap_or(&err))
> +        };
> +
> +        let token_name = remote
> +            .authid
> +            .tokenname()
> +            .ok_or_else(|| format_err!("Unable to find the token for the remote {id:?}"))?;
> +
> +        // connect to remote and delete the already existing token.
> +        match remote.ty {
> +            RemoteType::Pve => {
> +                let client = pve::connect_or_login(remote).await?;
> +                client
> +                    .delete_token(user.as_str(), token_name.as_str())
> +                    .await
> +                    .map_err(short_delete_err)?
> +            }
> +            RemoteType::Pbs => {
> +                let client = pbs::connect_or_login(remote).await?;
> +                client
> +                    .delete_admin_token(user, token_name.as_str())
> +                    .await
> +                    .map_err(short_delete_err)?
> +            }
> +        };
> +    }
> +
>      if remotes.remove(&id).is_none() {
>          http_bail!(NOT_FOUND, "no such entry {id:?}");
>      }

-- 
Maximiliano