release-2.1: server/ui: fix crash caused by race in server initialization #30838
Conversation
This file contains 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
HTTP requests are passed through an authentication mux, which checks that the user is authenticated based on the cookies on the request. To check authentication, the mux runs SQL to look up a session in the system.web_sessions table. Prior to this change, it was possible to hit the cluster with an HTTP request which would cause the SQL to be run, before the SQL system was fully initialized -- specifically before the cluster version setting was initialized. This would crash the node. This change delays installation of the authentication mux until after the SQL system has been initialized, removing the possibility of this crash. Release note: None
Teach the cluster-init test to reproduce the crash during bootup described in issue cockroachdb#25771. The trick is to send requests with a session cookie, as the admin UI would, while the cluster is waiting for init. Note that the crash was caused by the authenticationMux, which is used to protect secure clusters, but this test uses an insecure cluster. This is relying on a wart: an authenticationMux is installed for UI assets whether or not the cluster is secure. This could change in the future. A better test would use a secure cluster, however roachprod/roachtest do not easily support that at this time, nor is it easy to write a unit test that exercises the crash more robustly. Release note: None
Including - more comments - rename `maybeAuthMux` to `authenticatedUIHandler` - move `authHandler` down near where it's initialized Release note: None
|
This change is |
benesch
approved these changes
Oct 1, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Backport 3/3 commits from #30746.
/cc @cockroachdb/release
Fixes #25771
See commits for details.