Skip to content

G
gitlab-ce
Commit b7d55792 authored by Mayra Cabrera's avatar Mayra Cabrera
Browse files
Options

Merge branch '344216-remove-dp-ff' into 'master' 

Remove dependency_proxy_manifest_workhorse ff

See merge request gitlab-org/gitlab!74669
parents 5f0ec4df 73a85fd1
Show whitespace changes
Inline Side-by-side
Showing with 7 additions and 200 deletions
app/controllers/groups/dependency_proxy_for_containers_controller.rb
View file @ b7d55792
......@@ -19,7 +19,7 @@ class Groups::DependencyProxyForContainersController < ::Groups::DependencyProxy
feature_category :dependency_proxy
def manifest
result = DependencyProxy::FindOrCreateManifestService.new(group, image, tag, token).execute
result = DependencyProxy::FindCachedManifestService.new(group, image, tag, token).execute
if result[:status] == :success
if result[:manifest]
......
app/services/dependency_proxy/find_or_create_manifest_service.rb → app/services/dependency_proxy/find_cached_manifest_service.rb
View file @ b7d55792
# frozen_string_literal: true
module DependencyProxy
class FindOrCreateManifestService < DependencyProxy::BaseService
class FindCachedManifestService < DependencyProxy::BaseService
def initialize(group, image, tag, token)
@group = group
@image = image
......@@ -20,36 +20,13 @@ module DependencyProxy
return respond if cached_manifest_matches?(head_result)
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
rescue Timeout::Error, *Gitlab::HTTP::HTTP_ERRORS
respond
end
private
def pull_new_manifest
DependencyProxy::PullManifestService.new(@image, @tag, @token).execute_with_manifest do |new_manifest|
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
def cached_manifest_matches?(head_result)
return false if head_result[:status] == :error
......
app/services/dependency_proxy/pull_manifest_service.rb deleted 100644 → 0
View file @ 5f0ec4df
# frozen_string_literal: true
module DependencyProxy
class PullManifestService < DependencyProxy::BaseService
def initialize(image, tag, token)
@image = image
@tag = tag
@token = token
end
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(',')))
if response.success?
file = Tempfile.new
begin
file.write(response.body)
file.flush
yield(
success(
file: file,
digest: response.headers[DependencyProxy::Manifest::DIGEST_HEADER],
content_type: response.headers['content-type']
)
)
ensure
file.close
file.unlink
end
else
yield(error(response.body, response.code))
end
rescue Timeout::Error => exception
error(exception.message, 599)
end
private
def manifest_url
registry.manifest_url(@image, @tag)
end
end
end
config/feature_flags/development/dependency_proxy_manifest_workhorse.yml deleted 100644 → 0
View file @ 5f0ec4df
---
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: true
ee/spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
View file @ b7d55792
......@@ -92,7 +92,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
end
before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow_next_instance_of(DependencyProxy::FindCachedManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response)
end
allow_next_instance_of(DependencyProxy::HeadManifestService) do |instance|
......
spec/controllers/groups/dependency_proxy_for_containers_controller_spec.rb
View file @ b7d55792
......@@ -170,7 +170,7 @@ RSpec.describe Groups::DependencyProxyForContainersController do
let(:pull_response) { { status: :success, manifest: manifest, from_cache: false } }
before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow_next_instance_of(DependencyProxy::FindCachedManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response)
end
end
......
spec/requests/rack_attack_global_spec.rb
View file @ b7d55792
......@@ -520,7 +520,7 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
let(:head_response) { { status: :success } }
before do
allow_next_instance_of(DependencyProxy::FindOrCreateManifestService) do |instance|
allow_next_instance_of(DependencyProxy::FindCachedManifestService) do |instance|
allow(instance).to receive(:execute).and_return(pull_response)
end
allow_next_instance_of(DependencyProxy::HeadManifestService) do |instance|
......
spec/services/dependency_proxy/find_or_create_manifest_service_spec.rb → spec/services/dependency_proxy/find_cached_manifest_service_spec.rb
View file @ b7d55792
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe DependencyProxy::FindOrCreateManifestService do
RSpec.describe DependencyProxy::FindCachedManifestService do
include DependencyProxyHelpers
let_it_be(:image) { 'alpine' }
......@@ -49,14 +49,6 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
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'
end
end
context 'failed head request' do
......@@ -66,14 +58,6 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
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'
end
end
end
......@@ -105,20 +89,6 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
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
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
context 'when the cached manifest is expired' do
......@@ -129,14 +99,6 @@ RSpec.describe DependencyProxy::FindOrCreateManifestService do
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'
end
end
context 'failed connection' do
......
spec/services/dependency_proxy/pull_manifest_service_spec.rb deleted 100644 → 0
View file @ 5f0ec4df
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe DependencyProxy::PullManifestService do
include DependencyProxyHelpers
let(:image) { 'alpine' }
let(:tag) { '3.9' }
let(:token) { Digest::SHA256.hexdigest('123') }
let(:manifest) { { foo: 'bar' }.to_json }
let(:digest) { '12345' }
let(:content_type) { 'foo' }
let(:headers) do
{ DependencyProxy::Manifest::DIGEST_HEADER => digest, 'content-type' => content_type }
end
subject { described_class.new(image, tag, token).execute_with_manifest(&method(:check_response)) }
context 'remote request is successful' do
before do
stub_manifest_download(image, tag, headers: headers)
end
it 'successfully returns the manifest' do
def check_response(response)
response[:file].rewind
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
end
end
context 'remote request is not found' do
before do
stub_manifest_download(image, tag, status: 404, body: 'Not found')
end
it 'returns a 404 not found error' do
def check_response(response)
expect(response[:status]).to eq(:error)
expect(response[:http_status]).to eq(404)
expect(response[:message]).to eq('Not found')
end
subject
end
end
context 'net timeout exception' do
before do
manifest_link = DependencyProxy::Registry.manifest_url(image, tag)
stub_full_request(manifest_link).to_timeout
end
it 'returns a 599 error' do
def check_response(response)
expect(response[:status]).to eq(:error)
expect(response[:http_status]).to eq(599)
expect(response[:message]).to eq('execution expired')
end
subject
end
end
context 'no block is given' do
subject { described_class.new(image, tag, token).execute_with_manifest }
it { expect { subject }.to raise_error(ArgumentError, 'Block must be provided') }
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7