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>

Author: hiddeco

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,19 +142,35 @@ 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
-// path, excluding any VCS specific files and directories, or any of the excludes defined in
-// the excludeFiles. If successful, it sets the checksum and last update time on the artifact.
-func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, ignore *string) (err error) {
-	if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() {
-		return fmt.Errorf("invalid dir path: %s", dir)
+// ArchiveFileFilter must return true if a file should not be included
+// in the archive after inspecting the given path and/or os.FileInfo.
+type ArchiveFileFilter func(p string, fi os.FileInfo) bool
+
+// 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)
 	}
+}
 
-	ps, err := sourceignore.LoadExcludePatterns(dir, ignore)
-	if err != nil {
-		return err
+// 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)
 	}
-	matcher := sourceignore.NewMatcher(ps)
 
 	localPath := s.LocalPath(*artifact)
 	tf, err := ioutil.TempFile(filepath.Split(localPath))
@@ -181,43 +189,7 @@ 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 {
-		tw.Close()
-		gw.Close()
-		tf.Close()
-		return err
-	}
-
-	if err := tw.Close(); err != nil {
-		gw.Close()
-		tf.Close()
-		return err
-	}
-	if err := gw.Close(); err != nil {
-		tf.Close()
-		return err
-	}
-	if err := tf.Close(); err != nil {
-		return err
-	}
-
-	if err := os.Chmod(tmpName, 0644); err != nil {
-		return err
-	}
-
-	if err := fs.RenameWithFallback(tmpName, localPath); err != nil {
-		return err
-	}
-
-	artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil))
-	artifact.LastUpdateTime = metav1.Now()
-	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 := filepath.Walk(dir, func(p string, fi os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
@@ -227,8 +199,8 @@ func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer
 			return nil
 		}
 
-		// Ignore excluded extensions and files
-		if matcher.Match(strings.Split(p, "/"), false) {
+		// Skip filtered files
+		if filter != nil && filter(p, fi) {
 			return nil
 		}
 
@@ -248,7 +220,7 @@ func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer
 		}
 		header.Name = relFilePath
 
-		if err := writer.WriteHeader(header); err != nil {
+		if err := tw.WriteHeader(header); err != nil {
 			return err
 		}
 
@@ -257,13 +229,42 @@ func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer
 			f.Close()
 			return err
 		}
-		if _, err := io.Copy(writer, f); err != nil {
+		if _, err := io.Copy(tw, f); err != nil {
 			f.Close()
 			return err
 		}
 		return f.Close()
+	}); err != nil {
+		tw.Close()
+		gw.Close()
+		tf.Close()
+		return err
 	}
-	return filepath.Walk(dir, fn)
+
+	if err := tw.Close(); err != nil {
+		gw.Close()
+		tf.Close()
+		return err
+	}
+	if err := gw.Close(); err != nil {
+		tf.Close()
+		return err
+	}
+	if err := tf.Close(); err != nil {
+		return err
+	}
+
+	if err := os.Chmod(tmpName, 0644); err != nil {
+		return err
+	}
+
+	if err := fs.RenameWithFallback(tmpName, localPath); err != nil {
+		return err
+	}
+
+	artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil))
+	artifact.LastUpdateTime = metav1.Now()
+	return nil
 }
 
 // AtomicWriteFile atomically writes the io.Reader contents to the v1beta1.Artifact path.