Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix merge/automerge a pull request from actions runner task #31173

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ Gitea or set your environment appropriately.`, "")
PullRequestID: prID,
DeployKeyID: deployKeyID,
ActionPerm: int(actionPerm),
PushTrigger: repo_module.PushTrigger(os.Getenv(repo_module.EnvPushTrigger)),
}

scanner := bufio.NewScanner(os.Stdin)
Expand Down
2 changes: 1 addition & 1 deletion models/pull/automerge.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe
return false, nil, err
}

doer, err := user_model.GetUserByID(ctx, scheduledPRM.DoerID)
doer, err := user_model.GetPossibleUserByID(ctx, scheduledPRM.DoerID)
if err != nil {
return false, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion routers/private/hook_post_receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {

func loadContextCacheUser(ctx context.Context, id int64) (*user_model.User, error) {
return cache.GetWithContextCache(ctx, "hook_post_receive_user", id, func() (*user_model.User, error) {
return user_model.GetUserByID(ctx, id)
return user_model.GetPossibleUserByID(ctx, id)
})
}

Expand Down
12 changes: 6 additions & 6 deletions services/automerge/automerge.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
pull_model "code.gitea.io/gitea/models/pull"
repo_model "code.gitea.io/gitea/models/repo"
Expand Down Expand Up @@ -275,17 +276,16 @@ func handlePullRequestAutoMerge(pullID int64, sha string) {
}

// Merge if all checks succeeded
doer, err := user_model.GetUserByID(ctx, scheduledPRM.DoerID)
// Use GetPossibleUserByID to allow merging by deleted users or bot users
doer, err := user_model.GetPossibleUserByID(ctx, scheduledPRM.DoerID)
if err != nil {
log.Error("Unable to get scheduled User[%d]: %v", scheduledPRM.DoerID, err)
return
}

perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, doer)
if err != nil {
log.Error("GetUserRepoPermission %-v: %v", pr.HeadRepo, err)
return
}
// We don't check doer's permission here because their permissions have been checked
// before the pull request was scheduled to auto merge
perm := access_model.Permission{AccessMode: perm.AccessModeWrite}

if err := pull_service.CheckPullMergeable(ctx, doer, &perm, pr, pull_service.MergeCheckTypeGeneral, false); err != nil {
if errors.Is(err, pull_service.ErrUserNotAllowedToMerge) {
Expand Down
2 changes: 1 addition & 1 deletion services/automerge/notify.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (n *automergeNotifier) PullRequestReview(ctx context.Context, pr *issues_mo
// as a missing / blocking reviews could have blocked a pending automerge let's recheck
if review.Type == issues_model.ReviewTypeApprove {
if err := StartPRCheckAndAutoMergeBySHA(ctx, review.CommitID, pr.BaseRepo); err != nil {
log.Error("StartPullRequestAutoMergeCheckBySHA: %v", err)
log.Error("StartPRCheckAndAutoMergeBySHA: %v", err)
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions services/pull/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/perm"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
Expand Down Expand Up @@ -331,6 +332,9 @@ func doMergeAndPush(ctx context.Context, pr *issues_model.PullRequest, doer *use
)

mergeCtx.env = append(mergeCtx.env, repo_module.EnvPushTrigger+"="+string(pushTrigger))
if pushTrigger == repo_module.PushTriggerPRMergeToBase {
mergeCtx.env = append(mergeCtx.env, fmt.Sprintf("%s=%d", repo_module.EnvActionPerm, perm.AccessModeWrite))
}
pushCmd := git.NewCommand(ctx, "push", "origin").AddDynamicArguments(baseBranch + ":" + git.BranchPrefix + pr.BaseBranch)

// Push back to upstream.
Expand Down
2 changes: 1 addition & 1 deletion services/repository/commitstatus/commitstatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato

if status.State.IsSuccess() {
if err := automerge.StartPRCheckAndAutoMergeBySHA(ctx, sha, repo); err != nil {
return fmt.Errorf("MergeScheduledPullRequest[repo_id: %d, user_id: %d, sha: %s]: %w", repo.ID, creator.ID, sha, err)
return fmt.Errorf("StartPRCheckAndAutoMergeBySHA[repo_id: %d, user_id: %d, sha: %s]: %w", repo.ID, creator.ID, sha, err)
}
}

Expand Down
108 changes: 108 additions & 0 deletions tests/integration/api_pull_merge_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package integration

import (
"fmt"
"net/http"
"net/url"
"strings"
"testing"
"time"

"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
pull_model "code.gitea.io/gitea/models/pull"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/gitrepo"
api "code.gitea.io/gitea/modules/structs"
commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus"

"github.com/stretchr/testify/assert"
)

func TestAPIPullAutoMergeAfterCommitStatusSucceed(t *testing.T) {
onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
// create a pull request
session := loginUser(t, "user1")
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
forkedName := "repo1-1"
testRepoFork(t, session, "user2", "repo1", "user1", forkedName, "")
defer func() {
testDeleteRepository(t, session, "user1", forkedName)
}()
testEditFile(t, session, "user1", forkedName, "master", "README.md", "Hello, World (Edited)\n")
testPullCreate(t, session, "user1", forkedName, false, "master", "master", "Indexer notifier test pull")

baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user2", Name: "repo1"})
forkedRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: forkedName})
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{
BaseRepoID: baseRepo.ID,
BaseBranch: "master",
HeadRepoID: forkedRepo.ID,
HeadBranch: "master",
})

// add protected branch for commit status
csrf := GetCSRF(t, session, "/user2/repo1/settings/branches")

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / test-sqlite

undefined: GetCSRF

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / test-mysql

undefined: GetCSRF

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / test-mssql

undefined: GetCSRF

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / test-pgsql

undefined: GetCSRF

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / lint-backend

undefined: GetCSRF (typecheck)

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / lint-go-windows

undefined: GetCSRF (typecheck)

Check failure on line 51 in tests/integration/api_pull_merge_test.go

View workflow job for this annotation

GitHub Actions / lint-go-gogit

undefined: GetCSRF (typecheck)
// Change master branch to protected
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/edit", map[string]string{
"_csrf": csrf,
"rule_name": "master",
"enable_push": "true",
"enable_status_check": "true",
"status_check_contexts": "gitea/actions",
})
session.MakeRequest(t, req, http.StatusSeeOther)

// add automerge for this repo
req = NewRequestWithBody(t, http.MethodPost, fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge", baseRepo.OwnerName, baseRepo.Name, pr.Index),
strings.NewReader(url.Values{
"do": []string{"merge"},
"merge_when_checks_succeed": []string{"true"},
}.Encode())).
AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a")
MakeRequest(t, req, http.StatusCreated)

// reload pr again
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
assert.False(t, pr.HasMerged)
assert.Empty(t, pr.MergedCommitID)

// update commit status to success, then it should be merged automatically
baseGitRepo, err := gitrepo.OpenRepository(db.DefaultContext, baseRepo)
assert.NoError(t, err)
sha, err := baseGitRepo.GetRefCommitID(pr.GetGitRefName())
assert.NoError(t, err)
masterCommitID, err := baseGitRepo.GetBranchCommitID("master")
assert.NoError(t, err)

branches, _, err := baseGitRepo.GetBranchNames(0, 100)
assert.NoError(t, err)
assert.ElementsMatch(t, []string{"sub-home-md-img-check", "home-md-img-check", "pr-to-update", "branch2", "DefaultBranch", "develop", "feature/1", "master"}, branches)
baseGitRepo.Close()
defer func() {
testResetRepo(t, baseRepo.RepoPath(), "master", masterCommitID)
}()

err = commitstatus_service.CreateCommitStatus(db.DefaultContext, baseRepo, user1, sha, &git_model.CommitStatus{
State: api.CommitStatusSuccess,
TargetURL: "https://gitea.com",
Context: "gitea/actions",
})
assert.NoError(t, err)

time.Sleep(2 * time.Second)

// reload pr again
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
assert.True(t, pr.HasMerged)
assert.NotEmpty(t, pr.MergedCommitID)

unittest.AssertNotExistsBean(t, &pull_model.AutoMerge{PullID: pr.ID})
})
}
Loading