Fix IoP host UUID data consistency issues

app/controllers/concerns/foreman_rh_cloud/registration_manager_extensions.rb

@@ -13,13 +13,13 @@ def unregister_host(host, options = {})
       host.reload
 
       # Only delete from HBI in IoP mode (hosted mode uses async job for cleanup)
-      hbi_host_destroy(host) if ForemanRhCloud.with_iop_smart_proxy? && !organization_destroy && host.insights_facet&.uuid&.presence
+      hbi_host_destroy(host) if ForemanRhCloud.with_iop_smart_proxy? && !organization_destroy && host.insights_uuid.presence
       host.insights&.destroy!
       super(host, options)
     end
 
     def hbi_host_destroy(host)
-      uuid = host.insights_facet.uuid
+      uuid = host.insights_uuid
       logger.debug "Unregistering host #{uuid} from HBI"
       execute_cloud_request(
         organization: host.organization,

app/controllers/concerns/insights_cloud/package_profile_upload_extensions.rb

@@ -21,6 +21,9 @@ def generate_host_report
         @host.id
       )
 
+      # Ensure insights UUID matches subscription UUID (only runs in IoP mode per method guard above)
+      @host.ensure_iop_insights_uuid
+
       # in IoP case, the hosts are identified by the sub-man ID, and we can assume they already
       # exist in the local inventory. This will also handle facet creation for new hosts.
       return if @host.insights

app/models/concerns/rh_cloud_host.rb

@@ -26,5 +26,19 @@ module RhCloudHost
     def insights_facet
       insights
     end
+
+    # In IoP, read directly from the subscription facet to avoid stale data (see comment on ensure_iop_insights_uuid)
+    def insights_uuid
+      ForemanRhCloud.with_iop_smart_proxy? ? subscription_facet&.uuid : insights_facet&.uuid
+    end
+
+    # In non-IoP, insights_facet uuids are assigned by Hosted.
+    # In IoP, insights_facet uuids must match Katello subscription_facet uuids.
+    # If the host was previously registered to hosted Insights,
+    # we need to correct its uuid.
+    def ensure_iop_insights_uuid
+      return unless insights_facet.present? && subscription_facet.present? && insights_facet.uuid != subscription_facet.uuid
+      insights_facet.update!(uuid: subscription_facet.uuid)
+    end
   end
 end

app/views/api/v2/advisor_engine/host_details.json.rabl

@@ -1,9 +1,7 @@
 collection @hosts
 
 attributes :name
-node :insights_uuid do |host|
-  host.insights_facet&.uuid
-end
+node :insights_uuid, &:insights_uuid
 node :insights_hit_details do |host|
   host&.facts('insights::hit_details')&.values&.first
 end

app/views/api/v2/hosts/insights/base.rabl

@@ -1,5 +1,6 @@
-attributes :uuid
-
+node :uuid do |facet|
+  facet&.host&.insights_uuid
+end
 node :insights_hit_details do |facet|
   facet&.host&.facts('insights::hit_details')&.values&.first
 end

lib/foreman_rh_cloud/plugin.rb

@@ -118,6 +118,11 @@ def self.register
           end
         end
 
+        # Preload insights facet to avoid N+1 queries when rendering host list with facets
+        add_controller_action_scope('Api::V2::HostsController', :index) do |base_scope|
+          base_scope.preload(:insights)
+        end
+
         register_global_js_file 'global'
 
         register_custom_status InventorySync::InventoryStatus

lib/insights_cloud/async/connector_playbook_execution_reporter_task.rb

@@ -126,10 +126,10 @@ def report_url
 
       def invocation_status
         hosts_state = Hash[job_invocation.targeting.hosts.map do |host|
-          next unless host.insights&.uuid
+          next unless host.insights_uuid
           [
-            host.insights.uuid,
-            task_status(job_invocation.sub_task_for_host(host), host.insights.uuid),
+            host.insights_uuid,
+            task_status(job_invocation.sub_task_for_host(host), host.insights_uuid),
           ]
         end.compact]
 

test/controllers/insights_cloud/api/advisor_engine_controller_test.rb

@@ -13,14 +13,41 @@ class AdvisorEngineControllerTest < ActionController::TestCase
       end
 
       test 'shows hosts with uuids' do
-        uuids = [@host1.insights.uuid, @host2.insights.uuid]
+        uuids = [@host1.insights_uuid, @host2.insights_uuid]
         get :host_details, params: { organization_id: @test_org.id, host_uuids: uuids }
         assert_response :success
         assert_template 'api/v2/advisor_engine/host_details'
         assert_equal @test_org.hosts.joins(:insights).where(:insights => { :uuid => uuids }).count, assigns(:hosts).count
         refute_equal @test_org.hosts.count, assigns(:hosts).count
       end
 
+      test 'in IoP mode host_details uses subscription uuid when insights uuid is stale' do
+        ForemanRhCloud.stubs(:with_iop_smart_proxy?).returns(true)
+
+        stale_insights_uuid = 'stale-insights-uuid-123'
+        subscription_uuid   = 'subscription-uuid-456'
+
+        # Create host with diverging facet UUIDs
+        host = FactoryBot.create(:host, :with_subscription, organization: @test_org)
+        host.subscription_facet.update!(uuid: subscription_uuid)
+        host.insights = FactoryBot.create(:insights_facet, host_id: host.id, uuid: stale_insights_uuid)
+        host.save!
+
+        # Query using the stale insights UUID
+        get :host_details, params: {
+          organization_id: @test_org.id,
+          host_uuids: [stale_insights_uuid],
+        }
+
+        assert_response :success
+        body = JSON.parse(response.body)
+
+        # Should return the subscription UUID, not the stale insights UUID
+        insights_uuids = body.map { |h| h['insights_uuid'] }
+        assert_includes insights_uuids, subscription_uuid, "Should use subscription UUID in IoP mode"
+        refute_includes insights_uuids, stale_insights_uuid, "Should not use stale insights UUID"
+      end
+
       test 'shows error when no hosts found' do
         get :host_details, params: { organization_id: @test_org.id, host_uuids: ['nonexistentuuid'] }
         assert_response :not_found

test/factories/insights_factories.rb

@@ -58,5 +58,34 @@
         host.insights = FactoryBot.create(:insights_facet, :with_hits, host_id: host.id)
       end
     end
+
+    trait :with_mismatched_insights_uuid do
+      # Simulates host previously registered to cloud with stale UUID
+      # Requires :with_subscription trait to be used together
+      after(:create) do |host, _evaluator|
+        raise "subscription_facet required for :with_mismatched_insights_uuid trait" unless host.subscription_facet&.uuid
+
+        host.insights = FactoryBot.create(
+          :insights_facet,
+          host_id: host.id,
+          uuid: 'stale-cloud-uuid' # Different from subscription_facet
+        )
+      end
+    end
+
+    trait :with_synced_insights_uuid do
+      # Ideal state: insights_facet UUID matches subscription_facet UUID
+      # Requires :with_subscription trait to be used together
+      after(:create) do |host, _evaluator|
+        raise "subscription_facet required for :with_synced_insights_uuid trait" unless host.subscription_facet&.uuid
+
+        subscription_uuid = host.subscription_facet.uuid
+        host.insights = FactoryBot.create(
+          :insights_facet,
+          host_id: host.id,
+          uuid: subscription_uuid  # Same as subscription_facet
+        )
+      end
+    end
   end
 end

test/unit/lib/foreman_rh_cloud/registration_manager_extensions_test.rb

@@ -4,8 +4,8 @@ module ForemanRhCloud
   class RegistrationManagerExtensionsTest < ActiveSupport::TestCase
     setup do
       @org = FactoryBot.create(:organization)
-      @host = FactoryBot.create(:host, :managed, organization: @org)
-      @insights_facet = ::InsightsFacet.create!(host: @host, uuid: 'test-uuid-123')
+      @host = FactoryBot.create(:host, :managed, :with_subscription, organization: @org)
+      @insights_facet = ::InsightsFacet.create!(host: @host, uuid: @host.subscription_facet.uuid)
 
       # Stub Candlepin interaction (from Katello)
       ::Katello::Resources::Candlepin::Consumer.stubs(:destroy)
@@ -15,9 +15,9 @@ class RegistrationManagerExtensionsTest < ActiveSupport::TestCase
     end
 
     context 'unregister_host' do
-      test 'should call HBI delete in IoP mode when host has insights facet with UUID' do
+      test 'should call HBI delete in IoP mode when host has insights UUID' do
         ForemanRhCloud.stubs(:with_iop_smart_proxy?).returns(true)
-        expected_url = ForemanInventoryUpload.host_by_id_url('test-uuid-123')
+        expected_url = ForemanInventoryUpload.host_by_id_url(@host.subscription_facet.uuid)
 
         # Expect the cloud request to be made
         Katello::RegistrationManager.expects(:execute_cloud_request).with do |params|
@@ -45,44 +45,6 @@ class RegistrationManagerExtensionsTest < ActiveSupport::TestCase
         assert_nil InsightsFacet.find_by(id: @insights_facet.id)
       end
 
-      test 'should NOT call HBI delete when host has no insights_facet' do
-        host_without_facet = FactoryBot.create(:host, :managed, organization: @org)
-        ForemanRhCloud.stubs(:with_iop_smart_proxy?).returns(true)
-
-        Katello::RegistrationManager.expects(:execute_cloud_request).never
-
-        assert_nothing_raised do
-          Katello::RegistrationManager.unregister_host(host_without_facet, unregistering: true)
-        end
-      end
-
-      test 'should NOT call HBI delete when host has insights_facet with empty UUID' do
-        host_with_empty_uuid_facet = FactoryBot.create(:host, :managed, organization: @org)
-        empty_uuid_facet = InsightsFacet.create!(host: host_with_empty_uuid_facet, uuid: '')
-        ForemanRhCloud.stubs(:with_iop_smart_proxy?).returns(true)
-
-        Katello::RegistrationManager.expects(:execute_cloud_request).never
-
-        assert_nothing_raised do
-          Katello::RegistrationManager.unregister_host(host_with_empty_uuid_facet, unregistering: true)
-        end
-
-        assert_nil InsightsFacet.find_by(id: empty_uuid_facet.id)
-      end
-
-      test 'should NOT call HBI delete when insights_facet has no UUID' do
-        facet_id = @insights_facet.id
-        @insights_facet.update(uuid: nil)
-        ForemanRhCloud.stubs(:with_iop_smart_proxy?).returns(true)
-
-        Katello::RegistrationManager.expects(:execute_cloud_request).never
-
-        Katello::RegistrationManager.unregister_host(@host, unregistering: true)
-
-        # Verify facet was still destroyed
-        assert_nil InsightsFacet.find_by(id: facet_id)
-      end
-
       test 'should always destroy insights_facet regardless of IoP mode' do
         facet_id = @insights_facet.id
         ForemanRhCloud.stubs(:with_iop_smart_proxy?).returns(false)