Commit 4ea8fe08 authored by David Fernandez's avatar David Fernandez Committed by Saikat Sarkar

Fix the dependency worker purge feature

parent 81ba6674
...@@ -27,6 +27,6 @@ class PurgeDependencyProxyCacheWorker ...@@ -27,6 +27,6 @@ class PurgeDependencyProxyCacheWorker
def valid? def valid?
return unless @group return unless @group
can?(@current_user, :admin_group, @group) && @group.dependency_proxy_feature_available? can?(@current_user, :admin_group, @group)
end end
end end
...@@ -14,9 +14,6 @@ info: To determine the technical writer assigned to the Stage/Group associated w ...@@ -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) Deletes the cached manifests and blobs for a group. This endpoint requires the [Owner role](../user/permissions.md)
for the group. for the group.
WARNING:
[A bug exists](https://gitlab.com/gitlab-org/gitlab/-/issues/277161) for this API.
```plaintext ```plaintext
DELETE /groups/:id/dependency_proxy/cache DELETE /groups/:id/dependency_proxy/cache
``` ```
......
...@@ -15,7 +15,7 @@ module API ...@@ -15,7 +15,7 @@ module API
end end
end end
before do after_validation do
authorize! :admin_group, user_group authorize! :admin_group, user_group
end end
...@@ -35,6 +35,8 @@ module API ...@@ -35,6 +35,8 @@ module API
# rubocop:disable CodeReuse/Worker # rubocop:disable CodeReuse/Worker
PurgeDependencyProxyCacheWorker.perform_async(current_user.id, user_group.id) PurgeDependencyProxyCacheWorker.perform_async(current_user.id, user_group.id)
# rubocop:enable CodeReuse/Worker # rubocop:enable CodeReuse/Worker
status :accepted
end end
end end
end end
......
...@@ -13,60 +13,74 @@ RSpec.describe API::DependencyProxy, api: true do ...@@ -13,60 +13,74 @@ RSpec.describe API::DependencyProxy, api: true do
group.add_owner(user) group.add_owner(user)
stub_config(dependency_proxy: { enabled: true }) stub_config(dependency_proxy: { enabled: true })
stub_last_activity_update stub_last_activity_update
group.create_dependency_proxy_setting!(enabled: true)
end end
describe 'DELETE /groups/:id/dependency_proxy/cache' do 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 shared_examples 'responding to purge requests' do
let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" } context 'with feature available and enabled' do
let_it_be(:lease_key) { "dependency_proxy:delete_group_blobs:#{group.id}" }
context 'an admin user' do context 'an admin user' do
it 'deletes the blobs and returns no content' do it 'deletes the blobs and returns no content' do
stub_exclusive_lease(lease_key, timeout: 1.hour) stub_exclusive_lease(lease_key, timeout: 1.hour)
expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async) expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async)
subject subject
expect(response).to have_gitlab_http_status(:no_content) expect(response).to have_gitlab_http_status(:accepted)
end expect(response.body).to eq('202')
end
context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do
it 'returns 409 with an error message' do it 'returns 409 with an error message' do
stub_exclusive_lease_taken(lease_key, timeout: 1.hour) stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
subject subject
expect(response).to have_gitlab_http_status(:conflict) expect(response).to have_gitlab_http_status(:conflict)
expect(response.body).to include('This request has already been made.') 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
end
it 'executes service only for the first time' do context 'a non-admin' do
expect(PurgeDependencyProxyCacheWorker).to receive(:perform_async).once let(:user) { create(:user) }
2.times { subject } before do
group.add_maintainer(user)
end end
it_behaves_like 'returning response status', :forbidden
end end
end end
context 'a non-admin' do context 'depencency proxy is not enabled in the config' do
let(:user) { create(:user) }
before do before do
group.add_maintainer(user) stub_config(dependency_proxy: { enabled: false })
end end
it_behaves_like 'returning response status', :forbidden it_behaves_like 'returning response status', :not_found
end end
end end
context 'depencency proxy is not enabled' do context 'with a group id' do
before do let(:group_id) { group.id }
stub_config(dependency_proxy: { enabled: false })
end 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 end
end end
...@@ -11,14 +11,9 @@ RSpec.describe PurgeDependencyProxyCacheWorker do ...@@ -11,14 +11,9 @@ RSpec.describe PurgeDependencyProxyCacheWorker do
subject { described_class.new.perform(user.id, group_id) } 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 describe '#perform' do
shared_examples 'returns nil' do shared_examples 'not removing blobs and manifests' do
it 'returns nil', :aggregate_failures 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_blobs.size }
expect { subject }.not_to change { group.dependency_proxy_manifests.size } expect { subject }.not_to change { group.dependency_proxy_manifests.size }
expect(subject).to be_nil expect(subject).to be_nil
...@@ -43,26 +38,26 @@ RSpec.describe PurgeDependencyProxyCacheWorker do ...@@ -43,26 +38,26 @@ RSpec.describe PurgeDependencyProxyCacheWorker do
end end
context 'when admin mode is disabled' do context 'when admin mode is disabled' do
it_behaves_like 'returns nil' it_behaves_like 'not removing blobs and manifests'
end end
end end
context 'a non-admin user' do context 'a non-admin user' do
let(:user) { create(:user) } let(:user) { create(:user) }
it_behaves_like 'returns nil' it_behaves_like 'not removing blobs and manifests'
end end
context 'an invalid user id' do context 'an invalid user id' do
let(:user) { double('User', id: 99999 ) } let(:user) { double('User', id: 99999 ) }
it_behaves_like 'returns nil' it_behaves_like 'not removing blobs and manifests'
end end
context 'an invalid group' do context 'an invalid group' do
let(:group_id) { 99999 } let(:group_id) { 99999 }
it_behaves_like 'returns nil' it_behaves_like 'not removing blobs and manifests'
end end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment