Fail push if a ref update is rejected

controllers/git_test.go

@@ -1,20 +1,25 @@
 package controllers
 
 import (
+	"context"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"testing"
 	"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: "[email protected]",
@@ -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")
+	}
+}

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 {

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)),
+		},
 	})
 }
 

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

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=