Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JEP-222] WebSocket-based agents #357

Merged
merged 39 commits into from
Jan 14, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a5f4f7a
Add a WebSocket agent client.
jglick Nov 13, 2019
944b358
Reworked protocol to negotiate remote capabilities.
jglick Nov 21, 2019
43c6978
Comments.
jglick Nov 21, 2019
e4fe758
Simplifying handshake to use HTTP headers.
jglick Nov 21, 2019
12afe79
SpotBugs
jglick Nov 22, 2019
89397b1
Merge branch 'publish-incrementals' into websocket
jglick Nov 26, 2019
abd931b
Merge branch 'master' into websocket
jglick Dec 6, 2019
561713b
Add a -webSocket launch option.
jglick Dec 10, 2019
c877936
Better comment.
jglick Dec 10, 2019
ae63bd9
Misleading Javadoc.
jglick Dec 10, 2019
2f77265
Handling some close and error methods.
jglick Dec 10, 2019
aefc7b4
Checking X-Remoting-Minimum-Version if sent.
jglick Dec 11, 2019
4c701f3
SpotBugs
jglick Dec 11, 2019
47ea945
More targeted SpotBugs annotation placement.
jglick Dec 11, 2019
86cea5b
Documenting options incompatible with -webSocket.
jglick Dec 11, 2019
02db50c
Comments.
jglick Dec 13, 2019
6263aae
nginx urgh https://stackoverflow.com/a/49801326/12916
jglick Dec 13, 2019
827bfca
Trying to enable JAR cache.
jglick Dec 13, 2019
e25ee2c
JnlpConnectionState.fireXXX methods need to be public to support othe…
jglick Dec 13, 2019
cf08bcf
JnlpConnectionState.getRemoteEndpointDescription introduced to avoid …
jglick Dec 13, 2019
f2549aa
Do not advertise tyrus-standalone-client-jdk to Maven dependencies, t…
jglick Dec 16, 2019
ee1ecd9
SpotBugs
jglick Dec 17, 2019
50d4df7
Merge branch 'master' of github.com:jenkinsci/remoting into websocket
jglick Dec 17, 2019
21bf706
Shade dependencies needed for agent.jar.
jglick Dec 21, 2019
a54c157
Merge branch 'master' of github.com:jenkinsci/remoting into websocket
jglick Jan 3, 2020
526baab
More intuitive log message, as suggested by @jeffret-b.
jglick Jan 3, 2020
ce68090
FindSecBugs
jglick Jan 3, 2020
f77a3cf
Some updates to documentation to reflect WebSocket mode.
jglick Jan 3, 2020
0682149
Fixed formatting
jglick Jan 3, 2020
87ff8af
Apply suggestions from code review
jglick Jan 4, 2020
84d9d71
Merge branch 'master' of github.com:jenkinsci/remoting into websocket
jglick Jan 6, 2020
d220515
Expect to indicate version in which -webSocket was introduced.
jglick Jan 6, 2020
977bcc1
Introduced constant for X-Remoting-Minimum-Version.
jglick Jan 6, 2020
0cc1903
Merge branch 'websocket' of github.com:jglick/remoting into websocket
jglick Jan 6, 2020
91f8990
Take advantage of args4j declarative depends/forbids. Also reword err…
jglick Jan 6, 2020
739e338
No need to explicitly close connections after EngineListener.error: M…
jglick Jan 6, 2020
7eda193
Implement reconnection in WebSocket mode.
jglick Jan 6, 2020
b779630
Correcting option name.
jglick Jan 6, 2020
c238b7c
FindSecBugs
jglick Jan 6, 2020
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
16 changes: 16 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ THE SOFTWARE.
<artifactId>animal-sniffer-annotations</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.glassfish.tyrus.bundles</groupId>
<artifactId>tyrus-standalone-client-jdk</artifactId>
<version>1.12</version>
</dependency>
<!-- test dependencies -->
<dependency>
<groupId>junit</groupId>
Expand Down Expand Up @@ -343,6 +348,17 @@ THE SOFTWARE.
<includeGroupIds>org.jenkins-ci</includeGroupIds>
</configuration>
</execution>
<execution>
<id>bundle-tyrus</id>
<phase>process-classes</phase>
<goals>
<goal>unpack-dependencies</goal>
</goals>
<configuration>
<outputDirectory>${project.build.outputDirectory}</outputDirectory>
<includeArtifactIds>tyrus-standalone-client-jdk</includeArtifactIds>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
Expand Down
6 changes: 6 additions & 0 deletions src/findbugs/excludeFilter.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
<Match>
<Package name="~org\.jenkinsci\.constant_pool_scanner.*"/>
</Match>
<Match>
<Package name="~javax\.websocket.*"/>
</Match>
<Match>
<Package name="~org\.glassfish\.tyrus.*"/>
</Match>

