From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 11DFE1FF17A for ; Tue, 9 Dec 2025 13:34:19 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 68F6C276D7; Tue, 9 Dec 2025 13:34:57 +0100 (CET) Date: Tue, 09 Dec 2025 13:34:23 +0100 Message-Id: Cc: "pdm-devel" From: =?utf-8?q?Michael_K=C3=B6ppl?= To: "Proxmox Datacenter Manager development discussion" Mime-Version: 1.0 X-Mailer: aerc 0.21.0 References: <20251205180447.441371-1-s.shaji@proxmox.com> <20251205180447.441371-3-s.shaji@proxmox.com> In-Reply-To: <20251205180447.441371-3-s.shaji@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1765283657822 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.036 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [remotes.rs] Subject: Re: [pdm-devel] [PATCH datacenter-manager 2/5] server: api: add support to optionally delete token from remote X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Fri Dec 5, 2025 at 7:04 PM CET, Shan Shaji wrote: > 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 > --- > server/src/api/remotes.rs | 45 +++++++++++++++++++++++++++++++++++++-- > 1 file changed, 43 insertions(+), 2 deletions(-) > > diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs > index 298ad13..9f9786c 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,56 @@ 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.", > + 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) -> Result<(), Error> { > let (mut remotes, _) = pdm_config::remotes::config()?; Isn't that a bit racy, though, considering you're making asynchronous network calls if delete_token is true? By the time you arrive at the save_config(...) call, the list of remotes you fetched here might have changed because another call to remove a remote was made pretty much at the same time, causing you to overwrite those changes with the list of remotes you fetched here. Not too familiar with the code here, but we use pdm_config::remotes::lock_config()?; in other parts to avoid this, it seems. > > + if delete_token.unwrap_or(false) { > + let remote = remotes > + .get_mut(&id) nit: .get(&id) should be sufficient > + .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?; nit: this could just be 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?; nit: same here > + client > + .delete_admin_token(&user, token_name.as_str()) nit: same here for user > + .await > + .map_err(short_delete_err)? > + } > + }; > + } > + > if remotes.remove(&id).is_none() { > http_bail!(NOT_FOUND, "no such entry {id:?}"); > } _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel