Re: [PATCH proxmox-backup v6 4/8] api: add PUT endpoint for move_group

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On March 31, 2026 2:34 pm, Hannes Laimer wrote:
> Add a PUT handler on /admin/datastore/{store}/groups to move a single
> backup group to a different namespace within the same datastore. The
> handler performs fast pre-checks synchronously and spawns a worker
> task for the actual move.
> 
> Requires DATASTORE_MODIFY on both the source and target namespaces.
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>  src/api2/admin/datastore.rs | 78 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index cca34055..68b1bbfc 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -69,7 +69,9 @@ 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::{
> +    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};
>  
> @@ -278,6 +280,79 @@ pub async fn delete_group(
>      .await?
>  }
>  
> +#[api(
> +    input: {
> +        properties: {
> +            store: { schema: DATASTORE_SCHEMA },
> +            ns: {
> +                type: BackupNamespace,
> +                optional: true,
> +            },
> +            group: {
> +                type: pbs_api_types::BackupGroup,
> +                flatten: true,
> +            },
> +            "new-ns": {
> +                type: BackupNamespace,
> +                optional: true,

this isn't a new namespace, it's the target (like in the error messages
below) or destination namespace.

> +            },
> +        },
> +    },
> +    returns: {
> +        schema: UPID_SCHEMA,
> +    },
> +    access: {
> +        permission: &Permission::Anybody,
> +        description: "Requires DATASTORE_MODIFY on both the source and target namespace.",

this is a quite high requirement, wouldn't it be enough to be allowed to
delete the group in the source and create it in the target?

i.e.:

source: DATASTORE_MODIFY or DATASTORE_PRUNE+group ownership
target: DATASTORE_MODIFY or DATASTORE_BACKUP+group ownership

> +    },
> +)]
> +/// Move a backup group to a different namespace within the same datastore.
> +pub fn move_group(
> +    store: String,
> +    ns: Option<BackupNamespace>,
> +    group: pbs_api_types::BackupGroup,
> +    new_ns: Option<BackupNamespace>,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> +    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +    let ns = ns.unwrap_or_default();
> +    let new_ns = new_ns.unwrap_or_default();
> +
> +    check_ns_privs(&store, &ns, &auth_id, PRIV_DATASTORE_MODIFY)?;
> +    check_ns_privs(&store, &new_ns, &auth_id, PRIV_DATASTORE_MODIFY)?;
> +
> +    let datastore = DataStore::lookup_datastore(&store, Operation::Write)?;
> +
> +    // Best-effort pre-checks for a fast synchronous error before spawning a worker.
> +    if ns == new_ns {
> +        bail!("source and target namespace must be different");
> +    }
> +    if !datastore.namespace_exists(&new_ns) {
> +        bail!("target namespace '{new_ns}' does not exist");
> +    }
> +    let source_group = datastore.backup_group(ns.clone(), group.clone());
> +    if !source_group.exists() {
> +        bail!("group '{group}' does not exist in namespace '{ns}'");
> +    }
> +    let target_group = datastore.backup_group(new_ns.clone(), group.clone());
> +    if target_group.exists() {
> +        bail!("group '{group}' already exists in target namespace '{new_ns}'");
> +    }
> +
> +    let worker_id = format!("{store}:{ns}:{group}");

should we encode the target NS as well, to allow filtering on both ends?

> +    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> +    let upid_str = WorkerTask::new_thread(
> +        "move-group",
> +        Some(worker_id),
> +        auth_id.to_string(),
> +        to_stdout,
> +        move |_worker| datastore.move_group(&ns, &group, &new_ns),
> +    )?;
> +
> +    Ok(json!(upid_str))
> +}
> +
>  #[api(
>      input: {
>          properties: {
> @@ -2828,6 +2903,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
>          "groups",
>          &Router::new()
>              .get(&API_METHOD_LIST_GROUPS)
> +            .put(&API_METHOD_MOVE_GROUP)

I am not sure this is a good fit.. why not make it a separate endpoint?
either a single one for both move operations, or two endpoints?

the whole (pre-existing) structure here is a bit weird, because it's a
mix and match of datatore level, namespace level, group level and
snapshot level endpoints without an actual hierarchy/tree..

>              .delete(&API_METHOD_DELETE_GROUP),
>      ),
>      ("mount", &Router::new().post(&API_METHOD_MOUNT)),
> -- 
> 2.47.3
> 
> 
> 
> 
> 
>