Skip to content

Make the privilege drop down optional #14

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

Closed
wants to merge 3 commits into from
Closed

Make the privilege drop down optional #14

wants to merge 3 commits into from

Conversation

ggtools
Copy link
Contributor

@ggtools ggtools commented Mar 26, 2015

As in some situation, it is impossible to execute chown on the
data directory running elasticsearch as the elasticsearch user is
not possible. The entry point now supports to run elasticsearch
as root if the RUN_AS_ROOT environment variable is set to a non
empty string.

Fixes #5

As in some situation, it is impossible to execute `chown` on the
data directory running elasticsearch as the `elasticsearch` user is
not possible. The entry point now supports to run elasticsearch
as `root` if the `RUN_AS_ROOT` environment variable is set to a non
empty string.

Fixes #5
@md5
Copy link
Contributor

md5 commented Mar 26, 2015

How about having : ${ELASTICSEARCH_USER:=elasticsearch} instead of RUN_AS_ROOT?

@md5
Copy link
Contributor

md5 commented Mar 26, 2015

That would imply changing the chown and gosu commands to use $ELASTICSEARCH_USER and skipping them when "$ELASTICSEARCH_USER" = root.

@ggtools
Copy link
Contributor Author

ggtools commented Mar 26, 2015

Yes I thought of this but then I looked at the chown elasticsearch:elasticsearch and I did wonder if I wanted to replace it by:

  • chown $ELASTICSEARCH_USER:elasticsearch
  • chown $ELASTICSEARCH_USER:$ELASTICSEARCH_USER
  • chown $ELASTICSEARCH_USER
  • or find the primary group of $ELASTICSEARCH_USER and using it

@yosifkit
Copy link
Member

A user might want to make the ability to tack on the group; -e ELASTICSEARCH_USER='999:50' would be the common option.

@ggtools
Copy link
Contributor Author

ggtools commented Mar 26, 2015

Love this @yosifkit so the test would be to check if the user part is root

@yosifkit
Copy link
Member

Seems it would also need to check if someone decided to put 0 for root as well.

@ggtools
Copy link
Contributor Author

ggtools commented Mar 26, 2015

Yup. So working on it ...

@md5
Copy link
Contributor

md5 commented Mar 26, 2015

Not to complicate things, but ELASTICSEARCH_GROUP might be clearer.

@md5
Copy link
Contributor

md5 commented Mar 26, 2015

I also just thought about the fact that there will be some inconsistency across the official images in the use of *_USER variables. In the case of postgres, the POSTGRES_USER variable refers to a DB-level user, not a Unix user.

@yosifkit
Copy link
Member

I am thinking this change can apply for all "data/server" images (ex: mysql, mongo, elasticsearch, redis, memcached...), so I would like to have it just as -e RUN_AS user:group. That way it is consistent on all the images.

@ggtools
Copy link
Contributor Author

ggtools commented Mar 26, 2015

That's why RUN_AS_XXX might be a little be clearer

@md5
Copy link
Contributor

md5 commented Mar 26, 2015

👍

@yosifkit
Copy link
Member

Quick wrench to throw in, would the RUN_AS only apply for running the contained software (in this case elasticsearch) or would it also apply if I run something else in the container (ex: docker run -it --rm -e RUN_AS='999' elasticsearch bash)? I can easily see users get confused either way and we will need to be sure to document it.

@ggtools
Copy link
Contributor Author

ggtools commented Mar 26, 2015

Hmmm interesting ... current it would only apply to elasticsearch, not other commands. I can see your point however one goals of the entry point script was to run elasticsearch with the correct user and then give a bypass for everything else. Since the command would be executed directly you should probably resort to the normal docker mechanism and use -u 999 if you don't want your command to be run as root

@yosifkit
Copy link
Member

That is what I was thinking, if they really need to be another user they are root and have gosu or su. We just need to make sure users know that the ENV only applies to running the contained software and that actions like bash will run as root.

The user and group can now be choosen using the `RUN_AS` environment
variable. `RUN_AS` should follow the `user[:group]` pattern and both
user/group numbers and user/group names are accepted.

The `chown`/`gosu` mechanism is skipped when the user is either `0`
or `root` or if the command is not `elasticsearch`.
@ggtools
Copy link
Contributor Author

ggtools commented Mar 26, 2015

And voilà

@md5
Copy link
Contributor

md5 commented Mar 27, 2015

LGTM

@md5 md5 mentioned this pull request Mar 27, 2015
@tianon
Copy link
Member

tianon commented Apr 2, 2015

Here's my thoughts on this:

  • RUN_AS is a really generic name; more generic than any environment variable we've used in any official image thus far -- my concern with that is people will start thinking it applies in more places than it does, and either wondering why it isn't working or even worse not even noticing that it's not working and thinking that it is
  • chown and gosu accept the same syntax for their arguments -- user, uid, user:group, uid:gid, user:gid, uid:group are all reasonable and acceptable for both, so if we do this, having two knobs for it seems like overkill IMO

@ggtools
Copy link
Contributor Author

ggtools commented Apr 9, 2015

I mostly agree with both items however for the first one I don't have any idea of a replacement name.

@ei-grad
Copy link

ei-grad commented Apr 13, 2015

Isn't the docker run --user option made for this?.. Using gosu to drop priveledges is absolutely useless ugly hack. And making chown for all data on the provided volume is a very bad thing, too.

@tianon
Copy link
Member

tianon commented Apr 13, 2015 via email

@ei-grad
Copy link

ei-grad commented Apr 13, 2015

It is not the problem, as it could (and should) be easily fixed by user. If he really need to chown. But the --user flag makes it possible to don't make chown at all.

@tianon
Copy link
Member

tianon commented Apr 13, 2015 via email

@ggtools
Copy link
Contributor Author

ggtools commented May 13, 2015

I can add some support to make the script work when the container is launched with a --user option but I'm pretty sure the chown and drop privileges thing is probably the easiest way for the user especially in the future when Docker will use the user namespace which will make quite hard to make the chown to the right user from outside the container

@nizsheanez
Copy link

Guys, what is the right way to start elasticsearch:2 for now?
I using Docker Toolbox for Mac

With --user:

docker run --user elasticsearch -it -v $(pwd)/es/data:/usr/share/elasticsearch/data -v $(pwd)/es/config:/usr/share/elasticsearch/config elasticsearch
error: failed switching to "elasticsearch": setgroups operation not permitted

Without --user:

docker run -it -v $(pwd)/es/data:/usr/share/elasticsearch/data -v $(pwd)/es/config:/usr/share/elasticsearch/config elasticsearch
Exception in thread "main" java.lang.IllegalStateException: Unable to access 'path.scripts' (/usr/share/elasticsearch/config/scripts)

Guy in last comment https://hub.docker.com/_/elasticsearch/ has same problem

@ironspecs
Copy link

bump Seems this thread was abandoned, but the issue is still real.

@yosifkit
Copy link
Member

yosifkit commented Jan 6, 2016

After some discussion offline with @tianon, we are thinking that the best course of action is to gracefully skip operations that require root when the container is not run as root for all official images (as far as is possible). This will allow you to run with --user, but you would be responsible to chown/chmod any files and directories that the process needs.

So, something like:

if [ "$(id -u)" = '0' ]; then
    chown ...
    set -- gosu ...
fi

exec "$@"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run as user "elasticsearch"
7 participants