Skip to content

Commit

Permalink
Ruby 3 support
Browse files Browse the repository at this point in the history
  • Loading branch information
ofedoren committed Jan 18, 2024
1 parent 4439004 commit a3c67d8
Show file tree
Hide file tree
Showing 14 changed files with 48 additions and 45 deletions.
4 changes: 2 additions & 2 deletions app/lib/actions/katello/content_view/publish.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/lib/actions/katello/content_view_version/export.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
12 changes: 6 additions & 6 deletions app/lib/katello/lazy_accessor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions app/services/katello/pulp3/content_view_version/export.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion katello.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions test/controllers/api/v2/api_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions test/katello_test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
8 changes: 4 additions & 4 deletions test/models/authorization/organization_authorization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,17 @@ 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)
end

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)
Expand Down
4 changes: 2 additions & 2 deletions test/models/authorization/product_authorization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 4 additions & 4 deletions test/models/authorization/repository_authorization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
20 changes: 10 additions & 10 deletions test/services/katello/registration_manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

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

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

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
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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')
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')
end
end

Expand Down
4 changes: 2 additions & 2 deletions test/support/controller_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit a3c67d8

Please sign in to comment.