Fix TestHeadlessAuthenticationWatcher flaky test#23160
Merged
Conversation
espadolini
approved these changes
Mar 16, 2023
Contributor
espadolini
left a comment
espadolini
left a comment
There was a problem hiding this comment.
A 30x increase seems a bit drastic, does it actually take that long in practice? Should we put in effort to reduce how long it actually takes?
Contributor
Author
No, it should be much faster (<200ns on my machine), but since we aren't testing the context deadline in these tests, I figured a very conservative timeout would prevent any flakiness in the future. I'm surprised 500ms was too aggressive, I have to run almost 1000 tests simultaneously to reproduce the flakiness. I will just change it to 2 seconds for now though. |
Joerger
force-pushed
the
joerger/fix-flaky-test-TestHeadlessAuthenticationWatcher
branch
from
19a4719 to
500094f
Compare
… reduce flakiness.
Joerger
force-pushed
the
joerger/fix-flaky-test-TestHeadlessAuthenticationWatcher
branch
from
500094f to
67176ba
Compare
jakule
approved these changes
Mar 16, 2023
public-teleport-github-review-bot
Bot
removed the request for review
from atburke
github-merge-queue
Bot
removed this pull request from the merge queue due to no response for status checks
Joerger
deleted the
joerger/fix-flaky-test-TestHeadlessAuthenticationWatcher
branch
Joerger
added a commit
that referenced
this pull request
Apr 3, 2023
* RFD 105 - Headless Authentication (#21005) * Draft UX section. * Complete draft. * Minor edits. * Address comments, polish. * Condense headless login request into a single HTTP endpoint. * Update security section for limited certificate permissions. * Address doyensec comments. * Update RFD. * Remove certificate limitation from RFD scope; Add RFD number; smaller edits. * Small fixes. * * Update auth flow to use auth.AuthenticateSSHUser endpoint instead of CreateHeadlessAuthRequest and GenerateUserCerts endpoints * Remove CreateHeadlessAuthRequest rpc * Remove token and other unneeded fields from headless authentication * * Add resource watcher section * Don't insert backend data without authenticaion * Remove view headless requests page * Update diagram * Use the client's public key to derive a request ID. * Add HeadlessAuthentication protobuf type and Resource implementation. (#22350) * Add headless auth preference logic. (#22148) * Add Headless Authn backend service. (#22553) * Headless Login: add headless authentication resource watcher (#22699) * Add headless authentication resource watcher. * Handle OpInit event and Watcher errors. * Headless Login: proxy server changes (#22734) * * Add proper context handling to auth.AuthenticateUser. * Move PublicKey field to AuthenticateUserRequest where it can be used for actual authentication. * Use a simple switch statement in /webapi/ssh/certs logic to switch between password, otp, and eventually headless login. * Add Headless flow to /webapi/ssh/certs login enpdoint. * Add 3 minute callback timeout. * Headless Login: protobuf service (#22750) * Add Headless Authn proto server. * Add Headless Authn proto client. * Resolve comments. * Headless Login: tsh implementation (#22751) * * Implement tsh --headless * Implement tsh headless approve * Add better headless authn state handling. * Add godoc for new tsh field. * Headless login: Mlockall (#23159) * Use Mlockall for Headless login. * Skip memory lock on unsupported OSs. Resolve comments * Headless Login: auth server changes (#22726) * Add Headless Authn service. * Add/fix 3 minute headless login timeout. * * Prevent repeated updates to headless authentication state * Prevent user lock out from headless authentication failure * Delete headless authentication on failed attempts * Add auth_with_roles test. * Extend timeout in test to reduce flakiness. * Fix error typo. * Add context timeouts, remove initial GetHeadlessAuthentication call. * Resolve comments. * Move http client to it's own file; Add ability to clone HTTP client for per-request configuration changes. * Fix flaky test. * Remove shared state from test. * Update error handling and testing for auth_with_roles. * Fix rebase misshap. * Fix race condition in test. * update e ref * Fix ctx missing. * Extend test timeout to prevent flakiness. * Fix issue with roundtrip.ClientParams not being applied due to roundtripper wrapping. --------- Co-authored-by: Tim Ross <tim.ross@goteleport.com> * Extend context timeouts in TestHeadlessAuthenticationWatcher tests to reduce flakiness. (#23160) * Fix flaky test due to context deadline. (#23260) * Fix headless login with `second_factor: on | optional` (#23271) * Fix headless login with second_factor:on|optional. * Update ssh/certs endpoint to only configure necessary authentication fields; clarify comments; update test to cover headless authenticaiton preference. * Update test to cover user locking logic. * Change generic headless error. (#23331) * Headless SSO web endpoint and UI (#22914) * Update UI Update UI text Update the code to add headless request get Remove commented code Added simple UI and endpoints * Update UI Implement reject SSO handler and UI * Fix linter issues * Fix more linter issues * Fix UI tests * Use url.JoinPath. * center spinner on the page and animate it. * Address code review comments * Address code review comments * Renamed React component * Address PR comments --------- Co-authored-by: joerger <bjoerger@goteleport.com> Co-authored-by: Jeff Pihach <jeff.pihach@goteleport.com> * Fix flaky test `TestHeadlessAuthenticationWatcher` (#23417) * Fix race condition in test by using a helper function instead of complex channel mechanisms. * Avoid creating new methods solely for testing; resolve other comments. * Reuse more code; resolve other comments. * Fix race condition that could cause a new watcher to be marked as stale before the channel is consumed; Fix minor test issues. * Remove race condition on headless authentication expires field when (#23578) using memory storage. * Headless Authn: documentation (#23272) * Add docs. * Update docs/pages/access-controls/guides/headless-login.mdx Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> * Fix lint error. * Ellaborate on how headless login differs from standard login. * Resolve comments; Fix capitalization. * Resolves comments. * Add cli reference docs. * Restructure guide; Remove scoped blocks; Update descriptions; resolve other comments. * Make configuration options/alternatives collapsible; Fix typos. * Fix file names, titles, and make new config details begin as closed. * Fix hidden merge conflict. * Add line breaks. * Fix dead link. --------- Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> --------- Co-authored-by: Tim Ross <tim.ross@goteleport.com> Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> Co-authored-by: Jeff Pihach <jeff.pihach@goteleport.com> Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 500ms timeout was a bit aggressive, especially for
TestHeadlessAuthenticationWatcher/StaleCheck.