From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id CBBE469CF8 for ; Wed, 15 Sep 2021 13:35:36 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B9B0E18516 for ; Wed, 15 Sep 2021 13:35:06 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 1F8DD18508 for ; Wed, 15 Sep 2021 13:35:05 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E61AF4488A for ; Wed, 15 Sep 2021 13:35:04 +0200 (CEST) Message-ID: <742409f0-6c2a-2079-a3ab-c930f0640f5e@proxmox.com> Date: Wed, 15 Sep 2021 13:34:22 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:93.0) Gecko/20100101 Thunderbird/93.0 Content-Language: en-US To: Proxmox Backup Server development discussion , Dominik Csapak References: <20210913141829.2171301-1-d.csapak@proxmox.com> <20210913141829.2171301-4-d.csapak@proxmox.com> From: Thomas Lamprecht In-Reply-To: <20210913141829.2171301-4-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.250 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.969 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [tasks.rs, pbs-shell.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Sep 2021 11:35:36 -0000 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 { > }, > )] > /// Read task log. > -async fn read_task_log( > +pub async fn read_task_log( > param: Value, > mut rpcenv: &mut dyn RpcEnvironment, > ) -> Result { > 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) -> Vec { > + 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) -> Vec { > + pbs_runtime::main(async { complete_api_path_do(complete_me, Some("r")).await }) > +} > + > +fn complete_api_path_set(complete_me: &str, _map: &HashMap) -> Vec { > + pbs_runtime::main(async { complete_api_path_do(complete_me, Some("w")).await }) > +} > + > +fn complete_api_path_create(complete_me: &str, _map: &HashMap) -> Vec { > + pbs_runtime::main(async { complete_api_path_do(complete_me, Some("c")).await }) > +} > + > +fn complete_api_path_delete(complete_me: &str, _map: &HashMap) -> Vec { > + pbs_runtime::main(async { complete_api_path_do(complete_me, Some("d")).await }) > +} > + > +fn complete_api_path_ls(complete_me: &str, _map: &HashMap) -> Vec { > + 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 { > + 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, 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::, _>>()?) > + } > + None => bail!("link does not define child links"), > + } > +} > + > +fn get_api_method( > + method: Method, > + path: &str, > +) -> Result<(&'static ApiMethod, HashMap), 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, > + param: Value, > + schema: ParameterSchema, > +) -> Result { > + 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(¶m_list, schema, true)?; > + > + Ok(params) > +} > + > +async fn call_api( > + method: &'static ApiMethod, > + rpcenv: &mut dyn RpcEnvironment, > + params: Value, > +) -> Result { > + 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 > +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 > +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 > +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 > +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 > +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, 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 > +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") > +} >