Skip to content
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

Set --user on exec, not on main run command #130

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -24,6 +24,7 @@
package org.jenkinsci.plugins.docker.workflow;

import com.google.common.base.Optional;
import hudson.util.ArgumentListBuilder;
import org.jenkinsci.plugins.docker.workflow.client.DockerClient;
import com.google.inject.Inject;
import hudson.AbortException;
Expand Down Expand Up @@ -181,7 +182,13 @@ 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");
/**
* owner of the workspace. We need to ensure build process inside container do write files in workspace
* with same UID so agent can manage workspace when build inside container has completed.
*/
final String userId = dockerClient.whoAmI();

container = dockerClient.run(env, step.image, step.args, ws, volumes, volumesFromContainers, envReduced, null, /* expected to hang until killed */ "cat");
final List<String> ps = dockerClient.listProcess(env, container);
if (!ps.contains("cat")) {
listener.error("The container started but didn't run the expected command. Please double check your ENTRYPOINT does execute the command passed as docker run argument. See https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#entrypoint for entrypoint best practices.");
Expand All @@ -190,7 +197,7 @@ public static class Execution extends AbstractStepExecutionImpl {
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, toolName, dockerVersion))).
withContext(BodyInvoker.mergeLauncherDecorators(getContext().get(LauncherDecorator.class), new Decorator(container, envHost, ws, userId, toolName, dockerVersion))).
withCallback(new Callback(container, toolName)).
start();
return false;
Expand All @@ -216,17 +223,19 @@ private static class Decorator extends LauncherDecorator implements Serializable
private final String container;
private final String[] envHost;
private final String ws;
private final String user;
private final @CheckForNull String toolName;
private final boolean hasEnv;
private final boolean hasWorkdir;

Decorator(String container, EnvVars envHost, String ws, String toolName, VersionNumber dockerVersion) {
Decorator(String container, EnvVars envHost, String ws, String user, String toolName, VersionNumber dockerVersion) {
this.container = container;
this.envHost = Util.mapToEnv(envHost);
this.ws = ws;
this.toolName = toolName;
this.hasEnv = dockerVersion != null && dockerVersion.compareTo(new VersionNumber("1.13.0")) >= 0;
this.hasWorkdir = dockerVersion != null && dockerVersion.compareTo(new VersionNumber("17.12")) >= 0;
this.user = user;
}

@Override public Launcher decorate(final Launcher launcher, final Node node) {
Expand All @@ -238,7 +247,14 @@ private static class Decorator extends LauncherDecorator implements Serializable
} catch (InterruptedException x) {
throw new IOException(x);
}

List<String> prefix = new ArrayList<>(Arrays.asList(executable, "exec"));

if (user != null) {
prefix.add("-u");
prefix.add(user);
}

if (ws != null) {
FilePath cwd = starter.pwd();
if (cwd != null) {
Expand Down
Expand Up @@ -32,6 +32,9 @@
import hudson.util.ArgumentListBuilder;
import hudson.util.VersionNumber;
import org.jenkinsci.plugins.docker.commons.fingerprint.ContainerRecord;
import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
Expand All @@ -44,21 +47,18 @@
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.Map;
import java.util.List;
import java.util.Arrays;
import java.util.Map;
import java.util.StringTokenizer;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.jenkinsci.plugins.docker.commons.tools.DockerTool;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Simple docker client for Pipeline.
Expand Down Expand Up @@ -103,14 +103,18 @@ public DockerClient(@Nonnull Launcher launcher, @CheckForNull Node node, @CheckF
* @param command 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... command) 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... command) throws IOException, InterruptedException {
ArgumentListBuilder argb = new ArgumentListBuilder();

argb.add("run", "-t", "-d", "-u", user);
argb.add("run", "-t", "-d");

if (args != null) {
argb.addTokenized(args);
}


if (user != null) {
argb.add("-u", user);
}
if (workdir != null) {
argb.add("-w", workdir);
}
Expand Down
Expand Up @@ -228,6 +228,50 @@ 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);
}
});
}


@Issue("JENKINS-27152")
@Test public void configFile() throws Exception {
story.addStep(new Statement() {
Expand Down