support protected branch rules

This commit is contained in:
Lunny Xiao 2022-09-12 17:37:32 +08:00
parent 5e558751cf
commit 553869a7e2
No known key found for this signature in database
GPG key ID: C3B7C91B632F738A
13 changed files with 114 additions and 64 deletions

View file

@ -6,6 +6,7 @@ package git
import (
"context"
"errors"
"fmt"
"time"
@ -16,6 +17,8 @@ import (
"code.gitea.io/gitea/modules/timeutil"
)
var ErrBranchIsProtected = errors.New("branch is protected")
// DeletedBranch struct
type DeletedBranch struct {
ID int64 `xorm:"pk autoincr"`
@ -149,7 +152,7 @@ func RenameBranch(repo *repo_model.Repository, from, to string, gitAction func(i
}
// 2. Update protected branch if needed
protectedBranch, err := GetProtectedBranchBy(ctx, repo.ID, from)
protectedBranch, err := GetProtectedBranchRuleByName(ctx, repo.ID, from)
if err != nil {
return err
}
@ -160,6 +163,14 @@ func RenameBranch(repo *repo_model.Repository, from, to string, gitAction func(i
if err != nil {
return err
}
} else {
protected, err := IsBranchProtected(repo.ID, from)
if err != nil {
return err
}
if protected {
return ErrBranchIsProtected
}
}
// 3. Update all not merged pull request base branch name

View file

@ -26,10 +26,11 @@ import (
// ProtectedBranch struct
type ProtectedBranch struct {
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"UNIQUE(s)"`
BranchName string `xorm:"UNIQUE(s)"`
CanPush bool `xorm:"NOT NULL DEFAULT false"`
ID int64 `xorm:"pk autoincr"`
RepoID int64 `xorm:"UNIQUE(s)"`
BranchName string `xorm:"UNIQUE(s)"` // a branch name or a glob match to branch name
globRule glob.Glob `xorm:"-"`
CanPush bool `xorm:"NOT NULL DEFAULT false"`
EnableWhitelist bool
WhitelistUserIDs []int64 `xorm:"JSON TEXT"`
WhitelistTeamIDs []int64 `xorm:"JSON TEXT"`
@ -64,7 +65,10 @@ func (protectBranch *ProtectedBranch) Match(branchName string) bool {
if strings.EqualFold(protectBranch.BranchName, branchName) {
return true
}
return glob.MustCompile(protectBranch.BranchName).Match(branchName)
if protectBranch.globRule == nil {
protectBranch.globRule = glob.MustCompile(protectBranch.BranchName)
}
return protectBranch.globRule.Match(branchName)
}
// CanUserPush returns if some user could push to this protected branch
@ -234,9 +238,9 @@ func (protectBranch *ProtectedBranch) IsUnprotectedFile(patterns []glob.Glob, pa
return r
}
// GetProtectedBranchBy getting protected branch by ID/Name
func GetProtectedBranchBy(ctx context.Context, repoID int64, branchName string) (*ProtectedBranch, error) {
rel := &ProtectedBranch{RepoID: repoID, BranchName: branchName}
// GetProtectedBranchRuleByName getting protected branch rule by name
func GetProtectedBranchRuleByName(ctx context.Context, repoID int64, ruleName string) (*ProtectedBranch, error) {
rel := &ProtectedBranch{RepoID: repoID, BranchName: ruleName}
has, err := db.GetByBean(ctx, rel)
if err != nil {
return nil, err
@ -247,7 +251,7 @@ func GetProtectedBranchBy(ctx context.Context, repoID int64, branchName string)
return rel, nil
}
// GetProtectedBranchRuleByID getting protected branch by ID/Name
// GetProtectedBranchRuleByID getting protected branch rule by rule ID
func GetProtectedBranchRuleByID(ctx context.Context, repoID, ruleID int64) (*ProtectedBranch, error) {
rel := &ProtectedBranch{ID: ruleID, RepoID: repoID}
has, err := db.GetByBean(ctx, rel)
@ -333,20 +337,6 @@ func UpdateProtectBranch(ctx context.Context, repo *repo_model.Repository, prote
return nil
}
// IsProtectedBranch checks if branch is protected
func IsProtectedBranch(repoID int64, branchName string) (bool, error) {
protectedBranch := &ProtectedBranch{
RepoID: repoID,
BranchName: branchName,
}
has, err := db.GetEngine(db.DefaultContext).Exist(protectedBranch)
if err != nil {
return true, err
}
return has, nil
}
// updateApprovalWhitelist checks whether the user whitelist changed and returns a whitelist with
// the users from newWhitelist which have explicit read or write access to the repo.
func updateApprovalWhitelist(ctx context.Context, repo *repo_model.Repository, currentWhitelist, newWhitelist []int64) (whitelist []int64, err error) {

View file

@ -8,6 +8,8 @@ import (
"context"
"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/git"
"github.com/gobwas/glob"
)
type ProtectedBranchRules []*ProtectedBranch
@ -28,11 +30,37 @@ func FindRepoProtectedBranchRules(ctx context.Context, repoID int64) (ProtectedB
return rules, err
}
// FindAllMatchedBranches find all matched branches
func FindAllMatchedBranches(ctx context.Context, gitRepo *git.Repository, ruleName string) ([]string, error) {
// FIXME: how many should we get?
branches, _, err := gitRepo.GetBranchNames(0, 9999999)
if err != nil {
return nil, err
}
rule := glob.MustCompile(ruleName)
results := make([]string, 0, len(branches))
for _, branch := range branches {
if rule.Match(branch) {
results = append(results, branch)
}
}
return results, nil
}
// GetFirstMatchProtectedBranchRule returns the first matched rules
func GetFirstMatchProtectedBranchRule(ctx context.Context, repoID int64, ruleName string) (*ProtectedBranch, error) {
func GetFirstMatchProtectedBranchRule(ctx context.Context, repoID int64, branchName string) (*ProtectedBranch, error) {
rules, err := FindRepoProtectedBranchRules(ctx, repoID)
if err != nil {
return nil, err
}
return rules.GetFirstMatched(ruleName), nil
return rules.GetFirstMatched(branchName), nil
}
// IsBranchProtected checks if branch is protected
func IsBranchProtected(repoID int64, branchName string) (bool, error) {
rule, err := GetFirstMatchProtectedBranchRule(db.DefaultContext, repoID, branchName)
if err != nil {
return false, err
}
return rule != nil, nil
}

View file

@ -53,7 +53,7 @@ type BranchProtection struct {
// CreateBranchProtectionOption options for creating a branch protection
type CreateBranchProtectionOption struct {
BranchName string `json:"branch_name"`
RuleName string `json:"branch_name"` // it now in fact stores rule name not only branch name
EnablePush bool `json:"enable_push"`
EnablePushWhitelist bool `json:"enable_push_whitelist"`
PushWhitelistUsernames []string `json:"push_whitelist_usernames"`

View file

@ -125,7 +125,7 @@ func DeleteBranch(ctx *context.APIContext) {
ctx.NotFound(err)
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
case errors.Is(err, repo_service.ErrBranchIsProtected):
case errors.Is(err, git_model.ErrBranchIsProtected):
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
default:
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)
@ -323,7 +323,7 @@ func GetBranchProtection(ctx *context.APIContext) {
repo := ctx.Repo.Repository
bpName := ctx.Params(":name")
bp, err := git_model.GetProtectedBranchBy(ctx, repo.ID, bpName)
bp, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
return
@ -410,12 +410,13 @@ func CreateBranchProtection(ctx *context.APIContext) {
repo := ctx.Repo.Repository
// Currently protection must match an actual branch
if !git.IsBranchExist(ctx.Req.Context(), ctx.Repo.Repository.RepoPath(), form.BranchName) {
// FIXME: we should allow glob match
if !git.IsBranchExist(ctx.Req.Context(), ctx.Repo.Repository.RepoPath(), form.RuleName) {
ctx.NotFound()
return
}
protectBranch, err := git_model.GetProtectedBranchBy(ctx, repo.ID, form.BranchName)
protectBranch, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, form.RuleName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectBranchOfRepoByName", err)
return
@ -489,7 +490,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
protectBranch = &git_model.ProtectedBranch{
RepoID: ctx.Repo.Repository.ID,
BranchName: form.BranchName,
BranchName: form.RuleName,
CanPush: form.EnablePush,
EnableWhitelist: form.EnablePush && form.EnablePushWhitelist,
EnableMergeWhitelist: form.EnableMergeWhitelist,
@ -520,13 +521,22 @@ func CreateBranchProtection(ctx *context.APIContext) {
return
}
if err = pull_service.CheckPrsForBaseBranch(ctx.Repo.Repository, protectBranch.BranchName); err != nil {
ctx.Error(http.StatusInternalServerError, "CheckPrsForBaseBranch", err)
// FIXME: since we only need to recheck files protected rules, we could improve this
matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.GitRepo, form.RuleName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "FindAllMatchedBranches", err)
return
}
for _, branchName := range matchedBranches {
if err = pull_service.CheckPRsForBaseBranch(ctx.Repo.Repository, branchName); err != nil {
ctx.Error(http.StatusInternalServerError, "CheckPRsForBaseBranch", err)
return
}
}
// Reload from db to get all whitelists
bp, err := git_model.GetProtectedBranchBy(ctx, ctx.Repo.Repository.ID, form.BranchName)
bp, err := git_model.GetProtectedBranchRuleByName(ctx, ctx.Repo.Repository.ID, form.RuleName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
return
@ -578,7 +588,7 @@ func EditBranchProtection(ctx *context.APIContext) {
form := web.GetForm(ctx).(*api.EditBranchProtectionOption)
repo := ctx.Repo.Repository
bpName := ctx.Params(":name")
protectBranch, err := git_model.GetProtectedBranchBy(ctx, repo.ID, bpName)
protectBranch, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
return
@ -755,13 +765,22 @@ func EditBranchProtection(ctx *context.APIContext) {
return
}
if err = pull_service.CheckPrsForBaseBranch(ctx.Repo.Repository, protectBranch.BranchName); err != nil {
ctx.Error(http.StatusInternalServerError, "CheckPrsForBaseBranch", err)
// FIXME: since we only need to recheck files protected rules, we could improve this
matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.GitRepo, protectBranch.BranchName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "FindAllMatchedBranches", err)
return
}
for _, branchName := range matchedBranches {
if err = pull_service.CheckPRsForBaseBranch(ctx.Repo.Repository, branchName); err != nil {
ctx.Error(http.StatusInternalServerError, "CheckPrsForBaseBranch", err)
return
}
}
// Reload from db to ensure get all whitelists
bp, err := git_model.GetProtectedBranchBy(ctx, repo.ID, bpName)
bp, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchBy", err)
return
@ -805,7 +824,7 @@ func DeleteBranchProtection(ctx *context.APIContext) {
repo := ctx.Repo.Repository
bpName := ctx.Params(":name")
bp, err := git_model.GetProtectedBranchBy(ctx, repo.ID, bpName)
bp, err := git_model.GetProtectedBranchRuleByName(ctx, repo.ID, bpName)
if err != nil {
ctx.Error(http.StatusInternalServerError, "GetProtectedBranchByID", err)
return

View file

@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models"
activities_model "code.gitea.io/gitea/models/activities"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
pull_model "code.gitea.io/gitea/models/pull"
@ -896,7 +897,7 @@ func MergePullRequest(ctx *context.APIContext) {
ctx.NotFound(err)
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Error(http.StatusForbidden, "DefaultBranch", fmt.Errorf("can not delete default branch"))
case errors.Is(err, repo_service.ErrBranchIsProtected):
case errors.Is(err, git_model.ErrBranchIsProtected):
ctx.Error(http.StatusForbidden, "IsProtectedBranch", fmt.Errorf("branch protected"))
default:
ctx.Error(http.StatusInternalServerError, "DeleteBranch", err)

View file

@ -100,7 +100,7 @@ func DeleteBranchPost(ctx *context.Context) {
case errors.Is(err, repo_service.ErrBranchIsDefault):
log.Debug("DeleteBranch: Can't delete default branch '%s'", branchName)
ctx.Flash.Error(ctx.Tr("repo.branch.default_deletion_failed", branchName))
case errors.Is(err, repo_service.ErrBranchIsProtected):
case errors.Is(err, git_model.ErrBranchIsProtected):
log.Debug("DeleteBranch: Can't delete protected branch '%s'", branchName)
ctx.Flash.Error(ctx.Tr("repo.branch.protected_deletion_failed", branchName))
default:

View file

@ -1600,7 +1600,7 @@ func ViewIssue(ctx *context.Context) {
if perm.CanWrite(unit.TypeCode) {
// Check if branch is not protected
if pull.HeadBranch != pull.HeadRepo.DefaultBranch {
if protected, err := git_model.IsProtectedBranch(pull.HeadRepo.ID, pull.HeadBranch); err != nil {
if protected, err := git_model.IsBranchProtected(pull.HeadRepo.ID, pull.HeadBranch); err != nil {
log.Error("IsProtectedBranch: %v", err)
} else if !protected {
canDelete = true

View file

@ -1390,7 +1390,7 @@ func deleteBranch(ctx *context.Context, pr *issues_model.PullRequest, gitRepo *g
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
case errors.Is(err, repo_service.ErrBranchIsDefault):
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
case errors.Is(err, repo_service.ErrBranchIsProtected):
case errors.Is(err, git_model.ErrBranchIsProtected):
ctx.Flash.Error(ctx.Tr("repo.branch.deletion_failed", fullBranchName))
default:
log.Error("DeleteBranch: %v", err)

View file

@ -252,10 +252,20 @@ func SettingsProtectedBranchPost(ctx *context.Context) {
ctx.ServerError("UpdateProtectBranch", err)
return
}
if err = pull_service.CheckPrsForBaseBranch(ctx.Repo.Repository, protectBranch.BranchName); err != nil {
ctx.ServerError("CheckPrsForBaseBranch", err)
// FIXME: since we only need to recheck files protected rules, we could improve this
matchedBranches, err := git_model.FindAllMatchedBranches(ctx, ctx.Repo.GitRepo, protectBranch.BranchName)
if err != nil {
ctx.ServerError("FindAllMatchedBranches", err)
return
}
for _, branchName := range matchedBranches {
if err = pull_service.CheckPRsForBaseBranch(ctx.Repo.Repository, branchName); err != nil {
ctx.ServerError("CheckPRsForBaseBranch", err)
return
}
}
ctx.Flash.Success(ctx.Tr("repo.settings.update_protect_branch_success", protectBranch.BranchName))
ctx.Redirect(fmt.Sprintf("%s/settings/branches/%d", ctx.Repo.RepoLink, protectBranch.ID))
}

View file

@ -351,8 +351,8 @@ func testPR(id int64) {
checkAndUpdateStatus(pr)
}
// CheckPrsForBaseBranch check all pulls with bseBrannch
func CheckPrsForBaseBranch(baseRepo *repo_model.Repository, baseBranchName string) error {
// CheckPRsForBaseBranch check all pulls with baseBrannch
func CheckPRsForBaseBranch(baseRepo *repo_model.Repository, baseBranchName string) error {
prs, err := issues_model.GetUnmergedPullRequestsByBaseInfo(baseRepo.ID, baseBranchName)
if err != nil {
return err

View file

@ -149,8 +149,7 @@ func RenameBranch(repo *repo_model.Repository, doer *user_model.User, gitRepo *g
// enmuerates all branch related errors
var (
ErrBranchIsDefault = errors.New("branch is default")
ErrBranchIsProtected = errors.New("branch is protected")
ErrBranchIsDefault = errors.New("branch is default")
)
// DeleteBranch delete branch
@ -159,13 +158,12 @@ func DeleteBranch(doer *user_model.User, repo *repo_model.Repository, gitRepo *g
return ErrBranchIsDefault
}
isProtected, err := git_model.IsProtectedBranch(repo.ID, branchName)
isProtected, err := git_model.IsBranchProtected(repo.ID, branchName)
if err != nil {
return err
}
if isProtected {
return ErrBranchIsProtected
return git_model.ErrBranchIsProtected
}
commit, err := gitRepo.GetBranchCommit(branchName)

View file

@ -44,9 +44,10 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
csrf := GetCSRF(t, session, "/user2/repo1/settings/branches")
// Change master branch to protected
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/master", map[string]string{
"_csrf": csrf,
"protected": "on",
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/new", map[string]string{
"_csrf": csrf,
"rule_name": "master",
"enable_push": "true",
})
session.MakeRequest(t, req, http.StatusSeeOther)
// Check if master branch has been locked successfully
@ -78,14 +79,6 @@ func TestCreateFileOnProtectedBranch(t *testing.T) {
// remove the protected branch
csrf = GetCSRF(t, session, "/user2/repo1/settings/branches")
// Change master branch to protected
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/new", map[string]string{
"_csrf": csrf,
"rule_name": "master",
"enable_push": "true",
})
session.MakeRequest(t, req, http.StatusOK)
// Change master branch to protected
req = NewRequestWithValues(t, "POST", "/user2/repo1/settings/branches/1/delete", map[string]string{
"_csrf": csrf,