(FACT-2565) Debian development versions causes fatal error when resol…

…ving os.release (#466)

* (FACT-2565) fixed fact for when release does not have minor version
refactored unit tests
* (FACT-2565) when fact throws exception, it's value is not stored and the failure is logged
* (FACT-2565) simplified condition for release minor
* (FACT-2565) refactore release hash

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
 

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),

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!

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

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