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 2C8301FF142 for ; Fri, 22 May 2026 10:34:27 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8CD6D1C1B; Fri, 22 May 2026 10:34:21 +0200 (CEST) From: Dominik Csapak To: pdm-devel@lists.proxmox.com Subject: [PATCH datacenter-manager v3 2/6] lib/api: add 'location-info' api call with cached information Date: Fri, 22 May 2026 10:34:03 +0200 Message-ID: <20260522083412.1223719-8-d.csapak@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260522083412.1223719-1-d.csapak@proxmox.com> References: <20260522083412.1223719-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.050 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 Message-ID-Hash: BL7ZEIYKRRYHR7Y5HWYWFHPAWK7THFHU X-Message-ID-Hash: BL7ZEIYKRRYHR7Y5HWYWFHPAWK7THFHU X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: PVE and PBS remotes can have a location configured, and we want to query that, so add a cache for this location info and an API call to query it. Modeled after the other resource api calls (status/subscription) but lets the location info be optional since this is a relatively new feature. The biggest difference is that we try to get a location at all costs, so if there is no new enough location in the cache we try to update it. If that fails (e.g. because the remote is offline) we retry the cache with unlimited 'max-age'. Otherwise there could be a situation where the location info is so old that offline remotes won't be shown. Refactor some permission checks from the subscription api call so we can reuse that. Signed-off-by: Dominik Csapak --- lib/pdm-api-types/src/lib.rs | 34 ++++++++ server/src/api/resources.rs | 120 +++++++++++++++++++++----- server/src/lib.rs | 1 + server/src/location_cache.rs | 160 +++++++++++++++++++++++++++++++++++ 4 files changed, 294 insertions(+), 21 deletions(-) create mode 100644 server/src/location_cache.rs diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs index dab0f240..0cef8de9 100644 --- a/lib/pdm-api-types/src/lib.rs +++ b/lib/pdm-api-types/src/lib.rs @@ -519,3 +519,37 @@ pub const TASKLOG_DOWNLOAD_PARAM_SCHEMA: Schema = proxmox_schema::BooleanSchema: ) .default(false) .schema(); + +#[api( + properties: { + latitude: { + type: Number, + minimum: -90.0, + maximum: 90.0, + }, + longitude: { + type: Number, + minimum: -180.0, + maximum: 180.0, + }, + }, +)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +/// Represents a physical location +pub struct Location { + #[serde(skip_serializing_if = "Option::is_none")] + /// An optional short name/description of the location. + pub name: Option, + /// The latitude of the location + pub latitude: f64, + /// The longitude of the location + pub longitude: f64, +} + +#[derive(Clone, PartialEq, Serialize, Deserialize)] +/// Contains (cached) location information about a remote. +pub struct CachedLocationInfo { + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + /// The locations of the individual nodes (if not all the same). + pub node_locations: HashMap, +} diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs index 1a6a23d9..9ec1ce89 100644 --- a/server/src/api/resources.rs +++ b/server/src/api/resources.rs @@ -18,7 +18,7 @@ use pdm_api_types::resource::{ use pdm_api_types::subscription::{ NodeSubscriptionInfo, RemoteSubscriptionState, RemoteSubscriptions, SubscriptionLevel, }; -use pdm_api_types::{Authid, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA}; +use pdm_api_types::{Authid, CachedLocationInfo, PRIV_RESOURCE_AUDIT, VIEW_ID_SCHEMA}; use pdm_search::{Search, SearchTerm}; use proxmox_access_control::CachedUserInfo; use proxmox_router::{ @@ -41,6 +41,10 @@ pub const ROUTER: Router = Router::new() #[sortable] const SUBDIRS: SubdirMap = &sorted!([ ("list", &Router::new().get(&API_METHOD_GET_RESOURCES)), + ( + "location-info", + &Router::new().get(&API_METHOD_GET_LOCATION_INFO) + ), ("status", &Router::new().get(&API_METHOD_GET_STATUS)), ( "top-entities", @@ -214,6 +218,29 @@ impl From for RemoteResources { } } +fn check_remote_priv(user_info: &CachedUserInfo, auth_id: &Authid, remote: &str) -> bool { + user_info + .check_privs(auth_id, &["resource", remote], PRIV_RESOURCE_AUDIT, false) + .is_ok() +} + +/// When returning true, all remotes are allowed and no per-remote permission check should be +/// necessary +fn check_all_remotes_allowed( + user_info: &CachedUserInfo, + auth_id: &Authid, + view: Option<&str>, +) -> Result { + Ok(if let Some(view) = view { + user_info.check_privs(auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?; + false + } else { + user_info + .check_privs(auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false) + .is_ok() + }) +} + #[api( // FIXME:: see list-like API calls in resource routers, we probably want more fine-grained // checks.. @@ -641,34 +668,16 @@ pub async fn get_subscription_status( .parse()?; let user_info = CachedUserInfo::new()?; - let allow_all = if let Some(view) = &view { - user_info.check_privs(&auth_id, &["view", view], PRIV_RESOURCE_AUDIT, false)?; - false - } else { - user_info - .check_privs(&auth_id, &["resource"], PRIV_RESOURCE_AUDIT, false) - .is_ok() - }; + let allow_all = check_all_remotes_allowed(&user_info, &auth_id, view.as_deref())?; let view = views::get_optional_view(view.as_deref())?; - let check_priv = |remote_name: &str| -> bool { - user_info - .check_privs( - &auth_id, - &["resource", remote_name], - PRIV_RESOURCE_AUDIT, - false, - ) - .is_ok() - }; - for (remote_name, remote) in remotes_config { if let Some(view) = &view { if view.can_skip_remote(&remote_name) { continue; } - } else if !allow_all && !check_priv(&remote_name) { + } else if !allow_all && !check_remote_priv(&user_info, &auth_id, &remote_name) { continue; } @@ -1340,6 +1349,75 @@ fn map_pbs_datastore_status( } } +#[api( + // FIXME:: see list-like API calls in resource routers, we probably want more fine-grained + // checks.. + access: { + permission: &Permission::Anybody, + }, + input: { + properties: { + "max-age": { + description: "Maximum age (in seconds) of cached remote resources. If remote is not \ +reachable or returns an error for the location, the last value from the cache will be returned in \ +any case", + default: 24*60*60, + optional: true, + }, + view: { + schema: VIEW_ID_SCHEMA, + optional: true, + }, + } + }, +)] +/// Get the location info of the selected view (or all remotes) +async fn get_location_info( + max_age: u64, + view: Option, + rpcenv: &mut dyn RpcEnvironment, +) -> Result, Error> { + let (remotes_config, _) = pdm_config::remotes::config()?; + + let mut futures = Vec::new(); + + let auth_id = rpcenv + .get_auth_id() + .context("no authid available")? + .parse()?; + let user_info = CachedUserInfo::new()?; + + let allow_all = check_all_remotes_allowed(&user_info, &auth_id, view.as_deref())?; + + let view = views::get_optional_view(view.as_deref())?; + + for (remote_name, remote) in remotes_config { + if let Some(view) = &view { + if view.can_skip_remote(&remote_name) { + continue; + } + } else if !allow_all && !check_remote_priv(&user_info, &auth_id, &remote_name) { + continue; + } + + let future = async move { + match crate::location_cache::get_location_info_for_remote(&remote, max_age).await { + Ok(Some(info)) => Some((remote_name, info)), + Ok(None) => None, + Err(err) => { + log::debug!("error on getting location data from cache: {err}"); + None + } + } + }; + + futures.push(future); + } + + let res = join_all(futures).await; + Ok(res.into_iter().flatten().collect()) +} + #[cfg(test)] mod tests { use crate::api::resources::is_remotes_only; diff --git a/server/src/lib.rs b/server/src/lib.rs index 89ab3035..941f99f6 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -7,6 +7,7 @@ pub mod auth; pub mod context; pub mod env; pub mod jobstate; +pub mod location_cache; pub mod metric_collection; pub mod namespaced_cache; pub mod parallel_fetcher; diff --git a/server/src/location_cache.rs b/server/src/location_cache.rs new file mode 100644 index 00000000..cce33721 --- /dev/null +++ b/server/src/location_cache.rs @@ -0,0 +1,160 @@ +use std::collections::HashMap; + +use anyhow::Error; +use futures::future::join_all; + +use proxmox_schema::PropertyString; + +use pdm_api_types::remotes::{Remote, RemoteType}; +use pdm_api_types::{CachedLocationInfo, Location}; +use pve_api_types::{NodeConfigLocation, NodeConfigProperty}; +use serde::Deserialize; + +use crate::{api_cache, connection}; + +const LOCATION_STATE_CACHE_KEY: &str = "location-state"; + +/// Get the location info from a remote, flow is as follows: +/// * try to get from the cache with `max_age` parameter +/// * if that fails, try to get updated location value +/// * if that fails try the cache again with `max_age` set to `u64::MAX` +/// +/// This is done so we can get a location even if it was not cached within the last +/// day and the remote is e.g. offline or not reachable. +pub async fn get_location_info_for_remote( + remote: &Remote, + max_age: u64, +) -> Result, Error> { + if let Some(cached) = get_cached_location_info(&remote.id, max_age).await? { + Ok(Some(cached)) + } else { + let location_info = match fetch_remote_location_info(remote).await { + Ok(info) => info, + Err(err) => { + log::debug!( + "error getting location info for {}, falling back to cache: {err}", + remote.id + ); + + // last resort, if we can't get the location from here, we can't get any + get_cached_location_info(&remote.id, u64::MAX).await? + } + }; + let info = match location_info { + Some(info) => info, + None => return Ok(None), + }; + let now = proxmox_time::epoch_i64(); + + if let Some(existing_state) = + update_cached_location_info(&remote.id, info.clone(), now).await? + { + // Somebody else updated the cache while we performed the API request, + // return the more recent data instead of the data we just fetched. + return Ok(Some(existing_state)); + } + Ok(Some(info)) + } +} + +async fn get_cached_location_info( + remote: &str, + max_age: u64, +) -> Result, Error> { + let cache = api_cache::read_remote(remote).await?; + let location_state = cache + .get_with_max_age(LOCATION_STATE_CACHE_KEY, max_age as i64) + .await + .inspect_err(|err| log::error!("could not read location-state from API cache: {err}")) + .ok() + .flatten(); + + Ok(location_state) +} + +async fn update_cached_location_info( + remote: &str, + info: CachedLocationInfo, + now: i64, +) -> Result, Error> { + let cache = api_cache::write_remote(remote).await?; + + Ok(cache + .set_if_newer_with_timestamp(LOCATION_STATE_CACHE_KEY, info, now) + .await?) +} + +#[derive(Deserialize)] +struct DataCenterOptions { + location: Option>, +} + +async fn fetch_remote_location_info(remote: &Remote) -> Result, Error> { + match remote.ty { + RemoteType::Pve => { + let client = connection::make_pve_client(remote)?; + + // first, get datacenter location + let cluster_options: DataCenterOptions = + serde_json::from_value(client.cluster_options().await?)?; + let location = cluster_options.location.map(|loc| loc.into_inner()); + + // then get the individual node locations + let mut node_locations = HashMap::new(); + let nodes = client.list_nodes().await?; + let mut futures = Vec::with_capacity(nodes.len()); + for node in nodes.iter() { + let future = client.node_config(&node.node, Some(NodeConfigProperty::Location)); + futures.push(async move { (node.node.clone(), future.await) }); + } + + for (node_name, remote_info) in join_all(futures).await { + let mut node_location = None; + let remote_info = remote_info?; + if let Some(location) = remote_info.location { + if let Ok(location) = location.parse::>() { + let location = location.into_inner(); + node_location = Some(Location { + name: location.name, + latitude: location.latitude, + longitude: location.longitude, + }); + } + } + + match (node_location, &location) { + (Some(location), _) => { + node_locations.insert(node_name, location); + } + (None, Some(location)) => { + node_locations.insert(node_name, location.clone()); + } + _ => {} + } + } + if node_locations.is_empty() { + Ok(None) + } else { + Ok(Some(CachedLocationInfo { node_locations })) + } + } + RemoteType::Pbs => { + let client = connection::make_pbs_client(remote)?; + let loc = client.node_config().await?.location.map(|location| { + let location = location.into_inner(); + let mut node_locations = HashMap::new(); + node_locations.insert( + "localhost".to_string(), + Location { + name: location.name, + latitude: location.latitude, + longitude: location.longitude, + }, + ); + CachedLocationInfo { node_locations } + }); + + Ok(loc) + } + } +} -- 2.47.3