Make Storage#Archive file filtering configurable

This commit makes the filtering applied during the archiving
configurable by introducing an optional `ArchiveFileFilter`
callback argument and a `SourceIgnoreFilter` implementation.

`SourceIgnoreFilter` filters out files matching
sourceignore.VCSPatterns and any of the provided patterns.
If an empty gitignore.Pattern slice is given, the matcher is set to
sourceignore.NewDefaultMatcher.

The `GitRepository` now loads the ignore patterns before archiving
the repository contents by calling `sourceignore.LoadIgnorePatterns`
and other helpers. The loading behavior is **breaking** as
`.sourceignore` files in the (subdirectories of the) repository are
now still taken into account if `spec.ignore` for a resource is
defined, overwriting is still possible by creating an overwriting
rule in the `spec.ignore` of the resource.

This change also makes it possible for the `BucketReconciler` to not
configure a callback at all and prevent looking for ignore
matches twice. To finalize the bucket refactor, a change to the
reconciler has been made to look for a `.sourceignore` file in
the root of the bucket to provide an additional way of configuring
(global) exclusions.

Signed-off-by: Hidde Beydals <hello@hidde.co>

controllers/bucket_controller.go

@@ -17,7 +17,6 @@ limitations under the License.
 package controllers
 
 import (
-	"bytes"
 	"context"
 	"crypto/sha1"
 	"fmt"
@@ -204,7 +203,23 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
 		return sourcev1.BucketNotReady(bucket, sourcev1.BucketOperationFailedReason, err.Error()), err
 	}
 
-	ps := sourceignore.GetPatterns(bytes.NewBufferString(*bucket.Spec.Ignore), nil)
+	// Look for file with ignore rules first
+	// NB: S3 has flat filepath keys making it impossible to look
+	// for files in "subdirectories" without building up a tree first.
+	path := filepath.Join(tempDir, sourceignore.IgnoreFile)
+	if err := s3Client.FGetObject(ctxTimeout, bucket.Spec.BucketName, sourceignore.IgnoreFile, path, minio.GetObjectOptions{}); err != nil {
+		if resp, ok := err.(minio.ErrorResponse); ok && resp.Code != "NoSuchKey" {
+			return sourcev1.BucketNotReady(bucket, sourcev1.BucketOperationFailedReason, err.Error()), err
+		}
+	}
+	ps, err := sourceignore.ReadIgnoreFile(path, nil)
+	if err != nil {
+		return sourcev1.BucketNotReady(bucket, sourcev1.BucketOperationFailedReason, err.Error()), err
+	}
+	// In-spec patterns take precedence
+	if bucket.Spec.Ignore != nil {
+		ps = append(ps, sourceignore.ReadPatterns(strings.NewReader(*bucket.Spec.Ignore), nil)...)
+	}
 	matcher := sourceignore.NewMatcher(ps)
 
 	// download bucket content
@@ -217,7 +232,7 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
 			return sourcev1.BucketNotReady(bucket, sourcev1.BucketOperationFailedReason, err.Error()), err
 		}
 
