Commit a51fa7af authored by Steve Abrams's avatar Steve Abrams

Update DeleteTagsService to allow policy to work

Update the DeleteTagsService to check for a param that
tells it if the service is being run by a worker and
not a user.
parent 185b5541
......@@ -25,7 +25,10 @@ module Projects
tag_names = tags.map(&:name)
Projects::ContainerRepository::DeleteTagsService
.new(container_repository.project, current_user, tags: tag_names)
.new(container_repository.project,
current_user,
tags: tag_names,
container_expiration_policy: params['container_expiration_policy'])
.execute(container_repository)
end
......
......@@ -7,7 +7,10 @@ module Projects
def execute(container_repository)
@container_repository = container_repository
return error('access denied') unless can?(current_user, :destroy_container_image, project)
unless params[:container_expiration_policy]
return error('access denied') unless can?(current_user, :destroy_container_image, project)
end
@tag_names = params[:tags]
return error('not tags specified') if @tag_names.blank?
......
---
title: Fix bug to allow container cleanup policies to properly run
merge_request: 43359
author:
type: fixed
......@@ -245,7 +245,7 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
end
it 'succeeds without a user' do
expect_delete(%w(Bb Ba C))
expect_delete(%w(Bb Ba C), container_expiration_policy: true)
is_expected.to include(status: :success, deleted: %w(Bb Ba C))
end
......@@ -287,10 +287,10 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService do
end
end
def expect_delete(tags)
def expect_delete(tags, container_expiration_policy: nil)
expect(Projects::ContainerRepository::DeleteTagsService)
.to receive(:new)
.with(repository.project, user, tags: tags)
.with(repository.project, user, tags: tags, container_expiration_policy: container_expiration_policy)
.and_call_original
expect_any_instance_of(Projects::ContainerRepository::DeleteTagsService)
......
......@@ -85,6 +85,41 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
end
end
RSpec.shared_examples 'supporting fast delete' do
context 'when the registry supports fast delete' do
before do
allow(repository.client).to receive(:supports_tag_delete?).and_return(true)
end
it_behaves_like 'calling the correct delete tags service', ::Projects::ContainerRepository::Gitlab::DeleteTagsService
it_behaves_like 'handling invalid params'
context 'with the real service' do
before do
stub_delete_reference_requests(tags)
expect_delete_tag_by_names(tags)
end
it { is_expected.to include(status: :success) }
it_behaves_like 'logging a success response'
end
context 'with a timeout error' do
before do
expect_next_instance_of(::Projects::ContainerRepository::Gitlab::DeleteTagsService) do |delete_service|
expect(delete_service).to receive(:delete_tags).and_raise(::Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError)
end
end
it { is_expected.to include(status: :error, message: 'timeout while deleting tags') }
it_behaves_like 'logging an error response', message: 'timeout while deleting tags'
end
end
end
describe '#execute' do
let(:tags) { %w[A Ba] }
......@@ -103,47 +138,30 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do
project.add_developer(user)
end
context 'when the registry supports fast delete' do
it_behaves_like 'supporting fast delete'
context 'when the registry does not support fast delete' do
before do
allow(repository.client).to receive(:supports_tag_delete?).and_return(true)
allow(repository.client).to receive(:supports_tag_delete?).and_return(false)
end
it_behaves_like 'calling the correct delete tags service', ::Projects::ContainerRepository::Gitlab::DeleteTagsService
it_behaves_like 'calling the correct delete tags service', ::Projects::ContainerRepository::ThirdParty::DeleteTagsService
it_behaves_like 'handling invalid params'
end
end
context 'with the real service' do
before do
stub_delete_reference_requests(tags)
expect_delete_tag_by_names(tags)
end
it { is_expected.to include(status: :success) }
it_behaves_like 'logging a success response'
end
context 'with a timeout error' do
before do
expect_next_instance_of(::Projects::ContainerRepository::Gitlab::DeleteTagsService) do |delete_service|
expect(delete_service).to receive(:delete_tags).and_raise(::Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError)
end
end
it { is_expected.to include(status: :error, message: 'timeout while deleting tags') }
context 'without user' do
let_it_be(:user) { nil }
it_behaves_like 'logging an error response', message: 'timeout while deleting tags'
end
context 'when not run by a cleanup policy' do
it { is_expected.to include(status: :error) }
end
context 'when the registry does not support fast delete' do
before do
allow(repository.client).to receive(:supports_tag_delete?).and_return(false)
end
it_behaves_like 'calling the correct delete tags service', ::Projects::ContainerRepository::ThirdParty::DeleteTagsService
context 'when run by a cleanup policy' do
let(:params) { { tags: tags, container_expiration_policy: true } }
it_behaves_like 'handling invalid params'
it_behaves_like 'supporting fast delete'
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