Merge remote-tracking branch 'upstream/master' into 32815--Add-Custom-CI-Config-Path

* upstream/master: (149 commits)
  Revert change to design. Go back to scrollable page
  Fixes the column widths for the new navigation options in settings
  Migrate #submodule_url_for to Gitaly
  Add test example for external commit status retries
  Fix invalid Rails.logger call in lib/gitlab/health_checks/fs_shards_check.rb
  Fix build for !12300.
  Log rescued exceptions to Sentry
  Fix issues with non-UTF8 filenames by always fixing the encoding of tree and blob paths
  Revert "Merge branch 'revert-12499' into 'master'"
  Prevent accidental deletion of protected MR source branch by repeating checks before actual deletion
  Improve the overall UX for the new monitoring dashboard
  Document that GitLab 9.3 requires the TRIGGER permission on MySQL
  Instrument Unicorn with Ruby exporter
  Remove group modal like remove project modal. Closes #33130
  Update prometheus client gem
  Enables the option in user preferences to turn on the new navigation
  Add Jasmine tests for `OAuthRememberMe`
  Simplify authentication logic in the v4 users API for !12445.
  Use stub_application_setting when testing ApplicationHelper#support_url
  wait_for_requests is not needed when AJAX is not in play
  ...

21 files changed, 365 insertions, 99 deletions

diff --git a/lib/api/api.rb b/lib/api/api.rb
index d767af36e8e..efcf0976a81 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -2,6 +2,8 @@ module API
   class API < Grape::API
     include APIGuard
 
+    allow_access_with_scope :api
+
     version %w(v3 v4), using: :path
 
     version 'v3', using: :path do
@@ -44,7 +46,6 @@ module API
       mount ::API::V3::Variables
     end
 
-    before { allow_access_with_scope :api }
     before { header['X-Frame-Options'] = 'SAMEORIGIN' }
     before { Gitlab::I18n.locale = current_user&.preferred_language }
 
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index 9fcf04efa38..0d2d71e336a 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -23,6 +23,23 @@ module API
       install_error_responders(base)
     end
 
+    class_methods do
+      # Set the authorization scope(s) allowed for an API endpoint.
+      #
+      # A call to this method maps the given scope(s) to the current API
+      # endpoint class. If this method is called multiple times on the same class,
+      # the scopes are all aggregated.
+      def allow_access_with_scope(scopes, options = {})
+        Array(scopes).each do |scope|
+          allowed_scopes << Scope.new(scope, options)
+        end
+      end
+
+      def allowed_scopes
+        @scopes ||= []
+      end
+    end
+
     # Helper Methods for Grape Endpoint
     module HelperMethods
       # Invokes the doorkeeper guard.
@@ -47,7 +64,7 @@ module API
         access_token = find_access_token
         return nil unless access_token
 
-        case AccessTokenValidationService.new(access_token).validate(scopes: scopes)
+        case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
         when AccessTokenValidationService::INSUFFICIENT_SCOPE
           raise InsufficientScopeError.new(scopes)
 
@@ -74,18 +91,6 @@ module API
         @current_user
       end
 
-      # Set the authorization scope(s) allowed for the current request.
-      #
-      # Note: A call to this method adds to any previous scopes in place. This is done because
-      # `Grape` callbacks run from the outside-in: the top-level callback (API::API) runs first, then
-      # the next-level callback (API::API::Users, for example) runs. All these scopes are valid for the
-      # given endpoint (GET `/api/users` is accessible by the `api` and `read_user` scopes), and so they
-      # need to be stored.
-      def allow_access_with_scope(*scopes)
-        @scopes ||= []
-        @scopes.concat(scopes.map(&:to_s))
-      end
-
       private
 
       def find_user_by_authentication_token(token_string)
@@ -96,7 +101,7 @@ module API
         access_token = PersonalAccessToken.active.find_by_token(token_string)
         return unless access_token
 
-        if AccessTokenValidationService.new(access_token).include_any_scope?(scopes)
+        if AccessTokenValidationService.new(access_token, request: request).include_any_scope?(scopes)
           User.find(access_token.user_id)
         end
       end
