Add missing read:metrics scope for authenticated metrics endpoint
#3770
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
and ensure token auth is accepted
consideRatio
approved these changes
Jan 18, 2022
There was a problem hiding this comment.
This LGTM!
Go for a merge? I was a bit hesitant because there is a CI/CD build running still and it's a very recently opened PR.
I'm not sure what i think either about about minor vs patch version bump and backporting etc, but I lean towards thinking its fine to see this as a bugfix and go for a patch release.
|
I'd also maybe lean toward patch, as it's "strictly less broken" than other 2.0.x versions. I'll write up 2.0.3 with this tomorrow. |
|
Not sure how to label this, went for both labels. |
minrk
changed the title
read:metrics scope for metrics endpointread:metrics scope for authenticated metrics endpoint
Merged
|
I'm getting this issue on jupyterhub 2.3.1, any idea why this would be happening? Due to this its not hub isn't able to reach to single server and terminating after being spawned (about a min approx)
|
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.
and ensure token auth is accepted (missed this handler in #3686).
closes #3769
I'm reticent to release this PR as 2.0.3 because it defines a new scope. However, authenticated prometheus simply doesn't work right now (#3769), so one could argue that the 2.0.x bug is that the scope for authenticated metrics was missing.
we could also backport just the one-line
_accept_token_auth = Trueto 2.0.x, to allow any authenticated token to access metrics, as was the pre-2.0 behavior, but then 2.1 will be adding the new scope. Arguably doing so would break more things, because there would be a version that actually works without the scope.