(MODULES-6029) Skip unparsable rules with warning

The iptables rules parser has very strict/simplistic expectations
about how iptables rules should look like, and can easily fail to
parse rules that weren't produced by the module itself.

We should ignore the unfitting rules when parsing and produce a
warning rather than causing a fatal error and stopping the Puppet run.

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 1ffe28fe1d4b9595e34782de1df257d224f5646b..16eaa30f61dad109f5127680e6f10035649a5f3c 100644 (file)
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -484,7 +484,8 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
     valrev = values.scan(/("([^"\\]|\\.)*"|\S+)/).transpose[0].reverse
 
     if keys.length != valrev.length then
-      raise "Parser error: keys (#{keys.length}) and values (#{valrev.length}) count mismatch on line: #{line}"
+      warning "Skipping unparsable iptables rule: keys (#{keys.length}) and values (#{valrev.length}) count mismatch on line: #{line}"
+      return
     end
 
     # Here we generate the main hash by scanning arguments off the values

diff --git a/spec/fixtures/iptables/conversion_hash.rb b/spec/fixtures/iptables/conversion_hash.rb
index 0aa61e5f881546c48c1c355b514b0272745ad100..3dba7e7f48f577736c6e731915614b78b1950ebc 100644 (file)
--- a/spec/fixtures/iptables/conversion_hash.rb
+++ b/spec/fixtures/iptables/conversion_hash.rb
@@ -678,7 +678,7 @@ ARGS_TO_HASH = {
   'parser_sanity_check' => {
     :line   => '-A INPUT -s 1.2.3.4/32 -p tcp -m tcp --dport 80 --tcp-flags FIN,SYN,RST,ACK SYN -m comment --comment "004 parser sanity check" -j ACCEPT',
     :table  => 'filter',
-    :raise_error => true,
+    :produce_warning => true,
     :params => {},
   },
 }

diff --git a/spec/unit/puppet/provider/iptables_spec.rb b/spec/unit/puppet/provider/iptables_spec.rb
index f9769d5e03893a313a3938a12ad34c65c460543d..ce48e7a81669368defdfbbd8e07f74e74373acfa 100644 (file)
--- a/spec/unit/puppet/provider/iptables_spec.rb
+++ b/spec/unit/puppet/provider/iptables_spec.rb
@@ -216,9 +216,11 @@ describe 'iptables provider' do
       describe "for test data '#{test_name}'" do
         let(:resource) { provider.rule_to_hash(data[:line], data[:table], 0) }
         # If this option is enabled, make sure the error was raised
-        if data[:raise_error] then
-          it "the input rules should raise an error by rules_to_hash" do
-            expect{ resource }.to raise_error
+        if data[:produce_warning] then
+          puts data
+          it "the input rules should produce a warning by rules_to_hash" do
+            expect(provider).to receive(:warning).with(/Skipping unparsable iptables rule/)
+            resource  # force resource to get evaluated
           end
         end