diff --git a/lib/facts/debian/os/distro/release.rb b/lib/facts/debian/os/distro/release.rb index 7be674c19..cc7e8c6fb 100644 --- a/lib/facts/debian/os/distro/release.rb +++ b/lib/facts/debian/os/distro/release.rb @@ -13,8 +13,10 @@ def call_the_resolver return Facter::ResolvedFact.new(FACT_NAME, nil) unless fact_value versions = fact_value.split('.') - release = { 'full' => fact_value, 'major' => versions[0], 'minor' => versions[1].gsub(/^0([1-9])/, '\1') } - + release = {} + release['full'] = fact_value + release['major'] = versions[0] + release['minor'] = versions[1].gsub(/^0([1-9])/, '\1') if versions[1] Facter::ResolvedFact.new(FACT_NAME, release) end diff --git a/lib/facts/debian/os/release.rb b/lib/facts/debian/os/release.rb index b3a0db92c..3c1aa5e6d 100644 --- a/lib/facts/debian/os/release.rb +++ b/lib/facts/debian/os/release.rb @@ -13,11 +13,10 @@ def call_the_resolver return Facter::ResolvedFact.new(FACT_NAME, nil) unless fact_value versions = fact_value.split('.') - release = { - 'full' => fact_value, - 'major' => versions[0], - 'minor' => versions[1].gsub(/^0([1-9])/, '\1') - } + release = {} + release['full'] = fact_value + release['major'] = versions[0] + release['minor'] = versions[1].gsub(/^0([1-9])/, '\1') if versions[1] [Facter::ResolvedFact.new(FACT_NAME, release), Facter::ResolvedFact.new(ALIASES.first, versions[0], :legacy), diff --git a/lib/framework/core/fact/internal/internal_fact_manager.rb b/lib/framework/core/fact/internal/internal_fact_manager.rb index 95c4ce2d8..55d70ce47 100644 --- a/lib/framework/core/fact/internal/internal_fact_manager.rb +++ b/lib/framework/core/fact/internal/internal_fact_manager.rb @@ -2,6 +2,8 @@ module Facter class InternalFactManager + @@log = Facter::Log.new(self) + def resolve_facts(searched_facts) searched_facts = filter_internal_facts(searched_facts) @@ -18,14 +20,18 @@ def filter_internal_facts(searched_facts) def start_threads(searched_facts) threads = [] - # only resolve a fact once, even if multiple search facts depend on that fact searched_facts .uniq { |searched_fact| searched_fact.fact_class.name } .each do |searched_fact| threads << Thread.new do - fact = CoreFact.new(searched_fact) - fact.create + begin + fact = CoreFact.new(searched_fact) + fact.create + rescue StandardError => e + @@log.error(e.backtrace.join("\n")) + nil + end end end @@ -37,7 +43,7 @@ def join_threads(threads, searched_facts) threads.each do |thread| thread.join - resolved_facts << thread.value + resolved_facts << thread.value unless thread.value.nil? end resolved_facts.flatten! diff --git a/spec/facter/facts/debian/os/distro/release_spec.rb b/spec/facter/facts/debian/os/distro/release_spec.rb index 59a5f43f5..49457dfb5 100644 --- a/spec/facter/facts/debian/os/distro/release_spec.rb +++ b/spec/facter/facts/debian/os/distro/release_spec.rb @@ -4,79 +4,91 @@ describe '#call_the_resolver' do subject(:fact) { Facts::Debian::Os::Distro::Release.new } + shared_examples 'calls Facter::Resolvers::OsRelease with :name' do + it 'calls Facter::Resolvers::OsRelease with :name' do + fact.call_the_resolver + expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:name) + end + end + + shared_examples 'returns distro release fact' do + it 'returns release fact' do + expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ + have_attributes(name: 'os.distro.release', value: fact_value) + end + end + context 'when os is Ubuntu' do before do - allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(name) - allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:version_id).and_return(value) + allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(os_name) + allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:version_id).and_return(os_release_value) end - let(:name) { 'Ubuntu' } + let(:os_name) { 'Ubuntu' } - context 'when version_id is retrieved successful' do - let(:value) { '18.04' } - let(:value_final) { { 'full' => '18.04', 'major' => '18', 'minor' => '4' } } + context 'when version_id is retrieved successfully' do + let(:os_release_value) { '18.04' } + let(:fact_value) { { 'full' => '18.04', 'major' => '18', 'minor' => '4' } } - it 'calls Facter::Resolvers::OsRelease with :name' do - fact.call_the_resolver - expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:name) - end + it_behaves_like 'calls Facter::Resolvers::OsRelease with :name' it 'calls Facter::Resolvers::OsRelease with :version_id' do fact.call_the_resolver expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:version_id) end - it 'returns release fact' do - expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ - have_attributes(name: 'os.distro.release', value: value_final) - end + it_behaves_like 'returns distro release fact' end - context 'when version_id could not be retrieve' do - let(:value) { nil } + context 'when version_id could not be retrieved' do + let(:os_release_value) { nil } + let(:fact_value) { nil } - it 'returns release fact as nil' do - expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ - have_attributes(name: 'os.distro.release', value: value) - end + it_behaves_like 'returns distro release fact' + end + + context 'when version has no minor' do + let(:os_release_value) { 'experimental_release' } + let(:fact_value) { { 'full' => 'experimental_release', 'major' => 'experimental_release' } } + + it_behaves_like 'returns distro release fact' end end context 'when os is Debian' do - let(:name) { 'Debian' } + let(:os_name) { 'Debian' } before do - allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(name) - allow(Facter::Resolvers::DebianVersion).to receive(:resolve).with(:version).and_return(value) + allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(os_name) + allow(Facter::Resolvers::DebianVersion).to receive(:resolve).with(:version).and_return(os_release_value) end - context 'when version_id is retrieved successful' do - let(:value) { '10.02' } - let(:value_final) { { 'full' => '10.02', 'major' => '10', 'minor' => '2' } } + context 'when version_id is retrieved successfully' do + let(:os_release_value) { '10.02' } + let(:fact_value) { { 'full' => '10.02', 'major' => '10', 'minor' => '2' } } - it 'calls Facter::Resolvers::OsRelease with :name' do - fact.call_the_resolver - expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:name) - end + it_behaves_like 'calls Facter::Resolvers::OsRelease with :name' it 'calls Facter::Resolvers::DebianVersion' do fact.call_the_resolver expect(Facter::Resolvers::DebianVersion).to have_received(:resolve).with(:version) end - it 'returns release fact' do - expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ - have_attributes(name: 'os.distro.release', value: value_final) - end + it_behaves_like 'returns distro release fact' end context 'when version_id could not be retrieve' do - let(:value) { nil } + let(:os_release_value) { nil } + let(:fact_value) { nil } - it 'returns release fact as nil' do - expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ - have_attributes(name: 'os.distro.release', value: value) - end + it_behaves_like 'returns distro release fact' + end + + context 'when version has no minor' do + let(:os_release_value) { 'bullseye/sid' } + let(:fact_value) { { 'full' => 'bullseye/sid', 'major' => 'bullseye/sid' } } + + it_behaves_like 'returns distro release fact' end end end diff --git a/spec/facter/facts/debian/os/release_spec.rb b/spec/facter/facts/debian/os/release_spec.rb index aec3e93bd..47d8a95df 100644 --- a/spec/facter/facts/debian/os/release_spec.rb +++ b/spec/facter/facts/debian/os/release_spec.rb @@ -4,87 +4,104 @@ describe '#call_the_resolver' do subject(:fact) { Facts::Debian::Os::Release.new } + shared_examples 'calls Facter::Resolvers::OsRelease with :name' do + it 'calls Facter::Resolvers::OsRelease with :name' do + fact.call_the_resolver + expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:name) + end + end + + shared_examples 'returns os release fact' do + it 'returns os release fact' do + expect(fact.call_the_resolver).to match_array \ + [ + having_attributes(name: 'os.release', value: fact_value), + having_attributes(name: 'operatingsystemmajrelease', value: fact_value['major'], + type: :legacy), + having_attributes(name: 'operatingsystemrelease', value: fact_value['full'], + type: :legacy) + ] + end + end + + shared_examples 'returns os release fact as nil' do + it 'returns os release fact as nil' do + expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ + have_attributes(name: 'os.release', value: fact_value) + end + end + context 'when os is Ubuntu' do before do - allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(name) - allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:version_id).and_return(value) + allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(os_name) + allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:version_id).and_return(os_release_value) end - let(:name) { 'Ubuntu' } + let(:os_name) { 'Ubuntu' } context 'when version_id is retrieved successful' do - let(:value) { '18.04' } - let(:value_final) { { 'full' => '18.04', 'major' => '18', 'minor' => '4' } } + let(:os_release_value) { '18.04' } + let(:fact_value) { { 'full' => '18.04', 'major' => '18', 'minor' => '4' } } - it 'calls Facter::Resolvers::OsRelease with :name' do - fact.call_the_resolver - expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:name) - end + it_behaves_like 'calls Facter::Resolvers::OsRelease with :name' it 'calls Facter::Resolvers::OsRelease with :version_id' do fact.call_the_resolver expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:version_id) end - it 'returns release fact' do - expect(fact.call_the_resolver).to be_an_instance_of(Array).and \ - contain_exactly(an_object_having_attributes(name: 'os.release', value: value_final), - an_object_having_attributes(name: 'operatingsystemmajrelease', value: value_final['major'], - type: :legacy), - an_object_having_attributes(name: 'operatingsystemrelease', value: value_final['full'], - type: :legacy)) - end + it_behaves_like 'returns os release fact' + end + + context 'when version has no minor' do + let(:os_release_value) { 'bullseye/sid' } + let(:fact_value) { { 'full' => 'bullseye/sid', 'major' => 'bullseye/sid' } } + + it_behaves_like 'returns os release fact' end context 'when version_id could not be retrieve' do - let(:value) { nil } + let(:os_release_value) { nil } + let(:fact_value) { nil } - it 'returns release fact as nil' do - expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ - have_attributes(name: 'os.release', value: value) - end + it_behaves_like 'returns os release fact as nil' end end context 'when os is Debian' do - let(:name) { 'Debian' } + let(:os_name) { 'Debian' } before do - allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(name) - allow(Facter::Resolvers::DebianVersion).to receive(:resolve).with(:version).and_return(value) + allow(Facter::Resolvers::OsRelease).to receive(:resolve).with(:name).and_return(os_name) + allow(Facter::Resolvers::DebianVersion).to receive(:resolve).with(:version).and_return(os_release_value) end context 'when version_id is retrieved successful' do - let(:value) { '10.02' } - let(:value_final) { { 'full' => '10.02', 'major' => '10', 'minor' => '2' } } + let(:os_release_value) { '10.02' } + let(:fact_value) { { 'full' => '10.02', 'major' => '10', 'minor' => '2' } } - it 'calls Facter::Resolvers::OsRelease with :name' do - fact.call_the_resolver - expect(Facter::Resolvers::OsRelease).to have_received(:resolve).with(:name) - end + it_behaves_like 'calls Facter::Resolvers::OsRelease with :name' it 'calls Facter::Resolvers::DebianVersion' do fact.call_the_resolver expect(Facter::Resolvers::DebianVersion).to have_received(:resolve).with(:version) end - it 'returns release fact' do - expect(fact.call_the_resolver).to be_an_instance_of(Array).and \ - contain_exactly(an_object_having_attributes(name: 'os.release', value: value_final), - an_object_having_attributes(name: 'operatingsystemmajrelease', value: value_final['major'], - type: :legacy), - an_object_having_attributes(name: 'operatingsystemrelease', value: value_final['full'], - type: :legacy)) - end + it_behaves_like 'returns os release fact' end - context 'when version_id could not be retrieve' do - let(:value) { nil } + context 'when version has no minor' do + let(:os_release_value) { 'testing/release' } + let(:fact_value) { { 'full' => 'testing/release', 'major' => 'testing/release' } } - it 'returns release fact as nil' do - expect(fact.call_the_resolver).to be_an_instance_of(Facter::ResolvedFact).and \ - have_attributes(name: 'os.release', value: value) - end + it_behaves_like 'returns os release fact' + end + + context 'when version_id could not §be retrieve' do + let(:os_release_value) { nil } + let(:fact_value) { nil } + + it_behaves_like 'returns os release fact as nil' end end end diff --git a/spec/framework/core/fact/internal/internal_fact_manager_spec.rb b/spec/framework/core/fact/internal/internal_fact_manager_spec.rb index d1c093cb8..8fd7298f6 100644 --- a/spec/framework/core/fact/internal/internal_fact_manager_spec.rb +++ b/spec/framework/core/fact/internal/internal_fact_manager_spec.rb @@ -7,11 +7,9 @@ describe '#resolve_facts' do it 'resolved one core fact' do - resolved_fact = mock_resolved_fact('os', 'Ubuntu', nil, []) - + resolved_fact = mock_resolved_fact('os', 'Debian', nil, []) allow(os_name_class_spy).to receive(:new).and_return(os_name_instance_spy) allow(os_name_instance_spy).to receive(:call_the_resolver).and_return(resolved_fact) - searched_fact = instance_spy(Facter::SearchedFact, name: 'os', fact_class: os_name_class_spy, filter_tokens: [], user_query: '', type: :core) @@ -23,12 +21,9 @@ it 'resolved one legacy fact' do networking_interface_class_spy = class_spy(Facts::Windows::NetworkInterfaces) windows_networking_interface = instance_spy(Facts::Windows::NetworkInterfaces) - resolved_fact = mock_resolved_fact('network_Ethernet0', '192.168.5.121', nil, [], :legacy) - allow(networking_interface_class_spy).to receive(:new).and_return(windows_networking_interface) allow(windows_networking_interface).to receive(:call_the_resolver).and_return(resolved_fact) - searched_fact = instance_spy(Facter::SearchedFact, name: 'network_.*', fact_class: networking_interface_class_spy, filter_tokens: [], user_query: '', type: :core) @@ -39,7 +34,7 @@ context 'when there are multiple search facts pointing to the same fact' do before do - resolved_fact = mock_resolved_fact('os', 'Ubuntu', nil, []) + resolved_fact = mock_resolved_fact('os', 'Debian', nil, []) allow(os_name_class_spy).to receive(:new).and_return(os_name_instance_spy) allow(os_name_instance_spy).to receive(:call_the_resolver).and_return(resolved_fact) @@ -58,5 +53,45 @@ expect(os_name_instance_spy).to have_received(:call_the_resolver).once end end + + context 'when fact throws exception' do + let(:searched_fact) do + instance_spy(Facter::SearchedFact, + name: 'os', + fact_class: os_name_class_spy, + filter_tokens: [], + user_query: '', + type: :core) + end + + let(:multi_logger_double) { instance_spy(Facter::MultiLogger) } + + before do + allow(os_name_class_spy).to receive(:new).and_return(os_name_instance_spy) + + exception = StandardError.new('error') + exception.set_backtrace(%w[error backtrace]) + allow(os_name_instance_spy).to receive(:call_the_resolver).and_raise(exception) + + allow(multi_logger_double).to receive(:error) + Facter::Log.class_variable_set(:@@logger, multi_logger_double) + end + + after do + Facter::Log.class_variable_set(:@@logger, Facter::MultiLogger.new([])) + end + + it 'does not store the fact value' do + resolved_facts = internal_fact_manager.resolve_facts([searched_fact]) + + expect(resolved_facts).to match_array [] + end + + it 'logs backtrace as error' do + internal_fact_manager.resolve_facts([searched_fact]) + + expect(multi_logger_double).to have_received(:error).with("Facter::InternalFactManager - error\nbacktrace") + end + end end end