Skip to content

Add user to container to support running as non-root#428

Open
kinghrothgar wants to merge 1 commit into
hound-search:mainfrom
kinghrothgar:non-root-user
Open

Add user to container to support running as non-root#428
kinghrothgar wants to merge 1 commit into
hound-search:mainfrom
kinghrothgar:non-root-user

Conversation

@kinghrothgar
Copy link
Copy Markdown
Contributor

@kinghrothgar kinghrothgar commented Jul 1, 2022

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)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

The PR fulfills these requirements:

  • All tests are passing?
  • New/updated tests are included?
  • If any static assets have been updated, has ui/bindata.go been regenerated?
  • Are there doc blocks for functions that I updated/created?

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

Copy link
Copy Markdown
Contributor

@salemhilal salemhilal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread Dockerfile

# 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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread Dockerfile
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

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.

3 participants