diff --git a/lib/api/commits.rb b/lib/api/commits.rb
index c6fc17cc391..bcb842b9211 100644
--- a/lib/api/commits.rb
+++ b/lib/api/commits.rb
@@ -67,7 +67,7 @@ module API
         result = ::Files::MultiService.new(user_project, current_user, attrs).execute
 
         if result[:status] == :success
-          commit_detail = user_project.repository.commits(result[:result], limit: 1).first
+          commit_detail = user_project.repository.commit(result[:result])
           present commit_detail, with: Entities::RepoCommitDetail
         else
           render_api_error!(result[:message], 400)
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 2c73a6fdc4e..a2a661b205c 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -342,8 +342,8 @@ module API
     def initial_current_user
       return @initial_current_user if defined?(@initial_current_user)
       Gitlab::Auth::UniqueIpsLimiter.limit_user! do
-        @initial_current_user ||= find_user_by_private_token(scopes: @scopes)
-        @initial_current_user ||= doorkeeper_guard(scopes: @scopes)
+        @initial_current_user ||= find_user_by_private_token(scopes: scopes_registered_for_endpoint)
+        @initial_current_user ||= doorkeeper_guard(scopes: scopes_registered_for_endpoint)
         @initial_current_user ||= find_user_from_warden
 
         unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
@@ -407,5 +407,22 @@ module API
 
       exception.status == 500
     end
+
+    # An array of scopes that were registered (using `allow_access_with_scope`)
+    # for the current endpoint class. It also returns scopes registered on
+    # `API::API`, since these are meant to apply to all API routes.
+    def scopes_registered_for_endpoint
+      @scopes_registered_for_endpoint ||=
+        begin
+          endpoint_classes = [options[:for].presence, ::API::API].compact
+          endpoint_classes.reduce([]) do |memo, endpoint|
+            if endpoint.respond_to?(:allowed_scopes)
+              memo.concat(endpoint.allowed_scopes)
+            else
+              memo
+            end
+          end
+        end
+    end
   end
 end
diff --git a/lib/api/scope.rb b/lib/api/scope.rb
new file mode 100644
index 00000000000..d5165b2e482
--- /dev/null
+++ b/lib/api/scope.rb
@@ -0,0 +1,23 @@
+# Encapsulate a scope used for authorization, such as `api`, or `read_user`
+module API
+  class Scope
+    attr_reader :name, :if
+
+    def initialize(name, options = {})
+      @name = name.to_sym
+      @if = options[:if]
+    end
+
+    # Are the `scopes` passed in sufficient to adequately authorize the passed
+    # request for the scope represented by the current instance of this class?
+    def sufficient?(scopes, request)
+      scopes.include?(self.name) && verify_if_condition(request)
+    end
+
+    private
+
+    def verify_if_condition(request)
+      self.if.nil? || self.if.call(request)
+    end
+  end
+end
diff --git a/lib/api/users.rb b/lib/api/users.rb
index f9555842daf..88bca235692 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -1,13 +1,15 @@
 module API
   class Users < Grape::API
     include PaginationParams
+    include APIGuard
 
-    before do
-      allow_access_with_scope :read_user if request.get?
-      authenticate!
-    end
+    allow_access_with_scope :read_user, if: -> (request) { request.get? }
 
     resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
+      before do
+        authenticate_non_get!
+      end
+
       helpers do
         def find_user(params)
           id = params[:user_id] || params[:id]
@@ -51,15 +53,22 @@ module API
         use :pagination
       end
       get do
-        unless can?(current_user, :read_users_list)
-          render_api_error!("Not authorized.", 403)
-        end
-
         authenticated_as_admin! if params[:external].present? || (params[:extern_uid].present? && params[:provider].present?)
 
         users = UsersFinder.new(current_user, params).execute
 
-        entity = current_user.admin? ? Entities::UserWithAdmin : Entities::UserBasic
+        authorized = can?(current_user, :read_users_list)
+
+        # When `current_user` is not present, require that the `username`
+        # parameter is passed, to prevent an unauthenticated user from accessing
+        # a list of all the users on the GitLab instance. `UsersFinder` performs
+        # an exact match on the `username` parameter, so we are guaranteed to
+        # get either 0 or 1 `users` here.
+        authorized &&= params[:username].present? if current_user.blank?
+
+        forbidden!("Not authorized to access /api/v4/users") unless authorized
+
+        entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic
         present paginate(users), with: entity
       end
 
