Merge pull request #165 from rski/route_every_run_update

Fix issue 69, backwards incompatible change

Author: rnelson0

CHANGELOG.md

@@ -5,6 +5,7 @@ CHANGELOG
 
   * Drop support for Ruby 1.8.7
   * voxpupuli namespace release
+  * (#69) debian default route should now be named 'default', same as redhat
 
 # Releasing 0.5.0
 

README.markdown

@@ -42,10 +42,12 @@ network_config { 'eth1':
 
 Route configuration
 
+Route resources should be named in CIDR notation. If not, they will not be properly mapped to existing routes and puppet will apply them on every run. Default routes should be named 'default'.
+
   For Debian:
 
 ```puppet
-network_route { '172.17.67.0':
+network_route { '172.17.67.0/24':
   ensure    => 'present',
   gateway   => '172.18.6.2',
   interface => 'vlan200',
@@ -69,7 +71,7 @@ network_route { 'default':
   ensure    => 'present',
   gateway   => '10.0.2.2',
   interface => 'eth0',
-  netmask  	=> '0.0.0.0',
+  netmask   => '0.0.0.0',
   network   => 'default'
 }
 ```

lib/puppet/provider/network_route/routes.rb

@@ -66,14 +66,19 @@ def self.parse_file(_filename, contents)
 
       raise_malformed if route.length < 4
 
-      # use the CIDR version of the target as :name
-      cidr_target = "#{route[0]}/#{IPAddr.new(route[1]).to_i.to_s(2).count('1')}"
-
-      route_hash[cidr_target][:network] = route[0]
-      route_hash[cidr_target][:netmask] = route[1]
-      route_hash[cidr_target][:gateway] = route[2]
-      route_hash[cidr_target][:interface] = route[3]
-      route_hash[cidr_target][:options] = route[4] if route[4]
+      if route[0] == 'default'
+        name = 'default'
+        route_hash[name][:network] = 'default'
+        route_hash[name][:netmask] = '0.0.0.0'
+      else
+        # use the CIDR version of the target as :name
+        name = "#{route[0]}/#{IPAddr.new(route[1]).to_i.to_s(2).count('1')}"
+        route_hash[name][:network] = route[0]
+        route_hash[name][:netmask] = route[1]
+      end
+      route_hash[name][:gateway] = route[2]
+      route_hash[name][:interface] = route[3]
+      route_hash[name][:options] = route[4] if route[4]
     end
 
     route_hash.values
@@ -91,7 +96,9 @@ def self.format_file(_filename, providers)
       raise Puppet::Error, "#{provider.name} is missing the required parameter 'gateway'." if provider.gateway.nil?
       raise Puppet::Error, "#{provider.name} is missing the required parameter 'interface'." if provider.interface.nil?
 
-      contents << "#{provider.network} #{provider.netmask} #{provider.gateway} #{provider.interface}"
+      netmask = (provider.name == 'default' ? '0.0.0.0' : provider.netmask)
+
+      contents << "#{provider.network} #{netmask} #{provider.gateway} #{provider.interface}"
       contents << (provider.options == :absent ? "\n" : " #{provider.options}\n")
     end
 

spec/fixtures/provider/network_route/routes_spec/simple_routes

@@ -1,4 +1,5 @@
 172.28.45.0 255.255.255.0 172.18.6.2 vlan200
 172.17.67.0 255.255.255.0 172.18.6.2 vlan200
 10.10.10.0  255.255.255.0 172.18.6.2 vlan200
-2a01:4f8:211:9d5:53:: ffff:ffff:ffff:ffff:ffff:ffff:: 2a01:4f8:211:9d5::2 vlan200
+2a01:4f8:211:9d5:53:: ffff:ffff:ffff:ffff:ffff:ffff:: 2a01:4f8:211:9d5::2 vlan200
+default 0.0.0.0 172.18.6.2 vlan200

spec/unit/provider/network_config/interfaces_spec.rb

@@ -167,8 +167,7 @@ def fixture_data(file)
            netmask: '255.255.0.0',
            mtu: '1500',
            mode: nil,
-           options: nil
-      )
+           options: nil)
     end
 
     let(:eth0_1_provider) do
@@ -183,8 +182,7 @@ def fixture_data(file)
            netmask: '255.255.0.0',
            mtu: '1500',
            mode: :vlan,
-           options: nil
-      )
+           options: nil)
     end
 
     let(:eth1_provider) do
@@ -205,8 +203,7 @@ def fixture_data(file)
                '/bin/touch /tmp/eth1-down1',
                '/bin/touch /tmp/eth1-down2',
              ],
-           }
-      )
+           })
     end
 
     let(:lo_provider) do
@@ -220,8 +217,7 @@ def fixture_data(file)
            netmask: nil,
            mtu: '65536',
            mode: nil,
-           options: nil
-      )
+           options: nil)
     end
 
     before do

spec/unit/provider/network_config/redhat_spec.rb

@@ -356,8 +356,7 @@ def fixture_data(file)
            netmask: '255.255.255.0',
            mtu: '1500',
            mode: nil,
-           options: { 'NM_CONTROLLED' => 'no', 'USERCTL' => 'no' }
-      )
+           options: { 'NM_CONTROLLED' => 'no', 'USERCTL' => 'no' })
     end
 
     let(:eth0_1_provider) do
@@ -372,8 +371,7 @@ def fixture_data(file)
            netmask: '255.255.255.0',
            mtu: '1500',
            mode: :vlan,
-           options: { 'NM_CONTROLLED' => 'no', 'USERCTL' => 'no' }
-      )
+           options: { 'NM_CONTROLLED' => 'no', 'USERCTL' => 'no' })
     end
 
     let(:eth1_provider) do
@@ -388,8 +386,7 @@ def fixture_data(file)
            netmask: :absent,
            mtu: :absent,
            mode: :vlan,
-           options: :absent,
-          )
+           options: :absent)
     end
 
     let(:lo_provider) do
@@ -402,8 +399,7 @@ def fixture_data(file)
            ipaddress: nil,
            netmask: nil,
            mode: nil,
-           options: {}
-      )
+           options: {})
     end
 
     let(:bond0_provider) do
@@ -418,9 +414,7 @@ def fixture_data(file)
            mode: nil,
            options: {
              'BONDING_OPTS' => %(mode=4 miimon=100 xmit_hash_policy=layer3+4)
-           }
-
-      )
+           })
     end
 
     it 'should fail if multiple interfaces are flushed to one file' do

spec/unit/provider/network_route/redhat_spec.rb

@@ -79,8 +79,7 @@ def fixture_data(file)
            netmask: '30',
            gateway: '172.18.6.2',
            interface: 'vlan200',
-           options: 'table 200'
-      )
+           options: 'table 200')
     end
 
     let(:route2_provider) do
@@ -90,8 +89,7 @@ def fixture_data(file)
            netmask: '30',
            gateway: '172.18.6.2',
            interface: 'eth0',
-           options: 'table 200'
-      )
+           options: 'table 200')
     end
 
     let(:defaultroute_provider) do
@@ -101,8 +99,7 @@ def fixture_data(file)
            netmask: '',
            gateway: '10.0.0.1',
            interface: 'eth1',
-           options: 'table 200'
-      )
+           options: 'table 200')
     end
 
     let(:nooptions_provider) do
@@ -112,8 +109,7 @@ def fixture_data(file)
            netmask: '',
            gateway: '10.0.0.1',
            interface: 'eth2',
-           options: :absent
-      )
+           options: :absent)
     end
 
     let(:content) { described_class.format_file('', [route1_provider, route2_provider, defaultroute_provider, nooptions_provider]) }

spec/unit/provider/network_route/routes_spec.rb

@@ -21,6 +21,18 @@ def fixture_data(file)
                interface: 'vlan200')
     end
 
+    it 'should name default routes "default" and have a 0.0.0.0 netmask' do
+      fixture = fixture_data('simple_routes')
+      data = described_class.parse_file('', fixture)
+
+      expect(data.find { |h| h[:name] == 'default' })
+        .to eq(name: 'default',
+               network: 'default',
+               netmask: '0.0.0.0',
+               gateway: '172.18.6.2',
+               interface: 'vlan200')
+    end
+
     it 'should parse out simple ipv6 iface lines' do
       fixture = fixture_data('simple_routes')
       data = described_class.parse_file('', fixture)
@@ -74,8 +86,7 @@ def fixture_data(file)
            netmask: '255.255.255.0',
            gateway: '172.18.6.2',
            interface: 'vlan200',
-           options: 'table 200'
-      )
+           options: 'table 200')
     end
 
     let(:route2_provider) do
@@ -85,8 +96,7 @@ def fixture_data(file)
            netmask: '255.255.255.0',
            gateway: '172.18.6.2',
            interface: 'eth0',
-           options: 'table 200'
-      )
+           options: 'table 200')
     end
 
     let(:content) { described_class.format_file('', [route1_provider, route2_provider]) }
@@ -122,8 +132,7 @@ def fixture_data(file)
            netmask: '255.255.255.0',
            gateway: '172.18.6.2',
            interface: 'vlan200',
-           options: :absent,
-      )
+           options: :absent)
     end
 
     let(:route2_provider) do
@@ -133,8 +142,7 @@ def fixture_data(file)
            netmask: '255.255.255.0',
            gateway: '172.18.6.2',
            interface: 'eth0',
-           options: :absent,
-      )
+           options: :absent)
     end
 
     let(:content) { described_class.format_file('', [route1_provider, route2_provider]) }