public_git
/
kaniko
mirror of https://github.com/GoogleContainerTools/kaniko
1
0
Fork 0
Code Issues Releases Wiki Activity

Avoid redundant calls to filepath.Clean (#2652) 

filepath.Clean shows up in profiles as a hot spot, and there seem to be
many redundant calls, particularly in ignorelist handling. We can avoid
these redundant calls by pre-cleaning entries in the ignore list, and
providing fast paths when we know we're already dealing with a cleaned
candidate path.

Before:

     580ms  3.03% 72.35%      590ms  3.08%  path/filepath.(*lazybuf).append (inline)
     390ms  2.03% 74.39%      990ms  5.16%  path/filepath.Clean

After:

     0.13s  0.69% 84.01%      0.17s  0.91%  path/filepath.(*lazybuf).append (inline)
     0.13s  0.69% 84.70%      0.31s  1.65%  path/filepath.Clean
This commit is contained in:
Aaron Lehmann 2023-07-31 17:18:16 -07:00 committed by GitHub
parent 81a7294032
commit 32ce1bf67e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 54 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
pkg/commands/copy_test.go
View File

@ -153,7 +153,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
extractedFiles: []string{"/foo.txt"}, extractedFiles: []string{"/foo.txt"},
contextFiles: []string{"foo.txt"}, contextFiles: []string{"foo.txt"},
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
*tc.count++ *tc.count++
return nil return nil
} }
@ -166,7 +166,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
description: "with no image", description: "with no image",
expectErr: true, expectErr: true,
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }
tc.command = c tc.command = c
@ -176,7 +176,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
c := &CachingCopyCommand{ c := &CachingCopyCommand{
img: fakeImage{}, img: fakeImage{},
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }
return testCase{ return testCase{
@ -193,7 +193,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
}, },
}, },
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }
tc := testCase{ tc := testCase{

8
pkg/commands/run_test.go
View File

@ -204,7 +204,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
extractedFiles: []string{"/foo.txt"}, extractedFiles: []string{"/foo.txt"},
contextFiles: []string{"foo.txt"}, contextFiles: []string{"foo.txt"},
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
*tc.count++ *tc.count++
return nil return nil
} }
@ -217,7 +217,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
desctiption: "with no image", desctiption: "with no image",
expectErr: true, expectErr: true,
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }
tc.command = c tc.command = c
@ -228,7 +228,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
img: fakeImage{}, img: fakeImage{},
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }
@ -246,7 +246,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
}, },
}, },
} }
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error { c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }
tc := testCase{ tc := testCase{

2
pkg/filesystem/resolve.go
View File

@ -80,7 +80,7 @@ func ResolvePaths(paths []string, wl []util.IgnoreListEntry) (pathsToAdd []strin
// If the given path is a symlink and the target is part of the ignorelist // If the given path is a symlink and the target is part of the ignorelist
// ignore the target // ignore the target
if util.CheckProvidedIgnoreList(evaled, wl) { if util.CheckCleanedPathAgainstProvidedIgnoreList(evaled, wl) {
logrus.Debugf("Path %s is ignored, ignoring it", evaled) logrus.Debugf("Path %s is ignored, ignoring it", evaled)
continue continue
} }

71
pkg/util/fs_util.go
View File

@ -60,7 +60,7 @@ type IgnoreListEntry struct {
var defaultIgnoreList = []IgnoreListEntry{ var defaultIgnoreList = []IgnoreListEntry{
{ {
Path: config.KanikoDir, Path: filepath.Clean(config.KanikoDir),
PrefixMatchOnly: false, PrefixMatchOnly: false,
}, },
{ {
@ -86,7 +86,7 @@ type FileContext struct {
ExcludedFiles []string ExcludedFiles []string
} }
type ExtractFunction func(string, *tar.Header, io.Reader) error type ExtractFunction func(string, *tar.Header, string, io.Reader) error
type FSConfig struct { type FSConfig struct {
includeWhiteout bool includeWhiteout bool
@ -100,11 +100,17 @@ func IgnoreList() []IgnoreListEntry {
} }
func AddToIgnoreList(entry IgnoreListEntry) { func AddToIgnoreList(entry IgnoreListEntry) {
ignorelist = append(ignorelist, entry) ignorelist = append(ignorelist, IgnoreListEntry{
Path: filepath.Clean(entry.Path),
PrefixMatchOnly: entry.PrefixMatchOnly,
})
} }
func AddToDefaultIgnoreList(entry IgnoreListEntry) { func AddToDefaultIgnoreList(entry IgnoreListEntry) {
defaultIgnoreList = append(defaultIgnoreList, entry) defaultIgnoreList = append(defaultIgnoreList, IgnoreListEntry{
Path: filepath.Clean(entry.Path),
PrefixMatchOnly: entry.PrefixMatchOnly,
})
} }
func IncludeWhiteout() FSOpt { func IncludeWhiteout() FSOpt {
@ -175,7 +181,8 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
return nil, errors.Wrap(err, fmt.Sprintf("error reading tar %d", i)) return nil, errors.Wrap(err, fmt.Sprintf("error reading tar %d", i))
} }
path := filepath.Join(root, filepath.Clean(hdr.Name)) cleanedName := filepath.Clean(hdr.Name)
path := filepath.Join(root, cleanedName)
base := filepath.Base(path) base := filepath.Base(path)
dir := filepath.Dir(path) dir := filepath.Dir(path)
@ -185,7 +192,7 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
name := strings.TrimPrefix(base, archive.WhiteoutPrefix) name := strings.TrimPrefix(base, archive.WhiteoutPrefix)
path := filepath.Join(dir, name) path := filepath.Join(dir, name)
if CheckIgnoreList(path) { if CheckCleanedPathAgainstIgnoreList(path) {
logrus.Tracef("Not deleting %s, as it's ignored", path) logrus.Tracef("Not deleting %s, as it's ignored", path)
continue continue
} }
@ -205,11 +212,11 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
} }
if err := cfg.extractFunc(root, hdr, tr); err != nil { if err := cfg.extractFunc(root, hdr, cleanedName, tr); err != nil {
return nil, err return nil, err
} }
extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name))) extractedFiles = append(extractedFiles, filepath.Join(root, cleanedName))
} }
} }
return extractedFiles, nil return extractedFiles, nil
@ -224,7 +231,7 @@ func DeleteFilesystem() error {
return nil //nolint:nilerr return nil //nolint:nilerr
} }
if CheckIgnoreList(path) { if CheckCleanedPathAgainstIgnoreList(path) {
if !isExist(path) { if !isExist(path) {
logrus.Debugf("Path %s ignored, but not exists", path) logrus.Debugf("Path %s ignored, but not exists", path)
return nil return nil
@ -276,16 +283,17 @@ func UnTar(r io.Reader, dest string) ([]string, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
if err := ExtractFile(dest, hdr, tr); err != nil { cleanedName := filepath.Clean(hdr.Name)
if err := ExtractFile(dest, hdr, cleanedName, tr); err != nil {
return nil, err return nil, err
} }
extractedFiles = append(extractedFiles, filepath.Join(dest, filepath.Clean(hdr.Name))) extractedFiles = append(extractedFiles, filepath.Join(dest, cleanedName))
} }
return extractedFiles, nil return extractedFiles, nil
} }
func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error { func ExtractFile(dest string, hdr *tar.Header, cleanedName string, tr io.Reader) error {
path := filepath.Join(dest, filepath.Clean(hdr.Name)) path := filepath.Join(dest, cleanedName)
base := filepath.Base(path) base := filepath.Base(path)
dir := filepath.Dir(path) dir := filepath.Dir(path)
mode := hdr.FileInfo().Mode() mode := hdr.FileInfo().Mode()
@ -297,7 +305,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
return err return err
} }
if CheckIgnoreList(abs) && !checkIgnoreListRoot(dest) { if CheckCleanedPathAgainstIgnoreList(abs) && !checkIgnoreListRoot(dest) {
logrus.Debugf("Not adding %s because it is ignored", path) logrus.Debugf("Not adding %s because it is ignored", path)
return nil return nil
} }
@ -358,7 +366,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
if err != nil { if err != nil {
return err return err
} }
if CheckIgnoreList(abs) { if CheckCleanedPathAgainstIgnoreList(abs) {
logrus.Tracef("Skipping link from %s to %s because %s is ignored", hdr.Linkname, path, hdr.Linkname) logrus.Tracef("Skipping link from %s to %s because %s is ignored", hdr.Linkname, path, hdr.Linkname)
return nil return nil
} }
@ -399,6 +407,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
} }
func IsInProvidedIgnoreList(path string, wl []IgnoreListEntry) bool { func IsInProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
path = filepath.Clean(path)
for _, entry := range wl { for _, entry := range wl {
if !entry.PrefixMatchOnly && path == entry.Path { if !entry.PrefixMatchOnly && path == entry.Path {
return true return true
@ -412,9 +421,9 @@ func IsInIgnoreList(path string) bool {
return IsInProvidedIgnoreList(path, ignorelist) return IsInProvidedIgnoreList(path, ignorelist)
} }
func CheckProvidedIgnoreList(path string, wl []IgnoreListEntry) bool { func CheckCleanedPathAgainstProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
for _, wl := range ignorelist { for _, wl := range ignorelist {
if HasFilepathPrefix(path, wl.Path, wl.PrefixMatchOnly) { if hasCleanedFilepathPrefix(path, wl.Path, wl.PrefixMatchOnly) {
return true return true
} }
} }
@ -423,7 +432,11 @@ func CheckProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
} }
func CheckIgnoreList(path string) bool { func CheckIgnoreList(path string) bool {
return CheckProvidedIgnoreList(path, ignorelist) return CheckCleanedPathAgainstIgnoreList(filepath.Clean(path))
}
func CheckCleanedPathAgainstIgnoreList(path string) bool {
return CheckCleanedPathAgainstProvidedIgnoreList(path, ignorelist)
} }
func checkIgnoreListRoot(root string) bool { func checkIgnoreListRoot(root string) bool {
@ -463,7 +476,7 @@ func DetectFilesystemIgnoreList(path string) error {
} }
if lineArr[4] != config.RootDir { if lineArr[4] != config.RootDir {
logrus.Tracef("Adding ignore list entry %s from line: %s", lineArr[4], line) logrus.Tracef("Adding ignore list entry %s from line: %s", lineArr[4], line)
ignorelist = append(ignorelist, IgnoreListEntry{ AddToIgnoreList(IgnoreListEntry{
Path: lineArr[4], Path: lineArr[4],
PrefixMatchOnly: false, PrefixMatchOnly: false,
}) })
@ -480,12 +493,13 @@ func DetectFilesystemIgnoreList(path string) error {
func RelativeFiles(fp string, root string) ([]string, error) { func RelativeFiles(fp string, root string) ([]string, error) {
var files []string var files []string
fullPath := filepath.Join(root, fp) fullPath := filepath.Join(root, fp)
cleanedRoot := filepath.Clean(root)
logrus.Debugf("Getting files and contents at root %s for %s", root, 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 { err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error {
if err != nil { if err != nil {
return err return err
} }
if CheckIgnoreList(path) && !HasFilepathPrefix(path, root, false) { if CheckCleanedPathAgainstIgnoreList(path) && !hasCleanedFilepathPrefix(filepath.Clean(path), cleanedRoot, false) {
return nil return nil
} }
relPath, err := filepath.Rel(root, path) relPath, err := filepath.Rel(root, path)
@ -591,7 +605,7 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid
// AddVolumePath adds the given path to the volume ignorelist. // AddVolumePath adds the given path to the volume ignorelist.
func AddVolumePathToIgnoreList(path string) { func AddVolumePathToIgnoreList(path string) {
logrus.Infof("Adding volume %s to ignorelist", path) logrus.Infof("Adding volume %s to ignorelist", path)
ignorelist = append(ignorelist, IgnoreListEntry{ AddToIgnoreList(IgnoreListEntry{
Path: path, Path: path,
PrefixMatchOnly: true, PrefixMatchOnly: true,
}) })
@ -778,11 +792,12 @@ func (c FileContext) ExcludesFile(path string) bool {
// HasFilepathPrefix checks if the given file path begins with prefix // HasFilepathPrefix checks if the given file path begins with prefix
func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool { func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
prefix = filepath.Clean(prefix) return hasCleanedFilepathPrefix(filepath.Clean(path), filepath.Clean(prefix), prefixMatchOnly)
prefixArray := strings.Split(prefix, "/") }
path = filepath.Clean(path)
pathArray := strings.SplitN(path, "/", len(prefixArray)+1)
func hasCleanedFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
prefixArray := strings.Split(prefix, "/")
pathArray := strings.SplitN(path, "/", len(prefixArray)+1)
if len(pathArray) < len(prefixArray) { if len(pathArray) < len(prefixArray) {
return false return false
} }
@ -968,7 +983,7 @@ func CopyOwnership(src string, destDir string, root string) error {
} }
destPath := filepath.Join(destDir, relPath) destPath := filepath.Join(destDir, relPath)
if CheckIgnoreList(src) && CheckIgnoreList(destPath) { if CheckCleanedPathAgainstIgnoreList(src) && CheckCleanedPathAgainstIgnoreList(destPath) {
if !isExist(destPath) { if !isExist(destPath) {
logrus.Debugf("Path %s ignored, but not exists", destPath) logrus.Debugf("Path %s ignored, but not exists", destPath)
return nil return nil
@ -979,7 +994,7 @@ func CopyOwnership(src string, destDir string, root string) error {
logrus.Debugf("Not copying ownership for %s, as it's ignored", destPath) logrus.Debugf("Not copying ownership for %s, as it's ignored", destPath)
return nil return nil
} }
if CheckIgnoreList(destDir) && CheckIgnoreList(path) { if CheckIgnoreList(destDir) && CheckCleanedPathAgainstIgnoreList(path) {
if !isExist(path) { if !isExist(path) {
logrus.Debugf("Path %s ignored, but not exists", path) logrus.Debugf("Path %s ignored, but not exists", path)
return nil return nil
@ -1118,7 +1133,7 @@ func GetFSInfoMap(dir string, existing map[string]os.FileInfo) (map[string]os.Fi
timer := timing.Start("Walking filesystem with Stat") timer := timing.Start("Walking filesystem with Stat")
godirwalk.Walk(dir, &godirwalk.Options{ godirwalk.Walk(dir, &godirwalk.Options{
Callback: func(path string, ent *godirwalk.Dirent) error { Callback: func(path string, ent *godirwalk.Dirent) error {
if CheckIgnoreList(path) { if CheckCleanedPathAgainstIgnoreList(path) {
if IsDestDir(path) { if IsDestDir(path) {
logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path) logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path)
return filepath.SkipDir return filepath.SkipDir

4
pkg/util/fs_util_test.go
View File

@ -810,7 +810,7 @@ func TestExtractFile(t *testing.T) {
defer os.RemoveAll(r) defer os.RemoveAll(r)
for _, hdr := range tc.hdrs { for _, hdr := range tc.hdrs {
if err := ExtractFile(r, hdr, bytes.NewReader(tc.contents)); err != nil { if err := ExtractFile(r, hdr, filepath.Clean(hdr.Name), bytes.NewReader(tc.contents)); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
@ -1008,7 +1008,7 @@ func Test_CopyFile_skips_self(t *testing.T) {
} }
} }
func fakeExtract(dest string, hdr *tar.Header, tr io.Reader) error { func fakeExtract(_ string, _ *tar.Header, _ string, _ io.Reader) error {
return nil return nil
} }