]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(#10274) Nullify addresses with zero prefixlen
authorDan Carley <dan.carley@gmail.com>
Fri, 25 May 2012 06:41:36 +0000 (07:41 +0100)
committerDan Carley <dan.carley@gmail.com>
Mon, 28 May 2012 07:33:30 +0000 (08:33 +0100)
Modify the behaviour of Util::Firewall.host_to_ip, as used by the type to
parse source and destination addresses, to return nil if the resulting CIDR
represented address has a prefix length of zero. Includes type and provider
tests for IPv4 and IPv6.

IPtables silently omits rules with source and destination addresses that
have a prefix length of zero (eg. 0.0.0.0/0) because they are functionally
equivialent to not specifying any address. This was causing rules to be
unecessarily reloaded.

The behaviour of Util::IPcidr remains the same. Now includes some additional
tests for it's identification of zero prefixlen IPv4 and IPv6 addresses.

lib/puppet/util/firewall.rb
spec/fixtures/iptables/conversion_hash.rb
spec/unit/puppet/type/firewall_spec.rb
spec/unit/puppet/util/firewall_spec.rb
spec/unit/puppet/util/ipcidr_spec.rb

index 21e394d33dba58e1f9f0df00ea83d14bc6f4afa1..e36c4b15c1d95a0d290c4eb67b6d7f87a6aa5005 100644 (file)
@@ -83,12 +83,17 @@ module Puppet::Util::Firewall
   #     It will be normalised
   #   - An IP address with a dotted-quad netmask:
   #     It will be converted to CIDR notation
+  #   - Any address with a resulting prefix length of zero:
+  #     It will return nil which is equivilent to not specifying an address
   #
   def host_to_ip(value)
     begin
-      Puppet::Util::IPCidr.new(value).cidr
+      value = Puppet::Util::IPCidr.new(value)
     rescue
-      Puppet::Util::IPCidr.new(Resolv.getaddress(value)).cidr
+      value = Puppet::Util::IPCidr.new(Resolv.getaddress(value))
     end
+
+    return nil if value.prefixlen == 0
+    value.cidr
   end
 end
index a52d8b10ff2a02e57240352f3a39f7235ec7d8ec..053c1e4dc3378070cd5681d5425eaaf64cdacbb2 100644 (file)
@@ -274,6 +274,24 @@ HASH_TO_ARGS = {
     :args => ["-t", :filter, "-p", :tcp, "-m", "comment", "--comment",
       "100 no action"],
   },
+  'zero_prefixlen_ipv4' => {
+    :params => {
+      :name => '100 zero prefix length ipv4',
+      :table => 'filter',
+      :source => '0.0.0.0/0',
+      :destination => '0.0.0.0/0',
+    },
+    :args => ['-t', :filter, '-p', :tcp, '-m', 'comment', '--comment', '100 zero prefix length ipv4'],
+  },
+  'zero_prefixlen_ipv6' => {
+    :params => {
+      :name => '100 zero prefix length ipv6',
+      :table => 'filter',
+      :source => '::/0',
+      :destination => '::/0',
+    },
+    :args => ['-t', :filter, '-p', :tcp, '-m', 'comment', '--comment', '100 zero prefix length ipv6'],
+  },
   'sport_range_1' => {
     :params => {
       :name => "100 sport range",
index 4fb814a09dfa1be09948f3be5f72b5f5094cfaae..c90bda7d7f39e4dbc7b00786a27f6d66e9286eef 100755 (executable)
@@ -116,6 +116,12 @@ describe firewall do
         @resource[addr] = '127.0.0.1'
         @resource[addr].should == '127.0.0.1/32'
       end
+      ['0.0.0.0/0', '::/0'].each do |prefix|
+        it "should be nil for zero prefix length address #{prefix}" do
+          @resource[addr] = prefix
+          @resource[addr].should == nil
+        end
+      end
     end
   end
 
index a79e7330ee1ecf2b8471c5e98effa5e60691d2cd..822a3f8fd337065db50da57ae05562c8f80df5bd 100644 (file)
@@ -18,6 +18,8 @@ describe 'Puppet::Util::Firewall' do
     specify { subject.host_to_ip('96.126.112.51/32').should == '96.126.112.51/32' }
     specify { subject.host_to_ip('2001:db8:85a3:0:0:8a2e:370:7334').should == '2001:db8:85a3::8a2e:370:7334/128' }
     specify { subject.host_to_ip('2001:db8:1234::/48').should == '2001:db8:1234::/48' }
+    specify { subject.host_to_ip('0.0.0.0/0').should == nil }
+    specify { subject.host_to_ip('::/0').should == nil }
   end
 
   describe '#icmp_name_to_number' do
index 4e19e86377f570a6197dca8a2e79540c0e10e2ac..916f74a350aef974719bd152969ba743780c9147 100644 (file)
@@ -25,6 +25,14 @@ describe 'Puppet::Util::IPCidr' do
     specify { subject.netmask.should == '255.255.255.0' }
   end
 
+  describe 'ipv4 open range with cidr' do
+    before { @ipcidr = Puppet::Util::IPCidr.new('0.0.0.0/0') }
+    subject { @ipcidr }
+    specify { subject.cidr.should == '0.0.0.0/0' }
+    specify { subject.prefixlen.should == 0 }
+    specify { subject.netmask.should == '0.0.0.0' }
+  end
+
   describe 'ipv6 address' do
     before { @ipaddr = Puppet::Util::IPCidr.new('2001:db8:85a3:0:0:8a2e:370:7334') }
     subject { @ipaddr }
@@ -48,4 +56,12 @@ describe 'Puppet::Util::IPCidr' do
     specify { subject.prefixlen.should == 48 }
     specify { subject.netmask.should == 'ffff:ffff:ffff:0000:0000:0000:0000:0000' }
   end
+
+  describe 'ipv6 open range with cidr' do
+    before { @ipaddr = Puppet::Util::IPCidr.new('::/0') }
+    subject { @ipaddr }
+    specify { subject.cidr.should == '::/0' }
+    specify { subject.prefixlen.should == 0 }
+    specify { subject.netmask.should == '0000:0000:0000:0000:0000:0000:0000:0000' }
+  end
 end