Commit Graph

4906 Commits

Author SHA1 Message Date
Grzegorz Bizon c6f9a1c273 Enable Style/IdenticalConditionalBranches Rubocop cop 2016-07-08 11:04:04 +02:00
Rémy Coutable 2c650b6f30 Merge branch 'feature/option-set-new-users-external' into 'master'
Added setting to set new users by default as external

## What does this MR do?
This implements the feature request #14508. It adds an option in the application settings to set new users by default as external.

## Are there points in the code the reviewer needs to double check?
Everything. Like I mentioned in the discussion of the issue my knowledge of Ruby basically doesn't exists. I tested it on my machine and it seems to work, but as I am very unexperienced in Ruby I highly recommend to  take a close look at the code.

## Why was this MR needed?
It was requested by @DouweM to work on the issue with the proposed changes by me.

## What are the relevant issue numbers?
This MR is for the issue #14508 that followed up after the implementation of #4009.

See merge request !4545
2016-07-08 08:29:18 +00:00
Douwe Maan bf89e06a45 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](2f753e3ed2/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](43f9ce0e88/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](f7beedf122/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](20f3cfe8d5/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
2016-07-07 22:37:30 +00:00
Douwe Maan 86d238e4bd Merge branch 'new-diff-notes' into 'master'
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
2016-07-07 20:45:03 +00:00
Dravere a0a9494e4e Added setting to set new users by default as external
As requested by the issue #14508 this adds an option in the application
settings to set newly registered users by default as external. The
default setting is set to false to stay backward compatible.
2016-07-07 19:21:18 +02:00
Rémy Coutable 3c89a788c7 Merge branch 'fix/import-export-log' into 'master'
Fix log statements in import/export

Fixes - as seen in the logs:
```
Import/Export error raised on /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/import_export/file_importer.rb:19:in `rescue in import': undefined method `message' for #<String:0x007fc45e977e10>
```

See merge request !5129
2016-07-07 08:56:05 +00:00
James Lopez 91a183bc57 fix log statements in import/export 2016-07-07 09:49:46 +02:00
Douwe Maan ac26b23712 Remove duplication, unused methods, and some other style things 2016-07-06 19:29:41 -04:00
Douwe Maan 228d2a4cb1 Add some more code comments. 2016-07-06 19:28:13 -04:00
Douwe Maan ddec2ed0df Add send_git_patch helper 2016-07-06 18:51:01 -04:00
Douwe Maan 29d574868a Display new diff notes and allow creation through the web interface 2016-07-06 18:51:00 -04:00
Douwe Maan 521a0a20f7 Allow reply-by-email with diff notes 2016-07-06 18:51:00 -04:00
Douwe Maan db65954d78 Add Gitlab::Git::PositionTracer 2016-07-06 18:50:59 -04:00
Douwe Maan e9e06ca627 Add Gitlab::Diff::LineMapper 2016-07-06 18:50:59 -04:00
Douwe Maan 9abcc0a98f Add Gitlab::Git::Position 2016-07-06 18:50:59 -04:00
Douwe Maan a27462a5c6 Extract parts of LegacyDiffNote into DiffOnNote concern and move part of responsibility to other classes 2016-07-06 18:50:59 -04:00
Douwe Maan 375193455a Style diff and blob file headers the same way 2016-07-06 18:50:59 -04:00
Douwe Maan 9fc0e11e0d Add DiffFile#blob and #old_blob 2016-07-06 18:50:58 -04:00
Douwe Maan 17ab745e40 Add Timeless helper module to prevent updated_at from being updated 2016-07-06 18:50:58 -04:00
Douwe Maan a9fa45f09e Represent DiffRefs as proper class instead of tuple array 2016-07-06 18:50:58 -04:00
Douwe Maan 6ce25e7b4c Rename MergeRequest methods that return commits or shas to be more clear and consistent 2016-07-06 18:50:58 -04:00
Douglas Barbosa Alexandre 5f86a084f2 Consider that a GH exists only if both `ref`, and `sha` exist 2016-07-06 13:08:12 -03:00
Douglas Barbosa Alexandre 4e5de3e6ee Doesn't trigger Git hooks when cleaning up restored branches from GitHub 2016-07-06 13:08:12 -03:00
Robert Speicher 3df58f1499 Merge branch 'gh-labels' into 'master'
Allow `?`, or `&` for label names

Closes #18727 

See merge request !4724
2016-07-06 15:11:20 +00:00
Robert Speicher be018ba8c4 Merge branch 'fix/import-url-validator' into 'master'
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
2016-07-06 15:06:01 +00:00
Rémy Coutable b73da89591 Merge branch 'fix/project-export-error' into 'master'
Add more debug info to import/export and memory killer

This should help debug https://gitlab.com/gitlab-org/gitlab-ce/issues/19124 further

See merge request !5108
2016-07-06 13:09:02 +00:00
James Lopez 09452715d3 fix typo 2016-07-06 11:09:07 +02:00
James Lopez 95f630daeb even more debug 2016-07-06 10:29:31 +02:00
Douglas Barbosa Alexandre 5d11cf2e98 Use CGI.unescapeHTML rather than doing the gsub with a map 2016-07-05 20:57:09 -03:00
Douglas Barbosa Alexandre d6b60e83ed Move `unescape_html_entities` from LabelsHelper to Label model 2016-07-05 20:57:09 -03:00
Douglas Barbosa Alexandre ab811b6ab9 Render references for labels that name contains ?, or & 2016-07-05 20:57:09 -03:00
Patricio Cano be221a30ac Revert back to not defining a default Git access protocol. 2016-07-05 16:54:22 -05:00
Patricio Cano 29c50c5315 Default Git access protocol to `web` 2016-07-05 16:54:22 -05:00
Patricio Cano fbaabb3911 Rename `enabled_git_access_protocols` to singular. 2016-07-05 16:54:22 -05:00
Patricio Cano d1151f7621 Don't allow empty strings in the `protocol` check. 2016-07-05 16:54:22 -05:00
Patricio Cano 9397ce9137 Correct access control flow for Git HTTP requests. 2016-07-05 16:54:22 -05:00
Patricio Cano da15471bb1 Clarify protocol access check, and make Git HTTP access call more specific. 2016-07-05 16:54:22 -05:00
Patricio Cano ace309d775 Raise an error if no protocol is passed to the GitAccess check. 2016-07-05 16:54:22 -05:00
Patricio Cano 8b14d1d2c2 Rename ENV['PROTOCOL'] to ENV['GL_PROTOCOL'] to conform to what GitLab Shell expects and make the `protocol` param in `GitAccess` mandatory. 2016-07-05 16:54:22 -05:00
Patricio Cano 7735ef86f0 Only allow Git Access on the allowed protocol 2016-07-05 16:54:22 -05:00
Robert Speicher aefb8a1741 Merge branch 'update-gemoji' into 'master'
Add lib/gitlab/emoji.rb instead of gitlab_emoji gem and upgrade Gemojione

- No reason to split it into a separate gem when the gem barely did anything. We can use gemojione directly, making updating gemojione that much easier. Unless there's a particularly good reason we were using the gem?
- Fixes the Rake task since it broke after all the AwardEmoji changes.
- Update gemojione to 2.6.1.

Spring Update changes! http://emojione.com/releases/#2.2.0

See merge request !4919
2016-07-05 17:08:35 +00:00
Paco Guzman 330de255b7 RailsCache metrics now includes fetch_hit/fetch_miss and read_hit/read_miss info. 2016-07-05 12:28:06 +02:00
Paco Guzman e9a4d117f2 Instrument cache fetch hit and cache fetch misses 2016-07-05 12:28:06 +02:00
Rémy Coutable 06c7d6f3a8 Merge branch 'refactor/ci-config-move-global-entries' into 'master'
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
2016-07-05 08:37:16 +00:00
Timothy Andrew eb16e1e3c2 Improve the error message displayed when branch creation fails.
Note: This feature was developed independently on master while this was
in review. I've removed the conflicting bits and left the relevant
additions, mainly a test for `Gitlab::Git::Hook`. The original commit
message follows:

1. `gitlab-shell` outputs errors to `stderr`, but we weren't using this
   information, prior to this commit. Now we capture the `stderr`, and
   display it in the flash message when branch creation fails.

2. This can be used to display better errors for other git operation
   failures with small tweaks.

3. The return value of `Gitlab::Git::Hook#trigger` is changed from a
   simple `true`/`false` to a tuple of `[status, errors]`. All usages
   and tests have been updated to reflect this change.

4. This is only relevant to branch creation _from the Web UI_, since SSH
   and HTTP pushes access `gitlab-shell` either directly or through
   `gitlab-workhorse`.

5. A few minor changes need to be made on the `gitlab-shell` end. Right
   now, the `stderr` message it outputs is prefixed by "GitLab: ", which
   shows up in our flash message. This is better removed.
2016-07-05 11:05:16 +05:30
Dmitriy Zaporozhets fd2b2dc1de Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq 2016-07-04 21:52:43 +03:00
Dmitriy Zaporozhets b734167b7a Merge branch 'more_descriptive_git_shell_messages' into 'master'
Handle custom Git hook result in GitLab UI

## What does this MR do?

This MR fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19190
It allows to show messages from git custom hooks

## Screenshots (if relevant)

![joxi_screenshot_1467651998090](/uploads/7f814523c96712c07b4f4b52c843aab3/joxi_screenshot_1467651998090.png)

See merge request !5073
2016-07-04 17:47:30 +00:00
Valery Sizov 926a8ab476 Handle custom Git hook result in GitLab UI 2016-07-04 20:16:55 +03:00
Grzegorz Bizon bfad4c61f1 Add minor improvements in readability in CI config 2016-07-04 18:36:06 +02:00
Rémy Coutable 1a36493d13 Merge branch 'save-artifacts_sizes' into 'master'
Save artifacts sizes

## What does this MR do?

Introduce ci_builds.artifacts_size as an integer, so that it's easier to access than reading from the file again.

## What are the relevant issue numbers?

Closes #18869

See merge request !4964
2016-07-04 15:36:02 +00:00