Commit d685d996 authored by Dmitry Gruzd's avatar Dmitry Gruzd Committed by Luke Duncalfe

Send project permissions when repository indexing

This MR utilizes functionality introduced in
gitlab-elasticsearch-indexer to send visibility_level
and repository_access_level when indexing commit
documents. This MR also updates permission fields when the
underlying project permissions changes.
parent a41810b5
...@@ -23,6 +23,11 @@ module Elastic ...@@ -23,6 +23,11 @@ module Elastic
# avoid race condition if project is deleted before Elasticsearch update completes # avoid race condition if project is deleted before Elasticsearch update completes
return if pending_delete? return if pending_delete?
updated_attributes = updated_attributes.map(&:to_sym)
if updated_attributes.include?(:visibility_level) || updated_attributes.include?(:repository_access_level)
maintain_elasticsearch_permissions
end
super super
end end
...@@ -34,6 +39,12 @@ module Elastic ...@@ -34,6 +39,12 @@ module Elastic
def invalidate_elasticsearch_indexes_cache! def invalidate_elasticsearch_indexes_cache!
::Gitlab::CurrentSettings.invalidate_elasticsearch_indexes_cache_for_project!(self.id) ::Gitlab::CurrentSettings.invalidate_elasticsearch_indexes_cache_for_project!(self.id)
end end
private
def maintain_elasticsearch_permissions
::Elastic::ProcessInitialBookkeepingService.backfill_projects!(self)
end
end end
end end
end end
...@@ -27,7 +27,7 @@ module Elastic ...@@ -27,7 +27,7 @@ module Elastic
maintain_indexed_associations(project, INDEXED_PROJECT_ASSOCIATIONS) maintain_indexed_associations(project, INDEXED_PROJECT_ASSOCIATIONS)
ElasticCommitIndexerWorker.perform_async(project.id) ElasticCommitIndexerWorker.perform_async(project.id, false, { force: true })
ElasticCommitIndexerWorker.perform_async(project.id, true) ElasticCommitIndexerWorker.perform_async(project.id, true)
end end
end end
......
...@@ -17,16 +17,17 @@ class ElasticCommitIndexerWorker ...@@ -17,16 +17,17 @@ class ElasticCommitIndexerWorker
# #
# project_id - The ID of the project to index # project_id - The ID of the project to index
# wiki - Treat this project as a Wiki # wiki - Treat this project as a Wiki
# options - Options hash { force: bool } forces to reindex the repository
# #
# The indexation will cover all commits within INDEXED_SHA..HEAD # The indexation will cover all commits within INDEXED_SHA..HEAD
def perform(project_id, wiki = false) def perform(project_id, wiki = false, options = {})
return true unless Gitlab::CurrentSettings.elasticsearch_indexing? return true unless Gitlab::CurrentSettings.elasticsearch_indexing?
project = Project.find(project_id) project = Project.find(project_id)
return true unless project.use_elasticsearch? return true unless project.use_elasticsearch?
in_lock("#{self.class.name}/#{project_id}/#{wiki}", ttl: (Gitlab::Elastic::Indexer::TIMEOUT + 1.minute), retries: 0) do in_lock("#{self.class.name}/#{project_id}/#{wiki}", ttl: (Gitlab::Elastic::Indexer::TIMEOUT + 1.minute), retries: 0) do
Gitlab::Elastic::Indexer.new(project, wiki: wiki).run Gitlab::Elastic::Indexer.new(project, wiki: wiki, force: !!options['force']).run
end end
end end
end end
...@@ -18,12 +18,14 @@ module Gitlab ...@@ -18,12 +18,14 @@ module Gitlab
end end
end end
attr_reader :project, :index_status, :wiki attr_reader :project, :index_status, :wiki, :force
alias_method :index_wiki?, :wiki alias_method :index_wiki?, :wiki
alias_method :force_reindexing?, :force
def initialize(project, wiki: false) def initialize(project, wiki: false, force: false)
@project = project @project = project
@wiki = wiki @wiki = wiki
@force = force
# Use the eager-loaded association if available. # Use the eager-loaded association if available.
@index_status = project.index_status @index_status = project.index_status
...@@ -45,7 +47,17 @@ module Gitlab ...@@ -45,7 +47,17 @@ module Gitlab
from_sha: from_sha, from_sha: from_sha,
to_sha: commit.sha, to_sha: commit.sha,
index_wiki: index_wiki?) index_wiki: index_wiki?)
run_indexer!(commit.sha, target)
# This might happen when default branch has been reset or rebased.
base_sha = if purge_unreachable_commits_from_index?(commit.sha)
purge_unreachable_commits_from_index!(target)
Gitlab::Git::EMPTY_TREE_ID
else
from_sha
end
run_indexer!(base_sha, commit.sha, target)
end end
# update the index status only if all writes were successful # update the index status only if all writes were successful
...@@ -58,20 +70,17 @@ module Gitlab ...@@ -58,20 +70,17 @@ module Gitlab
!repository.empty? && repository.commit(ref) !repository.empty? && repository.commit(ref)
end end
def purge_unreachable_commits_from_index?(to_sha)
force_reindexing? || !last_commit_ancestor_of?(to_sha)
end
private private
def repository def repository
index_wiki? ? project.wiki.repository : project.repository index_wiki? ? project.wiki.repository : project.repository
end end
def run_indexer!(to_sha, target) def run_indexer!(base_sha, to_sha, target)
# This might happen when default branch has been reset or rebased.
base_sha = if purge_unreachable_commits_from_index!(to_sha, target)
Gitlab::Git::EMPTY_TREE_ID
else
from_sha
end
vars = build_envvars(base_sha, to_sha, target) vars = build_envvars(base_sha, to_sha, target)
path_to_indexer = Gitlab.config.elasticsearch.indexer_path path_to_indexer = Gitlab.config.elasticsearch.indexer_path
...@@ -81,7 +90,15 @@ module Gitlab ...@@ -81,7 +90,15 @@ module Gitlab
if index_wiki? if index_wiki?
[path_to_indexer, timeout_argument, "--blob-type=wiki_blob", "--skip-commits", "--project-path=#{project.full_path}", project.id.to_s, repository_path] [path_to_indexer, timeout_argument, "--blob-type=wiki_blob", "--skip-commits", "--project-path=#{project.full_path}", project.id.to_s, repository_path]
else else
[path_to_indexer, timeout_argument, "--project-path=#{project.full_path}", project.id.to_s, repository_path] [
path_to_indexer,
timeout_argument,
"--project-path=#{project.full_path}",
"--visibility-level=#{project.visibility_level}",
"--repository-access-level=#{project.repository_access_level}",
project.id.to_s,
repository_path
]
end end
output, status = Gitlab::Popen.popen(command, nil, vars) output, status = Gitlab::Popen.popen(command, nil, vars)
...@@ -92,11 +109,8 @@ module Gitlab ...@@ -92,11 +109,8 @@ module Gitlab
# Remove all indexed data for commits and blobs for a project. # Remove all indexed data for commits and blobs for a project.
# #
# @return: whether the index has been purged # @return: whether the index has been purged
def purge_unreachable_commits_from_index!(to_sha, target) def purge_unreachable_commits_from_index!(target)
return false if last_commit_ancestor_of?(to_sha)
target.delete_index_for_commits_and_blobs(wiki: index_wiki?) target.delete_index_for_commits_and_blobs(wiki: index_wiki?)
true
rescue ::Elasticsearch::Transport::Transport::Errors::BadRequest => e rescue ::Elasticsearch::Transport::Transport::Errors::BadRequest => e
Gitlab::ErrorTracking.track_exception(e, project_id: project.id) Gitlab::ErrorTracking.track_exception(e, project_id: project.id)
end end
......
...@@ -19,7 +19,9 @@ RSpec.describe Gitlab::Elastic::Indexer do ...@@ -19,7 +19,9 @@ RSpec.describe Gitlab::Elastic::Indexer do
let(:popen_success) { [[''], 0] } let(:popen_success) { [[''], 0] }
let(:popen_failure) { [['error'], 1] } let(:popen_failure) { [['error'], 1] }
subject(:indexer) { described_class.new(project) } let(:force_reindexing) { false }
subject(:indexer) { described_class.new(project, force: force_reindexing) }
context 'empty project', :elastic, :clean_gitlab_redis_shared_state do context 'empty project', :elastic, :clean_gitlab_redis_shared_state do
let(:project) { create(:project) } let(:project) { create(:project) }
...@@ -55,6 +57,25 @@ RSpec.describe Gitlab::Elastic::Indexer do ...@@ -55,6 +57,25 @@ RSpec.describe Gitlab::Elastic::Indexer do
end end
end end
describe '#purge_unreachable_commits_from_index?' do
using RSpec::Parameterized::TableSyntax
where(:force_reindexing, :ancestor_of, :result) do
true | false | true
false | false | true
false | true | false
true | true | true
end
with_them do
it 'returns correct result' do
allow(indexer).to receive(:last_commit_ancestor_of?).and_return(ancestor_of)
expect(indexer.purge_unreachable_commits_from_index?('sha')).to eq(result)
end
end
end
context 'with an indexed project', :elastic, :clean_gitlab_redis_shared_state do context 'with an indexed project', :elastic, :clean_gitlab_redis_shared_state do
let(:to_sha) { project.repository.commit.sha } let(:to_sha) { project.repository.commit.sha }
...@@ -94,6 +115,8 @@ RSpec.describe Gitlab::Elastic::Indexer do ...@@ -94,6 +115,8 @@ RSpec.describe Gitlab::Elastic::Indexer do
TestEnv.indexer_bin_path, TestEnv.indexer_bin_path,
"--timeout=#{Gitlab::Elastic::Indexer::TIMEOUT}s", "--timeout=#{Gitlab::Elastic::Indexer::TIMEOUT}s",
"--project-path=#{project.full_path}", "--project-path=#{project.full_path}",
"--visibility-level=#{project.visibility_level}",
"--repository-access-level=#{project.repository_access_level}",
project.id.to_s, project.id.to_s,
"#{project.repository.disk_path}.git" "#{project.repository.disk_path}.git"
], ],
...@@ -403,6 +426,7 @@ RSpec.describe Gitlab::Elastic::Indexer do ...@@ -403,6 +426,7 @@ RSpec.describe Gitlab::Elastic::Indexer do
before do before do
allow(Gitlab::Elasticsearch::Logger).to receive(:build).and_return(logger_double) allow(Gitlab::Elasticsearch::Logger).to receive(:build).and_return(logger_double)
allow(indexer).to receive(:run_indexer!) { Project.where(id: project.id).delete_all } allow(indexer).to receive(:run_indexer!) { Project.where(id: project.id).delete_all }
allow(indexer).to receive(:purge_unreachable_commits_from_index?).and_return(false)
allow(logger_double).to receive(:debug) allow(logger_double).to receive(:debug)
end end
......
...@@ -14,6 +14,10 @@ RSpec.describe Elastic::ProjectsSearch do ...@@ -14,6 +14,10 @@ RSpec.describe Elastic::ProjectsSearch do
def es_id def es_id
1 1
end end
def pending_delete?
false
end
end.new end.new
end end
...@@ -25,6 +29,15 @@ RSpec.describe Elastic::ProjectsSearch do ...@@ -25,6 +29,15 @@ RSpec.describe Elastic::ProjectsSearch do
end end
end end
describe '#maintain_elasticsearch_update' do
it 'initiates repository reindexing when permissions change' do
expect(::Elastic::ProcessBookkeepingService).to receive(:track!).and_return(true)
expect(::Elastic::ProcessInitialBookkeepingService).to receive(:backfill_projects!).and_return(true)
subject.maintain_elasticsearch_update(updated_attributes: %i[visibility_level repository_access_level])
end
end
describe '#maintain_elasticsearch_destroy' do describe '#maintain_elasticsearch_destroy' do
it 'calls delete worker' do it 'calls delete worker' do
expect(ElasticDeleteProjectWorker).to receive(:perform_async) expect(ElasticDeleteProjectWorker).to receive(:perform_async)
......
...@@ -9,7 +9,7 @@ RSpec.describe Elastic::ProcessInitialBookkeepingService do ...@@ -9,7 +9,7 @@ RSpec.describe Elastic::ProcessInitialBookkeepingService do
describe '.backfill_projects!' do describe '.backfill_projects!' do
it 'calls initial project indexing' do it 'calls initial project indexing' do
expect(described_class).to receive(:maintain_indexed_associations).with(project, Elastic::ProcessInitialBookkeepingService::INDEXED_PROJECT_ASSOCIATIONS) expect(described_class).to receive(:maintain_indexed_associations).with(project, Elastic::ProcessInitialBookkeepingService::INDEXED_PROJECT_ASSOCIATIONS)
expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id) expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id, false, { force: true })
expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id, true) expect(ElasticCommitIndexerWorker).to receive(:perform_async).with(project.id, true)
described_class.backfill_projects!(project) described_class.backfill_projects!(project)
......
...@@ -30,7 +30,7 @@ RSpec.describe ElasticCommitIndexerWorker do ...@@ -30,7 +30,7 @@ RSpec.describe ElasticCommitIndexerWorker do
indexer = double indexer = double
expect(indexer).to receive(:run) expect(indexer).to receive(:run)
expect(Gitlab::Elastic::Indexer).to receive(:new).with(project, wiki: true).and_return(indexer) expect(Gitlab::Elastic::Indexer).to receive(:new).with(project, wiki: true, force: false).and_return(indexer)
subject.perform(project.id, true) subject.perform(project.id, true)
end end
......
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