From dc5bcae462e11f9e3359ef9fc6ee7a522a71724a Mon Sep 17 00:00:00 2001 From: mo khan Date: Thu, 2 May 2024 13:41:58 -0600 Subject: [PATCH 1/3] Add `admin_runners` custom ability This new custom ability allows members to create, view, edit, and delete group or project Runners. Includes configuring Runner settings. Changelog: added EE: true --- .../groups/settings/ci_cd_controller.rb | 11 +- .../projects/runners_controller.rb | 2 +- .../projects/settings/ci_cd_controller.rb | 11 +- app/policies/group_policy.rb | 1 + .../namespaces/user_namespace_policy.rb | 1 + app/policies/project_policy.rb | 1 + .../groups/update_shared_runners_service.rb | 2 +- .../json_schemas/member_role_permissions.json | 3 + .../groups/settings/ci_cd/show.html.haml | 3 +- .../projects/settings/ci_cd/show.html.haml | 2 + doc/api/graphql/reference/index.md | 1 + doc/user/custom_roles/abilities.md | 6 + .../ci/namespace_ci_cd_settings_update.rb | 2 +- .../types/ci/namespace_ci_cd_setting_type.rb | 2 +- ee/app/policies/ee/ci/runner_policy.rb | 6 + ee/app/policies/ee/group_policy.rb | 2 + ee/app/policies/ee/project_policy.rb | 2 + ee/config/custom_abilities/admin_runners.yml | 13 ++ .../wip/custom_ability_admin_runners.yml | 9 + .../ee/sidebars/groups/menus/settings_menu.rb | 2 +- .../sidebars/projects/menus/settings_menu.rb | 3 +- .../groups/menus/settings_menu_spec.rb | 1 + .../projects/menus/settings_menu_spec.rb | 2 + ee/spec/policies/ee/ci/runner_policy_spec.rb | 39 +++++ ee/spec/policies/group_policy_spec.rb | 15 ++ ee/spec/policies/project_policy_spec.rb | 15 ++ .../admin_runners/request_spec.rb | 155 ++++++++++++++++++ .../namespaces/user_namespace_policy_spec.rb | 2 +- .../policies/group_policy_shared_context.rb | 1 + .../policies/project_policy_shared_context.rb | 2 +- 30 files changed, 306 insertions(+), 11 deletions(-) create mode 100644 ee/config/custom_abilities/admin_runners.yml create mode 100644 ee/config/feature_flags/wip/custom_ability_admin_runners.yml create mode 100644 ee/spec/requests/custom_roles/admin_runners/request_spec.rb diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index 9e00fa69f94ee0..5a0ce0a9bc9bf5 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -6,7 +6,7 @@ class CiCdController < Groups::ApplicationController layout 'group_settings' skip_cross_project_access_check :show before_action :authorize_admin_group!, except: :show - before_action :authorize_admin_cicd_variables!, only: :show + before_action :authorize_admin_cicd_settings!, only: :show before_action :authorize_update_max_artifacts_size!, only: [:update] before_action :define_variables, only: [:show] before_action :push_licensed_features, only: [:show] @@ -47,6 +47,15 @@ def update_auto_devops private + def authorize_admin_cicd_settings! + return if can_any?(current_user, [ + :admin_cicd_variables, + :admin_runner + ], group) + + access_denied! + end + def define_variables define_ci_variables end diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index c428f78960bab8..bbdd27a7184eeb 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Projects::RunnersController < Projects::ApplicationController - before_action :authorize_admin_build! + before_action :authorize_admin_runner! before_action :authorize_create_runner!, only: [:new, :register] before_action :runner, only: [:edit, :update, :destroy, :pause, :resume, :show, :register] diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 193ef7bb9afd65..c27c9c746f3432 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -9,7 +9,7 @@ class CiCdController < Projects::ApplicationController layout 'project_settings' before_action :authorize_admin_pipeline!, except: :show - before_action :authorize_admin_cicd_variables!, only: :show + before_action :authorize_admin_cicd_settings!, only: :show before_action :check_builds_available! before_action :define_variables @@ -75,6 +75,15 @@ def runner_setup_scripts private + def authorize_admin_cicd_settings! + return if can_any?(current_user, [ + :admin_cicd_variables, + :admin_runner + ], project) + + access_denied! + end + def highlight_badge(name, content, language = nil) Gitlab::Highlight.highlight(name, content, language: language) end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 554bdf97dfce1e..b4cf906e37ccc9 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -267,6 +267,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :admin_namespace enable :admin_group_member enable :admin_package + enable :admin_runner enable :admin_integrations enable :change_visibility_level diff --git a/app/policies/namespaces/user_namespace_policy.rb b/app/policies/namespaces/user_namespace_policy.rb index f2ac0f0403d419..325bd88027aa18 100644 --- a/app/policies/namespaces/user_namespace_policy.rb +++ b/app/policies/namespaces/user_namespace_policy.rb @@ -13,6 +13,7 @@ class UserNamespacePolicy < ::NamespacePolicy enable :create_projects enable :import_projects enable :admin_namespace + enable :admin_runner enable :read_namespace enable :read_namespace_via_membership enable :read_statistics diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index bb6f917d426916..22728c08b306fe 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -619,6 +619,7 @@ class ProjectPolicy < BasePolicy enable :read_import_error enable :admin_cicd_variables enable :admin_push_rules + enable :admin_runner enable :manage_deploy_tokens enable :manage_merge_request_settings enable :change_restrict_user_defined_variables diff --git a/app/services/groups/update_shared_runners_service.rb b/app/services/groups/update_shared_runners_service.rb index 60ef838a34c33c..dfa7631f499d38 100644 --- a/app/services/groups/update_shared_runners_service.rb +++ b/app/services/groups/update_shared_runners_service.rb @@ -3,7 +3,7 @@ module Groups class UpdateSharedRunnersService < Groups::BaseService def execute - return error('Operation not allowed', 403) unless can?(current_user, :admin_group, group) + return error('Operation not allowed', 403) unless can?(current_user, :admin_runner, group) validate_params diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index e56df0c903036a..843bcbc4f6c9e0 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -22,6 +22,9 @@ "admin_push_rules": { "type": "boolean" }, + "admin_runners": { + "type": "boolean" + }, "admin_terraform_state": { "type": "boolean" }, diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index c03747400348f3..3f6a9a976f1168 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/app/views/groups/settings/ci_cd/show.html.haml @@ -26,7 +26,7 @@ .settings-content = render 'ci/variables/index', save_endpoint: group_variables_path -- if can?(current_user, :admin_group, @group) +- if can?(current_user, :admin_runner, @group) %section.settings#runners-settings.no-animate{ class: ('expanded' if expanded) } .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only @@ -39,6 +39,7 @@ .settings-content = render 'groups/runners/settings' +- if can?(current_user, :admin_group, @group) %section.settings#auto-devops-settings.no-animate{ class: ('expanded' if expanded) } .settings-header %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only diff --git a/app/views/projects/settings/ci_cd/show.html.haml b/app/views/projects/settings/ci_cd/show.html.haml index 322d958763d395..6df016c4ed265e 100644 --- a/app/views/projects/settings/ci_cd/show.html.haml +++ b/app/views/projects/settings/ci_cd/show.html.haml @@ -34,6 +34,7 @@ = render_if_exists 'projects/settings/ci_cd/protected_environments', expanded: expanded +- if can?(current_user, :admin_runner, @project) - expand_runners = expanded || params[:expand_runners] %section.settings.no-animate#js-runners-settings{ class: ('expanded' if expand_runners), data: { testid: 'runners-settings-content' } } .settings-header @@ -47,6 +48,7 @@ .settings-content = render 'projects/runners/settings' +- if can?(current_user, :admin_pipeline, @project) - if Gitlab::CurrentSettings.current_application_settings.keep_latest_artifact? %section.settings.no-animate#js-artifacts-settings{ class: ('expanded' if expanded) } .settings-header diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a98d44929ad09b..4080f8a90dadac 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -34350,6 +34350,7 @@ Member role permission. | `ADMIN_INTEGRATIONS` | Create, read, update, and delete integrations with external applications. | | `ADMIN_MERGE_REQUEST` | Allows approval of merge requests. | | `ADMIN_PUSH_RULES` | Configure push rules for repositories at the group or project level. | +| `ADMIN_RUNNERS` | Create, view, edit, and delete group or project Runners. Includes configuring Runner settings. | | `ADMIN_TERRAFORM_STATE` | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | | `ADMIN_VULNERABILITY` | Edit the vulnerability object, including the status and linking an issue. Includes the `read_vulnerability` permission actions. | | `ADMIN_WEB_HOOK` | Manage webhooks. | diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index e9e5f33682115d..427a23ac22dad8 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -62,6 +62,12 @@ These requirements are documented in the `Required permission` column in the fol |:-----|:------------|:------------------|:---------|:--------------|:---------| | [`admin_integrations`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/TODO) | | Create, read, update, and delete integrations with external applications. | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/460522) | | | +## Runner + +| Name | Required permission | Description | Introduced in | Feature flag | Enabled in | +|:-----|:------------|:------------------|:---------|:--------------|:---------| +| [`admin_runners`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151825) | | Create, view, edit, and delete group or project Runners. Includes configuring Runner settings. | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/442851) | `custom_ability_admin_runners` | | + ## Secrets management | Name | Required permission | Description | Introduced in | Feature flag | Enabled in | diff --git a/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb b/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb index f8857a94beca08..4d00a2c5935c2c 100644 --- a/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb +++ b/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb @@ -7,7 +7,7 @@ class NamespaceCiCdSettingsUpdate < BaseMutation include ResolvesNamespace - authorize :admin_namespace + authorize :admin_runner argument :allow_stale_runner_pruning, GraphQL::Types::Boolean, required: false, diff --git a/ee/app/graphql/types/ci/namespace_ci_cd_setting_type.rb b/ee/app/graphql/types/ci/namespace_ci_cd_setting_type.rb index f7d722a663dff1..a3aba78fcafed8 100644 --- a/ee/app/graphql/types/ci/namespace_ci_cd_setting_type.rb +++ b/ee/app/graphql/types/ci/namespace_ci_cd_setting_type.rb @@ -5,7 +5,7 @@ module Ci class NamespaceCiCdSettingType < BaseObject graphql_name 'NamespaceCiCdSetting' - authorize :admin_namespace + authorize :admin_runner field :allow_stale_runner_pruning, GraphQL::Types::Boolean, null: true, diff --git a/ee/app/policies/ee/ci/runner_policy.rb b/ee/app/policies/ee/ci/runner_policy.rb index 70a43aed142977..a5be8e92bed93c 100644 --- a/ee/app/policies/ee/ci/runner_policy.rb +++ b/ee/app/policies/ee/ci/runner_policy.rb @@ -10,6 +10,12 @@ module RunnerPolicy enable :read_runner enable :read_builds end + + condition(:custom_role_enables_admin_runners) do + ::Authz::CustomAbility.allowed?(@user, :admin_runners, @subject) + end + + rule { custom_role_enables_admin_runners }.enable :read_runner end end end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index d7970ea5282dd8..478c47e8b00b0e 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -592,6 +592,8 @@ module GroupPolicy enable :read_licenses end + rule { custom_role_enables_admin_runners }.enable(*create_read_update_admin_destroy(:runner)) + rule { admin | owner }.policy do enable :owner_access enable :read_billable_member diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index b17b5d7b3602ff..7297de2a4b49cf 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -318,6 +318,8 @@ module ProjectPolicy enable :admin_integrations end + rule { custom_role_enables_admin_runners }.enable(*create_read_update_admin_destroy(:runner)) + condition(:ci_cancellation_maintainers_only, scope: :subject) do project.ci_cancellation_restriction.maintainers_only_allowed? end diff --git a/ee/config/custom_abilities/admin_runners.yml b/ee/config/custom_abilities/admin_runners.yml new file mode 100644 index 00000000000000..e09255970450fd --- /dev/null +++ b/ee/config/custom_abilities/admin_runners.yml @@ -0,0 +1,13 @@ +--- +name: admin_runners +description: Create, view, edit, and delete group or project Runners. Includes configuring + Runner settings. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/442851 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151825 +feature_category: runner +milestone: '17.1' +group_ability: true +project_ability: true +requirements: [] +available_from_access_level: +feature_flag: custom_ability_admin_runners diff --git a/ee/config/feature_flags/wip/custom_ability_admin_runners.yml b/ee/config/feature_flags/wip/custom_ability_admin_runners.yml new file mode 100644 index 00000000000000..6ade5562756799 --- /dev/null +++ b/ee/config/feature_flags/wip/custom_ability_admin_runners.yml @@ -0,0 +1,9 @@ +--- +name: custom_ability_admin_runners +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/442851 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151825 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/461448 +milestone: '17.1' +group: group::authorization +type: wip +default_enabled: false diff --git a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb index 4eb03cc003d7b1..31f7dafbc62fe3 100644 --- a/ee/lib/ee/sidebars/groups/menus/settings_menu.rb +++ b/ee/lib/ee/sidebars/groups/menus/settings_menu.rb @@ -29,7 +29,7 @@ def configure_menu_items add_menu_item_for_abilities(integrations_menu_item, :admin_integrations) add_menu_item_for_abilities(access_tokens_menu_item, :read_resource_access_tokens) add_menu_item_for_abilities(repository_menu_item, [:admin_push_rules, :manage_deploy_tokens]) - add_menu_item_for_abilities(ci_cd_menu_item, :admin_cicd_variables) + add_menu_item_for_abilities(ci_cd_menu_item, [:admin_cicd_variables, :admin_runner]) add_menu_item_for_abilities(billing_menu_item, :read_billing) end end diff --git a/ee/lib/ee/sidebars/projects/menus/settings_menu.rb b/ee/lib/ee/sidebars/projects/menus/settings_menu.rb index 00a068da2280a3..c22fc668da41cd 100644 --- a/ee/lib/ee/sidebars/projects/menus/settings_menu.rb +++ b/ee/lib/ee/sidebars/projects/menus/settings_menu.rb @@ -22,7 +22,8 @@ module SettingsMenu :manage_merge_request_settings ], ci_cd_menu_item: [ - :admin_cicd_variables + :admin_cicd_variables, + :admin_runner ], integrations_menu_item: [ :admin_integrations diff --git a/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb b/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb index 4a1ee1a491bd3e..d82eeb719ab508 100644 --- a/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb +++ b/ee/spec/lib/ee/sidebars/groups/menus/settings_menu_spec.rb @@ -314,6 +314,7 @@ :admin_cicd_variables | 'CI/CD' :admin_compliance_framework | 'General' :admin_push_rules | 'Repository' + :admin_runners | 'CI/CD' :manage_deploy_tokens | 'Repository' :manage_group_access_tokens | 'Access Tokens' :manage_merge_request_settings | 'General' diff --git a/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb b/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb index 9047663cec545b..89f1927225e9a8 100644 --- a/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb +++ b/ee/spec/lib/ee/sidebars/projects/menus/settings_menu_spec.rb @@ -119,6 +119,8 @@ where(:ability, :menu_item) do :admin_cicd_variables | 'CI/CD' :admin_push_rules | 'Repository' + :admin_runners | 'CI/CD' + :manage_deploy_tokens | 'Repository' :manage_merge_request_settings | 'Merge requests' :manage_project_access_tokens | 'Access Tokens' :admin_integrations | 'Integrations' diff --git a/ee/spec/policies/ee/ci/runner_policy_spec.rb b/ee/spec/policies/ee/ci/runner_policy_spec.rb index 22fc47815dd9c1..50eeeeb8b6c6d0 100644 --- a/ee/spec/policies/ee/ci/runner_policy_spec.rb +++ b/ee/spec/policies/ee/ci/runner_policy_spec.rb @@ -45,5 +45,44 @@ end end end + + context 'with `admin_runner` access via a custom role' do + let_it_be_with_reload(:user) { create(:user) } + let_it_be(:role) { create(:member_role, :guest, :admin_runners, namespace: project.group) } + + before do + stub_licensed_features(custom_roles: true) + end + + context 'with project runner' do + let_it_be(:membership) { create(:project_member, :guest, member_role: role, user: user, project: project) } + let_it_be_with_reload(:runner) { create(:ci_runner, :project, projects: [project]) } + + it { expect_allowed :read_runner } + + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new do + described_class.new(user, runner).allowed?(:read_runner) + end + + create_list(:project, 3).each do |project| + project.add_member(user, :guest) + runner.runner_projects.create!(project: project) + end + + expect do + described_class.new(user, runner).allowed?(:read_runner) + end.not_to exceed_query_limit(control) + end + + context 'with `custom_ability_admin_runners` disabled' do + before do + stub_feature_flags(custom_ability_admin_runners: false) + end + + it { expect_disallowed :read_runner } + end + end + end end end diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 33f0fd83c802d4..e35e04778a3f8d 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3671,6 +3671,21 @@ def create_member_role(member, abilities = member_role_abilities) end end + context 'for a member role with `admin_runners` true' do + let(:member_role_abilities) { { admin_runners: true } } + let(:allowed_abilities) do + [ + :admin_runner, + :create_runner, + :destroy_runner, + :read_runner, + :update_runner + ] + end + + it_behaves_like 'custom roles abilities' + end + context 'for a custom role with the `admin_integrations` permission' do let(:member_role_abilities) { { admin_integrations: true } } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 2deb620351467a..47bd43a06d2d0e 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2769,6 +2769,21 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + context 'for a member role with admin_runners true' do + let(:member_role_abilities) { { admin_runners: true } } + let(:allowed_abilities) do + [ + :admin_runner, + :create_runner, + :destroy_runner, + :read_runner, + :update_runner + ] + end + + it_behaves_like 'custom roles abilities' + end + context 'for a member role with read_vulnerability true' do let(:member_role_abilities) { { read_vulnerability: true } } let(:allowed_abilities) do diff --git a/ee/spec/requests/custom_roles/admin_runners/request_spec.rb b/ee/spec/requests/custom_roles/admin_runners/request_spec.rb new file mode 100644 index 00000000000000..58a52e305f64c5 --- /dev/null +++ b/ee/spec/requests/custom_roles/admin_runners/request_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "User with admin_runners custom role", feature_category: :runner do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :in_group) } + let_it_be_with_reload(:group) { project.group } + let_it_be(:role) { create(:member_role, :guest, :admin_runners, namespace: project.group) } + + before do + stub_licensed_features(custom_roles: true) + end + + describe Projects::RunnersController do + let_it_be(:membership) { create(:project_member, :guest, member_role: role, user: user, project: project) } + + before do + sign_in(user) + end + + it "#index" do + get project_runners_path(project) + + expect(response).to redirect_to(project_settings_ci_cd_path(project, anchor: 'js-runners-settings')) + end + + it "#new" do + get new_project_runner_path(project) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "#update" do + runner = create(:ci_runner, :project, active: true, projects: [project]) + + patch project_runner_path(project, runner), params: { runner: { description: "hello world" } } + + expect(response).to redirect_to(project_runner_path(project, runner)) + end + + it "#toggle_shared_runners" do + post toggle_shared_runners_project_runners_path(project) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "#destroy" do + runner = create(:ci_runner, :project, projects: [project]) + + expect_next_instance_of(Ci::Runners::UnregisterRunnerService, runner, user) do |service| + expect(service).to receive(:execute).once.and_call_original + end + + delete project_runner_path(project, runner) + + expect(response).to redirect_to(project_runners_path(project)) + end + + it "#pause" do + runner = create(:ci_runner, :project, active: true, projects: [project]) + + post pause_project_runner_path(project, runner) + + expect(response).to redirect_to(project_runners_path(project)) + end + end + + describe ::Projects::Settings::CiCdController do + let_it_be(:membership) { create(:project_member, :guest, member_role: role, user: user, project: project) } + + before do + sign_in(user) + end + + it "#show" do + get project_settings_ci_cd_path(project) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to include('CI/CD Settings') + end + end + + describe ::Groups::Settings::CiCdController do + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + before do + sign_in(user) + end + + it "#show" do + get group_settings_ci_cd_path(group) + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to include('CI/CD Settings') + end + end + + describe API::Groups do + include ApiHelpers + + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + pending "PUT /groups/:id" do + put api("/groups/#{group.id}", user), params: { + shared_runners_setting: 'disabled_and_unoverridable' + } + + expect(response).to have_gitlab_http_status(:ok) + expect(group.reload.shared_runners_setting).to eq('disabled_and_unoverridable') + end + end + + describe Mutations::Ci::Runner::Create do + include GraphqlHelpers + + let_it_be(:membership) { create(:project_member, :guest, member_role: role, user: user, project: project) } + + it "creates a runner" do + post_graphql_mutation(graphql_mutation(:runner_create, { + runner_type: 'PROJECT_TYPE', + project_id: project.to_global_id + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + mutation_response = graphql_mutation_response(:runner_create) + + expect(mutation_response).to be_present + expect(mutation_response['runner']).to be_present + expect(mutation_response['errors']).to be_empty + end + end + + describe Mutations::Ci::NamespaceCiCdSettingsUpdate do + include GraphqlHelpers + + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } + + it "updates the `allow_stale_runner_pruning` setting" do + post_graphql_mutation(graphql_mutation(:namespace_ci_cd_settings_update, { + full_path: group.full_path, + allow_stale_runner_pruning: true + }), current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(fresh_response_data['errors']).to be_blank + + mutation_response = graphql_mutation_response(:namespace_ci_cd_settings_update) + expect(mutation_response).to be_present + expect(mutation_response['ciCdSettings']).to be_present + expect(mutation_response['errors']).to be_empty + end + end +end diff --git a/spec/policies/namespaces/user_namespace_policy_spec.rb b/spec/policies/namespaces/user_namespace_policy_spec.rb index b4fbc7e0417a58..c88794ffe0140a 100644 --- a/spec/policies/namespaces/user_namespace_policy_spec.rb +++ b/spec/policies/namespaces/user_namespace_policy_spec.rb @@ -8,7 +8,7 @@ let_it_be(:admin) { create(:admin) } let_it_be(:namespace) { create(:user_namespace, owner: owner) } - let(:owner_permissions) { [:owner_access, :create_projects, :admin_namespace, :read_namespace, :read_namespace_via_membership, :read_statistics, :transfer_projects, :admin_package, :read_billing, :edit_billing, :import_projects] } + let(:owner_permissions) { [:owner_access, :create_projects, :admin_runner, :admin_namespace, :read_namespace, :read_namespace_via_membership, :read_statistics, :transfer_projects, :admin_package, :read_billing, :edit_billing, :import_projects] } subject { described_class.new(current_user, namespace) } diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 241ca8cf6fd174..fbcb4feeaad2c5 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -78,6 +78,7 @@ admin_namespace admin_group_member admin_package + admin_runner change_visibility_level set_note_created_at create_subgroup diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 984584f72a8b92..452fd74b3478f4 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -67,7 +67,7 @@ %i[ add_cluster admin_build admin_commit_status admin_container_image admin_cicd_variables admin_deployment admin_environment admin_note admin_pipeline - admin_project admin_project_member admin_push_rules admin_snippet admin_terraform_state + admin_project admin_project_member admin_push_rules admin_runner admin_snippet admin_terraform_state admin_wiki create_deploy_token destroy_deploy_token manage_deploy_tokens push_to_delete_protected_branch read_deploy_token update_snippet destroy_upload admin_member_access_request rename_project manage_merge_request_settings -- GitLab From b338ad90bc2fc483e3c96c2255c64675758d4c50 Mon Sep 17 00:00:00 2001 From: mo khan Date: Tue, 4 Jun 2024 10:35:12 -0600 Subject: [PATCH 2/3] Rename before action hook to be clearer --- app/controllers/groups/settings/ci_cd_controller.rb | 4 ++-- app/controllers/projects/settings/ci_cd_controller.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index 5a0ce0a9bc9bf5..2553074d0ffc44 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -6,7 +6,7 @@ class CiCdController < Groups::ApplicationController layout 'group_settings' skip_cross_project_access_check :show before_action :authorize_admin_group!, except: :show - before_action :authorize_admin_cicd_settings!, only: :show + before_action :authorize_show_cicd_settings!, only: :show before_action :authorize_update_max_artifacts_size!, only: [:update] before_action :define_variables, only: [:show] before_action :push_licensed_features, only: [:show] @@ -47,7 +47,7 @@ def update_auto_devops private - def authorize_admin_cicd_settings! + def authorize_show_cicd_settings! return if can_any?(current_user, [ :admin_cicd_variables, :admin_runner diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index c27c9c746f3432..8efae6f8a1d5e0 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -9,7 +9,7 @@ class CiCdController < Projects::ApplicationController layout 'project_settings' before_action :authorize_admin_pipeline!, except: :show - before_action :authorize_admin_cicd_settings!, only: :show + before_action :authorize_show_cicd_settings!, only: :show before_action :check_builds_available! before_action :define_variables @@ -75,7 +75,7 @@ def runner_setup_scripts private - def authorize_admin_cicd_settings! + def authorize_show_cicd_settings! return if can_any?(current_user, [ :admin_cicd_variables, :admin_runner -- GitLab From e204ceb14bf7fa5355daac2995b5841f7d0bea19 Mon Sep 17 00:00:00 2001 From: mo khan Date: Tue, 4 Jun 2024 17:27:45 -0600 Subject: [PATCH 3/3] Revert authz changes to Mutation.namespaceCiCdSettingsUpdate --- ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb | 2 +- ee/spec/requests/custom_roles/admin_runners/request_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb b/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb index 4d00a2c5935c2c..f8857a94beca08 100644 --- a/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb +++ b/ee/app/graphql/mutations/ci/namespace_ci_cd_settings_update.rb @@ -7,7 +7,7 @@ class NamespaceCiCdSettingsUpdate < BaseMutation include ResolvesNamespace - authorize :admin_runner + authorize :admin_namespace argument :allow_stale_runner_pruning, GraphQL::Types::Boolean, required: false, diff --git a/ee/spec/requests/custom_roles/admin_runners/request_spec.rb b/ee/spec/requests/custom_roles/admin_runners/request_spec.rb index 58a52e305f64c5..317032121447e5 100644 --- a/ee/spec/requests/custom_roles/admin_runners/request_spec.rb +++ b/ee/spec/requests/custom_roles/admin_runners/request_spec.rb @@ -137,7 +137,7 @@ let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, group: group) } - it "updates the `allow_stale_runner_pruning` setting" do + pending "updates the `allow_stale_runner_pruning` setting" do post_graphql_mutation(graphql_mutation(:namespace_ci_cd_settings_update, { full_path: group.full_path, allow_stale_runner_pruning: true -- GitLab