Re: [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 13.09.21 16:18, Dominik Csapak wrote:
> similar to pve/pmg, a user can call the api with this utility without
> going through the proxy/daemon, as well as list the api endpoints
> (with child links) and get the api description of endpoints
> 
> this is mainly intended for debugging, but it is also useful for

good point, can we move it into proxmox-backup-debug as "shell" sub-command
then?

It'd make it harder (or more pointless) to let the proxmox-backup-debug binary
lives in its own crate, but as debug CLI tool I'd figure that it's cursed to
accumulate a lot of pbs dependecies in the long run any how.

And I know the name is derived from pve, but it is not exactly a shell in the
sense of a REPL or the like; In PVE it originally was, that explains the name,
but we deprecated that functionality there due to its burden on a bigger
refactoring to CLIHandler, IIRC.

maybe "api-inspect" or just "api" as subcommand could be descriptive
alternatives now?



> situations where some api calls do not have an equivalent in a binary
> and a user does not want to go through the api
> 
> not implemented are the http2 api calls (since it is a separate api and
> it wouldn't be that easy to do)
> 
> there are a few quirks though, related to the 'ls' command:
> i extract the 'child-link' from the property name of the
> 'match_all' statement of the router, but this does not
> always match with the property from the relevant 'get' api call
> so it fails there (e.g. /tape/drive )
> 
> this can be fixed in the respective api calls (e.g. by renaming
> the parameter that comes from the path)
> 
> includes bash/zsh completion helpers and a basic manpage

nice, a docs patch would be nice too; which remainds me that I'm still
missing docs patches from Hannes for the debug CLI in genereal ^^

some more comments inline, but I did not give everthing a thorough look
yet, so those are rather some random things that got my attention from
skimming over the code to check some things.

> diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
> index 9aaf6f1a..e422a974 100644
> --- a/src/api2/node/tasks.rs
> +++ b/src/api2/node/tasks.rs
> @@ -258,7 +258,7 @@ fn extract_upid(param: &Value) -> Result<UPID, Error> {
>      },
>  )]
>  /// Read task log.
> -async fn read_task_log(
> +pub async fn read_task_log(
>      param: Value,
>      mut rpcenv: &mut dyn RpcEnvironment,
>  ) -> Result<Value, Error> {
> diff --git a/src/bin/pbs-shell.rs b/src/bin/pbs-shell.rs
> new file mode 100644
> index 00000000..a9f5ad29
> --- /dev/null
> +++ b/src/bin/pbs-shell.rs
> @@ -0,0 +1,528 @@
> +use anyhow::{bail, format_err, Error};
> +use hyper::Method;
> +use serde::{Deserialize, Serialize};
> +use serde_json::Value;
> +use tokio::signal::unix::{signal, SignalKind};
> +use futures::FutureExt;
> +
> +use std::collections::HashMap;
> +
> +use proxmox::api::{
> +    api,
> +    cli::*,
> +    format::DocumentationFormat,
> +    schema::{parse_parameter_strings, ApiType, ParameterSchema, Schema},
> +    ApiHandler, ApiMethod, RpcEnvironment, SubRoute,
> +};
> +
> +use pbs_api_types::{UPID, PROXMOX_UPID_REGEX};
> +
> +const PROG_NAME: &str = "pbs-shell";
> +
> +fn complete_api_path(complete_me: &str, _map: &HashMap<String, String>) -> Vec<String> {
> +    pbs_runtime::main(async { complete_api_path_do(complete_me, None).await })

1. do we really want to spawn the multi thread runtime for every completion?
2. why not define a macro that expands to a fn and the respective None/Some("?")
   and use that directly below in main when defining the commands?

> +}
> +
> +fn complete_api_path_get(complete_me: &str, _map: &HashMap<String, String>) -> Vec<String> {
> +    pbs_runtime::main(async { complete_api_path_do(complete_me, Some("r")).await })
> +}
> +
> +fn complete_api_path_set(complete_me: &str, _map: &HashMap<String, String>) -> Vec<String> {
> +    pbs_runtime::main(async { complete_api_path_do(complete_me, Some("w")).await })
> +}
> +
> +fn complete_api_path_create(complete_me: &str, _map: &HashMap<String, String>) -> Vec<String> {
> +    pbs_runtime::main(async { complete_api_path_do(complete_me, Some("c")).await })
> +}
> +
> +fn complete_api_path_delete(complete_me: &str, _map: &HashMap<String, String>) -> Vec<String> {
> +    pbs_runtime::main(async { complete_api_path_do(complete_me, Some("d")).await })
> +}
> +
> +fn complete_api_path_ls(complete_me: &str, _map: &HashMap<String, String>) -> Vec<String> {
> +    pbs_runtime::main(async { complete_api_path_do(complete_me, Some("D")).await })
> +}
> +
> +async fn complete_api_path_do(mut complete_me: &str, capability: Option<&str>) -> Vec<String> {
> +    if complete_me.is_empty() {
> +        complete_me = "/";
> +    }
> +
> +    let mut list = Vec::new();
> +
> +    let mut lookup_path = complete_me.to_string();
> +    let mut filter = "";
> +    let last_path_index = complete_me.rfind('/');
> +    if let Some(index) = last_path_index {
> +        if index != complete_me.len() - 1 {
> +            lookup_path = complete_me[..(index + 1)].to_string();
> +            if index < complete_me.len() - 1 {
> +                filter = &complete_me[(index + 1)..];
> +            }
> +        }
> +    }
> +
> +    let uid = nix::unistd::Uid::current();
> +
> +    let username = match nix::unistd::User::from_uid(uid) {
> +        Ok(Some(user)) => user.name,
> +        _ => "root@pam".to_string(),
> +    };
> +    let mut rpcenv = CliEnvironment::new();
> +    rpcenv.set_auth_id(Some(format!("{}@pam", username)));
> +
> +    while let Ok(children) = get_api_children(lookup_path.clone(), &mut rpcenv).await {
> +        let old_len = list.len();
> +        for entry in children {
> +            let name = entry.name;
> +            let caps = entry.capabilities;
> +
> +            if filter.is_empty() || name.starts_with(filter) {
> +                let mut path = format!("{}{}", lookup_path, name);
> +                if caps.contains('D') {
> +                    path.push('/');
> +                    list.push(path.clone());
> +                } else if let Some(cap) = capability {
> +                    if caps.contains(cap) {
> +                        list.push(path);
> +                    }
> +                } else {
> +                    list.push(path);
> +                }
> +            }
> +        }
> +
> +        if list.len() == 1 && old_len != 1 && list[0].ends_with('/') {
> +            // we added only one match and it was a directory, lookup again
> +            lookup_path = list[0].clone();
> +            filter = "";
> +            continue;
> +        }
> +
> +        break;
> +    }
> +
> +    list
> +}
> +
> +async fn get_child_links(
> +    path: &str,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Vec<String>, Error> {
> +    let mut uri_param = HashMap::new();
> +    let (path, components) = proxmox_backup::tools::normalize_uri_path(&path)?;
> +
> +    let info = &proxmox_backup::api2::ROUTER
> +        .find_route(&components, &mut uri_param)
> +        .ok_or_else(|| format_err!("no such resource"))?;
> +
> +    match info.subroute {
> +        Some(SubRoute::Map(map)) => Ok(map.iter().map(|(name, _)| name.to_string()).collect()),
> +        Some(SubRoute::MatchAll { param_name, .. }) => {
> +            let get_call = info.get.ok_or_else(|| format_err!("no such resource"))?;
> +            let list = call_api(get_call, rpcenv, serde_json::to_value(uri_param)?).await?;
> +            Ok(list
> +                .as_array()
> +                .ok_or_else(|| format_err!("{} did not return an array", path))?
> +                .iter()
> +                .map(|item| {
> +                    item[param_name]
> +                        .as_str()
> +                        .map(|c| c.to_string())
> +                        .ok_or_else(|| format_err!("no such property {}", param_name))
> +                })
> +                .collect::<Result<Vec<_>, _>>()?)
> +        }
> +        None => bail!("link does not define child links"),
> +    }
> +}
> +
> +fn get_api_method(
> +    method: Method,
> +    path: &str,
> +) -> Result<(&'static ApiMethod, HashMap<String, String>), Error> {
> +    let mut uri_param = HashMap::new();
> +    let (path, components) = proxmox_backup::tools::normalize_uri_path(&path)?;
> +    if let Some(method) =
> +        &proxmox_backup::api2::ROUTER.find_method(&components, method.clone(), &mut uri_param)
> +    {
> +        Ok((method, uri_param))
> +    } else {
> +        bail!("no {} handler defined for '{}'", method, path);
> +    }
> +}
> +
> +fn merge_parameters(
> +    uri_param: HashMap<String, String>,
> +    param: Value,
> +    schema: ParameterSchema,
> +) -> Result<Value, Error> {
> +    let mut param_list: Vec<(String, String)> = vec![];
> +
> +    for (k, v) in uri_param {
> +        param_list.push((k.clone(), v.clone()));
> +    }
> +
> +    if let Some(map) = param.as_object() {
> +        for (k, v) in map {
> +            param_list.push((k.clone(), v.as_str().unwrap().to_string()));
> +        }
> +    }
> +
> +    let params = parse_parameter_strings(&param_list, schema, true)?;
> +
> +    Ok(params)
> +}
> +
> +async fn call_api(
> +    method: &'static ApiMethod,
> +    rpcenv: &mut dyn RpcEnvironment,
> +    params: Value,
> +) -> Result<Value, Error> {
> +    match method.handler {
> +        ApiHandler::AsyncHttp(_handler) => {
> +            bail!("not implemented");
> +        }
> +        ApiHandler::Sync(handler) => (handler)(params, method, rpcenv),
> +        ApiHandler::Async(handler) => (handler)(params, method, rpcenv).await,
> +    }
> +}
> +
> +async fn handle_worker(upid_str: &str) -> Result<(), Error> {
> +
> +    let upid: UPID = upid_str.parse()?;
> +    let mut signal_stream = signal(SignalKind::interrupt())?;
> +    let abort_future = async move {
> +        while signal_stream.recv().await.is_some() {
> +            println!("got shutdown request (SIGINT)");
> +            proxmox_backup::server::abort_local_worker(upid.clone());
> +        }
> +        Ok::<_, Error>(())
> +    };
> +
> +    let result_future = proxmox_backup::server::wait_for_local_worker(upid_str);
> +
> +    futures::select!{
> +        result = result_future.fuse() => result?,
> +        abort = abort_future.fuse() => abort?,
> +    };
> +
> +    Ok(())
> +}
> +
> +async fn call_api_and_format_result(
> +    method: Method,
> +    path: String,
> +    mut param: Value,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<(), Error> {
> +    let mut output_format = extract_output_format(&mut param);
> +    let (method, uri_param) = get_api_method(method, &path)?;
> +    let params = merge_parameters(uri_param, param, method.parameters)?;
> +
> +    let mut result = call_api(method, rpcenv, params).await?;
> +
> +    if let Some(upid) = result.as_str() {
> +        if PROXMOX_UPID_REGEX.is_match(upid) {
> +            handle_worker(upid).await?;
> +
> +            if output_format == "text" {
> +                return Ok(());
> +            }
> +        }
> +    }
> +
> +    let options = default_table_format_options();
> +    let return_type = &method.returns;
> +    if matches!(return_type.schema, Schema::Null) {
> +        output_format = "json-pretty".to_string();
> +    }
> +
> +    format_and_print_result_full(&mut result, return_type, &output_format, &options);
> +
> +    Ok(())
> +}
> +
> +#[api(
> +    input: {
> +        additional_properties: true,
> +        properties: {
> +            "api-path": {
> +                type: String,
> +                description: "API path.",
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        },
> +    },
> +)]
> +/// Call API PUT on <api-path>
> +async fn set(api_path: String, param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    call_api_and_format_result(Method::PUT, api_path, param, rpcenv).await
> +}
> +
> +#[api(
> +    input: {
> +        additional_properties: true,
> +        properties: {
> +            "api-path": {
> +                type: String,
> +                description: "API path.",
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        },
> +    },
> +)]
> +/// Call API POST on <api-path>
> +async fn create(api_path: String, param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    call_api_and_format_result(Method::POST, api_path, param, rpcenv).await
> +}
> +
> +#[api(
> +    input: {
> +        additional_properties: true,
> +        properties: {
> +            "api-path": {
> +                type: String,
> +                description: "API path.",
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        },
> +    },
> +)]
> +/// Call API GET on <api-path>
> +async fn get(api_path: String, param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    call_api_and_format_result(Method::GET, api_path, param, rpcenv).await
> +}
> +
> +#[api(
> +    input: {
> +        additional_properties: true,
> +        properties: {
> +            "api-path": {
> +                type: String,
> +                description: "API path.",
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        },
> +    },
> +)]
> +/// Call API DELETE on <api-path>
> +async fn delete(api_path: String, param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    call_api_and_format_result(Method::DELETE, api_path, param, rpcenv).await
> +}

