diff options
author | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-08-16 13:00:24 +0200 |
---|---|---|
committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2018-08-16 15:06:03 +0200 |
commit | 2d212e401e099543f7612e67ff35b4e8b10593c0 (patch) | |
tree | c8d38ce1c033d3ad5acf4407a1447f3b95eb6cf2 | |
parent | fb8606f65a60808e52539f71f09fba871b5aba6b (diff) | |
download | gitlab-shell-2d212e401e099543f7612e67ff35b4e8b10593c0.tar.gz |
Remove repo_path from GitlabShellzj-remove-repo-path
The internal api returns '/' from gitlab, since
`8fad07383ada021fc995294fd0fe0f77fe37da35` from GitLab CE. To clean up
later, https://gitlab.com/gitlab-org/gitlab-shell/issues/135 was
created.
This change closes that issue, making it possible to remove the field
from the response on GitLab-CE too. Given the Rails app always returns
`/` as the repository_path, the associated checks are basically a noop
too. The tests are updated and at times look a little fishy, but those
are testing code that is to be removed in another MR.
Closes https://gitlab.com/gitlab-org/gitlab-shell/issues/135
-rw-r--r-- | lib/gitlab_access_status.rb | 4 | ||||
-rw-r--r-- | lib/gitlab_shell.rb | 11 | ||||
-rw-r--r-- | spec/gitlab_access_spec.rb | 2 | ||||
-rw-r--r-- | spec/gitlab_shell_spec.rb | 33 |
4 files changed, 7 insertions, 43 deletions
diff --git a/lib/gitlab_access_status.rb b/lib/gitlab_access_status.rb index afb5a9e..96dd6e8 100644 --- a/lib/gitlab_access_status.rb +++ b/lib/gitlab_access_status.rb @@ -3,14 +3,13 @@ require 'json' class GitAccessStatus attr_reader :message, :gl_repository, :gl_id, :gl_username, :repository_path, :gitaly, :git_protocol, :git_config_options - def initialize(status, message, gl_repository:, gl_id:, gl_username:, repository_path:, gitaly:, git_protocol:, git_config_options:) + def initialize(status, message, gl_repository:, gl_id:, gl_username:, gitaly:, git_protocol:, git_config_options:) @status = status @message = message @gl_repository = gl_repository @gl_id = gl_id @gl_username = gl_username @git_config_options = git_config_options - @repository_path = repository_path @gitaly = gitaly @git_protocol = git_protocol end @@ -23,7 +22,6 @@ class GitAccessStatus gl_id: values["gl_id"], gl_username: values["gl_username"], git_config_options: values["git_config_options"], - repository_path: values["repository_path"], gitaly: values["gitaly"], git_protocol: values["git_protocol"]) end diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb index 11494e0..4cabd37 100644 --- a/lib/gitlab_shell.rb +++ b/lib/gitlab_shell.rb @@ -19,7 +19,6 @@ class GitlabShell # rubocop:disable Metrics/ClassLength GL_PROTOCOL = 'ssh'.freeze attr_accessor :gl_id, :gl_repository, :repo_name, :command, :git_access, :git_protocol - attr_reader :repo_path def initialize(who) who_sym, = GitlabNet.parse_who(who) @@ -116,7 +115,6 @@ class GitlabShell # rubocop:disable Metrics/ClassLength raise AccessDeniedError, status.message unless status.allowed? - self.repo_path = status.repository_path @gl_repository = status.gl_repository @git_protocol = ENV['GIT_PROTOCOL'] @gitaly = status.gitaly @@ -139,7 +137,7 @@ class GitlabShell # rubocop:disable Metrics/ClassLength end executable = @command - args = [repo_path] + args = [] if GITALY_MIGRATED_COMMANDS.key?(executable) && @gitaly executable = GITALY_MIGRATED_COMMANDS[executable] @@ -293,11 +291,4 @@ class GitlabShell # rubocop:disable Metrics/ClassLength return false end end - - def repo_path=(repo_path) - raise ArgumentError, "Repository path not provided. Please make sure you're using GitLab v8.10 or later." unless repo_path - raise InvalidRepositoryPathError if File.absolute_path(repo_path) != repo_path - - @repo_path = repo_path - end end diff --git a/spec/gitlab_access_spec.rb b/spec/gitlab_access_spec.rb index ffaac8a..a4f633d 100644 --- a/spec/gitlab_access_spec.rb +++ b/spec/gitlab_access_spec.rb @@ -13,7 +13,6 @@ describe GitlabAccess do gl_id: 'user-123', gl_username: 'testuser', git_config_options: ['receive.MaxInputSize=10000'], - repository_path: '/home/git/repositories', gitaly: nil, git_protocol: 'version=2')) end @@ -51,7 +50,6 @@ describe GitlabAccess do gl_id: nil, gl_username: nil, git_config_options: nil, - repository_path: nil, gitaly: nil, git_protocol: nil )) diff --git a/spec/gitlab_shell_spec.rb b/spec/gitlab_shell_spec.rb index 8b08f35..96c4878 100644 --- a/spec/gitlab_shell_spec.rb +++ b/spec/gitlab_shell_spec.rb @@ -29,7 +29,6 @@ describe GitlabShell do gl_id: gl_id, gl_username: gl_username, git_config_options: git_config_options, - repository_path: repo_path, gitaly: { 'repository' => { 'relative_path' => repo_name, 'storage_name' => 'default'} , 'address' => 'unix:gitaly.socket' }, git_protocol: git_protocol ) @@ -45,7 +44,6 @@ describe GitlabShell do gl_id: gl_id, gl_username: gl_username, git_config_options: nil, - repository_path: repo_path, gitaly: nil, git_protocol: git_protocol)) allow(api).to receive(:two_factor_recovery_codes).and_return({ @@ -60,7 +58,6 @@ describe GitlabShell do let(:tmp_repos_path) { File.join(ROOT_PATH, 'tmp', 'repositories') } let(:repo_name) { 'gitlab-ci.git' } - let(:repo_path) { File.join(tmp_repos_path, repo_name) } let(:gl_repository) { 'project-1' } let(:gl_id) { 'user-1' } let(:gl_username) { 'testuser' } @@ -189,13 +186,13 @@ describe GitlabShell do end it "should execute the command" do - expect(subject).to receive(:exec_cmd).with('git-upload-pack', repo_path) + expect(subject).to receive(:exec_cmd).with('git-upload-pack') end it "should log the command execution" do message = "executing git command" user_string = "user with id #{gl_id}" - expect($logger).to receive(:info).with(message, command: "git-upload-pack #{repo_path}", user: user_string) + expect($logger).to receive(:info).with(message, command: "git-upload-pack", user: user_string) end it "should use usernames if configured to do so" do @@ -248,13 +245,13 @@ describe GitlabShell do end it "should execute the command" do - expect(subject).to receive(:exec_cmd).with('git-receive-pack', repo_path) + expect(subject).to receive(:exec_cmd).with('git-receive-pack') end it "should log the command execution" do message = "executing git command" user_string = "user with id #{gl_id}" - expect($logger).to receive(:info).with(message, command: "git-receive-pack #{repo_path}", user: user_string) + expect($logger).to receive(:info).with(message, command: "git-receive-pack", user: user_string) end end @@ -287,7 +284,7 @@ describe GitlabShell do shared_examples_for 'upload-archive' do |command| let(:ssh_cmd) { "#{command} gitlab-ci.git" } - let(:exec_cmd_params) { ['git-upload-archive', repo_path] } + let(:exec_cmd_params) { ['git-upload-archive'] } let(:exec_cmd_log_params) { exec_cmd_params } after { subject.exec(ssh_cmd) } @@ -441,7 +438,6 @@ describe GitlabShell do gl_id: nil, gl_username: nil, git_config_options: nil, - repository_path: nil, gitaly: nil, git_protocol: nil)) message = 'Access denied' @@ -449,25 +445,6 @@ describe GitlabShell do expect($logger).to receive(:warn).with(message, command: 'git-upload-pack gitlab-ci.git', user: user_string) end end - - describe 'set the repository path' do - context 'with a correct path' do - before { subject.exec(ssh_cmd) } - - it { expect(subject.repo_path).to eq repo_path } - end - - context "with a path that doesn't match an absolute path" do - before do - allow(File).to receive(:absolute_path) { 'y/gitlab-ci.git' } - end - - it "refuses to assign the path" do - expect($stderr).to receive(:puts).with("GitLab: Invalid repository path") - expect(subject.exec(ssh_cmd)).to be_falsey - end - end - end end describe :exec_cmd do |