diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index b4b0dfc3eb89080970c28a5bbe4b9737f2bb06bd..12e4a6999ae8795cb16fbbeb8880b8ef29c0d944 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -40,13 +40,15 @@ class Projects::ApplicationController < ApplicationController (current_user && current_user.already_forked?(project)) end - def authorize_project!(action) - return access_denied! unless can?(current_user, action, project) + def authorize_action!(action) + unless can?(current_user, action, project) + return access_denied! + end end def method_missing(method_sym, *arguments, &block) if method_sym.to_s =~ /\Aauthorize_(.*)!\z/ - authorize_project!($1.to_sym) + authorize_action!($1.to_sym) else super end diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index e24fc45d1666061a787325bed876eec5e78d053b..0fd35bcb790c9175e20a28e67e26a0ef27b9ca2c 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -1,7 +1,11 @@ class Projects::BuildsController < Projects::ApplicationController before_action :build, except: [:index, :cancel_all] - before_action :authorize_read_build!, only: [:index, :show, :status, :raw, :trace] - before_action :authorize_update_build!, except: [:index, :show, :status, :raw, :trace] + + before_action :authorize_read_build!, + only: [:index, :show, :status, :raw, :trace] + before_action :authorize_update_build!, + except: [:index, :show, :status, :raw, :trace, :cancel_all] + layout 'project' def index @@ -28,7 +32,12 @@ class Projects::BuildsController < Projects::ApplicationController end def cancel_all - @project.builds.running_or_pending.each(&:cancel) + return access_denied! unless can?(current_user, :update_build, project) + + @project.builds.running_or_pending.each do |build| + build.cancel if can?(current_user, :update_build, build) + end + redirect_to namespace_project_builds_path(project.namespace, project) end @@ -107,8 +116,13 @@ class Projects::BuildsController < Projects::ApplicationController private + def authorize_update_build! + return access_denied! unless can?(current_user, :update_build, build) + end + def build - @build ||= project.builds.find_by!(id: params[:id]).present(current_user: current_user) + @build ||= project.builds.find(params[:id]) + .present(current_user: current_user) end def build_path(build) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b426c27afbb2cd77d6bf8ea54e06acdae483304a..971ab7cb0ee131b325b5cfb2f222fabfa3856600 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -111,14 +111,9 @@ module Ci end def play(current_user) - # Try to queue a current build - if self.enqueue - self.update(user: current_user) - self - else - # Otherwise we need to create a duplicate - Ci::Build.retry(self, current_user) - end + Ci::PlayBuildService + .new(project, current_user) + .execute(self) end def cancelable? diff --git a/app/models/deployment.rb b/app/models/deployment.rb index afad001d50f915a4e369de7803556c6d2f659751..37adfb4de73420188d8265793f06937affa0de46 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -85,8 +85,8 @@ class Deployment < ActiveRecord::Base end def stop_action - return nil unless on_stop.present? - return nil unless manual_actions + return unless on_stop.present? + return unless manual_actions @stop_action ||= manual_actions.find_by(name: on_stop) end diff --git a/app/policies/base_policy.rb b/app/policies/base_policy.rb index 8890409d0565e8660af28c96485b1ad82eb1419d..623424c63e087aebbbba35eb47f90a4453da74b6 100644 --- a/app/policies/base_policy.rb +++ b/app/policies/base_policy.rb @@ -97,6 +97,10 @@ class BasePolicy rules end + def rules + raise NotImplementedError + end + def delegate!(new_subject) @rule_set.merge(Ability.allowed(@user, new_subject)) end diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb index 8b25332b73ceeeb2635ae78d1d0665b36dca8017..d4af4490608622bf64494f489249d5f21bb858ee 100644 --- a/app/policies/ci/build_policy.rb +++ b/app/policies/ci/build_policy.rb @@ -1,5 +1,7 @@ module Ci class BuildPolicy < CommitStatusPolicy + alias_method :build, :subject + def rules super @@ -8,6 +10,20 @@ module Ci %w[read create update admin].each do |rule| cannot! :"#{rule}_commit_status" unless can? :"#{rule}_build" end + + if can?(:update_build) && protected_action? + cannot! :update_build + end + end + + private + + def protected_action? + return false unless build.action? + + !::Gitlab::UserAccess + .new(user, project: build.project) + .can_push_to_branch?(build.ref) end end end diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index 3d2eef1c50cf95b13a188870ec98a38355ceb1a3..10aa2d3e72a5d857ff51d0c831cda108d8105786 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -1,4 +1,7 @@ module Ci - class PipelinePolicy < BuildPolicy + class PipelinePolicy < BasePolicy + def rules + delegate! @subject.project + end end end diff --git a/app/policies/environment_policy.rb b/app/policies/environment_policy.rb index f4219569161e92d36d054a70237e8271c4cb35ba..2fa15e645629ba69cf14112a380e64f3631ff771 100644 --- a/app/policies/environment_policy.rb +++ b/app/policies/environment_policy.rb @@ -1,5 +1,17 @@ class EnvironmentPolicy < BasePolicy + alias_method :environment, :subject + def rules - delegate! @subject.project + delegate! environment.project + + if can?(:create_deployment) && environment.stop_action? + can! :stop_environment if can_play_stop_action? + end + end + + private + + def can_play_stop_action? + Ability.allowed?(user, :update_build, environment.stop_action) end end diff --git a/app/serializers/build_action_entity.rb b/app/serializers/build_action_entity.rb index 184b4b7a6813eff1467842733230534d2d37c029..75dda1af709b12314d4083f2c632368defb9e330 100644 --- a/app/serializers/build_action_entity.rb +++ b/app/serializers/build_action_entity.rb @@ -13,4 +13,12 @@ class BuildActionEntity < Grape::Entity end expose :playable?, as: :playable + + private + + alias_method :build, :object + + def playable? + build.playable? && can?(request.user, :update_build, build) + end end diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb index b804d6d0e8a7bfec154adf965bf1adfba8155e6c..1380b347d8e96e35ecaba6fce9c9ac9c39f1d26a 100644 --- a/app/serializers/build_entity.rb +++ b/app/serializers/build_entity.rb @@ -12,7 +12,7 @@ class BuildEntity < Grape::Entity path_to(:retry_namespace_project_build, build) end - expose :play_path, if: ->(build, _) { build.playable? } do |build| + expose :play_path, if: -> (*) { playable? } do |build| path_to(:play_namespace_project_build, build) end @@ -25,11 +25,15 @@ class BuildEntity < Grape::Entity alias_method :build, :object - def path_to(route, build) - send("#{route}_path", build.project.namespace, build.project, build) + def playable? + build.playable? && can?(request.user, :update_build, build) end def detailed_status build.detailed_status(request.user) end + + def path_to(route, build) + send("#{route}_path", build.project.namespace, build.project, build) + end end diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb index ad8b4d43e8f0793858a6ddd42ff66d89ad39ab17..7eb7aac72eb5a7f337ac6d3cdd45c97646677150 100644 --- a/app/serializers/pipeline_entity.rb +++ b/app/serializers/pipeline_entity.rb @@ -48,15 +48,15 @@ class PipelineEntity < Grape::Entity end expose :commit, using: CommitEntity - expose :yaml_errors, if: ->(pipeline, _) { pipeline.has_yaml_errors? } + expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? } - expose :retry_path, if: proc { can_retry? } do |pipeline| + expose :retry_path, if: -> (*) { can_retry? } do |pipeline| retry_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) end - expose :cancel_path, if: proc { can_cancel? } do |pipeline| + expose :cancel_path, if: -> (*) { can_cancel? } do |pipeline| cancel_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) diff --git a/app/services/ci/play_build_service.rb b/app/services/ci/play_build_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..e24f48c2d1654ecf24c73b596f2a4c516d7acdb5 --- /dev/null +++ b/app/services/ci/play_build_service.rb @@ -0,0 +1,17 @@ +module Ci + class PlayBuildService < ::BaseService + def execute(build) + unless can?(current_user, :update_build, build) + raise Gitlab::Access::AccessDeniedError + end + + # Try to enqueue the build, otherwise create a duplicate. + # + if build.enqueue + build.tap { |action| action.update(user: current_user) } + else + Ci::Build.retry(build, current_user) + end + end + end +end diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index ecc6173a96a55309aaeafa230163d8ea0bf4cee9..5b2071573453e39b16a8fd522835e481c57da208 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -8,6 +8,8 @@ module Ci end pipeline.retryable_builds.find_each do |build| + next unless can?(current_user, :update_build, build) + Ci::RetryBuildService.new(project, current_user) .reprocess(build) end diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb index 42c72aba7ddfcfd445879ae38eda1288bb6edfd3..43c9a065fcf75e5c688faa9dd827f5690f3f9e37 100644 --- a/app/services/ci/stop_environments_service.rb +++ b/app/services/ci/stop_environments_service.rb @@ -5,10 +5,11 @@ module Ci def execute(branch_name) @ref = branch_name - return unless has_ref? + return unless @ref.present? environments.each do |environment| - next unless can?(current_user, :create_deployment, project) + next unless environment.stop_action? + next unless can?(current_user, :stop_environment, environment) environment.stop_with_action!(current_user) end @@ -16,13 +17,10 @@ module Ci private - def has_ref? - @ref.present? - end - def environments - @environments ||= - EnvironmentsFinder.new(project, current_user, ref: @ref, recently_updated: true).execute + @environments ||= EnvironmentsFinder + .new(project, current_user, ref: @ref, recently_updated: true) + .execute end end end diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 2c3fd1fcd4d054b1a57954f109a971d026eb58b1..c001999617665d488b3121f6966b7c0f9f461e15 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -102,7 +102,7 @@ = link_to cancel_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do = icon('remove', class: 'cred') - elsif allow_retry - - if job.playable? && !admin + - if job.playable? && !admin && can?(current_user, :update_build, job) = link_to play_namespace_project_build_path(job.project.namespace, job.project, job, return_to: request.original_url), method: :post, title: 'Play', class: 'btn btn-build' do = custom_icon('icon_play') - elsif job.retryable? diff --git a/changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml b/changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml new file mode 100644 index 0000000000000000000000000000000000000000..6f8e80e7d648a82d14960b8858b7bffdf645e5b8 --- /dev/null +++ b/changelogs/unreleased/feature-gb-manual-actions-protected-branches-permissions.yml @@ -0,0 +1,4 @@ +--- +title: Implement protected manual actions +merge_request: 10494 +author: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index ad3ebd144dfe40519c4331d6db6afa438dcd4773..16308a957cb31036255a0111c45e80be64d085b5 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -553,6 +553,8 @@ The above script will: #### Manual actions > Introduced in GitLab 8.10. +> Blocking manual actions were introduced in GitLab 9.0 +> Protected actions were introduced in GitLab 9.2 Manual actions are a special type of job that are not executed automatically; they need to be explicitly started by a user. Manual actions can be started @@ -578,7 +580,10 @@ Optional manual actions have `allow_failure: true` set by default. **Statuses of optional actions do not contribute to overall pipeline status.** -> Blocking manual actions were introduced in GitLab 9.0 +**Manual actions are considered to be write actions, so permissions for +protected branches are used when user wants to trigger an action. In other +words, in order to trigger a manual action assigned to a branch that the +pipeline is running for, user needs to have ability to push to this branch.** ### environment diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 288b03d940ce52ec9eb3631edf83b75f29f0fb74..0223957fde11d7034a1e1552fc42aacf29fa40b2 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -132,6 +132,7 @@ module API authorize_update_builds! build = get_build!(params[:job_id]) + authorize!(:update_build, build) build.cancel @@ -148,6 +149,7 @@ module API authorize_update_builds! build = get_build!(params[:job_id]) + authorize!(:update_build, build) return forbidden!('Job is not retryable') unless build.retryable? build = Ci::Build.retry(build, current_user) @@ -165,6 +167,7 @@ module API authorize_update_builds! build = get_build!(params[:job_id]) + authorize!(:update_build, build) return forbidden!('Job is not erasable!') unless build.erasable? build.erase(erased_by: current_user) @@ -181,6 +184,7 @@ module API authorize_update_builds! build = get_build!(params[:job_id]) + authorize!(:update_build, build) return not_found!(build) unless build.artifacts? build.keep_artifacts! @@ -201,6 +205,7 @@ module API build = get_build!(params[:job_id]) + authorize!(:update_build, build) bad_request!("Unplayable Job") unless build.playable? build.play(current_user) @@ -211,12 +216,12 @@ module API end helpers do - def get_build(id) + def find_build(id) user_project.builds.find_by(id: id.to_i) end def get_build!(id) - get_build(id) || not_found! + find_build(id) || not_found! end def present_artifacts!(artifacts_file) diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index 4dd03cdf24bc904824abb99e7783299306d2bf8f..219359224148e58211d4d4f487a287fc8d57dc98 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -134,6 +134,7 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) + authorize!(:update_build, build) build.cancel @@ -150,6 +151,7 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) + authorize!(:update_build, build) return forbidden!('Build is not retryable') unless build.retryable? build = Ci::Build.retry(build, current_user) @@ -167,6 +169,7 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) + authorize!(:update_build, build) return forbidden!('Build is not erasable!') unless build.erasable? build.erase(erased_by: current_user) @@ -183,6 +186,7 @@ module API authorize_update_builds! build = get_build!(params[:build_id]) + authorize!(:update_build, build) return not_found!(build) unless build.artifacts? build.keep_artifacts! @@ -202,7 +206,7 @@ module API authorize_read_builds! build = get_build!(params[:build_id]) - + authorize!(:update_build, build) bad_request!("Unplayable Job") unless build.playable? build.play(current_user) @@ -213,12 +217,12 @@ module API end helpers do - def get_build(id) + def find_build(id) user_project.builds.find_by(id: id.to_i) end def get_build!(id) - get_build(id) || not_found! + find_build(id) || not_found! end def present_artifacts!(artifacts_file) diff --git a/lib/gitlab/ci/status/build/action.rb b/lib/gitlab/ci/status/build/action.rb new file mode 100644 index 0000000000000000000000000000000000000000..45fd0d4aa073952646c1c9a4dbd6ba2fbe4bdbd8 --- /dev/null +++ b/lib/gitlab/ci/status/build/action.rb @@ -0,0 +1,21 @@ +module Gitlab + module Ci + module Status + module Build + class Action < Status::Extended + def label + if has_action? + @status.label + else + "#{@status.label} (not allowed)" + end + end + + def self.matches?(build, user) + build.action? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index 67bbc3c484932aae0eef91da8b82fee70b64ec2b..57b533bad99c604a5888b3efa5e786da9ba0cb53 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -2,9 +2,7 @@ module Gitlab module Ci module Status module Build - class Cancelable < SimpleDelegator - include Status::Extended - + class Cancelable < Status::Extended def has_action? can?(user, :update_build, subject) end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index 38ac6edc9f188ece3d76b7a273e37cd07982f9d9..c852d6073736d07d81a4d8e64752579e49ee73bb 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -8,7 +8,8 @@ module Gitlab Status::Build::Retryable], [Status::Build::FailedAllowed, Status::Build::Play, - Status::Build::Stop]] + Status::Build::Stop], + [Status::Build::Action]] end def self.common_helpers diff --git a/lib/gitlab/ci/status/build/failed_allowed.rb b/lib/gitlab/ci/status/build/failed_allowed.rb index 807afe24bd5765f695cd3cf88d8c6b618a4930c8..e42d3574357cb543c4b9caa346f98b318e3e70e3 100644 --- a/lib/gitlab/ci/status/build/failed_allowed.rb +++ b/lib/gitlab/ci/status/build/failed_allowed.rb @@ -2,9 +2,7 @@ module Gitlab module Ci module Status module Build - class FailedAllowed < SimpleDelegator - include Status::Extended - + class FailedAllowed < Status::Extended def label 'failed (allowed to fail)' end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 3495b8d0448b923d4f8b9592c47ebaffbe7e6ab7..c6139f1b71647d404ec068d82af0d00dd67a2774 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -2,9 +2,7 @@ module Gitlab module Ci module Status module Build - class Play < SimpleDelegator - include Status::Extended - + class Play < Status::Extended def label 'manual play action' end diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 6b362af76343e9e97a5fa5de5bf563b31d4f78b6..505f80848b2504907129c1dab6ac6ff709ef4e82 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -2,9 +2,7 @@ module Gitlab module Ci module Status module Build - class Retryable < SimpleDelegator - include Status::Extended - + class Retryable < Status::Extended def has_action? can?(user, :update_build, subject) end diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index e8530f2aaaed4ad041e792ee73f08d53e2574441..0b5199e5483a7ae6d708b6f6f37d78eb924f9955 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -2,9 +2,7 @@ module Gitlab module Ci module Status module Build - class Stop < SimpleDelegator - include Status::Extended - + class Stop < Status::Extended def label 'manual stop action' end diff --git a/lib/gitlab/ci/status/extended.rb b/lib/gitlab/ci/status/extended.rb index d367c9bda69ff2d71f6e6cef91c3ef66fbc7b2b2..1e8101f894950f4327dc473cc05b262a95e7ed80 100644 --- a/lib/gitlab/ci/status/extended.rb +++ b/lib/gitlab/ci/status/extended.rb @@ -1,13 +1,13 @@ module Gitlab module Ci module Status - module Extended - extend ActiveSupport::Concern + class Extended < SimpleDelegator + def initialize(status) + super(@status = status) + end - class_methods do - def matches?(_subject, _user) - raise NotImplementedError - end + def self.matches?(_subject, _user) + raise NotImplementedError end end end diff --git a/lib/gitlab/ci/status/pipeline/blocked.rb b/lib/gitlab/ci/status/pipeline/blocked.rb index a250c3fcb419dd8ebf4a57fef68e4676c376cab1..37dfe43fb621a019ac5c2641cd10a84a0e17d994 100644 --- a/lib/gitlab/ci/status/pipeline/blocked.rb +++ b/lib/gitlab/ci/status/pipeline/blocked.rb @@ -2,9 +2,7 @@ module Gitlab module Ci module Status module Pipeline - class Blocked < SimpleDelegator - include Status::Extended - + class Blocked < Status::Extended def text 'blocked' end diff --git a/lib/gitlab/ci/status/success_warning.rb b/lib/gitlab/ci/status/success_warning.rb index d4cdab6957a00c835c7996b02aac7f62bb14e88a..df6e76b015183957b2bd2718a8bc4164b3eb14a4 100644 --- a/lib/gitlab/ci/status/success_warning.rb +++ b/lib/gitlab/ci/status/success_warning.rb @@ -5,9 +5,7 @@ module Gitlab # Extended status used when pipeline or stage passed conditionally. # This means that failed jobs that are allowed to fail were present. # - class SuccessWarning < SimpleDelegator - include Status::Extended - + class SuccessWarning < Status::Extended def text 'passed' end diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb index 22193eac6722fa52495fedbe4bd54ac2b11f2887..3ce23c17cdc684d133c65410c632d0030b1d3255 100644 --- a/spec/controllers/projects/builds_controller_spec.rb +++ b/spec/controllers/projects/builds_controller_spec.rb @@ -261,7 +261,7 @@ describe Projects::BuildsController do describe 'POST play' do before do - project.add_developer(user) + project.add_master(user) sign_in(user) post_play diff --git a/spec/factories/environments.rb b/spec/factories/environments.rb index 3fbf24b5c7d2977aae87db27b92ffd4143e973ae..d8d699fb3aace201d49d8418312abc6d3bbf5978 100644 --- a/spec/factories/environments.rb +++ b/spec/factories/environments.rb @@ -18,15 +18,21 @@ FactoryGirl.define do # interconnected objects to simulate a review app. # after(:create) do |environment, evaluator| + pipeline = create(:ci_pipeline, project: environment.project) + + deployable = create(:ci_build, name: "#{environment.name}:deploy", + pipeline: pipeline) + deployment = create(:deployment, environment: environment, project: environment.project, + deployable: deployable, ref: evaluator.ref, sha: environment.project.commit(evaluator.ref).id) teardown_build = create(:ci_build, :manual, - name: "#{deployment.environment.name}:teardown", - pipeline: deployment.deployable.pipeline) + name: "#{environment.name}:teardown", + pipeline: pipeline) deployment.update_column(:on_stop, teardown_build.name) environment.update_attribute(:deployments, [deployment]) diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 1e12f8542e2dc22ef649036a411c326c5f907792..86ce50c976f0a7591e33651216ae6362d37219e6 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -62,6 +62,8 @@ feature 'Environment', :feature do name: 'deploy to production') end + given(:role) { :master } + scenario 'does show a play button' do expect(page).to have_link(action.name.humanize) end @@ -132,6 +134,8 @@ feature 'Environment', :feature do on_stop: 'close_app') end + given(:role) { :master } + scenario 'does allow to stop environment' do click_link('Stop') diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb index b6e924d67bebcd504942564b0889a6db1f0969fe..eb4f06b371c4346b5b857b9d9a74e57168496c5f 100644 --- a/spec/lib/gitlab/chat_commands/command_spec.rb +++ b/spec/lib/gitlab/chat_commands/command_spec.rb @@ -40,11 +40,15 @@ describe Gitlab::ChatCommands::Command, service: true do context 'when trying to do deployment' do let(:params) { { text: 'deploy staging to production' } } - let!(:build) { create(:ci_build, project: project) } + let!(:build) { create(:ci_build, pipeline: pipeline) } + let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:staging) { create(:environment, name: 'staging', project: project) } let!(:deployment) { create(:deployment, environment: staging, deployable: build) } + let!(:manual) do - create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'first', environment: 'production') + create(:ci_build, :manual, pipeline: pipeline, + name: 'first', + environment: 'production') end context 'and user can not create deployment' do @@ -56,7 +60,7 @@ describe Gitlab::ChatCommands::Command, service: true do context 'and user does have deployment permission' do before do - project.team << [user, :developer] + build.project.add_master(user) end it 'returns action' do @@ -66,7 +70,9 @@ describe Gitlab::ChatCommands::Command, service: true do context 'when duplicate action exists' do let!(:manual2) do - create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'second', environment: 'production') + create(:ci_build, :manual, pipeline: pipeline, + name: 'second', + environment: 'production') end it 'returns error' do diff --git a/spec/lib/gitlab/chat_commands/deploy_spec.rb b/spec/lib/gitlab/chat_commands/deploy_spec.rb index b3358a321618d5aa22f3709dd87be5a9bc6f788b..b33389d959e30dd52e93cde9aad3cf2af515e15c 100644 --- a/spec/lib/gitlab/chat_commands/deploy_spec.rb +++ b/spec/lib/gitlab/chat_commands/deploy_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::ChatCommands::Deploy, service: true do let(:regex_match) { described_class.match('deploy staging to production') } before do - project.team << [user, :master] + project.add_master(user) end subject do @@ -23,7 +23,8 @@ describe Gitlab::ChatCommands::Deploy, service: true do context 'with environment' do let!(:staging) { create(:environment, name: 'staging', project: project) } - let!(:build) { create(:ci_build, project: project) } + let!(:pipeline) { create(:ci_pipeline, project: project) } + let!(:build) { create(:ci_build, pipeline: pipeline) } let!(:deployment) { create(:deployment, environment: staging, deployable: build) } context 'without actions' do @@ -35,7 +36,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do context 'with action' do let!(:manual1) do - create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'first', environment: 'production') + create(:ci_build, :manual, pipeline: pipeline, + name: 'first', + environment: 'production') end it 'returns success result' do @@ -45,7 +48,9 @@ describe Gitlab::ChatCommands::Deploy, service: true do context 'when duplicate action exists' do let!(:manual2) do - create(:ci_build, :manual, project: project, pipeline: build.pipeline, name: 'second', environment: 'production') + create(:ci_build, :manual, pipeline: pipeline, + name: 'second', + environment: 'production') end it 'returns error' do @@ -57,8 +62,7 @@ describe Gitlab::ChatCommands::Deploy, service: true do context 'when teardown action exists' do let!(:teardown) do create(:ci_build, :manual, :teardown_environment, - project: project, pipeline: build.pipeline, - name: 'teardown', environment: 'production') + pipeline: pipeline, name: 'teardown', environment: 'production') end it 'returns the success message' do diff --git a/spec/lib/gitlab/ci/status/build/action_spec.rb b/spec/lib/gitlab/ci/status/build/action_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8c25f72804b6835838c826c3a8d2c8cfee35b1a5 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/action_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Action do + let(:status) { double('core status') } + let(:user) { double('user') } + + subject do + described_class.new(status) + end + + describe '#label' do + before do + allow(status).to receive(:label).and_return('label') + end + + context 'when status has action' do + before do + allow(status).to receive(:has_action?).and_return(true) + end + + it 'does not append text' do + expect(subject.label).to eq 'label' + end + end + + context 'when status does not have action' do + before do + allow(status).to receive(:has_action?).and_return(false) + end + + it 'appends text about action not allowed' do + expect(subject.label).to eq 'label (not allowed)' + end + end + end + + describe '.matches?' do + subject { described_class.matches?(build, user) } + + context 'when build is an action' do + let(:build) { create(:ci_build, :manual) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not manual' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(subject).to be false + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb index e648a3ac3a27c49752cdee98a7086707b710023f..185bb9098dac01ccd01022e7c049f58dbdc72210 100644 --- a/spec/lib/gitlab/ci/status/build/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -204,11 +204,12 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Play] + .to eq [Gitlab::Ci::Status::Build::Play, + Gitlab::Ci::Status::Build::Action] end - it 'fabricates a play detailed status' do - expect(status).to be_a Gitlab::Ci::Status::Build::Play + it 'fabricates action detailed status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Action end it 'fabricates status with correct details' do @@ -216,11 +217,26 @@ describe Gitlab::Ci::Status::Build::Factory do expect(status.group).to eq 'manual' expect(status.icon).to eq 'icon_status_manual' expect(status.favicon).to eq 'favicon_status_manual' - expect(status.label).to eq 'manual play action' + expect(status.label).to include 'manual play action' expect(status).to have_details - expect(status).to have_action expect(status.action_path).to include 'play' end + + context 'when user has ability to play action' do + before do + build.project.add_master(user) + end + + it 'fabricates status that has action' do + expect(status).to have_action + end + end + + context 'when user does not have ability to play action' do + it 'fabricates status that has no action' do + expect(status).not_to have_action + end + end end context 'when build is an environment stop action' do @@ -232,21 +248,24 @@ describe Gitlab::Ci::Status::Build::Factory do it 'matches correct extended statuses' do expect(factory.extended_statuses) - .to eq [Gitlab::Ci::Status::Build::Stop] + .to eq [Gitlab::Ci::Status::Build::Stop, + Gitlab::Ci::Status::Build::Action] end - it 'fabricates a stop detailed status' do - expect(status).to be_a Gitlab::Ci::Status::Build::Stop + it 'fabricates action detailed status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Action end - it 'fabricates status with correct details' do - expect(status.text).to eq 'manual' - expect(status.group).to eq 'manual' - expect(status.icon).to eq 'icon_status_manual' - expect(status.favicon).to eq 'favicon_status_manual' - expect(status.label).to eq 'manual stop action' - expect(status).to have_details - expect(status).to have_action + context 'when user is not allowed to execute manual action' do + it 'fabricates status with correct details' do + expect(status.text).to eq 'manual' + expect(status.group).to eq 'manual' + expect(status.icon).to eq 'icon_status_manual' + expect(status.favicon).to eq 'favicon_status_manual' + expect(status.label).to eq 'manual stop action (not allowed)' + expect(status).to have_details + expect(status).not_to have_action + end end end end diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index 6c97a4fe5cad435cd030937662c42c9041c71f71..f5d0f977768fc925e43d362c04a84eedefb9368a 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -1,43 +1,48 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Play do - let(:status) { double('core') } - let(:user) { double('user') } + let(:user) { create(:user) } + let(:build) { create(:ci_build, :manual) } + let(:status) { Gitlab::Ci::Status::Core.new(build, user) } subject { described_class.new(status) } describe '#label' do - it { expect(subject.label).to eq 'manual play action' } + it 'has a label that says it is a manual action' do + expect(subject.label).to eq 'manual play action' + end end - describe 'action details' do - let(:user) { create(:user) } - let(:build) { create(:ci_build) } - let(:status) { Gitlab::Ci::Status::Core.new(build, user) } - - describe '#has_action?' do - context 'when user is allowed to update build' do - before { build.project.team << [user, :developer] } + describe '#has_action?' do + context 'when user is allowed to update build' do + context 'when user can push to branch' do + before { build.project.add_master(user) } it { is_expected.to have_action } end - context 'when user is not allowed to update build' do + context 'when user can not push to the branch' do + before { build.project.add_developer(user) } + it { is_expected.not_to have_action } end end - describe '#action_path' do - it { expect(subject.action_path).to include "#{build.id}/play" } + context 'when user is not allowed to update build' do + it { is_expected.not_to have_action } end + end - describe '#action_icon' do - it { expect(subject.action_icon).to eq 'icon_action_play' } - end + describe '#action_path' do + it { expect(subject.action_path).to include "#{build.id}/play" } + end - describe '#action_title' do - it { expect(subject.action_title).to eq 'Play' } - end + describe '#action_icon' do + it { expect(subject.action_icon).to eq 'icon_action_play' } + end + + describe '#action_title' do + it { expect(subject.action_title).to eq 'Play' } end describe '.matches?' do diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb index c2d74ca5cde0cbad5026e10fd590829d02b33c9d..6eacb07078bf9f0d630e41f8dcbd586ddfad5841 100644 --- a/spec/lib/gitlab/ci/status/extended_spec.rb +++ b/spec/lib/gitlab/ci/status/extended_spec.rb @@ -1,12 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Extended do - subject do - Class.new.include(described_class) - end - it 'requires subclass to implement matcher' do - expect { subject.matches?(double, double) } + expect { described_class.matches?(double, double) } .to raise_error(NotImplementedError) end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6e8845cdcf42db3493c11cd11bd46c6672ab318c..5231ce28c9df10cf233dad37845ed956619acac4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -897,22 +897,26 @@ describe Ci::Build, :models do end describe '#persisted_environment' do - before do - @environment = create(:environment, project: project, name: "foo-#{project.default_branch}") + let!(:environment) do + create(:environment, project: project, name: "foo-#{project.default_branch}") end subject { build.persisted_environment } - context 'referenced literally' do - let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-#{project.default_branch}") } + context 'when referenced literally' do + let(:build) do + create(:ci_build, pipeline: pipeline, environment: "foo-#{project.default_branch}") + end - it { is_expected.to eq(@environment) } + it { is_expected.to eq(environment) } end - context 'referenced with a variable' do - let(:build) { create(:ci_build, pipeline: pipeline, environment: "foo-$CI_COMMIT_REF_NAME") } + context 'when referenced with a variable' do + let(:build) do + create(:ci_build, pipeline: pipeline, environment: "foo-$CI_COMMIT_REF_NAME") + end - it { is_expected.to eq(@environment) } + it { is_expected.to eq(environment) } end end @@ -923,26 +927,8 @@ describe Ci::Build, :models do project.add_developer(user) end - context 'when build is manual' do - it 'enqueues a build' do - new_build = build.play(user) - - expect(new_build).to be_pending - expect(new_build).to eq(build) - end - end - - context 'when build is passed' do - before do - build.update(status: 'success') - end - - it 'creates a new build' do - new_build = build.play(user) - - expect(new_build).to be_pending - expect(new_build).not_to eq(build) - end + it 'enqueues the build' do + expect(build.play(user)).to be_pending end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 070716e859a7b713856a950cae949d38753d207d..28e5c3f80f42c513cf82f2ff1268e73086eadd84 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -206,25 +206,52 @@ describe Environment, models: true do end context 'when matching action is defined' do - let(:build) { create(:ci_build) } - let!(:deployment) { create(:deployment, environment: environment, deployable: build, on_stop: 'close_app') } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } + + let!(:deployment) do + create(:deployment, environment: environment, + deployable: build, + on_stop: 'close_app') + end - context 'when action did not yet finish' do - let!(:close_action) { create(:ci_build, :manual, pipeline: build.pipeline, name: 'close_app') } + context 'when user is not allowed to stop environment' do + let!(:close_action) do + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') + end - it 'returns the same action' do - expect(subject).to eq(close_action) - expect(subject.user).to eq(user) + it 'raises an exception' do + expect { subject }.to raise_error(Gitlab::Access::AccessDeniedError) end end - context 'if action did finish' do - let!(:close_action) { create(:ci_build, :manual, :success, pipeline: build.pipeline, name: 'close_app') } + context 'when user is allowed to stop environment' do + before do + project.add_master(user) + end + + context 'when action did not yet finish' do + let!(:close_action) do + create(:ci_build, :manual, pipeline: pipeline, name: 'close_app') + end + + it 'returns the same action' do + expect(subject).to eq(close_action) + expect(subject.user).to eq(user) + end + end - it 'returns a new action of the same type' do - is_expected.to be_persisted - expect(subject.name).to eq(close_action.name) - expect(subject.user).to eq(user) + context 'if action did finish' do + let!(:close_action) do + create(:ci_build, :manual, :success, + pipeline: pipeline, name: 'close_app') + end + + it 'returns a new action of the same type' do + expect(subject).to be_persisted + expect(subject.name).to eq(close_action.name) + expect(subject.user).to eq(user) + end end end end diff --git a/spec/policies/ci/build_policy_spec.rb b/spec/policies/ci/build_policy_spec.rb index 0f280f32eac91fed932385e62e3176e018f44e91..3f4ce222b60fb68ca2da73c0d8c380e72e534058 100644 --- a/spec/policies/ci/build_policy_spec.rb +++ b/spec/policies/ci/build_policy_spec.rb @@ -89,5 +89,58 @@ describe Ci::BuildPolicy, :models do end end end + + describe 'rules for manual actions' do + let(:project) { create(:project) } + + before do + project.add_developer(user) + end + + context 'when branch build is assigned to is protected' do + before do + create(:protected_branch, :no_one_can_push, + name: 'some-ref', project: project) + end + + context 'when build is a manual action' do + let(:build) do + create(:ci_build, :manual, ref: 'some-ref', pipeline: pipeline) + end + + it 'does not include ability to update build' do + expect(policies).not_to include :update_build + end + end + + context 'when build is not a manual action' do + let(:build) do + create(:ci_build, ref: 'some-ref', pipeline: pipeline) + end + + it 'includes ability to update build' do + expect(policies).to include :update_build + end + end + end + + context 'when branch build is assigned to is not protected' do + context 'when build is a manual action' do + let(:build) { create(:ci_build, :manual, pipeline: pipeline) } + + it 'includes ability to update build' do + expect(policies).to include :update_build + end + end + + context 'when build is not a manual action' do + let(:build) { create(:ci_build, pipeline: pipeline) } + + it 'includes ability to update build' do + expect(policies).to include :update_build + end + end + end + end end end diff --git a/spec/policies/environment_policy_spec.rb b/spec/policies/environment_policy_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..0e15beaa5e8c12dbb07e5aaf1490e11b4dddece4 --- /dev/null +++ b/spec/policies/environment_policy_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe EnvironmentPolicy do + let(:user) { create(:user) } + let(:project) { create(:project) } + + let(:environment) do + create(:environment, :with_review_app, project: project) + end + + let(:policies) do + described_class.abilities(user, environment).to_set + end + + describe '#rules' do + context 'when user does not have access to the project' do + let(:project) { create(:project, :private) } + + it 'does not include ability to stop environment' do + expect(policies).not_to include :stop_environment + end + end + + context 'when anonymous user has access to the project' do + let(:project) { create(:project, :public) } + + it 'does not include ability to stop environment' do + expect(policies).not_to include :stop_environment + end + end + + context 'when team member has access to the project' do + let(:project) { create(:project, :public) } + + before do + project.add_master(user) + end + + context 'when team member has ability to stop environment' do + it 'does includes ability to stop environment' do + expect(policies).to include :stop_environment + end + end + + context 'when team member has no ability to stop environment' do + before do + create(:protected_branch, :no_one_can_push, + name: 'master', project: project) + end + + it 'does not include ability to stop environment' do + expect(policies).not_to include :stop_environment + end + end + end + end +end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index decb5b9194171dbf8bc2d0497ef7cd7de4e625d0..e5e5872dc1ff677080015d5465da4fc9799e8638 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -1,14 +1,26 @@ require 'spec_helper' -describe API::Jobs do +describe API::Jobs, :api do + let!(:project) do + create(:project, :repository, public_builds: false) + end + + let!(:pipeline) do + create(:ci_empty_pipeline, project: project, + sha: project.commit.id, + ref: project.default_branch) + end + + let!(:build) { create(:ci_build, pipeline: pipeline) } + let(:user) { create(:user) } let(:api_user) { user } - let!(:project) { create(:project, :repository, creator: user, public_builds: false) } - let!(:developer) { create(:project_member, :developer, user: user, project: project) } - let(:reporter) { create(:project_member, :reporter, project: project) } - let(:guest) { create(:project_member, :guest, project: project) } - let!(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) } - let!(:build) { create(:ci_build, pipeline: pipeline) } + let(:reporter) { create(:project_member, :reporter, project: project).user } + let(:guest) { create(:project_member, :guest, project: project).user } + + before do + project.add_developer(user) + end describe 'GET /projects/:id/jobs' do let(:query) { Hash.new } @@ -211,7 +223,7 @@ describe API::Jobs do end describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do - let(:api_user) { reporter.user } + let(:api_user) { reporter } let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) } before do @@ -235,7 +247,7 @@ describe API::Jobs do end context 'when logging as guest' do - let(:api_user) { guest.user } + let(:api_user) { guest } before do get_for_ref @@ -345,7 +357,7 @@ describe API::Jobs do end context 'user without :update_build permission' do - let(:api_user) { reporter.user } + let(:api_user) { reporter } it 'does not cancel job' do expect(response).to have_http_status(403) @@ -379,7 +391,7 @@ describe API::Jobs do end context 'user without :update_build permission' do - let(:api_user) { reporter.user } + let(:api_user) { reporter } it 'does not retry job' do expect(response).to have_http_status(403) @@ -455,16 +467,39 @@ describe API::Jobs do describe 'POST /projects/:id/jobs/:job_id/play' do before do - post api("/projects/#{project.id}/jobs/#{build.id}/play", user) + post api("/projects/#{project.id}/jobs/#{build.id}/play", api_user) end context 'on an playable job' do let(:build) { create(:ci_build, :manual, project: project, pipeline: pipeline) } - it 'plays the job' do - expect(response).to have_http_status(200) - expect(json_response['user']['id']).to eq(user.id) - expect(json_response['id']).to eq(build.id) + context 'when user is authorized to trigger a manual action' do + it 'plays the job' do + expect(response).to have_http_status(200) + expect(json_response['user']['id']).to eq(user.id) + expect(json_response['id']).to eq(build.id) + expect(build.reload).to be_pending + end + end + + context 'when user is not authorized to trigger a manual action' do + context 'when user does not have access to the project' do + let(:api_user) { create(:user) } + + it 'does not trigger a manual action' do + expect(build.reload).to be_manual + expect(response).to have_http_status(404) + end + end + + context 'when user is not allowed to trigger the manual action' do + let(:api_user) { reporter } + + it 'does not trigger a manual action' do + expect(build.reload).to be_manual + expect(response).to have_http_status(403) + end + end end end diff --git a/spec/serializers/build_action_entity_spec.rb b/spec/serializers/build_action_entity_spec.rb index 54ac17447b13d74ce9ee60fe81c2289bd08ab3f0..059deba54163deb2e6c01d3b949d3dce77a7b808 100644 --- a/spec/serializers/build_action_entity_spec.rb +++ b/spec/serializers/build_action_entity_spec.rb @@ -2,9 +2,10 @@ require 'spec_helper' describe BuildActionEntity do let(:build) { create(:ci_build, name: 'test_build') } + let(:request) { double('request') } let(:entity) do - described_class.new(build, request: double) + described_class.new(build, request: spy('request')) end describe '#as_json' do diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb index f76a5cf72d1a97644391a9ed90d3b37eae0b8577..897a28b7305a8a144c6f0f9689161aeaefbdd136 100644 --- a/spec/serializers/build_entity_spec.rb +++ b/spec/serializers/build_entity_spec.rb @@ -41,13 +41,37 @@ describe BuildEntity do it 'does not contain path to play action' do expect(subject).not_to include(:play_path) end + + it 'is not a playable job' do + expect(subject[:playable]).to be false + end end context 'when build is a manual action' do let(:build) { create(:ci_build, :manual) } - it 'contains path to play action' do - expect(subject).to include(:play_path) + context 'when user is allowed to trigger action' do + before do + build.project.add_master(user) + end + + it 'contains path to play action' do + expect(subject).to include(:play_path) + end + + it 'is a playable action' do + expect(subject[:playable]).to be true + end + end + + context 'when user is not allowed to trigger action' do + it 'does not contain path to play action' do + expect(subject).not_to include(:play_path) + end + + it 'is not a playable action' do + expect(subject[:playable]).to be false + end end end end diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..d6f9fa42045b3c6cc5b2a2af01bde7bfc1a0a41f --- /dev/null +++ b/spec/services/ci/play_build_service_spec.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +describe Ci::PlayBuildService, '#execute', :services do + let(:user) { create(:user) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, :manual, pipeline: pipeline) } + + let(:service) do + described_class.new(project, user) + end + + context 'when project does not have repository yet' do + let(:project) { create(:empty_project) } + + it 'allows user with master role to play build' do + project.add_master(user) + + service.execute(build) + + expect(build.reload).to be_pending + end + + it 'does not allow user with developer role to play build' do + project.add_developer(user) + + expect { service.execute(build) } + .to raise_error Gitlab::Access::AccessDeniedError + end + end + + context 'when project has repository' do + let(:project) { create(:project) } + + it 'allows user with developer role to play a build' do + project.add_developer(user) + + service.execute(build) + + expect(build.reload).to be_pending + end + end + + context 'when build is a playable manual action' do + let(:build) { create(:ci_build, :manual, pipeline: pipeline) } + + before do + project.add_master(user) + end + + it 'enqueues the build' do + expect(service.execute(build)).to eq build + expect(build.reload).to be_pending + end + + it 'reassignes build user correctly' do + service.execute(build) + + expect(build.reload.user).to eq user + end + end + + context 'when build is not a playable manual action' do + let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) } + + before do + project.add_master(user) + end + + it 'duplicates the build' do + duplicate = service.execute(build) + + expect(duplicate).not_to eq build + expect(duplicate).to be_pending + end + + it 'assigns users correctly' do + duplicate = service.execute(build) + + expect(build.user).not_to eq user + expect(duplicate.user).to eq user + end + end + + context 'when build is not action' do + let(:build) { create(:ci_build, :success, pipeline: pipeline) } + + it 'raises an error' do + expect { service.execute(build) } + .to raise_error Gitlab::Access::AccessDeniedError + end + end + + context 'when user does not have ability to trigger action' do + before do + create(:protected_branch, :no_one_can_push, + name: build.ref, project: project) + end + + it 'raises an error' do + expect { service.execute(build) } + .to raise_error Gitlab::Access::AccessDeniedError + end + end +end diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 245e19822f3383b854384b15a4b64123d6b01325..cf773866a6ff00b55e327af27dbeb4f68acc1607 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -314,6 +314,13 @@ describe Ci::ProcessPipelineService, '#execute', :services do end context 'when pipeline is promoted sequentially up to the end' do + before do + # We are using create(:empty_project), and users has to be master in + # order to execute manual action when repository does not exist. + # + project.add_master(user) + end + it 'properly processes entire pipeline' do process_pipeline diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index f1b2d3a47985ad90ba568a3c0e5940bba9bd445a..40e151545c9951b8223af5e085ff247ba1650e04 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -7,7 +7,9 @@ describe Ci::RetryPipelineService, '#execute', :services do let(:service) { described_class.new(project, user) } context 'when user has ability to modify pipeline' do - let(:user) { create(:admin) } + before do + project.add_master(user) + end context 'when there are already retried jobs present' do before do @@ -227,6 +229,46 @@ describe Ci::RetryPipelineService, '#execute', :services do end end + context 'when user is not allowed to trigger manual action' do + before do + project.add_developer(user) + end + + context 'when there is a failed manual action present' do + before do + create_build('test', :failed, 0) + create_build('deploy', :failed, 0, when: :manual) + create_build('verify', :canceled, 1) + end + + it 'does not reprocess manual action' do + service.execute(pipeline) + + expect(build('test')).to be_pending + expect(build('deploy')).to be_failed + expect(build('verify')).to be_created + expect(pipeline.reload).to be_running + end + end + + context 'when there is a failed manual action in later stage' do + before do + create_build('test', :failed, 0) + create_build('deploy', :failed, 1, when: :manual) + create_build('verify', :canceled, 2) + end + + it 'does not reprocess manual action' do + service.execute(pipeline) + + expect(build('test')).to be_pending + expect(build('deploy')).to be_failed + expect(build('verify')).to be_created + expect(pipeline.reload).to be_running + end + end + end + def statuses pipeline.reload.statuses end diff --git a/spec/services/ci/stop_environments_service_spec.rb b/spec/services/ci/stop_environments_service_spec.rb index 32c72a9cf5ef093c8ed89724a2f3f2f84c01cdf2..98044ad232ed9c03bdcc6fe1487fb516c3e014e1 100644 --- a/spec/services/ci/stop_environments_service_spec.rb +++ b/spec/services/ci/stop_environments_service_spec.rb @@ -55,8 +55,22 @@ describe Ci::StopEnvironmentsService, services: true do end context 'when user does not have permission to stop environment' do + context 'when user has no access to manage deployments' do + before do + project.team << [user, :guest] + end + + it 'does not stop environment' do + expect_environment_not_stopped_on('master') + end + end + end + + context 'when branch for stop action is protected' do before do - project.team << [user, :guest] + project.add_developer(user) + create(:protected_branch, :no_one_can_push, + name: 'master', project: project) end it 'does not stop environment' do