diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-16 21:25:58 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-16 21:25:58 +0300 |
commit | a5f4bba440d7f9ea47046a0a561d49adf0a1e6d4 (patch) | |
tree | fb69158581673816a8cd895f9d352dcb3c678b1e /lib/gitlab/checks | |
parent | d16b2e8639e99961de6ddc93909f3bb5c1445ba1 (diff) |
Add latest changes from gitlab-org/gitlab@14-0-stable-eev14.0.0-rc42
Diffstat (limited to 'lib/gitlab/checks')
-rw-r--r-- | lib/gitlab/checks/base_bulk_checker.rb | 18 | ||||
-rw-r--r-- | lib/gitlab/checks/base_checker.rb | 23 | ||||
-rw-r--r-- | lib/gitlab/checks/base_single_checker.rb | 34 | ||||
-rw-r--r-- | lib/gitlab/checks/branch_check.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/checks/changes_access.rb | 54 | ||||
-rw-r--r-- | lib/gitlab/checks/diff_check.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/checks/lfs_check.rb | 7 | ||||
-rw-r--r-- | lib/gitlab/checks/lfs_integrity.rb | 11 | ||||
-rw-r--r-- | lib/gitlab/checks/matching_merge_request.rb | 60 | ||||
-rw-r--r-- | lib/gitlab/checks/push_check.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/checks/push_file_count_check.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/checks/single_change_access.rb (renamed from lib/gitlab/checks/change_access.rb) | 10 | ||||
-rw-r--r-- | lib/gitlab/checks/snippet_check.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/checks/tag_check.rb | 2 |
14 files changed, 182 insertions, 47 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/matching_merge_request.rb b/lib/gitlab/checks/matching_merge_request.rb index 2635ad04770..e37cbc0442b 100644 --- a/lib/gitlab/checks/matching_merge_request.rb +++ b/lib/gitlab/checks/matching_merge_request.rb @@ -3,22 +3,74 @@ module Gitlab module Checks class MatchingMergeRequest + TOTAL_METRIC = :gitlab_merge_request_match_total + STALE_METRIC = :gitlab_merge_request_match_stale_secondary + def initialize(newrev, branch_name, project) @newrev = newrev @branch_name = branch_name @project = project end - # rubocop: disable CodeReuse/ActiveRecord def match? + if ::Gitlab::Database::LoadBalancing.enable? + # When a user merges a merge request, the following sequence happens: + # + # 1. Sidekiq: MergeService runs and updates the merge request in a locked state. + # 2. Gitaly: The UserMergeBranch RPC runs. + # 3. Gitaly (gitaly-ruby): This RPC calls the pre-receive hook. + # 4. Rails: This hook makes an API request to /api/v4/internal/allowed. + # 5. Rails: This API check does a SQL query for locked merge + # requests with a matching SHA. + # + # Since steps 1 and 5 will happen on different database + # sessions, replication lag could erroneously cause step 5 to + # report no matching merge requests. To avoid this, we check + # the write location to ensure the replica can make this query. + track_session_metrics do + ::Gitlab::Database::LoadBalancing::Sticking.select_valid_host(:project, @project.id) + end + end + + # rubocop: disable CodeReuse/ActiveRecord @project.merge_requests .with_state(:locked) .where(in_progress_merge_commit_sha: @newrev, target_branch: @branch_name) .exists? + # rubocop: enable CodeReuse/ActiveRecord + end + + private + + def track_session_metrics + before = ::Gitlab::Database::LoadBalancing::Session.current.use_primary? + + yield + + after = ::Gitlab::Database::LoadBalancing::Session.current.use_primary? + + increment_attempt_count + + if !before && after + increment_stale_secondary_count + end + end + + def increment_attempt_count + total_counter.increment + end + + def increment_stale_secondary_count + stale_counter.increment + end + + def total_counter + @total_counter ||= ::Gitlab::Metrics.counter(TOTAL_METRIC, 'Total number of merge request match attempts') + end + + def stale_counter + @stale_counter ||= ::Gitlab::Metrics.counter(STALE_METRIC, 'Total number of merge request match attempts with lagging secondary') end - # rubocop: enable CodeReuse/ActiveRecord end end end - -Gitlab::Checks::MatchingMergeRequest.prepend_mod_with('Gitlab::Checks::MatchingMergeRequest') 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.', |