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
70 changes: 59 additions & 11 deletions src/main/java/testingbot/TestingBotBuildWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@
import hudson.model.BuildableItemWithBuildWrappers;
import hudson.model.Job;
import hudson.model.Item;
import hudson.model.Computer;
import hudson.model.Node;
import hudson.model.listeners.ItemListener;
import hudson.remoting.VirtualChannel;
import hudson.security.ACL;
import hudson.util.DescribableList;
import testingbot.pipeline.TestingBotTunnelStep;
import java.util.Map;
import hudson.util.ListBoxModel;
import java.util.ArrayList;
Expand Down Expand Up @@ -92,24 +96,45 @@ public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener l
if (this.useTunnel) {
if (credentials == null) {
listener.getLogger().println("No TestingBot key/secret found while trying to start a TestingBot Tunnel");
return new TestingBotBuildEnvironment(null, null, null);
return new TestingBotBuildEnvironment(null, buildId, null, null);
}

// Run the tunnel on the build's node (not the controller) so that a test connecting to
// localhost reaches the tunnel. The identifier keeps parallel builds on one node isolated.
String tunnelIdentifier = TunnelManager.generateTunnelIdentifier(build.getParent().getFullName(), build.getNumber());
String key = credentials.getKey();
String secret = credentials.getDecryptedSecret();
VirtualChannel channel = launcher.getChannel();

if (channel != null) {
try {
TunnelManager.startOnChannel(channel, key, secret, "", tunnelIdentifier, listener);
} catch (InterruptedException ie) {
TunnelManager.stopOnChannel(channel, tunnelIdentifier, listener);
throw ie;
} catch (Exception ex) {
// Do not continue the build with a broken tunnel: clean up and fail.
TunnelManager.stopOnChannel(channel, tunnelIdentifier, listener);
throw new IOException("Failed to start TestingBot tunnel", ex);
}
return new TestingBotBuildEnvironment(credentials, buildId, tunnelIdentifier, null);
}

// No agent channel available (unusual launcher); fall back to a controller-local tunnel.
final App app = new App();
try {
TunnelManager.start(app, credentials.getKey(), credentials.getDecryptedSecret(), "", null, listener);
TunnelManager.start(app, key, secret, "", tunnelIdentifier, listener);
} catch (InterruptedException ie) {
TunnelManager.stop(app, listener);
throw ie;
} catch (Exception ex) {
// Do not continue the build with a broken tunnel: clean up and fail.
TunnelManager.stop(app, listener);
throw new IOException("Failed to start TestingBot tunnel", ex);
}
return new TestingBotBuildEnvironment(credentials, app, buildId);
return new TestingBotBuildEnvironment(credentials, buildId, tunnelIdentifier, app);
}

return new TestingBotBuildEnvironment(credentials, null, buildId);
return new TestingBotBuildEnvironment(credentials, buildId, null, null);
}

/**
Expand Down Expand Up @@ -151,14 +176,19 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath final Item context)

private class TestingBotBuildEnvironment extends BuildWrapper.Environment {

private final App app;
private final TestingBotCredentials credentials;
private final String buildId;
/** Non-null when a tunnel was started for this build. */
private final String tunnelIdentifier;
/** Controller-local tunnel handle, used only in the no-channel fallback (otherwise null → runs on the agent). */
private final App localApp;

public TestingBotBuildEnvironment(TestingBotCredentials credentials, @Nullable App app, @Nullable String buildId) {
public TestingBotBuildEnvironment(TestingBotCredentials credentials, @Nullable String buildId,
@Nullable String tunnelIdentifier, @Nullable App localApp) {
this.credentials = credentials;
this.app = app;
this.buildId = buildId;
this.tunnelIdentifier = tunnelIdentifier;
this.localApp = localApp;
}

@Override
Expand All @@ -171,19 +201,37 @@ public void buildEnvVars(Map<String, String> env) {
env.put(TestingBotBuildReportAction.TESTINGBOT_BUILD, buildId);
}

if (app != null) {
env.put(TESTINGBOT_TUNNEL,"true");
if (tunnelIdentifier != null) {
env.put(TESTINGBOT_TUNNEL, "true");
env.put(TestingBotTunnelStep.TESTINGBOT_TUNNEL_IDENTIFIER, tunnelIdentifier);
}

super.buildEnvVars(env);
}

@Override
public boolean tearDown(AbstractBuild build, BuildListener listener) throws IOException, InterruptedException {
TunnelManager.stop(app, listener);
if (tunnelIdentifier == null) {
return true;
}
if (localApp != null) {
// Controller-local fallback tunnel.
TunnelManager.stop(localApp, listener);
} else {
// Resolve the agent's live channel now rather than holding a reference captured in
// setUp, which could be stale if the agent reconnected during the build.
TunnelManager.stopOnChannel(currentChannel(build), tunnelIdentifier, listener);
}
return true;
}
}

/** The live channel of the node the build ran on, or null if it is offline/unavailable. */
private static VirtualChannel currentChannel(AbstractBuild<?, ?> build) {
Node node = build.getBuiltOn();
Computer computer = node == null ? null : node.toComputer();
return computer == null ? null : computer.getChannel();
}

protected boolean migrateCredentials(AbstractProject project) {
Logger.getLogger(TestingBotBuildWrapper.class.getName()).log(Level.INFO, "TestingBot Plugin: migrateCredentials: " + this.credentialsId);
Expand Down
138 changes: 138 additions & 0 deletions src/main/java/testingbot/TunnelManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@
import com.testingbot.tunnel.Api;
import com.testingbot.tunnel.App;
import hudson.model.TaskListener;
import hudson.remoting.VirtualChannel;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import jenkins.security.MasterToSlaveCallable;

/**
* Shared TestingBot Tunnel lifecycle helper used by both the freestyle
Expand Down Expand Up @@ -203,4 +207,138 @@ public static void stop(App app, TaskListener listener) {
app.stop();
}
}

// ------------------------------------------------------------------------------------------
// Remote (agent) tunnel lifecycle.
//
// The tunnel must boot on the same node where the tests run, so that a test connecting to
// {@code localhost} reaches the tunnel. Both the freestyle build wrapper and the pipeline step
// therefore run the tunnel inside a MasterToSlaveCallable on the build node's channel. The live
// App is kept in a per-JVM static registry (co-located with the tunnel on the agent) so teardown
// can find and stop it again without serializing the handle back across the channel.
// ------------------------------------------------------------------------------------------

/** Tunnels running in this JVM, keyed by tunnel identifier. Lives in the agent's static space. */
private static final ConcurrentHashMap<String, App> RUNNING_TUNNELS = new ConcurrentHashMap<>();

/** Monotonic suffix so multiple generated identifiers within one run stay distinct. */
private static final AtomicInteger TUNNEL_SEQ = new AtomicInteger();

/**
* Generates a tunnel identifier unique to a build (and to each tunnel within it), so that
* parallel tunnels on the same node stay isolated.
*/
public static String generateTunnelIdentifier(String jobFullName, int buildNumber) {
return "jenkins-" + jobFullName.replace('/', '-') + "-" + buildNumber + "-" + TUNNEL_SEQ.incrementAndGet();
}

/**
* Boots a tunnel on the given channel's node and registers it under {@code tunnelIdentifier}.
* Credentials are passed as already-decrypted strings (only plaintext crosses the encrypted
* remoting link, never the credential object).
*/
public static void startOnChannel(VirtualChannel channel, String key, String secret, String options,
String tunnelIdentifier, TaskListener listener) throws Exception {
channel.call(new StartTunnelHandler(key, secret, options, tunnelIdentifier, listener));
}

/**
* Stops the tunnel registered under {@code tunnelIdentifier} on the given channel's node.
* Best-effort: a {@code null} channel (offline agent) or a remoting failure is tolerated so a
* teardown never fails the build; an offline agent is reported to the log.
*/
public static void stopOnChannel(VirtualChannel channel, String tunnelIdentifier, TaskListener listener) {
if (channel == null) {
RUNNING_TUNNELS.remove(tunnelIdentifier);
if (listener != null) {
listener.getLogger().println("[TestingBot] Agent offline: remote teardown of tunnel "
+ tunnelIdentifier + " could not be performed. It may still be running on the agent "
+ "and should stop when the agent process exits (best-effort cleanup).");
}
return;
}
try {
channel.call(new StopTunnelHandler(tunnelIdentifier, listener));
} catch (InterruptedException ie) {
// Restore the interrupt status; teardown stays best-effort and does not rethrow.
Thread.currentThread().interrupt();
if (listener != null) {
listener.getLogger().println("[TestingBot] Interrupted while stopping tunnel " + tunnelIdentifier);
}
} catch (Exception e) {
// Best-effort, but do not silently discard the failure — record why teardown could not complete.
if (listener != null) {
listener.getLogger().println("[TestingBot] Failed to stop tunnel " + tunnelIdentifier + ": " + e);
}
}
Comment on lines +260 to +273

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the relevant file and surrounding methods
git ls-files src/main/java/testingbot/TunnelManager.java
ast-grep outline src/main/java/testingbot/TunnelManager.java --view expanded || true

# Show the target section with line numbers
sed -n '220,320p' src/main/java/testingbot/TunnelManager.java

# Search for teardown/logging helpers and interrupt handling in the file
rg -n "logTeardownFailure|InterruptedException|best effort|StopTunnelHandler|stopOnChannel|interrupt\\(" src/main/java/testingbot/TunnelManager.java

Repository: testingbot/TestingBot-Jenkins-Plugin

Length of output: 6168


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find call sites and related teardown logic
rg -n "stopOnChannel\\(|stop\\(App app, TaskListener listener\\)|channel.call\\(new StopTunnelHandler|RUNNING_TUNNELS.remove\\(" src/main/java src/test/java

# Show the public stop method and nearby context
sed -n '190,250p' src/main/java/testingbot/TunnelManager.java

Repository: testingbot/TestingBot-Jenkins-Plugin

Length of output: 4505


Preserve interruption and report teardown failures. stopOnChannel swallows InterruptedException and all remoting failures, so cancellation state is lost and a remote tunnel stop can fail silently. Restore the interrupt flag and log the failure before returning.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/testingbot/TunnelManager.java` around lines 260 - 264, Update
stopOnChannel’s exception handling around StopTunnelHandler so
InterruptedException restores the thread’s interrupted status, while other
teardown exceptions are logged with relevant failure details before returning.
Preserve the existing best-effort behavior without silently discarding remoting
failures.

}

private static final class StartTunnelHandler extends MasterToSlaveCallable<Void, Exception> {

private static final long serialVersionUID = 1L;
private final String key;
private final String secret;
private final String tunnelOptions;
private final String tunnelIdentifier;
private final TaskListener listener;

StartTunnelHandler(String key, String secret, String tunnelOptions, String tunnelIdentifier, TaskListener listener) {
this.key = key;
this.secret = secret;
this.tunnelOptions = tunnelOptions;
this.tunnelIdentifier = tunnelIdentifier;
this.listener = listener;
}

@Override
public Void call() throws Exception {
App app = new App();
// Reserve the identifier before booting: a duplicate identifier is rejected rather than
// silently overwriting (and leaking) an existing tunnel, and teardown can find and stop
// the App even while it is still booting.
App previous = RUNNING_TUNNELS.putIfAbsent(tunnelIdentifier, app);
if (previous != null) {
try {
app.stop();
} catch (Exception ignored) {
// best effort
}
throw new IllegalStateException(
"A TestingBot tunnel with identifier '" + tunnelIdentifier + "' is already running");
}
try {
start(app, key, secret, tunnelOptions, tunnelIdentifier, listener);
} catch (Exception e) {
// Startup failed: remove only our own registry entry (leaving any concurrent owner
// untouched) and stop the App so we don't leak a half-booted tunnel.
RUNNING_TUNNELS.remove(tunnelIdentifier, app);
try {
app.stop();
} catch (Exception ignored) {
// best effort
}
throw e;
}
return null;
Comment on lines +293 to +322

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Reserve registry ownership before booting the tunnel.

The tunnel is undiscoverable until start finishes. If channel.call is interrupted while the remote callable continues, cleanup can remove nothing before line 298 subsequently registers a live tunnel. The unconditional put also overwrites an existing user-supplied identifier, making later teardown stop the wrong instance.

Install a collision-aware lifecycle entry before booting, coordinate stop with in-progress startup, and remove it conditionally on failure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/testingbot/TunnelManager.java` around lines 284 - 299, Update
the tunnel startup flow in the callable’s call method to reserve
tunnelIdentifier in the running-tunnel registry before invoking start, reject
collisions without overwriting an existing entry, and coordinate cleanup so stop
waits for or safely handles in-progress startup. On startup failure, remove only
the entry owned by this tunnel and stop the created App as needed; preserve the
registry ownership through successful startup.

}
}

private static final class StopTunnelHandler extends MasterToSlaveCallable<Void, Exception> {

private static final long serialVersionUID = 1L;
private final String tunnelIdentifier;
private final TaskListener listener;

StopTunnelHandler(String tunnelIdentifier, TaskListener listener) {
this.tunnelIdentifier = tunnelIdentifier;
this.listener = listener;
}

@Override
public Void call() {
App app = RUNNING_TUNNELS.remove(tunnelIdentifier);
stop(app, listener);
return null;
}
}
}
Loading