Fix bug in copy command by refactoring whitelist checks (#231)
* Fixed bug * WIP * fix unit tests
This commit is contained in:
		
							parent
							
								
									a80a7ded35
								
							
						
					
					
						commit
						31b7cd3732
					
				|  | @ -0,0 +1 @@ | ||||||
|  | test | ||||||
|  | @ -230,6 +230,9 @@ func saveStageDependencies(index int, stages []instructions.Stage, buildArgs *do | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  | 	if len(dependencies) == 0 { | ||||||
|  | 		return nil | ||||||
|  | 	} | ||||||
| 	// Then, create the directory they will exist in
 | 	// Then, create the directory they will exist in
 | ||||||
| 	i := strconv.Itoa(index) | 	i := strconv.Itoa(index) | ||||||
| 	dependencyDir := filepath.Join(constants.KanikoDir, i) | 	dependencyDir := filepath.Join(constants.KanikoDir, i) | ||||||
|  |  | ||||||
|  | @ -93,7 +93,7 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { | ||||||
| 		if val, ok := snapshottedFiles[file]; ok && val { | 		if val, ok := snapshottedFiles[file]; ok && val { | ||||||
| 			continue | 			continue | ||||||
| 		} | 		} | ||||||
| 		if util.PathInWhitelist(file, s.directory) { | 		if util.CheckWhitelist(file) { | ||||||
| 			logrus.Debugf("Not adding %s to layer, as it's whitelisted", file) | 			logrus.Debugf("Not adding %s to layer, as it's whitelisted", file) | ||||||
| 			continue | 			continue | ||||||
| 		} | 		} | ||||||
|  | @ -151,7 +151,7 @@ func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) { | ||||||
| 
 | 
 | ||||||
| 	// Now create the tar.
 | 	// Now create the tar.
 | ||||||
| 	for path, info := range memFs { | 	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) | 			logrus.Debugf("Not adding %s to layer, as it's whitelisted", path) | ||||||
| 			continue | 			continue | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
|  | @ -38,9 +38,8 @@ func TestSnapshotFileChange(t *testing.T) { | ||||||
| 	} | 	} | ||||||
| 	// Make some changes to the filesystem
 | 	// Make some changes to the filesystem
 | ||||||
| 	newFiles := map[string]string{ | 	newFiles := map[string]string{ | ||||||
| 		"foo":        "newbaz1", | 		"foo":     "newbaz1", | ||||||
| 		"bar/bat":    "baz", | 		"bar/bat": "baz", | ||||||
| 		"kaniko/bat": "bat", |  | ||||||
| 	} | 	} | ||||||
| 	if err := testutil.SetupFiles(testDir, newFiles); err != nil { | 	if err := testutil.SetupFiles(testDir, newFiles); err != nil { | ||||||
| 		t.Fatalf("Error setting up fs: %s", err) | 		t.Fatalf("Error setting up fs: %s", err) | ||||||
|  | @ -135,8 +134,7 @@ func TestSnapshotFiles(t *testing.T) { | ||||||
| 	} | 	} | ||||||
| 	// Make some changes to the filesystem
 | 	// Make some changes to the filesystem
 | ||||||
| 	newFiles := map[string]string{ | 	newFiles := map[string]string{ | ||||||
| 		"foo":         "newbaz1", | 		"foo": "newbaz1", | ||||||
| 		"kaniko/file": "bat", |  | ||||||
| 	} | 	} | ||||||
| 	if err := testutil.SetupFiles(testDir, newFiles); err != nil { | 	if err := testutil.SetupFiles(testDir, newFiles); err != nil { | ||||||
| 		t.Fatalf("Error setting up fs: %s", err) | 		t.Fatalf("Error setting up fs: %s", err) | ||||||
|  |  | ||||||
|  | @ -91,12 +91,12 @@ func GetFSFromImage(img v1.Image) error { | ||||||
| 				continue | 				continue | ||||||
| 			} | 			} | ||||||
| 
 | 
 | ||||||
| 			if checkWhitelist(path, whitelist) { | 			if CheckWhitelist(path) { | ||||||
| 				logrus.Infof("Not adding %s because it is whitelisted", path) | 				logrus.Infof("Not adding %s because it is whitelisted", path) | ||||||
| 				continue | 				continue | ||||||
| 			} | 			} | ||||||
| 			if hdr.Typeflag == tar.TypeSymlink { | 			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) | 					logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname) | ||||||
| 					continue | 					continue | ||||||
| 				} | 				} | ||||||
|  | @ -115,7 +115,7 @@ func GetFSFromImage(img v1.Image) error { | ||||||
| func DeleteFilesystem() error { | func DeleteFilesystem() error { | ||||||
| 	logrus.Info("Deleting filesystem...") | 	logrus.Info("Deleting filesystem...") | ||||||
| 	err := filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, err error) error { | 	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) | 			logrus.Debugf("Not deleting %s, as it's whitelisted", path) | ||||||
| 			return nil | 			return nil | ||||||
| 		} | 		} | ||||||
|  | @ -233,21 +233,6 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { | ||||||
| 	return nil | 	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 { | func checkWhiteouts(path string, whiteouts map[string]struct{}) bool { | ||||||
| 	// Don't add the file if it or it's directory are whited out.
 | 	// Don't add the file if it or it's directory are whited out.
 | ||||||
| 	if _, ok := whiteouts[path]; ok { | 	if _, ok := whiteouts[path]; ok { | ||||||
|  | @ -262,7 +247,7 @@ func checkWhiteouts(path string, whiteouts map[string]struct{}) bool { | ||||||
| 	return false | 	return false | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func checkWhitelist(path string, whitelist []string) bool { | func CheckWhitelist(path string) bool { | ||||||
| 	for _, wl := range whitelist { | 	for _, wl := range whitelist { | ||||||
| 		if HasFilepathPrefix(path, wl) { | 		if HasFilepathPrefix(path, wl) { | ||||||
| 			return true | 			return true | ||||||
|  | @ -316,7 +301,7 @@ func RelativeFiles(fp string, root string) ([]string, error) { | ||||||
| 	fullPath := filepath.Join(root, fp) | 	fullPath := filepath.Join(root, fp) | ||||||
| 	logrus.Debugf("Getting files and contents at root %s", fullPath) | 	logrus.Debugf("Getting files and contents at root %s", fullPath) | ||||||
| 	err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error { | 	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 | 			return nil | ||||||
| 		} | 		} | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
|  | @ -337,7 +322,7 @@ func Files(root string) ([]string, error) { | ||||||
| 	var files []string | 	var files []string | ||||||
| 	logrus.Debugf("Getting files and contents at root %s", root) | 	logrus.Debugf("Getting files and contents at root %s", root) | ||||||
| 	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { | 	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { | ||||||
| 		if PathInWhitelist(path, root) { | 		if CheckWhitelist(path) { | ||||||
| 			return nil | 			return nil | ||||||
| 		} | 		} | ||||||
| 		files = append(files, path) | 		files = append(files, path) | ||||||
|  |  | ||||||
|  | @ -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 { | 	type args struct { | ||||||
| 		path      string | 		path      string | ||||||
| 		whitelist []string | 		whitelist []string | ||||||
|  | @ -261,8 +261,13 @@ func Test_checkWhitelist(t *testing.T) { | ||||||
| 	} | 	} | ||||||
| 	for _, tt := range tests { | 	for _, tt := range tests { | ||||||
| 		t.Run(tt.name, func(t *testing.T) { | 		t.Run(tt.name, func(t *testing.T) { | ||||||
| 			if got := checkWhitelist(tt.args.path, tt.args.whitelist); got != tt.want { | 			original := whitelist | ||||||
| 				t.Errorf("checkWhitelist() = %v, want %v", got, tt.want) | 			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) | ||||||
| 			} | 			} | ||||||
| 		}) | 		}) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue