From: Jiri Stransky <jistr@redhat.com>
Date: Thu, 23 Nov 2017 14:11:03 +0000 (+0100)
Subject: (MODULES-6029) Skip unparsable rules with warning
X-Git-Tag: 1.11.0~3^2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=3a9a1f7b6ef5ed5065d97ef02f69b37ca35a7110;p=puppet-modules%2Fpuppetlabs-firewall.git

(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 1ffe28f..16eaa30 100644
--- 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 0aa61e5..3dba7e7 100644
--- 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 f9769d5..ce48e7a 100644
--- 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