Rationale for keeping su-exec/gosu and docker-entrypoint.sh

[nrvnrvn]

Hi!

I'm in the middle of the process of reviewing the images and container security overall used by us and noticed that redis needs chown,setuid and setguid capabilities.
docker entrypoint script calls chown on the files in the current directory. This is at least three assumptions:

1. the user "forgot" to do it in advance.
2. we assume the . dir is actually /data and
3. the user did not bother to run the container as a non-root and thus we are doing it for him.

There is a cleaner, secure and more appropriate way to do it. Here is a simplified scenario for single host docker execution:

$ MYLOVELY_ID=982739
$ sudo mkdir /path/to/persistent/redis/data
$ sudo chown ${MYLOVELY_ID}:${MYLOVELY_ID} $_
$ docker run \
    --rm \
    --read-only \
    --name redis-server \
    --security-opt no-new-privileges \
    --cap-drop all \
    --sysctl net.core.somaxconn=512 \
    --user ${MYLOVELY_ID}:${MYLOVELY_ID} \
    -v /path/to/persistent/redis/data:/data \
    --entrypoint redis-server \
    redis:5-alpine

It basically does nothing with gosu/su-exec and you can avoid overriding entrypoint by amending the Dockerfile as follows:

diff --git a/5.0/alpine/Dockerfile b/5.0/alpine/Dockerfile
index 7983b2f..65f3fdf 100644
--- a/5.0/alpine/Dockerfile
+++ b/5.0/alpine/Dockerfile
@@ -1,11 +1,6 @@
 FROM alpine:3.8
 
-# add our user and group first to make sure their IDs get assigned consistently, regardless of whatever dependencies get added
-RUN addgroup -S redis && adduser -S -G redis redis
-
 RUN apk add --no-cache \
-# grab su-exec for easy step-down from root
-		'su-exec>=0.2' \
 # add tzdata for https://github.com/docker-library/redis/issues/138
 		tzdata
 
@@ -55,14 +50,8 @@ RUN set -ex; \
 	apk add --virtual .redis-rundeps $runDeps; \
 	apk del .build-deps; \
 	\
-	redis-server --version
-
-RUN mkdir /data && chown redis:redis /data
-VOLUME /data
-WORKDIR /data
-
-COPY docker-entrypoint.sh /usr/local/bin/
-ENTRYPOINT ["docker-entrypoint.sh"]
+	redis-server --version; \
+	mkdir /data
 
 EXPOSE 6379
-CMD ["redis-server"]
+ENTRYPOINT  ["redis-server"]

This change also covers #140 since this artificial volume causes problems with mounting real volumes at least in the naïve docker run test.

Running redis like this looks pretty legit and caused us no errors ever since we introduced it internally. This is true for sentinel and cluster as well.

Have we missed anything about redis/su-exec and entrypoint.sh?
Are there any pitfalls of our scenario apart from possible breakage of backwards compatibility with older docker versions?

[tianon]

Your example command is perfectly reasonable, and given the constraints you provide (already appropriate permissions on the data volume, etc), should work fine long-term. In the instance you provide, the entrypoint will be essentially a no-op anyhow, so there's really not much harm in either keeping it or skipping it as you do (it checks both that the command provided is redis-server and that the user we're running the image as is root before doing anything at all).

Closing since this is more of a usability question (not really something we're going to change about the image). 👍