Skip to content

Commit 04ca7f0

Browse files
lunnylafriks
authored andcommitted
Refuse merge until all required status checks success (#7481)
* refuse merge until ci successfully * deny merge request when required status checkes not succeed on merge Post and API * add database migration for added columns on protected_branch * fix migration * fix protected branch check bug * fix protected branch settings * remove duplicated code on check pull request's required commit statuses pass * remove unused codes * fix migration * add newline for template file * fix go mod * rename function name and some other fixes * fix template * fix bug pull view * remove go1.12 wrong dependencies * add administrator bypass when protected branch status check enabled * fix bug * improve the codes
1 parent 2945473 commit 04ca7f0

File tree

15 files changed

+387
-116
lines changed

15 files changed

+387
-116
lines changed

go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ require (
8080
github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5
8181
github.com/oliamb/cutter v0.2.2
8282
github.com/philhofer/fwd v1.0.0 // indirect
83+
github.com/pkg/errors v0.8.1
8384
github.com/pquerna/otp v0.0.0-20160912161815-54653902c20e
8485
github.com/prometheus/client_golang v1.1.0
8586
github.com/prometheus/procfs v0.0.4 // indirect

models/branches.go

+2
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ type ProtectedBranch struct {
3636
EnableMergeWhitelist bool `xorm:"NOT NULL DEFAULT false"`
3737
MergeWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
3838
MergeWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
39+
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
40+
StatusCheckContexts []string `xorm:"JSON TEXT"`
3941
ApprovalsWhitelistUserIDs []int64 `xorm:"JSON TEXT"`
4042
ApprovalsWhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
4143
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`

models/commit_status.go

+22
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"crypto/sha1"
1010
"fmt"
1111
"strings"
12+
"time"
1213

1314
"code.gitea.io/gitea/modules/log"
1415
"code.gitea.io/gitea/modules/setting"
@@ -205,6 +206,27 @@ func GetLatestCommitStatus(repo *Repository, sha string, page int) ([]*CommitSta
205206
return statuses, x.In("id", ids).Find(&statuses)
206207
}
207208

209+
// FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts
210+
func FindRepoRecentCommitStatusContexts(repoID int64, before time.Duration) ([]string, error) {
211+
start := timeutil.TimeStampNow().AddDuration(-before)
212+
ids := make([]int64, 0, 10)
213+
if err := x.Table("commit_status").
214+
Where("repo_id = ?", repoID).
215+
And("updated_unix >= ?", start).
216+
Select("max( id ) as id").
217+
GroupBy("context_hash").OrderBy("max( id ) desc").
218+
Find(&ids); err != nil {
219+
return nil, err
220+
}
221+
222+
var contexts = make([]string, 0, len(ids))
223+
if len(ids) == 0 {
224+
return contexts, nil
225+
}
226+
return contexts, x.Select("context").Table("commit_status").In("id", ids).Find(&contexts)
227+
228+
}
229+
208230
// NewCommitStatusOptions holds options for creating a CommitStatus
209231
type NewCommitStatusOptions struct {
210232
Repo *Repository

models/migrations/migrations.go

+2
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,8 @@ var migrations = []Migration{
242242
NewMigration("remove orphaned repository index statuses", removeLingeringIndexStatus),
243243
// v93 -> v94
244244
NewMigration("add email notification enabled preference to user", addEmailNotificationEnabledToUser),
245+
// v94 -> v95
246+
NewMigration("add enable_status_check, status_check_contexts to protected_branch", addStatusCheckColumnsForProtectedBranches),
245247
}
246248

247249
// Migrate database to current version

models/migrations/v94.go

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Copyright 2019 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import "github.com/go-xorm/xorm"
8+
9+
func addStatusCheckColumnsForProtectedBranches(x *xorm.Engine) error {
10+
type ProtectedBranch struct {
11+
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
12+
StatusCheckContexts []string `xorm:"JSON TEXT"`
13+
}
14+
15+
if err := x.Sync2(new(ProtectedBranch)); err != nil {
16+
return err
17+
}
18+
19+
_, err := x.Cols("enable_status_check", "status_check_contexts").Update(&ProtectedBranch{
20+
EnableStatusCheck: false,
21+
StatusCheckContexts: []string{},
22+
})
23+
return err
24+
}

models/pull.go

+14
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,20 @@ func (pr *PullRequest) LoadAttributes() error {
9999
return pr.loadAttributes(x)
100100
}
101101

102+
// LoadBaseRepo loads pull request base repository from database
103+
func (pr *PullRequest) LoadBaseRepo() error {
104+
if pr.BaseRepo == nil {
105+
var repo Repository
106+
if has, err := x.ID(pr.BaseRepoID).Get(&repo); err != nil {
107+
return err
108+
} else if !has {
109+
return ErrRepoNotExist{ID: pr.BaseRepoID}
110+
}
111+
pr.BaseRepo = &repo
112+
}
113+
return nil
114+
}
115+
102116
// LoadIssue loads issue information from database
103117
func (pr *PullRequest) LoadIssue() (err error) {
104118
return pr.loadIssue(x)

modules/auth/repo_form.go

+2
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ type ProtectBranchForm struct {
155155
EnableMergeWhitelist bool
156156
MergeWhitelistUsers string
157157
MergeWhitelistTeams string
158+
EnableStatusCheck bool `xorm:"NOT NULL DEFAULT false"`
159+
StatusCheckContexts []string
158160
RequiredApprovals int64
159161
ApprovalsWhitelistUsers string
160162
ApprovalsWhitelistTeams string

modules/pull/commit_status.go

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2019 The Gitea Authors.
2+
// All rights reserved.
3+
// Use of this source code is governed by a MIT-style
4+
// license that can be found in the LICENSE file.
5+
6+
package pull
7+
8+
import (
9+
"code.gitea.io/gitea/models"
10+
"code.gitea.io/gitea/modules/git"
11+
12+
"github.com/pkg/errors"
13+
)
14+
15+
// IsCommitStatusContextSuccess returns true if all required status check contexts succeed.
16+
func IsCommitStatusContextSuccess(commitStatuses []*models.CommitStatus, requiredContexts []string) bool {
17+
for _, ctx := range requiredContexts {
18+
var found bool
19+
for _, commitStatus := range commitStatuses {
20+
if commitStatus.Context == ctx {
21+
if commitStatus.State != models.CommitStatusSuccess {
22+
return false
23+
}
24+
25+
found = true
26+
break
27+
}
28+
}
29+
if !found {
30+
return false
31+
}
32+
}
33+
return true
34+
}
35+
36+
// IsPullCommitStatusPass returns if all required status checks PASS
37+
func IsPullCommitStatusPass(pr *models.PullRequest) (bool, error) {
38+
if err := pr.LoadProtectedBranch(); err != nil {
39+
return false, errors.Wrap(err, "GetLatestCommitStatus")
40+
}
41+
if pr.ProtectedBranch == nil || !pr.ProtectedBranch.EnableStatusCheck {
42+
return true, nil
43+
}
44+
45+
// check if all required status checks are successful
46+
headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
47+
if err != nil {
48+
return false, errors.Wrap(err, "OpenRepository")
49+
}
50+
51+
if !headGitRepo.IsBranchExist(pr.HeadBranch) {
52+
return false, errors.New("Head branch does not exist, can not merge")
53+
}
54+
55+
sha, err := headGitRepo.GetBranchCommitID(pr.HeadBranch)
56+
if err != nil {
57+
return false, errors.Wrap(err, "GetBranchCommitID")
58+
}
59+
60+
if err := pr.LoadBaseRepo(); err != nil {
61+
return false, errors.Wrap(err, "LoadBaseRepo")
62+
}
63+
64+
commitStatuses, err := models.GetLatestCommitStatus(pr.BaseRepo, sha, 0)
65+
if err != nil {
66+
return false, errors.Wrap(err, "GetLatestCommitStatus")
67+
}
68+
69+
return IsCommitStatusContextSuccess(commitStatuses, pr.ProtectedBranch.StatusCheckContexts), nil
70+
}

options/locale/locale_en-US.ini

+6
Original file line numberDiff line numberDiff line change
@@ -981,13 +981,16 @@ pulls.cannot_merge_work_in_progress = This pull request is marked as a work in p
981981
pulls.data_broken = This pull request is broken due to missing fork information.
982982
pulls.files_conflicted = This pull request has changes conflicting with the target branch.
983983
pulls.is_checking = "Merge conflict checking is in progress. Try again in few moments."
984+
pulls.required_status_check_failed = Some required checks were not successful.
985+
pulls.required_status_check_administrator = As an administrator, you may still merge this pull request.
984986
pulls.blocked_by_approvals = "This Pull Request doesn't have enough approvals yet. %d of %d approvals granted."
985987
pulls.can_auto_merge_desc = This pull request can be merged automatically.
986988
pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically due to conflicts.
987989
pulls.cannot_auto_merge_helper = Merge manually to resolve the conflicts.
988990
pulls.no_merge_desc = This pull request cannot be merged because all repository merge options are disabled.
989991
pulls.no_merge_helper = Enable merge options in the repository settings or merge the pull request manually.
990992
pulls.no_merge_wip = This pull request can not be merged because it is marked as being a work in progress.
993+
pulls.no_merge_status_check = This pull request cannot be merged because not all required status checkes are successful.
991994
pulls.merge_pull_request = Merge Pull Request
992995
pulls.rebase_merge_pull_request = Rebase and Merge
993996
pulls.rebase_merge_commit_pull_request = Rebase and Merge (--no-ff)
@@ -1311,6 +1314,9 @@ settings.protect_merge_whitelist_committers = Enable Merge Whitelist
13111314
settings.protect_merge_whitelist_committers_desc = Allow only whitelisted users or teams to merge pull requests into this branch.
13121315
settings.protect_merge_whitelist_users = Whitelisted users for merging:
13131316
settings.protect_merge_whitelist_teams = Whitelisted teams for merging:
1317+
settings.protect_check_status_contexts = Enable Status Check
1318+
settings.protect_check_status_contexts_desc = Require status checks to pass before merging Choose which status checks must pass before branches can be merged into a branch that matches this rule. When enabled, commits must first be pushed to another branch, then merged or pushed directly to a branch that matches this rule after status checks have passed.
1319+
settings.protect_check_status_contexts_list = Status checks found in the last week for this repository
13141320
settings.protect_required_approvals = Required approvals:
13151321
settings.protect_required_approvals_desc = Allow only to merge pull request with enough positive reviews of whitelisted users or teams.
13161322
settings.protect_approvals_whitelist_users = Whitelisted reviewers:

routers/api/v1/repo/pull.go

+12
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"code.gitea.io/gitea/modules/log"
1717
"code.gitea.io/gitea/modules/notification"
1818
"code.gitea.io/gitea/modules/pull"
19+
pull_service "code.gitea.io/gitea/modules/pull"
1920
api "code.gitea.io/gitea/modules/structs"
2021
"code.gitea.io/gitea/modules/timeutil"
2122
milestone_service "code.gitea.io/gitea/services/milestone"
@@ -571,6 +572,17 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
571572
return
572573
}
573574

575+
isPass, err := pull_service.IsPullCommitStatusPass(pr)
576+
if err != nil {
577+
ctx.Error(500, "IsPullCommitStatusPass", err)
578+
return
579+
}
580+
581+
if !isPass && !ctx.IsUserRepoAdmin() {
582+
ctx.Status(405)
583+
return
584+
}
585+
574586
if len(form.Do) == 0 {
575587
form.Do = string(models.MergeStyleMerge)
576588
}

routers/repo/pull.go

+30
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"code.gitea.io/gitea/modules/log"
2323
"code.gitea.io/gitea/modules/notification"
2424
"code.gitea.io/gitea/modules/pull"
25+
pull_service "code.gitea.io/gitea/modules/pull"
2526
"code.gitea.io/gitea/modules/setting"
2627
"code.gitea.io/gitea/modules/util"
2728
"code.gitea.io/gitea/services/gitdiff"
@@ -322,6 +323,12 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
322323

323324
setMergeTarget(ctx, pull)
324325

326+
if err = pull.LoadProtectedBranch(); err != nil {
327+
ctx.ServerError("GetLatestCommitStatus", err)
328+
return nil
329+
}
330+
ctx.Data["EnableStatusCheck"] = pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck
331+
325332
var headGitRepo *git.Repository
326333
var headBranchExist bool
327334
// HeadRepo may be missing
@@ -350,6 +357,18 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
350357
ctx.Data["LatestCommitStatuses"] = commitStatuses
351358
ctx.Data["LatestCommitStatus"] = models.CalcCommitStatus(commitStatuses)
352359
}
360+
361+
if pull.ProtectedBranch != nil && pull.ProtectedBranch.EnableStatusCheck {
362+
ctx.Data["is_context_required"] = func(context string) bool {
363+
for _, c := range pull.ProtectedBranch.StatusCheckContexts {
364+
if c == context {
365+
return true
366+
}
367+
}
368+
return false
369+
}
370+
ctx.Data["IsRequiredStatusCheckSuccess"] = pull_service.IsCommitStatusContextSuccess(commitStatuses, pull.ProtectedBranch.StatusCheckContexts)
371+
}
353372
}
354373
}
355374

@@ -608,6 +627,17 @@ func MergePullRequest(ctx *context.Context, form auth.MergePullRequestForm) {
608627
return
609628
}
610629

630+
isPass, err := pull_service.IsPullCommitStatusPass(pr)
631+
if err != nil {
632+
ctx.ServerError("IsPullCommitStatusPass", err)
633+
return
634+
}
635+
if !isPass && !ctx.IsUserRepoAdmin() {
636+
ctx.Flash.Error(ctx.Tr("repo.pulls.no_merge_status_check"))
637+
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))
638+
return
639+
}
640+
611641
if ctx.HasError() {
612642
ctx.Flash.Error(ctx.Data["ErrorMsg"].(string))
613643
ctx.Redirect(ctx.Repo.RepoLink + "/pulls/" + com.ToStr(pr.Index))

routers/repo/setting_protected_branch.go

+28
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package repo
77
import (
88
"fmt"
99
"strings"
10+
"time"
1011

1112
"code.gitea.io/gitea/models"
1213
"code.gitea.io/gitea/modules/auth"
@@ -125,6 +126,29 @@ func SettingsProtectedBranch(c *context.Context) {
125126
c.Data["whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.WhitelistUserIDs), ",")
126127
c.Data["merge_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.MergeWhitelistUserIDs), ",")
127128
c.Data["approvals_whitelist_users"] = strings.Join(base.Int64sToStrings(protectBranch.ApprovalsWhitelistUserIDs), ",")
129+
contexts, _ := models.FindRepoRecentCommitStatusContexts(c.Repo.Repository.ID, 7*24*time.Hour) // Find last week status check contexts
130+
for _, context := range protectBranch.StatusCheckContexts {
131+
var found bool
132+
for _, ctx := range contexts {
133+
if ctx == context {
134+
found = true
135+
break
136+
}
137+
}
138+
if !found {
139+
contexts = append(contexts, context)
140+
}
141+
}
142+
143+
c.Data["branch_status_check_contexts"] = contexts
144+
c.Data["is_context_required"] = func(context string) bool {
145+
for _, c := range protectBranch.StatusCheckContexts {
146+
if c == context {
147+
return true
148+
}
149+
}
150+
return false
151+
}
128152

129153
if c.Repo.Owner.IsOrganization() {
130154
teams, err := c.Repo.Owner.TeamsWithAccessToRepo(c.Repo.Repository.ID, models.AccessModeRead)
@@ -186,6 +210,10 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm)
186210
if strings.TrimSpace(f.MergeWhitelistTeams) != "" {
187211
mergeWhitelistTeams, _ = base.StringsToInt64s(strings.Split(f.MergeWhitelistTeams, ","))
188212
}
213+
214+
protectBranch.EnableStatusCheck = f.EnableStatusCheck
215+
protectBranch.StatusCheckContexts = f.StatusCheckContexts
216+
189217
protectBranch.RequiredApprovals = f.RequiredApprovals
190218
if strings.TrimSpace(f.ApprovalsWhitelistUsers) != "" {
191219
approvalsWhitelistUsers, _ = base.StringsToInt64s(strings.Split(f.ApprovalsWhitelistUsers, ","))

0 commit comments

Comments
 (0)