From: Dan Carley <dan.carley@gmail.com>
Date: Fri, 25 May 2012 06:41:36 +0000 (+0100)
Subject: (#10274) Nullify addresses with zero prefixlen
X-Git-Tag: 0.1.0~30^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2721826395e792030141239841f0818fb112ce7a;p=puppet-modules%2Fpuppetlabs-firewall.git

(#10274) Nullify addresses with zero prefixlen

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.
---

diff --git a/lib/puppet/util/firewall.rb b/lib/puppet/util/firewall.rb
index 21e394d..e36c4b1 100644
--- a/lib/puppet/util/firewall.rb
+++ b/lib/puppet/util/firewall.rb
@@ -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
diff --git a/spec/fixtures/iptables/conversion_hash.rb b/spec/fixtures/iptables/conversion_hash.rb
index a52d8b1..053c1e4 100644
--- a/spec/fixtures/iptables/conversion_hash.rb
+++ b/spec/fixtures/iptables/conversion_hash.rb
@@ -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",
diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 4fb814a..c90bda7 100755
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -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
 
diff --git a/spec/unit/puppet/util/firewall_spec.rb b/spec/unit/puppet/util/firewall_spec.rb
index a79e733..822a3f8 100644
--- a/spec/unit/puppet/util/firewall_spec.rb
+++ b/spec/unit/puppet/util/firewall_spec.rb
@@ -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
diff --git a/spec/unit/puppet/util/ipcidr_spec.rb b/spec/unit/puppet/util/ipcidr_spec.rb
index 4e19e86..916f74a 100644
--- a/spec/unit/puppet/util/ipcidr_spec.rb
+++ b/spec/unit/puppet/util/ipcidr_spec.rb
@@ -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