Repository: rancher/fleet
Pull request #1634 (2023-07-05 10:06:22)

Support exclusion of files and directories from bundles
This provides basic support for `.fleetignore` files, in the same
fashion as .gitignore, so that specific files or paths can be excluded
instead of enforcing a blanket exclusion of non-YAML files.

Basic support means that the following is supported:
* Glob syntax is used to match files or directories, using Golang's
      [`filepath.Match`](https://pkg.go.dev/path/filepath#Match)
* Empty lines are skipped, and can therefore be used to improve
      readability
* Comments, ie lines starting with unescaped `#`, are skipped
* A given line can match a file or a directory, even if no separator
      is provided (eg. `subdir/*` and `subdir` are both valid
`.fleetignore` lines)
* A match may be found for a file or directory at any level below
      the directory where a `.fleetignore` lives, ie `foo.yaml` will
    match `./foo.yaml` as well as `./path/to/foo.yaml`
* Multiple `.fleetignore` files are supported, in a similar fashion to how `.gitignore` works.

Current limitations:
* Double asterisks (`**`) and explicit inclusions with `!` are not
      supported.

Refers to #583

## Test

To test this pull request, you can run the following commands:

```shell
go test ./internal/bundlereader
```

## Additional Information

### Potential improvement

* Support for other `.gitignore`-like features (see unsupported features above)

<!--
@rancher/qa

## Additional QA
### Issue: #583
 
### Problem
< Describe the root cause of the issue you are resolving. This may include what behavior is observed and why it is not desirable. If this is a new feature describe why we need this feature and how it will be used. >
 
### Solution
< Describe what you changed to fix the issue. Relate your changes back to the original issue / feature and explain why this addresses the issue.>
 
### Testing
< Note: Confirm if the repro steps in the GitHub issue are valid, if not, please update the issue with accurate repro steps. >

### Engineering Testing
#### Manual Testing
< Describe what manual testing you did (if no testing was done, explain why). >

#### Automated Testing
<If you added/updated unit/integration/validation tests, describe what cases they cover and do not cover. >

### QA Testing Considerations
< Highlight areas or (additional) cases that QA should test w.r.t a fresh install as well as the upgrade scenarios >
 
#### Regressions Considerations
< Dedicated section to specifically call out any areas that with higher chance of regressions caused by this change, include estimation of probability of regressions >

-->



Patch:
diff --git a/internal/bundlereader/loaddirectory.go b/internal/bundlereader/loaddirectory.go
index 0c97a954..994e84e1 100644
--- a/internal/bundlereader/loaddirectory.go
+++ b/internal/bundlereader/loaddirectory.go
@@ -1,15 +1,18 @@
 package bundlereader
 
 import (
+	"bufio"
 	"context"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/base64"
 	"fmt"
+	"io/fs"
 	"net/http"
 	"net/url"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"unicode/utf8"
 
@@ -23,10 +26,133 @@ import (
 	"helm.sh/helm/v3/pkg/registry"
 )
 
+// ignoreTree represents a tree of ignored paths (read from .fleetignore files), each node being a directory.
+// It provides a means for ignored paths to be propagated down the tree, but not between subdirectories of a same
+// directory.
+type ignoreTree struct {
+	path         string
+	ignoredPaths []string
+	children     []*ignoreTree
+}
+
+// isIgnored checks whether any path within xt matches path, and returns true if so.
+func (xt *ignoreTree) isIgnored(path string) (bool, error) {
+	steps := xt.findNode(path, false, nil)
+
+	for _, step := range steps {
+		for _, ignoredPath := range step.ignoredPaths {
+			toIgnore, err := filepath.Match(ignoredPath, filepath.Base(path))
+			if err != nil {
+				return false, err
+			}
+
+			if toIgnore {
+				return true, nil
+			}
+		}
+	}
+
+	return false, nil
+}
+
+// addNode reads a `.fleetignore` file in dir's root and adds each of its entries to ignored paths for dir.
+// Returns an error if a `.fleetignore` file exists for dir but reading it fails.
+func (xt *ignoreTree) addNode(dir string) error {
+	toIgnore, err := readFleetIgnore(dir)
+	if err != nil {
+		return fmt.Errorf("read .fleetignore for %s: %v", dir, err)
+	}
+
+	if len(toIgnore) == 0 {
+		return nil
+	}
+
+	steps := xt.findNode(dir, true, nil)
+	if steps == nil {
+		return fmt.Errorf("ignore tree node not found for path %q", dir)
+	}
+
+	destNode := steps[len(steps)-1]
+	destNode.ignoredPaths = append(destNode.ignoredPaths, toIgnore...)
+
+	return nil
+}
+
+// findNode finds the right node for path, creating that node if needed and if isDir is true.
+// Returns a slice representing all relevant nodes in the path to the destination, in order of traversal from the root.
+// The last element of that slice is the destination node.
+func (xt *ignoreTree) findNode(path string, isDir bool, nodesRoute []*ignoreTree) []*ignoreTree {
+	if path == xt.path {
+		return append(nodesRoute, xt)
+	}
+
+	// The path doesn't even belong in the tree. This should never happen.
+	if !strings.HasPrefix(path, xt.path) {
+		return nil
+	}
+
+	for _, c := range xt.children {
+		if steps := c.findNode(path, isDir, nodesRoute); steps != nil {
+			crossed := append(nodesRoute, steps...)
+
+			return crossed
+		}
+	}
+
+	if isDir {
+		xt.children = append(xt.children, &ignoreTree{path: path})
+
+		createdChild := xt.children[len(xt.children)-1]
+
+		return append(nodesRoute, createdChild)
+	}
+
+	return append(nodesRoute, xt)
+}
+
+// readFleetIgnore reads a possible .fleetignore file within path and returns its entries as a slice of strings.
+// If no .fleetignore exists, then an empty slice and a nil error are returned.
+// If an error happens while opening an existing .fleetignore file, that error is returned along with an empty slice.
+func readFleetIgnore(path string) ([]string, error) {
+	file, err := os.Open(filepath.Join(path, ".fleetignore"))
+	if err != nil {
+		// No ignored paths to add if no .fleetignore exists.
+		if os.IsNotExist(err) {
+			return nil, nil
+		}
+		return nil, err
+	}
+
+	defer file.Close()
+
+	scanner := bufio.NewScanner(file)
+	scanner.Split(bufio.ScanLines)
+
+	var ignored []string
+
+	trailingSpaceRegex := regexp.MustCompile(`([^\\])\s+$`)
+
+	for scanner.Scan() {
+		path := scanner.Text()
+
+		// Trim trailing spaces unless escaped.
+		path = trailingSpaceRegex.ReplaceAllString(path, "$1")
+
+		// Ignore empty lines and comments (although they should not match any file).
+		if path == "" || strings.HasPrefix(path, "#") {
+			continue
+		}
+
+		ignored = append(ignored, path)
+	}
+
+	return ignored, nil
+}
+
 func loadDirectory(ctx context.Context, compress bool, prefix, base, source, version string, auth Auth) ([]fleet.BundleResource, error) {
 	var resources []fleet.BundleResource
 
-	files, err := getContent(ctx, base, source, version, auth)
+	files, err := GetContent(ctx, base, source, version, auth)
 	if err != nil {
 		return nil, err
 	}
@@ -52,8 +178,8 @@ func loadDirectory(ctx context.Context, compress bool, prefix, base, source, ver
 	return resources, nil
 }
 
-// getContent uses go-getter (and helm for oci) to read the files from directories and servers
-func getContent(ctx context.Context, base, source, version string, auth Auth) (map[string][]byte, error) {
+// GetContent uses go-getter (and Helm for OCI) to read the files from directories and servers.
+func GetContent(ctx context.Context, base, source, version string, auth Auth) (map[string][]byte, error) {
 	temp, err := os.MkdirTemp("", "fleet")
 	if err != nil {
 		return nil, err
@@ -120,23 +246,37 @@ func getContent(ctx context.Context, base, source, version string, auth Auth) (m
 		temp = dest
 	}
 
-	err = filepath.Walk(temp, func(path string, info os.FileInfo, err error) error {
+	ignoredPaths := ignoreTree{path: temp}
+
+	err = filepath.WalkDir(temp, func(path string, info fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		name, err := filepath.Rel(temp, path)
+		if err != nil {
+			return err
+		}
+
+		ignore, err := ignoredPaths.isIgnored(path)
 		if err != nil {
 			return err
 		}
 
 		if info.IsDir() {
-			if strings.HasPrefix(filepath.Base(path), ".") {
+			// Skip .fleetignore'd and hidden directories
+			if ignore || strings.HasPrefix(filepath.Base(path), ".") {
 				return filepath.SkipDir
 			}
-			return nil
+
+			return ignoredPaths.addNode(path)
 		}
 
-		name, err := filepath.Rel(temp, path)
-		if err != nil {
-			return err
+		if ignore {
+			return nil
 		}
 
+		// Skip hidden files
 		if strings.HasPrefix(filepath.Base(name), ".") {
 			return nil
 		}


Test patch:
diff --git a/internal/bundlereader/loaddirectory_test.go b/internal/bundlereader/loaddirectory_test.go
new file mode 100644
index 00000000..fabe4573
--- /dev/null
+++ b/internal/bundlereader/loaddirectory_test.go
@@ -0,0 +1,310 @@
+package bundlereader_test
+
+import (
+	"context"
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/rancher/fleet/internal/bundlereader"
+)
+
+// fsNode represents a directory structure used to model `.fleetignore` test cases.
+type fsNode struct {
+	name string
+
+	contents string   // if not a directory
+	children []fsNode // non-empty only in case of a directory
+
+	isDir bool
+}
+
+func TestGetContent(t *testing.T) {
+	cases := []struct {
+		name               string
+		directoryStructure fsNode
+		expectedFiles      map[string][]byte
+	}{
+		{
+			name: "no .fleetignore",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "no-fleetignore",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml": []byte("foo"),
+			},
+		},
+		{
+			name: "empty .fleetignore",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "empty-fleetignore",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "",
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml": []byte("foo"),
+			},
+		},
+		{
+			name: "ignore lines with leading # unless escaped",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "comments",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:     "#something_else.yaml",
+						contents: "bar",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "#something.yaml\n\\#something_else.yaml",
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml": []byte("foo"),
+			},
+		},
+		{
+			name: "simple .fleetignore",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "simple-fleetignore",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:     "something_else.yaml",
+						contents: "bar",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "not_here.txt\nsomething.yaml",
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something_else.yaml": []byte("bar"),
+			},
+		},
+		{
+			name: "glob syntax",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "glob-syntax",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "something*",
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{},
+		},
+		{
+			name: "ignore trailing spaces unless escaped",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "trim-space",
+				children: []fsNode{
+					{
+						name:     "something.yaml ",
+						contents: "foo",
+					},
+					{
+						name:     "something_else.yaml  ",
+						contents: "bar",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "something_else.yaml\\ \\ \nsomething.yaml ",
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml ": []byte("foo"),
+			},
+		},
+		{
+			name: "ignore directories",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "ignore-directories",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "subdir",
+					},
+					{
+						name:  "subdir",
+						isDir: true,
+						children: []fsNode{
+							{
+								name:     "in_dir.yaml",
+								contents: "baz",
+							},
+						},
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml": []byte("foo"),
+			},
+		},
+		{
+			name: "ignore file multiple levels below .fleetignore",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "ignore-file-multiple-levels",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:     ".fleetignore",
+						contents: "in_dir.yaml",
+					},
+					{
+						name:  "subdir",
+						isDir: true,
+						children: []fsNode{
+							{
+								name:  "subsubdir",
+								isDir: true,
+								children: []fsNode{
+									{
+										name:     "in_dir.yaml",
+										contents: "bar",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml": []byte("foo"),
+			},
+		},
+		{
+			name: ".fleetignore files in neighbour dirs do not interfere with one another",
+			directoryStructure: fsNode{
+				isDir: true,
+				name:  "multiple-files-same-level",
+				children: []fsNode{
+					{
+						name:     "something.yaml",
+						contents: "foo",
+					},
+					{
+						name:  "subdir1",
+						isDir: true,
+						children: []fsNode{
+							{
+								name:     "in_dir.yaml",
+								contents: "from dir 1",
+							},
+							{
+								name:     ".fleetignore",
+								contents: "in_dir.yaml",
+							},
+						},
+					},
+					{
+						name:  "subdir2",
+						isDir: true,
+						children: []fsNode{
+							{
+								name:     "in_dir.yaml",
+								contents: "from dir 2",
+							},
+						},
+					},
+				},
+			},
+			expectedFiles: map[string][]byte{
+				"something.yaml":      []byte("foo"),
+				"subdir2/in_dir.yaml": []byte("from dir 2"),
+			},
+		},
+	}
+
+	base, err := os.MkdirTemp("", "test-fleet")
+	require.NoError(t, err)
+
+	defer os.RemoveAll(base)
+
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+
+			root := createDirStruct(t, base, c.directoryStructure)
+
+			files, err := bundlereader.GetContent(context.Background(), root, root, "", bundlereader.Auth{})
+			assert.NoError(t, err)
+
+			assert.Equal(t, len(c.expectedFiles), len(files))
+			for k, v := range c.expectedFiles {
+				assert.Equal(t, v, files[k])
+			}
+		})
+	}
+}
+
+// createDirStruct generates and populates a directory structure which root is node, placing it at basePath.
+func createDirStruct(t *testing.T, basePath string, node fsNode) string {
+	path := filepath.Join(basePath, node.name)
+
+	if !node.isDir {
+		f, err := os.Create(path)
+		require.NoError(t, err)
+
+		_, err = io.WriteString(f, node.contents)
+		require.NoError(t, err)
+
+		return ""
+	}
+
+	err := os.Mkdir(path, 0777)
+	require.NoError(t, err)
+
+	for _, c := range node.children {
+		createDirStruct(t, path, c)
+	}
+
+	return path
+}