Fixes #33057 - apipie cache repo & content type params missing

app/controllers/katello/api/v2/content_uploads_controller.rb

@@ -10,9 +10,11 @@ class Api::V2::ContentUploadsController < Api::V2::ApiController
     param :repository_id, :number, :required => true, :desc => N_("repository id")
     param :size, :number, :required => true, :desc => N_("Size of file to upload")
     param :checksum, String, :required => false, :desc => N_("Checksum of file to upload")
-    param :content_type, RepositoryTypeManager.uploadable_content_types.map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
+    param :content_type, RepositoryTypeManager.uploadable_content_types(false).map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
     def create
       content_type = params[:content_type] || ::Katello::RepositoryTypeManager.find(@repository.content_type).default_managed_content_type.label
+      RepositoryTypeManager.check_content_matches_repo_type!(@repository, content_type)
+
       unit_type_id = SmartProxy.pulp_primary.content_service(content_type).content_type
       render :json => @repository.backend_content_service(::SmartProxy.pulp_primary).create_upload(params[:size], params[:checksum], unit_type_id)
     end

app/controllers/katello/api/v2/repositories_controller.rb

@@ -67,7 +67,7 @@ def custom_index_relation(collection)
     def_param_group :repo_create do
       param :label, String, :required => false
       param :product_id, :number, :required => true, :desc => N_("Product the repository belongs to")
-      param :content_type, RepositoryTypeManager.creatable_repository_types.keys, :required => true, :desc => N_("type of repo")
+      param :content_type, RepositoryTypeManager.creatable_repository_types(false).keys, :required => true, :desc => N_("type of repo")
     end
 
     api :GET, "/repositories", N_("List of enabled repositories")
@@ -95,7 +95,7 @@ def custom_index_relation(collection)
     param :description, String, :desc => N_("description of the repository")
     param :available_for, String, :desc => N_("interpret specified object to return only Repositories that can be associated with specified object.  Only 'content_view' & 'content_view_version' are supported."),
           :required => false
-    param :with_content, RepositoryTypeManager.enabled_content_types, :desc => N_("only repositories having at least one of the specified content type ex: rpm , erratum")
+    param :with_content, RepositoryTypeManager.enabled_content_types(false), :desc => N_("only repositories having at least one of the specified content type ex: rpm , erratum")
     param :download_policy, ::Runcible::Models::YumImporter::DOWNLOAD_POLICIES, :desc => N_("limit to only repositories with this download policy")
     param :username, String, :desc => N_("only show the repositories readable by this user with this username")
     param_group :search, Api::V2::ApiController
@@ -217,8 +217,9 @@ def index_relation_content_unit(query)
     param_group :repo
     def create
       repo_params = repository_params
-      unless RepositoryTypeManager.creatable_by_user?(repo_params[:content_type])
-        msg = _("Invalid params provided - content_type must be one of %s") % RepositoryTypeManager.creatable_repository_types.keys.join(",")
+      unless RepositoryTypeManager.creatable_by_user?(repo_params[:content_type], false)
+        msg = _("Invalid params provided - content_type must be one of %s") %
+          RepositoryTypeManager.creatable_repository_types(false).keys.join(",")
         fail HttpErrors::UnprocessableEntity, msg
       end
 
@@ -326,7 +327,7 @@ def destroy
     desc "Remove content from a repository"
     param :id, :number, :required => true, :desc => "repository ID"
     param 'ids', Array, :required => true, :desc => "Array of content ids to remove"
-    param :content_type, RepositoryTypeManager.removable_content_types.map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
+    param :content_type, RepositoryTypeManager.removable_content_types(false).map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
     param 'sync_capsule', :bool, :desc => N_("Whether or not to sync an external capsule after upload. Default: true")
     def remove_content
       sync_capsule = ::Foreman::Cast.to_bool(params.fetch(:sync_capsule, true))
@@ -337,7 +338,7 @@ def remove_content
     api :POST, "/repositories/:id/upload_content", N_("Upload content into the repository")
     param :id, :number, :required => true, :desc => N_("repository ID")
     param :content, File, :required => true, :desc => N_("Content files to upload. Can be a single file or array of files.")
-    param :content_type, RepositoryTypeManager.uploadable_content_types.map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
+    param :content_type, RepositoryTypeManager.uploadable_content_types(false).map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
     def upload_content
       fail Katello::Errors::InvalidRepositoryContent, _("Cannot upload Container Image content.") if @repository.docker?
 
@@ -366,7 +367,7 @@ def upload_content
     param :async, :bool, desc: N_("Do not wait for the ImportUpload action to finish. Default: false")
     param 'publish_repository', :bool, :desc => N_("Whether or not to regenerate the repository on disk. Default: true")
     param 'sync_capsule', :bool, :desc => N_("Whether or not to sync an external capsule after upload. Default: true")