Would man page generation get borked if you'd use just a single #api def here for
the CLI sub-commands and passed the method through the CLIHandler's .fixed_param()
mechanism? The value must be a String, but the `match *command` from usage fn could
be factored out and resued here. It'd reduce implementation size quite a bit, so if
doc generation could be made to still work out it'd be nice to do.

> +
> +#[api(
> +    input: {
> +        properties: {
> +            path: {
> +                type: String,
> +                description: "API path.",
> +            },
> +            verbose: {
> +                type: Boolean,
> +                description: "Verbose output format.",
> +                optional: true,
> +                default: false,
> +            }
> +        },
> +    },
> +)]
> +/// Get API usage information for <path>
> +async fn usage(
> +    path: String,
> +    verbose: bool,
> +    _param: Value,
> +    _rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<(), Error> {
> +    let docformat = if verbose {
> +        DocumentationFormat::Full
> +    } else {
> +        DocumentationFormat::Short
> +    };
> +    let mut found = false;
> +    for command in  &["get", "set", "create", "delete"] {
> +        let http_method = match *command {
> +            "get" => Method::GET,
> +            "set" => Method::PUT,
> +            "create" => Method::POST,
> +            "delete" => Method::DELETE,
> +            _ => unreachable!(),
> +        };
> +        let (info, uri_params) = match get_api_method(http_method, &path) {
> +            Ok(some) => some,
> +            Err(_) => continue,
> +        };
> +        found = true;
> +
> +        let skip_params: Vec<&str> = uri_params.keys().map(|s| &**s).collect();
> +
> +        let cmd = CliCommand::new(info);
> +        let prefix = format!("USAGE: {} {} {}", PROG_NAME, command, path);
> +
> +        print!(
> +            "{}",
> +            generate_usage_str(&prefix, &cmd, docformat, "", &skip_params)
> +        );
> +    }
> +
> +    if !found {
> +        bail!("no such resource '{}'", path);
> +    }
> +    Ok(())
> +}
> +
> +#[api()]
> +#[derive(Debug, Serialize, Deserialize)]
> +/// A child link with capabilities
> +struct ApiDirEntry {
> +    /// The name of the link
> +    name: String,
> +    /// The capabilities of the path (format Drwcd)
> +    capabilities: String,

why not have a real type with directoy and method-set as struct members and the
display trait implemented? not too hard feelings for that, but this just reads so
perlish to me.. ^^

> +}
> +
> +const LS_SCHEMA: &proxmox::api::schema::Schema =
> +    &proxmox::api::schema::ArraySchema::new("List of child links", &ApiDirEntry::API_SCHEMA)
> +        .schema();
> +
> +async fn get_api_children(
> +    path: String,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<Vec<ApiDirEntry>, Error> {
> +    let mut res = Vec::new();
> +    for link in get_child_links(&path, rpcenv).await? {
> +        let path = format!("{}/{}", path, link);
> +        let (path, _) = proxmox_backup::tools::normalize_uri_path(&path)?;
> +        let mut cap = String::new();
> +
> +        if get_child_links(&path, rpcenv).await.is_ok() {
> +            cap.push('D');
> +        } else {
> +            cap.push('-');
> +        }
> +
> +        let cap_list = &[
> +            (Method::GET,    'r'),
> +            (Method::PUT,    'w'),
> +            (Method::POST,   'c'),
> +            (Method::DELETE, 'd'),
> +        ];
> +
> +        for (method, c) in cap_list {
> +            if get_api_method(method.clone(), &path).is_ok() {

method is cloned here and in get_api_method, why not make it a reference and borrow?

> +                cap.push(*c);
> +            } else {
> +                cap.push('-');
> +            }
> +        }
> +
> +        res.push(ApiDirEntry {
> +            name: link.to_string(),
> +            capabilities: cap,
> +        });
> +    }
> +
> +    Ok(res)
> +}
> +
> +#[api(
> +    input: {
> +        properties: {
> +            path: {
> +                type: String,
> +                description: "API path.",
> +            },
> +            "output-format": {
> +                schema: OUTPUT_FORMAT,
> +                optional: true,
> +            },
> +        },
> +    },
> +)]
> +/// Get API usage information for <path>
> +async fn ls(path: String, mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
> +    let output_format = extract_output_format(&mut param);
> +
> +    let options = TableFormatOptions::new()
> +        .noborder(true)
> +        .noheader(true)
> +        .sortby("name", false);
> +
> +    let res = get_api_children(path, rpcenv).await?;
> +
> +    format_and_print_result_full(
> +        &mut serde_json::to_value(res)?,
> +        &proxmox::api::schema::ReturnType {
> +            optional: false,
> +            schema: &LS_SCHEMA,
> +        },
> +        &output_format,
> +        &options,
> +    );
> +
> +    Ok(())
> +}
> +
> +fn main() -> Result<(), Error> {
> +    let cmd_def = CliCommandMap::new()
> +        .insert(
> +            "get",
> +            CliCommand::new(&API_METHOD_GET)
> +                .arg_param(&["api-path"])
> +                .completion_cb("api-path", complete_api_path_get),
> +        )
> +        .insert(
> +            "set",
> +            CliCommand::new(&API_METHOD_SET)
> +                .arg_param(&["api-path"])
> +                .completion_cb("api-path", complete_api_path_set),
> +        )
> +        .insert(
> +            "create",
> +            CliCommand::new(&API_METHOD_CREATE)
> +                .arg_param(&["api-path"])
> +                .completion_cb("api-path", complete_api_path_create),
> +        )
> +        .insert(
> +            "delete",
> +            CliCommand::new(&API_METHOD_DELETE)
> +                .arg_param(&["api-path"])
> +                .completion_cb("api-path", complete_api_path_delete),
> +        )
> +        .insert(
> +            "ls",
> +            CliCommand::new(&API_METHOD_LS)
> +                .arg_param(&["path"])
> +                .completion_cb("path", complete_api_path_ls),
> +        )
> +        .insert(
> +            "usage",
> +            CliCommand::new(&API_METHOD_USAGE)
> +                .arg_param(&["path"])
> +                .completion_cb("path", complete_api_path),
> +        );
> +
> +    let uid = nix::unistd::Uid::current();
> +
> +    let username = match nix::unistd::User::from_uid(uid)? {
> +        Some(user) => user.name,
> +        None => bail!("unable to get user name"),
> +    };
> +    let mut rpcenv = CliEnvironment::new();
> +    rpcenv.set_auth_id(Some(format!("{}@pam", username)));
> +
> +    pbs_runtime::main(run_async_cli_command(cmd_def, rpcenv));
> +    Ok(())
> +}
> diff --git a/zsh-completions/_pbs-shell b/zsh-completions/_pbs-shell
> new file mode 100644
> index 00000000..507f15ae
> --- /dev/null
> +++ b/zsh-completions/_pbs-shell
> @@ -0,0 +1,13 @@
> +#compdef _pbs-shell() pbs-shell
> +
> +function _pbs-shell() {
> +    local cwords line point cmd curr prev
> +    cwords=${#words[@]}
> +    line=$words
> +    point=${#line}
> +    cmd=${words[1]}
> +    curr=${words[cwords]}
> +    prev=${words[cwords-1]}
> +    compadd -- $(COMP_CWORD="$cwords" COMP_LINE="$line" COMP_POINT="$point" \
> +        pbs-shell bashcomplete "$cmd" "$curr" "$prev")
> +}
>