diff --git a/controllers/git_test.go b/controllers/git_test.go index 3b4c30a9..59081ca7 100644 --- a/controllers/git_test.go +++ b/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) } diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index da4b630b..7b233cc3 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -259,7 +259,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") @@ -468,7 +468,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 @@ -479,25 +479,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)