Skip to content

Commit 9bf1af1

Browse files
diemoldiemol
committed
[grid] Forwarding CDP connections to the right place
When having a Dynamic Grid, and a user who wants to do CDP over Grid, we have the situation that the browser is not running on the same host where the Node receives the original CDP request. In this case, we are having a Node running on one host, and the browser in a different host/container. For this, a forward url is being added to identify this situation and redirect the request to the place where the browser is running. All this because it is not possible to access to the browser's debugger address remotely. Fixes #9803
1 parent 1cdc5a1 commit 9bf1af1

2 files changed

Lines changed: 64 additions & 16 deletions

File tree

‎java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java‎

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717

1818
package org.openqa.selenium.grid.node;
1919

20+
import static org.openqa.selenium.internal.Debug.getDebugLogLevel;
21+
import static org.openqa.selenium.remote.http.HttpMethod.GET;
22+
2023
import com.google.common.collect.ImmutableSet;
2124

2225
import org.openqa.selenium.Capabilities;
@@ -41,14 +44,13 @@
4144
import java.util.function.Consumer;
4245
import java.util.logging.Level;
4346
import java.util.logging.Logger;
44-
45-
import static org.openqa.selenium.internal.Debug.getDebugLogLevel;
46-
import static org.openqa.selenium.remote.http.HttpMethod.GET;
47+
import java.util.stream.Stream;
4748

