Re: [pbs-devel] [PATCH proxmox-backup v2 6/6] api: admin: datastore: implement streaming content api call

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Oct 08, 2025 at 03:43:36PM +0200, Dominik Csapak wrote:
> this is a new api call that utilizes `proxmox_router::Stream` to provide
> a streaming interface to querying the datastore content.
> 
> This can be done when a client requests this api call with the
> `application/json-seq` Accept header.
> 
> In contrast to the existing api calls, this one
> * returns all types of content items (namespaces, groups, snapshots; can
>   be filtered with a parameter)
> * iterates over them recursively (with the range that is given with the
>   parameter)
> 
> The api call returns the data in the following order:
> * first all visible namespaces
> * then for each ns in order
>   * each group
>   * each snapshot
> 
> This is done so that we can have a good way of building a tree view in
> the ui.
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> changes from v1:
> * don't use a tokio::spawn  around a 'spawn_blocking'
> * don't use async-stream, but proxmox-router::Stream
> * add new use statements from pbs_api_types in a new line
>   (makes the diff more readable..)
> * remove content type filter from api call (can be done later still)
> * return one level more for namespaces than given, otherwise
>   we'll list groups + snapshots for the levels, but not the existing
>   namespaces there
> 
>  src/api2/admin/datastore.rs | 175 +++++++++++++++++++++++++++++++-----
>  1 file changed, 155 insertions(+), 20 deletions(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 643d1694b..811ee8713 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -23,7 +23,7 @@ use proxmox_compression::zstd::ZstdEncoder;
>  use proxmox_log::LogContext;
>  use proxmox_router::{
>      http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission,
> -    Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
> +    Record, Router, RpcEnvironment, RpcEnvironmentType, SubdirMap,
>  };
>  use proxmox_rrd_api_types::{RrdMode, RrdTimeframe};
>  use proxmox_schema::*;
> @@ -49,6 +49,7 @@ use pbs_api_types::{
>      PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ,
>      PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
>  };
> +use pbs_api_types::{DatastoreContent, NamespaceListItem};
>  use pbs_client::pxar::{create_tar, create_zip};
>  use pbs_config::CachedUserInfo;
>  use pbs_datastore::backup_info::BackupInfo;
> @@ -70,7 +71,10 @@ use proxmox_rest_server::{formatter, worker_is_active, WorkerTask};
>  
>  use crate::api2::backup::optional_ns_param;
>  use crate::api2::node::rrd::create_value_from_rrd;
> -use crate::backup::{check_ns_privs_full, ListAccessibleBackupGroups, VerifyWorker, NS_PRIVS_OK};
> +use crate::backup::{
> +    can_access_any_namespace_below, check_ns_privs, check_ns_privs_full,
> +    ListAccessibleBackupGroups, VerifyWorker, NS_PRIVS_OK,
> +};
>  use crate::server::jobstate::{compute_schedule_status, Job, JobState};
>  use crate::tools::{backup_info_to_snapshot_list_item, get_all_snapshot_files, read_backup_index};
>  
> @@ -387,7 +391,7 @@ pub async fn delete_snapshot(
>  }
>  
>  #[api(
> -    serializing: true,
> +    stream: true,
>      input: {
>          properties: {
>              store: { schema: DATASTORE_SCHEMA },
> @@ -395,40 +399,125 @@ pub async fn delete_snapshot(
>                  type: BackupNamespace,
>                  optional: true,
>              },
> -            "backup-type": {
> -                optional: true,
> -                type: BackupType,
> -            },
> -            "backup-id": {
> +            "max-depth": {
> +                schema: NS_MAX_DEPTH_SCHEMA,
>                  optional: true,
> -                schema: BACKUP_ID_SCHEMA,
>              },
>          },
>      },
> -    returns: pbs_api_types::ADMIN_DATASTORE_LIST_SNAPSHOTS_RETURN_TYPE,
>      access: {
>          permission: &Permission::Anybody,
>          description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_AUDIT for any \
>              or DATASTORE_BACKUP and being the owner of the group",
>      },
>  )]
> -/// List backup snapshots.
> -pub async fn list_snapshots(
> +/// List datastore content, recursively through all namespaces.
> +pub async fn list_content(
>      store: String,
>      ns: Option<BackupNamespace>,
> -    backup_type: Option<BackupType>,
> -    backup_id: Option<String>,
> +    max_depth: Option<usize>,
>      _param: Value,
>      _info: &ApiMethod,
>      rpcenv: &mut dyn RpcEnvironment,
> -) -> Result<Vec<SnapshotListItem>, Error> {
> +) -> Result<proxmox_router::Stream, Error> {
> +    let (sender, receiver) = tokio::sync::mpsc::channel(128);
> +
>      let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let user_info = CachedUserInfo::new()?;
>  
> -    tokio::task::spawn_blocking(move || unsafe {
> -        list_snapshots_blocking(store, ns, backup_type, backup_id, auth_id)
> -    })
> -    .await
> -    .map_err(|err| format_err!("failed to await blocking task: {err}"))?
> +    let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?;
> +
> +    // show one level of namespaces more than we use for group/snapshot list
> +    let list_max_depth = max_depth.map(|depth| depth + 1);
> +    if !can_access_any_namespace_below(
> +        datastore.clone(),
> +        &auth_id,
> +        &user_info,
> +        ns.clone(),
> +        list_max_depth,
> +    ) {
> +        proxmox_router::http_bail!(FORBIDDEN, "permission check failed");
> +    }
> +
> +    let ns = ns.unwrap_or_default();
> +
> +    tokio::task::spawn_blocking(move || {
> +        for ns in datastore.recursive_iter_backup_ns_ok(ns.clone(), list_max_depth)? {
> +            match check_ns_privs(&store, &ns, &auth_id, NS_PRIVS_OK) {
> +                Ok(_) => sender.blocking_send(Record::new(DatastoreContent::from(
> +                    NamespaceListItem { ns, comment: None },
> +                )))?,
> +                // don't disclose errors about ns privileges, since that can be an info leak
> +                Err(_) => continue,
> +            }
> +        }
> +
> +        for ns in datastore.recursive_iter_backup_ns_ok(ns, max_depth)? {
> +            let list_all = match check_ns_privs_full(
> +                &store,
> +                &ns,
> +                &auth_id,
> +                PRIV_DATASTORE_AUDIT,
> +                PRIV_DATASTORE_BACKUP,
> +            ) {
> +                Ok(requires_owner) => !requires_owner,
> +                // don't disclose errors about ns privileges, since that can be an info leak
> +                Err(_) => continue,
> +            };
> +            for group in datastore.iter_backup_groups(ns.clone())? {
> +                let group = match group {
> +                    Ok(group) => group,
> +                    Err(err) => {
> +                        sender.blocking_send(Record::error(err))?;
> +                        continue;
> +                    }
> +                };
> +                let group = backup_group_to_group_list_item(
> +                    datastore.clone(),
> +                    group,
> +                    &ns,
> +                    &auth_id,
> +                    list_all,
> +                );
> +
> +                if let Some(group) = group {
> +                    sender
> +                        .blocking_send(Record::new(DatastoreContent::from((ns.clone(), group))))?;
> +                }
> +            }
> +
> +            for group in datastore.iter_backup_groups(ns.clone())? {
> +                let group = match group {
> +                    Ok(group) => group,
> +                    Err(err) => {
> +                        sender.blocking_send(Record::error(err))?;
> +                        continue;
> +                    }
> +                };
> +                let owner = match get_group_owner(&store, &ns, &group) {
> +                    Some(auth_id) => auth_id,
> +                    None => continue,
> +                };

I think we need a `check_group_owner()` call here - since this is what
decides whether this particular group is even listed in the loop above.

`backup_group_to_group_list_item()` does this and returns `None` if this
check fails.

> +                for backup_dir in group.iter_snapshots()? {
> +                    let backup_info = match backup_dir {
> +                        Ok(snap) => BackupInfo::new(snap)?,
> +                        Err(err) => {
> +                            sender.blocking_send(Record::error(err))?;
> +                            continue;
> +                        }
> +                    };
> +                    let snapshot = backup_info_to_snapshot_list_item(&backup_info, &owner);
> +                    sender.blocking_send(Record::new(DatastoreContent::from((
> +                        ns.clone(),
> +                        snapshot,
> +                    ))))?;
> +                }
> +            }
> +        }
> +        Ok::<_, Error>(())
> +    });
> +
> +    Ok(ReceiverStream::new(receiver).into())
>  }
>  
>  fn get_group_owner(
> @@ -514,6 +603,51 @@ unsafe fn list_snapshots_blocking(
>      })
>  }
>  
> +#[api(
> +    serializing: true,
> +    input: {
> +        properties: {
> +            store: { schema: DATASTORE_SCHEMA },
> +            ns: {
> +                type: BackupNamespace,
> +                optional: true,
> +            },
> +            "backup-type": {
> +                optional: true,
> +                type: BackupType,
> +            },
> +            "backup-id": {
> +                optional: true,
> +                schema: BACKUP_ID_SCHEMA,
> +            },
> +        },
> +    },
> +    returns: pbs_api_types::ADMIN_DATASTORE_LIST_SNAPSHOTS_RETURN_TYPE,
> +    access: {
> +        permission: &Permission::Anybody,
> +        description: "Requires on /datastore/{store}[/{namespace}] either DATASTORE_AUDIT for any \
> +            or DATASTORE_BACKUP and being the owner of the group",
> +    },
> +)]
> +/// List backup snapshots.
> +pub async fn list_snapshots(
> +    store: String,
> +    ns: Option<BackupNamespace>,
> +    backup_type: Option<BackupType>,
> +    backup_id: Option<String>,
> +    _param: Value,
> +    _info: &ApiMethod,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Vec<SnapshotListItem>, Error> {
> +    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +
> +    tokio::task::spawn_blocking(move || unsafe {
> +        list_snapshots_blocking(store, ns, backup_type, backup_id, auth_id)
> +    })
> +    .await
> +    .map_err(|err| format_err!("failed to await blocking task: {err}"))?
> +}
> +
>  async fn get_snapshots_count(
>      store: &Arc<DataStore>,
>      owner: Option<&Authid>,
> @@ -2764,6 +2898,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
>          "change-owner",
>          &Router::new().post(&API_METHOD_SET_BACKUP_OWNER),
>      ),
> +    ("content", &Router::new().get(&API_METHOD_LIST_CONTENT)),
>      (
>          "download",
>          &Router::new().download(&API_METHOD_DOWNLOAD_FILE),
> -- 
> 2.47.3


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