]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(1093) - Fix unresolved fact error 1093-fix_unresolved_fact_error
authorjordanbreen28 <jordan.breen@perforce.com>
Tue, 29 Nov 2022 12:28:58 +0000 (12:28 +0000)
committerjordanbreen28 <jordan.breen@perforce.com>
Tue, 6 Dec 2022 15:01:11 +0000 (15:01 +0000)
Prior to this commit, work was carried out on this module to update all instances of the now deprecated Facter::Util::Resolution, and replace all with its newer and supported counterpart Facter::Core::Execution.
However, these do not behave exactly the same. Facter::Util::Resolution initially ran a which to locate the binary before executing, preventing any errors from occuring. The newer Facter::Core::Execution method did not do this, instead it attempted to execut>

This commit aims to introduce an "on_fail:false" flag to each execute statement, so that a failed execute will return false (boolean) as oppose to an error, which can then be used for further logic.

lib/facter/ip6tables_version.rb
lib/facter/iptables_persistent_version.rb
lib/facter/iptables_version.rb
spec/unit/facter/iptables_persistent_version_spec.rb
spec/unit/facter/iptables_spec.rb

index ec4848acacf84224f7bbf062a002066486f220b9..31b0d5291508341db7e408d0520b03d76810c54b 100644 (file)
@@ -2,12 +2,9 @@
 
 Facter.add(:ip6tables_version) do
   confine kernel: :Linux
+  confine { Facter::Core::Execution.which('ip6tables') }
   setcode do
-    version = Facter::Core::Execution.execute('ip6tables --version')
-    if version
-      version.match(%r{\d+\.\d+\.\d+}).to_s
-    else
-      nil
-    end
+    version = Facter::Core::Execution.execute('ip6tables --version', { on_fail: nil })
+    version.match(%r{\d+\.\d+\.\d+}).to_s if version
   end
 end
index 5f3598cecc71d518cdd2006beb646b2056df841c..ff66f0d0d7c96f65ebc3682e7f88dac1bfeb5a8b 100644 (file)
@@ -6,7 +6,7 @@ Facter.add(:iptables_persistent_version) do
     # Throw away STDERR because dpkg >= 1.16.7 will make some noise if the
     # package isn't currently installed.
     cmd = "dpkg-query -Wf '${Version}' netfilter-persistent 2>/dev/null"
-    version = Facter::Core::Execution.execute(cmd)
+    version = Facter::Core::Execution.execute(cmd, { on_fail: nil })
 
     if version.nil? || !version.match(%r{\d+\.\d+})
       nil
index 6093d2ecc23fff6a5872a54f228051b12aad0976..3cc4931869679f87a20409622de34bf7d3be8718 100644 (file)
@@ -2,12 +2,9 @@
 
 Facter.add(:iptables_version) do
   confine kernel: :Linux
+  confine { Facter::Core::Execution.which('iptables') }
   setcode do
-    version = Facter::Core::Execution.execute('iptables --version')
-    if version
-      version.match(%r{\d+\.\d+\.\d+}).to_s
-    else
-      nil
-    end
+    version = Facter::Core::Execution.execute('iptables --version', { on_fail: nil })
+    version.match(%r{\d+\.\d+\.\d+}).to_s if version
   end
 end
index 91ab608fcb2e3ebc340606db86bc2d7c2b88b1f6..514e2eb8c280fa2a623ea2d84bed329d2ddae164 100644 (file)
@@ -17,7 +17,7 @@ describe 'Facter::Util::Fact iptables_persistent_version' do
         before(:each) do
           allow(Facter.fact(:operatingsystem)).to receive(:value).and_return(os)
           allow(Facter.fact(:operatingsystemrelease)).to receive(:value).and_return(os_release)
-          allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd)
+          allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd, { on_fail: nil })
                                                              .and_return(ver)
         end
         it { expect(Facter.fact(:iptables_persistent_version).value).to eql ver }
@@ -28,7 +28,7 @@ describe 'Facter::Util::Fact iptables_persistent_version' do
       before(:each) do
         allow(Facter.fact(:operatingsystem)).to receive(:value).and_return('Ubuntu')
         allow(Facter.fact(:operatingsystemrelease)).to receive(:value).and_return('20.04')
-        allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd)
+        allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd, { on_fail: nil })
                                                            .and_return(nil)
       end
       it { expect(Facter.fact(:iptables_persistent_version).value).to be_nil }
@@ -62,7 +62,7 @@ describe 'Facter::Util::Fact iptables_persistent_version' do
         before(:each) do
           allow(Facter.fact(:operatingsystem)).to receive(:value).and_return(os)
           allow(Facter.fact(:operatingsystemrelease)).to receive(:value).and_return(os_release)
-          allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd)
+          allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd, { on_fail: nil })
                                                              .and_return(ver)
         end
         it { expect(Facter.fact(:iptables_persistent_version).value).to eql ver }
@@ -74,7 +74,7 @@ describe 'Facter::Util::Fact iptables_persistent_version' do
       before(:each) do
         allow(Facter.fact(:operatingsystem)).to receive(:value).and_return('Ubuntu')
         allow(Facter.fact(:operatingsystemrelease)).to receive(:value).and_return(os_release)
-        allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd)
+        allow(Facter::Core::Execution).to receive(:execute).with(dpkg_cmd, { on_fail: nil })
                                                            .and_return(nil)
       end
       it { expect(Facter.fact(:iptables_persistent_version).value).to be_nil }
index 16b8f35c84163953bf1b67174e27859ffc835aa5..a343cbadb4ea7b6fec2750e9006b35d2b2fcee8a 100644 (file)
@@ -11,16 +11,20 @@ describe 'Facter::Util::Fact' do
 
   describe 'iptables_version' do
     it {
-      allow(Facter::Core::Execution).to receive(:execute).with('iptables --version')
-                                                         .and_return('iptables v1.4.7')
+      allow(Facter::Core::Execution).to receive(:which)
+        .with('iptables').and_return('/usr/sbin/iptables')
+      allow(Facter::Core::Execution).to receive(:execute)
+        .with('iptables --version', { on_fail: nil }).and_return('iptables v1.4.7')
       expect(Facter.fact(:iptables_version).value).to eql '1.4.7'
     }
   end
 
   describe 'ip6tables_version' do
     before(:each) do
+      allow(Facter::Core::Execution).to receive(:which)
+        .with('ip6tables').and_return('/usr/sbin/ip6tables')
       allow(Facter::Core::Execution).to receive(:execute)
-        .with('ip6tables --version').and_return('ip6tables v1.4.7')
+        .with('ip6tables --version', { on_fail: nil }).and_return('ip6tables v1.4.7')
     end
     it { expect(Facter.fact(:ip6tables_version).value).to eql '1.4.7' }
   end