Commit 9fbe1fa5 authored by Mark Chao's avatar Mark Chao

Merge branch 'id-remove-dependency-proxy-ff' into 'master'

Remove dependency proxy feature flag

See merge request gitlab-org/gitlab!77477
parents 1c00984e 5237844f
......@@ -33,17 +33,15 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
end
def blob
return blob_via_workhorse if Feature.enabled?(:dependency_proxy_workhorse, group, default_enabled: :yaml)
result = DependencyProxy::FindOrCreateBlobService
.new(group, image, token, params[:sha]).execute
blob = @group.dependency_proxy_blobs.find_by_file_name(blob_file_name)
if result[:status] == :success
event_name = tracking_event_name(object_type: :blob, from_cache: result[:from_cache])
if blob.present?
event_name = tracking_event_name(object_type: :blob, from_cache: true)
track_package_event(event_name, :dependency_proxy, namespace: group, user: auth_user)
send_upload(result[:blob].file)
send_upload(blob.file)
else
head result[:http_status]
send_dependency(token_header, DependencyProxy::Registry.blob_url(image, params[:sha]), blob_file_name)
end
end
......@@ -99,19 +97,6 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
private
def blob_via_workhorse
blob = @group.dependency_proxy_blobs.find_by_file_name(blob_file_name)
if blob.present?
event_name = tracking_event_name(object_type: :blob, from_cache: true)
track_package_event(event_name, :dependency_proxy, namespace: group, user: auth_user)
send_upload(blob.file)
else
send_dependency(token_header, DependencyProxy::Registry.blob_url(image, params[:sha]), blob_file_name)
end
end
def send_manifest(manifest, from_cache:)
response.headers[DependencyProxy::Manifest::DIGEST_HEADER] = manifest.digest
response.headers['Content-Length'] = manifest.size
......
# frozen_string_literal: true
module DependencyProxy
class DownloadBlobService < DependencyProxy::BaseService
def initialize(image, blob_sha, token)
@image = image
@blob_sha = blob_sha
@token = token
@temp_file = Tempfile.new
end
def execute
File.open(@temp_file.path, "wb") do |file|
Gitlab::HTTP.get(blob_url, headers: auth_headers, stream_body: true) do |fragment|
if [301, 302, 307].include?(fragment.code)
# do nothing
elsif fragment.code == 200
file.write(fragment)
else
raise DownloadError.new('Non-success response code on downloading blob fragment', fragment.code)
end
end
end
success(file: @temp_file)
rescue DownloadError => exception
error(exception.message, exception.http_status)
rescue Timeout::Error => exception
error(exception.message, 599)
end
private
def blob_url
registry.blob_url(@image, @blob_sha)
end
end
end
# frozen_string_literal: true
module DependencyProxy
class FindOrCreateBlobService < DependencyProxy::BaseService
def initialize(group, image, token, blob_sha)
@group = group
@image = image
@token = token
@blob_sha = blob_sha
end
def execute
from_cache = true
file_name = @blob_sha.sub('sha256:', '') + '.gz'
blob = @group.dependency_proxy_blobs.active.find_or_build(file_name)
unless blob.persisted?
from_cache = false
result = DependencyProxy::DownloadBlobService
.new(@image, @blob_sha, @token).execute
if result[:status] == :error
log_failure(result)
return error('Failed to download the blob', result[:http_status])
end
blob.file = result[:file]
blob.size = result[:file].size
blob.save!
end
blob.read! if from_cache
success(blob: blob, from_cache: from_cache)
end
private
def log_failure(result)
log_error(
"Dependency proxy: Failed to download the blob." \
"Blob sha: #{@blob_sha}." \
"Error message: #{result[:message][0, 100]}" \
"HTTP status: #{result[:http_status]}"
)
end
end
end
---
name: dependency_proxy_workhorse
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68157
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339639
milestone: '14.3'
type: development
group: group::source code
default_enabled: true
......@@ -348,74 +348,6 @@ RSpec.describe Groups::DependencyProxyForContainersController do
it_behaves_like 'a successful blob pull'
end
end
context 'when dependency_proxy_workhorse disabled' do
let(:blob_response) { { status: :success, blob: blob, from_cache: false } }
before do
stub_feature_flags(dependency_proxy_workhorse: false)
allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance|
allow(instance).to receive(:execute).and_return(blob_response)
end
end
context 'remote blob request fails' do
let(:blob_response) do
{
status: :error,
http_status: 400,
message: ''
}
end
before do
group.add_guest(user)
end
it 'proxies status from the remote blob request', :aggregate_failures do
subject
expect(response).to have_gitlab_http_status(:bad_request)
expect(response.body).to be_empty
end
end
context 'a valid user' do
before do
group.add_guest(user)
end
it_behaves_like 'a successful blob pull'
it_behaves_like 'a package tracking event', described_class.name, 'pull_blob'
context 'with a cache entry' do
let(:blob_response) { { status: :success, blob: blob, from_cache: true } }
it_behaves_like 'returning response status', :success
it_behaves_like 'a package tracking event', described_class.name, 'pull_blob_from_cache'
end
end
context 'a valid deploy token' do
let_it_be(:user) { create(:deploy_token, :group, :dependency_proxy_scopes) }
let_it_be(:group_deploy_token) { create(:group_deploy_token, deploy_token: user, group: group) }
it_behaves_like 'a successful blob pull'
context 'pulling from a subgroup' do
let_it_be_with_reload(:parent_group) { create(:group) }
let_it_be_with_reload(:group) { create(:group, parent: parent_group) }
before do
parent_group.create_dependency_proxy_setting!(enabled: true)
group_deploy_token.update_column(:group_id, parent_group.id)
end
it_behaves_like 'a successful blob pull'
end
end
end
end
it_behaves_like 'not found when disabled'
......
......@@ -81,28 +81,11 @@ RSpec.describe 'Group Dependency Proxy for containers', :js do
let!(:dependency_proxy_blob) { create(:dependency_proxy_blob, group: group) }
it_behaves_like 'responds with the file'
context 'dependency_proxy_workhorse feature flag disabled' do
before do
stub_feature_flags({ dependency_proxy_workhorse: false })
end
it_behaves_like 'responds with the file'
end
end
end
context 'when the blob must be downloaded' do
it_behaves_like 'responds with the file'
it_behaves_like 'caches the file'
context 'dependency_proxy_workhorse feature flag disabled' do
before do
stub_feature_flags({ dependency_proxy_workhorse: false })
end
it_behaves_like 'responds with the file'
it_behaves_like 'caches the file'
end
end
end
......@@ -533,16 +533,10 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
context 'getting a blob' do
let_it_be(:blob) { create(:dependency_proxy_blob) }
let_it_be(:other_blob) { create(:dependency_proxy_blob) }
let(:path) { "/v2/#{group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" }
let(:other_path) { "/v2/#{other_group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" }
let(:blob_response) { { status: :success, blob: blob, from_cache: false } }
before do
allow_next_instance_of(DependencyProxy::FindOrCreateBlobService) do |instance|
allow(instance).to receive(:execute).and_return(blob_response)
end
end
let(:path) { "/v2/#{blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" }
let(:other_path) { "/v2/#{other_blob.group.path}/dependency_proxy/containers/alpine/blobs/sha256:a0d0a0d46f8b52473982a3c466318f479767577551a53ffc9074c9fa7035982e" }
it_behaves_like 'rate-limited token-authenticated requests'
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe DependencyProxy::DownloadBlobService do
include DependencyProxyHelpers
let(:image) { 'alpine' }
let(:token) { Digest::SHA256.hexdigest('123') }
let(:blob_sha) { Digest::SHA256.hexdigest('ruby:2.7.0') }
subject(:download_blob) { described_class.new(image, blob_sha, token).execute }
context 'remote request is successful' do
before do
stub_blob_download(image, blob_sha)
end
it { expect(subject[:status]).to eq(:success) }
it { expect(subject[:file]).to be_a(Tempfile) }
it { expect(subject[:file].size).to eq(6) }
it 'streams the download' do
expected_options = { headers: anything, stream_body: true }
expect(Gitlab::HTTP).to receive(:perform_request).with(Net::HTTP::Get, anything, expected_options)
download_blob
end
it 'skips read_total_timeout', :aggregate_failures do
stub_const('GitLab::HTTP::DEFAULT_READ_TOTAL_TIMEOUT', 0)
expect(Gitlab::Metrics::System).not_to receive(:monotonic_time)
expect(download_blob).to include(status: :success)
end
end
context 'remote request is not found' do
before do
stub_blob_download(image, blob_sha, 404)
end
it { expect(subject[:status]).to eq(:error) }
it { expect(subject[:http_status]).to eq(404) }
it { expect(subject[:message]).to eq('Non-success response code on downloading blob fragment') }
end
context 'net timeout exception' do
before do
blob_url = DependencyProxy::Registry.blob_url(image, blob_sha)
stub_full_request(blob_url).to_timeout
end
it { expect(subject[:status]).to eq(:error) }
it { expect(subject[:http_status]).to eq(599) }
it { expect(subject[:message]).to eq('execution expired') }
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe DependencyProxy::FindOrCreateBlobService do
include DependencyProxyHelpers
let_it_be_with_reload(:blob) { create(:dependency_proxy_blob) }
let(:group) { blob.group }
let(:image) { 'alpine' }
let(:tag) { '3.9' }
let(:token) { Digest::SHA256.hexdigest('123') }
let(:blob_sha) { '40bd001563085fc35165329ea1ff5c5ecbdbbeef' }
subject { described_class.new(group, image, token, blob_sha).execute }
before do
stub_registry_auth(image, token)
end
shared_examples 'downloads the remote blob' do
it 'downloads blob from remote registry if there is no cached one' do
expect(subject[:status]).to eq(:success)
expect(subject[:blob]).to be_a(DependencyProxy::Blob)
expect(subject[:blob]).to be_persisted
expect(subject[:from_cache]).to eq false
end
end
context 'no cache' do
before do
stub_blob_download(image, blob_sha)
end
it_behaves_like 'downloads the remote blob'
end
context 'cached blob' do
let(:blob_sha) { blob.file_name.sub('.gz', '') }
it 'uses cached blob instead of downloading one' do
expect { subject }.to change { blob.reload.read_at }
expect(subject[:status]).to eq(:success)
expect(subject[:blob]).to be_a(DependencyProxy::Blob)
expect(subject[:blob]).to eq(blob)
expect(subject[:from_cache]).to eq true
end
context 'when the cached blob is expired' do
before do
blob.update_column(:status, DependencyProxy::Blob.statuses[:expired])
stub_blob_download(image, blob_sha)
end
it_behaves_like 'downloads the remote blob'
end
end
context 'no such blob exists remotely' do
before do
stub_blob_download(image, blob_sha, 404)
end
it 'returns error message and http status' do
expect(subject[:status]).to eq(:error)
expect(subject[:message]).to eq('Failed to download the blob')
expect(subject[:http_status]).to eq(404)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment