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
### Using the `GLDropdown` component
## Enforcing wildcard protected branches
### From the Web UI
### Over SSH
### Over HTTPS
## Listing matching branches
See merge request !4665
Added blank state to issues
## What does this MR do?
Adds new blank state to issues when no issues exist.
Part of #18519
## Screenshots (if relevant)
See merge request !4908
New diff notes
Fixes#12732, #14731, #19375, #14783
Builds on https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4110
To do:
- [x] Get it mostly working
- [x] Validate position validity
- [x] Fix: Don’t link to `#`
- [x] Fix: Base ref can be `nil`, potentially, when the MR has an oprhan source branch => Yep, doesn’t work. We need to store a `start_id`
- [x] Optimize: Fewer duplicate `git diff` compares
- [x] Optimize: Pass paths to `PositionTracer#diff` for faster diffs
- [x] Refactor: Use `head_id` in `MergeRequest`/`MergeRequestDiff` instead of `source_sha`
- [x] Refactor: Convert existing array-based diff refs to the DiffRefs model
- [x] Tweak: Use `note_type` in `Autosave` key
- [x] Tweak: Remove `line_code: note.line_code` from `link_to_reply_discussion`
- [x] Update: `SentNotifications` and reply-by-email receiver
- [x] Update: MR diff notification email
- [x] Update: API (MR, Commit note creation and entity)
- [x] Update: GitHub importer
- [x] Address any other TODO comments
- [x] Fix: Suppress "edited 4 minutes ago"
- [x] Write tests
- [x] `LineMapper`
- [x] `PositionTracer`
- [x] `Position`
- [x] `DiffPositionUpdateService`
- [x] `DiffNote`
- [x] `MergeRequests::RefreshService` / `MergeRequest#update_diff_notes_positions`
- [x] Make sure commits with diff notes don't get cleaned up, since this would prevent the diff notes from being rendered (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5062)
Future improvements:
- Display unresolved comments on files outside the diff, if the comment was added when that file _was_ part of the diff
- Allow commenting on sections between hunks, when expanding the diff using `...`
- (We'd need to generate line code based on Position if we have it, even if it falls outside bounds of diff)
- `diff_hunk` on diff note API entity
- Show diff hunk in notification email
- Resolved line notes would have a boolean, and be inactive through `notes.any? { !active? || resolved? }`
- Multi line notes would store a number of positions, and do the right thing (™) in grouping and then rendering if the first item is multiline? => true
- Image diff notes could store x,y,width,height instead of old_line,new_line for similar grouping. Does it need a reference to say if it's on old or new? These can't have line_codes, clearly. Rendering would be interesting.
- Show commit line comments in the MR diff
- Comment on specific selected words
- Comment on file header
- Unfold top of discussion diff note
- New diff notes API for commits and MRs
/cc @rspeicher
See merge request !4101
Exclude projects pending delete from notifications
Make `NotificationSetting.for_projects` exclude projects that are excluded by the default scope on `Project`. (At the moment, that's projects with `pending_delete: true`.)
See https://gitlab.com/gitlab-com/support-forum/issues/819
See merge request !5138
If the Sidekiq job fails for some reason, a project can be 'stuck'
pending deletion. The project can't be viewed, so it shouldn't be
available through the notification settings association as this will
throw an exception when we try to show the link.
1. The `open_branches` method is used to provide a list of branches
while creating a protected branch.
2. It makes sense to include branches which are matched by one or more
wildcard protected branches, since the user might want to make exact
protected branches from these as well.
3. This also provides a large performance improvement. On my machine, in
a project with 5000 branches and 2000 protected branches, the
`ProtectedBranches#index` page went from a 40 seconds load time to 4
seconds (10x speedup).
1. Modify the component to support a callback for every key press in the
filter. We need this so we can update the "Create: <branch_name"
label.
2. Modify the component to use `$(<selector>).first().click()` instead
of `$(selector)[0].click()`, because the latter is non-standard, and
doesn't work in PhantomJS.
Get rid of pluralize on stage names
## What does this MR do?
Removes `pluralize` from stage names
## What are the relevant issue numbers?
Closes#18283
Part of https://gitlab.com/gitlab-org/gitlab-ce/issues/18920
## Screenshots (if relevant)
cc @ayufan
See merge request !5117
Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests
Closes#17766
## What does this MR do?
GitHub importer fails when a repository has two Pull Requests, with the same branch name and different SHA, one of which was closed without merging, and one which is open or was merged in. This happens because we only checks if the branch exists in the [Gitlab::GithubImport::BranchFormatter#exists?](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/github_import/branch_formatter.rb#L6-8), before creating the missing references on Github. With this MR we check if both branch, and SHA exists in the current project. If no, we create the missing reference on GitHub before import the PR.
## Are there points in the code the reviewer needs to double check?
Nope.
## Why was this MR needed?
Github importer will fail if branch was reused across Pull Requests.
## What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ce/issues/19439
/cc @royaldark @thomasjachmann
See merge request !5103
Fix diff comments not showing up in activity feed
## What does this MR do?
It fixes the detection of note events to check for `Note` and `LegacyDiffNote`.
## Are there points in the code the reviewer needs to double check?
No? /cc @DouweM (since I believe you introduced `LegacyDiffNote`
## Why was this MR needed?
To fix#19092.
## What are the relevant issue numbers?
Fixes#19092.
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !5069
Fixing URL validation for import_url on projects
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/17536
This MR fixes problems related to bypassing `import_url` validation on projects. This makes sure the URL is properly validated so we don't enter crap and fail while running workers that handle this URL.
It also adds a migration to fix current invalid `import_url`s
See merge request !4753
Don't show "request access" button to project owners
This MR fixes an issue where project owners that are not in the project's members list (I believe this is how we handled project owners before, now we seem to create a "Master" member for the project creator) would see the `Request Access` button.
This MR fixes this issue in a clean way by adding a new `:request_access` ability to replace an ugly helper.
It also give project owners the ability to update & destroy a requester that would happen to be themselves (since owners could request access to their own project before this MR).
Related to #18790.
See merge request !5091
- Group / project members cannot request access
- Group members cannot request access to a group's project
This addresses an issue where project owners could request access
to their own project, leading to UI inconsistency where their requester
status would replace their owner status.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Move global ci entries handling from legacy to new config
## What does this MR do?
This MR moves responsibility of handling global CI config entries (like `image`, `services`), from legacy `GitlabCiYamlProcessor` to new CI Config
## Why was this MR needed?
This is the next iteration of CI configuration refactoring
## What are the relevant issue numbers?
#15060
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
See merge request !4820
Robert Speicher
Douwe Maan
Jacob Schatz
Robert Speicher
Douwe Maan
Sean McGivern
Alfredo Sumaran
James Lopez
Timothy Andrew
Rémy Coutable
Annabel Dunstone
Douglas Barbosa Alexandre
Patricio Cano
Dmitriy Zaporozhets
Paco Guzman
Phil Hughes