Skip to content

Commit

Permalink
Merge pull request #195 from fluxcd/rejected-branch-push
Browse files Browse the repository at this point in the history
Fail push if a ref update is rejected
  • Loading branch information
squaremo authored Jul 14, 2021
2 parents 3102d48 + 3476ecb commit 70eb6c4
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 10 deletions.
84 changes: 79 additions & 5 deletions controllers/git_test.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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]",
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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")
}
}
19 changes: 17 additions & 2 deletions controllers/imageupdateautomation_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion controllers/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
},
})
}

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down

0 comments on commit 70eb6c4

Please sign in to comment.