Re: [pbs-devel] [PATCH proxmox-backup v2 11/15] docs: user-management: add section about AD realm support

[Christoph Heiss <c.heiss@proxmox.com>]

Thanks for the review!

On Tue, Nov 28, 2023 at 09:33:02AM +0100, Fabian Grünbichler wrote:
>
> nit: the domains.cfg docs currently have:
>
> > You can use the proxmox-backup-manager openid and proxmox-backup-manager ldap commands to manipulate this file.
>
> in them, that might warrant adding the 'ad' command as well.

Ack.

> [..]
> > -in the LDAP realm configuration dialog window in the GUI and via the
> > -``proxmox-backup-manager ldap create/update`` command.
> > +Active Directory
> > +~~~~~~~~~~~~~~~~
> > +
> > +Proxmox Backup Server can also utilize external Microsoft Active Directory
> > +servers for user authentication.
> > +To achieve this, a realm of the type ``ad`` has to be configured.
> > +
> > +For an Active Directory realm, the authentication domain name and the server
> > +address must be specified. Most options from :ref:`_user_realms_ldap` apply to
>
> this ref doesn't work for me because it should be :ref:`user_realms_ldap` (without the leading '_')
>
> `make html` prints
>
> /home/fgruenbichler/Sources/proxmox-backup/docs/user-management.rst:658: WARNING: undefined label: '_user_realms_ldap'

Interesting .. I'll fix it, good catch.

>
> - maybe we could add a check for that to make such things a build error?

Makes sense, TBH. I see what I can do, if I'm already at it.

> [..]
> > +
> > +User Synchronization in LDAP/AD realms
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +It is possible to automatically sync users for LDAP and AD-based realms, rather
> > +than having to add them to Proxmox VE manually. Synchronization options can be
>
> that ends up being propagated here ;)

Ack.

>
> > +set in the LDAP realm configuration dialog window in the GUI and via the
> > +``proxmox-backup-manager ldap/ad create/update`` command.
>
> not sure I like that style, IMHO a command should be in a format that
> allows copying if possible. in this case, we could just refer to
>
> with the ``proxmox-backup-manager ldap`` and ``proxmox-backup-manager
> ad`` commands
>
> if I copy and paste that, I get the usage list with the relevant sub
> commands and parameters.
>
> [..]
> > -be started via the ``proxmox-backup-manager ldap sync`` command.
> > +be started via the ``proxmox-backup-manager ldap/ad sync`` command.
>
> same here, IMHO splitting the two commands makes it more user friendly.

I will "de-duplicate" and reword the above paragraphs as appropriately,
thanks!