Fix basic auth integration test#15679
Conversation
kfaraz
changed the title
abhishekrb19
left a comment
abhishekrb19
left a comment
There was a problem hiding this comment.
Thanks for the fix @kfaraz! I've one question, but I will merge the change to unblock IT failures in master
|
|
||
| // Add a delay to allow propagation of credentials to all services | ||
| try { | ||
| Thread.sleep(5000); |
There was a problem hiding this comment.
Do we know what changed here recently in auth land that this sleep is required? At any rate, I wonder if we could poll /status and/or issue a similar API call to verify things are setup instead of a static sleep.
There was a problem hiding this comment.
Yes, we recently introduced caching for basic auth in PR #15648 . That has sped up the authentication process triggering the flakiness in this test.
Only the coordinator exposes such APIs which can be used to fetch auth status, and indeed that API would return the expected response right away. The issue is the propagation of the creds from coordinator to other services (broker in this case) which may be delayed. Other services don't expose any API that would indicate the current set or version of auth creds that they have cached.
| try { | ||
| Thread.sleep(5000); | ||
| } | ||
| catch (Throwable t) { |
There was a problem hiding this comment.
| catch (Throwable t) { | |
| catch (InterruptedException e) { |
|
Thanks a lot for the review, @abhishekrb19 ! |
3 participants
Changes
ITBasicAuthConfigurationTestafter completing setup of users to allow propagation of credentials to other services.