-    param :content_type, RepositoryTypeManager.uploadable_content_types.map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
+    param :content_type, RepositoryTypeManager.uploadable_content_types(false).map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
     param :uploads, Array, :desc => N_("Array of uploads to import") do
       param 'id', String, :required => true
       param 'content_unit_id', String
@@ -521,11 +522,15 @@ def find_organization_from_product
     end
 
     def find_content
-      if params[:content_type]
-        @content = @repository.units_for_removal(params[:ids], params[:content_type])
+      content_type = params[:content_type]
+
+      if content_type
+        @content = @repository.units_for_removal(params[:ids], content_type)
       else
         @content = @repository.units_for_removal(params[:ids])
       end
+
+      RepositoryTypeManager.check_content_matches_repo_type!(@repository, @content.first.class::CONTENT_TYPE)
     end
 
     def filter_by_content_view(query, content_view_id, environment_id, is_available_for)

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

@@ -17,6 +17,7 @@ def plan(repository, content_units, options = {})
 
           content_unit_ids = content_units.map(&:id)
           content_unit_type = options[:content_type] || content_units.first.class::CONTENT_TYPE
+          ::Katello::RepositoryTypeManager.check_content_matches_repo_type!(repository, content_unit_type)
 
           generate_applicability = options.fetch(:generate_applicability, repository.yum?)
 

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

@@ -13,6 +13,8 @@ def plan(repository, files, content_type = nil, options = {})
           tmp_files = prepare_tmp_files(files)
 
           content_type ||= ::Katello::RepositoryTypeManager.find(repository.content_type).default_managed_content_type.label
+          ::Katello::RepositoryTypeManager.check_content_matches_repo_type!(repository, content_type)
+
           unit_type_id = SmartProxy.pulp_primary.content_service(content_type)::CONTENT_TYPE
           upload_actions = []
 

app/models/katello/concerns/smart_proxy_extensions.rb

@@ -229,7 +229,8 @@ def pulp3_repository_type_support?(repository_type)
 
       def pulp3_content_support?(content_type)
         content_type_obj = content_type.is_a?(String) ? Katello::RepositoryTypeManager.find_content_type(content_type) : content_type
-        fail "Cannot find content type #{content_type}." unless content_type_obj
+        content_type_string = content_type_obj&.label || content_type
+        fail "Content type #{content_type_string} does not belong to an enabled repo type." unless content_type_obj
 
         found_type = Katello::RepositoryTypeManager.enabled_repository_types.values.find { |repo_type| repo_type.content_types.include?(content_type_obj) }
         fail "Cannot find repository type for content_type #{content_type}, is it enabled?" unless found_type
@@ -289,8 +290,17 @@ def associate_default_locations
       end
 
       def content_service(content_type)
-        content_type = RepositoryTypeManager.find_content_type(content_type) if content_type.is_a?(String)
-        pulp3_content_support?(content_type) ? content_type.pulp3_service_class : content_type.pulp2_service_class
+        if content_type.is_a?(String)
+          content_type_obj = RepositoryTypeManager.find_content_type(content_type)
+        else
+          content_type_obj = content_type
+        end
+        content_type_string = content_type_obj&.label || content_type
+        unless content_type_obj
+          fail _("Content type %{content_type_string} does not belong to an enabled repo type.") %
+                { content_type_string: content_type_string }
+        end
+        pulp3_content_support?(content_type_obj) ? content_type_obj.pulp3_service_class : content_type_obj.pulp2_service_class
       end
 
       def set_default_download_policy

app/models/katello/root_repository.rb

