* [pbs-devel] [PATCH proxmox-backup 2/4] types: extract DataStoreListItem
2020-11-04 13:10 [pbs-devel] [PATCH proxmox-backup 1/4] www: don't default to hourly sync job schedule Fabian Grünbichler
@ 2020-11-04 13:10 ` Fabian Grünbichler
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan Fabian Grünbichler
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Fabian Grünbichler @ 2020-11-04 13:10 UTC (permalink / raw)
To: pbs-devel
for reuse in remote scan API call
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
src/api2/admin/datastore.rs | 23 ++++++++---------------
src/api2/types/mod.rs | 19 +++++++++++++++++++
2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index a5d3e979..b051e8dd 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -902,15 +902,7 @@ pub fn garbage_collection_status(
type: Array,
items: {
description: "Datastore name and description.",
- properties: {
- store: {
- schema: DATASTORE_SCHEMA,
- },
- comment: {
- optional: true,
- schema: SINGLE_LINE_COMMENT_SCHEMA,
- },
- },
+ type: DataStoreListItem,
},
},
access: {
@@ -922,7 +914,7 @@ fn get_datastore_list(
_param: Value,
_info: &ApiMethod,
rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Value, Error> {
+) -> Result<Vec<DataStoreListItem>, Error> {
let (config, _digest) = datastore::config()?;
@@ -935,11 +927,12 @@ fn get_datastore_list(
let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let allowed = (user_privs & (PRIV_DATASTORE_AUDIT| PRIV_DATASTORE_BACKUP)) != 0;
if allowed {
- let mut entry = json!({ "store": store });
- if let Some(comment) = data["comment"].as_str() {
- entry["comment"] = comment.into();
- }
- list.push(entry);
+ list.push(
+ DataStoreListItem {
+ store: store.clone(),
+ comment: data["comment"].as_str().map(String::from),
+ }
+ );
}
}
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index 7ee89f57..31fd89d2 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -370,6 +370,25 @@ pub const BLOCKDEVICE_NAME_SCHEMA: Schema = StringSchema::new("Block device name
// Complex type definitions
+#[api(
+ properties: {
+ store: {
+ schema: DATASTORE_SCHEMA,
+ },
+ comment: {
+ optional: true,
+ schema: SINGLE_LINE_COMMENT_SCHEMA,
+ },
+ },
+)]
+#[derive(Serialize, Deserialize)]
+#[serde(rename_all="kebab-case")]
+/// Basic information about a datastore.
+pub struct DataStoreListItem {
+ pub store: String,
+ pub comment: Option<String>,
+}
+
#[api(
properties: {
"backup-type": {
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-04 13:10 [pbs-devel] [PATCH proxmox-backup 1/4] www: don't default to hourly sync job schedule Fabian Grünbichler
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 2/4] types: extract DataStoreListItem Fabian Grünbichler
@ 2020-11-04 13:10 ` Fabian Grünbichler
2020-11-04 16:57 ` Thomas Lamprecht
2020-11-04 17:12 ` Thomas Lamprecht
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 4/4] www: add remote store selector Fabian Grünbichler
` (2 subsequent siblings)
4 siblings, 2 replies; 12+ messages in thread
From: Fabian Grünbichler @ 2020-11-04 13:10 UTC (permalink / raw)
To: pbs-devel
to allow on-demand scanning of remote datastores accessible for the
configured remote user.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
not 100% sure about PRIV_REMOTE_AUDIT vs PRIV_REMOTE_READ.. the latter is required to use a datastore for syncing/pull purposes
src/api2/config/remote.rs | 66 ++++++++++++++++++++++++++++++-
src/api2/pull.rs | 12 +-----
src/bin/proxmox-backup-manager.rs | 26 +++---------
3 files changed, 71 insertions(+), 33 deletions(-)
diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
index ffbba1d2..b415f63d 100644
--- a/src/api2/config/remote.rs
+++ b/src/api2/config/remote.rs
@@ -1,4 +1,4 @@
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
use serde_json::Value;
use ::serde::{Deserialize, Serialize};
@@ -6,6 +6,7 @@ use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
use proxmox::tools::fs::open_file_locked;
use crate::api2::types::*;
+use crate::client::{HttpClient, HttpClientOptions};
use crate::config::cached_user_info::CachedUserInfo;
use crate::config::remote;
use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
@@ -301,10 +302,71 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
Ok(())
}
+/// Helper to get client for remote.cfg entry
+pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
+ let options = HttpClientOptions::new()
+ .password(Some(remote.password.clone()))
+ .fingerprint(remote.fingerprint.clone());
+
+ let client = HttpClient::new(
+ &remote.host,
+ remote.port.unwrap_or(8007),
+ &remote.userid,
+ options)?;
+ let _auth_info = client.login() // make sure we can auth
+ .await
+ .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
+
+ Ok(client)
+}
+
+
+#[api(
+ input: {
+ properties: {
+ name: {
+ schema: REMOTE_ID_SCHEMA,
+ },
+ },
+ },
+ access: {
+ permission: &Permission::Privilege(&["remote", "{name}"], PRIV_REMOTE_AUDIT, false),
+ },
+ returns: {
+ description: "List the accessible datastores.",
+ type: Array,
+ items: {
+ description: "Datastore name and description.",
+ type: DataStoreListItem,
+ },
+ },
+)]
+/// List datastores of a remote.cfg entry
+pub async fn scan_remote_datastores(name: String) -> Result<Vec<DataStoreListItem>, Error> {
+ let (remote_config, _digest) = remote::config()?;
+ let remote: remote::Remote = remote_config.lookup("remote", &name)?;
+
+ let client = remote_client(remote).await?;
+ let api_res = client.get("api2/json/admin/datastore", None).await?;
+ let parse_res = match api_res.get("data") {
+ Some(data) => serde_json::from_value::<Vec<DataStoreListItem>>(data.to_owned()),
+ None => bail!("remote {} did not return any datastore list data", &name),
+ };
+
+ match parse_res {
+ Ok(parsed) => Ok(parsed),
+ Err(_) => bail!("Failed to parse remote scan api result."),
+ }
+}
+
+const SCAN_ROUTER: Router = Router::new()
+ .get(&API_METHOD_SCAN_REMOTE_DATASTORES);
+
const ITEM_ROUTER: Router = Router::new()
.get(&API_METHOD_READ_REMOTE)
.put(&API_METHOD_UPDATE_REMOTE)
- .delete(&API_METHOD_DELETE_REMOTE);
+ .delete(&API_METHOD_DELETE_REMOTE)
+ .subdirs(&[("scan", &SCAN_ROUTER)]);
pub const ROUTER: Router = Router::new()
.get(&API_METHOD_LIST_REMOTES)
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index d9e9d31d..87015c72 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -9,7 +9,7 @@ use proxmox::api::{ApiMethod, Router, RpcEnvironment, Permission};
use crate::server::{WorkerTask, jobstate::Job};
use crate::backup::DataStore;
-use crate::client::{HttpClient, HttpClientOptions, BackupRepository, pull::pull_store};
+use crate::client::{HttpClient, BackupRepository, pull::pull_store};
use crate::api2::types::*;
use crate::config::{
remote,
@@ -50,17 +50,9 @@ pub async fn get_pull_parameters(
let (remote_config, _digest) = remote::config()?;
let remote: remote::Remote = remote_config.lookup("remote", remote)?;
- let options = HttpClientOptions::new()
- .password(Some(remote.password.clone()))
- .fingerprint(remote.fingerprint.clone());
-
let src_repo = BackupRepository::new(Some(remote.userid.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string());
- let client = HttpClient::new(&src_repo.host(), src_repo.port(), &src_repo.auth_id(), options)?;
- let _auth_info = client.login() // make sure we can auth
- .await
- .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
-
+ let client = crate::api2::config::remote::remote_client(remote).await?;
Ok((client, src_repo, tgt_store))
}
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 7499446b..e52c2f76 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -413,29 +413,13 @@ pub fn complete_remote_datastore_name(_arg: &str, param: &HashMap<String, String
let _ = proxmox::try_block!({
let remote = param.get("remote").ok_or_else(|| format_err!("no remote"))?;
- let (remote_config, _digest) = config::remote::config()?;
- let remote: config::remote::Remote = remote_config.lookup("remote", &remote)?;
+ let data = crate::tools::runtime::block_on(async move {
+ crate::api2::config::remote::scan_remote_datastores(remote.clone()).await
+ })?;
- let options = HttpClientOptions::new()
- .password(Some(remote.password.clone()))
- .fingerprint(remote.fingerprint.clone());
-
- let client = HttpClient::new(
- &remote.host,
- remote.port.unwrap_or(8007),
- &remote.userid,
- options,
- )?;
-
- let result = crate::tools::runtime::block_on(client.get("api2/json/admin/datastore", None))?;
-
- if let Some(data) = result["data"].as_array() {
- for item in data {
- if let Some(store) = item["store"].as_str() {
- list.push(store.to_owned());
- }
- }
+ for item in data {
+ list.push(item.store);
}
Ok(())
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan Fabian Grünbichler
@ 2020-11-04 16:57 ` Thomas Lamprecht
2020-11-05 7:42 ` Fabian Grünbichler
2020-11-04 17:12 ` Thomas Lamprecht
1 sibling, 1 reply; 12+ messages in thread
From: Thomas Lamprecht @ 2020-11-04 16:57 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 04.11.20 14:10, Fabian Grünbichler wrote:
> to allow on-demand scanning of remote datastores accessible for the
> configured remote user.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>
> Notes:
> not 100% sure about PRIV_REMOTE_AUDIT vs PRIV_REMOTE_READ.. the latter is required to use a datastore for syncing/pull purposes
you are not syncing here, so why should the permissions required for
that matter, when getting a general list of datastores of a remote?
If, that would be an extra filter param to set.
I setup a remote with a token, got ->
GET /api2/json/config/remote/tuxis/scan: 401 Unauthorized: [client [::ffff:192.168.16.38]:47544] authentication failed - invalid user name in user id
>
> src/api2/config/remote.rs | 66 ++++++++++++++++++++++++++++++-
> src/api2/pull.rs | 12 +-----
> src/bin/proxmox-backup-manager.rs | 26 +++---------
> 3 files changed, 71 insertions(+), 33 deletions(-)
>
> diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
> index ffbba1d2..b415f63d 100644
> --- a/src/api2/config/remote.rs
> +++ b/src/api2/config/remote.rs
> @@ -1,4 +1,4 @@
> -use anyhow::{bail, Error};
> +use anyhow::{bail, format_err, Error};
> use serde_json::Value;
> use ::serde::{Deserialize, Serialize};
>
> @@ -6,6 +6,7 @@ use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
> use proxmox::tools::fs::open_file_locked;
>
> use crate::api2::types::*;
> +use crate::client::{HttpClient, HttpClientOptions};
> use crate::config::cached_user_info::CachedUserInfo;
> use crate::config::remote;
> use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
> @@ -301,10 +302,71 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
> Ok(())
> }
>
> +/// Helper to get client for remote.cfg entry
> +pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
> + let options = HttpClientOptions::new()
> + .password(Some(remote.password.clone()))
> + .fingerprint(remote.fingerprint.clone());
> +
> + let client = HttpClient::new(
> + &remote.host,
> + remote.port.unwrap_or(8007),
> + &remote.userid,
sure about userid, shouldn't this be authid or is that the same here?
At least would explain the error I get..
> + options)?;
> + let _auth_info = client.login() // make sure we can auth
> + .await
> + .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
> +
> + Ok(client)
> +}
> +
> +
> +#[api(
> + input: {
> + properties: {
> + name: {
> + schema: REMOTE_ID_SCHEMA,
> + },
> + },
> + },
> + access: {
> + permission: &Permission::Privilege(&["remote", "{name}"], PRIV_REMOTE_AUDIT, false),
> + },
> + returns: {
> + description: "List the accessible datastores.",
> + type: Array,
> + items: {
> + description: "Datastore name and description.",
> + type: DataStoreListItem,
> + },
> + },
> +)]
> +/// List datastores of a remote.cfg entry
> +pub async fn scan_remote_datastores(name: String) -> Result<Vec<DataStoreListItem>, Error> {
> + let (remote_config, _digest) = remote::config()?;
> + let remote: remote::Remote = remote_config.lookup("remote", &name)?;
> +
> + let client = remote_client(remote).await?;
> + let api_res = client.get("api2/json/admin/datastore", None).await?;
> + let parse_res = match api_res.get("data") {
> + Some(data) => serde_json::from_value::<Vec<DataStoreListItem>>(data.to_owned()),
> + None => bail!("remote {} did not return any datastore list data", &name),
> + };
> +
> + match parse_res {
> + Ok(parsed) => Ok(parsed),
> + Err(_) => bail!("Failed to parse remote scan api result."),
> + }
> +}
> +
> +const SCAN_ROUTER: Router = Router::new()
> + .get(&API_METHOD_SCAN_REMOTE_DATASTORES);
> +
> const ITEM_ROUTER: Router = Router::new()
> .get(&API_METHOD_READ_REMOTE)
> .put(&API_METHOD_UPDATE_REMOTE)
> - .delete(&API_METHOD_DELETE_REMOTE);
> + .delete(&API_METHOD_DELETE_REMOTE)
> + .subdirs(&[("scan", &SCAN_ROUTER)]);
>
> pub const ROUTER: Router = Router::new()
> .get(&API_METHOD_LIST_REMOTES)
> diff --git a/src/api2/pull.rs b/src/api2/pull.rs
> index d9e9d31d..87015c72 100644
> --- a/src/api2/pull.rs
> +++ b/src/api2/pull.rs
> @@ -9,7 +9,7 @@ use proxmox::api::{ApiMethod, Router, RpcEnvironment, Permission};
>
> use crate::server::{WorkerTask, jobstate::Job};
> use crate::backup::DataStore;
> -use crate::client::{HttpClient, HttpClientOptions, BackupRepository, pull::pull_store};
> +use crate::client::{HttpClient, BackupRepository, pull::pull_store};
> use crate::api2::types::*;
> use crate::config::{
> remote,
> @@ -50,17 +50,9 @@ pub async fn get_pull_parameters(
> let (remote_config, _digest) = remote::config()?;
> let remote: remote::Remote = remote_config.lookup("remote", remote)?;
>
> - let options = HttpClientOptions::new()
> - .password(Some(remote.password.clone()))
> - .fingerprint(remote.fingerprint.clone());
> -
> let src_repo = BackupRepository::new(Some(remote.userid.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string());
>
> - let client = HttpClient::new(&src_repo.host(), src_repo.port(), &src_repo.auth_id(), options)?;
> - let _auth_info = client.login() // make sure we can auth
> - .await
> - .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
> -
> + let client = crate::api2::config::remote::remote_client(remote).await?;
>
> Ok((client, src_repo, tgt_store))
> }
> diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
> index 7499446b..e52c2f76 100644
> --- a/src/bin/proxmox-backup-manager.rs
> +++ b/src/bin/proxmox-backup-manager.rs
> @@ -413,29 +413,13 @@ pub fn complete_remote_datastore_name(_arg: &str, param: &HashMap<String, String
>
> let _ = proxmox::try_block!({
> let remote = param.get("remote").ok_or_else(|| format_err!("no remote"))?;
> - let (remote_config, _digest) = config::remote::config()?;
>
> - let remote: config::remote::Remote = remote_config.lookup("remote", &remote)?;
> + let data = crate::tools::runtime::block_on(async move {
> + crate::api2::config::remote::scan_remote_datastores(remote.clone()).await
> + })?;
>
> - let options = HttpClientOptions::new()
> - .password(Some(remote.password.clone()))
> - .fingerprint(remote.fingerprint.clone());
> -
> - let client = HttpClient::new(
> - &remote.host,
> - remote.port.unwrap_or(8007),
> - &remote.userid,
> - options,
> - )?;
> -
> - let result = crate::tools::runtime::block_on(client.get("api2/json/admin/datastore", None))?;
> -
> - if let Some(data) = result["data"].as_array() {
> - for item in data {
> - if let Some(store) = item["store"].as_str() {
> - list.push(store.to_owned());
> - }
> - }
> + for item in data {
> + list.push(item.store);
> }
>
> Ok(())
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-04 16:57 ` Thomas Lamprecht
@ 2020-11-05 7:42 ` Fabian Grünbichler
2020-11-05 9:03 ` Thomas Lamprecht
0 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2020-11-05 7:42 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Thomas Lamprecht
On November 4, 2020 5:57 pm, Thomas Lamprecht wrote:
> On 04.11.20 14:10, Fabian Grünbichler wrote:
>> to allow on-demand scanning of remote datastores accessible for the
>> configured remote user.
>>
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>> ---
>>
>> Notes:
>> not 100% sure about PRIV_REMOTE_AUDIT vs PRIV_REMOTE_READ.. the latter is required to use a datastore for syncing/pull purposes
>
>
> you are not syncing here, so why should the permissions required for
> that matter, when getting a general list of datastores of a remote?
because the only thing that a remote datastore can currently be used for
is syncing ;) but I am fine with AUDIT as well, I just wanted to mention
it.
> If, that would be an extra filter param to set.
>
> I setup a remote with a token, got ->
> GET /api2/json/config/remote/tuxis/scan: 401 Unauthorized: [client [::ffff:192.168.16.38]:47544] authentication failed - invalid user name in user id
I think (as we discussed directly) this was an artifact of version
mismatch?
>
>>
>> src/api2/config/remote.rs | 66 ++++++++++++++++++++++++++++++-
>> src/api2/pull.rs | 12 +-----
>> src/bin/proxmox-backup-manager.rs | 26 +++---------
>> 3 files changed, 71 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
>> index ffbba1d2..b415f63d 100644
>> --- a/src/api2/config/remote.rs
>> +++ b/src/api2/config/remote.rs
>> @@ -1,4 +1,4 @@
>> -use anyhow::{bail, Error};
>> +use anyhow::{bail, format_err, Error};
>> use serde_json::Value;
>> use ::serde::{Deserialize, Serialize};
>>
>> @@ -6,6 +6,7 @@ use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
>> use proxmox::tools::fs::open_file_locked;
>>
>> use crate::api2::types::*;
>> +use crate::client::{HttpClient, HttpClientOptions};
>> use crate::config::cached_user_info::CachedUserInfo;
>> use crate::config::remote;
>> use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
>> @@ -301,10 +302,71 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
>> Ok(())
>> }
>>
>> +/// Helper to get client for remote.cfg entry
>> +pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
>> + let options = HttpClientOptions::new()
>> + .password(Some(remote.password.clone()))
>> + .fingerprint(remote.fingerprint.clone());
>> +
>> + let client = HttpClient::new(
>> + &remote.host,
>> + remote.port.unwrap_or(8007),
>> + &remote.userid,
>
> sure about userid, shouldn't this be authid or is that the same here?
> At least would explain the error I get..
the field in the config is called userid, it contains an Authid
(renaming would require postinst fixup, but if you want I can send a
patch for switching it over).
>
>> + options)?;
>> + let _auth_info = client.login() // make sure we can auth
>> + .await
>> + .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
>> +
>> + Ok(client)
>> +}
>> +
>> +
>> +#[api(
>> + input: {
>> + properties: {
>> + name: {
>> + schema: REMOTE_ID_SCHEMA,
>> + },
>> + },
>> + },
>> + access: {
>> + permission: &Permission::Privilege(&["remote", "{name}"], PRIV_REMOTE_AUDIT, false),
>> + },
>> + returns: {
>> + description: "List the accessible datastores.",
>> + type: Array,
>> + items: {
>> + description: "Datastore name and description.",
>> + type: DataStoreListItem,
>> + },
>> + },
>> +)]
>> +/// List datastores of a remote.cfg entry
>> +pub async fn scan_remote_datastores(name: String) -> Result<Vec<DataStoreListItem>, Error> {
>> + let (remote_config, _digest) = remote::config()?;
>> + let remote: remote::Remote = remote_config.lookup("remote", &name)?;
>> +
>> + let client = remote_client(remote).await?;
>> + let api_res = client.get("api2/json/admin/datastore", None).await?;
>> + let parse_res = match api_res.get("data") {
>> + Some(data) => serde_json::from_value::<Vec<DataStoreListItem>>(data.to_owned()),
>> + None => bail!("remote {} did not return any datastore list data", &name),
>> + };
>> +
>> + match parse_res {
>> + Ok(parsed) => Ok(parsed),
>> + Err(_) => bail!("Failed to parse remote scan api result."),
>> + }
>> +}
>> +
>> +const SCAN_ROUTER: Router = Router::new()
>> + .get(&API_METHOD_SCAN_REMOTE_DATASTORES);
>> +
>> const ITEM_ROUTER: Router = Router::new()
>> .get(&API_METHOD_READ_REMOTE)
>> .put(&API_METHOD_UPDATE_REMOTE)
>> - .delete(&API_METHOD_DELETE_REMOTE);
>> + .delete(&API_METHOD_DELETE_REMOTE)
>> + .subdirs(&[("scan", &SCAN_ROUTER)]);
>>
>> pub const ROUTER: Router = Router::new()
>> .get(&API_METHOD_LIST_REMOTES)
>> diff --git a/src/api2/pull.rs b/src/api2/pull.rs
>> index d9e9d31d..87015c72 100644
>> --- a/src/api2/pull.rs
>> +++ b/src/api2/pull.rs
>> @@ -9,7 +9,7 @@ use proxmox::api::{ApiMethod, Router, RpcEnvironment, Permission};
>>
>> use crate::server::{WorkerTask, jobstate::Job};
>> use crate::backup::DataStore;
>> -use crate::client::{HttpClient, HttpClientOptions, BackupRepository, pull::pull_store};
>> +use crate::client::{HttpClient, BackupRepository, pull::pull_store};
>> use crate::api2::types::*;
>> use crate::config::{
>> remote,
>> @@ -50,17 +50,9 @@ pub async fn get_pull_parameters(
>> let (remote_config, _digest) = remote::config()?;
>> let remote: remote::Remote = remote_config.lookup("remote", remote)?;
>>
>> - let options = HttpClientOptions::new()
>> - .password(Some(remote.password.clone()))
>> - .fingerprint(remote.fingerprint.clone());
>> -
>> let src_repo = BackupRepository::new(Some(remote.userid.clone()), Some(remote.host.clone()), remote.port, remote_store.to_string());
>>
>> - let client = HttpClient::new(&src_repo.host(), src_repo.port(), &src_repo.auth_id(), options)?;
>> - let _auth_info = client.login() // make sure we can auth
>> - .await
>> - .map_err(|err| format_err!("remote connection to '{}' failed - {}", remote.host, err))?;
>> -
>> + let client = crate::api2::config::remote::remote_client(remote).await?;
>>
>> Ok((client, src_repo, tgt_store))
>> }
>> diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
>> index 7499446b..e52c2f76 100644
>> --- a/src/bin/proxmox-backup-manager.rs
>> +++ b/src/bin/proxmox-backup-manager.rs
>> @@ -413,29 +413,13 @@ pub fn complete_remote_datastore_name(_arg: &str, param: &HashMap<String, String
>>
>> let _ = proxmox::try_block!({
>> let remote = param.get("remote").ok_or_else(|| format_err!("no remote"))?;
>> - let (remote_config, _digest) = config::remote::config()?;
>>
>> - let remote: config::remote::Remote = remote_config.lookup("remote", &remote)?;
>> + let data = crate::tools::runtime::block_on(async move {
>> + crate::api2::config::remote::scan_remote_datastores(remote.clone()).await
>> + })?;
>>
>> - let options = HttpClientOptions::new()
>> - .password(Some(remote.password.clone()))
>> - .fingerprint(remote.fingerprint.clone());
>> -
>> - let client = HttpClient::new(
>> - &remote.host,
>> - remote.port.unwrap_or(8007),
>> - &remote.userid,
>> - options,
>> - )?;
>> -
>> - let result = crate::tools::runtime::block_on(client.get("api2/json/admin/datastore", None))?;
>> -
>> - if let Some(data) = result["data"].as_array() {
>> - for item in data {
>> - if let Some(store) = item["store"].as_str() {
>> - list.push(store.to_owned());
>> - }
>> - }
>> + for item in data {
>> + list.push(item.store);
>> }
>>
>> Ok(())
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-05 7:42 ` Fabian Grünbichler
@ 2020-11-05 9:03 ` Thomas Lamprecht
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Lamprecht @ 2020-11-05 9:03 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 05.11.20 08:42, Fabian Grünbichler wrote:
> On November 4, 2020 5:57 pm, Thomas Lamprecht wrote:
>> On 04.11.20 14:10, Fabian Grünbichler wrote:
>>> to allow on-demand scanning of remote datastores accessible for the
>>> configured remote user.
>>>
>>> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
>>> ---
>>>
>>> Notes:
>>> not 100% sure about PRIV_REMOTE_AUDIT vs PRIV_REMOTE_READ.. the latter is required to use a datastore for syncing/pull purposes
>>
>> you are not syncing here, so why should the permissions required for
>> that matter, when getting a general list of datastores of a remote?
> because the only thing that a remote datastore can currently be used for
> is syncing ;) but I am fine with AUDIT as well, I just wanted to mention
> it.
yes, but just because it will be used for that now, it still has not anything
to do with that directly - so I'd see it just as it's own thing, datastore
scanner - with no opinion on what the user wants to do with that.
>
>> If, that would be an extra filter param to set.
>>
>> I setup a remote with a token, got ->
>> GET /api2/json/config/remote/tuxis/scan: 401 Unauthorized: [client [::ffff:192.168.16.38]:47544] authentication failed - invalid user name in user id
> I think (as we discussed directly) this was an artifact of version
> mismatch?
>
>>> src/api2/config/remote.rs | 66 ++++++++++++++++++++++++++++++-
>>> src/api2/pull.rs | 12 +-----
>>> src/bin/proxmox-backup-manager.rs | 26 +++---------
>>> 3 files changed, 71 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs
>>> index ffbba1d2..b415f63d 100644
>>> --- a/src/api2/config/remote.rs
>>> +++ b/src/api2/config/remote.rs
>>> @@ -1,4 +1,4 @@
>>> -use anyhow::{bail, Error};
>>> +use anyhow::{bail, format_err, Error};
>>> use serde_json::Value;
>>> use ::serde::{Deserialize, Serialize};
>>>
>>> @@ -6,6 +6,7 @@ use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission};
>>> use proxmox::tools::fs::open_file_locked;
>>>
>>> use crate::api2::types::*;
>>> +use crate::client::{HttpClient, HttpClientOptions};
>>> use crate::config::cached_user_info::CachedUserInfo;
>>> use crate::config::remote;
>>> use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY};
>>> @@ -301,10 +302,71 @@ pub fn delete_remote(name: String, digest: Option<String>) -> Result<(), Error>
>>> Ok(())
>>> }
>>>
>>> +/// Helper to get client for remote.cfg entry
>>> +pub async fn remote_client(remote: remote::Remote) -> Result<HttpClient, Error> {
>>> + let options = HttpClientOptions::new()
>>> + .password(Some(remote.password.clone()))
>>> + .fingerprint(remote.fingerprint.clone());
>>> +
>>> + let client = HttpClient::new(
>>> + &remote.host,
>>> + remote.port.unwrap_or(8007),
>>> + &remote.userid,
>> sure about userid, shouldn't this be authid or is that the same here?
>> At least would explain the error I get..
> the field in the config is called userid, it contains an Authid
> (renaming would require postinst fixup, but if you want I can send a
> patch for switching it over).
>
it's a bit confusing, but that's it, was probably more confused in the light
of the outdated server at the other end.. So sorry for the noise :)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan Fabian Grünbichler
2020-11-04 16:57 ` Thomas Lamprecht
@ 2020-11-04 17:12 ` Thomas Lamprecht
2020-11-05 7:43 ` Fabian Grünbichler
1 sibling, 1 reply; 12+ messages in thread
From: Thomas Lamprecht @ 2020-11-04 17:12 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
we probably also do not want to bubble up the 401 error our server gets
from the remote server, as I then get logged out by the gui which thinks
the 401 are from our server...
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-04 17:12 ` Thomas Lamprecht
@ 2020-11-05 7:43 ` Fabian Grünbichler
2020-11-05 8:58 ` Thomas Lamprecht
0 siblings, 1 reply; 12+ messages in thread
From: Fabian Grünbichler @ 2020-11-05 7:43 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Thomas Lamprecht
On November 4, 2020 6:12 pm, Thomas Lamprecht wrote:
> we probably also do not want to bubble up the 401 error our server gets
> from the remote server, as I then get logged out by the gui which thinks
> the 401 are from our server...
that's a good gotcha. should we just map errors to a generic 400
prefixed with 'scanning remote '{}' failed' ?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
2020-11-05 7:43 ` Fabian Grünbichler
@ 2020-11-05 8:58 ` Thomas Lamprecht
0 siblings, 0 replies; 12+ messages in thread
From: Thomas Lamprecht @ 2020-11-05 8:58 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
On 05.11.20 08:43, Fabian Grünbichler wrote:
> On November 4, 2020 6:12 pm, Thomas Lamprecht wrote:
>> we probably also do not want to bubble up the 401 error our server gets
>> from the remote server, as I then get logged out by the gui which thinks
>> the 401 are from our server...
>
> that's a good gotcha. should we just map errors to a generic 400
> prefixed with 'scanning remote '{}' failed' ?
or even a 5xx, as the server run into this error? but no hard feelings here.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 4/4] www: add remote store selector
2020-11-04 13:10 [pbs-devel] [PATCH proxmox-backup 1/4] www: don't default to hourly sync job schedule Fabian Grünbichler
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 2/4] types: extract DataStoreListItem Fabian Grünbichler
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan Fabian Grünbichler
@ 2020-11-04 13:10 ` Fabian Grünbichler
2020-11-04 13:42 ` [pbs-devel] [PATCH proxmox-backup 1/4] www: don't default to hourly sync job schedule Thomas Lamprecht
[not found] ` <dce0d21f-20dc-5443-bbb0-6b6f5be73e43@proxmox.com>
4 siblings, 0 replies; 12+ messages in thread
From: Fabian Grünbichler @ 2020-11-04 13:10 UTC (permalink / raw)
To: pbs-devel
(hopefully) improved upon NFS export selection in PVE
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
does not display loading errors nicely (the remote store picker needs to be
opened before the remote changes, otherwise the elements needed to display the
error don't exist yet)
www/window/SyncJobEdit.js | 97 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 96 insertions(+), 1 deletion(-)
diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index 43338cab..c71821d0 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -1,3 +1,90 @@
+Ext.define('PBS.form.RemoteStoreSelector', {
+ extend: 'Proxmox.form.ComboGrid',
+ alias: 'widget.pbsRemoteStoreSelector',
+
+ queryMode: 'local',
+
+ valueField: 'store',
+ displayField: 'store',
+ notFoundIsValid: true,
+
+ matchFieldWidth: false,
+ listConfig: {
+ loadingText: gettext('Scanning...'),
+ width: 350,
+ columns: [
+ {
+ header: gettext('Datastore'),
+ sortable: true,
+ dataIndex: 'store',
+ renderer: Ext.String.htmlEncode,
+ flex: 1,
+ },
+ {
+ header: gettext('Comment'),
+ dataIndex: 'comment',
+ renderer: Ext.String.htmlEncode,
+ flex: 1,
+ },
+ ],
+ },
+
+ doRawQuery: function() {
+ // do nothing.
+ },
+
+ setRemote: function(remote) {
+ let me = this;
+
+ if (me.remote === remote) {
+ return;
+ }
+
+ me.remote = remote;
+
+ let store = me.store;
+ store.removeAll();
+
+ if (me.remote) {
+ me.setDisabled(false);
+ if (me.actualChange) {
+ me.clearValue();
+ }
+
+ store.proxy.url = '/api2/json/config/remote/' + encodeURIComponent(me.remote) + '/scan';
+ store.load();
+
+ me.actualChange = true;
+ } else {
+ me.setDisabled(true);
+ me.clearValue();
+ }
+ },
+
+ initComponent: function() {
+ let me = this;
+
+ let store = Ext.create('Ext.data.Store', {
+ fields: ['store', 'comment'],
+ proxy: {
+ type: 'proxmox',
+ url: '/api2/json/config/remote/' + encodeURIComponent(me.remote) + '/scan',
+ },
+ });
+
+ store.sort('store', 'ASC');
+
+ Ext.apply(me, {
+ store: store,
+ });
+
+ me.callParent();
+
+ Proxmox.Utils.monStoreErrors(me.getPicker(), me.store);
+ },
+});
+
+
Ext.define('PBS.window.SyncJobEdit', {
extend: 'Proxmox.window.Edit',
alias: 'widget.pbsSyncJobEdit',
@@ -52,12 +139,20 @@ Ext.define('PBS.window.SyncJobEdit', {
xtype: 'pbsRemoteSelector',
allowBlank: false,
name: 'remote',
+ listeners: {
+ change: function(f, value) {
+ let me = this;
+ let remoteStoreField = me.up('pbsSyncJobEdit').down('field[name=remote-store]');
+ remoteStoreField.setRemote(value);
+ },
+ },
},
{
fieldLabel: gettext('Source Datastore'),
- xtype: 'proxmoxtextfield',
+ xtype: 'pbsRemoteStoreSelector',
allowBlank: false,
name: 'remote-store',
+ disabled: true,
},
],
--
2.20.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 1/4] www: don't default to hourly sync job schedule
2020-11-04 13:10 [pbs-devel] [PATCH proxmox-backup 1/4] www: don't default to hourly sync job schedule Fabian Grünbichler
` (2 preceding siblings ...)
2020-11-04 13:10 ` [pbs-devel] [PATCH proxmox-backup 4/4] www: add remote store selector Fabian Grünbichler
@ 2020-11-04 13:42 ` Thomas Lamprecht
[not found] ` <dce0d21f-20dc-5443-bbb0-6b6f5be73e43@proxmox.com>
4 siblings, 0 replies; 12+ messages in thread
From: Thomas Lamprecht @ 2020-11-04 13:42 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Fabian Grünbichler
(resend, as I forgot to keep the mailing list as recipient)
On 04.11.20 14:10, Fabian Grünbichler wrote:
> this breaks editing disabled sync jobs, and is also not consistent with
just means that we should only set it onCreate..
> the backend defaults.
which is? FWICT, there's no default in the backend - I want one here,
this can be easily edited and it's just better UX to avoid more decisions
which at this point may not be clear.
>
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> www/window/SyncJobEdit.js | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
> index c89b95d2..43338cab 100644
> --- a/www/window/SyncJobEdit.js
> +++ b/www/window/SyncJobEdit.js
> @@ -84,7 +84,6 @@ Ext.define('PBS.window.SyncJobEdit', {
> fieldLabel: gettext('Schedule'),
> xtype: 'pbsCalendarEvent',
> name: 'schedule',
> - value: 'hourly',
> emptyText: gettext('none (disabled)'),
> cbind: {
> deleteEmpty: '{!isCreate}',
>
^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <dce0d21f-20dc-5443-bbb0-6b6f5be73e43@proxmox.com>]