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

Add ability to set UID and GID during container runtime #105

Closed
wants to merge 1 commit into from

Conversation

r0bj
Copy link

@r0bj r0bj commented Nov 22, 2020

Add ability to set UID and GID during container runtime via env variables.

Description

Currently user download which is used as a flood process owner is created at image build. It doesn't give much flexibility if we would like to change process UID and GID. Changing those is useful for flood because it heavily rely on filesystem permissions for accessing files which are shared between other processes (rtorrent).
This PR changes the way process owner user is created, instead of creating user at build time it's created at container runtime. This way we can start container with desired UID and GID, e.g.:

docker run -it -e FLOOD_UID=2000 -e FLOOD_GID=2000 jesec/flood

Setting those env variables is optional.

Related Issue

Fixes #104

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link

codecov bot commented Nov 22, 2020

Codecov Report

Merging #105 (4da3008) into master (9cb821b) will decrease coverage by 0.26%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #105      +/-   ##
==========================================
- Coverage   77.02%   76.76%   -0.27%     
==========================================
  Files          55       55              
  Lines        8289     8289              
  Branches      852      844       -8     
==========================================
- Hits         6385     6363      -22     
- Misses       1892     1915      +23     
+ Partials       12       11       -1     
Impacted Files Coverage Δ
server/util/feedUtil.ts 43.82% <0.00%> (-20.23%) ⬇️
server/services/feedService.ts 66.74% <0.00%> (-0.95%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cb821b...4da3008. Read the comment docs.

@jesec
Copy link
Owner

jesec commented Nov 22, 2020

I am reluctant about this. It is a wide-known security issue to run with higher privileges first and then reduce the privileges in the same TTY context (ncopa/su-exec#23).

It might be more appropriate to use uid mapping (namespace) feature of Docker.

Alternatively, given Flood docker image is simply composed with sudo npm install --global flood, you may:

  • Simply use --user argument of docker run. Flood is globally installed inside the Docker image and it can be executed by any user inside the container as long as there is a readable/writeable rundir. Flood doesn't actually need a home directory. The image creates a user simply because the default runtime directory is $HOME/.local/share/flood. In practice, if you want the settings to be persistent, rundir is always a mapped folder.
  • Compose a Docker image with appropriate UID/GID. It is extremely easy to do so. docker build --tag flood https://github.com/jesec/flood/raw/master/Dockerfile.release If you think this solution suits you, you can create a separate PR that makes version, uid and gid configurable in the Dockerfile if necessary.
  • Or, just install Node.js and run sudo npm install --global flood in your own configured Docker image. It is that simple. Official Flood docker image is not that special, really.

@r0bj
Copy link
Author

r0bj commented Nov 22, 2020

Yes, you are right, it's easy enough to build custom flood image. Thanks!
Let's close PR then.

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.

Add docker runtime configuration for flood process UID and GID
2 participants