Skip to content
This repository has been archived by the owner on Jun 19, 2020. It is now read-only.

(FACT-2565) Debian development versions causes fatal error when resolving os.release #466

Merged
merged 7 commits into from
Apr 24, 2020
Merged

(FACT-2565) Debian development versions causes fatal error when resolving os.release #466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5444a39
(FACT-2565) fixed fact for when release does not have minor version
Apr 22, 2020
1c0f068
(FACT-2565) when fact throws exception, it's value is not stored and …
Apr 22, 2020
04a0c5b
(FACT-2565) simplified condition for release minor
Apr 22, 2020
6f29c46
Merge branch 'master' into FACT-2565
Apr 24, 2020
548be28
(FACT-2565) refactore release hash
Apr 24, 2020
4e50d8b
Merge remote-tracking branch 'origin/FACT-2565' into FACT-2565
Apr 24, 2020
fcb0520
Merge branch 'master' into FACT-2565
Filipovici-Andrei Apr 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/facts/debian/os/distro/release.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 4 additions & 5 deletions lib/facts/debian/os/release.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
14 changes: 10 additions & 4 deletions lib/framework/core/fact/internal/internal_fact_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Comment on lines +28 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will catch all the errors, even dev errors and unhandled cases even from resolvers and will just output a backtrace. I don't think this is the way to go for this situation.
If i call a method on nil in the resolver, this part will catch it for example and return nil, passing nil around and generating more errors.

end
end

Expand All @@ -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!
Expand Down
90 changes: 51 additions & 39 deletions spec/facter/facts/debian/os/distro/release_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
107 changes: 62 additions & 45 deletions spec/facter/facts/debian/os/release_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading