Skip to content

Add block on pending codeowner reviews branch protection #34995

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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 models/git/protected_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ type ProtectedBranch struct {
RequiredApprovals int64 `xorm:"NOT NULL DEFAULT 0"`
BlockOnRejectedReviews bool `xorm:"NOT NULL DEFAULT false"`
BlockOnOfficialReviewRequests bool `xorm:"NOT NULL DEFAULT false"`
BlockOnCodeownerReviews bool `xorm:"NOT NULL DEFAULT false"`
BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"`
DismissStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
IgnoreStaleApprovals bool `xorm:"NOT NULL DEFAULT false"`
Expand Down
5 changes: 5 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"code.gitea.io/gitea/models/migrations/v1_22"
"code.gitea.io/gitea/models/migrations/v1_23"
"code.gitea.io/gitea/models/migrations/v1_24"
"code.gitea.io/gitea/models/migrations/v1_25"
"code.gitea.io/gitea/models/migrations/v1_6"
"code.gitea.io/gitea/models/migrations/v1_7"
"code.gitea.io/gitea/models/migrations/v1_8"
Expand Down Expand Up @@ -382,6 +383,10 @@ func prepareMigrationTasks() []*migration {
newMigration(318, "Add anonymous_access_mode for repo_unit", v1_24.AddRepoUnitAnonymousAccessMode),
newMigration(319, "Add ExclusiveOrder to Label table", v1_24.AddExclusiveOrderColumnToLabelTable),
newMigration(320, "Migrate two_factor_policy to login_source table", v1_24.MigrateSkipTwoFactor),

// Gitea 1.24.0 ends at migration ID number 320 (database version 321)

newMigration(321, "Add block on codeowner reviews branch protection", v1_25.AddBlockOnCodeownerReviews),
}
return preparedMigrations
}
Expand Down
16 changes: 16 additions & 0 deletions models/migrations/v1_25/v321.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_25

import (
"xorm.io/xorm"
)

func AddBlockOnCodeownerReviews(x *xorm.Engine) error {
type ProtectedBranch struct {
BlockOnCodeownerReviews bool `xorm:"NOT NULL DEFAULT false"`
}

return x.Sync(new(ProtectedBranch))
}
3 changes: 3 additions & 0 deletions modules/structs/repo_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type BranchProtection struct {
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
BlockOnCodeownerReviews bool `json:"block_on_codeowner_reviews"`
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
IgnoreStaleApprovals bool `json:"ignore_stale_approvals"`
Expand Down Expand Up @@ -87,6 +88,7 @@ type CreateBranchProtectionOption struct {
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
BlockOnRejectedReviews bool `json:"block_on_rejected_reviews"`
BlockOnOfficialReviewRequests bool `json:"block_on_official_review_requests"`
BlockOnCodeownerReviews bool `json:"block_on_codeowner_reviews"`
BlockOnOutdatedBranch bool `json:"block_on_outdated_branch"`
DismissStaleApprovals bool `json:"dismiss_stale_approvals"`
IgnoreStaleApprovals bool `json:"ignore_stale_approvals"`
Expand Down Expand Up @@ -120,6 +122,7 @@ type EditBranchProtectionOption struct {
ApprovalsWhitelistTeams []string `json:"approvals_whitelist_teams"`
BlockOnRejectedReviews *bool `json:"block_on_rejected_reviews"`
BlockOnOfficialReviewRequests *bool `json:"block_on_official_review_requests"`
BlockOnCodeownerReviews *bool `json:"block_on_codeowner_reviews"`
BlockOnOutdatedBranch *bool `json:"block_on_outdated_branch"`
DismissStaleApprovals *bool `json:"dismiss_stale_approvals"`
IgnoreStaleApprovals *bool `json:"ignore_stale_approvals"`
Expand Down
3 changes: 3 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1903,6 +1903,7 @@ pulls.required_status_check_administrator = As an administrator, you may still m
pulls.blocked_by_approvals = "This pull request doesn't have enough required approvals yet. %d of %d official approvals granted."
pulls.blocked_by_approvals_whitelisted = "This pull request doesn't have enough required approvals yet. %d of %d approvals granted from users or teams on the allowlist."
pulls.blocked_by_rejection = "This pull request has changes requested by an official reviewer."
pulls.blocked_by_codeowners = "This pull request is missing approval from one or more code owners."
pulls.blocked_by_official_review_requests = "This pull request has official review requests."
pulls.blocked_by_outdated_branch = "This pull request is blocked because it's outdated."
pulls.blocked_by_changed_protected_files_1= "This pull request is blocked because it changes a protected file:"
Expand Down Expand Up @@ -2538,6 +2539,8 @@ settings.block_rejected_reviews = Block merge on rejected reviews
settings.block_rejected_reviews_desc = Merging will not be possible when changes are requested by official reviewers, even if there are enough approvals.
settings.block_on_official_review_requests = Block merge on official review requests
settings.block_on_official_review_requests_desc = Merging will not be possible when it has official review requests, even if there are enough approvals.
settings.block_on_codeowner_reviews = Require approval from code owners
settings.block_on_codeowner_reviews_desc = Merging will only be possible if at least one code owner per code owner rule has given an approving review.
settings.block_outdated_branch = Block merge if pull request is outdated
settings.block_outdated_branch_desc = Merging will not be possible when head branch is behind base branch.
settings.block_admin_merge_override = Administrators must follow branch protection rules
Expand Down
4 changes: 4 additions & 0 deletions routers/api/v1/repo/branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,10 @@ func EditBranchProtection(ctx *context.APIContext) {
protectBranch.BlockOnOfficialReviewRequests = *form.BlockOnOfficialReviewRequests
}

if form.BlockOnCodeownerReviews != nil {
protectBranch.BlockOnCodeownerReviews = *form.BlockOnCodeownerReviews
}

if form.DismissStaleApprovals != nil {
protectBranch.DismissStaleApprovals = *form.DismissStaleApprovals
}
Expand Down
1 change: 1 addition & 0 deletions routers/web/repo/issue_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ func preparePullViewReviewAndMerge(ctx *context.Context, issue *issues_model.Iss
ctx.Data["ProtectedBranch"] = pb
ctx.Data["IsBlockedByApprovals"] = !issues_model.HasEnoughApprovals(ctx, pb, pull)
ctx.Data["IsBlockedByRejection"] = issues_model.MergeBlockedByRejectedReview(ctx, pb, pull)
ctx.Data["IsBlockedByCodeowners"] = !issue_service.HasAllRequiredCodeownerReviews(ctx, pb, pull)
ctx.Data["IsBlockedByOfficialReviewRequests"] = issues_model.MergeBlockedByOfficialReviewRequests(ctx, pb, pull)
ctx.Data["IsBlockedByOutdatedBranch"] = issues_model.MergeBlockedByOutdatedBranch(pb, pull)
ctx.Data["GrantedApprovals"] = issues_model.GetGrantedApprovalsCount(ctx, pb, pull)
Expand Down
1 change: 1 addition & 0 deletions routers/web/repo/setting/protected_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
}
protectBranch.BlockOnRejectedReviews = f.BlockOnRejectedReviews
protectBranch.BlockOnOfficialReviewRequests = f.BlockOnOfficialReviewRequests
protectBranch.BlockOnCodeownerReviews = f.BlockOnCodeownerReviews
protectBranch.DismissStaleApprovals = f.DismissStaleApprovals
protectBranch.IgnoreStaleApprovals = f.IgnoreStaleApprovals
protectBranch.RequireSignedCommits = f.RequireSignedCommits
Expand Down
1 change: 1 addition & 0 deletions services/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ func ToBranchProtection(ctx context.Context, bp *git_model.ProtectedBranch, repo
ApprovalsWhitelistTeams: approvalsWhitelistTeams,
BlockOnRejectedReviews: bp.BlockOnRejectedReviews,
BlockOnOfficialReviewRequests: bp.BlockOnOfficialReviewRequests,
BlockOnCodeownerReviews: bp.BlockOnCodeownerReviews,
BlockOnOutdatedBranch: bp.BlockOnOutdatedBranch,
DismissStaleApprovals: bp.DismissStaleApprovals,
IgnoreStaleApprovals: bp.IgnoreStaleApprovals,
Expand Down
1 change: 1 addition & 0 deletions services/forms/repo_form.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ type ProtectBranchForm struct {
ApprovalsWhitelistTeams string
BlockOnRejectedReviews bool
BlockOnOfficialReviewRequests bool
BlockOnCodeownerReviews bool
BlockOnOutdatedBranch bool
DismissStaleApprovals bool
IgnoreStaleApprovals bool
Expand Down
146 changes: 146 additions & 0 deletions services/issue/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"slices"
"time"

git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
org_model "code.gitea.io/gitea/models/organization"
user_model "code.gitea.io/gitea/models/user"
Expand Down Expand Up @@ -47,6 +48,151 @@ func IsCodeOwnerFile(f string) bool {
return slices.Contains(codeOwnerFiles, f)
}

func HasAllRequiredCodeownerReviews(ctx context.Context, pb *git_model.ProtectedBranch, pr *issues_model.PullRequest) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A unit test or an integration test is required for this function to ensure its correctness.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I went and amended the existing code owner integration test to also check for branch protection rule enforcement.

if !pb.BlockOnCodeownerReviews {
return true
}

if err := pr.LoadHeadRepo(ctx); err != nil {
return false
}

if err := pr.LoadBaseRepo(ctx); err != nil {
return false
}

if err := pr.LoadIssue(ctx); err != nil {
return false
}

pr.Issue.Repo = pr.BaseRepo

if pr.BaseRepo.IsFork {
return true
}

repo, err := gitrepo.OpenRepository(ctx, pr.BaseRepo)
if err != nil {
return false
}

defer repo.Close()

commit, err := repo.GetBranchCommit(pr.BaseRepo.DefaultBranch)
if err != nil {
return false
}

var data string

for _, file := range codeOwnerFiles {
if blob, err := commit.GetBlobByPath(file); err == nil {
data, err = blob.GetBlobContent(setting.UI.MaxDisplayFileSize)
if err == nil {
break
}
}
}

// no code owner file = no one to approve
if data == "" {
return true
}

rules, _ := issues_model.GetCodeOwnersFromContent(ctx, data)
if len(rules) == 0 {
return true
}

// get the mergebase
mergeBase, err := getMergeBase(repo, pr, git.BranchPrefix+pr.BaseBranch, pr.GetGitRefName())
if err != nil {
return false
}

// https://github.com/go-gitea/gitea/issues/29763, we need to get the files changed
// between the merge base and the head commit but not the base branch and the head commit
changedFiles, err := repo.GetFilesChangedBetween(mergeBase, pr.GetGitRefName())
if err != nil {
return false
}

reviews, err := issues_model.FindLatestReviews(ctx, issues_model.FindReviewOptions{
Types: []issues_model.ReviewType{issues_model.ReviewTypeApprove},
IssueID: pr.IssueID,
})
if err != nil {
return false
}

hasApprovals := true

for _, rule := range rules {
for _, f := range changedFiles {
if (rule.Rule.MatchString(f) && !rule.Negative) || (!rule.Rule.MatchString(f) && rule.Negative) {
hasPotentialReviewers := false
hasRuleApproval := false

for _, u := range rule.Users {
if u.ID == pr.Issue.OriginalAuthorID {
continue
}

hasPotentialReviewers = true

for _, review := range reviews {
if review.ReviewerID == u.ID {
hasRuleApproval = true
break
}
}

if hasRuleApproval {
break
}
}

if hasRuleApproval {
continue
}

for _, t := range rule.Teams {
if !hasPotentialReviewers {
if err := t.LoadMembers(ctx); err != nil {
return false
}

for _, m := range t.Members {
if m.ID != pr.Issue.OriginalAuthorID {
hasPotentialReviewers = true
break
}
}
}

for _, review := range reviews {
if review.ReviewerTeamID == t.ID {
hasRuleApproval = true
break
}
}

if hasRuleApproval {
break
}
}

if !hasRuleApproval && hasPotentialReviewers {
hasApprovals = false
break
}
}
}
}

return hasApprovals
}

func PullRequestCodeOwnersReview(ctx context.Context, pr *issues_model.PullRequest) ([]*ReviewRequestNotifier, error) {
if err := pr.LoadIssue(ctx); err != nil {
return nil, 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 @@ -592,6 +592,10 @@ func CheckPullBranchProtections(ctx context.Context, pr *issues_model.PullReques
return util.ErrorWrap(ErrNotReadyToMerge, "The head branch is behind the base branch")
}

if !issue_service.HasAllRequiredCodeownerReviews(ctx, pb, pr) {
return util.ErrorWrap(ErrNotReadyToMerge, "There are missing code owner reviews.")
}

if skipProtectedFilesCheck {
return nil
}
Expand Down
13 changes: 12 additions & 1 deletion templates/repo/issue/view_content/pull_merge_box.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
{{- else if .IsBlockedByApprovals}}red
{{- else if .IsBlockedByRejection}}red
{{- else if .IsBlockedByOfficialReviewRequests}}red
{{- else if .IsBlockedByCodeowners}}red
{{- else if .IsBlockedByOutdatedBranch}}red
{{- else if .IsBlockedByChangedProtectedFiles}}red
{{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red
Expand Down Expand Up @@ -131,6 +132,11 @@
{{svg "octicon-x"}}
{{ctx.Locale.Tr "repo.pulls.blocked_by_official_review_requests"}}
</div>
{{else if .IsBlockedByCodeowners}}
<div class="item">
{{svg "octicon-x"}}
{{ctx.Locale.Tr "repo.pulls.blocked_by_codeowners"}}
</div>
{{else if .IsBlockedByOutdatedBranch}}
<div class="item">
{{svg "octicon-x"}}
Expand Down Expand Up @@ -167,7 +173,7 @@
</div>
{{end}}

{{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}}
{{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOfficialReviewRequests .IsBlockedByCodeowners .IsBlockedByOutdatedBranch .IsBlockedByChangedProtectedFiles (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}}

{{/* admin can merge without checks, writer can merge when checks succeed */}}
{{$canMergeNow := and (or (and (not $.ProtectedBranch.BlockAdminMergeOverride) $.IsRepoAdmin) (not $notAllOverridableChecksOk)) (or (not .AllowMerge) (not .RequireSigned) .WillSign)}}
Expand Down Expand Up @@ -337,6 +343,11 @@
{{svg "octicon-x"}}
{{ctx.Locale.Tr "repo.pulls.blocked_by_official_review_requests"}}
</div>
{{else if .IsBlockedByCodeowners}}
<div class="item text red">
{{svg "octicon-x"}}
{{ctx.Locale.Tr "repo.pulls.blocked_by_codeowners"}}
</div>
{{else if .IsBlockedByOutdatedBranch}}
<div class="item text red">
{{svg "octicon-x"}}
Expand Down
7 changes: 7 additions & 0 deletions templates/repo/settings/protected_branch.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,13 @@
<p class="help">{{ctx.Locale.Tr "repo.settings.block_on_official_review_requests_desc"}}</p>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="block_on_codeowner_reviews" type="checkbox" {{if .Rule.BlockOnCodeownerReviews}}checked{{end}}>
<label>{{ctx.Locale.Tr "repo.settings.block_on_codeowner_reviews"}}</label>
<p class="help">{{ctx.Locale.Tr "repo.settings.block_on_codeowner_reviews_desc"}}</p>
</div>
</div>
<div class="field">
<div class="ui checkbox">
<input name="block_on_outdated_branch" type="checkbox" {{if .Rule.BlockOnOutdatedBranch}}checked{{end}}>
Expand Down
12 changes: 12 additions & 0 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading