WIP: Make structured APT mandatory

quba42 · quba42 · commit 8e4562421dcb · 2025-09-02T14:29:23.000+02:00
diff --git a/app/controllers/katello/api/v2/repositories_controller.rb b/app/controllers/katello/api/v2/repositories_controller.rb
@@ -667,7 +667,6 @@ def construct_repo_from_params(repo_params) # rubocop:disable Metrics/AbcSize
         root.deb_releases = repo_params[:deb_releases] if repo_params[:deb_releases]
         root.deb_components = repo_params[:deb_components] if repo_params[:deb_components]
         root.deb_architectures = repo_params[:deb_architectures] if repo_params[:deb_architectures]
-        root.content_id = 'INITIAL_DUMMY_VALUE' unless Setting['deb_enable_structured_apt']
       end
 
       if root.ansible_collection?
diff --git a/app/lib/actions/candlepin/product/content_create.rb b/app/lib/actions/candlepin/product/content_create.rb
@@ -16,12 +16,8 @@ class ContentCreate < Candlepin::Abstract
         def run
           content_url = input[:content_url]
           if input[:type] == ::Katello::Repository::DEB_TYPE
-            # We must retrieve the repository in the run phase, so the latest Pulp version_href is
-            # already set. This is needed to retrieve the latest repository.deb_content_url_options!
             repository = ::Katello::Repository.find(input[:repository_id])
-            if repository.deb_using_structured_apt?
-              content_url += repository.deb_content_url_options
-            end
+            content_url += repository.deb_content_url_options
           end
 
           output[:response] = ::Katello::Resources::Candlepin::Content.
diff --git a/app/lib/actions/candlepin/product/content_update.rb b/app/lib/actions/candlepin/product/content_update.rb
@@ -16,11 +16,10 @@ class ContentUpdate < Candlepin::Abstract
         end
 
         def finalize
+          # We must use finalize, because Katello::Product::ContentCreate only updates repository.content_id during finalize!
           content_url = input[:content_url]
-          # We must retrieve the repository in the finalize phase, because Katello::Product::ContentCreate
-          # only updates the repository.content_id in the finalize phase!
           repository = ::Katello::Repository.find(input[:repository_id])
-          if repository.deb_using_structured_apt?
+          if repository.deb?
             content_url += repository.deb_content_url_options
           end
 
diff --git a/app/lib/actions/katello/content_view/refresh_rolling_repo.rb b/app/lib/actions/katello/content_view/refresh_rolling_repo.rb
@@ -26,7 +26,7 @@ def run
 
           repository.version_href = library_instance.version_href
           repository.publication_href = library_instance.publication_href
-          if repository.deb_using_structured_apt?
+          if repository.deb?
             repository.content_id = library_instance.content_id
           end
           repository.save!
diff --git a/app/lib/actions/katello/product/content_create.rb b/app/lib/actions/katello/product/content_create.rb
@@ -45,7 +45,7 @@ def plan(repository)
         def finalize
           root = ::Katello::RootRepository.find(input[:root_repository_id])
           content_url = root.custom_content_path
-          if root.deb_using_structured_apt?
+          if root.deb?
             repository = ::Katello::Repository.find(input[:repository_id])
             content_url += repository.deb_content_url_options
             repository.update(:content_id => input[:content_id])
diff --git a/app/lib/actions/katello/product/content_destroy.rb b/app/lib/actions/katello/product/content_destroy.rb
@@ -15,7 +15,7 @@ def plan(repository)
             ::Katello::ProductContent.where(product_id: root_repository.product_id,
                                             content_id: katello_content_id).destroy_all
 
-            if root_repository.repositories.count <= 1 || repository.deb_using_structured_apt?
+            if root_repository.repositories.count <= 1 || repository.deb?
               plan_action(Candlepin::Product::ContentDestroy,
                           owner: root_repository.product.organization.label,
                           content_id: repository.content_id)
diff --git a/app/lib/actions/katello/repository/clone_contents.rb b/app/lib/actions/katello/repository/clone_contents.rb
@@ -26,7 +26,7 @@ def plan(source_repositories, new_repository, options)
             index_options = {id: new_repository.id, force_index: true}
             index_options[:source_repository_id] = source_repositories.first.id if source_repositories.count == 1 && filters.empty? && rpm_filenames.nil?
 
