Adds container.user configuration #1109

CyrilleH · 2018-10-05T21:21:04Z

Towards #1029, this PR adds the possibility to sets the username or UID which the process in the container should run as.

For security reason, we prefer to start a tomcat container with non-root user. We have a generic base image with this (in our private registry) :

RUN addgroup -S tomcat && adduser -S -G tomcat tomcat && chown -R tomcat:tomcat /usr/local

coollog · 2018-10-05T21:28:00Z

Thanks for the contribution! We'll take a look at this soon. For the future, we'd recommend commenting on the issue thread with the design/solution you will be implementing before starting work to help save time potentially changing large portions of the implementation.

CyrilleH · 2018-10-05T21:30:50Z

Ok I understand. It was very easy to implement.
If you want I can close this PR.

coollog · 2018-10-05T21:36:49Z

We can definitely work with most of this :) It's mostly that we need to decide what we would support for the user configuration. Currently, as proposed in #1029 (comment), we are intending only to support user id and group id and not username, but we might decide to allow username as well. @GoogleContainerTools/java-tools

chanseokoh · 2018-10-05T21:44:15Z

Yeah, looks like we can work on this. There are a few things to think about though.

And thinking about it for a while, it might be possible to accept any string as the parameter value, not just numbers, if what we end up doing is to just set the value in the container config metadata. In that case, we may always be able to build an image (need to check), which may error at runtime due to non-existing username. ~~However, on the other hand, this may result in a Dockerfile that is not buildable.~~

chanseokoh · 2018-10-05T21:48:57Z

However, on the other hand, this may result in a Dockerfile that is not buildable.

Never mind. I think it is always buildable. Now I think there is no problem supporting usernames and it is the user's responsibility to use the base image that has the right user database. Same for usual Docker build.

CyrilleH · 2018-10-05T21:51:24Z

If we set the wrong username, we have this error at runtime :

docker: Error response from daemon: linux spec user: unable to find user tomcat2: no matching entries in passwd file.
ERRO[0000] error waiting for container: context canceled

chanseokoh · 2018-10-08T14:14:53Z

If we set the wrong username, we have this error at runtime

Yeah, it's the same story for Dockerfile, so perhaps we should say it's WAI. The big difference is, however, you can easily do adduser or addgroup with Dockerfile to make it work. But with Jib, it's a bit cumbersome in that you need to prepare a base image with the user or have /etc/passwd and /etc/group under /src/main/jib.

chanseokoh · 2018-10-08T14:16:37Z

jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java

@@ -136,14 +137,25 @@ public Builder setEntrypoint(@Nullable List<String> entrypoint) {
      return this;
    }

