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

Pin USER in Dockerfile complying with Docker best practices #46166

Closed
4 tasks
dliappis opened this issue Aug 30, 2019 · 1 comment · Fixed by #50277
Closed
4 tasks

Pin USER in Dockerfile complying with Docker best practices #46166

dliappis opened this issue Aug 30, 2019 · 1 comment · Fixed by #50277
Assignees
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement Team:Delivery Meta label for Delivery team

Comments

@dliappis
Copy link
Contributor

OpenShift (and it's open source variant OKD) run containers with an arbitrary user id which we've already added support for in elasticsearch-docker#125 (see also the old issue elasticsearch-docker#114).

However, USER needs to be additionally specified to a numeric value, according to the OpenShift guidelines.

The current Elasticsearch Dockerfile doesn't specifically set the USER, inherits 0 from the parent image (centos:7) and later inside the entrypoint script switches to user 1000 to start Elasticsearch.

Unfortunately this doesn't work in OpenShift. Unless the anyuid SCC property is set in OpenShift/OKD, the container won't be allowed to start.
In addition to that, Elastic Cloud on k8s sets the property runAsNonRoot: true which just won't work, without specifying a non privileged USER, on OpenShift/OKD.

@josgonza-rh raised a PR to set USER 1000 in the Dockerfile but we decided to close it and open this issue instead because explicitly setting USER 1000 requires a few changes tracked in this issue:

  • Remove all user switching code in the entrypoint.
  • Remove the TAKE_FILE_OWNERSHIP feature (code here). This feature was historically requested to assist with bind mount permission issues, but not sure how much usage it still has.
  • Adjust docs as required, e.g. removing TAKE_FILE_OWNERSHIP.
  • Add necessary tests to ensure the entrypoint changes don't break existing functionality.
@dliappis dliappis added >enhancement :Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts labels Aug 30, 2019
@dliappis dliappis self-assigned this Aug 30, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@dliappis dliappis assigned pugnascotia and unassigned dliappis Nov 12, 2019
pugnascotia added a commit to pugnascotia/elasticsearch that referenced this issue Jan 7, 2020
Closes elastic#49926 and elastic#46166.

   * Add a mini-init process (copied from Cloud)
   * Don't use root at all when running the container
   * Add an explicity TERM handler
   * Ensure no files in the image have the setuid flag

Also improve dependency tracking in the build.
pugnascotia added a commit that referenced this issue Jan 23, 2020
Closes #49926 and #46166. Rework the Docker image so that it comes with a tiny
init system, to ensure ML processes are correctly cleaned up, and to run ES
as a regular user instead of root.

Also:

   * Ensure no files in the image have the setuid/setgid flag
   * Also improve dependency tracking in the build
   * Remove TAKE_FILE_OWNERSHIP option and its documentation
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Packaging RPM and deb packaging, tar and zip archives, shell and batch scripts >enhancement Team:Delivery Meta label for Delivery team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants