[PATCH proxmox-backup RFC] cli: reorganize namespace and group commands

[Hannes Laimer <h.laimer@proxmox.com>]

Move 'datastore move-group' on the manager to 'group move' on the
client. Replace 'datastore move-namespace' on the manager with a
'datastore namespace' command group (list/create/delete/move), and add a
'move' subcommand to the client's 'namespace' group.

new CLI layout:
  client:
    group forget
[+] group move
    namespace list
    namespace create
    namespace delete
[+] namespace move

  manager:
[+] datastore namespace list
[+] datastore namespace create
[+] datastore namespace delete
[+] datastore namespace move (moved from `datastore move-namespace`)

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
would be on top of [1]

[1] https://lore.proxmox.com/pbs-devel/20260424112001.206897-1-h.laimer@proxmox.com/T/#u

 docs/storage.rst                            |  18 +-
 proxmox-backup-client/src/group.rs          |  86 ++++++--
 proxmox-backup-client/src/namespace.rs      |  73 ++++++-
 src/bin/proxmox_backup_manager/datastore.rs | 115 +----------
 src/bin/proxmox_backup_manager/mod.rs       |   2 +
 src/bin/proxmox_backup_manager/namespace.rs | 205 ++++++++++++++++++++
 6 files changed, 363 insertions(+), 136 deletions(-)
 create mode 100644 src/bin/proxmox_backup_manager/namespace.rs

diff --git a/docs/storage.rst b/docs/storage.rst
index c2405194..2d901b15 100644
--- a/docs/storage.rst
+++ b/docs/storage.rst
@@ -542,14 +542,14 @@ Backup groups can be moved between namespaces within the same datastore.
 This is useful for reorganizing backup hierarchies without having to
 re-run backups.
 
-A single group can be moved with ``move-group``. To relocate an entire
+A single group can be moved with ``group move``. To relocate an entire
 namespace subtree (including all child namespaces and their groups), use
-``move-namespace``.
+``namespace move``.
 
 .. code-block:: console
 
-  # proxmox-backup-manager datastore move-group <store> --ns <source> --target-ns <target> --backup-type <type> --backup-id <id>
-  # proxmox-backup-manager datastore move-namespace <store> --ns <source> --target-ns <target>
+  # proxmox-backup-client group move <type>/<id> --ns <source> --target-ns <target> --repository <repo>
+  # proxmox-backup-client namespace move <source> --target-ns <target> --repository <repo>
 
 If the target namespace already exists, groups are moved into it. When a
 group with the same type and ID already exists in the target and
@@ -561,16 +561,16 @@ group provided:
 
 Groups that cannot be merged or locked are skipped and reported in the
 task log. They remain at the source and can be retried individually with
-``move-group``.
+``group move``.
 
 .. note::
 
-  With defaults, ``move-namespace`` merges into existing target groups
+  With defaults, ``namespace move`` merges into existing target groups
   (``merge-groups=true``) and removes source namespaces once they are empty
   (``delete-source=true``). Pass ``--merge-groups false`` or
   ``--delete-source false`` to opt out.
 
-Optional parameters for ``move-namespace``:
+Optional parameters for ``namespace move``:
 
 ``merge-groups``
   Allow merging snapshots into groups that already exist in the target
@@ -587,10 +587,10 @@ Optional parameters for ``move-namespace``:
 
 Required privileges:
 
-- ``move-group``: ``DATASTORE_PRUNE`` on the source namespace and
+- ``group move``: ``DATASTORE_PRUNE`` on the source namespace and
   ``DATASTORE_BACKUP`` on the target namespace, plus ownership of the
   backup group; or ``DATASTORE_MODIFY`` on both.
-- ``move-namespace``: ``DATASTORE_MODIFY`` on the parent of both the
+- ``namespace move``: ``DATASTORE_MODIFY`` on the parent of both the
   source and the target namespace.
 
 
diff --git a/proxmox-backup-client/src/group.rs b/proxmox-backup-client/src/group.rs
index 42cb7ab7..a4291aa6 100644
--- a/proxmox-backup-client/src/group.rs
+++ b/proxmox-backup-client/src/group.rs
@@ -1,25 +1,38 @@
 use anyhow::{bail, Error};
 use serde_json::Value;
 
-use proxmox_router::cli::{CliCommand, CliCommandMap, Confirmation};
+use proxmox_router::cli::{
+    extract_output_format, CliCommand, CliCommandMap, Confirmation, OUTPUT_FORMAT,
+};
 use proxmox_schema::api;
 
 use crate::{
     complete_backup_group, complete_namespace, complete_repository, merge_group_into,
-    BackupTargetArgs,
+    optional_ns_param, record_repository, BackupTargetArgs,
 };
