Commit 81b67813 authored by James Edwards-Jones's avatar James Edwards-Jones

Only check LFS integrity for first branch in push

parent 0a5e840b
---
title: Only check LFS integrity for first ref in a push to avoid timeout
merge_request: 17098
author:
type: performance
......@@ -18,11 +18,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)
......@@ -30,6 +30,7 @@ module Gitlab
@user_access = user_access
@project = project
@skip_authorization = skip_authorization
@skip_lfs_integrity_check = skip_lfs_integrity_check
@protocol = protocol
end
......@@ -39,7 +40,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
......
......@@ -253,10 +253,12 @@ module Gitlab
push_size_in_bytes = 0
# 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)
if project.size_limit_enabled?
push_size_in_bytes += EE::Gitlab::Deltas.delta_size_check(change, project.repository)
......@@ -268,12 +270,13 @@ module Gitlab
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
......
......@@ -15,7 +15,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
let(:unprotected_branch) { 'unprotected_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