Re: [pbs-devel] [PATCH proxmox-backup 3/4] api: refactor remote client and add remote scan

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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 :)