Remove user from docker run command

demo/Dockerfile

@@ -26,14 +26,14 @@ FROM jenkinsci/workflow-demo:2.3
 
 USER root
 
-RUN apt-get update
-RUN apt-get install -y sudo
-RUN adduser jenkins sudo
-RUN echo 'Defaults !authenticate' >> /etc/sudoers
+RUN apt-get update && \
+    apt-get install -y sudo
+RUN adduser jenkins sudo && \
+    echo 'Defaults !authenticate' >> /etc/sudoers
 
 # Minimum for docker-workflow. Docker server does not accept a client newer than itself (older is OK).
-RUN wget -O /usr/bin/docker --no-check-certificate https://get.docker.com/builds/Linux/x86_64/docker-1.4.0
-RUN chmod a+x /usr/bin/docker
+RUN wget --no-verbose -O /usr/bin/docker --no-check-certificate https://get.docker.com/builds/Linux/x86_64/docker-1.7.0 && \
+    chmod a+x /usr/bin/docker
 
 ADD repo /tmp/repo
 RUN git config --global user.email "[email protected]" && git config --global user.name "Docker Workflow Demo" && cd /tmp/repo && git init && git add . && git commit -m 'demo'

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

@@ -69,7 +69,7 @@
 import org.kohsuke.stapler.DataBoundSetter;
 
 public class WithContainerStep extends AbstractStepImpl {
-    
+
     private static final Logger LOGGER = Logger.getLogger(WithContainerStep.class.getName());
     private final @Nonnull String image;
     private String args;
@@ -78,7 +78,7 @@ public class WithContainerStep extends AbstractStepImpl {
     @DataBoundConstructor public WithContainerStep(@Nonnull String image) {
         this.image = image;
     }
-    
+
     public String getImage() {
         return image;
     }
@@ -132,11 +132,11 @@ public static class Execution extends AbstractStepExecutionImpl {
             // Add a warning if the docker version is less than 1.4
             VersionNumber dockerVersion = dockerClient.version();
             if (dockerVersion != null) {
-                if (dockerVersion.isOlderThan(new VersionNumber("1.4"))) {
-                    throw new AbortException("The docker version is less than v1.4. Pipeline functions requiring 'docker exec' will not work e.g. 'docker.inside'.");
-                }
+              if (dockerVersion.isOlderThan(new VersionNumber("1.7"))) {
+                  throw new AbortException("The docker version is less than v1.7. Workflow functions requiring 'docker exec' will not work e.g. 'docker.inside'.");
+              }
             } else {
-                listener.error("Failed to parse docker version. Please note there is a minimum docker version requirement of v1.4.");
+                listener.error("Failed to parse docker version. Please note there is a minimum docker version requirement of v1.7.");
             }
 
             FilePath tempDir = tempDir(workspace);
@@ -170,11 +170,14 @@ public static class Execution extends AbstractStepExecutionImpl {
                 volumes.put(tmp, tmp);
             }
 
-            container = dockerClient.run(env, step.image, step.args, ws, volumes, volumesFromContainers, envReduced, dockerClient.whoAmI(), /* expected to hang until killed */ "cat");
+            // run command before starting container in case of exception
+            // Designed to prevent a situation where we cannot cleanup any files that have been created.
+            String user = dockerClient.whoAmI();
+            container = dockerClient.run(env, step.image, step.args, ws, volumes, volumesFromContainers, envReduced, null, /* expected to hang until killed */ "cat");
             DockerFingerprints.addRunFacet(dockerClient.getContainerRecord(env, container), run);
             ImageAction.add(step.image, run);
             getContext().newBodyInvoker().
-                    withContext(BodyInvoker.mergeLauncherDecorators(getContext().get(LauncherDecorator.class), new Decorator(container, envHost, ws))).
+                    withContext(BodyInvoker.mergeLauncherDecorators(getContext().get(LauncherDecorator.class), new Decorator(container, envHost, ws, user))).
                     withCallback(new Callback(container, toolName)).
                     start();
             return false;
@@ -200,17 +203,24 @@ private static class Decorator extends LauncherDecorator implements Serializable
         private final String container;
         private final String[] envHost;
         private final String ws;
+        private final String userId;
 
-        Decorator(String container, EnvVars envHost, String ws) {
+        Decorator(String container, EnvVars envHost, String ws, String userId) {
             this.container = container;
             this.envHost = Util.mapToEnv(envHost);
             this.ws = ws;
+            this.userId = userId;
         }
 
         @Override public Launcher decorate(final Launcher launcher, Node node) {
             return new Launcher.DecoratedLauncher(launcher) {
                 @Override public Proc launch(Launcher.ProcStarter starter) throws IOException {
-                    List<String> prefix = new ArrayList<String>(Arrays.asList("docker", "exec", container, "env"));
+                    List<String> prefix;
+                    if (userId == null) {
+                        prefix = new ArrayList<String>(Arrays.asList("docker", "exec", container, "env"));
+                    } else {
+                        prefix = new ArrayList<String>(Arrays.asList("docker", "exec", "--user", userId, container, "env"));
+                    }
                     if (ws != null) {
                         FilePath cwd = starter.pwd();
                         if (cwd != null) {
@@ -220,6 +230,7 @@ private static class Decorator extends LauncherDecorator implements Serializable
                             }
                         }
                     } // otherwise we are loading an old serialized Decorator
+
                     Set<String> envReduced = new TreeSet<String>(Arrays.asList(starter.envs()));
                     envReduced.removeAll(Arrays.asList(envHost));
                     prefix.addAll(envReduced);

src/main/java/org/jenkinsci/plugins/docker/workflow/client/DockerClient.java

@@ -95,10 +95,13 @@ public DockerClient(@Nonnull Launcher launcher, @CheckForNull Node node, @CheckF
      * @param entrypoint The command to execute in the image container being run.
      * @return The container ID.
      */
-    public String run(@Nonnull EnvVars launchEnv, @Nonnull String image, @CheckForNull String args, @CheckForNull String workdir, @Nonnull Map<String, String> volumes, @Nonnull Collection<String> volumesFromContainers, @Nonnull EnvVars containerEnv, @Nonnull String user, @Nonnull String entrypoint) throws IOException, InterruptedException {
+    public String run(@Nonnull EnvVars launchEnv, @Nonnull String image, @CheckForNull String args, @CheckForNull String workdir, @Nonnull Map<String, String> volumes, @Nonnull Collection<String> volumesFromContainers, @Nonnull EnvVars containerEnv, @CheckForNull String user, @Nonnull String entrypoint) throws IOException, InterruptedException {
         ArgumentListBuilder argb = new ArgumentListBuilder();
 
-        argb.add("run", "-t", "-d", "-u", user);
+        argb.add("run", "-t", "-d");
+        if (user != null) {
+            argb.add("-u", user);
+        }
         if (args != null) {
             argb.addTokenized(args);
         }

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

@@ -236,4 +236,47 @@ public class WithContainerStepTest {
         });
     }
 
+    @Test public void withExecAsUser() throws Exception {
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                DockerTestUtil.assumeDocker();
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj");
+                p.setDefinition(new CpsFlowDefinition(
+                    "node {" +
+                    "  withDockerContainer(args: '--entrypoint /usr/sbin/init -v /sys/fs/cgroup:/sys/fs/cgroup:ro --privileged', image: 'centos/systemd') {" +
+                    "    sh 'systemctl status'" +
+                    "  }" +
+                    "}", true));
+                WorkflowRun b = story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
+                story.j.assertLogContains("State: running", b);
+            }
+        });
+    }
+
+    /**
+     * When running systemd not as root, systemd will fail to start.  This will cause any systemctl commands
+     * to throw an error about it failing to get a D-Bus connection.
+     *
+     * This test simulates the (previous) behavior of the plugin due to the actual tests running as root.
+     * @throws Exception
+     */
+    @Test public void withUserInRunFails() throws Exception {
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                DockerTestUtil.assumeDocker();
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "prj");
+                p.setDefinition(new CpsFlowDefinition(
+                    "node {\n" +
+                    "  withDockerContainer(args: '--entrypoint /usr/sbin/init --user 1234:1234 -v /sys/fs/cgroup:/sys/fs/cgroup:ro --privileged', image: 'centos/systemd') {\n" +
+                    "    sh 'systemctl status'\n" +
+                    "  }\n" +
+                    "}\n", true));
+                WorkflowRun b = story.j.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+                story.j.assertLogContains("Failed to get D-Bus connection: Operation not permitted", b);
+            }
+        });
+    }
+
 }

src/test/java/org/jenkinsci/plugins/docker/workflow/client/DockerClientTest.java

@@ -77,6 +77,26 @@ public void test_run() throws IOException, InterruptedException {
         Assert.assertNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
     }
 
+    @Test
+    public void test_run_without_user() throws IOException, InterruptedException {
+        EnvVars launchEnv = newLaunchEnv();
+        String containerId =
+                dockerClient.run(launchEnv, "learn/tutorial", null, null, Collections.<String, String>emptyMap(), Collections.<String>emptyList(), new EnvVars(),
+                        null, "cat");
+        Assert.assertEquals(64, containerId.length());
+        ContainerRecord containerRecord = dockerClient.getContainerRecord(launchEnv, containerId);
+        Assert.assertEquals(dockerClient.inspect(launchEnv, "learn/tutorial", ".Id"), containerRecord.getImageId());
+        Assert.assertTrue(containerRecord.getContainerName().length() > 0);
+        Assert.assertTrue(containerRecord.getHost().length() > 0);
+        Assert.assertTrue(containerRecord.getCreated() > 1000000000000L);
+        Assert.assertEquals(Collections.<String>emptyList(), dockerClient.getVolumes(launchEnv, containerId));
+
+        // Also test that the stop works and cleans up after itself
+        Assert.assertNotNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
+        dockerClient.stop(launchEnv, containerId);
+        Assert.assertNull(dockerClient.inspect(launchEnv, containerId, ".Name"));
+    }
+
     @Test
     public void test_valid_version() {
         VersionNumber dockerVersion = DockerClient.parseVersionNumber("Docker version 1.5.0, build a8a31ef");