[pve-devel] [PATCH access-control 1/2] acl: allow more nesting for /mapping acl paths

Lukas Wagner l.wagner at proxmox.com
Fri Nov 10 09:47:06 CET 2023



On 11/10/23 09:18, Thomas Lamprecht wrote:
> Am 07/11/2023 um 13:46 schrieb Lukas Wagner:
>> This will be needed for ACL paths for the notification system,
>> which will get separate namespaces for targets and matchers:
>>
>> /mapping/notification/targets/<name>
>> as well as
>> /mapping/notification/matchers/<name>
> 
> Not that it matters much to this supporting patch, but above should all
> use the singular, or? I.e., like "notification" also use "target" and
> "matcher".
> 

Yeah, I also was kind of unsure about that, but in the end I used the 
plural form because that's what I use for the API routes.

e.g.
/cluster/notifications/targets
/cluster/notifications/matchers


However, now I see another discrepancy I missed before, the API route 
also uses 'notifications' in its plural form.
So maybe it would make sense to have the ACL tree nodes match that 
exactlty? E.g.
/mapping/notifications/targets

I don't have any strong preference for any form, I just think
that some consistency with the API would be nice - and changing
the API routes would be much more work ;)

And regarding the granularity: Yes, maybe that's a bit overkill now. The 
per-target permissions were kind of important with the 'old' system 
where we would select a target at the notification call site (e.g. a 
backup job), but with the new 'pub-sub'-alike system it probably does 
not matter that much any more. But I don't really have any strong 
preference here as well.

-- 
- Lukas





More information about the pve-devel mailing list