moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
7.83k stars 1.09k forks source link

[v0.13+] unexpected permissions on COPY'd files #5066

Open tianon opened 1 week ago

tianon commented 1 week ago

When upgrading from BuildKit 0.12 to 0.13, we've got a surprising change in the permission of files that get COPY'd into our images: https://github.com/docker-library/mysql/issues/1069

Previously (on BuildKit 0.12), the permissions of files from COPY config/ /etc/mysql/ was -rw-r--r--, but in 0.13 (and 0.14), the permissions unexpectedly change to -rw-rw-rw-:

$ docker buildx create --name bk-0.12 --driver-opt image=moby/buildkit:v0.12.5
$ docker buildx build --builder bk-0.12 https://github.com/docker-library/mysql.git#319db566ac7fef45c22f3df15ee5e194a7c43259:8.0 --file Dockerfile.debian --load
...
#15 exporting manifest sha256:3a2ad681a06e2d6492f246c29da6f29e28e5c127599bc349340e2e7d080a17b9 done
#15 exporting config sha256:a1e4724c25f314831faf64b92e5518550dbe38b09f5a346c89695c4f8cece893 done
...
$ docker run --rm sha256:a1e4724c25f314831faf64b92e5518550dbe38b09f5a346c89695c4f8cece893 ls -l /etc/mysql/my.cnf
-rw-r--r-- 1 root root 1080 Jun 20 22:31 /etc/mysql/my.cnf
$ docker buildx create --name bk-0.13 --driver-opt image=moby/buildkit:v0.13.2
$ docker buildx build --builder bk-0.13 https://github.com/docker-library/mysql.git#319db566ac7fef45c22f3df15ee5e194a7c43259:8.0 --file Dockerfile.debian --load
...
#16 exporting manifest sha256:54fc97c80362f0f197db2ef97423ca936eda02170c8b6eb3162f84b4132f1384 done
#16 exporting config sha256:471e7a42b881c6b9bc239c1423419ef9270848393c25a7e3083f9f79e8dda9c4 done
...
$ docker run --rm sha256:471e7a42b881c6b9bc239c1423419ef9270848393c25a7e3083f9f79e8dda9c4 ls -l /etc/mysql/my.cnf
-rw-rw-rw- 1 root root 1080 Jun 20 22:34 /etc/mysql/my.cnf
$ docker buildx create --name bk-0.14 --driver-opt image=moby/buildkit:v0.14.1
$ docker buildx build --builder bk-0.14 https://github.com/docker-library/mysql.git#319db566ac7fef45c22f3df15ee5e194a7c43259:8.0 --file Dockerfile.debian --load
...
#16 exporting manifest sha256:801f9f6b8b62bd90cb8f836ef6be6f8e4a1202b56bd4faa794f97b22dfbc49d6 done
#16 exporting config sha256:b470c539d81259b1dc2ee0f0cdd3fb5b692174c187f668b00f8a7ffb553037a1 done
...
$ docker run --rm sha256:b470c539d81259b1dc2ee0f0cdd3fb5b692174c187f668b00f8a7ffb553037a1 ls -l /etc/mysql/my.cnf
-rw-rw-rw- 1 root root 1080 Jun 20 22:36 /etc/mysql/my.cnf

(Obviously, there could be made a simpler/smaller reproducer than this, but this is one I had handy that's already in Git somewhere so it's trivial to let BuildKit do the clone and make certain local permissions aren't a factor.)

tianon commented 1 week ago

I think https://github.com/moby/buildkit/pull/2356 was a prior fix for a similar issue, but perhaps it wasn't sufficient or something else in 0.13 lost the fix? (I don't see anything obvious in 0.13's release notes that would explain it, but I'm not sure exactly what to look for since I don't think it was an intentional change :sweat_smile:)

thaJeztah commented 1 week ago

I recently was looking at permission issues as well, but there it was the reverse; but there was a difference between buildkit standalone and in docker engine; see

And most notably; https://github.com/moby/buildkit/pull/4936#issuecomment-2123870982

thaJeztah commented 1 week ago

That was added 5+ Years ago though, so not something recent https://github.com/moby/buildkit/commit/7210bf6806999432856be71eccea0b28c4131069

tianon commented 1 week ago

Happy to test again if I've done something wrong in my docker buildx creates that would break that (very old) umask line somehow :sweat_smile:

tonistiigi commented 1 week ago
d34b2471cd53edd462c2c7097e4de70e26961fa6 is the first bad commit
commit d34b2471cd53edd462c2c7097e4de70e26961fa6
Author: Justin Chadwell <me@jedevc.com>
Date:   Fri Aug 4 12:47:40 2023 +0100

    git: centralize git cli operations

    Move all of the git command line logic into a single object, inspired by
    the object already in buildx.

    The basic implemenation allows for configuring a git cli for a specific
    repository, along with various authorization settings and custom
    binaries. Commands can be run for that repository, and a few helpers are
    provided for accessing data on it - more to come in the future
    hopefully.

    Signed-off-by: Justin Chadwell <me@jedevc.com>

 source/git/source.go            | 248 +++++++++++++++++-----------------------
 source/git/source_test.go       |  21 +++-
 util/gitutil/git_cli.go         | 243 +++++++++++++++++++++++++++++++++++++++
 util/gitutil/git_cli_helpers.go |  44 +++++++
 4 files changed, 411 insertions(+), 145 deletions(-)
 create mode 100644 util/gitutil/git_cli.go
 create mode 100644 util/gitutil/git_cli_helpers.go

https://github.com/moby/buildkit/pull/4106#pullrequestreview-1592606139 🙋

cc @jedevc

grooverdan commented 3 days ago

I would have reported this earlier if the permission behaviour was documented on https://docs.docker.com/reference/dockerfile/#copy.

Request - please document the default chmod permissions for Dockerfile COPY.

thompson-shaun commented 12 minutes ago

/cc @dvdksn on the docs bit ;)