ensure that head refs are dealt with properly (#167)

pkg/events/check.go

@@ -130,7 +130,15 @@ func canonicalize(cloneURL string) (pkg.RepoURL, error) {
 	return parsed, nil
 }
 
-func (ce *CheckEvent) getRepo(ctx context.Context, cloneURL, branchName string) (*git.Repo, error) {
+func generateRepoKey(cloneURL pkg.RepoURL, branchName string) string {
+	return fmt.Sprintf("%s|||%s", cloneURL.CloneURL(""), branchName)
+}
+
+type hasUsername interface {
+	Username() string
+}
+
+func (ce *CheckEvent) getRepo(ctx context.Context, vcsClient hasUsername, cloneURL, branchName string) (*git.Repo, error) {
 	var (
 		err  error
 		repo *git.Repo
@@ -143,10 +151,13 @@ func (ce *CheckEvent) getRepo(ctx context.Context, cloneURL, branchName string)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to parse clone url")
 	}
-	cloneURL = parsed.CloneURL(ce.ctr.VcsClient.Username())
+	cloneURL = parsed.CloneURL(vcsClient.Username())
 
 	branchName = strings.TrimSpace(branchName)
-	reposKey := fmt.Sprintf("%s|||%s", cloneURL, branchName)
+	if branchName == "" {
+		branchName = "HEAD"
+	}
+	reposKey := generateRepoKey(parsed, branchName)
 
 	if repo, ok := ce.clonedRepos[reposKey]; ok {
 		return repo, nil
@@ -156,7 +167,33 @@ func (ce *CheckEvent) getRepo(ctx context.Context, cloneURL, branchName string)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to clone repo")
 	}
+
 	ce.clonedRepos[reposKey] = repo
+
+	// if we cloned 'HEAD', figure out its original branch and store a copy of the repo there
+	if branchName == "" || branchName == "HEAD" {
+		remoteHeadBranchName, err := repo.GetRemoteHead()
+		if err != nil {
+			return repo, errors.Wrap(err, "failed to determine remote head")
+		}
+
+		repo.BranchName = remoteHeadBranchName
+		ce.clonedRepos[generateRepoKey(parsed, remoteHeadBranchName)] = repo
+	}
+
+	// if we don't have a 'HEAD' saved for the cloned repo, figure out which branch HEAD points to,
+	// and if it's the one we just cloned, store a copy of it as 'HEAD' for usage later
+	headKey := generateRepoKey(parsed, "HEAD")
+	if _, ok := ce.clonedRepos[headKey]; !ok {
+		remoteHeadBranchName, err := repo.GetRemoteHead()
+		if err != nil {
+			return repo, errors.Wrap(err, "failed to determine remote head")
+		}
+		if remoteHeadBranchName == repo.BranchName {
+			ce.clonedRepos[headKey] = repo
+		}
+	}
+
 	return repo, nil
 }
 
@@ -165,7 +202,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error {
 	defer span.End()
 
 	// Clone the repo's BaseRef (main etc) locally into the temp dir we just made
-	repo, err := ce.getRepo(ctx, ce.pullRequest.CloneURL, ce.pullRequest.BaseRef)
+	repo, err := ce.getRepo(ctx, ce.ctr.VcsClient, ce.pullRequest.CloneURL, ce.pullRequest.BaseRef)
 	if err != nil {
 		return errors.Wrap(err, "failed to clone repo")
 	}
@@ -331,7 +368,7 @@ func (ce *CheckEvent) processApp(ctx context.Context, app v1alpha1.Application)
 	// Build a new section for this app in the parent comment
 	ce.vcsNote.AddNewApp(ctx, appName)
 
-	repo, err := ce.getRepo(ctx, appRepoUrl, appSrc.TargetRevision)
+	repo, err := ce.getRepo(ctx, ce.ctr.VcsClient, appRepoUrl, appSrc.TargetRevision)
 	if err != nil {
 		logger.Error().Err(err).Msg("Unable to clone repository")
 		ce.vcsNote.AddToAppMessage(ctx, appName, msg.Result{

pkg/events/check_test.go

@@ -1,9 +1,16 @@
 package events
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/zapier/kubechecks/pkg/config"
+	"github.com/zapier/kubechecks/pkg/git"
 )
 
 // TestCleanupGetManifestsError tests the cleanupGetManifestsError function.
@@ -46,3 +53,73 @@ func TestCleanupGetManifestsError(t *testing.T) {
 		})
 	}
 }
+
+type mockVcsClient struct{}
+
+func (m mockVcsClient) Username() string {
+	return "username"
+}
+
+func TestCheckEventGetRepo(t *testing.T) {
+	cloneURL := "https://github.com/zapier/kubechecks.git"
+	canonical, err := canonicalize(cloneURL)
+	cfg := config.ServerConfig{}
+	require.NoError(t, err)
+
+	ctx := context.TODO()
+
+	t.Run("empty branch name", func(t *testing.T) {
+		ce := CheckEvent{
+			clonedRepos: make(map[string]*git.Repo),
+			repoManager: git.NewRepoManager(cfg),
+		}
+
+		repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "")
+		require.NoError(t, err)
+		assert.Equal(t, "main", repo.BranchName)
+		assert.Len(t, ce.clonedRepos, 2)
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "HEAD"))
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "main"))
+	})
+
+	t.Run("branch is HEAD", func(t *testing.T) {
+		ce := CheckEvent{
+			clonedRepos: make(map[string]*git.Repo),
+			repoManager: git.NewRepoManager(cfg),
+		}
+
+		repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "HEAD")
+		require.NoError(t, err)
+		assert.Equal(t, "main", repo.BranchName)
+		assert.Len(t, ce.clonedRepos, 2)
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "HEAD"))
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "main"))
+	})
+
+	t.Run("branch is the same as HEAD", func(t *testing.T) {
+		ce := CheckEvent{
+			clonedRepos: make(map[string]*git.Repo),
+			repoManager: git.NewRepoManager(cfg),
+		}
+
+		repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "main")
+		require.NoError(t, err)
+		assert.Equal(t, "main", repo.BranchName)
+		assert.Len(t, ce.clonedRepos, 2)
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "HEAD"))
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "main"))
+	})
+
+	t.Run("branch is not the same as HEAD", func(t *testing.T) {
+		ce := CheckEvent{
+			clonedRepos: make(map[string]*git.Repo),
+			repoManager: git.NewRepoManager(cfg),
+		}
+
+		repo, err := ce.getRepo(ctx, mockVcsClient{}, cloneURL, "gh-pages")
+		require.NoError(t, err)
+		assert.Equal(t, "gh-pages", repo.BranchName)
+		assert.Len(t, ce.clonedRepos, 1)
+		assert.Contains(t, ce.clonedRepos, generateRepoKey(canonical, "gh-pages"))
+	})
+}

