-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rule: /-/reload
is inaccessible on v0.12.1 when no prefix has been specified
#2514
Comments
Does this problem also exist in the previous release? Maybe we need some tests to cover these endpoints. Like https://github.com/prometheus/prometheus/blob/84b4d079c8714be8e8ad071a35b0391df270364c/web/web_test.go#L92 |
I was hitting the exact same issue!
If I'm correct, it's been introduced by 48c41eb so only since v0.12.0. |
Valid for #2524 as well |
Great idea, we should file a separate issue for that, I think 👍 |
Fixed 👍 |
Relevant code part:
https://github.com/thanos-io/thanos/blob/master/cmd/thanos/rule.go#L553-L564
I wrote this in kind of pseudo-code, I hope that's understandable.
The prefix after all of the wrapping that we use for API end-points is
/{USER_STRING}api/v1
. In the/-/reload
case after all of the wrapping there, the prefix that we use when registering a route is/{USER_STRING}
. Then, the/-/reload
is registered on top of that prefix. The crux of the issue is that the prefix, in this case, ends with a/
when noUSER_STRING
has been specified leading to a 404 when/-/reload
is accessed.Testing on
v0.12.1
:With a simple change to:
It works again:
But this "fix" doesn't handle the case when the user passes us some string. If some string has been passed then the route needs to be with the prefix
/
. If no string has been passed then it needs to be without the prefix/
.Or just remove this
router.WithPrefix(webRoutePrefix)
part on that line because it is already handled by us previously.The text was updated successfully, but these errors were encountered: