Commit e3581b40 authored by Matthias Käppler's avatar Matthias Käppler

Merge branch 'revert-pull-by-digest' into 'master'

Revert '290944-pull-by-digest' and cleanup bad records

See merge request gitlab-org/gitlab!53506
parents 41f7a751 b9124381
......@@ -16,12 +16,7 @@ class Groups::DependencyProxyForContainersController < Groups::ApplicationContro
result = DependencyProxy::FindOrCreateManifestService.new(group, image, tag, token).execute
if result[:status] == :success
response.headers['Docker-Content-Digest'] = result[:manifest].digest
response.headers['Content-Length'] = result[:manifest].size
response.headers['Docker-Distribution-Api-Version'] = DependencyProxy::DISTRIBUTION_API_VERSION
response.headers['Etag'] = "\"#{result[:manifest].digest}\""
send_upload(result[:manifest].file, send_params: { type: result[:manifest].content_type })
send_upload(result[:manifest].file)
else
render status: result[:http_status], json: result[:message]
end
......
# frozen_string_literal: true
module DependencyProxy
URL_SUFFIX = '/dependency_proxy/containers'
DISTRIBUTION_API_VERSION = 'registry/2.0'
def self.table_name_prefix
'dependency_proxy_'
......
......@@ -12,10 +12,5 @@ class DependencyProxy::Manifest < ApplicationRecord
mount_file_store_uploader DependencyProxy::FileUploader
def self.find_or_initialize_by_file_name_or_digest(file_name:, digest:)
result = find_by(file_name: file_name) || find_by(digest: digest)
return result if result
new(file_name: file_name, digest: digest)
end
scope :find_or_initialize_by_file_name, ->(file_name) { find_or_initialize_by(file_name: file_name) }
end
......@@ -13,7 +13,7 @@ module DependencyProxy
def execute
@manifest = @group.dependency_proxy_manifests
.find_or_initialize_by_file_name_or_digest(file_name: @file_name, digest: @tag)
.find_or_initialize_by_file_name(@file_name)
head_result = DependencyProxy::HeadManifestService.new(@image, @tag, @token).execute
......@@ -30,7 +30,6 @@ module DependencyProxy
def pull_new_manifest
DependencyProxy::PullManifestService.new(@image, @tag, @token).execute_with_manifest do |new_manifest|
@manifest.update!(
content_type: new_manifest[:content_type],
digest: new_manifest[:digest],
file: new_manifest[:file],
size: new_manifest[:file].size
......@@ -39,9 +38,7 @@ module DependencyProxy
end
def cached_manifest_matches?(head_result)
return false if head_result[:status] == :error
@manifest && @manifest.digest == head_result[:digest] && @manifest.content_type == head_result[:content_type]
@manifest && @manifest.digest == head_result[:digest]
end
def respond
......
......@@ -2,8 +2,6 @@
module DependencyProxy
class HeadManifestService < DependencyProxy::BaseService
ACCEPT_HEADERS = ::ContainerRegistry::Client::ACCEPTED_TYPES.join(',')
def initialize(image, tag, token)
@image = image
@tag = tag
......@@ -11,10 +9,10 @@ module DependencyProxy
end
def execute
response = Gitlab::HTTP.head(manifest_url, headers: auth_headers.merge(Accept: ACCEPT_HEADERS))
response = Gitlab::HTTP.head(manifest_url, headers: auth_headers)
if response.success?
success(digest: response.headers['docker-content-digest'], content_type: response.headers['content-type'])
success(digest: response.headers['docker-content-digest'])
else
error(response.body, response.code)
end
......
......@@ -11,7 +11,7 @@ module DependencyProxy
def execute_with_manifest
raise ArgumentError, 'Block must be provided' unless block_given?
response = Gitlab::HTTP.get(manifest_url, headers: auth_headers.merge(Accept: ::ContainerRegistry::Client::ACCEPTED_TYPES.join(',')))
response = Gitlab::HTTP.get(manifest_url, headers: auth_headers)
if response.success?
file = Tempfile.new
......@@ -20,7 +20,7 @@ module DependencyProxy
file.write(response)
file.flush
yield(success(file: file, digest: response.headers['docker-content-digest'], content_type: response.headers['content-type']))
yield(success(file: file, digest: response.headers['docker-content-digest']))
ensure
file.close
file.unlink
......
---
title: Pull-by-digest and Docker 20.x support for the Dependency Proxy
title: Add content_type column to dependency_proxy_manifests
merge_request: 52805
author:
type: changed
---
title: Remove dependency_proxy_manifests records with content_type to prevent Dependency
Proxy failures
merge_request: 53506
author:
type: fixed
# frozen_string_literal: true
class RemoveBadDependencyProxyManifests < ActiveRecord::Migration[6.0]
DOWNTIME = false
def up
# We run destroy on each record because we need the callback to remove
# the underlying files
DependencyProxy::Manifest.where.not(content_type: nil).destroy_all # rubocop:disable Cop/DestroyAll
end
def down
# no op
end
end
483d1b4a24086fa57efe7f3b3fa872cf793352f80aba5c25614f07eafa2d30c5
\ No newline at end of file
......@@ -7,10 +7,9 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Dependency Proxy
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/7934) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.11.
> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/273655) to [GitLab Free](https://about.gitlab.com/pricing/) in GitLab 13.6.
> - [Support for private groups](https://gitlab.com/gitlab-org/gitlab/-/issues/11582) in [GitLab Free](https://about.gitlab.com/pricing/) 13.7.
> - Anonymous access to images in public groups is no longer available starting in [GitLab Free](https://about.gitlab.com/pricing/) 13.7.
> - [Support for pull-by-digest and Docker version 20.x](https://gitlab.com/gitlab-org/gitlab/-/issues/290944) in [GitLab Free](https://about.gitlab.com/pricing/) 13.9.
> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/273655) to [GitLab Core](https://about.gitlab.com/pricing/) in GitLab 13.6.
> - [Support for private groups](https://gitlab.com/gitlab-org/gitlab/-/issues/11582) in [GitLab Core](https://about.gitlab.com/pricing/) 13.7.
> - Anonymous access to images in public groups is no longer available starting in [GitLab Core](https://about.gitlab.com/pricing/) 13.7.
The GitLab Dependency Proxy is a local proxy you can use for your frequently-accessed
upstream images.
......@@ -18,6 +17,11 @@ upstream images.
In the case of CI/CD, the Dependency Proxy receives a request and returns the
upstream image from a registry, acting as a pull-through cache.
NOTE:
The Dependency Proxy is not compatible with Docker version 20.x and later.
If you are using the Dependency Proxy, Docker version 19.x.x is recommended until
[issue #290944](https://gitlab.com/gitlab-org/gitlab/-/issues/290944) is resolved.
## Prerequisites
The Dependency Proxy must be [enabled by an administrator](../../../administration/packages/dependency_proxy.md).
......@@ -56,7 +60,7 @@ Prerequisites:
### Authenticate with the Dependency Proxy
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/11582) in [GitLab Free](https://about.gitlab.com/pricing/) 13.7.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/11582) in [GitLab Core](https://about.gitlab.com/pricing/) 13.7.
> - It's [deployed behind a feature flag](../../feature_flags.md), enabled by default.
> - It's enabled on GitLab.com.
> - It's recommended for production use.
......@@ -158,7 +162,7 @@ the [Dependency Proxy API](../../../api/dependency_proxy.md).
## Docker Hub rate limits and the Dependency Proxy
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/241639) in [GitLab Free](https://about.gitlab.com/pricing/) 13.7.
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/241639) in [GitLab Core](https://about.gitlab.com/pricing/) 13.7.
<i class="fa fa-youtube-play youtube" aria-hidden="true"></i>
Watch how to [use the Dependency Proxy to help avoid Docker Hub rate limits](https://youtu.be/Nc4nUo7Pq08).
......
......@@ -130,7 +130,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
}
end
it 'proxies status from the remote token request', :aggregate_failures do
it 'proxies status from the remote token request' do
subject
expect(response).to have_gitlab_http_status(:service_unavailable)
......@@ -147,7 +147,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
}
end
it 'proxies status from the remote manifest request', :aggregate_failures do
it 'proxies status from the remote manifest request' do
subject
expect(response).to have_gitlab_http_status(:bad_request)
......@@ -156,19 +156,15 @@ RSpec.describe Groups::DependencyProxyForContainersController do
end
it 'sends a file' do
expect(controller).to receive(:send_file).with(manifest.file.path, type: manifest.content_type)
expect(controller).to receive(:send_file).with(manifest.file.path, {})
subject
end
it 'returns Content-Disposition: attachment', :aggregate_failures do
it 'returns Content-Disposition: attachment' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response.headers['Docker-Content-Digest']).to eq(manifest.digest)
expect(response.headers['Content-Length']).to eq(manifest.size)
expect(response.headers['Docker-Distribution-Api-Version']).to eq(DependencyProxy::DISTRIBUTION_API_VERSION)
expect(response.headers['Etag']).to eq("\"#{manifest.digest}\"")
expect(response.headers['Content-Disposition']).to match(/^attachment/)
end
end
......@@ -211,7 +207,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
}
end
it 'proxies status from the remote blob request', :aggregate_failures do
it 'proxies status from the remote blob request' do
subject
expect(response).to have_gitlab_http_status(:bad_request)
......@@ -225,7 +221,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
subject
end
it 'returns Content-Disposition: attachment', :aggregate_failures do
it 'returns Content-Disposition: attachment' do
subject
expect(response).to have_gitlab_http_status(:ok)
......
......@@ -10,8 +10,7 @@ FactoryBot.define do
factory :dependency_proxy_manifest, class: 'DependencyProxy::Manifest' do
group
file { fixture_file_upload('spec/fixtures/dependency_proxy/manifest') }
digest { 'sha256:d0710affa17fad5f466a70159cc458227bd25d4afb39514ef662ead3e6c99515' }
digest { 'sha256:5ab5a6872b264fe4fd35d63991b9b7d8425f4bc79e7cf4d563c10956581170c9' }
file_name { 'alpine:latest.json' }
content_type { 'application/vnd.docker.distribution.manifest.v2+json' }
end
end
{
"schemaVersion": 2,
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"config": {
"mediaType": "application/vnd.docker.container.image.v1+json",
"size": 1472,
"digest": "sha256:7731472c3f2a25edbb9c085c78f42ec71259f2b83485aa60648276d408865839"
},
"layers": [
"schemaVersion": 1,
"name": "library/alpine",
"tag": "latest",
"architecture": "amd64",
"fsLayers": [
{
"mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
"size": 2810825,
"digest": "sha256:596ba82af5aaa3e2fd9d6f955b8b94f0744a2b60710e3c243ba3e4a467f051d1"
"blobSum": "sha256:a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4"
},
{
"blobSum": "sha256:188c0c94c7c576fff0792aca7ec73d67a2f7f4cb3a6e53a84559337260b36964"
}
],
"history": [
{
"v1Compatibility": "{\"architecture\":\"amd64\",\"config\":{\"Hostname\":\"\",\"Domainname\":\"\",\"User\":\"\",\"AttachStdin\":false,\"AttachStdout\":false,\"AttachStderr\":false,\"Tty\":false,\"OpenStdin\":false,\"StdinOnce\":false,\"Env\":[\"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"],\"Cmd\":[\"/bin/sh\"],\"ArgsEscaped\":true,\"Image\":\"sha256:3543079adc6fb5170279692361be8b24e89ef1809a374c1b4429e1d560d1459c\",\"Volumes\":null,\"WorkingDir\":\"\",\"Entrypoint\":null,\"OnBuild\":null,\"Labels\":null},\"container\":\"8c59eb170e19b8c3768b8d06c91053b0debf4a6fa6a452df394145fe9b885ea5\",\"container_config\":{\"Hostname\":\"8c59eb170e19\",\"Domainname\":\"\",\"User\":\"\",\"AttachStdin\":false,\"AttachStdout\":false,\"AttachStderr\":false,\"Tty\":false,\"OpenStdin\":false,\"StdinOnce\":false,\"Env\":[\"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"],\"Cmd\":[\"/bin/sh\",\"-c\",\"#(nop) \",\"CMD [\\\"/bin/sh\\\"]\"],\"ArgsEscaped\":true,\"Image\":\"sha256:3543079adc6fb5170279692361be8b24e89ef1809a374c1b4429e1d560d1459c\",\"Volumes\":null,\"WorkingDir\":\"\",\"Entrypoint\":null,\"OnBuild\":null,\"Labels\":{}},\"created\":\"2020-10-22T02:19:24.499382102Z\",\"docker_version\":\"18.09.7\",\"id\":\"c5f1aab5bb88eaf1aa62bea08ea6654547d43fd4d15b1a476c77e705dd5385ba\",\"os\":\"linux\",\"parent\":\"dc0b50cc52bc340d7848a62cfe8a756f4420592f4984f7a680ef8f9d258176ed\",\"throwaway\":true}"
},
{
"v1Compatibility": "{\"id\":\"dc0b50cc52bc340d7848a62cfe8a756f4420592f4984f7a680ef8f9d258176ed\",\"created\":\"2020-10-22T02:19:24.33416307Z\",\"container_config\":{\"Cmd\":[\"/bin/sh -c #(nop) ADD file:f17f65714f703db9012f00e5ec98d0b2541ff6147c2633f7ab9ba659d0c507f4 in / \"]}}"
}
],
"signatures": [
{
"header": {
"jwk": {
"crv": "P-256",
"kid": "XOTE:DZ4C:YBPJ:3O3L:YI4B:NYXU:T4VR:USH6:CXXN:SELU:CSCC:FVPE",
"kty": "EC",
"x": "cR1zye_3354mdbD7Dn-mtXNXvtPtmLlUVDa5vH6Lp74",
"y": "rldUXSllLit6_2BW6AV8aqkwWJXHoYPG9OwkIBouwxQ"
},
"alg": "ES256"
},
"signature": "DYB2iB-XKIisqp5Q0OXFOBIOlBOuRV7pnZuKy0cxVB2Qj1VFRhWX4Tq336y0VMWbF6ma1he5A1E_Vk4jazrJ9g",
"protected": "eyJmb3JtYXRMZW5ndGgiOjIxMzcsImZvcm1hdFRhaWwiOiJDbjAiLCJ0aW1lIjoiMjAyMC0xMS0yNFQyMjowMTo1MVoifQ"
}
]
}
\ No newline at end of file
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20210205174154_remove_bad_dependency_proxy_manifests.rb')
RSpec.describe RemoveBadDependencyProxyManifests, schema: 20210128140157 do
let_it_be(:namespaces) { table(:namespaces) }
let_it_be(:dependency_proxy_manifests) { table(:dependency_proxy_manifests) }
let_it_be(:group) { namespaces.create!(type: 'Group', name: 'test', path: 'test') }
let_it_be(:dependency_proxy_manifest_with_content_type) do
dependency_proxy_manifests.create!(group_id: group.id, file: 'foo', file_name: 'foo', digest: 'asdf1234', content_type: 'content-type' )
end
let_it_be(:dependency_proxy_manifest_without_content_type) do
dependency_proxy_manifests.create!(group_id: group.id, file: 'bar', file_name: 'bar', digest: 'fdsa6789')
end
it 'removes the dependency_proxy_manifests with a content_type', :aggregate_failures do
expect(dependency_proxy_manifest_with_content_type).to be_present
expect(dependency_proxy_manifest_without_content_type).to be_present
expect { migrate! }.to change { dependency_proxy_manifests.count }.from(2).to(1)
expect(dependency_proxy_manifests.where.not(content_type: nil)).to be_empty
expect(dependency_proxy_manifest_without_content_type.reload).to be_present
end
end
......@@ -29,32 +29,24 @@ RSpec.describe DependencyProxy::Manifest, type: :model do
end
end
describe '.find_or_initialize_by_file_name_or_digest' do
let_it_be(:file_name) { 'foo' }
let_it_be(:digest) { 'bar' }
subject { DependencyProxy::Manifest.find_or_initialize_by_file_name_or_digest(file_name: file_name, digest: digest) }
describe '.find_or_initialize_by_file_name' do
subject { DependencyProxy::Manifest.find_or_initialize_by_file_name(file_name) }
context 'no manifest exists' do
let_it_be(:file_name) { 'foo' }
it 'initializes a manifest' do
expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name, digest: digest)
expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name)
subject
end
end
context 'manifest exists and matches file_name' do
context 'manifest exists' do
let_it_be(:dependency_proxy_manifest) { create(:dependency_proxy_manifest) }
let_it_be(:file_name) { dependency_proxy_manifest.file_name }
it { is_expected.to eq(dependency_proxy_manifest) }
end
context 'manifest exists and matches digest' do
let_it_be(:dependency_proxy_manifest) { create(:dependency_proxy_manifest) }
let_it_be(:digest) { dependency_proxy_manifest.digest }
it { is_expected.to eq(dependency_proxy_manifest) }
end
end
end
......@@ -10,12 +10,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
let(:manifest) { dependency_proxy_manifest.file.read }
let(:group) { dependency_proxy_manifest.group }
let(:token) { Digest::SHA256.hexdigest('123') }
let(:headers) do
{
'docker-content-digest' => dependency_proxy_manifest.digest,
'content-type' => dependency_proxy_manifest.content_type
}
end
let(:headers) { { 'docker-content-digest' => dependency_proxy_manifest.digest } }
describe '#execute' do
subject { described_class.new(group, image, tag, token).execute }
......@@ -23,37 +18,22 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
context 'when no manifest exists' do
let_it_be(:image) { 'new-image' }
shared_examples 'downloading the manifest' do
it 'downloads manifest from remote registry if there is no cached one', :aggregate_failures do
expect { subject }.to change { group.dependency_proxy_manifests.count }.by(1)
expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
expect(subject[:manifest]).to be_persisted
end
end
context 'successful head request' do
before do
stub_manifest_head(image, tag, headers: headers)
stub_manifest_download(image, tag, headers: headers)
end
it_behaves_like 'downloading the manifest'
before do
stub_manifest_head(image, tag, digest: dependency_proxy_manifest.digest)
stub_manifest_download(image, tag, headers: headers)
end
context 'failed head request' do
before do
stub_manifest_head(image, tag, status: :error)
stub_manifest_download(image, tag, headers: headers)
end
it_behaves_like 'downloading the manifest'
it 'downloads manifest from remote registry if there is no cached one', :aggregate_failures do
expect { subject }.to change { group.dependency_proxy_manifests.count }.by(1)
expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
expect(subject[:manifest]).to be_persisted
end
end
context 'when manifest exists' do
before do
stub_manifest_head(image, tag, headers: headers)
stub_manifest_head(image, tag, digest: dependency_proxy_manifest.digest)
end
shared_examples 'using the cached manifest' do
......@@ -68,17 +48,15 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
context 'when digest is stale' do
let(:digest) { 'new-digest' }
let(:content_type) { 'new-content-type' }
before do
stub_manifest_head(image, tag, headers: { 'docker-content-digest' => digest, 'content-type' => content_type })
stub_manifest_download(image, tag, headers: { 'docker-content-digest' => digest, 'content-type' => content_type })
stub_manifest_head(image, tag, digest: digest)
stub_manifest_download(image, tag, headers: { 'docker-content-digest' => digest })
end
it 'downloads the new manifest and updates the existing record', :aggregate_failures do
expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to eq(dependency_proxy_manifest)
expect(subject[:manifest].content_type).to eq(content_type)
expect(subject[:manifest].digest).to eq(digest)
end
end
......
......@@ -8,19 +8,12 @@ RSpec.describe DependencyProxy::HeadManifestService do
let(:tag) { 'latest' }
let(:token) { Digest::SHA256.hexdigest('123') }
let(:digest) { '12345' }
let(:content_type) { 'foo' }
let(:headers) do
{
'docker-content-digest' => digest,
'content-type' => content_type
}
end
subject { described_class.new(image, tag, token).execute }
context 'remote request is successful' do
before do
stub_manifest_head(image, tag, headers: headers)
stub_manifest_head(image, tag, digest: digest)
end
it { expect(subject[:status]).to eq(:success) }
......
......@@ -9,10 +9,7 @@ RSpec.describe DependencyProxy::PullManifestService do
let(:token) { Digest::SHA256.hexdigest('123') }
let(:manifest) { { foo: 'bar' }.to_json }
let(:digest) { '12345' }
let(:content_type) { 'foo' }
let(:headers) do
{ 'docker-content-digest' => digest, 'content-type' => content_type }
end
let(:headers) { { 'docker-content-digest' => digest } }
subject { described_class.new(image, tag, token).execute_with_manifest(&method(:check_response)) }
......@@ -28,7 +25,6 @@ RSpec.describe DependencyProxy::PullManifestService do
expect(response[:status]).to eq(:success)
expect(response[:file].read).to eq(manifest)
expect(response[:digest]).to eq(digest)
expect(response[:content_type]).to eq(content_type)
end
subject
......
......@@ -18,11 +18,11 @@ module DependencyProxyHelpers
.to_return(status: status, body: body || manifest, headers: headers)
end
def stub_manifest_head(image, tag, status: 200, body: nil, headers: {})
def stub_manifest_head(image, tag, status: 200, body: nil, digest: '123456')
manifest_url = registry.manifest_url(image, tag)
stub_full_request(manifest_url, method: :head)
.to_return(status: status, body: body, headers: headers )
.to_return(status: status, body: body, headers: { 'docker-content-digest' => digest } )
end
def stub_blob_download(image, blob_sha, status = 200, body = '123456')
......
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