From: Guillaume Virlet <github@virlet.org>
Date: Mon, 9 Sep 2013 22:11:13 +0000 (+0200)
Subject: ensure INPUT, OUTPUT and FORWARD firewallchain are not autorequired when table is... 
X-Git-Tag: 0.5.0~29^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=51d5792e1a87e560e06fc377f1c65a4be7dda3b4;p=puppet-modules%2Fpuppetlabs-firewall.git

ensure INPUT, OUTPUT and FORWARD firewallchain are not autorequired when table is :filter (to allow default drop policy at the end of rules)
---

diff --git a/lib/puppet/type/firewall.rb b/lib/puppet/type/firewall.rb
index 561cbf3..551f3cb 100644
--- a/lib/puppet/type/firewall.rb
+++ b/lib/puppet/type/firewall.rb
@@ -667,8 +667,9 @@ Puppet::Type.newtype(:firewall) do
     end
 
     unless protocol.nil?
+      table = value(:table)
       [value(:chain), value(:jump)].each do |chain|
-        reqs << "#{chain}:#{value(:table)}:#{protocol}" unless chain.nil?
+        reqs << "#{chain}:#{table}:#{protocol}" unless ( chain.nil? || (['INPUT', 'OUTPUT', 'FORWARD'].include?(chain) && table == :filter) )
       end
     end
 
diff --git a/spec/unit/puppet/type/firewall_spec.rb b/spec/unit/puppet/type/firewall_spec.rb
index 5b8abdb..1f9dc96 100755
--- a/spec/unit/puppet/type/firewall_spec.rb
+++ b/spec/unit/puppet/type/firewall_spec.rb
@@ -486,6 +486,67 @@ describe firewall do
         rel.source.ref.should == chain.ref
         rel.target.ref.should == @resource.ref
       end
+
+      # test where autorequire is still needed (table != filter)
+      ['INPUT', 'OUTPUT', 'FORWARD'].each do |test_chain|
+        it "should autorequire fwchain #{test_chain} when table is mangle and provider is undefined" do
+          @resource[param] = test_chain
+          @resource[:table] = :mangle
+          @resource[:provider].should == :iptables
+
+          chain = Puppet::Type.type(:firewallchain).new(:name => "#{test_chain}:mangle:IPv4")
+          catalog = Puppet::Resource::Catalog.new
+          catalog.add_resource @resource
+          catalog.add_resource chain
+          rel = @resource.autorequire[0]
+          rel.source.ref.should == chain.ref
+          rel.target.ref.should == @resource.ref
+        end
+
+        it "should autorequire fwchain #{test_chain} when table is mangle and provider is ip6tables" do
+          @resource[param] = test_chain
+          @resource[:table] = :mangle
+          @resource[:provider] = :ip6tables
+
+          chain = Puppet::Type.type(:firewallchain).new(:name => "#{test_chain}:mangle:IPv6")
+          catalog = Puppet::Resource::Catalog.new
+          catalog.add_resource @resource
+          catalog.add_resource chain
+          rel = @resource.autorequire[0]
+          rel.source.ref.should == chain.ref
+          rel.target.ref.should == @resource.ref
+        end
+      end
+
+      # test of case where autorequire should not happen
+      ['INPUT', 'OUTPUT', 'FORWARD'].each do |test_chain|
+
+        it "should not autorequire fwchain #{test_chain} when table and provider are undefined" do
+          @resource[param] = test_chain
+          @resource[:table].should == :filter
+          @resource[:provider].should == :iptables
+
+          chain = Puppet::Type.type(:firewallchain).new(:name => "#{test_chain}:filter:IPv4")
+          catalog = Puppet::Resource::Catalog.new
+          catalog.add_resource @resource
+          catalog.add_resource chain
+          rel = @resource.autorequire[0]
+          rel.should == nil
+        end
+
+        it "should not autorequire fwchain #{test_chain} when table is undefined and provider is ip6tables" do
+          @resource[param] = test_chain
+          @resource[:table].should == :filter
+          @resource[:provider] = :ip6tables
+
+          chain = Puppet::Type.type(:firewallchain).new(:name => "#{test_chain}:filter:IPv6")
+          catalog = Puppet::Resource::Catalog.new
+          catalog.add_resource @resource
+          catalog.add_resource chain
+          rel = @resource.autorequire[0]
+          rel.should == nil
+        end
+      end
     end
   end