Skip to content

Commit

Permalink
Allow external uri to be configurable for components that support ser…
Browse files Browse the repository at this point in the history
…ver functionality.

SE nodes might be behind some sort of proxy exposed to hub on a
different hostname(/ip) and/or port than component would by default
report themselves (e.g.: hub and nodes are in different k8s clusters
 and services are exposed via node ports).

Fixes SeleniumHQ#12491
  • Loading branch information
utamas committed Aug 10, 2023
1 parent 639737a commit 9dde64c
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 21 deletions.
8 changes: 8 additions & 0 deletions java/src/org/openqa/selenium/grid/server/BaseServerFlags.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ public class BaseServerFlags implements HasRoles {

private static final String SERVER_SECTION = "server";

@Parameter(
names = {"--external-uri"},
description = "External URI where component is generally available. " +
"Useful on complex network topologies when components are on different networks " +
"and proxy servers are involved.")
@ConfigValue(section = SERVER_SECTION, name = "external-uri", example = "http://10.0.1.1:33333")
private String externalUri;

@Parameter(
names = {"--host"},
description = "Server IP or hostname: usually determined automatically.")
Expand Down
52 changes: 31 additions & 21 deletions java/src/org/openqa/selenium/grid/server/BaseServerOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,28 +84,38 @@ public int getMaxServerThreads() {

@ManagedAttribute(name = "Uri")
public URI getExternalUri() {
// Assume the host given is addressable if it's been set
String host =
getHostname()
return config.get(SERVER_SECTION, "external-uri")
.map(url -> {
try {
return new URI(url);
} catch (URISyntaxException e) {
throw new RuntimeException("Supplied external URI is invalid: " + e.getMessage(), e);
}
})
.orElseGet(() -> {
// Assume the host given is addressable if it's been set
String host =
getHostname()
.orElseGet(
() -> {
try {
return new NetworkUtils().getNonLoopbackAddressOfThisMachine();
} catch (WebDriverException e) {
String name = HostIdentifier.getHostName();
LOG.info("No network connection, guessing name: " + name);
return name;
}
});

int port = getPort();

try {
return new URI(
(isSecure() || isSelfSigned()) ? "https" : "http", null, host, port, null, null, null);
} catch (URISyntaxException e) {
throw new ConfigException("Cannot determine external URI: " + e.getMessage());
}
() -> {
try {
return new NetworkUtils().getNonLoopbackAddressOfThisMachine();
} catch (WebDriverException e) {
String name = HostIdentifier.getHostName();
LOG.info("No network connection, guessing name: " + name);
return name;
}
});

int port = getPort();

try {
return new URI(
(isSecure() || isSelfSigned()) ? "https" : "http", null, host, port, null, null, null);
} catch (URISyntaxException e) {
throw new ConfigException("Cannot determine external URI: " + e.getMessage());
}
});
}

public boolean getAllowCORS() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@
package org.openqa.selenium.grid.server;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.net.URI;
import java.net.URISyntaxException;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.openqa.selenium.grid.config.MapConfig;
Expand All @@ -43,4 +46,43 @@ void serverConfigBindsToHostByDefault() {

assertThat(options.getBindHost()).isEqualTo(true);
}

@Test
void externalUriFailsForNonUriStrings() {
BaseServerOptions options =
new BaseServerOptions(new MapConfig(Map.of("server", Map.of("external-uri", "not a uri"))));

Exception exception = assertThrows(RuntimeException.class, () -> {
options.getExternalUri();
});

assertThat(exception.getMessage()).as("External URI must be parseable as URI.").isEqualTo("Supplied external URI is invalid: Illegal character in path at index 3: not a uri");
}

@Test
void externalUriTakesPriorityOverDefaults() throws URISyntaxException {
URI expected = new URI("http://10.0.1.1:33333");

BaseServerOptions options =
new BaseServerOptions(new MapConfig(Map.of("server", Map.of(
"external-uri", expected.toString(),
"host", "localhost",
"port", 5555
))));

assertThat(options.getExternalUri()).isEqualTo(expected);
}

@Test
void externalUriDefaultsToValueDerivedFromHostnameAndPortWhenNotDefined() throws URISyntaxException {
URI expected = new URI("http://localhost:5555");

BaseServerOptions options =
new BaseServerOptions(new MapConfig(Map.of("server", Map.of(
"host", expected.getHost(),
"port", expected.getPort()
))));

assertThat(options.getExternalUri()).isEqualTo(expected);
}
}

0 comments on commit 9dde64c

Please sign in to comment.