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

CLI - set uid:gid of Che and workspaces #4050

Merged
merged 13 commits into from
Feb 20, 2017
Merged

CLI - set uid:gid of Che and workspaces #4050

merged 13 commits into from
Feb 20, 2017

Conversation

TylerJewell
Copy link

@TylerJewell TylerJewell commented Feb 7, 2017

Signed-off-by: Tyler Jewell tjewell@codenvy.com

What does this PR do?

This PR adapts the Che CLI to support letting users provide a different uid:gid combination to be applied to the Che server and its resulting workspaces. Users can pass --user uid:gid on the docker run ... command line and this will be passed into the eclipse/che-server container. If no user is provided, the default is set to root.

This feature should only be used with Linux hosts and will have odd behaviors on Windows.

What issues does this PR fix or reference?

Only merge this PR when the user improvements of @snjeza are merged.
#3376

Changelog

Add --user uid:gid and -e CHE_USER=uid:gid option to CLI to override user identity of Che container

Release Notes

You can now run Eclipse Che's container with a different user identity on Linux or Mac. The default is to run the Che container as a root user. You can now pass --user uid:gid or -e CHE_USER=uid:gid on the command line as Docker options to the eclipse/che Docker image. This image will launch the eclipse/che-server image with the same uid:gid combination along with mounting /etc/group and etc/passwd. When Che is run as a user, all files written from within the Che server to the host (such as che.env or cli.log will be written to disk with the custom user as the owner of the files. Changing the user is not available on Windows.

Docs PR

N/A - covered by the main PR

Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
@TylerJewell TylerJewell self-assigned this Feb 7, 2017
@TylerJewell TylerJewell added the kind/enhancement A feature request - must adhere to the feature request template. label Feb 7, 2017
@codenvy-ci
Copy link

@snjeza
Copy link
Contributor

snjeza commented Feb 7, 2017

@TylerJewell

You should mount /etc/passwd and /etc/group.
A user must have the write permission to the data directory.
We don't have to apply #3376 to test this PR. It works with eclipse/che-server:nightly.

Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
@TylerJewell
Copy link
Author

@snjeza - fixed by adding those additional mounts to the compose file if you are changing the user.

But help me a little bit - why isn't #3376 needed here? I thought #3376 added internal modifications to the che server necessary to support running the server under a different user?

Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
@codenvy-ci
Copy link

Build # 1930 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1930/ to view the results.

@snjeza
Copy link
Contributor

snjeza commented Feb 10, 2017

@TylerJewell

But help me a little bit - why isn't #3376 needed here? I thought #3376 added internal modifications to the che server necessary to support running the server under a different user?

#3376 is related to running a workspace as an user. #3265 is related to running the che server as an user.

@TylerJewell
Copy link
Author

Yes but the expectation we discussed with Mario was that if you set the Che server to run under a single user we would enforce that with workspaces to have the same as well. So we are missing that coupling.

@TylerJewell
Copy link
Author

@l0rd @snjeza - once we address the last item on running a workspace as a user, then I think this is ready for merge.

@codenvy-ci
Copy link

@snjeza
Copy link
Contributor

snjeza commented Feb 13, 2017

Yes but the expectation we discussed with Mario was that if you set the Che server to run under a single user we would enforce that with workspaces to have the same as well. So we are missing that coupling.

You are right, but this PR can be tested/applied without #3376. If we use --user , the che server should start as user. When we apply #3376, a workspace should be started as user.

I have tried to start che using your PR.
It works when I add the following patch:

