Merge remote-tracking branch 'ee/master' into ce-to-ee-2017-08-18

* ee/master:
  Removes Gitlab::Metrics usage from StuckImportJobsWorker
  Fixes Audit Events Documentation issue link
  Add more sections in GPG docs
  Refactor GPG signing docs
  Change GPG docs location
  Fix store credentials
  Refactor `push_rule_check` GitAccess specs
  Fix invalid GitAccess specs for License and secondary Geo node
  Make issuable filter dropdown style consistent
  Refactor `run_group_permission_checks` helper in GitAccess spec
  Reduce duplication in GitAccess spec around error messages
  Greatly reduce test duration for git_access_spec
  Hijack AR::Base.connection in the DB load balancer
  Added time fields for csv export
  Use gitlab.yml.example instead of settings logic stubbing
  Fix url for object store artifacts
  fix Modify commit message button spacing

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.js

@@ -283,15 +283,14 @@ export default {
               <span v-if="mr.ffOnlyEnabled">
                 Fast-forward merge without a merge commit
               </span>
-              <span v-else>
               <button
+                v-else
                 @click="toggleCommitMessageEditor"
                 :disabled="isMergeButtonDisabled"
                 class="btn btn-default btn-xs"
                 type="button">
                 Modify commit message
               </button>
-              </span>
             </template>
             <template v-else>
               <span class="bold">

app/assets/stylesheets/framework/dropdowns.scss

@@ -728,18 +728,27 @@
 @mixin new-style-dropdown($selector: '') {
   #{$selector}.dropdown-menu,
   #{$selector}.dropdown-menu-nav {
-    .divider {
-      margin: 6px 0;
-    }
-
     li {
       padding: 0 1px;
 
+      &:hover {
+        background-color: transparent;
+      }
+
+      &.divider {
+        margin: 6px 0;
+
+        &:hover {
+          background-color: $dropdown-divider-color;
+        }
+      }
+
       &.dropdown-header {
         padding: 8px 16px;
       }
 
-      a {
+      a,
+      button {
         border-radius: 0;
         padding: 8px 16px;
 
@@ -753,6 +762,7 @@
         &:active,
         &:focus {
           background-color: $gray-darker;
+          color: $gl-text-color;
         }
 
         &.is-active {

app/assets/stylesheets/framework/filters.scss

@@ -50,6 +50,8 @@
 }
 
 .filtered-search-wrapper {
+  @include new-style-dropdown;
+
   display: -webkit-flex;
   display: flex;
 
@@ -411,8 +413,6 @@
 }
 
 %filter-dropdown-item-btn-hover {
-  background-color: $dropdown-hover-color;
-  color: $white-light;
   text-decoration: none;
   outline: 0;
 
@@ -422,8 +422,6 @@
 }
 
 .droplab-dropdown .dropdown-menu .filter-dropdown-item {
-  padding: 0;
-
   .btn {
     border: none;
     width: 100%;

app/services/issues/export_csv_service.rb

@@ -21,7 +21,7 @@ module Issues
 
     def csv_builder
       @csv_builder ||=
-        CsvBuilder.new(@issues.includes(:author, :assignees), header_to_value_hash)
+        CsvBuilder.new(@issues.includes(:author, :assignees, :timelogs), header_to_value_hash)
     end
 
     private
@@ -43,7 +43,9 @@ module Issues
        'Updated At (UTC)' => -> (issue) { issue.updated_at&.to_s(:csv) },
        'Closed At (UTC)' => -> (issue) { issue.closed_at&.to_s(:csv) },
        'Milestone' => -> (issue) { issue.milestone&.title },
-       'Labels' => -> (issue) { @labels[issue.id].sort.join(',').presence }
+       'Labels' => -> (issue) { @labels[issue.id].sort.join(',').presence },
+       'Time Estimate' => ->(issue) { issue.time_estimate.to_s(:csv) },
+       'Time Spent' => -> (issue) { issue.timelogs.map(&:time_spent).inject(0, :+)}
       }
     end
   end

app/uploaders/object_store_uploader.rb

@@ -20,6 +20,14 @@ class ObjectStoreUploader < CarrierWave::Uploader::Base
     def object_store_enabled?
       object_store_options&.enabled
     end
+
+    def object_store_credentials
+      @object_store_credentials ||= object_store_options&.connection&.to_hash&.deep_symbolize_keys
+    end
+
+    def object_store_directory
+      object_store_options&.remote_directory
+    end
   end
 
   attr_reader :subject, :field
@@ -98,11 +106,11 @@ class ObjectStoreUploader < CarrierWave::Uploader::Base
   end
 
   def fog_directory
-    self.class.object_store_options.remote_directory
+    self.class.object_store_directory
   end
 
   def fog_credentials
-    self.class.object_store_options.connection
+    self.class.object_store_credentials
   end
 
   def fog_public

app/views/profiles/gpg_keys/index.html.haml

@@ -12,7 +12,7 @@
       Add a GPG key
     %p.profile-settings-content
       Before you can add a GPG key you need to
-      = link_to 'generate it.', help_page_path('workflow/gpg_signed_commits/index.md')
+      = link_to 'generate it.', help_page_path('user/project/gpg_signed_commits/index.md')
     = render 'form'
     %hr
     %h5

app/views/projects/commit/_signature_badge.html.haml

@@ -12,7 +12,7 @@
   %span.monospace= signature.gpg_key_primary_keyid
 
 
-  = link_to('Learn more about signing commits', help_page_path('workflow/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link')
+  = link_to('Learn more about signing commits', help_page_path('user/project/gpg_signed_commits/index.md'), class: 'gpg-popover-help-link')
 
 %button{ class: css_classes, data: { toggle: 'popover', html: 'true', placement: 'auto top', title: title, content: content } }
   = label

app/views/shared/issuable/_search_bar.html.haml

@@ -61,7 +61,7 @@
                 %li.filter-dropdown-item{ data: { value: 'none' } }
                   %button.btn.btn-link
                     No Assignee
-                %li.divider
+                %li.divider.droplab-item-ignore
                 - if current_user
                   = render 'shared/issuable/user_dropdown_item',
                     user: current_user
@@ -80,7 +80,7 @@
                 %li.filter-dropdown-item{ 'data-value' => 'started' }
                   %button.btn.btn-link
                     Started
-                %li.divider
+                %li.divider.droplab-item-ignore
               %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } }
                 %li.filter-dropdown-item
                   %button.btn.btn-link.js-data-value
@@ -90,7 +90,7 @@
                 %li.filter-dropdown-item{ data: { value: 'none' } }
                   %button.btn.btn-link
                     No Label
-                %li.divider
+                %li.divider.droplab-item-ignore
               %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } }
                 %li.filter-dropdown-item
                   %button.btn.btn-link
@@ -107,7 +107,7 @@
                   %li.filter-dropdown-item{ 'data-value' => 'any' }
                     %button.btn.btn-link
                       Any Weight
-                  %li.divider
+                  %li.divider.droplab-item-ignore
                 %ul.filter-dropdown{ 'data-dropdown' => true }
                   - Issue.weight_filter_options.each do |weight|
                     %li.filter-dropdown-item{ 'data-value' => "#{weight}" }

app/workers/stuck_import_jobs_worker.rb

@@ -5,12 +5,8 @@ class StuckImportJobsWorker
   IMPORT_JOBS_EXPIRATION = 15.hours.to_i
 
   def perform
-    projects_without_jid_count = mark_projects_without_jid_as_failed!
-    projects_with_jid_count = mark_projects_with_jid_as_failed!
-
-    Gitlab::Metrics.add_event(:stuck_import_jobs,
-                             projects_without_jid_count: projects_without_jid_count,
-                             projects_with_jid_count: projects_with_jid_count)
+    mark_projects_without_jid_as_failed!
+    mark_projects_with_jid_as_failed!
   end
 
   private

changelogs/unreleased-ee/1406-new-export-fields.yml

+---
+title: Add Time estimate and Time spend fields in csv export
+merge_request: 2627
+author: g3dinua, LockiStrike
+type: changed

changelogs/unreleased-ee/db-load-balancing-hijack-connection.yml

+---
+title: >
+  Ensure all database queries are routed through the database load balancer when
+  load balancing is enabled
+merge_request: 2707
+author:
+type: changed

changelogs/unreleased-ee/fix-public-url-for-object-store.yml

+---
+title: Fix url for object store artifacts
+merge_request:
+author:
+type: fixed

config/gitlab.yml.example

@@ -729,6 +729,18 @@ test:
     enabled: true
   lfs:
     enabled: false
+  artifacts:
+    enabled: true
+    # The location where build artifacts are stored (default: shared/artifacts).
+    # path: shared/artifacts
+    object_store:
+      enabled: false
+      remote_directory: artifacts # The bucket name
+      connection:
+        provider: AWS # Only AWS supported at the moment
+        aws_access_key_id: AWS_ACCESS_KEY_ID
+        aws_secret_access_key: AWS_SECRET_ACCESS_KEY
+        region: eu-central-1
   gitlab:
     host: localhost
     port: 80

config/initializers/1_settings.rb

@@ -336,8 +336,8 @@ Settings.artifacts['max_size']   ||= 100 # in megabytes
 Settings.artifacts['object_store'] ||= Settingslogic.new({})
 Settings.artifacts['object_store']['enabled'] = false if Settings.artifacts['object_store']['enabled'].nil?
 Settings.artifacts['object_store']['remote_directory'] ||= nil
-# Convert upload connection settings to use symbol keys, to make Fog happy
-Settings.artifacts['object_store']['connection']&.deep_symbolize_keys!
+# Convert upload connection settings to use string keys, to make Fog happy
+Settings.artifacts['object_store']['connection']&.deep_stringify_keys!
 
 #
 # Registry

doc/README.md

@@ -111,7 +111,7 @@ Manage your [repositories](user/project/repository/index.md) from the UI (user i
 - [Git](topics/git/index.md): Getting started with Git, branching strategies, Git LFS, advanced use.
 - [Git cheatsheet](https://gitlab.com/gitlab-com/marketing/raw/master/design/print/git-cheatsheet/print-pdf/git-cheatsheet.pdf): Download a PDF describing the most used Git operations.
 - [GitLab Flow](workflow/gitlab_flow.md): explore the best of Git with the GitLab Flow strategy.
-- [Signing commits](workflow/gpg_signed_commits/index.md): use GPG to sign your commits.
+- [Signing commits](user/project/gpg_signed_commits/index.md): use GPG to sign your commits.
 
 ### Migrate and import your projects from other platforms
 

doc/administration/audit_events.md

@@ -59,4 +59,4 @@ You can further filter by specific group, project or user (for authentication ev
 
 ![audit log](audit_log.png)
 
-[ce-23361]: https://gitlab.com/gitlab-org/gitlab-ee/issues/2336
+[ee-2336]: https://gitlab.com/gitlab-org/gitlab-ee/issues/2336

doc/user/project/gpg_signed_commits/index.md

+# Signing commits with GPG
+
+> [Introduced][ce-9546] in GitLab 9.5.
+
+GitLab can show whether a commit is verified or not when signed with a GPG key.
+All you need to do is upload the public GPG key in your profile settings.
+
+GPG verified tags are not supported yet.
+
+## Getting started with GPG
+
+Here are a few guides to get you started with GPG:
+
+- [Git Tools - Signing Your Work](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work)
+- [Managing OpenPGP Keys](https://riseup.net/en/security/message-security/openpgp/gpg-keys)
+- [OpenPGP Best Practices](https://riseup.net/en/security/message-security/openpgp/best-practices)
+- [Creating a new GPG key with subkeys](https://www.void.gr/kargig/blog/2013/12/02/creating-a-new-gpg-key-with-subkeys/) (advanced)
+
+## How GitLab handles GPG
+
+GitLab uses its own keyring to verify the GPG signature. It does not access any
+public key server.
+
+In order to have a commit verified on GitLab the corresponding public key needs
+to be uploaded to GitLab. For a signature to be verified two prerequisites need
+to be met:
+
+1. The public key needs to be added your GitLab account
+1. One of the emails in the GPG key matches your **primary** email
+
+## Generating a GPG key
+
+If you don't already have a GPG key, the following steps will help you get
+started:
+
+1. [Install GPG](https://www.gnupg.org/download/index.html) for your operating system
+1. Generate the private/public key pair with the following command:
+
+    ```sh
+    gpg --full-gen-key
+    ```
+
+    This will spawn a series of questions.
+
+1. The first question is which algorithm can be used.  Select the kind you want
+   or press <kbd>Enter</kbd> to choose the default (RSA and RSA):
+
+    ```
+    Please select what kind of key you want:
+       (1) RSA and RSA (default)
+       (2) DSA and Elgamal
+       (3) DSA (sign only)
+       (4) RSA (sign only)
+    Your selection? 1
+    ```
+
+1. The next question is key length. We recommend to choose the highest value
+   which is `4096`:
+
+    ```
+    RSA keys may be between 1024 and 4096 bits long.
+    What keysize do you want? (2048) 4096
+    Requested keysize is 4096 bits
+    ```
+1. Next, you need to specify the validity period of your key. This is something
+   subjective, and you can use the default value which is to never expire:
+
+    ```
+    Please specify how long the key should be valid.
+             0 = key does not expire
+          <n>  = key expires in n days
+          <n>w = key expires in n weeks
+          <n>m = key expires in n months
+          <n>y = key expires in n years
+    Key is valid for? (0) 0
+    Key does not expire at all
+    ```
+
+1. Confirm that the answers you gave were correct by typing `y`:
+
+    ```
+    Is this correct? (y/N) y
+    ```
+
+1. Enter you real name, the email address to be associated with this key (should
+   match the primary email address you use in GitLab) and an optional comment
+   (press <kbd>Enter</kbd> to skip):
+
+    ```
+    GnuPG needs to construct a user ID to identify your key.
+
+    Real name: Mr. Robot
+    Email address: mr@robot.sh
+    Comment:
+    You selected this USER-ID:
+        "Mr. Robot <mr@robot.sh>"
+
+    Change (N)ame, (C)omment, (E)mail or (O)kay/(Q)uit? O
+    ```
+
+1. Pick a strong password when asked and type it twice to confirm.
+1. Use the following command to list the private GPG key you just created:
+
+    ```
+    gpg --list-secret-keys mr@robot.sh
+    ```
+
+    Replace `mr@robot.sh` with the email address you entered above.
+
+1. Copy the GPG key ID that starts with `sec`. In the following example, that's
+   `0x30F2B65B9246B6CA`:
+
+    ```
+    sec   rsa4096/0x30F2B65B9246B6CA 2017-08-18 [SC]
+          D5E4F29F3275DC0CDA8FFC8730F2B65B9246B6CA
+    uid                   [ultimate] Mr. Robot <mr@robot.sh>
+    ssb   rsa4096/0xB7ABC0813E4028C0 2017-08-18 [E]
+    ```
+
+1. Export the public key of that ID (replace your key ID from the previous step):
+
+    ```
+    gpg --armor --export 0x30F2B65B9246B6CA
+    ```
+
+1. Finally, copy the public key and [add it in your profile settings](#adding-a-gpg-key-to-your-account)
+
+## Adding a GPG key to your account
+
+>**Note:**
+Once you add a key, you cannot edit it, only remove it. In case the paste
+didn't work, you'll have to remove the offending key and re-add it.
+
+You can add a GPG key in your profile's settings:
+
+1. On the upper right corner, click on your avatar and go to your **Settings**.
+
+    ![Settings dropdown](../../profile/img/profile_settings_dropdown.png)
+
+1. Navigate to the **GPG keys** tab and paste your _public_ key in the 'Key'
+   box.
+
+    ![Paste GPG public key](img/profile_settings_gpg_keys_paste_pub.png)
+
+1. Finally, click on **Add key** to add it to GitLab. You will be able to see
+   its fingerprint, the corresponding email address and creation date.
+
+    ![GPG key single page](img/profile_settings_gpg_keys_single_key.png)
+
+## Associating your GPG key with Git
+
+After you have [created your GPG key](#generating-a-gpg-key) and [added it to
+your account](#adding-a-gpg-key-to-your-account), it's time to tell Git which
+key to use.
+
+1. Use the following command to list the private GPG key you just created:
+
+    ```
+    gpg --list-secret-keys mr@robot.sh
+    ```
+
+    Replace `mr@robot.sh` with the email address you entered above.
+
+1. Copy the GPG key ID that starts with `sec`. In the following example, that's
+   `0x30F2B65B9246B6CA`:
+
+    ```
+    sec   rsa4096/0x30F2B65B9246B6CA 2017-08-18 [SC]
+          D5E4F29F3275DC0CDA8FFC8730F2B65B9246B6CA
+    uid                   [ultimate] Mr. Robot <mr@robot.sh>
+    ssb   rsa4096/0xB7ABC0813E4028C0 2017-08-18 [E]
+    ```
+
+1. Tell Git to use that key to sign the commits:
+
+    ```
+    git config --global user.signingkey 0x30F2B65B9246B6CA
+    ```
+
+    Replace `0x30F2B65B9246B6CA` with your GPG key ID.
+
+## Signing commits
+
+After you have [created your GPG key](#generating-a-gpg-key) and [added it to
+your account](#adding-a-gpg-key-to-your-account), you can start signing your
+commits:
+
+1. Commit like you used to, the only difference is the addition of the `-S` flag:
+
+    ```
+    git commit -S -m "My commit msg"
+    ```
+
+1. Enter the passphrase of your GPG key when asked.
+1. Push to GitLab and check that your commits [are verified](#verifying-commits).
+
+If you don't want to type the `-S` flag every time you commit, you can tell Git
+to sign your commits automatically:
+
+```
+git config --global commit.gpgsign true
+```
+
+## Verifying commits
+
+1. Within a project or [merge request](../merge_requests/index.md), navigate to
+   the **Commits** tab. Signed commits will show a badge containing either
+   "Verified" or "Unverified", depending on the verification status of the GPG
+   signature.
+
+    ![Signed and unsigned commits](img/project_signed_and_unsigned_commits.png)
+
+1. By clicking on the GPG badge, details of the signature are displayed.
+
+    ![Signed commit with verified signature](img/project_signed_commit_verified_signature.png)
+
+    ![Signed commit with verified signature](img/project_signed_commit_unverified_signature.png)
+
+## Revoking a GPG key
+
+Revoking a key **unverifies** already signed commits. Commits that were
+verified by using this key will change to an unverified state. Future commits
+will also stay unverified once you revoke this key. This action should be used
+in case your key has been compromised.
+
+To revoke a GPG key:
+
+1. On the upper right corner, click on your avatar and go to your **Settings**.
+1. Navigate to the **GPG keys** tab.
+1. Click on **Revoke** besides the GPG key you want to delete.
+
+## Removing a GPG key
+
+Removing a key **does not unverify** already signed commits. Commits that were
+verified by using this key will stay verified. Only unpushed commits will stay
+unverified once you remove this key. To unverify already signed commits, you need
+to [revoke the associated GPG key](#revoking-a-gpg-key) from your account.
+
+To remove a GPG key from your account:
+
+1. On the upper right corner, click on your avatar and go to your **Settings**.
+1. Navigate to the **GPG keys** tab.
+1. Click on the trash icon besides the GPG key you want to delete.
+
+[ce-9546]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9546

doc/user/project/index.md

@@ -24,6 +24,7 @@ integrated platform
   from messing with history or pushing code without review
   - [Protected tags](protected_tags.md): Control over who has
   permission to create tags, and prevent accidental update or deletion
+  - [Signing commits](gpg_signed_commits/index.md): use GPG to sign your commits
 - [Merge Requests](merge_requests/index.md): Apply your branching
 strategy and get reviewed by your team
   - [Merge Request Approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) (**EES/EEP**): Ask for approval before

doc/workflow/gpg_signed_commits/index.md

-# Signing commits with GPG
-
-## Getting started
-
-- [Git Tools - Signing Your Work](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work)
-- [Git Tools - Signing Your Work: GPG introduction](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work#_gpg_introduction)
-- [Git Tools - Signing Your Work: Signing commits](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work#_signing_commits)
-
-## How GitLab handles GPG
-
-GitLab uses its own keyring to verify the GPG signature. It does not access any
-public key server.
-
-In order to have a commit verified on GitLab the corresponding public key needs
-to be uploaded to GitLab.
-
-For a signature to be verified two prerequisites need to be met:
-
-1. The public key needs to be added to GitLab
-1. One of the emails in the GPG key matches your **primary** email
-
-## Add a GPG key
-
-1. On the upper right corner, click on your avatar and go to your **Settings**.
-
-    ![Settings dropdown](../../gitlab-basics/img/profile_settings.png)
-
-1. Navigate to the **GPG keys** tab.
-
-    ![GPG Keys](img/profile_settings_gpg_keys.png)
-
-1. Paste your **public** key in the 'Key' box.
-
-    ![Paste GPG public key](img/profile_settings_gpg_keys_paste_pub.png)
-
-1. Finally, click on **Add key** to add it to GitLab. You will be able to see
-   its fingerprint, the corresponding email address and creation date.
-
-    ![GPG key single page](img/profile_settings_gpg_keys_single_key.png)
-
->**Note:**
-Once you add a key, you cannot edit it, only remove it. In case the paste
-didn't work, you will have to remove the offending key and re-add it.
-
-## Remove a GPG key
-
-1. On the upper right corner, click on your avatar and go to your **Settings**.
-
-1. Navigate to the **GPG keys** tab.
-
-1. Click on the trash icon besides the GPG key you want to delete.
-
->**Note:**
-Removing a key **does not unverify** already signed commits. Commits that were
-verified by using this key will stay verified. Only unpushed commits will stay
-unverified once you remove this key.
-
-## Revoke a GPG key
-
-1. On the upper right corner, click on your avatar and go to your **Settings**.
-
-1. Navigate to the **GPG keys** tab.
-
-1. Click on **Revoke** besides the GPG key you want to delete.
-
->**Note:**
-Revoking a key **unverifies** already signed commits. Commits that were
-verified by using this key will change to an unverified state. Future commits
-will also stay unverified once you revoke this key. This action should be used
-in case your key has been compromised.
-
-## Verifying commits
-
-1. Within a project navigate to the **Commits** tag. Signed commits will show a
-   badge containing either "Verified" or "Unverified", depending on the
-   verification status of the GPG signature.
-
-    ![Signed and unsigned commits](img/project_signed_and_unsigned_commits.png)
-
-1. By clicking on the GPG badge details of the signature are displayed.
-
-    ![Signed commit with verified signature](img/project_signed_commit_verified_signature.png)
-
-    ![Signed commit with verified signature](img/project_signed_commit_unverified_signature.png)

ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.js

@@ -24,7 +24,7 @@ export default {
     },
   },
   template: `
-    <div class="accept-control spacing inline">
+    <div class="accept-control inline">
       <label class="merge-param-checkbox">
         <input
           type="checkbox"

lib/gitlab/database/load_balancing.rb

@@ -61,15 +61,10 @@ module Gitlab
       def self.configure_proxy
         self.proxy = ConnectionProxy.new(hosts)
 
-        # ActiveRecordProxy's methods are made available as class methods in
-        # ActiveRecord::Base, while still allowing the use of `super`.
+        # This hijacks the "connection" method to ensure both
+        # `ActiveRecord::Base.connection` and all models use the same load
+        # balancing proxy.
         ActiveRecord::Base.singleton_class.prepend(ActiveRecordProxy)
-
-        # The above will only patch newly defined models, so we also need to
-        # patch existing ones.
-        active_record_models.each do |model|
-          model.singleton_class.prepend(ModelProxy)
-        end
       end
 
       def self.active_record_models

lib/gitlab/database/load_balancing/active_record_proxy.rb

 module Gitlab
   module Database
     module LoadBalancing
-      # Module injected into ActiveRecord::Base to allow proxying of subclasses.
+      # Module injected into ActiveRecord::Base to allow hijacking of the
+      # "connection" method.
       module ActiveRecordProxy
-        def inherited(by)
-          super(by)
-
-          # The methods in ModelProxy will become available as class methods for
-          # the class defined in `by`.
-          by.singleton_class.prepend(ModelProxy)
+        def connection
+          LoadBalancing.proxy
         end
       end
     end

lib/gitlab/database/load_balancing/load_balancer.rb

@@ -6,8 +6,8 @@ module Gitlab
       # Each host in the load balancer uses the same credentials as the primary
       # database.
       #
-      # This class *requires* that `ActiveRecord::Base.connection` always
-      # returns a connection to the primary.
+      # This class *requires* that `ActiveRecord::Base.retrieve_connection`
+      # always returns a connection to the primary.
       class LoadBalancer
         CACHE_KEY = :gitlab_load_balancer_host
 
@@ -63,7 +63,7 @@ module Gitlab
           # Instead of immediately grinding to a halt we'll retry the operation
           # a few times.
           retry_with_backoff do
-            yield ActiveRecord::Base.connection
+            yield ActiveRecord::Base.retrieve_connection
           end
         end
 

lib/gitlab/database/load_balancing/model_proxy.rb

-module Gitlab
-  module Database
-    module LoadBalancing
-      # Modle injected into models in order to redirect connections to a
-      # ConnectionProxy.
-      module ModelProxy
-        def connection
-          LoadBalancing.proxy
-        end
-      end
-    end
-  end
-end

lib/gitlab/git_access.rb

@@ -6,6 +6,7 @@ module Gitlab
     include PathLocksHelper
     UnauthorizedError = Class.new(StandardError)
     NotFoundError = Class.new(StandardError)
+    ProjectMovedError = Class.new(NotFoundError)
 
     ERROR_MESSAGES = {
       upload: 'You are not allowed to upload code for this project.',
@@ -95,7 +96,8 @@ module Gitlab
     end
 
     def check_project_moved!
-      if redirected_path
+      return unless redirected_path
+
       url = protocol == 'ssh' ? project.ssh_url_to_repo : project.http_url_to_repo
       message = <<-MESSAGE.strip_heredoc
         Project '#{redirected_path}' was moved to '#{project.full_path}'.
@@ -105,8 +107,7 @@ module Gitlab
           git remote set-url origin #{url}
       MESSAGE
 
-        raise NotFoundError, message
-      end
+      raise ProjectMovedError, message
     end
 
     def check_command_disabled!(cmd)

spec/lib/gitlab/database/load_balancing/active_record_proxy_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Database::LoadBalancing::ActiveRecordProxy do
-  describe '#inherited' do
-    it 'adds the ModelProxy module to the singleton class' do
-      base = Class.new do
+  describe '#connection' do
+    it 'returns a connection proxy' do
+      dummy = Class.new do
         include Gitlab::Database::LoadBalancing::ActiveRecordProxy
       end
 
-      model = Class.new(base)
+      proxy = double(:proxy)
 
-      expect(model.included_modules).to include(described_class)
+      expect(Gitlab::Database::LoadBalancing).to receive(:proxy)
+        .and_return(proxy)
+
+      expect(dummy.new.connection).to eq(proxy)
     end
   end
 end

spec/lib/gitlab/database/load_balancing/load_balancer_spec.rb

@@ -86,14 +86,14 @@ describe Gitlab::Database::LoadBalancing::LoadBalancer do
       expect(lb).to receive(:read_write).and_call_original
 
       expect { |b| lb.read(&b) }
-        .to yield_with_args(ActiveRecord::Base.connection)
+        .to yield_with_args(ActiveRecord::Base.retrieve_connection)
     end
   end
 
   describe '#read_write' do
     it 'yields a connection for a write' do
       expect { |b| lb.read_write(&b) }
-        .to yield_with_args(ActiveRecord::Base.connection)
+        .to yield_with_args(ActiveRecord::Base.retrieve_connection)
     end
 
     it 'uses a retry with exponential backoffs' do

spec/lib/gitlab/database/load_balancing/model_proxy_spec.rb

-require 'spec_helper'
-
-describe Gitlab::Database::LoadBalancing::ModelProxy do
-  describe '#connection' do
-    it 'returns a connection proxy' do
-      dummy = Class.new do
-        include Gitlab::Database::LoadBalancing::ModelProxy
-      end
-
-      proxy = double(:proxy)
-
-      expect(Gitlab::Database::LoadBalancing).to receive(:proxy)
-        .and_return(proxy)
-
-      expect(dummy.new.connection).to eq(proxy)
-    end
-  end
-end

spec/lib/gitlab/database/load_balancing_spec.rb

@@ -106,17 +106,9 @@ describe Gitlab::Database::LoadBalancing do
     end
 
     it 'configures the connection proxy' do
-      model = double(:model)
-
       expect(ActiveRecord::Base.singleton_class).to receive(:prepend)
         .with(Gitlab::Database::LoadBalancing::ActiveRecordProxy)
 
-      expect(described_class).to receive(:active_record_models)
-        .and_return([model])
-
-      expect(model.singleton_class).to receive(:prepend)
-        .with(Gitlab::Database::LoadBalancing::ModelProxy)
-
       described_class.configure_proxy
     end
   end

spec/lib/gitlab/git_access_spec.rb

 require 'spec_helper'
 
 describe Gitlab::GitAccess do
-  let(:pull_access_check) { access.check('git-upload-pack', '_any') }
-  let(:push_access_check) { access.check('git-receive-pack', '_any') }
-  let(:access) { described_class.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
+
   let(:actor) { user }
+  let(:project) { create(:project, :repository) }
   let(:protocol) { 'ssh' }
+  let(:authentication_abilities) { %i[read_project download_code push_code] }
   let(:redirected_path) { nil }
-  let(:authentication_abilities) do
-    [
-      :read_project,
-      :download_code,
-      :push_code
-    ]
-  end
+
+  let(:access) { described_class.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) }
+  let(:push_access_check) { access.check('git-receive-pack', '_any') }
+  let(:pull_access_check) { access.check('git-upload-pack', '_any') }
 
   describe '#check with single protocols allowed' do
     def disable_protocol(protocol)
@@ -27,14 +23,13 @@ describe Gitlab::GitAccess do
         disable_protocol('ssh')
       end
 
-      it 'blocks ssh git push' do
+      it 'blocks ssh git push and pull' do
+        aggregate_failures do
           expect { push_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
-      end
-
-      it 'blocks ssh git pull' do
           expect { pull_access_check }.to raise_unauthorized('Git access over SSH is not allowed')
         end
       end
+    end
 
     context 'http disabled' do
       let(:protocol) { 'http' }
@@ -43,15 +38,14 @@ describe Gitlab::GitAccess do
         disable_protocol('http')
       end
 
-      it 'blocks http push' do
+      it 'blocks http push and pull' do
+        aggregate_failures do
           expect { push_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
-      end
-
-      it 'blocks http git pull' do
           expect { pull_access_check }.to raise_unauthorized('Git access over HTTP is not allowed')
         end
       end
     end
+  end
 
   describe '#check_project_accessibility!' do
     context 'when the project exists' do
@@ -65,22 +59,20 @@ describe Gitlab::GitAccess do
               deploy_key.projects << project
             end
 
-            it 'allows pull access' do
+            it 'allows push and pull access' do
+              aggregate_failures do
+                expect { push_access_check }.not_to raise_error
                 expect { pull_access_check }.not_to raise_error
               end
-
-            it 'allows push access' do
-              expect { push_access_check }.not_to raise_error
             end
           end
 
           context 'when the Deploykey does not have access to the project' do
-            it 'blocks pulls with "not found"' do
-              expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            it 'blocks push and pull with "not found"' do
+              aggregate_failures do
+                expect { push_access_check }.to raise_not_found
+                expect { pull_access_check }.to raise_not_found
               end
-
-            it 'blocks pushes with "not found"' do
-              expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
             end
           end
         end
@@ -88,25 +80,23 @@ describe Gitlab::GitAccess do
         context 'when actor is a User' do
           context 'when the User can read the project' do
             before do
-              project.team << [user, :master]
+              project.add_master(user)
             end
 
-            it 'allows pull access' do
+            it 'allows push and pull access' do
+              aggregate_failures do
                 expect { pull_access_check }.not_to raise_error
-            end
-
-            it 'allows push access' do
                 expect { push_access_check }.not_to raise_error
               end
             end
+          end
 
           context 'when the User cannot read the project' do
-            it 'blocks pulls with "not found"' do
-              expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            it 'blocks push and pull with "not found"' do
+              aggregate_failures do
+                expect { push_access_check }.to raise_not_found
+                expect { pull_access_check }.to raise_not_found
               end
-
-            it 'blocks pushes with "not found"' do
-              expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
             end
           end
         end
@@ -121,7 +111,7 @@ describe Gitlab::GitAccess do
           end
 
           it 'does not block pushes with "not found"' do
-            expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.')
+            expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
           end
         end
       end
@@ -137,17 +127,17 @@ describe Gitlab::GitAccess do
           end
 
           it 'does not block pushes with "not found"' do
-            expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.')
+            expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload])
           end
         end
 
         context 'when guests cannot read the project' do
           it 'blocks pulls with "not found"' do
-            expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            expect { pull_access_check }.to raise_not_found
           end
 
           it 'blocks pushes with "not found"' do
-            expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+            expect { push_access_check }.to raise_not_found
           end
         end
       end
@@ -156,48 +146,50 @@ describe Gitlab::GitAccess do
     context 'when the project is nil' do
       let(:project) { nil }
 
-      it 'blocks any command with "not found"' do
-        expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.')
-        expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.')
+      it 'blocks push and pull with "not found"' do
+        aggregate_failures do
+          expect { pull_access_check }.to raise_not_found
+          expect { push_access_check }.to raise_not_found
+        end
       end
     end
   end
 
   describe '#check_project_moved!' do
     before do
-      project.team << [user, :master]
+      project.add_master(user)
     end
 
     context 'when a redirect was not followed to find the project' do
-      context 'pull code' do
-        it { expect { pull_access_check }.not_to raise_error }
+      it 'allows push and pull access' do
+        aggregate_failures do
+          expect { push_access_check }.not_to raise_error
+          expect { pull_access_check }.not_to raise_error
         end
-
-      context 'push code' do
-        it { expect { push_access_check }.not_to raise_error }
       end
     end
 
     context 'when a redirect was followed to find the project' do
       let(:redirected_path) { 'some/other-path' }
 
-      context 'pull code' do
-        it { expect { pull_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) }
-        it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
+      it 'blocks push and pull access' do
+        aggregate_failures do
+          expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /Project '#{redirected_path}' was moved to '#{project.full_path}'/)
+          expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/)
 
-        context 'http protocol' do
-          let(:protocol) { 'http' }
-          it { expect { pull_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
+          expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /Project '#{redirected_path}' was moved to '#{project.full_path}'/)
+          expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.ssh_url_to_repo}/)
         end
       end
 
-      context 'push code' do
-        it { expect { push_access_check }.to raise_not_found(/Project '#{redirected_path}' was moved to '#{project.full_path}'/) }
-        it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.ssh_url_to_repo}/) }
-
       context 'http protocol' do
         let(:protocol) { 'http' }
-          it { expect { push_access_check }.to raise_not_found(/git remote set-url origin #{project.http_url_to_repo}/) }
+
+        it 'includes the path to the project using HTTP' do
+          aggregate_failures do
+            expect { push_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/)
+            expect { pull_access_check }.to raise_error(described_class::ProjectMovedError, /git remote set-url origin #{project.http_url_to_repo}/)
+          end
         end
       end
     end
@@ -242,40 +234,28 @@ describe Gitlab::GitAccess do
   end
 
   describe '#check_download_access!' do
-    describe 'master permissions' do
-      before do
-        project.team << [user, :master]
-      end
+    it 'allows masters to pull' do
+      project.add_master(user)
 
-      context 'pull code' do
-        it { expect { pull_access_check }.not_to raise_error }
-      end
+      expect { pull_access_check }.not_to raise_error
     end
 
-    describe 'guest permissions' do
-      before do
-        project.team << [user, :guest]
-      end
+    it 'disallows guests to pull' do
+      project.add_guest(user)
 
-      context 'pull code' do
-        it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') }
-      end
+      expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download])
     end
 
-    describe 'blocked user' do
-      before do
-        project.team << [user, :master]
+    it 'disallows blocked users to pull' do
+      project.add_master(user)
       user.block
-      end
 
-      context 'pull code' do
-        it { expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.') }
-      end
+      expect { pull_access_check }.to raise_unauthorized('Your account has been blocked.')
     end
 
     describe 'without access to project' do
       context 'pull code' do
-        it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+        it { expect { pull_access_check }.to raise_not_found }
       end
 
       context 'when project is public' do
@@ -292,7 +272,7 @@ describe Gitlab::GitAccess do
           it 'does not give access to download code' do
             public_project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
 
-            expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.')
+            expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download])
           end
         end
       end
@@ -321,13 +301,13 @@ describe Gitlab::GitAccess do
           context 'from internal project' do
             let(:project) { create(:project, :internal, :repository) }
 
-            it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+            it { expect { pull_access_check }.to raise_not_found }
           end
 
           context 'from private project' do
             let(:project) { create(:project, :private, :repository) }
 
-            it { expect { pull_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+            it { expect { pull_access_check }.to raise_not_found }
           end
         end
       end
@@ -380,7 +360,7 @@ describe Gitlab::GitAccess do
 
         context 'when is not member of the project' do
           context 'pull code' do
-            it { expect { pull_access_check }.to raise_unauthorized('You are not allowed to download code from this project.') }
+            it { expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download]) }
           end
         end
       end
@@ -440,28 +420,30 @@ describe Gitlab::GitAccess do
       end
     end
 
-    # Run permission checks for a user
     def self.run_permission_checks(permissions_matrix)
-      permissions_matrix.keys.each do |role|
-        describe "#{role} access" do
-          before do
+      permissions_matrix.each_pair do |role, matrix|
+        # Run through the entire matrix for this role in one test to avoid
+        # repeated setup.
+        #
+        # Expectations are given a custom failure message proc so that it's
+        # easier to identify which check(s) failed.
+        it "has the correct permissions for #{role}s" do
           if role == :admin
             user.update_attribute(:admin, true)
           else
             project.team << [user, role]
           end
-          end
 
-          permissions_matrix[role].each do |action, allowed|
-            context action.to_s do
-              subject { access.send(:check_push_access!, changes[action]) }
+          aggregate_failures do
+            matrix.each do |action, allowed|
+              check = -> { access.send(:check_push_access!, changes[action]) }
 
-              it do
               if allowed
-                  expect { subject }.not_to raise_error
+                expect(&check).not_to raise_error,
+                  -> { "expected #{action} to be allowed" }
               else
-                  expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError)
-                end
+                expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError),
+                  -> { "expected #{action} to be disallowed" }
               end
             end
           end
@@ -471,24 +453,22 @@ describe Gitlab::GitAccess do
 
     # Run permission checks for a group
     def self.run_group_permission_checks(permissions_matrix)
-      permissions_matrix.keys.each do |role|
-        describe "#{role} access" do
-          before do
-            project.project_group_links.create(
-              group: group, group_access: Gitlab::Access.sym_options[role]
-            )
-          end
+      permissions_matrix.each_pair do |role, matrix|
+        it "has the correct permissions for group #{role}s" do
+          project
+            .project_group_links
+            .create(group: group, group_access: Gitlab::Access.sym_options[role])
 
-          permissions_matrix[role].each do |action, allowed|
-            context action.to_s do
-              subject { access.send(:check_push_access!, changes[action]) }
+          aggregate_failures do
+            matrix.each do |action, allowed|
+              check = -> { access.send(:check_push_access!, changes[action]) }
 
-              it do
               if allowed
-                  expect { subject }.not_to raise_error
+                expect(&check).not_to raise_error,
+                  -> { "expected #{action} to be allowed" }
               else
-                  expect { subject }.to raise_error(Gitlab::GitAccess::UnauthorizedError)
-                end
+                expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError),
+                  -> { "expected #{action} to be disallowed" }
               end
             end
           end
@@ -719,10 +699,11 @@ describe Gitlab::GitAccess do
         allow(License).to receive(:block_changes?).and_return(true)
       end
 
-      # All permissions are `false`
-      permissions_matrix = Hash.new(Hash.new(false))
+      # Only check admin; if an admin can't do it, other roles can't either
+      matrix = permissions_matrix[:admin].dup
+      matrix.each { |key, _| matrix[key] = false }
 
-      run_permission_checks(permissions_matrix)
+      run_permission_checks(admin: matrix)
     end
 
     context "when in a secondary gitlab geo node" do
@@ -732,38 +713,39 @@ describe Gitlab::GitAccess do
         allow(Gitlab::Geo).to receive(:secondary?) { true }
       end
 
-      # All permissions are `false`
-      permissions_matrix = Hash.new(Hash.new(false))
+      # Only check admin; if an admin can't do it, other roles can't either
+      matrix = permissions_matrix[:admin].dup
+      matrix.each { |key, _| matrix[key] = false }
 
-      run_permission_checks(permissions_matrix)
+      run_permission_checks(admin: matrix)
     end
 
     describe "push_rule_check" do
+      let(:start_sha) { '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' }
+      let(:end_sha)   { '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' }
+
       before do
-        project.team << [user, :developer]
+        project.add_developer(user)
 
-        allow(project.repository).to receive(:new_commits).and_return(
-          project.repository.commits_between('6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9', '570e7b2abdd848b95f2f578043fc23bd6f6fd24d')
-        )
+        allow(project.repository).to receive(:new_commits)
+          .and_return(project.repository.commits_between(start_sha, end_sha))
       end
 
       describe "author email check" do
         it 'returns true' do
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
 
         it 'returns false' do
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "@only.com")
+          project.create_push_rule(commit_message_regex: "@only.com")
 
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true for tags' do
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "@only.com")
+          project.create_push_rule(commit_message_regex: "@only.com")
 
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/tags/v1') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/tags/v1") }.not_to raise_error
         end
 
         it 'allows githook for new branch with an old bad commit' do
@@ -772,11 +754,10 @@ describe Gitlab::GitAccess do
           allow(bad_commit).to receive(:refs).and_return([ref_object])
           allow_any_instance_of(Repository).to receive(:commits_between).and_return([bad_commit])
 
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "Change some files")
+          project.create_push_rule(commit_message_regex: "Change some files")
 
           # push to new branch, so use a blank old rev and new ref
-          expect { access.send(:check_push_access!, "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new-branch") }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{Gitlab::Git::BLANK_SHA} #{end_sha} refs/heads/new-branch") }.not_to raise_error
         end
 
         it 'allows githook for any change with an old bad commit' do
@@ -785,11 +766,10 @@ describe Gitlab::GitAccess do
           allow(bad_commit).to receive(:refs).and_return([ref_object])
           allow(project.repository).to receive(:commits_between).and_return([bad_commit])
 
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "Change some files")
+          project.create_push_rule(commit_message_regex: "Change some files")
 
           # push to new branch, so use a blank old rev and new ref
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
 
         it 'does not allow any change from Web UI with bad commit' do
@@ -800,78 +780,76 @@ describe Gitlab::GitAccess do
           allow(project.repository).to receive(:commits_between).and_return([bad_commit])
           allow(project.repository).to receive(:new_commits).and_return([bad_commit])
 
-          project.create_push_rule
-          project.push_rule.update(commit_message_regex: "Change some files")
+          project.create_push_rule(commit_message_regex: "Change some files")
 
           # push to new branch, so use a blank old rev and new ref
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
       end
 
       describe "member_check" do
         before do
-          project.create_push_rule
-          project.push_rule.update(member_check: true)
+          project.create_push_rule(member_check: true)
         end
 
         it 'returns false for non-member user' do
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true if committer is a gitlab member' do
           create(:user, email: 'dmitriy.zaporozhets@gmail.com')
 
-          expect { access.send(:check_push_access!, '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
 
       describe "file names check" do
+        let(:start_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' }
+        let(:end_sha)   { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' }
+
         before do
-          allow(project.repository).to receive(:new_commits).and_return(
-            project.repository.commits_between('913c66a37b4a45b9769037c55c2d238bd0942d2e', '33f3729a45c02fc67d00adb1b8bca394b0e761d9')
-          )
+          allow(project.repository).to receive(:new_commits)
+            .and_return(project.repository.commits_between(start_sha, end_sha))
         end
 
         it 'returns false when filename is prohibited' do
-          project.create_push_rule
-          project.push_rule.update(file_name_regex: "jpg$")
+          project.create_push_rule(file_name_regex: "jpg$")
 
-          expect { access.send(:check_push_access!, '913c66a37b4a45b9769037c55c2d238bd0942d2e 33f3729a45c02fc67d00adb1b8bca394b0e761d9 refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true if file name is allowed' do
-          project.create_push_rule
-          project.push_rule.update(file_name_regex: "exe$")
+          project.create_push_rule(file_name_regex: "exe$")
 
-          expect { access.send(:check_push_access!, '913c66a37b4a45b9769037c55c2d238bd0942d2e 33f3729a45c02fc67d00adb1b8bca394b0e761d9 refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
 
       describe "max file size check" do
+        let(:start_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' }
+        let(:end_sha)   { '913c66a37b4a45b9769037c55c2d238bd0942d2e' }
+
         before do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(1.5.megabytes.to_i)
         end
 
         it "returns false when size is too large" do
-          project.create_push_rule
-          project.push_rule.update(max_file_size: 1)
+          project.create_push_rule(max_file_size: 1)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it "returns true when size is allowed" do
-          project.create_push_rule
-          project.push_rule.update(max_file_size: 2)
+          project.create_push_rule(max_file_size: 2)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
 
         it "returns true when size is nil" do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(nil)
-          project.create_push_rule
-          project.push_rule.update(max_file_size: 2)
+          project.create_push_rule(max_file_size: 2)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
 
@@ -883,13 +861,13 @@ describe Gitlab::GitAccess do
         it 'returns false when blob is too big' do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(100.megabytes.to_i)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.to raise_error(described_class::UnauthorizedError)
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.to raise_error(described_class::UnauthorizedError)
         end
 
         it 'returns true when blob is just right' do
           allow_any_instance_of(Gitlab::Git::Blob).to receive(:size).and_return(2.megabytes.to_i)
 
-          expect { access.send(:check_push_access!, 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660 913c66a37b4a45b9769037c55c2d238bd0942d2e refs/heads/master') }.not_to raise_error
+          expect { access.send(:check_push_access!, "#{start_sha} #{end_sha} refs/heads/master") }.not_to raise_error
         end
       end
     end
@@ -903,26 +881,26 @@ describe Gitlab::GitAccess do
         project.team << [user, :reporter]
       end
 
-      it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
+      it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
     end
 
     context 'when unauthorized' do
       context 'to public project' do
         let(:project) { create(:project, :public, :repository) }
 
-        it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
+        it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
       end
 
       context 'to internal project' do
         let(:project) { create(:project, :internal, :repository) }
 
-        it { expect { push_access_check }.to raise_unauthorized('You are not allowed to upload code for this project.') }
+        it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) }
       end
 
       context 'to private project' do
         let(:project) { create(:project, :private, :repository) }
 
-        it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+        it { expect { push_access_check }.to raise_not_found }
       end
     end
   end
@@ -931,7 +909,7 @@ describe Gitlab::GitAccess do
     let(:project) { create(:project, :repository, :read_only_repository) }
 
     it 'denies push access' do
-      project.team << [user, :master]
+      project.add_master(user)
 
       expect { push_access_check }.to raise_unauthorized('The repository is temporarily read-only. Please try again later.')
     end
@@ -956,19 +934,19 @@ describe Gitlab::GitAccess do
         context 'to public project' do
           let(:project) { create(:project, :public, :repository) }
 
-          it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+          it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
         end
 
         context 'to internal project' do
           let(:project) { create(:project, :internal, :repository) }
 
-          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+          it { expect { push_access_check }.to raise_not_found }
         end
 
         context 'to private project' do
           let(:project) { create(:project, :private, :repository) }
 
-          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+          it { expect { push_access_check }.to raise_not_found }
         end
       end
     end
@@ -981,26 +959,26 @@ describe Gitlab::GitAccess do
           key.projects << project
         end
 
-        it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+        it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
       end
 
       context 'when unauthorized' do
         context 'to public project' do
           let(:project) { create(:project, :public, :repository) }
 
-          it { expect { push_access_check }.to raise_unauthorized('This deploy key does not have write access to this project.') }
+          it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
         end
 
         context 'to internal project' do
           let(:project) { create(:project, :internal, :repository) }
 
-          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+          it { expect { push_access_check }.to raise_not_found }
         end
 
         context 'to private project' do
           let(:project) { create(:project, :private, :repository) }
 
-          it { expect { push_access_check }.to raise_not_found('The project you were looking for could not be found.') }
+          it { expect { push_access_check }.to raise_not_found }
         end
       end
     end
@@ -1012,8 +990,9 @@ describe Gitlab::GitAccess do
     raise_error(Gitlab::GitAccess::UnauthorizedError, message)
   end
 
-  def raise_not_found(message)
-    raise_error(Gitlab::GitAccess::NotFoundError, message)
+  def raise_not_found
+    raise_error(Gitlab::GitAccess::NotFoundError,
+                Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
   end
 
   def build_authentication_abilities

spec/services/issues/export_csv_service_spec.rb

@@ -4,6 +4,7 @@ describe Issues::ExportCsvService do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
   let!(:issue)  { create(:issue, project: project, author: user) }
+  let!(:bad_issue) { create(:issue, project: project, author: user) }
   let(:subject) { described_class.new(Issue.all) }
 
   it 'renders csv to string' do
@@ -40,7 +41,10 @@ describe Issues::ExportCsvService do
                     created_at: DateTime.new(2015, 4, 3, 2, 1, 0),
                     updated_at: DateTime.new(2016, 5, 4, 3, 2, 1),
                     closed_at: DateTime.new(2017, 6, 5, 4, 3, 2),
-                    labels: [feature_label, idea_label])
+                    labels: [feature_label, idea_label],
+                    time_estimate: 72000)
+      issue.timelogs.create(time_spent: 360, user: user)
+      issue.timelogs.create(time_spent: 200, user: user)
     end
 
     specify 'iid' do
@@ -61,6 +65,7 @@ describe Issues::ExportCsvService do
 
     specify 'description' do
       expect(csv[0]['Description']).to eq issue.description
+      expect(csv[1]['Description']).to eq nil
     end
 
     specify 'author name' do
@@ -73,10 +78,12 @@ describe Issues::ExportCsvService do
 
     specify 'assignee name' do
       expect(csv[0]['Assignee']).to eq user.name
+      expect(csv[1]['Assignee']).to eq ''
     end
 
     specify 'assignee username' do
       expect(csv[0]['Assignee Username']).to eq user.username
+      expect(csv[1]['Assignee Username']).to eq ''
     end
 
     specify 'confidential' do
@@ -85,14 +92,17 @@ describe Issues::ExportCsvService do
 
     specify 'milestone' do
       expect(csv[0]['Milestone']).to eq issue.milestone.title
+      expect(csv[1]['Milestone']).to eq nil
     end
 
     specify 'labels' do
       expect(csv[0]['Labels']).to eq 'Feature,Idea'
+      expect(csv[1]['Labels']).to eq nil
     end
 
     specify 'due_date' do
       expect(csv[0]['Due Date']).to eq '2014-03-02'
+      expect(csv[1]['Due Date']).to eq nil
     end
 
     specify 'created_at' do
@@ -105,6 +115,17 @@ describe Issues::ExportCsvService do
 
     specify 'closed_at' do
       expect(csv[0]['Closed At (UTC)']).to eq '2017-06-05 04:03:02'
+      expect(csv[1]['Closed At (UTC)']).to eq nil
+    end
+
+    specify 'time estimate' do
+      expect(csv[0]['Time Estimate']).to eq '72000'
+      expect(csv[1]['Time Estimate']).to eq '0'
+    end
+
+    specify 'time spent' do
+      expect(csv[0]['Time Spent']).to eq '560'
+      expect(csv[1]['Time Spent']).to eq '0'
     end
   end
 

spec/support/stub_artifacts.rb

 module StubConfiguration
   def stub_artifacts_object_storage(enabled: true)
     Fog.mock!
-    allow(Gitlab.config.artifacts.object_store).to receive_messages(
-      enabled: enabled,
-      remote_directory: 'artifacts',
-      connection: {
-        provider: 'AWS',
-        aws_access_key_id: 'AWS_ACCESS_KEY_ID',
-        aws_secret_access_key: 'AWS_SECRET_ACCESS_KEY',
-        region: 'eu-central-1'
-      }
-    )
 
+    allow(Gitlab.config.artifacts.object_store).to receive(:enabled) { enabled }
     allow_any_instance_of(ArtifactUploader).to receive(:verify_license!) { true }
 
     return unless enabled
 
-    ::Fog::Storage.new(Gitlab.config.artifacts.object_store.connection).tap do |connection|
+    ::Fog::Storage.new(ArtifactUploader.object_store_credentials).tap do |connection|
       begin
         connection.directories.create(key: 'artifacts')
       rescue Excon::Error::Conflict

spec/uploaders/object_store_uploader_spec.rb

@@ -239,7 +239,7 @@ describe ObjectStoreUploader do
   end
 
   describe '#fog_credentials' do
-    let(:connection) { 'connection' }
+    let(:connection) { Settingslogic.new("provider" => "AWS") }
 
     before do
       uploader_class.storage_options double(
@@ -248,7 +248,7 @@ describe ObjectStoreUploader do
 
     subject { uploader.fog_credentials }
 
-    it { is_expected.to eq(connection) }
+    it { is_expected.to eq(provider: 'AWS') }
   end
 
   describe '#fog_public' do