Re: [pve-devel] [PATCH pve-access-control v2 1/1] permissions: add ACL paths for SDN fabrics

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 07.04.25 um 10:51 schrieb Stefan Hanreich:
> On 4/7/25 10:12, Thomas Lamprecht wrote:
>> Am 07.04.25 um 09:24 schrieb Fabian Grünbichler:
>>> On April 4, 2025 7:20 pm, Thomas Lamprecht wrote:
>>>> So, without looking at the implementation, fabrics have the IDs unique
>>>> per sub-type? Could maybe also share an ID space, less confusion
>>>> potential, but naturally also less flexibility – what do you think?
>>>
>>> they share a section config (and thus ID-space), so I guess we could
> 
> There's one section config file per fabric, so its ID is unique per
> protocol. We'd need to load *all* configuration files everytime we
> change one configuration file (at least when adding a fabric) so we can
> validate unique IDs across all fabric types.

I faintly remember some lunch talks, but what was the story behind the
per type configs again? The per-node mappings?

Can you post some sample configs for my convenience, or point to them
if these patches here contain some (e.g., for testing), so that I (or
someone other core maintainer) can take a closer look at this part.


> Since we have plans of moving over the existing IS-IS + BGP controllers,
> as well as a new WireGuard fabric, we'd have to load and parse 5
> different configuration files for cross-validation then.
> 
>> Hmm, ok no real value for allowing a user use-access on such a fabric
>> (for now), and not sure if it is useful to allow certain admins to create
>> an openfabric fabric but not an ospf one (if the implementation even
>> uses such fine-grained checks)
> 
> I think that in practice, admins would create the fabrics and then only
> hand out permissions to edit that specific fabric, without handing out
> the permissions to create fabrics at all.
> 
> It might make sense for Wireguard (where the possible implications
> aren't nearly as bad as with e.g. BGP), but even then I think my
> previous point applies that you probably don't want to hand out blanket
> permissions for a whole subtype, in case you want to make heavy use of ACLs.
> 
> The current schema is more of a direct result of how the section config
> is structured, since IDs can be duplicated across different protocols,
> so you need the additional distinguisher in the ACL path.

With the configs you describe it probably indeed make sense to have
per-type ACL base paths, that said, I'd like to revisit the design
again with more time at hand; IMO this is a bit to short notice and
to big bags to hold for us to rush it in now.

As of now I'd pretty certain that I'll only get around taking a
closer look post releases, so this might be probably be a better fit
for the next major release in a two to three months. Or would you
prefer getting this in now to acquire feedback but then having to
potentially rework the config/acl including migrations in a bigger
way? Slapping some tech-preview label on it might set expectations
roughly right for users in that case. I cannot give you a promise
here but if you think it should go in and promise me to work on
such a transformation then I will set some time aside to take a
serious look today. But please consider whether a new feature at
the end of the new feature release cycle is worth the effort for
all of us. 

A side-benefit of waiting might be that you get around to also
move bgp and is-is over too until then, ensuring the config/api
design is really a good fit for that too.


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