Pull Req: Script to run more robust by continuing on specific errors that affect only individual users.

[BOW-el]

Right now, the script aborts sync whenever an error, for instance upon user creation is discovered. This leads to potentially many new users not being added until the cause is discovered.

I am suggesting to let the script continue for specific errors for which the cause can be clearly led to misconfiguration of single accounts in the AD. In this example, the script continues to run whenever an error occurs that hints that the user's email address is already in use (due to two accounts having the same email in the AD). The creation of this individual user is skipped and the script continues with the next user. The script still aborts for every other error (I might add other errors I stumble upon later).

[Adambean]

Thanks for your contribution BOW-el. I think this condition will be situation specific, and not widespread enough to relax for general purpose. Such behaviour should be an option, and off as standard.

A user not being created due to their email address being taken by someone else will be a serious problem for some, finding that they can't sign in because someone else has (possibly wrongly) had their email address. This is going to inappropriately flag up invalid logins, but then again, if the email address exists more than once why should the first instance of it through an LDAP search iteration implicitly take priority over any other iterations. -- On balance it would be more unusual in a directory for multiple user objects representing real people to share the same email address.

not being added until the cause is discovered

On the contrary I would suggest that the script immediately and gracefully stopping with a non-zero exit code would be more of an attention grabber than glossing over it.

If a lot of users are missing the administrator is either going to notice the big shortfall in the user count figure, or they will get significantly more complaints than say just one person complaining, thus the administrator is going to bother checking out the cause at a higher priority. The reason would be an immediate indication to correct their directory or exclude a person, along with running the script again interactively to ensure nobody else requires attention.

As this scales to directories with hundreds of users and beyond glossing over an error may not be seen until the particular person effected complains and the administrator searches through the potentially very long logs to find out why. After all they don't necessarily know the cause to be duplicate email address, only that a particular person can't login because "some reasons". The script exiting with a non-zero error code would either be noticed by the person running it interactively (as the error message is going to be at/near the end of their console), or be picked up by a task scheduler mechanism as a failed run (such as a failed CRON dispatching an email to the runner's mailbox or nominated email address).

[BOW-el]

Yep, I’m catching your line of thought here, and would be fully in line with this being a ‘hidden’ tweak or an optional setting. For our specific use case, dual email use is common, but I’ll look into your idea of a cron job sending an email when the script encounters an error. Do you happen to have that running and a working cron template for this? How do you avoid spamming the admins?

[Adambean]

but I’ll look into your idea of a cron job sending an email when the script encounters an error

You don't need to do this. It's a built in mechanism of the Linux CRON application to dispatch an email for any textual output, and something a Linux user would configure on a system account basis. E.g.

SHELL=/bin/bash
MAILTO=gitlab@example.com # Custom email address if the account's own mailbox isn't wanted

* * * * * /usr/bin/php ~/gitlab-ldap-sync/bin/console ldap:sync -vv # This sends everything
* * * * * /usr/bin/php ~/gitlab-ldap-sync/bin/console ldap:sync -vv >/dev/null # This sends errors only
* * * * * /usr/bin/php ~/gitlab-ldap-sync/bin/console ldap:sync -vv >/dev/null 2>&1 # This won't send errors either

[Adambean]

I think you may have sent your VSCode commit to the wrong issue? That's more a general point than specific to non-atomic operation, though happy to accept VSCode workspace. (This has gone to master.)

Edit: Could just be me not being used to how Github amalgamates an individuals concurrent changes to a single location.

[Adambean]

I've merged in your change for this issue on the basis that it requires explicit consent, which should satisfy us meeting in the middle.

To opt in to this behaviour you just need to include command option --continueOnFail.

[BOW-el]

I think you may have sent your VSCode commit to the wrong issue? That's more a general point than specific to non-atomic operation, though happy to accept VSCode workspace. (This has gone to master.)

Edit: Could just be me not being used to how Github amalgamates an individuals concurrent changes to a single location.

Jep, that was by mistake, thought I had pushed to my own repo without creating a request. However, glad this works :)

[BOW-el]

I've merged in your change for this issue on the basis that it requires explicit consent, which should satisfy us meeting in the middle.

To opt in to this behaviour you just need to include command option --continueOnFail.

perfect, that works for us, thank you for your help!