Fixes #35582 - properly send invalid ULN ACS remote errors

app/lib/actions/katello/alternate_content_source/alternate_content_source_common.rb

@@ -4,7 +4,7 @@ module AlternateContentSource
       module AlternateContentSourceCommon
         def create_simplified_acs(acs, smart_proxy)
           acs.products.each do |product|
-            product.repositories.has_url.library.with_type(acs.content_type).each do |repo|
+            product.acs_compatible_repositories.with_type(acs.content_type).each do |repo|
               smart_proxy_acs = ::Katello::SmartProxyAlternateContentSource.create(alternate_content_source_id: acs.id, smart_proxy_id: smart_proxy.id, repository_id: repo.id)
               plan_action(Pulp3::Orchestration::AlternateContentSource::Create, smart_proxy_acs)
             end

app/lib/actions/katello/alternate_content_source/update.rb

@@ -63,12 +63,13 @@ def update_acss(acs, smart_proxies_to_update, products_to_associate, products_to
               plan_action(Pulp3::Orchestration::AlternateContentSource::Update, smart_proxy_acs)
             elsif acs.simplified?
               products_to_associate.each do |product|
-                product.repositories.library.with_type(acs.content_type).each do |repo|
+                product.acs_compatible_repositories.with_type(acs.content_type).each do |repo|
                   smart_proxy_acs = ::Katello::SmartProxyAlternateContentSource.create(alternate_content_source_id: acs.id, smart_proxy_id: smart_proxy.id, repository_id: repo.id)
                   plan_action(Pulp3::Orchestration::AlternateContentSource::Create, smart_proxy_acs)
                 end
               end
               products_to_disassociate.each do |product|
+                # Don't use the acs_compatible_repositories filter here to ensure the proper repositories get disassociated
                 product.repositories.library.with_type(acs.content_type).each do |repo|
                   smart_proxy_acs = ::Katello::SmartProxyAlternateContentSource.find_by(alternate_content_source_id: acs.id, smart_proxy_id: smart_proxy.id, repository_id: repo.id)
                   plan_action(Pulp3::Orchestration::AlternateContentSource::Delete, smart_proxy_acs) if smart_proxy_acs.present?

app/lib/actions/katello/repository/create.rb

@@ -39,7 +39,7 @@ def plan(repository, args = {})
             unless root.is_container_push && repository.in_default_view?
               concurrence do
                 plan_self(:repository_id => repository.id, :clone => clone)
-                if !clone && repository.url.present?
+                if !clone && repository.url.present? && !repository.url.start_with?('uln')
                   repository.product.alternate_content_sources.with_type(repository.content_type).each do |acs|
                     acs.smart_proxies.each do |smart_proxy|
                       smart_proxy_acs = ::Katello::SmartProxyAlternateContentSource.create(alternate_content_source_id: acs.id, smart_proxy_id: smart_proxy.id, repository_id: repository.id)

app/lib/actions/katello/repository/update.rb

@@ -92,7 +92,7 @@ def handle_acs_product_removal(repository)
           # match the ACS content type and have a non-nil URL
           product = repository.product
           repo_content_types = Set.new
-          product.repositories.each do |test_repo|
+          product.acs_compatible_repositories.each do |test_repo|
             # we need to check id because test_repo will still contain the old, non-nil url
             repo_content_types.add(test_repo.content_type) if (repository.id != test_repo.id) && test_repo.url.present?
           end
@@ -110,11 +110,11 @@ def update_content?(repository)
         end
 
         def create_acs?(old_url, new_url)
-          old_url.nil? && new_url.present?
+          (old_url.nil? || old_url.start_with?('uln')) && new_url.present? && !new_url.start_with?('uln')
         end
 
         def delete_acs?(old_url, new_url)
-          old_url.present? && new_url.nil?
+          old_url.present? && (new_url.nil? || new_url.start_with?('uln'))
         end
       end
     end

app/lib/katello/validators/alternate_content_source_path_validator.rb

@@ -17,7 +17,7 @@ def validate_each(record, attribute, value)
       end
 
       def self.validate_base_url(base_url)
-        base_url =~ /\A#{URI::DEFAULT_PARSER.make_regexp}\z/
+        base_url =~ /\A(?!uln:\/\/)(#{URI::DEFAULT_PARSER.make_regexp})\z/
       end
 
       # Subpaths must have a slash at the end and none at the front: 'path/'

app/lib/katello/validators/alternate_content_source_products_validator.rb

@@ -5,7 +5,7 @@ def validate_each(record, attribute, value)
         if value && (attribute == :product_id)
           product = ::Katello::Product.find(value)
           content_type = record.alternate_content_source.content_type
-          if product.repositories.with_type(content_type).has_url.empty?
+          if product.acs_compatible_repositories.with_type(content_type).empty?
             record.errors.add(attribute, _("%{name} has no %{type} repositories with upstream URLs to add to the alternate content source.") % { name: product.name, type: content_type })
           end
         end

app/models/katello/product.rb

@@ -84,6 +84,10 @@ def self.enabled
 
     before_create :assign_unique_label
 
+    def acs_compatible_repositories
+      repositories.has_url.not_uln.library
+    end
+
     def orphaned?
       self.pool_products.empty?
     end

app/models/katello/repository.rb

@@ -121,6 +121,7 @@ class Repository < Katello::Model
     before_validation :set_container_repository_name, :unless => :skip_container_name?
 
     scope :has_url, -> { joins(:root).where.not("#{RootRepository.table_name}.url" => nil) }
+    scope :not_uln, -> { joins(:root).where("#{RootRepository.table_name}.url NOT LIKE 'uln%'") }
     scope :on_demand, -> { joins(:root).where("#{RootRepository.table_name}.download_policy" => ::Katello::RootRepository::DOWNLOAD_ON_DEMAND) }
     scope :immediate, -> { joins(:root).where("#{RootRepository.table_name}.download_policy" => ::Katello::RootRepository::DOWNLOAD_IMMEDIATE) }
     scope :non_immediate, -> { joins(:root).where.not("#{RootRepository.table_name}.download_policy" => ::Katello::RootRepository::DOWNLOAD_IMMEDIATE) }

test/actions/katello/repository_test.rb

@@ -75,6 +75,16 @@ class CreateTest < TestBase
       assert_action_planned action, acs_create_action_class
     end
 
+    it 'does not plan acs creation if ULN' do
+      simplified_acs = katello_alternate_content_sources(:yum_simplified_alternate_content_source)
+      simplified_acs.verify_ssl = nil
+      repository.root.update(url: 'uln://uln-repo.org')
+      simplified_acs.products << repository.product
+      ::Katello::SmartProxyAlternateContentSource.create(alternate_content_source_id: simplified_acs.id, smart_proxy_id: ::SmartProxy.pulp_primary.id, remote_href: 'remote_href', alternate_content_source_href: 'acs_href')
+      plan_action action, repository
+      refute_action_planned action, acs_create_action_class
+    end
+
     it 'does not plan acs creation if cloning' do
       plan_action action, repository, clone: true
       refute_action_planned action, acs_create_action_class
@@ -148,6 +158,15 @@ def setup
       assert_action_planned action, ::Actions::Pulp3::Orchestration::AlternateContentSource::Create
     end
 
+    it 'does not plan ACS creation when adding a ULN URL' do
+      action = create_action action_class
+      action.stubs(:action_subject).with(repository)
+
+      repository.root.update(url: nil)
+      plan_action action, repository.root, :url => 'uln://some-uln-repo'
+      refute_action_planned action, ::Actions::Pulp3::Orchestration::AlternateContentSource::Create
+    end
+
     it 'plans ACS deletion when removing the URL' do
       ::Katello::SmartProxyAlternateContentSource.create!(alternate_content_source_id: simplified_acs.id, smart_proxy_id: proxy.id, repository_id: repository.id)
       action = create_action action_class
@@ -158,6 +177,16 @@ def setup
                                          old_url: "http://www.pleaseack.com"
     end
 
+    it 'plans ACS deletion when changing to a ULN repo' do
+      ::Katello::SmartProxyAlternateContentSource.create!(alternate_content_source_id: simplified_acs.id, smart_proxy_id: proxy.id, repository_id: repository.id)
+      action = create_action action_class
+      action.stubs(:action_subject).with(repository)
+
+      plan_action action, repository.root, :url => 'uln://a-uln-repo'
+      assert_action_planned_with action, ::Actions::Pulp3::Orchestration::AlternateContentSource::Delete, ::Katello::SmartProxyAlternateContentSource.where(alternate_content_source_id: simplified_acs.id, smart_proxy_id: proxy.id, repository_id: repository.id).first,
+                                         old_url: "http://www.pleaseack.com"
+    end
+
     it 'plans ACS update when changing the URL' do
       ::Katello::SmartProxyAlternateContentSource.create!(alternate_content_source_id: simplified_acs.id, smart_proxy_id: proxy.id, repository_id: repository.id)
       action = create_action action_class

test/models/katello/alternate_content_source_test.rb

@@ -76,32 +76,34 @@ def test_http_proxy
       assert @yum_acs.use_http_proxies
     end
 
+    def test_custom_uln_base_url
+      @yum_acs.base_url = 'uln://uln-repo'
+      error = assert_raises(ActiveRecord::RecordInvalid) { @yum_acs.save! }
+      assert_match 'Validation failed: Base url uln://uln-repo is not a valid path', error.message
+    end
+
     def test_custom_missing_base_url
       @yum_acs.base_url = nil
-      assert_raises(ActiveRecord::RecordInvalid, "Base url can\'t be blank") do
-        @yum_acs.save!
-      end
+      error = assert_raises(ActiveRecord::RecordInvalid) { @yum_acs.save! }
+      assert_match "Base url can\'t be blank", error.message
     end
 
     def test_custom_missing_verify_ssl
       @yum_acs.verify_ssl = nil
-      assert_raises(ActiveRecord::RecordInvalid, "Verify ssl can\'t be blank") do
-        @yum_acs.save!
-      end
+      error = assert_raises(ActiveRecord::RecordInvalid) { @yum_acs.save! }
+      assert_match 'Validation failed: Verify ssl must be provided for custom or rhui ACS', error.message
     end
 
     def test_wrong_acs_type
       @yum_acs.alternate_content_source_type = 'definitely not an ACS type'
-      assert_raises(ActiveRecord::RecordInvalid, "Alternate content source type is not a valid type. Must be one of the following: #{AlternateContentSource::ACS_TYPES.join(',')}") do
-        @yum_acs.save!
-      end
+      error = assert_raises(ActiveRecord::RecordInvalid) { @yum_acs.save! }
+      assert_match "Alternate content source type is not a valid type. Must be one of the following: #{AlternateContentSource::ACS_TYPES.join(',')}", error.message
     end
 
     def test_wrong_content_type
       @yum_acs.content_type = 'emu'
-      assert_raises(ActiveRecord::RecordInvalid, "Content type is not allowed for ACS. Must be one of the following: #{AlternateContentSource::CONTENT_TYPES.join(',')}") do
-        @yum_acs.save!
-      end
+      error = assert_raises(ActiveRecord::RecordInvalid) { @yum_acs.save! }
+      assert_match "Validation failed: Content type is not allowed for ACS. Must be one of the following: #{AlternateContentSource::CONTENT_TYPES.sort.join(',')}, Content type cannot be modified once an ACS is created", error.message
     end
 
     def test_custom?