diff --git a/changelogs/unreleased/zj-stop-symlinking-hooks.yml b/changelogs/unreleased/zj-stop-symlinking-hooks.yml new file mode 100644 index 0000000000000000000000000000000000000000..b366b813e13cd78486929e2e653cbecd9e7dd121 --- /dev/null +++ b/changelogs/unreleased/zj-stop-symlinking-hooks.yml @@ -0,0 +1,6 @@ +--- +title: Stop symlinking hooks on repository creation +merge_request: 1052 +author: +type: added + diff --git a/internal/service/repository/create_from_snapshot_test.go b/internal/service/repository/create_from_snapshot_test.go index e252ce9e24aab9213369ab31810ad591eb4c0039..b092479d07897376717c009e2cc49c51770b0120 100644 --- a/internal/service/repository/create_from_snapshot_test.go +++ b/internal/service/repository/create_from_snapshot_test.go @@ -106,10 +106,6 @@ func TestCreateRepositoryFromSnapshotSuccess(t *testing.T) { // hooks/ and config were excluded, but the RPC should create them require.FileExists(t, filepath.Join(repoPath, "config"), "Config file not created") - - fi, err := os.Lstat(filepath.Join(repoPath, "hooks")) - require.NoError(t, err) - require.Equal(t, os.ModeSymlink, fi.Mode()&os.ModeSymlink, "Symlink to global hooks not created") } func TestCreateRepositoryFromSnapshotFailsIfRepositoryExists(t *testing.T) { diff --git a/internal/service/repository/create_test.go b/internal/service/repository/create_test.go index 7a36c40f7fe9ddf1078971a3ccd27a0de8e2bccd..e238f8e186becc72b10bcabf7291ae8fd65c9cc9 100644 --- a/internal/service/repository/create_test.go +++ b/internal/service/repository/create_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" - "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/testhelper" "google.golang.org/grpc/codes" @@ -40,16 +39,6 @@ func TestCreateRepositorySuccess(t *testing.T) { require.NoError(t, err) require.True(t, fi.IsDir(), "%q must be a directory", fi.Name()) } - - hooksDir := path.Join(repoDir, "hooks") - - fi, err := os.Lstat(hooksDir) - require.NoError(t, err) - require.True(t, fi.Mode()&os.ModeSymlink > 0, "expected %q to be a symlink, got mode %v", hooksDir, fi.Mode()) - - hooksTarget, err := os.Readlink(hooksDir) - require.NoError(t, err) - require.Equal(t, path.Join(config.Config.GitlabShell.Dir, "hooks"), hooksTarget) } func TestCreateRepositoryFailure(t *testing.T) { diff --git a/ruby/lib/gitlab/git/repository.rb b/ruby/lib/gitlab/git/repository.rb index 73373a0a9f16f7bbd017a8b5dea2a6afb9ca3804..f511b3ef69606b16872792884db6d332d84ad4ee 100644 --- a/ruby/lib/gitlab/git/repository.rb +++ b/ruby/lib/gitlab/git/repository.rb @@ -61,37 +61,6 @@ module Gitlab # Equivalent to `git --git-path=#{repo_path} init [--bare]` repo = Rugged::Repository.init_at(repo_path, true) repo.close - - # TODO: stop symlinking to the old hooks location in or after GitLab 12.0. - # https://gitlab.com/gitlab-org/gitaly/issues/1392 - create_hooks(repo_path, Gitlab::Git::Hook.legacy_hooks_directory) - end - - 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 diff --git a/ruby/spec/lib/gitlab/git/repository_spec.rb b/ruby/spec/lib/gitlab/git/repository_spec.rb index 962be8fc6a52e265d9a3afb7a58020e9c8313422..57b88ef597444e928f630a468050d3cddb57101f 100644 --- a/ruby/spec/lib/gitlab/git/repository_spec.rb +++ b/ruby/spec/lib/gitlab/git/repository_spec.rb @@ -37,53 +37,6 @@ describe Gitlab::Git::Repository do # rubocop:disable Metrics/BlockLength end end - 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::Git::Hook.legacy_hooks_directory } - let(:existing_target) { File.join(repo_path, 'foobar') } - - before do - GitlabShellHelper.setup_gitlab_shell - - 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 }