From f5ccd65e4e3db8cefe53053e3d9cf514d3e28d41 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 20 Sep 2024 21:51:17 -0400 Subject: [PATCH] More test fixups related to JTH change (#9767) --- .../hudson/slaves/JNLPLauncherRealTest.java | 4 +-- .../jenkins/security/Security3430Test.java | 36 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java index ececcf0f786e..9b4b9bed3f69 100644 --- a/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java +++ b/test/src/test/java/hudson/slaves/JNLPLauncherRealTest.java @@ -25,7 +25,7 @@ package hudson.slaves; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; import hudson.ExtensionList; @@ -104,7 +104,7 @@ public void run(JenkinsRule r) throws Throwable { p.setAssignedNode(agent); FreeStyleBuild b = r.buildAndAssertSuccess(p); if (webSocket) { - assertThat(agent.toComputer().getSystemProperties().get("java.class.path").toString(), endsWith("agent.jar")); + assertThat(agent.toComputer().getSystemProperties(), hasKey("os.name")); } System.err.println(JenkinsRule.getLog(b)); } diff --git a/test/src/test/java/jenkins/security/Security3430Test.java b/test/src/test/java/jenkins/security/Security3430Test.java index 828ae5018a21..09f227af4805 100644 --- a/test/src/test/java/jenkins/security/Security3430Test.java +++ b/test/src/test/java/jenkins/security/Security3430Test.java @@ -38,9 +38,9 @@ import org.hamcrest.Description; import org.hamcrest.Matcher; import org.hamcrest.TypeSafeMatcher; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import org.jvnet.hudson.test.InboundAgentRule; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RealJenkinsRule; @@ -54,6 +54,9 @@ public class Security3430Test { @Rule public InboundAgentRule agents = new InboundAgentRule(); + @Rule + public TemporaryFolder tmp = new TemporaryFolder(); + @Test public void runWithOldestSupportedAgentJar() throws Throwable { runWithRemoting(RemotingVersionInfo.getMinimumSupportedVersion().toString(), "/old-remoting/remoting-minimum-supported.jar", true); @@ -64,22 +67,16 @@ public void runWithPreviousAgentJar() throws Throwable { runWithRemoting("3256.v88a_f6e922152", "/old-remoting/remoting-before-SECURITY-3430-fix.jar", true); } - @Ignore("TODO Expected: is an empty collection; but: <[Allowing URL: file:/…/test/target/webroot…/WEB-INF/lib/stapler-1903.v994a_db_314d58.jar, Determined to be core jar: file:/…/test/target/webroot…/WEB-INF/lib/stapler-1903.v994a_db_314d58.jar]>") @Test public void runWithCurrentAgentJar() throws Throwable { - runWithRemoting(null, null, false); + runWithRemoting(Launcher.VERSION, null, false); } private void runWithRemoting(String expectedRemotingVersion, String remotingResourcePath, boolean requestingJarFromAgent) throws Throwable { - if (expectedRemotingVersion != null) { - // TODO brittle; rather call InboundAgentRule.start(AgentArguments, Options) with a known agentJar - FileUtils.copyURLToFile(Security3430Test.class.getResource(remotingResourcePath), new File(System.getProperty("java.io.tmpdir"), "agent.jar")); - } - jj.startJenkins(); final String agentName = "agent1"; try { - agents.createAgent(jj, InboundAgentRule.Options.newBuilder().name(agentName).build()); + createAgent(agentName, remotingResourcePath); jj.runRemotely(Security3430Test::_run, agentName, expectedRemotingVersion, requestingJarFromAgent, true); } finally { agents.stop(jj, agentName); @@ -87,13 +84,31 @@ private void runWithRemoting(String expectedRemotingVersion, String remotingReso jj.runRemotely(Security3430Test::disableJarURLValidatorImpl); final String agentName2 = "agent2"; try { - agents.createAgent(jj, InboundAgentRule.Options.newBuilder().name(agentName2).build()); + createAgent(agentName2, remotingResourcePath); jj.runRemotely(Security3430Test::_run, agentName2, expectedRemotingVersion, requestingJarFromAgent, false); } finally { agents.stop(jj, agentName2); } } + private void createAgent(String name, String remotingResourcePath) throws Throwable { + if (remotingResourcePath != null) { + var jar = tmp.newFile(name + ".jar"); + FileUtils.copyURLToFile(Security3430Test.class.getResource(remotingResourcePath), jar); + // TODO awkward, especially as InboundAgentRule.getAgentArguments is private; + // would be helpful to have an option for a specific agent JAR: + var opts = InboundAgentRule.Options.newBuilder().name(name).skipStart().build(); + agents.createAgent(jj, opts); + agents.start(new InboundAgentRule.AgentArguments(jj.getUrl() + "computer/" + name + "/slave-agent.jnlp", jar, jj.runRemotely(Security3430Test::getJnlpMac, name), 1, List.of()), opts); + } else { + agents.createAgent(jj, InboundAgentRule.Options.newBuilder().name(name).build()); + } + } + + private static String getJnlpMac(JenkinsRule r, String name) throws Throwable { + return ((SlaveComputer) r.jenkins.getComputer(name)).getJnlpMac(); + } + // This is quite artificial, but demonstrating that without JarURLValidatorImpl we do not allow any calls from the agent: private static void disableJarURLValidatorImpl(JenkinsRule j) { assertTrue(ExtensionList.lookup(ChannelConfigurator.class).remove(ExtensionList.lookupSingleton(JarURLValidatorImpl.class))); @@ -114,6 +129,7 @@ private static void _run(JenkinsRule j, String agentName, String expectedRemotin final Computer computer = j.jenkins.getComputer(agentName); assertThat(computer, instanceOf(SlaveComputer.class)); SlaveComputer agent = (SlaveComputer) computer; + j.waitOnline(agent.getNode()); final Channel channel = agent.getChannel(); if (expectedRemotingVersion != null) { final String result = channel.call(new AgentVersionCallable());