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

Fix userns option for rootless unix socket use case in install.md #209

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

jasyip
Copy link
Contributor

@jasyip jasyip commented Mar 16, 2024

Given that the container runs as nobody user (with uid 65534 by default) and host uid, which is most likely not 65534, should map to container's uid 65534

Given that the container runs as nobody user (with uid 65534 by default)
and host uid, which is most likely not 65534, should map to container's
uid 65534

Signed-off-by: Jason Yip <[email protected]>
@navidys
Copy link
Collaborator

navidys commented Mar 16, 2024

Hi @jasyip

I think its better to remove the USER nobody from Containerfile.
If you intrested you can update your PR and update the Containerfile instead of install.md.

Regads

@jasyip
Copy link
Contributor Author

jasyip commented Mar 16, 2024

I think its better to remove the USER nobody from Containerfile. If you intrested you can update your PR and update the Containerfile instead of install.md.

Most prometheus Dockerfiles follow the same template of using their busybox base image and using the nobody user. If the container were to run as root instead, it would be more convenient and the original command would work. Whereas running as nobody follows the principle of least privilege and it just only needs to be able to read/write to the socket that the host user owns. Since my new command works with the nobody user, I don't see any reason to opt for root privileges as they aren't necessary in this case and carry the risk of elevated privileges.

@navidys navidys merged commit d7f22c8 into containers:main Mar 16, 2024
33 checks passed
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