Merge branch '344460-send-project-permissions' into 'master'

Advanced Search: Send project permissions and keep them in sync

See merge request gitlab-org/gitlab!78549

ee/app/models/concerns/elastic/projects_search.rb

@@ -23,6 +23,11 @@ module Elastic
         # avoid race condition if project is deleted before Elasticsearch update completes
         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
       end
 
@@ -34,6 +39,12 @@ module Elastic
       def invalidate_elasticsearch_indexes_cache!
         ::Gitlab::CurrentSettings.invalidate_elasticsearch_indexes_cache_for_project!(self.id)
       end
+
+      private
+
+      def maintain_elasticsearch_permissions
+        ::Elastic::ProcessInitialBookkeepingService.backfill_projects!(self)
+      end
     end
   end
 end

ee/app/services/elastic/process_initial_bookkeeping_service.rb

@@ -27,7 +27,7 @@ module Elastic
 
           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)
         end
       end

ee/app/workers/elastic_commit_indexer_worker.rb

@@ -17,16 +17,17 @@ class ElasticCommitIndexerWorker
   #
   # project_id - The ID of the project to index
   # 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
-  def perform(project_id, wiki = false)
+  def perform(project_id, wiki = false, options = {})
     return true unless Gitlab::CurrentSettings.elasticsearch_indexing?
 
     project = Project.find(project_id)
     return true unless project.use_elasticsearch?
 
     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

ee/lib/gitlab/elastic/indexer.rb

@@ -18,12 +18,14 @@ module Gitlab
         end
       end
 
-      attr_reader :project, :index_status, :wiki
+      attr_reader :project, :index_status, :wiki, :force
       alias_method :index_wiki?, :wiki
+      alias_method :force_reindexing?, :force
 
-      def initialize(project, wiki: false)
+      def initialize(project, wiki: false, force: false)
         @project = project
         @wiki = wiki
+        @force = force
 
         # Use the eager-loaded association if available.
         @index_status = project.index_status
@@ -45,7 +47,17 @@ module Gitlab
                                from_sha: from_sha,
                                to_sha: commit.sha,
                                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
 
         # update the index status only if all writes were successful
@@ -58,20 +70,17 @@ module Gitlab
         !repository.empty? && repository.commit(ref)
       end
 
+      def purge_unreachable_commits_from_index?(to_sha)
+        force_reindexing? || !last_commit_ancestor_of?(to_sha)
+      end
+
       private
 
       def repository
         index_wiki? ? project.wiki.repository : project.repository
       end
 
-      def run_indexer!(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
-
+      def run_indexer!(base_sha, to_sha, target)
         vars = build_envvars(base_sha, to_sha, target)
         path_to_indexer = Gitlab.config.elasticsearch.indexer_path
 
@@ -81,7 +90,15 @@ module Gitlab
           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]
           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
 
         output, status = Gitlab::Popen.popen(command, nil, vars)
@@ -92,11 +109,8 @@ module Gitlab
       # Remove all indexed data for commits and blobs for a project.
       #
       # @return: whether the index has been purged
-      def purge_unreachable_commits_from_index!(to_sha, target)
-        return false if last_commit_ancestor_of?(to_sha)
-
+      def purge_unreachable_commits_from_index!(target)
         target.delete_index_for_commits_and_blobs(wiki: index_wiki?)
-        true
       rescue ::Elasticsearch::Transport::Transport::Errors::BadRequest => e
         Gitlab::ErrorTracking.track_exception(e, project_id: project.id)
       end

ee/spec/lib/gitlab/elastic/indexer_spec.rb

@@ -19,7 +19,9 @@ RSpec.describe Gitlab::Elastic::Indexer do
   let(:popen_success) { [[''], 0] }
   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
     let(:project) { create(:project) }
@@ -55,6 +57,25 @@ RSpec.describe Gitlab::Elastic::Indexer do
     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
     let(:to_sha) { project.repository.commit.sha }
 
@@ -94,6 +115,8 @@ RSpec.describe Gitlab::Elastic::Indexer do
             TestEnv.indexer_bin_path,
             "--timeout=#{Gitlab::Elastic::Indexer::TIMEOUT}s",
             "--project-path=#{project.full_path}",
+            "--visibility-level=#{project.visibility_level}",
+            "--repository-access-level=#{project.repository_access_level}",
             project.id.to_s,
             "#{project.repository.disk_path}.git"
           ],
@@ -403,6 +426,7 @@ RSpec.describe Gitlab::Elastic::Indexer do
     before do
       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(:purge_unreachable_commits_from_index?).and_return(false)
       allow(logger_double).to receive(:debug)
     end
 

ee/spec/models/concerns/elastic/projects_search_spec.rb

@@ -14,6 +14,10 @@ RSpec.describe Elastic::ProjectsSearch do
       def es_id
         1
       end
+
+      def pending_delete?
+        false
+      end
     end.new
   end
 
@@ -25,6 +29,15 @@ RSpec.describe Elastic::ProjectsSearch do
     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
     it 'calls delete worker' do
       expect(ElasticDeleteProjectWorker).to receive(:perform_async)

ee/spec/services/elastic/process_initial_bookkeeping_service_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Elastic::ProcessInitialBookkeepingService do
   describe '.backfill_projects!' do
     it 'calls initial project indexing' do
       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)
 
       described_class.backfill_projects!(project)

ee/spec/workers/elastic_commit_indexer_worker_spec.rb

@@ -30,7 +30,7 @@ RSpec.describe ElasticCommitIndexerWorker do
       indexer = double
 
       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)
     end