Re: [pdm-devel] [PATCH proxmox-datacenter-manager 1/4] server: add api for getting available updates/changelogs for remote nodes

[Stefan Hanreich <s.hanreich@proxmox.com>]

On 9/2/25 5:14 PM, Lukas Wagner wrote:
> This adds new APIs for update management:
> 
>     GET /pve/remotes/{remote}/nodes/{node}/apt/changelog
>       -> get package changelog
>     GET /pve/remotes/{remote}/nodes/{node}/apt/update
>       -> get list of updatable packages
>     POST /pve/remotes/{remote}/nodes/{node}/apt/update
>       -> refresh APT database
> 
> At this time these just pass the call through to PVE with no caching
> involved on the PDM side. This should be fine for this API, but once
> we have an API for 'give me a view of ALL available remote updates',
> we need to introduce a cache that is periodically refreshed.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  server/src/api/pve/apt.rs    | 119 +++++++++++++++++++++++++++++++++++
>  server/src/api/pve/mod.rs    |   3 +-
>  server/src/api/pve/node.rs   |   1 +
>  server/src/lib.rs            |   1 +
>  server/src/remote_updates.rs |  96 ++++++++++++++++++++++++++++
>  5 files changed, 219 insertions(+), 1 deletion(-)
>  create mode 100644 server/src/api/pve/apt.rs
>  create mode 100644 server/src/remote_updates.rs
> 
> diff --git a/server/src/api/pve/apt.rs b/server/src/api/pve/apt.rs
> new file mode 100644
> index 00000000..f5027fb8
> --- /dev/null
> +++ b/server/src/api/pve/apt.rs
> @@ -0,0 +1,119 @@
> +use anyhow::Error;
> +
> +use proxmox_apt_api_types::{APTGetChangelogOptions, APTUpdateInfo};
> +use proxmox_router::{list_subdirs_api_method, Permission, Router, SubdirMap};
> +use proxmox_schema::api;
> +use proxmox_schema::api_types::NODE_SCHEMA;
> +
> +use pdm_api_types::{remotes::REMOTE_ID_SCHEMA, RemoteUpid, PRIV_RESOURCE_MODIFY};
> +
> +use crate::{api::remotes::get_remote, remote_updates};
> +
> +#[api(
> +    input: {
> +        properties: {
> +            remote: {
> +                schema: REMOTE_ID_SCHEMA,
> +            },
> +            node: {
> +                schema: NODE_SCHEMA,
> +            },
> +        },
> +    },
> +    returns: {
> +        description: "A list of packages with available updates.",
> +        type: Array,
> +        items: {
> +            type: APTUpdateInfo
> +        },
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}", "system"], PRIV_RESOURCE_MODIFY, false),
> +    },
> +)]
> +/// List available APT updates for a remote PVE node.
> +async fn apt_update_available(remote: String, node: String) -> Result<Vec<APTUpdateInfo>, Error> {

potentially fine to take a ref here for both params as well?

> +    let (config, _digest) = pdm_config::remotes::config()?;
> +    let remote = get_remote(&config, &remote)?;
> +
> +    let updates = remote_updates::list_available_updates(remote.clone(), &node).await?;
> +
> +    Ok(updates)
> +}
> +
> +#[api(
> +    input: {
> +        properties: {
> +            remote: {
> +                schema: REMOTE_ID_SCHEMA,
> +            },
> +            node: {
> +                schema: NODE_SCHEMA,
> +            },
> +        },
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}", "system"], PRIV_RESOURCE_MODIFY, false),
> +    },
> +)]
> +/// Update the APT database of a remote PVE node.
> +pub async fn apt_update_database(remote: String, node: String) -> Result<RemoteUpid, Error> {

here too then

> +    let (config, _digest) = pdm_config::remotes::config()?;
> +    let remote = get_remote(&config, &remote)?;
> +
> +    let upid = remote_updates::update_apt_database(remote, &node).await?;
> +
> +    Ok(upid)
> +}
> +
> +#[api(
> +    input: {
> +        properties: {
> +            remote: {
> +                schema: REMOTE_ID_SCHEMA,
> +            },
> +            node: {
> +                schema: NODE_SCHEMA,
> +            },
> +            options: {
> +                type: APTGetChangelogOptions,
> +                flatten: true,
> +            },
> +        },
> +    },
> +    returns: {
> +        description: "The Package changelog.",
> +        type: String,
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["resource", "{remote}", "node", "{node}", "system"], PRIV_RESOURCE_MODIFY, false),
> +    },
> +)]
> +/// Retrieve the changelog of the specified package for a remote PVE node.
> +async fn apt_get_changelog(
> +    remote: String,
> +    node: String,
> +    options: APTGetChangelogOptions,

here too then

> +) -> Result<String, Error> {
> +    let (config, _digest) = pdm_config::remotes::config()?;
> +    let remote = get_remote(&config, &remote)?;
> +
> +    remote_updates::get_changelog(remote.clone(), &node, options.name).await
> +}
> +
> +const SUBDIRS: SubdirMap = &[
> +    (
> +        "changelog",
> +        &Router::new().get(&API_METHOD_APT_GET_CHANGELOG),
> +    ),
> +    (
> +        "update",
> +        &Router::new()
> +            .get(&API_METHOD_APT_UPDATE_AVAILABLE)
> +            .post(&API_METHOD_APT_UPDATE_DATABASE),
> +    ),
> +];
> +
> +pub const ROUTER: Router = Router::new()
> +    .get(&list_subdirs_api_method!(SUBDIRS))
> +    .subdirs(SUBDIRS);
> diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
> index 2cfdc5b7..0768083d 100644
> --- a/server/src/api/pve/mod.rs
> +++ b/server/src/api/pve/mod.rs
> @@ -31,6 +31,7 @@ use crate::connection::PveClient;
>  use crate::connection::{self, probe_tls_connection};
>  use crate::remote_tasks;
>  
> +mod apt;
>  mod lxc;
>  mod node;
>  mod qemu;
> @@ -77,7 +78,7 @@ const RESOURCES_ROUTER: Router = Router::new().get(&API_METHOD_CLUSTER_RESOURCES
>  const STATUS_ROUTER: Router = Router::new().get(&API_METHOD_CLUSTER_STATUS);
>  
>  // converts a remote + PveUpid into a RemoteUpid and starts tracking it
> -async fn new_remote_upid(remote: String, upid: PveUpid) -> Result<RemoteUpid, Error> {
> +pub async fn new_remote_upid(remote: String, upid: PveUpid) -> Result<RemoteUpid, Error> {
>      let remote_upid: RemoteUpid = (remote, upid.to_string()).try_into()?;
>      remote_tasks::track_running_task(remote_upid.clone()).await?;
>      Ok(remote_upid)
> diff --git a/server/src/api/pve/node.rs b/server/src/api/pve/node.rs
> index df96a1c3..99539d1c 100644
> --- a/server/src/api/pve/node.rs
> +++ b/server/src/api/pve/node.rs
> @@ -13,6 +13,7 @@ pub const ROUTER: Router = Router::new()
>  
>  #[sortable]
>  const SUBDIRS: SubdirMap = &sorted!([
> +    ("apt", &super::apt::ROUTER),
>      ("rrddata", &super::rrddata::NODE_RRD_ROUTER),
>      ("network", &Router::new().get(&API_METHOD_GET_NETWORK)),
>      ("storage", &Router::new().get(&API_METHOD_GET_STORAGES)),
> diff --git a/server/src/lib.rs b/server/src/lib.rs
> index 3f8b7708..a58190d8 100644
> --- a/server/src/lib.rs
> +++ b/server/src/lib.rs
> @@ -9,6 +9,7 @@ pub mod metric_collection;
>  pub mod parallel_fetcher;
>  pub mod remote_cache;
>  pub mod remote_tasks;
> +pub mod remote_updates;
>  pub mod resource_cache;
>  pub mod task_utils;
>  
> diff --git a/server/src/remote_updates.rs b/server/src/remote_updates.rs
> new file mode 100644
> index 00000000..809e5de1
> --- /dev/null
> +++ b/server/src/remote_updates.rs
> @@ -0,0 +1,96 @@
> +use anyhow::Error;
> +use pdm_api_types::RemoteUpid;
> +
> +use proxmox_apt_api_types::APTUpdateInfo;
> +
> +use pdm_api_types::remotes::{Remote, RemoteType};
> +
> +use crate::api::pve::new_remote_upid;
> +use crate::connection;
> +
> +/// Return a list of available updates for a given remote node.
> +pub async fn list_available_updates(
> +    remote: Remote,
> +    node: &str,
> +) -> Result<Vec<APTUpdateInfo>, Error> {
> +    let updates = fetch_available_updates(remote, node.to_string()).await?;
> +    Ok(updates)
> +}
> +

Is there a reason for having this as an additional function and not just
calling fetch_available_updates at its callsites (after changing its
visibility)?

At the very least it could return the result of the await directly? Or
do you want to reserve the option of changing the public API? In that
case we could still introduce a private helper later.

> +/// Trigger `apt update` on a remote node.
> +///
> +/// The function returns a `[RemoteUpid]` for the started update task.
> +pub async fn update_apt_database(remote: &Remote, node: &str) -> Result<RemoteUpid, Error> {
> +    match remote.ty {
> +        RemoteType::Pve => {
> +            let client = connection::make_pve_client(remote)?;
> +
> +            let params = pve_api_types::AptUpdateParams {
> +                notify: Some(false),
> +                quiet: Some(false),
> +            };
> +            let upid = client.update_apt_database(node, params).await?;
> +
> +            new_remote_upid(remote.id.clone(), upid).await
> +        }
> +        RemoteType::Pbs => todo!(),

return an Error here?

> +    }
> +}
> +
> +/// Get the changelog for a given package.
> +pub async fn get_changelog(remote: Remote, node: &str, package: String) -> Result<String, Error> {

Interestingly the generated binding takes a String even though it
wouldn't need to? Maybe something we should check in the generation
logic later?

> +    match remote.ty {
> +        RemoteType::Pve => {
> +            let client = connection::make_pve_client(&remote)?;
> +
> +            client
> +                .get_package_changelog(node, package, None)
> +                .await
> +                .map_err(Into::into)
> +        }
> +        RemoteType::Pbs => Ok("TODO: Return PBS package changelog".into()),
> +    }
> +}
> +
> +async fn fetch_available_updates(
> +    remote: Remote,
> +    node: String,

take a ref here too?

> +) -> Result<Vec<APTUpdateInfo>, Error> {
> +    match remote.ty {
> +        RemoteType::Pve => {
> +            let client = connection::make_pve_client(&remote)?;
> +
> +            let updates = client
> +                .list_available_updates(&node)
> +                .await?
> +                .into_iter()
> +                .map(map_pve_update_info)
> +                .collect();
> +
> +            Ok(updates)
> +        }
> +        RemoteType::Pbs => {
> +            let _client = connection::make_pbs_client(&remote)?;
> +
> +            // TODO: Fetch available updates
> +            //
> +
> +            Ok(Vec::new())
> +        }
> +    }
> +}
> +
> +fn map_pve_update_info(info: pve_api_types::AptUpdateInfo) -> APTUpdateInfo {
> +    APTUpdateInfo {
> +        package: info.package,
> +        title: info.title,
> +        arch: info.arch,
> +        description: info.description,
> +        version: info.version,
> +        old_version: info.old_version.unwrap_or_default(),
> +        origin: info.origin,
> +        priority: info.priority,
> +        section: info.section,
> +        extra_info: None,
> +    }
> +}



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel