From 5b2b0dfca28ad39e8c19083944dd24fe22ef286b Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Thu, 29 Sep 2022 15:48:20 -0700 Subject: [PATCH 1/4] Use enhanced `InboundAgentRule` --- test/pom.xml | 3 +- .../bugs/JnlpAccessWithSecuredHudsonTest.java | 43 ++---------- .../hudson/cli/OfflineNodeCommandTest.java | 31 ++------ .../hudson/slaves/AgentInboundUrlTest.java | 35 ++-------- .../hudson/slaves/JNLPLauncherRealTest.java | 69 +++++------------- .../java/hudson/slaves/NodeParallelTest.java | 8 ++- .../jenkins/agents/WebSocketAgentsTest.java | 41 ++--------- .../jenkins/security/Security218Test.java | 70 ++----------------- .../JnlpSlaveRestarterInstallerTest.java | 36 ++-------- 9 files changed, 61 insertions(+), 275 deletions(-) diff --git a/test/pom.xml b/test/pom.xml index b88413ec4c6c..fd83c62a6102 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -93,7 +93,8 @@ THE SOFTWARE. ${project.groupId} jenkins-test-harness - 1847.v10c7a_4c2e344 + + 1850.va_4a_a_01d2b_57b test diff --git a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java index 78b17c274b3c..34cd34e114fd 100644 --- a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java +++ b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java @@ -30,33 +30,22 @@ import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.gargoylesoftware.htmlunit.xml.XmlPage; -import hudson.Launcher; -import hudson.Proc; -import hudson.model.Node.Mode; import hudson.model.Slave; import hudson.model.User; import hudson.remoting.Channel; -import hudson.slaves.DumbSlave; -import hudson.slaves.JNLPLauncher; -import hudson.slaves.RetentionStrategy; -import hudson.util.StreamTaskListener; import java.io.File; import java.net.HttpURLConnection; import java.net.URL; -import java.util.Collections; -import java.util.List; import java.util.Locale; import jenkins.security.apitoken.ApiTokenTestHelper; import jenkins.security.s2m.AdminWhitelistRule; -import org.apache.commons.io.FileUtils; -import org.apache.tools.ant.util.JavaEnvUtils; import org.dom4j.Document; import org.dom4j.Element; import org.dom4j.io.DOMReader; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; import org.jvnet.hudson.test.Email; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.recipes.PresetData; import org.jvnet.hudson.test.recipes.PresetData.DataSet; @@ -72,14 +61,7 @@ public class JnlpAccessWithSecuredHudsonTest { public JenkinsRule r = new JenkinsRule(); @Rule - public TemporaryFolder tmp = new TemporaryFolder(); - - /** - * Creates a new agent that needs to be launched via JNLP. - */ - protected Slave createNewJnlpSlave(String name) throws Exception { - return new DumbSlave(name, "", System.getProperty("java.io.tmpdir") + '/' + name, "2", Mode.NORMAL, "", new JNLPLauncher(true), RetentionStrategy.INSTANCE, Collections.EMPTY_LIST); - } + public InboundAgentRule inboundAgents = new InboundAgentRule(); @PresetData(DataSet.NO_ANONYMOUS_READACCESS) @Email("http://markmail.org/message/on4wkjdaldwi2atx") @@ -87,7 +69,7 @@ protected Slave createNewJnlpSlave(String name) throws Exception { public void anonymousCanAlwaysLoadJARs() throws Exception { ApiTokenTestHelper.enableLegacyBehavior(); - r.jenkins.setNodes(List.of(createNewJnlpSlave("test"))); + inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().name("test").skipStart().build()); JenkinsRule.WebClient wc = r.createWebClient(); HtmlPage p = wc.withBasicApiToken(User.getById("alice", true)).goTo("computer/test/"); @@ -111,35 +93,20 @@ public void anonymousCanAlwaysLoadJARs() throws Exception { @PresetData(DataSet.ANONYMOUS_READONLY) @Test public void anonymousCannotGetSecrets() throws Exception { - r.jenkins.setNodes(List.of(createNewJnlpSlave("test"))); + inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().name("test").skipStart().build()); r.createWebClient().assertFails("computer/test/jenkins-agent.jnlp", HttpURLConnection.HTTP_FORBIDDEN); } @PresetData(DataSet.NO_ANONYMOUS_READACCESS) - @SuppressWarnings("SleepWhileInLoop") @Test public void serviceUsingDirectSecret() throws Exception { - Slave slave = createNewJnlpSlave("test"); - r.jenkins.setNodes(List.of(slave)); + Slave slave = inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().name("test").secret().build()); r.createWebClient().goTo("computer/test/jenkins-agent.jnlp?encrypt=true", "application/octet-stream"); - String secret = slave.getComputer().getJnlpMac(); - // To watch it fail: secret = secret.replace('1', '2'); - File slaveJar = tmp.newFile(); - FileUtils.copyURLToFile(new Slave.JnlpJar("agent.jar").getURL(), slaveJar); - Proc p = new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch(). - stdout(System.out).stderr(System.err). - cmds(JavaEnvUtils.getJreExecutable("java"), "-jar", slaveJar.getAbsolutePath(), "-jnlpUrl", r.getURL() + "computer/test/jenkins-agent.jnlp", "-secret", secret). - start(); - try { - r.waitOnline(slave); Channel channel = slave.getComputer().getChannel(); assertFalse("SECURITY-206", channel.isRemoteClassLoadingAllowed()); r.jenkins.getExtensionList(AdminWhitelistRule.class).get(AdminWhitelistRule.class).setMasterKillSwitch(false); final File f = new File(r.jenkins.getRootDir(), "config.xml"); assertTrue(f.exists()); - } finally { - p.kill(); - } } diff --git a/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java b/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java index 58c894392099..2dde783b564f 100644 --- a/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java @@ -27,8 +27,6 @@ import static hudson.cli.CLICommandInvoker.Matcher.failedWith; import static hudson.cli.CLICommandInvoker.Matcher.hasNoStandardOutput; import static hudson.cli.CLICommandInvoker.Matcher.succeededSilently; -import static org.awaitility.Awaitility.await; -import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -39,18 +37,16 @@ import hudson.model.Computer; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; +import hudson.model.Slave; import hudson.slaves.DumbSlave; -import hudson.slaves.JNLPLauncher; import hudson.slaves.OfflineCause; -import hudson.slaves.RetentionStrategy; import hudson.util.OneShotEvent; -import java.io.File; import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; import jenkins.model.Jenkins; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.JenkinsRule; /** @@ -63,6 +59,9 @@ public class OfflineNodeCommandTest { @Rule public final JenkinsRule j = new JenkinsRule(); + @Rule + public InboundAgentRule inboundAgents = new InboundAgentRule(); + @Before public void setUp() { command = new CLICommandInvoker(j, "offline-node"); @@ -112,15 +111,7 @@ public void offlineNodeShouldSucceedOnOnlineNode() throws Exception { @Test public void offlineNodeShouldSucceedOnOfflineNode() throws Exception { - DumbSlave slave = new DumbSlave( - "aNode", - new File(j.jenkins.getRootDir(), "agent-work-dirs/aNode").getAbsolutePath(), - new JNLPLauncher(true)); - slave.setRetentionStrategy(RetentionStrategy.NOOP); - j.jenkins.addNode(slave); - await().pollInterval(250, TimeUnit.MILLISECONDS) - .atMost(10, TimeUnit.SECONDS) - .until(() -> slave.toComputer().getOfflineCause(), notNullValue()); + Slave slave = inboundAgents.createAgent(j, InboundAgentRule.Options.newBuilder().name("aNode").skipStart().build()); slave.toComputer().setTemporarilyOffline(true, null); assertThat(slave.toComputer().isOffline(), equalTo(true)); assertThat(slave.toComputer().isTemporarilyOffline(), equalTo(true)); @@ -179,15 +170,7 @@ public void offlineNodeShouldSucceedOnOnlineNodeWithCause() throws Exception { @Test public void offlineNodeShouldSucceedOnOfflineNodeWithCause() throws Exception { - DumbSlave slave = new DumbSlave( - "aNode", - new File(j.jenkins.getRootDir(), "agent-work-dirs/aNode").getAbsolutePath(), - new JNLPLauncher(true)); - slave.setRetentionStrategy(RetentionStrategy.NOOP); - j.jenkins.addNode(slave); - await().pollInterval(250, TimeUnit.MILLISECONDS) - .atMost(10, TimeUnit.SECONDS) - .until(() -> slave.toComputer().getOfflineCause(), notNullValue()); + Slave slave = inboundAgents.createAgent(j, InboundAgentRule.Options.newBuilder().name("aNode").skipStart().build()); slave.toComputer().setTemporarilyOffline(true, null); assertThat(slave.toComputer().isOffline(), equalTo(true)); assertThat(slave.toComputer().isTemporarilyOffline(), equalTo(true)); diff --git a/test/src/test/java/hudson/slaves/AgentInboundUrlTest.java b/test/src/test/java/hudson/slaves/AgentInboundUrlTest.java index f4836b871f93..e19ba0987f8b 100644 --- a/test/src/test/java/hudson/slaves/AgentInboundUrlTest.java +++ b/test/src/test/java/hudson/slaves/AgentInboundUrlTest.java @@ -25,16 +25,9 @@ package hudson.slaves; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import com.gargoylesoftware.htmlunit.xml.XmlPage; -import hudson.Util; -import hudson.model.Computer; -import hudson.model.Node; import hudson.model.Slave; -import java.io.File; -import java.util.ArrayList; -import java.util.List; import java.util.logging.Level; import jenkins.model.Jenkins; import org.dom4j.Document; @@ -43,6 +36,7 @@ import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.FlagRule; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.LoggerRule; @@ -55,6 +49,9 @@ public class AgentInboundUrlTest { @Rule public JenkinsRule j = new JenkinsRule(); + @Rule + public InboundAgentRule inboundAgents = new InboundAgentRule(); + @Rule public LoggerRule logging = new LoggerRule().record(Slave.class, Level.FINE); @@ -73,34 +70,14 @@ public void testInboundAgentUrlOverride() throws Exception { j.jenkins.setAuthorizationStrategy(authorizationStrategy); // Create an agent - addTestAgent(); + inboundAgents.createAgent(j, InboundAgentRule.Options.newBuilder().name("test").skipStart().build()); // parse the JNLP page into DOM to inspect the jnlp url argument. JenkinsRule.WebClient agent = j.createWebClient(); XmlPage jnlp = (XmlPage) agent.goTo("computer/test/jenkins-agent.jnlp", "application/x-java-jnlp-file"); Document dom = new DOMReader().read(jnlp.getXmlDocument()); - Object arg = dom.selectSingleNode("//application-desc/argument[3]/following-sibling::argument[1]"); + Object arg = dom.selectSingleNode("//application-desc/argument[7]/following-sibling::argument[1]"); String val = ((Element) arg).getText(); assertEquals(customInboundUrl, val); } - - /** - * Adds an Inbound TCP agent to the system and returns it. - */ - private void addTestAgent() throws Exception { - addTestAgent(new JNLPLauncher(false)); - } - - /** - * Adds an Inbound TCP agent to the system and returns it. - */ - private void addTestAgent(ComputerLauncher launcher) throws Exception { - List agents = new ArrayList<>(j.jenkins.getNodes()); - File dir = Util.createTempDir(); - agents.add(new DumbSlave("test", "dummy", dir.getAbsolutePath(), "1", Node.Mode.NORMAL, "", - launcher, RetentionStrategy.INSTANCE, new ArrayList<>())); - j.jenkins.setNodes(agents); - Computer c = j.jenkins.getComputer("test"); - assertNotNull(c); - } } diff --git a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java index ac31919de8aa..83229ee5717e 100644 --- a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java +++ b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java @@ -29,21 +29,12 @@ import hudson.ExtensionList; import hudson.PluginWrapper; -import hudson.Proc; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; import hudson.model.Slave; import hudson.util.FormValidation; -import io.jenkins.lib.support_log_formatter.SupportLogFormatter; -import java.io.File; -import java.util.logging.ConsoleHandler; -import java.util.logging.Handler; -import java.util.logging.Level; -import java.util.logging.Logger; import jenkins.agents.WebSocketAgentsTest; import jenkins.slaves.JnlpSlaveAgentProtocol4; -import org.apache.commons.io.FileUtils; -import org.apache.tools.ant.util.JavaEnvUtils; import org.junit.Rule; import org.junit.Test; import org.junit.runner.Description; @@ -64,18 +55,31 @@ public class JNLPLauncherRealTest { /* Since RealJenkinsRuleInit.jpi will load detached plugins, to reproduce a failure use: FileUtils.touch(new File(rr.getHome(), "plugins/instance-identity.jpi.disabled")); */ - rr.then(JNLPLauncherRealTest::_smokes); + rr.then(new ConnectStep(false)); } - private static void _smokes(JenkinsRule r) throws Throwable { + private static class ConnectStep implements RealJenkinsRule.Step { + + private final boolean webSocket; + + private ConnectStep(boolean webSocket) { + this.webSocket = webSocket; + } + + @Override + public void run(JenkinsRule r) throws Throwable { InboundAgentRule inboundAgents = new InboundAgentRule(); // cannot use @Rule since it would not be accessible from the controller JVM inboundAgents.apply(new Statement() { @Override public void evaluate() throws Throwable { for (PluginWrapper plugin : r.jenkins.pluginManager.getPlugins()) { System.err.println(plugin + " active=" + plugin.isActive() + " enabled=" + plugin.isEnabled()); } - assertThat(ExtensionList.lookupSingleton(JNLPLauncher.DescriptorImpl.class).doCheckWebSocket(false, null).kind, is(FormValidation.Kind.OK)); - Slave agent = inboundAgents.createAgent(r, "static"); + assertThat(ExtensionList.lookupSingleton(JNLPLauncher.DescriptorImpl.class).doCheckWebSocket(webSocket, null).kind, is(FormValidation.Kind.OK)); + InboundAgentRule.Options.Builder builder = InboundAgentRule.Options.newBuilder().name("static").secret(); + if (webSocket) { + builder.webSocket(); + } + Slave agent = inboundAgents.createAgent(r, builder.build()); FreeStyleProject p = r.createFreeStyleProject(); p.setAssignedNode(agent); FreeStyleBuild b = r.buildAndAssertSuccess(p); @@ -83,6 +87,7 @@ private static void _smokes(JenkinsRule r) throws Throwable { } }, Description.EMPTY).evaluate(); + } } /** @@ -90,43 +95,7 @@ private static void _smokes(JenkinsRule r) throws Throwable { */ @Issue("JENKINS-68933") @Test public void webSocket() throws Throwable { - rr.then(JNLPLauncherRealTest::_webSocket); - } - - private static void _webSocket(JenkinsRule r) throws Throwable { - // TODO RealJenkinsRule does not yet support LoggerRule - Handler handler = new ConsoleHandler(); - handler.setFormatter(new SupportLogFormatter()); - handler.setLevel(Level.FINE); - Logger logger = Logger.getLogger("jenkins.websocket"); - logger.setLevel(Level.FINE); - logger.addHandler(handler); - assertThat(ExtensionList.lookupSingleton(JNLPLauncher.DescriptorImpl.class).doCheckWebSocket(true, null).kind, is(FormValidation.Kind.OK)); - // TODO InboundAgentRule does not yet support WebSocket - JNLPLauncher launcher = new JNLPLauncher(true); - launcher.setWebSocket(true); - DumbSlave s = new DumbSlave("remote", new File(r.jenkins.root, "agent").getAbsolutePath(), launcher); - r.jenkins.addNode(s); - String secret = ((SlaveComputer) s.toComputer()).getJnlpMac(); - File slaveJar = new File(r.jenkins.root, "agent.jar"); - FileUtils.copyURLToFile(new Slave.JnlpJar("agent.jar").getURL(), slaveJar); - Proc proc = r.createLocalLauncher().launch().cmds( - JavaEnvUtils.getJreExecutable("java"), "-jar", slaveJar.getAbsolutePath(), - "-jnlpUrl", r.getURL() + "computer/remote/jenkins-agent.jnlp", - "-secret", secret - ).stdout(System.out).start(); - try { - FreeStyleProject p = r.createFreeStyleProject(); - p.setAssignedNode(s); - r.buildAndAssertSuccess(p); - assertThat(s.toComputer().getSystemProperties().get("java.class.path"), is(slaveJar.getAbsolutePath())); - } finally { - proc.kill(); - while (r.jenkins.getComputer("remote").isOnline()) { - System.err.println("waiting for computer to go offline"); - Thread.sleep(250); - } - } + rr.then(new ConnectStep(true)); } } diff --git a/test/src/test/java/hudson/slaves/NodeParallelTest.java b/test/src/test/java/hudson/slaves/NodeParallelTest.java index 6e644268ffb7..05d1db6f11e4 100644 --- a/test/src/test/java/hudson/slaves/NodeParallelTest.java +++ b/test/src/test/java/hudson/slaves/NodeParallelTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.fail; +import hudson.model.Slave; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -15,6 +16,7 @@ import java.util.logging.Logger; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; @@ -23,6 +25,9 @@ public class NodeParallelTest { @Rule public JenkinsRule r = new JenkinsRule(); + @Rule + public InboundAgentRule inboundAgents = new InboundAgentRule(); + private static final Logger LOGGER = Logger.getLogger(NodeParallelTest.class.getName()); private final AtomicInteger count = new AtomicInteger(); @@ -37,8 +42,7 @@ public void createNodesWithParallelThreads() throws InterruptedException, Execut LOGGER.log(Level.INFO, "Creating slave " + i); // JenkinsRule sync on Jenkins singleton, so this doesn't work // r.createSlave(); - DumbSlave agent = new DumbSlave("agent-" + i, "/tmp", new JNLPLauncher(true)); - r.jenkins.addNode(agent); + Slave agent = inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().name("agent-" + i).skipStart().build()); agent.setNodeProperties(List.of(new EnvironmentVariablesNodeProperty(new EnvironmentVariablesNodeProperty.Entry("foo", "" + i)))); return null; } catch (Exception e1) { diff --git a/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java b/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java index 7d786c8bebac..e3b6b41c387c 100644 --- a/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java +++ b/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java @@ -28,29 +28,22 @@ import static org.junit.Assert.assertNotNull; import hudson.Functions; -import hudson.Proc; import hudson.model.FreeStyleProject; import hudson.model.Slave; import hudson.remoting.Engine; -import hudson.slaves.DumbSlave; -import hudson.slaves.JNLPLauncher; import hudson.slaves.SlaveComputer; import hudson.tasks.BatchFile; import hudson.tasks.Shell; -import java.io.File; import java.nio.charset.StandardCharsets; import java.util.Random; -import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; import jenkins.security.SlaveToMasterCallable; -import org.apache.commons.io.FileUtils; -import org.apache.tools.ant.util.JavaEnvUtils; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; import org.jvnet.hudson.test.BuildWatcher; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.LoggerRule; @@ -66,6 +59,9 @@ public class WebSocketAgentsTest { @Rule public JenkinsRule r = new JenkinsRule(); + @Rule + public InboundAgentRule inboundAgents = new InboundAgentRule(); + @Rule public LoggerRule logging = new LoggerRule(). record(Slave.class, Level.FINE). @@ -73,9 +69,6 @@ public class WebSocketAgentsTest { record(WebSocketAgents.class, Level.FINEST). record(Engine.class, Level.FINEST); - @Rule - public TemporaryFolder tmp = new TemporaryFolder(); - /** * Verify basic functionality of an agent in {@code -webSocket} mode. * Requires {@code remoting} to have been {@code mvn install}ed. @@ -83,24 +76,9 @@ public class WebSocketAgentsTest { * Related to {@link hudson.slaves.JNLPLauncherTest} (also see closer to {@link hudson.bugs.JnlpAccessWithSecuredHudsonTest}). * @see hudson.remoting.Launcher */ - @SuppressWarnings("ResultOfMethodCallIgnored") @Test public void smokes() throws Exception { - AtomicReference proc = new AtomicReference<>(); - try { - JNLPLauncher launcher = new JNLPLauncher(true); - launcher.setWebSocket(true); - DumbSlave s = new DumbSlave("remote", tmp.newFolder("agent").getAbsolutePath(), launcher); - r.jenkins.addNode(s); - String secret = ((SlaveComputer) s.toComputer()).getJnlpMac(); - File slaveJar = tmp.newFile(); - FileUtils.copyURLToFile(new Slave.JnlpJar("agent.jar").getURL(), slaveJar); - proc.set(r.createLocalLauncher().launch().cmds( - JavaEnvUtils.getJreExecutable("java"), "-jar", slaveJar.getAbsolutePath(), - "-jnlpUrl", r.getURL() + "computer/remote/jenkins-agent.jnlp", - "-secret", secret - ).stdout(System.out).start()); - r.waitOnline(s); + Slave s = inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().secret().webSocket().build()); assertEquals("response", s.getChannel().call(new DummyTask())); assertNotNull(s.getChannel().call(new FatTask())); FreeStyleProject p = r.createFreeStyleProject(); @@ -108,15 +86,6 @@ public void smokes() throws Exception { p.getBuildersList().add(Functions.isWindows() ? new BatchFile("echo hello") : new Shell("echo hello")); r.buildAndAssertSuccess(p); s.toComputer().getLogText().writeLogTo(0, System.out); - } finally { - if (proc.get() != null) { - proc.get().kill(); - while (r.jenkins.getComputer("remote").isOnline()) { - LOGGER.info("waiting for computer to go offline"); - Thread.sleep(250); - } - } - } } private static class DummyTask extends SlaveToMasterCallable { diff --git a/test/src/test/java/jenkins/security/Security218Test.java b/test/src/test/java/jenkins/security/Security218Test.java index ddaaaf7d9c32..e9aa2cf0dbd2 100644 --- a/test/src/test/java/jenkins/security/Security218Test.java +++ b/test/src/test/java/jenkins/security/Security218Test.java @@ -4,24 +4,14 @@ import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertThrows; -import hudson.model.Node.Mode; -import hudson.model.Slave; -import hudson.remoting.Channel; import hudson.slaves.DumbSlave; -import hudson.slaves.JNLPLauncher; -import hudson.slaves.RetentionStrategy; -import java.io.File; import java.io.IOException; import java.io.Serializable; -import java.util.Collections; import java.util.logging.Level; -import org.apache.commons.io.FileUtils; -import org.apache.tools.ant.util.JavaEnvUtils; import org.codehaus.groovy.runtime.MethodClosure; -import org.junit.After; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.LoggerRule; @@ -35,16 +25,11 @@ public class Security218Test implements Serializable { public transient JenkinsRule j = new JenkinsRule(); @Rule - public TemporaryFolder tmp = new TemporaryFolder(); + public transient InboundAgentRule inboundAgents = new InboundAgentRule(); @Rule public LoggerRule logging = new LoggerRule().record(ClassFilterImpl.class, Level.FINE); - /** - * JNLP agent. - */ - private transient Process jnlp; - /** * Makes sure SECURITY-218 fix also applies to agents. * @@ -62,8 +47,8 @@ public void dumbSlave() throws Exception { */ @Test public void jnlpSlave() throws Exception { - DumbSlave a = createJnlpSlave("test"); - launchJnlpSlave(a); + DumbSlave a = (DumbSlave) inboundAgents.createAgent(j, InboundAgentRule.Options.newBuilder().secret().build()); + j.createWebClient().goTo("computer/" + a.getNodeName() + "/jenkins-agent.jnlp?encrypt=true", "application/octet-stream"); check(a); } @@ -86,51 +71,4 @@ public Object call() { return new MethodClosure("oops", "trim"); } } - -// TODO: reconcile this duplicate with JnlpAccessWithSecuredHudsonTest - /** - * Creates a new agent that needs to be launched via JNLP. - * - * @see #launchJnlpSlave(Slave) - */ - public DumbSlave createJnlpSlave(String name) throws Exception { - DumbSlave s = new DumbSlave(name, "", System.getProperty("java.io.tmpdir") + '/' + name, "2", Mode.NORMAL, "", new JNLPLauncher(true), RetentionStrategy.INSTANCE, Collections.EMPTY_LIST); - j.jenkins.addNode(s); - return s; - } - -// TODO: reconcile this duplicate with JnlpAccessWithSecuredHudsonTest - /** - * Launch a JNLP agent created by {@link #createJnlpSlave(String)} - */ - public Channel launchJnlpSlave(Slave slave) throws Exception { - j.createWebClient().goTo("computer/" + slave.getNodeName() + "/jenkins-agent.jnlp?encrypt=true", "application/octet-stream"); - String secret = slave.getComputer().getJnlpMac(); - File slaveJar = tmp.newFile(); - FileUtils.copyURLToFile(new Slave.JnlpJar("agent.jar").getURL(), slaveJar); - // To watch it fail: secret = secret.replace('1', '2'); - ProcessBuilder pb = new ProcessBuilder(JavaEnvUtils.getJreExecutable("java"), - "-jar", slaveJar.getAbsolutePath(), - "-jnlpUrl", j.getURL() + "computer/" + slave.getNodeName() + "/jenkins-agent.jnlp", "-secret", secret); - - pb.inheritIO(); - System.err.println("Running: " + pb.command()); - - jnlp = pb.start(); - - for (int i = 0; i < /* one minute */600; i++) { - if (slave.getComputer().isOnline()) { - return slave.getComputer().getChannel(); - } - Thread.sleep(100); - } - - throw new AssertionError("The JNLP agent failed to connect"); - } - - @After - public void tearDown() { - if (jnlp != null) - jnlp.destroy(); - } } diff --git a/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java b/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java index fb451416d60f..250929f66ecd 100644 --- a/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java +++ b/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java @@ -26,21 +26,14 @@ import static org.junit.Assert.assertEquals; -import hudson.Proc; import hudson.model.Slave; import hudson.slaves.DumbSlave; -import hudson.slaves.JNLPLauncher; -import hudson.slaves.SlaveComputer; -import java.io.File; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import jenkins.security.MasterToSlaveCallable; -import org.apache.commons.io.FileUtils; -import org.apache.tools.ant.util.JavaEnvUtils; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; +import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsSessionRule; import org.jvnet.hudson.test.LoggerRule; @@ -51,7 +44,7 @@ public class JnlpSlaveRestarterInstallerTest { public JenkinsSessionRule rr = new JenkinsSessionRule(); @Rule - public TemporaryFolder tmp = new TemporaryFolder(); + public InboundAgentRule inboundAgents = new InboundAgentRule(); @Rule public LoggerRule logging = new LoggerRule().record(JnlpSlaveRestarterInstaller.class, Level.FINE).capture(10); @@ -69,23 +62,13 @@ public void webSocketReconnection() throws Throwable { } private void reconnection(boolean webSocket) throws Throwable { - AtomicReference proc = new AtomicReference<>(); AtomicBoolean canWork = new AtomicBoolean(); - try { rr.then(r -> { - JNLPLauncher launcher = new JNLPLauncher(true); - launcher.setWebSocket(webSocket); - DumbSlave s = new DumbSlave("remote", tmp.newFolder("agent").getAbsolutePath(), launcher); - r.jenkins.addNode(s); - String secret = ((SlaveComputer) s.toComputer()).getJnlpMac(); - File slaveJar = tmp.newFile(); - FileUtils.copyURLToFile(new Slave.JnlpJar("agent.jar").getURL(), slaveJar); - proc.set(r.createLocalLauncher().launch().cmds( - JavaEnvUtils.getJreExecutable("java"), "-jar", slaveJar.getAbsolutePath(), - "-jnlpUrl", r.getURL() + "computer/remote/jenkins-agent.jnlp", - "-secret", secret - ).stdout(System.out).start()); - r.waitOnline(s); + InboundAgentRule.Options.Builder builder = InboundAgentRule.Options.newBuilder().name("remote").secret(); + if (webSocket) { + builder.webSocket(); + } + Slave s = inboundAgents.createAgent(r, builder.build()); assertEquals(1, s.getChannel().call(new JVMCount()).intValue()); while (logging.getMessages().stream().noneMatch(msg -> msg.contains("Effective SlaveRestarter on remote:"))) { Thread.sleep(100); @@ -98,11 +81,6 @@ private void reconnection(boolean webSocket) throws Throwable { r.waitOnline(s); assertEquals(canWork.get() ? 1 : 2, s.getChannel().call(new JVMCount()).intValue()); }); - } finally { - if (proc.get() != null) { - proc.get().kill(); - } - } } private static final class JVMCount extends MasterToSlaveCallable { From 5f6f477b556a9459e14175e4514050cd678f898b Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Tue, 4 Oct 2022 07:47:16 -0700 Subject: [PATCH 2/4] Adapt to upstream changes --- .../test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java | 1 + test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java | 1 + test/src/test/java/jenkins/agents/WebSocketAgentsTest.java | 1 + test/src/test/java/jenkins/security/Security218Test.java | 1 + .../slaves/restarter/JnlpSlaveRestarterInstallerTest.java | 1 + 5 files changed, 5 insertions(+) diff --git a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java index 34cd34e114fd..502b80b03a8b 100644 --- a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java +++ b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java @@ -101,6 +101,7 @@ public void anonymousCannotGetSecrets() throws Exception { @Test public void serviceUsingDirectSecret() throws Exception { Slave slave = inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().name("test").secret().build()); + r.waitOnline(slave); r.createWebClient().goTo("computer/test/jenkins-agent.jnlp?encrypt=true", "application/octet-stream"); Channel channel = slave.getComputer().getChannel(); assertFalse("SECURITY-206", channel.isRemoteClassLoadingAllowed()); diff --git a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java index 83229ee5717e..0975caf13d8a 100644 --- a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java +++ b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java @@ -80,6 +80,7 @@ public void run(JenkinsRule r) throws Throwable { builder.webSocket(); } Slave agent = inboundAgents.createAgent(r, builder.build()); + r.waitOnline(agent); FreeStyleProject p = r.createFreeStyleProject(); p.setAssignedNode(agent); FreeStyleBuild b = r.buildAndAssertSuccess(p); diff --git a/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java b/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java index e3b6b41c387c..cfc094e5c739 100644 --- a/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java +++ b/test/src/test/java/jenkins/agents/WebSocketAgentsTest.java @@ -79,6 +79,7 @@ public class WebSocketAgentsTest { @Test public void smokes() throws Exception { Slave s = inboundAgents.createAgent(r, InboundAgentRule.Options.newBuilder().secret().webSocket().build()); + r.waitOnline(s); assertEquals("response", s.getChannel().call(new DummyTask())); assertNotNull(s.getChannel().call(new FatTask())); FreeStyleProject p = r.createFreeStyleProject(); diff --git a/test/src/test/java/jenkins/security/Security218Test.java b/test/src/test/java/jenkins/security/Security218Test.java index e9aa2cf0dbd2..f197ad0f4865 100644 --- a/test/src/test/java/jenkins/security/Security218Test.java +++ b/test/src/test/java/jenkins/security/Security218Test.java @@ -48,6 +48,7 @@ public void dumbSlave() throws Exception { @Test public void jnlpSlave() throws Exception { DumbSlave a = (DumbSlave) inboundAgents.createAgent(j, InboundAgentRule.Options.newBuilder().secret().build()); + j.waitOnline(a); j.createWebClient().goTo("computer/" + a.getNodeName() + "/jenkins-agent.jnlp?encrypt=true", "application/octet-stream"); check(a); } diff --git a/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java b/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java index 250929f66ecd..e2bcdd1d42e4 100644 --- a/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java +++ b/test/src/test/java/jenkins/slaves/restarter/JnlpSlaveRestarterInstallerTest.java @@ -69,6 +69,7 @@ private void reconnection(boolean webSocket) throws Throwable { builder.webSocket(); } Slave s = inboundAgents.createAgent(r, builder.build()); + r.waitOnline(s); assertEquals(1, s.getChannel().call(new JVMCount()).intValue()); while (logging.getMessages().stream().noneMatch(msg -> msg.contains("Effective SlaveRestarter on remote:"))) { Thread.sleep(100); From 7bf7d54813a4133fb1865527f430b6df48732c38 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Wed, 5 Oct 2022 11:47:11 -0700 Subject: [PATCH 3/4] Restore assertion --- test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java index 0975caf13d8a..96c7d9c0ffaf 100644 --- a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java +++ b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java @@ -33,6 +33,7 @@ import hudson.model.FreeStyleProject; import hudson.model.Slave; import hudson.util.FormValidation; +import java.io.File; import jenkins.agents.WebSocketAgentsTest; import jenkins.slaves.JnlpSlaveAgentProtocol4; import org.junit.Rule; @@ -84,6 +85,9 @@ public void run(JenkinsRule r) throws Throwable { FreeStyleProject p = r.createFreeStyleProject(); p.setAssignedNode(agent); FreeStyleBuild b = r.buildAndAssertSuccess(p); + if (webSocket) { + assertThat(agent.toComputer().getSystemProperties().get("java.class.path"), is(new File(r.jenkins.root, "agent.jar").getAbsolutePath())); + } System.err.println(JenkinsRule.getLog(b)); } }, Description.EMPTY).evaluate(); From 66a2eead72b7f0563bce6da2ea11eb999a3a1b94 Mon Sep 17 00:00:00 2001 From: Basil Crow Date: Wed, 5 Oct 2022 12:55:41 -0700 Subject: [PATCH 4/4] https://github.com/jenkinsci/jenkins-test-harness/pull/493 was released --- test/pom.xml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/pom.xml b/test/pom.xml index cb5a4d8def6f..a189c0e4509c 100644 --- a/test/pom.xml +++ b/test/pom.xml @@ -93,8 +93,7 @@ THE SOFTWARE. ${project.groupId} jenkins-test-harness - - 1857.vd7cd4f785585 + 1856.v943040a_5cd13 test