-
Notifications
You must be signed in to change notification settings - Fork 92
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
Allow setting pwsh as powershell executable #111
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -30,6 +30,8 @@ | |||||||||||||||||||||||
import hudson.FilePath; | ||||||||||||||||||||||||
import hudson.Plugin; | ||||||||||||||||||||||||
import hudson.Launcher; | ||||||||||||||||||||||||
import hudson.util.ListBoxModel; | ||||||||||||||||||||||||
import hudson.util.ListBoxModel.Option; | ||||||||||||||||||||||||
import jenkins.model.Jenkins; | ||||||||||||||||||||||||
import java.util.ArrayList; | ||||||||||||||||||||||||
import java.util.Arrays; | ||||||||||||||||||||||||
|
@@ -42,18 +44,29 @@ | |||||||||||||||||||||||
import java.io.InputStreamReader; | ||||||||||||||||||||||||
import java.io.OutputStream; | ||||||||||||||||||||||||
import java.nio.charset.Charset; | ||||||||||||||||||||||||
import org.kohsuke.stapler.DataBoundSetter; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Runs a Powershell script | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
public final class PowershellScript extends FileMonitoringTask { | ||||||||||||||||||||||||
private final String script; | ||||||||||||||||||||||||
private String powershellBinary = "powershell"; | ||||||||||||||||||||||||
private boolean capturingOutput; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
@DataBoundConstructor public PowershellScript(String script) { | ||||||||||||||||||||||||
this.script = script; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
public String getPowershellBinary() { | ||||||||||||||||||||||||
return powershellBinary; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
@DataBoundSetter | ||||||||||||||||||||||||
public void setPowershellBinary(String powershellBinary) { | ||||||||||||||||||||||||
this.powershellBinary = powershellBinary; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
public String getScript() { | ||||||||||||||||||||||||
return script; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
@@ -82,9 +95,7 @@ public String getScript() { | |||||||||||||||||||||||
quote(c.getLogFile(ws)), | ||||||||||||||||||||||||
quote(c.getResultFile(ws))); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Note: PowerShell core is now named pwsh. Workaround this issue on *nix systems by creating a symlink that maps 'powershell' to 'pwsh'. | ||||||||||||||||||||||||
String powershellBinary = "powershell"; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
String powershellArgs; | ||||||||||||||||||||||||
if (launcher.isUnix()) { | ||||||||||||||||||||||||
powershellArgs = "-NoProfile -NonInteractive"; | ||||||||||||||||||||||||
|
@@ -106,8 +117,8 @@ public String getScript() { | |||||||||||||||||||||||
// Copy the helper script from the resources directory into the workspace | ||||||||||||||||||||||||
c.getPowerShellHelperFile(ws).copyFrom(getClass().getResource("powershellHelper.ps1")); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if (launcher.isUnix()) { | ||||||||||||||||||||||||
// There is no need to add a BOM with Open PowerShell | ||||||||||||||||||||||||
if (launcher.isUnix() || "pwsh".equals(powershellBinary)) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: many launchers always say The binary is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you do not have to specify see existing code durable-task-plugin/src/main/java/org/jenkinsci/plugins/durabletask/PowershellScript.java Lines 86 to 96 in 93b86c9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you do not have to - but it is generally good practice to do so. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. despite this being a String it is not currently user editable from jelly so I guess this is a moot point |
||||||||||||||||||||||||
// There is no need to add a BOM with Open PowerShell / PowerShell Core | ||||||||||||||||||||||||
c.getPowerShellScriptFile(ws).write(scriptWithExit, "UTF-8"); | ||||||||||||||||||||||||
if (!capturingOutput) { | ||||||||||||||||||||||||
c.getPowerShellWrapperFile(ws).write(scriptWrapper, "UTF-8"); | ||||||||||||||||||||||||
|
@@ -171,6 +182,10 @@ public FilePath getPowerShellWrapperFile(FilePath ws) throws IOException, Interr | |||||||||||||||||||||||
return Messages.PowershellScript_powershell(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
public ListBoxModel doFillPowershellBinary() { | ||||||||||||||||||||||||
return new ListBoxModel(new Option("powershell"), new Option("pwsh")); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package org.jenkinsci.plugins.durabletask; | ||
|
||
import org.jenkinsci.test.acceptance.docker.DockerContainer; | ||
import org.jenkinsci.test.acceptance.docker.DockerFixture; | ||
|
||
@DockerFixture(id = "pwsh", ports = 22) | ||
public class PowerShellCoreFixture extends DockerContainer { | ||
public static final String PWSH_JAVA_LOCATION = "/usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java"; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,295 @@ | ||
package org.jenkinsci.plugins.durabletask; | ||
|
||
import com.cloudbees.plugins.credentials.Credentials; | ||
import com.cloudbees.plugins.credentials.CredentialsScope; | ||
import com.cloudbees.plugins.credentials.SystemCredentialsProvider; | ||
import com.cloudbees.plugins.credentials.domains.Domain; | ||
import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; | ||
import hudson.EnvVars; | ||
import hudson.FilePath; | ||
import hudson.Launcher; | ||
import hudson.model.Slave; | ||
import hudson.plugins.sshslaves.SSHLauncher; | ||
import hudson.slaves.DumbSlave; | ||
import hudson.util.StreamTaskListener; | ||
import java.io.ByteArrayOutputStream; | ||
import java.io.File; | ||
import java.util.Collections; | ||
import java.util.Properties; | ||
import java.util.logging.Level; | ||
import org.jenkinsci.test.acceptance.docker.Docker; | ||
import org.jenkinsci.test.acceptance.docker.DockerContainer; | ||
import org.jenkinsci.test.acceptance.docker.DockerRule; | ||
import org.junit.After; | ||
import org.junit.Assume; | ||
import org.junit.Before; | ||
import org.junit.BeforeClass; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.jvnet.hudson.test.JenkinsRule; | ||
import org.jvnet.hudson.test.LoggerRule; | ||
|
||
import static org.hamcrest.Matchers.containsString; | ||
import static org.jenkinsci.plugins.durabletask.BourneShellScriptTest.assumeDocker; | ||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertThat; | ||
import static org.junit.Assert.assertTrue; | ||
|
||
public class PowerShellCoreScriptTest { | ||
|
||
@Rule | ||
public JenkinsRule j = new JenkinsRule(); | ||
@Rule | ||
public DockerRule<PowerShellCoreFixture> dockerPWSH = new DockerRule<>(PowerShellCoreFixture.class); | ||
@Rule | ||
public LoggerRule logging = new LoggerRule().recordPackage(PowershellScript.class, Level.FINE); | ||
|
||
private StreamTaskListener listener; | ||
private FilePath ws; | ||
private Launcher launcher; | ||
private Slave s; | ||
static boolean pwshExists; | ||
|
||
@BeforeClass | ||
public static void pwshOrDocker() throws Exception { | ||
checkPwsh(); | ||
if (!pwshExists && new Docker().isAvailable()) { | ||
assumeDocker(); | ||
} else { | ||
Assume.assumeTrue("This test should only run if pwsh is available", pwshExists); | ||
} | ||
} | ||
|
||
private static void checkPwsh() { | ||
Properties properties = System.getProperties(); | ||
String pathSeparator = properties.getProperty("path.separator"); | ||
String[] paths = System.getenv("PATH").split(pathSeparator); | ||
String cmd = "pwsh"; | ||
for (String p : paths) { | ||
// If running on *nix then the binary does not have an extension. Check for both variants to ensure *nix and windows+cygwin are both supported. | ||
File withoutExtension = new File(p, cmd); | ||
File withExtension = new File(p, cmd + ".exe"); | ||
if (withoutExtension.exists() || withExtension.exists()) { | ||
pwshExists = true; | ||
break; | ||
} | ||
} | ||
} | ||
|
||
@Before | ||
public void setUp() throws Exception { | ||
listener = StreamTaskListener.fromStdout(); | ||
if (pwshExists) { | ||
s = j.createOnlineSlave(); | ||
} else { | ||
DockerContainer container = dockerPWSH.get(); | ||
SystemCredentialsProvider.getInstance().setDomainCredentialsMap(Collections.singletonMap( | ||
Domain | ||
.global(), Collections.<Credentials>singletonList(new UsernamePasswordCredentialsImpl( | ||
CredentialsScope.GLOBAL, "test", null, "test", "test")))); | ||
SSHLauncher sshLauncher = new SSHLauncher(container.ipBound(22), container.port(22), "test"); | ||
sshLauncher.setJavaPath(PowerShellCoreFixture.PWSH_JAVA_LOCATION); | ||
s = new DumbSlave("docker", "/home/test", sshLauncher); | ||
j.jenkins.addNode(s); | ||
j.waitOnline(s); | ||
} | ||
ws = s.getWorkspaceRoot().child("ws"); | ||
launcher = s.createLauncher(listener); | ||
} | ||
|
||
@After | ||
public void tearDown() throws Exception { | ||
if (s != null) { | ||
j.jenkins.removeNode(s); | ||
} | ||
} | ||
|
||
@Test | ||
public void explicitExit() throws Exception { | ||
PowershellScript s = new PowershellScript("Write-Output \"Hello, World!\"; exit 1;"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertEquals(Integer.valueOf(1), c.exitStatus(ws, launcher)); | ||
assertThat(baos.toString(), containsString("Hello, World!")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void implicitExit() throws Exception { | ||
PowershellScript s = new PowershellScript("Write-Output \"Success!\";"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertEquals(Integer.valueOf(0), c.exitStatus(ws, launcher)); | ||
assertThat(baos.toString(), containsString("Success!")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void implicitError() throws Exception { | ||
PowershellScript s = new PowershellScript("$ErrorActionPreference = 'Stop'; Write-Error \"Bogus error\""); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertTrue(c.exitStatus(ws, launcher, listener) != 0); | ||
assertThat(baos.toString(), containsString("Bogus error")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void implicitErrorNegativeTest() throws Exception { | ||
PowershellScript s = new PowershellScript("$ErrorActionPreference = 'SilentlyContinue'; Write-Error \"Bogus error\""); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
assertTrue(c.exitStatus(ws, launcher, listener) == 0); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void explicitThrow() throws Exception { | ||
PowershellScript s = new PowershellScript("Write-Output \"Hello, World!\"; throw \"explicit error\";"); | ||
s.setPowershellBinary("pwsh"); | ||
s.captureOutput(); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertTrue(c.exitStatus(ws, launcher, listener).intValue() != 0); | ||
assertThat(baos.toString(), containsString("explicit error")); | ||
if (launcher.isUnix()) { | ||
assertEquals("Hello, World!\n", new String(c.getOutput(ws, launcher))); | ||
} else { | ||
assertEquals("Hello, World!\r\n", new String(c.getOutput(ws, launcher))); | ||
} | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void implicitThrow() throws Exception { | ||
PowershellScript s = new PowershellScript("$ErrorActionPreference = 'Stop'; My-BogusCmdlet;"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertTrue(c.exitStatus(ws, launcher, listener).intValue() != 0); | ||
assertThat(baos.toString(), containsString("My-BogusCmdlet")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void noStdoutPollution() throws Exception { | ||
PowershellScript s = new PowershellScript("$VerbosePreference = \"Continue\"; " + | ||
"$WarningPreference = \"Continue\"; " + | ||
"$DebugPreference = \"Continue\"; " + | ||
"Write-Verbose \"Hello, Verbose!\"; " + | ||
"Write-Warning \"Hello, Warning!\"; " + | ||
"Write-Debug \"Hello, Debug!\"; " + | ||
"Write-Output \"Success\""); | ||
s.setPowershellBinary("pwsh"); | ||
s.captureOutput(); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertTrue(c.exitStatus(ws, launcher, listener).intValue() == 0); | ||
assertThat(baos.toString(), containsString("Hello, Verbose!")); | ||
assertThat(baos.toString(), containsString("Hello, Warning!")); | ||
assertThat(baos.toString(), containsString("Hello, Debug!")); | ||
if (launcher.isUnix()) { | ||
assertEquals("Success\n", new String(c.getOutput(ws, launcher))); | ||
} else { | ||
assertEquals("Success\r\n", new String(c.getOutput(ws, launcher))); | ||
} | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void specialStreams() throws Exception { | ||
PowershellScript s = new PowershellScript("$VerbosePreference = \"Continue\"; " + | ||
"$WarningPreference = \"Continue\"; " + | ||
"$DebugPreference = \"Continue\"; " + | ||
"Write-Verbose \"Hello, Verbose!\"; " + | ||
"Write-Warning \"Hello, Warning!\"; " + | ||
"Write-Debug \"Hello, Debug!\";"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertEquals(0, c.exitStatus(ws, launcher, listener).intValue()); | ||
assertThat(baos.toString(), containsString("VERBOSE: Hello, Verbose!")); | ||
assertThat(baos.toString(), containsString("WARNING: Hello, Warning!")); | ||
assertThat(baos.toString(), containsString("DEBUG: Hello, Debug!")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void spacesInWorkspace() throws Exception { | ||
final FilePath newWs = new FilePath(ws, "subdirectory with spaces"); | ||
PowershellScript s = new PowershellScript("Write-Host 'Running in a workspace with spaces in the path'"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), newWs, launcher, listener); | ||
while (c.exitStatus(newWs, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
assertEquals(0, c.exitStatus(newWs, launcher).intValue()); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void echoEnvVar() throws Exception { | ||
PowershellScript s = new PowershellScript("echo envvar=$env:MYVAR"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars("MYVAR", "power$hell"), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws,baos); | ||
assertEquals(0, c.exitStatus(ws, launcher, listener).intValue()); | ||
assertThat(baos.toString(), containsString("envvar=power$hell")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void unicodeChars() throws Exception { | ||
PowershellScript s = new PowershellScript("Write-Output \"Helló, Wõrld ®\";"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
ByteArrayOutputStream baos = new ByteArrayOutputStream(); | ||
c.writeLog(ws, baos); | ||
assertEquals(Integer.valueOf(0), c.exitStatus(ws, launcher)); | ||
String log = baos.toString("UTF-8"); | ||
assertTrue(log, log.contains("Helló, Wõrld ®")); | ||
c.cleanup(ws); | ||
} | ||
|
||
@Test public void correctExitCode() throws Exception { | ||
PowershellScript s = new PowershellScript("exit 5;"); | ||
s.setPowershellBinary("pwsh"); | ||
Controller c = s.launch(new EnvVars(), ws, launcher, listener); | ||
while (c.exitStatus(ws, launcher, listener) == null) { | ||
Thread.sleep(100); | ||
} | ||
assertEquals(Integer.valueOf(5), c.exitStatus(ws, launcher)); | ||
c.cleanup(ws); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to do some validation here, or do you intentionally want to let users specify anything in case they've set up a symlink or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UI only allows two options however from API it could by anything.
Do you see an issue with that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into it some more, since jenkinsci/workflow-durable-task-step-plugin#122 is the only way users would be exposed to the change, and they wouldn't specify the actual binary, just the new
pwsh
step, I don't think validation is necessary.It seems a bit confusing that we even have Jelly files in this plugin, since IIUC they are not currently used anywhere, because the steps in workflow-durable-step define their own config and help files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jelly files here should appear in FreeStyle but as I wrote in the PR description. This does not currently work for master branch nor this PR.
Should I try and do a git bisect to determine when PowerShell option disappeared from Jelly view?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that feature was provided by this plugin: https://github.com/jenkinsci/powershell-plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, do you mind if I switch the
powershell
andpwsh
to public static final string or do you prefer enum?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean, I think the code in the PR is good as-is, no changes necessary. I'm happy to merge it once the CI build passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the CI build is unavailable? Getting a 503 page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think ci.jenkins.io is down right now (unfortunately a common occurrence these days). Looks like the infra team is investigating based on #jenkins-infra on IRC.