Fixes #39299 - Add additive fact import mode

app/services/fact_importer.rb

@@ -28,17 +28,17 @@ def initialize(host, facts = {})
     @error    = false
     @host     = host
     @facts    = normalize(facts)
-    @counters = {}
+    @counters = { added: 0, updated: 0, deleted: 0 }
   end
 
   # expect a facts hash
-  def import!
+  def import!(additive: false)
     # This function uses its own transactions that should not be included
     # in the transaction that handles fact values
     ensure_fact_names
 
     ActiveRecord::Base.transaction do
-      delete_removed_facts
+      delete_removed_facts unless additive
       update_facts
       add_new_facts
     end

app/services/host_fact_importer.rb

@@ -11,7 +11,7 @@ def create_new_record_when_facts_are_uploaded?
     Setting[:create_new_host_when_facts_are_uploaded]
   end
 
-  def import_facts(facts, source_proxy = nil)
+  def import_facts(facts, source_proxy = nil, additive: false)
     return false if !create_new_record_when_facts_are_uploaded? && host.new_record?
 
     # we are not importing facts for hosts in build state (e.g. waiting for a re-installation)
@@ -24,7 +24,7 @@ def import_facts(facts, source_proxy = nil)
     facts_importer = Foreman::Plugin.fact_importer_registry.get(type).new(host, facts)
     telemetry_observe_histogram(:importer_facts_import_duration, facts.size, type: type)
     telemetry_duration_histogram(:importer_facts_import_duration, 1000, type: type) do
-      facts_importer.import!
+      facts_importer.import!(additive: additive)
     end
 
     skipping_orchestration do

test/unit/fact_importer_test.rb

@@ -143,6 +143,25 @@ def fact_name_class
       assert_equal 0, importer.counters[:added]
     end
 
+    test 'additive import preserves existing facts not in the new set' do
+      # baseline: two facts in DB
+      assert_equal '2.6.9', value('kernelversion')
+      assert_equal '10.0.19.33', value('ipaddress')
+
+      # additive import of a single new fact — existing facts must survive
+      @importer = FactImporter.new(host, 'uptime' => '1 day')
+      allow_transactions_for @importer
+      @importer.stubs(:fact_name_class).returns(FactName)
+      @importer.import!(additive: true)
+
+      assert_equal '2.6.9', value('kernelversion')
+      assert_equal '10.0.19.33', value('ipaddress')
+      assert_equal '1 day', value('uptime')
+      assert_equal 0, @importer.counters[:deleted]
+      assert_equal 1, @importer.counters[:added]
+      assert_equal 0, @importer.counters[:updated]
+    end
+
     test 'importer updates fact values' do
       assert_equal '2.6.9', value('kernelversion')
       assert_equal '10.0.19.33', value('ipaddress')

test/unit/host_fact_importer_test.rb

@@ -47,7 +47,7 @@ class HostFactImporterTest < ActiveSupport::TestCase
 
   test 'import_facts only needs operatingsystem and operatingsystemrelease fact' do
     host = Host.import_host('host', 'puppet')
-    assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS')
+    assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS'})
   end
 
   test 'should import facts from json of a new host when certname is not specified' do
@@ -70,17 +70,17 @@ class HostFactImporterTest < ActiveSupport::TestCase
   test 'domain updated from facts' do
     host = FactoryBot.create(:host, :with_operatingsystem)
     FactoryBot.create(:domain, :name => 'foo.bar')
-    assert HostFactImporter.new(host).import_facts(:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name)
+    assert HostFactImporter.new(host).import_facts({:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name})
     assert_equal 'foo.bar', host.domain.to_s
   end
 
   test 'domain not updated from facts when ignore_facts_for_domain false' do
     domain = FactoryBot.create(:domain)
     host = FactoryBot.create(:host, :managed, :domain => domain)
     FactoryBot.create(:domain, :name => 'foo.bar')
-    assert HostFactImporter.new(host).import_facts(:domain => domain.name, :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name)
+    assert HostFactImporter.new(host).import_facts({:domain => domain.name, :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name})
     Setting[:ignore_facts_for_domain] = true
-    assert HostFactImporter.new(host).import_facts(:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name)
+    assert HostFactImporter.new(host).import_facts({:domain => 'foo.bar', :operatingsystemrelease => host.operatingsystem.release, :operatingsystem => host.operatingsystem.name})
     assert_equal domain.name, host.domain.name
     Setting[:ignore_facts_for_domain] =
       Setting.find_by_name('ignore_facts_for_domain').default
@@ -236,13 +236,13 @@ class HostFactImporterTest < ActiveSupport::TestCase
 
   test 'comment updated from facts when present' do
     host = Host.import_host('host')
-    assert HostFactImporter.new(host).import_facts(foreman_comment: 'new comment', operatingsystemrelease: '6.7', operatingsystem: 'CentOS')
+    assert HostFactImporter.new(host).import_facts({foreman_comment: 'new comment', operatingsystemrelease: '6.7', operatingsystem: 'CentOS'})
     assert_equal 'new comment', host.comment
   end
 
   test 'operatingsystem updated from facts' do
     host = Host.import_host('host')
-    assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS')
+    assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS'})
     assert_equal 'CentOS 6.7', host.operatingsystem.to_s
   end
 
@@ -251,7 +251,7 @@ class HostFactImporterTest < ActiveSupport::TestCase
     os2 = FactoryBot.create(:operatingsystem)
     medium = os1.media.first
     host = FactoryBot.create(:host, :operatingsystem => os1, :medium => medium)
-    HostFactImporter.new(host).import_facts(:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s)
+    HostFactImporter.new(host).import_facts({:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s})
 
     assert_equal host.operatingsystem, os2
     assert_nil host.medium
@@ -264,17 +264,17 @@ class HostFactImporterTest < ActiveSupport::TestCase
     medium.operatingsystems << os2
 
     host = FactoryBot.create(:host, :operatingsystem => os1, :medium => medium)
-    HostFactImporter.new(host).import_facts(:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s)
+    HostFactImporter.new(host).import_facts({:operatingsystem => os2.name, :operatingsystemrelease => os2.major.to_s})
 
     assert_equal host.operatingsystem, os2
     assert_equal host.medium, medium
   end
 
   test 'operatingsystem not updated from facts when ignore_facts_for_operatingsystem false' do
     host = Host.import_host('host')
-    assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS')
+    assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.7', :operatingsystem => 'CentOS'})
     Setting[:ignore_facts_for_operatingsystem] = true
-    assert HostFactImporter.new(host).import_facts(:operatingsystemrelease => '6.8', :operatingsystem => 'CentOS')
+    assert HostFactImporter.new(host).import_facts({:operatingsystemrelease => '6.8', :operatingsystem => 'CentOS'})
     assert_equal 'CentOS 6.7', host.operatingsystem.to_s
     Setting[:ignore_facts_for_operatingsystem] =
       Setting.find_by_name('ignore_facts_for_operatingsystem').default
@@ -360,7 +360,7 @@ class HostFactImporterTest < ActiveSupport::TestCase
           payload[:object].operatingsystem.name == 'CentOS'
         end
 
-        HostFactImporter.new(host).import_facts(operatingsystemrelease: '6.7', operatingsystem: 'CentOS')
+        HostFactImporter.new(host).import_facts({operatingsystemrelease: '6.7', operatingsystem: 'CentOS'})
       end
     end
   end