-use pbs_api_types::BackupGroup;
+use pbs_api_types::{BackupGroup, BackupNamespace};
 use pbs_client::tools::{connect, remove_repository_from_value};
+use pbs_client::view_task_result;
 
 pub fn group_mgmt_cli() -> CliCommandMap {
-    CliCommandMap::new().insert(
-        "forget",
-        CliCommand::new(&API_METHOD_FORGET_GROUP)
-            .arg_param(&["group"])
-            .completion_cb("ns", complete_namespace)
-            .completion_cb("repository", complete_repository)
-            .completion_cb("group", complete_backup_group),
-    )
+    CliCommandMap::new()
+        .insert(
+            "forget",
+            CliCommand::new(&API_METHOD_FORGET_GROUP)
+                .arg_param(&["group"])
+                .completion_cb("ns", complete_namespace)
+                .completion_cb("repository", complete_repository)
+                .completion_cb("group", complete_backup_group),
+        )
+        .insert(
+            "move",
+            CliCommand::new(&API_METHOD_MOVE_GROUP)
+                .arg_param(&["group"])
+                .completion_cb("ns", complete_namespace)
+                .completion_cb("target-ns", complete_namespace)
+                .completion_cb("repository", complete_repository)
+                .completion_cb("group", complete_backup_group),
+        )
 }
 
 #[api(
@@ -78,3 +91,54 @@ async fn forget_group(group: String, mut param: Value) -> Result<(), Error> {
 
     Ok(())
 }
