Fix secret mounting with userns_remap enabled#2723
Fix secret mounting with userns_remap enabled#2723jiashuChen wants to merge 2 commits intomoby:masterfrom jiashuChen:master
Conversation
|
While trying to compile the buidkit for local testing. I notice the same problem seems to occur in the localmounter_unix module. As I get the following error with |
|
Isn't the place you are looking to configure buildkit/solver/llbsolver/mounts/mount.go Line 284 in a45b769 |
yeah, this is the same file I was trying to modify (my repo seems to be behind master change (which removed deprecated |
The previous commit here moby/moby@bfedd27 modified the owner and permission of the DOCKER_TMP to root:root and 700 respectively, and it seems to have assumed that the folder is not needed by the remapped root user, however, this is not actually the case for buildkit secrets. The solution here is to move the secret file created on host into the `/var/lib/docker/${UID}:${GID}/buildkit/mounts/secrets/` directory Since the `/var/lib/docker/${UID}:${GID}/buildkit` is created with `711` permission, this should resolve the issue with secret mount. Based on code logic here: moby/moby@5263bea/cmd/dockerd/daemon.go#L299 moby/moby@5c01d06/builder/builder-next/controller.go#L47-L49 Signed-off-by: Jiashu Chen <cjs20080808@hotmail.com>
This is because docker create `TMPDIR` at`/var/lib/docker/tmp` it only allows `root:root` user to access. However, this means when we use userns_remap mode in docker daemon, we will not be able to mount any volume in the container from this tmp directory The solution is to create a separate directory under the env var `BUILDKIT_TMPDIR`, and treat it as the default temp directory. Signed-off-by: Jiashu Chen <cjs20080808@hotmail.com>
|
Looks like this issue has been fixed in the latest docker release For others experiencing the same issue. Please upgrade docker engine to |
2 participants
This Pull Request is created based on feedback from @cpuguy83 in this pull request moby/moby#43354
Tested by compiling my own version of dockerd from this pull request moby/moby#43354 with a cherry-picked version of this changed because moby still depends on buildkit commit 9f254e1 released in 2021.
Related issue:
The previous commit here moby/moby@bfedd27 modified
the owner and permission of the DOCKER_TMP to root:root and 700 respectively,
and it seems to have assumed that the folder is not needed by the remapped
root user, however, this is not actually the case for buildkit secrets.
Although the original issue was raised only relating to docker secrets mounts which uses temp file
created in
DOCKER_TMPdirectory. This issue actually impact other locations as well.The proposed solution will created an environment variable
BUILDKIT_TMPDIRwhich buildkit will lookup and thisdirectory will take priority over the
TMPDIRthatthe default os.tempDirreturns.Main change here https://github.com/jiashuChen/buildkit/blob/256c02dcc6d50c99fe6396fe83d241940eefbe96/util/system/tempdir.go#L9.
I have then refactored all location that uses
os.MkdirTempto use the newly created function above.NOTE:
In order to resolve the issue, the solution proposed here need both this pull request and moby/moby#43354, currently moby/moby#43354 uses a private fork from my repo in order to demostrate the idea, however, it would highly appreciated if code owner from moby can help with either upgrade buildkit from moby/moby to the latest version or provide a guide on whether backport is allowed.
Signed-off-by: Jiashu Chen cjs20080808@hotmail.com