@@ -398,6 +407,10 @@ module API
     end
 
     resource :user do
+      before do
+        authenticate!
+      end
+
       desc 'Get the currently authenticated user' do
         success Entities::UserPublic
       end
diff --git a/lib/api/v3/users.rb b/lib/api/v3/users.rb
index 37020019e07..cf106f2552d 100644
--- a/lib/api/v3/users.rb
+++ b/lib/api/v3/users.rb
@@ -2,9 +2,11 @@ module API
   module V3
     class Users < Grape::API
       include PaginationParams
+      include APIGuard
+
+      allow_access_with_scope :read_user, if: -> (request) { request.get? }
 
       before do
-        allow_access_with_scope :read_user if request.get?
         authenticate!
       end
 
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 3933c3b04dd..ccb5d886bab 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -130,13 +130,13 @@ module Gitlab
 
         token = PersonalAccessTokensFinder.new(state: 'active').find_by(token: password)
 
-        if token && valid_scoped_token?(token, AVAILABLE_SCOPES.map(&:to_s))
+        if token && valid_scoped_token?(token, AVAILABLE_SCOPES)
           Gitlab::Auth::Result.new(token.user, nil, :personal_token, abilities_for_scope(token.scopes))
         end
       end
 
       def valid_oauth_token?(token)
-        token && token.accessible? && valid_scoped_token?(token, ["api"])
+        token && token.accessible? && valid_scoped_token?(token, [:api])
       end
 
       def valid_scoped_token?(token, scopes)
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index a7aceab4c14..ffe4f3ca95f 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -175,6 +175,10 @@ module Gitlab
         encode! @name
       end
 
+      def path
+        encode! @path
+      end
+
       def truncated?
         size && (size > loaded_size)
       end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 23d0c8a9bdb..dd5a4d5ad55 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -554,11 +554,14 @@ module Gitlab
       #   # => git@localhost:rack.git
       #
       def submodule_url_for(ref, path)
-        if submodules(ref).any?
-          submodule = submodules(ref)[path]
-
-          if submodule
-            submodule['url']
+        Gitlab::GitalyClient.migrate(:submodule_url_for) do |is_enabled|
+          if is_enabled
+            gitaly_submodule_url_for(ref, path)
+          else
+            if submodules(ref).any?
+              submodule = submodules(ref)[path]
+              submodule['url'] if submodule
+            end
           end
         end
       end
@@ -915,6 +918,18 @@ module Gitlab
         fill_submodule_ids(commit, parser.parse)
       end
 
+      def gitaly_submodule_url_for(ref, path)
+        # We don't care about the contents so 1 byte is enough. Can't request 0 bytes, 0 means unlimited.
+        commit_object = gitaly_commit_client.tree_entry(ref, path, 1)
+
+        return unless commit_object && commit_object.type == :COMMIT
+
+        gitmodules = gitaly_commit_client.tree_entry(ref, '.gitmodules', Blob::MAX_DATA_DISPLAY_SIZE)
+        found_module = GitmodulesParser.new(gitmodules.data).parse[path]
+
+        found_module && found_module['url']
+      end
+
       def alternate_object_directories
         Gitlab::Git::Env.all.values_at(*ALLOWED_OBJECT_DIRECTORIES_VARIABLES).compact
       end
diff --git a/lib/gitlab/git/tree.rb b/lib/gitlab/git/tree.rb
index b9afa05c819..b6d4e6cfe46 100644
--- a/lib/gitlab/git/tree.rb
+++ b/lib/gitlab/git/tree.rb
@@ -80,6 +80,10 @@ module Gitlab
         encode! @name
       end
 
+      def path
+        encode! @path
+      end
+
       def dir?
         type == :tree
       end
diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb
index 2d61992f595..6edc69de078 100644
--- a/lib/gitlab/gitaly_client/ref.rb
+++ b/lib/gitlab/gitaly_client/ref.rb
@@ -34,7 +34,7 @@ module Gitlab
           commit_id: commit_id,
           prefix: ref_prefix
         )
