[pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan
Fabian Grünbichler
f.gruenbichler at proxmox.com
Thu Nov 5 08:42:02 CET 2020
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 at 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(())
>>
>
>
>
More information about the pbs-devel
mailing list