pkg/git/manager.go

@@ -2,11 +2,9 @@ package git
 
 import (
 	"context"
-	"os"
 	"sync"
 
 	"github.com/pkg/errors"
-	"github.com/rs/zerolog/log"
 	"go.opentelemetry.io/otel"
 
 	"github.com/zapier/kubechecks/pkg/config"
@@ -38,20 +36,11 @@ func (rm *RepoManager) Clone(ctx context.Context, cloneUrl, branchName string) (
 	return repo, nil
 }
 
-func wipeDir(dir string) {
-	if err := os.RemoveAll(dir); err != nil {
-		log.Error().
-			Err(err).
-			Str("path", dir).
-			Msg("failed to wipe path")
-	}
-}
-
 func (rm *RepoManager) Cleanup() {
 	rm.lock.Lock()
 	defer rm.lock.Unlock()
 
 	for _, repo := range rm.repos {
-		wipeDir(repo.Directory)
+		repo.Wipe()
 	}
 }

pkg/git/repo.go

@@ -17,6 +17,7 @@ import (
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/trace"
 
+	"github.com/zapier/kubechecks/pkg"
 	"github.com/zapier/kubechecks/pkg/config"
 	"github.com/zapier/kubechecks/pkg/vcs"
 	"github.com/zapier/kubechecks/telemetry"
@@ -63,7 +64,7 @@ func (r *Repo) Clone(ctx context.Context) error {
 	defer span.End()
 
 	args := []string{"clone", r.CloneURL, r.Directory}
-	if r.BranchName != "" && r.BranchName != "HEAD" {
+	if r.BranchName != "HEAD" {
 		args = append(args, "--branch", r.BranchName)
 	}
 
@@ -93,6 +94,19 @@ func printFile(s string, d fs.DirEntry, err error) error {
 	return nil
 }
 
+func (r *Repo) GetRemoteHead() (string, error) {
+	cmd := r.execCommand("git", "symbolic-ref", "refs/remotes/origin/HEAD", "--short")
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		return "", errors.Wrap(err, "failed to determine which branch HEAD points to")
+	}
+
+	branchName := strings.TrimSpace(string(out))
+	branchName = strings.TrimPrefix(branchName, "origin/")
+
+	return branchName, nil
+}
+
 func (r *Repo) MergeIntoTarget(ctx context.Context, sha string) error {
 	// Merge the last commit into a tmp branch off of the target branch
 	_, span := tracer.Start(ctx, "Repo - RepoMergeIntoTarget",
@@ -133,6 +147,10 @@ func (r *Repo) execCommand(name string, args ...string) *exec.Cmd {
 	return cmd
 }
 
+func (r *Repo) Wipe() {
+	pkg.WipeDir(r.Directory)
+}
+
 func (r *Repo) censorVcsToken(args []string) []string {
 	return censorVcsToken(r.Config, args)
 }

pkg/git/repo_test.go

@@ -87,3 +87,18 @@ git commit -m "commit three"
 	require.NoError(t, err)
 	assert.Equal(t, []string{"abc.txt", "ghi.txt"}, files)
 }
+
+func TestRepoGetRemoteHead(t *testing.T) {
+	cfg := config.ServerConfig{}
+	ctx := context.TODO()
+
+	repo := New(cfg, "https://github.com/zapier/kubechecks.git", "")
+	err := repo.Clone(ctx)
+	require.NoError(t, err)
+
+	t.Cleanup(repo.Wipe)
+
+	branch, err := repo.GetRemoteHead()
+	require.NoError(t, err)
+	assert.Equal(t, "main", branch)
+}