Prevent merge of outdated PRs on protected branches (#11012)
* Block PR on Outdated Branch * finalize * cleanup * fix typo and sentences thanks @guillep2k Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com> Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com> Co-authored-by: Lauris BH <lauris@nix.lv>
This commit is contained in:
		
							parent
							
								
									2cb5878529
								
							
						
					
					
						commit
						c52d48aae4
					
				| @ -47,6 +47,7 @@ type ProtectedBranch struct { | ||||
| 	ApprovalsWhitelistTeamIDs []int64  `xorm:"JSON TEXT"` | ||||
| 	RequiredApprovals         int64    `xorm:"NOT NULL DEFAULT 0"` | ||||
| 	BlockOnRejectedReviews    bool     `xorm:"NOT NULL DEFAULT false"` | ||||
| 	BlockOnOutdatedBranch     bool     `xorm:"NOT NULL DEFAULT false"` | ||||
| 	DismissStaleApprovals     bool     `xorm:"NOT NULL DEFAULT false"` | ||||
| 	RequireSignedCommits      bool     `xorm:"NOT NULL DEFAULT false"` | ||||
| 	ProtectedFilePatterns     string   `xorm:"TEXT"` | ||||
| @ -194,6 +195,11 @@ func (protectBranch *ProtectedBranch) MergeBlockedByRejectedReview(pr *PullReque | ||||
| 	return rejectExist | ||||
| } | ||||
| 
 | ||||
| // MergeBlockedByOutdatedBranch returns true if merge is blocked by an outdated head branch
 | ||||
| func (protectBranch *ProtectedBranch) MergeBlockedByOutdatedBranch(pr *PullRequest) bool { | ||||
| 	return protectBranch.BlockOnOutdatedBranch && pr.CommitsBehind > 0 | ||||
| } | ||||
| 
 | ||||
| // GetProtectedFilePatterns parses a semicolon separated list of protected file patterns and returns a glob.Glob slice
 | ||||
| func (protectBranch *ProtectedBranch) GetProtectedFilePatterns() []glob.Glob { | ||||
| 	extarr := make([]glob.Glob, 0, 10) | ||||
|  | ||||
| @ -202,10 +202,12 @@ var migrations = []Migration{ | ||||
| 	NewMigration("Add EmailHash Table", addEmailHashTable), | ||||
| 	// v134 -> v135
 | ||||
| 	NewMigration("Refix merge base for merged pull requests", refixMergeBase), | ||||
| 	// v135 -> 136
 | ||||
| 	// v135 -> v136
 | ||||
| 	NewMigration("Add OrgID column to Labels table", addOrgIDLabelColumn), | ||||
| 	// v136 -> 137
 | ||||
| 	// v136 -> v137
 | ||||
| 	NewMigration("Add CommitsAhead and CommitsBehind Column to PullRequest Table", addCommitDivergenceToPulls), | ||||
| 	// v137 -> v138
 | ||||
| 	NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch), | ||||
| } | ||||
| 
 | ||||
| // GetCurrentDBVersion returns the current db version
 | ||||
|  | ||||
							
								
								
									
										16
									
								
								models/migrations/v137.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										16
									
								
								models/migrations/v137.go
									
									
									
									
									
										Normal file
									
								
							| @ -0,0 +1,16 @@ | ||||
| // Copyright 2020 The Gitea Authors. All rights reserved.
 | ||||
| // Use of this source code is governed by a MIT-style
 | ||||
| // license that can be found in the LICENSE file.
 | ||||
| 
 | ||||
| package migrations | ||||
| 
 | ||||
| import ( | ||||
| 	"xorm.io/xorm" | ||||
| ) | ||||
| 
 | ||||
| func addBlockOnOutdatedBranch(x *xorm.Engine) error { | ||||
| 	type ProtectedBranch struct { | ||||
| 		BlockOnOutdatedBranch bool `xorm:"NOT NULL DEFAULT false"` | ||||
| 	} | ||||
| 	return x.Sync2(new(ProtectedBranch)) | ||||
| } | ||||
| @ -173,6 +173,7 @@ type ProtectBranchForm struct { | ||||
| 	ApprovalsWhitelistUsers  string | ||||
| 	ApprovalsWhitelistTeams  string | ||||
| 	BlockOnRejectedReviews   bool | ||||
| 	BlockOnOutdatedBranch    bool | ||||
| 	DismissStaleApprovals    bool | ||||
| 	RequireSignedCommits     bool | ||||
| 	ProtectedFilePatterns    string | ||||
|  | ||||
| @ -118,6 +118,7 @@ func ToBranchProtection(bp *models.ProtectedBranch) *api.BranchProtection { | ||||
| 		ApprovalsWhitelistUsernames: approvalsWhitelistUsernames, | ||||
| 		ApprovalsWhitelistTeams:     approvalsWhitelistTeams, | ||||
| 		BlockOnRejectedReviews:      bp.BlockOnRejectedReviews, | ||||
| 		BlockOnOutdatedBranch:       bp.BlockOnOutdatedBranch, | ||||
| 		DismissStaleApprovals:       bp.DismissStaleApprovals, | ||||
| 		RequireSignedCommits:        bp.RequireSignedCommits, | ||||
| 		ProtectedFilePatterns:       bp.ProtectedFilePatterns, | ||||
|  | ||||
| @ -39,6 +39,7 @@ type BranchProtection struct { | ||||
| 	ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` | ||||
| 	ApprovalsWhitelistTeams     []string `json:"approvals_whitelist_teams"` | ||||
| 	BlockOnRejectedReviews      bool     `json:"block_on_rejected_reviews"` | ||||
| 	BlockOnOutdatedBranch       bool     `json:"block_on_outdated_branch"` | ||||
| 	DismissStaleApprovals       bool     `json:"dismiss_stale_approvals"` | ||||
| 	RequireSignedCommits        bool     `json:"require_signed_commits"` | ||||
| 	ProtectedFilePatterns       string   `json:"protected_file_patterns"` | ||||
| @ -66,6 +67,7 @@ type CreateBranchProtectionOption struct { | ||||
| 	ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` | ||||
| 	ApprovalsWhitelistTeams     []string `json:"approvals_whitelist_teams"` | ||||
| 	BlockOnRejectedReviews      bool     `json:"block_on_rejected_reviews"` | ||||
| 	BlockOnOutdatedBranch       bool     `json:"block_on_outdated_branch"` | ||||
| 	DismissStaleApprovals       bool     `json:"dismiss_stale_approvals"` | ||||
| 	RequireSignedCommits        bool     `json:"require_signed_commits"` | ||||
| 	ProtectedFilePatterns       string   `json:"protected_file_patterns"` | ||||
| @ -88,6 +90,7 @@ type EditBranchProtectionOption struct { | ||||
| 	ApprovalsWhitelistUsernames []string `json:"approvals_whitelist_username"` | ||||
| 	ApprovalsWhitelistTeams     []string `json:"approvals_whitelist_teams"` | ||||
| 	BlockOnRejectedReviews      *bool    `json:"block_on_rejected_reviews"` | ||||
| 	BlockOnOutdatedBranch       *bool    `json:"block_on_outdated_branch"` | ||||
| 	DismissStaleApprovals       *bool    `json:"dismiss_stale_approvals"` | ||||
| 	RequireSignedCommits        *bool    `json:"require_signed_commits"` | ||||
| 	ProtectedFilePatterns       *string  `json:"protected_file_patterns"` | ||||
|  | ||||
| @ -1102,6 +1102,7 @@ pulls.required_status_check_missing = Some required checks are missing. | ||||
| pulls.required_status_check_administrator = As an administrator, you may still merge this pull request. | ||||
| pulls.blocked_by_approvals = "This Pull Request doesn't have enough approvals yet. %d of %d approvals granted." | ||||
| pulls.blocked_by_rejection = "This Pull Request has changes requested by an official reviewer." | ||||
| pulls.blocked_by_outdated_branch = "This Pull Request is blocked because it's outdated." | ||||
| pulls.can_auto_merge_desc = This pull request can be merged automatically. | ||||
| pulls.cannot_auto_merge_desc = This pull request cannot be merged automatically due to conflicts. | ||||
| pulls.cannot_auto_merge_helper = Merge manually to resolve the conflicts. | ||||
| @ -1528,6 +1529,8 @@ settings.protected_branch_deletion = Disable Branch Protection | ||||
| settings.protected_branch_deletion_desc = Disabling branch protection allows users with write permission to push to the branch. Continue? | ||||
| 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_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.default_branch_desc = Select a default repository branch for pull requests and code commits: | ||||
| settings.choose_branch = Choose a branch… | ||||
| settings.no_protected_branch = There are no protected branches. | ||||
|  | ||||
| @ -340,6 +340,7 @@ func CreateBranchProtection(ctx *context.APIContext, form api.CreateBranchProtec | ||||
| 		DismissStaleApprovals:    form.DismissStaleApprovals, | ||||
| 		RequireSignedCommits:     form.RequireSignedCommits, | ||||
| 		ProtectedFilePatterns:    form.ProtectedFilePatterns, | ||||
| 		BlockOnOutdatedBranch:    form.BlockOnOutdatedBranch, | ||||
| 	} | ||||
| 
 | ||||
| 	err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ | ||||
| @ -475,6 +476,10 @@ func EditBranchProtection(ctx *context.APIContext, form api.EditBranchProtection | ||||
| 		protectBranch.ProtectedFilePatterns = *form.ProtectedFilePatterns | ||||
| 	} | ||||
| 
 | ||||
| 	if form.BlockOnOutdatedBranch != nil { | ||||
| 		protectBranch.BlockOnOutdatedBranch = *form.BlockOnOutdatedBranch | ||||
| 	} | ||||
| 
 | ||||
| 	var whitelistUsers []int64 | ||||
| 	if form.PushWhitelistUsernames != nil { | ||||
| 		whitelistUsers, err = models.GetUserIDsByNames(form.PushWhitelistUsernames, false) | ||||
|  | ||||
| @ -263,6 +263,7 @@ func HookPreReceive(ctx *macaron.Context, opts private.HookOptions) { | ||||
| 				} | ||||
| 			} | ||||
| 
 | ||||
| 			// Detect Protected file pattern
 | ||||
| 			globs := protectBranch.GetProtectedFilePatterns() | ||||
| 			if len(globs) > 0 { | ||||
| 				err := checkFileProtection(oldCommitID, newCommitID, globs, gitRepo, env) | ||||
|  | ||||
| @ -1065,6 +1065,7 @@ func ViewIssue(ctx *context.Context) { | ||||
| 			cnt := pull.ProtectedBranch.GetGrantedApprovalsCount(pull) | ||||
| 			ctx.Data["IsBlockedByApprovals"] = !pull.ProtectedBranch.HasEnoughApprovals(pull) | ||||
| 			ctx.Data["IsBlockedByRejection"] = pull.ProtectedBranch.MergeBlockedByRejectedReview(pull) | ||||
| 			ctx.Data["IsBlockedByOutdatedBranch"] = pull.ProtectedBranch.MergeBlockedByOutdatedBranch(pull) | ||||
| 			ctx.Data["GrantedApprovals"] = cnt | ||||
| 			ctx.Data["RequireSigned"] = pull.ProtectedBranch.RequireSignedCommits | ||||
| 		} | ||||
|  | ||||
| @ -248,6 +248,7 @@ func SettingsProtectedBranchPost(ctx *context.Context, f auth.ProtectBranchForm) | ||||
| 		protectBranch.DismissStaleApprovals = f.DismissStaleApprovals | ||||
| 		protectBranch.RequireSignedCommits = f.RequireSignedCommits | ||||
| 		protectBranch.ProtectedFilePatterns = f.ProtectedFilePatterns | ||||
| 		protectBranch.BlockOnOutdatedBranch = f.BlockOnOutdatedBranch | ||||
| 
 | ||||
| 		err = models.UpdateProtectBranch(ctx.Repo.Repository, protectBranch, models.WhitelistOptions{ | ||||
| 			UserIDs:          whitelistUsers, | ||||
|  | ||||
| @ -574,16 +574,22 @@ func CheckPRReadyToMerge(pr *models.PullRequest) (err error) { | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if enoughApprovals := pr.ProtectedBranch.HasEnoughApprovals(pr); !enoughApprovals { | ||||
| 	if !pr.ProtectedBranch.HasEnoughApprovals(pr) { | ||||
| 		return models.ErrNotAllowedToMerge{ | ||||
| 			Reason: "Does not have enough approvals", | ||||
| 		} | ||||
| 	} | ||||
| 	if rejected := pr.ProtectedBranch.MergeBlockedByRejectedReview(pr); rejected { | ||||
| 	if pr.ProtectedBranch.MergeBlockedByRejectedReview(pr) { | ||||
| 		return models.ErrNotAllowedToMerge{ | ||||
| 			Reason: "There are requested changes", | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	if pr.ProtectedBranch.MergeBlockedByOutdatedBranch(pr) { | ||||
| 		return models.ErrNotAllowedToMerge{ | ||||
| 			Reason: "The head branch is behind the base branch", | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	return nil | ||||
| } | ||||
|  | ||||
| @ -66,6 +66,7 @@ | ||||
| 	{{- else if .IsPullRequestBroken}}red | ||||
| 	{{- else if .IsBlockedByApprovals}}red | ||||
| 	{{- else if .IsBlockedByRejection}}red | ||||
| 	{{- else if .IsBlockedByOutdatedBranch}}red | ||||
| 	{{- else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsFailure .RequiredStatusCheckState.IsError)}}red | ||||
| 	{{- else if and .EnableStatusCheck (or (not $.LatestCommitStatus) .RequiredStatusCheckState.IsPending .RequiredStatusCheckState.IsWarning)}}yellow | ||||
| 	{{- else if and .RequireSigned (not .WillSign)}}}red | ||||
| @ -138,6 +139,11 @@ | ||||
| 						<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i> | ||||
| 					{{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} | ||||
| 					</div> | ||||
| 				{{else if .IsBlockedByOutdatedBranch}} | ||||
| 					<div class="item text red"> | ||||
| 						<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i> | ||||
| 					{{$.i18n.Tr "repo.pulls.blocked_by_outdated_branch"}} | ||||
| 					</div> | ||||
| 				{{else if and .EnableStatusCheck (or .RequiredStatusCheckState.IsError .RequiredStatusCheckState.IsFailure)}} | ||||
| 					<div class="item text red"> | ||||
| 						<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i> | ||||
| @ -158,7 +164,7 @@ | ||||
| 						{{$.i18n.Tr (printf "repo.signing.wont_sign.%s" .WontSignReason) }} | ||||
| 					</div> | ||||
| 				{{end}} | ||||
| 				{{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} | ||||
| 				{{$notAllOverridableChecksOk := or .IsBlockedByApprovals .IsBlockedByRejection .IsBlockedByOutdatedBranch (and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess))}} | ||||
| 				{{if and (or $.IsRepoAdmin (not $notAllOverridableChecksOk)) (or (not .RequireSigned) .WillSign)}} | ||||
| 					{{if $notAllOverridableChecksOk}} | ||||
| 						<div class="item text yellow"> | ||||
| @ -342,6 +348,11 @@ | ||||
| 						{{svg "octicon-x" 16}} | ||||
| 					{{$.i18n.Tr "repo.pulls.blocked_by_rejection"}} | ||||
| 					</div> | ||||
| 				{{else if .IsBlockedByOutdatedBranch}} | ||||
| 					<div class="item text red"> | ||||
| 						<i class="icon icon-octicon">{{svg "octicon-x" 16}}</i> | ||||
| 					{{$.i18n.Tr "repo.pulls.blocked_by_outdated_branch"}} | ||||
| 					</div> | ||||
| 				{{else if and .EnableStatusCheck (not .RequiredStatusCheckState.IsSuccess)}} | ||||
| 					<div class="item text red"> | ||||
| 						{{svg "octicon-x" 16}} | ||||
|  | ||||
| @ -225,6 +225,13 @@ | ||||
| 							<p class="help">{{.i18n.Tr "repo.settings.require_signed_commits_desc"}}</p> | ||||
| 						</div> | ||||
| 					</div> | ||||
| 					<div class="field"> | ||||
| 						<div class="ui checkbox"> | ||||
| 							<input name="block_on_outdated_branch" type="checkbox" {{if .Branch.BlockOnOutdatedBranch}}checked{{end}}> | ||||
| 							<label for="block_on_outdated_branch">{{.i18n.Tr "repo.settings.block_outdated_branch"}}</label> | ||||
| 							<p class="help">{{.i18n.Tr "repo.settings.block_outdated_branch_desc"}}</p> | ||||
| 						</div> | ||||
| 					</div> | ||||
| 					<div class="field"> | ||||
| 						<label for="protected_file_patterns">{{.i18n.Tr "repo.settings.protect_protected_file_patterns"}}</label> | ||||
| 						<input name="protected_file_patterns" id="protected_file_patterns" type="text" value="{{.Branch.ProtectedFilePatterns}}"> | ||||
|  | ||||
| @ -10072,6 +10072,10 @@ | ||||
|           }, | ||||
|           "x-go-name": "ApprovalsWhitelistUsernames" | ||||
|         }, | ||||
|         "block_on_outdated_branch": { | ||||
|           "type": "boolean", | ||||
|           "x-go-name": "BlockOnOutdatedBranch" | ||||
|         }, | ||||
|         "block_on_rejected_reviews": { | ||||
|           "type": "boolean", | ||||
|           "x-go-name": "BlockOnRejectedReviews" | ||||
| @ -10392,6 +10396,10 @@ | ||||
|           }, | ||||
|           "x-go-name": "ApprovalsWhitelistUsernames" | ||||
|         }, | ||||
|         "block_on_outdated_branch": { | ||||
|           "type": "boolean", | ||||
|           "x-go-name": "BlockOnOutdatedBranch" | ||||
|         }, | ||||
|         "block_on_rejected_reviews": { | ||||
|           "type": "boolean", | ||||
|           "x-go-name": "BlockOnRejectedReviews" | ||||
| @ -11204,6 +11212,10 @@ | ||||
|           }, | ||||
|           "x-go-name": "ApprovalsWhitelistUsernames" | ||||
|         }, | ||||
|         "block_on_outdated_branch": { | ||||
|           "type": "boolean", | ||||
|           "x-go-name": "BlockOnOutdatedBranch" | ||||
|         }, | ||||
|         "block_on_rejected_reviews": { | ||||
|           "type": "boolean", | ||||
|           "x-go-name": "BlockOnRejectedReviews" | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user