[JENKINS-70035] Use User.getById() instead of User.get()#194
[JENKINS-70035] Use User.getById() instead of User.get()#194Kevin-CB wants to merge 1 commit intojenkinsci:masterfrom
Conversation
| containsString(WILL_TRY_NEXT_CONFIGURATION), | ||
| containsString(planetExpress.getUrl())), | ||
| CoreMatchers.<Throwable>instanceOf(UserMayOrMayNotExistException2.class))); | ||
| CoreMatchers.<Throwable>instanceOf(AuthenticationServiceException.class))); |
There was a problem hiding this comment.
Sorry, but I do not remember if the specific exception type here is important (I would assume it does not matter though, and I doubt that the multi-server mode is used very frequently anyways). What is the full stack trace in the logs before and after the change? Maybe the only difference is that there is a new outermost exception?
There was a problem hiding this comment.
UserMayOrMayNotExistException2 seems important as it is used to say "I can not tell you that this users exists. or not" and as such can allow the use of a token (wheras another excpetion would be treated as the form - this user does not exist).
At the very least I think some manual testing with user impersonation needs to occur when the server is down (before and after this change)
3 participants
You can find more information in JENKINS-70035
I had to adapt LdapMultiEmbeddedTest#when_first_is_unavailable_and_ignorable_and_login_on_second_then_login & LdapMultiEmbeddedTest#when_first_is_wrong_and_ignorable_and_login_on_second_then_login to match
AuthenticationServiceExceptionand notUserMayOrMayNotExistException2because they were not thrown anymore.As I did not fully understand their uses, I would ask you to review this part carefully to make sure that I didn't broke them.