better error wrapping and add more tests for copy

This commit is contained in:
Tejal Desai
2020-03-06 17:18:36 -08:00
parent b4b70d040a
commit 6c14d202a3
9 changed files with 324 additions and 91 deletions
+5 -4
View File
@@ -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
+20 -14
View File
@@ -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
+265 -54
View File
@@ -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")
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)
setupDirs := func(t *testing.T) (string, string) {
testDir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatal(err)
}
t.Run("copy dir to another dir", func(t *testing.T) {
testDir, targetDir := setupDirs(t)
defer os.RemoveAll(testDir)
dir := filepath.Join(testDir, "bar")
cmd := CopyCommand{
cmd: &instructions.CopyCommand{
SourcesAndDest: []string{targetDir, "dest"},
},
buildcontext: testDir,
}
if err := os.MkdirAll(dir, 0777); err != nil {
t.Fatal(err)
}
cfg := &v1.Config{
Cmd: nil,
Env: []string{},
WorkingDir: testDir,
}
file := filepath.Join(dir, "bam.txt")
err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{}))
testutil.CheckNoError(t, err)
})
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 file to a dir", func(t *testing.T) {
testDir, targetDir := setupDirs(t)
return testDir, filepath.Base(dir)
}
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)
}
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{}))
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, 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,22 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
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 {
// Check if "dest" dir exists with file bam.txt
files, err := ioutil.ReadDir(filepath.Join(testDir, "dest"))
if err != nil {
t.Fatal(err)
}
symlink := filepath.Join(another, "sym.link")
os.Symlink(filepath.Join(targetDir, "bam.txt"), symlink)
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, srcDir := setupDirs(t)
defer os.RemoveAll(testDir)
cmd := CopyCommand{
cmd: &instructions.CopyCommand{
SourcesAndDest: []string{symlink, "dest"},
SourcesAndDest: []string{filepath.Join(srcDir, "sym.link"), "dest/"},
},
buildcontext: testDir,
}
@@ -576,5 +609,183 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
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")
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)
}
cmd := CopyCommand{
cmd: &instructions.CopyCommand{
SourcesAndDest: []string{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" 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)
})
}
+1 -1
View File
@@ -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)
+4 -6
View File
@@ -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)
-1
View File
@@ -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"
+14 -6
View File
@@ -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
+14 -4
View File
@@ -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
+1 -1
View File
@@ -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 {