Re: [pbs-devel] [PATCH proxmox-backup] add datastore info api call

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On October 21, 2020 4:01 pm, Oguz Bektas wrote:
> to be able to copy/paste easily when adding a new PBS datastore remote
> in PVE
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  src/api2/admin/datastore.rs | 55 +++++++++++++++++++++++++++++++++++++
>  src/api2/types/mod.rs       | 23 ++++++++++++++++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
> index 91ca3570..41059f98 100644
> --- a/src/api2/admin/datastore.rs
> +++ b/src/api2/admin/datastore.rs
> @@ -25,6 +25,7 @@ use pxar::EntryKind;
>  
>  use crate::api2::types::*;
>  use crate::api2::node::rrd::create_value_from_rrd;
> +use crate::config::network::{self};
>  use crate::backup::*;
>  use crate::config::datastore;
>  use crate::config::cached_user_info::CachedUserInfo;
> @@ -36,6 +37,7 @@ use crate::tools::{
>      AsyncChannelWriter, AsyncReaderStream, WrappedReaderStream,
>  };
>  
> +use crate::tools::cert::CertInfo;
>  use crate::config::acl::{
>      PRIV_DATASTORE_AUDIT,
>      PRIV_DATASTORE_MODIFY,
> @@ -448,6 +450,54 @@ pub fn status(
>      crate::tools::disks::disk_usage(&datastore.base_path())
>  }
>  
> +#[api(
> +    input: {
> +        properties: {
> +            store: {
> +                schema: DATASTORE_SCHEMA,
> +            },
> +        },
> +    },
> +    returns: {
> +        type: DataStoreInfo,
> +    },
> +    access: {
> +        permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_READ, true),

why READ and not AUDIT | BACKUP ? why partial if you only pass a single 
privilege?

> +    },
> +)]
> +/// Get information about the datastore.
> +///
> +/// Provides PBS node fingerprint, address and datastore name
> +pub fn info(
> +    store: String,
> +    _info: &ApiMethod,
> +    _rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<DataStoreInfo, Error> {
> +    let _datastore = DataStore::lookup_datastore(&store)?;
> +    let cert = CertInfo::new()?;
> +    let fingerprint = cert.fingerprint()?;
> +
> +    // get all possible interface IP addresses since there's
> +    // no explicit way to tell which is needed
> +    let (config, _) = network::config()?;
> +    let mut address_list = Vec::new();
> +    for (_ , interface) in config.interfaces.iter() {
> +        if let Some(cidr) = &interface.cidr {
> +            address_list.push(cidr.to_owned());
> +        }
> +    }

doesn't this leak information that the user would/should not have access 
to? I mean, if I can do an API call I already have some way to reach the 
PBS server and we could just default to that on the client side.. 
possibly it would make sense to declare some interface as the 
'external/public' one and return that if configured, but just returning 
all addresses of all interfaces seems a bit much..

> +
> +    let result_item = DataStoreInfo {
> +        name: store,
> +        address_list,
> +        fingerprint,
> +    };
> +
> +    Ok(result_item)
> +}
> +
> +
> +
>  #[api(
>      input: {
>          properties: {
> @@ -1673,6 +1723,11 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
>          &Router::new()
>              .get(&API_METHOD_LIST_GROUPS)
>      ),
> +    (
> +        "info",
> +        &Router::new()
> +            .get(&API_METHOD_INFO)
> +    ),
>      (
>          "notes",
>          &Router::new()
> diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> index f97db557..9e61f15c 100644
> --- a/src/api2/types/mod.rs
> +++ b/src/api2/types/mod.rs
> @@ -1070,3 +1070,26 @@ pub struct APTUpdateInfo {
>      /// URL under which the package's changelog can be retrieved
>      pub change_log_url: String,
>  }
> +
> +#[api(
> +    properties: {
> +        "address-list": {
> +            description: "List of IPs from node",
> +            type: Array,
> +            items: {
> +                description: "CIDR",
> +                type: String,
> +            },
> +        },
> +})]
> +#[derive(Serialize, Deserialize)]
> +#[serde(rename_all = "kebab-case")]
> +/// Necessary information for adding a remote
> +pub struct DataStoreInfo {
> +    /// Name of the datastore
> +    pub name: String,
> +    /// Available IP addresses from the node
> +    pub address_list: Vec<String>,
> +    /// x509 fingerprint of the node
> +    pub fingerprint: String,
> +}
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
>