Commit b40de727 authored by David Fernandez's avatar David Fernandez Committed by Jarka Košanová

Split the DeleteTagsService into two smaller services

`Projects::ContainerRepository::DeleteTagsService` acts as a
facade service that will select the appropriate sub service
according to the type of container registry used.

`Projects::ContainerRepository::Gitlab::DeleteTagsService` is
used when the Gitlab Container Registry is running. It provides a
faster execution.

`Projects::ContainerRepository::ThirdParty::DeleteTagsService` is
used when a third party Container Registry is running. It provides
a slower execution time.
parent 8b56281c
...@@ -42,8 +42,8 @@ module Projects ...@@ -42,8 +42,8 @@ module Projects
# Technical Debt: https://gitlab.com/gitlab-org/gitlab/issues/207267 # Technical Debt: https://gitlab.com/gitlab-org/gitlab/issues/207267
# name_regex to be removed when container_expiration_policies is updated # name_regex to be removed when container_expiration_policies is updated
# to have both regex columns # to have both regex columns
regex_delete = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_delete'] || params['name_regex']}\\z") regex_delete = ::Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_delete'] || params['name_regex']}\\z")
regex_retain = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_keep']}\\z") regex_retain = ::Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_keep']}\\z")
tags.select do |tag| tags.select do |tag|
# regex_retain will override any overlapping matches by regex_delete # regex_retain will override any overlapping matches by regex_delete
...@@ -81,11 +81,11 @@ module Projects ...@@ -81,11 +81,11 @@ module Projects
def valid_regex? def valid_regex?
%w(name_regex_delete name_regex name_regex_keep).each do |param_name| %w(name_regex_delete name_regex name_regex_keep).each do |param_name|
regex = params[param_name] regex = params[param_name]
Gitlab::UntrustedRegexp.new(regex) unless regex.blank? ::Gitlab::UntrustedRegexp.new(regex) unless regex.blank?
end end
true true
rescue RegexpError => e rescue RegexpError => e
Gitlab::ErrorTracking.log_exception(e, project_id: project.id) ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id)
false false
end end
end end
......
...@@ -6,65 +6,35 @@ module Projects ...@@ -6,65 +6,35 @@ module Projects
LOG_DATA_BASE = { service_class: self.to_s }.freeze LOG_DATA_BASE = { service_class: self.to_s }.freeze
def execute(container_repository) def execute(container_repository)
@container_repository = container_repository
return error('access denied') unless can?(current_user, :destroy_container_image, project) return error('access denied') unless can?(current_user, :destroy_container_image, project)
tag_names = params[:tags] @tag_names = params[:tags]
return error('not tags specified') if tag_names.blank? return error('not tags specified') if @tag_names.blank?
smart_delete(container_repository, tag_names) delete_tags
end end
private private
# Delete tags by name with a single DELETE request. This is only supported def delete_tags
# by the GitLab Container Registry fork. See delete_service.execute
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23325 for details. .tap(&method(:log_response))
def fast_delete(container_repository, tag_names)
deleted_tags = tag_names.select do |name|
container_repository.delete_tag_by_name(name)
end
deleted_tags.any? ? success(deleted: deleted_tags) : error('could not delete tags')
end end
# Replace a tag on the registry with a dummy tag. def delete_service
# This is a hack as the registry doesn't support deleting individual fast_delete_enabled = Feature.enabled?(:container_registry_fast_tag_delete, default_enabled: true)
# tags. This code effectively pushes a dummy image and assigns the tag to it.
# This way when the tag is deleted only the dummy image is affected.
# This is used to preverse compatibility with third-party registries that
# don't support fast delete.
# See https://gitlab.com/gitlab-org/gitlab/issues/15737 for a discussion
def slow_delete(container_repository, tag_names)
# generates the blobs for the dummy image
dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path)
return error('could not generate manifest') if dummy_manifest.nil?
deleted_tags = replace_tag_manifests(container_repository, dummy_manifest, tag_names)
# Deletes the dummy image if fast_delete_enabled && @container_repository.client.supports_tag_delete?
# All created tag digests are the same since they all have the same dummy image. ::Projects::ContainerRepository::Gitlab::DeleteTagsService.new(@container_repository, @tag_names)
# a single delete is sufficient to remove all tags with it
if deleted_tags.any? && container_repository.delete_tag_by_digest(deleted_tags.each_value.first)
success(deleted: deleted_tags.keys)
else else
error('could not delete tags') ::Projects::ContainerRepository::ThirdParty::DeleteTagsService.new(@container_repository, @tag_names)
end end
end end
def smart_delete(container_repository, tag_names) def log_response(response)
fast_delete_enabled = Feature.enabled?(:container_registry_fast_tag_delete, default_enabled: true)
response = if fast_delete_enabled && container_repository.client.supports_tag_delete?
fast_delete(container_repository, tag_names)
else
slow_delete(container_repository, tag_names)
end
response.tap { |r| log_response(r, container_repository) }
end
def log_response(response, container_repository)
log_data = LOG_DATA_BASE.merge( log_data = LOG_DATA_BASE.merge(
container_repository_id: container_repository.id, container_repository_id: @container_repository.id,
message: 'deleted tags' message: 'deleted tags'
) )
...@@ -76,26 +46,6 @@ module Projects ...@@ -76,26 +46,6 @@ module Projects
log_error(log_data) log_error(log_data)
end end
end end
# update the manifests of the tags with the new dummy image
def replace_tag_manifests(container_repository, dummy_manifest, tag_names)
deleted_tags = {}
tag_names.each do |name|
digest = container_repository.client.put_tag(container_repository.path, name, dummy_manifest)
next unless digest
deleted_tags[name] = digest
end
# make sure the digests are the same (it should always be)
digests = deleted_tags.values.uniq
# rubocop: disable CodeReuse/ActiveRecord
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ArgumentError.new('multiple tag digests')) if digests.many?
deleted_tags
end
end end
end end
end end
# frozen_string_literal: true
module Projects
module ContainerRepository
module Gitlab
class DeleteTagsService
include BaseServiceUtility
def initialize(container_repository, tag_names)
@container_repository = container_repository
@tag_names = tag_names
end
# Delete tags by name with a single DELETE request. This is only supported
# by the GitLab Container Registry fork. See
# https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23325 for details.
def execute
return success(deleted: []) if @tag_names.empty?
deleted_tags = @tag_names.select do |name|
@container_repository.delete_tag_by_name(name)
end
deleted_tags.any? ? success(deleted: deleted_tags) : error('could not delete tags')
end
end
end
end
end
# frozen_string_literal: true
module Projects
module ContainerRepository
module ThirdParty
class DeleteTagsService
include BaseServiceUtility
def initialize(container_repository, tag_names)
@container_repository = container_repository
@tag_names = tag_names
end
# Replace a tag on the registry with a dummy tag.
# This is a hack as the registry doesn't support deleting individual
# tags. This code effectively pushes a dummy image and assigns the tag to it.
# This way when the tag is deleted only the dummy image is affected.
# This is used to preverse compatibility with third-party registries that
# don't support fast delete.
# See https://gitlab.com/gitlab-org/gitlab/issues/15737 for a discussion
def execute
return success(deleted: []) if @tag_names.empty?
# generates the blobs for the dummy image
dummy_manifest = @container_repository.client.generate_empty_manifest(@container_repository.path)
return error('could not generate manifest') if dummy_manifest.nil?
deleted_tags = replace_tag_manifests(dummy_manifest)
# Deletes the dummy image
# All created tag digests are the same since they all have the same dummy image.
# a single delete is sufficient to remove all tags with it
if deleted_tags.any? && @container_repository.delete_tag_by_digest(deleted_tags.each_value.first)
success(deleted: deleted_tags.keys)
else
error('could not delete tags')
end
end
private
# update the manifests of the tags with the new dummy image
def replace_tag_manifests(dummy_manifest)
deleted_tags = {}
@tag_names.each do |name|
digest = @container_repository.client.put_tag(@container_repository.path, name, dummy_manifest)
next unless digest
deleted_tags[name] = digest
end
# make sure the digests are the same (it should always be)
digests = deleted_tags.values.uniq
# rubocop: disable CodeReuse/ActiveRecord
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ArgumentError.new('multiple tag digests')) if digests.many?
deleted_tags
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do
include_context 'container repository delete tags service shared context'
let(:service) { described_class.new(repository, tags) }
describe '#execute' do
let(:tags) { %w[A Ba] }
subject { service.execute }
context 'with tags to delete' do
it 'deletes the tags by name' do
stub_delete_reference_requests(tags)
expect_delete_tag_by_names(tags)
is_expected.to eq(status: :success, deleted: tags)
end
it 'succeeds when tag delete returns 404' do
stub_delete_reference_requests('A' => 200, 'Ba' => 404)
is_expected.to eq(status: :success, deleted: tags)
end
it 'succeeds when a tag delete returns 500' do
stub_delete_reference_requests('A' => 200, 'Ba' => 500)
is_expected.to eq(status: :success, deleted: ['A'])
end
context 'with failures' do
context 'when the delete request fails' do
before do
stub_delete_reference_requests('A' => 500, 'Ba' => 500)
end
it { is_expected.to eq(status: :error, message: 'could not delete tags') }
end
end
end
context 'with empty tags' do
let_it_be(:tags) { [] }
it 'does not remove anything' do
expect_any_instance_of(ContainerRegistry::Client).not_to receive(:delete_repository_tag_by_name)
is_expected.to eq(status: :success, deleted: [])
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Projects::ContainerRepository::ThirdParty::DeleteTagsService do
include_context 'container repository delete tags service shared context'
let(:service) { described_class.new(repository, tags) }
describe '#execute' do
let(:tags) { %w[A Ba] }
subject { service.execute }
context 'with tags to delete' do
it 'deletes the tags by name' do
stub_upload('sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
tags.each { |tag| stub_put_manifest_request(tag) }
expect_delete_tag_by_digest('sha256:dummy')
is_expected.to eq(status: :success, deleted: tags)
end
it 'succeeds when tag delete returns 404' do
stub_upload('sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
stub_put_manifest_request('A')
stub_put_manifest_request('Ba')
stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/manifests/sha256:dummy")
.to_return(status: 404, body: '', headers: {})
is_expected.to eq(status: :success, deleted: tags)
end
context 'with failures' do
context 'when the dummy manifest generation fails' do
before do
stub_upload('sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3', success: false)
end
it { is_expected.to eq(status: :error, message: 'could not generate manifest') }
end
context 'when updating tags fails' do
before do
stub_upload('sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/manifests/sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3")
.to_return(status: 200, body: '', headers: {})
end
context 'all tag updates fail' do
before do
stub_put_manifest_request('A', 500, {})
stub_put_manifest_request('Ba', 500, {})
end
it { is_expected.to eq(status: :error, message: 'could not delete tags') }
end
context 'a single tag update fails' do
before do
stub_put_manifest_request('A')
stub_put_manifest_request('Ba', 500, {})
stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/manifests/sha256:dummy")
.to_return(status: 404, body: '', headers: {})
end
it { is_expected.to eq(status: :success, deleted: ['A']) }
end
end
end
end
context 'with empty tags' do
let_it_be(:tags) { [] }
it 'does not remove anything' do
expect_any_instance_of(ContainerRegistry::Client).not_to receive(:delete_repository_tag_by_name)
is_expected.to eq(status: :success, deleted: [])
end
end
end
end
# frozen_string_literal: true
RSpec.shared_context 'container repository delete tags service shared context' do
let_it_be(:user) { create(:user) }
let_it_be(:project, reload: true) { create(:project, :private) }
let_it_be(:repository) { create(:container_repository, :root, project: project) }
let(:params) { { tags: tags } }
before do
stub_container_registry_config(enabled: true,
api_url: 'http://registry.gitlab',
host_port: 'registry.gitlab')
stub_container_registry_tags(
repository: repository.path,
tags: %w(latest A Ba Bb C D E))
end
def stub_delete_reference_request(tag, status = 200)
stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/tags/reference/#{tag}")
.to_return(status: status, body: '')
end
def stub_delete_reference_requests(tags)
tags = Hash[Array.wrap(tags).map { |tag| [tag, 200] }] unless tags.is_a?(Hash)
tags.each do |tag, status|
stub_request(:delete, "http://registry.gitlab/v2/#{repository.path}/tags/reference/#{tag}")
.to_return(status: status, body: '')
end
end
def stub_put_manifest_request(tag, status = 200, headers = { 'docker-content-digest' => 'sha256:dummy' })
stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/#{tag}")
.to_return(status: status, body: '', headers: headers)
end
def stub_tag_digest(tag, digest)
stub_request(:head, "http://registry.gitlab/v2/#{repository.path}/manifests/#{tag}")
.to_return(status: 200, body: '', headers: { 'docker-content-digest' => digest })
end
def stub_digest_config(digest, created_at)
allow_any_instance_of(ContainerRegistry::Client)
.to receive(:blob)
.with(repository.path, digest, nil) do
{ 'created' => created_at.to_datetime.rfc3339 }.to_json if created_at
end
end
def stub_upload(digest, success: true)
content = "{\n \"config\": {\n }\n}"
expect_any_instance_of(ContainerRegistry::Client)
.to receive(:upload_blob)
.with(repository.path, content, digest) { double(success?: success ) }
end
def expect_delete_tag_by_digest(digest)
expect_any_instance_of(ContainerRegistry::Client)
.to receive(:delete_repository_tag_by_digest)
.with(repository.path, digest) { true }
expect_any_instance_of(ContainerRegistry::Client)
.not_to receive(:delete_repository_tag_by_name)
end
def expect_delete_tag_by_names(names)
Array.wrap(names).each do |name|
expect_any_instance_of(ContainerRegistry::Client)
.to receive(:delete_repository_tag_by_name)
.with(repository.path, name) { true }
expect_any_instance_of(ContainerRegistry::Client)
.not_to receive(:delete_repository_tag_by_digest)
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