-        GitalyClient.call(@storage, :ref, :find_ref_name, request).name
+        encode!(GitalyClient.call(@storage, :ref, :find_ref_name, request).name.dup)
       end
 
       def count_tag_names
diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb
index e78b7f22e03..70da4080cae 100644
--- a/lib/gitlab/health_checks/fs_shards_check.rb
+++ b/lib/gitlab/health_checks/fs_shards_check.rb
@@ -52,7 +52,7 @@ module Gitlab
             ]
           end
         rescue RuntimeError => ex
-          Rails.logger("unexpected error #{ex} when checking #{ok_metric}")
+          Rails.logger.error("unexpected error #{ex} when checking #{ok_metric}")
           [metric(ok_metric, 0, **labels)]
         end
 
diff --git a/lib/gitlab/i18n.rb b/lib/gitlab/i18n.rb
index db7cdf4b5c7..f3d489aad0d 100644
--- a/lib/gitlab/i18n.rb
+++ b/lib/gitlab/i18n.rb
@@ -12,7 +12,8 @@ module Gitlab
       'zh_HK' => '繁體中文(香港)',
       'zh_TW' => '繁體中文(臺灣)',
       'bg' => 'български',
-      'eo' => 'Esperanto'
+      'eo' => 'Esperanto',
+      'it' => 'Italiano'
     }.freeze
 
     def available_locales
diff --git a/lib/gitlab/metrics/base_sampler.rb b/lib/gitlab/metrics/base_sampler.rb
new file mode 100644
index 00000000000..219accfc029
--- /dev/null
+++ b/lib/gitlab/metrics/base_sampler.rb
@@ -0,0 +1,94 @@
+require 'logger'
+module Gitlab
+  module Metrics
+    class BaseSampler
+      def self.initialize_instance(*args)
+        raise "#{name} singleton instance already initialized" if @instance
+        @instance = new(*args)
+        at_exit(&@instance.method(:stop))
+        @instance
+      end
+
+      def self.instance
+        @instance
+      end
+
+      attr_reader :running
+
+      # interval - The sampling interval in seconds.
+      def initialize(interval)
+        interval_half = interval.to_f / 2
+
+        @interval = interval
+        @interval_steps = (-interval_half..interval_half).step(0.1).to_a
+
+        @mutex = Mutex.new
+      end
+
+      def enabled?
+        true
+      end
+
+      def start
+        return unless enabled?
+
+        @mutex.synchronize do
+          return if running
+          @running = true
+
+          @thread = Thread.new do
+            sleep(sleep_interval)
+
+            while running
+              safe_sample
+
+              sleep(sleep_interval)
+            end
+          end
+        end
+      end
+
+      def stop
+        @mutex.synchronize do
+          return unless running
+
+          @running = false
+
+          if @thread
+            @thread.wakeup if @thread.alive?
+            @thread.join
+            @thread = nil
+          end
+        end
+      end
+
+      def safe_sample
+        sample
+      rescue => e
+        Rails.logger.warn("#{self.class}: #{e}, stopping")
+        stop
+      end
+
+      def sample
+        raise NotImplementedError
+      end
+
+      # Returns the sleep interval with a random adjustment.
+      #
+      # The random adjustment is put in place to ensure we:
+      #
+      # 1. Don't generate samples at the exact same interval every time (thus
+      #    potentially missing anything that happens in between samples).
+      # 2. Don't sample data at the same interval two times in a row.
+      def sleep_interval
+        while step = @interval_steps.sample
+          if step != @last_step
+            @last_step = step
+
+            return @interval + @last_step
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/metrics/connection_rack_middleware.rb b/lib/gitlab/metrics/connection_rack_middleware.rb
new file mode 100644
index 00000000000..b3da360be8f
--- /dev/null
+++ b/lib/gitlab/metrics/connection_rack_middleware.rb
@@ -0,0 +1,45 @@
+module Gitlab
+  module Metrics
+    class ConnectionRackMiddleware
+      def initialize(app)
+        @app = app
+      end
+
+      def self.rack_request_count
+        @rack_request_count ||= Gitlab::Metrics.counter(:rack_request, 'Rack request count')
+      end
+
+      def self.rack_response_count
+        @rack_response_count ||= Gitlab::Metrics.counter(:rack_response, 'Rack response count')
+      end
+
+      def self.rack_uncaught_errors_count
+        @rack_uncaught_errors_count ||= Gitlab::Metrics.counter(:rack_uncaught_errors, 'Rack connections handling uncaught errors count')
+      end
+
+      def self.rack_execution_time
+        @rack_execution_time ||= Gitlab::Metrics.histogram(:rack_execution_time, 'Rack connection handling execution time',
+                                                           {}, [0.05, 0.1, 0.25, 0.5, 0.7, 1, 1.5, 2, 2.5, 3, 5, 7, 10])
+      end
+
+      def call(env)
+        method = env['REQUEST_METHOD'].downcase
+        started = Time.now.to_f
+        begin
+          ConnectionRackMiddleware.rack_request_count.increment(method: method)
+
+          status, headers, body = @app.call(env)
+
+          ConnectionRackMiddleware.rack_response_count.increment(method: method, status: status)
+          [status, headers, body]
+        rescue
+          ConnectionRackMiddleware.rack_uncaught_errors_count.increment
+          raise
+        ensure
+          elapsed = Time.now.to_f - started
+          ConnectionRackMiddleware.rack_execution_time.observe({}, elapsed)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/metrics/sampler.rb b/lib/gitlab/metrics/influx_sampler.rb
index 0000450d9bb..6db1dd755b7 100644
--- a/lib/gitlab/metrics/sampler.rb
+++ b/lib/gitlab/metrics/influx_sampler.rb
@@ -5,14 +5,11 @@ module Gitlab
     # This class is used to gather statistics that can't be directly associated
     # with a transaction such as system memory usage, garbage collection
     # statistics, etc.
