docker-entrypoint.sh fails if config file is mounted as read-only

[st3v]

Background

When deploying RabbitMQ to Kubernetes it is pretty common to mount a configMap that contains a rabbitmq.conf file inside the container. As of Kubernetes 1.9.4 configMaps are always mounted as read-only (see changelog for K8s 1.9.4). That change was made to address a CVE. Now the problem is that docker-entrypoint.sh tries to write to the rabbitmq config whenever certain environment variables are set in the container (e.g. RABBITMQ_DEFAULT_USER). Obviously, this fails when the file gets mounted as read-only.

Happy Case 🙂

Kubernetes pods start and run perfectly fine as long as I don't set one of the magic env vars that causes docker-entrypoint.sh to write to the mounted /etc/rabbitmq/rabbitmq.conf.

$ kubectl apply -f https://bit.ly/2lTBBki
configmap/rabbitmq-config created
statefulset.apps/rabbitmq created

$ kubectl get pod
NAME         READY   STATUS    RESTARTS   AGE
rabbitmq-0   1/1     Running   0          27s

Sad Case 😢

Pods don't start as soon as I set RABBITMQ_DEFAULT_USER. The pod logs show that sed couldn't write to the mounted directory.

$ kubectl apply -f https://bit.ly/2koRD5s
configmap/rabbitmq-config created
statefulset.apps/rabbitmq created

$ kubectl get pod
NAME         READY   STATUS   RESTARTS   AGE
rabbitmq-0   0/1     Error    2          20s

$ kubectl logs rabbitmq-0
sed: couldn't open temporary file /etc/rabbitmq/seda5g8kq: Read-only file system

Difference between both cases

$ diff <(curl -sSL https://bit.ly/2lTBBki 2>&1) <(curl -sSL https://bit.ly/2koRD5s 2>&1)
51a52,54
>         env:
>           - name: RABBITMQ_DEFAULT_USER
>             value: bunny

Workaround

• Add an initContainer to the pod that copies the config file to a write-able location
• Have the RabbitMQ container pick up the config file from that location

Example: https://gist.github.com/st3v/ffefe94c84c196d90a420bb561f2878f#file-repro_workaround-yaml

Desired Behavior

I'd argue that the RabbitMQ docker image should add out-of-the-box support for read-only config files. The workaround using an initContainer is kinda hacky and shouldn't be necessary for something that is arguably a pretty common use case given the Kubernetes behavior described above.

Resources

YAML files used above: https://gist.github.com/st3v/ffefe94c84c196d90a420bb561f2878f

[yosifkit]

Duplicate of #292. See specifically #292 (comment)

[st3v]

Thanks for the reference! I guess the suggestion in #292 (comment) would work indeed. However, may I ask you to consider the following. The fact that this issue and the previous one (#292) both exist in the first place should make clear that this is a pretty common use-case and it isn't quite obvious to people why they are seeing these failures. Asking everyone that runs into this to read and understand the entrypoint script or look at the issues in this repo doesn't seem very user-friendly and a bit of a waste of our collective time.

[wglambert]

The limitations of Kubernetes' configmaps and how they convey those is really a user-environment issue and not something we can make compatible with the typical Docker functionality of modifying a config file with environment variables. Potentially you could extend the entrypoint functionality for mounting it in a separate location then copying it over with the copy having appropriate permissions. Since when you pass an environment variable it will modify the config file. And if the permissions are non-permissive then you get a descriptive error of sed: couldn't open temporary file /etc/rabbitmq/seda5g8kq: Read-only file system.

The alternative with Kubernetes is to use an init container to mount the config, or supply the full config without any environment variables that the entrypoint interprets, so that way the config's permissions can be read-only without error. Variables like TZ can be used as the rabbitmq.conf wouldn't be modified for that

[Srokap]

All of that could be solved if the file replacement didn't happen in original config directory. Just letting specify writable location for config to be copied to would be enough to solve the issue.

[michaelklishin]

@tianon @yosifkit we keep seeing this issue pop up for RabbitMQ users deploying to Kubernetes. In fact, it comes up every few days and it's particularly frustrating to troubleshoot if you don't know where to start.

Would you please reconsider the decision to label this as a "question" and consider @st3v's ideas or at least making #292 (comment) (a user-provided config file) to be the first option documentation recommends? It would save RabbitMQ users and the core team a lot of frustration. Thank you.

[michaelklishin]

I'm not sure if dropping support for some environment variables this image invents on top of RabbitMQ would be an option; the culture of "env variables for everything" is counterproductive IMO. RabbitMQ does not use environment variables heavily (they mostly control locations of things, which is reasonable), and in particular does not support or recommend them for pre-seeding users at deployment time.

[michaelklishin]

#369 seems like a reasonable approach to me: if the directory is not writeable, move the config file to another directory and point RABBITMQ_CONFIG_FILE to it. Given the number of potential configurations this image can be used in, it does not seem particularly inelegant.

[yosifkit]

@michaelklishin, making a copy doesn't seem unreasonable, since that is what we already do to ssl certificate files:

rabbitmq/docker-entrypoint.sh

Lines 91 to 101 in 88ac0b2

	*_ssl_*file | ssl_*file )
	if [ -f "$val" ] && ! gosu rabbitmq test -r "$val"; then
	newFile="/tmp/rabbitmq-ssl/$conf.pem"
	echo >&2
	echo >&2 "WARNING: '$val' ($var) is not readable by rabbitmq ($(id rabbitmq)); copying to '$newFile'"
	echo >&2
	cat "$val" > "$newFile"
	chown rabbitmq "$newFile"
	chmod 0400 "$newFile"
	eval 'export '$var'="$newFile"'
	fi

I think we'll also need to check if the config file is a mount since the permissions won't really indicate whether a sed will work (since replace won't work on a mounted file).

---

I'd be fairly tempted to drop all the environment variables from the image in the next major release (like 3.9), but that would be a large break for users.

[michaelklishin]

@st3v would you have a chance to update this PR given @yosifkit's suggestion above?