+    /**
+     * Sets the username or UID which the process in the container should run as.


I believe this can be a mix of user and group.

You're right, I changed the javadoc.

chanseokoh · 2018-10-08T14:17:10Z

jib-core/src/main/java/com/google/cloud/tools/jib/frontend/JavaDockerContextGenerator.java

@@ -299,6 +311,7 @@ public void generate(Path targetDirectory) throws IOException {
   * LABEL [key1]="[value1]" \
   *     [key2]="[value2]" \
   *     [...]
+   * USER [user or UID]


chanseokoh · 2018-10-08T14:18:30Z

jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java

+    /**
+     * Sets the username or UID which the process in the container should run as.
+     *
+     * @param user the username or UID


chanseokoh · 2018-10-08T14:18:44Z

jib-core/src/main/java/com/google/cloud/tools/jib/frontend/JavaDockerContextGenerator.java

+  /**
+   * Sets the username or UID which the process in the container should run as.
+   *
+   * @param user the username or UID.


Javadoc modified.

jib-core/src/main/java/com/google/cloud/tools/jib/image/json/ImageToJsonTranslator.java

coollog

Looks great! Just some comment nits.

coollog · 2018-10-09T19:20:28Z

jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java

@@ -136,14 +137,26 @@ public Builder setEntrypoint(@Nullable List<String> entrypoint) {
      return this;
    }

+    /**
+     * Sets the user name (or UID) and optionally the user group (or GID) which the process in the


Wording suggestion: Sets the user and group to run the container as. {@code user} can be a username or UID along with an optional groupname or GID. The following are all valid: {@code user}, {@code uid}, {@code user:group}, {@code uid:gid}, {@code uid:group}, {@code user:gid}.

coollog · 2018-10-09T19:21:05Z

jib-core/src/main/java/com/google/cloud/tools/jib/frontend/JavaDockerContextGenerator.java

@@ -191,6 +192,18 @@ public JavaDockerContextGenerator setEntrypoint(List<String> entrypoint) {
    return this;
  }

+  /**
+   * Sets the user name (or UID) and optionally the user group (or GID) which the process in the


Wording suggestion: Sets the user for the {@code USER} directive.

coollog · 2018-10-09T19:25:04Z

jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java

@@ -89,6 +90,18 @@
      return this;
    }

+    /**
+     * Sets the user name (or UID) and optionally the user group (or GID) which the process in the


Wording suggestion: Sets the user/group to run the container as.

coollog · 2018-10-09T19:25:24Z

jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java

+     * Sets the user name (or UID) and optionally the user group (or GID) which the process in the
+     * container should run as.
+     *
+     * @param user the username and optionally the user group


the username/UID and optionally the groupname/GID

coollog · 2018-10-09T19:25:42Z

jib-core/src/main/java/com/google/cloud/tools/jib/configuration/ContainerConfiguration.java

+     * Sets the user name (or UID) and optionally the user group (or GID) which the process in the
+     * container should run as.
+     *
+     * @param user the username and optionally the user group


the username/UID and optionally the groupname/GID

chanseokoh · 2018-10-09T19:42:16Z

Looks good to me. Updating some wordings, and I think we're good to go.

CyrilleH · 2018-10-09T21:40:50Z

Great. I've rebased on master and fixed wordings.

coollog

LGTM. Thanks for your contribution again! I think this just fixes #1029. We should also add CHANGELOG entries for this new feature, but that can be done in a follow-up PR.

jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/DockerContextTaskTest.java

googlebot added the cla: yes label Oct 5, 2018

chanseokoh reviewed Oct 8, 2018

View reviewed changes

chanseokoh added the kokoro:run label Oct 8, 2018

kokoro-team removed the kokoro:run label Oct 8, 2018

CyrilleH force-pushed the user-container-config branch 3 times, most recently from 3216c1a to dadc2f5 Compare October 8, 2018 19:53

chanseokoh added the kokoro:run label Oct 9, 2018

kokoro-team removed the kokoro:run label Oct 9, 2018

coollog reviewed Oct 9, 2018

View reviewed changes

CyrilleH added 3 commits October 9, 2018 23:21

Adds container.user configuration

1d92846

Fixes review comments

24420d3

Fixes some wordings

0082168

CyrilleH force-pushed the user-container-config branch from dadc2f5 to 0082168 Compare October 9, 2018 21:32

chanseokoh requested a review from a team October 9, 2018 21:44

chanseokoh added the kokoro:run label Oct 9, 2018

chanseokoh removed the request for review from a team October 9, 2018 22:03

kokoro-team removed the kokoro:run label Oct 9, 2018

coollog approved these changes Oct 9, 2018

View reviewed changes

jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/DockerContextTaskTest.java Show resolved Hide resolved

Adds a comment in unit test

aefb550

chanseokoh added the kokoro:run label Oct 10, 2018

kokoro-team removed the kokoro:run label Oct 10, 2018

chanseokoh merged commit a0bf210 into GoogleContainerTools:master Oct 10, 2018

chanseokoh mentioned this pull request Oct 10, 2018

Create images with a built-in non-root user #1029

Closed

CyrilleH deleted the user-container-config branch October 10, 2018 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds container.user configuration #1109

Adds container.user configuration #1109

CyrilleH commented Oct 5, 2018 •

edited

Loading

coollog commented Oct 5, 2018

CyrilleH commented Oct 5, 2018

coollog commented Oct 5, 2018

chanseokoh commented Oct 5, 2018 •

edited

Loading

chanseokoh commented Oct 5, 2018

CyrilleH commented Oct 5, 2018

chanseokoh commented Oct 8, 2018

chanseokoh Oct 8, 2018

CyrilleH Oct 8, 2018

chanseokoh Oct 8, 2018

chanseokoh Oct 8, 2018

chanseokoh Oct 8, 2018

CyrilleH Oct 8, 2018

coollog left a comment

coollog Oct 9, 2018

CyrilleH Oct 9, 2018

coollog Oct 9, 2018

CyrilleH Oct 9, 2018

coollog Oct 9, 2018

CyrilleH Oct 9, 2018

coollog Oct 9, 2018

CyrilleH Oct 9, 2018

coollog Oct 9, 2018

CyrilleH Oct 9, 2018

chanseokoh commented Oct 9, 2018

CyrilleH commented Oct 9, 2018

coollog left a comment

Adds container.user configuration #1109

Adds container.user configuration #1109

Conversation

CyrilleH commented Oct 5, 2018 • edited Loading

coollog commented Oct 5, 2018

CyrilleH commented Oct 5, 2018

coollog commented Oct 5, 2018

chanseokoh commented Oct 5, 2018 • edited Loading

chanseokoh commented Oct 5, 2018

CyrilleH commented Oct 5, 2018

chanseokoh commented Oct 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coollog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chanseokoh commented Oct 9, 2018

CyrilleH commented Oct 9, 2018

coollog left a comment

Choose a reason for hiding this comment

CyrilleH commented Oct 5, 2018 •

edited

Loading

chanseokoh commented Oct 5, 2018 •

edited

Loading