refactor: replace filepath.Walk with WalkDir (#5416)

* refactor: replace filepath.Walk with WalkDir Walk is less efficient than [WalkDir], introduced in Go 1.16 which avoids calling os.Lstat on every visited file or directory. * lint: fix linter issues * lint: fix linter issues * fix: only use walkdir when it makes sense * lint: fix linter issue
elastic · Sep 6, 2024 · 781f529 · 781f529
1 parent 542db76
commit 781f529
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 28 deletions.
diff --git a/dev-tools/mage/common.go b/dev-tools/mage/common.go
@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"log"
 	"net/http"
 	"os"
@@ -365,17 +366,22 @@ func Tar(src string, targetFile string) error {
 	tw := tar.NewWriter(zr)
 
 	// walk through every file in the folder
-	err = filepath.Walk(src, func(file string, fi os.FileInfo, errFn error) error {
+	err = filepath.WalkDir(src, func(file string, d fs.DirEntry, errFn error) error {
 		if errFn != nil {
 			return fmt.Errorf("error traversing the file system: %w", errFn)
 		}
 
 		// if a symlink, skip file
-		if fi.Mode().Type() == os.ModeSymlink {
+		if d.Type() == os.ModeSymlink {
 			fmt.Printf(">> skipping symlink: %s\n", file)
 			return nil
 		}
 
+		fi, err := d.Info()
+		if err != nil {
+			return fmt.Errorf("error getting direntry info: %w", err)
+		}
+
 		// generate tar header
 		header, err := tar.FileInfoHeader(fi, file)
 		if err != nil {
@@ -640,23 +646,28 @@ func FindFiles(globs ...string) ([]string, error) {
 // FindFilesRecursive recursively traverses from the CWD and invokes the given
 // match function on each regular file to determine if the given path should be
 // returned as a match. It ignores files in .git directories.
-func FindFilesRecursive(match func(path string, info os.FileInfo) bool) ([]string, error) {
+func FindFilesRecursive(match func(path string, d fs.FileInfo) bool) ([]string, error) {
 	var matches []string
-	err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
+	err := filepath.WalkDir(".", func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
 
 		// Don't look for files in git directories
-		if info.Mode().IsDir() && filepath.Base(path) == ".git" {
+		if d.IsDir() && filepath.Base(path) == ".git" {
 			return filepath.SkipDir
 		}
 
-		if !info.Mode().IsRegular() {
+		if !d.Type().IsRegular() {
 			// continue
 			return nil
 		}
 
+		info, err := d.Info()
+		if err != nil {
+			return err
+		}
+
 		if match(filepath.ToSlash(path), info) {
 			matches = append(matches, path)
 		}
@@ -786,15 +797,15 @@ func IsUpToDate(dst string, sources ...string) bool {
 
 	var files []string
 	for _, s := range sources {
-		err := filepath.Walk(s, func(path string, info os.FileInfo, err error) error {
+		err := filepath.WalkDir(s, func(path string, d fs.DirEntry, err error) error {
 			if err != nil {
 				if os.IsNotExist(err) {
 					return nil
 				}
 				return err
 			}
 
-			if info.Mode().IsRegular() {
+			if d.Type().IsRegular() {
 				files = append(files, path)
 			}
 

diff --git a/dev-tools/mage/dockerbuilder.go b/dev-tools/mage/dockerbuilder.go
@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -115,8 +116,8 @@ func (b *dockerBuilder) prepareBuild() error {
 		"Variant":     b.DockerVariant.String(),
 	}
 
-	err = filepath.Walk(templatesDir, func(path string, info os.FileInfo, _ error) error {
-		if !info.IsDir() && !isDockerFile(path) {
+	err = filepath.WalkDir(templatesDir, func(path string, d fs.DirEntry, _ error) error {
+		if !d.Type().IsDir() && !isDockerFile(path) {
 			target := strings.TrimSuffix(
 				filepath.Join(b.buildDir, filepath.Base(path)),
 				".tmpl",

diff --git a/dev-tools/mage/pkgtypes.go b/dev-tools/mage/pkgtypes.go
@@ -11,6 +11,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"io/fs"
 	"log"
 	"os"
 	"path/filepath"
@@ -828,7 +829,7 @@ func addUIDGidEnvArgs(args []string) ([]string, error) {
 
 // addFileToZip adds a file (or directory) to a zip archive.
 func addFileToZip(ar *zip.Writer, baseDir string, pkgFile PackageFile) error {
-	return filepath.Walk(pkgFile.Source, func(path string, info os.FileInfo, err error) error {
+	return filepath.Walk(pkgFile.Source, func(path string, info fs.FileInfo, err error) error {
 		if err != nil {
 			if pkgFile.SkipOnMissing && os.IsNotExist(err) {
 				return nil
@@ -903,22 +904,27 @@ func addFileToTar(ar *tar.Writer, baseDir string, pkgFile PackageFile) error {
 		"cloud-defend.spec.yml",
 	}
 
-	return filepath.Walk(pkgFile.Source, func(path string, info os.FileInfo, err error) error {
+	return filepath.WalkDir(pkgFile.Source, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			if pkgFile.SkipOnMissing && os.IsNotExist(err) {
 				return nil
 			}
 			return err
 		}
 
-		if slices.Contains(excludedFiles, info.Name()) {
+		if slices.Contains(excludedFiles, d.Name()) {
 			// it's a file we have to exclude
 			if mg.Verbose() {
 				log.Printf("Skipping file %q...", path)
 			}
 			return nil
 		}
 
+		info, err := d.Info()
+		if err != nil {
+			return err
+		}
+
 		header, err := tar.FileInfoHeader(info, info.Name())
 		if err != nil {
 			return err
@@ -984,7 +990,7 @@ func addSymlinkToTar(tmpdir string, ar *tar.Writer, baseDir string, pkgFile Pack
 		return err
 	}
 
-	return filepath.Walk(link, func(path string, info os.FileInfo, err error) error {
+	return filepath.Walk(link, func(path string, info fs.FileInfo, err error) error {
 		if err != nil {
 			if pkgFile.SkipOnMissing && os.IsNotExist(err) {
 				return nil

diff --git a/internal/pkg/agent/perms/windows.go b/internal/pkg/agent/perms/windows.go
@@ -82,7 +82,7 @@ func FixPermissions(topPath string, opts ...OptFunc) error {
 		// call to `takeOwnership` which sets the ownership information requires the current process
 		// token to have the 'SeRestorePrivilege' or it's unable to adjust the ownership
 		return winio.RunWithPrivileges([]string{winio.SeRestorePrivilege}, func() error {
-			return filepath.Walk(topPath, func(name string, info fs.FileInfo, err error) error {
+			return filepath.WalkDir(topPath, func(name string, _ fs.DirEntry, err error) error {
 				if err == nil {
 					// first level doesn't inherit
 					inherit := true
@@ -111,7 +111,7 @@ func FixPermissions(topPath string, opts ...OptFunc) error {
 	}
 
 	// ownership cannot be changed, this will keep the ownership as it currently is but apply the ACL's
-	return filepath.Walk(topPath, func(name string, info fs.FileInfo, err error) error {
+	return filepath.WalkDir(topPath, func(name string, _ fs.DirEntry, err error) error {
 		if err == nil {
 			// first level doesn't inherit
 			inherit := true

diff --git a/magefile.go b/magefile.go
@@ -14,6 +14,7 @@ import (
 	"errors"
 	"fmt"
 	"html/template"
+	"io/fs"
 	"log"
 	"maps"
 	"math/rand/v2"
@@ -1599,16 +1600,16 @@ func selectedPackageTypes() string {
 }
 
 func copyAll(from, to string, suffixes ...[]string) error {
-	return filepath.Walk(from, func(path string, info os.FileInfo, err error) error {
+	return filepath.WalkDir(from, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
 
-		if info.IsDir() {
+		if d.IsDir() {
 			return nil
 		}
 
-		targetFile := filepath.Join(to, info.Name())
+		targetFile := filepath.Join(to, d.Name())
 
 		// overwrites with current build
 		return sh.Copy(targetFile, path)
@@ -1760,8 +1761,8 @@ func prepareIronbankBuild() error {
 		"MajorMinor": majorMinor(),
 	}
 
-	err := filepath.Walk(templatesDir, func(path string, info os.FileInfo, _ error) error {
-		if !info.IsDir() {
+	err := filepath.WalkDir(templatesDir, func(path string, d fs.DirEntry, _ error) error {
+		if !d.IsDir() {
 			target := strings.TrimSuffix(
 				filepath.Join(buildDir, filepath.Base(path)),
 				".tmpl",

diff --git a/pkg/testing/fixture_install.go b/pkg/testing/fixture_install.go
@@ -721,11 +721,11 @@ func (f *Fixture) archiveInstallDirectory(installPath string, outputPath string)
 	w := zip.NewWriter(file)
 	defer w.Close()
 
-	walker := func(path string, info os.FileInfo, err error) error {
+	walker := func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
-		if info.IsDir() {
+		if d.IsDir() {
 			return nil
 		}
 		file, err := os.Open(path)
@@ -748,7 +748,7 @@ func (f *Fixture) archiveInstallDirectory(installPath string, outputPath string)
 		return nil
 	}
 
-	err = filepath.Walk(f.workDir, walker)
+	err = filepath.WalkDir(f.workDir, walker)
 	if err != nil {
 		return fmt.Errorf("walking %s to create zip: %w", f.workDir, err)
 	}

diff --git a/testing/installtest/checks_unix.go b/testing/installtest/checks_unix.go
@@ -9,6 +9,7 @@ package installtest
 import (
 	"context"
 	"fmt"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -103,14 +104,18 @@ func checkPlatform(ctx context.Context, _ *atesting.Fixture, topPath string, opt
 }
 
 func validateFileTree(dir string, uid uint32, gid uint32) error {
-	return filepath.Walk(dir, func(file string, info os.FileInfo, err error) error {
+	return filepath.WalkDir(dir, func(file string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return fmt.Errorf("error traversing the file tree: %w", err)
 		}
-		if info.Mode().Type() == os.ModeSymlink {
+		if d.Type() == os.ModeSymlink {
 			// symlink don't check permissions
 			return nil
 		}
+		info, err := d.Info()
+		if err != nil {
+			return fmt.Errorf("error caling info: %w", err)
+		}
 		fs, ok := info.Sys().(*syscall.Stat_t)
 		if !ok {
 			return fmt.Errorf("failed to convert info.Sys() into *syscall.Stat_t")

diff --git a/testing/integration/diagnostics_test.go b/testing/integration/diagnostics_test.go
@@ -245,12 +245,14 @@ func verifyDiagnosticArchive(t *testing.T, compSetup map[string]integrationtest.
 
 	actualExtractedDiagFiles := map[string]struct{}{}
 
-	err = filepath.Walk(extractionDir, func(path string, info fs.FileInfo, err error) error {
+	err = filepath.WalkDir(extractionDir, func(path string, entry fs.DirEntry, err error) error {
 		require.NoErrorf(t, err, "error walking extracted path %q", path)
 
 		// we are not interested in directories
-		if !info.IsDir() {
+		if !entry.IsDir() {
 			actualExtractedDiagFiles[path] = struct{}{}
+			info, err := entry.Info()
+			require.NoError(t, err, path)
 			assert.Greaterf(t, info.Size(), int64(0), "file %q has an invalid size", path)
 		}