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

Run the containers without root #386

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

jmaddencom
Copy link

This should improve potential exposure quite a bit by only requiring root in a busybox init container to set the directories up.

Since I've only changed the production configs this won't work alongside a dev install if you're switching between the two. Let me know if you consider this worthwhile and I'll get that working too.

…d default 2000:2000 for the photonix container, add init container just for setting up the filesystem. Don't install cron.
…thout root) put retrain_face_similarity_index into a shell loop with a sleep.
@damianmoore
Copy link
Collaborator

Thanks for this contribution @jmaddencom. I'd been meaning to look and this for a while but hadn't got round to it. Couple of points...

  • How did you decide on the user IDs to use? I assume 70 and 999 are the ones that the third-party containers are already using.
  • Yes please carry out the changes to dev environment if you can - it would be good to have them as close as possible to each other.
  • Could you explain the need for the depends_on: init you added please in the compose file?
  • I'm intending to use cron more for parts of the background processing soon (to save memory etc.). Would you be able to add that back if you think it would still work with your approach?

Thanks again, and looking forward to merging this in.

@jmaddencom
Copy link
Author

jmaddencom commented Mar 13, 2022

Sure thing @damianmoore, will do.

  • User ids were somewhat arbitrary, I wanted to provide some defaults that seemed logical and debian-based distributions start at 1000 for non-system users. (Ubuntu anyway, I think that's a debian thing.)
  • depends_on ensures init happens before the other containers come up.
  • I tried to get cron to work as non-root but failed. (There are some setuid stackoverflows/etc out there but that felt like it wasn't really fixing the issue.) It's kind of counter to the principal of containerization that the container runs a single process...I'll take another stab at it though.

Edit: Right, 70 and 999 are the ones shipped by the containers. At least in the pgsql case I found that difficult to change and probably not worth it as long as it isn't 0.

@jmaddencom
Copy link
Author

This updates docs and dev files (but doesn't remove cron from dev, which means cron won't run properly there.) I'll take another swing at non-root cron asap but I've run out of weekend. :/

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.

2 participants