Print localhost instead of 0.0.0.0 in serverListeningMessage when in dev/test mode under WSL #40497
Conversation
There was a problem hiding this comment.
Thanks for opening the PR. I'm not sure this patch is quite right but it raises interesting questions.
Let's wait for @radcortez and @cescoffier to chime in.
| */ | ||
| private boolean isWSL() { | ||
| var sysEnv = System.getenv(); | ||
| return sysEnv.containsKey("IS_WSL") || sysEnv.containsKey("WSL_DISTRO_NAME"); |
There was a problem hiding this comment.
@dmlloyd @radcortez I wonder if it's something we should include in SmallRye Common (OS maybe?)?
There was a problem hiding this comment.
A safer check is to look for Microsoft in /proc/version based on some quick research.
There was a problem hiding this comment.
I created smallrye/smallrye-common#311 .
...ons/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/HttpHostConfigSource.java
Outdated
Show resolved
Hide resolved
...nsions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java
Outdated
Show resolved
Hide resolved
| @@ -45,19 +45,11 @@ public String getValue(String propertyName) { | |||
| // In dev-mode we want to only listen on localhost so others on the network cannot connect to the application. | |||
| // However, in WSL this would result in the application not being accessible, | |||
| // so in that case, we launch it on all interfaces. | |||
| return (LaunchMode.current().isDevOrTest() && !isWSL()) ? "localhost" : ALL_INTERFACES; | |||
| return (LaunchMode.current().isDevOrTest() && !LaunchMode.isWSL()) ? "localhost" : ALL_INTERFACES; | |||
There was a problem hiding this comment.
Also not related to your patch but it seems weird we only override quarkus.http.host as there are other problematic values as seen in the VertxHttpRecorder extract below.
I think it makes sense to default to 0.0.0.0 in the WSL Linux case for most cases. WDYT @cescoffier @radcortez ?
|
@tamaro-skaljic could you clarify what you're trying to do here? My understanding is that in the case of WSL, we want to listen on all interfaces for things to work. What you are reporting is that with WSL, you cannot access the Quarkus app with |
@gsmet Of course! I created this issue here #40463 which was fixed through #40467. But for me, the issue was more a matter of (practical) developer experience than documentation (=theoretical developer experience). When starting a quarkus application, it displays the URL to access the application in the start up message. As a developer that's a nice feature because you can click on the URL in your Terminal and it's opened in your default browser. Unfortunately because of HttpHostConfigSource.java#L46 the host isn't set to localhost under WSL in dev/test mode. This results in the following URL displayed:
Unfortunately, the word 'listening' in the startup message is then no longer correct (under WSL dev/test) - as we are actually listening on 0.0.0.0. It's more like 'accessible in your browser on:' Caution Applications, including the amazon-lambda extension (AbstractLambdaPollLoop.java#L76) and the test-framework (LauncherUtil.java#L251) depend on this log message and I'm sure some developers will use it too for health checks and so on... At least i've seen this in the wild. |
|
@tamaro-skaljic OK, I better see your point. I need to think a bit about it as I think there are multiple things at play here and we need to take them all into account. I'll have a closer look on Monday. |
There was a problem hiding this comment.
Hey there,
Sorry it took some time for me to think about it.
I agree with you that it would be better to make the link clickable. I also think that WSL networking is a bit of a corner case and it's not that big of a deal if we display localhost instead of 0.0.0.0 there. But we should only do it if the host is actually 0.0.0.0.
From what I could see in https://stackoverflow.com/questions/49835559/access-a-web-server-which-is-running-on-wsl-windows-subsystem-for-linux-from-t, even listening to 0.0.0.0 will only expose the service to the host when using WSL but maybe you could check?
Thanks for having a look, I think it's important to also have a seamless DevEx with WSL!
...nsions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/VertxHttpRecorder.java
Outdated
Show resolved
Hide resolved
|
Ah, also, there is a conflict now due to the changes made by @radcortez here: https://github.com/quarkusio/quarkus/pull/40337/files . So you will have to adjust your patch a bit. |
Is related to #22956 and #40463.