search

bgruening / docker-ipython-notebook

:whale: :microscope: :books: IPython running in a docker container. This image can be used to integrate IPython into Galaxy
MIT License
17 stars 9 forks source link

Change the container to a non-root environment #17

Closed bgruening closed 10 years ago

bgruening commented 10 years ago
hexylena commented 10 years ago

@bgruening flying to NIH today, I'll test this evening when I get to my hotel.

bgruening commented 10 years ago

Should we merge this before or after @jmchilton is giving his awesome presentation?

hexylena commented 10 years ago

I can test+merge in ~6 hours, will be the first thing I do when I get home. @jmchilton is that an acceptable time frame for you?

On October 22, 2014 1:31:45 PM GMT+00:00, "Björn Grüning" notifications@github.com wrote:

Should we merge this before or after @jmchilton is giving his awesome presentation?

Reply to this email directly or view it on GitHub: https://github.com/bgruening/docker-ipython-notebook/pull/17#issuecomment-60084539

Sent from my Android device with K-9 Mail. Please excuse my brevity.

jmchilton commented 10 years ago

I am already working with a heavily modified container - so I need to merge back at some point either way - might as well be to this instead of original :).

hexylena commented 10 years ago

Finally got testing up...so so so sorry Björn! I'll be more available in the future.

Okay, so things are looking mostly good. Saw 

Step 4 : apt-get autoremove -y && apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
# Skipping unknown instruction APT-GET
...
Step 11 : RUN python -c 'from IPython.external import mathjax; mathjax.install_mathjax("2.4.0")'
 ---> Running in 0291d8294e28
/usr/local/lib/python2.7/dist-packages/IPython/utils/path.py:317: UserWarning: IPython parent '/' is not a writable location, using a temp directory.
  " using a temp directory."%parent)
Downloading mathjax source from https://github.com/mathjax/MathJax/archive/2.4.0.tar.gz
Extracting to /tmp/tmpfQehx1/nbextensions/mathjax

which was interesting, but that's probably an out-of-date docker issue/no idea about mathjax's issue.

I'm seeing a permissions issue :(. As the Galaxy user on my test server, 

galaxy@cpt:/home/galaxy/galaxy-test/config/plugins/visualizations/docker-ipython-notebook$ id
uid=10029(galaxy) gid=10003(unprivsys) groups=10003(unprivsys),120(docker)

What's in the container?

galaxy@cpt:/home/galaxy/galaxy-test/config/plugins/visualizations/docker-ipython-notebook$ docker run  --sig-proxy=true -p 14808:6789 -v "/home/galaxy/galaxy-test/database/tmp/tmpGHv8aX:/import/" ipynb-test ls -al /
total 8992
<snip>
drwxr-xr-x   1 root    root         14 Oct 23 00:40 home
drwx------   1   10029   10003      76 Oct 23 00:46 import
drwxr-xr-x   1 root    root          0 Sep 21 18:17 mnt
<snip>
-rwxr-xr-x   1 ipyuser ipyuser     873 Oct 23 00:04 monitor_traffic.sh
drwxr-xr-x   1 root    root          0 Oct 20 17:44 opt
<snip>

Noting that the import directory has the same uid/gid that I do running the container...which != ipyuser, which I am inside the container

galaxy@cpt:/home/galaxy/galaxy-test/config/plugins/visualizations/docker-ipython-notebook$ docker run  --sig-proxy=true -p 14808:6789 -v "/home/galaxy/galaxy-test/database/tmp/tmpGHv8aX:/import/" ipynb-test id
uid=1000(ipyuser) gid=500(ipyuser) groups=500(ipyuser)

How should we solve this? We obviously need ipyuser to have the same uid/gid as the user running the container. Should we add it to the conf.yaml spec? It seems like that file is growing a lot, which is a little bit worrisome. Perhaps this is an already solved issue and I just can't think of the solution right now. I'll comment more if things come to me.

jmchilton commented 10 years ago

So I guess perhaps we need to work toward installing everything globally and allowing any user to write to /import and then start the container with -u=<galaxy_uid>. @kellrott recently made me switch all of Galaxy job running to use this by default - https://bitbucket.org/galaxy/galaxy-central/pull-request/525/adding-set-user-flag-to-docker-calls/diff? Does that make sense or would there be issues with that?

I definitely need to research this more - I don't really understand permissions in and out of the container.

hexylena commented 10 years ago

Will test that out tomorrow, it's late today. Looks likely though, I didn't debug at all. Perhaps Björn will chime in before then though, knowing him and his love of docker.

(If I say Docker three times, will it summon the Björn? Let us see...Docker docker docker! :P)

On October 23, 2014 1:13:07 AM GMT+00:00, John Chilton notifications@github.com wrote:

So I guess perhaps we need to work toward installing everything globally and allowing any user to write to /import and then start the container with -u=<galaxy_uid>. @kellrott recently made me switch all of Galaxy job running to use this by default - https://bitbucket.org/galaxy/galaxy-central/pull-request/525/adding-set-user-flag-to-docker-calls/diff? Does that make sense or would there be issues with that?

