entrypoint.sh setting of UID/GID doesn't play nice with NFS-provisioned storage

[rmpinchback]

Request details

I don't really consider what I'll describe as necessarily a bug per se, since it is probably a bit on the fringe of how storage provisioning was conceived of for reposilite. However I think a reasonable argument can be made for future improvement.

Depending on the NFS server you have, sometimes UID:GID gets squashed into particular values mandated by the server configuration. This can be the case with a Synology NAS for example, if you are trying to use it to back persistent volumes so that container/pod death or removal or migration to a different host, doesn't cause a corresponding loss of persisted data. Part of what makes this a little extra challenging on Synology is that it is hard to force specific UID/GID numbers. You are better off playing nicely with what the vendor provides: UID=1024/GID=100 (100=users).

If you don't accept this limitation, then on an O/S-level (i.e. DSM) upgrade, your efforts to tweak the UID and GID can get blown away. Even if that were not a concern, Synology creates a lot of obfuscation around how such overrides interact with their software. Knowing how to make NFS configuration changes alone is about 10% of the problem, the rest is research and experimentation to dance around vendor-specific cruft hidden in layers of web UI handlers. And the vendor routinely makes substantial changes to those layers.

With most helm charts and pod deployments, this doesn't cause me any problems. Once in awhile though, a pod tries to do some extra magic with storage instead of just relying on the provisioning to work. The reposilite entrypoint.sh script is an example. It wants to set ownership of a directory, which in a squashed NFS provisioning is going to fail unless that directory already has the same UID/GID ownership that 'chown' will try to establish.

So far as I can tell, the PUID/PGID environment variables in entrypoint.sh have no association with the pod or container security contexts for fsGroup/runAsUser/runAsGroup values. Setting values to override those in the contexts, contrary to the implications of values.yaml, appears to serve no purpose. The PUID/PGID can be overridden as env settings for the pod though, and I tried that.

Which would almost work, if it it wasn't for the fact that entrypoint.sh tries to create a group without first checking to see if the GID is already used. It wants to make "reposilite" be the group name, instead of taking a variable for the group name or re-using an existing group, or not caring about names for UID and GID at all and simply relying on the numbers alone.

End result, while you can override PGID, you can only override it with an unused number for a GID.

Obviously a lot of this could be attributed to "figure out your squirrely NFS setup", which is why I don't consider what I ran into a bug to blame reposilite for. More this is just a suggestion that, given the unknowns that could come from externally-determined storage provisioning, the entrypoint.sh content may be a little more rigid than the possibilities allowed by a kubernetes cluster.

[dzikoysk]

Hey! It'd be great if we could find out a solution - I'm open to any suggestion as long as we'll stay backwards compatible with the old behavior. I guess your case could be easily covered by some sort of variable that'd basically just launch the jar, without any additional work related to creating these non-root users etc., right? :)

[rmpinchback]

I was able to easily hack around the issue, just with a Dockerfile that extended from yours, and I overwrite the entrypoint.sh script with a simpler version of yours:

#!/bin/bash
set -e

REPOSILITE_ARGS="$REPOSILITE_OPTS"
case "$REPOSILITE_OPTS" in
  *"--working-directory"*) ;;
  *"-wd"*                ) ;;
  *                      ) REPOSILITE_ARGS="--working-directory=/app/data $REPOSILITE_ARGS";;
esac

exec java \
     -Dtinylog.writerFile.file="/var/log/reposilite/log_{date}.txt" \
     -Dtinylog.writerFile.latest=/var/log/reposilite/latest.log \
     $JAVA_OPTS \
     -jar reposilite.jar \
     $REPOSILITE_ARGS

Everything was fine. That doesn't make the above be the correct fix, it just proofed out that it was possible to have a fix.

I think the problem I ran into with the original entrypoint.sh script begins in the following line:

if [ "$UID" -ne "0" ]; then

In a container running in Kubernetes, nothing sets the UID by default. I think it only gets set if the Dockerfile itself specifies a USER. Without an existing UID environment variable, that line will always fail, and the first section of your script containing exec java... will always be passed over and the next section will be used instead.

