diff --git a/copier/copier.go b/copier/copier.go index 84b636202a4..ef0e4778d7f 100644 --- a/copier/copier.go +++ b/copier/copier.go @@ -10,6 +10,7 @@ import ( "net" "os" "os/user" + "path" "path/filepath" "strconv" "strings" @@ -202,11 +203,11 @@ type StatOptions struct { // If root and directory are both not specified, the current root directory is // used, and relative names in the globs list are treated as being relative to // the current working directory. -// If root is specified and the current OS supports it, the stat() is performed -// in a chrooted context. If the directory is specified as an absolute path, -// it should either be the root directory or a subdirectory of the root -// directory. Otherwise, the directory is treated as a path relative to the -// root directory. +// If root is specified and the current OS supports it, and the calling process +// has the necessary privileges, the stat() is performed in a chrooted context. +// If the directory is specified as an absolute path, it should either be the +// root directory or a subdirectory of the root directory. Otherwise, the +// directory is treated as a path relative to the root directory. // Relative names in the glob list are treated as being relative to the // directory. func Stat(root string, directory string, options StatOptions, globs []string) ([]*StatsForGlob, error) { @@ -229,18 +230,19 @@ func Stat(root string, directory string, options StatOptions, globs []string) ([ // GetOptions controls parts of Get()'s behavior. type GetOptions struct { - UIDMap, GIDMap []idtools.IDMap // map from hostIDs to containerIDs in the output archive - Excludes []string // contents to pretend don't exist, using the OS-specific path separator - ExpandArchives bool // extract the contents of named items that are archives - ChownDirs *idtools.IDPair // set ownership on directories. no effect on archives being extracted - ChmodDirs *os.FileMode // set permissions on directories. no effect on archives being extracted - ChownFiles *idtools.IDPair // set ownership of files. no effect on archives being extracted - ChmodFiles *os.FileMode // set permissions on files. no effect on archives being extracted - StripSetuidBit bool // strip the setuid bit off of items being copied. no effect on archives being extracted - StripSetgidBit bool // strip the setgid bit off of items being copied. no effect on archives being extracted - StripStickyBit bool // strip the sticky bit off of items being copied. no effect on archives being extracted - StripXattrs bool // don't record extended attributes of items being copied. no effect on archives being extracted - KeepDirectoryNames bool // don't strip the top directory's basename from the paths of items in subdirectories + UIDMap, GIDMap []idtools.IDMap // map from hostIDs to containerIDs in the output archive + Excludes []string // contents to pretend don't exist, using the OS-specific path separator + ExpandArchives bool // extract the contents of named items that are archives + ChownDirs *idtools.IDPair // set ownership on directories. no effect on archives being extracted + ChmodDirs *os.FileMode // set permissions on directories. no effect on archives being extracted + ChownFiles *idtools.IDPair // set ownership of files. no effect on archives being extracted + ChmodFiles *os.FileMode // set permissions on files. no effect on archives being extracted + StripSetuidBit bool // strip the setuid bit off of items being copied. no effect on archives being extracted + StripSetgidBit bool // strip the setgid bit off of items being copied. no effect on archives being extracted + StripStickyBit bool // strip the sticky bit off of items being copied. no effect on archives being extracted + StripXattrs bool // don't record extended attributes of items being copied. no effect on archives being extracted + KeepDirectoryNames bool // don't strip the top directory's basename from the paths of items in subdirectories + Rename map[string]string // rename items with the specified names, or under the specified names } // Get produces an archive containing items that match the specified glob @@ -248,11 +250,11 @@ type GetOptions struct { // If root and directory are both not specified, the current root directory is // used, and relative names in the globs list are treated as being relative to // the current working directory. -// If root is specified and the current OS supports it, the contents are read -// in a chrooted context. If the directory is specified as an absolute path, -// it should either be the root directory or a subdirectory of the root -// directory. Otherwise, the directory is treated as a path relative to the -// root directory. +// If root is specified and the current OS supports it, and the calling process +// has the necessary privileges, the contents are read in a chrooted context. +// If the directory is specified as an absolute path, it should either be the +// root directory or a subdirectory of the root directory. Otherwise, the +// directory is treated as a path relative to the root directory. // Relative names in the glob list are treated as being relative to the // directory. func Get(root string, directory string, options GetOptions, globs []string, bulkWriter io.Writer) error { @@ -278,25 +280,28 @@ func Get(root string, directory string, options GetOptions, globs []string, bulk // PutOptions controls parts of Put()'s behavior. type PutOptions struct { - UIDMap, GIDMap []idtools.IDMap // map from containerIDs to hostIDs when writing contents to disk - DefaultDirOwner *idtools.IDPair // set ownership of implicitly-created directories, default is ChownDirs, or 0:0 if ChownDirs not set - DefaultDirMode *os.FileMode // set permissions on implicitly-created directories, default is ChmodDirs, or 0755 if ChmodDirs not set - ChownDirs *idtools.IDPair // set ownership of newly-created directories - ChmodDirs *os.FileMode // set permissions on newly-created directories - ChownFiles *idtools.IDPair // set ownership of newly-created files - ChmodFiles *os.FileMode // set permissions on newly-created files - StripXattrs bool // don't bother trying to set extended attributes of items being copied - IgnoreXattrErrors bool // ignore any errors encountered when attempting to set extended attributes + UIDMap, GIDMap []idtools.IDMap // map from containerIDs to hostIDs when writing contents to disk + DefaultDirOwner *idtools.IDPair // set ownership of implicitly-created directories, default is ChownDirs, or 0:0 if ChownDirs not set + DefaultDirMode *os.FileMode // set permissions on implicitly-created directories, default is ChmodDirs, or 0755 if ChmodDirs not set + ChownDirs *idtools.IDPair // set ownership of newly-created directories + ChmodDirs *os.FileMode // set permissions on newly-created directories + ChownFiles *idtools.IDPair // set ownership of newly-created files + ChmodFiles *os.FileMode // set permissions on newly-created files + StripXattrs bool // don't bother trying to set extended attributes of items being copied + IgnoreXattrErrors bool // ignore any errors encountered when attempting to set extended attributes + NoOverwriteDirNonDir bool // instead of quietly overwriting directories with non-directories, return an error + Rename map[string]string // rename items with the specified names, or under the specified names } // Put extracts an archive from the bulkReader at the specified directory. // If root and directory are both not specified, the current root directory is // used. -// If root is specified and the current OS supports it, the contents are written -// in a chrooted context. If the directory is specified as an absolute path, -// it should either be the root directory or a subdirectory of the root -// directory. Otherwise, the directory is treated as a path relative to the -// root directory. +// If root is specified and the current OS supports it, and the calling process +// has the necessary privileges, the contents are written in a chrooted +// context. If the directory is specified as an absolute path, it should +// either be the root directory or a subdirectory of the root directory. +// Otherwise, the directory is treated as a path relative to the root +// directory. func Put(root string, directory string, options PutOptions, bulkReader io.Reader) error { req := request{ Request: requestPut, @@ -325,11 +330,12 @@ type MkdirOptions struct { // need to be created will be given the specified ownership and permissions. // If root and directory are both not specified, the current root directory is // used. -// If root is specified and the current OS supports it, the directory is -// created in a chrooted context. If the directory is specified as an absolute -// path, it should either be the root directory or a subdirectory of the root -// directory. Otherwise, the directory is treated as a path relative to the -// root directory. +// If root is specified and the current OS supports it, and the calling process +// has the necessary privileges, the directory is created in a chrooted +// context. If the directory is specified as an absolute path, it should +// either be the root directory or a subdirectory of the root directory. +// Otherwise, the directory is treated as a path relative to the root +// directory. func Mkdir(root string, directory string, options MkdirOptions) error { req := request{ Request: requestMkdir, @@ -547,13 +553,13 @@ func copierWithSubprocess(bulkReader io.Reader, bulkWriter io.Writer, req reques return nil, errors.Wrap(err, step) } if err = encoder.Encode(req); err != nil { - return killAndReturn(err, "error encoding request") + return killAndReturn(err, "error encoding request for copier subprocess") } if err = decoder.Decode(&resp); err != nil { - return killAndReturn(err, "error decoding response") + return killAndReturn(err, "error decoding response from copier subprocess") } if err = encoder.Encode(&request{Request: requestQuit}); err != nil { - return killAndReturn(err, "error encoding request") + return killAndReturn(err, "error encoding request for copier subprocess") } stdinWrite.Close() stdinWrite = nil @@ -626,7 +632,7 @@ func copierMain() { // Read a request. req := new(request) if err := decoder.Decode(req); err != nil { - fmt.Fprintf(os.Stderr, "error decoding request: %v", err) + fmt.Fprintf(os.Stderr, "error decoding request from copier parent process: %v", err) os.Exit(1) } if req.Request == requestQuit { @@ -717,12 +723,12 @@ func copierMain() { } resp, cb, err := copierHandler(bulkReader, bulkWriter, *req) if err != nil { - fmt.Fprintf(os.Stderr, "error handling request %#v: %v", *req, err) + fmt.Fprintf(os.Stderr, "error handling request %#v from copier parent process: %v", *req, err) os.Exit(1) } // Encode the response. if err := encoder.Encode(resp); err != nil { - fmt.Fprintf(os.Stderr, "error encoding response %#v: %v", *req, err) + fmt.Fprintf(os.Stderr, "error encoding response %#v for copier parent process: %v", *req, err) os.Exit(1) } // If there's bulk data to transfer, run the callback to either @@ -1118,6 +1124,34 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa return &response{Stat: statResponse.Stat, Get: getResponse{}}, cb, nil } +func handleRename(rename map[string]string, name string) string { + if rename == nil { + return name + } + // header names always use '/', so use path instead of filepath to manipulate it + if directMapping, ok := rename[name]; ok { + return directMapping + } + prefix, remainder := path.Split(name) + for prefix != "" { + if mappedPrefix, ok := rename[prefix]; ok { + return path.Join(mappedPrefix, remainder) + } + if prefix[len(prefix)-1] == '/' { + if mappedPrefix, ok := rename[prefix[:len(prefix)-1]]; ok { + return path.Join(mappedPrefix, remainder) + } + } + newPrefix, middlePart := path.Split(prefix) + if newPrefix == prefix { + return name + } + prefix = newPrefix + remainder = path.Join(middlePart, remainder) + } + return name +} + func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath string, options GetOptions, tw *tar.Writer, hardlinkChecker *util.HardlinkChecker, idMappings *idtools.IDMappings) error { // build the header using the name provided hdr, err := tar.FileInfoHeader(srcfi, symlinkTarget) @@ -1127,6 +1161,9 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str if name != "" { hdr.Name = filepath.ToSlash(name) } + if options.Rename != nil { + hdr.Name = handleRename(options.Rename, hdr.Name) + } if options.StripSetuidBit { hdr.Mode &^= cISUID } @@ -1164,6 +1201,9 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str tr := tar.NewReader(rc) hdr, err := tr.Next() for err == nil { + if options.Rename != nil { + hdr.Name = handleRename(options.Rename, hdr.Name) + } if err = tw.WriteHeader(hdr); err != nil { return errors.Wrapf(err, "error writing tar header from %q to pipe", contentPath) } @@ -1311,8 +1351,13 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM createFile := func(path string, tr *tar.Reader) (int64, error) { f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC|os.O_EXCL, 0600) if err != nil && os.IsExist(err) { - if err = os.Remove(path); err != nil { - return 0, errors.Wrapf(err, "copier: put: error removing file to be overwritten %q", path) + if req.PutOptions.NoOverwriteDirNonDir { + if st, err2 := os.Lstat(path); err2 == nil && st.IsDir() { + return 0, errors.Wrapf(err, "copier: put: error creating file at %q", path) + } + } + if err = os.RemoveAll(path); err != nil { + return 0, errors.Wrapf(err, "copier: put: error removing item to be overwritten %q", path) } f, err = os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC|os.O_EXCL, 0600) } @@ -1360,6 +1405,14 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM tr := tar.NewReader(bulkReader) hdr, err := tr.Next() for err == nil { + if len(hdr.Name) == 0 { + // no name -> ignore the entry + hdr, err = tr.Next() + continue + } + if req.PutOptions.Rename != nil { + hdr.Name = handleRename(req.PutOptions.Rename, hdr.Name) + } // figure out who should own this new item if idMappings != nil && !idMappings.Empty() { containerPair := idtools.IDPair{UID: hdr.Uid, GID: hdr.Gid} @@ -1412,35 +1465,70 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM } case tar.TypeLink: var linkTarget string + if req.PutOptions.Rename != nil { + hdr.Linkname = handleRename(req.PutOptions.Rename, hdr.Linkname) + } if linkTarget, err = resolvePath(targetDirectory, filepath.Join(req.Root, filepath.FromSlash(hdr.Linkname)), nil); err != nil { return errors.Errorf("error resolving hardlink target path %q under root %q", hdr.Linkname, req.Root) } if err = os.Link(linkTarget, path); err != nil && os.IsExist(err) { + if req.PutOptions.NoOverwriteDirNonDir { + if st, err := os.Lstat(path); err == nil && st.IsDir() { + break + } + } if err = os.Remove(path); err == nil { err = os.Link(linkTarget, path) } } case tar.TypeSymlink: + // if req.PutOptions.Rename != nil { + // todo: the general solution requires resolving to an absolute path, handling + // renaming, and then possibly converting back to a relative symlink + // } if err = os.Symlink(filepath.FromSlash(hdr.Linkname), filepath.FromSlash(path)); err != nil && os.IsExist(err) { + if req.PutOptions.NoOverwriteDirNonDir { + if st, err := os.Lstat(path); err == nil && st.IsDir() { + break + } + } if err = os.Remove(path); err == nil { err = os.Symlink(filepath.FromSlash(hdr.Linkname), filepath.FromSlash(path)) } } case tar.TypeChar: if err = mknod(path, chrMode(0600), int(mkdev(devMajor, devMinor))); err != nil && os.IsExist(err) { + if req.PutOptions.NoOverwriteDirNonDir { + if st, err := os.Lstat(path); err == nil && st.IsDir() { + break + } + } if err = os.Remove(path); err == nil { err = mknod(path, chrMode(0600), int(mkdev(devMajor, devMinor))) } } case tar.TypeBlock: if err = mknod(path, blkMode(0600), int(mkdev(devMajor, devMinor))); err != nil && os.IsExist(err) { + if req.PutOptions.NoOverwriteDirNonDir { + if st, err := os.Lstat(path); err == nil && st.IsDir() { + break + } + } if err = os.Remove(path); err == nil { err = mknod(path, blkMode(0600), int(mkdev(devMajor, devMinor))) } } case tar.TypeDir: if err = os.Mkdir(path, 0700); err != nil && os.IsExist(err) { - err = nil + var st os.FileInfo + if st, err = os.Stat(path); err == nil && !st.IsDir() { + // it's not a directory, so remove it and mkdir + if err = os.Remove(path); err == nil { + err = os.Mkdir(path, 0700) + } + } + // either we removed it and retried, or it was a directory, + // in which case we want to just add the new stuff under it } // make a note of the directory's times. we // might create items under it, which will @@ -1453,6 +1541,11 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM }) case tar.TypeFifo: if err = mkfifo(path, 0600); err != nil && os.IsExist(err) { + if req.PutOptions.NoOverwriteDirNonDir { + if st, err := os.Lstat(path); err == nil && st.IsDir() { + break + } + } if err = os.Remove(path); err == nil { err = mkfifo(path, 0600) } diff --git a/copier/copier_test.go b/copier/copier_test.go index ac1f2fb9812..2660c23a45e 100644 --- a/copier/copier_test.go +++ b/copier/copier_test.go @@ -196,6 +196,11 @@ var ( excludes []string expectedGetErrors []expectedError subdirContents map[string][]string + renames []struct { + name string + renames map[string]string + expected []string + } }{ { name: "regular", @@ -244,6 +249,156 @@ var ( {inSubdir: true, name: "subdir-a/file-b", err: syscall.ENOENT}, {inSubdir: true, name: "subdir-a/file-c", err: syscall.ENOENT}, }, + renames: []struct { + name string + renames map[string]string + expected []string + }{ + { + name: "no-match-dir", + renames: map[string]string{"subdir-z": "subdir-y"}, + expected: []string{ + "file-0", + "file-a", + "file-b", + "file-c", + "file-u", + "file-g", + "file-t", + "link-0", + "link-a", + "link-b", + "hlink-0", + "hlink-a", + "hlink-b", + "subdir-a", + "subdir-a/file-n", + "subdir-a/file-o", + "subdir-a/file-a", + "subdir-a/file-b", + "subdir-a/file-c", + "subdir-b", + "subdir-b/file-n", + "subdir-b/file-o", + "subdir-c", + "subdir-c/file-n", + "subdir-c/file-o", + "subdir-d", + "subdir-d/hlink-0", + "subdir-d/hlink-a", + "subdir-d/hlink-b", + "archive-a", + }, + }, + { + name: "no-match-file", + renames: map[string]string{"file-n": "file-z"}, + expected: []string{ + "file-0", + "file-a", + "file-b", + "file-c", + "file-u", + "file-g", + "file-t", + "link-0", + "link-a", + "link-b", + "hlink-0", + "hlink-a", + "hlink-b", + "subdir-a", + "subdir-a/file-n", + "subdir-a/file-o", + "subdir-a/file-a", + "subdir-a/file-b", + "subdir-a/file-c", + "subdir-b", + "subdir-b/file-n", + "subdir-b/file-o", + "subdir-c", + "subdir-c/file-n", + "subdir-c/file-o", + "subdir-d", + "subdir-d/hlink-0", + "subdir-d/hlink-a", + "subdir-d/hlink-b", + "archive-a", + }, + }, + { + name: "directory", + renames: map[string]string{"subdir-a": "subdir-z"}, + expected: []string{ + "file-0", + "file-a", + "file-b", + "file-c", + "file-u", + "file-g", + "file-t", + "link-0", + "link-a", + "link-b", + "hlink-0", + "hlink-a", + "hlink-b", + "subdir-z", + "subdir-z/file-n", + "subdir-z/file-o", + "subdir-z/file-a", + "subdir-z/file-b", + "subdir-z/file-c", + "subdir-b", + "subdir-b/file-n", + "subdir-b/file-o", + "subdir-c", + "subdir-c/file-n", + "subdir-c/file-o", + "subdir-d", + "subdir-d/hlink-0", + "subdir-d/hlink-a", + "subdir-d/hlink-b", + "archive-a", + }, + }, + { + name: "file-in-directory", + renames: map[string]string{"subdir-a/file-n": "subdir-a/file-z"}, + expected: []string{ + "file-0", + "file-a", + "file-b", + "file-c", + "file-u", + "file-g", + "file-t", + "link-0", + "link-a", + "link-b", + "hlink-0", + "hlink-a", + "hlink-b", + "subdir-a", + "subdir-a/file-z", + "subdir-a/file-o", + "subdir-a/file-a", + "subdir-a/file-b", + "subdir-a/file-c", + "subdir-b", + "subdir-b/file-n", + "subdir-b/file-o", + "subdir-c", + "subdir-c/file-n", + "subdir-c/file-o", + "subdir-d", + "subdir-d/hlink-0", + "subdir-d/hlink-a", + "subdir-d/hlink-b", + "archive-a", + }, + }, + }, }, { name: "devices", @@ -263,17 +418,13 @@ func TestPutNoChroot(t *testing.T) { canChroot = couldChroot } -func TestPutChroot(t *testing.T) { - if uid != 0 { - t.Skipf("chroot() requires root privileges, skipping") - } - testPut(t) -} - func testPut(t *testing.T) { - for _, topdir := range []string{"", ".", "top"} { - for i := range testArchives { - t.Run(fmt.Sprintf("topdir=%s,archive=%s", topdir, testArchives[i].name), func(t *testing.T) { + uidMap := []idtools.IDMap{{HostID: os.Getuid(), ContainerID: 0, Size: 1}} + gidMap := []idtools.IDMap{{HostID: os.Getgid(), ContainerID: 0, Size: 1}} + + for i := range testArchives { + for _, topdir := range []string{"", ".", "top"} { + t.Run(fmt.Sprintf("archive=%s,topdir=%s", testArchives[i].name, topdir), func(t *testing.T) { if uid != 0 && testArchives[i].rootOnly { t.Skipf("test archive %q can only be tested with root privileges, skipping", testArchives[i].name) } @@ -333,7 +484,69 @@ func testPut(t *testing.T) { } }) } + + for _, renames := range testArchives[i].renames { + t.Run(fmt.Sprintf("archive=%s,rename=%s", testArchives[i].name, renames.name), func(t *testing.T) { + if uid != 0 && testArchives[i].rootOnly { + t.Skipf("test archive %q can only be tested with root privileges, skipping", testArchives[i].name) + } + + tmp, err := ioutil.TempDir("", "copier-test-") + require.Nil(t, err, "error creating temporary directory") + defer os.RemoveAll(tmp) + + archive := makeArchive(testArchives[i].headers, testArchives[i].contents) + err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, Rename: renames.renames}, archive) + require.Nil(t, err, "error extracting archive %q to directory %q", testArchives[i].name, tmp) + + var found []string + err = filepath.Walk(tmp, func(path string, info os.FileInfo, err error) error { + if info == nil || err != nil { + return err + } + rel, err := filepath.Rel(tmp, path) + if err != nil { + return err + } + if rel == "." { + return nil + } + found = append(found, rel) + return nil + }) + require.Nil(t, err, "error walking context directory for archive %q under %q", testArchives[i].name, tmp) + sort.Strings(found) + + expected := renames.expected + sort.Strings(expected) + assert.Equal(t, expected, found, "renaming did not work as expected") + }) + } } + + for _, overwrite := range []bool{false, true} { + for _, typeFlag := range []byte{tar.TypeReg, tar.TypeLink, tar.TypeSymlink, tar.TypeChar, tar.TypeBlock, tar.TypeFifo} { + t.Run(fmt.Sprintf("overwrite=%v,type=%c", overwrite, typeFlag), func(t *testing.T) { + archive := makeArchiveSlice([]tar.Header{ + {Name: "target", Typeflag: tar.TypeReg, Size: 123, Mode: 0755, ModTime: testDate}, + {Name: "test", Typeflag: tar.TypeDir, Size: 0, Mode: 0755, ModTime: testDate}, + {Name: "test", Typeflag: typeFlag, Size: 0, Mode: 0755, Linkname: "target", ModTime: testDate}, + }) + tmp, err := ioutil.TempDir("", "copier-test-") + require.Nil(t, err, "error creating temporary directory") + defer os.RemoveAll(tmp) + err = Put(tmp, tmp, PutOptions{UIDMap: uidMap, GIDMap: gidMap, NoOverwriteDirNonDir: !overwrite}, bytes.NewReader(archive)) + if overwrite { + if unwrapError(err) != syscall.EPERM { + assert.Nilf(t, err, "expected to overwrite directory with type %c: %v", typeFlag, err) + } + } else { + assert.NotNilf(t, err, "expected an error trying to overwrite directory with type %c", typeFlag) + } + }) + } + } + } func isExpectedError(err error, inSubdir bool, name string, expectedErrors []expectedError) bool { @@ -362,13 +575,6 @@ func TestStatNoChroot(t *testing.T) { canChroot = couldChroot } -func TestStatChroot(t *testing.T) { - if uid != 0 { - t.Skipf("chroot() requires root privileges, skipping") - } - testStat(t) -} - func testStat(t *testing.T) { for _, absolute := range []bool{false, true} { for _, topdir := range []string{"", ".", "top"} { @@ -456,13 +662,6 @@ func TestGetSingleNoChroot(t *testing.T) { canChroot = couldChroot } -func TestGetSingleChroot(t *testing.T) { - if uid != 0 { - t.Skipf("chroot() requires root privileges, skipping") - } - testGetSingle(t) -} - func testGetSingle(t *testing.T) { for _, absolute := range []bool{false, true} { for _, topdir := range []string{"", ".", "top"} { @@ -590,13 +789,6 @@ func TestGetMultipleNoChroot(t *testing.T) { canChroot = couldChroot } -func TestGetMultipleChroot(t *testing.T) { - if uid != 0 { - t.Skipf("chroot() requires root privileges, skipping") - } - testGetMultiple(t) -} - func testGetMultiple(t *testing.T) { type getTestArchiveCase struct { name string @@ -609,6 +801,7 @@ func testGetMultiple(t *testing.T) { stripStickyBit bool stripXattrs bool keepDirectoryNames bool + renames map[string]string } var getTestArchives = []struct { name string @@ -1001,6 +1194,43 @@ func testGetMultiple(t *testing.T) { "file-o", }, }, + { + name: "wildcard-with-rename", + pattern: "*-a", + keepDirectoryNames: false, + renames: map[string]string{"file-a": "renamed"}, + items: []string{ + "renamed", // from file-a + "link-a", + "archive-a", + "non-archive-a", + "something-a", + "file-n", // from subdir-a + "file-o", // from subdir-a + "renamed", // from subdir-a/file-a -> file-a -> renamed + "file-b", // from subdir-a + "file-c", // from subdir-a + }, + }, + { + name: "wildcard-with-rename-keep", + pattern: "*-a", + keepDirectoryNames: true, + renames: map[string]string{"subdir-a": "subdir-b"}, + items: []string{ + "file-a", + "link-a", + "archive-a", + "non-archive-a", + "something-a", + "subdir-b", + "subdir-b/file-n", + "subdir-b/file-o", + "subdir-b/file-a", + "subdir-b/file-b", + "subdir-b/file-c", + }, + }, }, }, } @@ -1035,6 +1265,7 @@ func testGetMultiple(t *testing.T) { StripStickyBit: testCase.stripStickyBit, StripXattrs: testCase.stripXattrs, KeepDirectoryNames: testCase.keepDirectoryNames, + Rename: testCase.renames, } t.Run(fmt.Sprintf("topdir=%s,archive=%s,case=%s,pattern=%s", topdir, testArchive.name, testCase.name, testCase.pattern), func(t *testing.T) { @@ -1086,13 +1317,6 @@ func TestMkdirNoChroot(t *testing.T) { canChroot = couldChroot } -func TestMkdirChroot(t *testing.T) { - if uid != 0 { - t.Skipf("chroot() requires root privileges, skipping") - } - testMkdir(t) -} - func testMkdir(t *testing.T) { type testCase struct { name string diff --git a/copier/copier_unix_test.go b/copier/copier_unix_test.go new file mode 100644 index 00000000000..a68b95964b1 --- /dev/null +++ b/copier/copier_unix_test.go @@ -0,0 +1,57 @@ +// +build !windows + +package copier + +import ( + "testing" +) + +func TestPutChroot(t *testing.T) { + if uid != 0 { + t.Skipf("chroot() requires root privileges, skipping") + } + couldChroot := canChroot + canChroot = true + testPut(t) + canChroot = couldChroot +} + +func TestStatChroot(t *testing.T) { + if uid != 0 { + t.Skipf("chroot() requires root privileges, skipping") + } + couldChroot := canChroot + canChroot = true + testStat(t) + canChroot = couldChroot +} + +func TestGetSingleChroot(t *testing.T) { + if uid != 0 { + t.Skipf("chroot() requires root privileges, skipping") + } + couldChroot := canChroot + canChroot = true + testGetSingle(t) + canChroot = couldChroot +} + +func TestGetMultipleChroot(t *testing.T) { + if uid != 0 { + t.Skipf("chroot() requires root privileges, skipping") + } + couldChroot := canChroot + canChroot = true + testGetMultiple(t) + canChroot = couldChroot +} + +func TestMkdirChroot(t *testing.T) { + if uid != 0 { + t.Skipf("chroot() requires root privileges, skipping") + } + couldChroot := canChroot + canChroot = true + testMkdir(t) + canChroot = couldChroot +} diff --git a/copier/syscall_unix.go b/copier/syscall_unix.go index 55f2f368a93..2c2806d0a30 100644 --- a/copier/syscall_unix.go +++ b/copier/syscall_unix.go @@ -10,7 +10,7 @@ import ( "golang.org/x/sys/unix" ) -var canChroot = true +var canChroot = os.Getuid() == 0 func chroot(root string) (bool, error) { if canChroot { diff --git a/tests/conformance/conformance_test.go b/tests/conformance/conformance_test.go index 5238c8ec08f..266b301dd94 100644 --- a/tests/conformance/conformance_test.go +++ b/tests/conformance/conformance_test.go @@ -928,8 +928,8 @@ func applyLayerToFSTree(t *testing.T, layer *Layer, root *FSEntry) { delete(dirEntry.Children, strings.TrimPrefix(base, ".wh.")) continue } - // if the item already exists, just make sure we're not trying - // to turn a directory into a non-directory or vice-versa + // if the item already exists, make sure we don't get confused + // by replacing a directory with a non-directory or vice-versa if child, ok := dirEntry.Children[base]; ok { if child.Children != nil { // it's a directory @@ -939,8 +939,7 @@ func applyLayerToFSTree(t *testing.T, layer *Layer, root *FSEntry) { child.FSHeader = entry continue } - // oh no - require.Failf(t, "layer diff error", "layer diff %q includes entry for directory %q, but it already exists and is not a directory", layer.UncompressedDigest, entry.Name) + // nope, a directory no longer } else { // it's not a directory if entry.Typeflag != tar.TypeDir { @@ -949,11 +948,10 @@ func applyLayerToFSTree(t *testing.T, layer *Layer, root *FSEntry) { dirEntry.Children[base].FSHeader = entry continue } - // oh no - require.Failf(t, "layer diff error", "layer diff %q includes entry for non-directory %q, but it already exists and is a directory", layer.UncompressedDigest, entry.Name) + // well, it's a directory now } } - // the item doesn't already exist, so we need to add it + // the item doesn't already exist, or it needs to be replaced, so we need to add it var children map[string]*FSEntry if entry.Typeflag == tar.TypeDir { // only directory entries can hold items @@ -2647,6 +2645,21 @@ var internalTestCases = []testCase{ fsSkip: []string{"(dir):sub:mtime"}, }, + { + name: "add-dir-not-dir", + contextDir: "add/dir-not-dir", + }, + + { + name: "add-not-dir-dir", + contextDir: "add/not-dir-dir", + }, + + { + name: "add-populated-dir-not-dir", + contextDir: "add/populated-dir-not-dir", + }, + { name: "dockerignore-allowlist-subdir-nofile-dir", contextDir: "dockerignore/allowlist/subdir-nofile", diff --git a/tests/conformance/testdata/add/dir-not-dir/Dockerfile b/tests/conformance/testdata/add/dir-not-dir/Dockerfile new file mode 100644 index 00000000000..d64852ff030 --- /dev/null +++ b/tests/conformance/testdata/add/dir-not-dir/Dockerfile @@ -0,0 +1,3 @@ +FROM busybox +RUN mkdir fileone +ADD test.tar / diff --git a/tests/conformance/testdata/add/dir-not-dir/test.tar b/tests/conformance/testdata/add/dir-not-dir/test.tar new file mode 100644 index 00000000000..a453463fbdc Binary files /dev/null and b/tests/conformance/testdata/add/dir-not-dir/test.tar differ diff --git a/tests/conformance/testdata/add/not-dir-dir/Dockerfile b/tests/conformance/testdata/add/not-dir-dir/Dockerfile new file mode 100644 index 00000000000..5cf8016fca5 --- /dev/null +++ b/tests/conformance/testdata/add/not-dir-dir/Dockerfile @@ -0,0 +1,4 @@ +FROM busybox +RUN touch /new-directory +ADD test.tar / +RUN ls -ld /new-directory diff --git a/tests/conformance/testdata/add/not-dir-dir/test.tar b/tests/conformance/testdata/add/not-dir-dir/test.tar new file mode 100644 index 00000000000..80c397220f4 Binary files /dev/null and b/tests/conformance/testdata/add/not-dir-dir/test.tar differ diff --git a/tests/conformance/testdata/add/populated-dir-not-dir/Dockerfile b/tests/conformance/testdata/add/populated-dir-not-dir/Dockerfile new file mode 100644 index 00000000000..50f0f3ff097 --- /dev/null +++ b/tests/conformance/testdata/add/populated-dir-not-dir/Dockerfile @@ -0,0 +1,5 @@ +FROM busybox +RUN mkdir dirone +RUN echo one > dirone/onefile.txt +RUN echo two > dirone/twofile.txt +ADD test.tar / diff --git a/tests/conformance/testdata/add/populated-dir-not-dir/test.tar b/tests/conformance/testdata/add/populated-dir-not-dir/test.tar new file mode 100644 index 00000000000..4cd067cedaa Binary files /dev/null and b/tests/conformance/testdata/add/populated-dir-not-dir/test.tar differ