From 31b7cd3732300fed6d7d31ce49b61327a285b60c Mon Sep 17 00:00:00 2001 From: priyawadhwa Date: Tue, 10 Jul 2018 08:23:35 -0700 Subject: [PATCH] Fix bug in copy command by refactoring whitelist checks (#231) * Fixed bug * WIP * fix unit tests --- integration/context/workspace/test | 1 + pkg/executor/executor.go | 3 +++ pkg/snapshot/snapshot.go | 4 ++-- pkg/snapshot/snapshot_test.go | 8 +++----- pkg/util/fs_util.go | 27 ++++++--------------------- pkg/util/fs_util_test.go | 11 ++++++++--- 6 files changed, 23 insertions(+), 31 deletions(-) create mode 100644 integration/context/workspace/test diff --git a/integration/context/workspace/test b/integration/context/workspace/test new file mode 100644 index 000000000..9daeafb98 --- /dev/null +++ b/integration/context/workspace/test @@ -0,0 +1 @@ +test diff --git a/pkg/executor/executor.go b/pkg/executor/executor.go index dccfffa05..18a6e1fb0 100644 --- a/pkg/executor/executor.go +++ b/pkg/executor/executor.go @@ -230,6 +230,9 @@ func saveStageDependencies(index int, stages []instructions.Stage, buildArgs *do if err != nil { return err } + if len(dependencies) == 0 { + return nil + } // Then, create the directory they will exist in i := strconv.Itoa(index) dependencyDir := filepath.Join(constants.KanikoDir, i) diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 478095afc..d6e80dc17 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -93,7 +93,7 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { if val, ok := snapshottedFiles[file]; ok && val { continue } - if util.PathInWhitelist(file, s.directory) { + if util.CheckWhitelist(file) { logrus.Debugf("Not adding %s to layer, as it's whitelisted", file) continue } @@ -151,7 +151,7 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { // Now create the tar. for path, info := range memFs { - if util.PathInWhitelist(path, s.directory) { + if util.CheckWhitelist(path) { logrus.Debugf("Not adding %s to layer, as it's whitelisted", path) continue } diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index f8f3c0161..934e862d9 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/pkg/snapshot/snapshot_test.go @@ -38,9 +38,8 @@ func TestSnapshotFileChange(t *testing.T) { } // Make some changes to the filesystem newFiles := map[string]string{ - "foo": "newbaz1", - "bar/bat": "baz", - "kaniko/bat": "bat", + "foo": "newbaz1", + "bar/bat": "baz", } if err := testutil.SetupFiles(testDir, newFiles); err != nil { t.Fatalf("Error setting up fs: %s", err) @@ -135,8 +134,7 @@ func TestSnapshotFiles(t *testing.T) { } // Make some changes to the filesystem newFiles := map[string]string{ - "foo": "newbaz1", - "kaniko/file": "bat", + "foo": "newbaz1", } if err := testutil.SetupFiles(testDir, newFiles); err != nil { t.Fatalf("Error setting up fs: %s", err) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 42c339667..7667c68c0 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -91,12 +91,12 @@ func GetFSFromImage(img v1.Image) error { continue } - if checkWhitelist(path, whitelist) { + if CheckWhitelist(path) { logrus.Infof("Not adding %s because it is whitelisted", path) continue } if hdr.Typeflag == tar.TypeSymlink { - if checkWhitelist(hdr.Linkname, whitelist) { + if CheckWhitelist(hdr.Linkname) { logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname) continue } @@ -115,7 +115,7 @@ func GetFSFromImage(img v1.Image) error { func DeleteFilesystem() error { logrus.Info("Deleting filesystem...") err := filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, err error) error { - if PathInWhitelist(path, constants.RootDir) || ChildDirInWhitelist(path, constants.RootDir) { + if CheckWhitelist(path) || ChildDirInWhitelist(path, constants.RootDir) { logrus.Debugf("Not deleting %s, as it's whitelisted", path) return nil } @@ -233,21 +233,6 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { return nil } -func PathInWhitelist(path, directory string) bool { - for _, c := range constants.KanikoBuildFiles { - if path == c { - return false - } - } - for _, d := range whitelist { - dirPath := filepath.Join(directory, d) - if HasFilepathPrefix(path, dirPath) { - return true - } - } - return false -} - func checkWhiteouts(path string, whiteouts map[string]struct{}) bool { // Don't add the file if it or it's directory are whited out. if _, ok := whiteouts[path]; ok { @@ -262,7 +247,7 @@ func checkWhiteouts(path string, whiteouts map[string]struct{}) bool { return false } -func checkWhitelist(path string, whitelist []string) bool { +func CheckWhitelist(path string) bool { for _, wl := range whitelist { if HasFilepathPrefix(path, wl) { return true @@ -316,7 +301,7 @@ func RelativeFiles(fp string, root string) ([]string, error) { fullPath := filepath.Join(root, fp) logrus.Debugf("Getting files and contents at root %s", fullPath) err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error { - if PathInWhitelist(path, root) { + if CheckWhitelist(path) && !HasFilepathPrefix(path, root) { return nil } if err != nil { @@ -337,7 +322,7 @@ func Files(root string) ([]string, error) { var files []string logrus.Debugf("Getting files and contents at root %s", root) err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { - if PathInWhitelist(path, root) { + if CheckWhitelist(path) { return nil } files = append(files, path) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 404a0f088..21ff3a766 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -216,7 +216,7 @@ func Test_checkWhiteouts(t *testing.T) { } } -func Test_checkWhitelist(t *testing.T) { +func Test_CheckWhitelist(t *testing.T) { type args struct { path string whitelist []string @@ -261,8 +261,13 @@ func Test_checkWhitelist(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := checkWhitelist(tt.args.path, tt.args.whitelist); got != tt.want { - t.Errorf("checkWhitelist() = %v, want %v", got, tt.want) + original := whitelist + defer func() { + whitelist = original + }() + whitelist = tt.args.whitelist + if got := CheckWhitelist(tt.args.path); got != tt.want { + t.Errorf("CheckWhitelist() = %v, want %v", got, tt.want) } }) }