From b4b70d040aae2d554308be970fd36859d0bf7fbf Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 6 Mar 2020 13:07:41 -0800 Subject: [PATCH 1/6] wip --- pkg/commands/copy.go | 13 ++-- pkg/commands/copy_test.go | 136 ++++++++++++++++++++++++++++++++++++++ pkg/filesystem/resolve.go | 4 +- pkg/util/command_util.go | 8 +-- pkg/util/fs_util.go | 13 ++-- testutil/util.go | 6 ++ 6 files changed, 162 insertions(+), 18 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index c4102ef812..1ced9e40e0 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -82,14 +82,13 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu return err } - // If the destination dir is a symlink we need to resolve the path and use - // that instead of the symlink path - destPath, err = resolveIfSymlink(destPath) - if err != nil { - return err - } - if fi.IsDir() { + // If the destination dir is a symlink we need to resolve the path and use + // that instead of the symlink path + destPath, err = resolveIfSymlink(destPath) + if err != nil { + return err + } copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { return err diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index c69d007db5..09bec8e79b 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -442,3 +442,139 @@ func TestGetUserGroup(t *testing.T) { }) } } + +func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { + setupDirs := func(t *testing.T) (string, string) { + testDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + + targetDir := filepath.Join(testDir, "bar") + + if err := os.MkdirAll(targetDir, 0777); err != nil { + t.Fatal(err) + } + + targetPath := filepath.Join(targetDir, "bam.txt") + + if err := ioutil.WriteFile(targetPath, []byte("woof"), 0777); err != nil { + t.Fatal(err) + } + + return testDir, filepath.Base(targetDir) + } + + t.Run("copy dir to another dir", func(t *testing.T) { + testDir, targetDir := setupDirs(t) + defer os.RemoveAll(testDir) + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{targetDir, "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + }) + + t.Run("copy file to a dir", func(t *testing.T) { + testDir, targetDir := setupDirs(t) + defer os.RemoveAll(testDir) + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{filepath.Join(targetDir, "bam.txt"), "dest/"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + }) + + t.Run("copy file to a filepath", func(t *testing.T) { + testDir, targetDir := setupDirs(t) + defer os.RemoveAll(testDir) + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{filepath.Join(targetDir, "bam.txt"), "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + }) + t.Run("copy file to a dir without trailing /", func(t *testing.T) { + testDir, targetDir := setupDirs(t) + defer os.RemoveAll(testDir) + + destDir := filepath.Join(testDir, "dest") + if err := os.MkdirAll(destDir, 0777); err != nil { + t.Fatal(err) + } + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{filepath.Join(targetDir, "bam.txt"), "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + }) + t.Run("copy symlink file to a dir", func(t *testing.T) { + testDir, targetDir := setupDirs(t) + defer os.RemoveAll(testDir) + + another := filepath.Join(testDir, "another") + if err := os.MkdirAll(another, 0777); err != nil { + t.Fatal(err) + } + symlink := filepath.Join(another, "sym.link") + os.Symlink(filepath.Join(targetDir, "bam.txt"), symlink) + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{symlink, "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + }) +} \ No newline at end of file diff --git a/pkg/filesystem/resolve.go b/pkg/filesystem/resolve.go index 63beff7484..920473eb58 100644 --- a/pkg/filesystem/resolve.go +++ b/pkg/filesystem/resolve.go @@ -17,6 +17,7 @@ limitations under the License. package filesystem import ( + "fmt" "os" "path/filepath" @@ -126,6 +127,7 @@ func resolveSymlinkAncestor(path string) (string, error) { return "", errors.New("dest path must be abs") } + fmt.Println(path) last := "" newPath := filepath.Clean(path) @@ -133,7 +135,7 @@ loop: for newPath != "/" { fi, err := os.Lstat(newPath) if err != nil { - return "", errors.Wrap(err, "failed to lstat") + return "", errors.Wrap(err, "resolvePaths: failed to lstat") } if util.IsSymlink(fi) { diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 23f035ec45..96f8993981 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -171,14 +171,14 @@ func DestinationFilepath(src, dest, cwd string) (string, error) { _, srcFileName := filepath.Split(src) newDest := dest - if IsDestDir(newDest) { - newDest = filepath.Join(newDest, srcFileName) - } - if !filepath.IsAbs(newDest) { newDest = filepath.Join(cwd, newDest) } + if IsDestDir(newDest) { + newDest = filepath.Join(newDest, srcFileName) + } + if len(srcFileName) <= 0 && !strings.HasSuffix(newDest, "/") { newDest += "/" } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 40c705dd83..770eef2526 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -505,15 +505,16 @@ func FilepathExists(path string) bool { 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 { - return err + return errors.Wrap(err, "creating parent dir") } + dest, err := os.Create(path) if err != nil { - return err + return errors.Wrap(err, "creating file") } defer dest.Close() if _, err := io.Copy(dest, reader); err != nil { - return err + return errors.Wrap(err, "copying file") } return setFilePermissions(path, perm, int(uid), int(gid)) } @@ -614,9 +615,9 @@ func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, er logrus.Debugf("%s found in .dockerignore, ignoring", src) return true, nil } - link, err := filepath.EvalSymlinks(src) + link, err := os.Readlink(src) if err != nil { - return false, err + logrus.Debugf("could not evaluate %s, probably a dead link", src) } if FilepathExists(dest) { if err := os.RemoveAll(dest); err != nil { @@ -626,7 +627,7 @@ func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, er if err := createParentDirectory(dest); err != nil { return false, err } - return CopyFile(link, dest, buildcontext, uid, gid) + return true, os.Symlink(link, dest) } // CopyFile copies the file at src to dest diff --git a/testutil/util.go b/testutil/util.go index a1e72dd08c..c983b53e41 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -68,6 +68,12 @@ func CheckError(t *testing.T, shouldErr bool, err error) { } } +func CheckNoError(t *testing.T, err error) { + if err != nil { + t.Error(err) + } +} + func checkErr(shouldErr bool, err error) error { if err == nil && shouldErr { return fmt.Errorf("Expected error, but returned none") From 6c14d202a30b8d7dda6215a0351e2c803a197425 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 6 Mar 2020 16:27:15 -0800 Subject: [PATCH 2/6] better error wrapping and add more tests for copy --- pkg/commands/add.go | 9 +- pkg/commands/copy.go | 34 +++-- pkg/commands/copy_test.go | 303 ++++++++++++++++++++++++++++++++------ pkg/executor/build.go | 2 +- pkg/filesystem/resolve.go | 10 +- pkg/snapshot/snapshot.go | 1 - pkg/util/command_util.go | 20 ++- pkg/util/fs_util.go | 18 ++- pkg/util/fs_util_test.go | 2 +- 9 files changed, 316 insertions(+), 83 deletions(-) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 3ade7ea69f..57b6a08f2f 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -21,6 +21,7 @@ import ( v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" + "github.com/pkg/errors" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" @@ -66,18 +67,18 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui } logrus.Infof("Adding remote URL %s to %s", src, urlDest) if err := util.DownloadFileToDest(src, urlDest); err != nil { - return err + return errors.Wrap(err, "downloading remote source file") } a.snapshotFiles = append(a.snapshotFiles, urlDest) } else if util.IsFileLocalTarArchive(fullPath) { tarDest, err := util.DestinationFilepath("", dest, config.WorkingDir) if err != nil { - return err + return errors.Wrap(err, "determining dest for tar") } logrus.Infof("Unpacking local tar archive %s to %s", src, tarDest) extractedFiles, err := util.UnpackLocalTarArchive(fullPath, tarDest) if err != nil { - return err + return errors.Wrap(err, "unpacking local tar") } logrus.Debugf("Added %v from local tar archive %s", extractedFiles, src) a.snapshotFiles = append(a.snapshotFiles, extractedFiles...) @@ -98,7 +99,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui } if err := copyCmd.ExecuteCommand(config, buildArgs); err != nil { - return err + return errors.Wrap(err, "executing copy command") } a.snapshotFiles = append(a.snapshotFiles, copyCmd.snapshotFiles...) return nil diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 1ced9e40e0..bb97bba32c 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -54,18 +54,23 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs) if err != nil { - return err + return errors.Wrap(err, "getting user group from chowm") } srcs, dest, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.buildcontext, replacementEnvs) if err != nil { - return err + return errors.Wrap(err, "resolving src") } // For each source, iterate through and copy it over for _, src := range srcs { fullPath := filepath.Join(c.buildcontext, src) - fi, err := os.Lstat(fullPath) + srcPath, err := resolveIfSymlink(fullPath) + if err != nil { + return errors.Wrap(err, "resolving src symlink") + } + + fi, err := os.Lstat(srcPath) if err != nil { return errors.Wrap(err, "could not copy source") } @@ -79,26 +84,27 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu destPath, err := util.DestinationFilepath(fullPath, dest, cwd) if err != nil { - return err + return errors.Wrap(err, "find destination path") + } + + // If the destination dir is a symlink we need to resolve the path and use + // that instead of the symlink path + destPath, err = resolveIfSymlink(destPath) + if err != nil { + return errors.Wrap(err, "resolving dest symlink") } if fi.IsDir() { - // If the destination dir is a symlink we need to resolve the path and use - // that instead of the symlink path - destPath, err = resolveIfSymlink(destPath) - if err != nil { - return err - } copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { - return err + return errors.Wrap(err, "copying dir") } c.snapshotFiles = append(c.snapshotFiles, copiedFiles...) } else if util.IsSymlink(fi) { // If file is a symlink, we want to copy the target file to destPath - exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid) + exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid, false) if err != nil { - return err + return errors.Wrap(err, "copying symlink") } if exclude { continue @@ -108,7 +114,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu // ... Else, we want to copy over a file exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { - return err + return errors.Wrap(err, "copying file") } if exclude { continue diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 09bec8e79b..648249a23d 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -444,54 +444,75 @@ func TestGetUserGroup(t *testing.T) { } func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { - setupDirs := func(t *testing.T) (string, string) { - testDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatal(err) - } - - targetDir := filepath.Join(testDir, "bar") + setupDirs := func(t *testing.T) (string, string) { + testDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } - if err := os.MkdirAll(targetDir, 0777); err != nil { - t.Fatal(err) - } + dir := filepath.Join(testDir, "bar") - targetPath := filepath.Join(targetDir, "bam.txt") + if err := os.MkdirAll(dir, 0777); err != nil { + t.Fatal(err) + } - if err := ioutil.WriteFile(targetPath, []byte("woof"), 0777); err != nil { - t.Fatal(err) - } + file := filepath.Join(dir, "bam.txt") - return testDir, filepath.Base(targetDir) + if err := ioutil.WriteFile(file, []byte("meow"), 0777); err != nil { + t.Fatal(err) } + targetPath := filepath.Join(dir, "dam.txt") + if err := ioutil.WriteFile(targetPath, []byte("woof"), 0777); err != nil { + t.Fatal(err) + } + os.Symlink(targetPath, filepath.Join(dir, "sym.link")) - t.Run("copy dir to another dir", func(t *testing.T) { - testDir, targetDir := setupDirs(t) - defer os.RemoveAll(testDir) + return testDir, filepath.Base(dir) + } - cmd := CopyCommand{ - cmd: &instructions.CopyCommand{ - SourcesAndDest: []string{targetDir, "dest"}, - }, - buildcontext: testDir, - } + t.Run("copy dir to another dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) + if err != nil { + t.Fatal(err) + } - cfg := &v1.Config{ - Cmd: nil, - Env: []string{}, - WorkingDir: testDir, - } + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{srcDir, "dest"}, + }, + buildcontext: testDir, + } - err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) - testutil.CheckNoError(t, err) - }) + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err = cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + if err != nil { + t.Fatal(err) + } + testutil.CheckNoError(t, err) + // Check if "dest" dir exists with contents of srcDir + actual, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + for i, f := range actual { + testutil.CheckDeepEqual(t, expected[i].Name(), f.Name()) + testutil.CheckDeepEqual(t, expected[i].Mode(), f.Mode()) + } + }) - t.Run("copy file to a dir", func(t *testing.T) { - testDir, targetDir := setupDirs(t) + t.Run("copy file to a dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) defer os.RemoveAll(testDir) cmd := CopyCommand{ cmd: &instructions.CopyCommand{ - SourcesAndDest: []string{filepath.Join(targetDir, "bam.txt"), "dest/"}, + SourcesAndDest: []string{filepath.Join(srcDir, "bam.txt"), "dest/"}, }, buildcontext: testDir, } @@ -504,14 +525,21 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) testutil.CheckNoError(t, err) + // Check if "dest" dir exists with file bam.txt + files, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, 1, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "bam.txt") }) t.Run("copy file to a filepath", func(t *testing.T) { - testDir, targetDir := setupDirs(t) + testDir, srcDir := setupDirs(t) defer os.RemoveAll(testDir) cmd := CopyCommand{ cmd: &instructions.CopyCommand{ - SourcesAndDest: []string{filepath.Join(targetDir, "bam.txt"), "dest"}, + SourcesAndDest: []string{filepath.Join(srcDir, "bam.txt"), "dest"}, }, buildcontext: testDir, } @@ -524,9 +552,13 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) testutil.CheckNoError(t, err) + // Check if bam.txt is copied to dest file + if _, err := os.Lstat(filepath.Join(testDir, "dest")); err != nil { + t.Fatal(err) + } }) t.Run("copy file to a dir without trailing /", func(t *testing.T) { - testDir, targetDir := setupDirs(t) + testDir, srcDir := setupDirs(t) defer os.RemoveAll(testDir) destDir := filepath.Join(testDir, "dest") @@ -536,7 +568,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd := CopyCommand{ cmd: &instructions.CopyCommand{ - SourcesAndDest: []string{filepath.Join(targetDir, "bam.txt"), "dest"}, + SourcesAndDest: []string{filepath.Join(srcDir, "bam.txt"), "dest"}, }, buildcontext: testDir, } @@ -549,21 +581,185 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) testutil.CheckNoError(t, err) + // Check if "dest" dir exists with file bam.txt + files, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, 1, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "bam.txt") + }) t.Run("copy symlink file to a dir", func(t *testing.T) { - testDir, targetDir := setupDirs(t) + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{filepath.Join(srcDir, "sym.link"), "dest/"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + // Check if "dest" dir exists with link sym.link + files, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + // sym.link should be present as a Regular file with contents of target "woof" + testutil.CheckDeepEqual(t, 1, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "sym.link") + testutil.CheckDeepEqual(t, false, files[0].Mode()&os.ModeSymlink != 0) + c, err := ioutil.ReadFile(filepath.Join(testDir, "dest", "sym.link")) + testutil.CheckDeepEqual(t, "woof", string(c)) + }) + + t.Run("copy src symlink dir to a dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) + + another := filepath.Join(testDir, "another") + os.Symlink(filepath.Join(testDir, srcDir), another) + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{"another", "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err = cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + // Check if "dest" dir exists with contents of srcDir + actual, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + for i, f := range actual { + testutil.CheckDeepEqual(t, expected[i].Name(), f.Name()) + testutil.CheckDeepEqual(t, expected[i].Mode(), f.Mode()) + } + }) + t.Run("copy dir with a symlink to a file outside of current src dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) + if err != nil { + t.Fatal(err) + } + + anotherSrc := filepath.Join(testDir, "anotherSrc") + if err := os.MkdirAll(anotherSrc, 0777); err != nil { + t.Fatal(err) + } + targetPath := filepath.Join(anotherSrc, "target.txt") + if err := ioutil.WriteFile(targetPath, []byte("woof"), 0777); err != nil { + t.Fatal(err) + } + if err := os.Symlink(targetPath, filepath.Join(testDir, srcDir, "zSym.link")); err != nil { + t.Fatal(err) + } + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{srcDir, "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err = cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + // Check if "dest" dir exists contests of srcDir and an extra zSym.link created + // in this test + actual, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, 4, len(actual)) + for i, f := range expected { + testutil.CheckDeepEqual(t, f.Name(), actual[i].Name()) + testutil.CheckDeepEqual(t, f.Mode(), actual[i].Mode()) + } + linkName, err := os.Readlink(filepath.Join(testDir, "dest", "zSym.link")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, linkName, targetPath) + }) + t.Run("copy src symlink dir to a dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) defer os.RemoveAll(testDir) + expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) another := filepath.Join(testDir, "another") - if err := os.MkdirAll(another, 0777); err != nil { + os.Symlink(filepath.Join(testDir, srcDir), another) + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{"another", "dest"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err = cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + // Check if "dest" dir exists with bam.txt and "dest" dir is a symlink + actual, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + for i, f := range actual { + testutil.CheckDeepEqual(t, expected[i].Name(), f.Name()) + testutil.CheckDeepEqual(t, expected[i].Mode(), f.Mode()) + } + }) + t.Run("copy file to a dest which is a symlink", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + //defer os.RemoveAll(testDir) + expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) + if err != nil { + t.Fatal(err) + } + + dest := filepath.Join(testDir, "dest") + if err := os.MkdirAll(dest, 0777); err != nil { + t.Fatal(err) + } + linkedDest := filepath.Join(testDir, "linkDest") + if err := os.Symlink(dest, linkedDest); err != nil { t.Fatal(err) } - symlink := filepath.Join(another, "sym.link") - os.Symlink(filepath.Join(targetDir, "bam.txt"), symlink) cmd := CopyCommand{ cmd: &instructions.CopyCommand{ - SourcesAndDest: []string{symlink, "dest"}, + SourcesAndDest: []string{srcDir, linkedDest}, }, buildcontext: testDir, } @@ -574,7 +770,22 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { WorkingDir: testDir, } - err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + err = cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) testutil.CheckNoError(t, err) + // Check if "linkdest" dir exists with contents of srcDir + actual, err := ioutil.ReadDir(filepath.Join(testDir, "linkDest")) + if err != nil { + t.Fatal(err) + } + for i, f := range expected { + testutil.CheckDeepEqual(t, f.Name(), actual[i].Name()) + testutil.CheckDeepEqual(t, f.Mode(), actual[i].Mode()) + } + // Check if linkDest -> dest + linkName, err := os.Readlink(filepath.Join(testDir, "linkDest")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, linkName, dest) }) -} \ No newline at end of file +} diff --git a/pkg/executor/build.go b/pkg/executor/build.go index dd7021a770..cf7e8bfbfb 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -339,7 +339,7 @@ func (s *stageBuilder) build() error { v := command.(commands.Cached) layer := v.Layer() if err := s.saveLayerToImage(layer, command.String()); err != nil { - return err + return errors.Wrap(err, "failed to save layer") } } else { tarPath, err := s.takeSnapshot(files) diff --git a/pkg/filesystem/resolve.go b/pkg/filesystem/resolve.go index 920473eb58..a779e6e104 100644 --- a/pkg/filesystem/resolve.go +++ b/pkg/filesystem/resolve.go @@ -17,7 +17,6 @@ limitations under the License. package filesystem import ( - "fmt" "os" "path/filepath" @@ -48,7 +47,7 @@ func ResolvePaths(paths []string, wl []util.WhitelistEntry) (pathsToAdd []string link, e := resolveSymlinkAncestor(f) if e != nil { - err = e + err = errors.Wrap(e, "resolving symlinks") return } @@ -65,9 +64,9 @@ func ResolvePaths(paths []string, wl []util.WhitelistEntry) (pathsToAdd []string // If the path is a symlink we need to also consider the target of that // link - evaled, err = filepath.EvalSymlinks(f) - if err != nil { - if !os.IsNotExist(err) { + evaled, e = filepath.EvalSymlinks(f) + if e != nil { + if !os.IsNotExist(e) { logrus.Errorf("couldn't eval %s with link %s", f, link) return } @@ -127,7 +126,6 @@ func resolveSymlinkAncestor(path string) (string, error) { return "", errors.New("dest path must be abs") } - fmt.Println(path) last := "" newPath := filepath.Clean(path) diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 143f72ba84..e64e28dd38 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -26,7 +26,6 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/filesystem" "github.com/GoogleContainerTools/kaniko/pkg/timing" - "github.com/karrick/godirwalk" "github.com/GoogleContainerTools/kaniko/pkg/constants" diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 96f8993981..2880c167d7 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -35,6 +35,10 @@ import ( "github.com/sirupsen/logrus" ) +const ( + pathSeparator = "/" +) + // ResolveEnvironmentReplacementList resolves a list of values by calling resolveEnvironmentReplacement func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ([]string, error) { var resolvedValues []string @@ -114,13 +118,14 @@ func ResolveSources(srcs []string, root string) ([]string, error) { logrus.Infof("Resolving srcs %v...", srcs) files, err := RelativeFiles("", root) if err != nil { - return nil, err + return nil, errors.Wrap(err, "resolving sources") } resolved, err := matchSources(srcs, files) if err != nil { - return nil, err + return nil, errors.Wrap(err, "matching sources") } logrus.Debugf("Resolved sources to %v", resolved) + fmt.Println("end of resolve sources") return resolved, nil } @@ -154,7 +159,7 @@ func IsDestDir(path string) bool { fileInfo, err := os.Stat(path) if err != nil { // fall back to string-based determination - return strings.HasSuffix(path, "/") || path == "." + return strings.HasSuffix(path, pathSeparator) || path == "." } // if it's a real path, check the fs response return fileInfo.IsDir() @@ -173,14 +178,17 @@ func DestinationFilepath(src, dest, cwd string) (string, error) { if !filepath.IsAbs(newDest) { newDest = filepath.Join(cwd, newDest) + // join call clean on all results. + if strings.HasSuffix(dest, pathSeparator) || strings.HasSuffix(dest, ".") { + newDest += pathSeparator + } } - if IsDestDir(newDest) { newDest = filepath.Join(newDest, srcFileName) } - if len(srcFileName) <= 0 && !strings.HasSuffix(newDest, "/") { - newDest += "/" + if len(srcFileName) <= 0 && !strings.HasSuffix(newDest, pathSeparator) { + newDest += pathSeparator } return newDest, nil diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 770eef2526..b6ded4ebaa 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -571,7 +571,7 @@ func DetermineTargetFileOwnership(fi os.FileInfo, uid, gid int64) (int64, int64) func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { files, err := RelativeFiles("", src) if err != nil { - return nil, err + return nil, errors.Wrap(err, "copying dir") } var copiedFiles []string for _, file := range files { @@ -595,7 +595,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } } else if IsSymlink(fi) { // If file is a symlink, we want to create the same relative symlink - if _, err := CopySymlink(fullPath, destPath, buildcontext, uid, gid); err != nil { + if _, err := CopySymlink(fullPath, destPath, buildcontext, uid, gid, true); err != nil { return nil, err } } else { @@ -610,7 +610,14 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } // CopySymlink copies the symlink at src to dest -func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, error) { +// if asSymlink is true, then the file is copied as symlink. This is done for a symlink file in the directory is copied +// COPY src/ dest/ +// where src/ contains a symlink file. +// if asSymlink is false, then target file of the symlink is copied as a regular file. +// This done when copying a single file via +// COPY sym.link dest +// where sym.link is a link to target file. +func CopySymlink(src, dest, buildcontext string, uid int64, gid int64, asSymlink bool) (bool, error) { if ExcludeFile(src, buildcontext) { logrus.Debugf("%s found in .dockerignore, ignoring", src) return true, nil @@ -627,7 +634,10 @@ func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, er if err := createParentDirectory(dest); err != nil { return false, err } - return true, os.Symlink(link, dest) + if asSymlink { + return true, os.Symlink(link, dest) + } + return CopyFile(src, dest, buildcontext, uid, gid) } // CopyFile copies the file at src to dest diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index b8eb6329e9..fd401b770c 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -835,7 +835,7 @@ func TestCopySymlink(t *testing.T) { if err := os.Symlink(tc.linkTarget, link); err != nil { t.Fatal(err) } - if _, err := CopySymlink(link, dest, "", DoNotChangeUID, DoNotChangeGID); err != nil { + if _, err := CopySymlink(link, dest, "", DoNotChangeUID, DoNotChangeGID, true); err != nil { t.Fatal(err) } if _, err := os.Lstat(dest); err != nil { From 8f87267002ed63baa84b3e383c6d1e30cbfc1b88 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 6 Mar 2020 21:37:00 -0800 Subject: [PATCH 3/6] fix lint error --- pkg/commands/copy_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 648249a23d..5d665d60ad 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -619,6 +619,9 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { testutil.CheckDeepEqual(t, files[0].Name(), "sym.link") testutil.CheckDeepEqual(t, false, files[0].Mode()&os.ModeSymlink != 0) c, err := ioutil.ReadFile(filepath.Join(testDir, "dest", "sym.link")) + if err != nil { + t.Fatal(err) + } testutil.CheckDeepEqual(t, "woof", string(c)) }) @@ -626,6 +629,9 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { testDir, srcDir := setupDirs(t) defer os.RemoveAll(testDir) expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) + if err != nil { + t.Fatal(err) + } another := filepath.Join(testDir, "another") os.Symlink(filepath.Join(testDir, srcDir), another) @@ -711,6 +717,9 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { testDir, srcDir := setupDirs(t) defer os.RemoveAll(testDir) expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) + if err != nil { + t.Fatal(err) + } another := filepath.Join(testDir, "another") os.Symlink(filepath.Join(testDir, srcDir), another) From c523c691eb2b8a17c8e062c2f1530eb0f807de56 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Fri, 6 Mar 2020 23:48:31 -0800 Subject: [PATCH 4/6] revert back to old 0.17.1 behavior --- pkg/commands/copy.go | 11 ++++---- pkg/commands/copy_test.go | 59 ++++++++++++++++++++++++++++++++++----- pkg/filesystem/resolve.go | 3 +- pkg/util/.editorconfig | 3 ++ pkg/util/fs_util.go | 28 +++++++++++-------- pkg/util/fs_util_test.go | 2 +- 6 files changed, 78 insertions(+), 28 deletions(-) create mode 100644 pkg/util/.editorconfig diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index bb97bba32c..6f1834def2 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -65,12 +65,8 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu // For each source, iterate through and copy it over for _, src := range srcs { fullPath := filepath.Join(c.buildcontext, src) - srcPath, err := resolveIfSymlink(fullPath) - if err != nil { - return errors.Wrap(err, "resolving src symlink") - } - fi, err := os.Lstat(srcPath) + fi, err := os.Lstat(fullPath) if err != nil { return errors.Wrap(err, "could not copy source") } @@ -102,7 +98,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.snapshotFiles = append(c.snapshotFiles, copiedFiles...) } else if util.IsSymlink(fi) { // If file is a symlink, we want to copy the target file to destPath - exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid, false) + exclude, target, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid) if err != nil { return errors.Wrap(err, "copying symlink") } @@ -110,6 +106,9 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu continue } c.snapshotFiles = append(c.snapshotFiles, destPath) + if target != "" { + c.snapshotFiles = append(c.snapshotFiles, target) + } } else { // ... Else, we want to copy over a file exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext, uid, gid) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 5d665d60ad..81fe58a604 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -465,8 +465,9 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { if err := ioutil.WriteFile(targetPath, []byte("woof"), 0777); err != nil { t.Fatal(err) } - os.Symlink(targetPath, filepath.Join(dir, "sym.link")) - + if err := os.Symlink("dam.txt", filepath.Join(dir, "sym.link")); err != nil { + t.Fatal(err) + } return testDir, filepath.Base(dir) } @@ -614,15 +615,59 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { if err != nil { t.Fatal(err) } - // sym.link should be present as a Regular file with contents of target "woof" + // bam.txt and sym.link should be present + testutil.CheckDeepEqual(t, 2, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "dam.txt") + testutil.CheckDeepEqual(t, files[1].Name(), "sym.link") + testutil.CheckDeepEqual(t, true, files[1].Mode()&os.ModeSymlink != 0) + linkName, err := os.Readlink(filepath.Join(testDir, "dest", "sym.link")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, linkName, "dam.txt") + }) + t.Run("copy deadlink symlink file to a dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + doesNotExists := filepath.Join(testDir, "dead.txt") + if err := ioutil.WriteFile(doesNotExists, []byte("remove me"), 0777); err != nil { + t.Fatal(err) + } + if err := os.Symlink("../dead.txt", filepath.Join(testDir, srcDir, "dead.link")); err != nil { + t.Fatal(err) + } + if err := os.Remove(doesNotExists); err != nil { + t.Fatal(err) + } + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{filepath.Join(srcDir, "dead.link"), "dest/"}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + // Check if "dest" dir exists with link dead.link + files, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } testutil.CheckDeepEqual(t, 1, len(files)) - testutil.CheckDeepEqual(t, files[0].Name(), "sym.link") - testutil.CheckDeepEqual(t, false, files[0].Mode()&os.ModeSymlink != 0) - c, err := ioutil.ReadFile(filepath.Join(testDir, "dest", "sym.link")) + testutil.CheckDeepEqual(t, files[0].Name(), "dead.link") + testutil.CheckDeepEqual(t, true, files[0].Mode()&os.ModeSymlink != 0) + linkName, err := os.Readlink(filepath.Join(testDir, "dest", "dead.link")) if err != nil { t.Fatal(err) } - testutil.CheckDeepEqual(t, "woof", string(c)) + testutil.CheckDeepEqual(t, linkName, "../dead.txt") }) t.Run("copy src symlink dir to a dir", func(t *testing.T) { diff --git a/pkg/filesystem/resolve.go b/pkg/filesystem/resolve.go index a779e6e104..586e220953 100644 --- a/pkg/filesystem/resolve.go +++ b/pkg/filesystem/resolve.go @@ -47,8 +47,7 @@ func ResolvePaths(paths []string, wl []util.WhitelistEntry) (pathsToAdd []string link, e := resolveSymlinkAncestor(f) if e != nil { - err = errors.Wrap(e, "resolving symlinks") - return + continue } if f != link { diff --git a/pkg/util/.editorconfig b/pkg/util/.editorconfig new file mode 100644 index 0000000000..c5319e7de1 --- /dev/null +++ b/pkg/util/.editorconfig @@ -0,0 +1,3 @@ +root = true + +[*] diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index b6ded4ebaa..fb275874b8 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -595,7 +595,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } } else if IsSymlink(fi) { // If file is a symlink, we want to create the same relative symlink - if _, err := CopySymlink(fullPath, destPath, buildcontext, uid, gid, true); err != nil { + if _, _, err := CopySymlink(fullPath, destPath, buildcontext, uid, gid); err != nil { return nil, err } } else { @@ -617,27 +617,31 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { // This done when copying a single file via // COPY sym.link dest // where sym.link is a link to target file. -func CopySymlink(src, dest, buildcontext string, uid int64, gid int64, asSymlink bool) (bool, error) { +func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, string, error) { if ExcludeFile(src, buildcontext) { logrus.Debugf("%s found in .dockerignore, ignoring", src) - return true, nil - } - link, err := os.Readlink(src) - if err != nil { - logrus.Debugf("could not evaluate %s, probably a dead link", src) + return true, "", nil } if FilepathExists(dest) { if err := os.RemoveAll(dest); err != nil { - return false, err + return false, "", err } } if err := createParentDirectory(dest); err != nil { - return false, err + return false, "", err } - if asSymlink { - return true, os.Symlink(link, dest) + target := "" + link, err := os.Readlink(src) + if err != nil { + logrus.Debugf("could not read link for %s", src) + } else { + cpDest := filepath.Clean(filepath.Join(filepath.Dir(dest), link)) + srcLink := filepath.Clean(filepath.Join(filepath.Dir(src), link)) + if t, err := CopyFile(srcLink, cpDest, buildcontext, uid, gid); !t && err != nil { + target = link + } } - return CopyFile(src, dest, buildcontext, uid, gid) + return false, target, os.Symlink(link, dest) } // CopyFile copies the file at src to dest diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index fd401b770c..fdda2018cf 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -835,7 +835,7 @@ func TestCopySymlink(t *testing.T) { if err := os.Symlink(tc.linkTarget, link); err != nil { t.Fatal(err) } - if _, err := CopySymlink(link, dest, "", DoNotChangeUID, DoNotChangeGID, true); err != nil { + if _, _, err := CopySymlink(link, dest, "", DoNotChangeUID, DoNotChangeGID); err != nil { t.Fatal(err) } if _, err := os.Lstat(dest); err != nil { From 2181c5e6f5d198856123f7f5c38aadbe16bbf132 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Sat, 7 Mar 2020 11:28:16 -0800 Subject: [PATCH 5/6] create correct link across Multistage builds --- .../dockerfiles/Dockerfile_test_copy_symlink | 16 ++++++---------- pkg/filesystem/resolve.go | 1 + pkg/util/fs_util.go | 16 ++++------------ 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_copy_symlink b/integration/dockerfiles/Dockerfile_test_copy_symlink index 82f5774c68..2f721ae04c 100644 --- a/integration/dockerfiles/Dockerfile_test_copy_symlink +++ b/integration/dockerfiles/Dockerfile_test_copy_symlink @@ -1,14 +1,10 @@ FROM busybox as t -RUN echo "hello" > /tmp/target -RUN ln -s /tmp/target /tmp/link - -## Relative link -RUN cd tmp && ln -s target relative_link - +RUN mkdir temp +RUN echo "hello" > temp/target +RUN ln -s target temp/link ## Relative link with paths -RUN mkdir workdir && cd workdir && ln -s ../tmp/target relative_dir_link +RUN mkdir workdir && cd workdir && ln -s ../temp/target relative_link FROM scratch -COPY --from=t /tmp/link /tmp/ -COPY --from=t /tmp/relative_link /tmp/ -COPY --from=t /workdir/relative_dir_link /workdir/ \ No newline at end of file +COPY --from=t temp/ dest/ +COPY --from=t /workdir/relative_link /workdirAnother/ \ No newline at end of file diff --git a/pkg/filesystem/resolve.go b/pkg/filesystem/resolve.go index 586e220953..a753e8776c 100644 --- a/pkg/filesystem/resolve.go +++ b/pkg/filesystem/resolve.go @@ -35,6 +35,7 @@ import ( // * Add all ancestors of each path to the output set. func ResolvePaths(paths []string, wl []util.WhitelistEntry) (pathsToAdd []string, err error) { logrus.Info("Resolving paths") + logrus.Debugf("Resolving paths %s", paths) fileSet := make(map[string]bool) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index fb275874b8..d068979d44 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -442,7 +442,7 @@ func DetectFilesystemWhitelist(path string) error { func RelativeFiles(fp string, root string) ([]string, error) { var files []string fullPath := filepath.Join(root, fp) - logrus.Debugf("Getting files and contents at root %s", fullPath) + logrus.Debugf("Getting files and contents at root %s for %s", root, fullPath) err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err @@ -609,14 +609,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { return copiedFiles, nil } -// CopySymlink copies the symlink at src to dest -// if asSymlink is true, then the file is copied as symlink. This is done for a symlink file in the directory is copied -// COPY src/ dest/ -// where src/ contains a symlink file. -// if asSymlink is false, then target file of the symlink is copied as a regular file. -// This done when copying a single file via -// COPY sym.link dest -// where sym.link is a link to target file. +// CopySymlink copies the symlink at src to dest along with the regular file. func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, string, error) { if ExcludeFile(src, buildcontext) { logrus.Debugf("%s found in .dockerignore, ignoring", src) @@ -842,15 +835,14 @@ func getSymlink(path string) error { func CopyFileOrSymlink(src string, destDir string) error { destFile := filepath.Join(destDir, src) if fi, _ := os.Lstat(src); IsSymlink(fi) { - link, err := EvalSymLink(src) + link, err := os.Readlink(src) if err != nil { return err } - linkPath := filepath.Join(destDir, link) if err := createParentDirectory(destFile); err != nil { return err } - return os.Symlink(linkPath, destFile) + return os.Symlink(link, destFile) } return otiai10Cpy.Copy(src, destFile) } From 9592f2640ff0e8689b0b04fe9bece35d24ac22fb Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Mon, 9 Mar 2020 15:54:35 -0700 Subject: [PATCH 6/6] more tests similar to docker cp --- pkg/commands/copy.go | 5 +--- pkg/commands/copy_test.go | 59 ++++++++++++++++++++++++++++++++++----- pkg/util/fs_util.go | 21 +++++--------- pkg/util/fs_util_test.go | 2 +- 4 files changed, 61 insertions(+), 26 deletions(-) diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 6f1834def2..fb084e2089 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -98,7 +98,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.snapshotFiles = append(c.snapshotFiles, copiedFiles...) } else if util.IsSymlink(fi) { // If file is a symlink, we want to copy the target file to destPath - exclude, target, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid) + exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext) if err != nil { return errors.Wrap(err, "copying symlink") } @@ -106,9 +106,6 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu continue } c.snapshotFiles = append(c.snapshotFiles, destPath) - if target != "" { - c.snapshotFiles = append(c.snapshotFiles, target) - } } else { // ... Else, we want to copy over a file exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext, uid, gid) diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 81fe58a604..b57fda9c5b 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -616,10 +616,9 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { t.Fatal(err) } // bam.txt and sym.link should be present - testutil.CheckDeepEqual(t, 2, len(files)) - testutil.CheckDeepEqual(t, files[0].Name(), "dam.txt") - testutil.CheckDeepEqual(t, files[1].Name(), "sym.link") - testutil.CheckDeepEqual(t, true, files[1].Mode()&os.ModeSymlink != 0) + testutil.CheckDeepEqual(t, 1, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "sym.link") + testutil.CheckDeepEqual(t, true, files[0].Mode()&os.ModeSymlink != 0) linkName, err := os.Readlink(filepath.Join(testDir, "dest", "sym.link")) if err != nil { t.Fatal(err) @@ -741,7 +740,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { err = cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) testutil.CheckNoError(t, err) - // Check if "dest" dir exists contests of srcDir and an extra zSym.link created + // Check if "dest" dir exists contents of srcDir and an extra zSym.link created // in this test actual, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) if err != nil { @@ -794,9 +793,9 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { testutil.CheckDeepEqual(t, expected[i].Mode(), f.Mode()) } }) - t.Run("copy file to a dest which is a symlink", func(t *testing.T) { + t.Run("copy src dir to a dest dir which is a symlink", func(t *testing.T) { testDir, srcDir := setupDirs(t) - //defer os.RemoveAll(testDir) + defer os.RemoveAll(testDir) expected, err := ioutil.ReadDir(filepath.Join(testDir, srcDir)) if err != nil { t.Fatal(err) @@ -842,4 +841,50 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { } testutil.CheckDeepEqual(t, linkName, dest) }) + t.Run("copy src file to a dest dir which is a symlink", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + + dest := filepath.Join(testDir, "dest") + if err := os.MkdirAll(dest, 0777); err != nil { + t.Fatal(err) + } + linkedDest := filepath.Join(testDir, "linkDest") + if err := os.Symlink(dest, linkedDest); err != nil { + t.Fatal(err) + } + + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{fmt.Sprintf("%s/bam.txt", srcDir), linkedDest}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + // Check if "linkDest" link is same. + actual, err := ioutil.ReadDir(filepath.Join(testDir, "dest")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, "bam.txt", actual[0].Name()) + c, err := ioutil.ReadFile(filepath.Join(testDir, "dest", "bam.txt")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, "meow", string(c)) + // Check if linkDest -> dest + linkName, err := os.Readlink(filepath.Join(testDir, "linkDest")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, linkName, dest) + }) } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index d068979d44..457ee886a7 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -595,7 +595,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } } else if IsSymlink(fi) { // If file is a symlink, we want to create the same relative symlink - if _, _, err := CopySymlink(fullPath, destPath, buildcontext, uid, gid); err != nil { + if _, err := CopySymlink(fullPath, destPath, buildcontext); err != nil { return nil, err } } else { @@ -609,32 +609,25 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { return copiedFiles, nil } -// CopySymlink copies the symlink at src to dest along with the regular file. -func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, string, error) { +// CopySymlink copies the symlink at src to dest. +func CopySymlink(src, dest, buildcontext string) (bool, error) { if ExcludeFile(src, buildcontext) { logrus.Debugf("%s found in .dockerignore, ignoring", src) - return true, "", nil + return true, nil } if FilepathExists(dest) { if err := os.RemoveAll(dest); err != nil { - return false, "", err + return false, err } } if err := createParentDirectory(dest); err != nil { - return false, "", err + return false, err } - target := "" link, err := os.Readlink(src) if err != nil { logrus.Debugf("could not read link for %s", src) - } else { - cpDest := filepath.Clean(filepath.Join(filepath.Dir(dest), link)) - srcLink := filepath.Clean(filepath.Join(filepath.Dir(src), link)) - if t, err := CopyFile(srcLink, cpDest, buildcontext, uid, gid); !t && err != nil { - target = link - } } - return false, target, os.Symlink(link, dest) + return false, os.Symlink(link, dest) } // CopyFile copies the file at src to dest diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index fdda2018cf..93638b6d0b 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -835,7 +835,7 @@ func TestCopySymlink(t *testing.T) { if err := os.Symlink(tc.linkTarget, link); err != nil { t.Fatal(err) } - if _, _, err := CopySymlink(link, dest, "", DoNotChangeUID, DoNotChangeGID); err != nil { + if _, err := CopySymlink(link, dest, ""); err != nil { t.Fatal(err) } if _, err := os.Lstat(dest); err != nil {