Commit c88fe70f authored by James Edwards-Jones's avatar James Edwards-Jones

Only check LFS integrity for first branch in push

parent f7c662c7
---
title: Only check LFS integrity for first ref in a push to avoid timeout
merge_request: 17098
author:
type: performance
......@@ -16,11 +16,11 @@ module Gitlab
lfs_objects_missing: 'LFS objects are missing. Ensure LFS is properly set up or try a manual "git lfs push --all".'
}.freeze
attr_reader :user_access, :project, :skip_authorization, :protocol, :oldrev, :newrev, :ref, :branch_name, :tag_name
attr_reader :user_access, :project, :skip_authorization, :skip_lfs_integrity_check, :protocol, :oldrev, :newrev, :ref, :branch_name, :tag_name
def initialize(
change, user_access:, project:, skip_authorization: false,
protocol:
skip_lfs_integrity_check: false, protocol:
)
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref)
......@@ -28,6 +28,7 @@ module Gitlab
@user_access = user_access
@project = project
@skip_authorization = skip_authorization
@skip_lfs_integrity_check = skip_lfs_integrity_check
@protocol = protocol
end
......@@ -37,7 +38,7 @@ module Gitlab
push_checks
branch_checks
tag_checks
lfs_objects_exist_check
lfs_objects_exist_check unless skip_lfs_integrity_check
commits_check unless skip_commits_check
true
......
......@@ -238,19 +238,22 @@ module Gitlab
changes_list = Gitlab::ChangesList.new(changes)
# Iterate over all changes to find if user allowed all of them to be applied
changes_list.each do |change|
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)
check_single_change_access(change, skip_lfs_integrity_check: !first_change)
end
end
def check_single_change_access(change)
def check_single_change_access(change, skip_lfs_integrity_check: false)
Checks::ChangeAccess.new(
change,
user_access: user_access,
project: project,
skip_authorization: deploy_key?,
skip_lfs_integrity_check: skip_lfs_integrity_check,
protocol: protocol
).exec
end
......
......@@ -13,7 +13,7 @@ module Gitlab
authentication_abilities.include?(:download_code) && user_access.can_do_action?(:download_wiki_code)
end
def check_single_change_access(change)
def check_single_change_access(change, _options = {})
unless user_access.can_do_action?(:create_wiki)
raise UnauthorizedError, ERROR_MESSAGES[:write_to_wiki]
end
......
......@@ -18,8 +18,9 @@ describe Gitlab::GitAccess do
redirected_path: redirected_path)
end
let(:push_access_check) { access.check('git-receive-pack', '_any') }
let(:pull_access_check) { access.check('git-upload-pack', '_any') }
let(:changes) { '_any' }
let(:push_access_check) { access.check('git-receive-pack', changes) }
let(:pull_access_check) { access.check('git-upload-pack', changes) }
describe '#check with single protocols allowed' do
def disable_protocol(protocol)
......@@ -646,6 +647,20 @@ describe Gitlab::GitAccess do
end
end
describe 'check LFS integrity' do
let(:changes) { ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'] }
before do
project.add_developer(user)
end
it 'checks LFS integrity only for first change' do
expect_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).exactly(1).times
push_access_check
end
end
describe '#check_push_access!' do
before do
merge_into_protected_branch
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment