Skip to content

Commit 040c4ab

Browse files
committed
Allow external uri to be configurable for components that support server 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 #12491
1 parent 22f61cf commit 040c4ab

File tree

3 files changed

+81
-21
lines changed

3 files changed

+81
-21
lines changed

java/src/org/openqa/selenium/grid/server/BaseServerFlags.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ public class BaseServerFlags implements HasRoles {
3333

3434
private static final String SERVER_SECTION = "server";
3535

36+
@Parameter(
37+
names = {"--external-uri"},
38+
description = "External URI where component is generally available. " +
39+
"Useful on complex network topologies when components are on different networks " +
40+
"and proxy servers are involved.")
41+
@ConfigValue(section = SERVER_SECTION, name = "external-uri", example = "http://10.0.1.1:33333")
42+
private String externalUri;
43+
3644
@Parameter(
3745
names = {"--host"},
3846
description = "Server IP or hostname: usually determined automatically.")

java/src/org/openqa/selenium/grid/server/BaseServerOptions.java

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -84,28 +84,38 @@ public int getMaxServerThreads() {
8484

8585
@ManagedAttribute(name = "Uri")
8686
public URI getExternalUri() {
87-
// Assume the host given is addressable if it's been set
88-
String host =
89-
getHostname()
87+
return config.get(SERVER_SECTION, "external-uri")
88+
.map(url -> {
89+
try {
90+
return new URI(url);
91+
} catch (URISyntaxException e) {
92+
throw new RuntimeException("Supplied external URI is invalid: " + e.getMessage(), e);
93+
}
94+
})
95+
.orElseGet(() -> {
96+
// Assume the host given is addressable if it's been set
97+
String host =
98+
getHostname()
9099
.orElseGet(
91-
() -> {
92-
try {
93-
return new NetworkUtils().getNonLoopbackAddressOfThisMachine();
94-
} catch (WebDriverException e) {
95-
String name = HostIdentifier.getHostName();
96-
LOG.info("No network connection, guessing name: " + name);
97-
return name;
98-
}
99-
});
100-
101-
int port = getPort();
102-
103-
try {
104-
return new URI(
105-
(isSecure() || isSelfSigned()) ? "https" : "http", null, host, port, null, null, null);
106-
} catch (URISyntaxException e) {
107-
throw new ConfigException("Cannot determine external URI: " + e.getMessage());
108-
}
100+
() -> {
101+
try {
102+
return new NetworkUtils().getNonLoopbackAddressOfThisMachine();
103+
} catch (WebDriverException e) {
104+
String name = HostIdentifier.getHostName();
105+
LOG.info("No network connection, guessing name: " + name);
106+
return name;
107+
}
108+
});
109+
110+
int port = getPort();
111+
112+
try {
113+
return new URI(
114+
(isSecure() || isSelfSigned()) ? "https" : "http", null, host, port, null, null, null);
115+
} catch (URISyntaxException e) {
116+
throw new ConfigException("Cannot determine external URI: " + e.getMessage());
117+
}
118+
});
109119
}
110120

111121
public boolean getAllowCORS() {

java/test/org/openqa/selenium/grid/server/BaseServerOptionsTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
package org.openqa.selenium.grid.server;
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
21+
import static org.junit.jupiter.api.Assertions.assertThrows;
2122

23+
import java.net.URI;
24+
import java.net.URISyntaxException;
2225
import java.util.Map;
2326
import org.junit.jupiter.api.Test;
2427
import org.openqa.selenium.grid.config.MapConfig;
@@ -43,4 +46,43 @@ void serverConfigBindsToHostByDefault() {
4346

4447
assertThat(options.getBindHost()).isEqualTo(true);
4548
}
49+
50+
@Test
51+
void externalUriFailsForNonUriStrings() {
52+
BaseServerOptions options =
53+
new BaseServerOptions(new MapConfig(Map.of("server", Map.of("external-uri", "not a uri"))));
54+
55+
Exception exception = assertThrows(RuntimeException.class, () -> {
56+
options.getExternalUri();
57+
});
58+
59+
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");
60+
}
61+
62+
@Test
63+
void externalUriTakesPriorityOverDefaults() throws URISyntaxException {
64+
URI expected = new URI("http://10.0.1.1:33333");
65+
66+
BaseServerOptions options =
67+
new BaseServerOptions(new MapConfig(Map.of("server", Map.of(
68+
"external-uri", expected.toString(),
69+
"host", "localhost",
70+
"port", 5555
71+
))));
72+
73+
assertThat(options.getExternalUri()).isEqualTo(expected);
74+
}
75+
76+
@Test
77+
void externalUriDefaultsToValueDerivedFromHostnameAndPortWhenNotDefined() throws URISyntaxException {
78+
URI expected = new URI("http://localhost:5555");
79+
80+
BaseServerOptions options =
81+
new BaseServerOptions(new MapConfig(Map.of("server", Map.of(
82+
"host", expected.getHost(),
83+
"port", expected.getPort()
84+
))));
85+
86+
assertThat(options.getExternalUri()).isEqualTo(expected);
87+
}
4688
}

0 commit comments

Comments
 (0)