[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