Merge branch...

Merge branch '457518-nwittstruck-protected-containers-rest-api-delete-protection-rule' into 'master' 

Protected containers: Add DELETE REST API for container protection rules

See merge request !159619



Merged-by: Kamil Trzciński <ayufan@ayufan.eu>
Approved-by: Moaz Khalifa <mkhalifa@gitlab.com>
Approved-by: Kamil Trzciński <ayufan@ayufan.eu>
Reviewed-by: Kamil Trzciński <ayufan@ayufan.eu>
Reviewed-by: Marcel Amirault <mamirault@gitlab.com>
Reviewed-by: Moaz Khalifa <mkhalifa@gitlab.com>
Co-authored-by: Nicholas Wittstruck <nicholas@b310.de>
Co-authored-by: Nicholas <1494491-nwittstruck@users.noreply.gitlab.com>

doc/api/project_container_registry_protection_rules.md

@@ -158,3 +158,37 @@ curl --request PATCH \
        "repository_path_pattern": "flight/flight-*"
     }'
 ```
+
+## Delete a container registry protection rule
+
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/457518) in GitLab 17.3.
+
+Deletes a container registry protection rule from a project.
+
+```plaintext
+DELETE /api/v4/projects/:id/registry/protection/rules/:protection_rule_id
+```
+
+Supported attributes:
+
+| Attribute            | Type           | Required | Description |
+|----------------------|----------------|----------|-------------|
+| `id`                 | integer/string | Yes      | ID or [URL-encoded path of the project](rest/index.md#namespaced-path-encoding) owned by the authenticated user. |
+| `protection_rule_id` | integer        | Yes      | ID of the container registry protection rule to be deleted. |
+
+If successful, returns [`204 No Content`](rest/index.md#status-codes).
+
+Can return the following status codes:
+
+- `204 No Content`: The protection rule was deleted successfully.
+- `400 Bad Request`: The `id` or the `protection_rule_id` are missing or are invalid.
+- `401 Unauthorized`: The access token is invalid.
+- `403 Forbidden`: The user does not have permission to delete the protection rule.
+- `404 Not Found`: The project or the protection rule was not found.
+
+Example request:
+
+```shell
+curl --request DELETE --header "PRIVATE-TOKEN: <your_access_token>" \
+  --url "https://gitlab.example.com/api/v4/projects/7/registry/protection/rules/1"
+```

lib/api/project_container_registry_protection_rules.rb

@@ -110,6 +110,28 @@ class ProjectContainerRegistryProtectionRules < ::API::Base
             present response[:container_registry_protection_rule],
               with: Entities::Projects::ContainerRegistry::Protection::Rule
           end
+
+          desc 'Delete container protection rule' do
+            success code: 204, message: '204 No Content'
+            failure [
+              { code: 400, message: 'Bad Request' },
+              { code: 401, message: 'Unauthorized' },
+              { code: 403, message: 'Forbidden' },
+              { code: 404, message: 'Not Found' }
+            ]
+            tags %w[projects]
+            hidden true
+          end
+          delete do
+            protection_rule = user_project.container_registry_protection_rules.find(params[:protection_rule_id])
+
+            destroy_conditionally!(protection_rule) do |protection_rule|
+              response = ::ContainerRegistry::Protection::DeleteRuleService.new(protection_rule,
+                current_user: current_user).execute
+
+              render_api_error!({ error: response.message }, :bad_request) if response.error?
+            end
+          end
         end
       end
     end

spec/requests/api/project_container_registry_protection_rules_spec.rb

@@ -7,7 +7,8 @@
 
   let_it_be(:project) { create(:project, :private) }
   let_it_be(:other_project) { create(:project, :private) }
-  let_it_be(:container_registry_protection_rule) { create(:container_registry_protection_rule, project: project) }
+  let_it_be(:protection_rule) { create(:container_registry_protection_rule, project: project) }
+  let_it_be(:protection_rule_id) { protection_rule.id }
 
   let_it_be(:maintainer) { create(:user, maintainer_of: [project, other_project]) }
   let_it_be(:api_user) { create(:user) }
@@ -15,29 +16,13 @@
   let_it_be(:invalid_token) { 'invalid-token123' }
   let_it_be(:headers_with_invalid_token) { { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => invalid_token } }
 
-  let(:params) do
-    { repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-unique",
-      minimum_access_level_for_push: container_registry_protection_rule.minimum_access_level_for_push,
-      minimum_access_level_for_delete: container_registry_protection_rule.minimum_access_level_for_delete }
-  end
-
-  shared_examples 'rejecting project container protection rules request when not enough permissions' do
-    using RSpec::Parameterized::TableSyntax
-
-    where(:user_role, :status) do
-      :reporter  | :forbidden
-      :developer | :forbidden
-      :guest     | :forbidden
-      nil        | :not_found
-    end
-
-    with_them do
-      before do
-        project.send(:"add_#{user_role}", api_user) if user_role
-      end
+  let(:path) { 'registry/protection/rules' }
+  let(:url) { "/projects/#{project.id}/#{path}" }
 
-      it_behaves_like 'returning response status', params[:status]
-    end
+  let(:params) do
+    { repository_path_pattern: "#{protection_rule.repository_path_pattern}-unique",
+      minimum_access_level_for_push: protection_rule.minimum_access_level_for_push,
+      minimum_access_level_for_delete: protection_rule.minimum_access_level_for_delete }
   end
 
   shared_examples 'rejecting container registry protection rules request when enough permissions' do
@@ -49,45 +34,13 @@
       it_behaves_like 'returning response status', :not_found
     end
 
-    context 'when the project id is invalid' do
-      let(:url) { "/projects/invalid/registry/protection/rules" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
-
-    context 'when the project id does not exist' do
-      let(:url) { "/projects/#{non_existing_record_id}/registry/protection/rules" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
-  end
-
-  shared_examples 'rejecting container registry protection rules request when handling rule ids' do
-    context 'when the rule id is invalid' do
-      let(:url) { "/projects/#{project.id}/registry/protection/rules/invalid" }
-
-      it_behaves_like 'returning response status', :bad_request
-    end
-
-    context 'when the rule id does not exist' do
-      let(:url) { "/projects/#{project.id}/registry/protection/rules/#{non_existing_record_id}" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
-
-    context 'when the container registry protection rule does belong to another project' do
-      let(:url) { "/projects/#{other_project.id}/registry/protection/rules/#{container_registry_protection_rule.id}" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
+    it_behaves_like 'rejecting protection rules request when invalid project'
   end
 
   describe 'GET /projects/:id/registry/protection/rules' do
-    let(:url) { "/projects/#{project.id}/registry/protection/rules" }
-
     subject(:get_container_registry_rules) { get(api(url, api_user)) }
 
-    it_behaves_like 'rejecting project container protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -96,7 +49,7 @@
         let_it_be(:other_container_registry_protection_rule) do
           create(:container_registry_protection_rule,
             project: project,
-            repository_path_pattern: "#{container_registry_protection_rule.repository_path_pattern}-unique")
+            repository_path_pattern: "#{protection_rule.repository_path_pattern}-unique")
         end
 
         it 'gets the container registry protection rules', :aggregate_failures do
@@ -104,7 +57,7 @@
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response.count).to eq(2)
-          expect(json_response.pluck('id')).to match_array([container_registry_protection_rule.id,
+          expect(json_response.pluck('id')).to match_array([protection_rule.id,
             other_container_registry_protection_rule.id])
         end
       end
@@ -113,10 +66,10 @@
         get_container_registry_rules
 
         expect(json_response.first).to include(
-          'project_id' => container_registry_protection_rule.project.id,
-          'repository_path_pattern' => container_registry_protection_rule.repository_path_pattern,
-          'minimum_access_level_for_push' => container_registry_protection_rule.minimum_access_level_for_push,
-          'minimum_access_level_for_delete' => container_registry_protection_rule.minimum_access_level_for_delete
+          'project_id' => protection_rule.project.id,
+          'repository_path_pattern' => protection_rule.repository_path_pattern,
+          'minimum_access_level_for_push' => protection_rule.minimum_access_level_for_push,
+          'minimum_access_level_for_delete' => protection_rule.minimum_access_level_for_delete
         )
       end
 
@@ -131,11 +84,9 @@
   end
 
   describe 'POST /projects/:id/registry/protection/rules' do
-    let(:url) { "/projects/#{project.id}/registry/protection/rules" }
-
     subject(:post_container_registry_rule) { post(api(url, api_user), params: params) }
 
-    it_behaves_like 'rejecting project container protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -191,7 +142,7 @@
 
       context 'with already existing repository_path_pattern' do
         before do
-          params[:repository_path_pattern] = container_registry_protection_rule.repository_path_pattern
+          params[:repository_path_pattern] = protection_rule.repository_path_pattern
         end
 
         it 'does not create a container registry protection rule' do
@@ -223,16 +174,16 @@
   end
 
   describe 'PATCH /projects/:id/registry/protection/rules/:protection_rule_id' do
-    let(:url) { "/projects/#{project.id}/registry/protection/rules/#{container_registry_protection_rule.id}" }
+    let(:path) { "registry/protection/rules/#{protection_rule_id}" }
 
     subject(:patch_container_registry_protection_rule) { patch(api(url, api_user), params: params) }
 
-    it_behaves_like 'rejecting project container protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
       let_it_be(:changed_repository_path_pattern) do
-        "#{container_registry_protection_rule.repository_path_pattern}-changed"
+        "#{protection_rule.repository_path_pattern}-changed"
       end
 
       context 'with full changeset' do
@@ -244,10 +195,10 @@
           patch_container_registry_protection_rule
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response).to match(hash_including({
-            'project_id' => container_registry_protection_rule.project.id,
+            'project_id' => protection_rule.project.id,
             'repository_path_pattern' => changed_repository_path_pattern,
-            'minimum_access_level_for_push' => container_registry_protection_rule.minimum_access_level_for_push,
-            'minimum_access_level_for_delete' => container_registry_protection_rule.minimum_access_level_for_delete
+            'minimum_access_level_for_push' => protection_rule.minimum_access_level_for_push,
+            'minimum_access_level_for_delete' => protection_rule.minimum_access_level_for_delete
           }))
         end
       end
@@ -331,7 +282,7 @@
         it_behaves_like 'returning response status', :unprocessable_entity
       end
 
-      it_behaves_like 'rejecting container registry protection rules request when handling rule ids'
+      it_behaves_like 'rejecting protection rules request when handling rule ids'
       it_behaves_like 'rejecting container registry protection rules request when enough permissions'
     end
 
@@ -343,4 +294,33 @@
       it_behaves_like 'returning response status', :unauthorized
     end
   end
+
+  describe 'DELETE /projects/:id/registry/protection/rules/:protection_rule_id' do
+    let(:path) { "registry/protection/rules/#{protection_rule_id}" }
+
+    subject(:delete_protection_rule) { delete(api(url, api_user)) }
+
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
+
+    context 'for maintainer' do
+      let(:api_user) { maintainer }
+
+      it 'deletes the container registry protection rule' do
+        delete_protection_rule
+        expect do
+          ContainerRegistry::Protection::Rule.find(protection_rule.id)
+        end.to raise_error(ActiveRecord::RecordNotFound)
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+
+      it_behaves_like 'rejecting protection rules request when handling rule ids'
+      it_behaves_like 'rejecting container registry protection rules request when enough permissions'
+    end
+
+    context 'with invalid token' do
+      subject(:delete_protection_rules) { delete(api(url), headers: headers_with_invalid_token) }
+
+      it_behaves_like 'returning response status', :unauthorized
+    end
+  end
 end

spec/requests/api/project_packages_protection_rules_spec.rb

@@ -7,7 +7,8 @@
 
   let_it_be(:project) { create(:project, :private) }
   let_it_be(:other_project) { create(:project, :private) }
-  let_it_be(:package_protection_rule) { create(:package_protection_rule, project: project) }
+  let_it_be(: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]) }
   let_it_be(:api_user) { create(:user) }
@@ -15,29 +16,13 @@
   let_it_be(:invalid_token) { 'invalid-token123' }
   let_it_be(:headers_with_invalid_token) { { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => invalid_token } }
 
+  let(:path) { 'packages/protection/rules' }
+  let(:url) { "/projects/#{project.id}/#{path}" }
+
   let(:params) do
     { package_name_pattern: '@my-new-scope/my-package-*',
-      package_type: package_protection_rule.package_type,
-      minimum_access_level_for_push: package_protection_rule.minimum_access_level_for_push }
-  end
-
-  shared_examples 'rejecting project packages protection rules request when not enough permissions' do
-    using RSpec::Parameterized::TableSyntax
-
-    where(:user_role, :status) do
-      :reporter  | :forbidden
-      :developer | :forbidden
-      :guest     | :forbidden
-      nil        | :not_found
-    end
-
-    with_them do
-      before do
-        project.send(:"add_#{user_role}", api_user) if user_role
-      end
-
-      it_behaves_like 'returning response status', params[:status]
-    end
+      package_type: protection_rule.package_type,
+      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
@@ -49,37 +34,7 @@
       it_behaves_like 'returning response status', :not_found
     end
 
-    context 'when the project id is invalid' do
-      let(:url) { "/projects/invalid/packages/protection/rules" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
-
-    context 'when the project id does not exist' do
-      let(:url) { "/projects/#{non_existing_record_id}/packages/protection/rules" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
-  end
-
-  shared_examples 'rejecting project packages protection rules request when handling rule ids' do
-    context 'when the rule id is invalid' do
-      let(:url) { "/projects/#{project.id}/packages/protection/rules/invalid" }
-
-      it_behaves_like 'returning response status', :bad_request
-    end
-
-    context 'when the rule id does not exist' do
-      let(:url) { "/projects/#{project.id}/packages/protection/rules/#{non_existing_record_id}" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
-
-    context 'when the package protection rule does belong to another project' do
-      let(:url) { "/projects/#{other_project.id}/packages/protection/rules/#{package_protection_rule.id}" }
-
-      it_behaves_like 'returning response status', :not_found
-    end
+    it_behaves_like 'rejecting protection rules request when invalid project'
   end
 
   describe 'GET /projects/:id/packages/protection/rules' do
@@ -87,7 +42,7 @@
 
     subject(:get_package_rules) { get(api(url, api_user)) }
 
-    it_behaves_like 'rejecting project packages protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -118,7 +73,7 @@
 
     subject(:post_package_rule) { post(api(url, api_user), params: params) }
 
-    it_behaves_like 'rejecting project packages protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -152,7 +107,7 @@
 
       context 'with already existing package_name_pattern' do
         before do
-          params[:package_name_pattern] = package_protection_rule.package_name_pattern
+          params[:package_name_pattern] = protection_rule.package_name_pattern
         end
 
         it 'does not create a package protection rule' do
@@ -172,11 +127,11 @@
   end
 
   describe 'PATCH /projects/:id/packages/protection/rules/:package_protection_rule_id' do
-    let(:url) { "/projects/#{project.id}/packages/protection/rules/#{package_protection_rule.id}" }
+    let(:path) { "packages/protection/rules/#{protection_rule_id}" }
 
     subject(:patch_package_rule) { patch(api(url, api_user), params: params) }
 
-    it_behaves_like 'rejecting project packages protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -192,7 +147,7 @@
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response["package_name_pattern"]).to eq(changed_scope)
-          expect(json_response["package_type"]).to eq(package_protection_rule.package_type)
+          expect(json_response["package_type"]).to eq(protection_rule.package_type)
         end
       end
 
@@ -233,7 +188,7 @@
         it_behaves_like 'returning response status', :unprocessable_entity
       end
 
-      it_behaves_like 'rejecting project packages protection rules request when handling rule ids'
+      it_behaves_like 'rejecting protection rules request when handling rule ids'
       it_behaves_like 'rejecting project packages protection rules request when enough permissions'
     end
 
@@ -245,11 +200,11 @@
   end
 
   describe 'DELETE /projects/:id/packages/protection/rules/:package_protection_rule_id' do
-    let(:url) { "/projects/#{project.id}/packages/protection/rules/#{package_protection_rule.id}" }
+    let(:path) { "packages/protection/rules/#{protection_rule_id}" }
 
     subject(:destroy_package_rule) { delete(api(url, api_user)) }
 
-    it_behaves_like 'rejecting project packages protection rules request when not enough permissions'
+    it_behaves_like 'rejecting project protection rules request when not enough permissions'
 
     context 'for maintainer' do
       let(:api_user) { maintainer }
@@ -257,12 +212,12 @@
       it 'deletes the package protection rule' do
         destroy_package_rule
         expect do
-          Packages::Protection::Rule.find(package_protection_rule.id)
+          Packages::Protection::Rule.find(protection_rule.id)
         end.to raise_error(ActiveRecord::RecordNotFound)
         expect(response).to have_gitlab_http_status(:no_content)
       end
 
-      it_behaves_like 'rejecting project packages protection rules request when handling rule ids'
+      it_behaves_like 'rejecting protection rules request when handling rule ids'
       it_behaves_like 'rejecting project packages protection rules request when enough permissions'
     end
 

spec/support/shared_examples/requests/api/protection_rules_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'rejecting project protection rules request when not enough permissions' do
+  using RSpec::Parameterized::TableSyntax
+
+  where(:user_role, :status) do
+    :reporter  | :forbidden
+    :developer | :forbidden
+    :guest     | :forbidden
+    nil        | :not_found
+  end
+
+  with_them do
+    before do
+      project.send(:"add_#{user_role}", api_user) if user_role
+    end
+
+    it_behaves_like 'returning response status', params[:status]
+  end
+end
+
+RSpec.shared_examples 'rejecting protection rules request when handling rule ids' do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:valid_project_id) { project.id }
+  let(:valid_protection_rule_id) { protection_rule.id }
+  let(:other_project_id) { other_project.id }
+  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(:other_project_id) | ref(:valid_protection_rule_id) | :not_found
+  end
+
+  with_them do
+    it_behaves_like 'returning response status', params[:status]
+  end
+end
+
+RSpec.shared_examples 'rejecting protection rules request when invalid project' do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:url) { "/projects/#{project_id}/#{path}" }
+
+  where(:project_id, :status) do
+    'invalid'               | :not_found
+    non_existing_record_id  | :not_found
+  end
+
+  with_them do
+    it_behaves_like 'returning response status', params[:status]
+  end
+end