From 11b8890b5e1a5124a68e878f7560d10cb161859d Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Mon, 3 Feb 2025 18:32:02 +0100
Subject: [PATCH 1/8] Protected packages: Add minimum_access_level_for_delete
 to REST API

The attribute `minimum_access_level_for_delete` that was introduced
in the MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179739 .

This commit makes the attribute `minimum_access_level_for_delete`
available in the REST API also updates the documentation.

Changelog: added
---
 doc/api/project_packages_protection_rules.md  |  9 +-
 .../projects/packages/protection/rule.rb      |  1 +
 lib/api/project_packages_protection_rules.rb  | 25 +++--
 .../projects/packages/protection/rule_spec.rb |  1 +
 .../project_packages_protection_rules_spec.rb | 96 ++++++++++++++-----
 .../api/protection_rules_shared_examples.rb   |  8 +-
 6 files changed, 104 insertions(+), 36 deletions(-)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index dbd60366d2379f..19096f54f6fc1b 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -62,6 +62,7 @@ Example response:
   "project_id": 7,
   "package_name_pattern": "@flightjs/flight-package-0",
   "package_type": "npm",
+  "minimum_access_level_for_delete": "owner",
   "minimum_access_level_for_push": "maintainer"
  },
  {
@@ -69,6 +70,7 @@ Example response:
   "project_id": 7,
   "package_name_pattern": "@flightjs/flight-package-1",
   "package_type": "npm",
+  "minimum_access_level_for_delete": "owner",
   "minimum_access_level_for_push": "maintainer"
  }
 ]
@@ -89,7 +91,8 @@ Supported attributes:
 | `id`                                  | integer/string  | Yes      | ID or [URL-encoded path of the project](rest/_index.md#namespaced-paths). |
 | `package_name_pattern`                | string          | Yes      | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | Yes      | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Must be at least `maintainer`. For example `maintainer`, `owner` or `admin`. |
+| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Available only when feature flag `packages_protected_packages_delete` is enabled. |
+| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Can be empty or must be `owner` or `admin`. |
 
 If successful, returns [`201`](rest/troubleshooting.md#status-codes) and the created package protection rule.
 
@@ -112,6 +115,7 @@ curl --request POST \
   --data '{
        "package_name_pattern": "package-name-pattern-*",
        "package_type": "npm",
+       "minimum_access_level_for_delete": "owner",
        "minimum_access_level_for_push": "maintainer"
     }'
 ```
@@ -132,7 +136,8 @@ Supported attributes:
 | `package_protection_rule_id`          | integer         | Yes      | ID of the package protection rule to be updated. |
 | `package_name_pattern`                | string          | No       | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | No       | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level able to push a package. Must be at least `maintainer`. For example `maintainer`, `owner` or `admin`. |
+| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Available only when feature flag `packages_protected_packages_delete` is enabled. |
+| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level able to push a package. Can be empty or must be `owner` or `admin`. |
 
 If successful, returns [`200`](rest/troubleshooting.md#status-codes) and the updated package protection rule.
 
diff --git a/lib/api/entities/projects/packages/protection/rule.rb b/lib/api/entities/projects/packages/protection/rule.rb
index 3cc5f9a6bd7b1d..a05f23fe794219 100644
--- a/lib/api/entities/projects/packages/protection/rule.rb
+++ b/lib/api/entities/projects/packages/protection/rule.rb
@@ -10,6 +10,7 @@ class Rule < Grape::Entity
             expose :project_id, documentation: { type: 'integer', example: 1 }
             expose :package_name_pattern, documentation: { type: 'string', example: 'flightjs/flight' }
             expose :package_type, documentation: { type: 'string', example: 'npm' }
+            expose :minimum_access_level_for_delete, documentation: { type: 'string', example: 'owner' }
             expose :minimum_access_level_for_push, documentation: { type: 'string', example: 'maintainer' }
           end
         end
diff --git a/lib/api/project_packages_protection_rules.rb b/lib/api/project_packages_protection_rules.rb
index 5faa9d09ccb4b3..914d79cde2d8a9 100644
--- a/lib/api/project_packages_protection_rules.rb
+++ b/lib/api/project_packages_protection_rules.rb
@@ -46,14 +46,21 @@ class ProjectPackagesProtectionRules < ::API::Base
             Wildcard character * allowed.'
           requires :package_type, type: String, values: Packages::Protection::Rule.package_types.keys,
             desc: 'Package type protected by the rule. For example npm.'
-          requires :minimum_access_level_for_push, type: String,
+          optional :minimum_access_level_for_delete, type: String,
+            values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
+            desc: 'Minimum GitLab access level able to delete a package. Must be `owner` or `admin`. ' \
+              'To unset the value, use an empty string `""`.'
+          optional :minimum_access_level_for_push, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
-            desc: 'Minimum GitLab access level able to push a package. Must be at least `maintainer`.
-            For example `maintainer`, `owner` or `admin`.'
+            desc: 'Minimum GitLab access level able to push a package. Must be `owner` or `admin`. ' \
+              'To unset the value, use an empty string `""`.'
+          at_least_one_of :minimum_access_level_for_push, :minimum_access_level_for_delete
         end
         post do
-          response = ::Packages::Protection::CreateRuleService.new(project: user_project, current_user: current_user,
-            params: declared_params).execute
+          response =
+            ::Packages::Protection::CreateRuleService
+              .new(project: user_project, current_user: current_user, params: declared_params)
+              .execute
 
           render_api_error!({ error: response.message }, :unprocessable_entity) if response.error?
 
@@ -81,10 +88,14 @@ class ProjectPackagesProtectionRules < ::API::Base
               Wildcard character * allowed.'
             optional :package_type, type: String, values: Packages::Protection::Rule.package_types.keys,
               desc: 'Package type protected by the rule. For example npm.'
+            optional :minimum_access_level_for_delete, type: String,
+              values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
+              desc: 'Minimum GitLab access level able to delete a package. Must be `owner` or `admin`. ' \
+                'To unset the value, use an empty string `""`.'
             optional :minimum_access_level_for_push, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
-              desc: 'Minimum GitLab access level able to push a package. Must be at least `maintainer`.
-              For example `maintainer`, `owner` or `admin`.'
+              desc: 'Minimum GitLab access level able to push a package. Must be `owner` or `admin`. ' \
+                'To unset the value, use an empty string `""`.'
           end
           patch do
             package_protection_rule = user_project.package_protection_rules.find(params[:package_protection_rule_id])
diff --git a/spec/lib/api/entities/projects/packages/protection/rule_spec.rb b/spec/lib/api/entities/projects/packages/protection/rule_spec.rb
index 14d4a4cc7b95b6..c895d45b3ed473 100644
--- a/spec/lib/api/entities/projects/packages/protection/rule_spec.rb
+++ b/spec/lib/api/entities/projects/packages/protection/rule_spec.rb
@@ -13,6 +13,7 @@
       :project_id,
       :package_name_pattern,
       :package_type,
+      :minimum_access_level_for_delete,
       :minimum_access_level_for_push
     ]
   end
diff --git a/spec/requests/api/project_packages_protection_rules_spec.rb b/spec/requests/api/project_packages_protection_rules_spec.rb
index 0742165580ace6..b04febf0218251 100644
--- a/spec/requests/api/project_packages_protection_rules_spec.rb
+++ b/spec/requests/api/project_packages_protection_rules_spec.rb
@@ -18,9 +18,12 @@
   let(:url) { "/projects/#{project.id}/#{path}" }
 
   let(:params) do
-    { package_name_pattern: '@my-new-scope/my-package-*',
+    {
+      package_name_pattern: '@my-new-scope/my-package-*',
       package_type: protection_rule.package_type,
-      minimum_access_level_for_push: protection_rule.minimum_access_level_for_push }
+      minimum_access_level_for_delete: protection_rule.minimum_access_level_for_delete,
+      minimum_access_level_for_push: protection_rule.minimum_access_level_for_push
+    }
   end
 
   shared_examples 'rejecting project packages protection rules request when enough permissions' do
@@ -46,6 +49,14 @@
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(json_response.count).to eq(2)
+        expect(json_response).to include({
+          'id' => protection_rule.id,
+          'project_id' => protection_rule.project_id,
+          'package_name_pattern' => protection_rule.package_name_pattern,
+          'package_type' => protection_rule.package_type,
+          'minimum_access_level_for_delete' => protection_rule.minimum_access_level_for_delete,
+          'minimum_access_level_for_push' => protection_rule.minimum_access_level_for_push
+        })
       end
 
       it_behaves_like 'rejecting project packages protection rules request when enough permissions'
@@ -71,12 +82,37 @@
       it 'creates a package protection rule' do
         expect { post_package_rule }.to change { Packages::Protection::Rule.count }.by(1)
         expect(response).to have_gitlab_http_status(:created)
+
+        expect(json_response).to include(
+          'id' => Integer,
+          'project_id' => project.id,
+          'package_name_pattern' => params[:package_name_pattern],
+          'package_type' => params[:package_type],
+          'minimum_access_level_for_delete' => params[:minimum_access_level_for_delete],
+          'minimum_access_level_for_push' => params[:minimum_access_level_for_push]
+        )
       end
 
-      context 'with invalid package_type' do
-        before do
-          params[:package_type] = "not in enum"
+      context 'without minimum_access_level_for_delete' do
+        let(:params) { super().except(:minimum_access_level_for_delete) }
+
+        it 'creates a package protection rule' do
+          expect { post_package_rule }.to change { Packages::Protection::Rule.count }.by(1)
+          expect(response).to have_gitlab_http_status(:created)
+
+          expect(json_response).to include(
+            'id' => Integer,
+            'project_id' => project.id,
+            'package_name_pattern' => params[:package_name_pattern],
+            'package_type' => params[:package_type],
+            'minimum_access_level_for_delete' => nil,
+            'minimum_access_level_for_push' => params[:minimum_access_level_for_push]
+          )
         end
+      end
+
+      context 'with invalid package_type' do
+        let(:params) { super().merge(package_type: 'not in enum') }
 
         it 'does not create a package protection rule' do
           expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
@@ -85,9 +121,16 @@
       end
 
       context 'with invalid minimum_access_level_for_push' do
-        before do
-          params[:minimum_access_level_for_push] = "not in enum"
+        let(:params) { super().merge(minimum_access_level_for_push: 'not in enum') }
+
+        it 'does not create a package protection rule' do
+          expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
+          expect(response).to have_gitlab_http_status(:bad_request)
         end
+      end
+
+      context 'without minimum_access_levels' do
+        let(:params) { super().except(:minimum_access_level_for_push, :minimum_access_level_for_delete) }
 
         it 'does not create a package protection rule' do
           expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
@@ -95,10 +138,17 @@
         end
       end
 
-      context 'with already existing package_name_pattern' do
-        before do
-          params[:package_name_pattern] = protection_rule.package_name_pattern
+      context 'with invalid minimum_access_levels' do
+        let(:params) { super().merge(minimum_access_level_for_push: nil, minimum_access_level_for_delete: nil) }
+
+        it 'does not create a package protection rule' do
+          expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
+          expect(response).to have_gitlab_http_status(:unprocessable_entity)
         end
+      end
+
+      context 'with already existing package_name_pattern' do
+        let(:params) { super().merge(package_name_pattern: protection_rule.package_name_pattern) }
 
         it 'does not create a package protection rule' do
           expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
@@ -128,9 +178,7 @@
       let_it_be(:changed_scope) { '@my-changed-scope/my-package-*' }
 
       context 'with full changeset' do
-        before do
-          params[:package_name_pattern] = changed_scope
-        end
+        let(:params) { super().merge(package_name_pattern: changed_scope) }
 
         it 'updates a package protection rule' do
           patch_package_rule
@@ -153,28 +201,30 @@
       end
 
       context 'with invalid package_type' do
-        before do
-          params[:package_type] = "not in enum"
-        end
+        let(:params) { super().merge(package_type: 'not in enum') }
 
         it_behaves_like 'returning response status', :bad_request
       end
 
       context 'with invalid minimum_access_level_for_push' do
-        before do
-          params[:minimum_access_level_for_push] = "not in enum"
-        end
+        let(:params) { super().merge(minimum_access_level_for_push: 'not in enum') }
 
         it_behaves_like 'returning response status', :bad_request
       end
 
+      context 'with invalid minimum_access_levels' do
+        let(:params) { super().merge(minimum_access_level_for_push: nil, minimum_access_level_for_delete: nil) }
+
+        it_behaves_like 'returning response status', :unprocessable_entity
+      end
+
       context 'with already existing package_name_pattern' do
-        before do
-          other_package_protection_rule = create(:package_protection_rule, project: project,
-            package_name_pattern: "@my-scope/my-package-*")
-          params[:package_name_pattern] = other_package_protection_rule.package_name_pattern
+        let_it_be(:existing_package_protection_rule) do
+          create(:package_protection_rule, project: project, package_name_pattern: '@my-scope/my-package-*')
         end
 
+        let(:params) { super().merge(package_name_pattern: existing_package_protection_rule.package_name_pattern) }
+
         it_behaves_like 'returning response status', :unprocessable_entity
       end
 
diff --git a/spec/support/shared_examples/requests/api/protection_rules_shared_examples.rb b/spec/support/shared_examples/requests/api/protection_rules_shared_examples.rb
index 018299c538bd78..cea4293b395bbe 100644
--- a/spec/support/shared_examples/requests/api/protection_rules_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/protection_rules_shared_examples.rb
@@ -28,8 +28,8 @@
   let(:url) { "/projects/#{project_id}/#{path}" }
 
   where(:project_id, :protection_rule_id, :status) do
-    ref(:valid_project_id) | 'invalid'                | :bad_request
-    ref(:valid_project_id) | non_existing_record_id   | :not_found
+    ref(:valid_project_id) | 'invalid'                      | :bad_request
+    ref(:valid_project_id) | non_existing_record_id         | :not_found
     ref(:other_project_id) | ref(:valid_protection_rule_id) | :not_found
   end
 
@@ -44,8 +44,8 @@
   let(:url) { "/projects/#{project_id}/#{path}" }
 
   where(:project_id, :status) do
-    'invalid'               | :not_found
-    non_existing_record_id  | :not_found
+    'invalid'              | :not_found
+    non_existing_record_id | :not_found
   end
 
   with_them do
-- 
GitLab


From b919cf99f5972521f92cb7d9bed1b25a78422242 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Wed, 12 Mar 2025 09:11:07 +0100
Subject: [PATCH 2/8] docs: Apply suggestions from @z_painter

- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2370869091
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2370869103
---
 doc/api/project_packages_protection_rules.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index 19096f54f6fc1b..4f1c63effded4a 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -91,7 +91,7 @@ Supported attributes:
 | `id`                                  | integer/string  | Yes      | ID or [URL-encoded path of the project](rest/_index.md#namespaced-paths). |
 | `package_name_pattern`                | string          | Yes      | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | Yes      | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Available only when feature flag `packages_protected_packages_delete` is enabled. |
+| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
 | `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Can be empty or must be `owner` or `admin`. |
 
 If successful, returns [`201`](rest/troubleshooting.md#status-codes) and the created package protection rule.
@@ -136,7 +136,7 @@ Supported attributes:
 | `package_protection_rule_id`          | integer         | Yes      | ID of the package protection rule to be updated. |
 | `package_name_pattern`                | string          | No       | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | No       | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Available only when feature flag `packages_protected_packages_delete` is enabled. |
+| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
 | `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level able to push a package. Can be empty or must be `owner` or `admin`. |
 
 If successful, returns [`200`](rest/troubleshooting.md#status-codes) and the updated package protection rule.
-- 
GitLab


From 24b4d479e4ef668ebc7a16d2b8ded4386d2bbfa7 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Fri, 14 Mar 2025 10:50:47 +0100
Subject: [PATCH 3/8] refactor: Apply suggestions from @lwanko

- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2372776773
---
 doc/api/project_packages_protection_rules.md  |  4 +--
 lib/api/project_packages_protection_rules.rb  | 24 ++++++++++++------
 .../project_packages_protection_rules_spec.rb | 25 ++++++++++++++++---
 3 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index 4f1c63effded4a..92195819116dcd 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -91,8 +91,8 @@ Supported attributes:
 | `id`                                  | integer/string  | Yes      | ID or [URL-encoded path of the project](rest/_index.md#namespaced-paths). |
 | `package_name_pattern`                | string          | Yes      | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | Yes      | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
-| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Can be empty or must be `owner` or `admin`. |
+| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. |
+| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`201`](rest/troubleshooting.md#status-codes) and the created package protection rule.
 
diff --git a/lib/api/project_packages_protection_rules.rb b/lib/api/project_packages_protection_rules.rb
index 914d79cde2d8a9..f651a97ebba05b 100644
--- a/lib/api/project_packages_protection_rules.rb
+++ b/lib/api/project_packages_protection_rules.rb
@@ -48,12 +48,16 @@ class ProjectPackagesProtectionRules < ::API::Base
             desc: 'Package type protected by the rule. For example npm.'
           optional :minimum_access_level_for_delete, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
-            desc: 'Minimum GitLab access level able to delete a package. Must be `owner` or `admin`. ' \
-              'To unset the value, use an empty string `""`.'
+            desc: 'Minimum GitLab access level able to delete a package. ' \
+              'Valid values include `null`, `owner` or `admin`. ' \
+              'If the value is `nil`, the default minimum access level is `maintainer`. ' \
+              'Must be provided when `minimum_access_level_for_push` is not set.'
           optional :minimum_access_level_for_push, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
-            desc: 'Minimum GitLab access level able to push a package. Must be `owner` or `admin`. ' \
-              'To unset the value, use an empty string `""`.'
+            desc: 'Minimum GitLab access level able to push a package. ' \
+              'Valid values include `null`, `maintainer`, `owner` or `admin`. ' \
+              'If the value is `nil`, the default minimum access level is `developer`. ' \
+              'Must be provided when `minimum_access_level_for_delete` is not set.'
           at_least_one_of :minimum_access_level_for_push, :minimum_access_level_for_delete
         end
         post do
@@ -90,12 +94,16 @@ class ProjectPackagesProtectionRules < ::API::Base
               desc: 'Package type protected by the rule. For example npm.'
             optional :minimum_access_level_for_delete, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
-              desc: 'Minimum GitLab access level able to delete a package. Must be `owner` or `admin`. ' \
-                'To unset the value, use an empty string `""`.'
+              desc: 'Minimum GitLab access level able to delete a package. ' \
+                'Valid values include `null`, `owner` or `admin`. ' \
+                'If the value is `nil`, the default minimum access level is `maintainer`. ' \
+                'Must be provided when `minimum_access_level_for_push` is not set.'
             optional :minimum_access_level_for_push, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
-              desc: 'Minimum GitLab access level able to push a package. Must be `owner` or `admin`. ' \
-                'To unset the value, use an empty string `""`.'
+              desc: 'Minimum GitLab access level able to push a package. ' \
+                'Valid values include `null`, `maintainer`, `owner` or `admin`. ' \
+                'If the value is `nil`, the default minimum access level is `developer`. ' \
+                'Must be provided when `minimum_access_level_for_delete` is not set.'
           end
           patch do
             package_protection_rule = user_project.package_protection_rules.find(params[:package_protection_rule_id])
diff --git a/spec/requests/api/project_packages_protection_rules_spec.rb b/spec/requests/api/project_packages_protection_rules_spec.rb
index b04febf0218251..8c634fea7f8f10 100644
--- a/spec/requests/api/project_packages_protection_rules_spec.rb
+++ b/spec/requests/api/project_packages_protection_rules_spec.rb
@@ -111,12 +111,23 @@
         end
       end
 
+      context 'with blank minimum_access_level_for_delete' do
+        let(:params) { super().merge(minimum_access_level_for_delete: '') }
+
+        it 'does not create a package protection rule' do
+          expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response).to include 'error' => 'minimum_access_level_for_delete does not have a valid value'
+        end
+      end
+
       context 'with invalid package_type' do
         let(:params) { super().merge(package_type: 'not in enum') }
 
         it 'does not create a package protection rule' do
           expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
           expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response).to include 'error' => 'package_type does not have a valid value'
         end
       end
 
@@ -212,6 +223,16 @@
         it_behaves_like 'returning response status', :bad_request
       end
 
+      context 'with blank minimum_access_level_for_delete' do
+        let(:params) { super().merge(minimum_access_level_for_delete: '') }
+
+        it_behaves_like 'returning response status', :bad_request
+        it 'returns error message' do
+          expect { patch_package_rule }.to not_change(Packages::Protection::Rule, :count)
+          expect(json_response).to include 'error' => 'minimum_access_level_for_delete does not have a valid value'
+        end
+      end
+
       context 'with invalid minimum_access_levels' do
         let(:params) { super().merge(minimum_access_level_for_push: nil, minimum_access_level_for_delete: nil) }
 
@@ -251,9 +272,7 @@
 
       it 'deletes the package protection rule' do
         destroy_package_rule
-        expect do
-          Packages::Protection::Rule.find(protection_rule.id)
-        end.to raise_error(ActiveRecord::RecordNotFound)
+        expect { Packages::Protection::Rule.find(protection_rule.id) }.to raise_error(ActiveRecord::RecordNotFound)
         expect(response).to have_gitlab_http_status(:no_content)
       end
 
-- 
GitLab


From c9a5820ac9d3d6760b68ed30ae7b04972f2a5dd7 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Fri, 14 Mar 2025 11:47:28 +0100
Subject: [PATCH 4/8] refactor: Apply best practices for feature flag in REST
 API

- The Gitlab API Style Guide suggests to not expose or ignore new fields
  when the feature flag is disabled,
  see https://docs.gitlab.com/development/api_styleguide/#experimental-beta-and-generally-available-features
- We apply this best practice to
  the `protected_packages_delete` feature flag
  in the `ProjectPackagesProtectionRules` API
- Aligning documentation for
  REST API field :minimum_access_level_for_delete
  with documentation for GraphQL field :minimumAccessLevelForDelete.
---
 doc/api/project_packages_protection_rules.md  |  6 +-
 .../projects/packages/protection/rule.rb      |  6 ++
 lib/api/project_packages_protection_rules.rb  | 22 ++++--
 .../project_packages_protection_rules_spec.rb | 73 ++++++++++++++++++-
 4 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index 92195819116dcd..a78812dbbfba8c 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -91,7 +91,7 @@ Supported attributes:
 | `id`                                  | integer/string  | Yes      | ID or [URL-encoded path of the project](rest/_index.md#namespaced-paths). |
 | `package_name_pattern`                | string          | Yes      | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | Yes      | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. |
+| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
 | `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`201`](rest/troubleshooting.md#status-codes) and the created package protection rule.
@@ -136,8 +136,8 @@ Supported attributes:
 | `package_protection_rule_id`          | integer         | Yes      | ID of the package protection rule to be updated. |
 | `package_name_pattern`                | string          | No       | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | No       | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level able to delete a package. Can be empty or must be `owner` or `admin`. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
-| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level able to push a package. Can be empty or must be `owner` or `admin`. |
+| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level able to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
+| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level able to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`200`](rest/troubleshooting.md#status-codes) and the updated package protection rule.
 
diff --git a/lib/api/entities/projects/packages/protection/rule.rb b/lib/api/entities/projects/packages/protection/rule.rb
index a05f23fe794219..8cc4453cc5ea6e 100644
--- a/lib/api/entities/projects/packages/protection/rule.rb
+++ b/lib/api/entities/projects/packages/protection/rule.rb
@@ -12,6 +12,12 @@ class Rule < Grape::Entity
             expose :package_type, documentation: { type: 'string', example: 'npm' }
             expose :minimum_access_level_for_delete, documentation: { type: 'string', example: 'owner' }
             expose :minimum_access_level_for_push, documentation: { type: 'string', example: 'maintainer' }
+
+            def minimum_access_level_for_delete
+              return if ::Feature.disabled?(:packages_protected_packages_delete, object.project)
+
+              object.minimum_access_level_for_delete
+            end
           end
         end
       end
diff --git a/lib/api/project_packages_protection_rules.rb b/lib/api/project_packages_protection_rules.rb
index f651a97ebba05b..c41b231b5fc65f 100644
--- a/lib/api/project_packages_protection_rules.rb
+++ b/lib/api/project_packages_protection_rules.rb
@@ -51,7 +51,8 @@ class ProjectPackagesProtectionRules < ::API::Base
             desc: 'Minimum GitLab access level able to delete a package. ' \
               'Valid values include `null`, `owner` or `admin`. ' \
               'If the value is `nil`, the default minimum access level is `maintainer`. ' \
-              'Must be provided when `minimum_access_level_for_push` is not set.'
+              'Must be provided when `minimum_access_level_for_push` is not set. ' \
+              'Behind a feature flag named `packages_protected_packages_delete` (disabled by default).'
           optional :minimum_access_level_for_push, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
             desc: 'Minimum GitLab access level able to push a package. ' \
@@ -61,9 +62,13 @@ class ProjectPackagesProtectionRules < ::API::Base
           at_least_one_of :minimum_access_level_for_push, :minimum_access_level_for_delete
         end
         post do
+          params = declared_params
+          params.except!(:minimum_access_level_for_delete) if Feature.disabled?(:packages_protected_packages_delete,
+            user_project)
+
           response =
             ::Packages::Protection::CreateRuleService
-              .new(project: user_project, current_user: current_user, params: declared_params)
+              .new(project: user_project, current_user: current_user, params: params)
               .execute
 
           render_api_error!({ error: response.message }, :unprocessable_entity) if response.error?
@@ -97,7 +102,8 @@ class ProjectPackagesProtectionRules < ::API::Base
               desc: 'Minimum GitLab access level able to delete a package. ' \
                 'Valid values include `null`, `owner` or `admin`. ' \
                 'If the value is `nil`, the default minimum access level is `maintainer`. ' \
-                'Must be provided when `minimum_access_level_for_push` is not set.'
+                'Must be provided when `minimum_access_level_for_push` is not set. ' \
+                'Behind a feature flag named `packages_protected_packages_delete` (disabled by default).'
             optional :minimum_access_level_for_push, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
               desc: 'Minimum GitLab access level able to push a package. ' \
@@ -108,8 +114,14 @@ class ProjectPackagesProtectionRules < ::API::Base
           patch do
             package_protection_rule = user_project.package_protection_rules.find(params[:package_protection_rule_id])
 
-            response = ::Packages::Protection::UpdateRuleService.new(package_protection_rule,
-              current_user: current_user, params: declared_params(include_missing: false)).execute
+            params = declared_params(include_missing: false)
+            params.except!(:minimum_access_level_for_delete) if Feature.disabled?(:packages_protected_packages_delete,
+              user_project)
+
+            response =
+              ::Packages::Protection::UpdateRuleService
+                .new(package_protection_rule, current_user: current_user, params: params)
+                .execute
 
             render_api_error!({ error: response.message }, :unprocessable_entity) if response.error?
 
diff --git a/spec/requests/api/project_packages_protection_rules_spec.rb b/spec/requests/api/project_packages_protection_rules_spec.rb
index 8c634fea7f8f10..52611792e84813 100644
--- a/spec/requests/api/project_packages_protection_rules_spec.rb
+++ b/spec/requests/api/project_packages_protection_rules_spec.rb
@@ -5,7 +5,7 @@
 RSpec.describe API::ProjectPackagesProtectionRules, :aggregate_failures, feature_category: :package_registry do
   let_it_be(:project) { create(:project, :private) }
   let_it_be(:other_project) { create(:project, :private) }
-  let_it_be(:protection_rule) { create(:package_protection_rule, project: project) }
+  let_it_be_with_reload(:protection_rule) { create(:package_protection_rule, project: project) }
   let_it_be(:protection_rule_id) { protection_rule.id }
 
   let_it_be(:maintainer) { create(:user, maintainer_of: [project, other_project]) }
@@ -59,6 +59,19 @@
         })
       end
 
