Fixes 4108; remove hidden files in kustomize edit command to correctly mimic shell globbing behaviour (#4170)

* util and util_test corrected

* fixed behaviour of fsondisk with test updated

* glob behaviour fixed in fsnode

* removed commented code

kyaml/filesys/fsnode.go

@@ -612,6 +612,7 @@ func (n *fsNode) RegExpGlob(pattern string) ([]string, error) {
 // This is how /bin/ls behaves.
 func (n *fsNode) Glob(pattern string) ([]string, error) {
 	var result []string
+	var allFiles []string
 	err := n.WalkMe(func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -622,7 +623,7 @@ func (n *fsNode) Glob(pattern string) ([]string, error) {
 				return err
 			}
 			if match {
-				result = append(result, path)
+				allFiles = append(allFiles, path)
 			}
 		}
 		return nil
@@ -630,6 +631,11 @@ func (n *fsNode) Glob(pattern string) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
+	if IsHiddenFilePath(pattern) {
+		result = allFiles
+	} else {
+		result = RemoveHiddenFiles(allFiles)
+	}
 	sort.Strings(result)
 	return result, nil
 }

kyaml/filesys/fsnode_test.go

@@ -464,6 +464,13 @@ var bunchOfFiles = []struct {
 	{
 		path: filepath.Join("b", "d", "a", "c", "u"),
 	},
+	{
+		path: filepath.Join("b", "d", ".hidden_file"),
+	},
+	{
+		path:     filepath.Join("b", "d", ".hidden_dir"),
+		addAsDir: true,
+	},
 }
 
 func makeLoadedFileTree(t *testing.T) *fsNode {
@@ -580,6 +587,7 @@ func TestExists(t *testing.T) {
 func TestRegExpGlob(t *testing.T) {
 	n := makeLoadedFileTree(t)
 	expected := []string{
+		filepath.Join("b", "d", ".hidden_file"),
 		filepath.Join("b", "d", "a", "c", "i", "beans"),
 		filepath.Join("b", "d", "a", "c", "m"),
 		filepath.Join("b", "d", "a", "c", "u"),
@@ -599,16 +607,36 @@ func TestRegExpGlob(t *testing.T) {
 
 func TestGlob(t *testing.T) {
 	n := makeLoadedFileTree(t)
-	expected := []string{
-		filepath.Join("b", "d", "x"),
-		filepath.Join("b", "d", "y"),
-		filepath.Join("b", "d", "z"),
+
+	tests := map[string]struct {
+		globPattern   string
+		expectedFiles []string
+	}{
+		"VisibleFiles": {
+			globPattern: "b/d/*",
+			expectedFiles: []string{
+				filepath.Join("b", "d", "x"),
+				filepath.Join("b", "d", "y"),
+				filepath.Join("b", "d", "z"),
+			},
+		},
+		"HiddenFiles": {
+			globPattern: "b/d/.*",
+			expectedFiles: []string{
+				filepath.Join("b", "d", ".hidden_file"),
+			},
+		},
 	}
-	paths, err := n.Glob("b/d/*")
-	if err != nil {
-		t.Fatalf("glob error: %v", err)
+
+	for test, c := range tests {
+		t.Run(test, func(t *testing.T) {
+			paths, err := n.Glob(c.globPattern)
+			if err != nil {
+				t.Fatalf("glob error: %v", err)
+			}
+			assertEqualStringSlices(t, c.expectedFiles, paths, "glob test")
+		})
 	}
-	assertEqualStringSlices(t, expected, paths, "glob test")
 }
 
 func assertEqualStringSlices(t *testing.T, expected, actual []string, message string) {

kyaml/filesys/fsondisk.go

@@ -88,7 +88,17 @@ func (fsOnDisk) Exists(name string) bool {
 
 // Glob returns the list of matching files
 func (fsOnDisk) Glob(pattern string) ([]string, error) {
-	return filepath.Glob(pattern)
+	var result []string
+	allFilePaths, err := filepath.Glob(pattern)
+	if err != nil {
+		return nil, err
+	}
+	if IsHiddenFilePath(pattern) {
+		result = allFilePaths
+	} else {
+		result = RemoveHiddenFiles(allFilePaths)
+	}
+	return result, nil
 }
 
 // IsDir delegates to os.Stat and FileInfo.IsDir

kyaml/filesys/fsondisk_test.go

@@ -12,6 +12,7 @@ import (
 	"path"
 	"path/filepath"
 	"reflect"
+	"sort"
 	"testing"
 )
 
@@ -136,31 +137,113 @@ func TestReadFilesRealFS(t *testing.T) {
 	fSys, testDir := makeTestDir(t)
 	defer os.RemoveAll(testDir)
 
-	err := fSys.WriteFile(path.Join(testDir, "foo"), []byte(`foo`))
-	if err != nil {
-		t.Fatalf("unexpected error %s", err)
+	dir := path.Join(testDir, "dir")
+	nestedDir := path.Join(dir, "nestedDir")
+	hiddenDir := path.Join(testDir, ".hiddenDir")
+	dirs := []string{
+		testDir,
+		dir,
+		nestedDir,
+		hiddenDir,
 	}
-	if !fSys.Exists(path.Join(testDir, "foo")) {
-		t.Fatalf("expected foo")
-	}
-	if fSys.IsDir(path.Join(testDir, "foo")) {
-		t.Fatalf("expected foo not to be a directory")
+	// all directories will have all these files
+	files := []string{
+		"bar",
+		"foo",
+		"file-1.xtn",
+		".file-2.xtn",
+		".some-file-3.xtn",
+		".some-file-4.xtn",
 	}
 
-	err = fSys.WriteFile(path.Join(testDir, "bar"), []byte(`bar`))
+	err := fSys.MkdirAll(nestedDir)
 	if err != nil {
-		t.Fatalf("unexpected error %s", err)
+		t.Fatalf("Unexpected Error %v\n", err)
+	}
+	err = fSys.MkdirAll(hiddenDir)
+	if err != nil {
+		t.Fatalf("Unexpected Error %v\n", err)
 	}
 
-	files, err := fSys.Glob(path.Join("testDir", "*"))
-	expected := []string{
-		path.Join(testDir, "bar"),
-		path.Join(testDir, "foo"),
+	// adding all files in every directory that we had defined
+	for _, d := range dirs {
+		if !fSys.IsDir(d) {
+			t.Fatalf("Expected %s to be a dir\n", d)
+		}
+		for _, f := range files {
+			err = fSys.WriteFile(path.Join(d, f), []byte(f))
+			if err != nil {
+				t.Fatalf("unexpected error %s", err)
+			}
+			if !fSys.Exists(path.Join(d, f)) {
+				t.Fatalf("expected %s", f)
+			}
+		}
 	}
-	if err != nil {
-		t.Fatalf("expected no error")
+
+	tests := map[string]struct {
+		globPattern   string
+		expectedFiles []string
+		expectedDirs  map[string][]string // glob returns directories as well, so we need to add those to expected files
+	}{
+		"AllVisibleFiles": {
+			globPattern: "*",
+			expectedFiles: []string{
+				"bar",
+				"foo",
+				"file-1.xtn",
+			},
+			expectedDirs: map[string][]string{
+				testDir: []string{dir},
+				dir:     []string{nestedDir},
+			},
+		},
+		"AllHiddenFiles": {
+			globPattern: ".*",
+			expectedFiles: []string{
+				".file-2.xtn",
+				".some-file-3.xtn",
+				".some-file-4.xtn",
+			},
+			expectedDirs: map[string][]string{
+				testDir: []string{hiddenDir},
+			},
+		},
+		"foo_File": {
+			globPattern: "foo",
+			expectedFiles: []string{
+				"foo",
+			},
+		},
+		"dotsome-file_PrefixedFiles": {
+			globPattern: ".some-file*",
+			expectedFiles: []string{
+				".some-file-3.xtn",
+				".some-file-4.xtn",
+			},
+		},
 	}
-	if reflect.DeepEqual(files, expected) {
-		t.Fatalf("incorrect files found by glob: %v", files)
+
+	for n, c := range tests {
+		t.Run(n, func(t *testing.T) {
+			for _, d := range dirs {
+				var expectedPaths []string
+				for _, f := range c.expectedFiles {
+					expectedPaths = append(expectedPaths, path.Join(d, f))
+				}
+				if c.expectedDirs != nil {
+					expectedPaths = append(expectedPaths, c.expectedDirs[d]...)
+				}
+				actualPaths, globErr := fSys.Glob(path.Join(d, c.globPattern))
+				if globErr != nil {
+					t.Fatalf("Unexpected Error : %v\n", globErr)
+				}
+				sort.Strings(actualPaths)
+				sort.Strings(expectedPaths)
+				if !reflect.DeepEqual(actualPaths, expectedPaths) {
+					t.Fatalf("incorrect files found by glob: expected=%v, actual=%v", expectedPaths, actualPaths)
+				}
+			}
+		})
 	}
 }

kyaml/filesys/util.go

@@ -123,3 +123,21 @@ func InsertPathPart(path string, pos int, part string) string {
 	result[pos] = part
 	return PathJoin(append(result, parts[pos:]...))
 }
+
+func IsHiddenFilePath(pattern string) bool {
+	return strings.HasPrefix(filepath.Base(pattern), ".")
+}
+
+// Removes paths containing hidden files/folders from a list of paths
+func RemoveHiddenFiles(paths []string) []string {
+	if len(paths) == 0 {
+		return paths
+	}
+	var result []string
+	for _, path := range paths {
+		if !IsHiddenFilePath(path) {
+			result = append(result, path)
+		}
+	}
+	return result
+}

kyaml/filesys/util_test.go

@@ -9,6 +9,7 @@ package filesys
 import (
 	"os"
 	"path/filepath"
+	"reflect"
 	"testing"
 )
 
@@ -376,3 +377,90 @@ func TestStripLeadingSeps(t *testing.T) {
 		}
 	}
 }
+
+func TestIsHiddenFilePath(t *testing.T) {
+	tests := map[string]struct {
+		paths        []string
+		expectHidden bool
+	}{
+		"hiddenGlobs": {
+			expectHidden: true,
+			paths: []string{
+				".*",
+				"/.*",
+				"dir/.*",
+				"dir1/dir2/dir3/.*",
+				"../../.*",
+				"../../dir/.*",
+			},
+		},
+		"visibleGlobes": {
+			expectHidden: false,
+			paths: []string{
+				"*",
+				"/*",
+				"dir/*",
+				"dir1/dir2/dir3/*",
+				"../../*",
+				"../../dir/*",
+			},
+		},
+		"hiddenFiles": {
+			expectHidden: true,
+			paths: []string{
+				".root_file.xtn",
+				"/.file_1.xtn",
+				"dir/.file_2.xtn",
+				"dir1/dir2/dir3/.file_3.xtn",
+				"../../.file_4.xtn",
+				"../../dir/.file_5.xtn",
+			},
+		},
+		"visibleFiles": {
+			expectHidden: false,
+			paths: []string{
+				"root_file.xtn",
+				"/file_1.xtn",
+				"dir/file_2.xtn",
+				"dir1/dir2/dir3/file_3.xtn",
+				"../../file_4.xtn",
+				"../../dir/file_5.xtn",
+			},
+		},
+	}
+	for n, c := range tests {
+		t.Run(n, func(t *testing.T) {
+			for _, path := range c.paths {
+				actual := IsHiddenFilePath(path)
+				if actual != c.expectHidden {
+					t.Fatalf("For file path %q, expected hidden: %v, got hidden: %v", path, c.expectHidden, actual)
+				}
+			}
+		})
+	}
+}
+
+func TestRemoveHiddenFiles(t *testing.T) {
+	paths := []string{
+		"file1.xtn",
+		".file2.xtn",
+		"dir/fa1",
+		"dir/fa2",
+		"dir/.fa3",
+		"../../.fa4",
+		"../../fa5",
+		"../../dir/fa6",
+		"../../dir/.fa7",
+	}
+	result := RemoveHiddenFiles(paths)
+	expected := []string{
+		"file1.xtn",
+		"dir/fa1",
+		"dir/fa2",
+		"../../fa5",
+		"../../dir/fa6",
+	}
+	if !reflect.DeepEqual(result, expected) {
+		t.Fatalf("Hidden dirs not correctly removed, expected %v but got %v\n", expected, result)
+	}
+}