Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

proposal : use a "non-root" node docker base image #225

Open
looztra opened this issue Jan 7, 2018 · 15 comments
Open

proposal : use a "non-root" node docker base image #225

looztra opened this issue Jan 7, 2018 · 15 comments

Comments

@looztra
Copy link
Contributor

looztra commented Jan 7, 2018

The base image for the docker image uses library/node which is suboptimal because every directory created by this image (like the PDF one for example) will be owned by root on the docker host.

  • having to sudo rm -rf PDF to get it cleaned under linux is cumbersome
  • under windows cleaning requires admin rights

I propose that we either use mkenney/npm as the base image or that we get inspired from it to build the fwk docker image. This image automagically switches to the current uid/gid inside the running container, hence every directory/file created by node inside the container stick to the user that created the directory on the docker host.

First option : use it as the base image. Problem: the image uses /src inside the container to detect the target uid/gid, so we will need to adjust run.sh to also mount a host dir on /src in the container

Second option : mimic the behaviour of the mkenney/npm image and copy and adjust the run-as-user script to, for instance, use one of the already mounted dirs

For the moment, I'd favour the first option because I think that adjusting run.sh will require less efforts than having to sync our fork of the run-as-user with the original one.

What's your point of view regarding this?

@hgwood
Copy link
Contributor

hgwood commented Jan 8, 2018

Please keep your language professional. I have edited your message.

@hgwood
Copy link
Contributor

hgwood commented Jan 8, 2018

I see your problem, however I'm not a fan of the proposed solution as it makes the project deviates from the use of a standard well-known image, which makes on-boarding more difficult. I think the underlying issue is that the container is writing files to the host file system. Maybe we should not have it to that. Maybe the PDFs should be produced on stdout? Or maybe the container should server the PDFs over HTTP and the outside world would need to download them. What do you think?

@looztra
Copy link
Contributor Author

looztra commented Jan 8, 2018

Please keep your language professional. I have edited your message.

What? Not sure to appreciate your judgement if I'm professional or not in my comment to be honnest

@looztra
Copy link
Contributor Author

looztra commented Jan 8, 2018

  • I don't see why not using a "library" base image would make on-boarding more difficult
  • In my opinion outputing the pdf on stdout or making it available through http would make the life of users more complicated

My point of view: ease the life of users as much as possible even if it means that the life of maintainers may be more difficult.

@hgwood
Copy link
Contributor

hgwood commented Jan 8, 2018

  • What I mean is: if I'm a new contributor to this project and I see that the base image is library/node, then I'm not surprised, while if I see mkenney/npm, I wonder why that is.
  • The handling of stdout or HTTP would be the problem of run.sh, not the users, wouldn't it?

@looztra
Copy link
Contributor Author

looztra commented Jan 8, 2018

I prefer to add a comment explaining why we use mkenney/npm over library/node and keep the same logic in run.sh (we will just need to change one of the volume mounts I think)

rather than

keeping library/node and change the behaviour of grunt pdf and grunt generateCahierExercice and the logic in run.sh

Maybe we could ask what other contributors think about it?

@hgwood
Copy link
Contributor

hgwood commented Jan 9, 2018

Sure, ping anyone you like.

@looztra
Copy link
Contributor Author

looztra commented Jan 10, 2018

dear @jlandure @zigarn @gmembre-zenika @FBerthelot could you tell us what you think about the solutions proposed by @hgwood and myself?

@gmembre-zenika
Copy link
Contributor

I prefer the way @looztra want to change the behaviour.
Let me explain :

  1. It is a good practice today to have a container that don't run program as root inside. maybe later, the libray/node will change to adopt this security mesure but in the mean time, we can go further by using another image. A simple comment to explain should do it.
  2. Modifying the run.sh in order to handle pdf stream to stdout will be difficult task, it'll involved being able to split the file received at the correct position as there are multiple files and pdf produced : slides in HTML / PDF and student book. => Good luck with it without installing any other tool on the host.

@hgwood
Copy link
Contributor

hgwood commented Jan 11, 2018

@gmembre-zenika if stdout would be chosen then there would be one command by generated file (run.sh pdf-slides, run.sh pdf-labs, ...).

Are there any official images that adopt the non-root practice?

@hgwood
Copy link
Contributor

hgwood commented Jan 11, 2018

@looztra I've changed my mind and I encourage you to go forward with your change, for two reasons:

  • the burden of complexity is better placed: while my solution puts the complexity on run.sh, which is duplicated in every depending project and hard to keep properly updated and versioned, yours puts it in the Dockerfile of the framework, which is centralized and properly versioned
  • lower cost: we do not have much time allocated to maintaining this framework, and your solution is a lot faster to implement

My question from my last comment still stands though, for my own culture. :)

@gmembre-zenika
Copy link
Contributor

Another way to satisfy the non root : why not use the -u <id> (doc) option while running the official image library/node ?
Taken care must taken while choosing the uid, is must match the current user but it simple to get : id -u on linux.

@looztra : any suggestions ?

@looztra
Copy link
Contributor Author

looztra commented Jan 12, 2018

@gmembre-zenika : the -u <id> doesn't seem to work with the nodejs image. I never tried to find out why to be honest.

@hgwood: the answer is related to my reply to @gmembre-zenika : you can use the mvn docker image with the -u option (that specifies the user used by the docker image at runtime) with no problem, but that does not work with node. The behaviour is related to the pid 1 process in the docker image not to the way the image is built it seems.

@hgwood
Copy link
Contributor

hgwood commented Jan 12, 2018

@looztra Have you seen this?

@looztra
Copy link
Contributor Author

looztra commented Jan 12, 2018

Indeed, that's better than running as root.
The mkenney/npm goes further and

... executes the specified command as a user who's uid and gid matches those properties on that directory

It means that it is not limited to run with a uid/groupid of 1000/1000

I will make some experiments and play with both solutions...and report my findings :)

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

No branches or pull requests

3 participants