+      context 'when feature flag :packages_protected_packages_delete is disabled' do
+        before do
+          stub_feature_flags(packages_protected_packages_delete: false)
+        end
+
+        it 'gets the package protection rules without minimum_access_level_for_delete' do
+          get_package_rules
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response).to all include('minimum_access_level_for_delete' => nil)
+        end
+      end
+
       it_behaves_like 'rejecting project packages protection rules request when enough permissions'
     end
 
@@ -93,6 +106,28 @@
         )
       end
 
+      context 'when feature flag :packages_protected_packages_delete is disabled' do
+        before do
+          stub_feature_flags(packages_protected_packages_delete: false)
+        end
+
+        it 'creates a package protection rule with blank minimum_access_level_for_delete' do
+          expect { post_package_rule }.to change { Packages::Protection::Rule.count }.by(1)
+          expect(response).to have_gitlab_http_status(:created)
+
+          expect(json_response).to include(
+            'id' => Integer,
+            'project_id' => project.id,
+            'package_name_pattern' => params[:package_name_pattern],
+            'package_type' => params[:package_type],
+            'minimum_access_level_for_delete' => nil,
+            'minimum_access_level_for_push' => params[:minimum_access_level_for_push]
+          )
+
+          expect(Packages::Protection::Rule.find(json_response['id']).minimum_access_level_for_delete).to be_nil
+        end
+      end
+
       context 'without minimum_access_level_for_delete' do
         let(:params) { super().except(:minimum_access_level_for_delete) }
 
@@ -211,6 +246,42 @@
         end
       end
 
+      context 'with changing minimum_access_level_for_delete' do
+        let(:params) { super().merge(minimum_access_level_for_delete: 'admin') }
+
+        it 'updates a package protection rule' do
+          patch_package_rule
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response["minimum_access_level_for_delete"]).to eq('admin')
+        end
+
+        context 'when feature flag :packages_protected_packages_delete is disabled' do
+          before do
+            stub_feature_flags(packages_protected_packages_delete: false)
+          end
+
+          it 'keeps old value for minimum_access_level_for_delete' do
+            expect { patch_package_rule }.to not_change { protection_rule.reload.minimum_access_level_for_delete }
+
+            expect(response).to have_gitlab_http_status(:ok)
+
+            expect(json_response["minimum_access_level_for_delete"]).to be_nil
+          end
+        end
+      end
+
+      context 'with nil value for minimum_access_level_for_delete' do
+        let(:params) { super().merge(minimum_access_level_for_delete: nil) }
+
+        it 'updates a package protection rule' do
+          patch_package_rule
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response["minimum_access_level_for_delete"]).to be_nil
+        end
+      end
+
       context 'with invalid package_type' do
         let(:params) { super().merge(package_type: 'not in enum') }
 
-- 
GitLab


