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

deprecate manually assigning DOCKER_USER #220

Merged
merged 4 commits into from
Oct 14, 2020
Merged

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Oct 9, 2020

Potential fix for #214 and #219

We're having an increasing number of issues coming in due to users setting the file permissions of the DATA_DIR (or subdirectories such as ${DATA_DIR}/elasticsearch/nodes) in such a way that the DOCKER_USER cannot read/write to them.

This PR is a proposal for how we might be able to improve that situation.

  • Add a fatal error if running the pelias binary as root, this no longer allowed.
  • Attempt to determine the correct user from the shell
  • Explicitly set DOCKER_USER to the current user, overriding it if already set in the env
  • Emit a warning when overriding so that the user is aware

What this doesn't cover.

  • Checking that the DATA_DIR is readable/writable by the current user (although that's simpler now).

@orangejulius
Copy link
Member

Cool, looks about right. In a quick test from scratch this works well for me (at least creating an Elasticsearch schema with an empty datadir).

Maybe we should test on a CentOS VM, since that seems to be one of the distros that commonly uses UIDs other than 1000 and therefore might run into issues.

@missinglink
Copy link
Member Author

I'm on Darwin with a default user of 501 so we should have that base covered.

@missinglink
Copy link
Member Author

missinglink commented Oct 9, 2020

I'd be interested to see the output of these commands on other systems:

MacOS Catalina

uname -a
Darwin Peters-MacBook-Pro-2.local 19.6.0 Darwin Kernel Version 19.6.0: Thu Jun 18 20:49:00 PDT 2020; root:xnu-6153.141.1~1/RELEASE_X86_64 x86_64

bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
501:20

sudo bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
501:20

Ubuntu 18.04.3 LTS

uname -a
Linux hetzner 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
1000:1000

sudo bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
1000:1000

@missinglink
Copy link
Member Author

CentOS Linux 8

uname -a
Linux ip-172-31-29-20.ec2.internal 4.18.0-193.6.3.el8_2.x86_64 #1 SMP Wed Jun 10 11:09:32 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
1000:1000

sudo bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
1000:1000

RHEL-8.2.0

uname -a
Linux ip-172-31-93-176.ec2.internal 4.18.0-193.el8.x86_64 #1 SMP Fri Mar 27 14:35:58 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
1000:1000

sudo bash -c 'echo $(id -u ${SUDO_USER-${USER}}):$(id -g ${SUDO_USER-${USER}})'
1000:1000

@missinglink
Copy link
Member Author

Should we just disallow running the pelias command as sudo?

I see no real reason to allow such a thing, except for the purpose of fixing already broken permissions, and in that case we can request sudo from the user.

@missinglink missinglink marked this pull request as ready for review October 12, 2020 15:43
@missinglink
Copy link
Member Author

On refection there is a valid use for sudo to impersonate a non-root user.

@orangejulius
Copy link
Member

orangejulius commented Oct 12, 2020

Don't forget to remove the section about setting DOCKER_USER from the quickstart in the readme:

# configure docker to write files as your local user
# see: https://github.com/pelias/docker#variable-docker_user
# note: use 'gsed' instead of 'sed' on a Mac
sed -i '/DOCKER_USER/d' .env
echo "DOCKER_USER=$(id -u)" >> .env

Otherwise I think this is the right path forward: autodetect $DOCKER_USER, do not allow running as root.

@orangejulius
Copy link
Member

Okay I've gone ahead and updated the README and rebased this. I think it's good to go now, anything left for us to test?

@orangejulius
Copy link
Member

I've added a new section in the readme about permissions. It specifically covers using a non-root user and ensuring Docker commands can be executed by that user. @missinglink let me know what you think.

@hennihaus
Copy link

Hey, I also checked out this repo on Ubuntu 18.04, have followed the new readme and the new docker guide. It works for me now. Thanks for the fast support regarding #219 .

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.

3 participants