Commit bf89e06a authored by Douwe Maan's avatar Douwe Maan

Merge branch '18627-wildcard-branch-protection' into 'master'

Allow specifying protected branches using wildcards

Closes #18627 

# Tasks

- [ ]  #18627 !4665 Allow specifying protected branches using wildcards
    - [x]  Find existing usages of protected branches
        - Protecting branches
            - `ProtectedBranchesController` is used to mark a branch protected/unprotected
            - `API::Branches` can be used to mark a branch protected/unprotected
        - Enforcing branch protection
            - `Gitlab::GitAccess` has helpers (`can_push_to_branch?`, `check`) that are used to deny pushes if a branch is protected
            - Over SSH: `gitlab-shell` receives a push, and calls `/allowed` on the GitLab API, which calls `GitAccess.check`
            - Over HTTP: 
                - `gitlab-workhorse` receives the request, and forwards it to rails
                - Rails (in the `GitHttpController#git-recieve-pack`) runs basic checks (is the user logged in, not protected branch checks) and returns ok with `GL_ID` and `RepoPath`
                - `gitlab-workhorse` looks at the response, and calls the relevant `gitlab-shell` action from `git-http/handlePostRPC`
                - Rest of this flow is the same as the SSH flow above
    - [x]  Implementation
        - [x]  Backend
            - [x]  Change `project#protected_branch?` to look at wildcard protected branches
            - [x]  Change `project#developers_can_push_to_protected_branch?`
            - [x]  Change `project#open_branches`
            - [x]  Better error message when creating a disallowed branch from the Web UI
        - [x]  Frontend
            - [x]  Protected branches page should allow typing out a wildcard pattern
            - [x]  Add help text explaining the use of wildcards
            - [x]  Show matching branches for each protected branch
                - [x]  ~~On the index page~~
                - [x]  On a show page
                - [x]  Index?
            - [x]  Can't have the "last commit" column for wildcard protected branches
    - [x]  Fix / write tests
    - [x]  What happens if a hook is missing in dev?
    - [x]  Refactor
    - [x]  Test workflows
        - Create a branch matching a wildcard pattern
        - Push to a branch matching a wildcard pattern
        - Force push to a branch matching a wildcard pattern
        - Delete a branch matching a wildcard pattern
        - [x]  Test using Web UI
        - [x]  Test over SSH
        - [x]  Test over HTTP
        - [x]  Test as developer and master
    - [x]  Investigate performance
        - [x]  Test with a large number of protected branches / branches
        - [x]  Paginate list of protected branches
        - [x]  ~~Possibly rewrite `open_branches`~~
    - [x]  Add `iid`s to existing `ProtectedBranch`es
    - [x]  Add documentation
    - [x]  Add CHANGELOG entry
    - [x]  Add screenshots
    - [x]  Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/2f753e3ed2ce681b4444944d521f4419e8ed37f7/builds) passes
    - [x]  Assign to endboss for review
    - [x]  Address @DouweM's comments
        - [x]  `protected_branch_params`
        - [x]  `exact_match` instead of `explicit_match`
        - [x]  When would self.name be blank?
        - [x]  Move `protected_branches.each` to a partial
        - [x]  Move `matching_branches.each` to a partial
        - [x]  If the branch is in @matching_branches, it's not been removed
        - [x]  move this regex to a method and memoize it
        - [x]  `commit_sha` directly for exact matches
        - [x]  Number of matches for wildcard matches, with a link
    - [x]  Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/43f9ce0e88194b8f719bb1c1e656b7fc13278d56/builds) to pass
    - [x]  Respond to @DouweM's comments
        - [x]  Don't use iid
        - [x]  Controller should use `@project.protected_branches.new`
        - [x]  move the memoization to `def wildcard_regex`
        - [x]  render with `collection: @protected_branches`
    - [x]  Wait for [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/f7beedf122fa0c7aa89e86181fe7499321fb10ca/builds) to pass
    - [x]  Wait for @DouweM's review
    - [x]  Wait for @jschatz1's review
    - [x]  Respond to @jschatz1's comments
        - [x]  Use the new dropdown style
        - [x]  description should be moved to the description section without the styling
        - [x]  Protect button should be disabled when no branch is selected
    - [x]  Update screenshots
    - [x]  Merge conflicts
    - [x]  Make sure [build](https://gitlab.com/gitlab-org/gitlab-ce/commit/20f3cfe8d5540eab64c2ba548043d600b28c61ba/builds) passes
    - [ ]  Revisit performance, possibly with staging/production data
        - [ ]  Get a dump of staging / run against staging live
            - [ ]  Get SSH access to staging
    - [ ]  Wait for review/merge






# Screenshots

## Creating wildcard protected branches

![1](/uploads/9446afccfdf6fa381e00c800dd2cc82e/1.png)
![2](/uploads/0b154503b297a818d3577488c575d845/2.png)
![3](/uploads/36217f79df9e41cc1550601f02627fe8/3.png)
![4](/uploads/041ca9bd529bcfa5373fca67e917cbcb/4.png)

### Using the `GLDropdown` component

![2016-06-30_14-16-15](/uploads/508afc2a5e2463c2954641409a560d88/2016-06-30_14-16-15.gif)

## Enforcing wildcard protected branches

### From the Web UI

![Screen_Shot_2016-06-20_at_1.21.18_PM](/uploads/8b5d4b1911e9152698a0488daf1880bc/Screen_Shot_2016-06-20_at_1.21.18_PM.png)

### Over SSH

![SSH](/uploads/7365989d7e4c406ef37b6ae5106442c9/SSH.gif)

### Over HTTPS

![HTTPS](/uploads/a7c0f56ae58efcffc75e6700fa2f4ac0/HTTPS.gif)

## Listing matching branches

![Screen_Shot_2016-06-20_at_1.33.44_PM](/uploads/d054113022f5d7ec64c0e57e501ac104/Screen_Shot_2016-06-20_at_1.33.44_PM.png)

See merge request !4665
parents 39fbec94 b1c81f84
...@@ -25,6 +25,7 @@ v 8.10.0 (unreleased) ...@@ -25,6 +25,7 @@ v 8.10.0 (unreleased)
- Updated layout for Projects, Groups, Users on Admin area !4424 - Updated layout for Projects, Groups, Users on Admin area !4424
- Fix changing issue state columns in milestone view - Fix changing issue state columns in milestone view
- Add notification settings dropdown for groups - Add notification settings dropdown for groups
- Wildcards for protected branches. !4665
- Allow importing from Github using Personal Access Tokens. (Eric K Idema) - Allow importing from Github using Personal Access Tokens. (Eric K Idema)
- API: Todos !3188 (Robert Schilling) - API: Todos !3188 (Robert Schilling)
- Add "Enabled Git access protocols" to Application Settings - Add "Enabled Git access protocols" to Application Settings
......
...@@ -56,6 +56,7 @@ class GitLabDropdownFilter ...@@ -56,6 +56,7 @@ class GitLabDropdownFilter
return BLUR_KEYCODES.indexOf(keyCode) >= 0 return BLUR_KEYCODES.indexOf(keyCode) >= 0
filter: (search_text) -> filter: (search_text) ->
@options.onFilter(search_text) if @options.onFilter
data = @options.data() data = @options.data()
if data? and not @options.filterByText if data? and not @options.filterByText
...@@ -195,6 +196,7 @@ class GitLabDropdown ...@@ -195,6 +196,7 @@ class GitLabDropdown
@filter = new GitLabDropdownFilter @filterInput, @filter = new GitLabDropdownFilter @filterInput,
filterInputBlur: @filterInputBlur filterInputBlur: @filterInputBlur
filterByText: @options.filterByText filterByText: @options.filterByText
onFilter: @options.onFilter
remote: @options.filterRemote remote: @options.filterRemote
query: @options.data query: @options.data
keys: searchFields keys: searchFields
...@@ -530,7 +532,7 @@ class GitLabDropdown ...@@ -530,7 +532,7 @@ class GitLabDropdown
if $el.length if $el.length
e.preventDefault() e.preventDefault()
e.stopImmediatePropagation() e.stopImmediatePropagation()
$(selector, @dropdown)[0].click() $el.first().trigger('click')
addArrowKeyEvent: -> addArrowKeyEvent: ->
ARROW_KEY_CODES = [38, 40] ARROW_KEY_CODES = [38, 40]
......
class @ProtectedBranchSelect
constructor: (currentProject) ->
$('.dropdown-footer').hide();
@dropdown = $('.js-protected-branch-select').glDropdown(
data: @getProtectedBranches
filterable: true
remote: false
search:
fields: ['title']
selectable: true
toggleLabel: (selected) -> if (selected and 'id' of selected) then selected.title else 'Protected Branch'
fieldName: 'protected_branch[name]'
text: (protected_branch) -> _.escape(protected_branch.title)
id: (protected_branch) -> _.escape(protected_branch.id)
onFilter: @toggleCreateNewButton
clicked: () -> $('.protect-branch-btn').attr('disabled', false)
)
$('.create-new-protected-branch').on 'click', (event) =>
# Refresh the dropdown's data, which ends up calling `getProtectedBranches`
@dropdown.data('glDropdown').remote.execute()
@dropdown.data('glDropdown').selectRowAtIndex(event, 0)
getProtectedBranches: (term, callback) =>
if @selectedBranch
callback(gon.open_branches.concat(@selectedBranch))
else
callback(gon.open_branches)
toggleCreateNewButton: (branchName) =>
@selectedBranch = { title: branchName, id: branchName, text: branchName }
if branchName is ''
$('.protected-branch-select-footer-list').addClass('hidden')
$('.dropdown-footer').hide();
else
$('.create-new-protected-branch').text("Create Protected Branch: #{branchName}")
$('.protected-branch-select-footer-list').removeClass('hidden')
$('.dropdown-footer').show();
...@@ -11,7 +11,8 @@ $ -> ...@@ -11,7 +11,8 @@ $ ->
dataType: "json" dataType: "json"
data: data:
id: id id: id
developers_can_push: checked protected_branch:
developers_can_push: checked
success: -> success: ->
row = $(e.target) row = $(e.target)
......
...@@ -2,12 +2,14 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController ...@@ -2,12 +2,14 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
# Authorize # Authorize
before_action :require_non_empty_project before_action :require_non_empty_project
before_action :authorize_admin_project! before_action :authorize_admin_project!
before_action :load_protected_branch, only: [:show, :update, :destroy]
layout "project_settings" layout "project_settings"
def index def index
@branches = @project.protected_branches.to_a @protected_branches = @project.protected_branches.order(:name).page(params[:page])
@protected_branch = @project.protected_branches.new @protected_branch = @project.protected_branches.new
gon.push({ open_branches: @project.open_branches.map { |br| { text: br.name, id: br.name, title: br.name } } })
end end
def create def create
...@@ -16,26 +18,24 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController ...@@ -16,26 +18,24 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
@project) @project)
end end
def update def show
protected_branch = @project.protected_branches.find(params[:id]) @matching_branches = @protected_branch.matching(@project.repository.branches)
end
if protected_branch &&
protected_branch.update_attributes(
developers_can_push: params[:developers_can_push]
)
def update
if @protected_branch && @protected_branch.update_attributes(protected_branch_params)
respond_to do |format| respond_to do |format|
format.json { render json: protected_branch, status: :ok } format.json { render json: @protected_branch, status: :ok }
end end
else else
respond_to do |format| respond_to do |format|
format.json { render json: protected_branch.errors, status: :unprocessable_entity } format.json { render json: @protected_branch.errors, status: :unprocessable_entity }
end end
end end
end end
def destroy def destroy
@project.protected_branches.find(params[:id]).destroy @protected_branch.destroy
respond_to do |format| respond_to do |format|
format.html { redirect_to namespace_project_protected_branches_path } format.html { redirect_to namespace_project_protected_branches_path }
...@@ -45,6 +45,10 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController ...@@ -45,6 +45,10 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController
private private
def load_protected_branch
@protected_branch = @project.protected_branches.find(params[:id])
end
def protected_branch_params def protected_branch_params
params.require(:protected_branch).permit(:name, :developers_can_push) params.require(:protected_branch).permit(:name, :developers_can_push)
end end
......
...@@ -802,18 +802,12 @@ class Project < ActiveRecord::Base ...@@ -802,18 +802,12 @@ class Project < ActiveRecord::Base
@repo_exists = false @repo_exists = false
end end
# Branches that are not _exactly_ matched by a protected branch.
def open_branches def open_branches
# We're using a Set here as checking values in a large Set is faster than exact_protected_branch_names = protected_branches.reject(&:wildcard?).map(&:name)
# checking values in a large Array. branch_names = repository.branches.map(&:name)
protected_set = Set.new(protected_branch_names) non_open_branch_names = Set.new(exact_protected_branch_names).intersection(Set.new(branch_names))
repository.branches.reject { |branch| non_open_branch_names.include? branch.name }
repository.branches.reject do |branch|
protected_set.include?(branch.name)
end
end
def protected_branch_names
@protected_branch_names ||= protected_branches.pluck(:name)
end end
def root_ref?(branch) def root_ref?(branch)
...@@ -830,11 +824,12 @@ class Project < ActiveRecord::Base ...@@ -830,11 +824,12 @@ class Project < ActiveRecord::Base
# Check if current branch name is marked as protected in the system # Check if current branch name is marked as protected in the system
def protected_branch?(branch_name) def protected_branch?(branch_name)
protected_branch_names.include?(branch_name) @protected_branches ||= self.protected_branches.to_a
ProtectedBranch.matching(branch_name, protected_branches: @protected_branches).present?
end end
def developers_can_push_to_protected_branch?(branch_name) def developers_can_push_to_protected_branch?(branch_name)
protected_branches.any? { |pb| pb.name == branch_name && pb.developers_can_push } protected_branches.matching(branch_name).any?(&:developers_can_push)
end end
def forked? def forked?
......
...@@ -8,4 +8,51 @@ class ProtectedBranch < ActiveRecord::Base ...@@ -8,4 +8,51 @@ class ProtectedBranch < ActiveRecord::Base
def commit def commit
project.commit(self.name) project.commit(self.name)
end end
# Returns all protected branches that match the given branch name.
# This realizes all records from the scope built up so far, and does
# _not_ return a relation.
#
# This method optionally takes in a list of `protected_branches` to search
# through, to avoid calling out to the database.
def self.matching(branch_name, protected_branches: nil)
(protected_branches || all).select { |protected_branch| protected_branch.matches?(branch_name) }
end
# Returns all branches (among the given list of branches [`Gitlab::Git::Branch`])
# that match the current protected branch.
def matching(branches)
branches.select { |branch| self.matches?(branch.name) }
end
# Checks if the protected branch matches the given branch name.
def matches?(branch_name)
return false if self.name.blank?
exact_match?(branch_name) || wildcard_match?(branch_name)
end
# Checks if this protected branch contains a wildcard
def wildcard?
self.name && self.name.include?('*')
end
protected
def exact_match?(branch_name)
self.name == branch_name
end
def wildcard_match?(branch_name)
wildcard_regex === branch_name
end
def wildcard_regex
@wildcard_regex ||= begin
name = self.name.gsub('*', 'STAR_DONT_ESCAPE')
quoted_name = Regexp.quote(name)
regex_string = quoted_name.gsub('STAR_DONT_ESCAPE', '.*?')
/\A#{regex_string}\z/
end
end
end end
%h5.prepend-top-0 %h5.prepend-top-0
Already Protected (#{@branches.size}) Already Protected (#{@protected_branches.size})
- if @branches.empty? - if @protected_branches.empty?
%p.settings-message.text-center %p.settings-message.text-center
No branches are protected, protect a branch with the form above. No branches are protected, protect a branch with the form above.
- else - else
...@@ -9,33 +9,18 @@ ...@@ -9,33 +9,18 @@
%table.table.protected-branches-list %table.table.protected-branches-list
%colgroup %colgroup
%col{ width: "30%" } %col{ width: "30%" }
%col{ width: "30%" } %col{ width: "25%" }
%col{ width: "25%" } %col{ width: "25%" }
- if can_admin_project - if can_admin_project
%col %col
%thead %thead
%tr %tr
%th Branch %th Protected Branch
%th Last commit %th Commit
%th Developers can push %th Developers Can Push
- if can_admin_project - if can_admin_project
%th %th
%tbody %tbody
- @branches.each do |branch| = render partial: @protected_branches, locals: { can_admin_project: can_admin_project }
- @url = namespace_project_protected_branch_path(@project.namespace, @project, branch)
%tr = paginate @protected_branches, theme: 'gitlab'
%td
= link_to(branch.name, namespace_project_commits_path(@project.namespace, @project, branch.name))
- if @project.root_ref?(branch.name)
%span.label.label-info.prepend-left-5 default
%td
- if commit = branch.commit
= link_to(commit.short_id, namespace_project_commit_path(@project.namespace, @project, commit.id), class: 'commit_short_id')
#{time_ago_with_tooltip(commit.committed_date)}
- else
(branch was removed from repository)
%td
= check_box_tag("developers_can_push", branch.id, branch.developers_can_push, data: { url: @url })
- if can_admin_project
%td
= link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm"
= f.hidden_field(:name)
= dropdown_tag("Protected Branch",
options: { title: "Pick protected branch", toggle_class: 'js-protected-branch-select js-filter-submit',
filter: true, dropdown_class: "dropdown-menu-selectable", placeholder: "Search protected branches",
footer_content: true,
data: { show_no: true, show_any: true, show_upcoming: true,
selected: params[:protected_branch_name],
project_id: @project.try(:id) } }) do
%ul.dropdown-footer-list.hidden.protected-branch-select-footer-list
%li
= link_to '#', title: "New Protected Branch", class: "create-new-protected-branch" do
Create new
:javascript
new ProtectedBranchSelect();
%tr
%td
= link_to matching_branch.name, namespace_project_tree_path(@project.namespace, @project, matching_branch.name)
- if @project.root_ref?(matching_branch.name)
%span.label.label-info.prepend-left-5 default
%td
- commit = @project.commit(matching_branch.name)
= link_to(commit.short_id, namespace_project_commit_path(@project.namespace, @project, commit.id), class: 'commit_short_id')
= time_ago_with_tooltip(commit.committed_date)
- url = namespace_project_protected_branch_path(@project.namespace, @project, protected_branch)
%tr
%td
= protected_branch.name
- if @project.root_ref?(protected_branch.name)
%span.label.label-info.prepend-left-5 default
%td
- if protected_branch.wildcard?
- matching_branches = protected_branch.matching(repository.branches)
= link_to pluralize(matching_branches.count, "matching branch"), namespace_project_protected_branch_path(@project.namespace, @project, protected_branch)
- else
- if commit = protected_branch.commit
= link_to(commit.short_id, namespace_project_commit_path(@project.namespace, @project, commit.id), class: 'commit_short_id')
= time_ago_with_tooltip(commit.committed_date)
- else
(branch was removed from repository)
%td
= check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url })
- if can_admin_project
%td
= link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right"
...@@ -4,30 +4,38 @@ ...@@ -4,30 +4,38 @@
.col-lg-3 .col-lg-3
%h4.prepend-top-0 %h4.prepend-top-0
= page_title = page_title
%p Keep stable branches secure and force developers to use Merge Requests %p Keep stable branches secure and force developers to use merge requests.
.col-lg-9 %p.prepend-top-20
%h5.prepend-top-0 Protected branches are designed to:
Protect a branch
.account-well.append-bottom-default
%p.light-header.append-bottom-0 Protected branches are designed to
%ul %ul
%li prevent pushes from everybody except #{link_to "masters", help_page_path("permissions", "permissions"), class: "vlink"} %li prevent pushes from everybody except #{link_to "masters", help_page_path("permissions", "permissions"), class: "vlink"}
%li prevent anyone from force pushing to the branch %li prevent anyone from force pushing to the branch
%li prevent anyone from deleting the branch %li prevent anyone from deleting the branch
%p.append-bottom-0 Read more about #{link_to "project permissions", help_page_path("permissions", "permissions"), class: "underlined-link"} %p.append-bottom-0 Read more about #{link_to "project permissions", help_page_path("permissions", "permissions"), class: "underlined-link"}
.col-lg-9
%h5.prepend-top-0
Protect a branch
- if can? current_user, :admin_project, @project - if can? current_user, :admin_project, @project
= form_for [@project.namespace.becomes(Namespace), @project, @protected_branch] do |f| = form_for [@project.namespace.becomes(Namespace), @project, @protected_branch] do |f|
= form_errors(@protected_branch) = form_errors(@protected_branch)
.form-group .form-group
= f.label :name, "Branch", class: "label-light" = f.label :name, "Branch", class: "label-light"
= f.select(:name, @project.open_branches.map { |br| [br.name, br.name] } , {include_blank: true}, {class: "select2", data: {placeholder: "Select branch"}}) = render partial: "dropdown", locals: { f: f }
%p.help-block
= link_to "Wildcards", help_page_path(category: 'workflow', file: 'protected_branches', format: 'md', anchor: "wildcard-protected-branches")
such as
%code *-stable
or
%code production/*
are supported.
.form-group .form-group
= f.check_box :developers_can_push, class: "pull-left" = f.check_box :developers_can_push, class: "pull-left"
.prepend-left-20 .prepend-left-20
= f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0" = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0"
%p.light.append-bottom-0 %p.light.append-bottom-0
Allow developers to push to this branch Allow developers to push to this branch
= f.submit "Protect", class: "btn-create btn" = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true
%hr %hr
= render "branches_list" = render "branches_list"
- page_title @protected_branch.name, "Protected Branches"
.row.prepend-top-default.append-bottom-default
.col-lg-3
%h4.prepend-top-0
= @protected_branch.name
.col-lg-9
%h5 Matching Branches
- if @matching_branches.present?
.table-responsive
%table.table.protected-branches-list
%colgroup
%col{ width: "30%" }
%col{ width: "30%" }
%thead
%tr
%th Branch
%th Last commit
%tbody
- @matching_branches.each do |matching_branch|
= render partial: "matching_branch", object: matching_branch
- else
%p.settings-message.text-center
Couldn't find any matching branches.
...@@ -720,7 +720,7 @@ Rails.application.routes.draw do ...@@ -720,7 +720,7 @@ Rails.application.routes.draw do
resource :release, only: [:edit, :update] resource :release, only: [:edit, :update]
end end
resources :protected_branches, only: [:index, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex } resources :protected_branches, only: [:index, :show, :create, :update, :destroy], constraints: { id: Gitlab::Regex.git_reference_regex }
resources :variables, only: [:index, :show, :update, :create, :destroy] resources :variables, only: [:index, :show, :update, :create, :destroy]
resources :triggers, only: [:index, :create, :destroy] resources :triggers, only: [:index, :create, :destroy]
......
# Protected branches # Protected Branches
Permissions in GitLab are fundamentally defined around the idea of having read or write permission to the repository and branches. Permissions in GitLab are fundamentally defined around the idea of having read or write permission to the repository and branches.
...@@ -28,4 +28,28 @@ For those workflows, you can allow everyone with write access to push to a prote ...@@ -28,4 +28,28 @@ For those workflows, you can allow everyone with write access to push to a prote
On already protected branches you can also allow developers to push to the repository by selecting the `Developers can push` check box. On already protected branches you can also allow developers to push to the repository by selecting the `Developers can push` check box.
![Developers can push](protected_branches/protected_branches2.png) ![Developers can push](protected_branches/protected_branches2.png)
\ No newline at end of file
## Wildcard Protected Branches
>**Note:**
This feature was added in GitLab 8.10.
1. You can specify a wildcard protected branch, which will protect all branches matching the wildcard. For example:
| Wildcard Protected Branch | Matching Branches |
|---------------------------+--------------------------------------------------------|
| `*-stable` | `production-stable`, `staging-stable` |
| `production/*` | `production/app-server`, `production/load-balancer` |
| `*gitlab*` | `gitlab`, `gitlab/staging`, `master/gitlab/production` |
1. Protected branch settings (like "Developers Can Push") apply to all matching branches.
1. Two different wildcards can potentially match the same branch. For example, `*-stable` and `production-*` would both match a `production-stable` branch.
>**Note:**
If _any_ of these protected branches have "Developers Can Push" set to true, then `production-stable` has it set to true.
1. If you click on a protected branch's name, you will be presented with a list of all matching branches:
![protected branch matches](protected_branches/protected_branches3.png)
...@@ -15,7 +15,7 @@ module Gitlab ...@@ -15,7 +15,7 @@ module Gitlab
end end
def trigger(gl_id, oldrev, newrev, ref) def trigger(gl_id, oldrev, newrev, ref)
return true unless exists? return [true, nil] unless exists?
case name case name
when "pre-receive", "post-receive" when "pre-receive", "post-receive"
...@@ -70,13 +70,10 @@ module Gitlab ...@@ -70,13 +70,10 @@ module Gitlab
end end
def call_update_hook(gl_id, oldrev, newrev, ref) def call_update_hook(gl_id, oldrev, newrev, ref)
status = nil
Dir.chdir(repo_path) do Dir.chdir(repo_path) do
status = system({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev) stdout, stderr, status = Open3.capture3({ 'GL_ID' => gl_id }, path, ref, oldrev, newrev)
[status.success?, stderr.presence || stdout]
end end
[status, nil]
end end
def retrieve_error_message(stderr, stdout) def retrieve_error_message(stderr, stdout)
......
require 'spec_helper'
feature 'Projected Branches', feature: true, js: true do
let(:user) { create(:user, :admin) }
let(:project) { create(:project) }
before { login_as(user) }
def set_protected_branch_name(branch_name)
find(".js-protected-branch-select").click
find(".dropdown-input-field").set(branch_name)
click_on "Create Protected Branch: #{branch_name}"
end
describe "explicit protected branches" do
it "allows creating explicit protected branches" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('some-branch') }
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.name).to eq('some-branch')
end
it "displays the last commit on the matching branch if it exists" do
commit = create(:commit, project: project)
project.repository.add_branch(user, 'some-branch', commit.id)
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content(commit.id[0..7]) }
end
it "displays an error message if the named branch does not exist" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('branch was removed') }
end
end
describe "wildcard protected branches" do
it "allows creating protected branches with a wildcard" do
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('*-stable')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('*-stable') }
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.name).to eq('*-stable')
end
it "displays the number of matching branches" do
project.repository.add_branch(user, 'production-stable', 'master')
project.repository.add_branch(user, 'staging-stable', 'master')
visit namespace_project_protected_branches_path(project.namespace, project)
set_protected_branch_name('*-stable')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content("2 matching branches") }
end
it "displays all the branches matching the wildcard" do
project.repository.add_branch(user, 'production-stable', 'master')
project.repository.add_branch(user, 'staging-stable', 'master')
project.repository.add_branch(user, 'development', 'master')
create(:protected_branch, project: project, name: "*-stable")
visit namespace_project_protected_branches_path(project.namespace, project)
click_on "2 matching branches"
within(".protected-branches-list") do
expect(page).to have_content("production-stable")
expect(page).to have_content("staging-stable")
expect(page).not_to have_content("development")
end
end
end
end
require 'spec_helper'
require 'fileutils'
describe Gitlab::Git::Hook, lib: true do
describe "#trigger" do
let(:project) { create(:project) }
let(:user) { create(:user) }
def create_hook(name)
FileUtils.mkdir_p(File.join(project.repository.path, 'hooks'))
File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f|
f.write('exit 0')
end
end
def create_failing_hook(name)
FileUtils.mkdir_p(File.join(project.repository.path, 'hooks'))
File.open(File.join(project.repository.path, 'hooks', name), 'w', 0755) do |f|
f.write(<<-HOOK)
echo 'regular message from the hook'
echo 'error message from the hook' 1>&2
exit 1
HOOK
end
end
['pre-receive', 'post-receive', 'update'].each do |hook_name|
context "when triggering a #{hook_name} hook" do
context "when the hook is successful" do
it "returns success with no errors" do
create_hook(hook_name)
hook = Gitlab::Git::Hook.new(hook_name, project.repository.path)
blank = Gitlab::Git::BLANK_SHA
ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch'
status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref)
expect(status).to be true
expect(errors).to be_blank
end
end
context "when the hook is unsuccessful" do
it "returns failure with errors" do
create_failing_hook(hook_name)
hook = Gitlab::Git::Hook.new(hook_name, project.repository.path)
blank = Gitlab::Git::BLANK_SHA
ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch'
status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref)
expect(status).to be false
expect(errors).to eq("error message from the hook\n")
end
end
end
end
context "when the hook doesn't exist" do
it "returns success with no errors" do
hook = Gitlab::Git::Hook.new('unknown_hook', project.repository.path)
blank = Gitlab::Git::BLANK_SHA
ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch'
status, errors = hook.trigger(Gitlab::GlId.gl_id(user), blank, blank, ref)
expect(status).to be true
expect(errors).to be_nil
end
end
end
end
...@@ -449,6 +449,14 @@ describe Project, models: true do ...@@ -449,6 +449,14 @@ describe Project, models: true do
it { expect(project.open_branches.map(&:name)).to include('feature') } it { expect(project.open_branches.map(&:name)).to include('feature') }
it { expect(project.open_branches.map(&:name)).not_to include('master') } it { expect(project.open_branches.map(&:name)).not_to include('master') }
it "includes branches matching a protected branch wildcard" do
expect(project.open_branches.map(&:name)).to include('feature')
create(:protected_branch, name: 'feat*', project: project)
expect(Project.find(project.id).open_branches.map(&:name)).to include('feature')
end
end end
describe '#star_count' do describe '#star_count' do
...@@ -949,15 +957,67 @@ describe Project, models: true do ...@@ -949,15 +957,67 @@ describe Project, models: true do
describe '#protected_branch?' do describe '#protected_branch?' do
let(:project) { create(:empty_project) } let(:project) { create(:empty_project) }
it 'returns true when a branch is a protected branch' do it 'returns true when the branch matches a protected branch via direct match' do
project.protected_branches.create!(name: 'foo') project.protected_branches.create!(name: 'foo')
expect(project.protected_branch?('foo')).to eq(true) expect(project.protected_branch?('foo')).to eq(true)
end end
it 'returns false when a branch is not a protected branch' do it 'returns true when the branch matches a protected branch via wildcard match' do
project.protected_branches.create!(name: 'production/*')
expect(project.protected_branch?('production/some-branch')).to eq(true)
end
it 'returns false when the branch does not match a protected branch via direct match' do
expect(project.protected_branch?('foo')).to eq(false) expect(project.protected_branch?('foo')).to eq(false)
end end
it 'returns false when the branch does not match a protected branch via wildcard match' do
project.protected_branches.create!(name: 'production/*')
expect(project.protected_branch?('staging/some-branch')).to eq(false)
end
end
describe "#developers_can_push_to_protected_branch?" do
let(:project) { create(:empty_project) }
context "when the branch matches a protected branch via direct match" do
it "returns true if 'Developers can Push' is turned on" do
create(:protected_branch, name: "production", project: project, developers_can_push: true)
expect(project.developers_can_push_to_protected_branch?('production')).to be true
end
it "returns false if 'Developers can Push' is turned off" do
create(:protected_branch, name: "production", project: project, developers_can_push: false)
expect(project.developers_can_push_to_protected_branch?('production')).to be false
end
end
context "when the branch matches a protected branch via wilcard match" do
it "returns true if 'Developers can Push' is turned on" do
create(:protected_branch, name: "production/*", project: project, developers_can_push: true)
expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be true
end
it "returns false if 'Developers can Push' is turned off" do
create(:protected_branch, name: "production/*", project: project, developers_can_push: false)
expect(project.developers_can_push_to_protected_branch?('production/some-branch')).to be false
end
end
context "when the branch does not match a protected branch" do
it "returns false" do
create(:protected_branch, name: "production/*", project: project, developers_can_push: true)
expect(project.developers_can_push_to_protected_branch?('staging/some-branch')).to be false
end
end
end end
describe '#container_registry_path_with_namespace' do describe '#container_registry_path_with_namespace' do
......
require 'spec_helper' require 'spec_helper'
describe ProtectedBranch, models: true do describe ProtectedBranch, models: true do
subject { build_stubbed(:protected_branch) }
describe 'Associations' do describe 'Associations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
end end
...@@ -12,4 +14,127 @@ describe ProtectedBranch, models: true do ...@@ -12,4 +14,127 @@ describe ProtectedBranch, models: true do
it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_presence_of(:name) }
end end
describe "#matches?" do
context "when the protected branch setting is not a wildcard" do
let(:protected_branch) { build(:protected_branch, name: "production/some-branch") }
it "returns true for branch names that are an exact match" do
expect(protected_branch.matches?("production/some-branch")).to be true
end
it "returns false for branch names that are not an exact match" do
expect(protected_branch.matches?("staging/some-branch")).to be false
end
end
context "when the protected branch name contains wildcard(s)" do
context "when there is a single '*'" do
let(:protected_branch) { build(:protected_branch, name: "production/*") }
it "returns true for branch names matching the wildcard" do
expect(protected_branch.matches?("production/some-branch")).to be true
expect(protected_branch.matches?("production/")).to be true
end
it "returns false for branch names not matching the wildcard" do
expect(protected_branch.matches?("staging/some-branch")).to be false
expect(protected_branch.matches?("production")).to be false
end
end
context "when the wildcard contains regex symbols other than a '*'" do
let(:protected_branch) { build(:protected_branch, name: "pro.duc.tion/*") }
it "returns true for branch names matching the wildcard" do
expect(protected_branch.matches?("pro.duc.tion/some-branch")).to be true
end
it "returns false for branch names not matching the wildcard" do
expect(protected_branch.matches?("production/some-branch")).to be false
expect(protected_branch.matches?("proXducYtion/some-branch")).to be false
end
end
context "when there are '*'s at either end" do
let(:protected_branch) { build(:protected_branch, name: "*/production/*") }
it "returns true for branch names matching the wildcard" do
expect(protected_branch.matches?("gitlab/production/some-branch")).to be true
expect(protected_branch.matches?("/production/some-branch")).to be true
expect(protected_branch.matches?("gitlab/production/")).to be true
expect(protected_branch.matches?("/production/")).to be true
end
it "returns false for branch names not matching the wildcard" do
expect(protected_branch.matches?("gitlabproductionsome-branch")).to be false
expect(protected_branch.matches?("production/some-branch")).to be false
expect(protected_branch.matches?("gitlab/production")).to be false
expect(protected_branch.matches?("production")).to be false
end
end
context "when there are arbitrarily placed '*'s" do
let(:protected_branch) { build(:protected_branch, name: "pro*duction/*/gitlab/*") }
it "returns true for branch names matching the wildcard" do
expect(protected_branch.matches?("production/some-branch/gitlab/second-branch")).to be true
expect(protected_branch.matches?("proXYZduction/some-branch/gitlab/second-branch")).to be true
expect(protected_branch.matches?("proXYZduction/gitlab/gitlab/gitlab")).to be true
expect(protected_branch.matches?("proXYZduction//gitlab/")).to be true
expect(protected_branch.matches?("proXYZduction/some-branch/gitlab/")).to be true
expect(protected_branch.matches?("proXYZduction//gitlab/some-branch")).to be true
end
it "returns false for branch names not matching the wildcard" do
expect(protected_branch.matches?("production/some-branch/not-gitlab/second-branch")).to be false
expect(protected_branch.matches?("prodXYZuction/some-branch/gitlab/second-branch")).to be false
expect(protected_branch.matches?("proXYZduction/gitlab/some-branch/gitlab")).to be false
expect(protected_branch.matches?("proXYZduction/gitlab//")).to be false
expect(protected_branch.matches?("proXYZduction/gitlab/")).to be false
expect(protected_branch.matches?("proXYZduction//some-branch/gitlab")).to be false
end
end
end
end
describe "#matching" do
context "for direct matches" do
it "returns a list of protected branches matching the given branch name" do
production = create(:protected_branch, name: "production")
staging = create(:protected_branch, name: "staging")
expect(ProtectedBranch.matching("production")).to include(production)
expect(ProtectedBranch.matching("production")).not_to include(staging)
end
it "accepts a list of protected branches to search from, so as to avoid a DB call" do
production = build(:protected_branch, name: "production")
staging = build(:protected_branch, name: "staging")
expect(ProtectedBranch.matching("production")).to be_empty
expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).to include(production)
expect(ProtectedBranch.matching("production", protected_branches: [production, staging])).not_to include(staging)
end
end
context "for wildcard matches" do
it "returns a list of protected branches matching the given branch name" do
production = create(:protected_branch, name: "production/*")
staging = create(:protected_branch, name: "staging/*")
expect(ProtectedBranch.matching("production/some-branch")).to include(production)
expect(ProtectedBranch.matching("production/some-branch")).not_to include(staging)
end
it "accepts a list of protected branches to search from, so as to avoid a DB call" do
production = build(:protected_branch, name: "production/*")
staging = build(:protected_branch, name: "staging/*")
expect(ProtectedBranch.matching("production/some-branch")).to be_empty
expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).to include(production)
expect(ProtectedBranch.matching("production/some-branch", protected_branches: [production, staging])).not_to include(staging)
end
end
end
end end
...@@ -63,7 +63,7 @@ module TestEnv ...@@ -63,7 +63,7 @@ module TestEnv
end end
def disable_pre_receive def disable_pre_receive
allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return(true) allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
end end
# Clean /tmp/tests # Clean /tmp/tests
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment