From a3c67d8afb2d89af42b4ec736f459d768f28cbd8 Mon Sep 17 00:00:00 2001 From: Oleh Fedorenko Date: Thu, 18 Jan 2024 16:18:01 +0000 Subject: [PATCH] Ruby 3 support --- .../actions/katello/content_view/publish.rb | 4 ++-- .../katello/content_view_version/export.rb | 2 +- app/lib/katello/lazy_accessor.rb | 12 +++++------ .../pulp3/content_view_version/export.rb | 6 +++--- katello.gemspec | 5 ++++- .../controllers/api/v2/api_controller_test.rb | 4 ++-- test/katello_test_helper.rb | 4 ++-- .../activation_key_authorization_test.rb | 4 ++-- ...ifecycle_environment_authorization_test.rb | 8 ++++---- .../organization_authorization_test.rb | 8 ++++---- .../product_authorization_test.rb | 4 ++-- .../repository_authorization_test.rb | 8 ++++---- .../katello/registration_manager_test.rb | 20 +++++++++---------- test/support/controller_support.rb | 4 ++-- 14 files changed, 48 insertions(+), 45 deletions(-) diff --git a/app/lib/actions/katello/content_view/publish.rb b/app/lib/actions/katello/content_view/publish.rb index d41914c66f8..a6bca8af910 100644 --- a/app/lib/actions/katello/content_view/publish.rb +++ b/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 @@ -57,7 +57,7 @@ def plan(content_view, description = "", options = {importing: false, syncable: separated_repo_map = separated_repo_mapping(repository_mapping, content_view.solve_dependencies) if options[:importing] - handle_import(version, options.slice(:path, :metadata)) + handle_import(version, **options.slice(:path, :metadata)) elsif separated_repo_map[:pulp3_yum_multicopy].keys.flatten.present? plan_action(Repository::MultiCloneToVersion, separated_repo_map[:pulp3_yum_multicopy], version) end diff --git a/app/lib/actions/katello/content_view_version/export.rb b/app/lib/actions/katello/content_view_version/export.rb index bb6ad89a768..9aeba112bee 100644 --- a/app/lib/actions/katello/content_view_version/export.rb +++ b/app/lib/actions/katello/content_view_version/export.rb @@ -11,7 +11,7 @@ def plan(options) action_subject(options.fetch(:content_view_version)) sequence do - export_output = plan_action(::Actions::Pulp3::Orchestration::ContentViewVersion::Export, options).output + export_output = plan_action(::Actions::Pulp3::Orchestration::ContentViewVersion::Export, **options).output plan_self(export_history_id: export_output[:export_history_id], exported_file_checksum: export_output[:exported_file_checksum], diff --git a/app/lib/katello/lazy_accessor.rb b/app/lib/katello/lazy_accessor.rb index ed6821cd35b..7dd454aba32 100644 --- a/app/lib/katello/lazy_accessor.rb +++ b/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 diff --git a/app/services/katello/pulp3/content_view_version/export.rb b/app/services/katello/pulp3/content_view_version/export.rb index aca1565f409..95df4bb0475 100644 --- a/app/services/katello/pulp3/content_view_version/export.rb +++ b/app/services/katello/pulp3/content_view_version/export.rb @@ -8,11 +8,11 @@ class Export FORMATS = [SYNCABLE, IMPORTABLE].freeze attr_reader :smart_proxy, :content_view_version, :destination_server, :from_content_view_version, :repository, :base_path - def self.create(options) + def self.create(**options) if options.delete(:format) == SYNCABLE - SyncableFormatExport.new(options) + SyncableFormatExport.new(**options) else - self.new(options) + self.new(**options) end end diff --git a/katello.gemspec b/katello.gemspec index eb203233933..345a6227e5c 100644 --- a/katello.gemspec +++ b/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', '< 3.1' gem.files = Dir["{app,webpack,vendor,lib,db,ca,config,locale}/**/*"] + Dir['LICENSE.txt', 'README.md', 'package.json'] @@ -49,6 +49,9 @@ Gem::Specification.new do |gem| # Pulp gem.add_dependency "anemone" + # required by anemone: https://github.com/chriskite/anemone/blob/next/lib/anemone/page.rb#L3 + # webrick is no longer a part of Ruby's stdlib: https://bugs.ruby-lang.org/issues/17303 + gem.add_dependency "webrick" #pulp3 gem.add_dependency "pulpcore_client", ">= 3.39.0", "< 3.40.0" diff --git a/test/controllers/api/v2/api_controller_test.rb b/test/controllers/api/v2/api_controller_test.rb index ba60a39bb98..41ee6a3e5f6 100644 --- a/test/controllers/api/v2/api_controller_test.rb +++ b/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) diff --git a/test/katello_test_helper.rb b/test/katello_test_helper.rb index f91159cec31..9b65f319a21 100644 --- a/test/katello_test_helper.rb +++ b/test/katello_test_helper.rb @@ -187,8 +187,8 @@ def assert_response(type, message = nil) module DynflowFullTreePlanning IGNORED_INPUT = [:remote_user, :remote_cp_user].freeze - def plan_action_tree(action_class, *args) - Rails.application.dynflow.world.plan(action_class, *args) + def plan_action_tree(action_class, *args, **kwargs) + Rails.application.dynflow.world.plan(action_class, *args, **kwargs) end def assert_tree_planned_steps(execution_plan, action_class) diff --git a/test/models/authorization/activation_key_authorization_test.rb b/test/models/authorization/activation_key_authorization_test.rb index 077fa362b54..03fd3f33f12 100644 --- a/test/models/authorization/activation_key_authorization_test.rb +++ b/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 diff --git a/test/models/authorization/lifecycle_environment_authorization_test.rb b/test/models/authorization/lifecycle_environment_authorization_test.rb index 53fe99434e9..7b345cbe7e8 100644 --- a/test/models/authorization/lifecycle_environment_authorization_test.rb +++ b/test/models/authorization/lifecycle_environment_authorization_test.rb @@ -75,8 +75,8 @@ 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? @@ -84,8 +84,8 @@ def test_readables 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? diff --git a/test/models/authorization/organization_authorization_test.rb b/test/models/authorization/organization_authorization_test.rb index 1d1eb815f9d..386a075f19a 100644 --- a/test/models/authorization/organization_authorization_test.rb +++ b/test/models/authorization/organization_authorization_test.rb @@ -42,8 +42,8 @@ def test_read_promotion_paths def test_read_promotion_paths_one 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}\"" }) refute_equal(@org.promotion_paths, @org.readable_promotion_paths) assert_equal(1, @org.readable_promotion_paths.size) @@ -51,8 +51,8 @@ def test_read_promotion_paths_one def test_promotable_promotion_paths_one 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}\"" }) refute_equal(@org.promotion_paths, @org.promotable_promotion_paths) assert_equal(1, @org.promotable_promotion_paths.size) diff --git a/test/models/authorization/product_authorization_test.rb b/test/models/authorization/product_authorization_test.rb index 24419cc3156..c47c92c4129 100644 --- a/test/models/authorization/product_authorization_test.rb +++ b/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( diff --git a/test/models/authorization/repository_authorization_test.rb b/test/models/authorization/repository_authorization_test.rb index 4d0ab927af2..16411045eb8 100644 --- a/test/models/authorization/repository_authorization_test.rb +++ b/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 } diff --git a/test/services/katello/registration_manager_test.rb b/test/services/katello/registration_manager_test.rb index ab71df39461..32564df0db7 100644 --- a/test/services/katello/registration_manager_test.rb +++ b/test/services/katello/registration_manager_test.rb @@ -42,14 +42,14 @@ 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) } 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) } assert_match(/matches other registered/, error.message) end @@ -57,7 +57,7 @@ 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) } assert_match(/matches other registered/, error.message) end @@ -65,7 +65,7 @@ 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') } 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') } 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') 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, 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') end def test_existing_uuid_and_name @@ -118,7 +118,7 @@ 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') end def test_existing_host_null_uuid @@ -126,7 +126,7 @@ def test_existing_host_null_uuid # 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') end end diff --git a/test/support/controller_support.rb b/test/support/controller_support.rb index 7cb205921d4..dea1a1f1d30 100644 --- a/test/support/controller_support.rb +++ b/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