From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 743881FF17A for ; Tue, 11 Nov 2025 11:50:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 199DF641F; Tue, 11 Nov 2025 11:51:14 +0100 (CET) From: Lukas Wagner To: pdm-devel@lists.proxmox.com Date: Tue, 11 Nov 2025 11:50:56 +0100 Message-ID: <20251111105059.148997-6-l.wagner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251111105059.148997-1-l.wagner@proxmox.com> References: <20251111105059.148997-1-l.wagner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762858243692 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.029 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pdm-devel] [RFC datacenter-manager 5/8] pdm-api-types: remote upid: add type field to RemoteUpid 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" In quite a few places in the code where we handle RemoteUpids, we need to know the actual type of the remote, for instance to - build correct API paths from it, if the UPID is passed to a product-specific API endpoint - get fields from the actual UPID, which requires parsing the native UPID type - if the type is unknown here, we have to either guess (attempt to parse one type, and if it does not work, try the other type), or get the type from somewhere else (some other parameter, or from remotes.cfg) These can be easily solved by storing the type of the remote in the RemoteUpid. The serialized representation is changed in such a way that the type is prepended to the original represenation, e.g. pve:remote-name! This change aims to avoid breakage by being backward-compatible with the old representation without the type field. In this case the type is simply inferred by parsing the UPID. This adds some runtime cost, but this is only really relevant for migrating the contents of the remote task cache over to the new format. Once it has been migrated (which happens automatically when rewriting the archive files, or if that does not happen, they are simply rotated out after a while), the inefficient code path is not really needed any more. Signed-off-by: Lukas Wagner --- lib/pdm-api-types/Cargo.toml | 1 + lib/pdm-api-types/src/remote_upid.rs | 122 +++++++++++++++++++++++---- lib/pdm-api-types/src/remotes.rs | 4 +- 3 files changed, 110 insertions(+), 17 deletions(-) diff --git a/lib/pdm-api-types/Cargo.toml b/lib/pdm-api-types/Cargo.toml index e66558bb..8e70c092 100644 --- a/lib/pdm-api-types/Cargo.toml +++ b/lib/pdm-api-types/Cargo.toml @@ -24,4 +24,5 @@ proxmox-time.workspace = true proxmox-serde.workspace = true proxmox-subscription = { workspace = true, features = ["api-types"], default-features = false } +pbs-api-types = { workspace = true } pve-api-types = { workspace = true } diff --git a/lib/pdm-api-types/src/remote_upid.rs b/lib/pdm-api-types/src/remote_upid.rs index 454c9b1f..32106e42 100644 --- a/lib/pdm-api-types/src/remote_upid.rs +++ b/lib/pdm-api-types/src/remote_upid.rs @@ -5,19 +5,30 @@ use anyhow::{bail, Error}; use proxmox_schema::api_types::SAFE_ID_REGEX; use proxmox_schema::{ApiType, Schema, StringSchema}; +use crate::remotes::RemoteType; + pub const REMOTE_UPID_SCHEMA: Schema = StringSchema::new("A remote UPID") - .min_length("C!UPID:N:12345678:12345678:12345678:::".len()) + .min_length("abc:C!UPID:N:12345678:12345678:12345678:::".len()) .schema(); #[derive(Clone, Debug, Eq, PartialEq, Hash)] /// A UPID type for tasks on a specific remote. pub struct RemoteUpid { remote: String, + remote_type: RemoteType, // This can either be a PVE UPID or a PBS UPID, both have distinct, incompatible formats. upid: String, } impl RemoteUpid { + /// Create a new remote UPID. + pub fn new(remote: String, remote_type: RemoteType, upid: String) -> Self { + Self { + remote, + upid, + remote_type, + } + } /// Get the remote for this UPID. pub fn remote(&self) -> &str { &self.remote @@ -37,6 +48,21 @@ impl RemoteUpid { pub fn into_upid(self) -> String { self.upid } + + /// Return the type of the remote which corresponds to this UPID. + pub fn remote_type(&self) -> RemoteType { + self.remote_type + } + + fn deduce_type(raw_upid: &str) -> Result { + if raw_upid.parse::().is_ok() { + Ok(RemoteType::Pve) + } else if raw_upid.parse::().is_ok() { + Ok(RemoteType::Pbs) + } else { + bail!("invalid upid: {raw_upid}"); + } + } } impl ApiType for RemoteUpid { @@ -51,7 +77,13 @@ impl TryFrom<(String, String)> for RemoteUpid { bail!("bad remote id in remote upid"); } - Ok(Self { remote, upid }) + let ty = Self::deduce_type(&upid)?; + + Ok(Self { + remote, + upid, + remote_type: ty, + }) } } @@ -63,9 +95,12 @@ impl TryFrom<(String, &str)> for RemoteUpid { bail!("bad remote id in remote upid"); } + let ty = Self::deduce_type(upid)?; + Ok(Self { remote, upid: upid.to_string(), + remote_type: ty, }) } } @@ -78,9 +113,30 @@ impl TryFrom<(&str, &str)> for RemoteUpid { bail!("bad remote id in remote upid"); } + let ty = Self::deduce_type(upid)?; + Ok(Self { remote: remote.to_string(), upid: upid.to_string(), + remote_type: ty, + }) + } +} + +impl TryFrom<(&str, &str, &str)> for RemoteUpid { + type Error = Error; + + fn try_from((ty, remote, upid): (&str, &str, &str)) -> Result { + if !SAFE_ID_REGEX.is_match(remote) { + bail!("bad remote id in remote upid"); + } + + let ty = ty.parse()?; + + Ok(Self { + remote: remote.to_string(), + upid: upid.to_string(), + remote_type: ty, }) } } @@ -89,16 +145,19 @@ impl std::str::FromStr for RemoteUpid { type Err = Error; fn from_str(s: &str) -> Result { - match s.find('!') { + match s.split_once('!') { None => bail!("missing '!' separator in remote upid"), - Some(pos) => (&s[..pos], &s[(pos + 1)..]).try_into(), + Some((remote_and_type, upid)) => match remote_and_type.split_once(':') { + Some((ty, remote)) => (ty, remote, upid).try_into(), + None => (remote_and_type, upid).try_into(), + }, } } } impl fmt::Display for RemoteUpid { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}!{}", self.remote, self.upid) + write!(f, "{}:{}!{}", self.remote_type, self.remote, self.upid) } } @@ -110,13 +169,14 @@ mod tests { use super::*; #[test] - fn test_from_str() { + fn test_from_str_old_format() { let pve_upid: RemoteUpid = "pve-remote!UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:" .parse() .unwrap(); assert_eq!(pve_upid.remote(), "pve-remote"); + assert_eq!(pve_upid.remote_type(), RemoteType::Pve); assert_eq!( pve_upid.upid(), "UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:" @@ -128,6 +188,34 @@ mod tests { .unwrap(); assert_eq!(pbs_upid.remote(), "pbs-remote"); + assert_eq!(pbs_upid.remote_type(), RemoteType::Pbs); + assert_eq!( + pbs_upid.upid(), + "UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:" + ); + } + + #[test] + fn test_from_str_new_format() { + let pve_upid: RemoteUpid = + "pve:pve-remote!UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:" + .parse() + .unwrap(); + + assert_eq!(pve_upid.remote(), "pve-remote"); + assert_eq!(pve_upid.remote_type(), RemoteType::Pve); + assert_eq!( + pve_upid.upid(), + "UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:" + ); + + let pbs_upid: RemoteUpid = + "pbs:pbs-remote!UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:" + .parse() + .unwrap(); + + assert_eq!(pbs_upid.remote(), "pbs-remote"); + assert_eq!(pbs_upid.remote_type(), RemoteType::Pbs); assert_eq!( pbs_upid.upid(), "UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:" @@ -136,24 +224,26 @@ mod tests { #[test] fn test_display() { - let pve_upid = RemoteUpid { - remote: "pve-remote".to_string(), - upid: "UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:".to_string(), - }; + let pve_upid = RemoteUpid::new( + "pve-remote".to_string(), + RemoteType::Pve, + "UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:".to_string(), + ); assert_eq!( pve_upid.to_string(), - "pve-remote!UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:" + "pve:pve-remote!UPID:pve:00039E4D:002638B8:67B4A9D1:stopall::root@pam:" ); - let pbs_upid = RemoteUpid { - remote: "pbs-remote".to_string(), - upid: "UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:".to_string(), - }; + let pbs_upid = RemoteUpid::new( + "pbs-remote".to_string(), + RemoteType::Pbs, + "UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:".to_string(), + ); assert_eq!( pbs_upid.to_string(), - "pbs-remote!UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:" + "pbs:pbs-remote!UPID:pbs:000002B2:00000158:00000000:674D828C:logrotate::root@pam:" ); } } diff --git a/lib/pdm-api-types/src/remotes.rs b/lib/pdm-api-types/src/remotes.rs index dd6afa68..bd90ef1e 100644 --- a/lib/pdm-api-types/src/remotes.rs +++ b/lib/pdm-api-types/src/remotes.rs @@ -39,7 +39,9 @@ pub struct NodeUrl { #[api] /// The type of a remote entry. -#[derive(Clone, Copy, Default, Debug, Eq, PartialEq, Deserialize, Serialize, Ord, PartialOrd)] +#[derive( + Clone, Copy, Default, Debug, Eq, PartialEq, Deserialize, Serialize, Ord, PartialOrd, Hash, +)] #[serde(rename_all = "lowercase")] pub enum RemoteType { /// A Proxmox VE node. -- 2.47.3 _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel