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

Regression: Non-application roots no longer under application root #15488

Closed
knutwannheden opened this issue Mar 5, 2021 · 8 comments · Fixed by quarkusio/quarkusio.github.io#915
Labels
kind/bug Something isn't working

Comments

@knutwannheden
Copy link
Contributor

Describe the bug
In Quarkus 1.12.0 I could define quarkus.http.root-path=/foo and quarkus.http.non-application-root-path=/. The effect would be that the Swagger UI would be served under /foo/swagger-ui. Now with Quarkus 1.12.1 the same config leads to the Swagger UI being served as /swagger-ui.

This was tested with RESTEasy but not with RESTEasy Reactive.

Expected behavior
Since 1.12.1 is a patch release the Swagger UI should remain at /foo/swagger-ui using the configuration as described above.

Actual behavior
See issue description.

To Reproduce
Create a Quarkus application using the RESTEasy extension.

Configuration

quarkus.http.root-path=/foo
quarkus.http.non-application-root-path=/

Environment (please complete the following information):

  • Output of uname -a or ver:
  • Output of java -version: Java 11
  • GraalVM version (if different from Java):
  • Quarkus version or git rev: 1.12.1.Final
  • Build tool (ie. output of mvnw --version or gradlew --version): Maven 3.6.3

Additional context
This appears to be related to #15030.

@knutwannheden knutwannheden added the kind/bug Something isn't working label Mar 5, 2021
@gsmet
Copy link
Member

gsmet commented Mar 5, 2021

The situation in 1.12 was a bit of a mess so 1.12.1 fixes things that weren't working properly before. This could cause a change in behavior.

But I will let @ebullient confirm if it's the desired behavior or not.

@knutwannheden
Copy link
Contributor Author

I should also mention that in my configuration I have quarkus.swagger-ui.path=/swagger-ui. Possibly removing the leading slash there resolves my issue.

@knutwannheden
Copy link
Contributor Author

If I want to restore the behavior of 1.12.0 I have to set:

quarkus.http.root-path=/foo
quarkus.http.non-application-root-path=${quarkus.http.root-path}
quarkus.swagger-ui.path=swagger-ui

It appears like Quarkus behaves differently now with leading slashes for these paths. If I want the non-application root to be the same as the application root I have to set it to the same value (as a lone / doesn't work anymore). Should it possibly be allowed to set quarkus.http.non-application-root-path to the empty string?

@ebullient
Copy link
Member

ebullient commented Mar 5, 2021

yes. i have a blog post in flight to explain this. It is best to be explicit with your intentions in config. If you want them to be the same, set them to be the same, either using the value explicitly, or using the variable as you have done.

Yes, leading slashes have meaning now (relative vs absolute paths). This was important to allow all of the configuration options people wanted.

I need to finish the blog post, so we can link to it from docs, but the config reference has been updated to explain this. Let me know if it needs more detail. So I guess I would say that this isn’t a bug, it is an expected result of some intentional changes to make things more consistent and predictable. Would a blog post be sufficient to resolve this issue?

@knutwannheden
Copy link
Contributor Author

I like the changes, as it makes things more consistent.

This is an excerpt from the blog announcing the new release:

1.12.1.Final is a maintenance release fixing bugs and improving the documentation.

1.12.1.Final is a safe upgrade for everyone using Quarkus 1.12.

Given this I would have expected a change like this (for non-application root path, etc.) in a minor release, not a patch release, since it requires me to change the configuration.

I need to finish the blog post, so we can link to it from docs, but the config reference has been updated to explain this. Let me know if it needs more detail. So I guess I would say that this isn’t a bug, it is an expected result of some intentional changes to make things more consistent and predictable. Would a blog post be sufficient to resolve this issue?

Yes. And a note in the 1.12 migration guide would also be nice. Thanks!

@ebullient
Copy link
Member

Draft here, if you would like to review. quarkusio/quarkusio.github.io#915

@knutwannheden
Copy link
Contributor Author

Looks great! A link from the 1.12 migration guide would probably also make sense.

@ebullient
Copy link
Member

Yes. We will link as soon as it's live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants