Skip to content

server: exempt healthcheck endpoint from authentication check#24945

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
vilterp:exempt-healthcheck
May 16, 2018
Merged

server: exempt healthcheck endpoint from authentication check#24945
craig[bot] merged 1 commit intocockroachdb:masterfrom
vilterp:exempt-healthcheck

Conversation

@vilterp
Copy link
Copy Markdown
Contributor

@vilterp vilterp commented Apr 19, 2018

Otherwise, you get the red "connection lost" banner until you log in.

This will have no effect until the auth mux is put on the request path (#24944).

Fixes #24942

@vilterp vilterp requested review from a team, couchand and mrtracy April 19, 2018 22:38
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@vilterp vilterp removed the request for review from a team April 19, 2018 22:39
@mrtracy
Copy link
Copy Markdown
Contributor

mrtracy commented Apr 21, 2018

This is not the correct way to do this. In server/server.go, you'll see a block that registers path/handler combinations with the top level http mux, with lines such as:

s.mux.Handle(statusPrefix, authHandler)
s.mux.Handle(authPrefix, gwMux)

adminPrefix currently goes to authHandler, but add this line:

s.mux.Handle("/_admin/v1/health", gwMux)

And it will bypass the auth handler for just that one endpoint.


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@couchand
Copy link
Copy Markdown
Contributor

:lgtm:


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@couchand
Copy link
Copy Markdown
Contributor

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented May 15, 2018

Build failed (retrying...)

@couchand
Copy link
Copy Markdown
Contributor

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented May 15, 2018

Build failed (retrying...)

@vilterp
Copy link
Copy Markdown
Contributor Author

vilterp commented May 15, 2018 via email

@couchand couchand force-pushed the exempt-healthcheck branch from 4d14bf3 to 7c2d6f4 Compare May 16, 2018 20:37
@couchand
Copy link
Copy Markdown
Contributor

Rebased off of #25195 and updated TestHealthAPI to reflect the change.

@couchand couchand force-pushed the exempt-healthcheck branch from 7c2d6f4 to f0eab2e Compare May 16, 2018 20:51
@couchand
Copy link
Copy Markdown
Contributor

bors r+

craig bot pushed a commit that referenced this pull request May 16, 2018
24945: server: exempt healthcheck endpoint from authentication check r=couchand a=vilterp

Otherwise, you get the red "connection lost" banner until you log in.

This will have no effect until the auth mux is put on the request path (#24944).

Fixes #24942 

25581: storage: fix use of context with closed trace r=a-robinson a=a-robinson

Fixes #25575

Release note: None

Co-authored-by: Pete Vilter <vilterp@cockroachlabs.com>
Co-authored-by: Alex Robinson <alexdwanerobinson@gmail.com>
@craig
Copy link
Copy Markdown
Contributor

craig bot commented May 16, 2018

Build succeeded

@craig craig bot merged commit f0eab2e into cockroachdb:master May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants