Skip to content

Commit

Permalink
Fixes #37487 - Refactor proxy selection (#572)
Browse files Browse the repository at this point in the history
The primary reason for this is to avoid unnecessary proxy calls and
version comparisons. All currently supported versions of Foreman already
ship with smart_proxy_openscap >= 0.5 so we can skip the check altogether.

(cherry picked from commit be7e1f1)
  • Loading branch information
adamruzicka authored and ofedoren committed May 22, 2024
1 parent bc00792 commit 511a3ba
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 59 deletions.
44 changes: 0 additions & 44 deletions app/lib/proxy_api/available_proxy.rb

This file was deleted.

11 changes: 11 additions & 0 deletions app/lib/proxy_api/openscap.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
module ::ProxyAPI
class Openscap < ::ProxyAPI::Resource
HTTP_ERRORS = [
EOFError,
Errno::ECONNRESET,
Errno::EINVAL,
Net::HTTPBadResponse,
Net::HTTPHeaderSyntaxError,
Net::ProtocolError,
Timeout::Error,
ProxyAPI::ProxyException
].freeze

def initialize(args)
@url = args[:url] + '/compliance/'
super args
Expand Down
6 changes: 1 addition & 5 deletions app/models/concerns/foreman_openscap/data_stream_content.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,7 @@ module DataStreamContent
end

def proxy_url
@proxy_url ||= SmartProxy.with_features('Openscap').find do |proxy|
available = ProxyAPI::AvailableProxy.new(:url => proxy.url)
available.available?
end.try(:url)
@proxy_url
@proxy_url ||= SmartProxy.with_features('Openscap').find(&:ping)&.url
end

def create_profiles
Expand Down
2 changes: 1 addition & 1 deletion app/validators/foreman_openscap/data_stream_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def validate(data_stream_content)
errors['errors'].each { |error| data_stream_content.errors.add(:scap_file, _(error)) }
return false
end
rescue *ProxyAPI::AvailableProxy::HTTP_ERRORS => e
rescue *ProxyAPI::Openscap::HTTP_ERRORS => e
data_stream_content.errors.add(:base, _('No available proxy to validate. Returned with error: %s') % e)
return false
end
Expand Down
13 changes: 7 additions & 6 deletions lib/foreman_openscap/data_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
module ForemanOpenscap
class DataMigration
def initialize(proxy_id)
@proxy = ::SmartProxy.find(proxy_id)
puts "Found proxy #{@proxy.to_label}"
@url = @proxy.url
@proxy = ::SmartProxy.with_features('Openscap').where(id: proxy_id).first
if @proxy
puts "Found proxy #{@proxy.to_label}"
@url = @proxy.url
end
end

def available?
return false unless @proxy && @url
::ProxyAPI::AvailableProxy.new(:url => @url).available? && foreman_available?
@proxy&.ping && foreman_available?
end

def migrate
Expand Down Expand Up @@ -47,7 +48,7 @@ def foreman_available?
foreman_status_url = Setting[:foreman_url] + '/status'
response = JSON.parse(RestClient.get(foreman_status_url))
return true if response["status"] == "ok"
rescue *::ProxyAPI::AvailableProxy::HTTP_ERRORS
rescue *::ProxyAPI::Openscap::HTTP_ERRORS
return false
end

Expand Down
5 changes: 2 additions & 3 deletions test/unit/scap_content_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ class ScapContentTest < ActiveSupport::TestCase

test 'scap content should fail if no openscap proxy' do
SmartProxy.stubs(:with_features).returns([])
ProxyAPI::AvailableProxy.any_instance.stubs(:available?).returns(false)
scap_content = ForemanOpenscap::ScapContent.new(:title => 'Fedora', :scap_file => @scap_file)
refute(scap_content.save)
assert_includes(scap_content.errors.messages[:base], 'No proxy with OpenSCAP feature was found.')
Expand All @@ -26,8 +25,8 @@ class ScapContentTest < ActiveSupport::TestCase
test 'proxy_url should return the first available proxy it finds' do
available_proxy = SmartProxy.with_features('Openscap').first
unavailable_proxy = FactoryBot.create(:smart_proxy, :url => 'http://proxy.example.com:8443', :features => [FactoryBot.create(:feature, :name => 'Openscap')])
available_proxy.stubs(:proxy_url).returns(available_proxy.url)
unavailable_proxy.stubs(:proxy_url).returns(nil)
SmartProxy.expects(:with_features).with('Openscap').returns([unavailable_proxy, available_proxy])
SmartProxy.any_instance.expects(:ping).twice.returns(false).then.returns(true)
scap_content = ForemanOpenscap::ScapContent.new(:title => 'Fedora', :scap_file => @scap_file)
assert_equal(available_proxy.url, scap_content.proxy_url)
end
Expand Down

0 comments on commit 511a3ba

Please sign in to comment.