Merge branch 'pks-set-full-path' into 'master'

Convert setting the full path via SetFullPath RPC

See merge request gitlab-org/gitlab!66929

GITALY_SERVER_VERSION

-818f3d85a2c8e6596376f1d2276aa22660203a6c
+d513d220b183d83ae7219ec52f49aa3b4f7fc551

Gemfile

@@ -472,7 +472,7 @@ end
 gem 'spamcheck', '~> 0.1.0'
 
 # Gitaly GRPC protocol definitions
-gem 'gitaly', '~> 14.1.0.pre.rc4'
+gem 'gitaly', '~> 14.2.0.pre.rc2'
 
 # KAS GRPC protocol definitions
 gem 'kas-grpc', '~> 0.0.2'

Gemfile.lock

@@ -452,7 +452,7 @@ GEM
       rails (>= 3.2.0)
     git (1.7.0)
       rchardet (~> 1.8)
-    gitaly (14.1.0.pre.rc4)
+    gitaly (14.2.0.pre.rc2)
       grpc (~> 1.0)
     github-markup (1.7.0)
     gitlab (4.16.1)
@@ -1464,7 +1464,7 @@ DEPENDENCIES
   gettext (~> 3.3)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly (~> 14.1.0.pre.rc4)
+  gitaly (~> 14.2.0.pre.rc2)
   github-markup (~> 1.7.0)
   gitlab-chronic (~> 0.10.5)
   gitlab-dangerfiles (~> 2.3.0)

app/models/namespace.rb

@@ -528,7 +528,7 @@ class Namespace < ApplicationRecord
 
   def write_projects_repository_config
     all_projects.find_each do |project|
-      project.write_repository_config
+      project.set_full_path
       project.track_project_repository
     end
   end

app/models/project.rb

@@ -1889,11 +1889,11 @@ class Project < ApplicationRecord
       .update_all(deployed: deployment.present?, pages_deployment_id: deployment&.id)
   end
 
-  def write_repository_config(gl_full_path: full_path)
+  def set_full_path(gl_full_path: full_path)
     # We'd need to keep track of project full path otherwise directory tree
     # created with hashed storage enabled cannot be usefully imported using
     # the import rake task.
-    repository.raw_repository.write_config(full_path: gl_full_path)
+    repository.raw_repository.set_full_path(full_path: gl_full_path)
   rescue Gitlab::Git::Repository::NoRepository => e
     Gitlab::AppLogger.error("Error writing to .git/config for project #{full_path} (#{id}): #{e.message}.")
     nil
@@ -1917,7 +1917,7 @@ class Project < ApplicationRecord
     after_create_default_branch
     join_pool_repository
     refresh_markdown_cache!
-    write_repository_config
+    set_full_path
   end
 
   def update_project_counter_caches

app/services/projects/after_rename_service.rb

@@ -83,7 +83,7 @@ module Projects
 
     def update_repository_configuration
       project.reload_repository!
-      project.write_repository_config
+      project.set_full_path
       project.track_project_repository
     end
 

app/services/projects/create_service.rb

@@ -92,7 +92,7 @@ module Projects
       # Skip writing the config for project imports/forks because it
       # will always fail since the Git directory doesn't exist until
       # a background job creates it (see Project#add_import_job).
-      @project.write_repository_config unless @project.import?
+      @project.set_full_path unless @project.import?
 
       unless @project.gitlab_project_import?
         @project.create_wiki unless skip_wiki?

app/services/projects/hashed_storage/migrate_repository_service.rb

@@ -14,7 +14,7 @@ module Projects
         result = move_repositories
 
         if result
-          project.write_repository_config
+          project.set_full_path
           project.track_project_repository
         else
           rollback_folder_move

app/services/projects/hashed_storage/rollback_repository_service.rb

@@ -14,7 +14,7 @@ module Projects
         result = move_repositories
 
         if result
-          project.write_repository_config
+          project.set_full_path
           project.track_project_repository
         else
           rollback_folder_move

