Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions java/src/org/openqa/selenium/grid/node/ActiveSession.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,19 @@ public interface ActiveSession extends HttpHandler {

Dialect getDownstreamDialect();

/**
* Indicates whether the browser backing this session runs in a separate environment from the Node
* process (for example, a Docker container, a Kubernetes Pod, or a relayed remote endpoint) and
* therefore does not share the Node's local filesystem. When {@code true}, file upload and
* download commands must be forwarded to the session so files are written to (or read from) the
* environment where the browser actually runs, instead of being handled on the Node's own
* filesystem.
*
* @return {@code true} if file transfer commands must be forwarded to the browser environment
*/
default boolean isRemoteFileSystem() {
return false;
}

void stop();
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ public class DockerSession extends DefaultActiveSession {
Require.nonNull("Video container stop timeout", videoContainerStopTimeout);
}

@Override
public boolean isRemoteFileSystem() {
// The browser runs inside a Docker container that does not share the Node's filesystem, so
// file upload and download commands must be forwarded to the container.
return true;
}

@Override
public void stop() {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ public class KubernetesSession extends DefaultActiveSession {
this.portForward = portForward;
}

@Override
public boolean isRemoteFileSystem() {
// The browser runs in a separate Kubernetes Job Pod that does not share the Node Pod's
// filesystem, so file upload and download commands must be forwarded to the browser Pod.
return true;
}

@Override
public void stop() {
LOG.info(String.format("Stopping session, deleting K8s Job: %s/%s", namespace, jobName));
Expand Down
17 changes: 10 additions & 7 deletions java/src/org/openqa/selenium/grid/node/local/LocalNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@
import org.openqa.selenium.grid.node.NodeCommandInterceptor;
import org.openqa.selenium.grid.node.SessionFactory;
import org.openqa.selenium.grid.node.config.NodeOptions;
import org.openqa.selenium.grid.node.docker.DockerSession;
import org.openqa.selenium.grid.security.Secret;
import org.openqa.selenium.internal.Debug;
import org.openqa.selenium.internal.Either;
Expand Down Expand Up @@ -901,10 +900,11 @@ private static HttpResponse callUnchecked(Callable<HttpResponse> callable) {

@Override
public HttpResponse downloadFile(HttpRequest req, SessionId id) {
// When the session is running in a Docker container, the download file command
// needs to be forwarded to the container as well.
// When the browser runs in a separate environment from the Node (e.g. a Docker container, a
// Kubernetes Pod, or a relayed remote endpoint), the download file command needs to be
// forwarded to that environment, since the Node does not share a filesystem with the browser.
SessionSlot slot = currentSessions.getIfPresent(id);
if (slot != null && slot.getSession() instanceof DockerSession) {
if (slot != null && slot.getSession() != null && slot.getSession().isRemoteFileSystem()) {
return executeWebDriverCommand(req);
}
if (!this.managedDownloadsEnabled) {
Expand Down Expand Up @@ -1109,10 +1109,13 @@ private HttpResponse deleteDownloadedFile(File downloadsDirectory) {
@Override
public HttpResponse uploadFile(HttpRequest req, SessionId id) {

// When the session is running in a Docker container, the upload file command
// needs to be forwarded to the container as well.
// When the browser runs in a separate environment from the Node (e.g. a Docker container, a
// Kubernetes Pod, or a relayed remote endpoint), the upload file command needs to be forwarded
// to that environment, since the Node does not share a filesystem with the browser. Otherwise
// the file would be written to the Node's filesystem and be unreachable from the browser when
// sendKeys runs.
SessionSlot slot = currentSessions.getIfPresent(id);
if (slot != null && slot.getSession() instanceof DockerSession) {
if (slot != null && slot.getSession() != null && slot.getSession().isRemoteFileSystem()) {
return executeWebDriverCommand(req);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,16 @@ public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sess
upstream,
stereotype,
mergedCapabilities,
Instant.now()) {});
Instant.now()) {
@Override
public boolean isRemoteFileSystem() {
// The relay forwards the session to an external endpoint (for example a cloud
// provider or an Appium device farm) that does not share the Node's filesystem, so
// file upload and download commands must be forwarded to that endpoint rather than
// handled on the Node's own filesystem.
return true;
}
});
} catch (Exception e) {
span.setAttribute(AttributeKey.ERROR.getKey(), true);
span.setStatus(Status.CANCELLED);
Expand Down
162 changes: 162 additions & 0 deletions java/test/org/openqa/selenium/grid/node/local/LocalNodeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,21 @@
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.openqa.selenium.remote.Dialect.W3C;
import static org.openqa.selenium.remote.http.HttpMethod.GET;
import static org.openqa.selenium.remote.http.HttpMethod.POST;

import java.io.UncheckedIOException;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.Callable;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
Expand All @@ -51,7 +56,10 @@
import org.openqa.selenium.grid.data.NodeStatus;
import org.openqa.selenium.grid.data.Session;
import org.openqa.selenium.grid.data.Slot;
import org.openqa.selenium.grid.node.ActiveSession;
import org.openqa.selenium.grid.node.BaseActiveSession;
import org.openqa.selenium.grid.node.Node;
import org.openqa.selenium.grid.node.SessionFactory;
import org.openqa.selenium.grid.security.Secret;
import org.openqa.selenium.grid.testing.EitherAssert;
import org.openqa.selenium.grid.testing.TestSessionFactory;
Expand Down Expand Up @@ -545,6 +553,160 @@ public HttpResponse intercept(SessionId id, HttpRequest req, Callable<HttpRespon
.containsExactly("outer-before", "inner-before", "inner-after", "outer-after");
}

@Test
void uploadFileIsForwardedWhenSessionUsesRemoteFileSystem() throws URISyntaxException {
Tracer tracer = DefaultTestTracer.createTracer();
EventBus bus = new GuavaEventBus();
URI uri = new URI("http://localhost:1234");
Capabilities caps = new ImmutableCapabilities("browserName", "cheese");

RecordingSessionFactory factory = new RecordingSessionFactory(uri, caps, true);
LocalNode remoteFsNode =
LocalNode.builder(tracer, bus, uri, uri, registrationSecret).add(caps, factory).build();

SessionId id = createSession(remoteFsNode, caps);

HttpRequest uploadReq = new HttpRequest(POST, "/session/" + id + "/se/file");
HttpResponse response = remoteFsNode.uploadFile(uploadReq, id);

// The command must be forwarded to the session (browser environment) rather than written to the
// Node's own filesystem. Docker and Kubernetes sessions both report a remote filesystem.
assertThat(factory.session.lastRequest).isNotNull();
assertThat(factory.session.lastRequest.getUri()).isEqualTo("/session/" + id + "/se/file");
assertThat(response.getHeader("X-Forwarded")).isEqualTo("true");
}

@Test
void downloadFileIsForwardedWhenSessionUsesRemoteFileSystem() throws URISyntaxException {
Tracer tracer = DefaultTestTracer.createTracer();
EventBus bus = new GuavaEventBus();
URI uri = new URI("http://localhost:1234");
Capabilities caps = new ImmutableCapabilities("browserName", "cheese");

RecordingSessionFactory factory = new RecordingSessionFactory(uri, caps, true);
LocalNode remoteFsNode =
LocalNode.builder(tracer, bus, uri, uri, registrationSecret).add(caps, factory).build();

SessionId id = createSession(remoteFsNode, caps);

HttpRequest downloadReq = new HttpRequest(GET, "/session/" + id + "/se/files");
HttpResponse response = remoteFsNode.downloadFile(downloadReq, id);

assertThat(factory.session.lastRequest).isNotNull();
assertThat(factory.session.lastRequest.getUri()).isEqualTo("/session/" + id + "/se/files");
assertThat(response.getHeader("X-Forwarded")).isEqualTo("true");
}

@Test
void fileCommandsAreHandledLocallyForSessionsSharingTheNodeFilesystem()
throws URISyntaxException {
Tracer tracer = DefaultTestTracer.createTracer();
EventBus bus = new GuavaEventBus();
URI uri = new URI("http://localhost:1234");
Capabilities caps = new ImmutableCapabilities("browserName", "cheese");

// A plain session (e.g. a static Grid Node) shares the Node's filesystem, so file commands are
// handled locally and are never forwarded to the session.
RecordingSessionFactory factory = new RecordingSessionFactory(uri, caps, false);
LocalNode localFsNode =
LocalNode.builder(tracer, bus, uri, uri, registrationSecret).add(caps, factory).build();

SessionId id = createSession(localFsNode, caps);

// Managed downloads are disabled, so a local download attempt fails here instead of forwarding.
assertThatExceptionOfType(WebDriverException.class)
.isThrownBy(
() ->
localFsNode.downloadFile(new HttpRequest(GET, "/session/" + id + "/se/files"), id))
.withMessageContaining("enable-managed-downloads");
assertThat(factory.session.lastRequest).isNull();
}

private static SessionId createSession(LocalNode node, Capabilities caps) {
Either<WebDriverException, CreateSessionResponse> response =
node.newSession(new CreateSessionRequest(Set.of(W3C), caps, emptyMap()));
if (response.isLeft()) {
throw new AssertionError("Unable to create session: " + response.left().getMessage());
}
return response.right().getSession().getId();
}

/**
* An {@link ActiveSession} that records the last request it was asked to execute, so tests can
* assert whether a file command was forwarded to the session. Its {@link #isRemoteFileSystem()}
* value is configurable to emulate both remote (Docker/Kubernetes) and local (static Grid Node)
* sessions.
*/
private static class RecordingActiveSession extends BaseActiveSession {
private final boolean remoteFileSystem;
private volatile HttpRequest lastRequest;

RecordingActiveSession(
SessionId id,
URL url,
Capabilities stereotype,
Capabilities capabilities,
boolean remoteFileSystem) {
super(id, url, W3C, W3C, stereotype, capabilities, Instant.now());
this.remoteFileSystem = remoteFileSystem;
}

@Override
public boolean isRemoteFileSystem() {
return remoteFileSystem;
}

@Override
public void stop() {
// Do nothing.
}

@Override
public HttpResponse execute(HttpRequest req) throws UncheckedIOException {
this.lastRequest = req;
return new HttpResponse().setHeader("X-Forwarded", "true");
}
}

private static class RecordingSessionFactory implements SessionFactory {
private final URI uri;
private final Capabilities stereotype;
private final boolean remoteFileSystem;
private volatile RecordingActiveSession session;

RecordingSessionFactory(URI uri, Capabilities stereotype, boolean remoteFileSystem) {
this.uri = uri;
this.stereotype = ImmutableCapabilities.copyOf(stereotype);
this.remoteFileSystem = remoteFileSystem;
}

@Override
public Capabilities getStereotype() {
return stereotype;
}

@Override
public boolean test(Capabilities capabilities) {
return true;
}

@Override
public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) {
SessionId id = new SessionId(UUID.randomUUID());
URL url;
try {
url = uri.toURL();
} catch (MalformedURLException e) {
throw new UncheckedIOException(e);
}
RecordingActiveSession created =
new RecordingActiveSession(
id, url, stereotype, sessionRequest.getDesiredCapabilities(), remoteFileSystem);
this.session = created;
return Either.right(created);
}
}

private void waitUntilNodeStopped(SessionId sessionId) {
long timeout = Duration.ofSeconds(5).toMillis();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,58 @@ void doesNotForwardClientRemoteUrlToTheRemoteEnd() {
assertThat(result.right().getCapabilities().getCapability("se:remoteUrl"))
.isEqualTo("http://localhost:4444");
}

@Test
void relaySessionReportsRemoteFileSystemSoFileCommandsAreForwarded() {
String fakeSessionId = UUID.randomUUID().toString();

Map<String, Object> responsePayload =
Map.of(
"value",
Map.of(
"sessionId",
fakeSessionId,
"capabilities",
Map.of("browserName", "chrome", "platformName", "android")));

Route route =
Route.post("/session")
.to(
() ->
req -> {
HttpResponse response = new HttpResponse();
response.setStatus(200);
response.setContent(Contents.asJson(responsePayload));
return response;
});

PassthroughHttpClient.Factory clientFactory = new PassthroughHttpClient.Factory(route);
Tracer tracer = DefaultTestTracer.createTracer();

Capabilities stereotype =
new ImmutableCapabilities("browserName", "chrome", "platformName", "android");

RelaySessionFactory factory =
new RelaySessionFactory(
tracer,
clientFactory,
Duration.ofSeconds(300),
URI.create("http://localhost:4723"),
null,
"",
stereotype);

Capabilities requestCaps =
new ImmutableCapabilities("browserName", "chrome", "platformName", "android");

CreateSessionRequest sessionRequest =
new CreateSessionRequest(Set.of(Dialect.W3C), requestCaps, Map.of());

Either<WebDriverException, ActiveSession> result = factory.apply(sessionRequest);

assertThat(result.isRight()).isTrue();
// The browser runs on an external endpoint that does not share the Node's filesystem, so
// LocalNode must forward file upload/download commands to it instead of handling them locally.
assertThat(result.right().isRemoteFileSystem()).isTrue();
}
}
Loading