[pbs-devel] [PATCH proxmox{, -backup} v7 0/2] close #4763: client: add command to forget backup group

[pbs-devel] [PATCH proxmox{, -backup} v7 0/2] close #4763: client: add command to forget backup group

[Gabriel Goller]

Add the command `proxmox-backup-client group forget <group>` so
that we can forget (delete) whole groups with all the containing
snapshots. As this is quite dangerous, we added a prompt, so the
user has to confirm the operation.

v7, thanks @Wolfgang:
 - style nit: return unit type on success in api fn
 - let `Confirmation` impl Clone, Copy, and Eq as well
 - with Copy we can let the fn's consume self
 - organize imports

v6, thanks @Thomas:
 - add more generic confirmation helper drawn up by @Thomas
 - stricter yes/no matching with default

v5, thanks @Christian:
 - stricter input matching ($ in regex)
 - avoid printing full datastore path

v4:
 - removed localization

--version 3 has been skipped--

v2:
 - added localization

proxmox:

Gabriel Goller (1):
  router: cli: add confirmation helper

 proxmox-router/Cargo.toml     |   1 +
 proxmox-router/src/cli/mod.rs | 114 +++++++++++++++++++++++++++++++++-
 2 files changed, 114 insertions(+), 1 deletion(-)


proxmox-backup:

Gabriel Goller (1):
  close #4763: client: add command to forget backup group

 proxmox-backup-client/src/group.rs | 88 ++++++++++++++++++++++++++++++
 proxmox-backup-client/src/main.rs  |  3 +
 2 files changed, 91 insertions(+)
 create mode 100644 proxmox-backup-client/src/group.rs


Summary over all repositories:
  4 files changed, 205 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.5.0


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

[pbs-devel] [PATCH proxmox v7 1/2] router: cli: add confirmation helper

[Gabriel Goller]

Add confirmation helper that outputs a prompt and lets the user
confirm or deny it.
Implemented to close #4763.

Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-router/Cargo.toml     |   1 +
 proxmox-router/src/cli/mod.rs | 114 +++++++++++++++++++++++++++++++++-
 2 files changed, 114 insertions(+), 1 deletion(-)

diff --git a/proxmox-router/Cargo.toml b/proxmox-router/Cargo.toml
index dcd71a40..0b9d361f 100644
--- a/proxmox-router/Cargo.toml
+++ b/proxmox-router/Cargo.toml
@@ -19,6 +19,7 @@ percent-encoding.workspace = true
 serde_json.workspace = true
 serde.workspace = true
 unicode-width ="0.1.8"
+regex.workspace = true
 
 # cli:
 tokio = { workspace = true, features = [], optional = true }
diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs
index 7df94ad9..78f08920 100644
--- a/proxmox-router/src/cli/mod.rs
+++ b/proxmox-router/src/cli/mod.rs
@@ -12,7 +12,12 @@
 //! - Ability to create interactive commands (using ``rustyline``)
 //! - Supports complex/nested commands
 
-use std::collections::HashMap;
+use std::{
+    collections::HashMap,
+    io::{self, Write},
+};
+
+use anyhow::{bail, Error};
 
 use crate::ApiMethod;
 
@@ -61,6 +66,113 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) {
     .init();
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+/// Use for simple yes or no questions, where booleans can be confusing, especially if there's a
+/// default response to consider. The implementation provides query helper for the CLI.
+pub enum Confirmation {
+    Yes,
+    No,
+}
+
+impl Confirmation {
+    /// Get the formatted choice for the query prompt, with self being the highlighted (default)
+    /// one displayed as upper case.
+    pub fn default_choice_str(self) -> &'static str {
+        match self {
+            Self::Yes => "Y/n",
+            Self::No => "y/N",
+        }
+    }
+
+    /// Returns true if the answer is Yes
+    pub fn is_yes(self) -> bool {
+        self == Self::Yes
+    }
+
+    /// Returns true if the answer is No
+    pub fn is_no(self) -> bool {
+        self == Self::No
+    }
+
+    /// Parse an input string reference as yes or no confirmation.
+    ///
+    /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y',
+    /// 'n', or 'N'. You must trim the string before calling, if needed, or use one of the query
+    /// helper functions.
+    ///
+    /// ```
+    /// use proxmox_router::cli::Confirmation;
+    ///
+    /// let answer = Confirmation::from_str("y");
+    /// assert!(answer.expect("valid").is_yes());
+    ///
+    /// let answer = Confirmation::from_str("N");
+    /// assert!(answer.expect("valid").is_no());
+    ///
+    /// let answer = Confirmation::from_str("bogus");
+    /// assert!(answer.is_err());
+    /// ```
+    pub fn from_str(input: &str) -> Result<Self, Error> {
+        match input.trim() {
+            "y" | "Y" => Ok(Self::Yes),
+            "n" | "N" => Ok(Self::No),
+            _ => bail!("unexpected choice '{input}'! Use 'y' or 'n'"),
+        }
+    }
+
+    /// Parse a input string reference as yes or no confirmation, allowing a fallback default
+    /// answer if the user enters an empty choice.
+    ///
+    /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y',
+    /// 'n', or 'N'. The empty string maps to the default. You must trim the string before calling,
+    /// if needed, or use one of the query helper functions.
+    ///
+    /// ```
+    /// use proxmox_router::cli::Confirmation;
+    ///
+    /// let answer = Confirmation::from_str_with_default("", Confirmation::No);
+    /// assert!(answer.expect("valid").is_no());
+    ///
+    /// let answer = Confirmation::from_str_with_default("n", Confirmation::Yes);
+    /// assert!(answer.expect("valid").is_no());
+    ///
+    /// let answer = Confirmation::from_str_with_default("yes", Confirmation::Yes);
+    /// assert!(answer.is_err()); // full-word answer not allowed for now.
+    /// ```
+    pub fn from_str_with_default(input: &str, default: Self) -> Result<Self, Error> {
+        match input.trim() {
+            "y" | "Y" => Ok(Self::Yes),
+            "n" | "N" => Ok(Self::No),
+            "" => Ok(default),
+            _ => bail!("unexpected choice '{input}'! Use enter for default or use 'y' or 'n'"),
+        }
+    }
+
+    /// Print a query prompt with available yes no choices and returns the String the user enters.
+    fn read_line(query: &str, choices: &str) -> Result<String, io::Error> {
+        print!("{query} [{choices}]: ");
+
+        io::stdout().flush()?;
+        let stdin = io::stdin();
+        let mut line = String::new();
+        stdin.read_line(&mut line)?;
+        Ok(line)
+    }
+
+    /// Print a query prompt and parse the white-space trimmed answer using `from_str`.
+    pub fn query(query: &str) -> Result<Self, Error> {
+        let line = Self::read_line(query, "y/n")?;
+        Confirmation::from_str(line.trim())
+    }
+
+    /// Print a query prompt and parse the answer using `from_str_with_default`, falling back to the
+    /// default_answer if the user provided an empty string.
+    pub fn query_with_default(query: &str, default_answer: Self) -> Result<Self, Error> {
+        let line = Self::read_line(query, default_answer.default_choice_str())?;
+        Confirmation::from_str_with_default(line.trim(), default_answer)
+    }
+}
+
 /// Define a simple CLI command.
 pub struct CliCommand {
     /// The Schema definition.
-- 
2.43.0



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

[pbs-devel] [PATCH proxmox-backup v7 2/2] close #4763: client: add command to forget backup group

[Gabriel Goller]

Add the command `proxmox-backup-client group forget <group>` so
that we can forget (delete) whole groups with all the containing
snapshots.
To avoid printing full datastore paths (which are in the error messages)
we filter out the most common one (group not found) and rephrase it.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-backup-client/src/group.rs | 88 ++++++++++++++++++++++++++++++
 proxmox-backup-client/src/main.rs  |  3 +
 2 files changed, 91 insertions(+)
 create mode 100644 proxmox-backup-client/src/group.rs

diff --git a/proxmox-backup-client/src/group.rs b/proxmox-backup-client/src/group.rs
new file mode 100644
index 00000000..4b572b26
--- /dev/null
+++ b/proxmox-backup-client/src/group.rs
@@ -0,0 +1,88 @@
+use anyhow::{bail, Error};
+use serde_json::Value;
+
+use proxmox_router::cli::{CliCommand, CliCommandMap, Confirmation};
+use proxmox_schema::api;
+
+use crate::{
+    complete_backup_group, complete_namespace, complete_repository, merge_group_into,
+    REPO_URL_SCHEMA,
+};
+use pbs_api_types::{BackupGroup, BackupNamespace};
+use pbs_client::tools::{connect, extract_repository_from_value};
+
+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),
+    )
+}
+
+#[api(
+    input: {
+        properties: {
+            group: {
+                type: String,
+                description: "Backup group",
+            },
+            repository: {
+                schema: REPO_URL_SCHEMA,
+                optional: true,
+            },
+            ns: {
+                type: BackupNamespace,
+                optional: true,
+            },
+        }
+    }
+)]
+/// Forget (remove) backup snapshots.
+async fn forget_group(group: String, param: Value) -> Result<(), Error> {
+    let backup_group: BackupGroup = group.parse()?;
+    let repo = extract_repository_from_value(&param)?;
+    let client = connect(&repo)?;
+
+    let mut api_params = param;
+    merge_group_into(api_params.as_object_mut().unwrap(), backup_group.clone());
+
+    let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store());
+    let result = client.get(&path, Some(api_params.clone())).await?;
+    let snapshots = result["data"].as_array().unwrap().len();
+
+    let confirmation = Confirmation::query_with_default(
+        format!(
+            "Delete group \"{}\" with {} snapshot(s)?",
+            backup_group, snapshots
+        )
+        .as_str(),
+        Confirmation::No,
+    )?;
+    if confirmation.is_yes() {
+        let path = format!("api2/json/admin/datastore/{}/groups", repo.store());
+        if let Err(err) = client.delete(&path, Some(api_params)).await {
+            // "ENOENT: No such file or directory" is part of the error returned when the group
+            // has not been found. The full error contains the full datastore path and we would
+            // like to avoid printing that to the console. Checking if it exists before deleting
+            // the group doesn't work because we currently do not differentiate between an empty
+            // and a nonexistent group. This would make it impossible to remove empty groups.
+            if err
+                .root_cause()
+                .to_string()
+                .contains("ENOENT: No such file or directory")
+            {
+                bail!("Unable to find backup group!");
+            } else {
+                bail!(err);
+            }
+        }
+        println!("Successfully deleted group!");
+    } else {
+        println!("Abort.");
+    }
+
+    Ok(())
+}
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 32fe914c..45f1be5f 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -72,6 +72,8 @@ mod catalog;
 pub use catalog::*;
 mod snapshot;
 pub use snapshot::*;
+mod group;
+pub use group::*;
 pub mod key;
 pub mod namespace;
 
@@ -1791,6 +1793,7 @@ fn main() {
         .insert("benchmark", benchmark_cmd_def)
         .insert("change-owner", change_owner_cmd_def)
         .insert("namespace", namespace::cli_map())
+        .insert("group", group_mgmt_cli())
         .alias(&["files"], &["snapshot", "files"])
         .alias(&["forget"], &["snapshot", "forget"])
         .alias(&["upload-log"], &["snapshot", "upload-log"])
-- 
2.43.0



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

[pbs-devel] applied-series: [PATCH proxmox{, -backup} v7 0/2] close #4763: client: add command to forget backup group

[Wolfgang Bumiller]

applied this but replaced the `extract_repository_from_value` call with
a newly added `remove_repository_from_value` call since this code
actually passed the `--repository` parameter on to the remote side,
which does not accept unknown parameters anymore

On Mon, Jun 03, 2024 at 10:43:09AM GMT, Gabriel Goller wrote:
> Add the command `proxmox-backup-client group forget <group>` so
> that we can forget (delete) whole groups with all the containing
> snapshots. As this is quite dangerous, we added a prompt, so the
> user has to confirm the operation.
> 
> v7, thanks @Wolfgang:
>  - style nit: return unit type on success in api fn
>  - let `Confirmation` impl Clone, Copy, and Eq as well
>  - with Copy we can let the fn's consume self
>  - organize imports
> 
> v6, thanks @Thomas:
>  - add more generic confirmation helper drawn up by @Thomas
>  - stricter yes/no matching with default
> 
> v5, thanks @Christian:
>  - stricter input matching ($ in regex)
>  - avoid printing full datastore path
> 
> v4:
>  - removed localization
> 
> --version 3 has been skipped--
> 
> v2:
>  - added localization
> 
> proxmox:
> 
> Gabriel Goller (1):
>   router: cli: add confirmation helper
> 
>  proxmox-router/Cargo.toml     |   1 +
>  proxmox-router/src/cli/mod.rs | 114 +++++++++++++++++++++++++++++++++-
>  2 files changed, 114 insertions(+), 1 deletion(-)
> 
> 
> proxmox-backup:
> 
> Gabriel Goller (1):
>   close #4763: client: add command to forget backup group
> 
>  proxmox-backup-client/src/group.rs | 88 ++++++++++++++++++++++++++++++
>  proxmox-backup-client/src/main.rs  |  3 +
>  2 files changed, 91 insertions(+)
>  create mode 100644 proxmox-backup-client/src/group.rs
> 
> 
> Summary over all repositories:
>   4 files changed, 205 insertions(+), 1 deletions(-)
> 
> -- 
> Generated by git-murpp 0.5.0


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