Skip to content
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

cmd: rule: do not wrap reload endpoint with '/' #2515

Closed

Conversation

GiedriusS
Copy link
Member

Do not wrap the router with / on the /-/reload endpoint. Otherwise,
it is inaccessible when no prefix has been specified by the user.

Fixes #2514.

Signed-off-by: Giedrius Statkevičius giedriuswork@gmail.com

Do not wrap the router with `/` on the `/-/reload` endpoint. Otherwise,
it is inaccessible when no prefix has been specified by the user.

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>
Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>
@simonpasquier
Copy link
Contributor

LGTM. Shouldn't it be against release-012 and then backported to master?

@GiedriusS
Copy link
Member Author

LGTM. Shouldn't it be against release-012 and then backported to master?

I haven't checked whether this bug was introduced in 0.12 before creating this, sorry. But either order should work albeit with more effort on the person who is doing the release. I'm fine with reopening it to a different branch. @squat care to take a look?

@lilic
Copy link
Contributor

lilic commented Apr 27, 2020

Should we add a test for reload that it picks up new rules, wdyt?

@squat
Copy link
Member

squat commented Apr 27, 2020

@GiedriusS let's make this PR to the release-0.12 branch to we can cut a 0.12.2 with this fix in it, yeah?

@bwplotka
Copy link
Member

👍

Some e2e test for rule reload would be nice indeed.

@GiedriusS
Copy link
Member Author

👍 I will open up a PR against release-0.12 with a simple test-case.

@GiedriusS GiedriusS closed this Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rule: /-/reload is inaccessible on v0.12.1 when no prefix has been specified
5 participants