app/services/projects/transfer_service.rb

@@ -135,7 +135,7 @@ module Projects
     end
 
     def update_repository_configuration(full_path)
-      project.write_repository_config(gl_full_path: full_path)
+      project.set_full_path(gl_full_path: full_path)
       project.track_project_repository
     end
 

config/feature_flags/development/set_full_path.yml

+---
+name: set_full_path
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66929
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337002
+milestone: '14.2'
+type: development
+group: group::gitaly
+default_enabled: false

doc/raketasks/import.md

@@ -142,7 +142,7 @@ to do so. In a Rails console session, run the following to migrate a project:
 
 ```ruby
 project = Project.find_by_full_path('gitlab-org/gitlab')
-project.write_repository_config
+project.set_full_path
 ```
 
 In a Rails console session, run the following to migrate all of a namespace's

lib/gitlab/bare_repository_import/importer.rb

@@ -73,7 +73,7 @@ module Gitlab
         if project.persisted? && mv_repositories(project)
           log " * Created #{project.name} (#{project_full_path})".color(:green)
 
-          project.write_repository_config
+          project.set_full_path
 
           ProjectCacheWorker.perform_async(project.id)
         else

lib/gitlab/git/repository.rb

@@ -905,13 +905,17 @@ module Gitlab
       end
       # rubocop:enable Metrics/ParameterLists
 
-      def write_config(full_path:)
+      def set_full_path(full_path:)
         return unless full_path.present?
 
         # This guard avoids Gitaly log/error spam
         raise NoRepository, 'repository does not exist' unless exists?
 
-        set_config('gitlab.fullpath' => full_path)
+        if Feature.enabled?(:set_full_path)
+          gitaly_repository_client.set_full_path(full_path)
+        else
+          set_config('gitlab.fullpath' => full_path)
+        end
       end
 
       def set_config(entries)

lib/gitlab/gitaly_client/repository_service.rb

@@ -263,6 +263,21 @@ module Gitlab
         GitalyClient.call(@storage, :repository_service, :write_ref, request, timeout: GitalyClient.fast_timeout)
       end
 
+      def set_full_path(path)
+        GitalyClient.call(
+          @storage,
+          :repository_service,
+          :set_full_path,
+          Gitaly::SetFullPathRequest.new(
+            repository: @gitaly_repo,
+            path: path
+          ),
+          timeout: GitalyClient.fast_timeout
+        )
+
+        nil
+      end
+
       def set_config(entries)
         return if entries.empty?
 

spec/lib/gitlab/git/repository_spec.rb

@@ -1729,43 +1729,61 @@ RSpec.describe Gitlab::Git::Repository, :seed_helper do
     end
   end
 
-  describe '#write_config' do
-    before do
-      repository_rugged.config["gitlab.fullpath"] = repository_path
-    end
+  describe '#set_full_path' do
+    shared_examples '#set_full_path' do
+      before do
+        repository_rugged.config["gitlab.fullpath"] = repository_path
+      end
 
