aerospike / aerospike-server.docker

Dockerfiles for Aerospike Server
Other
141 stars 68 forks source link

Dockerfile best practices #7

Closed jcross-linkup closed 6 years ago

jcross-linkup commented 8 years ago

I've been ramping up docker use within my company, and we happen to be starting to use Areospike as well. I'd thought I'd put my 2 cents in on how the Dockerfile could be improved and better align with Docker best practices. I could create pull requests for all these, but I wasn't sure if that would be over kill for these little items. If there are good reasons to not do these things, I'd like to know too since I'm still learning how to write proper Dockerfiles.

  1. The config file is placed into the image using ADD. COPY is preferred for static files that aren't downloaded: https://docs.docker.com/articles/dockerfile_best-practices/#add-or-copy
  2. CMD is used instead of ENTRYPOINT plus CMD. Basically it allows parameters to be passed to the main command via docker run without having to know the command being run. See: https://docs.docker.com/articles/dockerfile_best-practices/#entrypoint
  3. The UID the container will run as is root. The aerospike.conf is also set set to use root. It's undesirable to have a container run as root, and with some docker orchestration systems such as OpenShift 3 it may cause problems. It would be preferable to use an aerospike user/group with a high number UID/GID. There was actually a really informative and fun video put out by Red Hat at the last summit that explains best practices for running docker containers from a security perspective: https://youtu.be/a9lE9Urr6AQ
  4. If a non-root UID is used to run aerospike then a static UID/GID should be chosen so that the data in the defined VOLUME can be accessed by containers created from later versions of the image than the original.
alvinr commented 8 years ago

Feedback

1 agree

2 the entrypoint method would allow passing of ages from compose etc. so agree

3-#4 adding a specific Aerospike Group & User into the Docker file make sense.

Happy to take a pull request!

tianon commented 8 years ago

In implementing the ENTRYPOINT bit, be sure to keep https://github.com/docker-library/official-images/blob/master/README.md#consistency in mind. :wink:

jcross-linkup commented 8 years ago

Thanks for the tip! I'll work on a pull request.

chrrrles commented 7 years ago

Hello,

I see that most of the changes suggested by jcross have already been implemented -- aside from the non-root UID. Is there any chance of seeing that changed in this upstream Dockerfile, or are there concerns regarding backwards compatibility?

Thanks, Charles

jcross-linkup commented 7 years ago

Most of the official docker images seem to not use random UIDs instead root. Changing it now could cause issues for people due to the ownership of the files being different in an existing volume. There is now a way to avoid the root UID problem with the fairly recent addition of docker user namespaces: https://success.docker.com/KBase/Introduction_to_User_Namespaces_in_Docker_Engine

Before user namespaces, the root UID issue was a bigger annoyance for those of us that didn't want docker processes running as root.

chrrrles commented 7 years ago

Thanks @jcross-linkup - Configuring the container to run as unprivileged is easy enough... Bringing this up only because Openshift applies security constraints to applications without a service account; preventing them from accessing root.