diff --git a/app/policies/ci/pipeline_policy.rb b/app/policies/ci/pipeline_policy.rb index bd6e580c33ae1bb7d08726217dbbfd03c8f2335b..c01162a86dfbb515086f72a27f24fe059a8fd312 100644 --- a/app/policies/ci/pipeline_policy.rb +++ b/app/policies/ci/pipeline_policy.rb @@ -27,10 +27,14 @@ class PipelinePolicy < BasePolicy prevent :read_pipeline end - rule { protected_ref }.prevent :update_pipeline + rule { protected_ref }.policy do + prevent :update_pipeline + prevent :cancel_pipeline + end rule { can?(:public_access) & branch_allows_collaboration }.policy do enable :update_pipeline + enable :cancel_pipeline end rule { can?(:owner_access) }.policy do @@ -45,15 +49,6 @@ class PipelinePolicy < BasePolicy enable :read_pipeline_variable end - # TODO: splitting out cancel from update in Issue #20207 - rule { can?(:update_pipeline) }.policy do - enable :cancel_pipeline - end - - rule { ~can?(:update_pipeline) }.policy do - prevent :cancel_pipeline - end - rule { project_allows_read_dependency }.policy do enable :read_dependency end diff --git a/app/services/ci/cancel_pipeline_service.rb b/app/services/ci/cancel_pipeline_service.rb index 52d38520fc3d90401ce3c85b158d9b70e018f680..38053b1392110f6848f81b7083e6223186e0325b 100644 --- a/app/services/ci/cancel_pipeline_service.rb +++ b/app/services/ci/cancel_pipeline_service.rb @@ -24,11 +24,7 @@ def initialize( end def execute - unless can?(current_user, :cancel_pipeline, pipeline) - return ServiceResponse.error( - message: 'Insufficient permissions to cancel the pipeline', - reason: :insufficient_permissions) - end + return permission_error_response unless can?(current_user, :cancel_pipeline, pipeline) force_execute end @@ -103,6 +99,13 @@ def cancel_job(job) job.cancel end + def permission_error_response + ServiceResponse.error( + message: 'Insufficient permissions to cancel the pipeline', + reason: :insufficient_permissions + ) + end + # For parent child-pipelines only (not multi-project) def cancel_children pipeline.all_child_pipelines.each do |child_pipeline| diff --git a/config/feature_flags/development/restrict_pipeline_cancellation_by_role.yml b/config/feature_flags/development/restrict_pipeline_cancellation_by_role.yml new file mode 100644 index 0000000000000000000000000000000000000000..0ef8a5d38dbec33f49d3209969f4814186a321f9 --- /dev/null +++ b/config/feature_flags/development/restrict_pipeline_cancellation_by_role.yml @@ -0,0 +1,8 @@ +--- +name: restrict_pipeline_cancellation_by_role +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135047 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/429699 +milestone: '16.6' +type: development +group: group::pipeline execution +default_enabled: false diff --git a/db/migrate/20231024212214_add_pipeline_cancel_role_restriction_enum.rb b/db/migrate/20231024212214_add_pipeline_cancel_role_restriction_enum.rb new file mode 100644 index 0000000000000000000000000000000000000000..ab26a1d783be2cb49a98938328aa32d51b495c9c --- /dev/null +++ b/db/migrate/20231024212214_add_pipeline_cancel_role_restriction_enum.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddPipelineCancelRoleRestrictionEnum < Gitlab::Database::Migration[2.1] + def up + add_column :project_ci_cd_settings, :restrict_pipeline_cancellation_role, + :integer, limit: 2, default: 0, null: false + end + + def down + remove_column :project_ci_cd_settings, :restrict_pipeline_cancellation_role + end +end diff --git a/db/schema_migrations/20231024212214 b/db/schema_migrations/20231024212214 new file mode 100644 index 0000000000000000000000000000000000000000..d3ad27bd4dd9c6080991deb1140a6cfdbcdfcc5c --- /dev/null +++ b/db/schema_migrations/20231024212214 @@ -0,0 +1 @@ +c5884c327b3be31122ca36302f8fbd36666ddee07229480884c8c64af825c03f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0da8e0b7ce14b675d0bab218e0f15b13289a1bad..c2a98c75d49fc5be7f49fd50e99ea078cc70e2e7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -21368,7 +21368,8 @@ CREATE TABLE project_ci_cd_settings ( allow_fork_pipelines_to_run_in_parent_project boolean DEFAULT true NOT NULL, inbound_job_token_scope_enabled boolean DEFAULT true NOT NULL, forward_deployment_rollback_allowed boolean DEFAULT true NOT NULL, - merge_trains_skip_train_allowed boolean DEFAULT false NOT NULL + merge_trains_skip_train_allowed boolean DEFAULT false NOT NULL, + restrict_pipeline_cancellation_role smallint DEFAULT 0 NOT NULL ); CREATE SEQUENCE project_ci_cd_settings_id_seq diff --git a/ee/app/controllers/ee/projects_controller.rb b/ee/app/controllers/ee/projects_controller.rb index e901a242fa2590224747d1ccb3f47a459f6b2e46..b9c2c63c5d5aaa8804a2547ee8d649d4aa87e0a6 100644 --- a/ee/app/controllers/ee/projects_controller.rb +++ b/ee/app/controllers/ee/projects_controller.rb @@ -130,6 +130,7 @@ def project_params_ee attrs << %i[merge_pipelines_enabled] if allow_merge_pipelines_params? attrs << %i[merge_trains_enabled] if allow_merge_trains_params? attrs << %i[merge_trains_skip_train_allowed] if allow_merge_trains_params? + attrs << %i[restrict_pipeline_cancellation_role] if project&.ci_cancellation_restriction&.enabled? attrs += merge_request_rules_params diff --git a/ee/app/models/ci/project_cancellation_restriction.rb b/ee/app/models/ci/project_cancellation_restriction.rb new file mode 100644 index 0000000000000000000000000000000000000000..7eaca4f6a619df543a52c840eb8f0e17d58f4042 --- /dev/null +++ b/ee/app/models/ci/project_cancellation_restriction.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Ci + class ProjectCancellationRestriction + # Checks if cancellation restrictions are applied for piplines and processables + # based on the given project + include Gitlab::Utils::StrongMemoize + + def initialize(project) + @project = project + @ci_settings = project.ci_cd_settings + end + + def maintainers_only_allowed? + return false unless enabled? + + @ci_settings.restrict_pipeline_cancellation_role_maintainer? + end + + def no_one_allowed? + return false unless enabled? + + @ci_settings.restrict_pipeline_cancellation_role_no_one? + end + + def enabled? + Feature.enabled?(:restrict_pipeline_cancellation_by_role, @project) && + @project.licensed_feature_available?(:ci_pipeline_cancellation_restrictions) + end + strong_memoize_attr :enabled? + end +end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index 5c8deff985cd969f31a7fa367e3a6a7b26c0ece8..320abfc99f7da3c3dda7bee6b898a20691e85a27 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -342,6 +342,7 @@ def lock_for_confirmation!(id) delegate :merge_pipelines_enabled, :merge_pipelines_enabled=, to: :ci_cd_settings, allow_nil: true delegate :merge_trains_enabled, :merge_trains_enabled=, to: :ci_cd_settings, allow_nil: true delegate :merge_trains_skip_train_allowed, :merge_trains_skip_train_allowed=, to: :ci_cd_settings, allow_nil: true + delegate :restrict_pipeline_cancellation_role, :restrict_pipeline_cancellation_role=, to: :ci_cd_settings, allow_nil: false delegate :auto_rollback_enabled, :auto_rollback_enabled=, to: :ci_cd_settings, allow_nil: true @@ -1101,6 +1102,10 @@ def downstream_projects_count downstream_project_subscriptions.count end + def ci_cancellation_restriction + ::Ci::ProjectCancellationRestriction.new(self) + end + def merge_pipelines_enabled? return false unless ci_cd_settings diff --git a/ee/app/models/ee/project_ci_cd_setting.rb b/ee/app/models/ee/project_ci_cd_setting.rb index eefd0e7985c102591569530685764dcb9bd10f77..b965e12023b8180bbd00dc37e0c7512f11ed5411 100644 --- a/ee/app/models/ee/project_ci_cd_setting.rb +++ b/ee/app/models/ee/project_ci_cd_setting.rb @@ -4,6 +4,14 @@ module EE module ProjectCiCdSetting extend ActiveSupport::Concern + prepended do + enum restrict_pipeline_cancellation_role: { + developer: 0, + maintainer: 1, + no_one: 2 + }, _prefix: true + end + def merge_pipelines_enabled? project.feature_available?(:merge_pipelines) && super end diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index 4bf50f63407f100452a70766a188eeb48ec88235..48e796f868c06f6405a08e69612d2bbafcb6ab47 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -83,6 +83,7 @@ class Features ci_cd_projects ci_namespace_catalog ci_secrets_management + ci_pipeline_cancellation_restrictions cluster_agents_ci_impersonation cluster_agents_user_impersonation cluster_deployments diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 3ee7600382ada934b80833c00cbc0fa1eceb7862..8f5dbfde6d4afa4b2aa0074cf35916eddb60f192 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -331,6 +331,14 @@ module ProjectPolicy (custom_roles_allowed? && merge_requests_is_a_private_feature? && role_enables_admin_merge_request?)) end + condition(:ci_cancellation_maintainers_only, scope: :subject) do + project.ci_cancellation_restriction.maintainers_only_allowed? + end + + condition(:ci_cancellation_no_one, scope: :subject) do + project.ci_cancellation_restriction.no_one_allowed? + end + rule { visual_review_bot }.policy do prevent :read_note enable :create_note @@ -769,6 +777,16 @@ module ProjectPolicy rule { can?(:reporter_access) & observability_metrics_enabled }.policy do enable :read_observability_metrics end + + rule { ci_cancellation_maintainers_only & ~can?(:maintainer_access) }.policy do + prevent :cancel_pipeline + prevent :cancel_build + end + + rule { ci_cancellation_no_one }.policy do + prevent :cancel_pipeline + prevent :cancel_build + end end override :lookup_access_level! diff --git a/ee/spec/controllers/projects_controller_spec.rb b/ee/spec/controllers/projects_controller_spec.rb index 4a062fc9222838d399a515cbcddf1abb7d011feb..613d1a02c4d0caf18b9a87b7324fdbcaced1816c 100644 --- a/ee/spec/controllers/projects_controller_spec.rb +++ b/ee/spec/controllers/projects_controller_spec.rb @@ -382,6 +382,43 @@ end end + context 'when restrict_pipeline_cancellation_role is specified' do + let(:settings) { project.ci_cd_settings } + let(:params) { { restrict_pipeline_cancellation_role: :maintainer } } + let(:request) do + put :update, params: { namespace_id: project.namespace, id: project, project: params } + project.reload + end + + let(:developer_restriction) { settings.restrict_pipeline_cancellation_role_developer? } + + context 'when the feature is enabled' do + before do + allow_next_instance_of(Ci::ProjectCancellationRestriction) do |cr| + allow(cr).to receive(:enabled?).and_return(true) + end + end + + it 'updates the parameter' do + expect { request }.to change { + project.reload.ci_cd_settings.restrict_pipeline_cancellation_role_maintainer? + }.from(false).to(true) + end + end + + context 'when the feature is disabled' do + before do + allow_next_instance_of(Ci::ProjectCancellationRestriction) do |cr| + allow(cr).to receive(:enabled?).and_return(false) + end + end + + it 'will not update the parameter' do + expect { request }.not_to change { developer_restriction } + end + end + end + context 'when only_allow_merge_if_all_status_checks_passed param is specified' do let(:params) { { project_setting_attributes: { only_allow_merge_if_all_status_checks_passed: true } } } diff --git a/ee/spec/models/ci/project_cancellation_restriction_spec.rb b/ee/spec/models/ci/project_cancellation_restriction_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..26f57ad4a9403781204a64de8bc4c56e624f560b --- /dev/null +++ b/ee/spec/models/ci/project_cancellation_restriction_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ci::ProjectCancellationRestriction, feature_category: :continuous_integration do + let(:project) { create_default(:project) } + let(:cancellation_restriction) { described_class.new(project) } + let(:settings) { project.ci_cd_settings } + let(:roles) { settings.class.restrict_pipeline_cancellation_roles } + + describe '#maintainers_only_allowed?' do + context 'when cancellation restrictions are enabled' do + before do + stub_enabled + end + + it 'returns true if maintainers are the only ones allowed to cancel' do + settings.update!(restrict_pipeline_cancellation_role: roles[:maintainer]) + + expect(cancellation_restriction.maintainers_only_allowed?).to be_truthy + end + + [:no_one, :developer].each do |role| + it "returns false if #{role} is allowed to cancel" do + settings.update!(restrict_pipeline_cancellation_role: role) + + expect(cancellation_restriction.maintainers_only_allowed?).to be_falsy + end + end + end + + context 'when cancellation restrictions are disabled' do + before do + stub_disabled + end + + it 'returns false' do + expect(cancellation_restriction.maintainers_only_allowed?).to be_falsy + end + end + end + + describe '#no_one_allowed?' do + context 'when cancellation restrictions are enabled' do + before do + stub_enabled + end + + it 'returns true if no one is allowed to cancel' do + settings.update!(restrict_pipeline_cancellation_role: roles[:no_one]) + + expect(cancellation_restriction.no_one_allowed?).to be_truthy + end + + [:maintainer, :developer].each do |role| + it "returns false if #{role} is allowed to cancel" do + settings.update!(restrict_pipeline_cancellation_role: role) + + expect(cancellation_restriction.no_one_allowed?).to be_falsy + end + end + end + + context 'when cancellation restrictions are disabled' do + before do + stub_disabled + end + + it 'returns false' do + expect(cancellation_restriction.no_one_allowed?).to be_falsy + end + end + end + + describe '#enabled?' do + context 'when the feature is enabled and licensed' do + before do + stub_enabled + end + + it 'returns true' do + expect(cancellation_restriction.enabled?).to be_truthy + end + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(restrict_pipeline_cancellation_by_role: false) + stub_licensed_features(ci_pipeline_cancellation_restrictions: true) + end + + it 'returns false' do + expect(cancellation_restriction.enabled?).to be_falsy + end + end + + context 'when the feature is enabled but not licensed' do + before do + stub_licensed_features(ci_pipeline_cancellation_restrictions: false) + end + + it 'returns false' do + expect(cancellation_restriction.enabled?).to be_falsy + end + end + + context 'when the feature is disabled and not licensed' do + before do + stub_disabled + end + + it 'returns false' do + expect(cancellation_restriction.enabled?).to be_falsy + end + end + end + + def stub_enabled + stub_licensed_features(ci_pipeline_cancellation_restrictions: true) + end + + def stub_disabled + stub_feature_flags(restrict_pipeline_cancellation_by_role: false) + stub_licensed_features(ci_pipeline_cancellation_restrictions: false) + end +end diff --git a/ee/spec/models/ee/project_spec.rb b/ee/spec/models/ee/project_spec.rb index 93d75fb0869733c47e7ff0c44bf796e18d6fe308..693ac30687166106525e2934c97f1d0f2576f39e 100644 --- a/ee/spec/models/ee/project_spec.rb +++ b/ee/spec/models/ee/project_spec.rb @@ -97,9 +97,16 @@ } end - let(:exclude_attributes) { [] } + let(:exclude_attributes) do + [ + 'restrict_pipeline_cancellation_role' + ] + end end + # can't be tested above because it can be nil + it { is_expected.to delegate_method(:restrict_pipeline_cancellation_role).to(:ci_cd_settings) } + describe '#merge_pipelines_enabled?' do it_behaves_like 'a ci_cd_settings predicate method' do let(:delegated_method) { :merge_pipelines_enabled? } @@ -3575,6 +3582,13 @@ def stub_default_url_options(host) end end + describe '#ci_cancellation_restriction' do + it 'returns the initalized cancellation restrication object' do + expect(project.ci_cancellation_restriction.class).to be Ci::ProjectCancellationRestriction + expect(project.ci_cancellation_restriction).to respond_to(:enabled?) + end + end + describe '#visible_approval_rules' do let(:scan_finding_rule) { create(:approval_project_rule, :scan_finding, project: project) } diff --git a/ee/spec/models/project_ci_cd_setting_spec.rb b/ee/spec/models/project_ci_cd_setting_spec.rb index 89cc8f9152967d14f0b94c36f124ac9849184603..2fc9db61ff8c8951b51a9acc0260eeb7399079fa 100644 --- a/ee/spec/models/project_ci_cd_setting_spec.rb +++ b/ee/spec/models/project_ci_cd_setting_spec.rb @@ -5,8 +5,24 @@ RSpec.describe ProjectCiCdSetting, feature_category: :continuous_integration do using RSpec::Parameterized::TableSyntax + let_it_be(:project) { create_default(:project) } + let(:settings) { project.reload.ci_cd_settings } + it { is_expected.to validate_inclusion_of(:merge_trains_skip_train_allowed).in_array([true, false]) } + describe '#restrict_pipeline_cancellation_role' do + it 'defines an enum' do + described_class.restrict_pipeline_cancellation_roles.each_key do |role| + settings.update!(restrict_pipeline_cancellation_role: role) + expect(settings.restrict_pipeline_cancellation_role).to eq role + end + end + + it 'defaults to developer' do + expect(settings.restrict_pipeline_cancellation_role).to eq('developer') + end + end + describe '#merge_pipelines_enabled?' do subject { project.merge_pipelines_enabled? } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 534112338d1dd926f31239d55038c362388b2b17..d2390931ffd138caf3b83528f6fdd3f98d2c9fab 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -1679,6 +1679,84 @@ end end + shared_examples_for 'prevents CI cancellation ability' do + using RSpec::Parameterized::TableSyntax + + context 'when feature is enabled' do + where(:restricted_role, :actual_role, :allowed) do + :developer | :guest | false + :developer | :reporter | false + :developer | :developer | true + :developer | :maintainer | true + :developer | :owner | true + :maintainer | :guest | false + :maintainer | :reporter | false + :maintainer | :developer | false + :maintainer | :maintainer | true + :maintainer | :owner | true + :no_one | :guest | false + :no_one | :reporter | false + :no_one | :developer | false + :no_one | :maintainer | false + :no_one | :owner | false + end + + with_them do + let(:current_user) { public_send(actual_role) } + + before do + stub_licensed_features(ci_pipeline_cancellation_restrictions: true) + project.update!(restrict_pipeline_cancellation_role: restricted_role) + end + + it { is_expected.to(allowed ? be_allowed(ability) : be_disallowed(ability)) } + end + end + + context 'when feature is disabled' do + where(:restricted_role, :actual_role, :allowed) do + :developer | :guest | false + :developer | :reporter | false + :developer | :developer | true + :developer | :maintainer | true + :developer | :owner | true + :maintainer | :guest | false + :maintainer | :reporter | false + :maintainer | :developer | true + :maintainer | :maintainer | true + :maintainer | :owner | true + :no_one | :guest | false + :no_one | :reporter | false + :no_one | :developer | true + :no_one | :maintainer | true + :no_one | :owner | true + end + + with_them do + let(:current_user) { public_send(actual_role) } + + before do + stub_feature_flags(restrict_pipeline_cancellation_by_role: false) + project.update!(restrict_pipeline_cancellation_role: restricted_role) + end + + it { is_expected.to(allowed ? be_allowed(ability) : be_disallowed(ability)) } + end + end + end + + describe 'prevents cancel_pipeline when CI cancllation restricted' do + let(:ability) { :cancel_pipeline } + + it_behaves_like 'prevents CI cancellation ability' + end + + describe 'prevents cancel_build when CI cancllation restricted' do + let(:ability) { :cancel_build } + + it_behaves_like 'prevents CI cancellation ability' + end + describe ':compliance_framework_available' do using RSpec::Parameterized::TableSyntax diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5f1054a35bbae8bb325526af6be447706413ea63..290f557cdd67dabfd2d1af242ce4eeb72fe28cb4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1182,6 +1182,7 @@ merge_trains_enabled auto_rollback_enabled merge_trains_skip_train_allowed + restrict_pipeline_cancellation_role ] end end diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index ec98df22af792b22154ad3e2881ae62223faeb39..165ea7bf66e36a6543159f351ab872b44f07e276 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -98,6 +98,7 @@ ci_cd_settings: - merge_pipelines_enabled - auto_rollback_enabled - inbound_job_token_scope_enabled + - restrict_pipeline_cancellation_role remapped_attributes: default_git_depth: ci_default_git_depth forward_deployment_enabled: ci_forward_deployment_enabled