Merge branch '277161-fix-dependency-proxy-purge-job' into 'master'

Fix the dependency worker purge feature

See merge request gitlab-org/gitlab!69736

app/workers/purge_dependency_proxy_cache_worker.rb

@@ -27,6 +27,6 @@ class PurgeDependencyProxyCacheWorker
   def valid?
     return unless @group
 
-    can?(@current_user, :admin_group, @group) && @group.dependency_proxy_feature_available?
+    can?(@current_user, :admin_group, @group)
   end
 end

doc/api/dependency_proxy.md

@@ -14,9 +14,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 Deletes the cached manifests and blobs for a group. This endpoint requires the [Owner role](../user/permissions.md)
 for the group.
 
-WARNING:
-[A bug exists](https://gitlab.com/gitlab-org/gitlab/-/issues/277161) for this API.
-
 ```plaintext
 DELETE /groups/:id/dependency_proxy/cache
 ```

lib/api/dependency_proxy.rb

@@ -15,7 +15,7 @@ module API
       end
     end
 
-    before do
+    after_validation do
       authorize! :admin_group, user_group
     end
 
@@ -35,6 +35,8 @@ module API
         # rubocop:disable CodeReuse/Worker
         PurgeDependencyProxyCacheWorker.perform_async(current_user.id, user_group.id)
         # rubocop:enable CodeReuse/Worker
+
+        status :accepted
       end
     end
   end

spec/requests/api/dependency_proxy_spec.rb

@@ -13,60 +13,74 @@ RSpec.describe API::DependencyProxy, api: true do
     group.add_owner(user)
     stub_config(dependency_proxy: { enabled: true })
     stub_last_activity_update
-    group.create_dependency_proxy_setting!(enabled: true)
   end
 
   describe 'DELETE /groups/:id/dependency_proxy/cache' do
-    subject { delete api("/groups/#{group.id}/dependency_proxy/cache", user) }
+    subject { delete api("/groups/#{group_id}/dependency_proxy/cache", user) }
 
-    context 'with feature available and enabled' do
-      let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" }
+    shared_examples 'responding to purge requests' do
+      context 'with feature available and enabled' do
+        let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" }
 
-      context 'an admin user' do
-        it 'deletes the blobs and returns no content' do
-          stub_exclusive_lease(lease_key, timeout: 1.hour)
-          expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async)
+        context 'an admin user' do
+          it 'deletes the blobs and returns no content' do
+            stub_exclusive_lease(lease_key, timeout: 1.hour)
+            expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async)
 
-          subject
+            subject
 
-          expect(response).to have_gitlab_http_status(:no_content)
-        end
+            expect(response).to have_gitlab_http_status(:accepted)
+            expect(response.body).to eq('202')
+          end
 
-        context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do
-          it 'returns 409 with an error message' do
-            stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
+          context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do
+            it 'returns 409 with an error message' do
+              stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
 
-            subject
+              subject
 
-            expect(response).to have_gitlab_http_status(:conflict)
-            expect(response.body).to include('This request has already been made.')
+              expect(response).to have_gitlab_http_status(:conflict)
+              expect(response.body).to include('This request has already been made.')
+            end
+
+            it 'executes service only for the first time' do
+              expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async).once
+
+              2.times { subject }
+            end
           end
+        end
 
-          it 'executes service only for the first time' do
-            expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async).once
+        context 'a non-admin' do
+          let(:user) { create(:user) }
 
-            2.times { subject }
+          before do
+            group.add_maintainer(user)
           end
+
+          it_behaves_like 'returning response status', :forbidden
         end
       end
 
-      context 'a non-admin' do
-        let(:user) { create(:user) }
-
+      context 'depencency proxy is not enabled in the config' do
         before do
-          group.add_maintainer(user)
+          stub_config(dependency_proxy: { enabled: false })
         end
 
-        it_behaves_like 'returning response status', :forbidden
+        it_behaves_like 'returning response status', :not_found
       end
     end
 
-    context 'depencency proxy is not enabled' do
-      before do
-        stub_config(dependency_proxy: { enabled: false })
-      end
+    context 'with a group id' do
+      let(:group_id) { group.id }
+
+      it_behaves_like 'responding to purge requests'
+    end
+
+    context 'with an url encoded group id' do
+      let(:group_id) { ERB::Util.url_encode(group.full_path) }
 
-      it_behaves_like 'returning response status', :not_found
+      it_behaves_like 'responding to purge requests'
     end
   end
 end

spec/workers/purge_dependency_proxy_cache_worker_spec.rb

@@ -11,14 +11,9 @@ RSpec.describe PurgeDependencyProxyCacheWorker do
 
   subject { described_class.new.perform(user.id, group_id) }
 
-  before do
-    stub_config(dependency_proxy: { enabled: true })
-    group.create_dependency_proxy_setting!(enabled: true)
-  end
-
   describe '#perform' do
-    shared_examples 'returns nil' do
-      it 'returns nil', :aggregate_failures do
+    shared_examples 'not removing blobs and manifests' do
+      it 'does not remove blobs and manifests', :aggregate_failures do
         expect { subject }.not_to change { group.dependency_proxy_blobs.size }
         expect { subject }.not_to change { group.dependency_proxy_manifests.size }
         expect(subject).to be_nil
@@ -43,26 +38,26 @@ RSpec.describe PurgeDependencyProxyCacheWorker do
       end
 
       context 'when admin mode is disabled' do
-        it_behaves_like 'returns nil'
+        it_behaves_like 'not removing blobs and manifests'
       end
     end
 
     context 'a non-admin user' do
       let(:user) { create(:user) }
 
-      it_behaves_like 'returns nil'
+      it_behaves_like 'not removing blobs and manifests'
     end
 
     context 'an invalid user id' do
       let(:user) { double('User', id: 99999 ) }
 
-      it_behaves_like 'returns nil'
+      it_behaves_like 'not removing blobs and manifests'
     end
 
     context 'an invalid group' do
       let(:group_id) { 99999 }
 
-      it_behaves_like 'returns nil'
+      it_behaves_like 'not removing blobs and manifests'
     end
   end
 end