OHAI-529 ensure virtual IPs are never chosen for privateaddress

- Refactor privateaddress logic into smaller, more clear methods based on comments from @btm - Make rspec descriptions a little more readable
prein · Nov 26, 2013 · 85bb7dd · 85bb7dd
1 parent 669cd21
commit 85bb7dd
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 13 deletions.
diff --git a/lib/ohai/plugins/ip_scopes.rb b/lib/ohai/plugins/ip_scopes.rb
@@ -31,8 +31,10 @@
         interface['addresses'].each do |address,attrs|
           begin
             attrs.merge! 'ip_scope' => address.to_ip.scope
-            next unless address.to_ip.scope =~ /PRIVATE/
-            privateaddress(address) if (privateaddress.nil? || interface['type'] != 'ppp')
+
+            if private_addr?(address) && !tunnel_iface?(interface)
+              privateaddress(address)
+            end
           rescue ArgumentError
             # Just silently fail if we can't create an IP from the string.
           end
@@ -44,4 +46,12 @@
       Ohai::Log.debug("ip_scopes: cannot load gem, plugin disabled: #{e}")
     end
   end
+
+  def private_addr?(address)
+    address.to_ip.scope =~ /PRIVATE/
+  end
+
+  def tunnel_iface?(interface)
+    interface['type'] == 'ppp'
+  end
 end
diff --git a/spec/unit/plugins/ip_scopes_spec.rb b/spec/unit/plugins/ip_scopes_spec.rb
@@ -29,44 +29,46 @@
       let(:addresses1) { Hash[ip1, {}] }
       let(:addresses2) { Hash[ip2, {}, ip3, {}] }
 
-      it 'adds ip_scope to each address' do
+      it "adds ip_scope to each address's information hash" do
         plugin.run
         expect(plugin[:network][:interfaces][:eth0][:addresses]['10.0.0.1'][:ip_scope]).to eq('RFC1918 PRIVATE')
         expect(plugin[:network][:interfaces][:eth1][:addresses]['1.2.3.4'][:ip_scope]).to eq('GLOBAL UNICAST')
         expect(plugin[:network][:interfaces][:eth1][:addresses]['fe80::8638:35ff:fe4e:dc74'][:ip_scope]).to eq('LINK LOCAL UNICAST')
       end
 
-      describe 'privateaddress' do
+      describe 'privateaddress attribute' do
         before { plugin.run }
 
-        context 'host has multiple RFC1918 ethernet addresses' do
+        context 'when host has multiple RFC1918 ethernet addresses' do
           let(:ip1) { '10.0.0.1' }
           let(:ip2) { '192.168.1.1' }
           let(:interface1_type) { 'eth' }
           let(:interface2_type) { 'eth' }
 
-          it 'picks the last address' do
+          it 'picks the last RFC1918 address' do
             expect(plugin[:privateaddress]).to eq('192.168.1.1')
           end
         end
 
-        context 'host has PPP and ethernet RFC1918 addresses' do
+        context 'when host has virtual and ethernet RFC1918 addresses' do
           let(:ip1) { '10.0.0.1' }
           let(:ip2) { '192.168.1.1' }
           let(:interface1_type) { 'eth' }
           let(:interface2_type) { 'ppp' }
 
-          it 'prefers the eth address' do
+          it 'picks the non-virtual address' do
             expect(plugin[:privateaddress]).to eq('10.0.0.1')
           end
         end
 
-        context 'host only has ppp RFC1918 address' do
+        context 'when host only has virtual RFC1918 addresses' do
           let(:ip1) { '10.0.0.1' }
+          let(:ip2) { '192.168.1.1' }
           let(:interface1_type) { 'ppp' }
+          let(:interface2_type) { 'ppp' }
 
-          it 'uses it' do
-            expect(plugin[:privateaddress]).to eq('10.0.0.1')
+          it 'ignores them' do
+            expect(plugin[:privateaddress]).to be nil
           end
         end
       end
@@ -83,11 +85,11 @@
         plugin.run
       end
 
-      it 'does not add ip_scope to address' do
+      it 'does not add ip_scope to addresses' do
         expect(plugin[:network][:interfaces][:eth0][:addresses]['10.0.0.1'][:ip_scope]).to be nil
       end
 
-      it 'does not add a privateaddress key' do
+      it 'does not add a privateaddress attribute' do
         expect(plugin[:privateaddress]).to be nil
       end
     end