@@ -83,7 +83,7 @@ class RootRepository < Katello::Model
     validates :content_type, :inclusion => {
       :in => ->(_) { Katello::RepositoryTypeManager.enabled_repository_types.keys },
       :allow_blank => false,
-      :message => ->(_, _) { _("must be one of the following: %s") % Katello::RepositoryTypeManager.enabled_repository_types.keys.join(', ') }
+      :message => ->(_, _) { _("is not enabled. must be one of the following: %s") % Katello::RepositoryTypeManager.enabled_repository_types.keys.join(', ') }
     }
     validates :download_policy, inclusion: {
       :in => ::Runcible::Models::YumImporter::DOWNLOAD_POLICIES,

app/services/katello/repository_type_manager.rb

@@ -45,9 +45,10 @@ def enabled_repository_types(update = true)
         @enabled_repository_types
       end
 
-      def creatable_repository_types
-        enabled_repository_types.select do |repo_type, _|
-          creatable_by_user?(repo_type)
+      def creatable_repository_types(enabled_only = true)
+        repo_types = enabled_only ? enabled_repository_types : defined_repository_types
+        repo_types.select do |repo_type, _|
+          creatable_by_user?(repo_type, enabled_only)
         end
       end
 
@@ -56,8 +57,9 @@ def pulp3_plugin_installed?(repository_type)
         @pulp_primary&.capabilities(PULP3_FEATURE)&.include?(@defined_repository_types[repository_type].pulp3_plugin)
       end
 
-      def enabled_content_types
-        list = enabled_repository_types.values.map do |type|
+      def enabled_content_types(enabled_only = true)
+        repo_types = enabled_only ? enabled_repository_types : defined_repository_types
+        list = repo_types.values.map do |type|
           type.content_types.map(&:model_class).flatten.map { |ct| ct::CONTENT_TYPE }
         end
         list.flatten
@@ -70,20 +72,23 @@ def indexable_content_types
                   flatten
       end
 
-      def creatable_by_user?(repository_type)
-        return false unless (type = find(repository_type))
+      def creatable_by_user?(repository_type, enabled_only = true)
+        type = enabled_only ? find(repository_type) : find_defined(repository_type)
+        return false unless type
         type.allow_creation_by_user
       end
 
-      def removable_content_types
-        list = enabled_repository_types.values.map do |type|
+      def removable_content_types(enabled_only = true)
+        repo_types = enabled_only ? enabled_repository_types : defined_repository_types
+        list = repo_types.values.map do |type|
           type.content_types.select(&:removable)
         end
         list.flatten
       end
 
-      def uploadable_content_types
-        list = enabled_repository_types.values.map do |type|
+      def uploadable_content_types(enabled_only = true)
+        repo_types = enabled_only ? enabled_repository_types : defined_repository_types
+        list = repo_types.values.map do |type|
           type.content_types.select(&:uploadable)
         end
         list.flatten
@@ -140,6 +145,14 @@ def enabled?(repository_type)
         find(repository_type).present?
       end
 
+      def check_content_matches_repo_type!(repository, content_type)
+        repo_content_types = repository.repository_type.content_types.collect { |type| type.label }
+        unless repo_content_types.include?(content_type)
+          fail _("Content type %{content_type} is incompatible with repositories of type %{repo_type}") %
+                 { content_type: content_type, repo_type: repository.content_type }
+        end
+      end
+
       private
 
       def update_enabled_repository_type(repository_type)

test/controllers/api/v2/content_uploads_controller_test.rb

@@ -27,6 +27,7 @@ def setup
 
     def test_create_upload_request
       mock_pulp_server(:create_upload_request => [])
+      ::Katello::RepositoryTypeManager.expects(:check_content_matches_repo_type!).returns(true)
       post :create, params: { :repository_id => @repo.id, :size => 100, :checksum => 'test_checksum' }
       assert_response :success
     end

test/controllers/api/v2/repositories_controller_test.rb

@@ -262,6 +262,9 @@ def test_index_available_for_content_view_version
     end
 
     def test_create
+      # Ensure that the content_type params don't rely on the enabled_repository_types
+      SmartProxy.pulp_primary.features.detect { |f| f.name == "Pulpcore" }.smart_proxy_features.first.update(capabilities: [])
+      ::Katello::RepositoryTypeManager.instance_variable_set(:@enabled_repository_types, {})
       product = mock
       product.expects(:add_repo).with({
         :label => 'Fedora_Repository',
@@ -666,11 +669,12 @@ def test_remove_content
     end
 
     def test_remove_content_protected
+      @repository.rpms << @rpm
       allowed_perms = [@update_permission]
       denied_perms = [@read_permission, @create_permission, @destroy_permission]
 
       assert_protected_action(:remove_content, allowed_perms, denied_perms) do
-        put :remove_content, params: { :id => @repository.id, :uuids => ['foo', 'bar'] }
+        put :remove_content, params: { :id => @repository.id, :ids => [@rpm.id] }
       end
     end
 

test/services/katello/repository_type_manager_test.rb

@@ -30,5 +30,19 @@ def test_find_calls_update_enabled_types_once
       ::Katello::RepositoryTypeManager.expects(:update_enabled_repository_type).once
       ::Katello::RepositoryTypeManager.find('yum')
     end
+
+    def test_check_content_type_matches_repo_type_fails_properly
+      @feature.update(capabilities: ['ansible', 'certguard', 'container', 'core', 'deb', 'file', 'rpm', 'python'])
+      repo = Repository.find_by(pulp_id: "pulp-uuid-rhel_6_x86_64")
+      assert_raises_with_message(RuntimeError, 'Content type ostree is incompatible with repositories of type yum') do
+        RepositoryTypeManager.check_content_matches_repo_type!(repo, 'ostree')
+      end
+    end
+
+    def test_check_content_type_matches_repo_type_passes_properly
+      @feature.update(capabilities: ['ansible', 'certguard', 'container', 'core', 'deb', 'file', 'rpm', 'python'])
+      repo = Repository.find_by(pulp_id: "pulp-uuid-rhel_6_x86_64")
+      RepositoryTypeManager.check_content_matches_repo_type!(repo, 'rpm')
+    end
   end
 end