From 143e69492db7ed535f065758973a44eebad7b463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Garriga-Alonso?= Date: Tue, 28 Nov 2023 21:13:42 +0530 Subject: [PATCH] Create intermediate directories in COPY with correct uid and gid (#2795) * Create directories with the right UID/GID * Forgot to create the actual directory * Integration test creation of intermediate files with correct ownership * ADD version of the test --- ...ockerfile_test_add_chown_intermediate_dirs | 16 ++++++++ ...ckerfile_test_copy_chown_intermediate_dirs | 16 ++++++++ pkg/commands/copy_test.go | 9 +++++ pkg/util/fs_util.go | 40 ++++++++++++++++--- 4 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_add_chown_intermediate_dirs create mode 100644 integration/dockerfiles/Dockerfile_test_copy_chown_intermediate_dirs diff --git a/integration/dockerfiles/Dockerfile_test_add_chown_intermediate_dirs b/integration/dockerfiles/Dockerfile_test_add_chown_intermediate_dirs new file mode 100644 index 000000000..8d5697386 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_add_chown_intermediate_dirs @@ -0,0 +1,16 @@ +FROM alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a +# Create dev user and group, with id 1001 +RUN yes | adduser -u 1001 dev + +ADD --chown=dev:dev context/foo /path/to/foo +ADD --chown=dev:dev context/qux /path/to/qux +ADD --chown=1001:1001 context/foo /path2/to/foo +ADD --chown=1001:1001 context/qux /path2/to/qux + +USER dev + +# `mkdir` fails when `dev` does not own all of `/path{,2}/to{,/qux}` +RUN mkdir /path/to/new_dir +RUN mkdir /path/to/qux/new_dir +RUN mkdir /path2/to/new_dir +RUN mkdir /path2/to/qux/new_dir diff --git a/integration/dockerfiles/Dockerfile_test_copy_chown_intermediate_dirs b/integration/dockerfiles/Dockerfile_test_copy_chown_intermediate_dirs new file mode 100644 index 000000000..8133a60a7 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_copy_chown_intermediate_dirs @@ -0,0 +1,16 @@ +FROM alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a +# Create dev user and group, with id 1001 +RUN yes | adduser -u 1001 dev + +COPY --chown=dev:dev context/foo /path/to/foo +COPY --chown=dev:dev context/qux /path/to/qux +COPY --chown=1001:1001 context/foo /path2/to/foo +COPY --chown=1001:1001 context/qux /path2/to/qux + +USER dev + +# `mkdir` fails when `dev` does not own all of `/path{,2}/to{,/qux}` +RUN mkdir /path/to/new_dir +RUN mkdir /path/to/qux/new_dir +RUN mkdir /path2/to/new_dir +RUN mkdir /path2/to/qux/new_dir diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 3eba9c6b2..ed89b1673 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -67,6 +67,11 @@ var copyTests = []struct { sourcesAndDest: []string{"f[o][osp]", "tempCopyExecuteTest"}, expectedDest: []string{"tempCopyExecuteTest"}, }, + { + name: "Copy into several to-be-created directories", + sourcesAndDest: []string{"f[o][osp]", "tempCopyExecuteTest/foo/bar"}, + expectedDest: []string{"bar"}, + }, } func setupTestTemp(t *testing.T) string { @@ -310,6 +315,10 @@ func TestCopyExecuteCmd(t *testing.T) { if err != nil { t.Error() } + if fstat == nil { + t.Error() + return // Unrecoverable, will segfault in the next line + } if fstat.IsDir() { files, err := ioutil.ReadDir(dest) if err != nil { diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 8818207a6..eea6da739 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -578,7 +578,7 @@ func resetFileOwnershipIfNotMatching(path string, newUID, newGID uint32) error { // CreateFile creates a file at path and copies over contents from the reader func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid uint32) error { // Create directory path if it doesn't exist - if err := createParentDirectory(path); err != nil { + if err := createParentDirectory(path, int(uid), int(gid)); err != nil { return errors.Wrap(err, "creating parent dir") } @@ -707,7 +707,7 @@ func CopySymlink(src, dest string, context FileContext) (bool, error) { return false, err } } - if err := createParentDirectory(dest); err != nil { + if err := createParentDirectory(dest, DoNotChangeUID, DoNotChangeGID); err != nil { return false, err } link, err := os.Readlink(src) @@ -951,7 +951,7 @@ func CopyFileOrSymlink(src string, destDir string, root string) error { if err != nil { return errors.Wrap(err, "copying file or symlink") } - if err := createParentDirectory(destFile); err != nil { + if err := createParentDirectory(destFile, DoNotChangeUID, DoNotChangeGID); err != nil { return err } return os.Symlink(link, destFile) @@ -1015,12 +1015,40 @@ func CopyOwnership(src string, destDir string, root string) error { }) } -func createParentDirectory(path string) error { +func createParentDirectory(path string, uid int, gid int) error { baseDir := filepath.Dir(path) if info, err := os.Lstat(baseDir); os.IsNotExist(err) { logrus.Tracef("BaseDir %s for file %s does not exist. Creating.", baseDir, path) - if err := os.MkdirAll(baseDir, 0755); err != nil { - return err + + dir := baseDir + dirs := []string{baseDir} + for { + if dir == "/" || dir == "." || dir == "" { + break + } + dir = filepath.Dir(dir) + dirs = append(dirs, dir) + } + + for i := len(dirs) - 1; i >= 0; i-- { + dir := dirs[i] + + if _, err := os.Lstat(dir); os.IsNotExist(err) { + os.Mkdir(dir, 0755) + if uid != DoNotChangeUID { + if gid != DoNotChangeGID { + os.Chown(dir, uid, gid) + } else { + return errors.New(fmt.Sprintf("UID=%d but GID=-1, i.e. it is not set for %s", uid, dir)) + } + } else { + if gid != DoNotChangeGID { + return errors.New(fmt.Sprintf("GID=%d but UID=-1, i.e. it is not set for %s", gid, dir)) + } + } + } else if err != nil { + return err + } } } else if IsSymlink(info) { logrus.Infof("Destination cannot be a symlink %v", baseDir)