+
+#[api(
+    input: {
+        properties: {
+            group: {
+                type: String,
+                description: "Backup group.",
+            },
+            repo: {
+                type: BackupTargetArgs,
+                flatten: true,
+            },
+            "target-ns": {
+                type: BackupNamespace,
+            },
+            "merge-group": {
+                type: bool,
+                optional: true,
+                default: true,
+                description: "If the group already exists in the target namespace, merge \
+                    snapshots into it. Requires matching ownership and non-overlapping \
+                    snapshot times.",
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Move a backup group to a different namespace within the same datastore.
+async fn move_group(group: String, mut param: Value) -> Result<(), Error> {
+    let output_format = extract_output_format(&mut param);
+    let backup_group: BackupGroup = group.parse()?;
+    let repo = remove_repository_from_value(&mut param)?;
+    let ns = optional_ns_param(&param)?;
+    if !ns.is_root() {
+        param["ns"] = serde_json::to_value(&ns)?;
+    }
+    merge_group_into(param.as_object_mut().unwrap(), backup_group);
+
+    let client = connect(&repo)?;
+    let path = format!("api2/json/admin/datastore/{}/move-group", repo.store());
+    let result = client.post(&path, Some(param)).await?;
+
+    record_repository(&repo);
+
+    view_task_result(&client, result, &output_format).await?;
+
+    Ok(())
+}
diff --git a/proxmox-backup-client/src/namespace.rs b/proxmox-backup-client/src/namespace.rs
index 15ec085a..26c5cfb5 100644
--- a/proxmox-backup-client/src/namespace.rs
+++ b/proxmox-backup-client/src/namespace.rs
@@ -1,16 +1,18 @@
 use anyhow::{bail, Error};
 use serde_json::{json, Value};
 
-use pbs_client::BackupTargetArgs;
+use pbs_api_types::{BackupNamespace, NS_MAX_DEPTH_SCHEMA};
+use pbs_client::{view_task_result, BackupTargetArgs};
 
 use proxmox_router::cli::{
-    format_and_print_result, get_output_format, CliCommand, CliCommandMap, OUTPUT_FORMAT,
+    extract_output_format, format_and_print_result, get_output_format, CliCommand, CliCommandMap,
+    OUTPUT_FORMAT,
 };
 use proxmox_schema::api;
 
 use crate::{
     complete_namespace, connect, extract_repository_from_value, optional_ns_param,
-    record_repository,
+    record_repository, remove_repository_from_value,
 };
 
 #[api(
@@ -151,6 +153,64 @@ async fn delete_namespace(param: Value, delete_groups: Option<bool>) -> Result<(
     Ok(())
 }
 
+#[api(
+    input: {
+        properties: {
+            repo: {
+                type: BackupTargetArgs,
+                flatten: true,
+            },
+            "target-ns": {
+                type: BackupNamespace,
+            },
+            "max-depth": {
+                schema: NS_MAX_DEPTH_SCHEMA,
+                optional: true,
+            },
+            "delete-source": {
+                type: bool,
+                optional: true,
+                default: true,
+                description: "Remove the source namespace after moving all contents. \
+                    Defaults to true.",
+            },
+            "merge-groups": {
+                type: bool,
+                optional: true,
+                default: true,
+                description: "If a group with the same name already exists in the target \
+                    namespace, merge snapshots into it. Requires matching ownership and \
+                    non-overlapping snapshot times.",
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    },
+)]
+/// Move a backup namespace to a new location within the same datastore.
+async fn move_namespace(mut param: Value) -> Result<(), Error> {
+    let output_format = extract_output_format(&mut param);
+    let source_ns = optional_ns_param(&param)?;
+    if source_ns.is_root() {
+        bail!("root namespace cannot be moved");
+    }
+    let repo = remove_repository_from_value(&mut param)?;
+    // Forward the source ns even if it only came from PBS_NAMESPACE.
+    param["ns"] = serde_json::to_value(&source_ns)?;
+
+    let client = connect(&repo)?;
+    let path = format!("api2/json/admin/datastore/{}/move-namespace", repo.store());
+    let result = client.post(&path, Some(param)).await?;
+
+    record_repository(&repo);
+
+    view_task_result(&client, result, &output_format).await?;
+
+    Ok(())
+}
+
 pub fn cli_map() -> CliCommandMap {
     CliCommandMap::new()
         .insert(
@@ -171,4 +231,11 @@ pub fn cli_map() -> CliCommandMap {
                 .arg_param(&["ns"])
                 .completion_cb("ns", complete_namespace),
         )
+        .insert(
+            "move",
+            CliCommand::new(&API_METHOD_MOVE_NAMESPACE)
+                .arg_param(&["ns"])
+                .completion_cb("ns", complete_namespace)
+                .completion_cb("target-ns", complete_namespace),
+        )
 }
diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs
index 738d0fa5..b547cfac 100644
--- a/src/bin/proxmox_backup_manager/datastore.rs
+++ b/src/bin/proxmox_backup_manager/datastore.rs
@@ -1,6 +1,5 @@
 use pbs_api_types::{
-    BackupNamespace, DataStoreConfig, DataStoreConfigUpdater, DATASTORE_SCHEMA,
-    NS_MAX_DEPTH_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
+    DataStoreConfig, DataStoreConfigUpdater, DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA,
 };
 use pbs_client::view_task_result;
 use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
@@ -324,101 +323,6 @@ async fn uuid_mount(mut param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Resul
     Ok(Value::Null)
 }
 
-#[api(
-    input: {
-        properties: {
-            store: {
-                schema: DATASTORE_SCHEMA,
-            },
-            ns: {
-                type: BackupNamespace,
-            },
-            "target-ns": {
-                type: BackupNamespace,
-            },
-            "max-depth": {
-                schema: NS_MAX_DEPTH_SCHEMA,
-                optional: true,
-            },
-            "delete-source": {
-                type: bool,
-                optional: true,
-                default: true,
-                description: "Remove the source namespace after moving all contents. \
-                    Defaults to true.",
-            },
-            "merge-groups": {
-                type: bool,
-                optional: true,
-                default: true,
-                description: "If a group with the same name already exists in the target \
-                    namespace, merge snapshots into it. Requires matching ownership and \
-                    non-overlapping snapshot times.",
-            },
-            "output-format": {
-                schema: OUTPUT_FORMAT,
-                optional: true,
-            },
-        },
-    },
-)]
-/// Move a backup namespace to a new location within the same datastore.
-async fn cli_move_namespace(store: String, mut param: Value) -> Result<(), Error> {
-    let output_format = extract_output_format(&mut param);
-
-    let client = connect_to_localhost()?;
-    let path = format!("api2/json/admin/datastore/{store}/move-namespace");
-    let result = client.post(&path, Some(param)).await?;
-
-    view_task_result(&client, result, &output_format).await?;
-
-    Ok(())
-}
-
-#[api(
-    input: {
-        properties: {
-            store: {
-                schema: DATASTORE_SCHEMA,
-            },
-            ns: {
-                type: BackupNamespace,
-            },
-            group: {
-                type: pbs_api_types::BackupGroup,
-                flatten: true,
-            },
-            "target-ns": {
-                type: BackupNamespace,
-            },
-            "merge-group": {
-                type: bool,
-                optional: true,
-                default: true,
-                description: "If the group already exists in the target namespace, merge \
-                    snapshots into it. Requires matching ownership and non-overlapping \
-                    snapshot times.",
-            },
-            "output-format": {
-                schema: OUTPUT_FORMAT,
-                optional: true,
-            },
-        },
-    },
-)]
-/// Move a backup group to a different namespace within the same datastore.
-async fn cli_move_group(store: String, mut param: Value) -> Result<(), Error> {
-    let output_format = extract_output_format(&mut param);
-
-    let client = connect_to_localhost()?;
-    let path = format!("api2/json/admin/datastore/{store}/move-group");
-    let result = client.post(&path, Some(param)).await?;
-
-    view_task_result(&client, result, &output_format).await?;
-
-    Ok(())
-}
-
 #[api(
     protected: true,
     input: {
@@ -446,6 +350,7 @@ async fn s3_refresh(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result
 pub fn datastore_commands() -> CommandLineInterface {
     let cmd_def = CliCommandMap::new()
         .insert("list", CliCommand::new(&API_METHOD_LIST_DATASTORES))
+        .insert("namespace", super::namespace_commands())
         .insert(
             "mount",
             CliCommand::new(&API_METHOD_MOUNT_DATASTORE)
@@ -503,22 +408,6 @@ pub fn datastore_commands() -> CommandLineInterface {
             CliCommand::new(&API_METHOD_DELETE_DATASTORE)
                 .arg_param(&["name"])
                 .completion_cb("name", pbs_config::datastore::complete_datastore_name),
-        )
-        .insert(
-            "move-namespace",
-            CliCommand::new(&API_METHOD_CLI_MOVE_NAMESPACE)
-                .arg_param(&["store"])
-                .completion_cb("store", pbs_config::datastore::complete_datastore_name)
-                .completion_cb("ns", crate::complete_sync_local_datastore_namespace)
-                .completion_cb("target-ns", crate::complete_sync_local_datastore_namespace),
-        )
-        .insert(
-            "move-group",
-            CliCommand::new(&API_METHOD_CLI_MOVE_GROUP)
-                .arg_param(&["store"])
-                .completion_cb("store", pbs_config::datastore::complete_datastore_name)
-                .completion_cb("ns", crate::complete_sync_local_datastore_namespace)
-                .completion_cb("target-ns", crate::complete_sync_local_datastore_namespace),
         );
 
     cmd_def.into()
diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
index a9b02604..8204e843 100644
--- a/src/bin/proxmox_backup_manager/mod.rs
+++ b/src/bin/proxmox_backup_manager/mod.rs
@@ -15,6 +15,8 @@ pub use dns::*;
 mod ldap;
 pub use ldap::*;
 pub mod migrate_config;
+mod namespace;
+pub use namespace::*;
 mod network;
 pub use network::*;
 mod node;
diff --git a/src/bin/proxmox_backup_manager/namespace.rs b/src/bin/proxmox_backup_manager/namespace.rs
new file mode 100644
index 00000000..63542514
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/namespace.rs
@@ -0,0 +1,205 @@
+use anyhow::{bail, Error};
+use serde_json::{json, Value};
+
+use pbs_api_types::{BackupNamespace, DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA};
+use pbs_client::view_task_result;
+
+use proxmox_router::cli::*;
+use proxmox_schema::api;
+
+use proxmox_backup::api2;
+use proxmox_backup::client_helpers::connect_to_localhost;
+
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            parent: {
+                type: BackupNamespace,
+                optional: true,
+            },
+            "max-depth": {
+                schema: NS_MAX_DEPTH_SCHEMA,
+                optional: true,
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// List the namespaces of a datastore.
+async fn list_namespaces(
+    store: String,
+    parent: Option<BackupNamespace>,
+    max_depth: Option<usize>,
+    param: Value,
+) -> Result<Value, Error> {
+    let output_format = get_output_format(&param);
+
+    let client = connect_to_localhost()?;
+    let path = format!("api2/json/admin/datastore/{store}/namespace");
+
+    let mut args = json!({});
+    if let Some(parent) = parent {
+        args["parent"] = serde_json::to_value(parent)?;
+    }
+    if let Some(max_depth) = max_depth {
+        args["max-depth"] = max_depth.into();
+    }
+
+    let mut result = client.get(&path, Some(args)).await?;
+    let mut data = result["data"].take();
+    let return_type = &api2::admin::namespace::API_METHOD_LIST_NAMESPACES.returns;
+
+    let options = default_table_format_options()
+        .column(ColumnConfig::new("ns"))
+        .column(ColumnConfig::new("comment"));
+
+    format_and_print_result_full(&mut data, return_type, &output_format, &options);
+
+    Ok(Value::Null)
+}
+
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            ns: { type: BackupNamespace },
+        }
+    }
+)]
+/// Create a new datastore namespace.
+async fn create_namespace(store: String, mut ns: BackupNamespace) -> Result<(), Error> {
+    let name = match ns.pop() {
+        Some(name) => name,
+        None => bail!("root namespace is always present"),
+    };
+
+    let client = connect_to_localhost()?;
+    let path = format!("api2/json/admin/datastore/{store}/namespace");
+    let args = json!({ "parent": ns, "name": name });
+
+    let _result = client.post(&path, Some(args)).await?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            ns: { type: BackupNamespace },
+            "delete-groups": {
+                type: bool,
+                optional: true,
+                default: false,
+                description: "If set, destroy all groups in the hierarchy below and \
+                    including `ns`. If not set, only empty namespaces will be pruned.",
+            },
+        }
+    }
+)]
+/// Delete a backup namespace, optionally including all snapshots.
+async fn delete_namespace(
+    store: String,
+    ns: BackupNamespace,
+    delete_groups: Option<bool>,
+) -> Result<(), Error> {
+    if ns.is_root() {
+        bail!("root namespace cannot be deleted");
+    }
+
+    let client = connect_to_localhost()?;
+    let path = format!("api2/json/admin/datastore/{store}/namespace");
+
+    let mut args = json!({ "ns": ns });
+    if let Some(delete_groups) = delete_groups {
+        args["delete-groups"] = delete_groups.into();
+    }
+
+    let _result = client.delete(&path, Some(args)).await?;
+
+    Ok(())
+}
+
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            ns: { type: BackupNamespace },
+            "target-ns": { type: BackupNamespace },
+            "max-depth": {
+                schema: NS_MAX_DEPTH_SCHEMA,
+                optional: true,
+            },
+            "delete-source": {
+                type: bool,
+                optional: true,
+                default: true,
+                description: "Remove the source namespace after moving all contents. \
+                    Defaults to true.",
+            },
+            "merge-groups": {
+                type: bool,
+                optional: true,
+                default: true,
+                description: "If a group with the same name already exists in the target \
+                    namespace, merge snapshots into it. Requires matching ownership and \
+                    non-overlapping snapshot times.",
+            },
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Move a backup namespace to a new location within the same datastore.
+async fn move_namespace(store: String, mut param: Value) -> Result<(), Error> {
+    let output_format = extract_output_format(&mut param);
+
+    let client = connect_to_localhost()?;
+    let path = format!("api2/json/admin/datastore/{store}/move-namespace");
+    let result = client.post(&path, Some(param)).await?;
+
+    view_task_result(&client, result, &output_format).await?;
+
+    Ok(())
+}
+
+pub fn namespace_commands() -> CommandLineInterface {
+    let cmd_def = CliCommandMap::new()
+        .insert(
+            "list",
+            CliCommand::new(&API_METHOD_LIST_NAMESPACES)
+                .arg_param(&["store"])
+                .completion_cb("store", pbs_config::datastore::complete_datastore_name)
+                .completion_cb("parent", crate::complete_sync_local_datastore_namespace),
+        )
+        .insert(
+            "create",
+            CliCommand::new(&API_METHOD_CREATE_NAMESPACE)
+                .arg_param(&["store", "ns"])
+                .completion_cb("store", pbs_config::datastore::complete_datastore_name)
+                .completion_cb("ns", crate::complete_sync_local_datastore_namespace),
+        )
+        .insert(
+            "delete",
+            CliCommand::new(&API_METHOD_DELETE_NAMESPACE)
+                .arg_param(&["store", "ns"])
+                .completion_cb("store", pbs_config::datastore::complete_datastore_name)
+                .completion_cb("ns", crate::complete_sync_local_datastore_namespace),
+        )
+        .insert(
+            "move",
+            CliCommand::new(&API_METHOD_MOVE_NAMESPACE)
+                .arg_param(&["store", "ns", "target-ns"])
+                .completion_cb("store", pbs_config::datastore::complete_datastore_name)
+                .completion_cb("ns", crate::complete_sync_local_datastore_namespace)
+                .completion_cb("target-ns", crate::complete_sync_local_datastore_namespace),
+        );
+
+    cmd_def.into()
+}
-- 
2.47.3