refactor to add unit tests

This commit is contained in:
Tejal Desai
2020-03-23 17:48:49 -07:00
parent 9f4fead7b5
commit ffc372a63b
14 changed files with 362 additions and 72 deletions
+3 -2
View File
@@ -26,13 +26,14 @@ import (
"strconv"
"strings"
"github.com/GoogleContainerTools/kaniko/pkg/constants"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/moby/buildkit/frontend/dockerfile/instructions"
"github.com/moby/buildkit/frontend/dockerfile/parser"
"github.com/moby/buildkit/frontend/dockerfile/shell"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/GoogleContainerTools/kaniko/pkg/config"
)
// for testing
@@ -145,7 +146,7 @@ func matchSources(srcs, files []string) ([]string, error) {
src = filepath.Clean(src)
for _, file := range files {
if filepath.IsAbs(src) {
file = filepath.Join(constants.RootDir, file)
file = filepath.Join(config.RootDir, file)
}
matched, err := filepath.Match(src, file)
if err != nil {
+22 -19
View File
@@ -33,7 +33,7 @@ import (
otiai10Cpy "github.com/otiai10/copy"
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/docker/docker/builder/dockerignore"
"github.com/docker/docker/pkg/fileutils"
v1 "github.com/google/go-containerregistry/pkg/v1"
@@ -51,7 +51,7 @@ type WhitelistEntry struct {
var initialWhitelist = []WhitelistEntry{
{
Path: "/kaniko",
Path: config.KanikoDir,
PrefixMatchOnly: false,
},
{
@@ -125,7 +125,7 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
return nil, errors.New("must supply an extract function")
}
if err := DetectFilesystemWhitelist(constants.WhitelistPath); err != nil {
if err := DetectFilesystemWhitelist(config.WhitelistPath); err != nil {
return nil, err
}
@@ -188,7 +188,7 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
// DeleteFilesystem deletes the extracted image file system
func DeleteFilesystem() error {
logrus.Info("Deleting filesystem...")
return filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, err error) error {
return filepath.Walk(config.RootDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
// ignore errors when deleting.
return nil
@@ -209,7 +209,7 @@ func DeleteFilesystem() error {
logrus.Debugf("Not deleting %s, as it contains a whitelisted path", path)
return nil
}
if path == constants.RootDir {
if path == config.RootDir {
return nil
}
return os.RemoveAll(path)
@@ -388,7 +388,7 @@ func CheckWhitelist(path string) bool {
}
func checkWhitelistRoot(root string) bool {
if root == constants.RootDir {
if root == config.RootDir {
return false
}
return CheckWhitelist(root)
@@ -423,7 +423,7 @@ func DetectFilesystemWhitelist(path string) error {
}
continue
}
if lineArr[4] != constants.RootDir {
if lineArr[4] != config.RootDir {
logrus.Tracef("Appending %s from line: %s", lineArr[4], line)
whitelist = append(whitelist, WhitelistEntry{
Path: lineArr[4],
@@ -463,16 +463,18 @@ func RelativeFiles(fp string, root string) ([]string, error) {
// ParentDirectories returns a list of paths to all parent directories
// Ex. /some/temp/dir -> [/, /some, /some/temp, /some/temp/dir]
func ParentDirectories(path string) []string {
path = filepath.Clean(path)
dirs := strings.Split(path, "/")
dirPath := constants.RootDir
paths := []string{constants.RootDir}
for index, dir := range dirs {
if dir == "" || index == (len(dirs)-1) {
continue
dir := filepath.Clean(path)
var paths []string
for {
if dir == filepath.Clean(config.RootDir) || dir == "" || dir == "." {
break
}
dirPath = filepath.Join(dirPath, dir)
paths = append(paths, dirPath)
dir, _ = filepath.Split(dir)
dir = filepath.Clean(dir)
paths = append(paths, dir)
}
if len(paths) == 0 {
paths = append(paths, config.RootDir)
}
return paths
}
@@ -484,7 +486,7 @@ func ParentDirectoriesWithoutLeadingSlash(path string) []string {
path = filepath.Clean(path)
dirs := strings.Split(path, "/")
dirPath := ""
paths := []string{constants.RootDir}
paths := []string{config.RootDir}
for index, dir := range dirs {
if dir == "" || index == (len(dirs)-1) {
continue
@@ -824,12 +826,13 @@ func getSymlink(path string) error {
// For cross stage dependencies kaniko must persist the referenced path so that it can be used in
// the dependent stage. For symlinks we copy the target path because copying the symlink would
// result in a dead link
func CopyFileOrSymlink(src string, destDir string) error {
func CopyFileOrSymlink(src string, destDir string, root string) error {
destFile := filepath.Join(destDir, src)
src = filepath.Join(root, src)
if fi, _ := os.Lstat(src); IsSymlink(fi) {
link, err := os.Readlink(src)
if err != nil {
return err
return errors.Wrap(err, "copying file or symlink")
}
if err := createParentDirectory(destFile); err != nil {
return err
+40 -4
View File
@@ -30,6 +30,7 @@ import (
"testing"
"time"
"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/GoogleContainerTools/kaniko/pkg/mocks/go-containerregistry/mockv1"
"github.com/GoogleContainerTools/kaniko/testutil"
"github.com/golang/mock/gomock"
@@ -157,11 +158,13 @@ func Test_ParentDirectories(t *testing.T) {
tests := []struct {
name string
path string
rootDir string
expected []string
}{
{
name: "regular path",
path: "/path/to/dir",
name: "regular path",
path: "/path/to/dir",
rootDir: "/",
expected: []string{
"/",
"/path",
@@ -169,17 +172,50 @@ func Test_ParentDirectories(t *testing.T) {
},
},
{
name: "current directory",
path: ".",
name: "current directory",
path: ".",
rootDir: "/",
expected: []string{
"/",
},
},
{
name: "non / root directory",
path: "/tmp/kaniko/test/another/dir",
rootDir: "/tmp/kaniko/",
expected: []string{
"/tmp/kaniko",
"/tmp/kaniko/test",
"/tmp/kaniko/test/another",
},
},
{
name: "non / root director same path",
path: "/tmp/123",
rootDir: "/tmp/123",
expected: []string{
"/tmp/123",
},
},
{
name: "non / root directory path",
path: "/tmp/120162240/kaniko",
rootDir: "/tmp/120162240",
expected: []string{
"/tmp/120162240",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
original := config.RootDir
defer func() { config.RootDir = original }()
config.RootDir = tt.rootDir
actual := ParentDirectories(tt.path)
sort.Strings(actual)
sort.Strings(tt.expected)
testutil.CheckErrorAndDeepEqual(t, false, nil, tt.expected, actual)
})
}
+7 -6
View File
@@ -28,6 +28,7 @@ import (
"strings"
"syscall"
"github.com/GoogleContainerTools/kaniko/pkg/config"
"github.com/docker/docker/pkg/archive"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@@ -76,14 +77,14 @@ func (t *Tar) AddFileToTar(p string) error {
return err
}
if p != "/" {
// Docker uses no leading / in the tarball
hdr.Name = strings.TrimLeft(p, "/")
} else {
if p == config.RootDir {
// allow entry for / to preserve permission changes etc. (currently ignored anyway by Docker runtime)
hdr.Name = p
hdr.Name = "/"
} else {
// Docker uses no leading / in the tarball
hdr.Name = strings.TrimPrefix(p, config.RootDir)
hdr.Name = strings.TrimLeft(hdr.Name, "/")
}
// rootfs may not have been extracted when using cache, preventing uname/gname from resolving
// this makes this layer unnecessarily differ from a cached layer which does contain this information
hdr.Uname = ""