-            if new_repository.deb_using_structured_apt? && generate_metadata
+            if new_repository.deb? && generate_metadata
               plan_action(Candlepin::Product::ContentUpdate,
                           owner:           new_repository.organization.label,
                           repository_id:   new_repository.id,
diff --git a/app/lib/actions/katello/repository/create.rb b/app/lib/actions/katello/repository/create.rb
@@ -64,7 +64,7 @@ def humanized_name
         end
 
         def create_content?(repository, clone)
-          if repository.deb_using_structured_apt?
+          if repository.deb?
             return true
           elsif clone || repository.content
             return false
diff --git a/app/lib/actions/katello/repository/destroy.rb b/app/lib/actions/katello/repository/destroy.rb
@@ -86,10 +86,10 @@ def delete_empty_repo_filters(repository)
         end
 
         def handle_custom_content(repository, remove_from_content_view_versions)
-          #if this is the last instance of a custom repo or a deb repo using structured APT, destroy the content
-          if remove_from_content_view_versions || repository.root.repositories.where.not(id: repository.id).empty? || repository.deb_using_structured_apt?
-            # Never destroy content for structured apt rolling repo clones, because it belongs to the library instance
-            return if repository.deb_using_structured_apt? && repository.content_view.rolling?
+          # If this is the last instance of a custom repo or a deb repo, then destroy the content
+          if remove_from_content_view_versions || repository.root.repositories.where.not(id: repository.id).empty? || repository.deb?
+            # Never destroy content for deb rolling repo clones, it belongs to the library instance!
+            return if repository.deb? && repository.content_view.rolling?
 
             plan_action(::Actions::Katello::Product::ContentDestroy, repository)
           end
diff --git a/app/lib/actions/katello/repository/finish_upload.rb b/app/lib/actions/katello/repository/finish_upload.rb
@@ -15,7 +15,7 @@ def plan(repository, options = {})
           generate_metadata = options.fetch(:generate_metadata, true)
           plan_action(Katello::Repository::MetadataGenerate, repository, :dependency => import_upload_task, :force_publication => true) if generate_metadata
 
-          if repository.deb_using_structured_apt? && generate_metadata
+          if repository.deb? && generate_metadata
             plan_action(::Actions::Candlepin::Product::ContentUpdate,
                         owner:           repository.organization.label,
                         repository_id:   repository.id,
diff --git a/app/lib/actions/katello/repository/sync.rb b/app/lib/actions/katello/repository/sync.rb
@@ -48,7 +48,7 @@ def plan(repo, options = {})
                                         pulp_sync_options)
               output = sync_action.output
 
-              update_deb_content(repo)
+              update_deb_content(repo) if repo.deb?
               plan_action(Katello::Repository::IndexContent, :id => repo.id, :force_index => skip_metadata_check)
               plan_action(Katello::Foreman::ContentUpdate, repo.environment, repo.content_view, repo)
               plan_action(Katello::Repository::FetchPxeFiles, :id => repo.id)
@@ -101,8 +101,6 @@ def presenter
         end
 
         def update_deb_content(repo)
-          return unless repo.deb_using_structured_apt?
-
           plan_action(::Actions::Candlepin::Product::ContentUpdate,
                       owner:           repo.organization.label,
                       repository_id:   repo.id,
diff --git a/app/models/katello/host/content_facet.rb b/app/models/katello/host/content_facet.rb
@@ -202,7 +202,7 @@ def update_repositories_by_paths(paths)
         #           "/pulp/content/Org/Library/custom/Test_product/test2/%3Fcomp%3Dmain%26rel%3Dstable"]
         paths.each do |path|
           if (prefix = prefixes.find { |pre| path.start_with?(pre) })
-            # strip prefix and structured_apt postfix before adding to relative_paths
+            # strip prefix and deb content postfix before adding to relative_paths
             relative_paths << path.sub(prefix, '').sub(%r{/?(%3F|\?).*}, '')
           else
             Rails.logger.warn("System #{self.host.name} (#{self.host.id}) requested binding to repo with unknown prefix. #{path}")
diff --git a/app/models/katello/product_content.rb b/app/models/katello/product_content.rb
@@ -46,8 +46,8 @@ def self.content_table_name
 
     def self.enabled(organization)
       content_ids = Katello::RootRepository.in_organization(organization).where.not(content_id: nil).pluck(:content_id)
-      structured_apt_content_ids = Katello::Repository.in_organization(organization).library.pluck(:content_id)
-      joins(:content).where("#{self.content_table_name}.cp_content_id" => content_ids + structured_apt_content_ids)
+      deb_content_ids = Katello::Repository.in_organization(organization).library.pluck(:content_id)
+      joins(:content).where("#{self.content_table_name}.cp_content_id" => content_ids + deb_content_ids)
     end
 
     def self.with_valid_subscription(organization)
diff --git a/app/models/katello/repository.rb b/app/models/katello/repository.rb
@@ -170,7 +170,7 @@ class Repository < Katello::Model
     scoped_search :on => :label, :relation => :root, :complete_value => true, :only_explicit => true
     scoped_search :on => :content_label, :ext_method => :search_by_content_label, :default_operator => :like
 
-    delegate :product, :redhat?, :custom?, :deb_using_structured_apt?, :to => :root
+    delegate :product, :redhat?, :custom?, :to => :root
     delegate :yum?, :docker?, :deb?, :file?, :ostree?, :ansible_collection?, :generic?, :to => :root
     delegate :name, :label, :docker_upstream_name, :url, :download_concurrency, :to => :root
 
@@ -1045,33 +1045,15 @@ def in_content_view?(content_view)
     end
 
     def deb_content_url_options
+      return '' unless self.deb?
       return '' unless version_href
-      return '' if backend_service(SmartProxy.pulp_primary).version_missing_structure_content?
 
-      components = deb_pulp_components.join(',')
-      distributions = deb_pulp_distributions.join(',')
+      backend_service = self.backend_service(SmartProxy.pulp_primary)
+      components = backend_service.pulp_components.join(',')
+      distributions = backend_service.pulp_distributions.join(',')
       "/?comp=#{components}&rel=#{distributions}"
     end
 
-    def deb_pulp_components(version_href = self.version_href)
-      return [] if version_href.blank?
-
-      pulp_api = Katello::Pulp3::Repository.instance_for_type(self, SmartProxy.pulp_primary).api.content_release_components_api
-      pulp_api.list({:repository_version => version_href}).results.map { |x| x.plain_component }.uniq
-    end
-
-    def deb_sanitize_pulp_distribution(distribution)
-      return "flat-repo" if distribution == "/"
-      return distribution.chomp("/") if distribution&.end_with?("/")
-      distribution
-    end
-
-    def deb_pulp_distributions(version_href = self.version_href)
-      return [] if version_href.blank?
-      pulp_api = Katello::Pulp3::Repository.instance_for_type(self, SmartProxy.pulp_primary).api.content_release_components_api
-      pulp_api.list({:repository_version => version_href}).results.map { |x| deb_sanitize_pulp_distribution(x.distribution) }.uniq
-    end
-
     def sync_status
       return latest_dynflow_sync
     end
diff --git a/app/models/katello/root_repository.rb b/app/models/katello/root_repository.rb
@@ -389,10 +389,6 @@ def deb?
       self.content_type == Repository::DEB_TYPE
     end
 
-    def deb_using_structured_apt?
-      self.deb? && !self.content_id
-    end
-
     def ansible_collection?
       self.content_type == Repository::ANSIBLE_COLLECTION_TYPE
     end
diff --git a/app/services/katello/product_content_finder.rb b/app/services/katello/product_content_finder.rb
@@ -23,9 +23,9 @@ def product_content
       roots = Katello::RootRepository.where(:product_id => considered_products).subscribable
       roots = roots.in_content_view_version(versions).distinct if versions.present?
       content_ids = roots.where.not(:content_id => nil).pluck(:content_id)
-      structured_apt_roots = roots.where(:content_id => nil)
-      if structured_apt_roots.any?
-        deb_repos_query = Katello::Repository.where(root: structured_apt_roots)
+      deb_roots = roots.where(:content_id => nil)
+      if deb_roots.any?
+        deb_repos_query = Katello::Repository.where(root: deb_roots)
         environment = consumable.respond_to?(:environment) ? consumable.environment : consumable.content_view_environments.select(:environment_id).map(&:environment_id)
         deb_repos = match_environment ? deb_repos_query.where(content_view_version: versions, environment: environment) : deb_repos_query.where(:library_instance_id => nil)
         content_ids += deb_repos.pluck(:content_id)
diff --git a/app/services/katello/pulp3/repository/apt.rb b/app/services/katello/pulp3/repository/apt.rb
@@ -7,6 +7,15 @@ class Apt < ::Katello::Pulp3::Repository
         UNIT_LIMIT = 10_000
         SIGNING_SERVICE_NAME = 'katello_deb_sign'.freeze
 
+        def pulp_primary_api
+          # Sometimes we need to make sure we are talking to the pulp primary and not a smart proxy!
+          if smart_proxy.pulp_primary?
+            api
+          else
+            self.class.instance_for_type(repo, ::SmartProxy.pulp_primary).api
+          end
+        end
+
         def initialize_empty
           # For every empty APT library instance repository we must add at least a release component to
           # ensure we have a publishable repo with consumable metadata. Otherwise smart proxy syncs will
@@ -15,6 +24,24 @@ def initialize_empty
           api.content_release_components_api.create(opts)
         end
 
+        def pulp_components
+          return [] if repo.version_href.blank?
+          return ["all"] if version_missing_structure_content?
+          pulp_primary_api.content_release_components_api.list({:repository_version => repo.version_href}).results.map { |x| x.plain_component }.uniq
+        end
+
+        def sanitize_pulp_distribution(distribution)
+          return "flat-repo" if distribution == "/"
+          return distribution.chomp("/") if distribution&.end_with?("/")
+          distribution
+        end
+
+        def pulp_distributions
+          return [] if repo.version_href.blank?
+          return ["default"] if version_missing_structure_content?
+          pulp_primary_api.content_release_components_api.list({:repository_version => repo.version_href}).results.map { |x| sanitize_pulp_distribution(x.distribution) }.uniq
+        end
+
         def remote_options
           deb_remote_options = {
             policy: root.download_policy,
@@ -39,13 +66,7 @@ def sync_url_params(sync_options)
         end
 
         def mirror_remote_options
-          distributions = if repo.deb_using_structured_apt? && !version_missing_structure_content?
-                            repo.deb_pulp_distributions.join(' ')
-                          else
-                            'default'
-                          end
-
-          super.merge({distributions: distributions})
+          super.merge({distributions: pulp_distributions.join(' ')})
         end
 
         def version_missing_structure_content?
@@ -54,9 +75,7 @@ def version_missing_structure_content?
           # It may also affect filtered CV versions created with very old Katello versions.
           # This method can identify such cases, so that we may fall back to simple publishing.
           return false if repo.version_href.blank?
-          # We cannot just use api here, because this is sometimes the proxy api, and we always want to talk to the primary!
-          api_primary = self.class.instance_for_type(repo, ::SmartProxy.pulp_primary).api
-          version = api_primary.repository_versions_api.read(repo.version_href)
+          version = pulp_primary_api.repository_versions_api.read(repo.version_href)
           apt_content_types = version&.content_summary&.present&.keys
           return apt_content_types.include?('deb.package') && !apt_content_types.include?('deb.package_release_component')
         end
@@ -67,9 +86,6 @@ def publication_options(repository)
           if version_missing_structure_content?
             popts.merge!({ structured: false })
             popts.merge!({ simple: true })
-          else
-            popts.merge!({ structured: true })
-            popts.merge!({ simple: true }) unless repository.deb_using_structured_apt?
           end
           popts[:signing_service] = ss[0].pulp_href if ss && ss.length == 1
           popts
diff --git a/lib/katello/plugin.rb b/lib/katello/plugin.rb
@@ -704,13 +704,6 @@ def katello_template_setting_values(name)
         default: false,
         full_name: N_('Hide Reclaim Space Warning'),
         description: N_('If this is enabled, the Smart Proxy page will suppress the warning message about reclaiming space.')
-
-      setting 'deb_enable_structured_apt',
-        type: :boolean,
-        default: false,
-        full_name: N_('Enable structured APT for deb content'),
-        description: N_("If set, newly created APT repos in Katello will use the same repo structure as the remote repos they are synchronized from. " \
-                        "You may migrate existing APT repos to match the setting, by running 'foreman-rake katello:migrate_structure_content_for_deb'.")
     end
   end
 
diff --git a/lib/katello/tasks/migrate_structure_content_for_deb.rake b/lib/katello/tasks/migrate_structure_content_for_deb.rake
diff --git a/lib/katello/tasks/upgrades/4.19/enable_structured_apt_for_deb.rake b/lib/katello/tasks/upgrades/4.19/enable_structured_apt_for_deb.rake
diff --git a/test/actions/katello/repository_test.rb b/test/actions/katello/repository_test.rb
diff --git a/test/controllers/api/v2/host_subscriptions_controller_test.rb b/test/controllers/api/v2/host_subscriptions_controller_test.rb
diff --git a/test/models/product_content_test.rb b/test/models/product_content_test.rb
diff --git a/test/services/katello/pulp3/repository/apt/apt_test.rb b/test/services/katello/pulp3/repository/apt/apt_test.rb
