diff options
| -rw-r--r-- | config/initializers/1_settings.rb | 6 | ||||
| -rw-r--r-- | config/initializers/6_validations.rb | 10 | ||||
| -rw-r--r-- | lib/gitlab/cycle_analytics/summary/commit.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/git/repository.rb | 22 | ||||
| -rw-r--r-- | lib/gitlab/git/storage/checker.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/git/storage/circuit_breaker.rb | 15 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client.rb | 7 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client/storage_settings.rb | 26 | ||||
| -rw-r--r-- | lib/gitlab/health_checks/fs_shards_check.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/temporarily_allow.rb | 42 | ||||
| -rw-r--r-- | spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb | 12 | ||||
| -rw-r--r-- | spec/lib/gitlab/checks/lfs_integrity_spec.rb | 4 | ||||
| -rw-r--r-- | spec/lib/gitlab/conflict/file_spec.rb | 2 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 133 | ||||
| -rw-r--r-- | spec/support/gitaly.rb | 5 |
15 files changed, 226 insertions, 68 deletions
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index dd36700964a..a0e3ab0d343 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -391,8 +391,10 @@ repositories_storages = Settings.repositories.storages.values repository_downloads_path = Settings.gitlab['repository_downloads_path'].to_s.gsub(%r{/$}, '') repository_downloads_full_path = File.expand_path(repository_downloads_path, Settings.gitlab['user_home']) -if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs.legacy_disk_path.gsub(%r{/$}, '')) } - Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') +Gitlab::GitalyClient::StorageSettings.allow_disk_access do + if repository_downloads_path.blank? || repositories_storages.any? { |rs| [repository_downloads_path, repository_downloads_full_path].include?(rs.legacy_disk_path.gsub(%r{/$}, '')) } + Settings.gitlab['repository_downloads_path'] = File.join(Settings.shared['path'], 'cache/archive') + end end # diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index 89aabe530fe..362a23164ab 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -38,10 +38,12 @@ def validate_storages_config end def validate_storages_paths - Gitlab.config.repositories.storages.each do |name, repository_storage| - parent_name, _parent_path = find_parent_path(name, repository_storage.legacy_disk_path) - if parent_name - storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.each do |name, repository_storage| + parent_name, _parent_path = find_parent_path(name, repository_storage.legacy_disk_path) + if parent_name + storage_validation_error("#{name} is a nested path of #{parent_name}. Nested paths are not supported for repository storages") + end end end end diff --git a/lib/gitlab/cycle_analytics/summary/commit.rb b/lib/gitlab/cycle_analytics/summary/commit.rb index bea78862757..0a88e052f60 100644 --- a/lib/gitlab/cycle_analytics/summary/commit.rb +++ b/lib/gitlab/cycle_analytics/summary/commit.rb @@ -7,7 +7,9 @@ module Gitlab end def value - @value ||= count_commits + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + @value ||= count_commits + end end private diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 1a21625a322..4cbf20bfe76 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1185,15 +1185,17 @@ module Gitlab end def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) - with_repo_branch_commit(source_repository, source_branch_name) do |commit| - break unless commit - - Gitlab::Git::Compare.new( - self, - target_branch_name, - commit.sha, - straight: straight - ) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + with_repo_branch_commit(source_repository, source_branch_name) do |commit| + break unless commit + + Gitlab::Git::Compare.new( + self, + target_branch_name, + commit.sha, + straight: straight + ) + end end end @@ -1455,7 +1457,7 @@ module Gitlab gitaly_repository_client.cleanup if is_enabled && exists? end rescue Gitlab::Git::CommandError => e # Don't fail if we can't cleanup - Rails.logger.error("Unable to clean repository on storage #{storage} with path #{path}: #{e.message}") + Rails.logger.error("Unable to clean repository on storage #{storage} with relative path #{relative_path}: #{e.message}") Gitlab::Metrics.counter( :failed_repository_cleanup_total, 'Number of failed repository cleanup events' diff --git a/lib/gitlab/git/storage/checker.rb b/lib/gitlab/git/storage/checker.rb index 2f611cef37b..391f0d70583 100644 --- a/lib/gitlab/git/storage/checker.rb +++ b/lib/gitlab/git/storage/checker.rb @@ -35,7 +35,7 @@ module Gitlab def initialize(storage, logger = Rails.logger) @storage = storage config = Gitlab.config.repositories.storages[@storage] - @storage_path = config.legacy_disk_path + @storage_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { config.legacy_disk_path } @logger = logger @hostname = Gitlab::Environment.hostname diff --git a/lib/gitlab/git/storage/circuit_breaker.rb b/lib/gitlab/git/storage/circuit_breaker.rb index e35054466ff..62427ac9cc4 100644 --- a/lib/gitlab/git/storage/circuit_breaker.rb +++ b/lib/gitlab/git/storage/circuit_breaker.rb @@ -22,13 +22,14 @@ module Gitlab def self.build(storage, hostname = Gitlab::Environment.hostname) config = Gitlab.config.repositories.storages[storage] - - if !config.present? - NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured")) - elsif !config.legacy_disk_path.present? - NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured")) - else - new(storage, hostname) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + if !config.present? + NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured")) + elsif !config.legacy_disk_path.present? + NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured")) + else + new(storage, hostname) + end end end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 0abae70c443..550294916a4 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -33,6 +33,11 @@ module Gitlab MAXIMUM_GITALY_CALLS = 35 CLIENT_NAME = (Sidekiq.server? ? 'gitlab-sidekiq' : 'gitlab-web').freeze + # We have a mechanism to let GitLab automatically opt in to all Gitaly + # features. We want to be able to exclude some features from automatic + # opt-in. That is what EXPLICIT_OPT_IN_REQUIRED is for. + EXPLICIT_OPT_IN_REQUIRED = [Gitlab::GitalyClient::StorageSettings::DISK_ACCESS_DENIED_FLAG].freeze + MUTEX = Mutex.new class << self @@ -234,7 +239,7 @@ module Gitlab when MigrationStatus::OPT_OUT true when MigrationStatus::OPT_IN - opt_into_all_features? + opt_into_all_features? && !EXPLICIT_OPT_IN_REQUIRED.include?(feature_name) else false end diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb index 9a576e463e3..02fcb413abd 100644 --- a/lib/gitlab/gitaly_client/storage_settings.rb +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -4,6 +4,8 @@ module Gitlab # where production code (app, config, db, lib) touches Git repositories # directly. class StorageSettings + extend Gitlab::TemporarilyAllow + DirectPathAccessError = Class.new(StandardError) InvalidConfigurationError = Class.new(StandardError) @@ -17,7 +19,21 @@ module Gitlab # This class will give easily recognizable NoMethodErrors Deprecated = Class.new - attr_reader :legacy_disk_path + MUTEX = Mutex.new + + DISK_ACCESS_DENIED_FLAG = :deny_disk_access + ALLOW_KEY = :allow_disk_access + + # If your code needs this method then your code needs to be fixed. + def self.allow_disk_access + temporarily_allow(ALLOW_KEY) { yield } + end + + def self.disk_access_denied? + !temporarily_allowed?(ALLOW_KEY) && GitalyClient.feature_enabled?(DISK_ACCESS_DENIED_FLAG) + rescue + false # Err on the side of caution, don't break gitlab for people + end def initialize(storage) raise InvalidConfigurationError, "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash) @@ -34,6 +50,14 @@ module Gitlab @hash.fetch(:gitaly_address) end + def legacy_disk_path + if self.class.disk_access_denied? + raise DirectPathAccessError, "git disk access denied via the gitaly_#{DISK_ACCESS_DENIED_FLAG} feature" + end + + @legacy_disk_path + end + private def method_missing(m, *args, &block) diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb index 6e554383270..fcbf266b80b 100644 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ b/lib/gitlab/health_checks/fs_shards_check.rb @@ -77,7 +77,9 @@ module Gitlab end def storage_path(storage_name) - storages_paths[storage_name]&.legacy_disk_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + storages_paths[storage_name]&.legacy_disk_path + end end # All below test methods use shell commands to perform actions on storage volumes. diff --git a/lib/gitlab/temporarily_allow.rb b/lib/gitlab/temporarily_allow.rb new file mode 100644 index 00000000000..880e55f71df --- /dev/null +++ b/lib/gitlab/temporarily_allow.rb @@ -0,0 +1,42 @@ +module Gitlab + module TemporarilyAllow + TEMPORARILY_ALLOW_MUTEX = Mutex.new + + def temporarily_allow(key) + temporarily_allow_add(key, 1) + yield + ensure + temporarily_allow_add(key, -1) + end + + def temporarily_allowed?(key) + if RequestStore.active? + temporarily_allow_request_store[key] > 0 + else + TEMPORARILY_ALLOW_MUTEX.synchronize do + temporarily_allow_ivar[key] > 0 + end + end + end + + private + + def temporarily_allow_ivar + @temporarily_allow ||= Hash.new(0) + end + + def temporarily_allow_request_store + RequestStore[:temporarily_allow] ||= Hash.new(0) + end + + def temporarily_allow_add(key, value) + if RequestStore.active? + temporarily_allow_request_store[key] += value + else + TEMPORARILY_ALLOW_MUTEX.synchronize do + temporarily_allow_ivar[key] += value + end + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb index 007e93c1db6..211e3aaa94b 100644 --- a/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb +++ b/spec/lib/gitlab/background_migration/deserialize_merge_request_diffs_and_commits_spec.rb @@ -299,7 +299,11 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { project.repository.commit(merge_request_diff.head_commit_sha) } let(:expected_commits) { commits } - let(:diffs) { first_commit.rugged_diff_from_parent.patches } + let(:diffs) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + first_commit.rugged_diff_from_parent.patches + end + end let(:expected_diffs) { [] } include_examples 'updated MR diff' @@ -309,7 +313,11 @@ describe Gitlab::BackgroundMigration::DeserializeMergeRequestDiffsAndCommits, :m let(:commits) { merge_request_diff.commits.map(&:to_hash) } let(:first_commit) { project.repository.commit(merge_request_diff.head_commit_sha) } let(:expected_commits) { commits } - let(:diffs) { first_commit.rugged_diff_from_parent.deltas } + let(:diffs) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + first_commit.rugged_diff_from_parent.deltas + end + end let(:expected_diffs) { [] } include_examples 'updated MR diff' diff --git a/spec/lib/gitlab/checks/lfs_integrity_spec.rb b/spec/lib/gitlab/checks/lfs_integrity_spec.rb index 7201e4f7bf6..ec22e3a198e 100644 --- a/spec/lib/gitlab/checks/lfs_integrity_spec.rb +++ b/spec/lib/gitlab/checks/lfs_integrity_spec.rb @@ -6,7 +6,9 @@ describe Gitlab::Checks::LfsIntegrity do let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:newrev) do - operations = BareRepoOperations.new(repository.path) + operations = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + BareRepoOperations.new(repository.path) + end # Create a commit not pointed at by any ref to emulate being in the # pre-receive hook so that `--not --all` returns some objects diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb index 92792144429..5b343920429 100644 --- a/spec/lib/gitlab/conflict/file_spec.rb +++ b/spec/lib/gitlab/conflict/file_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::Conflict::File do let(:project) { create(:project, :repository) } let(:repository) { project.repository } - let(:rugged) { repository.rugged } + let(:rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } } let(:their_commit) { rugged.branches['conflict-start'].target } let(:our_commit) { rugged.branches['conflict-resolvable'].target } let(:merge_request) { create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) } diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index dd5c498706d..7a9621d9c78 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -114,7 +114,9 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'raises a no repository exception when there is no repo' do broken_repo = described_class.new('default', 'a/path.git', '') - expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Repository::NoRepository) + expect do + Gitlab::GitalyClient::StorageSettings.allow_disk_access { broken_repo.rugged } + end.to raise_error(Gitlab::Git::Repository::NoRepository) end describe 'alternates keyword argument' do @@ -124,9 +126,9 @@ describe Gitlab::Git::Repository, seed_helper: true do end it "is passed an empty array" do - expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: []) + expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: []) - repository.rugged + repository_rugged end end @@ -142,10 +144,10 @@ describe Gitlab::Git::Repository, seed_helper: true do end it "is passed the relative object dir envvars after being converted to absolute ones" do - alternates = %w[foo bar baz].map { |d| File.join(repository.path, './objects', d) } - expect(Rugged::Repository).to receive(:new).with(repository.path, alternates: alternates) + alternates = %w[foo bar baz].map { |d| File.join(repository_path, './objects', d) } + expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: alternates) - repository.rugged + repository_rugged end end end @@ -156,16 +158,22 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:feature) { 'feature' } let(:feature2) { 'feature2' } + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + it "returns 'master' when master exists" do expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, master]) expect(repository.discover_default_branch).to eq('master') end it "returns non-master when master exists but default branch is set to something else" do - File.write(File.join(repository.path, 'HEAD'), 'ref: refs/heads/feature') + File.write(File.join(repository_path, 'HEAD'), 'ref: refs/heads/feature') expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, master]) expect(repository.discover_default_branch).to eq('feature') - File.write(File.join(repository.path, 'HEAD'), 'ref: refs/heads/master') + File.write(File.join(repository_path, 'HEAD'), 'ref: refs/heads/master') end it "returns a non-master branch when only one exists" do @@ -364,6 +372,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end context '#submodules' do + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context 'where repo has submodules' do @@ -474,8 +488,8 @@ describe Gitlab::Git::Repository, seed_helper: true do # Sanity check expect(repository.has_local_branches?).to eq(true) - FileUtils.rm_rf(File.join(repository.path, 'packed-refs')) - heads_dir = File.join(repository.path, 'refs/heads') + FileUtils.rm_rf(File.join(repository_path, 'packed-refs')) + heads_dir = File.join(repository_path, 'refs/heads') FileUtils.rm_rf(heads_dir) FileUtils.mkdir_p(heads_dir) @@ -516,10 +530,10 @@ describe Gitlab::Git::Repository, seed_helper: true do branch_name = "to-be-deleted-soon" repository.create_branch(branch_name) - expect(repository.rugged.branches[branch_name]).not_to be_nil + expect(repository_rugged.branches[branch_name]).not_to be_nil repository.delete_branch(branch_name) - expect(repository.rugged.branches[branch_name]).to be_nil + expect(repository_rugged.branches[branch_name]).to be_nil end context "when branch does not exist" do @@ -577,6 +591,12 @@ describe Gitlab::Git::Repository, seed_helper: true do shared_examples 'deleting refs' do let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + def repo_rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repo.rugged + end + end + after do ensure_seeds end @@ -584,7 +604,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'deletes the ref' do repo.delete_refs('refs/heads/feature') - expect(repo.rugged.references['refs/heads/feature']).to be_nil + expect(repo_rugged.references['refs/heads/feature']).to be_nil end it 'deletes all refs' do @@ -592,7 +612,7 @@ describe Gitlab::Git::Repository, seed_helper: true do repo.delete_refs(*refs) refs.each do |ref| - expect(repo.rugged.references[ref]).to be_nil + expect(repo_rugged.references[ref]).to be_nil end end @@ -615,7 +635,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#branch_names_contains_sha' do - let(:head_id) { repository.rugged.head.target.oid } + let(:head_id) { repository_rugged.head.target.oid } let(:new_branch) { head_id } let(:utf8_branch) { 'branch-é' } @@ -699,7 +719,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'fetches a repository as a mirror remote' do subject - expect(refs(new_repository.path)).to eq(refs(repository.path)) + expect(refs(new_repository_path)).to eq(refs(repository_path)) end context 'with keep-around refs' do @@ -708,15 +728,15 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } before do - repository.rugged.references.create(keep_around_ref, sha, force: true) - repository.rugged.references.create(tmp_ref, sha, force: true) + repository_rugged.references.create(keep_around_ref, sha, force: true) + repository_rugged.references.create(tmp_ref, sha, force: true) end it 'includes the temporary and keep-around refs' do subject - expect(refs(new_repository.path)).to include(keep_around_ref) - expect(refs(new_repository.path)).to include(tmp_ref) + expect(refs(new_repository_path)).to include(keep_around_ref) + expect(refs(new_repository_path)).to include(tmp_ref) end end end @@ -728,6 +748,12 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with gitaly enabled', :skip_gitaly_mock do it_behaves_like 'repository mirror fecthing' end + + def new_repository_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + new_repository.path + end + end end describe '#remote_tags' do @@ -739,10 +765,17 @@ describe Gitlab::Git::Repository, seed_helper: true do Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') end + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + subject { repository.remote_tags(remote_name) } before do - repository.add_remote(remote_name, remote_repository.path) + remote_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { remote_repository.path } + repository.add_remote(remote_name, remote_repository_path) remote_repository.add_tag(tag_name, user: user, target: target_commit_id) end @@ -975,8 +1008,10 @@ describe Gitlab::Git::Repository, seed_helper: true do let(:options) { { ref: 'master', path: ['PROCESS.md', 'README.md'] } } def commit_files(commit) - commit.rugged_diff_from_parent.deltas.flat_map do |delta| - [delta.old_file[:path], delta.new_file[:path]].uniq.compact + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + commit.rugged_diff_from_parent.deltas.flat_map do |delta| + [delta.old_file[:path], delta.new_file[:path]].uniq.compact + end end end @@ -1019,6 +1054,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe "#rugged_commits_between" do + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + context 'two SHAs' do let(:first_sha) { 'b0e52af38d7ea43cf41d8a6f2471351ac036d6c9' } let(:second_sha) { '0e50ec4d3c7ce42ab74dda1d422cb2cbffe1e326' } @@ -1363,7 +1404,7 @@ describe Gitlab::Git::Repository, seed_helper: true do allow(ref).to receive(:target) { raise Rugged::ReferenceError } branches = double() allow(branches).to receive(:each) { [ref].each } - allow(repository.rugged).to receive(:branches) { branches } + allow(repository_rugged).to receive(:branches) { branches } expect(subject).to be_empty end @@ -1661,6 +1702,12 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#batch_existence' do let(:refs) { ['deadbeef', SeedRepo::RubyBlob::ID, '909e6157199'] } + around do |example| + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + example.run + end + end + it 'returns existing refs back' do result = repository.batch_existence(refs) @@ -1840,7 +1887,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'when the branch exists' do context 'when the commit does not exist locally' do let(:source_branch) { 'new-branch-for-fetch-source-branch' } - let(:source_rugged) { source_repository.rugged } + let(:source_rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { source_repository.rugged } } let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } before do @@ -1898,7 +1945,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it "removes the branch from the repo" do repository.rm_branch(branch_name, user: user) - expect(repository.rugged.branches[branch_name]).to be_nil + expect(repository_rugged.branches[branch_name]).to be_nil end end @@ -1930,7 +1977,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#write_config' do before do - repository.rugged.config["gitlab.fullpath"] = repository.path + repository_rugged.config["gitlab.fullpath"] = repository_path end shared_examples 'writing repo config' do @@ -1938,7 +1985,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'writes it to disk' do repository.write_config(full_path: "not-the/real-path.git") - config = File.read(File.join(repository.path, "config")) + config = File.read(File.join(repository_path, "config")) expect(config).to include("[gitlab]") expect(config).to include("fullpath = not-the/real-path.git") @@ -1949,10 +1996,10 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'does not write it to disk' do repository.write_config(full_path: "") - config = File.read(File.join(repository.path, "config")) + config = File.read(File.join(repository_path, "config")) expect(config).to include("[gitlab]") - expect(config).to include("fullpath = #{repository.path}") + expect(config).to include("fullpath = #{repository_path}") end end end @@ -2173,7 +2220,11 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#gitlab_projects' do subject { repository.gitlab_projects } - it { expect(subject.shard_path).to eq(storage_path) } + it do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + expect(subject.shard_path).to eq(storage_path) + end + end it { expect(subject.repository_relative_path).to eq(repository.relative_path) } end @@ -2189,7 +2240,7 @@ describe Gitlab::Git::Repository, seed_helper: true do repository.bundle_to_disk(save_path) success = system( - *%W(#{Gitlab.config.git.bin_path} -C #{repository.path} bundle verify #{save_path}), + *%W(#{Gitlab.config.git.bin_path} -C #{repository_path} bundle verify #{save_path}), [:out, :err] => '/dev/null' ) expect(success).to be true @@ -2231,7 +2282,7 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'creates a symlink to the global hooks dir' do imported_repo.create_from_bundle(bundle_path) - hooks_path = File.join(imported_repo.path, 'hooks') + hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') } expect(File.readlink(hooks_path)).to eq(Gitlab.config.gitlab_shell.hooks_path) end @@ -2360,7 +2411,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#clean_stale_repository_files' do - let(:worktree_path) { File.join(repository.path, 'worktrees', 'delete-me') } + let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') } it 'cleans up the files' do repository.with_worktree(worktree_path, 'master', env: ENV) do @@ -2507,7 +2558,7 @@ describe Gitlab::Git::Repository, seed_helper: true do def create_remote_branch(repository, remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } - rugged = repository.rugged + rugged = repository_rugged rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) end @@ -2586,4 +2637,16 @@ describe Gitlab::Git::Repository, seed_helper: true do line.split("\t").last end end + + def repository_rugged + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.rugged + end + end + + def repository_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repository.path + end + end end diff --git a/spec/support/gitaly.rb b/spec/support/gitaly.rb index 9cf541372b5..5a1dd44bc9d 100644 --- a/spec/support/gitaly.rb +++ b/spec/support/gitaly.rb @@ -7,7 +7,10 @@ RSpec.configure do |config| next if example.metadata[:skip_gitaly_mock] # Use 'and_wrap_original' to make sure the arguments are valid - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original { |m, *args| m.call(*args) || true } + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_wrap_original do |m, *args| + m.call(*args) + !Gitlab::GitalyClient::EXPLICIT_OPT_IN_REQUIRED.include?(args.first) + end end end end |