Re: [pbs-devel] [PATCH proxmox-backup] fix #5190: api-types: openid acr format regex

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

Am 06/02/2024 um 09:59 schrieb Gabriel Goller:
> On Mon Feb 5, 2024 at 4:45 PM CET, Thomas Lamprecht wrote:
>> on it's own the change itself looks relatively OK, what I'm missing is
>> a more direct reference to what the issue was in the report, as while
>> your example closely matches that, it would be still great to actually
>> mention this explicitly.
> In the commit messages or as a comment above the regex?

That would IMO go in the commit message, as it's meta information
quite relevant for git history, but rather noise for the current
code.

>> The other thing is that the reporter writes that it works fine for
>> Proxmox VE already, so what's the limitation there, does your patch
>> aligns it to that, and if not it would great to state why you chose
>> a different approach (which can be fine, but really should be reasoned
>> about)
> Hmm AFAIK on pve we don't have any limitation at all, it just has to be
> a string. It's probably best if I copy the same regex to pve although I
> don't want to suddenly have user input rejected.
> The pve regex would get stricter, thus it would be a breaking change.

It would not break existing setups, so it rather would be for adding
a new realm, or updating that part of an existing one only though?

That I'd see OK to go for the `"break" it, release it and see if some
one complains things` route.
E specially as it's not really a strict regex, any complaint would
mean that the PBS one needs to be further relaxed too.