Merge pull request #2941 from mortent/FixIgnoreFilesMatcher

Fix issue where ignoreFilesMatcher doesn't work correctly

kyaml/kio/ignorefilesmatcher.go

@@ -12,7 +12,7 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/ext"
 )
 
-// IgnoreFilesMatcher handles `.krmignore` files, which allows for ignoring
+// ignoreFilesMatcher handles `.krmignore` files, which allows for ignoring
 // files or folders in a package. The format of this file is a subset of the
 // gitignore format, with recursive patterns (like a/**/c) not supported. If a
 // file or folder matches any of the patterns in the .krmignore file for the
@@ -30,14 +30,15 @@ import (
 //   package contains a pattern that ignores the directory foo, if foo is a
 //   subpackage, it will still be included if the IncludeSubpackages property
 //   is set to true
-type IgnoreFilesMatcher struct {
+type ignoreFilesMatcher struct {
 	matchers []matcher
 }
 
 // readIgnoreFile checks whether there is a .krmignore file in the path, and
 // if it is, reads it in and turns it into a matcher. If we can't find a file,
 // we just add a matcher that match nothing.
-func (i *IgnoreFilesMatcher) readIgnoreFile(path string) error {
+func (i *ignoreFilesMatcher) readIgnoreFile(path string) error {
+	i.verifyPath(path)
 	m, err := gitignore.NewGitIgnore(filepath.Join(path, ext.GetIgnoreFileName()))
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -60,11 +61,11 @@ func (i *IgnoreFilesMatcher) readIgnoreFile(path string) error {
 // is correct for the provided filepath. Matchers are removed once
 // we encounter a filepath that is not a subpath of the basepath for
 // the matcher.
-func (i *IgnoreFilesMatcher) verifyPath(path string) {
+func (i *ignoreFilesMatcher) verifyPath(path string) {
 	for j := len(i.matchers) - 1; j >= 0; j-- {
 		matcher := i.matchers[j]
-		if !strings.HasPrefix(path, matcher.basePath) {
+		if strings.HasPrefix(path, matcher.basePath) || path == matcher.basePath {
-			i.matchers = i.matchers[:j]
+			i.matchers = i.matchers[:j+1]
 			return
 		}
 	}
@@ -72,17 +73,17 @@ func (i *IgnoreFilesMatcher) verifyPath(path string) {
 
 // matchFile checks whether the file given by the provided path matches
 // any of the patterns in the .krmignore file for the package.
-func (i *IgnoreFilesMatcher) matchFile(path string) bool {
+func (i *ignoreFilesMatcher) matchFile(path string) bool {
 	if len(i.matchers) == 0 {
 		return false
 	}
-	i.verifyPath(path)
+	i.verifyPath(filepath.Dir(path))
 	return i.matchers[len(i.matchers)-1].matcher.Match(path, false)
 }
 
 // matchFile checks whether the directory given by the provided path matches
 // any of the patterns in the .krmignore file for the package.
-func (i *IgnoreFilesMatcher) matchDir(path string) bool {
+func (i *ignoreFilesMatcher) matchDir(path string) bool {
 	if len(i.matchers) == 0 {
 		return false
 	}

kyaml/kio/ignorefilesmatcher_test.go

@@ -53,7 +53,7 @@ testfile.yaml
 				assert.FailNow(t, err.Error())
 			}
 
-			ignoreFilesMatcher := IgnoreFilesMatcher{}
+			ignoreFilesMatcher := ignoreFilesMatcher{}
 			err = ignoreFilesMatcher.readIgnoreFile(dir)
 			if !assert.NoError(t, err) {
 				t.FailNow()
@@ -165,6 +165,52 @@ a_test.yaml
 				`b: b`,
 			},
 		},
+		{
+			name: "handles a combination of packages and directories",
+			directories: []string{
+				filepath.Join("a"),
+				filepath.Join("d", "e"),
+				filepath.Join("f"),
+			},
+			files: map[string][]byte{
+				filepath.Join("pkgFile"):                 {},
+				filepath.Join("d", "pkgFile"):            {},
+				filepath.Join("d", "e", "pkgFile"):       {},
+				filepath.Join("f", "pkgFile"):            {},
+				filepath.Join("manifest.yaml"):           []byte(`root: root`),
+				filepath.Join("a", "manifest.yaml"):      []byte(`a: a`),
+				filepath.Join("d", "manifest.yaml"):      []byte(`d: d`),
+				filepath.Join("d", "e", "manifest.yaml"): []byte(`e: e`),
+				filepath.Join("f", "manifest.yaml"):      []byte(`f: f`),
+				filepath.Join("d", ".krmignore"): []byte(`
+manifest.yaml
+`),
+			},
+			expected: []string{
+				`a: a`,
+				`e: e`,
+				`f: f`,
+				`root: root`,
+			},
+		},
+		{
+			name: "ignore file can exclude subpackages",
+			directories: []string{
+				filepath.Join("a"),
+			},
+			files: map[string][]byte{
+				filepath.Join("pkgFile"):            {},
+				filepath.Join("a", "pkgFile"):       {},
+				filepath.Join("manifest.yaml"):      []byte(`root: root`),
+				filepath.Join("a", "manifest.yaml"): []byte(`a: a`),
+				filepath.Join(".krmignore"): []byte(`
+a
+`),
+			},
+			expected: []string{
+				`root: root`,
+			},
+		},
 	}
 
 	for i := range testCases {

kyaml/kio/pkgio_reader.go

@@ -186,7 +186,7 @@ func (r LocalPackageReader) Read() ([]*yaml.RNode, error) {
 	var operand ResourceNodeSlice
 	var pathRelativeTo string
 	var err error
-	ignoreFilesMatcher := &IgnoreFilesMatcher{}
+	ignoreFilesMatcher := &ignoreFilesMatcher{}
 	r.PackagePath, err = filepath.Abs(r.PackagePath)
 	if err != nil {
 		return nil, errors.Wrap(err)
@@ -213,9 +213,9 @@ func (r LocalPackageReader) Read() ([]*yaml.RNode, error) {
 
 		// check if we should skip the directory or file
 		if info.IsDir() {
-			return r.ShouldSkipDir(path, ignoreFilesMatcher)
+			return r.shouldSkipDir(path, ignoreFilesMatcher)
 		}
-		if match, err := r.ShouldSkipFile(path, ignoreFilesMatcher); err != nil {
+		if match, err := r.shouldSkipFile(path, ignoreFilesMatcher); err != nil {
 			return err
 		} else if !match {
 			// skip this file
@@ -257,8 +257,8 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode
 	return rr.Read()
 }
 
-// ShouldSkipFile returns true if the file should be skipped
+// shouldSkipFile returns true if the file should be skipped
-func (r *LocalPackageReader) ShouldSkipFile(path string, matcher *IgnoreFilesMatcher) (bool, error) {
+func (r *LocalPackageReader) shouldSkipFile(path string, matcher *ignoreFilesMatcher) (bool, error) {
 	// check if the file is covered by a .krmignore file.
 	if matcher.matchFile(path) {
 		return false, nil
@@ -285,23 +285,18 @@ func (r *LocalPackageReader) initReaderAnnotations(path string, _ os.FileInfo) {
 	}
 }
 
-// ShouldSkipDir returns a filepath.SkipDir if the directory should be skipped
+// shouldSkipDir returns a filepath.SkipDir if the directory should be skipped
-func (r *LocalPackageReader) ShouldSkipDir(path string, matcher *IgnoreFilesMatcher) error {
+func (r *LocalPackageReader) shouldSkipDir(path string, matcher *ignoreFilesMatcher) error {
-	if r.PackageFileName == "" {
-		// If the folder is not a package, but covered by the .krmignore file,
-		// we skip it.
 	if matcher.matchDir(path) {
 		return filepath.SkipDir
 	}
+
+	if r.PackageFileName == "" {
 		return nil
 	}
 	// check if this is a subpackage
 	_, err := os.Stat(filepath.Join(path, r.PackageFileName))
 	if os.IsNotExist(err) {
-		// Skip the folder if it is covered by the .krmignore file.
-		if matcher.matchDir(path) {
-			return filepath.SkipDir
-		}
 		return nil
 	} else if err != nil {
 		return errors.Wrap(err)
@@ -309,9 +304,5 @@ func (r *LocalPackageReader) ShouldSkipDir(path string, matcher *IgnoreFilesMatc
 	if !r.IncludeSubpackages {
 		return filepath.SkipDir
 	}
-	// We don't allow the .krmignore file in a package cause us to skip
-	// a subpackage. So if we have found a package file in the folder and
-	// we should include subpackages, we don't check the .krmignore file. We
-	// do however check whether the package contains a .krmignore file.
 	return matcher.readIgnoreFile(path)
 }