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

Create jovyan in Dockerfile, reset UID at startup #31

Merged
merged 6 commits into from
Sep 13, 2015

Conversation

parente
Copy link
Member

@parente parente commented Sep 11, 2015

  • Create user jovyan with UID=1000 in the default users group in the Dockerfile
  • Set group ownership of user home and conda to root to avoid 'users' group from host access when mounted
  • Set stick bit on both paths so root owns subdirs too
  • Change jovyan UID if NB_UID is specified and is not the default 1000

@costerwi Can you give this a shot and see if it retains what you're looking for via the NB_UID runtime env var plus fixes the problems you were correcting in your latest PRs? Meanwhile, I'm going to see if it fixes both the bash and julia kernel install problems people have reported with the current setup.

@parente parente force-pushed the early-user-late-uid branch from a596a07 to fa8d7b5 Compare September 11, 2015 02:37
@parente
Copy link
Member Author

parente commented Sep 11, 2015

Argh, docker fun:

drwx--S--- 4 jovyan root 4096 Sep 11 02:58 share
jovyan@a7de91967404:~/.local$ cd share/
bash: cd: share/: Permission denied

So after installing ijulia, the /home/jovyan/.local folder gets created. Later, when the script switches the jovyan uid to 1005 from 1000 (for testing), the jovyan user can no access the .local folder even though that user clearly owns the folder. All other folders (e.g., .jupyter) it has no problems with. Poking around a bit in the docker issues, it looks like this could be a storage driver specific thing (e.g., AUFS).

I tried various workarounds to no avail. Might be at a dead end.

@parente
Copy link
Member Author

parente commented Sep 11, 2015

Found slight workarounds for the docker permission weirdness for both ijulia and bash kernel. Will kick off builds of all the other subimages tonight and see if anything else breaks with this change.

* Create user jovyan with UID=1000 in the default users group in the Dockerfile
* Set group ownership of user home and conda to root to avoid 'users' group from host access when mounted
* Set stick bit on both paths so root owns subdirs too
* Change jovyan UID if NB_UID is specified and is not the default 1000

Contribution (c) Copyright IBM Corp. 2015
@parente parente force-pushed the early-user-late-uid branch 2 times, most recently from fdd348a to 0276cb5 Compare September 11, 2015 03:59
Contribution (c) Copyright IBM Corp. 2015
@parente parente force-pushed the early-user-late-uid branch from 0276cb5 to f13a494 Compare September 11, 2015 04:36
… but install kernel specs global to avoid problems when switching NB_UID

Contribution (c) Copyright IBM Corp. 2015
@parente parente force-pushed the early-user-late-uid branch from f13a494 to 69c5c35 Compare September 11, 2015 04:36
@parente
Copy link
Member Author

parente commented Sep 11, 2015

I've locally rebuilt every stack using the new minimal-notebook image defined by the Dockerfile changes in this PR. I've confirmed the changes fix #30 and make #25 possible without any serious hoop-jumping. I've run each image in the following two ways (i.e., with and without a UID change for jovyan):

make dev STACK=scipy-notebook'
make dev STACK=scipy-notebook DARGS='-e NB_UID=1005'

In every case, I've confirmed the containers to be working properly, meaning:

  • all expected kernels appear and function
  • jovyan has full access to its home directory
  • jovyan can conda install / upgrade anything in any conda environment

Remaining problem: I don't know what I don't know. For instance, does setting the default group for jovyan to users to avoid having to reset groups everywhere when the UID changes cause a problem for certain host mounted use cases?

I'm just looking to be a bit more careful up front this time to avoid having to come back to updating the base again and again with slight tweaks for different user / mount scenarios.

@rgbkrk Can I get a quick sanity check?

@rgbkrk
Copy link
Member

rgbkrk commented Sep 11, 2015

Looking really good. Only thing I'm wondering is what the reasoning is for ending as root user in the final user bound images.

@parente
Copy link
Member Author

parente commented Sep 11, 2015

Turns out that if you don't switch back to root, the CMD from the base image launches as the last USER. 👀

@costerwi
Copy link
Contributor

I agree; this has great promise. Simpler is better!

mkdir /home/$NB_USER/.jupyter && \
mkdir /home/$NB_USER/.local && \
chown -R $NB_USER:root $CONDA_DIR && \
chown -R $NB_USER:root /home/$NB_USER
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why set the group ownership to root?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly what I wanted to ask you. :) The -N on useradd puts the jovyan user in the default users group (GID=100). Anything that user touches in a host mount home or work dir will be accessible by GID=100 on the host unless the root of the mount has proper permissions. To head off any "Oops, I didn't think to protect against a container / host group overlap as well as UID overlap", I figured root was a safe bet.

But I agree it's quirky. I'm open to suggestions here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't notice the -N switch but I like the effect. Coincidentally, I am using the users group on my host CentOS system where the GID is also 100 so I'd prefer that over root but I respect that it may not satisfy all applications.
This must be a common Docker problem... Some research may be required for a sensible suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long running issue with lots of solutions but no clear best practice:
moby/moby#7198

The real solution probably lies in docker + kernel support for user namespace mapping: moby/moby#12648

My gut tells me we should treat whatever we do as a patch for the time being and so simpler is better. We can state the limitations and exactly what's going to happen in host mounts in the READMEs. My vote is for calling out user=jovyan (fixed), primary group = users (fixed), UID=configurable via env var NB_UID (default: 1000) and GID=100 (fixed).

@rgbkrk
Copy link
Member

rgbkrk commented Sep 11, 2015

Turns out that if you don't switch back to root, the CMD from the base image launches as the last USER.

I guess I assumed that was the opinionated behavior we wanted to provide people, so they don't run as root by default.

@parente
Copy link
Member Author

parente commented Sep 11, 2015

supervisord runs as root and changes to the jovyan user. If / when we switch away from that (issue #24), it'll need to change, yes.

@rgbkrk
Copy link
Member

rgbkrk commented Sep 12, 2015

Oh durr. I totally forgot about that. So sorry.

@rgbkrk
Copy link
Member

rgbkrk commented Sep 12, 2015

👍

@parente
Copy link
Member Author

parente commented Sep 13, 2015

Last set of pushes now has:

  • users group set for /home/jovyan and /opt/conda (KISS)
  • READMEs stating defaults for jovyan (uid, group, gid, and what it owns)
  • Fix for r-notebook

Going to put this into master and trigger the DH builds. datascience-notebook will fail to rebuild because IJulia has a regression at the moment and I can't find a way to point at a specific commit in Julia.

parente added a commit that referenced this pull request Sep 13, 2015
Create jovyan in Dockerfile, reset UID at startup
@parente parente merged commit e8df991 into jupyter:master Sep 13, 2015
@parente parente deleted the early-user-late-uid branch March 26, 2018 01:20
rochaporto pushed a commit to rochaporto/docker-stacks that referenced this pull request Jan 23, 2019
Enable requesting a specific OAuth scope in subclasses
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