Remove hook directory requirement from Shellgitaly-version-v1.43.0

It used to be the case that GitLab created symlinks for each repository
to one copy of the Git hooks, so these ran when required. This changed
to set the hooks dynamically on Gitaly when invoking Git.

The side effect is that we didn't need all these symlinks anymore, which
Gitaly doesn't create anymore either. Now that means that the tests in
GitLab-Rails should test for it either.

Related: https://gitlab.com/gitlab-org/gitaly/issues/1392#note_175619926

10 files changed, 3 insertions, 155 deletions

diff --git a/doc/administration/raketasks/maintenance.md b/doc/administration/raketasks/maintenance.md
index b295b7d5dc4..0b4c1ae15d6 100644
--- a/doc/administration/raketasks/maintenance.md
+++ b/doc/administration/raketasks/maintenance.md
@@ -205,25 +205,6 @@ cd /home/git/gitlab
 sudo -u git -H bundle exec rake gitlab:track_deployment RAILS_ENV=production
 ```
 
-## Create or repair repository hooks symlink
-
-If the GitLab shell hooks directory location changes or another circumstance
-leads to the hooks symlink becoming missing or invalid, run this Rake task
-to create or repair the symlinks.
-
-**Omnibus Installation**
-
-```
-sudo gitlab-rake gitlab:shell:create_hooks
-```
-
-**Source Installation**
-
-```
-cd /home/git/gitlab
-sudo -u git -H bundle exec rake gitlab:shell:create_hooks RAILS_ENV=production
-```
-
 ## Check TCP connectivity to a remote site
 
 Sometimes you need to know if your GitLab installation can connect to a TCP
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 455588f3c66..dcdd3581d92 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -34,34 +34,6 @@ module Gitlab
       TagExistsError = Class.new(StandardError)
       ChecksumError = Class.new(StandardError)
 
-      class << self
-        def create_hooks(repo_path, global_hooks_path)
-          local_hooks_path = File.join(repo_path, 'hooks')
-          real_local_hooks_path = :not_found
-
-          begin
-            real_local_hooks_path = File.realpath(local_hooks_path)
-          rescue Errno::ENOENT
-            # real_local_hooks_path == :not_found
-          end
-
-          # Do nothing if hooks already exist
-          unless real_local_hooks_path == File.realpath(global_hooks_path)
-            if File.exist?(local_hooks_path)
-              # Move the existing hooks somewhere safe
-              FileUtils.mv(
-                local_hooks_path,
-                "#{local_hooks_path}.old.#{Time.now.to_i}")
-            end
-
-            # Create the hooks symlink
-            FileUtils.ln_sf(global_hooks_path, local_hooks_path)
-          end
-
-          true
-        end
-      end
-
       # Directory name of repo
       attr_reader :name
 
diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake
index ee3ef9dad6e..487808a7baa 100644
--- a/lib/tasks/gitlab/shell.rake
+++ b/lib/tasks/gitlab/shell.rake
@@ -51,9 +51,6 @@ namespace :gitlab do
         end
       end
 
-      # (Re)create hooks
-      Rake::Task['gitlab:shell:create_hooks'].invoke
-
       Gitlab::Shell.ensure_secret_token!
     end
 
@@ -78,15 +75,6 @@ namespace :gitlab do
         end
       end
     end
-
-    desc 'Create or repair repository hooks symlink'
-    task create_hooks: :gitlab_environment do
-      warn_user_is_not_gitlab
-
-      puts 'Creating/Repairing hooks symlinks for all repositories'
-      system(*%W(#{Gitlab.config.gitlab_shell.path}/bin/create-hooks) + repository_storage_paths_args)
-      puts 'done'.color(:green)
-    end
   end
 
   def setup
diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb
index 28ae90bc0de..8d2b1fc7e30 100644
--- a/spec/features/projects/import_export/import_file_spec.rb
+++ b/spec/features/projects/import_export/import_file_spec.rb
@@ -47,7 +47,6 @@ describe 'Import/Export - project import integration test', :js do
         expect(project.description).to eq("Foo Bar")
         expect(project.issues).not_to be_empty
         expect(project.merge_requests).not_to be_empty
-        expect(project_hook_exists?(project)).to be true
         expect(wiki_exists?(project)).to be true
         expect(project.import_state.status).to eq('finished')
       end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 7644d83992f..cb4701e8edc 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -29,51 +29,6 @@ describe Gitlab::Git::Repository, :seed_helper do
   let(:storage_path) { TestEnv.repos_path }
   let(:user) { build(:user) }
 
-  describe '.create_hooks' do
-    let(:repo_path) { File.join(storage_path, 'hook-test.git') }
-    let(:hooks_dir) { File.join(repo_path, 'hooks') }
-    let(:target_hooks_dir) { Gitlab::Shell.new.hooks_path }
-    let(:existing_target) { File.join(repo_path, 'foobar') }
-
-    before do
-      FileUtils.rm_rf(repo_path)
-      FileUtils.mkdir_p(repo_path)
-    end
-
-    context 'hooks is a directory' do
-      let(:existing_file) { File.join(hooks_dir, 'my-file') }
-
-      before do
-        FileUtils.mkdir_p(hooks_dir)
-        FileUtils.touch(existing_file)
-        described_class.create_hooks(repo_path, target_hooks_dir)
-      end
-
-      it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) }
-      it { expect(Dir[File.join(repo_path, "hooks.old.*/my-file")].count).to eq(1) }
-    end
-
-    context 'hooks is a valid symlink' do
-      before do
-        FileUtils.mkdir_p existing_target
-        File.symlink(existing_target, hooks_dir)
-        described_class.create_hooks(repo_path, target_hooks_dir)
-      end
-
-      it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) }
-    end
-
-    context 'hooks is a broken symlink' do
-      before do
-        FileUtils.rm_f(existing_target)
-        File.symlink(existing_target, hooks_dir)
-        described_class.create_hooks(repo_path, target_hooks_dir)
-      end
-
-      it { expect(File.readlink(hooks_dir)).to eq(target_hooks_dir) }
-    end
-  end
-
   describe "Respond to" do
     subject { repository }
 
@@ -1959,13 +1914,6 @@ describe Gitlab::Git::Repository, :seed_helper do
       expect { imported_repo.fsck }.not_to raise_exception
     end
 
-    it 'creates a symlink to the global hooks dir' do
-      imported_repo.create_from_bundle(valid_bundle_path)
-      hooks_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access { File.join(imported_repo.path, 'hooks') }
-
-      expect(File.readlink(hooks_path)).to eq(Gitlab::Shell.new.hooks_path)
-    end
-
     it 'raises an error if the bundle is an attempted malicious payload' do
       expect do
         imported_repo.create_from_bundle(malicious_bundle_path)
diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb
index 8a699eb1461..e2ffb2adb9b 100644
--- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb
@@ -34,11 +34,5 @@ describe Gitlab::ImportExport::RepoRestorer do
     it 'restores the repo successfully' do
       expect(restorer.restore).to be_truthy
     end
-
-    it 'has the webhooks' do
-      restorer.restore
-
-      expect(project_hook_exists?(project)).to be true
-    end
   end
 end
diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb
index e2f09de2808..bce2e754176 100644
--- a/spec/lib/gitlab/shell_spec.rb
+++ b/spec/lib/gitlab/shell_spec.rb
@@ -612,16 +612,6 @@ describe Gitlab::Shell do
         FileUtils.rm_rf(created_path)
       end
 
-      it 'creates a repository' do
-        expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_truthy
-
-        expect(File.stat(created_path).mode & 0o777).to eq(0o770)
-
-        hooks_path = File.join(created_path, 'hooks')
-        expect(File.lstat(hooks_path)).to be_symlink
-        expect(File.realpath(hooks_path)).to eq(gitlab_shell_hooks_path)
-      end
-
       it 'returns false when the command fails' do
         FileUtils.mkdir_p(File.dirname(created_path))
         # This file will block the creation of the repo's .git directory. That
diff --git a/spec/support/helpers/git_helpers.rb b/spec/support/helpers/git_helpers.rb
index 99a7c39852e..99c5871ba54 100644
--- a/spec/support/helpers/git_helpers.rb
+++ b/spec/support/helpers/git_helpers.rb
@@ -6,12 +6,4 @@ module GitHelpers
 
     Rugged::Repository.new(path)
   end
-
-  def project_hook_exists?(project)
-    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      project_path = project.repository.raw_repository.path
-
-      File.exist?(File.join(project_path, 'hooks', 'post-receive'))
-    end
-  end
 end
diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb
index a9d14070177..c3e912b02c5 100644
--- a/spec/tasks/gitlab/shell_rake_spec.rb
+++ b/spec/tasks/gitlab/shell_rake_spec.rb
@@ -7,14 +7,8 @@ describe 'gitlab:shell rake tasks' do
     stub_warn_user_is_not_gitlab
   end
 
-  after do
-    TestEnv.sabotage_gitlab_shell_hooks
-  end
-
   describe 'install task' do
-    it 'invokes create_hooks task' do
-      expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke)
-
+    it 'installs and compiles gitlab-shell' do
       storages = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
         Gitlab.config.repositories.storages.values.map(&:legacy_disk_path)
       end
@@ -24,14 +18,4 @@ describe 'gitlab:shell rake tasks' do
       run_rake_task('gitlab:shell:install')
     end
   end
-
-  describe 'create_hooks task' do
-    it 'calls gitlab-shell bin/create_hooks' do
-      expect_any_instance_of(Object).to receive(:system)
-        .with("#{Gitlab.config.gitlab_shell.path}/bin/create-hooks",
-              *Gitlab::TaskHelpers.repository_storage_paths_args)
-
-      run_rake_task('gitlab:shell:create_hooks')
-    end
-  end
 end
diff --git a/spec/tasks/tokens_spec.rb b/spec/tasks/tokens_spec.rb
index 555a58e9aa1..4188e7caccb 100644
--- a/spec/tasks/tokens_spec.rb
+++ b/spec/tasks/tokens_spec.rb
@@ -8,13 +8,13 @@ describe 'tokens rake tasks' do
   end
 
   describe 'reset_all_email task' do
-    it 'invokes create_hooks task' do
+    it 'changes the incoming email token' do
       expect { run_rake_task('tokens:reset_all_email') }.to change { user.reload.incoming_email_token }
     end
   end
 
   describe 'reset_all_feed task' do
-    it 'invokes create_hooks task' do
+    it 'changes the feed token for the user' do
       expect { run_rake_task('tokens:reset_all_feed') }.to change { user.reload.feed_token }
     end
   end