Add latest changes from gitlab-org/gitlab@master

22 files changed, 177 insertions, 99 deletions

diff --git a/lib/gitlab/checks/base_bulk_checker.rb b/lib/gitlab/checks/base_bulk_checker.rb
new file mode 100644
index 00000000000..46a68fdf485
--- /dev/null
+++ b/lib/gitlab/checks/base_bulk_checker.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Checks
+    class BaseBulkChecker < BaseChecker
+      attr_reader :changes_access
+      delegate(*ChangesAccess::ATTRIBUTES, to: :changes_access)
+
+      def initialize(changes_access)
+        @changes_access = changes_access
+      end
+
+      def validate!
+        raise NotImplementedError
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/checks/base_checker.rb b/lib/gitlab/checks/base_checker.rb
index 68873610408..2b0af7dc4f6 100644
--- a/lib/gitlab/checks/base_checker.rb
+++ b/lib/gitlab/checks/base_checker.rb
@@ -5,39 +5,16 @@ module Gitlab
     class BaseChecker
       include Gitlab::Utils::StrongMemoize
 
-      attr_reader :change_access
-      delegate(*ChangeAccess::ATTRIBUTES, to: :change_access)
-
-      def initialize(change_access)
-        @change_access = change_access
-      end
-
       def validate!
         raise NotImplementedError
       end
 
       private
 
-      def creation?
-        Gitlab::Git.blank_ref?(oldrev)
-      end
-
-      def deletion?
-        Gitlab::Git.blank_ref?(newrev)
-      end
-
-      def update?
-        !creation? && !deletion?
-      end
-
       def updated_from_web?
         protocol == 'web'
       end
 
-      def tag_exists?
-        project.repository.tag_exists?(tag_name)
-      end
-
       def validate_once(resource)
         Gitlab::SafeRequestStore.fetch(cache_key_for_resource(resource)) do
           yield(resource)
diff --git a/lib/gitlab/checks/base_single_checker.rb b/lib/gitlab/checks/base_single_checker.rb
new file mode 100644
index 00000000000..f93902055c9
--- /dev/null
+++ b/lib/gitlab/checks/base_single_checker.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Checks
+    class BaseSingleChecker < BaseChecker
+      attr_reader :change_access
+      delegate(*SingleChangeAccess::ATTRIBUTES, to: :change_access)
+
+      def initialize(change_access)
+        @change_access = change_access
+      end
+
+      private
+
+      def creation?
+        Gitlab::Git.blank_ref?(oldrev)
+      end
+
+      def deletion?
+        Gitlab::Git.blank_ref?(newrev)
+      end
+
+      def update?
+        !creation? && !deletion?
+      end
+
+      def tag_exists?
+        project.repository.tag_exists?(tag_name)
+      end
+    end
+  end
+end
+
+Gitlab::Checks::BaseSingleChecker.prepend_mod_with('Gitlab::Checks::BaseSingleChecker')
diff --git a/lib/gitlab/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb
index a8287a97cc3..a2d74d36b58 100644
--- a/lib/gitlab/checks/branch_check.rb
+++ b/lib/gitlab/checks/branch_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class BranchCheck < BaseChecker
+    class BranchCheck < BaseSingleChecker
       ERROR_MESSAGES = {
         delete_default_branch: 'The default branch of a project cannot be deleted.',
         force_push_protected_branch: 'You are not allowed to force push code to a protected branch on this project.',
diff --git a/lib/gitlab/checks/changes_access.rb b/lib/gitlab/checks/changes_access.rb
new file mode 100644
index 00000000000..4e8b293a3e6
--- /dev/null
+++ b/lib/gitlab/checks/changes_access.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Checks
+    class ChangesAccess
+      ATTRIBUTES = %i[user_access project protocol changes logger].freeze
+
+      attr_reader(*ATTRIBUTES)
+
+      def initialize(
+        changes, user_access:, project:, protocol:, logger:
+      )
+        @changes = changes
+        @user_access = user_access
+        @project = project
+        @protocol = protocol
+        @logger = logger
+      end
+
+      def validate!
+        return if changes.empty?
+
+        single_access_checks!
+
+        logger.log_timed("Running checks for #{changes.length} changes") do
+          bulk_access_checks!
+        end
+
+        true
+      end
+
+      protected
+
+      def single_access_checks!
+        # Iterate over all changes to find if user allowed all of them to be applied
+        changes.each do |change|
+          # If user does not have access to make at least one change, cancel all
+          # push by allowing the exception to bubble up
+          Checks::SingleChangeAccess.new(
+            change,
+            user_access: user_access,
+            project: project,
+            protocol: protocol,
+            logger: logger
+          ).validate!
+        end
+      end
+
+      def bulk_access_checks!
+        Gitlab::Checks::LfsCheck.new(self).validate!
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/checks/diff_check.rb b/lib/gitlab/checks/diff_check.rb
index a05181ab58e..d8f5cec8a4a 100644
--- a/lib/gitlab/checks/diff_check.rb
+++ b/lib/gitlab/checks/diff_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class DiffCheck < BaseChecker
+    class DiffCheck < BaseSingleChecker
       include Gitlab::Utils::StrongMemoize
 
       LOG_MESSAGES = {
diff --git a/lib/gitlab/checks/lfs_check.rb b/lib/gitlab/checks/lfs_check.rb
index 38f0b82c8b4..51013b69755 100644
--- a/lib/gitlab/checks/lfs_check.rb
+++ b/lib/gitlab/checks/lfs_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class LfsCheck < BaseChecker
+    class LfsCheck < BaseBulkChecker
       LOG_MESSAGE = 'Scanning repository for blobs stored in LFS and verifying their files have been uploaded to GitLab...'
       ERROR_MESSAGE = 'LFS objects are missing. Ensure LFS is properly set up or try a manual "git lfs push --all".'
 
@@ -12,11 +12,10 @@ module Gitlab
         return unless Feature.enabled?(:lfs_check, default_enabled: true)
 
         return unless project.lfs_enabled?
-        return if skip_lfs_integrity_check
-        return if deletion?
 
         logger.log_timed(LOG_MESSAGE) do
-          lfs_check = Checks::LfsIntegrity.new(project, newrev, logger.time_left)
+          newrevs = changes.map { |change| change[:newrev] }
+          lfs_check = Checks::LfsIntegrity.new(project, newrevs, logger.time_left)
 
           if lfs_check.objects_missing?
             raise GitAccess::ForbiddenError, ERROR_MESSAGE
diff --git a/lib/gitlab/checks/lfs_integrity.rb b/lib/gitlab/checks/lfs_integrity.rb
index 78952db7a3e..845fb2da925 100644
--- a/lib/gitlab/checks/lfs_integrity.rb
+++ b/lib/gitlab/checks/lfs_integrity.rb
@@ -3,16 +3,19 @@
 module Gitlab
   module Checks
     class LfsIntegrity
-      def initialize(project, newrev, time_left)
+      def initialize(project, newrevs, time_left)
         @project = project
-        @newrev = newrev
+        @newrevs = newrevs
         @time_left = time_left
       end
 
       def objects_missing?
-        return false unless @newrev && @project.lfs_enabled?
+        return false unless @project.lfs_enabled?
 
-        new_lfs_pointers = Gitlab::Git::LfsChanges.new(@project.repository, @newrev)
+        newrevs = @newrevs.reject { |rev| rev.blank? || Gitlab::Git.blank_ref?(rev) }
+        return if newrevs.blank?
+
+        new_lfs_pointers = Gitlab::Git::LfsChanges.new(@project.repository, newrevs)
                                                   .new_pointers(object_limit: ::Gitlab::Git::Repository::REV_LIST_COMMIT_LIMIT, dynamic_timeout: @time_left)
 
         return false unless new_lfs_pointers.present?
diff --git a/lib/gitlab/checks/push_check.rb b/lib/gitlab/checks/push_check.rb
index 47aa25aae4c..50002e00a77 100644
--- a/lib/gitlab/checks/push_check.rb
+++ b/lib/gitlab/checks/push_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class PushCheck < BaseChecker
+    class PushCheck < BaseSingleChecker
       def validate!
         logger.log_timed("Checking if you are allowed to push...") do
           unless can_push?
diff --git a/lib/gitlab/checks/push_file_count_check.rb b/lib/gitlab/checks/push_file_count_check.rb
index 288a7e0d41a..707d4cfbcbe 100644
--- a/lib/gitlab/checks/push_file_count_check.rb
+++ b/lib/gitlab/checks/push_file_count_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class PushFileCountCheck < BaseChecker
+    class PushFileCountCheck < BaseSingleChecker
       attr_reader :repository, :newrev, :limit, :logger
 
       LOG_MESSAGES = {
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/single_change_access.rb
index a2c3de3e775..280b2dd25e2 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/single_change_access.rb
@@ -2,23 +2,22 @@
 
 module Gitlab
   module Checks
-    class ChangeAccess
+    class SingleChangeAccess
       ATTRIBUTES = %i[user_access project skip_authorization
-                      skip_lfs_integrity_check protocol oldrev newrev ref
+                      protocol oldrev newrev ref
                       branch_name tag_name logger commits].freeze
 
       attr_reader(*ATTRIBUTES)
 
       def initialize(
         change, user_access:, project:,
-        skip_lfs_integrity_check: false, protocol:, logger:
+        protocol:, logger:
       )
         @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
         @branch_name = Gitlab::Git.branch_name(@ref)
         @tag_name = Gitlab::Git.tag_name(@ref)
         @user_access = user_access
         @project = project
-        @skip_lfs_integrity_check = skip_lfs_integrity_check
         @protocol = protocol
 
         @logger = logger
@@ -44,7 +43,6 @@ module Gitlab
         Gitlab::Checks::PushCheck.new(self).validate!
         Gitlab::Checks::BranchCheck.new(self).validate!
         Gitlab::Checks::TagCheck.new(self).validate!
-        Gitlab::Checks::LfsCheck.new(self).validate!
       end
 
       def commits_check
@@ -54,4 +52,4 @@ module Gitlab
   end
 end
 
-Gitlab::Checks::ChangeAccess.prepend_mod_with('Gitlab::Checks::ChangeAccess')
+Gitlab::Checks::SingleChangeAccess.prepend_mod_with('Gitlab::Checks::SingleChangeAccess')
diff --git a/lib/gitlab/checks/snippet_check.rb b/lib/gitlab/checks/snippet_check.rb
index d5efbfcc5bc..43168600ec9 100644
--- a/lib/gitlab/checks/snippet_check.rb
+++ b/lib/gitlab/checks/snippet_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class SnippetCheck < BaseChecker
+    class SnippetCheck < BaseSingleChecker
       ERROR_MESSAGES = {
         create_delete_branch: 'You can not create or delete branches.'
       }.freeze
diff --git a/lib/gitlab/checks/tag_check.rb b/lib/gitlab/checks/tag_check.rb
index a47e55cb160..a45db85301a 100644
--- a/lib/gitlab/checks/tag_check.rb
+++ b/lib/gitlab/checks/tag_check.rb
@@ -2,7 +2,7 @@
 
 module Gitlab
   module Checks
-    class TagCheck < BaseChecker
+    class TagCheck < BaseSingleChecker
       ERROR_MESSAGES = {
         change_existing_tags: 'You are not allowed to change existing tags on this project.',
         update_protected_tag: 'Protected tags cannot be updated.',
diff --git a/lib/gitlab/git/lfs_changes.rb b/lib/gitlab/git/lfs_changes.rb
index a8d1ea08275..6c4191ce25b 100644
--- a/lib/gitlab/git/lfs_changes.rb
+++ b/lib/gitlab/git/lfs_changes.rb
@@ -3,13 +3,13 @@
 module Gitlab
   module Git
     class LfsChanges
-      def initialize(repository, newrev = nil)
+      def initialize(repository, newrevs = nil)
         @repository = repository
-        @newrev = newrev
+        @newrevs = newrevs
       end
 
       def new_pointers(object_limit: nil, not_in: nil, dynamic_timeout: nil)
-        @repository.gitaly_blob_client.get_new_lfs_pointers(@newrev, object_limit, not_in, dynamic_timeout)
+        @repository.gitaly_blob_client.get_new_lfs_pointers(@newrevs, object_limit, not_in, dynamic_timeout)
       end
 
       def all_pointers
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index b5e7220889e..b2a65d9f2d8 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -334,23 +334,15 @@ module Gitlab
         # clear stale lock files.
         project.repository.clean_stale_repository_files if project.present?
 
-        # Iterate over all changes to find if user allowed all of them to be applied
-        changes_list.each.with_index do |change, index|
-          first_change = index == 0
-
-          # If user does not have access to make at least one change, cancel all
-          # push by allowing the exception to bubble up
-          check_single_change_access(change, skip_lfs_integrity_check: !first_change)
-        end
+        check_access!
       end
     end
 
-    def check_single_change_access(change, skip_lfs_integrity_check: false)
-      Checks::ChangeAccess.new(
-        change,
+    def check_access!
+      Checks::ChangesAccess.new(
+        changes_list.changes,
         user_access: user_access,
         project: project,
-        skip_lfs_integrity_check: skip_lfs_integrity_check,
         protocol: protocol,
         logger: logger
       ).validate!
diff --git a/lib/gitlab/git_access_snippet.rb b/lib/gitlab/git_access_snippet.rb
index 9a431dc7088..4d87b91764a 100644
--- a/lib/gitlab/git_access_snippet.rb
+++ b/lib/gitlab/git_access_snippet.rb
@@ -109,20 +109,18 @@ module Gitlab
       end
 
       check_size_before_push!
+      check_access!
+      check_push_size!
+    end
 
+    override :check_access!
+    def check_access!
       changes_list.each do |change|
         # If user does not have access to make at least one change, cancel all
         # push by allowing the exception to bubble up
-        check_single_change_access(change)
+        Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, root_ref: snippet.repository.root_ref, logger: logger).validate!
+        Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit, logger: logger).validate!
       end
-
-      check_push_size!
-    end
-
-    override :check_single_change_access
-    def check_single_change_access(change, _skip_lfs_integrity_check: false)
-      Checks::SnippetCheck.new(change, default_branch: snippet.default_branch, root_ref: snippet.repository.root_ref, logger: logger).validate!
-      Checks::PushFileCountCheck.new(change, repository: repository, limit: Snippet.max_file_limit, logger: logger).validate!
     rescue Checks::TimedLogger::TimeoutError
       raise TimeoutError, logger.full_message
     end
diff --git a/lib/gitlab/gitaly_client/blob_service.rb b/lib/gitlab/gitaly_client/blob_service.rb
index affd3986381..e4c8dc150a5 100644
--- a/lib/gitlab/gitaly_client/blob_service.rb
+++ b/lib/gitlab/gitaly_client/blob_service.rb
@@ -77,8 +77,8 @@ module Gitlab
         map_blob_types(response)
       end
 
-      def get_new_lfs_pointers(revision, limit, not_in, dynamic_timeout = nil)
-        request, rpc = create_new_lfs_pointers_request(revision, limit, not_in)
+      def get_new_lfs_pointers(revisions, limit, not_in, dynamic_timeout = nil)
+        request, rpc = create_new_lfs_pointers_request(revisions, limit, not_in)
 
         timeout =
           if dynamic_timeout
@@ -109,7 +109,7 @@ module Gitlab
 
       private
 
-      def create_new_lfs_pointers_request(revision, limit, not_in)
+      def create_new_lfs_pointers_request(revisions, limit, not_in)
         # If the check happens for a change which is using a quarantine
         # environment for incoming objects, then we can avoid doing the
         # necessary graph walk to detect only new LFS pointers and instead scan
@@ -126,7 +126,7 @@ module Gitlab
 
           [request, :list_all_lfs_pointers]
         else
-          revisions = [revision]
+          revisions = Array.wrap(revisions)
           revisions += if not_in.nil? || not_in == :all
                          ["--not", "--all"]
                        else
diff --git a/lib/gitlab/graphql/authorize/authorize_resource.rb b/lib/gitlab/graphql/authorize/authorize_resource.rb
index 4d575b964e5..dc49c806398 100644
--- a/lib/gitlab/graphql/authorize/authorize_resource.rb
+++ b/lib/gitlab/graphql/authorize/authorize_resource.rb
@@ -51,14 +51,11 @@ module Gitlab
           object
         end
 
-        # authorizes the object using the current class authorization.
         def authorize!(object)
           raise_resource_not_available_error! unless authorized_resource?(object)
         end
 
         def authorized_resource?(object)
-          # Sanity check. We don't want to accidentally allow a developer to authorize
-          # without first adding permissions to authorize against
           raise ConfigurationError, "#{self.class.name} has no authorizations" if self.class.authorization.none?
 
           self.class.authorization.ok?(object, current_user)
diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb
index 7bd55cce363..4c4942c12d5 100644
--- a/lib/gitlab/metrics.rb
+++ b/lib/gitlab/metrics.rb
@@ -16,6 +16,10 @@ module Gitlab
       @error
     end
 
+    def self.record_duration_for_status?(status)
+      status.to_i.between?(200, 499)
+    end
+
     # Tracks an event.
     #
     # See `Gitlab::Metrics::Transaction#add_event` for more details.
diff --git a/lib/gitlab/metrics/requests_rack_middleware.rb b/lib/gitlab/metrics/requests_rack_middleware.rb
index bf2af57759a..b99261b5c4d 100644
--- a/lib/gitlab/metrics/requests_rack_middleware.rb
+++ b/lib/gitlab/metrics/requests_rack_middleware.rb
@@ -66,16 +66,16 @@ module Gitlab
       def call(env)
         method = env['REQUEST_METHOD'].downcase
         method = 'INVALID' unless HTTP_METHODS.key?(method)
-        started = Time.now.to_f
+        started = Gitlab::Metrics::System.monotonic_time
         health_endpoint = health_endpoint?(env['PATH_INFO'])
         status = 'undefined'
 
         begin
           status, headers, body = @app.call(env)
 
-          elapsed = Time.now.to_f - started
+          elapsed = Gitlab::Metrics::System.monotonic_time - started
 
-          unless health_endpoint
+          if !health_endpoint && Gitlab::Metrics.record_duration_for_status?(status)
             RequestsRackMiddleware.http_request_duration_seconds.observe({ method: method }, elapsed)
           end
 
diff --git a/lib/gitlab/metrics/transaction.rb b/lib/gitlab/metrics/transaction.rb
index 3ebafb5c5e4..97cc8bed564 100644
--- a/lib/gitlab/metrics/transaction.rb
+++ b/lib/gitlab/metrics/transaction.rb
@@ -13,8 +13,6 @@ module Gitlab
 
       THREAD_KEY = :_gitlab_metrics_transaction
 
-      SMALL_BUCKETS = [0.1, 0.25, 0.5, 1.0, 2.5, 5.0].freeze
-
       # The series to store events (e.g. Git pushes) in.
       EVENT_SERIES = 'events'
 
@@ -39,29 +37,10 @@ module Gitlab
 
       def initialize
         @methods = {}
-
-        @started_at = nil
-        @finished_at = nil
-      end
-
-      def duration
-        @finished_at ? (@finished_at - @started_at) : 0.0
       end
 
       def run
-        Thread.current[THREAD_KEY] = self
-
-        @started_at = System.monotonic_time
-
-        yield
-      ensure
-        @finished_at = System.monotonic_time
-
-        observe(:gitlab_transaction_duration_seconds, duration) do
-          buckets SMALL_BUCKETS
-        end
-
-        Thread.current[THREAD_KEY] = nil
+        raise NotImplementedError
       end
 
       # Tracks a business level event
diff --git a/lib/gitlab/metrics/web_transaction.rb b/lib/gitlab/metrics/web_transaction.rb
index ee9e6f449d3..3ebfcc43b0b 100644
--- a/lib/gitlab/metrics/web_transaction.rb
+++ b/lib/gitlab/metrics/web_transaction.rb
@@ -6,12 +6,29 @@ module Gitlab
       CONTROLLER_KEY = 'action_controller.instance'
       ENDPOINT_KEY = 'api.endpoint'
       ALLOWED_SUFFIXES = Set.new(%w[json js atom rss xml zip])
+      SMALL_BUCKETS = [0.1, 0.25, 0.5, 1.0, 2.5, 5.0].freeze
 
       def initialize(env)
         super()
         @env = env
       end
 
+      def run
+        Thread.current[THREAD_KEY] = self
+
+        started_at = System.monotonic_time
+
+        status, _, _ = retval = yield
+
+        finished_at = System.monotonic_time
+        duration = finished_at - started_at
+        record_duration_if_needed(status, duration)
+
+        retval
+      ensure
+        Thread.current[THREAD_KEY] = nil
+      end
+
       def labels
         return @labels if @labels
 
@@ -27,6 +44,14 @@ module Gitlab
 
       private
 
+      def record_duration_if_needed(status, duration)
+        return unless Gitlab::Metrics.record_duration_for_status?(status)
+
+        observe(:gitlab_transaction_duration_seconds, duration) do
+          buckets SMALL_BUCKETS
+        end
+      end
+
       def labels_from_controller
         controller = @env[CONTROLLER_KEY]