From db82e8e20c137d52cab67e57056d13865d69a82c Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Fri, 14 Mar 2025 15:25:34 +0100
Subject: [PATCH 5/8] docs: Add history hint regarding feature flag

---
 doc/api/project_packages_protection_rules.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index a78812dbbfba8c..c5f9f8ee66060b 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -18,6 +18,7 @@ title: Protected packages API
 - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151741) in GitLab 17.1 [with a flag](../administration/feature_flags.md) named `packages_protected_packages`. Disabled by default.
 - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/472655) in GitLab 17.5.
 - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/472655) in GitLab 17.6. Feature flag `packages_protected_packages` removed.
+- [Added field `minimum_access_level_for_delete`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063) in GitLab 17.11 [with a flag](../administration/feature_flags.md) named `packages_protected_packages_delete`. Disabled by default.
 
 {{< /history >}}
 
-- 
GitLab


From 5a217c3af03af06aa69b219b7c8290672063b6a4 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Sat, 15 Mar 2025 07:33:20 +0100
Subject: [PATCH 6/8] docs: Apply suggestions from @z_painter

- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2398677450
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2398677447
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2398677444
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2398677432
- etc.
---
 doc/api/project_packages_protection_rules.md | 10 +++++-----
 lib/api/project_packages_protection_rules.rb | 12 ++++++------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index c5f9f8ee66060b..5b108cc743b625 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -18,7 +18,7 @@ title: Protected packages API
 - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151741) in GitLab 17.1 [with a flag](../administration/feature_flags.md) named `packages_protected_packages`. Disabled by default.
 - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/472655) in GitLab 17.5.
 - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/472655) in GitLab 17.6. Feature flag `packages_protected_packages` removed.
-- [Added field `minimum_access_level_for_delete`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063) in GitLab 17.11 [with a flag](../administration/feature_flags.md) named `packages_protected_packages_delete`. Disabled by default.
+- [Added](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063) `minimum_access_level_for_delete` attribute in GitLab 17.11 [with a flag](../administration/feature_flags.md) named `packages_protected_packages_delete`. Disabled by default.
 
 {{< /history >}}
 
@@ -92,8 +92,8 @@ Supported attributes:
 | `id`                                  | integer/string  | Yes      | ID or [URL-encoded path of the project](rest/_index.md#namespaced-paths). |
 | `package_name_pattern`                | string          | Yes      | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | Yes      | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level able to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
-| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level able to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
+| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level required to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete`. Disabled by default. |
+| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level required to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`201`](rest/troubleshooting.md#status-codes) and the created package protection rule.
 
@@ -137,8 +137,8 @@ Supported attributes:
 | `package_protection_rule_id`          | integer         | Yes      | ID of the package protection rule to be updated. |
 | `package_name_pattern`                | string          | No       | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | No       | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level able to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete` (disabled by default). |
-| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level able to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
+| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level required to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete`. Disabled by default. |
+| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level required to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`200`](rest/troubleshooting.md#status-codes) and the updated package protection rule.
 
diff --git a/lib/api/project_packages_protection_rules.rb b/lib/api/project_packages_protection_rules.rb
index c41b231b5fc65f..2f8185f0add660 100644
--- a/lib/api/project_packages_protection_rules.rb
+++ b/lib/api/project_packages_protection_rules.rb
@@ -48,14 +48,14 @@ class ProjectPackagesProtectionRules < ::API::Base
             desc: 'Package type protected by the rule. For example npm.'
           optional :minimum_access_level_for_delete, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
-            desc: 'Minimum GitLab access level able to delete a package. ' \
+            desc: 'Minimum GitLab access level required to delete a package. ' \
               'Valid values include `null`, `owner` or `admin`. ' \
               'If the value is `nil`, the default minimum access level is `maintainer`. ' \
               'Must be provided when `minimum_access_level_for_push` is not set. ' \
-              'Behind a feature flag named `packages_protected_packages_delete` (disabled by default).'
+              'Behind a feature flag named `packages_protected_packages_delete`. Disabled by default.'
           optional :minimum_access_level_for_push, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
-            desc: 'Minimum GitLab access level able to push a package. ' \
+            desc: 'Minimum GitLab access level required to push a package. ' \
               'Valid values include `null`, `maintainer`, `owner` or `admin`. ' \
               'If the value is `nil`, the default minimum access level is `developer`. ' \
               'Must be provided when `minimum_access_level_for_delete` is not set.'
@@ -99,14 +99,14 @@ class ProjectPackagesProtectionRules < ::API::Base
               desc: 'Package type protected by the rule. For example npm.'
             optional :minimum_access_level_for_delete, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
-              desc: 'Minimum GitLab access level able to delete a package. ' \
+              desc: 'Minimum GitLab access level required to delete a package. ' \
                 'Valid values include `null`, `owner` or `admin`. ' \
                 'If the value is `nil`, the default minimum access level is `maintainer`. ' \
                 'Must be provided when `minimum_access_level_for_push` is not set. ' \
-                'Behind a feature flag named `packages_protected_packages_delete` (disabled by default).'
+                'Behind a feature flag named `packages_protected_packages_delete`. Disabled by default.'
             optional :minimum_access_level_for_push, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
-              desc: 'Minimum GitLab access level able to push a package. ' \
+              desc: 'Minimum GitLab access level required to push a package. ' \
                 'Valid values include `null`, `maintainer`, `owner` or `admin`. ' \
                 'If the value is `nil`, the default minimum access level is `developer`. ' \
                 'Must be provided when `minimum_access_level_for_delete` is not set.'
-- 
GitLab


From 4187267028d513af26dc7e711f88e949844a03a8 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Mon, 24 Mar 2025 15:32:24 +0100
Subject: [PATCH 7/8] docs: Apply suggestions from @dmeshcharakou

---
 doc/api/project_packages_protection_rules.md | 8 ++++----
 lib/api/project_packages_protection_rules.rb | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/api/project_packages_protection_rules.md b/doc/api/project_packages_protection_rules.md
index 5b108cc743b625..72dcb9d9af4720 100644
--- a/doc/api/project_packages_protection_rules.md
+++ b/doc/api/project_packages_protection_rules.md
@@ -92,8 +92,8 @@ Supported attributes:
 | `id`                                  | integer/string  | Yes      | ID or [URL-encoded path of the project](rest/_index.md#namespaced-paths). |
 | `package_name_pattern`                | string          | Yes      | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | Yes      | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level required to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete`. Disabled by default. |
-| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level required to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
+| `minimum_access_level_for_delete`     | string          | Yes      | Minimum GitLab access level required to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `null`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete`. Disabled by default. |
+| `minimum_access_level_for_push`       | string          | Yes      | Minimum GitLab access level required to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `null`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`201`](rest/troubleshooting.md#status-codes) and the created package protection rule.
 
@@ -137,8 +137,8 @@ Supported attributes:
 | `package_protection_rule_id`          | integer         | Yes      | ID of the package protection rule to be updated. |
 | `package_name_pattern`                | string          | No       | Package name protected by the protection rule. For example `@my-scope/my-package-*`. Wildcard character `*` allowed. |
 | `package_type`                        | string          | No       | Package type protected by the protection rule. For example `npm`. |
-| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level required to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete`. Disabled by default. |
-| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level required to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `nil`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
+| `minimum_access_level_for_delete`     | string          | No       | Minimum GitLab access level required to delete a package. Valid values include `null`, `owner` or `admin`. If the value is `null`, the default minimum access level is `maintainer`. Must be provided when `minimum_access_level_for_push` is not set. Behind a feature flag named `packages_protected_packages_delete`. Disabled by default. |
+| `minimum_access_level_for_push`       | string          | No       | Minimum GitLab access level required to push a package. Valid values include `null`, `maintainer`, `owner` or `admin`. If the value is `null`, the default minimum access level is `developer`. Must be provided when `minimum_access_level_for_delete` is not set. |
 
 If successful, returns [`200`](rest/troubleshooting.md#status-codes) and the updated package protection rule.
 
diff --git a/lib/api/project_packages_protection_rules.rb b/lib/api/project_packages_protection_rules.rb
index 2f8185f0add660..cd75de51f6611d 100644
--- a/lib/api/project_packages_protection_rules.rb
+++ b/lib/api/project_packages_protection_rules.rb
@@ -50,14 +50,14 @@ class ProjectPackagesProtectionRules < ::API::Base
             values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
             desc: 'Minimum GitLab access level required to delete a package. ' \
               'Valid values include `null`, `owner` or `admin`. ' \
-              'If the value is `nil`, the default minimum access level is `maintainer`. ' \
+              'If the value is `null`, the default minimum access level is `maintainer`. ' \
               'Must be provided when `minimum_access_level_for_push` is not set. ' \
               'Behind a feature flag named `packages_protected_packages_delete`. Disabled by default.'
           optional :minimum_access_level_for_push, type: String,
             values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
             desc: 'Minimum GitLab access level required to push a package. ' \
               'Valid values include `null`, `maintainer`, `owner` or `admin`. ' \
-              'If the value is `nil`, the default minimum access level is `developer`. ' \
+              'If the value is `null`, the default minimum access level is `developer`. ' \
               'Must be provided when `minimum_access_level_for_delete` is not set.'
           at_least_one_of :minimum_access_level_for_push, :minimum_access_level_for_delete
         end
@@ -101,14 +101,14 @@ class ProjectPackagesProtectionRules < ::API::Base
               values: Packages::Protection::Rule.minimum_access_level_for_deletes.keys,
               desc: 'Minimum GitLab access level required to delete a package. ' \
                 'Valid values include `null`, `owner` or `admin`. ' \
-                'If the value is `nil`, the default minimum access level is `maintainer`. ' \
+                'If the value is `null`, the default minimum access level is `maintainer`. ' \
                 'Must be provided when `minimum_access_level_for_push` is not set. ' \
                 'Behind a feature flag named `packages_protected_packages_delete`. Disabled by default.'
             optional :minimum_access_level_for_push, type: String,
               values: Packages::Protection::Rule.minimum_access_level_for_pushes.keys,
               desc: 'Minimum GitLab access level required to push a package. ' \
                 'Valid values include `null`, `maintainer`, `owner` or `admin`. ' \
-                'If the value is `nil`, the default minimum access level is `developer`. ' \
+                'If the value is `null`, the default minimum access level is `developer`. ' \
                 'Must be provided when `minimum_access_level_for_delete` is not set.'
           end
           patch do
-- 
GitLab


From 88f342a570594614a2a947801203f2e991969cf3 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Mon, 24 Mar 2025 16:00:29 +0100
Subject: [PATCH 8/8] test: Apply suggestions from @lwanko

Omnly small non-blocking suggestions
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2407960526
- https://gitlab.com/gitlab-org/gitlab/-/merge_requests/180063#note_2407058846
---
 spec/requests/api/project_packages_protection_rules_spec.rb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/spec/requests/api/project_packages_protection_rules_spec.rb b/spec/requests/api/project_packages_protection_rules_spec.rb
index 52611792e84813..b1c597fdba4acd 100644
--- a/spec/requests/api/project_packages_protection_rules_spec.rb
+++ b/spec/requests/api/project_packages_protection_rules_spec.rb
@@ -181,6 +181,10 @@
         it 'does not create a package protection rule' do
           expect { post_package_rule }.to not_change(Packages::Protection::Rule, :count)
           expect(response).to have_gitlab_http_status(:bad_request)
+          expect(json_response).to include(
+            'error' => 'minimum_access_level_for_push, minimum_access_level_for_delete are missing, ' \
+              'at least one parameter must be provided'
+          )
         end
       end
 
@@ -298,6 +302,7 @@
         let(:params) { super().merge(minimum_access_level_for_delete: '') }
 
         it_behaves_like 'returning response status', :bad_request
+
         it 'returns error message' do
           expect { patch_package_rule }.to not_change(Packages::Protection::Rule, :count)
           expect(json_response).to include 'error' => 'minimum_access_level_for_delete does not have a valid value'
-- 
GitLab