-		if strings.HasSuffix(object.Key, "/") {
+		if strings.HasSuffix(object.Key, "/") || object.Key == sourceignore.IgnoreFile {
 			continue
 		}
 
@@ -264,7 +279,7 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
 	defer unlock()
 
 	// archive artifact and check integrity
-	if err := r.Storage.Archive(&artifact, tempDir, bucket.Spec.Ignore); err != nil {
+	if err := r.Storage.Archive(&artifact, tempDir, nil); err != nil {
 		err = fmt.Errorf("storage archive error: %w", err)
 		return sourcev1.BucketNotReady(bucket, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}

controllers/gitrepository_controller.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -45,6 +46,7 @@ import (
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
 	"github.com/fluxcd/source-controller/pkg/git"
 	"github.com/fluxcd/source-controller/pkg/git/strategy"
+	"github.com/fluxcd/source-controller/pkg/sourceignore"
 )
 
 // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories,verbs=get;list;watch;create;update;patch;delete
@@ -270,7 +272,15 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour
 	defer unlock()
 
 	// archive artifact and check integrity
-	if err := r.Storage.Archive(&artifact, tmpGit, repository.Spec.Ignore); err != nil {
+	ps, err := sourceignore.LoadIgnorePatterns(tmpGit, nil)
+	if err != nil {
+		err = fmt.Errorf(".sourceignore error: %w", err)
+		return sourcev1.GitRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
+	}
+	if repository.Spec.Ignore != nil {
+		ps = append(ps, sourceignore.ReadPatterns(strings.NewReader(*repository.Spec.Ignore), nil)...)
+	}
+	if err := r.Storage.Archive(&artifact, tmpGit, SourceIgnoreFilter(ps, nil)); err != nil {
 		err = fmt.Errorf("storage archive error: %w", err)
 		return sourcev1.GitRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}

controllers/storage.go

@@ -40,14 +40,6 @@ import (
 	"github.com/fluxcd/source-controller/pkg/sourceignore"
 )
 
-const (
-	excludeFile  = ".sourceignore"
-	excludeVCS   = ".git/,.gitignore,.gitmodules,.gitattributes"
-	excludeExt   = "*.jpg,*.jpeg,*.gif,*.png,*.wmv,*.flv,*.tar.gz,*.zip"
-	excludeCI    = ".github/,.circleci/,.travis.yml,.gitlab-ci.yml,appveyor.yml,.drone.yml,cloudbuild.yaml,codeship-services.yml,codeship-steps.yml"
-	excludeExtra = "**/.goreleaser.yml,**/.sops.yaml,**/.flux.yaml"
-)
-
 // Storage manages artifacts
 type Storage struct {
 	// BasePath is the local directory path where the source artifacts are stored.
@@ -150,20 +142,36 @@ func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool {
 	return fi.Mode().IsRegular()
 }
 
-// Archive atomically archives the given directory as a tarball to the given v1beta1.Artifact
+// ArchiveFileFilter must return true if a file should not be included
-// path, excluding any VCS specific files and directories, or any of the excludes defined in
+// in the archive after inspecting the given path and/or os.FileInfo.
-// the excludeFiles. If successful, it sets the checksum and last update time on the artifact.
+type ArchiveFileFilter func(p string, fi os.FileInfo) bool
-func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, ignore *string) (err error) {
+
+// SourceIgnoreFilter returns an ArchiveFileFilter that filters out
+// files matching sourceignore.VCSPatterns and any of the provided
+// patterns. If an empty gitignore.Pattern slice is given, the matcher
+// is set to sourceignore.NewDefaultMatcher.
+func SourceIgnoreFilter(ps []gitignore.Pattern, domain []string) ArchiveFileFilter {
+	matcher := sourceignore.NewDefaultMatcher(ps, domain)
+	if len(ps) > 0 {
+		ps = append(sourceignore.VCSPatterns(domain), ps...)
+		matcher = sourceignore.NewMatcher(ps)
+	}
+	return func(p string, fi os.FileInfo) bool {
+		// The directory is always false as the archiver does already skip
+		// directories.
+		return matcher.Match(strings.Split(p, string(filepath.Separator)), false)
+	}
+}
+
+// Archive atomically archives the given directory as a tarball to the
+// given v1beta1.Artifact path, excluding directories and any
+// ArchiveFileFilter matches. If successful, it sets the checksum and
+// last update time on the artifact.
+func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, filter ArchiveFileFilter) (err error) {
 	if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() {
 		return fmt.Errorf("invalid dir path: %s", dir)
 	}
 
-	ps, err := sourceignore.LoadExcludePatterns(dir, ignore)
-	if err != nil {
-		return err
-	}
-	matcher := sourceignore.NewMatcher(ps)
-
 	localPath := s.LocalPath(*artifact)
 	tf, err := ioutil.TempFile(filepath.Split(localPath))
 	if err != nil {
@@ -181,7 +189,52 @@ func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, ignore *strin
 
 	gw := gzip.NewWriter(mw)
 	tw := tar.NewWriter(gw)
-	if err := writeToArchiveExcludeMatches(dir, matcher, tw); err != nil {
+	if err := filepath.Walk(dir, func(p string, fi os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// Ignore anything that is not a file (directories, symlinks)
+		if !fi.Mode().IsRegular() {
+			return nil
+		}
+
+		// Skip filtered files
+		if filter != nil && filter(p, fi) {
+			return nil
+		}
+
+		header, err := tar.FileInfoHeader(fi, p)
+		if err != nil {
+			return err
+		}
+		// The name needs to be modified to maintain directory structure
+		// as tar.FileInfoHeader only has access to the base name of the file.
+		// Ref: https://golang.org/src/archive/tar/common.go?#L626
+		relFilePath := p
+		if filepath.IsAbs(dir) {
+			relFilePath, err = filepath.Rel(dir, p)
+			if err != nil {
+				return err
+			}
+		}
+		header.Name = relFilePath
+
+		if err := tw.WriteHeader(header); err != nil {
+			return err
+		}
+
+		f, err := os.Open(p)
+		if err != nil {
+			f.Close()
+			return err
+		}
+		if _, err := io.Copy(tw, f); err != nil {
+			f.Close()
+			return err
+		}
+		return f.Close()
+	}); err != nil {
 		tw.Close()
 		gw.Close()
 		tf.Close()
@@ -214,58 +267,6 @@ func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, ignore *strin
 	return nil
 }
 
-// writeToArchiveExcludeMatches walks over the given dir and writes any regular file that does
-// not match the given gitignore.Matcher.
-func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer *tar.Writer) error {
-	fn := func(p string, fi os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-
-		// Ignore anything that is not a file (directories, symlinks)
-		if !fi.Mode().IsRegular() {
-			return nil
-		}
-
-		// Ignore excluded extensions and files
-		if matcher.Match(strings.Split(p, "/"), false) {
-			return nil
-		}
-
-		header, err := tar.FileInfoHeader(fi, p)
-		if err != nil {
-			return err
-		}
-		// The name needs to be modified to maintain directory structure
-		// as tar.FileInfoHeader only has access to the base name of the file.
-		// Ref: https://golang.org/src/archive/tar/common.go?#L626
-		relFilePath := p
-		if filepath.IsAbs(dir) {
-			relFilePath, err = filepath.Rel(dir, p)
-			if err != nil {
-				return err
-			}
-		}
-		header.Name = relFilePath
-
-		if err := writer.WriteHeader(header); err != nil {
-			return err
-		}
-
-		f, err := os.Open(p)
-		if err != nil {
-			f.Close()
-			return err
-		}
-		if _, err := io.Copy(writer, f); err != nil {
-			f.Close()
-			return err
-		}
-		return f.Close()
-	}
-	return filepath.Walk(dir, fn)
-}
-
 // AtomicWriteFile atomically writes the io.Reader contents to the v1beta1.Artifact path.
 // If successful, it sets the checksum and last update time on the artifact.
 func (s *Storage) AtomicWriteFile(artifact *sourcev1.Artifact, reader io.Reader, mode os.FileMode) (err error) {

controllers/storage_test.go

@@ -1,3 +1,19 @@
+/*
+Copyright 2020, 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
 package controllers
 
 import (
@@ -7,28 +23,16 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"os/exec"
 	"path"
 	"path/filepath"
 	"testing"
 	"time"
 
+	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
+
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
 )
 
-type ignoreMap map[string]bool
-
-var remoteRepository = "https://github.com/fluxcd/source-controller"
-
-func init() {
-	// if this remote repo ever gets in your way, this is an escape; just set
-	// this to the url you want to clone. Be the source you want to be.
-	s := os.Getenv("REMOTE_REPOSITORY")
-	if s != "" {
-		remoteRepository = s
-	}
-}
-
 func createStoragePath() (string, error) {
 	return ioutil.TempDir("", "")
 }
@@ -67,16 +71,16 @@ func TestStorageConstructor(t *testing.T) {
 
 // walks a tar.gz and looks for paths with the basename. It does not match
 // symlinks properly at this time because that's painful.
-func walkTar(tarFile string, match string) (bool, error) {
+func walkTar(tarFile string, match string) (int64, bool, error) {
 	f, err := os.Open(tarFile)
 	if err != nil {
-		return false, fmt.Errorf("could not open file: %w", err)
+		return 0, false, fmt.Errorf("could not open file: %w", err)
 	}
 	defer f.Close()
 
 	gzr, err := gzip.NewReader(f)
 	if err != nil {
-		return false, fmt.Errorf("could not unzip file: %w", err)
+		return 0, false, fmt.Errorf("could not unzip file: %w", err)
 	}
 	defer gzr.Close()
 
@@ -86,177 +90,156 @@ func walkTar(tarFile string, match string) (bool, error) {
 		if err == io.EOF {
 			break
 		} else if err != nil {
-			return false, fmt.Errorf("Corrupt tarball reading header: %w", err)
+			return 0, false, fmt.Errorf("corrupt tarball reading header: %w", err)
 		}
 
 		switch header.Typeflag {
 		case tar.TypeDir, tar.TypeReg:
-			if filepath.Base(header.Name) == match {
+			if header.Name == match {
-				return true, nil
+				return header.Size, true, nil
 			}
 		default:
 			// skip
 		}
 	}
 
-	return false, nil
+	return 0, false, nil
 }
 
-func testPatterns(t *testing.T, storage *Storage, artifact sourcev1.Artifact, table ignoreMap) {
+func TestStorage_Archive(t *testing.T) {
-	for name, expected := range table {
+	dir, err := createStoragePath()
-		res, err := walkTar(storage.LocalPath(artifact), name)
+	if err != nil {
-		if err != nil {
+		t.Fatal(err)
-			t.Fatalf("while reading tarball: %v", err)
+	}
-		}
+	t.Cleanup(cleanupStoragePath(dir))
 
-		if res != expected {
+	storage, err := NewStorage(dir, "hostname", time.Minute)
-			if expected {
+	if err != nil {
-				t.Fatalf("Could not find repository file matching %q in tarball for repo %q", name, remoteRepository)
+		t.Fatalf("error while bootstrapping storage: %v", err)
-			} else {
+	}
-				t.Fatalf("Repository contained ignored file %q in tarball for repo %q", name, remoteRepository)
+
+	createFiles := func(files map[string][]byte) (dir string, err error) {
+		defer func() {
+			if err != nil && dir != "" {
+				os.RemoveAll(dir)
+			}
+		}()
+		dir, err = ioutil.TempDir("", "archive-test-files-")
+		if err != nil {
+			return
+		}
+		for name, b := range files {
+			absPath := filepath.Join(dir, name)
+			if err = os.MkdirAll(filepath.Dir(absPath), 0755); err != nil {
+				return
+			}
+			f, err := os.Create(absPath)
+			if err != nil {
+				return "", fmt.Errorf("could not create file %q: %w", absPath, err)
+			}
+			if n, err := f.Write(b); err != nil {
+				f.Close()
+				return "", fmt.Errorf("could not write %d bytes to file %q: %w", n, f.Name(), err)
+			}
+			f.Close()
+		}
+		return
+	}
+
+	matchFiles := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, files map[string][]byte) {
+		for name, b := range files {
+			mustExist := !(name[0:1] == "!")
+			if !mustExist {
+				name = name[1:]
+			}
+			s, exist, err := walkTar(storage.LocalPath(artifact), name)
+			if err != nil {
+				t.Fatalf("failed reading tarball: %v", err)
+			}
+			if bs := int64(len(b)); s != bs {
+				t.Fatalf("%q size %v != %v", name, s, bs)
+			}
+			if exist != mustExist {
+				if mustExist {
+					t.Errorf("could not find file %q in tarball", name)
+				} else {
+					t.Errorf("tarball contained excluded file %q", name)
+				}
 			}
 		}
 	}
-}
 
-func createArchive(t *testing.T, storage *Storage, filenames []string, sourceIgnore string, spec sourcev1.GitRepositorySpec) sourcev1.Artifact {
+	tests := []struct {
-	gitDir, err := ioutil.TempDir("", "")
+		name    string
-	if err != nil {
+		files   map[string][]byte
-		t.Fatalf("could not create temporary directory: %v", err)
+		filter  ArchiveFileFilter
+		want    map[string][]byte
+		wantErr bool
+	}{
+		{
+			name: "no filter",
+			files: map[string][]byte{
+				".git/config":   nil,
+				"file.jpg":      []byte(`contents`),
+				"manifest.yaml": nil,
+			},
+			filter: nil,
+			want: map[string][]byte{
+				".git/config":   nil,
+				"file.jpg":      []byte(`contents`),
+				"manifest.yaml": nil,
+			},
+		},
+		{
+			name: "exclude VCS",
+			files: map[string][]byte{
+				".git/config":   nil,
+				"manifest.yaml": nil,
+			},
+			filter: SourceIgnoreFilter(nil, nil),
+			want: map[string][]byte{
+				"!.git/config":  nil,
+				"manifest.yaml": nil,
+			},
+		},
+		{
+			name: "custom",
+			files: map[string][]byte{
+				".git/config": nil,
+				"custom":      nil,
+				"horse.jpg":   nil,
+			},
+			filter: SourceIgnoreFilter([]gitignore.Pattern{
+				gitignore.ParsePattern("custom", nil),
+			}, nil),
+			want: map[string][]byte{
+				"!git/config": nil,
+				"!custom":     nil,
+				"horse.jpg":   nil,
+			},
+			wantErr: false,
+		},
 	}
-	t.Cleanup(func() { os.RemoveAll(gitDir) })
+	for _, tt := range tests {
-
+		t.Run(tt.name, func(t *testing.T) {
-	if err := exec.Command("git", "clone", remoteRepository, gitDir).Run(); err != nil {
+			dir, err := createFiles(tt.files)
-		t.Fatalf("Could not clone remote repository: %v", err)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			defer os.RemoveAll(dir)
+			artifact := sourcev1.Artifact{
+				Path: filepath.Join(randStringRunes(10), randStringRunes(10), randStringRunes(10)+".tar.gz"),
+			}
+			if err := storage.MkdirAll(artifact); err != nil {
+				t.Fatalf("artifact directory creation failed: %v", err)
+			}
+			if err := storage.Archive(&artifact, dir, tt.filter); (err != nil) != tt.wantErr {
+				t.Errorf("Archive() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			matchFiles(t, storage, artifact, tt.want)
+		})
 	}
-
-	// inject files.. just empty files
-	for _, name := range filenames {
-		f, err := os.Create(filepath.Join(gitDir, name))
-		if err != nil {
-			t.Fatalf("Could not inject filename %q: %v", name, err)
-		}
-		f.Close()
-	}
-
-	// inject sourceignore if not empty
-	if sourceIgnore != "" {
-		si, err := os.Create(filepath.Join(gitDir, ".sourceignore"))
-		if err != nil {
-			t.Fatalf("Could not create .sourceignore: %v", err)
-		}
-
-		if _, err := io.WriteString(si, sourceIgnore); err != nil {
-			t.Fatalf("Could not write to .sourceignore: %v", err)
-		}
-
-		si.Close()
-	}
-	artifact := sourcev1.Artifact{
-		Path: filepath.Join(randStringRunes(10), randStringRunes(10), randStringRunes(10)+".tar.gz"),
-	}
-	if err := storage.MkdirAll(artifact); err != nil {
-		t.Fatalf("artifact directory creation failed: %v", err)
-	}
-
-	if err := storage.Archive(&artifact, gitDir, spec.Ignore); err != nil {
-		t.Fatalf("archiving failed: %v", err)
-	}
-
-	if !storage.ArtifactExist(artifact) {
-		t.Fatalf("artifact was created but does not exist: %+v", artifact)
-	}
-
-	return artifact
-}
-
-func stringPtr(s string) *string {
-	return &s
-}
-
-func TestArchiveBasic(t *testing.T) {
-	table := ignoreMap{
-		"README.md":  true,
-		".gitignore": false,
-	}
-
-	dir, err := createStoragePath()
-	if err != nil {
-		t.Fatal(err)
-	}
-	t.Cleanup(cleanupStoragePath(dir))
-
-	storage, err := NewStorage(dir, "hostname", time.Minute)
-	if err != nil {
-		t.Fatalf("Error while bootstrapping storage: %v", err)
-	}
-
-	testPatterns(t, storage, createArchive(t, storage, []string{"README.md", ".gitignore"}, "", sourcev1.GitRepositorySpec{}), table)
-}
-
-func TestArchiveIgnore(t *testing.T) {
-	// this is a list of files that will be created in the repository for each
-	// subtest. it is manipulated later on.
-	filenames := []string{
-		"foo.tar.gz",
-		"bar.jpg",
-		"bar.gif",
-		"foo.jpeg",
-		"video.flv",
-		"video.wmv",
-		"bar.png",
-		"foo.zip",
-		".drone.yml",
-		".flux.yaml",
-	}
-
-	// this is the table of ignored files and their values. true means that it's
-	// present in the resulting tarball.
-	table := ignoreMap{}
-	for _, item := range filenames {
-		table[item] = false
-	}
-
-	dir, err := createStoragePath()
-	if err != nil {
-		t.Fatal(err)
-	}
-	t.Cleanup(cleanupStoragePath(dir))
-
-	storage, err := NewStorage(dir, "hostname", time.Minute)
-	if err != nil {
-		t.Fatalf("Error while bootstrapping storage: %v", err)
-	}
-
-	t.Run("automatically ignored files", func(t *testing.T) {
-		testPatterns(t, storage, createArchive(t, storage, filenames, "", sourcev1.GitRepositorySpec{}), table)
-	})
-
-	table = ignoreMap{}
-	for _, item := range filenames {
-		table[item] = true
-	}
-
-	t.Run("only vcs ignored files", func(t *testing.T) {
-		testPatterns(t, storage, createArchive(t, storage, filenames, "", sourcev1.GitRepositorySpec{Ignore: stringPtr("")}), table)
-	})
-
-	filenames = append(filenames, "test.txt")
-	table["test.txt"] = false
-	sourceIgnoreFile := "*.txt"
-
-	t.Run("sourceignore injected via CRD", func(t *testing.T) {
-		testPatterns(t, storage, createArchive(t, storage, filenames, "", sourcev1.GitRepositorySpec{Ignore: stringPtr(sourceIgnoreFile)}), table)
-	})
-
-	table = ignoreMap{}
-	for _, item := range filenames {
-		table[item] = false
-	}
-
-	t.Run("sourceignore injected via filename", func(t *testing.T) {
-		testPatterns(t, storage, createArchive(t, storage, filenames, sourceIgnoreFile, sourcev1.GitRepositorySpec{}), table)
-	})
 }
 
 func TestStorageRemoveAllButCurrent(t *testing.T) {

go.mod

@@ -28,6 +28,7 @@ require (
 	github.com/spf13/pflag v1.0.5
 	golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2
 	golang.org/x/sync v0.0.0-20201207232520-09787c993a3a
+	gotest.tools v2.2.0+incompatible
 	helm.sh/helm/v3 v3.5.3
 	k8s.io/api v0.20.2
 	k8s.io/apimachinery v0.20.2

pkg/sourceignore/sourceignore.go

@@ -18,8 +18,8 @@ package sourceignore
 
 import (
 	"bufio"
-	"bytes"
 	"io"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -28,7 +28,7 @@ import (
 )
 
 const (
-	ExcludeFile  = ".sourceignore"
+	IgnoreFile   = ".sourceignore"
 	ExcludeVCS   = ".git/,.gitignore,.gitmodules,.gitattributes"
 	ExcludeExt   = "*.jpg,*.jpeg,*.gif,*.png,*.wmv,*.flv,*.tar.gz,*.zip"
 	ExcludeCI    = ".github/,.circleci/,.travis.yml,.gitlab-ci.yml,appveyor.yml,.drone.yml,cloudbuild.yaml,codeship-services.yml,codeship-steps.yml"
@@ -41,47 +41,85 @@ func NewMatcher(ps []gitignore.Pattern) gitignore.Matcher {
 	return gitignore.NewMatcher(ps)
 }
 
-// GetPatterns collects ignore patterns from the given reader and
+// NewDefaultMatcher returns a gitignore.Matcher with the DefaultPatterns
-// returns them as a gitignore.Pattern slice.
+// as lowest priority patterns.
-func GetPatterns(reader io.Reader, path []string) []gitignore.Pattern {
+func NewDefaultMatcher(ps []gitignore.Pattern, domain []string) gitignore.Matcher {
+	var defaultPs []gitignore.Pattern
+	defaultPs = append(defaultPs, VCSPatterns(domain)...)
+	defaultPs = append(defaultPs, DefaultPatterns(domain)...)
+	ps = append(defaultPs, ps...)
+	return gitignore.NewMatcher(ps)
+}
+
+// VCSPatterns returns a gitignore.Pattern slice with ExcludeVCS
+// patterns.
+func VCSPatterns(domain []string) []gitignore.Pattern {
 	var ps []gitignore.Pattern
-	scanner := bufio.NewScanner(reader)
+	for _, p := range strings.Split(ExcludeVCS, ",") {
-
+		ps = append(ps, gitignore.ParsePattern(p, domain))
-	for scanner.Scan() {
-		s := scanner.Text()
-		if !strings.HasPrefix(s, "#") && len(strings.TrimSpace(s)) > 0 {
-			ps = append(ps, gitignore.ParsePattern(s, path))
-		}
 	}
-
 	return ps
 }
 
-// LoadExcludePatterns loads the excluded patterns from .sourceignore or other
+// DefaultPatterns returns a gitignore.Pattern slice with the default
-// sources and returns the gitignore.Pattern slice.
+// ExcludeExt, ExcludeCI, ExcludeExtra patterns.
-func LoadExcludePatterns(dir string, ignore *string) ([]gitignore.Pattern, error) {
+func DefaultPatterns(domain []string) []gitignore.Pattern {
-	path := strings.Split(dir, "/")
+	all := strings.Join([]string{ExcludeExt, ExcludeCI, ExcludeExtra}, ",")
-
 	var ps []gitignore.Pattern
-	for _, p := range strings.Split(ExcludeVCS, ",") {
+	for _, p := range strings.Split(all, ",") {
-		ps = append(ps, gitignore.ParsePattern(p, path))
+		ps = append(ps, gitignore.ParsePattern(p, domain))
 	}
+	return ps
+}
 
-	if ignore == nil {
+// ReadPatterns collects ignore patterns from the given reader and
-		all := strings.Join([]string{ExcludeExt, ExcludeCI, ExcludeExtra}, ",")
+// returns them as a gitignore.Pattern slice.
-		for _, p := range strings.Split(all, ",") {
+// If a domain is supplied, this is used as the scope of the read
-			ps = append(ps, gitignore.ParsePattern(p, path))
+// patterns.
+func ReadPatterns(reader io.Reader, domain []string) []gitignore.Pattern {
+	var ps []gitignore.Pattern
+	scanner := bufio.NewScanner(reader)
+	for scanner.Scan() {
+		s := scanner.Text()
+		if !strings.HasPrefix(s, "#") && len(strings.TrimSpace(s)) > 0 {
+			ps = append(ps, gitignore.ParsePattern(s, domain))
 		}
-
-		if f, err := os.Open(filepath.Join(dir, ExcludeFile)); err == nil {
-			defer f.Close()
-			ps = append(ps, GetPatterns(f, path)...)
-		} else if !os.IsNotExist(err) {
-			return nil, err
-		}
-	} else {
-		ps = append(ps, GetPatterns(bytes.NewBufferString(*ignore), path)...)
 	}
+	return ps
+}
 
+// ReadIgnoreFile attempts to read the file at the given path and
+// returns the read patterns.
+func ReadIgnoreFile(path string, domain []string) ([]gitignore.Pattern, error) {
+	var ps []gitignore.Pattern
+	if f, err := os.Open(path); err == nil {
+		defer f.Close()
+		ps = append(ps, ReadPatterns(f, domain)...)
+	} else if !os.IsNotExist(err) {
+		return nil, err
+	}
+	return ps, nil
+}
+
+// LoadIgnorePatterns recursively loads the the IgnoreFile patterns found
+// in the directory.
+func LoadIgnorePatterns(dir string, domain []string) ([]gitignore.Pattern, error) {
+	ps, err := ReadIgnoreFile(filepath.Join(dir, IgnoreFile), domain)
+	if err != nil {
+		return nil, err
+	}
+	fis, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return nil, err
+	}
+	for _, fi := range fis {
+		if fi.IsDir() && fi.Name() != ".git" {
+			var subps []gitignore.Pattern
+			subps, err = LoadIgnorePatterns(filepath.Join(dir, fi.Name()), append(domain, fi.Name()))
+			if len(subps) > 0 {
+				ps = append(ps, subps...)
+			}
+		}
+	}
 	return ps, nil
 }

pkg/sourceignore/sourceignore_test.go

@@ -0,0 +1,261 @@
+/*
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package sourceignore
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"reflect"
+	"strings"
+	"testing"
+
+	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
+	"gotest.tools/assert"
+)
+
+func TestReadPatterns(t *testing.T) {
+	tests := []struct {
+		name       string
+		ignore     string
+		domain     []string
+		matches    []string
+		mismatches []string
+	}{
+		{
+			name: "simple",
+			ignore: `ignore-dir/*
+!ignore-dir/include
+`,
+			matches:    []string{"ignore-dir/file.yaml"},
+			mismatches: []string{"file.yaml", "ignore-dir/include"},
+		},
+		{
+			name: "with comments",
+			ignore: `ignore-dir/*
+# !ignore-dir/include`,
+			matches: []string{"ignore-dir/file.yaml", "ignore-dir/include"},
+		},
+		{
+			name:       "domain scoped",
+			domain:     []string{"domain", "scoped"},
+			ignore:     "ignore-dir/*",
+			matches:    []string{"domain/scoped/ignore-dir/file.yaml"},
+			mismatches: []string{"ignore-dir/file.yaml"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			reader := strings.NewReader(tt.ignore)
+			ps := ReadPatterns(reader, tt.domain)
+			matcher := NewMatcher(ps)
+			for _, m := range tt.matches {
+				assert.Equal(t, matcher.Match(strings.Split(m, "/"), false), true, "expected %s to match", m)
+			}
+			for _, m := range tt.mismatches {
+				assert.Equal(t, matcher.Match(strings.Split(m, "/"), false), false, "expected %s to not match", m)
+			}
+		})
+	}
+}
+
+func TestReadIgnoreFile(t *testing.T) {
+	f, err := ioutil.TempFile("", IgnoreFile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(f.Name())
+	if _, err = f.Write([]byte(`# .sourceignore
+ignore-this.txt`)); err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+
+	tests := []struct {
+		name   string
+		path   string
+		domain []string
+		want   []gitignore.Pattern
+	}{
+		{
+			name: IgnoreFile,
+			path: f.Name(),
+			want: []gitignore.Pattern{
+				gitignore.ParsePattern("ignore-this.txt", nil),
+			},
+		},
+		{
+			name:   "with domain",
+			path:   f.Name(),
+			domain: strings.Split(filepath.Dir(f.Name()), string(filepath.Separator)),
+			want: []gitignore.Pattern{
+				gitignore.ParsePattern("ignore-this.txt", strings.Split(filepath.Dir(f.Name()), string(filepath.Separator))),
+			},
+		},
+		{
+			name: "non existing",
+			path: "",
+			want: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ReadIgnoreFile(tt.path, tt.domain)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("ReadIgnoreFile() got = %d, want %#v", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestVCSPatterns(t *testing.T) {
+	tests := []struct {
+		name       string
+		domain     []string
+		patterns   []gitignore.Pattern
+		matches    []string
+		mismatches []string
+	}{
+		{
+			name:       "simple matches",
+			matches:    []string{".git/config", ".gitignore"},
+			mismatches: []string{"workload.yaml", "workload.yml", "simple.txt"},
+		},
+		{
+			name:       "domain scoped matches",
+			domain:     []string{"directory"},
+			matches:    []string{"directory/.git/config", "directory/.gitignore"},
+			mismatches: []string{"other/.git/config"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			matcher := NewDefaultMatcher(tt.patterns, tt.domain)
+			for _, m := range tt.matches {
+				assert.Equal(t, matcher.Match(strings.Split(m, "/"), false), true, "expected %s to match", m)
+			}
+			for _, m := range tt.mismatches {
+				assert.Equal(t, matcher.Match(strings.Split(m, "/"), false), false, "expected %s to not match", m)
+			}
+		})
+	}
+}
+
+func TestDefaultPatterns(t *testing.T) {
+	tests := []struct {
+		name       string
+		domain     []string
+		patterns   []gitignore.Pattern
+		matches    []string
+		mismatches []string
+	}{
+		{
+			name:       "simple matches",
+			matches:    []string{"image.jpg", "archive.tar.gz", ".github/workflows/workflow.yaml", "subdir/.flux.yaml", "subdir2/.sops.yaml"},
+			mismatches: []string{"workload.yaml", "workload.yml", "simple.txt"},
+		},
+		{
+			name:       "domain scoped matches",
+			domain:     []string{"directory"},
+			matches:    []string{"directory/image.jpg", "directory/archive.tar.gz"},
+			mismatches: []string{"other/image.jpg", "other/archive.tar.gz"},
+		},
+		{
+			name:       "patterns",
+			patterns:   []gitignore.Pattern{gitignore.ParsePattern("!*.jpg", nil)},
+			mismatches: []string{"image.jpg"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			matcher := NewDefaultMatcher(tt.patterns, tt.domain)
+			for _, m := range tt.matches {
+				assert.Equal(t, matcher.Match(strings.Split(m, "/"), false), true, "expected %s to match", m)
+			}
+			for _, m := range tt.mismatches {
+				assert.Equal(t, matcher.Match(strings.Split(m, "/"), false), false, "expected %s to not match", m)
+			}
+		})
+	}
+}
+
+func TestLoadExcludePatterns(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", "sourceignore-load-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+	files := map[string]string{
+		".sourceignore":     "root.txt",
+		"d/.gitignore":      "ignored",
+		"z/.sourceignore":   "last.txt",
+		"a/b/.sourceignore": "subdir.txt",
+	}
+	for n, c := range files {
+		if err = os.MkdirAll(filepath.Join(tmpDir, filepath.Dir(n)), 0755); err != nil {
+			t.Fatal(err)
+		}
+		if err = os.WriteFile(filepath.Join(tmpDir, n), []byte(c), 0644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	tests := []struct {
+		name   string
+		dir    string
+		domain []string
+		want   []gitignore.Pattern
+	}{
+		{
+			name: "traverse loads",
+			dir:  tmpDir,
+			want: []gitignore.Pattern{
+				gitignore.ParsePattern("root.txt", nil),
+				gitignore.ParsePattern("subdir.txt", []string{"a", "b"}),
+				gitignore.ParsePattern("last.txt", []string{"z"}),
+			},
+		},
+		{
+			name:   "domain",
+			dir:    tmpDir,
+			domain: strings.Split(tmpDir, string(filepath.Separator)),
+			want: []gitignore.Pattern{
+				gitignore.ParsePattern("root.txt", strings.Split(tmpDir, string(filepath.Separator))),
+				gitignore.ParsePattern("subdir.txt", append(strings.Split(tmpDir, string(filepath.Separator)), "a", "b")),
+				gitignore.ParsePattern("last.txt", append(strings.Split(tmpDir, string(filepath.Separator)), "z")),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := LoadIgnorePatterns(tt.dir, tt.domain)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("LoadIgnorePatterns() got = %#v, want %#v", got, tt.want)
+				for _, v := range got {
+					t.Error(v)
+				}
+			}
+		})
+	}
+}