-    context 'is given a path' do
-      it 'writes it to disk' do
-        repository.write_config(full_path: "not-the/real-path.git")
+      context 'is given a path' do
+        it 'writes it to disk' do
+          repository.set_full_path(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")
+          expect(config).to include("[gitlab]")
+          expect(config).to include("fullpath = not-the/real-path.git")
+        end
       end
-    end
 
-    context 'it is given an empty path' do
-      it 'does not write it to disk' do
-        repository.write_config(full_path: "")
+      context 'it is given an empty path' do
+        it 'does not write it to disk' do
+          repository.set_full_path(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("[gitlab]")
+          expect(config).to include("fullpath = #{repository_path}")
+        end
+      end
+
+      context 'repository does not exist' do
+        it 'raises NoRepository and does not call Gitaly WriteConfig' do
+          repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project')
+
+          expect(repository.gitaly_repository_client).not_to receive(:set_full_path)
+
+          expect do
+            repository.set_full_path(full_path: 'foo/bar.git')
+          end.to raise_error(Gitlab::Git::Repository::NoRepository)
+        end
       end
     end
 
-    context 'repository does not exist' do
-      it 'raises NoRepository and does not call Gitaly WriteConfig' do
-        repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project')
+    context 'with :set_full_path enabled' do
+      before do
+        stub_feature_flags(set_full_path: true)
+      end
 
-        expect(repository.gitaly_repository_client).not_to receive(:write_config)
+      it_behaves_like '#set_full_path'
+    end
 
-        expect do
-          repository.write_config(full_path: 'foo/bar.git')
-        end.to raise_error(Gitlab::Git::Repository::NoRepository)
+    context 'with :set_full_path disabled' do
+      before do
+        stub_feature_flags(set_full_path: false)
       end
+
+      it_behaves_like '#set_full_path'
     end
   end
 

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -333,4 +333,17 @@ RSpec.describe Gitlab::GitalyClient::RepositoryService do
       client.replicate(source_repository)
     end
   end
+
+  describe '#set_full_path' do
+    let(:path) { 'repo/path' }
+
+    it 'sends a set_full_path message' do
+      expect_any_instance_of(Gitaly::RepositoryService::Stub)
+        .to receive(:set_full_path)
+        .with(gitaly_request_with_params(path: path), kind_of(Hash))
+        .and_return(double)
+
+      client.set_full_path(path)
+    end
+  end
 end

spec/models/project_spec.rb

@@ -5228,7 +5228,7 @@ RSpec.describe Project, factory_default: :keep do
       expect(InternalId).to receive(:flush_records!).with(project: project)
       expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:repository_size])
       expect(DetectRepositoryLanguagesWorker).to receive(:perform_async).with(project.id)
-      expect(project).to receive(:write_repository_config)
+      expect(project).to receive(:set_full_path)
 
       project.after_import
     end
@@ -5297,25 +5297,25 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
-  describe '#write_repository_config' do
+  describe '#set_full_path' do
     let_it_be(:project) { create(:project, :repository) }
 
     it 'writes full path in .git/config when key is missing' do
-      project.write_repository_config
+      project.set_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')
+      project.set_full_path(gl_full_path: 'old/path')
 
-      expect { project.write_repository_config }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path)
+      expect { project.set_full_path }.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
       project = create(:project_empty_repo)
 
-      expect { project.write_repository_config }.not_to raise_error
+      expect { project.set_full_path }.not_to raise_error
     end
   end
 

spec/services/projects/create_service_spec.rb

@@ -335,7 +335,7 @@ RSpec.describe Projects::CreateService, '#execute' do
 
     it 'does not write repository config' do
       expect_next_instance_of(Project) do |project|
-        expect(project).not_to receive(:write_repository_config)
+        expect(project).not_to receive(:set_full_path)
       end
 
       imported_project

spec/services/projects/hashed_storage/migrate_repository_service_spec.rb

@@ -96,13 +96,13 @@ RSpec.describe Projects::HashedStorage::MigrateRepositoryService do
       end
 
       it 'handles Gitlab::Git::CommandError' do
-        expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError)
+        expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError)
 
         expect { service.execute }.not_to raise_exception
       end
 
       it 'ensures rollback when Gitlab::Git::CommandError' do
-        expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError)
+        expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError)
         expect(service).to receive(:rollback_folder_move).and_call_original
 
         service.execute

spec/services/projects/hashed_storage/rollback_repository_service_spec.rb

@@ -96,13 +96,13 @@ RSpec.describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab
       end
 
       it 'handles Gitlab::Git::CommandError' do
-        expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError)
+        expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError)
 
         expect { service.execute }.not_to raise_exception
       end
 
       it 'ensures rollback when Gitlab::Git::CommandError' do
-        expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError)
+        expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError)
         expect(service).to receive(:rollback_folder_move).and_call_original
 
         service.execute