On Thu Jul 27, 2023 at 3:33 PM CEST, Christoph Heiss wrote: > Removes the dreaded DN regex, instead introducing a connect/bind check > on creation/update, aligning it with the way PBS does it. This is > enabled by default for new realms, but opt-in for existing, to not break > backwards-compatibility. > > Additionally, it has the benefit that instead of letting a sync fail on > the first try due to e.g. bad bind credentials, it gives the user some > direct feedback when trying to add/update a LDAP realm, if enabled. > > Should be rather a corner case, but it's the easiest way for us to > accomodate and the most versatile for users needing this. >
I think it would be enough to have the 'check-connection' parameter only for the API call itself, I wouldn't store it in the domains.cfg configuration file. That would imply that the 'Check configuration' checkbox that you introduce in the next patch could *always* be ticked, even for old realms. So whenever you create/update an LDAP/AD realm configuration you have to explicitly tell it "hey, I do not want the check right now, my LDAP server is down currently". My main point in making the check behavior opt-in rather was so that scripts/API consumers continue to work as before. For the GUI however, it should be fine to just always check by default, unless the behavior is explicitly turned off. > + }, > + 'check-connection' => { > + description => 'Check bind connection to LDAP server.', > + type => 'boolean', > + optional => 1, > + # TODO: Make it enabled-by-default with PVE 9.0? ^ This wouldn't be necessary any more. > + default => 0, > + }, > }; > } _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel