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
Add support for additional paths for HomepageForwardingFilterConfig #2136
Conversation
@@ -58,7 +58,9 @@ public class AdminServerUiAutoConfigurationTest implements WithAssertions { | |||
public class ReactiveUiConfigurationTest { | |||
|
|||
private final ReactiveWebApplicationContextRunner contextRunner = new ReactiveWebApplicationContextRunner() | |||
.withPropertyValues("--spring.boot.admin.ui.available-languages=de", "--spring.webflux.base-path=test") | |||
.withPropertyValues("--spring.boot.admin.ui.available-languages=de", "--spring.webflux.base-path=test", | |||
"--spring.boot.admin.contextPath=test", "--spring.boot.admin.ui.additional-routes[0]=/info/**", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SBA context path is used for filtering in case of webflux application. I noticed that when added my testcases before implementation (tests didn't fail).
I'm not sure whether it's a bug or intended behaviour, so I leave the auto configuration logic as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you, there was a bug, should be fixed with #2141
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose, I should wait for that PR to be merged, rebase mine and then merge. Will that be ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes sounds good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
...test/java/de/codecentric/boot/admin/server/ui/config/AdminServerUiAutoConfigurationTest.java
Outdated
Show resolved
Hide resolved
* Additional routes for home page redirecting filter. Any request to these routes | ||
* will be redirected to home page internally | ||
*/ | ||
private List<String> additionalRoutes = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should additional routes be used for?
We understand the need to add excludes, but why includes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, that's a good question. I added them just like excludes and don't have an example. Maybe, it's better not add this property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok please remove them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
77c1641
to
74a2c8a
Compare
Fixed comments and rebased. |
Thank you! 👍 |
Closes #2135