kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Calculating repository checksums executed by Gitaly

Browse source 
OPT_OUT status has been removed, and alternative implementation removed.
Also checks if the repository exists before executing the checksum RPC
to guard against NotFound errors.

Closes gitlab-org/gitaly#1105
This commit is contained in:
Zeger-Jan van de Weg 2018-05-11 16:44:38 +02:00
parent 14507fd181
commit 18a8eb96b3
No known key found for this signature in database
GPG key ID: 65F6A8D64A88ABAC
3 changed files with 39 additions and 90 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
changelogs/unreleased/zj-calculate-checksum-mandator.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Remove shellout implementation for Repository checksums
merge_request:
author:
type: other

44
lib/gitlab/git/repository.rb
View file

@ -1576,14 +1576,12 @@ module Gitlab
end
def checksum
gitaly_migrate(:calculate_checksum,
status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
if is_enabled
gitaly_repository_client.calculate_checksum
else
calculate_checksum_by_shelling_out
end
end
# The exists? RPC is much cheaper, so we perform this request first
raise NoRepository, "Repository does not exists" unless exists?
gitaly_repository_client.calculate_checksum
rescue GRPC::NotFound
raise NoRepository # Guard against data races.
end
private
@ -2498,36 +2496,6 @@ module Gitlab
rev_parse_target(ref).oid
end
def calculate_checksum_by_shelling_out
raise NoRepository unless exists?
args = %W(--git-dir=#{path} show-ref --heads --tags)
output, status = run_git(args)
if status.nil? || !status.zero?
# Non-valid git repositories return 128 as the status code and an error output
raise InvalidRepository if status == 128 && output.to_s.downcase =~ /not a git repository/
# Empty repositories returns with a non-zero status and an empty output.
raise ChecksumError, output unless output.blank?
return EMPTY_REPOSITORY_CHECKSUM
end
refs = output.split("\n")
result = refs.inject(nil) do |checksum, ref|
value = Digest::SHA1.hexdigest(ref).hex
if checksum.nil?
value
else
checksum ^ value
end
end
result.to_s(16)
end
def build_git_cmd(*args)
object_directories = alternate_object_directories.join(File::PATH_SEPARATOR)

80
spec/lib/gitlab/git/repository_spec.rb
View file

@ -2247,66 +2247,42 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
describe '#checksum' do
shared_examples 'calculating checksum' do
it 'calculates the checksum for non-empty repo' do
expect(repository.checksum).to eq '54f21be4c32c02f6788d72207fa03ad3bce725e4'
end
it 'returns 0000000000000000000000000000000000000000 for an empty repo' do
FileUtils.rm_rf(File.join(storage_path, 'empty-repo.git'))
system(git_env, *%W(#{Gitlab.config.git.bin_path} init --bare empty-repo.git),
chdir: storage_path,
out: '/dev/null',
err: '/dev/null')
empty_repo = described_class.new('default', 'empty-repo.git', '')
expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000'
end
it 'raises Gitlab::Git::Repository::InvalidRepository error for non-valid git repo' do
FileUtils.rm_rf(File.join(storage_path, 'non-valid.git'))
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --bare #{TEST_REPO_PATH} non-valid.git),
chdir: SEED_STORAGE_PATH,
out: '/dev/null',
err: '/dev/null')
File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0)
non_valid = described_class.new('default', 'non-valid.git', '')
expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository)
end
it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do
broken_repo = described_class.new('default', 'a/path.git', '')
expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository)
end
it 'calculates the checksum for non-empty repo' do
expect(repository.checksum).to eq '54f21be4c32c02f6788d72207fa03ad3bce725e4'
end
context 'when calculate_checksum Gitaly feature is enabled' do
it_behaves_like 'calculating checksum'
it 'returns 0000000000000000000000000000000000000000 for an empty repo' do
FileUtils.rm_rf(File.join(storage_path, 'empty-repo.git'))
system(git_env, *%W(#{Gitlab.config.git.bin_path} init --bare empty-repo.git),
chdir: storage_path,
out: '/dev/null',
err: '/dev/null')
empty_repo = described_class.new('default', 'empty-repo.git', '')
expect(empty_repo.checksum).to eq '0000000000000000000000000000000000000000'
end
context 'when calculate_checksum Gitaly feature is disabled', :disable_gitaly do
it_behaves_like 'calculating checksum'
it 'raises Gitlab::Git::Repository::InvalidRepository error for non-valid git repo' do
FileUtils.rm_rf(File.join(storage_path, 'non-valid.git'))
describe 'when storage is broken', :broken_storage do
it 'raises a storage exception when storage is not available' do
broken_repo = described_class.new('broken', 'a/path.git', '')
system(git_env, *%W(#{Gitlab.config.git.bin_path} clone --bare #{TEST_REPO_PATH} non-valid.git),
chdir: SEED_STORAGE_PATH,
out: '/dev/null',
err: '/dev/null')
expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible)
end
end
File.truncate(File.join(storage_path, 'non-valid.git/HEAD'), 0)
it "raises a Gitlab::Git::Repository::Failure error if the `popen` call to git returns a non-zero exit code" do
allow(repository).to receive(:popen).and_return(['output', nil])
non_valid = described_class.new('default', 'non-valid.git', '')
expect { repository.checksum }.to raise_error Gitlab::Git::Repository::ChecksumError
end
expect { non_valid.checksum }.to raise_error(Gitlab::Git::Repository::InvalidRepository)
end
it 'raises Gitlab::Git::Repository::NoRepository error when there is no repo' do
broken_repo = described_class.new('default', 'a/path.git', '')
expect { broken_repo.checksum }.to raise_error(Gitlab::Git::Repository::NoRepository)
end
end