diff options
29 files changed, 211 insertions, 389 deletions
diff --git a/.flayignore b/.flayignore index 7faa6c7bb90..3e5063674ff 100644 --- a/.flayignore +++ b/.flayignore @@ -14,3 +14,12 @@ lib/gitlab/gitaly_client/ref_service.rb lib/gitlab/gitaly_client/commit_service.rb lib/gitlab/git/commit.rb lib/gitlab/git/tag.rb + +ee/db/**/* +ee/app/serializers/ee/merge_request_widget_entity.rb +ee/lib/api/epics.rb +ee/lib/api/geo_nodes.rb +ee/lib/ee/gitlab/ldap/sync/admin_users.rb +ee/app/workers/geo/file_download_dispatch_worker/job_artifact_job_finder.rb +ee/app/workers/geo/file_download_dispatch_worker/lfs_object_job_finder.rb +ee/spec/**/* diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index d498a2d6d11..9d3772d7541 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -54,7 +54,10 @@ class SnippetsFinder < UnionFinder end def authorized_snippets - Snippet.where(feature_available_projects.or(not_project_related)) + # This query was intentionally converted to a raw one to get it work in Rails 5.0. + # In Rails 5.0 and 5.1 there's a bug: https://github.com/rails/arel/issues/531 + # Please convert it back when on rails 5.2 as it works again as expected since 5.2. + Snippet.where("#{feature_available_projects} OR #{not_project_related}") .public_or_visible_to_user(current_user) end @@ -86,18 +89,20 @@ class SnippetsFinder < UnionFinder def feature_available_projects # Don't return any project related snippets if the user cannot read cross project - return table[:id].eq(nil) unless Ability.allowed?(current_user, :read_cross_project) + return table[:id].eq(nil).to_sql unless Ability.allowed?(current_user, :read_cross_project) projects = projects_for_user do |part| part.with_feature_available_for_user(:snippets, current_user) end.select(:id) - arel_query = Arel::Nodes::SqlLiteral.new(projects.to_sql) - table[:project_id].in(arel_query) + # This query was intentionally converted to a raw one to get it work in Rails 5.0. + # In Rails 5.0 and 5.1 there's a bug: https://github.com/rails/arel/issues/531 + # Please convert it back when on rails 5.2 as it works again as expected since 5.2. + "snippets.project_id IN (#{projects.to_sql})" end def not_project_related - table[:project_id].eq(nil) + table[:project_id].eq(nil).to_sql end def table diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index e847baf0d52..daad829faa2 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -176,7 +176,7 @@ module ProjectsHelper controller.action_name, Gitlab::CurrentSettings.cache_key, "cross-project:#{can?(current_user, :read_cross_project)}", - 'v2.5' + 'v2.6' ] key << pipeline_status_cache_key(project.pipeline_status) if project.pipeline_status.has_status? diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb index 84248f9590b..8a6b0ed1a5f 100644 --- a/app/models/project_services/gemnasium_service.rb +++ b/app/models/project_services/gemnasium_service.rb @@ -43,13 +43,18 @@ class GemnasiumService < Service def execute(data) return unless supported_events.include?(data[:object_kind]) + # Gitaly: this class will be removed https://gitlab.com/gitlab-org/gitlab-ee/issues/6010 + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path_to_repo + end + Gemnasium::GitlabService.execute( ref: data[:ref], before: data[:before], after: data[:after], token: token, api_key: api_key, - repo: project.repository.path_to_repo # Gitaly: fixed by https://gitlab.com/gitlab-org/security-products/gemnasium-migration/issues/9 + repo: repo_path ) end end diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb index f3c9e2b1582..ae5c5fac834 100644 --- a/app/workers/git_garbage_collect_worker.rb +++ b/app/workers/git_garbage_collect_worker.rb @@ -6,12 +6,6 @@ class GitGarbageCollectWorker # Timeout set to 24h LEASE_TIMEOUT = 86400 - GITALY_MIGRATED_TASKS = { - gc: :garbage_collect, - full_repack: :repack_full, - incremental_repack: :repack_incremental - }.freeze - def perform(project_id, task = :gc, lease_key = nil, lease_uuid = nil) project = Project.find(project_id) active_uuid = get_lease_uuid(lease_key) @@ -27,21 +21,7 @@ class GitGarbageCollectWorker end task = task.to_sym - cmd = command(task) - - gitaly_migrate(GITALY_MIGRATED_TASKS[task], status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_call(task, project.repository.raw_repository) - else - repo_path = project.repository.path_to_repo - description = "'#{cmd.join(' ')}' in #{repo_path}" - Gitlab::GitLogger.info(description) - - output, status = Gitlab::Popen.popen(cmd, repo_path) - - Gitlab::GitLogger.error("#{description} failed:\n#{output}") unless status.zero? - end - end + gitaly_call(task, project.repository.raw_repository) # Refresh the branch cache in case garbage collection caused a ref lookup to fail flush_ref_caches(project) if task == :gc @@ -82,21 +62,12 @@ class GitGarbageCollectWorker when :incremental_repack client.repack_incremental end - end - - def command(task) - case task - when :gc - git(write_bitmaps: bitmaps_enabled?) + %w[gc] - when :full_repack - git(write_bitmaps: bitmaps_enabled?) + %w[repack -A -d --pack-kept-objects] - when :incremental_repack - # Normal git repack fails when bitmaps are enabled. It is impossible to - # create a bitmap here anyway. - git(write_bitmaps: false) + %w[repack -d] - else - raise "Invalid gc task: #{task.inspect}" - end + rescue GRPC::NotFound => e + Gitlab::GitLogger.error("#{method} failed:\nRepository not found") + raise Gitlab::Git::Repository::NoRepository.new(e) + rescue GRPC::BadStatus => e + Gitlab::GitLogger.error("#{method} failed:\n#{e}") + raise Gitlab::Git::CommandError.new(e) end def flush_ref_caches(project) @@ -108,19 +79,4 @@ class GitGarbageCollectWorker def bitmaps_enabled? Gitlab::CurrentSettings.housekeeping_bitmaps_enabled end - - def git(write_bitmaps:) - config_value = write_bitmaps ? 'true' : 'false' - %W[git -c repack.writeBitmaps=#{config_value}] - end - - def gitaly_migrate(method, status: Gitlab::GitalyClient::MigrationStatus::OPT_IN, &block) - Gitlab::GitalyClient.migrate(method, status: status, &block) - rescue GRPC::NotFound => e - Gitlab::GitLogger.error("#{method} failed:\nRepository not found") - raise Gitlab::Git::Repository::NoRepository.new(e) - rescue GRPC::BadStatus => e - Gitlab::GitLogger.error("#{method} failed:\n#{e}") - raise Gitlab::Git::CommandError.new(e) - end end diff --git a/changelogs/unreleased/blackst0ne-rails5-fix-snippets-finder.yml b/changelogs/unreleased/blackst0ne-rails5-fix-snippets-finder.yml new file mode 100644 index 00000000000..597b85de26f --- /dev/null +++ b/changelogs/unreleased/blackst0ne-rails5-fix-snippets-finder.yml @@ -0,0 +1,5 @@ +--- +title: "[Rails5] Fix snippets_finder arel queries" +merge_request: 19796 +author: "@blackst0ne" +type: fixed diff --git a/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb b/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb index a96ea7d9db4..dc16d5c5169 100644 --- a/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb +++ b/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb @@ -12,7 +12,9 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration end def repository_storage_path - Gitlab.config.repositories.storages[repository_storage].legacy_disk_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages[repository_storage].legacy_disk_path + end end def repository_path diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb index 8986cd8cb4b..133435523e1 100644 --- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb +++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb @@ -64,7 +64,9 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration # we rename suffix instead of removing it path = path.sub(/\.git\z/, '_git') - check_routes(path.dup, 0, path) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + check_routes(path.dup, 0, path) + end end def check_routes(base, counter, path) diff --git a/lib/gitlab/git/blame.rb b/lib/gitlab/git/blame.rb index 40b65f6c0da..e25e15f5c80 100644 --- a/lib/gitlab/git/blame.rb +++ b/lib/gitlab/git/blame.rb @@ -22,24 +22,9 @@ module Gitlab private def load_blame - raw_output = @repo.gitaly_migrate(:blame, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - load_blame_by_gitaly - else - load_blame_by_shelling_out - end - end - - output = encode_utf8(raw_output) - process_raw_blame output - end - - def load_blame_by_gitaly - @repo.gitaly_commit_client.raw_blame(@sha, @path) - end + output = encode_utf8(@repo.gitaly_commit_client.raw_blame(@sha, @path)) - def load_blame_by_shelling_out - @repo.shell_blame(@sha, @path) + process_raw_blame(output) end def process_raw_blame(output) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 61ae42a116b..eb5d6318dcb 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -120,13 +120,11 @@ module Gitlab # Default branch in the repository def root_ref - @root_ref ||= gitaly_migrate(:root_ref, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.default_branch_name - else - discover_default_branch - end - end + gitaly_ref_client.default_branch_name + rescue GRPC::NotFound => e + raise NoRepository.new(e.message) + rescue GRPC::Unknown => e + raise Gitlab::Git::CommandError.new(e.message) end def rugged @@ -152,23 +150,15 @@ module Gitlab # Returns an Array of branch names # sorted by name ASC def branch_names - gitaly_migrate(:branch_names, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.branch_names - else - branches.map(&:name) - end + wrapped_gitaly_errors do + gitaly_ref_client.branch_names end end # Returns an Array of Branches def branches - gitaly_migrate(:branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.branches - else - branches_filter - end + wrapped_gitaly_errors do + gitaly_ref_client.branches end end @@ -200,12 +190,8 @@ module Gitlab end def local_branches(sort_by: nil) - gitaly_migrate(:local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.local_branches(sort_by: sort_by) - else - branches_filter(filter: :local, sort_by: sort_by) - end + wrapped_gitaly_errors do + gitaly_ref_client.local_branches(sort_by: sort_by) end end @@ -245,18 +231,6 @@ module Gitlab # This refs by default not visible in project page and not cloned to client side. alias_method :has_visible_content?, :has_local_branches? - def has_local_branches_rugged? - rugged.branches.each(:local).any? do |ref| - begin - ref.name && ref.target # ensures the branch is valid - - true - rescue Rugged::ReferenceError - false - end - end - end - # Returns the number of valid tags def tag_count gitaly_migrate(:tag_names) do |is_enabled| @@ -270,12 +244,8 @@ module Gitlab # Returns an Array of tag names def tag_names - gitaly_migrate(:tag_names, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_ref_client.tag_names - else - rugged.tags.map { |t| t.name } - end + wrapped_gitaly_errors do + gitaly_ref_client.tag_names end end @@ -283,12 +253,8 @@ module Gitlab # # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/390 def tags - gitaly_migrate(:tags, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - tags_from_gitaly - else - tags_from_rugged - end + wrapped_gitaly_errors do + gitaly_ref_client.tags end end @@ -364,31 +330,6 @@ module Gitlab end.map(&:name) end - # Discovers the default branch based on the repository's available branches - # - # - If no branches are present, returns nil - # - If one branch is present, returns its name - # - If two or more branches are present, returns current HEAD or master or first branch - def discover_default_branch - names = branch_names - - return if names.empty? - - return names[0] if names.length == 1 - - if rugged_head - extracted_name = Ref.extract_branch_name(rugged_head.name) - - return extracted_name if names.include?(extracted_name) - end - - if names.include?('master') - 'master' - else - names[0] - end - end - def rugged_head rugged.head rescue Rugged::ReferenceError @@ -1453,6 +1394,16 @@ module Gitlab raise CommandError.new(e) end + def wrapped_gitaly_errors(&block) + yield block + rescue GRPC::NotFound => e + raise NoRepository.new(e) + rescue GRPC::InvalidArgument => e + raise ArgumentError.new(e) + rescue GRPC::BadStatus => e + raise CommandError.new(e) + end + def clean_stale_repository_files gitaly_migrate(:repository_cleanup, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| gitaly_repository_client.cleanup if is_enabled && exists? @@ -1606,12 +1557,8 @@ module Gitlab private def uncached_has_local_branches? - gitaly_migrate(:has_local_branches, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - gitaly_repository_client.has_local_branches? - else - has_local_branches_rugged? - end + wrapped_gitaly_errors do + gitaly_repository_client.has_local_branches? end end @@ -1731,20 +1678,6 @@ module Gitlab } end - # Gitaly note: JV: Trying to get rid of the 'filter' option so we can implement this with 'git'. - def branches_filter(filter: nil, sort_by: nil) - branches = rugged.branches.each(filter).map do |rugged_ref| - begin - target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target) - Gitlab::Git::Branch.new(self, rugged_ref.name, rugged_ref.target, target_commit) - rescue Rugged::ReferenceError - # Omit invalid branch - end - end.compact - - sort_branches(branches, sort_by) - end - def git_merged_branch_names(branch_names, root_sha) git_arguments = %W[branch --merged #{root_sha} @@ -1956,37 +1889,11 @@ module Gitlab end end - def tags_from_rugged - rugged.references.each("refs/tags/*").map do |ref| - message = nil - - if ref.target.is_a?(Rugged::Tag::Annotation) - tag_message = ref.target.message - - if tag_message.respond_to?(:chomp) - message = tag_message.chomp - end - end - - target_commit = Gitlab::Git::Commit.find(self, ref.target) - Gitlab::Git::Tag.new(self, { - name: ref.name, - target: ref.target, - target_commit: target_commit, - message: message - }) - end.sort_by(&:name) - end - def last_commit_for_path_by_rugged(sha, path) sha = last_commit_id_for_path_by_shelling_out(sha, path) commit(sha) end - def tags_from_gitaly - gitaly_ref_client.tags - end - def size_by_shelling_out popen(%w(du -sk), path).first.strip.to_i end diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 36e9adf27da..620362b52a9 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -33,11 +33,6 @@ 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 @@ -249,7 +244,7 @@ module Gitlab when MigrationStatus::OPT_OUT true when MigrationStatus::OPT_IN - opt_into_all_features? && !EXPLICIT_OPT_IN_REQUIRED.include?(feature_name) + opt_into_all_features? && !explicit_opt_in_required.include?(feature_name) else false end @@ -259,6 +254,13 @@ module Gitlab false end + # 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. This function has an override in EE. + def self.explicit_opt_in_required + [] + end + # opt_into_all_features? returns true when the current environment # is one in which we opt into features automatically def self.opt_into_all_features? diff --git a/lib/system_check/orphans/repository_check.rb b/lib/system_check/orphans/repository_check.rb index 5ef0b93ad08..2695c658874 100644 --- a/lib/system_check/orphans/repository_check.rb +++ b/lib/system_check/orphans/repository_check.rb @@ -5,16 +5,18 @@ module SystemCheck attr_accessor :orphans def multi_check - Gitlab.config.repositories.storages.each do |storage_name, repository_storage| - storage_path = repository_storage.legacy_disk_path + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.each do |storage_name, repository_storage| + storage_path = repository_storage.legacy_disk_path - $stdout.puts - $stdout.puts "* Storage: #{storage_name} (#{storage_path})".color(:yellow) + $stdout.puts + $stdout.puts "* Storage: #{storage_name} (#{storage_path})".color(:yellow) - repositories = disk_repositories(storage_path) - orphans = (repositories - fetch_repositories(storage_name)) + repositories = disk_repositories(storage_path) + orphans = (repositories - fetch_repositories(storage_name)) - print_orphans(orphans, storage_name) + print_orphans(orphans, storage_name) + end end end diff --git a/lib/tasks/migrate/setup_postgresql.rake b/lib/tasks/migrate/setup_postgresql.rake index e7aab50e42a..f69d204c579 100644 --- a/lib/tasks/migrate/setup_postgresql.rake +++ b/lib/tasks/migrate/setup_postgresql.rake @@ -22,3 +22,18 @@ task setup_postgresql: :environment do ProjectNameLowerIndex.new.up AddPathIndexToRedirectRoutes.new.up end + +desc 'GitLab | Generate PostgreSQL Password Hash' +task :postgresql_md5_hash do + require 'digest' + username = ENV.fetch('USERNAME') do |missing| + puts "You must provide an username with '#{missing}' ENV variable" + exit(1) + end + password = ENV.fetch('PASSWORD') do |missing| + puts "You must provide a password with '#{missing}' ENV variable" + exit(1) + end + hash = Digest::MD5.hexdigest("#{password}#{username}") + puts "The MD5 hash of your database password for user: #{username} -> #{hash}" +end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 5bd22ea803c..705b30f0130 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -296,16 +296,22 @@ describe ProjectsController do shared_examples_for 'updating a project' do context 'when only renaming a project path' do it "sets the repository to the right path after a rename" do - original_repository_path = project.repository.path + original_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.path + end expect { update_project path: 'renamed_path' } .to change { project.reload.path } expect(project.path).to include 'renamed_path' + assign_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + assigns(:repository).path + end + if project.hashed_storage?(:repository) - expect(assigns(:repository).path).to eq(original_repository_path) + expect(assign_repository_path).to eq(original_repository_path) else - expect(assigns(:repository).path).to include(project.path) + expect(assign_repository_path).to include(project.path) end expect(response).to have_gitlab_http_status(302) diff --git a/spec/features/projects/issues/user_comments_on_issue_spec.rb b/spec/features/projects/issues/user_comments_on_issue_spec.rb index c45fdc7642f..353f487485d 100644 --- a/spec/features/projects/issues/user_comments_on_issue_spec.rb +++ b/spec/features/projects/issues/user_comments_on_issue_spec.rb @@ -31,11 +31,14 @@ describe "User comments on issue", :js do end it "adds comment with code block" do - comment = "```\nCommand [1]: /usr/local/bin/git , see [text](doc/text)\n```" + code_block_content = "Command [1]: /usr/local/bin/git , see [text](doc/text)" + comment = "```\n#{code_block_content}\n```" add_note(comment) - expect(page).to have_content(comment) + wait_for_requests + + expect(page.find('pre code').text).to eq code_block_content end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 815b4035114..5cf9e9e8f12 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -280,7 +280,11 @@ describe ProjectsHelper do describe '#sanitizerepo_repo_path' do let(:project) { create(:project, :repository) } - let(:storage_path) { Gitlab.config.repositories.storages.default.legacy_disk_path } + let(:storage_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + Gitlab.config.repositories.storages.default.legacy_disk_path + end + end before do allow(Settings.shared).to receive(:[]).with('path').and_return('/base/repo/export/path') diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 793228701cf..ba790b717ae 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::Git::Blame, seed_helper: true do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end - shared_examples 'blaming a file' do + describe 'blaming a file' do context "each count" do it do data = [] @@ -68,12 +68,4 @@ describe Gitlab::Git::Blame, seed_helper: true do end end end - - context 'when Gitaly blame feature is enabled' do - it_behaves_like 'blaming a file' - end - - context 'when Gitaly blame feature is disabled', :skip_gitaly_mock do - it_behaves_like 'blaming a file' - end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 1744db1b17e..5bae99101e6 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -77,17 +77,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#root_ref' do - context 'with gitaly disabled' do - before do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) - end - - it 'calls #discover_default_branch' do - expect(repository).to receive(:discover_default_branch) - repository.root_ref - end - end - it 'returns UTF-8' do expect(repository.root_ref).to be_utf8 end @@ -153,46 +142,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe "#discover_default_branch" do - let(:master) { 'master' } - let(:feature) { 'feature' } - let(:feature2) { 'feature2' } - - around do |example| - # discover_default_branch will be moved to gitaly-ruby - 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') - 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') - end - - it "returns a non-master branch when only one exists" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([feature]) - expect(repository.discover_default_branch).to eq('feature') - end - - it "returns a non-master branch when more than one exists and master does not" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([feature, feature2]) - expect(repository.discover_default_branch).to eq('feature') - end - - it "returns nil when no branch exists" do - expect(repository).to receive(:branch_names).at_least(:once).and_return([]) - expect(repository.discover_default_branch).to be_nil - end - end - describe '#branch_names' do subject { repository.branch_names } @@ -476,7 +425,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#has_local_branches?' do - shared_examples 'check for local branches' do + context 'check for local branches' do it { expect(repository.has_local_branches?).to eq(true) } context 'mutable' do @@ -510,14 +459,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end end - - context 'with gitaly' do - it_behaves_like 'check for local branches' - end - - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like 'check for local branches' - end end describe "#delete_branch" do @@ -1395,24 +1336,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - # With Gitaly enabled, Gitaly just doesn't return deleted branches. - context 'with deleted branch with Gitaly disabled' do - before do - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) - end - - it 'returns no results' do - ref = double() - allow(ref).to receive(:name) { 'bad-branch' } - allow(ref).to receive(:target) { raise Rugged::ReferenceError } - branches = double() - allow(branches).to receive(:each) { [ref].each } - allow(repository_rugged).to receive(:branches) { branches } - - expect(subject).to be_empty - end - end - it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :branches end diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb index 730ede99fc9..9c6c9fe13bf 100644 --- a/spec/lib/gitlab/git_access_wiki_spec.rb +++ b/spec/lib/gitlab/git_access_wiki_spec.rb @@ -52,7 +52,9 @@ describe Gitlab::GitAccessWiki do context 'when the wiki repository does not exist' do it 'returns not found' do wiki_repo = project.wiki.repository - FileUtils.rm_rf(wiki_repo.path) + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + FileUtils.rm_rf(wiki_repo.path) + end # Sanity check for rm_rf expect(wiki_repo.exists?).to eq(false) diff --git a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb index 4ee1d255fbd..ac34efa4f9d 100644 --- a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb +++ b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb @@ -6,7 +6,11 @@ require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_ describe MigrateProcessCommitWorkerJobs do let(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs let(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs - let(:commit) { project.commit.raw.rugged_commit } + let(:commit) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.commit.raw.rugged_commit + end + end describe 'Project' do describe 'find_including_path' do diff --git a/spec/migrations/turn_nested_groups_into_regular_groups_for_mysql_spec.rb b/spec/migrations/turn_nested_groups_into_regular_groups_for_mysql_spec.rb index 560409f08de..5f5ba426d69 100644 --- a/spec/migrations/turn_nested_groups_into_regular_groups_for_mysql_spec.rb +++ b/spec/migrations/turn_nested_groups_into_regular_groups_for_mysql_spec.rb @@ -49,10 +49,14 @@ describe TurnNestedGroupsIntoRegularGroupsForMysql do end it 'renames the repository of any projects' do - expect(updated_project.repository.path) + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + updated_project.repository.path + end + + expect(repo_path) .to end_with("#{parent_group.name}-#{child_group.name}/#{updated_project.path}.git") - expect(File.directory?(updated_project.repository.path)).to eq(true) + expect(File.directory?(repo_path)).to eq(true) end it 'creates a redirect route for renamed projects' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 585cf7aab44..bc9cce6b0c3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2943,7 +2943,7 @@ describe Project do project.rename_repo - expect(project.repository.rugged.config['gitlab.fullpath']).to eq(project.full_path) + expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) end end @@ -3104,7 +3104,7 @@ describe Project do it 'updates project full path in .git/config' do project.rename_repo - expect(project.repository.rugged.config['gitlab.fullpath']).to eq(project.full_path) + expect(rugged_config['gitlab.fullpath']).to eq(project.full_path) end end @@ -3525,13 +3525,13 @@ describe Project do it 'writes full path in .git/config when key is missing' do project.write_repository_config - expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path + expect(rugged_config['gitlab.fullpath']).to eq project.full_path end it 'updates full path in .git/config when key is present' do project.write_repository_config(gl_full_path: 'old/path') - expect { project.write_repository_config }.to change { project.repository.rugged.config['gitlab.fullpath'] }.from('old/path').to(project.full_path) + expect { project.write_repository_config }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path) end it 'does not raise an error with an empty repository' do @@ -3817,4 +3817,10 @@ describe Project do let(:uploader_class) { AttachmentUploader } end end + + def rugged_config + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged.config + end + end end diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index f1142832f1a..a3c20b3b3c1 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -188,7 +188,11 @@ describe ProjectWiki do before do subject.wiki # Make sure the wiki repo exists - BareRepoOperations.new(subject.repository.path_to_repo).commit_file(image, 'image.png') + repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.repository.path_to_repo + end + + BareRepoOperations.new(repo_path).commit_file(image, 'image.png') end it 'returns the latest version of the file if it exists' do diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 4c086eeadfc..3597b080021 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -74,7 +74,9 @@ describe RemoteMirror do mirror.update_attribute(:url, 'http://foo:baz@test.com') - config = repo.raw_repository.rugged.config + config = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + repo.raw_repository.rugged.config + end expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com') end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index bc32372d3a9..a56b913198c 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -522,7 +522,6 @@ describe API::Internal do context 'the project path was changed' do let(:project) { create(:project, :repository, :legacy_storage) } - let!(:old_path_to_repo) { project.repository.path_to_repo } let!(:repository) { project.repository } before do diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index a8f003b1073..e8cbf84e3be 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -272,8 +272,11 @@ describe Projects::CreateService, '#execute' do it 'writes project full path to .git/config' do project = create_project(user, opts) + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end - expect(project.repository.rugged.config['gitlab.fullpath']).to eq project.full_path + expect(rugged.config['gitlab.fullpath']).to eq project.full_path end def create_project(user, opts) diff --git a/spec/support/gitaly.rb b/spec/support/gitaly.rb index 5a1dd44bc9d..614aaa73693 100644 --- a/spec/support/gitaly.rb +++ b/spec/support/gitaly.rb @@ -9,7 +9,7 @@ RSpec.configure do |config| # Use 'and_wrap_original' to make sure the arguments are valid 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) + !Gitlab::GitalyClient.explicit_opt_in_required.include?(args.first) end end end diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 807d1b8c084..e39dec556fc 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -11,36 +11,63 @@ describe GitGarbageCollectWorker do subject { described_class.new } describe "#perform" do - shared_examples 'flushing ref caches' do |gitaly| - context 'with active lease_uuid' do + context 'with active lease_uuid' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + end + + it "flushes ref caches when the task if 'gc'" do + expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) + .and_return(nil) + expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original + expect_any_instance_of(Repository).to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original + expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original + + subject.perform(project.id, :gc, lease_key, lease_uuid) + end + end + + context 'with different lease than the active one' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) + end + + it 'returns silently' do + expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original + expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original + + subject.perform(project.id, :gc, lease_key, lease_uuid) + end + end + + context 'with no active lease' do + before do + allow(subject).to receive(:get_lease_uuid).and_return(false) + end + + context 'when is able to get the lease' do before do - allow(subject).to receive(:get_lease_uuid).and_return(lease_uuid) + allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) end it "flushes ref caches when the task if 'gc'" do - expect(subject).to receive(:renew_lease).with(lease_key, lease_uuid).and_call_original - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - - if gitaly - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) - .and_return(nil) - else - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) - end - + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) + .and_return(nil) expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original - subject.perform(project.id, :gc, lease_key, lease_uuid) + subject.perform(project.id) end end - context 'with different lease than the active one' do + context 'when no lease can be obtained' do before do - allow(subject).to receive(:get_lease_uuid).and_return(SecureRandom.uuid) + expect(subject).to receive(:try_obtain_lease).and_return(false) end it 'returns silently' do @@ -49,63 +76,9 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original - subject.perform(project.id, :gc, lease_key, lease_uuid) + subject.perform(project.id) end end - - context 'with no active lease' do - before do - allow(subject).to receive(:get_lease_uuid).and_return(false) - end - - context 'when is able to get the lease' do - before do - allow(subject).to receive(:try_obtain_lease).and_return(SecureRandom.uuid) - end - - it "flushes ref caches when the task if 'gc'" do - expect(subject).to receive(:command).with(:gc).and_return([:the, :command]) - - if gitaly - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive(:garbage_collect) - .and_return(nil) - else - expect(Gitlab::Popen).to receive(:popen) - .with([:the, :command], project.repository.path_to_repo).and_return(["", 0]) - end - - expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original - expect_any_instance_of(Repository).to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original - - subject.perform(project.id) - end - end - - context 'when no lease can be obtained' do - before do - expect(subject).to receive(:try_obtain_lease).and_return(false) - end - - it 'returns silently' do - expect(subject).not_to receive(:command) - expect_any_instance_of(Repository).not_to receive(:after_create_branch).and_call_original - expect_any_instance_of(Repository).not_to receive(:branch_names).and_call_original - expect_any_instance_of(Repository).not_to receive(:has_visible_content?).and_call_original - - subject.perform(project.id) - end - end - end - end - - context "with Gitaly turned on" do - it_should_behave_like 'flushing ref caches', true - end - - context "with Gitaly turned off", :disable_gitaly do - it_should_behave_like 'flushing ref caches', false end context "repack_full" do diff --git a/spec/workers/repository_remove_remote_worker_spec.rb b/spec/workers/repository_remove_remote_worker_spec.rb index f22d7c1d073..5968c5da3c9 100644 --- a/spec/workers/repository_remove_remote_worker_spec.rb +++ b/spec/workers/repository_remove_remote_worker_spec.rb @@ -44,7 +44,9 @@ describe RepositoryRemoveRemoteWorker do end def create_remote_branch(remote_name, branch_name, target) - rugged = project.repository.rugged + rugged = Gitlab::GitalyClient::StorageSettings.allow_disk_access do + project.repository.rugged + end rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target.id) end end |