diff --git a/app/controllers/groups/dependency_proxy_for_containers_controller.rb b/app/controllers/groups/dependency_proxy_for_containers_controller.rb
index ef52800a2034ee709a2b84578d62d973428cf187..5e6195296f763dcec1bf994e37c92e772b4690cc 100644
--- a/app/controllers/groups/dependency_proxy_for_containers_controller.rb
+++ b/app/controllers/groups/dependency_proxy_for_containers_controller.rb
@@ -11,8 +11,8 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
before_action :ensure_token_granted!, only: [:blob, :manifest]
before_action :ensure_feature_enabled!
- before_action :verify_workhorse_api!, only: [:authorize_upload_blob, :upload_blob]
- skip_before_action :verify_authenticity_token, only: [:authorize_upload_blob, :upload_blob]
+ before_action :verify_workhorse_api!, only: [:authorize_upload_blob, :upload_blob, :authorize_upload_manifest, :upload_manifest]
+ skip_before_action :verify_authenticity_token, only: [:authorize_upload_blob, :upload_blob, :authorize_upload_manifest, :upload_manifest]
attr_reader :token
@@ -22,20 +22,11 @@ def manifest
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}\""
- content_type = result[:manifest].content_type
-
- event_name = tracking_event_name(object_type: :manifest, from_cache: result[:from_cache])
- track_package_event(event_name, :dependency_proxy, namespace: group, user: auth_user)
- send_upload(
- result[:manifest].file,
- proxy: true,
- redirect_params: { query: { 'response-content-type' => content_type } },
- send_params: { type: content_type }
- )
+ if result[:manifest]
+ send_manifest(result[:manifest], from_cache: result[:from_cache])
+ else
+ send_dependency(manifest_header, DependencyProxy::Registry.manifest_url(image, tag), manifest_file_name)
+ end
else
render status: result[:http_status], json: result[:message]
end
@@ -59,7 +50,7 @@ def blob
def authorize_upload_blob
set_workhorse_internal_api_content_type
- render json: DependencyProxy::FileUploader.workhorse_authorize(has_length: false, maximum_size: 5.gigabytes)
+ render json: DependencyProxy::FileUploader.workhorse_authorize(has_length: false, maximum_size: DependencyProxy::Blob::MAX_FILE_SIZE)
end
def upload_blob
@@ -75,6 +66,27 @@ def upload_blob
head :ok
end
+ def authorize_upload_manifest
+ set_workhorse_internal_api_content_type
+
+ render json: DependencyProxy::FileUploader.workhorse_authorize(has_length: false, maximum_size: DependencyProxy::Manifest::MAX_FILE_SIZE)
+ end
+
+ def upload_manifest
+ @group.dependency_proxy_manifests.create!(
+ file_name: manifest_file_name,
+ content_type: request.headers[Gitlab::Workhorse::SEND_DEPENDENCY_CONTENT_TYPE_HEADER],
+ digest: request.headers['Docker-Content-Digest'],
+ file: params[:file],
+ size: params[:file].size
+ )
+
+ event_name = tracking_event_name(object_type: :manifest, from_cache: false)
+ track_package_event(event_name, :dependency_proxy, namespace: group, user: auth_user)
+
+ head :ok
+ end
+
private
def blob_via_workhorse
@@ -86,14 +98,38 @@ def blob_via_workhorse
send_upload(blob.file)
else
- send_dependency(token, DependencyProxy::Registry.blob_url(image, params[:sha]), blob_file_name)
+ send_dependency(token_header, DependencyProxy::Registry.blob_url(image, params[:sha]), blob_file_name)
end
end
+ def send_manifest(manifest, from_cache:)
+ # Technical debt: change to read_at https://gitlab.com/gitlab-org/gitlab/-/issues/341536
+ manifest.touch
+ response.headers['Docker-Content-Digest'] = manifest.digest
+ response.headers['Content-Length'] = manifest.size
+ response.headers['Docker-Distribution-Api-Version'] = DependencyProxy::DISTRIBUTION_API_VERSION
+ response.headers['Etag'] = "\"#{manifest.digest}\""
+ content_type = manifest.content_type
+
+ event_name = tracking_event_name(object_type: :manifest, from_cache: from_cache)
+ track_package_event(event_name, :dependency_proxy, namespace: group, user: auth_user)
+
+ send_upload(
+ manifest.file,
+ proxy: true,
+ redirect_params: { query: { 'response-content-type' => content_type } },
+ send_params: { type: content_type }
+ )
+ end
+
def blob_file_name
@blob_file_name ||= params[:sha].sub('sha256:', '') + '.gz'
end
+ def manifest_file_name
+ @manifest_file_name ||= "#{image}:#{tag}.json"
+ end
+
def group
strong_memoize(:group) do
Group.find_by_full_path(params[:group_id], follow_redirects: true)
@@ -137,4 +173,12 @@ def ensure_token_granted!
render status: result[:http_status], json: result[:message]
end
end
+
+ def token_header
+ { Authorization: ["Bearer #{token}"] }
+ end
+
+ def manifest_header
+ token_header.merge(Accept: ::ContainerRegistry::Client::ACCEPTED_TYPES)
+ end
end
diff --git a/app/helpers/workhorse_helper.rb b/app/helpers/workhorse_helper.rb
index 4862282bc735432b4071499562a23171d395d9ea..2460c956bb611c54437649362eb6bd23bd9c803a 100644
--- a/app/helpers/workhorse_helper.rb
+++ b/app/helpers/workhorse_helper.rb
@@ -41,8 +41,8 @@ def send_artifacts_entry(file, entry)
head :ok
end
- def send_dependency(token, url, filename)
- headers.store(*Gitlab::Workhorse.send_dependency(token, url))
+ def send_dependency(dependency_headers, url, filename)
+ headers.store(*Gitlab::Workhorse.send_dependency(dependency_headers, url))
headers['Content-Disposition'] =
ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: filename)
headers['Content-Type'] = 'application/gzip'
diff --git a/app/models/dependency_proxy/blob.rb b/app/models/dependency_proxy/blob.rb
index 7ca15652586e06e9ba11f0506e744e45510ed176..bd5c022e6923e14cc75f8f34dbfd694e82a33cd9 100644
--- a/app/models/dependency_proxy/blob.rb
+++ b/app/models/dependency_proxy/blob.rb
@@ -7,6 +7,8 @@ class DependencyProxy::Blob < ApplicationRecord
belongs_to :group
+ MAX_FILE_SIZE = 5.gigabytes.freeze
+
validates :group, presence: true
validates :file, presence: true
validates :file_name, presence: true
diff --git a/app/models/dependency_proxy/manifest.rb b/app/models/dependency_proxy/manifest.rb
index b83047efe54b6c50b6f239e59d50c8a473635200..6a5ccd12cac51ffcafd3723dbc129572ba303af6 100644
--- a/app/models/dependency_proxy/manifest.rb
+++ b/app/models/dependency_proxy/manifest.rb
@@ -7,6 +7,8 @@ class DependencyProxy::Manifest < ApplicationRecord
belongs_to :group
+ MAX_FILE_SIZE = 10.megabytes.freeze
+
validates :group, presence: true
validates :file, presence: true
validates :file_name, presence: true
@@ -14,10 +16,7 @@ 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)
+ def self.find_by_file_name_or_digest(file_name:, digest:)
+ find_by(file_name: file_name) || find_by(digest: digest)
end
end
diff --git a/app/services/dependency_proxy/find_or_create_manifest_service.rb b/app/services/dependency_proxy/find_or_create_manifest_service.rb
index 1976d4d47f499265e5c4d8c970db93dd0ec95bb6..055980c431f85b9ba77d28c13644ac853d59be40 100644
--- a/app/services/dependency_proxy/find_or_create_manifest_service.rb
+++ b/app/services/dependency_proxy/find_or_create_manifest_service.rb
@@ -14,18 +14,18 @@ def initialize(group, image, tag, token)
def execute
@manifest = @group.dependency_proxy_manifests
.active
- .find_or_initialize_by_file_name_or_digest(file_name: @file_name, digest: @tag)
+ .find_by_file_name_or_digest(file_name: @file_name, digest: @tag)
head_result = DependencyProxy::HeadManifestService.new(@image, @tag, @token).execute
- if cached_manifest_matches?(head_result)
- @manifest.touch
+ return respond if cached_manifest_matches?(head_result)
- return success(manifest: @manifest, from_cache: true)
+ if Feature.enabled?(:dependency_proxy_manifest_workhorse, @group, default_enabled: :yaml)
+ success(manifest: nil, from_cache: false)
+ else
+ pull_new_manifest
+ respond(from_cache: false)
end
-
- pull_new_manifest
- respond(from_cache: false)
rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS
respond
end
@@ -34,12 +34,19 @@ def execute
def pull_new_manifest
DependencyProxy::PullManifestService.new(@image, @tag, @token).execute_with_manifest do |new_manifest|
- @manifest.update!(
+ params = {
+ file_name: @file_name,
content_type: new_manifest[:content_type],
digest: new_manifest[:digest],
file: new_manifest[:file],
size: new_manifest[:file].size
- )
+ }
+
+ if @manifest
+ @manifest.update!(params)
+ else
+ @manifest = @group.dependency_proxy_manifests.create!(params)
+ end
end
end
@@ -50,10 +57,7 @@ def cached_manifest_matches?(head_result)
end
def respond(from_cache: true)
- if @manifest.persisted?
- # Technical debt: change to read_at https://gitlab.com/gitlab-org/gitlab/-/issues/341536
- @manifest.touch if from_cache
-
+ if @manifest
success(manifest: @manifest, from_cache: from_cache)
else
error('Failed to download the manifest from the external registry', 503)
diff --git a/config/feature_flags/development/dependency_proxy_manifest_workhorse.yml b/config/feature_flags/development/dependency_proxy_manifest_workhorse.yml
new file mode 100644
index 0000000000000000000000000000000000000000..5ff6fc93e0a8865e399eae1c363fbe9c34a23400
--- /dev/null
+++ b/config/feature_flags/development/dependency_proxy_manifest_workhorse.yml
@@ -0,0 +1,8 @@
+---
+name: dependency_proxy_manifest_workhorse
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/73033
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344216
+milestone: '14.4'
+type: development
+group: group::package
+default_enabled: false
diff --git a/config/routes/group.rb b/config/routes/group.rb
index c093be6db4d9c735c79089f89a0acfa4c00ffad6..9a50d580747b9d4006111b091a5892d7c25cf8d2 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -155,5 +155,7 @@
get 'v2/*group_id/dependency_proxy/containers/*image/blobs/:sha' => 'groups/dependency_proxy_for_containers#blob' # rubocop:todo Cop/PutGroupRoutesUnderScope
post 'v2/*group_id/dependency_proxy/containers/*image/blobs/:sha/upload/authorize' => 'groups/dependency_proxy_for_containers#authorize_upload_blob' # rubocop:todo Cop/PutGroupRoutesUnderScope
post 'v2/*group_id/dependency_proxy/containers/*image/blobs/:sha/upload' => 'groups/dependency_proxy_for_containers#upload_blob' # rubocop:todo Cop/PutGroupRoutesUnderScope
+ post 'v2/*group_id/dependency_proxy/containers/*image/manifests/*tag/upload/authorize' => 'groups/dependency_proxy_for_containers#authorize_upload_manifest' # rubocop:todo Cop/PutGroupRoutesUnderScope
+ post 'v2/*group_id/dependency_proxy/containers/*image/manifests/*tag/upload' => 'groups/dependency_proxy_for_containers#upload_manifest' # rubocop:todo Cop/PutGroupRoutesUnderScope
end
end
diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md
index c7481abc84e6305c9722fe65c0200aa959cb1df8..7ca9e9d5110f6c13f45976cc8ab4dcfdbeb05c9d 100644
--- a/doc/administration/instance_limits.md
+++ b/doc/administration/instance_limits.md
@@ -828,6 +828,17 @@ Set the limit to `0` to allow any file size.
When asking for versions of a given NuGet package name, the GitLab Package Registry returns a maximum of 300 versions.
+## Dependency Proxy Limits
+
+> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/6396) in GitLab 14.5.
+
+The maximum file size for an image cached in the
+[Dependency Proxy](../user/packages/dependency_proxy/index.md)
+varies by file type:
+
+- Image blob: 5 GB
+- Image manifest: 10 MB
+
## Branch retargeting on merge
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/320902) in GitLab 13.9.
diff --git a/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
index c50e889755dcdc02267a749f8f5d80841d04a7ea..46a9b518090dc6f0e4b0c1fa400292ca183ea4bf 100644
--- a/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
+++ b/ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
@@ -81,18 +81,23 @@
end
describe 'GET #manifest' do
- let_it_be(:manifest) { create(:dependency_proxy_manifest) }
+ let_it_be(:manifest) { create(:dependency_proxy_manifest, group: group) }
let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
+ let(:head_response) { { status: :success, digest: manifest.digest, content_type: manifest.content_type } }
+ let(:tag) { manifest.file_name.sub('.json', '').split(':').last }
subject(:get_manifest) do
- get :manifest, params: { group_id: group.to_param, image: 'alpine', tag: '3.9.2' }
+ get :manifest, params: { group_id: group.to_param, image: 'alpine', tag: tag }
end
before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response)
end
+ allow_next_instance_of(DependencyProxy::HeadManifestService) do |instance|
+ allow(instance).to receive(:execute).and_return(head_response)
+ end
end
it_behaves_like 'when sso is enabled for the group', 'a successful manifest pull'
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index c40aa2273aae4f31aea72eba2f7a4ce8535bf4fb..3a905a2e1c5a2ec17673d94bacb33ed07b70d019 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -8,6 +8,7 @@
module Gitlab
class Workhorse
SEND_DATA_HEADER = 'Gitlab-Workhorse-Send-Data'
+ SEND_DEPENDENCY_CONTENT_TYPE_HEADER = 'Workhorse-Proxy-Content-Type'
VERSION_FILE = 'GITLAB_WORKHORSE_VERSION'
INTERNAL_API_CONTENT_TYPE = 'application/vnd.gitlab-workhorse+json'
INTERNAL_API_REQUEST_HEADER = 'Gitlab-Workhorse-Api-Request'
@@ -170,9 +171,9 @@ def send_scaled_image(location, width, content_type)
]
end
- def send_dependency(token, url)
+ def send_dependency(headers, url)
params = {
- 'Header' => { Authorization: ["Bearer #{token}"] },
+ 'Header' => headers,
'Url' => url
}
diff --git a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
index 5c8e080199b131e4a1dc4d3b0d84e187c6fab948..61f38e7e379eca8870d676f2e9284f5f6117201b 100644
--- a/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
+++ b/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
@@ -124,6 +124,34 @@
end
end
+ shared_examples 'authorize action with permission' do
+ context 'with a valid user' do
+ before do
+ group.add_guest(user)
+ end
+
+ it 'sends Workhorse local file instructions', :aggregate_failures do
+ subject
+
+ expect(response.headers['Content-Type']).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+ expect(json_response['TempPath']).to eq(DependencyProxy::FileUploader.workhorse_local_upload_path)
+ expect(json_response['RemoteObject']).to be_nil
+ expect(json_response['MaximumSize']).to eq(maximum_size)
+ end
+
+ it 'sends Workhorse remote object instructions', :aggregate_failures do
+ stub_dependency_proxy_object_storage(direct_upload: true)
+
+ subject
+
+ expect(response.headers['Content-Type']).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+ expect(json_response['TempPath']).to be_nil
+ expect(json_response['RemoteObject']).not_to be_nil
+ expect(json_response['MaximumSize']).to eq(maximum_size)
+ end
+ end
+ end
+
before do
allow(Gitlab.config.dependency_proxy)
.to receive(:enabled).and_return(true)
@@ -136,9 +164,10 @@
end
describe 'GET #manifest' do
- let_it_be(:manifest) { create(:dependency_proxy_manifest) }
+ let_it_be(:manifest) { create(:dependency_proxy_manifest, group: group) }
let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
+ let(:tag) { 'latest1' }
before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
@@ -146,7 +175,7 @@
end
end
- subject { get_manifest }
+ subject { get_manifest(tag) }
context 'feature enabled' do
before do
@@ -207,11 +236,26 @@
it_behaves_like 'a successful manifest pull'
it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest'
- context 'with a cache entry' do
- let(:pull_response) { { status: :success, manifest: manifest, from_cache: true } }
+ context 'with workhorse response' do
+ let(:pull_response) { { status: :success, manifest: nil, from_cache: false } }
- it_behaves_like 'returning response status', :success
- it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest_from_cache'
+ it 'returns Workhorse send-dependency instructions', :aggregate_failures do
+ subject
+
+ send_data_type, send_data = workhorse_send_data
+ header, url = send_data.values_at('Header', 'Url')
+
+ expect(send_data_type).to eq('send-dependency')
+ expect(header).to eq(
+ "Authorization" => ["Bearer abcd1234"],
+ "Accept" => ::ContainerRegistry::Client::ACCEPTED_TYPES
+ )
+ expect(url).to eq(DependencyProxy::Registry.manifest_url('alpine', tag))
+ expect(response.headers['Content-Type']).to eq('application/gzip')
+ expect(response.headers['Content-Disposition']).to eq(
+ ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: manifest.file_name)
+ )
+ end
end
end
@@ -237,8 +281,8 @@
it_behaves_like 'not found when disabled'
- def get_manifest
- get :manifest, params: { group_id: group.to_param, image: 'alpine', tag: '3.9.2' }
+ def get_manifest(tag)
+ get :manifest, params: { group_id: group.to_param, image: 'alpine', tag: tag }
end
end
@@ -383,53 +427,71 @@ def get_blob
describe 'GET #authorize_upload_blob' do
let(:blob_sha) { 'a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4' }
+ let(:maximum_size) { DependencyProxy::Blob::MAX_FILE_SIZE }
- subject(:authorize_upload_blob) do
+ subject do
request.headers.merge!(workhorse_internal_api_request_header)
get :authorize_upload_blob, params: { group_id: group.to_param, image: 'alpine', sha: blob_sha }
end
+ it_behaves_like 'without permission'
+ it_behaves_like 'authorize action with permission'
+ end
+
+ describe 'GET #upload_blob' do
+ let(:blob_sha) { 'a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4' }
+ let(:file) { fixture_file_upload("spec/fixtures/dependency_proxy/#{blob_sha}.gz", 'application/gzip') }
+
+ subject do
+ request.headers.merge!(workhorse_internal_api_request_header)
+
+ get :upload_blob, params: {
+ group_id: group.to_param,
+ image: 'alpine',
+ sha: blob_sha,
+ file: file
+ }
+ end
+
it_behaves_like 'without permission'
context 'with a valid user' do
before do
group.add_guest(user)
- end
- it 'sends Workhorse local file instructions', :aggregate_failures do
- authorize_upload_blob
-
- expect(response.headers['Content-Type']).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
- expect(json_response['TempPath']).to eq(DependencyProxy::FileUploader.workhorse_local_upload_path)
- expect(json_response['RemoteObject']).to be_nil
- expect(json_response['MaximumSize']).to eq(5.gigabytes)
+ expect_next_found_instance_of(Group) do |instance|
+ expect(instance).to receive_message_chain(:dependency_proxy_blobs, :create!)
+ end
end
- it 'sends Workhorse remote object instructions', :aggregate_failures do
- stub_dependency_proxy_object_storage(direct_upload: true)
+ it_behaves_like 'a package tracking event', described_class.name, 'pull_blob'
+ end
+ end
- authorize_upload_blob
+ describe 'GET #authorize_upload_manifest' do
+ let(:maximum_size) { DependencyProxy::Manifest::MAX_FILE_SIZE }
- expect(response.headers['Content-Type']).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
- expect(json_response['TempPath']).to be_nil
- expect(json_response['RemoteObject']).not_to be_nil
- expect(json_response['MaximumSize']).to eq(5.gigabytes)
- end
+ subject do
+ request.headers.merge!(workhorse_internal_api_request_header)
+
+ get :authorize_upload_manifest, params: { group_id: group.to_param, image: 'alpine', tag: 'latest' }
end
+
+ it_behaves_like 'without permission'
+ it_behaves_like 'authorize action with permission'
end
- describe 'GET #upload_blob' do
- let(:blob_sha) { 'a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4' }
- let(:file) { fixture_file_upload("spec/fixtures/dependency_proxy/#{blob_sha}.gz", 'application/gzip') }
+ describe 'GET #upload_manifest' do
+ let(:file) { fixture_file_upload("spec/fixtures/dependency_proxy/manifest", 'application/json') }
subject do
request.headers.merge!(workhorse_internal_api_request_header)
- get :upload_blob, params: {
+ get :upload_manifest, params: {
group_id: group.to_param,
image: 'alpine',
- sha: blob_sha,
+ tag: 'latest',
file: file
}
end
@@ -441,11 +503,11 @@ def get_blob
group.add_guest(user)
expect_next_found_instance_of(Group) do |instance|
- expect(instance).to receive_message_chain(:dependency_proxy_blobs, :create!)
+ expect(instance).to receive_message_chain(:dependency_proxy_manifests, :create!)
end
end
- it_behaves_like 'a package tracking event', described_class.name, 'pull_blob'
+ it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest'
end
end
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index 8ba56af561dc92d83f087fa2cf7b2e46b84c31dd..3bab9aec454810e9078ad4df88df9a57883808df 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -512,6 +512,24 @@ def call_verify(headers)
end
end
+ describe '.send_dependency' do
+ let(:headers) { { Accept: 'foo', Authorization: 'Bearer asdf1234' } }
+ let(:url) { 'https://foo.bar.com/baz' }
+
+ subject { described_class.send_dependency(headers, url) }
+
+ it 'sets the header correctly', :aggregate_failures do
+ key, command, params = decode_workhorse_header(subject)
+
+ expect(key).to eq("Gitlab-Workhorse-Send-Data")
+ expect(command).to eq("send-dependency")
+ expect(params).to eq({
+ 'Header' => headers,
+ 'Url' => url
+ }.deep_stringify_keys)
+ end
+ end
+
describe '.send_git_snapshot' do
let(:url) { 'http://example.com' }
diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb
index e7f0889345a3edba3289bd4b9f8c85785fb0f5b1..4629ff4b5b3743e6c3065335dcfb0a877a3dfd5b 100644
--- a/spec/models/dependency_proxy/manifest_spec.rb
+++ b/spec/models/dependency_proxy/manifest_spec.rb
@@ -31,18 +31,14 @@
end
end
- describe '.find_or_initialize_by_file_name_or_digest' do
+ describe '.find_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) }
+ subject { DependencyProxy::Manifest.find_by_file_name_or_digest(file_name: file_name, digest: digest) }
context 'no manifest exists' do
- it 'initializes a manifest' do
- expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name, digest: digest)
-
- subject
- end
+ it { is_expected.to be_nil }
end
context 'manifest exists and matches file_name' do
diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb
index 35ce942ed7e3362a97753a001c721090406d754c..ab0c76397e48623dd463bb040161685ffb6ff8b3 100644
--- a/spec/requests/rack_attack_global_spec.rb
+++ b/spec/requests/rack_attack_global_spec.rb
@@ -517,11 +517,15 @@ def do_request
let(:path) { "/v2/#{group.path}/dependency_proxy/containers/alpine/manifests/latest" }
let(:other_path) { "/v2/#{other_group.path}/dependency_proxy/containers/alpine/manifests/latest" }
let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
+ let(:head_response) { { status: :success } }
before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response)
end
+ allow_next_instance_of(DependencyProxy::HeadManifestService) do |instance|
+ allow(instance).to receive(:execute).and_return(head_response)
+ end
end
it_behaves_like 'rate-limited token-authenticated requests'
diff --git a/spec/routing/group_routing_spec.rb b/spec/routing/group_routing_spec.rb
index f171c2faf5e6820f72ef4a36a1137429f5b86a06..5c2ef62683eb754ff572f380446f13e6a478cb89 100644
--- a/spec/routing/group_routing_spec.rb
+++ b/spec/routing/group_routing_spec.rb
@@ -85,6 +85,26 @@
expect(get('/v2')).to route_to('groups/dependency_proxy_auth#authenticate')
end
+ it 'routes to #upload_manifest' do
+ expect(post('v2/gitlabhq/dependency_proxy/containers/alpine/manifests/latest/upload'))
+ .to route_to('groups/dependency_proxy_for_containers#upload_manifest', group_id: 'gitlabhq', image: 'alpine', tag: 'latest')
+ end
+
+ it 'routes to #upload_blob' do
+ expect(post('v2/gitlabhq/dependency_proxy/containers/alpine/blobs/abc12345/upload'))
+ .to route_to('groups/dependency_proxy_for_containers#upload_blob', group_id: 'gitlabhq', image: 'alpine', sha: 'abc12345')
+ end
+
+ it 'routes to #upload_manifest_authorize' do
+ expect(post('v2/gitlabhq/dependency_proxy/containers/alpine/manifests/latest/upload/authorize'))
+ .to route_to('groups/dependency_proxy_for_containers#authorize_upload_manifest', group_id: 'gitlabhq', image: 'alpine', tag: 'latest')
+ end
+
+ it 'routes to #upload_blob_authorize' do
+ expect(post('v2/gitlabhq/dependency_proxy/containers/alpine/blobs/abc12345/upload/authorize'))
+ .to route_to('groups/dependency_proxy_for_containers#authorize_upload_blob', group_id: 'gitlabhq', image: 'alpine', sha: 'abc12345')
+ end
+
context 'image name without namespace' do
it 'routes to #manifest' do
expect(get('/v2/gitlabhq/dependency_proxy/containers/ruby/manifests/2.3.6'))
diff --git a/spec/services/dependency_proxy/find_or_create_manifest_service_spec.rb b/spec/services/dependency_proxy/find_or_create_manifest_service_spec.rb
index b3f88f91289ae4a2fb1cb18bd05cef89d57dd31a..f2a605756fb03aab84e30010dc3c0437672cc876 100644
--- a/spec/services/dependency_proxy/find_or_create_manifest_service_spec.rb
+++ b/spec/services/dependency_proxy/find_or_create_manifest_service_spec.rb
@@ -31,6 +31,14 @@
end
end
+ shared_examples 'returning no manifest' do
+ it 'returns a nil manifest' do
+ expect(subject[:status]).to eq(:success)
+ expect(subject[:from_cache]).to eq false
+ expect(subject[:manifest]).to be_nil
+ end
+ end
+
context 'when no manifest exists' do
let_it_be(:image) { 'new-image' }
@@ -40,7 +48,15 @@
stub_manifest_download(image, tag, headers: headers)
end
- it_behaves_like 'downloading the manifest'
+ it_behaves_like 'returning no manifest'
+
+ context 'with dependency_proxy_manifest_workhorse feature disabled' do
+ before do
+ stub_feature_flags(dependency_proxy_manifest_workhorse: false)
+ end
+
+ it_behaves_like 'downloading the manifest'
+ end
end
context 'failed head request' do
@@ -49,7 +65,15 @@
stub_manifest_download(image, tag, headers: headers)
end
- it_behaves_like 'downloading the manifest'
+ it_behaves_like 'returning no manifest'
+
+ context 'with dependency_proxy_manifest_workhorse feature disabled' do
+ before do
+ stub_feature_flags(dependency_proxy_manifest_workhorse: false)
+ end
+
+ it_behaves_like 'downloading the manifest'
+ end
end
end
@@ -60,7 +84,7 @@
shared_examples 'using the cached manifest' do
it 'uses cached manifest instead of downloading one', :aggregate_failures do
- expect { subject }.to change { dependency_proxy_manifest.reload.updated_at }
+ subject
expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
@@ -80,12 +104,20 @@
stub_manifest_download(image, tag, headers: { 'docker-content-digest' => digest, 'content-type' => content_type })
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)
- expect(subject[:from_cache]).to eq false
+ it_behaves_like 'returning no manifest'
+
+ context 'with dependency_proxy_manifest_workhorse feature disabled' do
+ before do
+ stub_feature_flags(dependency_proxy_manifest_workhorse: false)
+ 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)
+ expect(subject[:from_cache]).to eq false
+ end
end
end
@@ -96,7 +128,15 @@
stub_manifest_download(image, tag, headers: headers)
end
- it_behaves_like 'downloading the manifest'
+ it_behaves_like 'returning no manifest'
+
+ context 'with dependency_proxy_manifest_workhorse feature disabled' do
+ before do
+ stub_feature_flags(dependency_proxy_manifest_workhorse: false)
+ end
+
+ it_behaves_like 'downloading the manifest'
+ end
end
context 'failed connection' do
diff --git a/workhorse/internal/dependencyproxy/dependencyproxy.go b/workhorse/internal/dependencyproxy/dependencyproxy.go
index 0bba2610d9ecf0f3caf76eaed98cbd431f5e90ea..90f3042a342c452fa7a3dbbcbf1fbe406103a025 100644
--- a/workhorse/internal/dependencyproxy/dependencyproxy.go
+++ b/workhorse/internal/dependencyproxy/dependencyproxy.go
@@ -75,6 +75,19 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin
helper.Fail500(w, r, fmt.Errorf("dependency proxy: failed to create request: %w", err))
}
saveFileRequest.Header = helper.HeaderClone(r.Header)
+
+ // forward headers from dependencyResponse to rails and client
+ for key, values := range dependencyResponse.Header {
+ saveFileRequest.Header.Del(key)
+ w.Header().Del(key)
+ for _, value := range values {
+ saveFileRequest.Header.Add(key, value)
+ w.Header().Add(key, value)
+ }
+ }
+
+ // workhorse hijack overwrites the Content-Type header, but we need this header value
+ saveFileRequest.Header.Set("Workhorse-Proxy-Content-Type", dependencyResponse.Header.Get("Content-Type"))
saveFileRequest.ContentLength = dependencyResponse.ContentLength
nrw := &nullResponseWriter{header: make(http.Header)}
diff --git a/workhorse/internal/dependencyproxy/dependencyproxy_test.go b/workhorse/internal/dependencyproxy/dependencyproxy_test.go
index 657ea388e18bdd0c129a01543ef74bf6da6a689d..d9169b2b4ce126788ea840dacee519be66b88e80 100644
--- a/workhorse/internal/dependencyproxy/dependencyproxy_test.go
+++ b/workhorse/internal/dependencyproxy/dependencyproxy_test.go
@@ -113,8 +113,14 @@ func TestInject(t *testing.T) {
func TestSuccessfullRequest(t *testing.T) {
content := []byte("result")
contentLength := strconv.Itoa(len(content))
+ contentType := "foo"
+ dockerContentDigest := "sha256:asdf1234"
+ overriddenHeader := "originResourceServer"
originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Length", contentLength)
+ w.Header().Set("Content-Type", contentType)
+ w.Header().Set("Docker-Content-Digest", dockerContentDigest)
+ w.Header().Set("Overridden-Header", overriddenHeader)
w.Write(content)
}))
@@ -131,12 +137,16 @@ func TestSuccessfullRequest(t *testing.T) {
require.Equal(t, "/target/upload", uploadHandler.request.URL.Path)
require.Equal(t, int64(6), uploadHandler.request.ContentLength)
+ require.Equal(t, contentType, uploadHandler.request.Header.Get("Workhorse-Proxy-Content-Type"))
+ require.Equal(t, dockerContentDigest, uploadHandler.request.Header.Get("Docker-Content-Digest"))
+ require.Equal(t, overriddenHeader, uploadHandler.request.Header.Get("Overridden-Header"))
require.Equal(t, content, uploadHandler.body)
require.Equal(t, 200, response.Code)
require.Equal(t, string(content), response.Body.String())
require.Equal(t, contentLength, response.Header().Get("Content-Length"))
+ require.Equal(t, dockerContentDigest, response.Header().Get("Docker-Content-Digest"))
}
func TestIncorrectSendData(t *testing.T) {
@@ -177,6 +187,7 @@ func TestFailedOriginServer(t *testing.T) {
func makeRequest(injector *Injector, data string) *httptest.ResponseRecorder {
w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/target", nil)
+ r.Header.Set("Overridden-Header", "request")
sendData := base64.StdEncoding.EncodeToString([]byte(data))
injector.Inject(w, r, sendData)