Re: [pbs-devel] [PATCH v5 proxmox-backup 06/31] api types: implement remote acl path method for sync job

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

On October 25, 2024 2:46 pm, Christian Ebner wrote:
> On 10/25/24 13:44, Fabian Grünbichler wrote:
>> On October 18, 2024 10:42 am, Christian Ebner wrote:
>>> Add `remote_acl_path` method which generates the acl path from the sync
>>> job configuration. This helper allows to easily generate the acl path
>>> from a given sync job config for privilege checks.
>>>
>>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>>> ---
>>> changes since version 4:
>>> - no changes
>>>
>>> changes since version 3:
>>> - not present in previous version
>>>
>>>   pbs-api-types/src/jobs.rs | 13 +++++++++++++
>>>   1 file changed, 13 insertions(+)
>>>
>>> diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
>>> index 868702bc0..3971dd49f 100644
>>> --- a/pbs-api-types/src/jobs.rs
>>> +++ b/pbs-api-types/src/jobs.rs
>>> @@ -594,6 +594,19 @@ impl SyncJobConfig {
>>>               None => vec!["datastore", &self.store],
>>>           }
>>>       }
>>> +
>>> +    pub fn remote_acl_path(&self) -> Option<Vec<&str>> {
>>> +        let remote = self.remote.as_ref()?;
>>> +        let mut remote_acl_path = vec!["remote", remote, &self.remote_store];
>> 
>> this could/should use BackupNamespace::acl_path
> 
> `BackupNamespace::acl_path` would however add the additional `datastore` 
> component?
> 
> So instead of the pre-existing `/remote/{remote}/{store}` schema for 
> remote datastore acls, this would create a 
> `/remote/{remote}/datastore/{store}` for e.g. the root namespace, not 
> being backwards compatible.
> 
> Therefore I did not use the pre-existing helper. Or did you intend for 
> the helper to be adapted accordingly, by e.g. passing an optional remote?

ah right, missed that.. that's probably not worth it just for the two
call sites.. I guess we could just replace the first member of the Vec,
but that also seems a bit risky in case of an overhaul of the ACL paths
down the line.. so let's keep this (mostly) as it is :)

> 
>>> +
>>> +        if let Some(namespace) = self.remote_ns.as_ref() {
>>> +            if !namespace.is_root() {
>>> +                let comp: Vec<&str> = namespace.components().collect();
>>> +                remote_acl_path.extend(comp);

and just drop the extra copy via collect here and extend directly from
the components iterator?

>>> +            }
>>> +        }
>>> +        Some(remote_acl_path)
>>> +    }
>>>   }
>>>   
>>>   #[api(
>>> -- 
>>> 2.39.5
>>>
>>>
>>>
>>> _______________________________________________
>>> pbs-devel mailing list
>>> pbs-devel@lists.proxmox.com
>>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>>
>>>
>>>
>> 
>> 
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>> 
>> 
> 
> 


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel