Add user to container to support running as non-root #428
Conversation
There was a problem hiding this comment.
Hey! Thank you for all of the PRs you've opened, and apologies for the radio silence on our end (there were a few overlapping sabbaticals and vacations).
Supporting running this image as non-root seems like a good idea. I'm a little unclear on the comment you left and I'd like to understand that a bit more. That being said, no real concerns here. Help me understand that comment and then this is good to merge.
| @@ -13,6 +13,11 @@ RUN apk update \ | |||
| && rm -f /var/cache/apk/* \ | |||
| && rm -rf /go/src /go/pkg | |||
|
|
|||
| # Hardcode gid and uid so that it never changes. This changing will break | |||
| # users running this as nonroot in production as you run it with the uid directly, | |||
| # not the user name. | |||
There was a problem hiding this comment.
This comment is a little unclear (I'm admittedly less versed in user management) — When you say "This change will break users running this as nonroot", do you mean running the docker image as nonroot? I don't totally understand what the breaking change here is.
| # users running this as nonroot in production as you run it with the uid directly, | ||
| # not the user name. | ||
| RUN addgroup -g 1000 hound && adduser -u 1000 -G hound -D hound | ||
|
|
There was a problem hiding this comment.
Why not add USER 1000:1000 right here? Then you can skip the advice in the readme. Is there an actual reason to run hound as root, like ever?
Running docker containers as root in production is not good security practice for a lot of reasons. It is difficult to use the images provide without running as root there is no user to run it as. This PR adds a user so that one can run hound as non-root.
I did not change the default user of the container so this should have no effect on those already utilizing the container and expecting it to run as root.
This will allow one to easily run the container as non-root with Docker, Kuberntes, etc (See README change).
What kind of change does this PR introduce? (check at least one)
The PR fulfills these requirements:
If adding a new feature, the PR's description includes:
Other information: