Skip to content

Commit fadf42e

Browse files
MarkRxMarkRx
committed
JENKINS-75102 Fix Windows docker running Windows container with spaces in workspace path
1 parent 668294c commit fadf42e

File tree

3 files changed

+155
-28
lines changed

3 files changed

+155
-28
lines changed

‎src/main/java/org/jenkinsci/plugins/docker/workflow/WithContainerStep.java‎

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,6 @@
4242
import hudson.os.WindowsUtil;
4343
import hudson.slaves.WorkspaceList;
4444
import hudson.util.VersionNumber;
45-
import java.util.ArrayList;
46-
import java.util.Arrays;
47-
import java.util.Collection;
48-
import java.util.Iterator;
49-
import java.util.LinkedHashMap;
50-
import java.util.LinkedHashSet;
51-
import java.util.List;
52-
import java.util.Map;
53-
import java.util.Set;
54-
import java.util.TreeSet;
5545
import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
5646
import org.jenkinsci.plugins.docker.workflow.client.DockerClient;
5747
import org.jenkinsci.plugins.docker.workflow.client.WindowsDockerClient;
@@ -69,6 +59,16 @@
6959
import java.io.IOException;
7060
import java.io.Serializable;
7161
import java.nio.charset.Charset;
62+
import java.util.ArrayList;
63+
import java.util.Arrays;
64+
import java.util.Collection;
65+
import java.util.Iterator;
66+
import java.util.LinkedHashMap;
67+
import java.util.LinkedHashSet;
68+
import java.util.List;
69+
import java.util.Map;
70+
import java.util.Set;
71+
import java.util.TreeSet;
7272
import java.util.concurrent.TimeUnit;
7373
import java.util.logging.Level;
7474
import java.util.logging.Logger;
@@ -277,7 +277,11 @@ private static class Decorator extends LauncherDecorator implements Serializable
277277
if (hasWorkdir) {
278278
prefix.add("--workdir");
279279
masksPrefixList.add(false);
280-
prefix.add(path);
280+
if (super.isUnix()) {
281+
prefix.add(path);
282+
} else {
283+
prefix.add(WindowsUtil.quoteArgument(path));
284+
}
281285
masksPrefixList.add(false);
282286
} else {
283287
String safePath = path.replace("'", "'\"'\"'");
@@ -333,8 +337,20 @@ private static class Decorator extends LauncherDecorator implements Serializable
333337
originalMasks = new boolean[starter.cmds().size()];
334338
}
335339

336-
// Adapted from decorateByPrefix:
337-
starter.cmds().addAll(0, prefix);
340+
List<String> cmds = new ArrayList<>();
341+
cmds.addAll(prefix);
342+
343+
if (!super.isUnix() && starter.cmds().size() >= 3 && "cmd".equals(starter.cmds().get(0)) && "/c".equalsIgnoreCase(starter.cmds().get(1))) {
344+
// JENKINS-75102 Docker exec on Windows processes character escaping differently.
345+
// Modify launch to work with special characters in a way that docker exec can handle.
346+
cmds.addAll(starter.cmds().subList(0, 2));
347+
cmds.add("call");
348+
cmds.addAll(starter.cmds().subList(2, starter.cmds().size()).stream()
349+
.map(cmd -> cmd.replaceAll("\"\"(.*)\"\"", "\"$1\"")).collect(Collectors.toList()));
350+
} else {
351+
cmds.addAll(starter.cmds());
352+
}
353+
starter.cmds(cmds);
338354

339355
boolean[] masks = new boolean[originalMasks.length + prefix.size()];
340356
boolean[] masksPrefix = new boolean[masksPrefixList.size()];

‎src/test/java/org/jenkinsci/plugins/docker/workflow/DockerTestUtil.java‎

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,40 @@
2424
package org.jenkinsci.plugins.docker.workflow;
2525

2626
import hudson.EnvVars;
27-
import org.jenkinsci.plugins.docker.workflow.client.DockerClient;
2827
import hudson.Launcher;
2928
import hudson.util.StreamTaskListener;
3029
import hudson.util.VersionNumber;
30+
import org.hamcrest.Matchers;
31+
import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
32+
import org.jenkinsci.plugins.docker.workflow.client.DockerClient;
3133
import org.junit.Assume;
3234

35+
import java.io.ByteArrayOutputStream;
3336
import java.io.IOException;
37+
import java.util.Arrays;
38+
import java.util.List;
39+
import java.util.Optional;
3440
import java.util.concurrent.TimeUnit;
35-
36-
import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
41+
import java.util.regex.Matcher;
42+
import java.util.regex.Pattern;
3743

3844
/**
3945
* @author <a href="mailto:tom.fennelly@gmail.com">tom.fennelly@gmail.com</a>
4046
*/
4147
public class DockerTestUtil {
4248
public static String DEFAULT_MINIMUM_VERSION = "1.3";
4349

50+
// Major Windows kernel versions. See https://hub.docker.com/r/microsoft/windows-nanoserver
51+
private static List<String> MAJOR_WINDOWS_KERNEL_VERSIONS = Arrays.asList(
52+
"10.0.17763.6659", // 1809
53+
"10.0.18363.1556", // 1909
54+
"10.0.19041.1415", // 2004
55+
"10.0.19042.1889", // 20H2
56+
"10.0.20348.2966", // 2022
57+
"10.0.26100.2605" // 2025
58+
);
59+
60+
4461
public static void assumeDocker() throws Exception {
4562
assumeDocker(new VersionNumber(DEFAULT_MINIMUM_VERSION));
4663
}
@@ -61,10 +78,78 @@ public static void assumeDocker(VersionNumber minimumVersion) throws Exception {
6178
Assume.assumeFalse("Docker version not < " + minimumVersion.toString(), dockerClient.version().isOlderThan(minimumVersion));
6279
}
6380

81+
/**
82+
* Used to assume docker Windows is running in a particular os mode
83+
* @param os The os [windows, linux]
84+
* @throws Exception
85+
*/
86+
public static void assumeDockerServerOSMode(String os) throws Exception {
87+
Launcher.LocalLauncher localLauncher = new Launcher.LocalLauncher(StreamTaskListener.NULL);
88+
try {
89+
ByteArrayOutputStream out = new ByteArrayOutputStream();
90+
int status = localLauncher
91+
.launch()
92+
.cmds(DockerTool.getExecutable(null, null, null, null), "version", "-f", "{{.Server.Os}}")
93+
.stdout(out)
94+
.start()
95+
.joinWithTimeout(DockerClient.CLIENT_TIMEOUT, TimeUnit.SECONDS, localLauncher.getListener());
96+
Assume.assumeTrue("Docker working", status == 0);
97+
Assume.assumeThat("Docker running in " + os + " mode", out.toString().trim(), Matchers.equalToIgnoringCase(os));
98+
} catch (IOException x) {
99+
Assume.assumeNoException("Docker retrieve OS", x);
100+
}
101+
}
102+
103+
public static void assumeWindows() throws Exception {
104+
Assume.assumeTrue(System.getProperty("os.name").toLowerCase().contains("windows"));
105+
}
106+
64107
public static void assumeNotWindows() throws Exception {
65108
Assume.assumeFalse(System.getProperty("os.name").toLowerCase().contains("windows"));
66109
}
67110

111+
public static String getWindowsKernelVersion() throws Exception {
112+
Launcher.LocalLauncher localLauncher = new Launcher.LocalLauncher(StreamTaskListener.NULL);
113+
ByteArrayOutputStream out = new ByteArrayOutputStream();
114+
ByteArrayOutputStream err = new ByteArrayOutputStream();
115+
116+
int status = localLauncher
117+
.launch()
118+
.cmds("cmd", "/c", "ver")
119+
.stdout(out)
120+
.stderr(err)
121+
.start()
122+
.joinWithTimeout(DockerClient.CLIENT_TIMEOUT, TimeUnit.SECONDS, localLauncher.getListener());
123+
124+
if (status != 0) {
125+
throw new RuntimeException(String.format("Failed to obtain Windows kernel version with exit code: %d stdout: %s stderr: %s", status, out, err));
126+
}
127+
128+
Matcher matcher = Pattern.compile("Microsoft Windows \\[Version ([^\\]]+)\\]").matcher(out.toString().trim());
129+
130+
if (matcher.matches()) {
131+
return matcher.group(1);
132+
} else {
133+
throw new RuntimeException("Unable to obtain Windows kernel version from output: " + out);
134+
}
135+
}
136+
137+
/**
138+
* @return The image tag of an image with a kernel version corresponding to the closest compatible Windows release
139+
* @throws Exception
140+
*/
141+
public static String getWindowsImageTag() throws Exception {
142+
// Kernel must match when running Windows containers on docker on Windows if < Windows 11 with Server 2022
143+
String kernelVersion = DockerTestUtil.getWindowsKernelVersion();
144+
145+
// Select the highest well known kernel version <= ours since sometimes an image may not exist for our version
146+
Optional<String> wellKnownKernelVersion = MAJOR_WINDOWS_KERNEL_VERSIONS.stream()
147+
.filter(k -> k.compareTo(kernelVersion) <= 0).max(java.util.Comparator.naturalOrder());
148+
149+
// Fall back to trying our kernel version
150+
return wellKnownKernelVersion.orElse(kernelVersion);
151+
}
152+
68153
public static EnvVars newDockerLaunchEnv() {
69154
// Create the KeyMaterial for connecting to the docker host/server.
70155
// E.g. currently need to add something like the following to your env

‎src/test/java/org/jenkinsci/plugins/docker/workflow/WithContainerStepTest.java‎

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,10 @@
3737
import hudson.util.ArgumentListBuilder;
3838
import hudson.util.Secret;
3939
import hudson.util.StreamTaskListener;
40-
import java.io.File;
41-
import java.util.Collection;
42-
import java.util.Collections;
43-
import java.util.logging.Level;
44-
4540
import hudson.util.VersionNumber;
46-
import java.io.IOException;
47-
import java.lang.reflect.Field;
48-
import java.util.Set;
49-
import java.util.concurrent.TimeUnit;
5041
import org.apache.commons.fileupload.FileItem;
5142
import org.apache.commons.io.FileUtils;
5243
import org.hamcrest.Matchers;
53-
import static org.hamcrest.Matchers.is;
5444
import org.jenkinsci.lib.configprovider.ConfigProvider;
5545
import org.jenkinsci.lib.configprovider.model.Config;
5646
import org.jenkinsci.plugins.configfiles.custom.CustomConfig;
@@ -69,11 +59,10 @@
6959
import org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution;
7060
import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
7161
import org.junit.Assume;
72-
import static org.junit.Assume.assumeTrue;
7362
import org.junit.ClassRule;
7463
import org.junit.Ignore;
75-
import org.junit.Test;
7664
import org.junit.Rule;
65+
import org.junit.Test;
7766
import org.junit.rules.TemporaryFolder;
7867
import org.junit.runners.model.Statement;
7968
import org.jvnet.hudson.test.BuildWatcher;
@@ -84,6 +73,18 @@
8473
import org.jvnet.hudson.test.TestExtension;
8574
import org.kohsuke.stapler.DataBoundConstructor;
8675

76+
import java.io.File;
77+
import java.io.IOException;
78+
import java.lang.reflect.Field;
79+
import java.util.Collection;
80+
import java.util.Collections;
81+
import java.util.Set;
82+
import java.util.concurrent.TimeUnit;
83+
import java.util.logging.Level;
84+
85+
import static org.hamcrest.Matchers.is;
86+
import static org.junit.Assume.assumeTrue;
87+
8788
public class WithContainerStepTest {
8889

8990
@ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
@@ -491,4 +492,29 @@ private static final class Execution extends SynchronousNonBlockingStepExecution
491492
});
492493
}
493494

495+
@Issue("JENKINS-75102")
496+
@Test public void windowsRunningWindowsContainerSpaceInPath() {
497+
// Launching batch scripts through cmd /c in docker exec gets tricky with special characters
498+
// By default, the path of the temporary Jenkins install and workspace have a space in a folder name and a prj@tmp folder
499+
story.addStep(new Statement() {
500+
@Override public void evaluate() throws Throwable {
501+
DockerTestUtil.assumeWindows();
502+
DockerTestUtil.assumeDocker();
503+
DockerTestUtil.assumeDockerServerOSMode("windows");
504+
505+
// Kernel must match when running Windows containers on docker on Windows
506+
String releaseTag = DockerTestUtil.getWindowsImageTag();
507+
508+
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj");
509+
p.setDefinition(new CpsFlowDefinition(
510+
"node {\n" +
511+
" withDockerContainer('mcr.microsoft.com/windows/nanoserver:" + releaseTag + "') { \n" +
512+
" bat 'echo ran OK' \n" +
513+
" }\n" +
514+
"}", true));
515+
WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
516+
story.j.assertLogContains("ran OK", b);
517+
}
518+
});
519+
}
494520
}

0 commit comments

Comments
 (0)