[pbs-devel] [PATCH proxmox-backup] add datastore info api call
Oguz Bektas
o.bektas at proxmox.com
Thu Oct 22 11:17:29 CEST 2020
hi,
On Thu, Oct 22, 2020 at 10:02:23AM +0200, Fabian Grünbichler wrote:
>
> why READ and not AUDIT | BACKUP ? why partial if you only pass a single
> privilege?
i thought the minimum privilege should be view. one might want to add a
datastore where only read access is given to them, to be able to restore
backups from it for example. imposing audit/backup privs would prevent
this, afaict
>
> > + },
> > +)]
> > +/// 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..
yes, i wasn't sure how to handle this since in PVE we just take the
corosync link but here it can be any interface.
i do like the suggestion to declare an interface the "public" one.
but there could be multiple interfaces being utilized as well (like f.e.
if the server has 2 addresses on two different subnets, with different
datastores). then it would make things harder.
i'm open to different suggestions.
>
> > +
> > + 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 at lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> >
> >
> >
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
More information about the pbs-devel
mailing list