* [pbs-devel] [PATCH proxmox v2 1/1] proxmox: generate_usage_str: don't require static lifetimes
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/7] server: refactor abort_local_worker Dominik Csapak
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
this prevents us from using it under certain conditions and it's
actually not necessary, so drop them
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
proxmox/src/api/cli/format.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox/src/api/cli/format.rs b/proxmox/src/api/cli/format.rs
index 17efcd6..a4fb78d 100644
--- a/proxmox/src/api/cli/format.rs
+++ b/proxmox/src/api/cli/format.rs
@@ -60,7 +60,7 @@ pub fn generate_usage_str(
cli_cmd: &CliCommand,
format: DocumentationFormat,
indent: &str,
- skip_options: &'static [&'static str],
+ skip_options: &[&str],
) -> String {
let arg_param = cli_cmd.arg_param;
let fixed_param = &cli_cmd.fixed_param;
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 1/7] server: refactor abort_local_worker
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox v2 1/1] proxmox: generate_usage_str: don't require static lifetimes Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility Dominik Csapak
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
we'll need this outside the module
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/server/worker_task.rs | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 2ef8ba9d..c4696a2f 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -102,9 +102,8 @@ pub fn register_task_control_commands(
commando_sock.register_command("worker-task-abort".into(), move |args| {
let upid = get_upid(args)?;
- if let Some(ref worker) = WORKER_TASK_LIST.lock().unwrap().get(&upid.task_id) {
- worker.request_abort();
- }
+ abort_local_worker(upid);
+
Ok(Value::Null)
})?;
commando_sock.register_command("worker-task-status".into(), move |args| {
@@ -841,3 +840,10 @@ pub async fn wait_for_local_worker(upid_str: &str) -> Result<(), Error> {
}
Ok(())
}
+
+/// Request abort of a local worker (if existing and running)
+pub fn abort_local_worker(upid: UPID) {
+ if let Some(ref worker) = WORKER_TASK_LIST.lock().unwrap().get(&upid.task_id) {
+ worker.request_abort();
+ }
+}
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox v2 1/1] proxmox: generate_usage_str: don't require static lifetimes Dominik Csapak
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 1/7] server: refactor abort_local_worker Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-15 11:34 ` Thomas Lamprecht
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 3/7] api2: add missing token list match_all property Dominik Csapak
` (5 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
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
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
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
Makefile | 2 +
debian/pbs-shell.bc | 3 +
debian/proxmox-backup-server.bash-completion | 1 +
debian/proxmox-backup-server.install | 3 +
docs/Makefile | 8 +
docs/pbs-shell/description.rst | 3 +
docs/pbs-shell/man1.rst | 40 ++
src/api2/node/tasks.rs | 2 +-
src/bin/pbs-shell.rs | 528 +++++++++++++++++++
zsh-completions/_pbs-shell | 13 +
10 files changed, 602 insertions(+), 1 deletion(-)
create mode 100644 debian/pbs-shell.bc
create mode 100644 docs/pbs-shell/description.rst
create mode 100644 docs/pbs-shell/man1.rst
create mode 100644 src/bin/pbs-shell.rs
create mode 100644 zsh-completions/_pbs-shell
diff --git a/Makefile b/Makefile
index c1aecf61..abeaff37 100644
--- a/Makefile
+++ b/Makefile
@@ -11,6 +11,7 @@ USR_BIN := \
proxmox-backup-client \
proxmox-file-restore \
pxar \
+ pbs-shell \
proxmox-tape \
pmtx \
pmt
@@ -172,6 +173,7 @@ $(COMPILED_BINS) $(COMPILEDIR)/dump-catalog-shell-cli $(COMPILEDIR)/docgen: .do-
--bin proxmox-backup-api \
--bin proxmox-backup-proxy \
--bin proxmox-backup-manager \
+ --bin pbs-shell \
--bin docgen
$(CARGO) build $(CARGO_BUILD_ARGS) \
--package proxmox-backup-banner \
diff --git a/debian/pbs-shell.bc b/debian/pbs-shell.bc
new file mode 100644
index 00000000..3d17187c
--- /dev/null
+++ b/debian/pbs-shell.bc
@@ -0,0 +1,3 @@
+# pbs-shell bash completion
+
+complete -C 'pbs-shell bashcomplete' pbs-shell
diff --git a/debian/proxmox-backup-server.bash-completion b/debian/proxmox-backup-server.bash-completion
index a2165699..8d6a7047 100644
--- a/debian/proxmox-backup-server.bash-completion
+++ b/debian/proxmox-backup-server.bash-completion
@@ -2,3 +2,4 @@ debian/proxmox-backup-manager.bc proxmox-backup-manager
debian/proxmox-tape.bc proxmox-tape
debian/pmtx.bc pmtx
debian/pmt.bc pmt
+debian/pbs-shell.bc pbs-shell
diff --git a/debian/proxmox-backup-server.install b/debian/proxmox-backup-server.install
index 6e2219b4..5e1071fa 100644
--- a/debian/proxmox-backup-server.install
+++ b/debian/proxmox-backup-server.install
@@ -14,6 +14,7 @@ usr/sbin/proxmox-backup-manager
usr/bin/pmtx
usr/bin/pmt
usr/bin/proxmox-tape
+usr/bin/pbs-shell
usr/share/javascript/proxmox-backup/index.hbs
usr/share/javascript/proxmox-backup/css/ext6-pbs.css
usr/share/javascript/proxmox-backup/images
@@ -24,6 +25,7 @@ usr/share/man/man1/proxmox-backup-proxy.1
usr/share/man/man1/proxmox-tape.1
usr/share/man/man1/pmtx.1
usr/share/man/man1/pmt.1
+usr/share/man/man1/pbs-shell.1
usr/share/man/man5/acl.cfg.5
usr/share/man/man5/datastore.cfg.5
usr/share/man/man5/user.cfg.5
@@ -38,3 +40,4 @@ usr/share/zsh/vendor-completions/_proxmox-backup-manager
usr/share/zsh/vendor-completions/_proxmox-tape
usr/share/zsh/vendor-completions/_pmtx
usr/share/zsh/vendor-completions/_pmt
+usr/share/zsh/vendor-completions/_pbs-shell
diff --git a/docs/Makefile b/docs/Makefile
index 5e37f7d1..e67df2ea 100644
--- a/docs/Makefile
+++ b/docs/Makefile
@@ -10,6 +10,7 @@ GENERATED_SYNOPSIS := \
pxar/synopsis.rst \
pmtx/synopsis.rst \
pmt/synopsis.rst \
+ pbs-shell/synopsis.rst \
config/media-pool/config.rst \
config/tape/config.rst \
config/tape-job/config.rst \
@@ -24,6 +25,7 @@ MAN1_PAGES := \
pxar.1 \
pmtx.1 \
pmt.1 \
+ pbs-shell.1 \
proxmox-tape.1 \
proxmox-backup-proxy.1 \
proxmox-backup-client.1 \
@@ -117,6 +119,12 @@ pmt/synopsis.rst: ${COMPILEDIR}/pmt
pmt.1: pmt/man1.rst pmt/description.rst pmt/options.rst pmt/synopsis.rst
rst2man $< >$@
+pbs-shell/synopsis.rst: ${COMPILEDIR}/pbs-shell
+ ${COMPILEDIR}/pbs-shell printdoc > pbs-shell/synopsis.rst
+
+pbs-shell.1: pbs-shell/man1.rst pbs-shell/description.rst pbs-shell/synopsis.rst
+ rst2man $< >$@
+
config/datastore/config.rst: ${COMPILEDIR}/docgen
${COMPILEDIR}/docgen datastore.cfg >$@
diff --git a/docs/pbs-shell/description.rst b/docs/pbs-shell/description.rst
new file mode 100644
index 00000000..8dfcae15
--- /dev/null
+++ b/docs/pbs-shell/description.rst
@@ -0,0 +1,3 @@
+The ``pbs-shell`` command can show and execute api calls and their parameters.
+It is mainly intended for use during debugging.
+
diff --git a/docs/pbs-shell/man1.rst b/docs/pbs-shell/man1.rst
new file mode 100644
index 00000000..d0a1d07b
--- /dev/null
+++ b/docs/pbs-shell/man1.rst
@@ -0,0 +1,40 @@
+==========================
+pbs-shell
+==========================
+
+.. include:: ../epilog.rst
+
+-------------------------------------------------------------
+Show and execute PBS API calls
+-------------------------------------------------------------
+
+:Author: |AUTHOR|
+:Version: Version |VERSION|
+:Manual section: 1
+
+
+Synopsis
+==========
+
+.. include:: synopsis.rst
+
+
+Common Options
+==============
+
+Commands generating output supports the ``--output-format``
+parameter. It accepts the following values:
+
+:``text``: Text format (default). Human readable.
+
+:``json``: JSON (single line).
+
+:``json-pretty``: JSON (multiple lines, nicely formatted).
+
+
+Description
+============
+
+.. include:: description.rst
+
+.. include:: ../pbs-copyright.rst
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 })
+}
+
+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(¶m_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
+}
+
+#[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,
+}
+
+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() {
+ 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")
+}
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility Dominik Csapak
@ 2021-09-15 11:34 ` Thomas Lamprecht
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2021-09-15 11:34 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
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(¶m_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")
> +}
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2 3/7] api2: add missing token list match_all property
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
` (2 preceding siblings ...)
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 2/7] add 'pbs-shell' utility Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-15 10:30 ` Thomas Lamprecht
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/7] api2: disks/directory: refactor BASE_MOUNT_DIR Dominik Csapak
` (4 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
to have the proper link between the token list and the sub routes
in the api, include the 'tokenname' property in the token listing
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/access/user.rs | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 75071cf1..97c336ab 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -655,6 +655,21 @@ pub fn delete_token(
Ok(())
}
+#[api(
+ properties: {
+ tokenname: { type: Tokenname },
+ token: { type: ApiToken },
+ }
+)]
+#[derive(Serialize, Deserialize)]
+/// A Token Entry of a user
+pub struct TokenInfo {
+ /// The Token name
+ pub tokenname: Tokenname,
+ #[serde(flatten)]
+ pub token: ApiToken,
+}
+
#[api(
input: {
properties: {
@@ -666,7 +681,7 @@ pub fn delete_token(
returns: {
description: "List user's API tokens (with config digest).",
type: Array,
- items: { type: ApiToken },
+ items: { type: TokenInfo },
},
access: {
permission: &Permission::Or(&[
@@ -680,7 +695,7 @@ pub fn list_tokens(
userid: Userid,
_info: &ApiMethod,
mut rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Vec<ApiToken>, Error> {
+) -> Result<Vec<TokenInfo>, Error> {
let (config, digest) = pbs_config::user::config()?;
@@ -688,15 +703,21 @@ pub fn list_tokens(
rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
- let filter_by_owner = |token: &ApiToken| {
- if token.tokenid.is_token() {
- token.tokenid.user() == &userid
+ let filter_by_owner = |token: ApiToken| {
+ if token.tokenid.is_token() && token.tokenid.user() == &userid {
+ let tokenname = token.tokenid.tokenname().unwrap().to_owned();
+ Some(TokenInfo {
+ tokenname,
+ token,
+ })
} else {
- false
+ None
}
};
- Ok(list.into_iter().filter(filter_by_owner).collect())
+ let res = list.into_iter().filter_map(filter_by_owner).collect();
+
+ Ok(res)
}
const TOKEN_ITEM_ROUTER: Router = Router::new()
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [pbs-devel] [PATCH proxmox-backup v2 3/7] api2: add missing token list match_all property
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 3/7] api2: add missing token list match_all property Dominik Csapak
@ 2021-09-15 10:30 ` Thomas Lamprecht
0 siblings, 0 replies; 15+ messages in thread
From: Thomas Lamprecht @ 2021-09-15 10:30 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
s/api2/api/ please.
On 13.09.21 16:18, Dominik Csapak wrote:
> to have the proper link between the token list and the sub routes
> in the api, include the 'tokenname' property in the token listing
>
this sounds like that the link did not work or the like, but that is not
exactly the case, isn't it? Can we please state what provoked this patch
here.
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> src/api2/access/user.rs | 35 ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
> index 75071cf1..97c336ab 100644
> --- a/src/api2/access/user.rs
> +++ b/src/api2/access/user.rs
> @@ -655,6 +655,21 @@ pub fn delete_token(
> Ok(())
> }
>
> +#[api(
> + properties: {
> + tokenname: { type: Tokenname },
we normally try to use kebab case, or is it not possible to use "token-name" here?
> + token: { type: ApiToken },
> + }
> +)]
> +#[derive(Serialize, Deserialize)]
> +/// A Token Entry of a user
that doc-comment is "meh" at best, casing and info is lacking and not sure if
"of an user" is relevant here. I'd mention that the token name is already
encoded and explicitly split out here for API's sake.
> +pub struct TokenInfo {
I'd find some merit in renaming this to clarify that it is rather a API specific
type that does not provides more info but just more explicit, that the original
type is named "ApiToken" doesn't make this easier though, and atm. I have no good
proposal, sorry, so just putting that opinion out there.
> + /// The Token name
> + pub tokenname: Tokenname,
> + #[serde(flatten)]
> + pub token: ApiToken,
> +}
> +
> #[api(
> input: {
> properties: {
> @@ -666,7 +681,7 @@ pub fn delete_token(
> returns: {
> description: "List user's API tokens (with config digest).",
> type: Array,
> - items: { type: ApiToken },
> + items: { type: TokenInfo },
> },
> access: {
> permission: &Permission::Or(&[
> @@ -680,7 +695,7 @@ pub fn list_tokens(
> userid: Userid,
> _info: &ApiMethod,
> mut rpcenv: &mut dyn RpcEnvironment,
> -) -> Result<Vec<ApiToken>, Error> {
> +) -> Result<Vec<TokenInfo>, Error> {
>
> let (config, digest) = pbs_config::user::config()?;
>
> @@ -688,15 +703,21 @@ pub fn list_tokens(
>
> rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
>
> - let filter_by_owner = |token: &ApiToken| {
> - if token.tokenid.is_token() {
> - token.tokenid.user() == &userid
> + let filter_by_owner = |token: ApiToken| {
> + if token.tokenid.is_token() && token.tokenid.user() == &userid {
> + let tokenname = token.tokenid.tokenname().unwrap().to_owned();
I'd like to try adding comments about why unwrap is safe. A panic possibility is
always a bit of a bummer, so showing that one thought about that and recording
that in a comment would be nice.
> + Some(TokenInfo {
> + tokenname,
> + token,
> + })
> } else {
> - false
> + None
> }
> };
>
> - Ok(list.into_iter().filter(filter_by_owner).collect())
> + let res = list.into_iter().filter_map(filter_by_owner).collect();
> +
> + Ok(res)
nit: why adding a (useless?) intermediated `res` variable in above change?
> }
>
> const TOKEN_ITEM_ROUTER: Router = Router::new()
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2 4/7] api2: disks/directory: refactor BASE_MOUNT_DIR
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
` (3 preceding siblings ...)
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 3/7] api2: add missing token list match_all property Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-15 10:02 ` [pbs-devel] applied: " Thomas Lamprecht
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 5/7] api2: disks/directory: add 'name' property to directory mount listing Dominik Csapak
` (3 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/node/disks/directory.rs | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index 552d7e4b..c6d230de 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -19,6 +19,8 @@ use crate::tools::systemd::{self, types::*};
use crate::server::WorkerTask;
+const BASE_MOUNT_DIR: &str = "/mnt/datastore/";
+
#[api(
properties: {
"filesystem": {
@@ -143,7 +145,7 @@ pub fn create_datastore_disk(
bail!("disk '{}' is already in use.", disk);
}
- let mount_point = format!("/mnt/datastore/{}", &name);
+ let mount_point = format!("{}{}", BASE_MOUNT_DIR, &name);
// check if the default path does exist already and bail if it does
let default_path = std::path::PathBuf::from(&mount_point);
@@ -218,7 +220,7 @@ pub fn create_datastore_disk(
/// Remove a Filesystem mounted under '/mnt/datastore/<name>'.".
pub fn delete_datastore_disk(name: String) -> Result<(), Error> {
- let path = format!("/mnt/datastore/{}", name);
+ let path = format!("{}{}", BASE_MOUNT_DIR, name);
// path of datastore cannot be changed
let (config, _) = pbs_config::datastore::config()?;
let datastores: Vec<DataStoreConfig> = config.convert_to_typed_array("datastore")?;
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 5/7] api2: disks/directory: add 'name' property to directory mount listing
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
` (4 preceding siblings ...)
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 4/7] api2: disks/directory: refactor BASE_MOUNT_DIR Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-15 10:03 ` [pbs-devel] applied: " Thomas Lamprecht
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 6/7] api2: nodes: add missing node list api call Dominik Csapak
` (2 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
so that we have the properties that match with 'match_all'
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/node/disks/directory.rs | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/src/api2/node/disks/directory.rs b/src/api2/node/disks/directory.rs
index c6d230de..ea01352e 100644
--- a/src/api2/node/disks/directory.rs
+++ b/src/api2/node/disks/directory.rs
@@ -35,6 +35,8 @@ const BASE_MOUNT_DIR: &str = "/mnt/datastore/";
pub struct DatastoreMountInfo {
/// The path of the mount unit.
pub unitfile: String,
+ /// The name of the mount
+ pub name: String,
/// The mount path.
pub path: String,
/// The mounted device.
@@ -83,8 +85,15 @@ pub fn list_datastore_mounts() -> Result<Vec<DatastoreMountInfo>, Error> {
let config = systemd::config::parse_systemd_mount(&unitfile)?;
let data: SystemdMountSection = config.lookup("Mount", "Mount")?;
+ let name = data
+ .Where
+ .strip_prefix(BASE_MOUNT_DIR)
+ .unwrap_or_else(|| &data.Where)
+ .to_string();
+
list.push(DatastoreMountInfo {
unitfile,
+ name,
device: data.What,
path: data.Where,
filesystem: data.Type,
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 6/7] api2: nodes: add missing node list api call
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
` (5 preceding siblings ...)
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 5/7] api2: disks/directory: add 'name' property to directory mount listing Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 7/7] api2: make some workers log on CLI Dominik Csapak
2021-09-15 9:28 ` [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Hannes Laimer
8 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
to have an api call for api path traversal
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/mod.rs | 4 +---
src/api2/node/mod.rs | 12 +++++++++++-
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/api2/mod.rs b/src/api2/mod.rs
index 132e2c2a..0ede4165 100644
--- a/src/api2/mod.rs
+++ b/src/api2/mod.rs
@@ -18,14 +18,12 @@ use proxmox::api::router::SubdirMap;
use proxmox::api::Router;
use proxmox::list_subdirs_api_method;
-const NODES_ROUTER: Router = Router::new().match_all("node", &node::ROUTER);
-
const SUBDIRS: SubdirMap = &[
("access", &access::ROUTER),
("admin", &admin::ROUTER),
("backup", &backup::ROUTER),
("config", &config::ROUTER),
- ("nodes", &NODES_ROUTER),
+ ("nodes", &node::ROUTER),
("ping", &ping::ROUTER),
("pull", &pull::ROUTER),
("reader", &reader::ROUTER),
diff --git a/src/api2/node/mod.rs b/src/api2/node/mod.rs
index f1a17934..194ec920 100644
--- a/src/api2/node/mod.rs
+++ b/src/api2/node/mod.rs
@@ -315,6 +315,12 @@ fn upgrade_to_websocket(
.boxed()
}
+#[api]
+/// List Nodes (only for compatiblity)
+fn list_nodes() -> Result<Value, Error> {
+ Ok(json!([ { "node": proxmox::tools::nodename().to_string() } ]))
+}
+
pub const SUBDIRS: SubdirMap = &[
("apt", &apt::ROUTER),
("certificates", &certificates::ROUTER),
@@ -338,6 +344,10 @@ pub const SUBDIRS: SubdirMap = &[
),
];
-pub const ROUTER: Router = Router::new()
+pub const ITEM_ROUTER: Router = Router::new()
.get(&list_subdirs_api_method!(SUBDIRS))
.subdirs(SUBDIRS);
+
+pub const ROUTER: Router = Router::new()
+ .get(&API_METHOD_LIST_NODES)
+ .match_all("node", &ITEM_ROUTER);
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pbs-devel] [PATCH proxmox-backup v2 7/7] api2: make some workers log on CLI
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
` (6 preceding siblings ...)
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 6/7] api2: nodes: add missing node list api call Dominik Csapak
@ 2021-09-13 14:18 ` Dominik Csapak
2021-09-13 14:51 ` [pbs-devel] [PATCH proxmox-backup v3] " Dominik Csapak
2021-09-15 9:28 ` [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Hannes Laimer
8 siblings, 1 reply; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:18 UTC (permalink / raw)
To: pbs-devel
some workers did not log when called via cli
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
src/api2/admin/datastore.rs | 4 +++-
src/api2/admin/sync.rs | 6 ++++--
src/api2/admin/verify.rs | 5 +++--
src/api2/config/datastore.rs | 5 +++--
src/api2/pull.rs | 3 ++-
src/api2/tape/backup.rs | 7 +++++--
src/bin/proxmox-backup-proxy.rs | 4 ++--
src/server/verify_job.rs | 3 ++-
8 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 33700a90..b42de25d 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -951,11 +951,13 @@ pub fn prune_datastore(
let datastore = DataStore::lookup_datastore(&store)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
let upid_str = WorkerTask::new_thread(
"prune",
Some(store.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| crate::server::prune_datastore(
worker.clone(),
auth_id,
diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index 07f268b5..94990adb 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -3,7 +3,7 @@
use anyhow::{bail, format_err, Error};
use serde_json::Value;
-use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment};
+use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType};
use proxmox::api::router::SubdirMap;
use proxmox::{list_subdirs_api_method, sortable};
@@ -120,7 +120,9 @@ pub fn run_sync_job(
let job = Job::new("syncjob", &id)?;
- let upid_str = do_sync_job(job, sync_job, &auth_id, None)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ let upid_str = do_sync_job(job, sync_job, &auth_id, None, to_stdout)?;
Ok(upid_str)
}
diff --git a/src/api2/admin/verify.rs b/src/api2/admin/verify.rs
index a5e5c83f..6bdfdede 100644
--- a/src/api2/admin/verify.rs
+++ b/src/api2/admin/verify.rs
@@ -5,7 +5,7 @@ use serde_json::Value;
use proxmox::api::router::SubdirMap;
use proxmox::{list_subdirs_api_method, sortable};
-use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment};
+use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType};
use pbs_api_types::{
VerificationJobConfig, VerificationJobStatus, JOB_ID_SCHEMA, Authid,
@@ -117,8 +117,9 @@ pub fn run_verification_job(
user_info.check_privs(&auth_id, &["datastore", &verification_job.store], PRIV_DATASTORE_VERIFY, true)?;
let job = Job::new("verificationjob", &id)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
- let upid_str = do_verification_job(job, verification_job, &auth_id, None)?;
+ let upid_str = do_verification_job(job, verification_job, &auth_id, None, to_stdout)?;
Ok(upid_str)
}
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 6301ffaa..c6036fc3 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -4,7 +4,7 @@ use anyhow::{bail, Error};
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
-use proxmox::api::{api, Router, RpcEnvironment, Permission};
+use proxmox::api::{api, Router, RpcEnvironment, RpcEnvironmentType, Permission};
use proxmox::api::section_config::SectionConfigData;
use proxmox::api::schema::{ApiType, parse_property_string};
@@ -114,12 +114,13 @@ pub fn create_datastore(
}
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
WorkerTask::new_thread(
"create-datastore",
Some(config.name.to_string()),
auth_id,
- false,
+ to_stdout,
move |worker| do_create_datastore(lock, section_config, config, Some(&worker)),
)
}
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index d7b155a1..1eb86ea3 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -66,6 +66,7 @@ pub fn do_sync_job(
sync_job: SyncJobConfig,
auth_id: &Authid,
schedule: Option<String>,
+ to_stdout: bool,
) -> Result<String, Error> {
let job_id = format!("{}:{}:{}:{}",
@@ -81,7 +82,7 @@ pub fn do_sync_job(
&worker_type,
Some(job_id.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| async move {
job.start(&worker.upid().to_string())?;
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 39e5feea..808fccfd 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -165,6 +165,7 @@ pub fn do_tape_backup_job(
setup: TapeBackupJobSetup,
auth_id: &Authid,
schedule: Option<String>,
+ to_stdout: bool,
) -> Result<String, Error> {
let job_id = format!("{}:{}:{}:{}",
@@ -196,7 +197,7 @@ pub fn do_tape_backup_job(
&worker_type,
Some(job_id.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| {
job.start(&worker.upid().to_string())?;
let mut drive_lock = drive_lock;
@@ -308,7 +309,9 @@ pub fn run_tape_backup_job(
let job = Job::new("tape-backup-job", &id)?;
- let upid_str = do_tape_backup_job(job, backup_job.setup, &auth_id, None)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ let upid_str = do_tape_backup_job(job, backup_job.setup, &auth_id, None, to_stdout)?;
Ok(upid_str)
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 4240711f..92d94cd5 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -541,7 +541,7 @@ async fn schedule_datastore_sync_jobs() {
};
let auth_id = Authid::root_auth_id().clone();
- if let Err(err) = do_sync_job(job, job_config, &auth_id, Some(event_str)) {
+ if let Err(err) = do_sync_job(job, job_config, &auth_id, Some(event_str), false) {
eprintln!("unable to start datastore sync job {} - {}", &job_id, err);
}
};
@@ -577,7 +577,7 @@ async fn schedule_datastore_verify_jobs() {
Ok(job) => job,
Err(_) => continue, // could not get lock
};
- if let Err(err) = do_verification_job(job, job_config, &auth_id, Some(event_str)) {
+ if let Err(err) = do_verification_job(job, job_config, &auth_id, Some(event_str), false) {
eprintln!("unable to start datastore verification job {} - {}", &job_id, err);
}
};
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index 968e6bd4..7f6d73ff 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -19,6 +19,7 @@ pub fn do_verification_job(
verification_job: VerificationJobConfig,
auth_id: &Authid,
schedule: Option<String>,
+ to_stdout: bool,
) -> Result<String, Error> {
let datastore = DataStore::lookup_datastore(&verification_job.store)?;
@@ -36,7 +37,7 @@ pub fn do_verification_job(
&worker_type,
Some(job_id.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| {
job.start(&worker.upid().to_string())?;
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [pbs-devel] [PATCH proxmox-backup v3] api2: make some workers log on CLI
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 7/7] api2: make some workers log on CLI Dominik Csapak
@ 2021-09-13 14:51 ` Dominik Csapak
0 siblings, 0 replies; 15+ messages in thread
From: Dominik Csapak @ 2021-09-13 14:51 UTC (permalink / raw)
To: pbs-devel
some workers did not log when called via cli
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
chnages from v2:
* add missing hunk
src/api2/admin/datastore.rs | 4 +++-
src/api2/admin/sync.rs | 6 ++++--
src/api2/admin/verify.rs | 5 +++--
src/api2/config/datastore.rs | 5 +++--
src/api2/pull.rs | 3 ++-
src/api2/tape/backup.rs | 7 +++++--
src/bin/proxmox-backup-proxy.rs | 6 +++---
src/server/verify_job.rs | 3 ++-
8 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 33700a90..b42de25d 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -951,11 +951,13 @@ pub fn prune_datastore(
let datastore = DataStore::lookup_datastore(&store)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
let upid_str = WorkerTask::new_thread(
"prune",
Some(store.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| crate::server::prune_datastore(
worker.clone(),
auth_id,
diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index 07f268b5..94990adb 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -3,7 +3,7 @@
use anyhow::{bail, format_err, Error};
use serde_json::Value;
-use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment};
+use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType};
use proxmox::api::router::SubdirMap;
use proxmox::{list_subdirs_api_method, sortable};
@@ -120,7 +120,9 @@ pub fn run_sync_job(
let job = Job::new("syncjob", &id)?;
- let upid_str = do_sync_job(job, sync_job, &auth_id, None)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ let upid_str = do_sync_job(job, sync_job, &auth_id, None, to_stdout)?;
Ok(upid_str)
}
diff --git a/src/api2/admin/verify.rs b/src/api2/admin/verify.rs
index a5e5c83f..6bdfdede 100644
--- a/src/api2/admin/verify.rs
+++ b/src/api2/admin/verify.rs
@@ -5,7 +5,7 @@ use serde_json::Value;
use proxmox::api::router::SubdirMap;
use proxmox::{list_subdirs_api_method, sortable};
-use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment};
+use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment, RpcEnvironmentType};
use pbs_api_types::{
VerificationJobConfig, VerificationJobStatus, JOB_ID_SCHEMA, Authid,
@@ -117,8 +117,9 @@ pub fn run_verification_job(
user_info.check_privs(&auth_id, &["datastore", &verification_job.store], PRIV_DATASTORE_VERIFY, true)?;
let job = Job::new("verificationjob", &id)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
- let upid_str = do_verification_job(job, verification_job, &auth_id, None)?;
+ let upid_str = do_verification_job(job, verification_job, &auth_id, None, to_stdout)?;
Ok(upid_str)
}
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index 6301ffaa..c6036fc3 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -4,7 +4,7 @@ use anyhow::{bail, Error};
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
-use proxmox::api::{api, Router, RpcEnvironment, Permission};
+use proxmox::api::{api, Router, RpcEnvironment, RpcEnvironmentType, Permission};
use proxmox::api::section_config::SectionConfigData;
use proxmox::api::schema::{ApiType, parse_property_string};
@@ -114,12 +114,13 @@ pub fn create_datastore(
}
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
WorkerTask::new_thread(
"create-datastore",
Some(config.name.to_string()),
auth_id,
- false,
+ to_stdout,
move |worker| do_create_datastore(lock, section_config, config, Some(&worker)),
)
}
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index d7b155a1..1eb86ea3 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -66,6 +66,7 @@ pub fn do_sync_job(
sync_job: SyncJobConfig,
auth_id: &Authid,
schedule: Option<String>,
+ to_stdout: bool,
) -> Result<String, Error> {
let job_id = format!("{}:{}:{}:{}",
@@ -81,7 +82,7 @@ pub fn do_sync_job(
&worker_type,
Some(job_id.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| async move {
job.start(&worker.upid().to_string())?;
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index 39e5feea..808fccfd 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -165,6 +165,7 @@ pub fn do_tape_backup_job(
setup: TapeBackupJobSetup,
auth_id: &Authid,
schedule: Option<String>,
+ to_stdout: bool,
) -> Result<String, Error> {
let job_id = format!("{}:{}:{}:{}",
@@ -196,7 +197,7 @@ pub fn do_tape_backup_job(
&worker_type,
Some(job_id.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| {
job.start(&worker.upid().to_string())?;
let mut drive_lock = drive_lock;
@@ -308,7 +309,9 @@ pub fn run_tape_backup_job(
let job = Job::new("tape-backup-job", &id)?;
- let upid_str = do_tape_backup_job(job, backup_job.setup, &auth_id, None)?;
+ let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+ let upid_str = do_tape_backup_job(job, backup_job.setup, &auth_id, None, to_stdout)?;
Ok(upid_str)
}
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 4240711f..3f60d348 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -541,7 +541,7 @@ async fn schedule_datastore_sync_jobs() {
};
let auth_id = Authid::root_auth_id().clone();
- if let Err(err) = do_sync_job(job, job_config, &auth_id, Some(event_str)) {
+ if let Err(err) = do_sync_job(job, job_config, &auth_id, Some(event_str), false) {
eprintln!("unable to start datastore sync job {} - {}", &job_id, err);
}
};
@@ -577,7 +577,7 @@ async fn schedule_datastore_verify_jobs() {
Ok(job) => job,
Err(_) => continue, // could not get lock
};
- if let Err(err) = do_verification_job(job, job_config, &auth_id, Some(event_str)) {
+ if let Err(err) = do_verification_job(job, job_config, &auth_id, Some(event_str), false) {
eprintln!("unable to start datastore verification job {} - {}", &job_id, err);
}
};
@@ -613,7 +613,7 @@ async fn schedule_tape_backup_jobs() {
Ok(job) => job,
Err(_) => continue, // could not get lock
};
- if let Err(err) = do_tape_backup_job(job, job_config.setup, &auth_id, Some(event_str)) {
+ if let Err(err) = do_tape_backup_job(job, job_config.setup, &auth_id, Some(event_str), false) {
eprintln!("unable to start tape backup job {} - {}", &job_id, err);
}
};
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index 968e6bd4..7f6d73ff 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -19,6 +19,7 @@ pub fn do_verification_job(
verification_job: VerificationJobConfig,
auth_id: &Authid,
schedule: Option<String>,
+ to_stdout: bool,
) -> Result<String, Error> {
let datastore = DataStore::lookup_datastore(&verification_job.store)?;
@@ -36,7 +37,7 @@ pub fn do_verification_job(
&worker_type,
Some(job_id.clone()),
auth_id.clone(),
- false,
+ to_stdout,
move |worker| {
job.start(&worker.upid().to_string())?;
--
2.30.2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool
2021-09-13 14:18 [pbs-devel] [PATCH proxmox/proxmox-backup v2] add 'pbs-shell' tool Dominik Csapak
` (7 preceding siblings ...)
2021-09-13 14:18 ` [pbs-devel] [PATCH proxmox-backup v2 7/7] api2: make some workers log on CLI Dominik Csapak
@ 2021-09-15 9:28 ` Hannes Laimer
8 siblings, 0 replies; 15+ messages in thread
From: Hannes Laimer @ 2021-09-15 9:28 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Dominik Csapak
After testing everything seems to work as expected.
One thing I noticed is that for create, delete, get and set the api-path
parameter is called 'api-path', for ls and usage it is called 'path'. I
know why this is, but it kind of gave the feeling of 'path' and
'api-path' being different things, even though they're not. However, it
probably won't be a problem since the description is pretty clear.
Am 13.09.21 um 16:18 schrieb Dominik Csapak:
> this series adds the tool 'pbs-shell', similar to 'pvesh' and 'pmgsh'
> this tool is intended mainly for debugging, but can be useful for
> api calls not exposed via the client/manager.
>
> proxmox (and the dependency in proxmox-backup) need to be bumped
>
> proxmox-backup patches 3-6 are not strictly necessary, but improve
> the api so that the 'ls' command works better
> (there are still some api paths to be fixed)
>
> patch 7 is als not strictly necessary, but changes some workers
> to print to stdout on the cli
>
> changes from v1:
> * rebase on master
> * rename 'path' parameter to 'api-path' since it clashed with some
> api calls that had a 'path variable'
> * better handle workers:
> we must always wait for the local workers and do not need to
> print their logs, since most workers print to stdout if the
> rpcenv type is CLI. Also catch Ctrl+C and try to abort the worker
>
> proxmox:
>
> Dominik Csapak (1):
> proxmox: generate_usage_str: don't require static lifetimes
>
> proxmox/src/api/cli/format.rs | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> proxmox-backup:
>
> Dominik Csapak (7):
> server: refactor abort_local_worker
> add 'pbs-shell' utility
> api2: add missing token list match_all property
> api2: disks/directory: refactor BASE_MOUNT_DIR
> api2: disks/directory: add 'name' property to directory mount listing
> api2: nodes: add missing node list api call
> api2: make some workers log on CLI
>
> Makefile | 2 +
> debian/pbs-shell.bc | 3 +
> debian/proxmox-backup-server.bash-completion | 1 +
> debian/proxmox-backup-server.install | 3 +
> docs/Makefile | 8 +
> docs/pbs-shell/description.rst | 3 +
> docs/pbs-shell/man1.rst | 40 ++
> src/api2/access/user.rs | 35 +-
> src/api2/admin/datastore.rs | 4 +-
> src/api2/admin/sync.rs | 6 +-
> src/api2/admin/verify.rs | 5 +-
> src/api2/config/datastore.rs | 5 +-
> src/api2/mod.rs | 4 +-
> src/api2/node/disks/directory.rs | 15 +-
> src/api2/node/mod.rs | 12 +-
> src/api2/node/tasks.rs | 2 +-
> src/api2/pull.rs | 3 +-
> src/api2/tape/backup.rs | 7 +-
> src/bin/pbs-shell.rs | 528 +++++++++++++++++++
> src/bin/proxmox-backup-proxy.rs | 4 +-
> src/server/verify_job.rs | 3 +-
> src/server/worker_task.rs | 12 +-
> zsh-completions/_pbs-shell | 13 +
> 23 files changed, 688 insertions(+), 30 deletions(-)
> create mode 100644 debian/pbs-shell.bc
> create mode 100644 docs/pbs-shell/description.rst
> create mode 100644 docs/pbs-shell/man1.rst
> create mode 100644 src/bin/pbs-shell.rs
> create mode 100644 zsh-completions/_pbs-shell
>
^ permalink raw reply [flat|nested] 15+ messages in thread