Avoid hardcoded UID and GID in docker build #67
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The [Dockerfile](https://github.com/nextstrain/docs.nextstrain.org/blob/1da467787db9a8f9b43724f3c41264fa30ff4db6/Dockerfile) uses `UID` and `GID` args to create a user inside the container. Files are then written during build on behalf of this user, to avoid permission problems. Mistakenly, these variables were both hardcoded to `1000`, which is fine for a single-user Linux host, bot not for macOS. This PR passes `UID` and `GID` from the host system as build args, so that the files are written on behalf of the the current user.
ivan-aksamentov
force-pushed
the
fix/docker-uid-gid
branch
from
8ac1c37
to
1ea65c1
Compare
tsibley
reviewed
Aug 3, 2021
Makefile
Outdated
| @@ -22,7 +22,11 @@ help: | |||
| .ONESHELL: | |||
| docker-html: | |||
| set -euox | |||
| docker build -t nextstrain-docs-builder --network=host . | |||
| docker build -t nextclade-docs-builder \ | |||
eharkins
reviewed
Aug 3, 2021
There was a problem hiding this comment.
I get this error when I run make docker-html:
docker build -t nextstrain-docs-builder \
--network=host \
--build-arg UID=501 \
--build-arg GID=20 \
.
Sending build context to Docker daemon 142MB
Step 1/16 : FROM continuumio/miniconda3:4.9.2
---> 52daacd3dd5d
Step 2/16 : ARG DEBIAN_FRONTEND=noninteractive
---> Using cache
---> 30387e9a57da
Step 3/16 : ARG USER=user
---> Using cache
---> 45e964da4f9b
Step 4/16 : ARG GROUP=user
---> Using cache
---> e404ca0a624f
Step 5/16 : ARG UID
---> Running in 81e937f813c5
Removing intermediate container 81e937f813c5
---> e3fc6241cef1
Step 6/16 : ARG GID
---> Running in 5e75f0008202
Removing intermediate container 5e75f0008202
---> 0abdb7769eea
Step 7/16 : ENV TERM="xterm-256color"
---> Running in e366dce5f017
Removing intermediate container e366dce5f017
---> 843c7d6522e0
Step 8/16 : ENV HOME="/home/user"
---> Running in 3bfc15c2b77b
Removing intermediate container 3bfc15c2b77b
---> f4d853ad795c
Step 9/16 : RUN set -x && mkdir -p ${HOME}/data && addgroup --system --gid ${GID} ${GROUP} && useradd --system --create-home --home-dir ${HOME} --shell /bin/bash --gid ${GROUP} --groups sudo --uid ${UID} ${USER} && touch ${HOME}.hushlogin
---> Running in 157c76e264da
+ mkdir -p /home/user/data
+ addgroup --system --gid 20 user
addgroup: The GID `20' is already in use.
The command '/bin/sh -c set -x && mkdir -p ${HOME}/data && addgroup --system --gid ${GID} ${GROUP} && useradd --system --create-home --home-dir ${HOME} --shell /bin/bash --gid ${GROUP} --groups sudo --uid ${UID} ${USER} && touch ${HOME}.hushlogin' returned a non-zero code: 1
make: *** [docker-html] Error 1
The error was ``` addgroup: The GID `20' is already in use ``` Turns out nowadays Docker for Mac and Docker for Windows translate UID and GID automagically. And apparently it makes it so that the GID (and perhaps UID?) already exist in the container. This is not the case on Linux. All Dockerfiles relying on consistent UID and GID across platforms are now broken. And it's the only good way (not mentioning half-baked rootles mode that noone ever bothers to confugure) to get docker containers to not run as root and to not create bunch of root-owned files in volumes. What a wonderful decision! So maybe if we try to only create the group and the user if not already exist, this will avoid errors? I cannot test it because I don't have a mac. See: https://stackoverflow.com/questions/43097341/docker-on-macosx-does-not-translate-file-ownership-correctly-in-volumes
Can you retry with the new version? Please delete the This version works on Linux okay, but Mac and Windows have some quirks (described in the commit message). And I don't have a Mac handy to test it. P.S. Unless you are going to actually use the Dockerfile, I don't think we should dig too deeply into it though. |
|
Thanks @ivan-aksamentov it works for me now! |
|
Is any one using the image produced by the Dockerfile in this repo? My sense is not. If that's the case, then we should close this PR without merging and opt to delete the Dockerfile instead. Otherwise, we should finalize this PR one way or the other. |
Not me. I vote to delete. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of proposed changes
The Dockerfile uses
UIDandGIDargs to create a user inside the container. Files are then written during build on behalf of this user, to avoid permission problems.Mistakenly, these variables were both hardcoded to
1000, which is fine for a single-user Linux host, bot not for macOS.This PR passes
UIDandGIDfrom the host system as build args, so that the files are written on behalf of the the current user.Related issue(s)
Amends #63
Testing
Works on my machine (tm)