Ruby 3 minimal changes

.rubocop.yml

@@ -31,7 +31,7 @@ AllCops:
     - '**/*.rb'
     - app/views/**/*.rabl
     - '**/*.rake'
-  TargetRubyVersion: 2.5
+  TargetRubyVersion: 2.7
 
 Metrics/MethodLength:
   Description: 'Avoid methods longer than 30 lines of code.'

app/lib/actions/katello/content_view/publish.rb

@@ -13,7 +13,7 @@ class Publish < Actions::EntryAction
         def plan(content_view, description = "", options = {importing: false, syncable: false}) # rubocop:disable Metrics/PerceivedComplexity
           action_subject(content_view)
 
-          content_view.check_ready_to_publish!(options.slice(:importing, :syncable))
+          content_view.check_ready_to_publish!(**options.slice(:importing, :syncable))
           unless options[:importing] || options[:syncable]
             ::Katello::Util::CandlepinRepositoryChecker.check_repositories_for_publish!(content_view)
           end

app/lib/actions/katello/organization/manifest_delete.rb

@@ -10,7 +10,7 @@ def plan(organization)
           action_subject(organization)
 
           sequence do
-            plan_action(Candlepin::Owner::DestroyImports, label: organization.label)
+            plan_action(Candlepin::Owner::DestroyImports, { label: organization.label })
 
             repositories = ::Katello::Repository.in_default_view.in_product(::Katello::Product.redhat.in_org(organization))
             repositories.each do |repo|

app/lib/actions/katello/organization/manifest_refresh.rb

@@ -19,26 +19,26 @@ def plan(organization)
               :organization_name => organization.name
             )
             upstream_update = plan_action(Candlepin::Owner::UpstreamUpdate,
-                        :organization_id => organization.id,
-                        :upstream => upstream)
+                        { :organization_id => organization.id,
+                          :upstream => upstream })
             export_action = plan_action(Candlepin::Owner::StartUpstreamExport,
-                        :organization_id => organization.id,
-                        :upstream => upstream,
-                        :path => path,
-                        :dependency => upstream_update.output)
+                        { :organization_id => organization.id,
+                          :upstream => upstream,
+                          :path => path,
+                          :dependency => upstream_update.output })
             retrieved_export = plan_action(Candlepin::Owner::RetrieveUpstreamExport,
-                        :export_id => export_action.output[:task]['resultData']['exportId'],
-                        :organization_id => organization.id,
-                        :upstream => upstream,
-                        :path => path,
-                        :dependency => export_action.output)
+                        { :export_id => export_action.output[:task]['resultData']['exportId'],
+                          :organization_id => organization.id,
+                          :upstream => upstream,
+                          :path => path,
+                          :dependency => export_action.output })
             owner_import = plan_action(Candlepin::Owner::Import,
-                        :label => organization.label,
-                        :path => path,
-                        :dependency => retrieved_export.output)
+                        { :label => organization.label,
+                          :path => path,
+                          :dependency => retrieved_export.output })
             import_products = plan_action(Candlepin::Owner::ImportProducts,
-              :organization_id => organization.id,
-              :dependency => owner_import.output)
+              { :organization_id => organization.id,
+                :dependency => owner_import.output })
             plan_action(Katello::Organization::EnvironmentContentsRefresh,
               organization)
             if manifest_update

app/lib/actions/pulp3/orchestration/content_view_version/export_library.rb

@@ -3,30 +3,33 @@ module Pulp3
     module Orchestration
       module ContentViewVersion
         class ExportLibrary < Actions::EntryAction
-          def plan(organization, destination_server: nil,
-                                 chunk_size: nil,
-                                 from_history: nil,
-                                 fail_on_missing_content: false,
-                                 format: ::Katello::Pulp3::ContentViewVersion::Export::IMPORTABLE)
+          def plan(organization, opts = {})
+            options = {
+              destination_server: nil,
+              chunk_size: nil,
+              from_history: nil,
+              fail_on_missing_content: false,
+              format: ::Katello::Pulp3::ContentViewVersion::Export::IMPORTABLE
+            }.merge(opts)
             action_subject(organization)
-            validate_repositories_immediate!(organization) if fail_on_missing_content
-            content_view = ::Katello::Pulp3::ContentViewVersion::Export.find_library_export_view(destination_server: destination_server,
+            validate_repositories_immediate!(organization) if options[:fail_on_missing_content]
+            content_view = ::Katello::Pulp3::ContentViewVersion::Export.find_library_export_view(destination_server: options[:destination_server],
                                                                            organization: organization,
                                                                            create_by_default: true,
-                                                                           format: format)
-            repo_ids_in_library = organization.default_content_view_version.repositories.exportable(format: format).immediate_or_none.pluck(:id)
+                                                                           format: options[:format])
+            repo_ids_in_library = organization.default_content_view_version.repositories.exportable(format: options[:format]).immediate_or_none.pluck(:id)
             content_view.update!(repository_ids: repo_ids_in_library)
 
             sequence do
               publish_action = plan_action(::Actions::Katello::ContentView::Publish, content_view, '')
               export_action = plan_action(Actions::Katello::ContentViewVersion::Export,
                                           content_view_version: publish_action.version,
-                                          destination_server: destination_server,
-                                          chunk_size: chunk_size,
-                                          from_history: from_history,
+                                          destination_server: options[:destination_server],
+                                          chunk_size: options[:chunk_size],
+                                          from_history: options[:from_history],
                                           validate_incremental: false,
-                                          fail_on_missing_content: fail_on_missing_content,
-                                          format: format)
+                                          fail_on_missing_content: options[:fail_on_missing_content],
+                                          format: options[:format])
               plan_self(export_action_output: export_action.output)
             end
           end

app/lib/katello/lazy_accessor.rb

@@ -56,21 +56,21 @@ def changed_remote_attributes=(val)
         @changed_remote_attributes = val
       end
 
-      def save(*)
-        if (status = super)
+      def save(...)
+        if (status = super(...))
           changed_remote_attributes.clear
         end
         status
       end
 
-      def save!(*)
-        super.tap do
+      def save!(...)
+        super(...).tap do
           changed_remote_attributes.clear
         end
       end
 
-      def reload(*)
-        super.tap do
+      def reload(...)
+        super(...).tap do
           changed_remote_attributes.clear
         end
       end

app/services/katello/pulp3/content_view_version/export.rb

@@ -12,7 +12,7 @@ def self.create(options)
           if options.delete(:format) == SYNCABLE
             SyncableFormatExport.new(options)
           else
-            self.new(options)
+            self.new(**options)
           end
         end
 

gemfile.d/test.rb

@@ -1,4 +1,6 @@
 group :test do
   gem "vcr", "~> 6.1"
   gem 'theforeman-rubocop', '~> 0.0.6', require: false
+  # TODO: Remove me
+  gem 'foreman-tasks', git: 'https://github.com/ofedoren/foreman-tasks.git', branch: 'ruby3-min'
 end

katello.gemspec

@@ -13,7 +13,7 @@ Gem::Specification.new do |gem|
   gem.homepage    = "http://www.katello.org"
   gem.summary     = "Content and Subscription Management plugin for Foreman"
   gem.description = "Katello adds Content and Subscription Management to Foreman. For this it relies on Candlepin and Pulp."
-  gem.required_ruby_version = '~> 2.5'
+  gem.required_ruby_version = '>= 2.7'
 
   gem.files = Dir["{app,webpack,vendor,lib,db,ca,config,locale}/**/*"] +
     Dir['LICENSE.txt', 'README.md', 'package.json']
@@ -49,7 +49,7 @@ Gem::Specification.new do |gem|
 
   # Pulp
   gem.add_dependency "anemone"
-
+  gem.add_dependency "webrick"
   #pulp3
   gem.add_dependency "pulpcore_client", ">= 3.39.0", "< 3.40.0"
   gem.add_dependency "pulp_file_client", ">= 1.15.0", "< 1.16.0"

test/actions/pulp3/content_view_version/export_library_test.rb

@@ -39,7 +39,7 @@ class ExportLibraryTest < ActiveSupport::TestCase
         assert content_view.generated_for_library?
       end
 
-      assert_action_planned_with(action, Actions::Katello::ContentViewVersion::Export) do |**options|
+      assert_action_planned_with(action, Actions::Katello::ContentViewVersion::Export) do |options, _|
         assert_equal version, options[:content_view_version]
         assert_equal destination_server, options[:destination_server]
       end

test/controllers/api/v2/api_controller_test.rb

@@ -73,8 +73,8 @@ def test_with_reduced_perms
 
       User.current = users(:restricted)
       key = katello_activation_keys(:simple_key)
