Fixes #37487 - Refactor proxy selection

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.
theforeman · May 20, 2024 · 9ddffca · 9ddffca
1 parent a7864ed
commit 9ddffca
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 57 deletions.
diff --git a/app/lib/proxy_api/available_proxy.rb b/app/lib/proxy_api/available_proxy.rb
diff --git a/app/lib/proxy_api/openscap.rb b/app/lib/proxy_api/openscap.rb
@@ -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

diff --git a/app/models/concerns/foreman_openscap/data_stream_content.rb b/app/models/concerns/foreman_openscap/data_stream_content.rb
@@ -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

diff --git a/app/validators/foreman_openscap/data_stream_validator.rb b/app/validators/foreman_openscap/data_stream_validator.rb
@@ -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

diff --git a/lib/foreman_openscap/data_migration.rb b/lib/foreman_openscap/data_migration.rb
@@ -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
@@ -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
 

diff --git a/test/unit/scap_content_test.rb b/test/unit/scap_content_test.rb
@@ -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.')