Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/policies/ci/build_policy.rb2
-rw-r--r--app/views/projects/deployments/_actions.haml1
-rw-r--r--app/views/projects/environments/show.html.haml2
-rw-r--r--changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml4
-rw-r--r--doc/ci/yaml/README.md2
-rw-r--r--spec/controllers/projects/builds_controller_spec.rb6
-rw-r--r--spec/features/projects/environments/environment_spec.rb57
-rw-r--r--spec/lib/gitlab/chat_commands/command_spec.rb7
-rw-r--r--spec/lib/gitlab/chat_commands/deploy_spec.rb7
-rw-r--r--spec/lib/gitlab/ci/status/build/factory_spec.rb5
-rw-r--r--spec/lib/gitlab/ci/status/build/play_spec.rb10
-rw-r--r--spec/models/environment_spec.rb5
-rw-r--r--spec/serializers/build_entity_spec.rb6
-rw-r--r--spec/services/ci/play_build_service_spec.rb17
-rw-r--r--spec/services/ci/process_pipeline_service_spec.rb7
-rw-r--r--spec/services/ci/retry_pipeline_service_spec.rb7
16 files changed, 104 insertions, 41 deletions
diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb
index d4af4490608..2d7405dc240 100644
--- a/app/policies/ci/build_policy.rb
+++ b/app/policies/ci/build_policy.rb
@@ -23,7 +23,7 @@ module Ci
!::Gitlab::UserAccess
.new(user, project: build.project)
- .can_push_to_branch?(build.ref)
+ .can_merge_to_branch?(build.ref)
end
end
end
diff --git a/app/views/projects/deployments/_actions.haml b/app/views/projects/deployments/_actions.haml
index 506246f2ee6..e2baaa625ae 100644
--- a/app/views/projects/deployments/_actions.haml
+++ b/app/views/projects/deployments/_actions.haml
@@ -8,6 +8,7 @@
= icon('caret-down')
%ul.dropdown-menu.dropdown-menu-align-right
- actions.each do |action|
+ - next unless can?(current_user, :update_build, action)
%li
= link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do
= custom_icon('icon_play')
diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml
index 7315e671056..9e221240cf2 100644
--- a/app/views/projects/environments/show.html.haml
+++ b/app/views/projects/environments/show.html.haml
@@ -13,7 +13,7 @@
= render 'projects/environments/metrics_button', environment: @environment
- if can?(current_user, :update_environment, @environment)
= link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn'
- - if can?(current_user, :create_deployment, @environment) && @environment.can_stop?
+ - if can?(current_user, :stop_environment, @environment)
= link_to 'Stop', stop_namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to stop this environment?' }, class: 'btn btn-danger', method: :post
.environments-container
diff --git a/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml b/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml
new file mode 100644
index 00000000000..43c18502cd6
--- /dev/null
+++ b/changelogs/unreleased/fix-gb-use-merge-ability-for-protected-manual-actions.yml
@@ -0,0 +1,4 @@
+---
+title: Respect merge, instead of push, permissions for protected actions
+merge_request: 11648
+author:
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index da20076da52..2df03196f80 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -591,7 +591,7 @@ Optional manual actions have `allow_failure: true` set by default.
**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.**
+pipeline is running for, user needs to have ability to merge to this branch.**
### environment
diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb
index f41503fd34e..932276ce380 100644
--- a/spec/controllers/projects/builds_controller_spec.rb
+++ b/spec/controllers/projects/builds_controller_spec.rb
@@ -234,7 +234,11 @@ describe Projects::BuildsController do
describe 'POST play' do
before do
- project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: 'master', project: project)
+
sign_in(user)
post_play
diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb
index 86ce50c976f..18b608c863e 100644
--- a/spec/features/projects/environments/environment_spec.rb
+++ b/spec/features/projects/environments/environment_spec.rb
@@ -12,6 +12,7 @@ feature 'Environment', :feature do
feature 'environment details page' do
given!(:environment) { create(:environment, project: project) }
+ given!(:permissions) { }
given!(:deployment) { }
given!(:action) { }
@@ -62,20 +63,31 @@ feature 'Environment', :feature do
name: 'deploy to production')
end
- given(:role) { :master }
+ context 'when user has ability to trigger deployment' do
+ given(:permissions) do
+ create(:protected_branch, :developers_can_merge,
+ name: action.ref, project: project)
+ end
- scenario 'does show a play button' do
- expect(page).to have_link(action.name.humanize)
- end
+ it 'does show a play button' do
+ expect(page).to have_link(action.name.humanize)
+ end
+
+ it 'does allow to play manual action' do
+ expect(action).to be_manual
- scenario 'does allow to play manual action' do
- expect(action).to be_manual
+ expect { click_link(action.name.humanize) }
+ .not_to change { Ci::Pipeline.count }
- expect { click_link(action.name.humanize) }
- .not_to change { Ci::Pipeline.count }
+ expect(page).to have_content(action.name)
+ expect(action.reload).to be_pending
+ end
+ end
- expect(page).to have_content(action.name)
- expect(action.reload).to be_pending
+ context 'when user has no ability to trigger a deployment' do
+ it 'does not show a play button' do
+ expect(page).not_to have_link(action.name.humanize)
+ end
end
context 'with external_url' do
@@ -134,12 +146,23 @@ feature 'Environment', :feature do
on_stop: 'close_app')
end
- given(:role) { :master }
+ context 'when user has ability to stop environment' do
+ given(:permissions) do
+ create(:protected_branch, :developers_can_merge,
+ name: action.ref, project: project)
+ end
- scenario 'does allow to stop environment' do
- click_link('Stop')
+ it 'allows to stop environment' do
+ click_link('Stop')
- expect(page).to have_content('close_app')
+ expect(page).to have_content('close_app')
+ end
+ end
+
+ context 'when user has no ability to stop environment' do
+ it 'does not allow to stop environment' do
+ expect(page).to have_no_link('Stop')
+ end
end
context 'for reporter' do
@@ -150,12 +173,6 @@ feature 'Environment', :feature do
end
end
end
-
- context 'without stop action' do
- scenario 'does allow to stop environment' do
- click_link('Stop')
- end
- end
end
context 'when environment is stopped' do
diff --git a/spec/lib/gitlab/chat_commands/command_spec.rb b/spec/lib/gitlab/chat_commands/command_spec.rb
index eb4f06b371c..13e6953147b 100644
--- a/spec/lib/gitlab/chat_commands/command_spec.rb
+++ b/spec/lib/gitlab/chat_commands/command_spec.rb
@@ -58,9 +58,12 @@ describe Gitlab::ChatCommands::Command, service: true do
end
end
- context 'and user does have deployment permission' do
+ context 'and user has deployment permission' do
before do
- build.project.add_master(user)
+ build.project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: build.ref, project: project)
end
it 'returns action' do
diff --git a/spec/lib/gitlab/chat_commands/deploy_spec.rb b/spec/lib/gitlab/chat_commands/deploy_spec.rb
index b33389d959e..46dbdeae37c 100644
--- a/spec/lib/gitlab/chat_commands/deploy_spec.rb
+++ b/spec/lib/gitlab/chat_commands/deploy_spec.rb
@@ -7,7 +7,12 @@ describe Gitlab::ChatCommands::Deploy, service: true do
let(:regex_match) { described_class.match('deploy staging to production') }
before do
- project.add_master(user)
+ # Make it possible to trigger protected manual actions for developers.
+ #
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: 'master', project: project)
end
subject do
diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb
index 185bb9098da..3f30b2c38f2 100644
--- a/spec/lib/gitlab/ci/status/build/factory_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb
@@ -224,7 +224,10 @@ describe Gitlab::Ci::Status::Build::Factory do
context 'when user has ability to play action' do
before do
- build.project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: build.ref, project: project)
end
it 'fabricates status that has action' do
diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb
index f5d0f977768..0e15a5f3c6b 100644
--- a/spec/lib/gitlab/ci/status/build/play_spec.rb
+++ b/spec/lib/gitlab/ci/status/build/play_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
describe Gitlab::Ci::Status::Build::Play do
let(:user) { create(:user) }
+ let(:project) { build.project }
let(:build) { create(:ci_build, :manual) }
let(:status) { Gitlab::Ci::Status::Core.new(build, user) }
@@ -15,8 +16,13 @@ describe Gitlab::Ci::Status::Build::Play do
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) }
+ context 'when user is allowed to trigger protected action' do
+ before do
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: build.ref, project: project)
+ end
it { is_expected.to have_action }
end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 12519de8636..9fbe19b04d5 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -227,7 +227,10 @@ describe Environment, models: true do
context 'when user is allowed to stop environment' do
before do
- project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: 'master', project: project)
end
context 'when action did not yet finish' do
diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb
index b5eb84ae43b..6d5e1046e86 100644
--- a/spec/serializers/build_entity_spec.rb
+++ b/spec/serializers/build_entity_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
describe BuildEntity do
let(:user) { create(:user) }
let(:build) { create(:ci_build) }
+ let(:project) { build.project }
let(:request) { double('request') }
before do
@@ -52,7 +53,10 @@ describe BuildEntity do
context 'when user is allowed to trigger action' do
before do
- build.project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: 'master', project: project)
end
it 'contains path to play action' do
diff --git a/spec/services/ci/play_build_service_spec.rb b/spec/services/ci/play_build_service_spec.rb
index d6f9fa42045..ea211de1f82 100644
--- a/spec/services/ci/play_build_service_spec.rb
+++ b/spec/services/ci/play_build_service_spec.rb
@@ -13,8 +13,11 @@ describe Ci::PlayBuildService, '#execute', :services do
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)
+ it 'allows user to play build if protected branch rules are met' do
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: build.ref, project: project)
service.execute(build)
@@ -45,7 +48,10 @@ describe Ci::PlayBuildService, '#execute', :services do
let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
before do
- project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: build.ref, project: project)
end
it 'enqueues the build' do
@@ -64,7 +70,10 @@ describe Ci::PlayBuildService, '#execute', :services do
let(:build) { create(:ci_build, when: :manual, pipeline: pipeline) }
before do
- project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: build.ref, project: project)
end
it 'duplicates the build' do
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index fc5de5d069a..1557cb3c938 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/spec/services/ci/process_pipeline_service_spec.rb
@@ -333,10 +333,11 @@ describe Ci::ProcessPipelineService, '#execute', :services do
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.
+ # Users need ability to merge into a branch in order to trigger
+ # protected manual actions.
#
- project.add_master(user)
+ create(:protected_branch, :developers_can_merge,
+ name: 'master', project: project)
end
it 'properly processes entire pipeline' do
diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb
index d941d56c0d8..3e860203063 100644
--- a/spec/services/ci/retry_pipeline_service_spec.rb
+++ b/spec/services/ci/retry_pipeline_service_spec.rb
@@ -6,9 +6,12 @@ describe Ci::RetryPipelineService, '#execute', :services do
let(:pipeline) { create(:ci_pipeline, project: project) }
let(:service) { described_class.new(project, user) }
- context 'when user has ability to modify pipeline' do
+ context 'when user has full ability to modify pipeline' do
before do
- project.add_master(user)
+ project.add_developer(user)
+
+ create(:protected_branch, :developers_can_merge,
+ name: pipeline.ref, project: project)
end
context 'when there are already retried jobs present' do
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-31 10:38:47 +0300