Re: [PATCH proxmox-backup v4 5/7] api: add PUT endpoint for move_namespace

[Christian Ebner <c.ebner@proxmox.com>]

One note inline.

On 3/11/26 4:13 PM, Hannes Laimer wrote:
> Requires DATASTORE_MODIFY on the parent of both the source and
> destination namespaces, matching the permissions used by
> create_namespace() and delete_namespace().
> 
> Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
> ---
>   src/api2/admin/namespace.rs | 78 ++++++++++++++++++++++++++++++++++++-
>   1 file changed, 76 insertions(+), 2 deletions(-)
> 
> diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
> index ec913001..522133b1 100644
> --- a/src/api2/admin/namespace.rs
> +++ b/src/api2/admin/namespace.rs
> @@ -1,12 +1,16 @@
>   use anyhow::{bail, Context, Error};
>   
>   use pbs_config::CachedUserInfo;
> -use proxmox_router::{http_bail, ApiMethod, Permission, Router, RpcEnvironment};
> +use proxmox_rest_server::WorkerTask;
> +use proxmox_router::{
> +    http_bail, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType,
> +};
>   use proxmox_schema::*;
> +use serde_json::{json, Value};
>   
>   use pbs_api_types::{
>       Authid, BackupGroupDeleteStats, BackupNamespace, NamespaceListItem, Operation,
> -    DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PROXMOX_SAFE_ID_FORMAT,
> +    DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PROXMOX_SAFE_ID_FORMAT, UPID_SCHEMA,
>   };
>   
>   use pbs_datastore::DataStore;
> @@ -193,7 +197,77 @@ pub fn delete_namespace(
>       Ok(stats)
>   }
>   
> +#[api(
> +    input: {
> +        properties: {
> +            store: { schema: DATASTORE_SCHEMA },
> +            ns: {
> +                type: BackupNamespace,
> +            },
> +            "new-ns": {
> +                type: BackupNamespace,
> +            },
> +        },
> +    },
> +    returns: {
> +        schema: UPID_SCHEMA,
> +    },
> +    access: {
> +        permission: &Permission::Anybody,
> +        description: "Requires DATASTORE_MODIFY on the parent of 'ns' and on the parent of 'new-ns'.",
> +    },
> +)]
> +/// Move a backup namespace (including all child namespaces and groups) to a new location.
> +pub fn move_namespace(
> +    store: String,
> +    ns: BackupNamespace,
> +    new_ns: BackupNamespace,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Value, Error> {
> +    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
> +
> +    check_ns_modification_privs(&store, &ns, &auth_id)?;
> +    check_ns_modification_privs(&store, &new_ns, &auth_id)?;
> +
> +    let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;

Same note as for the previous patch, the operation parameter is now 
non-optional and needs adaption.

> +
> +    // Best-effort pre-checks for a fast synchronous error before spawning a worker.
> +    if ns.is_root() {
> +        bail!("cannot move root namespace");
> +    }
> +    if ns == new_ns {
> +        bail!("source and target namespace must be different");
> +    }
> +    if !datastore.namespace_exists(&ns) {
> +        bail!("source namespace '{ns}' does not exist");
> +    }
> +    if datastore.namespace_exists(&new_ns) {
> +        bail!("target namespace '{new_ns}' already exists");
> +    }
> +    let target_parent = new_ns.parent();
> +    if !datastore.namespace_exists(&target_parent) {
> +        bail!("target parent namespace '{target_parent}' does not exist");
> +    }
> +    if ns.contains(&new_ns).is_some() {
> +        bail!("cannot move namespace '{ns}' into its own subtree (target: '{new_ns}')");
> +    }
> +
> +    let worker_id = format!("{store}:{ns}");
> +    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
> +
> +    let upid_str = WorkerTask::new_thread(
> +        "move-namespace",
> +        Some(worker_id),
> +        auth_id.to_string(),
> +        to_stdout,
> +        move |_worker| datastore.move_namespace(&ns, &new_ns),
> +    )?;
> +
> +    Ok(json!(upid_str))
> +}
> +
>   pub const ROUTER: Router = Router::new()
>       .get(&API_METHOD_LIST_NAMESPACES)
>       .post(&API_METHOD_CREATE_NAMESPACE)
> +    .put(&API_METHOD_MOVE_NAMESPACE)
>       .delete(&API_METHOD_DELETE_NAMESPACE);