I did some experimenting with my container (which I needed to, because yours couldn't finish starting due to the NFS problem). What I found was:

• TEST1: if I didn't enable security context settings at all (which is the default), then everything runs as root. I could exec into the container, verify no UID, verify that if I touched a file in /tmp that it was owned by root.
• TEST2: if I did enable security context settings with the values from your helm chart's values.yaml file. I could exec in, verify no UID, but if I touched a file in /tmp then the ownership of the file now reflected the runAsUser/runAsGroup numbers from your values.yaml file.
• TEST3: if I made a slight modification by adding "runAsNonRoot: true", there was no net difference to TEST2.

Bottom line, if I did a "set" from the shell to look at the environment variables, nothing there reflected any difference between TEST1, TEST2, TEST3. Only in creating a file did I see that the container knew what userid and groupid I was running as, but the environment variables were silent about that.

If you look at Dockerfiles from projects like jenkinsci/jenkins, they sometimes have ARG settings that take uid/gid numbers, do some initial setup magic with them, then later declare a USER based on the uid. In looking at various helm charts I'm currently running, I can confirm that the only one with a UID set when I exec into the pod is for Jenkins; all the others I've looked at do not have a UID, and I confirmed that their Dockerfiles do not set a USER. However it's only a small number of charts so don't take that as massive statistical proof; it's just consistent with my hypothesis to the degree I've looked.

So, if I had to guess at the most minimal fix you could make, it would be to experiment with setting a USER, and seeing if it results in a UID. If there is a UID, then the first portion of your entrypoint.sh script would be invoked, instead of the second portion which is what I was encountering. A clean fix is probably:

1. Consider changing "set -e" to "set -eu" at the top of the entrypoint.sh script, since that would have bombed out the entrypoint and let you know that this issue was lurking. However you'll want to test this change carefully, sometimes setting "-u" opens up more bash weirdness to track down.
2. Add a USER to the Dockerfile and confirm that it gets you a UID. I don't think that Jenkins using an ARG named 'uid' is why that container has a UID variable in the environment, but I could be wrong.
3. Instead of grepping for reposilite when deciding to create groups or users, really you have a more involved decision to make than currently scripted:
   • If a PGID/PUID was given and that number is found in /etc/group or /etc/passwd, you use the name of the user or group that was found in those files; otherwise grep for 'reposilite' and if that is found then use 'reposilite' as the name of the user or group; if neither were found, create a group or user 'reposilite' with the number given. When finally flowing down to the chown and exec steps, use the user and group names that were ultimately decided by this process. This should be backwards-compatible because any violation of the logic I described already should have been a failed deploy anyways.

[rmpinchback]

One cautionary note I'll add to my described potential solution. The Jenkins handling of userid/groupid is not necessarily a best-of-breed base to work from. It's nearly impossible to change what those numbers are. For all that the helm chart makes it look like you can change those numbers, you can't really make it work without creating your own docker image (in part due to some git security wierdness). Tweaking the kubernetes security context settings alone tends to get you only part way.

I don't really have enough context on what 'backward compatible' needs to mean for you. So take my suggestions as only that: suggestions. Container images that know something about user and group specifics seems to be a bit of an inversion of responsibility at least from a k8s perspective, and I have to suspect that how fussy it is to get right depends entirely on what you were trying to achieve originally.

[dzikoysk]

Thanks for the details, I'll try to take a look at this thing on the weekend. In case you'd notice I'm not picking this up for a few days, feel free to just ping me :)

[rmpinchback]

In doing a little digging, I think one easy change to make to entrypoint.sh is to alter line 11 from:

if [ "$UID" -ne "0" ]; then

to:

if (( `id -u` != 0 )); then

This eliminates any need to care about the existence of a UID variable. It gets you exactly the original intended behavior of that line of the script. Anybody who sets deployment.containerSecurityContext.enabled: true in the values.yaml will now correctly get the upper part of entrypoint.sh behavior. That doesn't mean the other improvements aren't worth considering, but I'm a big fan of small surgical changes when it is trivially obvious that they are always a net improvement. This line is absolutely a bug, everything else is more of a debate about intended function.

Also with this change, there is less merit to considering changing the top of the script from "set -e" to "set -eu" because there is no initial environment variable comparison to worry about. Everywhere else in the script is intentionally concerning itself with variables that may or may not exist, making "set -u" more problematic.

[dzikoysk]

Add a USER to the Dockerfile and confirm that it gets you a UID. I don't think that Jenkins using an ARG named 'uid' is why that container has a UID variable in the environment, but I could be wrong.

If think we had some issues with hardcoded USER at Dockerfile level, so it was just fully moved to the entrypoint script. I think this approach works quite well, so I'd just try to keep the whole exec logic there if possible.

If a PGID/PUID was given and that number is found in /etc/group or /etc/passwd, you use the name of the user or group that was found in those files; otherwise grep for 'reposilite' and if that is found then use 'reposilite' as the name of the user or group; if neither were found, create a group or user 'reposilite' with the number given. When finally flowing down to the chown and exec steps, use the user and group names that were ultimately decided by this process. This should be backwards-compatible because any violation of the logic I described already should have been a failed deploy anyways.

I really like the idea of finding user/group based on the provided PGID/PUID. Would you like to submit a PR? :)

In doing a little digging, I think one easy change to make to entrypoint.sh is to alter line 11 from [...]

Done 👍 You should be able to test it using the latest nightly build.