Commit 0b143a90 authored by David Fernandez's avatar David Fernandez

Merge branch '335560-dp-manifest-workhorse-uploads' into 'master'

Dependency Proxy uses workhorse for manifest pulls

See merge request gitlab-org/gitlab!73033
parents fbd30643 0c11cd40
...@@ -11,8 +11,8 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy ...@@ -11,8 +11,8 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
before_action :ensure_token_granted!, only: [:blob, :manifest] before_action :ensure_token_granted!, only: [:blob, :manifest]
before_action :ensure_feature_enabled! before_action :ensure_feature_enabled!
before_action :verify_workhorse_api!, 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] skip_before_action :verify_authenticity_token, only: [:authorize_upload_blob, :upload_blob, :authorize_upload_manifest, :upload_manifest]
attr_reader :token attr_reader :token
...@@ -22,20 +22,11 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy ...@@ -22,20 +22,11 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
result = DependencyProxy::FindOrCreateManifestService.new(group, image, tag, token).execute result = DependencyProxy::FindOrCreateManifestService.new(group, image, tag, token).execute
if result[:status] == :success if result[:status] == :success
response.headers['Docker-Content-Digest'] = result[:manifest].digest if result[:manifest]
response.headers['Content-Length'] = result[:manifest].size send_manifest(result[:manifest], from_cache: result[:from_cache])
response.headers['Docker-Distribution-Api-Version'] = DependencyProxy::DISTRIBUTION_API_VERSION else
response.headers['Etag'] = "\"#{result[:manifest].digest}\"" send_dependency(manifest_header, DependencyProxy::Registry.manifest_url(image, tag), manifest_file_name)
content_type = result[:manifest].content_type end
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 }
)
else else
render status: result[:http_status], json: result[:message] render status: result[:http_status], json: result[:message]
end end
...@@ -59,7 +50,7 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy ...@@ -59,7 +50,7 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
def authorize_upload_blob def authorize_upload_blob
set_workhorse_internal_api_content_type 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 end
def upload_blob def upload_blob
...@@ -75,6 +66,27 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy ...@@ -75,6 +66,27 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
head :ok head :ok
end 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 private
def blob_via_workhorse def blob_via_workhorse
...@@ -86,14 +98,38 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy ...@@ -86,14 +98,38 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
send_upload(blob.file) send_upload(blob.file)
else 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
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 def blob_file_name
@blob_file_name ||= params[:sha].sub('sha256:', '') + '.gz' @blob_file_name ||= params[:sha].sub('sha256:', '') + '.gz'
end end
def manifest_file_name
@manifest_file_name ||= "#{image}:#{tag}.json"
end
def group def group
strong_memoize(:group) do strong_memoize(:group) do
Group.find_by_full_path(params[:group_id], follow_redirects: true) Group.find_by_full_path(params[:group_id], follow_redirects: true)
...@@ -137,4 +173,12 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy ...@@ -137,4 +173,12 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
render status: result[:http_status], json: result[:message] render status: result[:http_status], json: result[:message]
end end
end end
def token_header
{ Authorization: ["Bearer #{token}"] }
end
def manifest_header
token_header.merge(Accept: ::ContainerRegistry::Client::ACCEPTED_TYPES)
end
end end
...@@ -41,8 +41,8 @@ module WorkhorseHelper ...@@ -41,8 +41,8 @@ module WorkhorseHelper
head :ok head :ok
end end
def send_dependency(token, url, filename) def send_dependency(dependency_headers, url, filename)
headers.store(*Gitlab::Workhorse.send_dependency(token, url)) headers.store(*Gitlab::Workhorse.send_dependency(dependency_headers, url))
headers['Content-Disposition'] = headers['Content-Disposition'] =
ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: filename) ActionDispatch::Http::ContentDisposition.format(disposition: 'attachment', filename: filename)
headers['Content-Type'] = 'application/gzip' headers['Content-Type'] = 'application/gzip'
......
...@@ -7,6 +7,8 @@ class DependencyProxy::Blob < ApplicationRecord ...@@ -7,6 +7,8 @@ class DependencyProxy::Blob < ApplicationRecord
belongs_to :group belongs_to :group
MAX_FILE_SIZE = 5.gigabytes.freeze
validates :group, presence: true validates :group, presence: true
validates :file, presence: true validates :file, presence: true
validates :file_name, presence: true validates :file_name, presence: true
......
...@@ -7,6 +7,8 @@ class DependencyProxy::Manifest < ApplicationRecord ...@@ -7,6 +7,8 @@ class DependencyProxy::Manifest < ApplicationRecord
belongs_to :group belongs_to :group
MAX_FILE_SIZE = 10.megabytes.freeze
validates :group, presence: true validates :group, presence: true
validates :file, presence: true validates :file, presence: true
validates :file_name, presence: true validates :file_name, presence: true
...@@ -14,10 +16,7 @@ class DependencyProxy::Manifest < ApplicationRecord ...@@ -14,10 +16,7 @@ class DependencyProxy::Manifest < ApplicationRecord
mount_file_store_uploader DependencyProxy::FileUploader mount_file_store_uploader DependencyProxy::FileUploader
def self.find_or_initialize_by_file_name_or_digest(file_name:, digest:) def self.find_by_file_name_or_digest(file_name:, digest:)
result = find_by(file_name: file_name) || find_by(digest: digest) find_by(file_name: file_name) || find_by(digest: digest)
return result if result
new(file_name: file_name, digest: digest)
end end
end end
...@@ -14,18 +14,18 @@ module DependencyProxy ...@@ -14,18 +14,18 @@ module DependencyProxy
def execute def execute
@manifest = @group.dependency_proxy_manifests @manifest = @group.dependency_proxy_manifests
.active .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 head_result = DependencyProxy::HeadManifestService.new(@image, @tag, @token).execute
if cached_manifest_matches?(head_result) return respond if cached_manifest_matches?(head_result)
@manifest.touch
return success(manifest: @manifest, from_cache: true)
end
if Feature.enabled?(:dependency_proxy_manifest_workhorse, @group, default_enabled: :yaml)
success(manifest: nil, from_cache: false)
else
pull_new_manifest pull_new_manifest
respond(from_cache: false) respond(from_cache: false)
end
rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS
respond respond
end end
...@@ -34,12 +34,19 @@ module DependencyProxy ...@@ -34,12 +34,19 @@ module DependencyProxy
def pull_new_manifest def pull_new_manifest
DependencyProxy::PullManifestService.new(@image, @tag, @token).execute_with_manifest do |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], content_type: new_manifest[:content_type],
digest: new_manifest[:digest], digest: new_manifest[:digest],
file: new_manifest[:file], file: new_manifest[:file],
size: new_manifest[:file].size size: new_manifest[:file].size
) }
if @manifest
@manifest.update!(params)
else
@manifest = @group.dependency_proxy_manifests.create!(params)
end
end end
end end
...@@ -50,10 +57,7 @@ module DependencyProxy ...@@ -50,10 +57,7 @@ module DependencyProxy
end end
def respond(from_cache: true) def respond(from_cache: true)
if @manifest.persisted? if @manifest
# Technical debt: change to read_at https://gitlab.com/gitlab-org/gitlab/-/issues/341536
@manifest.touch if from_cache
success(manifest: @manifest, from_cache: from_cache) success(manifest: @manifest, from_cache: from_cache)
else else
error('Failed to download the manifest from the external registry', 503) error('Failed to download the manifest from the external registry', 503)
......
---
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
...@@ -155,5 +155,7 @@ scope format: false do ...@@ -155,5 +155,7 @@ scope format: false do
get 'v2/*group_id/dependency_proxy/containers/*image/blobs/:sha' => 'groups/dependency_proxy_for_containers#blob' # rubocop:todo Cop/PutGroupRoutesUnderScope 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/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/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
end end
...@@ -831,6 +831,17 @@ Set the limit to `0` to allow any file size. ...@@ -831,6 +831,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. 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 ## Branch retargeting on merge
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/320902) in GitLab 13.9. > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/320902) in GitLab 13.9.
......
...@@ -81,18 +81,23 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -81,18 +81,23 @@ RSpec.describe Groups::DependencyProxyForContainersController do
end end
describe 'GET #manifest' do 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(: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 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 end
before do before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance| allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response) allow(instance).to receive(:execute).and_return(pull_response)
end end
allow_next_instance_of(DependencyProxy::HeadManifestService) do |instance|
allow(instance).to receive(:execute).and_return(head_response)
end
end end
it_behaves_like 'when sso is enabled for the group', 'a successful manifest pull' it_behaves_like 'when sso is enabled for the group', 'a successful manifest pull'
......
...@@ -8,6 +8,7 @@ require 'uri' ...@@ -8,6 +8,7 @@ require 'uri'
module Gitlab module Gitlab
class Workhorse class Workhorse
SEND_DATA_HEADER = 'Gitlab-Workhorse-Send-Data' SEND_DATA_HEADER = 'Gitlab-Workhorse-Send-Data'
SEND_DEPENDENCY_CONTENT_TYPE_HEADER = 'Workhorse-Proxy-Content-Type'
VERSION_FILE = 'GITLAB_WORKHORSE_VERSION' VERSION_FILE = 'GITLAB_WORKHORSE_VERSION'
INTERNAL_API_CONTENT_TYPE = 'application/vnd.gitlab-workhorse+json' INTERNAL_API_CONTENT_TYPE = 'application/vnd.gitlab-workhorse+json'
INTERNAL_API_REQUEST_HEADER = 'Gitlab-Workhorse-Api-Request' INTERNAL_API_REQUEST_HEADER = 'Gitlab-Workhorse-Api-Request'
...@@ -170,9 +171,9 @@ module Gitlab ...@@ -170,9 +171,9 @@ module Gitlab
] ]
end end
def send_dependency(token, url) def send_dependency(headers, url)
params = { params = {
'Header' => { Authorization: ["Bearer #{token}"] }, 'Header' => headers,
'Url' => url 'Url' => url
} }
......
...@@ -124,6 +124,34 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -124,6 +124,34 @@ RSpec.describe Groups::DependencyProxyForContainersController do
end end
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 before do
allow(Gitlab.config.dependency_proxy) allow(Gitlab.config.dependency_proxy)
.to receive(:enabled).and_return(true) .to receive(:enabled).and_return(true)
...@@ -136,9 +164,10 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -136,9 +164,10 @@ RSpec.describe Groups::DependencyProxyForContainersController do
end end
describe 'GET #manifest' do 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(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
let(:tag) { 'latest1' }
before do before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance| allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
...@@ -146,7 +175,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -146,7 +175,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
end end
end end
subject { get_manifest } subject { get_manifest(tag) }
context 'feature enabled' do context 'feature enabled' do
before do before do
...@@ -207,11 +236,26 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -207,11 +236,26 @@ RSpec.describe Groups::DependencyProxyForContainersController do
it_behaves_like 'a successful manifest pull' it_behaves_like 'a successful manifest pull'
it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest' it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest'
context 'with a cache entry' do context 'with workhorse response' do
let(:pull_response) { { status: :success, manifest: manifest, from_cache: true } } let(:pull_response) { { status: :success, manifest: nil, from_cache: false } }
it_behaves_like 'returning response status', :success it 'returns Workhorse send-dependency instructions', :aggregate_failures do
it_behaves_like 'a package tracking event', described_class.name, 'pull_manifest_from_cache' 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
end end
...@@ -237,8 +281,8 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -237,8 +281,8 @@ RSpec.describe Groups::DependencyProxyForContainersController do
it_behaves_like 'not found when disabled' it_behaves_like 'not found when disabled'
def get_manifest def get_manifest(tag)
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 end
end end
...@@ -383,53 +427,71 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -383,53 +427,71 @@ RSpec.describe Groups::DependencyProxyForContainersController do
describe 'GET #authorize_upload_blob' do describe 'GET #authorize_upload_blob' do
let(:blob_sha) { 'a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4' } 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) request.headers.merge!(workhorse_internal_api_request_header)
get :authorize_upload_blob, params: { group_id: group.to_param, image: 'alpine', sha: blob_sha } get :authorize_upload_blob, params: { group_id: group.to_param, image: 'alpine', sha: blob_sha }
end 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' it_behaves_like 'without permission'
context 'with a valid user' do context 'with a valid user' do
before do before do
group.add_guest(user) group.add_guest(user)
end
it 'sends Workhorse local file instructions', :aggregate_failures do expect_next_found_instance_of(Group) do |instance|
authorize_upload_blob expect(instance).to receive_message_chain(:dependency_proxy_blobs, :create!)
end
end
expect(response.headers['Content-Type']).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) it_behaves_like 'a package tracking event', described_class.name, 'pull_blob'
expect(json_response['TempPath']).to eq(DependencyProxy::FileUploader.workhorse_local_upload_path) end
expect(json_response['RemoteObject']).to be_nil
expect(json_response['MaximumSize']).to eq(5.gigabytes)
end end
it 'sends Workhorse remote object instructions', :aggregate_failures do describe 'GET #authorize_upload_manifest' do
stub_dependency_proxy_object_storage(direct_upload: true) let(:maximum_size) { DependencyProxy::Manifest::MAX_FILE_SIZE }
authorize_upload_blob subject do
request.headers.merge!(workhorse_internal_api_request_header)
expect(response.headers['Content-Type']).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) get :authorize_upload_manifest, params: { group_id: group.to_param, image: 'alpine', tag: 'latest' }
expect(json_response['TempPath']).to be_nil
expect(json_response['RemoteObject']).not_to be_nil
expect(json_response['MaximumSize']).to eq(5.gigabytes)
end
end end
it_behaves_like 'without permission'
it_behaves_like 'authorize action with permission'
end end
describe 'GET #upload_blob' do describe 'GET #upload_manifest' do
let(:blob_sha) { 'a3ed95caeb02ffe68cdd9fd84406680ae93d633cb16422d00e8a7c22955b46d4' } let(:file) { fixture_file_upload("spec/fixtures/dependency_proxy/manifest", 'application/json') }
let(:file) { fixture_file_upload("spec/fixtures/dependency_proxy/#{blob_sha}.gz", 'application/gzip') }
subject do subject do
request.headers.merge!(workhorse_internal_api_request_header) request.headers.merge!(workhorse_internal_api_request_header)
get :upload_blob, params: { get :upload_manifest, params: {
group_id: group.to_param, group_id: group.to_param,
image: 'alpine', image: 'alpine',
sha: blob_sha, tag: 'latest',
file: file file: file
} }
end end
...@@ -441,11 +503,11 @@ RSpec.describe Groups::DependencyProxyForContainersController do ...@@ -441,11 +503,11 @@ RSpec.describe Groups::DependencyProxyForContainersController do
group.add_guest(user) group.add_guest(user)
expect_next_found_instance_of(Group) do |instance| 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
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
end end
......
...@@ -512,6 +512,24 @@ RSpec.describe Gitlab::Workhorse do ...@@ -512,6 +512,24 @@ RSpec.describe Gitlab::Workhorse do
end end
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 describe '.send_git_snapshot' do
let(:url) { 'http://example.com' } let(:url) { 'http://example.com' }
......
...@@ -31,18 +31,14 @@ RSpec.describe DependencyProxy::Manifest, type: :model do ...@@ -31,18 +31,14 @@ RSpec.describe DependencyProxy::Manifest, type: :model do
end end
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(:file_name) { 'foo' }
let_it_be(:digest) { 'bar' } 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 context 'no manifest exists' do
it 'initializes a manifest' do it { is_expected.to be_nil }
expect(DependencyProxy::Manifest).to receive(:new).with(file_name: file_name, digest: digest)
subject
end
end end
context 'manifest exists and matches file_name' do context 'manifest exists and matches file_name' do
......
...@@ -517,11 +517,15 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac ...@@ -517,11 +517,15 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
let(:path) { "/v2/#{group.path}/dependency_proxy/containers/alpine/manifests/latest" } 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(:other_path) { "/v2/#{other_group.path}/dependency_proxy/containers/alpine/manifests/latest" }
let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } } let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
let(:head_response) { { status: :success } }
before do before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance| allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response) allow(instance).to receive(:execute).and_return(pull_response)
end end
allow_next_instance_of(DependencyProxy::HeadManifestService) do |instance|
allow(instance).to receive(:execute).and_return(head_response)
end
end end
it_behaves_like 'rate-limited token-authenticated requests' it_behaves_like 'rate-limited token-authenticated requests'
......
...@@ -85,6 +85,26 @@ RSpec.describe "Groups", "routing" do ...@@ -85,6 +85,26 @@ RSpec.describe "Groups", "routing" do
expect(get('/v2')).to route_to('groups/dependency_proxy_auth#authenticate') expect(get('/v2')).to route_to('groups/dependency_proxy_auth#authenticate')
end 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 context 'image name without namespace' do
it 'routes to #manifest' do it 'routes to #manifest' do
expect(get('/v2/gitlabhq/dependency_proxy/containers/ruby/manifests/2.3.6')) expect(get('/v2/gitlabhq/dependency_proxy/containers/ruby/manifests/2.3.6'))
......
...@@ -31,6 +31,14 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -31,6 +31,14 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
end end
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 context 'when no manifest exists' do
let_it_be(:image) { 'new-image' } let_it_be(:image) { 'new-image' }
...@@ -40,8 +48,16 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -40,8 +48,16 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
stub_manifest_download(image, tag, headers: headers) stub_manifest_download(image, tag, headers: headers)
end end
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' it_behaves_like 'downloading the manifest'
end end
end
context 'failed head request' do context 'failed head request' do
before do before do
...@@ -49,9 +65,17 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -49,9 +65,17 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
stub_manifest_download(image, tag, headers: headers) stub_manifest_download(image, tag, headers: headers)
end end
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' it_behaves_like 'downloading the manifest'
end end
end end
end
context 'when manifest exists' do context 'when manifest exists' do
before do before do
...@@ -60,7 +84,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -60,7 +84,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
shared_examples 'using the cached manifest' do shared_examples 'using the cached manifest' do
it 'uses cached manifest instead of downloading one', :aggregate_failures 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[:status]).to eq(:success)
expect(subject[:manifest]).to be_a(DependencyProxy::Manifest) expect(subject[:manifest]).to be_a(DependencyProxy::Manifest)
...@@ -80,6 +104,13 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -80,6 +104,13 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
stub_manifest_download(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 })
end end
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 it 'downloads the new manifest and updates the existing record', :aggregate_failures do
expect(subject[:status]).to eq(:success) expect(subject[:status]).to eq(:success)
expect(subject[:manifest]).to eq(dependency_proxy_manifest) expect(subject[:manifest]).to eq(dependency_proxy_manifest)
...@@ -88,6 +119,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -88,6 +119,7 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
expect(subject[:from_cache]).to eq false expect(subject[:from_cache]).to eq false
end end
end end
end
context 'when the cached manifest is expired' do context 'when the cached manifest is expired' do
before do before do
...@@ -96,8 +128,16 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do ...@@ -96,8 +128,16 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
stub_manifest_download(image, tag, headers: headers) stub_manifest_download(image, tag, headers: headers)
end end
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' it_behaves_like 'downloading the manifest'
end end
end
context 'failed connection' do context 'failed connection' do
before do before do
......
...@@ -75,6 +75,19 @@ func (p *Injector) Inject(w http.ResponseWriter, r *http.Request, sendData strin ...@@ -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)) helper.Fail500(w, r, fmt.Errorf("dependency proxy: failed to create request: %w", err))
} }
saveFileRequest.Header = helper.HeaderClone(r.Header) 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 saveFileRequest.ContentLength = dependencyResponse.ContentLength
nrw := &nullResponseWriter{header: make(http.Header)} nrw := &nullResponseWriter{header: make(http.Header)}
......
...@@ -113,8 +113,14 @@ func TestInject(t *testing.T) { ...@@ -113,8 +113,14 @@ func TestInject(t *testing.T) {
func TestSuccessfullRequest(t *testing.T) { func TestSuccessfullRequest(t *testing.T) {
content := []byte("result") content := []byte("result")
contentLength := strconv.Itoa(len(content)) contentLength := strconv.Itoa(len(content))
contentType := "foo"
dockerContentDigest := "sha256:asdf1234"
overriddenHeader := "originResourceServer"
originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { originResourceServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Length", contentLength) 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) w.Write(content)
})) }))
...@@ -131,12 +137,16 @@ func TestSuccessfullRequest(t *testing.T) { ...@@ -131,12 +137,16 @@ func TestSuccessfullRequest(t *testing.T) {
require.Equal(t, "/target/upload", uploadHandler.request.URL.Path) require.Equal(t, "/target/upload", uploadHandler.request.URL.Path)
require.Equal(t, int64(6), uploadHandler.request.ContentLength) 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, content, uploadHandler.body)
require.Equal(t, 200, response.Code) require.Equal(t, 200, response.Code)
require.Equal(t, string(content), response.Body.String()) require.Equal(t, string(content), response.Body.String())
require.Equal(t, contentLength, response.Header().Get("Content-Length")) require.Equal(t, contentLength, response.Header().Get("Content-Length"))
require.Equal(t, dockerContentDigest, response.Header().Get("Docker-Content-Digest"))
} }
func TestIncorrectSendData(t *testing.T) { func TestIncorrectSendData(t *testing.T) {
...@@ -177,6 +187,7 @@ func TestFailedOriginServer(t *testing.T) { ...@@ -177,6 +187,7 @@ func TestFailedOriginServer(t *testing.T) {
func makeRequest(injector *Injector, data string) *httptest.ResponseRecorder { func makeRequest(injector *Injector, data string) *httptest.ResponseRecorder {
w := httptest.NewRecorder() w := httptest.NewRecorder()
r := httptest.NewRequest("GET", "/target", nil) r := httptest.NewRequest("GET", "/target", nil)
r.Header.Set("Overridden-Header", "request")
sendData := base64.StdEncoding.EncodeToString([]byte(data)) sendData := base64.StdEncoding.EncodeToString([]byte(data))
injector.Inject(w, r, sendData) injector.Inject(w, r, sendData)
......
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