fix: Allow main branch invocation with merge strategy (#5617)

Signed-off-by: Osher De Paz <osher.depaz@island.io>
Co-authored-by: Marcus Bastian <marcus.bastian@rev.com>
Co-authored-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>

Author: 3 people

server/events/vcs/instrumented_client.go

@@ -206,6 +206,12 @@ func (c *InstrumentedClient) PullIsMergeable(logger logging.SimpleLogging, repo
 }
 
 func (c *InstrumentedClient) UpdateStatus(logger logging.SimpleLogging, repo models.Repo, pull models.PullRequest, state models.CommitStatus, src string, description string, url string) error {
+	// If the plan isn't coming from a pull request,
+	// don't attempt to update the status.
+	if pull.Num == 0 {
+		return nil
+	}
+
 	scope := c.StatsScope.SubScope("update_status")
 	scope = SetGitScopeTags(scope, repo.FullName, pull.Num)
 

server/events/working_dir.go

@@ -77,9 +77,9 @@ type FileWorkspace struct {
 	// TestingOverrideBaseCloneURL can be used during testing to override the
 	// URL of the base repo to be cloned. If it's empty then we clone normally.
 	TestingOverrideBaseCloneURL string
-	// GithubAppEnabled is true if we should fetch the ref "pull/PR_NUMBER/head"
-	// from the "origin" remote. If this is false, we fetch "+refs/heads/$HEAD_BRANCH"
-	// from the "head" remote.
+	// GithubAppEnabled is true and a PR number is supplied, we should fetch
+	// the ref "pull/PR_NUMBER/head" from the "origin" remote. If this is false,
+	// we fetch "+refs/heads/$HEAD_BRANCH" from the "head" remote.
 	GithubAppEnabled bool
 	// use the global setting without overriding
 	GpgNoSigningEnabled bool
@@ -109,12 +109,13 @@ func (w *FileWorkspace) Clone(logger logging.SimpleLogging, headRepo models.Repo
 		logger.Debug("clone directory '%s' already exists, checking if it's at the right commit", cloneDir)
 
 		// We use git rev-parse to see if our repo is at the right commit.
-		// If just checking out the pull request branch, we can use HEAD.
+		// If just checking out the pull request branch or if there is no
+		// pull request (API triggered with a custom git ref), we can use HEAD.
 		// If doing a merge, then HEAD won't be at the pull request's HEAD
 		// because we'll already have performed a merge. Instead, we'll check
 		// HEAD^2 since that will be the commit before our merge.
 		pullHead := "HEAD"
-		if w.CheckoutMerge {
+		if w.CheckoutMerge && c.pr.Num > 0 {
 			pullHead = "HEAD^2"
 		}
 		revParseCmd := exec.Command("git", "rev-parse", pullHead) // #nosec
@@ -351,7 +352,7 @@ func (w *FileWorkspace) wrappedGit(logger logging.SimpleLogging, c wrappedGitCon
 func (w *FileWorkspace) mergeToBaseBranch(logger logging.SimpleLogging, c wrappedGitContext) error {
 	fetchRef := fmt.Sprintf("+refs/heads/%s:", c.pr.HeadBranch)
 	fetchRemote := "head"
-	if w.GithubAppEnabled {
+	if w.GithubAppEnabled && c.pr.Num > 0 {
 		fetchRef = fmt.Sprintf("pull/%d/head:", c.pr.Num)
 		fetchRemote = "origin"
 	}

server/events/working_dir_test.go

@@ -56,6 +56,53 @@ func TestClone_NoneExisting(t *testing.T) {
 	Equals(t, expCommit, actCommit)
 }
 
+// Test running on main branch with merge strategy
+func TestClone_MainBranchWithMergeStrategy(t *testing.T) {
+	// Initialize the git repo.
+	repoDir := initRepo(t)
+	expCommit := runCmd(t, repoDir, "git", "rev-parse", "main")
+
+	dataDir := t.TempDir()
+
+	logger := logging.NewNoopLogger(t)
+
+	overrideURL := fmt.Sprintf("file://%s", repoDir)
+	wd := &events.FileWorkspace{
+		DataDir:                     dataDir,
+		CheckoutMerge:               true,
+		TestingOverrideHeadCloneURL: overrideURL,
+		TestingOverrideBaseCloneURL: overrideURL,
+		GpgNoSigningEnabled:         true,
+	}
+
+	_, err := wd.Clone(logger, models.Repo{}, models.PullRequest{
+		Num:        0,
+		BaseRepo:   models.Repo{},
+		HeadBranch: "branch",
+		BaseBranch: "main",
+	}, "default")
+	Ok(t, err)
+
+	// Create a file that we can use to check if the repo was recloned.
+	runCmd(t, dataDir, "touch", "repos/0/default/proof")
+
+	// re-clone to make sure we don't try to merge main into itself
+	cloneDir, err := wd.Clone(logger, models.Repo{}, models.PullRequest{
+		BaseRepo:   models.Repo{},
+		HeadBranch: "branch",
+		BaseBranch: "main",
+	}, "default")
+	Ok(t, err)
+
+	// Use rev-parse to verify at correct commit.
+	actCommit := runCmd(t, cloneDir, "git", "rev-parse", "HEAD")
+	Equals(t, expCommit, actCommit)
+
+	// Check that our proof file is still there, proving that we didn't reclone.
+	_, err = os.Stat(filepath.Join(cloneDir, "proof"))
+	Ok(t, err)
+}
+
 // Test that if we don't have any existing files, we check out the repo
 // successfully when we're using the merge method.
 func TestClone_CheckoutMergeNoneExisting(t *testing.T) {