diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2017-05-06 21:36:03 +0100 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2017-05-06 21:36:03 +0100 |
commit | cc27180ecd7236e78b3b495d05e87fefc17220cb (patch) | |
tree | 9b747b02feee39e94c26c9738e9e4313ad2b9cbe /app | |
parent | 13b31d25ec9c5f8d17f95ca8ad756a88b5e3dbd9 (diff) | |
parent | 6ad3814e1b31bfacfae7a2aabb4e4607b12ca66f (diff) | |
download | gitlab-ce-cc27180ecd7236e78b3b495d05e87fefc17220cb.tar.gz |
Merge branch 'master' into 25226-realtime-pipelines-fe
* master: (40 commits)
Use GitLab Pages v0.4.2
Do not reprocess actions when user retries pipeline
Add specs for extended status for manual actions
Refine inheritance model of extended CI/CD statuses
Introduce generic manual action extended status class
Check ability to update build on the API resource
Require build to be present in the controller
Authorize build update on per object basis
Use update build policy instead of new play policy
Improve environment policy class
Rephrase documentation for protected actions feature
Improve code style related to protected actions
Add changelog entry for external env URL btn fix
Hide environment external URL button if not defined
Fix builds controller spec related to protected actions
Fix environment policy class name in specs
Add Changelog entry for protected manual actions
Document protected manual actions feature
Improve specs for jobs API regarding manual actions
Fix Rubocop offense in environments policy class
...
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/projects/application_controller.rb | 8 | ||||
-rw-r--r-- | app/controllers/projects/builds_controller.rb | 22 | ||||
-rw-r--r-- | app/models/ci/build.rb | 11 | ||||
-rw-r--r-- | app/models/deployment.rb | 4 | ||||
-rw-r--r-- | app/policies/base_policy.rb | 4 | ||||
-rw-r--r-- | app/policies/ci/build_policy.rb | 16 | ||||
-rw-r--r-- | app/policies/ci/pipeline_policy.rb | 5 | ||||
-rw-r--r-- | app/policies/environment_policy.rb | 14 | ||||
-rw-r--r-- | app/serializers/build_action_entity.rb | 8 | ||||
-rw-r--r-- | app/serializers/build_entity.rb | 10 | ||||
-rw-r--r-- | app/serializers/pipeline_entity.rb | 6 | ||||
-rw-r--r-- | app/services/ci/play_build_service.rb | 17 | ||||
-rw-r--r-- | app/services/ci/retry_pipeline_service.rb | 2 | ||||
-rw-r--r-- | app/services/ci/stop_environments_service.rb | 14 | ||||
-rw-r--r-- | app/views/projects/ci/builds/_build.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/environments/terminal.html.haml | 5 |
16 files changed, 112 insertions, 36 deletions
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index b4b0dfc3eb8..12e4a6999ae 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 e24fc45d166..0fd35bcb790 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 b426c27afbb..971ab7cb0ee 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 afad001d50f..37adfb4de73 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 8890409d056..623424c63e0 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 8b25332b73c..d4af4490608 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 3d2eef1c50c..10aa2d3e72a 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 f4219569161..2fa15e64562 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 184b4b7a681..75dda1af709 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 b804d6d0e8a..1380b347d8e 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 ad8b4d43e8f..7eb7aac72eb 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 00000000000..e24f48c2d16 --- /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 ecc6173a96a..5b207157345 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 42c72aba7dd..43c9a065fcf 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 2c3fd1fcd4d..c0019996176 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/app/views/projects/environments/terminal.html.haml b/app/views/projects/environments/terminal.html.haml index c8363087d6a..4c4aa0baff3 100644 --- a/app/views/projects/environments/terminal.html.haml +++ b/app/views/projects/environments/terminal.html.haml @@ -16,8 +16,9 @@ .col-sm-6 .nav-controls - = link_to @environment.external_url, class: 'btn btn-default', target: '_blank', rel: 'noopener noreferrer nofollow' do - = icon('external-link') + - if @environment.external_url.present? + = link_to @environment.external_url, class: 'btn btn-default', target: '_blank', rel: 'noopener noreferrer nofollow' do + = icon('external-link') = render 'projects/deployments/actions', deployment: @environment.last_deployment .terminal-container{ class: container_class } |