From 6a771c958b908555fcdb884b5e2ce9a1cd95704d Mon Sep 17 00:00:00 2001 From: Matthew MacRae-Bovell Date: Mon, 22 Sep 2025 14:53:57 -0400 Subject: [PATCH 1/3] Rename all destroy deployment permissions to delete --- app/controllers/groups/application_controller.rb | 2 +- app/graphql/types/permission_types/deployment.rb | 2 +- app/policies/group_policy.rb | 4 ++-- app/policies/project_policy.rb | 8 ++++---- app/services/ci/deployments/destroy_service.rb | 2 +- doc/api/graphql/reference/_index.md | 2 +- ee/app/policies/ee/deployment_policy.rb | 2 +- ee/app/policies/ee/group_policy.rb | 2 +- ee/app/policies/ee/project_policy.rb | 2 +- ee/spec/policies/deployment_policy_spec.rb | 6 +++--- ee/spec/policies/group_policy_spec.rb | 2 +- ee/spec/policies/project_policy_spec.rb | 2 +- lib/api/deploy_tokens.rb | 4 ++-- lib/api/deployments.rb | 2 +- spec/policies/project_policy_spec.rb | 4 ++-- .../requests/api/graphql/environments/deployments_spec.rb | 2 +- 16 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb index 2c38625fd1f767..093195e29bc095 100644 --- a/app/controllers/groups/application_controller.rb +++ b/app/controllers/groups/application_controller.rb @@ -44,7 +44,7 @@ def authorize_create_deploy_token! end def authorize_destroy_deploy_token! - render_404 unless can?(current_user, :destroy_deploy_token, group) + render_404 unless can?(current_user, :delete_deploy_token, group) end def authorize_admin_group_member! diff --git a/app/graphql/types/permission_types/deployment.rb b/app/graphql/types/permission_types/deployment.rb index fce376552b1bfd..870ca60f4a8730 100644 --- a/app/graphql/types/permission_types/deployment.rb +++ b/app/graphql/types/permission_types/deployment.rb @@ -5,7 +5,7 @@ module PermissionTypes class Deployment < BasePermissionType graphql_name 'DeploymentPermissions' - abilities :destroy_deployment + abilities :delete_deployment ability_field :update_deployment, calls_gitaly: true end end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index f42492023ed9fe..451c66016673cd 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -216,7 +216,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy prevent :admin_protected_environments prevent :manage_merge_request_settings prevent :create_deploy_token - prevent :destroy_deploy_token + prevent :delete_deploy_token prevent :register_group_runners prevent :update_runners_registration_token prevent :admin_runners @@ -433,7 +433,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :change_visibility_level enable :create_deploy_token enable :create_subgroup - enable :destroy_deploy_token + enable :delete_deploy_token enable :destroy_issue enable :edit_billing enable :manage_merge_request_settings diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index a185d4a185c623..ef5788eab949e0 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -517,7 +517,7 @@ class ProjectPolicy < BasePolicy prevent :create_deployment prevent :update_deployment prevent :admin_deployment - prevent :destroy_deployment + prevent :delete_deployment end rule { feature_flags_disabled }.policy do @@ -668,7 +668,7 @@ class ProjectPolicy < BasePolicy enable :admin_pipeline enable :admin_environment enable :admin_deployment - enable :destroy_deployment + enable :delete_deployment enable :admin_pages enable :read_pages enable :update_pages @@ -684,7 +684,7 @@ class ProjectPolicy < BasePolicy enable :admin_sentry enable :read_deploy_token enable :create_deploy_token - enable :destroy_deploy_token + enable :delete_deploy_token enable :admin_terraform_state enable :create_freeze_period enable :read_freeze_period @@ -868,7 +868,7 @@ class ProjectPolicy < BasePolicy prevent :create_deployment prevent :update_deployment prevent :admin_deployment - prevent :destroy_deployment + prevent :delete_deployment prevent :read_resource_group prevent :update_resource_group diff --git a/app/services/ci/deployments/destroy_service.rb b/app/services/ci/deployments/destroy_service.rb index ac51fa55537257..a117980f6abb10 100644 --- a/app/services/ci/deployments/destroy_service.rb +++ b/app/services/ci/deployments/destroy_service.rb @@ -4,7 +4,7 @@ module Ci module Deployments class DestroyService < BaseService def execute(deployment) - raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_deployment, deployment) + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :delete_deployment, deployment) return ServiceResponse.error(message: 'Cannot destroy running deployment') if deployment&.running? return ServiceResponse.error(message: 'Deployment currently deployed to environment') if deployment&.last? diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 2a776e60b2ab1d..0ceede562c4746 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -28484,7 +28484,7 @@ Approval summary of the deployment. | Name | Type | Description | | ---- | ---- | ----------- | | `approveDeployment` | [`Boolean!`](#boolean) | Indicates the user can perform `approve_deployment` on this resource. This field can only be resolved for one environment in any single request. | -| `destroyDeployment` | [`Boolean!`](#boolean) | If `true`, the user can perform `destroy_deployment` on this resource. | +| `deleteDeployment` | [`Boolean!`](#boolean) | If `true`, the user can perform `delete_deployment` on this resource. | | `updateDeployment` | [`Boolean!`](#boolean) | If `true`, the user can perform `update_deployment` on this resource. | ### `DeploymentTag` diff --git a/ee/app/policies/ee/deployment_policy.rb b/ee/app/policies/ee/deployment_policy.rb index f6e9f2cda1ccdb..51686ab27b2db4 100644 --- a/ee/app/policies/ee/deployment_policy.rb +++ b/ee/app/policies/ee/deployment_policy.rb @@ -22,7 +22,7 @@ module DeploymentPolicy end rule { protected_environment }.policy do - prevent :destroy_deployment + prevent :delete_deployment end rule { needs_approval & ~has_approval_rules & can?(:update_deployment) }.policy do diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index ec06029083d6c6..c93c2d544ff5da 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -811,7 +811,7 @@ module GroupPolicy enable :manage_deploy_tokens enable :read_deploy_token enable :create_deploy_token - enable :destroy_deploy_token + enable :delete_deploy_token end rule { can?(:read_vulnerability) }.policy do diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index eb8c148bc9b0f9..36cc0becab3786 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -1060,7 +1060,7 @@ module ProjectPolicy enable :manage_deploy_tokens enable :read_deploy_token enable :create_deploy_token - enable :destroy_deploy_token + enable :delete_deploy_token end rule { custom_role_enables_admin_protected_branch }.policy do diff --git a/ee/spec/policies/deployment_policy_spec.rb b/ee/spec/policies/deployment_policy_spec.rb index e12a2bc2317e0d..7cd6ba2f059c87 100644 --- a/ee/spec/policies/deployment_policy_spec.rb +++ b/ee/spec/policies/deployment_policy_spec.rb @@ -27,12 +27,12 @@ create(:protected_environment, name: environment.name, project: project, authorize_user_to_deploy: user) end - it { expect_allowed(:destroy_deployment) } + it { expect_allowed(:delete_deployment) } context 'when user is developer' do let(:user) { developer } - it { expect_disallowed(:destroy_deployment) } + it { expect_disallowed(:delete_deployment) } end end @@ -88,6 +88,6 @@ create(:protected_environment, name: environment.name, project: project, authorize_user_to_deploy: create(:user)) end - it { expect_disallowed(:destroy_deployment) } + it { expect_disallowed(:delete_deployment) } end end diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 6d6d93a0041c0c..9c7b8a4cdeb904 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -4398,7 +4398,7 @@ def create_member_role(member, abilities = member_role_abilities) let(:member_role_abilities) { { manage_deploy_tokens: true } } let(:allowed_abilities) do - [:manage_deploy_tokens, :read_deploy_token, :create_deploy_token, :destroy_deploy_token, :view_edit_page] + [:manage_deploy_tokens, :read_deploy_token, :create_deploy_token, :delete_deploy_token, :view_edit_page] end it_behaves_like 'custom roles abilities' diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 17f1c46555d494..ab13ecdab7118f 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3120,7 +3120,7 @@ def create_member_role(member, abilities = member_role_abilities) context 'for a member role with the `manage_deploy_tokens` ability' do let(:member_role_abilities) { { manage_deploy_tokens: true } } - let(:allowed_abilities) { [:manage_deploy_tokens, :read_deploy_token, :create_deploy_token, :destroy_deploy_token] } + let(:allowed_abilities) { [:manage_deploy_tokens, :read_deploy_token, :create_deploy_token, :delete_deploy_token] } it_behaves_like 'custom roles abilities' diff --git a/lib/api/deploy_tokens.rb b/lib/api/deploy_tokens.rb index 5440f960ee6958..476fab59c5ba0c 100644 --- a/lib/api/deploy_tokens.rb +++ b/lib/api/deploy_tokens.rb @@ -152,7 +152,7 @@ def scope_params requires :token_id, type: Integer, desc: 'The ID of the deploy token' end delete ':id/deploy_tokens/:token_id' do - authorize!(:destroy_deploy_token, user_project) + authorize!(:delete_deploy_token, user_project) ::Projects::DeployTokens::DestroyService.new( user_project, current_user, token_id: params[:token_id] @@ -259,7 +259,7 @@ def scope_params requires :token_id, type: Integer, desc: 'The ID of the deploy token' end delete ':id/deploy_tokens/:token_id' do - authorize!(:destroy_deploy_token, user_group) + authorize!(:delete_deploy_token, user_group) ::Groups::DeployTokens::DestroyService.new( user_group, current_user, token_id: params[:token_id] diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb index 17bcc029283407..1fb766dc24de37 100644 --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -222,7 +222,7 @@ class Deployments < ::API::Base delete ':id/deployments/:deployment_id' do deployment = user_project.deployments.find(params[:deployment_id]) - authorize!(:destroy_deployment, deployment) + authorize!(:delete_deployment, deployment) destroy_conditionally!(deployment) do result = ::Ci::Deployments::DestroyService.new(user_project, current_user).execute(deployment) diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index d3b3b852dcf365..7c653fae9b741b 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -579,7 +579,7 @@ def set_access_level(access_level) :create_build, :read_build, :update_build, :cancel_build, :admin_build, :destroy_build, :create_pipeline_schedule, :read_pipeline_schedule_variables, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule, :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment, - :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment, + :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :delete_deployment, :read_resource_group, :update_resource_group ] @@ -613,7 +613,7 @@ def set_access_level(access_level) :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule, :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment, :create_cluster, :read_cluster, :update_cluster, :admin_cluster, - :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment, + :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :delete_deployment, :download_code, :build_download_code, :read_code, :read_resource_group, :update_resource_group ] diff --git a/spec/requests/api/graphql/environments/deployments_spec.rb b/spec/requests/api/graphql/environments/deployments_spec.rb index 894bd8008821b5..67826c58aea530 100644 --- a/spec/requests/api/graphql/environments/deployments_spec.rb +++ b/spec/requests/api/graphql/environments/deployments_spec.rb @@ -505,7 +505,7 @@ def set_deployment_attributes(deployment, factory_type) expect(deployment['userPermissions']['updateDeployment']) .to eq(Ability.allowed?(user, :update_deployment, deployment_in_record)) expect(deployment['userPermissions']['destroyDeployment']) - .to eq(Ability.allowed?(user, :destroy_deployment, deployment_in_record)) + .to eq(Ability.allowed?(user, :delete_deployment, deployment_in_record)) end end end -- GitLab From 4dcd4679de5e818ae003f3cb266cfc50c8fd7d41 Mon Sep 17 00:00:00 2001 From: Matthew MacRae-Bovell Date: Tue, 23 Sep 2025 13:29:08 -0400 Subject: [PATCH 2/3] Update broken spec --- config/authz/permissions/definitions_todo.txt | 2 -- spec/policies/group_policy_spec.rb | 2 +- .../shared_contexts/policies/project_policy_shared_context.rb | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/config/authz/permissions/definitions_todo.txt b/config/authz/permissions/definitions_todo.txt index 5118759acc3d76..2a53ddf4d75870 100644 --- a/config/authz/permissions/definitions_todo.txt +++ b/config/authz/permissions/definitions_todo.txt @@ -296,8 +296,6 @@ destroy_commit_status destroy_container_image destroy_container_image_tag destroy_container_registry_protection_tag_rule -destroy_deploy_token -destroy_deployment destroy_design destroy_duo_workflow destroy_environment diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index bbc8f6d15c0981..965f9c43e08a40 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -653,7 +653,7 @@ let(:destroy_abilities) do %i[ - destroy_deploy_token + delete_deploy_token destroy_epic delete_custom_emoji delete_o11y_settings 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 ec36bf8f8453ce..fe3c4ecdf8ec94 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -80,7 +80,7 @@ add_cluster admin_build admin_container_image admin_cicd_variables admin_deployment admin_environment admin_note admin_pipeline admin_project admin_project_member admin_push_rules admin_runners admin_snippet admin_terraform_state - admin_wiki create_deploy_token destroy_deploy_token manage_deploy_tokens + admin_wiki create_deploy_token delete_deploy_token manage_deploy_tokens push_to_delete_protected_branch read_deploy_token update_snippet admin_upload destroy_upload admin_member_access_request read_member_access_request rename_project manage_merge_request_settings admin_integrations create_protected_branch admin_protected_branch -- GitLab From a6a9ab422281c078397f661e68166391eb4f1594 Mon Sep 17 00:00:00 2001 From: Matthew MacRae-Bovell Date: Tue, 23 Sep 2025 14:08:14 -0400 Subject: [PATCH 3/3] Update missing broken specs --- spec/graphql/types/permission_types/deployment_spec.rb | 2 +- spec/requests/api/graphql/environments/deployments_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/graphql/types/permission_types/deployment_spec.rb b/spec/graphql/types/permission_types/deployment_spec.rb index ccf5798984c752..b1211f9bd929cc 100644 --- a/spec/graphql/types/permission_types/deployment_spec.rb +++ b/spec/graphql/types/permission_types/deployment_spec.rb @@ -4,7 +4,7 @@ RSpec.describe Types::PermissionTypes::Deployment, feature_category: :continuous_delivery do it do - expected_permissions = %i[update_deployment destroy_deployment] + expected_permissions = %i[update_deployment delete_deployment] expect(described_class).to include_graphql_fields(*expected_permissions) end diff --git a/spec/requests/api/graphql/environments/deployments_spec.rb b/spec/requests/api/graphql/environments/deployments_spec.rb index 67826c58aea530..18c53f7e146201 100644 --- a/spec/requests/api/graphql/environments/deployments_spec.rb +++ b/spec/requests/api/graphql/environments/deployments_spec.rb @@ -484,7 +484,7 @@ def set_deployment_attributes(deployment, factory_type) iid userPermissions { updateDeployment - destroyDeployment + deleteDeployment } } } @@ -504,7 +504,7 @@ def set_deployment_attributes(deployment, factory_type) expect(deployment['userPermissions']['updateDeployment']) .to eq(Ability.allowed?(user, :update_deployment, deployment_in_record)) - expect(deployment['userPermissions']['destroyDeployment']) + expect(deployment['userPermissions']['deleteDeployment']) .to eq(Ability.allowed?(user, :delete_deployment, deployment_in_record)) end end -- GitLab