-    class Sampler
+    class InfluxSampler < BaseSampler
       # interval - The sampling interval in seconds.
       def initialize(interval = Metrics.settings[:sample_interval])
-        interval_half = interval.to_f / 2
-
-        @interval       = interval
-        @interval_steps = (-interval_half..interval_half).step(0.1).to_a
-        @last_step      = nil
+        super(interval)
+        @last_step = nil
 
         @metrics = []
 
@@ -26,18 +23,6 @@ module Gitlab
         end
       end
 
-      def start
-        Thread.new do
-          Thread.current.abort_on_exception = true
-
-          loop do
-            sleep(sleep_interval)
-
-            sample
-          end
-        end
-      end
-
       def sample
         sample_memory_usage
         sample_file_descriptors
@@ -86,7 +71,7 @@ module Gitlab
       end
 
       def sample_gc
-        time  = GC::Profiler.total_time * 1000.0
+        time = GC::Profiler.total_time * 1000.0
         stats = GC.stat.merge(total_time: time)
 
         # We want the difference of GC runs compared to the last sample, not the
@@ -111,23 +96,6 @@ module Gitlab
       def sidekiq?
         Sidekiq.server?
       end
-
-      # Returns the sleep interval with a random adjustment.
-      #
-      # The random adjustment is put in place to ensure we:
-      #
-      # 1. Don't generate samples at the exact same interval every time (thus
-      #    potentially missing anything that happens in between samples).
-      # 2. Don't sample data at the same interval two times in a row.
-      def sleep_interval
-        while step = @interval_steps.sample
-          if step != @last_step
-            @last_step = step
-
-            return @interval + @last_step
-          end
-        end
-      end
     end
   end
 end
diff --git a/lib/gitlab/metrics/prometheus.rb b/lib/gitlab/metrics/prometheus.rb
index 9d314a56e58..fb7bbc7cfc7 100644
--- a/lib/gitlab/metrics/prometheus.rb
+++ b/lib/gitlab/metrics/prometheus.rb
@@ -29,8 +29,8 @@ module Gitlab
         provide_metric(name) || registry.summary(name, docstring, base_labels)
       end
 
-      def gauge(name, docstring, base_labels = {})
-        provide_metric(name) || registry.gauge(name, docstring, base_labels)
+      def gauge(name, docstring, base_labels = {}, multiprocess_mode = :all)
+        provide_metric(name) || registry.gauge(name, docstring, base_labels, multiprocess_mode)
       end
 
       def histogram(name, docstring, base_labels = {}, buckets = ::Prometheus::Client::Histogram::DEFAULT_BUCKETS)
diff --git a/lib/gitlab/metrics/unicorn_sampler.rb b/lib/gitlab/metrics/unicorn_sampler.rb
new file mode 100644
index 00000000000..f6987252039
--- /dev/null
+++ b/lib/gitlab/metrics/unicorn_sampler.rb
@@ -0,0 +1,48 @@
+module Gitlab
+  module Metrics
+    class UnicornSampler < BaseSampler
+      def initialize(interval)
+        super(interval)
+      end
+
+      def unicorn_active_connections
+        @unicorn_active_connections ||= Gitlab::Metrics.gauge(:unicorn_active_connections, 'Unicorn active connections', {}, :max)
+      end
+
+      def unicorn_queued_connections
+        @unicorn_queued_connections ||= Gitlab::Metrics.gauge(:unicorn_queued_connections, 'Unicorn queued connections', {}, :max)
+      end
+
+      def enabled?
+        # Raindrops::Linux.tcp_listener_stats is only present on Linux
+        unicorn_with_listeners? && Raindrops::Linux.respond_to?(:tcp_listener_stats)
+      end
+
+      def sample
+        Raindrops::Linux.tcp_listener_stats(tcp_listeners).each do |addr, stats|
+          unicorn_active_connections.set({ type: 'tcp', address: addr }, stats.active)
+          unicorn_queued_connections.set({ type: 'tcp', address: addr }, stats.queued)
+        end
+
+        Raindrops::Linux.unix_listener_stats(unix_listeners).each do |addr, stats|
+          unicorn_active_connections.set({ type: 'unix', address: addr }, stats.active)
+          unicorn_queued_connections.set({ type: 'unix', address: addr }, stats.queued)
+        end
+      end
+
+      private
+
+      def tcp_listeners
+        @tcp_listeners ||= Unicorn.listener_names.grep(%r{\A[^/]+:\d+\z})
+      end
+
+      def unix_listeners
+        @unix_listeners ||= Unicorn.listener_names - tcp_listeners
+      end
+
+      def unicorn_with_listeners?
+        defined?(Unicorn) && Unicorn.listener_names.any?
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 22554236c38..0baea092e6a 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -2,6 +2,8 @@ require 'securerandom'
 
 module Gitlab
   class Shell
+    GITLAB_SHELL_ENV_VARS = %w(GIT_TERMINAL_PROMPT).freeze
+
     Error = Class.new(StandardError)
 
     KeyAdder = Struct.new(:io) do
@@ -67,8 +69,8 @@ module Gitlab
     #   add_repository("/path/to/storage", "gitlab/gitlab-ci")
     #
     def add_repository(storage, name)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path,
-                                   'add-project', storage, "#{name}.git"])
+      gitlab_shell_fast_execute([gitlab_shell_projects_path,
+                                 'add-project', storage, "#{name}.git"])
     end
 
     # Import repository
@@ -82,10 +84,9 @@ module Gitlab
     def import_repository(storage, name, url)
       # Timeout should be less than 900 ideally, to prevent the memory killer
       # to silently kill the process without knowing we are timing out here.
-      output, status = Popen.popen([gitlab_shell_projects_path, 'import-project',
-                                    storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"])
-      raise Error, output unless status.zero?
-      true
+      cmd = [gitlab_shell_projects_path, 'import-project',
+             storage, "#{name}.git", url, "#{Gitlab.config.gitlab_shell.git_timeout}"]
+      gitlab_shell_fast_execute_raise_error(cmd)
     end
 
     # Fetch remote for repository
@@ -103,9 +104,7 @@ module Gitlab
       args << '--force' if forced
       args << '--no-tags' if no_tags
 
-      output, status = Popen.popen(args)
-      raise Error, output unless status.zero?
-      true
+      gitlab_shell_fast_execute_raise_error(args)
     end
 
     # Move repository
@@ -117,8 +116,8 @@ module Gitlab
     #   mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new")
     #
     def mv_repository(storage, path, new_path)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'mv-project',
-                                   storage, "#{path}.git", "#{new_path}.git"])
+      gitlab_shell_fast_execute([gitlab_shell_projects_path, 'mv-project',
+                                 storage, "#{path}.git", "#{new_path}.git"])
     end
 
     # Fork repository to new namespace
