all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH access-control 1/2] acl: allow more nesting for /mapping acl paths
Date: Fri, 10 Nov 2023 09:47:06 +0100	[thread overview]
Message-ID: <8e0e37cd-3ff3-4070-b1e3-b5834d97ec52@proxmox.com> (raw)
In-Reply-To: <40f1858f-2294-4e60-9a93-319a5efdb4be@proxmox.com>



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




  reply	other threads:[~2023-11-10  8:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-07 12:46 Lukas Wagner
2023-11-07 12:46 ` [pve-devel] [PATCH manager 2/2] api: notifications: give targets and matchers their own ACL namespace Lukas Wagner
2023-11-10  8:18 ` [pve-devel] [PATCH access-control 1/2] acl: allow more nesting for /mapping acl paths Thomas Lamprecht
2023-11-10  8:47   ` Lukas Wagner [this message]
2023-11-13 15:41     ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8e0e37cd-3ff3-4070-b1e3-b5834d97ec52@proxmox.com \
    --to=l.wagner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal