From 9d6355f4ad31686714f7aa05355267e0226aea9f Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 15 Apr 2019 13:51:16 +0200 Subject: [PATCH] Warn user about non-existing configured branch When a user tried to configure Flux against an empty repository or a branch that did not exist we would report a bogus status and left the user confused. This commit makes it possible to configure a branch that should be checked for existence on creation of a new repository. In case there is no HEAD found for this branch, it does not exist in git terms, and we report back a meaningful error message to the user. As a bonus we also use this configured branch when we create a tag for our write check, so we do not touch anything the user has not told us to. --- cmd/fluxd/main.go | 2 +- git/operations.go | 5 ++++- git/operations_test.go | 2 +- git/repo.go | 27 ++++++++++++++++++++++++--- 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/cmd/fluxd/main.go b/cmd/fluxd/main.go index 76304c243..497c3b557 100644 --- a/cmd/fluxd/main.go +++ b/cmd/fluxd/main.go @@ -485,7 +485,7 @@ func main() { SkipMessage: *gitSkipMessage, } - repo := git.NewRepo(gitRemote, git.PollInterval(*gitPollInterval), git.Timeout(*gitTimeout)) + repo := git.NewRepo(gitRemote, git.PollInterval(*gitPollInterval), git.Timeout(*gitTimeout), git.Branch(*gitBranch)) { shutdownWg.Add(1) go func() { diff --git a/git/operations.go b/git/operations.go index 26419daed..8f841377d 100644 --- a/git/operations.go +++ b/git/operations.go @@ -78,9 +78,12 @@ func checkout(ctx context.Context, workingDir, ref string) error { // checkPush sanity-checks that we can write to the upstream repo // (being able to `clone` is an adequate check that we can read the // upstream). -func checkPush(ctx context.Context, workingDir, upstream string) error { +func checkPush(ctx context.Context, workingDir, upstream, branch string) error { // --force just in case we fetched the tag from upstream when cloning args := []string{"tag", "--force", CheckPushTag} + if branch != "" { + args = append(args, branch) + } if err := execGitCmd(ctx, args, gitCmdConfig{dir: workingDir}); err != nil { return errors.Wrap(err, "tag for write check") } diff --git a/git/operations_test.go b/git/operations_test.go index af2919a0f..74fcedae7 100644 --- a/git/operations_test.go +++ b/git/operations_test.go @@ -213,7 +213,7 @@ func TestCheckPush(t *testing.T) { if err != nil { t.Fatal(err) } - err = checkPush(context.Background(), working, upstreamDir) + err = checkPush(context.Background(), working, upstreamDir, "") if err != nil { t.Fatal(err) } diff --git a/git/repo.go b/git/repo.go index 5f104521d..f9ac0f611 100644 --- a/git/repo.go +++ b/git/repo.go @@ -2,6 +2,7 @@ package git import ( "errors" + "fmt" "io/ioutil" "os" "sync" @@ -47,6 +48,7 @@ const ( type Repo struct { // As supplied to constructor origin Remote + branch string interval time.Duration timeout time.Duration readonly bool @@ -83,6 +85,12 @@ func (t Timeout) apply(r *Repo) { r.timeout = time.Duration(t) } +type Branch string + +func (b Branch) apply(r *Repo) { + r.branch = string(b) +} + var ReadOnly optionFunc = func(r *Repo) { r.readonly = true } @@ -261,10 +269,23 @@ func (r *Repo) step(bg context.Context) bool { return false case RepoCloned: + ctx, cancel := context.WithTimeout(bg, r.timeout) + defer cancel() + + if r.branch != "" { + ok, err := refExists(ctx, dir, "refs/heads/"+r.branch) + if err != nil { + r.setUnready(RepoCloned, err) + return false + } + if !ok { + r.setUnready(RepoCloned, fmt.Errorf("configured branch '%s' does not exist", r.branch)) + return false + } + } + if !r.readonly { - ctx, cancel := context.WithTimeout(bg, r.timeout) - err := checkPush(ctx, dir, url) - cancel() + err := checkPush(ctx, dir, url, r.branch) if err != nil { r.setUnready(RepoCloned, err) return false