From bac44f7936b1868f52057368be24363c97dd6815 Mon Sep 17 00:00:00 2001 From: Robert Waffen Date: Fri, 14 Jun 2024 15:33:16 +0200 Subject: [PATCH 1/5] Extend fact to handle debian packages too Signed-off-by: Robert Waffen --- lib/facter/puppetdb_version.rb | 23 +++++++++++++++++++++-- spec/unit/facter/puppetdb_version_spec.rb | 22 ++++++++++++++++++++-- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/lib/facter/puppetdb_version.rb b/lib/facter/puppetdb_version.rb index b04fb26c..0e7014eb 100644 --- a/lib/facter/puppetdb_version.rb +++ b/lib/facter/puppetdb_version.rb @@ -2,7 +2,26 @@ confine { Facter::Util::Resolution.which('puppetdb') } setcode do - output = Facter::Core::Execution.execute('puppetdb --version') - output.split(':').last.strip + require 'open3' + + # check if os is debian/ubuntu and the package is not from puppetlabs + if Facter.value(:osfamily) == 'Debian' + package_maintainer = Facter::Core::Execution.execute('apt-cache show puppetdb | grep "Maintainer:" | head -1') + unless package_maintainer.include? 'Puppet Labs' + output, status = Open3.capture2('dpkg-query --showformat=\'${Version}\' --show puppetdb') + if status.success? + output.strip.split('-').first + else + nil + end + end + else + output, status = Open3.capture2('puppetdb --version') + if status.success? + output.split(':').last.strip + else + nil + end + end end end diff --git a/spec/unit/facter/puppetdb_version_spec.rb b/spec/unit/facter/puppetdb_version_spec.rb index 3c2783fe..53cc63b8 100644 --- a/spec/unit/facter/puppetdb_version_spec.rb +++ b/spec/unit/facter/puppetdb_version_spec.rb @@ -10,13 +10,31 @@ Facter.clear end - it 'returns the correct puppetdb version' do + it 'returns a version on non-Debian family with puppetlabs package' do allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/bin/puppetdb') - allow(Facter::Core::Execution).to receive(:execute).with('puppetdb --version').and_return("puppetdb version: 7.18.0\n") + allow(Open3).to receive(:capture2).with('puppetdb --version').and_return("puppetdb version: 7.18.0\n") expect(Facter.fact(:puppetdb_version).value).to eq('7.18.0') end + it 'returns a version on Debian family with non-puppetlabs package' do + allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/sbin/puppetdb') + allow(Facter).to receive(:value).with(:osfamily).and_return('Debian') + allow(Facter::Core::Execution).to receive(:execute).with('apt-cache show puppetdb | grep "Maintainer:" | head -1').and_return('Maintainer: Ubuntu Developers') + allow(Open3).to receive(:capture2).with('dpkg-query --showformat=\'${Version}\' --show puppetdb').and_return("6.2.0-5") + + expect(Facter.fact(:puppetdb_version).value).to eq('6.2.0') + end + + it 'returns a version on Debian family with puppetlabs package' do + allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/sbin/puppetdb') + allow(Facter).to receive(:value).with(:osfamily).and_return('Debian') + allow(Facter::Core::Execution).to receive(:execute).with('apt-cache show puppetdb | grep "Maintainer:" | head -1').and_return('Maintainer: Puppet Labs') + allow(Open3).to receive(:capture2).with('dpkg-query --showformat=\'${Version}\' --show puppetdb').and_return("7.19.0-1jammy") + + expect(Facter.fact(:puppetdb_version).value).to eq('7.19.0') + end + it 'returns nil if puppetdb command is not available' do allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return(nil) From 1a5ee16fabee64dd2c413d6f3b369fc4b129deed Mon Sep 17 00:00:00 2001 From: Robert Waffen Date: Fri, 14 Jun 2024 15:38:14 +0200 Subject: [PATCH 2/5] add open3 require to spec Signed-off-by: Robert Waffen --- spec/unit/facter/puppetdb_version_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/unit/facter/puppetdb_version_spec.rb b/spec/unit/facter/puppetdb_version_spec.rb index 53cc63b8..55da2c61 100644 --- a/spec/unit/facter/puppetdb_version_spec.rb +++ b/spec/unit/facter/puppetdb_version_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' require 'facter' +require 'open3' describe 'puppetdb_version' do subject(:fact) { Facter.fact(:puppetdb_version) } From 0faf2ce971ad2a87caf458659532f878616b0692 Mon Sep 17 00:00:00 2001 From: Robert Waffen Date: Fri, 14 Jun 2024 15:47:04 +0200 Subject: [PATCH 3/5] Update lib/facter/puppetdb_version.rb Co-authored-by: Tim Meusel --- lib/facter/puppetdb_version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/facter/puppetdb_version.rb b/lib/facter/puppetdb_version.rb index 0e7014eb..3e367f12 100644 --- a/lib/facter/puppetdb_version.rb +++ b/lib/facter/puppetdb_version.rb @@ -5,7 +5,7 @@ require 'open3' # check if os is debian/ubuntu and the package is not from puppetlabs - if Facter.value(:osfamily) == 'Debian' + if Facter.value(:os)('family') == 'Debian' package_maintainer = Facter::Core::Execution.execute('apt-cache show puppetdb | grep "Maintainer:" | head -1') unless package_maintainer.include? 'Puppet Labs' output, status = Open3.capture2('dpkg-query --showformat=\'${Version}\' --show puppetdb') From 17ff4e28518755a14e730abc2dc9de2798890626 Mon Sep 17 00:00:00 2001 From: Robert Waffen Date: Mon, 17 Jun 2024 09:11:21 +0200 Subject: [PATCH 4/5] Refactor code again: - use Facter-functions Signed-off-by: Robert Waffen --- lib/facter/puppetdb_version.rb | 31 ++++--- spec/unit/facter/puppetdb_version_spec.rb | 103 ++++++++++++++++------ 2 files changed, 91 insertions(+), 43 deletions(-) diff --git a/lib/facter/puppetdb_version.rb b/lib/facter/puppetdb_version.rb index 3e367f12..69aecea9 100644 --- a/lib/facter/puppetdb_version.rb +++ b/lib/facter/puppetdb_version.rb @@ -2,26 +2,25 @@ confine { Facter::Util::Resolution.which('puppetdb') } setcode do - require 'open3' + command = 'puppetdb --version' + splitter = ':' + postion = 'last' - # check if os is debian/ubuntu and the package is not from puppetlabs - if Facter.value(:os)('family') == 'Debian' + if Facter.value(:os)['family'] == 'Debian' package_maintainer = Facter::Core::Execution.execute('apt-cache show puppetdb | grep "Maintainer:" | head -1') + unless package_maintainer.include? 'Puppet Labs' - output, status = Open3.capture2('dpkg-query --showformat=\'${Version}\' --show puppetdb') - if status.success? - output.strip.split('-').first - else - nil - end - end - else - output, status = Open3.capture2('puppetdb --version') - if status.success? - output.split(':').last.strip - else - nil + command = 'dpkg-query --showformat=\'${Version}\' --show puppetdb' + splitter = '-' + postion = 'first' end end + + begin + output = Facter::Core::Execution.execute(command) + output.split(splitter).send(postion).strip + rescue Facter::Core::Execution::ExecutionFailure + nil + end end end diff --git a/spec/unit/facter/puppetdb_version_spec.rb b/spec/unit/facter/puppetdb_version_spec.rb index 55da2c61..e63e258a 100644 --- a/spec/unit/facter/puppetdb_version_spec.rb +++ b/spec/unit/facter/puppetdb_version_spec.rb @@ -1,44 +1,93 @@ -# frozen_string_literal: true - -require 'spec_helper' require 'facter' -require 'open3' describe 'puppetdb_version' do - subject(:fact) { Facter.fact(:puppetdb_version) } - before(:each) do Facter.clear end - it 'returns a version on non-Debian family with puppetlabs package' do - allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/bin/puppetdb') - allow(Open3).to receive(:capture2).with('puppetdb --version').and_return("puppetdb version: 7.18.0\n") + context 'when puppetdb is available' do + before do + allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/bin/puppetdb') + end - expect(Facter.fact(:puppetdb_version).value).to eq('7.18.0') - end + context 'on a Debian-based system' do + before do + allow(Facter).to receive(:value).with(:os).and_return({ 'family' => 'Debian' }) + end - it 'returns a version on Debian family with non-puppetlabs package' do - allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/sbin/puppetdb') - allow(Facter).to receive(:value).with(:osfamily).and_return('Debian') - allow(Facter::Core::Execution).to receive(:execute).with('apt-cache show puppetdb | grep "Maintainer:" | head -1').and_return('Maintainer: Ubuntu Developers') - allow(Open3).to receive(:capture2).with('dpkg-query --showformat=\'${Version}\' --show puppetdb').and_return("6.2.0-5") + context 'when Puppet Labs is the maintainer' do + before do + allow(Facter::Core::Execution).to receive(:execute) + .with('apt-cache show puppetdb | grep "Maintainer:" | head -1') + .and_return('Maintainer: Puppet Labs') + end - expect(Facter.fact(:puppetdb_version).value).to eq('6.2.0') - end + it 'returns the correct version from puppetdb --version' do + expect(Facter::Core::Execution).to receive(:execute) + .with('puppetdb --version') + .and_return('puppetdb version: 7.19.0') + + expect(Facter.fact(:puppetdb_version).value).to eq('7.19.0') + end + + it 'returns nil if the command execution fails' do + allow(Facter::Core::Execution).to receive(:execute).with('puppetdb --version').and_raise(Facter::Core::Execution::ExecutionFailure) + + expect(Facter.fact(:puppetdb_version).value).to be_nil + end + end + + context 'when Puppet Labs is not the maintainer' do + before do + allow(Facter::Core::Execution).to receive(:execute) + .with('apt-cache show puppetdb | grep "Maintainer:" | head -1') + .and_return('Maintainer: Other Maintainer') + end + + it 'returns the correct version from dpkg-query' do + expect(Facter::Core::Execution).to receive(:execute) + .with("dpkg-query --showformat='${Version}' --show puppetdb") + .and_return('7.9.0-1ubuntu1') + + expect(Facter.fact(:puppetdb_version).value).to eq('7.9.0') + end + + it 'returns nil if the command execution fails' do + allow(Facter::Core::Execution).to receive(:execute).with("dpkg-query --showformat='${Version}' --show puppetdb").and_raise(Facter::Core::Execution::ExecutionFailure) + + expect(Facter.fact(:puppetdb_version).value).to be_nil + end + end + end + + context 'on a non-Debian-based system' do + before do + allow(Facter).to receive(:value).with(:os).and_return({ 'family' => 'RedHat' }) + end + + it 'returns the correct version from puppetdb --version' do + expect(Facter::Core::Execution).to receive(:execute) + .with('puppetdb --version') + .and_return('puppetdb version: 7.19.0') + + expect(Facter.fact(:puppetdb_version).value).to eq('7.19.0') + end - it 'returns a version on Debian family with puppetlabs package' do - allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/sbin/puppetdb') - allow(Facter).to receive(:value).with(:osfamily).and_return('Debian') - allow(Facter::Core::Execution).to receive(:execute).with('apt-cache show puppetdb | grep "Maintainer:" | head -1').and_return('Maintainer: Puppet Labs') - allow(Open3).to receive(:capture2).with('dpkg-query --showformat=\'${Version}\' --show puppetdb').and_return("7.19.0-1jammy") + it 'returns nil if the command execution fails' do + allow(Facter::Core::Execution).to receive(:execute).with('puppetdb --version').and_raise(Facter::Core::Execution::ExecutionFailure) - expect(Facter.fact(:puppetdb_version).value).to eq('7.19.0') + expect(Facter.fact(:puppetdb_version).value).to be_nil + end + end end - it 'returns nil if puppetdb command is not available' do - allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return(nil) + context 'when puppetdb is not available' do + before do + allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return(nil) + end - expect(Facter.fact(:puppetdb_version).value).to be_nil + it 'returns nil' do + expect(Facter.fact(:puppetdb_version).value).to be_nil + end end end From a535c4d8996aba86db98de458394d2fd60e16b64 Mon Sep 17 00:00:00 2001 From: Robert Waffen Date: Fri, 28 Jun 2024 10:34:55 +0200 Subject: [PATCH 5/5] Drop work on also handling debian package. - just report nil if command fails Signed-off-by: Robert Waffen --- lib/facter/puppetdb_version.rb | 24 ++------- spec/unit/facter/puppetdb_version_spec.rb | 60 ++--------------------- 2 files changed, 7 insertions(+), 77 deletions(-) diff --git a/lib/facter/puppetdb_version.rb b/lib/facter/puppetdb_version.rb index 69aecea9..097f4cc8 100644 --- a/lib/facter/puppetdb_version.rb +++ b/lib/facter/puppetdb_version.rb @@ -2,25 +2,9 @@ confine { Facter::Util::Resolution.which('puppetdb') } setcode do - command = 'puppetdb --version' - splitter = ':' - postion = 'last' - - if Facter.value(:os)['family'] == 'Debian' - package_maintainer = Facter::Core::Execution.execute('apt-cache show puppetdb | grep "Maintainer:" | head -1') - - unless package_maintainer.include? 'Puppet Labs' - command = 'dpkg-query --showformat=\'${Version}\' --show puppetdb' - splitter = '-' - postion = 'first' - end - end - - begin - output = Facter::Core::Execution.execute(command) - output.split(splitter).send(postion).strip - rescue Facter::Core::Execution::ExecutionFailure - nil - end + output = Facter::Core::Execution.execute('puppetdb --version') + output.split(':').last.strip + rescue Facter::Core::Execution::ExecutionFailure + nil end end diff --git a/spec/unit/facter/puppetdb_version_spec.rb b/spec/unit/facter/puppetdb_version_spec.rb index e63e258a..587c9a08 100644 --- a/spec/unit/facter/puppetdb_version_spec.rb +++ b/spec/unit/facter/puppetdb_version_spec.rb @@ -6,65 +6,11 @@ end context 'when puppetdb is available' do - before do + before(:each) do allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return('/usr/bin/puppetdb') end - context 'on a Debian-based system' do - before do - allow(Facter).to receive(:value).with(:os).and_return({ 'family' => 'Debian' }) - end - - context 'when Puppet Labs is the maintainer' do - before do - allow(Facter::Core::Execution).to receive(:execute) - .with('apt-cache show puppetdb | grep "Maintainer:" | head -1') - .and_return('Maintainer: Puppet Labs') - end - - it 'returns the correct version from puppetdb --version' do - expect(Facter::Core::Execution).to receive(:execute) - .with('puppetdb --version') - .and_return('puppetdb version: 7.19.0') - - expect(Facter.fact(:puppetdb_version).value).to eq('7.19.0') - end - - it 'returns nil if the command execution fails' do - allow(Facter::Core::Execution).to receive(:execute).with('puppetdb --version').and_raise(Facter::Core::Execution::ExecutionFailure) - - expect(Facter.fact(:puppetdb_version).value).to be_nil - end - end - - context 'when Puppet Labs is not the maintainer' do - before do - allow(Facter::Core::Execution).to receive(:execute) - .with('apt-cache show puppetdb | grep "Maintainer:" | head -1') - .and_return('Maintainer: Other Maintainer') - end - - it 'returns the correct version from dpkg-query' do - expect(Facter::Core::Execution).to receive(:execute) - .with("dpkg-query --showformat='${Version}' --show puppetdb") - .and_return('7.9.0-1ubuntu1') - - expect(Facter.fact(:puppetdb_version).value).to eq('7.9.0') - end - - it 'returns nil if the command execution fails' do - allow(Facter::Core::Execution).to receive(:execute).with("dpkg-query --showformat='${Version}' --show puppetdb").and_raise(Facter::Core::Execution::ExecutionFailure) - - expect(Facter.fact(:puppetdb_version).value).to be_nil - end - end - end - - context 'on a non-Debian-based system' do - before do - allow(Facter).to receive(:value).with(:os).and_return({ 'family' => 'RedHat' }) - end - + context 'on a default system' do it 'returns the correct version from puppetdb --version' do expect(Facter::Core::Execution).to receive(:execute) .with('puppetdb --version') @@ -82,7 +28,7 @@ end context 'when puppetdb is not available' do - before do + before(:each) do allow(Facter::Util::Resolution).to receive(:which).with('puppetdb').and_return(nil) end