Skip to content

Commit 358d4dd

Browse files
authored
Fixes #39213 - Import only OS-detection facts at registration time (#11701)
Import only the registration-time facts needed for immediate OS and architecture detection while preserving additive fact-import behavior. Keep the stricter registration failure handling, remove the unnecessary Candlepin UUID fallback, and align the registration tests with the resource layer's indifferent-access return contract.
1 parent a992083 commit 358d4dd

6 files changed

Lines changed: 100 additions & 25 deletions

File tree

app/models/katello/host/subscription_facet.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,15 @@ def self.new_host_from_facts(facts, org, location)
176176
::Host::Managed.new(:name => name, :organization => org, :location => location, :managed => false)
177177
end
178178

179-
def self.update_facts(host, rhsm_facts)
179+
def self.update_facts(host, rhsm_facts = nil, additive: false, **legacy_rhsm_facts)
180+
rhsm_facts ||= legacy_rhsm_facts.presence
180181
return if host.build? || rhsm_facts.nil?
181182
rhsm_facts[:_type] = RhsmFactName::FACT_TYPE
182183
rhsm_facts[:_timestamp] = Time.now.to_s
183184
if ignore_os?(host.operatingsystem, rhsm_facts)
184185
rhsm_facts[:ignore_os] = true
185186
end
186-
::HostFactImporter.new(host).import_facts(rhsm_facts)
187+
::HostFactImporter.new(host).import_facts(rhsm_facts, additive: additive)
187188
end
188189

189190
def self.ignore_os?(host_os, rhsm_facts)

app/services/katello/registration_manager.rb

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22

33
module Katello
44
class RegistrationManager
5+
# Facts strictly required at registration time so that RhelLifecycleStatus can be
6+
# computed immediately. These drive OS and architecture detection in HostFactImporter.
7+
# All remaining facts arrive via the subsequent PUT /rhsm/consumers/:id call (step 6).
8+
REGISTRATION_CRITICAL_FACTS = %w[
9+
distribution.name
10+
distribution.version
11+
uname.machine
12+
].freeze
13+
514
class << self
615
private :new
716
delegate :propose_existing_hostname, :new_host_from_facts, to: Katello::Host::SubscriptionFacet
@@ -185,20 +194,29 @@ def register_host(host, consumer_params, content_view_environments, activation_k
185194

186195
consumer_data = nil
187196
User.as_anonymous_admin do
188-
consumer_data = begin
189-
create_in_candlepin(host, content_view_environments, consumer_params, activation_keys)
197+
begin
198+
consumer_data = begin
199+
create_in_candlepin(host, content_view_environments, consumer_params, activation_keys)
200+
rescue StandardError => e
201+
# Invalidate the cached Candlepin status immediately so subsequent
202+
# registrations fail fast at check_registration_services rather than
203+
# proceeding through DB writes only to roll back when CP is down.
204+
Katello::Resources::Candlepin::CandlepinPing.clear_cache
205+
raise e
206+
end
207+
208+
# Import only the facts needed for OS/architecture detection so that
209+
# RhelLifecycleStatus is correct immediately after registration.
210+
# The full fact set arrives via PUT /rhsm/consumers/:id (step 6).
211+
import_critical_registration_facts(host, consumer_params[:facts])
212+
213+
finalize_registration(host, consumer_data)
190214
rescue StandardError => e
191-
# Invalidate the cached Candlepin status immediately so subsequent
192-
# registrations fail fast at check_registration_services rather than
193-
# proceeding through DB writes only to roll back when CP is down.
194-
Katello::Resources::Candlepin::CandlepinPing.clear_cache
195215
# we can't call CP here since something bad already happened. Just clean up our DB as best as we can.
196216
host.subscription_facet.try(:destroy!)
197217
new_host ? remove_partially_registered_new_host(host) : remove_host_artifacts(host)
198218
raise e
199219
end
200-
201-
finalize_registration(host, consumer_data)
202220
end
203221

204222
consumer_data
@@ -235,11 +253,11 @@ def remove_partially_registered_new_host(host)
235253
def create_in_candlepin(host, content_view_environments, consumer_params, activation_keys)
236254
# if CP fails, nothing to clean up yet w.r.t. backend services
237255
cp_create = ::Katello::Resources::Candlepin::Consumer.create(content_view_environments.map(&:cp_id), consumer_params, activation_keys.map(&:cp_name), host.organization)
238-
::Katello::Host::SubscriptionFacet.update_facts(host, consumer_params[:facts]) unless consumer_params[:facts].blank?
239-
fail ::Katello::Errors::CandlepinError, _("Candlepin consumer registration response is missing a uuid") if cp_create&.[]('uuid').blank?
240-
if cp_create['uuid'] != host.subscription_facet.uuid
241-
Rails.logger.info(_("Candlepin returned different consumer uuid than requested (%s), updating uuid in subscription_facet.") % cp_create['uuid'])
242-
host.subscription_facet.uuid = cp_create['uuid']
256+
uuid = cp_create['uuid']
257+
fail ::Katello::Errors::CandlepinError, _("Candlepin consumer registration response is missing a uuid") if uuid.blank?
258+
if uuid != host.subscription_facet.uuid
259+
Rails.logger.info(_("Candlepin returned different consumer uuid than requested (%s), updating uuid in subscription_facet.") % uuid)
260+
host.subscription_facet.uuid = uuid
243261
host.subscription_facet.save!
244262
end
245263
cp_create
@@ -316,6 +334,12 @@ def populate_subscription_facet(host, activation_keys, consumer_params, uuid)
316334
subscription_facet
317335
end
318336

337+
def import_critical_registration_facts(host, facts)
338+
critical = facts&.slice(*REGISTRATION_CRITICAL_FACTS)
339+
return if critical.blank?
340+
::Katello::Host::SubscriptionFacet.update_facts(host, critical, additive: true)
341+
end
342+
319343
def remove_host_artifacts(host, clear_content_facet: true, preserve_for_provisioning: false)
320344
Rails.logger.debug "Host ID: #{host.id}, clear_content_facet: #{clear_content_facet}"
321345
if host.content_facet && clear_content_facet

test/services/katello/registration_manager_test.rb

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,56 @@ def test_registration_dead_candlepin_clears_cache
499499
end
500500
end
501501

502+
def test_import_critical_registration_facts_imports_critical_subset
503+
new_host = ::Host::Managed.new(:name => 'foobar.example.com', :managed => false, :organization => @org)
504+
critical_facts = {'distribution.name' => 'RHEL', 'distribution.version' => '9.2', 'uname.machine' => 'x86_64'}
505+
all_facts = critical_facts.merge('network.hostname' => 'foobar.example.com')
506+
507+
::Katello::RegistrationManager.expects(:get_uuid).returns('fake-uuid')
508+
::Katello::Resources::Candlepin::Consumer.expects(:create).returns({'uuid' => 'fake-uuid'}.with_indifferent_access)
509+
::Katello::Host::SubscriptionFacet.any_instance.stubs(:update_hypervisor)
510+
::Katello::Host::SubscriptionFacet.any_instance.stubs(:update_guests)
511+
::Host::Managed.any_instance.stubs(:refresh_statuses)
512+
513+
::Katello::Host::SubscriptionFacet.expects(:update_facts).with(new_host, critical_facts, additive: true)
514+
515+
::Katello::RegistrationManager.register_host(new_host, rhsm_params.merge(:facts => all_facts), [@content_view_environment])
516+
end
517+
518+
def test_import_critical_registration_facts_skips_when_no_critical_facts_present
519+
new_host = ::Host::Managed.new(:name => 'foobar.example.com', :managed => false, :organization => @org)
520+
non_critical_facts = {'network.hostname' => 'foobar.example.com'}
521+
522+
::Katello::RegistrationManager.expects(:get_uuid).returns('fake-uuid')
523+
::Katello::Resources::Candlepin::Consumer.expects(:create).returns({'uuid' => 'fake-uuid'}.with_indifferent_access)
524+
::Katello::Host::SubscriptionFacet.any_instance.stubs(:update_hypervisor)
525+
::Katello::Host::SubscriptionFacet.any_instance.stubs(:update_guests)
526+
::Host::Managed.any_instance.stubs(:refresh_statuses)
527+
528+
::Katello::Host::SubscriptionFacet.expects(:update_facts).never
529+
530+
::Katello::RegistrationManager.register_host(new_host, rhsm_params.merge(:facts => non_critical_facts), [@content_view_environment])
531+
end
532+
533+
def test_import_critical_registration_facts_failure_aborts_registration
534+
new_host = ::Host::Managed.new(:name => 'foobar.example.com', :managed => false, :organization => @org)
535+
critical_facts = {'distribution.name' => 'RHEL', 'uname.machine' => 'x86_64'}
536+
537+
::Katello::RegistrationManager.expects(:get_uuid).returns('fake-uuid')
538+
::Katello::Resources::Candlepin::Consumer.expects(:create).returns({'uuid' => 'fake-uuid'}.with_indifferent_access)
539+
::Katello::Host::SubscriptionFacet.any_instance.stubs(:update_hypervisor)
540+
::Katello::Host::SubscriptionFacet.any_instance.stubs(:update_guests)
541+
::Host::Managed.any_instance.stubs(:refresh_statuses)
542+
::Katello::RegistrationManager.expects(:remove_partially_registered_new_host).with(new_host).once
543+
::Katello::Resources::Candlepin::CandlepinPing.expects(:clear_cache).never
544+
545+
::Katello::Host::SubscriptionFacet.expects(:update_facts).raises(StandardError, 'db failure')
546+
547+
assert_raises(StandardError, 'db failure') do
548+
::Katello::RegistrationManager.register_host(new_host, rhsm_params.merge(:facts => critical_facts), [@content_view_environment])
549+
end
550+
end
551+
502552
# this case can only happen if candlepin/pulp dies after the host is unregistered, but before it's re-registered.
503553
def test_registration_existing_host_dead_backend_service
504554
::Host::Managed.any_instance.stubs(:update_candlepin_associations).twice
@@ -516,7 +566,7 @@ def test_registration_existing_host_dead_backend_service
516566
::Katello::RegistrationManager.register_host(@host, rhsm_params, [@content_view_environment])
517567
end
518568
end
519-
end
569+
end # rubocop:enable Metrics/ClassLength
520570

521571
class CheckRegistrationServicesTest < ActiveSupport::TestCase
522572
def test_delegates_to_candlepin_ping

webpack/scenes/ContentCredentials/Create/CreateContentCredentialForm.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ const CreateContentCredentialForm = ({ setModalOpen, setFormState, refreshTable
113113
<Form>
114114
<FormGroup label={__('Name')} isRequired fieldId="name">
115115
<TextInput
116-
ouiaId="create-content-credential-name-input"
116+
ouiaId="name-input"
117117
isRequired
118118
type="text"
119119
id="name"
@@ -127,7 +127,7 @@ const CreateContentCredentialForm = ({ setModalOpen, setFormState, refreshTable
127127

128128
<FormGroup label={__('Type')} isRequired fieldId="content_type">
129129
<FormSelect
130-
ouiaId="create-content-credential-content-type-select"
130+
ouiaId="content-type-select"
131131
value={contentType}
132132
onChange={(_event, value) => setContentType(value)}
133133
id="content_type"
@@ -174,7 +174,7 @@ const CreateContentCredentialForm = ({ setModalOpen, setFormState, refreshTable
174174
icon={<FileUploadIcon />}
175175
isDisabled={saving}
176176
onClick={() => fileInputRef.current?.click()}
177-
ouiaId="create-content-credential-upload-file-button"
177+
ouiaId="upload-file-button"
178178
>
179179
{file ? file.name : __('Choose file')}
180180
</Button>

webpack/scenes/ContentCredentials/Create/CreateContentCredentialModal.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const CreateContentCredentialModal = ({ show, setIsOpen, refreshTable }) => {
2828
actions={[
2929
<Button
3030
key="create"
31-
ouiaId="create-content-credential-create-button"
31+
ouiaId="create-button"
3232
variant="primary"
3333
onClick={handleSave}
3434
isDisabled={formState.submitDisabled}
@@ -38,7 +38,7 @@ const CreateContentCredentialModal = ({ show, setIsOpen, refreshTable }) => {
3838
</Button>,
3939
<Button
4040
key="cancel"
41-
ouiaId="create-content-credential-cancel-button"
41+
ouiaId="cancel-button"
4242
variant="link"
4343
onClick={handleClose}
4444
>

webpack/scenes/ContentCredentials/Delete/DeleteContentCredentialModal.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@ const DeleteContentCredentialModal = ({
3535

3636
return (
3737
<Modal
38-
ouiaId="delete-content-credential-modal"
38+
ouiaId="content-credential-delete-modal"
3939
variant={ModalVariant.small}
4040
title={[
4141
<Flex key="delete-modal-header">
4242
<Icon status="warning" key="exclamation-triangle">
4343
<ExclamationTriangleIcon />
4444
</Icon>
4545
<Title
46-
ouiaId="delete-content-credential-title"
46+
ouiaId="content-credential-delete-header"
4747
key="delete-content-credential-title"
4848
headingLevel="h5"
4949
size="2xl"
@@ -56,7 +56,7 @@ const DeleteContentCredentialModal = ({
5656
onClose={handleModalToggle}
5757
actions={[
5858
<Button
59-
ouiaId="delete-content-credential-delete-button"
59+
ouiaId="delete-button"
6060
key="delete"
6161
variant="danger"
6262
isDisabled={!credentialId}
@@ -65,7 +65,7 @@ const DeleteContentCredentialModal = ({
6565
{__('Delete')}
6666
</Button>,
6767
<Button
68-
ouiaId="delete-content-credential-cancel-button"
68+
ouiaId="cancel-button"
6969
key="cancel"
7070
variant="link"
7171
onClick={handleModalToggle}

0 commit comments

Comments
 (0)