Browse Source

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.
WolverineFan 9 years ago
parent
commit
e7fee16589

+ 8 - 3
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

+ 26 - 4
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

+ 2 - 2
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'

+ 1 - 1
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

+ 1 - 1
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