<Match>
<!--We do not want do break API by converting potentially usefull classes-->
Expand Down
11 changes: 9 additions & 2 deletions src/main/java/hudson/remoting/Capability.java
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,17 @@ public boolean supportsProxyExceptionFallback() {

//TODO: ideally preamble handling needs to be reworked in order to avoid FB suppression
/**
* Writes out the capacity preamble.
* Writes {@link #PREAMBLE} then uses {@link #write}.
*/
void writePreamble(OutputStream os) throws IOException {
os.write(PREAMBLE);
write(os);
}

/**
* Writes this capability to a stream.
*/
public void write(OutputStream os) throws IOException {
try (ObjectOutputStream oos = new ObjectOutputStream(Mode.TEXT.wrap(os)) {
@Override
public void close() throws IOException {
Expand All @@ -143,7 +150,7 @@ protected void annotateClass(Class<?> c) throws IOException {
}

/**
* The opposite operation of {@link #writePreamble(OutputStream)}.
* The opposite operation of {@link #write}.
*/
public static Capability read(InputStream is) throws IOException {
try (ObjectInputStream ois = new ObjectInputStream(Mode.TEXT.wrap(is)) {
Expand Down
113 changes: 112 additions & 1 deletion src/main/java/hudson/remoting/Engine.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,19 @@
*/
package hudson.remoting;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.remoting.Channel.Mode;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.net.Socket;
import java.net.URI;
import java.net.URL;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.security.AccessController;
import java.security.KeyManagementException;
Expand All @@ -44,13 +50,15 @@
import java.security.cert.X509Certificate;
import java.security.interfaces.RSAPublicKey;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
Expand All @@ -61,6 +69,13 @@
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.TrustManager;
import javax.net.ssl.TrustManagerFactory;
import javax.websocket.ClientEndpointConfig;
import javax.websocket.CloseReason;
import javax.websocket.ContainerProvider;
import javax.websocket.Endpoint;
import javax.websocket.EndpointConfig;
import javax.websocket.HandshakeResponse;
import javax.websocket.Session;

import org.jenkinsci.remoting.engine.JnlpEndpointResolver;
import org.jenkinsci.remoting.engine.Jnlp4ConnectionState;
Expand Down Expand Up @@ -437,10 +452,105 @@ public void removeListener(EngineListener el) {
events.remove(el);
}

@SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "checked exceptions were a mistake to begin with")
@Override
public void run() {
// Create the engine
try {
if (true) { // TODO conditionally enable WS support
AtomicReference<Channel> ch = new AtomicReference<>();
String localCap;
try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
new Capability().write(baos);
localCap = baos.toString("US-ASCII"); // Base-64
}
class HeaderHandler extends ClientEndpointConfig.Configurator {
Capability remoteCapability = new Capability();
@Override
public void beforeRequest(Map<String, List<String>> headers) {
headers.put(JnlpConnectionState.CLIENT_NAME_KEY, Collections.singletonList(slaveName));
headers.put(JnlpConnectionState.SECRET_KEY, Collections.singletonList(secretKey));
headers.put(Capability.class.getName(), Collections.singletonList(localCap));
// TODO use JnlpConnectionState.COOKIE_KEY somehow
LOGGER.fine(() -> "Sending: " + headers);
}
@Override
public void afterResponse(HandshakeResponse hr) {
LOGGER.fine(() -> "Receiving: " + hr.getHeaders());
try (ByteArrayInputStream bais = new ByteArrayInputStream(hr.getHeaders().get(Capability.class.getName()).get(0).getBytes(StandardCharsets.US_ASCII))) {
remoteCapability = Capability.read(bais);
LOGGER.fine(() -> "received " + remoteCapability);
} catch (IOException x) {
events.error(x);
}
}
}
HeaderHandler headerHandler = new HeaderHandler();
ContainerProvider.getWebSocketContainer().connectToServer(new Endpoint() {
AbstractByteArrayCommandTransport.ByteArrayReceiver receiver;
@Override
public void onOpen(Session session, EndpointConfig config) {
events.status("WebSocket connection open");
session.addMessageHandler(byte[].class, this::onMessage);
try {
ch.set(new ChannelBuilder(slaveName, executor).build(new Transport(session)));
} catch (IOException x) {
events.error(x);
}
}
@SuppressFBWarnings(value = "UWF_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", justification = "just trust me here")
private void onMessage(byte[] message) {
LOGGER.finest(() -> "received message of length " + message.length);
receiver.handle(message);
}
@Override
public void onClose(Session session, CloseReason closeReason) {
// TODO
}
@Override
public void onError(Session session, Throwable thr) {
// TODO
}
class Transport extends AbstractByteArrayCommandTransport {
final Session session;
Transport(Session session) {
this.session = session;
}
@Override
public void setup(AbstractByteArrayCommandTransport.ByteArrayReceiver _receiver) {
events.status("Setting up channel");
receiver = _receiver;
}
@Override
public void writeBlock(Channel channel, byte[] payload) throws IOException {
LOGGER.finest(() -> "sending message of length " + payload.length);
session.getBasicRemote().sendBinary(ByteBuffer.wrap(payload));
}
@Override
public Capability getRemoteCapability() throws IOException {
return headerHandler.remoteCapability;
}
@Override
public void closeWrite() throws IOException {
events.status("Write side closed");
// TODO
}
@Override
public void closeRead() throws IOException {
events.status("Read side closed");
// TODO
}
}
}, ClientEndpointConfig.Builder.create().configurator(headerHandler).build(), URI.create(candidateUrls.get(0).toString().replaceFirst("^http", "ws") + "wsagents/"));
while (ch.get() == null) {
Thread.sleep(100);
}
LOGGER.info(() -> "Waiting for channel");
ch.get().join();
// TODO handle multiple candidate URLs
// TODO handle reconnection
return;
}
IOHub hub = IOHub.create(executor);
try {
SSLContext context;
Expand Down Expand Up @@ -489,11 +599,12 @@ public void run() {
} finally {
hub.close();
}
} catch (IOException e) {
} catch (Exception e) {
events.error(e);
}
}

@SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD", justification = "TODO pending WS/TCP switch")
@SuppressWarnings({"ThrowableInstanceNeverThrown"})
private void innerRun(IOHub hub, SSLContext context, ExecutorService service) {
// Create the protocols that will be attempted to connect to the master.
Expand Down
1 change: 1 addition & 0 deletions src/main/java/hudson/remoting/Launcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,7 @@ public void run() throws Exception {
// But we set it up just in case there are overrides somewhere in the logic
jnlpArgs.add("-disableHttpsCertValidation");
}
// TODO ws support
try {
hudson.remoting.jnlp.Main._main(jnlpArgs.toArray(new String[jnlpArgs.size()]));
} catch (CmdLineException e) {
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/hudson/remoting/jnlp/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ public class Main {
usage="Specify the Jenkins root URLs to connect to.")
public List<URL> urls = new ArrayList<>();

// TODO option to enable WS

@Option(name="-credentials",metaVar="USER:PASSWORD",
usage="HTTP BASIC AUTH header to pass in for making HTTP requests.")
public String credentials;
Expand Down