Remove hook directory requirement from Shell

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
This commit is contained in:
Zeger-Jan van de Weg 2019-05-29 14:07:48 +02:00
parent 71635afcb3
commit 951afba624
No known key found for this signature in database
GPG Key ID: 65F6A8D64A88ABAC
10 changed files with 3 additions and 155 deletions

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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)

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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