Fix apt_has_updates fact not parsing apt-check output correctly

The /usr/lib/update-notifier/apt-check script returns its output
to STDERR but a recent change to the script redirects STDERR to
/dev/null.  This will cause the array to always be empty.

Combined with that problem, while we were checking for the result
being nil, we never checked for an invalid array.  As a result,
the apt_has_updates was always true and the apt_updates and
apt_security_updates facts were trying to read from an empty array
and failing.

diff --git a/lib/facter/apt_updates.rb b/lib/facter/apt_updates.rb
index 77e667e6d76c18f8f2eed5adc3b2dad187035c32..014782eab2edaa33c5f29f79bac19d81a901d7da 100644 (file)
--- a/lib/facter/apt_updates.rb
+++ b/lib/facter/apt_updates.rb
@@ -2,18 +2,23 @@ apt_package_updates = nil
 Facter.add("apt_has_updates") do
   confine :osfamily => 'Debian'
   if File.executable?("/usr/lib/update-notifier/apt-check")
-    apt_package_updates = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check 2>/dev/null').split(';')
+    apt_check_result = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check 2>&1')
+    if not apt_check_result.nil? and apt_check_result =~ /^\d+;\d+$/
+      apt_package_updates = apt_check_result.split(';')
+    end
   end
 
   setcode do
-    apt_package_updates != ['0', '0'] unless apt_package_updates.nil?
+    if not apt_package_updates.nil? and apt_package_updates.length == 2
+      apt_package_updates != ['0', '0']
+    end
   end
 end
 
 Facter.add("apt_package_updates") do
   confine :apt_has_updates => true
   setcode do
-    packages = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check -p 2>/dev/null').split("\n")
+    packages = Facter::Util::Resolution.exec('/usr/lib/update-notifier/apt-check -p 2>&1').split("\n")
     if Facter.version < '2.0.0'
       packages.join(',')
     else

diff --git a/spec/unit/facter/apt_has_updates_spec.rb b/spec/unit/facter/apt_has_updates_spec.rb
index 691fb32280308af603eb66a8a2b8132cfed9e090..f8a3f20a1393add81b0605c220bb7d9773426277 100644 (file)
--- a/spec/unit/facter/apt_has_updates_spec.rb
+++ b/spec/unit/facter/apt_has_updates_spec.rb
@@ -6,27 +6,49 @@ describe 'apt_has_updates fact' do
 
   describe 'on non-Debian distro' do
     before {
-      Facter.fact(:osfamily).expects(:value).returns 'RedHat'
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'RedHat'
     }
     it { should be_nil }
   end
 
   describe 'on Debian based distro missing update-notifier-common' do
     before {
-      Facter.fact(:osfamily).expects(:value).returns 'Debian'
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
       File.stubs(:executable?) # Stub all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns false
     }
     it { should be_nil }
   end
 
+  describe 'on Debian based distro with broken packages' do
+    before {
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
+      File.stubs(:executable?) # Stub all other calls
+      Facter::Util::Resolution.stubs(:exec) # Catch all other calls
+      File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "E: Error: BrokenCount > 0"
+    }
+    it { should be_nil }
+  end
+
+  describe 'on Debian based distro with unknown error with semicolons' do
+    before {
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
+      File.stubs(:executable?) # Stub all other calls
+      Facter::Util::Resolution.stubs(:exec) # Catch all other calls
+      File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "E: Unknown Error: 'This error contains something that could be parsed like 4;3' (10)"
+    }
+    it { should be_nil }
+  end
+
   describe 'on Debian based distro' do
     before {
-      Facter.fact(:osfamily).expects(:value).returns 'Debian'
+      Facter.fact(:osfamily).expects(:value).at_least(1).returns 'Debian'
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "4;3"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "4;3"
     }
     it { should be true }
   end

diff --git a/spec/unit/facter/apt_package_updates_spec.rb b/spec/unit/facter/apt_package_updates_spec.rb
index c0f54f23af80260a752240f598bfc069ce188377..5c7a624f4d7f2688ca367b62676870cfc35b70ce 100644 (file)
--- a/spec/unit/facter/apt_package_updates_spec.rb
+++ b/spec/unit/facter/apt_package_updates_spec.rb
@@ -17,8 +17,8 @@ describe 'apt_package_updates fact' do
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "1;2"
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check -p 2>/dev/null').returns "puppet-common\nlinux-generic\nlinux-image-generic"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "1;2"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check -p 2>&1').returns "puppet-common\nlinux-generic\nlinux-image-generic"
     }
     it {
       if Facter.version < '2.0.0'

diff --git a/spec/unit/facter/apt_security_updates_spec.rb b/spec/unit/facter/apt_security_updates_spec.rb
index a5d26c76cf7408dd1c2b24a827f566cb49aabdb0..4bc760f7898ca406a20053fcb61917aa6879063f 100644 (file)
--- a/spec/unit/facter/apt_security_updates_spec.rb
+++ b/spec/unit/facter/apt_security_updates_spec.rb
@@ -17,7 +17,7 @@ describe 'apt_security_updates fact' do
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "14;7"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "14;7"
     }
     it { should == 7 }
   end

diff --git a/spec/unit/facter/apt_updates_spec.rb b/spec/unit/facter/apt_updates_spec.rb
index 538466f999e628572501b285016f4ab0f2a1604f..7e9b77f84f695cecb108c9cc9897ea7254d411a6 100644 (file)
--- a/spec/unit/facter/apt_updates_spec.rb
+++ b/spec/unit/facter/apt_updates_spec.rb
@@ -17,7 +17,7 @@ describe 'apt_updates fact' do
       File.stubs(:executable?) # Stub all other calls
       Facter::Util::Resolution.stubs(:exec) # Catch all other calls
       File.expects(:executable?).with('/usr/lib/update-notifier/apt-check').returns true
-      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>/dev/null').returns "14;7"
+      Facter::Util::Resolution.expects(:exec).with('/usr/lib/update-notifier/apt-check 2>&1').returns "14;7"
     }
     it { should == 14 }
   end