I definitely need to research this more - I don't really understand permissions in and out of the container.

Reply to this email directly or view it on GitHub: https://github.com/bgruening/docker-ipython-notebook/pull/17#issuecomment-60179998

Sent from my Android device with K-9 Mail. Please excuse my brevity.

bgruening commented 10 years ago

Uh something woke me up! Docker, Docker, Docker? Here I'm! @erasche this seems to be strange. Which version are you using? is mathjax down again? This happens to me as well a few times. Maybe it's better to ship it with the Dockerfile? -u is probably the best idea. But I don't think we need to add this to .yaml file. Uh now I need to get up properly ;)

hexylena commented 10 years ago

Maybe so. I'll dig into the error in more detail today. I hadn't known about the -u option, that definitely looks like the way to go. It means we'll probably need to update any chowns to chown $USER:

hexylena commented 10 years ago

Okay, making more progress. Ran with -u $USER_ID today, since running with -u galaxy had issues.

root@cpt:/home/galaxy/galaxy-test/config/plugins/visualizations/ipython# docker run  -u 10029 --sig-proxy=true -p 11247:6789 -v "/home/galaxy/galaxy-test/database/tmp/tmpEbIPtm:/import/" ipynb-test
/usr/local/lib/python2.7/dist-packages/IPython/utils/path.py:317: UserWarning: IPython parent '/' is not a writable location, using a temp directory.
  " using a temp directory."%parent)
Signing notebook: /import/ipython_galaxy_notebook.ipynb
/usr/local/lib/python2.7/dist-packages/IPython/utils/path.py:317: UserWarning: IPython parent '/' is not a writable location, using a temp directory.
  " using a temp directory."%parent)
2014-10-23 15:22:22.835 [NotebookApp] Created profile dir: u'/tmp/tmpNdinBg/profile_default'
2014-10-23 15:22:22.842 [NotebookApp] Using MathJax from CDN: https://cdn.mathjax.org/mathjax/latest/MathJax.js
2014-10-23 15:22:22.890 [NotebookApp] Serving notebooks from local directory: /import
2014-10-23 15:22:22.890 [NotebookApp] 0 active kernels 
2014-10-23 15:22:22.890 [NotebookApp] The IPython Notebook is running at: http://localhost:8888/
2014-10-23 15:22:22.890 [NotebookApp] Use Control-C to stop this server and shut down all kernels (twice to skip confirmation).
2014-10-23 15:22:22.891 [NotebookApp] WARNING | No web browser found: could not locate runnable browser..

We are no longer running as ipyuser, so it isn't picking up their data from their home directory. (As you all know, when user isn't found, home directory can't be located, and is "set" as /).

Due to @bgruening 's change of USER ipyuser, every command in the Dockerfile after that was executed as the ipyuser, hence this failure:

Step 11 : RUN python -c 'from IPython.external import mathjax; mathjax.install_mathjax("2.4.0")'
 ---> Running in 94c50916a7be
/usr/local/lib/python2.7/dist-packages/IPython/utils/path.py:317: UserWarning: IPython parent '/' is not a writable location, using a temp directory.
  " using a temp directory."%parent)
Downloading mathjax source from https://github.com/mathjax/MathJax/archive/2.4.0.tar.gz
Extracting to /tmp/tmpE32xb1/nbextensions/mathjax

Here we can see that for whatever reason, IPython believes it should be installing to /, which it, of course, can't do with ipyuser's permissions. I imagine we'd have to export HOME=/home/ipyuser/ or similar to get the IPython command to function properly.

With all of that said...I have some tentative patches, which I'll push to another branch for review. They essentially amount to swapping out a dedicated IPython user for nobody. My thought process is this:

Thus I came to the conclusion that the ipyuser, while providing a lot of benefit, if we're just going to switch over to -u ###, then we'll need to deprecate that.

Unfortunately, this means that the container MUST be started as -u. There's no way for the container to easily run otherwise due to data access/permissions -- no user that we add (other than root) will be able to read from /import, as that will always have 700 permissions (since it's mounted from galaxy).

We could chmod g+rwx permissions on the import directory, but that doesn't seem like a good option. If we changed permissions to give group access to the ipyuser in the container, we'd also be exposing it to that user/group on the galaxy host.

While I dislike the whole "must start container with X, Y, and Z", we've already required the end user to have a conf.yaml, at this point adding the requiremnet of starting with -u isn't that onerous

jmchilton commented 10 years ago

Thanks for the great report - I am fine with requiring that it is started with -u - galaxy will take care of that detail right - deployers will have to see it. Or were you thinking of uses outside of Galaxy?

hexylena commented 10 years ago

Well, I like to try and bear in mind that eventually someone may want to use this outside of Galaxy, or in another environment, so we've been trying to keep down on some of the less portable things, however at this point we're pretty far gone. We'll have to come back and deal with our technical debt later, perhaps switching out conf.yaml for environment variables, or whatever other changes will get us in line with the proposed base ipython docker image (maintained by them) that was discussed.