4849
public class ProxyNodeWebsockets implements BiFunction<String, Consumer<Message>,
4950
Optional<Consumer<Message>>> {
5051

5152
private static final UrlTemplate CDP_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/cdp");
53+
private static final UrlTemplate FWD_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/fwd");
5254
private static final UrlTemplate VNC_TEMPLATE = new UrlTemplate("/session/{sessionId}/se/vnc");
5355
private static final Logger LOG = Logger.getLogger(ProxyNodeWebsockets.class.getName());
5456
private static final ImmutableSet<String> CDP_ENDPOINT_CAPS =
@@ -66,16 +68,20 @@ public ProxyNodeWebsockets(HttpClient.Factory clientFactory, Node node) {
6668

6769
@Override
6870
public Optional<Consumer<Message>> apply(String uri, Consumer<Message> downstream) {
71+
UrlTemplate.Match fwdMatch = FWD_TEMPLATE.match(uri);
6972
UrlTemplate.Match cdpMatch = CDP_TEMPLATE.match(uri);
7073
UrlTemplate.Match vncMatch = VNC_TEMPLATE.match(uri);
7174

72-
if (cdpMatch == null && vncMatch == null) {
75+
if (cdpMatch == null && vncMatch == null && fwdMatch == null) {
7376
return Optional.empty();
7477
}
7578

76-
String sessionId = cdpMatch != null ?
77-
cdpMatch.getParameters().get("sessionId") :
78-
vncMatch.getParameters().get("sessionId");
79+
String sessionId = Stream.of(fwdMatch, cdpMatch, vncMatch)
80+
.filter(Objects::nonNull)
81+
.findFirst()
82+
.get()
83+
.getParameters()
84+
.get("sessionId");
7985

8086
LOG.fine("Matching websockets for session id: " + sessionId);
8187
SessionId id = new SessionId(sessionId);
@@ -89,10 +95,20 @@ public Optional<Consumer<Message>> apply(String uri, Consumer<Message> downstrea
8995
Capabilities caps = session.getCapabilities();
9096
LOG.fine("Scanning for endpoint: " + caps);
9197

92-
if (cdpMatch != null) {
98+
if (vncMatch != null) {
99+
return findVncEndpoint(downstream, caps);
100+
}
101+
102+
// This match happens when a user wants to do CDP over Dynamic Grid
103+
if (fwdMatch != null) {
104+
LOG.info("Matched endpoint where CDP connection is being forwarded");
93105
return findCdpEndpoint(downstream, caps);
94106
}
95-
return findVncEndpoint(downstream, caps);
107+
if (caps.getCapabilityNames().contains("se:forwardCdp")) {
108+
LOG.info("Found endpoint where CDP connection needs to be forwarded");
109+
return findForwardCdpEndpoint(downstream, caps);
110+
}
111+
return findCdpEndpoint(downstream, caps);
96112
}
97113

98114
private Optional<Consumer<Message>> findCdpEndpoint(Consumer<Message> downstream,
@@ -109,6 +125,18 @@ private Optional<Consumer<Message>> findCdpEndpoint(Consumer<Message> downstream
109125
return Optional.empty();
110126
}
111127

128+
private Optional<Consumer<Message>> findForwardCdpEndpoint(Consumer<Message> downstream,
129+
Capabilities caps) {
130+
// When using Dynamic Grid, we need to connect to a container before using the debuggerAddress
131+
try {
132+
URI uri = new URI(String.valueOf(caps.getCapability("se:forwardCdp")));
133+
return Optional.of(uri).map(cdp -> createWsEndPoint(cdp, downstream));
134+
} catch (URISyntaxException e) {
135+
LOG.warning("Unable to create URI from: " + caps.getCapability("se:forwardCdp"));
136+
return Optional.empty();
137+
}
138+
}
139+
112140
private Optional<Consumer<Message>> findVncEndpoint(Consumer<Message> downstream,
113141
Capabilities caps) {
114142
String vncLocalAddress = (String) caps.getCapability("se:vncLocalAddress");

‎java/src/org/openqa/selenium/grid/node/docker/DockerSessionFactory.java‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.openqa.selenium.Capabilities;
2828
import org.openqa.selenium.Dimension;
2929
import org.openqa.selenium.ImmutableCapabilities;
30+
import org.openqa.selenium.PersistentCapabilities;
3031
import org.openqa.selenium.RetrySessionRequestException;
3132
import org.openqa.selenium.SessionNotCreatedException;
3233
import org.openqa.selenium.TimeoutException;
@@ -211,7 +212,11 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
211212

212213
SessionId id = new SessionId(response.getSessionId());
213214
Capabilities capabilities = new ImmutableCapabilities((Map<?, ?>) response.getValue());
214-
Capabilities mergedCapabilities = capabilities.merge(sessionRequest.getDesiredCapabilities());
215+
Capabilities mergedCapabilities = sessionRequest.getDesiredCapabilities().merge(capabilities);
216+
mergedCapabilities = addForwardCdpEndpoint(mergedCapabilities,
217+
containerIp,
218+
port,
219+
id.toString());
215220

216221
Container videoContainer = null;
217222
Optional<DockerAssetsPath> path = ofNullable(this.assetsPath);
@@ -235,10 +240,10 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
235240

236241
span.addEvent("Docker driver service created session", attributeMap);
237242
LOG.fine(String.format(
238-
"Created session: %s - %s (container id: %s)",
239-
id,
240-
capabilities,
241-
container.getId()));
243+
"Created session: %s - %s (container id: %s)",
244+
id,
245+
mergedCapabilities,
246+
container.getId()));
242247
return Either.right(new DockerSession(
243248
container,
244249
videoContainer,
@@ -254,6 +259,22 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
254259
}
255260
}
256261

262+
private Capabilities addForwardCdpEndpoint(Capabilities sessionCapabilities, String containerIp,
263+
int port, String sessionId) {
264+
// We add this endpoint to go around the situation where a user wants to do CDP over
265+
// Dynamic Grid. In a conventional Grid setup, this is not needed because the browser will
266+
// be running on the same host where the Node is running. However, in Dynamic Grid, the Docker
267+
// Node is running on a different host/container. Therefore, we need to forward the websocket
268+
// connection to the container where the actual browser is running.
269+
String forwardCdpPath = String.format(
270+
"ws://%s:%s/session/%s/se/fwd",
271+
containerIp,
272+
port,
273+
sessionId);
274+
return new PersistentCapabilities(sessionCapabilities)
275+
.setCapability("se:forwardCdp", forwardCdpPath);
276+
}
277+
257278
private Container createBrowserContainer(int port, Capabilities sessionCapabilities) {
258279
Map<String, String> browserContainerEnvVars = getBrowserContainerEnvVars(sessionCapabilities);
259280
long browserContainerShmMemorySize = 2147483648L; //2GB
@@ -378,8 +399,7 @@ private void saveSessionCapabilities(Capabilities sessionRequestCapabilities, St
378399
Paths.get(path, "sessionCapabilities.json"),
379400
capsToJson.getBytes(Charset.defaultCharset()));
380401
} catch (IOException e) {
381-
LOG.log(Level.WARNING,
382-
"Failed to save session capabilities", e);
402+
LOG.log(Level.WARNING, "Failed to save session capabilities", e);
383403
}
384404
}
385405

0 commit comments

Comments
 (0)