From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Christoph Heiss <c.heiss@proxmox.com>,
Markus Frank <m.frank@proxmox.com>
Cc: pmg-devel <pmg-devel@lists.proxmox.com>
Subject: Re: [pmg-devel] [PATCH pmg-api/pmg-gui v4 0/3] add default realm option and OIDC configuration panel
Date: Wed, 26 Mar 2025 08:41:20 +0100 [thread overview]
Message-ID: <804b4047-e0c1-447c-aab2-6a69e284afd6@proxmox.com> (raw)
In-Reply-To: <D8KZPW853GO0.3E6H7ZAY2QKNI@proxmox.com>
Am 20.03.25 um 10:36 schrieb Christoph Heiss:
> W.r.t patch #3: Extending the `AuthEditOpenId` panel from
> proxmox-widget-toolkit would probably be more work than its worth,
> FWICS? No hard feelings from my side, looking at the required changes,
> just that duplicating mostly-similar code is always bit of a PITA, if it
> can be avoided.
It's a trade-off and IME coupling is a much bigger and active PITA than
having some code duplicated.
I think we should consider moving bigger widgets to common libraries
like widget-toolkit on a case-by-case basis to ensure it actually brings
a net benefit and not lots of edge cases that are all relevant only for
a specific implementation in a product and needs to be chained through
multiple components.
Note that I do not propose that we should not share anything, but rather
prioritize sharing the smaller building blocks like fields and keep the
bigger ones that are only used once or twice in a product and just use
these smaller building blocks to create a local copy that targets the
specific capabilities of the product. Or, if two products not only use
basically the same backend but also share feature/implementation goals
then share between them but keep a dedicated local implementation for
another UI for a different product instead of adding chained-through
edge cases to the common implementation.
Anyway, this is definitively something that needs a rather nuanced view
and where there often is no very clear answer, but when integrating
Markus' OIDC implementation in PMG it noticed quite some friction
stemming from using the common bigger components.
> And there isn't any documentation about the role assignment feature yet,
> right? That should be done too, although a separate patch would be
> enough too IMO, in case you don't respin this series.
Yeah, that would be nice to have.
> Just a short explanation and mentioning the available values for the
> role assignment from an OIDC claim.
>
> In any case, please consider this series:
>
> Tested-by: Christoph Heiss <c.heiss@proxmox.com>
> Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
_______________________________________________
pmg-devel mailing list
pmg-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pmg-devel
prev parent reply other threads:[~2025-03-26 7:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-19 13:29 Markus Frank
2025-03-19 13:29 ` [pmg-devel] [PATCH pmg-api v4 1/3] Auth Plugin: stop forcing the default realm to be the pmg realm Markus Frank
2025-03-19 13:29 ` [pmg-devel] [PATCH pmg-gui v4 2/3] realms: enable default realm support Markus Frank
2025-03-19 13:29 ` [pmg-devel] [PATCH pmg-gui v4 3/3] add OIDC configuration panel for PMG Markus Frank
2025-03-20 9:36 ` [pmg-devel] [PATCH pmg-api/pmg-gui v4 0/3] add default realm option and OIDC configuration panel Christoph Heiss
2025-03-26 7:41 ` Thomas Lamprecht [this message]
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=804b4047-e0c1-447c-aab2-6a69e284afd6@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=c.heiss@proxmox.com \
--cc=m.frank@proxmox.com \
--cc=pmg-devel@lists.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal