Commit a5618bba authored by Giorgenes Gelatti's avatar Giorgenes Gelatti Committed by Kamil Trzciński

Add support for updating manifests

- Adds Client#upload() to upload docker blobs
- Adds Client#generate_manifest() to generate random
blobs for manifests.
- Adds Client#generate_dummy_image() to generate a dummy image
- Adds Client#put_tag() to replace tag manifests
parent ab690af8
......@@ -19,14 +19,12 @@ module Projects
end
def destroy
if tag.delete
respond_to do |format|
format.json { head :no_content }
end
else
respond_to do |format|
format.json { head :bad_request }
end
result = Projects::ContainerRepository::DeleteTagsService
.new(image.project, current_user, tags: [params[:id]])
.execute(image)
respond_to do |format|
format.json { head(result[:status] == :success ? :ok : bad_request) }
end
end
......@@ -42,21 +40,12 @@ module Projects
return
end
@tags = tag_names.map { |tag_name| image.tag(tag_name) }
unless @tags.all? { |tag| tag.valid_name? }
head :bad_request
return
end
success_count = 0
@tags.each do |tag|
if tag.delete
success_count += 1
end
end
result = Projects::ContainerRepository::DeleteTagsService
.new(image.project, current_user, tags: tag_names)
.execute(image)
respond_to do |format|
format.json { head(success_count == @tags.size ? :no_content : :bad_request) }
format.json { head(result[:status] == :success ? :no_content : :bad_request) }
end
end
......@@ -70,10 +59,6 @@ module Projects
@image ||= project.container_repositories
.find(params[:repository_id])
end
def tag
@tag ||= image.tag(params[:id])
end
end
end
end
......@@ -40,7 +40,7 @@ module Projects
return unless tags.count == other_tags.count
# delete all tags
tags.map(&:delete)
tags.map(&:unsafe_delete)
end
def group_by_digest(tags)
......
# frozen_string_literal: true
module Projects
module ContainerRepository
class DeleteTagsService < BaseService
def execute(container_repository)
return error('access denied') unless can?(current_user, :destroy_container_image, project)
tag_names = params[:tags]
return error('not tags specified') if tag_names.blank?
if can_use?
smart_delete(container_repository, tag_names)
else
unsafe_delete(container_repository, tag_names)
end
end
private
def unsafe_delete(container_repository, tag_names)
deleted_tags = tag_names.select do |tag_name|
container_repository.tag(tag_name).unsafe_delete
end
return error('could not delete tags') if deleted_tags.empty?
success(deleted: deleted_tags)
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.
# See https://gitlab.com/gitlab-org/gitlab-ce/issues/21405 for a discussion
def smart_delete(container_repository, tag_names)
# generates the blobs for the dummy image
dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path)
# update the manifests of the tags with the new dummy image
tag_digests = tag_names.map do |name|
container_repository.client.put_tag(container_repository.path, name, dummy_manifest)
end
# make sure the digests are the same (it should always be)
tag_digests.uniq!
# rubocop: disable CodeReuse/ActiveRecord
Gitlab::Sentry.track_exception(ArgumentError.new('multiple tag digests')) if tag_digests.many?
# 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 container_repository.client.delete_repository_tag(container_repository.path, tag_digests.first)
success(deleted: tag_names)
else
error('could not delete tags')
end
end
def can_use?
Feature.enabled?(:container_registry_smart_delete, project, default_enabled: true)
end
end
end
end
---
title: 'Adds the ability to delete single tags from the docker registry. Fix the issue that caused all related tags and image to be deleted at the same time.'
merge_request: 16886
author:
type: fixed
......@@ -252,8 +252,8 @@ This action does not delete blobs. In order to delete them and recycle disk spac
[run the garbage collection](https://docs.gitlab.com/omnibus/maintenance/README.html#removing-unused-layers-not-referenced-by-manifests).
NOTE: **Note:**
Due to a [Docker Distribution deficiency](https://gitlab.com/gitlab-org/gitlab-foss/issues/21405),
it doesn't remove tags whose manifest is shared by multiple tags.
Since GitLab 12.4, individual tags are deleted.
For more details, see the [discussion](https://gitlab.com/gitlab-org/gitlab/issues/15737).
Examples:
......
......@@ -106,9 +106,15 @@ module API
authorize_destroy_container_image!
validate_tag!
tag.delete
status :ok
result = ::Projects::ContainerRepository::DeleteTagsService
.new(repository.project, current_user, tags: [declared_params[:tag_name]])
.execute(repository)
if result[:status] == :success
status :ok
else
status :bad_request
end
end
end
......
......@@ -2,6 +2,7 @@
require 'faraday'
require 'faraday_middleware'
require 'digest'
module ContainerRegistry
class Client
......@@ -9,6 +10,8 @@ module ContainerRegistry
DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE = 'application/vnd.docker.distribution.manifest.v2+json'
OCI_MANIFEST_V1_TYPE = 'application/vnd.oci.image.manifest.v1+json'
CONTAINER_IMAGE_V1_TYPE = 'application/vnd.docker.container.image.v1+json'
ACCEPTED_TYPES = [DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE, OCI_MANIFEST_V1_TYPE].freeze
# Taken from: FaradayMiddleware::FollowRedirects
......@@ -36,6 +39,45 @@ module ContainerRegistry
faraday.delete("/v2/#{name}/manifests/#{reference}").success?
end
def upload_raw_blob(path, blob)
digest = "sha256:#{Digest::SHA256.hexdigest(blob)}"
if upload_blob(path, blob, digest).success?
[blob, digest]
end
end
def upload_blob(name, content, digest)
upload = faraday.post("/v2/#{name}/blobs/uploads/")
return unless upload.success?
location = URI(upload.headers['location'])
faraday.put("#{location.path}?#{location.query}") do |req|
req.params['digest'] = digest
req.headers['Content-Type'] = 'application/octet-stream'
req.body = content
end
end
def generate_empty_manifest(path)
image = {
config: {}
}
image, image_digest = upload_raw_blob(path, JSON.pretty_generate(image))
return unless image
{
schemaVersion: 2,
mediaType: DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE,
config: {
mediaType: CONTAINER_IMAGE_V1_TYPE,
size: image.size,
digest: image_digest
}
}
end
def blob(name, digest, type = nil)
type ||= 'application/octet-stream'
response_body faraday_blob.get("/v2/#{name}/blobs/#{digest}", nil, 'Accept' => type), allow_redirect: true
......@@ -45,6 +87,15 @@ module ContainerRegistry
faraday.delete("/v2/#{name}/blobs/#{digest}").success?
end
def put_tag(name, reference, manifest)
response = faraday.put("/v2/#{name}/manifests/#{reference}") do |req|
req.headers['Content-Type'] = DOCKER_DISTRIBUTION_MANIFEST_V2_TYPE
req.body = JSON.pretty_generate(manifest)
end
response.headers['docker-content-digest'] if response.success?
end
private
def initialize_connection(conn, options)
......
......@@ -98,6 +98,10 @@ module ContainerRegistry
end
end
def put(digests)
repository.client.put_tag(repository.path, name, digests)
end
# rubocop: disable CodeReuse/ActiveRecord
def total_size
return unless layers
......@@ -106,7 +110,10 @@ module ContainerRegistry
end
# rubocop: enable CodeReuse/ActiveRecord
def delete
# Deletes the image associated with this tag
# Note this will delete the image and all tags associated with it.
# Consider using DeleteTagsService instead.
def unsafe_delete
return unless digest
client.delete_repository_tag(repository.path, digest)
......
......@@ -10,6 +10,8 @@ describe Projects::Registry::TagsController do
create(:container_repository, name: 'image', project: project)
end
let(:service) { double('service') }
before do
sign_in(user)
stub_container_registry_config(enabled: true)
......@@ -84,17 +86,17 @@ describe Projects::Registry::TagsController do
context 'when there is matching tag present' do
before do
stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.])
stub_container_registry_tags(repository: repository.path, tags: %w[rc1], with_manifest: true)
end
it 'makes it possible to delete regular tag' do
expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete)
expect_delete_tags(%w[rc1])
destroy_tag('rc1')
end
it 'makes it possible to delete a tag that ends with a dot' do
expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete)
expect_delete_tags(%w[test.])
destroy_tag('test.')
end
......@@ -125,11 +127,12 @@ describe Projects::Registry::TagsController do
stub_container_registry_tags(repository: repository.path, tags: %w[rc1 test.])
end
let(:tags) { %w[tc1 test.] }
it 'makes it possible to delete tags in bulk' do
allow_any_instance_of(ContainerRegistry::Tag).to receive(:delete) { |*args| ContainerRegistry::Tag.delete(*args) }
expect(ContainerRegistry::Tag).to receive(:delete).exactly(2).times
expect_delete_tags(tags)
bulk_destroy_tags(['rc1', 'test.'])
bulk_destroy_tags(tags)
end
end
end
......@@ -146,4 +149,9 @@ describe Projects::Registry::TagsController do
format: :json
end
end
def expect_delete_tags(tags, status = :success)
expect(service).to receive(:execute).with(repository) { { status: status } }
expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(repository.project, user, tags: tags) { service }
end
end
......@@ -53,7 +53,9 @@ describe 'Container Registry', :js do
find('.js-toggle-repo').click
wait_for_requests
expect_any_instance_of(ContainerRegistry::Tag).to receive(:delete).and_return(true)
service = double('service')
expect(service).to receive(:execute).with(container_repository) { { status: :success } }
expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(container_repository.project, user, tags: ['latest']) { service }
click_on(class: 'js-delete-registry-row', visible: false)
expect(find('.modal .modal-title')).to have_content 'Remove image'
......
......@@ -73,4 +73,69 @@ describe ContainerRegistry::Client do
expect(response).to eq('Successfully redirected')
end
end
def stub_upload(path, content, digest, status = 200)
stub_request(:post, "http://container-registry/v2/#{path}/blobs/uploads/")
.to_return(status: status, body: "", headers: { 'location' => 'http://container-registry/next_upload?id=someid' })
stub_request(:put, "http://container-registry/next_upload?digest=#{digest}&id=someid")
.with(body: content)
.to_return(status: status, body: "", headers: {})
end
describe '#upload_blob' do
subject { client.upload_blob('path', 'content', 'sha256:123') }
context 'with successful uploads' do
it 'starts the upload and posts the blob' do
stub_upload('path', 'content', 'sha256:123')
expect(subject).to be_success
end
end
context 'with a failed upload' do
before do
stub_upload('path', 'content', 'sha256:123', 400)
end
it 'returns nil' do
expect(subject).to be nil
end
end
end
describe '#generate_empty_manifest' do
subject { client.generate_empty_manifest('path') }
let(:result_manifest) do
{
schemaVersion: 2,
mediaType: 'application/vnd.docker.distribution.manifest.v2+json',
config: {
mediaType: 'application/vnd.docker.container.image.v1+json',
size: 21,
digest: 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3'
}
}
end
it 'uploads a random image and returns the manifest' do
stub_upload('path', "{\n \"config\": {\n }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
expect(subject).to eq(result_manifest)
end
end
describe '#put_tag' do
subject { client.put_tag('path', 'tagA', { foo: :bar }) }
it 'uploads the manifest and returns the digest' do
stub_request(:put, "http://container-registry/v2/path/manifests/tagA")
.with(body: "{\n \"foo\": \"bar\"\n}")
.to_return(status: 200, body: "", headers: { 'docker-content-digest' => 'sha256:123' })
expect(subject).to eq 'sha256:123'
end
end
end
......@@ -179,7 +179,7 @@ describe ContainerRegistry::Tag do
end
end
describe '#delete' do
describe '#unsafe_delete' do
before do
stub_request(:delete, 'http://registry.gitlab/v2/group/test/manifests/sha256:digest')
.with(headers: headers)
......@@ -187,7 +187,7 @@ describe ContainerRegistry::Tag do
end
it 'correctly deletes the tag' do
expect(tag.delete).to be_truthy
expect(tag.unsafe_delete).to be_truthy
end
end
end
......
......@@ -150,7 +150,7 @@ describe API::ProjectContainerRepositories do
expect(response).to have_gitlab_http_status(:accepted)
end
context 'called multiple times in one hour' do
context 'called multiple times in one hour', :clean_gitlab_redis_shared_state do
it 'returns 400 with an error message' do
stub_exclusive_lease_taken(lease_key, timeout: 1.hour)
subject
......@@ -202,6 +202,8 @@ describe API::ProjectContainerRepositories do
end
describe 'DELETE /projects/:id/registry/repositories/:repository_id/tags/:tag_name' do
let(:service) { double('service') }
subject { delete api("/projects/#{project.id}/registry/repositories/#{root_repository.id}/tags/rootA", api_user) }
it_behaves_like 'rejected container repository access', :reporter, :forbidden
......@@ -210,18 +212,34 @@ describe API::ProjectContainerRepositories do
context 'for developer' do
let(:api_user) { developer }
before do
stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true)
context 'when there are multiple tags' do
before do
stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA rootB), with_manifest: true)
end
it 'properly removes tag' do
expect(service).to receive(:execute).with(root_repository) { { status: :success } }
expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(root_repository.project, api_user, tags: %w[rootA]) { service }
subject
expect(response).to have_gitlab_http_status(:ok)
end
end
it 'properly removes tag' do
expect_any_instance_of(ContainerRegistry::Client)
.to receive(:delete_repository_tag).with(root_repository.path,
'sha256:4c8e63ca4cb663ce6c688cb06f1c372b088dac5b6d7ad7d49cd620d85cf72a15')
context 'when there\'s only one tag' do
before do
stub_container_registry_tags(repository: root_repository.path, tags: %w(rootA), with_manifest: true)
end
subject
it 'properly removes tag' do
expect(service).to receive(:execute).with(root_repository) { { status: :success } }
expect(Projects::ContainerRepository::DeleteTagsService).to receive(:new).with(root_repository.project, api_user, tags: %w[rootA]) { service }
expect(response).to have_gitlab_http_status(:ok)
subject
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
describe Projects::ContainerRepository::DeleteTagsService do
set(:user) { create(:user) }
set(:project) { create(:project, :private) }
set(:repository) { create(:container_repository, :root, project: project) }
let(:params) { { tags: tags } }
let(:service) { described_class.new(project, user, params) }
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))
stub_tag_digest('latest', 'sha256:configA')
stub_tag_digest('A', 'sha256:configA')
stub_tag_digest('Ba', 'sha256:configB')
end
describe '#execute' do
let(:tags) { %w[A] }
subject { service.execute(repository) }
context 'without permissions' do
it { is_expected.to include(status: :error) }
end
context 'with permissions' do
before do
project.add_developer(user)
end
context 'when no params are specified' do
let(:params) { {} }
it 'does not remove anything' do
expect_any_instance_of(ContainerRegistry::Client).not_to receive(:delete_repository_tag)
is_expected.to include(status: :error)
end
end
context 'with empty tags' do
let(:tags) { [] }
it 'does not remove anything' do
expect_any_instance_of(ContainerRegistry::Client).not_to receive(:delete_repository_tag)
is_expected.to include(status: :error)
end
end
context 'with dummy tags disabled' do
let(:tags) { %w[A Ba] }
before do
stub_feature_flags(container_registry_smart_delete: false)
end
it 'deletes tags one by one' do
expect_delete_tag('sha256:configA')
expect_delete_tag('sha256:configB')
is_expected.to include(status: :success)
end
end
context 'with dummy tags enabled' do
let(:tags) { %w[A Ba] }
it 'deletes the tags using a dummy image' do
stub_upload("{\n \"config\": {\n }\n}", 'sha256:4435000728ee66e6a80e55637fc22725c256b61de344a2ecdeaac6bdb36e8bc3')
stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/A")
.to_return(status: 200, body: "", headers: { 'docker-content-digest' => 'sha256:dummy' })
stub_request(:put, "http://registry.gitlab/v2/#{repository.path}/manifests/Ba")
.to_return(status: 200, body: "", headers: { 'docker-content-digest' => 'sha256:dummy' })
expect_delete_tag('sha256:dummy')
is_expected.to include(status: :success)
end
end
end
end
private
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(content, digest)
expect_any_instance_of(ContainerRegistry::Client)
.to receive(:upload_blob)
.with(repository.path, content, digest) { double(success?: true ) }
end
def expect_delete_tag(digest)
expect_any_instance_of(ContainerRegistry::Client)
.to receive(:delete_repository_tag)
.with(repository.path, digest) { true }
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