Assign random ports in Config

[radcortez]

• Fixes Assign random ports at configuration start (not later) #42986

It assigns a random port during configuration and overrides the ports configuration (when they are 0) with the assigned random port, meaning that any access to the port configuration reflects the requested port without any workarounds.

[github-actions]

🎊 PR Preview 5c73d3c has been successfully built and deployed to https://quarkus-pr-main-43220-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[geoand]

This is really nice, but the CI failures seem related

[cescoffier]

This approach is not going to work, there are several things missing:

• 0 and negative ports are random, BUT every server starting on 0/-1/-2 has the same one (like if you use 0 in gRPC and gRPC client, it's the same port
• the whole rest assured support has been dropped - that's why we have these system properties.
• the socker open and immediately closed followed by a bind is likely going to fail or be very brittle.

[cescoffier]

Could you open issues for all these extensions to ask them to switch to the new interface-based model?

(Damned, I'm involved in all 3 :-))

[radcortez]

Sure, but I'm happy to do that.

I have all the vertx-http work already done, but I haven't pushed it yet because many extensions rely on the config object to obtain quarkus.http.port, and this will be a big breaking change.

To move that forward, I think we need to agree on this random port implementation, and then come up with a specific API (that I've intended to proposed separately), to retrieve useful things. Something like:

class QuarkusRuntime {
  static Integer httpPort()
  ...
}

Ideally, we should offer this API before the vert-x config changes so that users who have to move can use the new API instead.

[cescoffier]

No that is not correct,

Port 0 is a random port, but everything with port 0 uses the same
Port -1 is a random port, but everything with port -1 uses the same
...

[cescoffier]

and that's across all extensions.

[radcortez]

I didn't notice that the client was in a separate group. Let me fix that.

[cescoffier]

You cannot do that, that's super brittle.
OS may close things asynchronously, the bind on the same port may fail.

[radcortez]

Locally and in the CI, things seem to have worked.

What do you suggest to do instead?

[cescoffier]

What I proposed in the issue somewhere:

• a config source that provides the properties for 0, -1, -2...
• they get the port as you do but add a grace period (configurable)
• the config source does not recompute for a given "random id"

Another tricky thing is that I would like to have a way to keep the current behavior somehow because it got hardened enough.

[radcortez]

Ok, I didn't notice that we have some special meaning with port numbers here:

quarkus/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/options/HttpServerOptionsUtils.java

Lines 48 to 61 in b41b6ed

	/**
	* When the http port is set to 0, replace it by this value to let Vert.x choose a random port
	*/
	public static final int RANDOM_PORT_MAIN_HTTP = -1;
	/**
	* When the https port is set to 0, replace it by this value to let Vert.x choose a random port
	*/
	public static final int RANDOM_PORT_MAIN_TLS = -2;
	/**
	* When the management port is set to 0, replace it by this value to let Vert.x choose a random port
	*/
	public static final int RANDOM_PORT_MANAGEMENT = -3;

I can implement that behaviour. What threw me off, is that when I was trying the current version and set 0 to port and ssl-port I would get two different values.

Another tricky thing is that I would like to have a way to keep the current behavior somehow because it got hardened enough.

Hum... we could add a flag to use new / old behaviour.

[radcortez]

This is really nice, but the CI failures seem related

Yes, but not related to the main changes. One is an inject I added that does not work in native, and the other is a config update in the rest client test because I've removed the workaround we had for the URL reload. I'll fix those.

[radcortez]

• the whole rest assured support has been dropped - that's why we have these system properties.

It still works. REST Assured relies on Config for setup:

quarkus/test-framework/common/src/main/java/io/quarkus/test/common/RestAssuredURLManager.java

Lines 67 to 119 in be7d0e7

	public static void setURL(boolean useSecureConnection, Integer port, String additionalPath) {
	if (!REST_ASSURED_PRESENT) {
	return;
	}
	oldPort = RestAssured.port;
	if (port == null) {
	port = useSecureConnection ? getPortFromConfig(DEFAULT_HTTPS_PORT, "quarkus.http.test-ssl-port")
	: getPortFromConfig(DEFAULT_HTTP_PORT, "quarkus.lambda.mock-event-server.test-port",
	"quarkus.http.test-port");
	}
	RestAssured.port = port;
	oldBaseURI = RestAssured.baseURI;
	final String protocol = useSecureConnection ? "https://" : "http://";
	String host = ConfigProvider.getConfig().getOptionalValue("quarkus.http.host", String.class)
	.orElse("localhost");
	if (host.equals("0.0.0.0")) {
	host = "localhost";
	}
	RestAssured.baseURI = protocol + host;
	oldBasePath = RestAssured.basePath;
	Optional<String> basePath = ConfigProvider.getConfig().getOptionalValue("quarkus.http.root-path",
	String.class);
	if (basePath.isPresent() || additionalPath != null) {
	StringBuilder bp = new StringBuilder();
	if (basePath.isPresent()) {
	if (basePath.get().startsWith("/")) {
	bp.append(basePath.get().substring(1));
	} else {
	bp.append(basePath.get());
	}
	if (bp.toString().endsWith("/")) {
	bp.setLength(bp.length() - 1);
	}
	}
	if (additionalPath != null) {
	if (!additionalPath.startsWith("/")) {
	bp.append("/");
	}
	bp.append(additionalPath);
	if (bp.toString().endsWith("/")) {
	bp.setLength(bp.length() - 1);
	}
	}
	RestAssured.basePath = bp.toString();
	}
	Duration timeout = ConfigProvider.getConfig()
	.getOptionalValue("quarkus.http.test-timeout", Duration.class).orElse(Duration.ofSeconds(30));
	configureTimeouts(timeout);
	}

The system properties trick was just a way to mutate system properties and have Config get the updated value. The idea here is to provide a source upfront with the expected random port.

[cescoffier]

We may need to add a grace period here.
I remember that windows can be tricky.
(optional, user runtime config)

[radcortez]

My understanding is that if you set serverSocket.setReuseAddress(true); if will allow you to bind it again even if the socket is in timeout: https://docs.oracle.com/javase/8/docs/api/java/net/ServerSocket.html#setReuseAddress-boolean-

Or are you thinking about something else?

[cescoffier]

It starts to look good.

We need to see if we can have a way to restore the previous behavior (with a flag or something, to ease migration).

[radcortez]

We need to see if we can have a way to restore the previous behavior (with a flag or something, to ease migration).

From the user / consumer side there is no migration required. Is the concern that this may not work and that we have something to fall back to?

My concern is that some of the fixes that we need are incompatible with the current implementation, and we can't do the fixes if we have such flag :)

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 2c3e19e.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 2c3e19e.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

[cescoffier]

Unfortunately, I'm pretty sure that the new way can fail in some environments. So, we need a way to go back to the previous behavior where the OS gives the port during the bind and not "leased". It's just a safety net.

[radcortez]

Unfortunately, I'm pretty sure that the new way can fail in some environments. So, we need a way to go back to the previous behavior where the OS gives the port during the bind and not "leased". It's just a safety net.

Do you know which environments may fail so we can have a look?

I understand the concern. On the other hand, if we are required to have such a safety net, then I feel there is no point in this PR. We would have to keep all our workarounds with this on top. Plus, any fixes that we need to do require awareness of both modes, and unfortunately, the current mode is incompatible with the fixes we need, hence the PR.

Other options to explore:

• Use a custom source for the port and set the port directly. We will still need the workarounds to look up the configuration referencing the port as an expression. We can assume a few places (like the REST Client url example), but we cannot control where a user sets ${quarkus.http.port} (we could provide a warning). The user must do that manually for any occurrences we don't handle. At least we would control the source more easily than the current system properties by providing a proper ordinal, and we may be able to include / exclude it as required.

• Old Roots / Mappings are populated at the very start (hence the problem). We could introduce another phase (RUNTIME_PREPARE or something similar), that includes the Vertx configuration (and any other configuration required), do all the port binding and additional operations, and then populate the remaining configuration. This does seem like an overkill.

[radcortez]

@dmlloyd @gsmet do you have other ideas?

[cescoffier]

Windows has always been the tricky one, of course. It may have improved, but we cannot discard the fact that old windows would still need seconds after the close to release the resources.

[radcortez]

Windows has always been the tricky one, of course. It may have improved, but we cannot discard the fact that old windows would still need seconds after the close to release the resources.

Windows CI runs, seem happy :)

Did you check https://docs.oracle.com/javase/8/docs/api/java/net/ServerSocket.html#setReuseAddress-boolean-?

[cescoffier]

Yes, I already tried that when I had the issue, no luck. But that was a long time ago. I would still enable it, as it may prevent the issue.

If we merge this, we need to be aware that we might have to revert abruptly if it makes random port unusable on some platforms.

[radcortez]

We can add a temporary configuration to revert to the old behavior if something goes wrong and hold some additional fixes. If there are issues, users set that configuration, and we avoid having to revert abruptly. The goal would be to have this mode only if there are no issues.

Is this what you were advocating? I was under the impression that you preferred to keep the current mode permanently.

[dmlloyd]

@dmlloyd @gsmet do you have other ideas?

This is definitely the wrong approach. Configuration is a one-way street for good reason. Also, Clement is completely correct that the OS is assigning these ports taking into account multiple factors that we cannot predict, and we therefore cannot safely emulate this behavior without introducing problems that will require workarounds which in turn will lead to more problems.

This is all an end-run around what seems to be the fundamental problem of not having a good way to get information out of the running system in a uniform way. Let's solve that actual problem instead. Configuration is not the right tool for this. Perhaps we need an analogous management API instead.

I'll also add one comment on this:

Locally and in the CI, things seem to have worked.

I hope we are all very, very clear that "running in CI" is not the same as "this is correct" or even "this is a good idea". There's no reason we can't come up with a solution here that is guaranteed to work 100% of the time.

[radcortez]

This is all an end-run around what seems to be the fundamental problem of not having a good way to get information out of the running system in a uniform way. Let's solve that actual problem instead. Configuration is not the right tool for this. Perhaps we need an analogous management API instead.

We did discuss this some time ago, and I agree. On the other hand, I think it is reasonable to expect that ${quarkus.http.port} works as any other expansion.

All is fine when I set it to a fixed value. We may argue that we are not getting the correct runtime value, but I'm explicitly saying: "I want to use this port" and I'm configuring my server this way, so any check for the configuration must return the same values as the server.

When I delegate the configuration to the server (as in the case of quarkus.http.port=0), then yes, I shouldn't be getting the port from quarkus.http.port because we need to mutate the value, but we still expect that expansions of ${quarkus.http.port} work correctly.

Even with a management API, we have two problems to solve:

• How to expand ${quarkus.http.port}
• How to communicate the actual port for the configuration that needs it (remember, it is already populated before we know the real port).

We could disallow ${quarkus.http.port}, but it is a use case that works perfectly fine when we don't have a random port. It would be confusing to say: "Well, it works in this case, and not in that other case," or "To make it consistent for every case, you can't use it.". I can't think of a good solution here at the moment, so I hope we can discuss this further to see if we can come up with something good.

For the second part is more or less solvable by another config phase (yacks!). Alternatively, we would have to selectively reload configurations that may contain the port (like we did for the REST Client and other places), knowing that the port can be referenced in other configurations that we are not reloading.

[dmlloyd]

This is all an end-run around what seems to be the fundamental problem of not having a good way to get information out of the running system in a uniform way. Let's solve that actual problem instead. Configuration is not the right tool for this. Perhaps we need an analogous management API instead.

We did discuss this some time ago, and I agree. On the other hand, I think it is reasonable to expect that ${quarkus.http.port} works as any other expansion.

It does! But the confusion is the meaning of this. What it means - in fact - is "the port number that was configured". It does not mean "the port number being used".

When I delegate the configuration to the server (as in the case of quarkus.http.port=0), then yes, I shouldn't be getting the port from quarkus.http.port because we need to mutate the value, but we still expect that expansions of ${quarkus.http.port} work correctly.

The expectation is what is wrong. The configured value should always be returned.

Even with a management API, we have two problems to solve:

• How to expand ${quarkus.http.port}

You don't.

• How to communicate the actual port for the configuration that needs it (remember, it is already populated before we know the real port).

If there is a configuration which can either be an explicit port number (i.e. socket address) or the port number (i.e. socket address) of an existing service, then the configuration property type should not be SocketAddress or int, it should be a type which represents either a socket address (explicit) or a symbolic name (either from an enumerated list or open-ended syntax) which represents the service. Expressions are not the right thing for this. This is the idea I want to convey. It's the wrong tool for the job.

We could disallow ${quarkus.http.port}, but it is a use case that works perfectly fine when we don't have a random port.

It works fine when you're referencing other configuration, which is what expressions are for; but in this case we don't want to reference configuration, we want to reference the run time reality. It may well be that the run time reality reflects the configuration in, say, 80% of cases (which I wouldn't call "perfect"). But that doesn't make it the right tool. It's the right tool when it works in 100% of cases.

[radcortez]

If we agree to stop using quarkus.http.port to display the real port, I'm up for that. I can come up with an alternate proposal to fix the issues we have been discussing.