@@ -131,9 +130,9 @@ module Gitlab
     #  fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "randx")
     #
     def fork_repository(forked_from_storage, path, forked_to_storage, fork_namespace)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'fork-project',
-                                   forked_from_storage, "#{path}.git", forked_to_storage,
-                                   fork_namespace])
+      gitlab_shell_fast_execute([gitlab_shell_projects_path, 'fork-project',
+                                 forked_from_storage, "#{path}.git", forked_to_storage,
+                                 fork_namespace])
     end
 
     # Remove repository from file system
@@ -145,8 +144,8 @@ module Gitlab
     #   remove_repository("/path/to/storage", "gitlab/gitlab-ci")
     #
     def remove_repository(storage, name)
-      Gitlab::Utils.system_silent([gitlab_shell_projects_path,
-                                   'rm-project', storage, "#{name}.git"])
+      gitlab_shell_fast_execute([gitlab_shell_projects_path,
+                                 'rm-project', storage, "#{name}.git"])
     end
 
     # Add new key to gitlab-shell
@@ -155,8 +154,8 @@ module Gitlab
     #   add_key("key-42", "sha-rsa ...")
     #
     def add_key(key_id, key_content)
-      Gitlab::Utils.system_silent([gitlab_shell_keys_path,
-                                   'add-key', key_id, self.class.strip_key(key_content)])
+      gitlab_shell_fast_execute([gitlab_shell_keys_path,
+                                 'add-key', key_id, self.class.strip_key(key_content)])
     end
 
     # Batch-add keys to authorized_keys
@@ -175,8 +174,10 @@ module Gitlab
     #   remove_key("key-342", "sha-rsa ...")
     #
     def remove_key(key_id, key_content)
-      Gitlab::Utils.system_silent([gitlab_shell_keys_path,
-                                   'rm-key', key_id, key_content])
+      args = [gitlab_shell_keys_path, 'rm-key', key_id]
+      args << key_content if key_content
+
+      gitlab_shell_fast_execute(args)
     end
 
     # Remove all ssh keys from gitlab shell
@@ -185,7 +186,7 @@ module Gitlab
     #   remove_all_keys
     #
     def remove_all_keys
-      Gitlab::Utils.system_silent([gitlab_shell_keys_path, 'clear'])
+      gitlab_shell_fast_execute([gitlab_shell_keys_path, 'clear'])
     end
 
     # Add empty directory for storing repositories
@@ -267,5 +268,31 @@ module Gitlab
     def gitlab_shell_keys_path
       File.join(gitlab_shell_path, 'bin', 'gitlab-keys')
     end
+
+    private
+
+    def gitlab_shell_fast_execute(cmd)
+      output, status = gitlab_shell_fast_execute_helper(cmd)
+
+      return true if status.zero?
+
+      Rails.logger.error("gitlab-shell failed with error #{status}: #{output}")
+      false
+    end
+
+    def gitlab_shell_fast_execute_raise_error(cmd)
+      output, status = gitlab_shell_fast_execute_helper(cmd)
+
+      raise Error, output unless status.zero?
+      true
+    end
+
+    def gitlab_shell_fast_execute_helper(cmd)
+      vars = ENV.to_h.slice(*GITLAB_SHELL_ENV_VARS)
+
+      # Don't pass along the entire parent environment to prevent gitlab-shell
+      # from wasting I/O by searching through GEM_PATH
+      Bundler.with_original_env { Popen.popen(cmd, nil, vars) }
+    end
   end
 end
diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake
index e3883278886..e9fb6a008b0 100644
--- a/lib/tasks/gitlab/info.rake
+++ b/lib/tasks/gitlab/info.rake
@@ -42,8 +42,7 @@ namespace :gitlab do
       http_clone_url = project.http_url_to_repo
       ssh_clone_url  = project.ssh_url_to_repo
 
-      omniauth_providers = Gitlab.config.omniauth.providers
-      omniauth_providers.map! { |provider| provider['name'] }
+      omniauth_providers = Gitlab.config.omniauth.providers.map { |provider| provider['name'] }
 
       puts ""
       puts "GitLab information".color(:yellow)