Remove check for modified file outside path

In the interest of sticking to fixing the problem (go-git reporting
broken symlinks as modified), this commit removes the check for
whether a modified file is under the path specified.

It's possible to set things up so that an update changes a file
outside the path (e.g., via a symlink), and I shouldn't presume that's
erroneous.

Signed-off-by: Michael Bridgen <[email protected]>

controllers/git_test.go

@@ -7,8 +7,6 @@ import (
 	"testing"
 	"time"
 
-	"k8s.io/apimachinery/pkg/types"
-
 	"github.com/go-git/go-billy/v5/memfs"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/plumbing/object"
@@ -85,61 +83,6 @@ func TestRepoForFixture(t *testing.T) {
 	}
 }
 
-func TestCommitChangedFiles(t *testing.T) {
-	// init a git repo in the filesystem so we can operate on files there
-	tmp, err := ioutil.TempDir("", "flux-test")
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(tmp)
-
-	repo, err := git.PlainInit(tmp, false)
-	if err != nil {
-		t.Fatal(err)
-	}
-	err = populateRepoFromFixture(repo, "testdata/pathconfig")
-	if err != nil {
-		t.Fatal(err)
-	}
-	_, err = commitChangedManifests(repo, tmp, tmp, nil, nil, "unused")
-	if err != errNoChanges {
-		t.Fatalf("expected no changes, got error: %s", err)
-	}
-
-	// change a file _not_ in the path. `replaceMarker` is just a
-	// convenient file-changing procedure.
-	replaceMarker(filepath.Join(tmp, "no"), types.NamespacedName{
-		Namespace: "irrelevant",
-		Name:      "unimportant",
-	})
-	_, err = commitChangedManifests(repo, tmp, filepath.Join(tmp, "yes"), nil, nil, "unused")
-	if err != errNoChanges {
-		t.Fatalf("expected no changes but got: %v", err)
-	}
-
-	working, err := repo.Worktree()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err = working.Reset(&git.ResetOptions{Mode: git.HardReset}); err != nil {
-		t.Fatal(err)
-	}
-	// this is more to verify that we're starting afresh
-	_, err = commitChangedManifests(repo, tmp, tmp, nil, nil, "unused")
-	if err != errNoChanges {
-		t.Fatalf("expected no changes but got: %v", err)
-	}
-
-	replaceMarker(filepath.Join(tmp, "yes"), types.NamespacedName{
-		Namespace: "irrelevant",
-		Name:      "unimportant",
-	})
-	_, err = commitChangedManifests(repo, tmp, filepath.Join(tmp, "yes"), nil, nil, "unused")
-	if err != nil {
-		t.Fatalf("expected no error, got: %v", err)
-	}
-}
-
 func TestIgnoreBrokenSymlink(t *testing.T) {
 	// init a git repo in the filesystem so we can operate on files there
 	tmp, err := ioutil.TempDir("", "flux-test")
@@ -157,7 +100,7 @@ func TestIgnoreBrokenSymlink(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err = commitChangedManifests(repo, tmp, tmp, nil, nil, "unused")
+	_, err = commitChangedManifests(repo, tmp, nil, nil, "unused")
 	if err != errNoChanges {
 		t.Fatalf("expected no changes but got: %v", err)
 	}

controllers/imageupdateautomation_controller.go

@@ -264,7 +264,7 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
 		Email: auto.Spec.Commit.AuthorEmail,
 		When:  time.Now(),
 	}
-	if rev, err := commitChangedManifests(repo, tmp, manifestsPath, signingEntity, author, messageBuf.String()); err != nil {
+	if rev, err := commitChangedManifests(repo, tmp, signingEntity, author, messageBuf.String()); err != nil {
 		if err == errNoChanges {
 			r.event(ctx, auto, events.EventSeverityInfo, "no updates made")
 			log.V(debug).Info("no changes made in working directory; no commit")
@@ -496,7 +496,7 @@ func switchBranch(repo *gogit.Repository, pushBranch string) error {
 
 var errNoChanges error = errors.New("no changes made to working directory")
 
-func commitChangedManifests(repo *gogit.Repository, absRepoPath, absManifestsPath string, ent *openpgp.Entity, author *object.Signature, message string) (string, error) {
+func commitChangedManifests(repo *gogit.Repository, absRepoPath string, ent *openpgp.Entity, author *object.Signature, message string) (string, error) {
 	working, err := repo.Worktree()
 	if err != nil {
 		return "", err
@@ -507,25 +507,13 @@ func commitChangedManifests(repo *gogit.Repository, absRepoPath, absManifestsPat
 		return "", err
 	}
 
-	// 1. we only care about files under the path given in the
-	// automation object 2. go-git has a bug
-	// (https://github.com/go-git/go-git/issues/253) whereby it thinks
-	// broken symlinks to absolute paths are modified; and, there's no
-	// circumstance in which we want to commit a change to a broken
-	// symlink, so skip those.
+	// go-git has [a bug](https://github.com/go-git/go-git/issues/253)
+	// whereby it thinks broken symlinks to absolute paths are
+	// modified; and, there's no circumstance in which we want to
+	// commit a change to a broken symlink: so, skip those.
 	var changed bool
 	for file, _ := range status {
 		abspath := filepath.Join(absRepoPath, file)
-		p, err := filepath.Rel(absManifestsPath, abspath)
-		if err != nil { // we passed two absolute paths, so this is a hard failure
-			return "", fmt.Errorf("comparing repo path %s to manifest path: %w", file, err)
-		}
-		// if the relative path has to ascend to a parent directory,
-		// it's not under the manifests directory
-		if strings.HasPrefix(p, "..") {
-			continue
-		}
-
 		info, err := os.Lstat(abspath)
 		if err != nil {
 			return "", fmt.Errorf("checking if %s is a symlink: %w", file, err)