diff options
| -rw-r--r-- | Gemfile | 3 | ||||
| -rw-r--r-- | Gemfile.lock | 7 | ||||
| -rw-r--r-- | app/controllers/registrations_controller.rb | 2 | ||||
| -rw-r--r-- | app/services/users/build_service.rb | 100 | ||||
| -rw-r--r-- | app/services/users/create_service.rb | 95 | ||||
| -rw-r--r-- | app/views/projects/show.html.haml | 2 | ||||
| -rw-r--r-- | app/views/shared/_import_form.html.haml | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/30349-create-users-build-service.yml | 4 | ||||
| -rw-r--r-- | changelogs/unreleased/use-hashie-forbidden_attributes.yml | 4 | ||||
| -rw-r--r-- | lib/gitlab/o_auth/user.rb | 2 | ||||
| -rw-r--r-- | lib/tasks/gitlab/gitaly.rake | 4 | ||||
| -rw-r--r-- | lib/tasks/gitlab/shell.rake | 7 | ||||
| -rw-r--r-- | lib/tasks/gitlab/task_helpers.rb | 41 | ||||
| -rw-r--r-- | lib/tasks/gitlab/workhorse.rake | 4 | ||||
| -rw-r--r-- | spec/services/users/build_service_spec.rb | 55 | ||||
| -rw-r--r-- | spec/services/users/create_service_spec.rb | 78 | ||||
| -rw-r--r-- | spec/tasks/gitlab/gitaly_rake_spec.rb | 12 | ||||
| -rw-r--r-- | spec/tasks/gitlab/task_helpers_spec.rb | 73 | ||||
| -rw-r--r-- | spec/tasks/gitlab/workhorse_rake_spec.rb | 12 |
19 files changed, 257 insertions, 250 deletions
@@ -73,6 +73,9 @@ gem 'grape', '~> 0.19.0' gem 'grape-entity', '~> 0.6.0' gem 'rack-cors', '~> 0.4.0', require: 'rack/cors' +# Disable strong_params so that Mash does not respond to :permitted? +gem 'hashie-forbidden_attributes' + # Pagination gem 'kaminari', '~> 0.17.0' diff --git a/Gemfile.lock b/Gemfile.lock index 965c888ca79..bb91db1e805 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -330,7 +330,7 @@ GEM grape-entity (0.6.0) activesupport multi_json (>= 1.3.2) - grpc (1.2.2) + grpc (1.1.2) google-protobuf (~> 3.1) googleauth (~> 0.5.1) haml (4.0.7) @@ -346,6 +346,8 @@ GEM tilt hashdiff (0.3.2) hashie (3.5.5) + hashie-forbidden_attributes (0.1.1) + hashie (>= 3.0) health_check (2.6.0) rails (>= 4.0) hipchat (1.5.2) @@ -915,6 +917,7 @@ DEPENDENCIES grape-entity (~> 0.6.0) haml_lint (~> 0.21.0) hamlit (~> 2.6.1) + hashie-forbidden_attributes health_check (~> 2.6.0) hipchat (~> 1.5.0) html-pipeline (~> 1.11.0) @@ -1035,4 +1038,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.14.5 + 1.14.6 diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 8109427a45f..3ca14dee33c 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -60,7 +60,7 @@ class RegistrationsController < Devise::RegistrationsController end def resource - @resource ||= Users::CreateService.new(current_user, sign_up_params).build + @resource ||= Users::BuildService.new(current_user, sign_up_params).execute end def devise_mapping diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb new file mode 100644 index 00000000000..9a0a5a12f91 --- /dev/null +++ b/app/services/users/build_service.rb @@ -0,0 +1,100 @@ +module Users + # Service for building a new user. + class BuildService < BaseService + def initialize(current_user, params = {}) + @current_user = current_user + @params = params.dup + end + + def execute + raise Gitlab::Access::AccessDeniedError unless can_create_user? + + user = User.new(build_user_params) + + if current_user&.admin? + if params[:reset_password] + user.generate_reset_token + params[:force_random_password] = true + end + + if params[:force_random_password] + random_password = Devise.friendly_token.first(Devise.password_length.min) + user.password = user.password_confirmation = random_password + end + end + + identity_attrs = params.slice(:extern_uid, :provider) + + if identity_attrs.any? + user.identities.build(identity_attrs) + end + + user + end + + private + + def can_create_user? + (current_user.nil? && current_application_settings.signup_enabled?) || current_user&.admin? + end + + # Allowed params for creating a user (admins only) + def admin_create_params + [ + :access_level, + :admin, + :avatar, + :bio, + :can_create_group, + :color_scheme_id, + :email, + :external, + :force_random_password, + :hide_no_password, + :hide_no_ssh_key, + :key_id, + :linkedin, + :name, + :password, + :password_automatically_set, + :password_expires_at, + :projects_limit, + :remember_me, + :skip_confirmation, + :skype, + :theme_id, + :twitter, + :username, + :website_url + ] + end + + # Allowed params for user signup + def signup_params + [ + :email, + :email_confirmation, + :password_automatically_set, + :name, + :password, + :username + ] + end + + def build_user_params + if current_user&.admin? + user_params = params.slice(*admin_create_params) + user_params[:created_by_id] = current_user&.id + + if params[:reset_password] + user_params.merge!(force_random_password: true, password_expires_at: nil) + end + else + user_params = params.slice(*signup_params) + user_params[:skip_confirmation] = !current_application_settings.send_user_confirmation_email + end + + user_params + end + end +end diff --git a/app/services/users/create_service.rb b/app/services/users/create_service.rb index 93ca7b1141a..a2105d31f71 100644 --- a/app/services/users/create_service.rb +++ b/app/services/users/create_service.rb @@ -6,34 +6,10 @@ module Users @params = params.dup end - def build - raise Gitlab::Access::AccessDeniedError unless can_create_user? - - user = User.new(build_user_params) - - if current_user&.admin? - if params[:reset_password] - @reset_token = user.generate_reset_token - params[:force_random_password] = true - end - - if params[:force_random_password] - random_password = Devise.friendly_token.first(Devise.password_length.min) - user.password = user.password_confirmation = random_password - end - end - - identity_attrs = params.slice(:extern_uid, :provider) - - if identity_attrs.any? - user.identities.build(identity_attrs) - end - - user - end - def execute - user = build + user = Users::BuildService.new(current_user, params).execute + + @reset_token = user.generate_reset_token if user.recently_sent_password_reset? if user.save log_info("User \"#{user.name}\" (#{user.email}) was created") @@ -43,70 +19,5 @@ module Users user end - - private - - def can_create_user? - (current_user.nil? && current_application_settings.signup_enabled?) || current_user&.admin? - end - - # Allowed params for creating a user (admins only) - def admin_create_params - [ - :access_level, - :admin, - :avatar, - :bio, - :can_create_group, - :color_scheme_id, - :email, - :external, - :force_random_password, - :password_automatically_set, - :hide_no_password, - :hide_no_ssh_key, - :key_id, - :linkedin, - :name, - :password, - :password_expires_at, - :projects_limit, - :remember_me, - :skip_confirmation, - :skype, - :theme_id, - :twitter, - :username, - :website_url - ] - end - - # Allowed params for user signup - def signup_params - [ - :email, - :email_confirmation, - :password_automatically_set, - :name, - :password, - :username - ] - end - - def build_user_params - if current_user&.admin? - user_params = params.slice(*admin_create_params) - user_params[:created_by_id] = current_user&.id - - if params[:reset_password] - user_params.merge!(force_random_password: true, password_expires_at: nil) - end - else - user_params = params.slice(*signup_params) - user_params[:skip_confirmation] = !current_application_settings.send_user_confirmation_email - end - - user_params - end end end diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index de1229d58aa..fd7bd21677c 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -12,7 +12,7 @@ = render "projects/last_push" = render "home_panel" -- if current_user && can?(current_user, :download_code, @project) +- if can?(current_user, :download_code, @project) %nav.project-stats{ class: container_class } %ul.nav %li diff --git a/app/views/shared/_import_form.html.haml b/app/views/shared/_import_form.html.haml index 54b5ae2402e..1c7c73be933 100644 --- a/app/views/shared/_import_form.html.haml +++ b/app/views/shared/_import_form.html.haml @@ -2,7 +2,7 @@ = f.label :import_url, class: 'control-label' do %span Git repository URL .col-sm-10 - = f.text_field :import_url, autocomplete: 'off', class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git', disabled: true + = f.text_field :import_url, autocomplete: 'off', class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git' .well.prepend-top-20 %ul diff --git a/changelogs/unreleased/30349-create-users-build-service.yml b/changelogs/unreleased/30349-create-users-build-service.yml new file mode 100644 index 00000000000..49b571f5646 --- /dev/null +++ b/changelogs/unreleased/30349-create-users-build-service.yml @@ -0,0 +1,4 @@ +--- +title: Implement Users::BuildService +merge_request: 30349 +author: George Andrinopoulos diff --git a/changelogs/unreleased/use-hashie-forbidden_attributes.yml b/changelogs/unreleased/use-hashie-forbidden_attributes.yml new file mode 100644 index 00000000000..4f429b03a0d --- /dev/null +++ b/changelogs/unreleased/use-hashie-forbidden_attributes.yml @@ -0,0 +1,4 @@ +--- +title: Add hashie-forbidden_attributes gem +merge_request: 10579 +author: Andy Brown diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index f98481c6d3a..6e42d8941fb 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -148,7 +148,7 @@ module Gitlab def build_new_user user_params = user_attributes.merge(extern_uid: auth_hash.uid, provider: auth_hash.provider, skip_confirmation: true) - Users::CreateService.new(nil, user_params).build + Users::BuildService.new(nil, user_params).execute end def user_attributes diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 9f6cfe3957c..8079c6e416c 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -7,10 +7,10 @@ namespace :gitlab do abort %(Please specify the directory where you want to install gitaly:\n rake "gitlab:gitaly:install[/home/git/gitaly]") end - tag = "v#{Gitlab::GitalyClient.expected_server_version}" + version = Gitlab::GitalyClient.expected_server_version repo = 'https://gitlab.com/gitlab-org/gitaly.git' - checkout_or_clone_tag(tag: tag, repo: repo, target_dir: args.dir) + checkout_or_clone_version(version: version, repo: repo, target_dir: args.dir) _, status = Gitlab::Popen.popen(%w[which gmake]) command = status.zero? ? 'gmake' : 'make' diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index dd2fda54e62..95687066819 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -1,19 +1,18 @@ namespace :gitlab do namespace :shell do desc "GitLab | Install or upgrade gitlab-shell" - task :install, [:tag, :repo] => :environment do |t, args| + task :install, [:repo] => :environment do |t, args| warn_user_is_not_gitlab default_version = Gitlab::Shell.version_required - default_version_tag = "v#{default_version}" - args.with_defaults(tag: default_version_tag, repo: 'https://gitlab.com/gitlab-org/gitlab-shell.git') + args.with_defaults(repo: 'https://gitlab.com/gitlab-org/gitlab-shell.git') gitlab_url = Gitlab.config.gitlab.url # gitlab-shell requires a / at the end of the url gitlab_url += '/' unless gitlab_url.end_with?('/') target_dir = Gitlab.config.gitlab_shell.path - checkout_or_clone_tag(tag: default_version_tag, repo: args.repo, target_dir: target_dir) + checkout_or_clone_version(version: default_version, repo: args.repo, target_dir: target_dir) # Make sure we're on the right tag Dir.chdir(target_dir) do diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index cdba2262bc2..e3c9d3b491c 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -147,41 +147,30 @@ module Gitlab Rails.env.test? ? Rails.root.join('tmp/tests') : Gitlab.config.gitlab.user_home end - def checkout_or_clone_tag(tag:, repo:, target_dir:) - if Dir.exist?(target_dir) - checkout_tag(tag, target_dir) - else - clone_repo(repo, target_dir) - end + def checkout_or_clone_version(version:, repo:, target_dir:) + version = + if version.starts_with?("=") + version.sub(/\A=/, '') # tag or branch + else + "v#{version}" # tag + end - reset_to_tag(tag, target_dir) + clone_repo(repo, target_dir) unless Dir.exist?(target_dir) + checkout_version(version, target_dir) + reset_to_version(version, target_dir) end def clone_repo(repo, target_dir) run_command!(%W[#{Gitlab.config.git.bin_path} clone -- #{repo} #{target_dir}]) end - def checkout_tag(tag, target_dir) - run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} fetch --tags --quiet]) - run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} checkout --quiet #{tag}]) + def checkout_version(version, target_dir) + run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} fetch --quiet]) + run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} checkout --quiet #{version}]) end - def reset_to_tag(tag_wanted, target_dir) - tag = - begin - # First try to checkout without fetching - # to avoid stalling tests if the Internet is down. - run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} describe -- #{tag_wanted}]) - rescue Gitlab::TaskFailedError - run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} fetch origin]) - run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} describe -- origin/#{tag_wanted}]) - end - - if tag - run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} reset --hard #{tag.strip}]) - else - raise Gitlab::TaskFailedError - end + def reset_to_version(version, target_dir) + run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} reset --hard #{version}]) end end end diff --git a/lib/tasks/gitlab/workhorse.rake b/lib/tasks/gitlab/workhorse.rake index baea94bf8ca..a00b02188cf 100644 --- a/lib/tasks/gitlab/workhorse.rake +++ b/lib/tasks/gitlab/workhorse.rake @@ -7,10 +7,10 @@ namespace :gitlab do abort %(Please specify the directory where you want to install gitlab-workhorse:\n rake "gitlab:workhorse:install[/home/git/gitlab-workhorse]") end - tag = "v#{Gitlab::Workhorse.version}" + version = Gitlab::Workhorse.version repo = 'https://gitlab.com/gitlab-org/gitlab-workhorse.git' - checkout_or_clone_tag(tag: tag, repo: repo, target_dir: args.dir) + checkout_or_clone_version(version: version, repo: repo, target_dir: args.dir) _, status = Gitlab::Popen.popen(%w[which gmake]) command = status.zero? ? 'gmake' : 'make' diff --git a/spec/services/users/build_service_spec.rb b/spec/services/users/build_service_spec.rb new file mode 100644 index 00000000000..2a6bfc1b3a0 --- /dev/null +++ b/spec/services/users/build_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Users::BuildService, services: true do + describe '#execute' do + let(:params) do + { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' } + end + + context 'with an admin user' do + let(:admin_user) { create(:admin) } + let(:service) { described_class.new(admin_user, params) } + + it 'returns a valid user' do + expect(service.execute).to be_valid + end + end + + context 'with non admin user' do + let(:user) { create(:user) } + let(:service) { described_class.new(user, params) } + + it 'raises AccessDeniedError exception' do + expect { service.execute }.to raise_error Gitlab::Access::AccessDeniedError + end + end + + context 'with nil user' do + let(:service) { described_class.new(nil, params) } + + it 'returns a valid user' do + expect(service.execute).to be_valid + end + + context 'when "send_user_confirmation_email" application setting is true' do + before do + stub_application_setting(send_user_confirmation_email: true, signup_enabled?: true) + end + + it 'does not confirm the user' do + expect(service.execute).not_to be_confirmed + end + end + + context 'when "send_user_confirmation_email" application setting is false' do + before do + stub_application_setting(send_user_confirmation_email: false, signup_enabled?: true) + end + + it 'confirms the user' do + expect(service.execute).to be_confirmed + end + end + end + end +end diff --git a/spec/services/users/create_service_spec.rb b/spec/services/users/create_service_spec.rb index a111aec2f89..75746278573 100644 --- a/spec/services/users/create_service_spec.rb +++ b/spec/services/users/create_service_spec.rb @@ -1,38 +1,6 @@ require 'spec_helper' describe Users::CreateService, services: true do - describe '#build' do - let(:params) do - { name: 'John Doe', username: 'jduser', email: 'jd@example.com', password: 'mydummypass' } - end - - context 'with an admin user' do - let(:admin_user) { create(:admin) } - let(:service) { described_class.new(admin_user, params) } - - it 'returns a valid user' do - expect(service.build).to be_valid - end - end - - context 'with non admin user' do - let(:user) { create(:user) } - let(:service) { described_class.new(user, params) } - - it 'raises AccessDeniedError exception' do - expect { service.build }.to raise_error Gitlab::Access::AccessDeniedError - end - end - - context 'with nil user' do - let(:service) { described_class.new(nil, params) } - - it 'returns a valid user' do - expect(service.build).to be_valid - end - end - end - describe '#execute' do let(:admin_user) { create(:admin) } @@ -185,40 +153,18 @@ describe Users::CreateService, services: true do end let(:service) { described_class.new(nil, params) } - context 'when "send_user_confirmation_email" application setting is true' do - before do - current_application_settings = double(:current_application_settings, send_user_confirmation_email: true, signup_enabled?: true) - allow(service).to receive(:current_application_settings).and_return(current_application_settings) - end - - it 'does not confirm the user' do - expect(service.execute).not_to be_confirmed - end - end - - context 'when "send_user_confirmation_email" application setting is false' do - before do - current_application_settings = double(:current_application_settings, send_user_confirmation_email: false, signup_enabled?: true) - allow(service).to receive(:current_application_settings).and_return(current_application_settings) - end - - it 'confirms the user' do - expect(service.execute).to be_confirmed - end - - it 'persists the given attributes' do - user = service.execute - user.reload - - expect(user).to have_attributes( - name: params[:name], - username: params[:username], - email: params[:email], - password: params[:password], - created_by_id: nil, - admin: false - ) - end + it 'persists the given attributes' do + user = service.execute + user.reload + + expect(user).to have_attributes( + name: params[:name], + username: params[:username], + email: params[:email], + password: params[:password], + created_by_id: nil, + admin: false + ) end end end diff --git a/spec/tasks/gitlab/gitaly_rake_spec.rb b/spec/tasks/gitlab/gitaly_rake_spec.rb index b369dcbb305..aaf998a546f 100644 --- a/spec/tasks/gitlab/gitaly_rake_spec.rb +++ b/spec/tasks/gitlab/gitaly_rake_spec.rb @@ -8,7 +8,7 @@ describe 'gitlab:gitaly namespace rake task' do describe 'install' do let(:repo) { 'https://gitlab.com/gitlab-org/gitaly.git' } let(:clone_path) { Rails.root.join('tmp/tests/gitaly').to_s } - let(:tag) { "v#{File.read(Rails.root.join(Gitlab::GitalyClient::SERVER_VERSION_FILE)).chomp}" } + let(:version) { File.read(Rails.root.join(Gitlab::GitalyClient::SERVER_VERSION_FILE)).chomp } context 'no dir given' do it 'aborts and display a help message' do @@ -21,7 +21,7 @@ describe 'gitlab:gitaly namespace rake task' do context 'when an underlying Git command fail' do it 'aborts and display a help message' do expect_any_instance_of(Object). - to receive(:checkout_or_clone_tag).and_raise 'Git error' + to receive(:checkout_or_clone_version).and_raise 'Git error' expect { run_rake_task('gitlab:gitaly:install', clone_path) }.to raise_error 'Git error' end @@ -32,9 +32,9 @@ describe 'gitlab:gitaly namespace rake task' do expect(Dir).to receive(:chdir).with(clone_path) end - it 'calls checkout_or_clone_tag with the right arguments' do + it 'calls checkout_or_clone_version with the right arguments' do expect_any_instance_of(Object). - to receive(:checkout_or_clone_tag).with(tag: tag, repo: repo, target_dir: clone_path) + to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path) run_rake_task('gitlab:gitaly:install', clone_path) end @@ -48,7 +48,7 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_tag) + expect_any_instance_of(Object).to receive(:checkout_or_clone_version) allow_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) end @@ -62,7 +62,7 @@ describe 'gitlab:gitaly namespace rake task' do context 'gmake is not available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_tag) + expect_any_instance_of(Object).to receive(:checkout_or_clone_version) allow_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) end diff --git a/spec/tasks/gitlab/task_helpers_spec.rb b/spec/tasks/gitlab/task_helpers_spec.rb index 86e42d845ce..3d9ba7cdc6f 100644 --- a/spec/tasks/gitlab/task_helpers_spec.rb +++ b/spec/tasks/gitlab/task_helpers_spec.rb @@ -10,19 +10,38 @@ describe Gitlab::TaskHelpers do let(:repo) { 'https://gitlab.com/gitlab-org/gitlab-test.git' } let(:clone_path) { Rails.root.join('tmp/tests/task_helpers_tests').to_s } + let(:version) { '1.1.0' } let(:tag) { 'v1.1.0' } - describe '#checkout_or_clone_tag' do + describe '#checkout_or_clone_version' do before do allow(subject).to receive(:run_command!) - expect(subject).to receive(:reset_to_tag).with(tag, clone_path) end - context 'target_dir does not exist' do - it 'clones the repo, retrieve the tag from origin, and checkout the tag' do + it 'checkout the version and reset to it' do + expect(subject).to receive(:checkout_version).with(tag, clone_path) + expect(subject).to receive(:reset_to_version).with(tag, clone_path) + + subject.checkout_or_clone_version(version: version, repo: repo, target_dir: clone_path) + end + + context 'with a branch version' do + let(:version) { '=branch_name' } + let(:branch) { 'branch_name' } + + it 'checkout the version and reset to it with a branch name' do + expect(subject).to receive(:checkout_version).with(branch, clone_path) + expect(subject).to receive(:reset_to_version).with(branch, clone_path) + + subject.checkout_or_clone_version(version: version, repo: repo, target_dir: clone_path) + end + end + + context "target_dir doesn't exist" do + it 'clones the repo' do expect(subject).to receive(:clone_repo).with(repo, clone_path) - subject.checkout_or_clone_tag(tag: tag, repo: repo, target_dir: clone_path) + subject.checkout_or_clone_version(version: version, repo: repo, target_dir: clone_path) end end @@ -31,10 +50,10 @@ describe Gitlab::TaskHelpers do expect(Dir).to receive(:exist?).and_return(true) end - it 'fetch and checkout the tag' do - expect(subject).to receive(:checkout_tag).with(tag, clone_path) + it "doesn't clone the repository" do + expect(subject).not_to receive(:clone_repo) - subject.checkout_or_clone_tag(tag: tag, repo: repo, target_dir: clone_path) + subject.checkout_or_clone_version(version: version, repo: repo, target_dir: clone_path) end end end @@ -48,49 +67,23 @@ describe Gitlab::TaskHelpers do end end - describe '#checkout_tag' do + describe '#checkout_version' do it 'clones the repo in the target dir' do expect(subject). - to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} fetch --tags --quiet]) + to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} fetch --quiet]) expect(subject). to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} checkout --quiet #{tag}]) - subject.checkout_tag(tag, clone_path) + subject.checkout_version(tag, clone_path) end end - describe '#reset_to_tag' do - let(:tag) { 'v1.1.0' } - before do + describe '#reset_to_version' do + it 'resets --hard to the given version' do expect(subject). to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} reset --hard #{tag}]) - end - context 'when the tag is not checked out locally' do - before do - expect(subject). - to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} describe -- #{tag}]).and_raise(Gitlab::TaskFailedError) - end - - it 'fetch origin, ensure the tag exists, and resets --hard to the given tag' do - expect(subject). - to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} fetch origin]) - expect(subject). - to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} describe -- origin/#{tag}]).and_return(tag) - - subject.reset_to_tag(tag, clone_path) - end - end - - context 'when the tag is checked out locally' do - before do - expect(subject). - to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} describe -- #{tag}]).and_return(tag) - end - - it 'resets --hard to the given tag' do - subject.reset_to_tag(tag, clone_path) - end + subject.reset_to_version(tag, clone_path) end end end diff --git a/spec/tasks/gitlab/workhorse_rake_spec.rb b/spec/tasks/gitlab/workhorse_rake_spec.rb index 8a66a4aa047..63d1cf2bbe5 100644 --- a/spec/tasks/gitlab/workhorse_rake_spec.rb +++ b/spec/tasks/gitlab/workhorse_rake_spec.rb @@ -8,7 +8,7 @@ describe 'gitlab:workhorse namespace rake task' do describe 'install' do let(:repo) { 'https://gitlab.com/gitlab-org/gitlab-workhorse.git' } let(:clone_path) { Rails.root.join('tmp/tests/gitlab-workhorse').to_s } - let(:tag) { "v#{File.read(Rails.root.join(Gitlab::Workhorse::VERSION_FILE)).chomp}" } + let(:version) { File.read(Rails.root.join(Gitlab::Workhorse::VERSION_FILE)).chomp } context 'no dir given' do it 'aborts and display a help message' do @@ -21,7 +21,7 @@ describe 'gitlab:workhorse namespace rake task' do context 'when an underlying Git command fail' do it 'aborts and display a help message' do expect_any_instance_of(Object). - to receive(:checkout_or_clone_tag).and_raise 'Git error' + to receive(:checkout_or_clone_version).and_raise 'Git error' expect { run_rake_task('gitlab:workhorse:install', clone_path) }.to raise_error 'Git error' end @@ -32,9 +32,9 @@ describe 'gitlab:workhorse namespace rake task' do expect(Dir).to receive(:chdir).with(clone_path) end - it 'calls checkout_or_clone_tag with the right arguments' do + it 'calls checkout_or_clone_version with the right arguments' do expect_any_instance_of(Object). - to receive(:checkout_or_clone_tag).with(tag: tag, repo: repo, target_dir: clone_path) + to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path) run_rake_task('gitlab:workhorse:install', clone_path) end @@ -48,7 +48,7 @@ describe 'gitlab:workhorse namespace rake task' do context 'gmake is available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_tag) + expect_any_instance_of(Object).to receive(:checkout_or_clone_version) allow_any_instance_of(Object).to receive(:run_command!).with(['gmake']).and_return(true) end @@ -62,7 +62,7 @@ describe 'gitlab:workhorse namespace rake task' do context 'gmake is not available' do before do - expect_any_instance_of(Object).to receive(:checkout_or_clone_tag) + expect_any_instance_of(Object).to receive(:checkout_or_clone_version) allow_any_instance_of(Object).to receive(:run_command!).with(['make']).and_return(true) end |