diff --git a/dockerfiles/base/scripts/base/startup_02_pre_docker.sh b/dockerfiles/base/scripts/base/startup_02_pre_docker.sh
index 290dfb9..ee05557 100644
--- a/dockerfiles/base/scripts/base/startup_02_pre_docker.sh
+++ b/dockerfiles/base/scripts/base/startup_02_pre_docker.sh
@@ -404,7 +404,7 @@ check_interactive() {
 check_user() {
   CHE_USER=""
   CHE_USER=$(docker inspect --format='{{.Config.User}}' $(get_this_container_id))
-  if custom_user; then
+  if ! custom_user; then
     # Add checks / warnings if a custom user is set
     CHE_USER="root"
   fi
diff --git a/dockerfiles/che/entrypoint.sh b/dockerfiles/che/entrypoint.sh
index b2ca8ed..43441a6 100755
--- a/dockerfiles/che/entrypoint.sh
+++ b/dockerfiles/che/entrypoint.sh
@@ -237,9 +237,10 @@ init() {
       echo "!!!"
       exit 1
     fi
-    export CHE_USER_ID=`id -u ${CHE_USER}`:`getent group docker | cut -d: -f3`
-    sudo chown -R ${CHE_USER}:docker ${CHE_DATA}
-    sudo chown -R ${CHE_USER}:docker ${CHE_HOME}
+    export CHE_USER_ID=${CHE_USER}
+    sudo chown -R ${CHE_USER} ${CHE_DATA}
+    sudo chown -R ${CHE_USER} ${CHE_HOME}
+    sudo chown -R ${CHE_USER} ${CHE_LOGS_DIR}
   fi
   ### Are we going to use the embedded che.properties or one provided by user?`
   ### CHE_LOCAL_CONF_DIR is internal Che variable that sets where to load
@@ -247,7 +248,7 @@ init() {
     echo "Found custom che.properties..."
     export CHE_LOCAL_CONF_DIR="/conf"
     if [ "$CHE_USER" != "root" ]; then
-      sudo chown -R ${CHE_USER}:docker ${CHE_LOCAL_CONF_DIR}
+      sudo chown -R ${CHE_USER} ${CHE_LOCAL_CONF_DIR}
     fi
   else
     echo "Using embedded che.properties... Copying template to ${CHE_DATA_HOST}/conf."

Call the following commands to run the che server as an user:

$ git fetch origin pull/4050/head:che-4050
$ git checkout che-4050
# apply the patch
$ cd dockerfiles/init
$ ./build.sh
$ cd ../base
$ ./build.sh
$ cd ../cli
$ ./build.sh 
$ cd ../che
$ ./build.sh
$ grep test /etc/passwd
test:x:1001:1002::/home/test:/bin/bash
$ grep docker /etc/group
docker:x:1001:test,snjeza
$ sudo chown -R test:docker /tmp/data
$ docker run -it --rm \
  -v /var/run/docker.sock:/var/run/docker.sock \
  -v /tmp/data:/data \
  -v /etc/group:/etc/group:ro \
  -v /etc/passwd:/etc/passwd:ro \
  --user 1001:1001 \
    eclipse/che-cli:nightly config --skip:nightly --skip:pull
$ mkdir ~/test
$ cd ~/test
$ cp /tmp/data/instance/docker-compose.yml .
$ docker-compose up
...
$ $ docker exec che id
uid=1001(test) gid=1001(docker)

Che-cli creates the following docker-compose.yml:

# ###################################
# This file is generated by puppet
# PLEASE DON'T MODIFY BY HAND
# ###################################

che:
  image: eclipse/che-server:nightly
  env_file:
    - '/tmp/data/instance/config/che.env'
  volumes:
    - '/var/run/docker.sock:/var/run/docker.sock'
    - '/tmp/data/instance/data:/data'
    - '/tmp/data/instance/logs:/logs'
    - '/tmp/data/instance/config:/conf'
    - '/etc/group:/etc/group:ro'
    - '/etc/passwd:/etc/passwd:ro'
  ports:
    - '8080:8080'
  restart: always
  container_name: che
  user: 1001:1001

and adds the following env variable to che.env

CHE_USER=1001:1001

We have to mount /etc/passwd and /etc/group when running che-cli. Otherwise, che-cli will return an error.
A user that runs the che server, has to have the write permission to the /data directory (/tmp/data in my example).

Tyler Jewell added 6 commits February 14, 2017 16:06
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
@TylerJewell
Copy link
Author

Thanks for the pointers @snjeza. I did some refactoring to make the code more manageable long term. It was ok on my windows machine. I also provided the ability to set the uid:gid through either use of --user or with -e CHE_USER= syntax. Why don't you give this a shot and if it's good we'll merge it at the beginning of the next release cycle after 5.3 is released tomorrow or Thursday.

@codenvy-ci
Copy link

@snjeza
Copy link
Contributor

snjeza commented Feb 16, 2017

@TylerJewell

It works fine.

@TylerJewell
Copy link
Author

@l0rd - what is the merge plan here? At a minimum we should wait for 5.3 to release tomorrow. Will we also co-release this with the changes that allow for setting user identity within a workspace, too or should we merge this separately?

@codenvy-ci
Copy link

Build # 1993 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/1993/ to view the results.

@TylerJewell TylerJewell added this to the 5.4.0 milestone Feb 20, 2017
@TylerJewell TylerJewell merged commit b53932d into master Feb 20, 2017
@TylerJewell TylerJewell deleted the cli-user branch February 20, 2017 15:34
@JamesDrummond JamesDrummond mentioned this pull request Mar 8, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Signed-off-by: Tyler Jewell <tjewell@codenvy.com>
* Update docker-compose.yml.erb
* Add uid checks
@tsmaeder tsmaeder mentioned this pull request Mar 20, 2019
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants