diff --git a/controllers/git_test.go b/controllers/git_test.go index e0e0a606..ca38d56a 100644 --- a/controllers/git_test.go +++ b/controllers/git_test.go @@ -1,6 +1,7 @@ package controllers import ( + "context" "io/ioutil" "os" "path/filepath" @@ -8,13 +9,17 @@ import ( "time" "github.com/go-git/go-billy/v5/memfs" - "github.com/go-git/go-git/v5" + gogit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" "github.com/go-git/go-git/v5/storage/memory" "github.com/go-logr/logr" + + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/source-controller/pkg/git" ) -func populateRepoFromFixture(repo *git.Repository, fixture string) error { +func populateRepoFromFixture(repo *gogit.Repository, fixture string) error { working, err := repo.Worktree() if err != nil { return err @@ -59,7 +64,7 @@ func populateRepoFromFixture(repo *git.Repository, fixture string) error { return err } - if _, err = working.Commit("Initial revision from "+fixture, &git.CommitOptions{ + if _, err = working.Commit("Initial revision from "+fixture, &gogit.CommitOptions{ Author: &object.Signature{ Name: "Testbot", Email: "test@example.com", @@ -73,7 +78,7 @@ func populateRepoFromFixture(repo *git.Repository, fixture string) error { } func TestRepoForFixture(t *testing.T) { - repo, err := git.Init(memory.NewStorage(), memfs.New()) + repo, err := gogit.Init(memory.NewStorage(), memfs.New()) if err != nil { t.Fatal(err) } @@ -92,7 +97,7 @@ func TestIgnoreBrokenSymlink(t *testing.T) { } defer os.RemoveAll(tmp) - repo, err := git.PlainInit(tmp, false) + repo, err := gogit.PlainInit(tmp, false) if err != nil { t.Fatal(err) } @@ -106,3 +111,72 @@ func TestIgnoreBrokenSymlink(t *testing.T) { t.Fatalf("expected no changes but got: %v", err) } } + +// this is a hook script that will reject a ref update for a branch +// that's not `main` +const rejectBranch = ` +if [ "$1" != "refs/heads/main" ]; then + echo "*** Rejecting push to non-main branch $1" >&2 + exit 1 +fi +` + +func TestPushRejected(t *testing.T) { + // Check that pushing to a repository which rejects a ref update + // results in an error. Why would a repo reject an update? If yu + // use e.g., branch protection in GitHub, this is what happens -- + // see + // https://github.com/fluxcd/image-automation-controller/issues/194. + + branch := "push-branch" + + gitServer, err := gittestserver.NewTempGitServer() + if err != nil { + t.Fatal(err) + } + gitServer.AutoCreate() + gitServer.InstallUpdateHook(rejectBranch) + + if err = gitServer.StartHTTP(); err != nil { + t.Fatal(err) + } + + // this is currently defined in update_test.go, but handy right here .. + if err = initGitRepo(gitServer, "testdata/appconfig", "main", "/appconfig.git"); err != nil { + t.Fatal(err) + } + + tmp, err := ioutil.TempDir("", "gotest-imageauto-git") + if err != nil { + t.Fatal(err) + } + repoURL := gitServer.HTTPAddress() + "/appconfig.git" + repo, err := gogit.PlainClone(tmp, false, &gogit.CloneOptions{ + URL: repoURL, + ReferenceName: plumbing.NewBranchReferenceName("main"), + }) + + // This is here to guard against push in general being broken + err = push(context.TODO(), tmp, "main", repoAccess{ + url: repoURL, + auth: &git.Auth{}, + }) + if err != nil { + t.Fatal(err) + } + + // This is not under test, but needed for the next bit + if err = switchBranch(repo, branch); err != nil { + t.Fatal(err) + } + + // This is supposed to fail, because the hook rejects the branch + // pushed to. + err = push(context.TODO(), tmp, branch, repoAccess{ + url: repoURL, + auth: &git.Auth{}, + }) + if err == nil { + t.Error("push to a forbidden branch is expected to fail, but succeeded") + } +} diff --git a/controllers/imageupdateautomation_controller.go b/controllers/imageupdateautomation_controller.go index fef01c5a..2aee00bb 100644 --- a/controllers/imageupdateautomation_controller.go +++ b/controllers/imageupdateautomation_controller.go @@ -674,10 +674,25 @@ func push(ctx context.Context, path, branch string, access repoAccess) error { if err != nil { return err } + + callbacks := access.remoteCallbacks() + + // calling repo.Push will succeed even if a reference update is + // rejected; to detect this case, this callback is supplied. + var callbackErr error + callbacks.PushUpdateReferenceCallback = func(refname, status string) libgit2.ErrorCode { + if status != "" { + callbackErr = fmt.Errorf("ref %s rejected: %s", refname, status) + } + return libgit2.ErrOk + } err = origin.Push([]string{fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch)}, &libgit2.PushOptions{ - RemoteCallbacks: access.remoteCallbacks(), + RemoteCallbacks: callbacks, }) - return libgit2PushError(err) + if err != nil { + return libgit2PushError(err) + } + return callbackErr } func libgit2PushError(err error) error { diff --git a/controllers/update_test.go b/controllers/update_test.go index 31be00a1..99bbc614 100644 --- a/controllers/update_test.go +++ b/controllers/update_test.go @@ -1149,7 +1149,9 @@ func initGitRepo(gitServer *gittestserver.GitServer, fixture, branch, repository } return remote.Push(&git.PushOptions{ - RefSpecs: []config.RefSpec{"refs/heads/*:refs/heads/*"}, + RefSpecs: []config.RefSpec{ + config.RefSpec(fmt.Sprintf("refs/heads/%s:refs/heads/%s", branch, branch)), + }, }) } diff --git a/go.mod b/go.mod index dd3279d0..a020c037 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( // If you bump this, change REFLECTOR_VER in the Makefile to match github.com/fluxcd/image-reflector-controller/api v0.11.0 github.com/fluxcd/pkg/apis/meta v0.10.0 - github.com/fluxcd/pkg/gittestserver v0.3.0 + github.com/fluxcd/pkg/gittestserver v0.3.1 github.com/fluxcd/pkg/runtime v0.12.0 github.com/fluxcd/pkg/ssh v0.1.0 // If you bump this, change SOURCE_VER in the Makefile to match diff --git a/go.sum b/go.sum index 11f30a66..81e50404 100644 --- a/go.sum +++ b/go.sum @@ -303,8 +303,9 @@ github.com/fluxcd/image-reflector-controller/api v0.11.0 h1:Pz9GuUQvmJO5nJPEtGBR github.com/fluxcd/image-reflector-controller/api v0.11.0/go.mod h1:X4qZ11pfA5w1ajbkYbWmQ3hBW3gzCyIjhU87AvV6o2A= github.com/fluxcd/pkg/apis/meta v0.10.0 h1:N7wVGHC1cyPdT87hrDC7UwCwRwnZdQM46PBSLjG2rlE= github.com/fluxcd/pkg/apis/meta v0.10.0/go.mod h1:CW9X9ijMTpNe7BwnokiUOrLl/h13miwVr/3abEQLbKE= -github.com/fluxcd/pkg/gittestserver v0.3.0 h1:6aa30mybecBwBWaJ2IEk7pQzefWnjWjxkTSrHMHawvg= github.com/fluxcd/pkg/gittestserver v0.3.0/go.mod h1:8j36Z6B0BuKNZZ6exAWoyDEpyQoFcjz1IX3WBT7PZNg= +github.com/fluxcd/pkg/gittestserver v0.3.1 h1:TlzLjm5T30iGybOgkVz4TNDeIWLjDmsqlREnFuCSQ0A= +github.com/fluxcd/pkg/gittestserver v0.3.1/go.mod h1:8j36Z6B0BuKNZZ6exAWoyDEpyQoFcjz1IX3WBT7PZNg= github.com/fluxcd/pkg/gitutil v0.1.0 h1:VO3kJY/CKOCO4ysDNqfdpTg04icAKBOSb3lbR5uE/IE= github.com/fluxcd/pkg/gitutil v0.1.0/go.mod h1:Ybz50Ck5gkcnvF0TagaMwtlRy3X3wXuiri1HVsK5id4= github.com/fluxcd/pkg/helmtestserver v0.2.0/go.mod h1:Yie8n7xuu5Nvf1Q7302LKsubJhWpwzCaK0rLJvmF7aI=