-      setup_current_user_with_permissions(:name => "view_activation_keys",
-                                          :search => "environment = #{key.environment}")
+      setup_current_user_with_permissions({ :name => "view_activation_keys",
+                                            :search => "environment = #{key.environment}" })
 
       @options = { :resource_class => Katello::ActivationKey }
       keys = @controller.scoped_search(ActivationKey.readable, @default_sort[0], @default_sort[1], @options)

test/models/authorization/activation_key_authorization_test.rb

@@ -83,8 +83,8 @@ def test_all_editable?
 
       clause = keys.map { |key| "name=\"#{key.name}\"" }.join(" or ")
 
-      setup_current_user_with_permissions(:name => "edit_activation_keys",
-                                          :search => clause)
+      setup_current_user_with_permissions({ :name => "edit_activation_keys",
+                                            :search => clause })
       assert ActivationKey.all_editable?(ak.content_view, ak.environment)
     end
   end

test/models/authorization/lifecycle_environment_authorization_test.rb

@@ -75,17 +75,17 @@ def setup
 
     def test_readables
       environment = katello_environments(:staging_path1)
-      setup_current_user_with_permissions(:name => "view_lifecycle_environments",
-                                          :search => "name=\"#{environment.name}\"")
+      setup_current_user_with_permissions({ :name => "view_lifecycle_environments",
+                                            :search => "name=\"#{environment.name}\"" })
       assert_equal([environment.id], KTEnvironment.readable.pluck(:id))
       assert environment.readable?
       refute environment.prior.readable?
     end
 
     def test_promotables
       environment = katello_environments(:staging_path1)
-      setup_current_user_with_permissions(:name => "promote_or_remove_content_views_to_environments",
-                                          :search => "name=\"#{environment.name}\"")
+      setup_current_user_with_permissions({ :name => "promote_or_remove_content_views_to_environments",
+                                            :search => "name=\"#{environment.name}\"" })
       assert_equal([environment.id], KTEnvironment.promotable.pluck(:id))
       assert environment.promotable_or_removable?
       refute environment.prior.promotable_or_removable?

test/models/authorization/product_authorization_test.rb

@@ -101,8 +101,8 @@ def test_readable_repositories_with_ids
 
     def test_readable_repositories_with_search
       repo = @fedora_17_x86_64
-      setup_current_user_with_permissions(:name => "view_products",
-                                          :search => "name=\"#{repo.product.name}\"")
+      setup_current_user_with_permissions({ :name => "view_products",
+                                            :search => "name=\"#{repo.product.name}\"" })
 
       assert_equal([repo], Product.readable_repositories([repo.id]))
       assert_empty(Product.readable_repositories(

test/models/authorization/repository_authorization_test.rb

@@ -100,25 +100,25 @@ def test_readable
 
     def test_readable_with_product
       refute_includes Repository.readable, @fedora_17_x86_64
-      setup_current_user_with_permissions(:name => "view_products", :search => nil)
+      setup_current_user_with_permissions({ :name => "view_products", :search => nil })
       assert_includes Repository.readable, @fedora_17_x86_64
     end
 
     def test_readable_with_content_view
       refute_includes Repository.readable, @fedora_17_x86_64
-      setup_current_user_with_permissions(:name => "view_content_views", :search => nil)
+      setup_current_user_with_permissions({ :name => "view_content_views", :search => nil })
       assert_includes Repository.readable, @fedora_17_x86_64
     end
 
     def test_readable_with_versions
       refute_includes Repository.readable, @fedora_17_x86_64_dev
-      setup_current_user_with_permissions(:name => "view_content_views", :search => "name = \"#{@fedora_17_x86_64_dev.content_view_version.content_view.name}\"")
+      setup_current_user_with_permissions({ :name => "view_content_views", :search => "name = \"#{@fedora_17_x86_64_dev.content_view_version.content_view.name}\"" })
       assert_includes Repository.readable, @fedora_17_x86_64_dev
     end
 
     def test_readable_with_environment
       refute_includes Repository.readable, @fedora_17_x86_64
-      setup_current_user_with_permissions(:name => "view_lifecycle_environments", :search => "name = \"#{@fedora_17_x86_64.environment.name}\"")
+      setup_current_user_with_permissions({ :name => "view_lifecycle_environments", :search => "name = \"#{@fedora_17_x86_64.environment.name}\"" })
       repos = Repository.readable
       refute_empty repos
       assert repos.all? { |repo| repo.environment_id == @fedora_17_x86_64.environment_id }

test/services/katello/registration_manager_test.rb

@@ -42,30 +42,30 @@ def setup
         def test_different_org
           org2 = taxonomies(:organization2)
 
-          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, org2, nil, host_uuid_overridden: nil) }
+          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, org2, nil, nil, host_uuid_overridden: nil) }
 
           assert_match(/different org/, error.message)
         end
 
         def test_multiple_hosts
           assert ::Host.all.size > 1
-          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(::Host.all, @org, nil, host_uuid_overridden: nil) }
+          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(::Host.all, @org, nil, nil, host_uuid_overridden: nil) }
           assert_match(/matches other registered/, error.message)
         end
 
         def test_new_host_existing_uuid
           existing_uuid = 'existing_system_uuid'
           @host.subscription_facet.update(dmi_uuid: existing_uuid)
 
-          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, 'new_host_name', host_uuid_overridden: existing_uuid) }
+          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, 'new_host_name', existing_uuid, host_uuid_overridden: existing_uuid) }
           assert_match(/matches other registered/, error.message)
         end
 
         def test_existing_host_mismatch_uuid
           @host.subscription_facet.update(dmi_uuid: 'existing_system_uuid')
           Setting[:host_profile_assume] = false
 
-          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: 'different-uuid') }
+          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, @host.name, 'different-uuid', host_uuid_overridden: false) }
           assert_match(/DMI UUID that differs/, error.message)
 
           # if a registering client is matched by hostname to an existing profile
@@ -80,7 +80,7 @@ def test_host_profile_assume_build_mode_only_not_in_build
           Setting[:host_profile_assume] = false
           Setting[:host_profile_assume_build_can_change] = true
           refute @host.build
-          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: 'different-uuid') }
+          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, @host.name, 'different-uuid', host_uuid_overridden: false) }
           assert_match(/DMI UUID that differs/, error.message)
         end
 
@@ -92,7 +92,7 @@ def test_host_profile_assume_build_mode_only_in_build
           # if a registering client is matched by hostname to an existing profile
           # but its UUID has changed *and* is still unique, also it is in build mode
           # then allow the registration when enabled
-          assert @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: 'different-uuid')
+          assert @klass.validate_hosts(hosts, @org, @host.name, 'different-uuid', host_uuid_overridden: true)
         end
 
         def test_re_register_build_mode
@@ -101,11 +101,11 @@ def test_re_register_build_mode
           refute @host.build
           Setting[:host_re_register_build_only] = true
 
-          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: nil) }
+          error = assert_raises(Katello::Errors::RegistrationError) { @klass.validate_hosts(hosts, @org, @host.name, @host.uuid, host_uuid_overridden: nil) }
           assert_match(/currently registered/, error.message)
 
           @host.update(build: true)
-          assert @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: 'existing_system_uuid')
+          assert @klass.validate_hosts(hosts, @org, @host.name, 'existing_system_uuid', host_uuid_overridden: true)
         end
 
         def test_existing_uuid_and_name
@@ -118,15 +118,15 @@ def test_build_matching_hostname_new_uuid
           @host = FactoryBot.create(:host, :with_subscription, :managed, organization: @org, build: true)
           @host.subscription_facet.update(dmi_uuid: SecureRandom.uuid)
 
-          assert @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: 'different-uuid')
+          assert @klass.validate_hosts(hosts, @org, @host.name, 'different-uuid', host_uuid_overridden: true)
         end
 
         def test_existing_host_null_uuid
           # this test case is critical for bootstrap.py which creates a host via API (which lacks the dmi uuid fact)
           # and *then* registers to it with subscription-manager
           assert_empty @host.fact_values
 
-          assert @klass.validate_hosts(hosts, @org, @host.name, host_uuid_overridden: 'different-uuid')
+          assert @klass.validate_hosts(hosts, @org, @host.name, 'different-uuid', host_uuid_overridden: true)
         end
       end
 

test/support/controller_support.rb

@@ -67,11 +67,11 @@ def assert_protected_object(action_name, allowed_perms, denied_perms = [],
 
   def assert_authorized(params)
     check_params = params.merge(authorized: true)
-    check_permission(check_params)
+    check_permission(**check_params)
   end
 
   def refute_authorized(params)
     check_params = params.merge(authorized: false)
-    check_permission(check_params)
+    check_permission(**check_params)
   end
 end