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

Non root #186

Merged
merged 3 commits into from Dec 20, 2019
Merged

Non root #186

merged 3 commits into from Dec 20, 2019

Conversation

cimnine
Copy link
Collaborator

@cimnine cimnine commented Nov 17, 2019

This PR adds a netbox user, so that Netbox could be run as non-root user.

But I'm not satisfied with the solution in the Dockerfile on line 79:

RUN mkdir static && chmod a+w static media

This is more a work-around than a solution. It is needed, because the static files are generated in the entrypoint (because they have to be copied to a Volume which is shared with nginx).

I would like to see if there are other solutions to this problem before going ahead.

Closes #172

@cimnine cimnine added discussion This issue requires further input from the community. enhancement The issue describes an enhancement that we would like to implement in the future. help wanted We seek out help for implementing this issue. labels Nov 17, 2019
@tobiasge
Copy link
Member

I have some comments here:

  • We have this image running in Openshift, so it is already possible to run it as an non-root user (Mounted volumes have to be chmod a+w)
  • When running the image in Openshift it is started with a random uid und gid = 0. So when we change the the user it should have the primary gid 0 (Note: This is the root group, but it doesn't have any special privileges)
  • On the static and media directories use chmod g+w and set a suitable umask (probably 002)in the docker-entrypoint.sh script.

Another completely different approach would be to collect the static files at image build, but this would mean we have to merge the nginx and netbox images into one using some startup controller like supervisord.

@cimnine
Copy link
Collaborator Author

cimnine commented Nov 18, 2019

Another completely different approach would be to collect the static files at image build, but this would mean we have to merge the nginx and netbox images into one using some startup controller like supervisord.

I was thinking of this as well. We could even have the the entrypoint script copy the files to a specific mountpoint if it exists, e.g. like this:

if [ -d /mnt/nginx ]; then
  cp -a /opt/netbox/netbox/static /mnt/nginx/
fi

But in order to build the static files, a valid configuration.py is required (at least the SECRET_KEY has to be defined). And I don't know yet about the implications, when this is defined as A during build-time, but re-defined as B during run-time.

@cimnine
Copy link
Collaborator Author

cimnine commented Nov 18, 2019

When running the image in Openshift it is started with a random uid und gid = 0. So when we change the the user it should have the primary gid 0 (Note: This is the root group, but it doesn't have any special privileges)

I forgot that one can just set an arbitrary UID without having to have a corresponding /etc/passwd entry. This fells a lot better already.

@cimnine cimnine changed the title WIP: Non root Non root Nov 25, 2019
@tobiasge
Copy link
Member

I think we should add the umask 002 to the entrypoint script. This would make files that are created group writable. This would prevent problems when the effective userid is changed when running this in Openshift or Kubernetes.

Also the changes from the "label" pull request are in this PR too. To keep it clear, those should be removed from this PR.

@cimnine
Copy link
Collaborator Author

cimnine commented Nov 26, 2019

Also the changes from the "label" pull request are in this PR too. To keep it clear, those should be removed from this PR.

I have rebased onto master.

@cimnine
Copy link
Collaborator Author

cimnine commented Nov 26, 2019

I think we should add the umask 002 to the entrypoint script. This would make files that are created group writable.

Would you put it to the beginning of the entrypoint.sh, or right before the exec?

@tobiasge
Copy link
Member

I think we should add the umask 002 to the entrypoint script. This would make files that are created group writable.

Would you put it to the beginning of the entrypoint.sh, or right before the exec?

It should be placed before any files that are created, so at the moment before the "collectstatic".
But if we later add features that also created files we have to move the umask. So I think we place this at the beginning of the script after set -e

@cimnine cimnine added this to the 0.16.0 milestone Nov 26, 2019
@cimnine cimnine changed the base branch from master to develop December 10, 2019 20:48
@cimnine
Copy link
Collaborator Author

cimnine commented Dec 20, 2019

@tobiasge do you see a reason not to merge this? Otherwise I would appreciate your Approval and then I would proceed to make the 0.16.0 milestone reality.

@cimnine cimnine merged commit 0a9991d into develop Dec 20, 2019
@cimnine cimnine deleted the non-root branch December 20, 2019 13:21
@cimnine cimnine mentioned this pull request Dec 21, 2019
@jkldgoefgkljefogeg
Copy link

The permission setup does not work out of box
#298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This issue requires further input from the community. enhancement The issue describes an enhancement that we would like to implement in the future. help wanted We seek out help for implementing this issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a non-root image?
3 participants