From: Mateusz Gozdek <mateusz.gozdek@sociomantic.com>
Date: Mon, 5 Nov 2018 10:53:06 +0000 (+0100)
Subject: (MODULES-7990) Merge multiple comments into one while parsing rules
X-Git-Tag: 1.15.0~10^2~1
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=491848910d11606759f21a070e6ff0b675e8828c;p=puppet-modules%2Fpuppetlabs-firewall.git

(MODULES-7990) Merge multiple comments into one while parsing rules

As iptables/iptables-save accepts multiple '-m comment --comment' parameters,
we should find and merge them all together to avoid generating warnings.

Since puppet resource allows you to create only single comment, this should only
affect rules, which are not managed by puppet.
---

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 15c99d1..fbef28e 100644
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -393,6 +393,15 @@ Puppet::Type.type(:firewall).provide :iptables, parent: Puppet::Provider::Firewa
       values = values.gsub(%r{-m set --match-set (!\s+)?\S* \S* }, '')
       values.insert(ind, "-m set --match-set \"#{sets.join(';')}\" ")
     end
+    # --comment can have multiple values, the same as --match-set
+    if values =~ %r{-m comment --comment}
+      ind = values.index('-m comment --comment')
+      comments = values.scan(%r{-m comment --comment "(.*?[^\\"])"})
+      comments += values.scan(%r{-m comment --comment ([^"]+?)\b})
+      values = values.gsub(%r{-m comment --comment (".*?[^\\"]"|[^ ].*)( |$)}, '')
+      values = values.gsub(%r{-m comment --comment ([^"].*?)[ $]}, '')
+      values.insert(ind, "-m comment --comment \"#{comments.join(';')}\" ")
+    end
     # the actual rule will have the ! mark before the option.
     values = values.gsub(%r{(!)\s*(-\S+)\s*(\S*)}, '\2 "\1 \3"')
     # we do a similar thing for negated address masks (source and destination).
diff --git a/spec/acceptance/resource_cmd_spec.rb b/spec/acceptance/resource_cmd_spec.rb
index 6ed69eb..c9462c0 100644
--- a/spec/acceptance/resource_cmd_spec.rb
+++ b/spec/acceptance/resource_cmd_spec.rb
@@ -64,6 +64,22 @@ describe 'puppet resource firewall command' do
     end
   end
 
+  context 'when accepts rules with multiple comments', unless: (fact('operatingsystem') == 'RedHat' && fact('operatingsystemmajrelease') <= '5') ||
+                                                               (fact('operatingsystem') == 'CentOS' && fact('operatingsystemmajrelease') <= '5') do
+    before(:all) do
+      iptables_flush_all_tables
+      shell('iptables -A INPUT -j ACCEPT -p tcp --dport 80 -m comment --comment "http" -m comment --comment "http"')
+    end
+
+    it do
+      shell('puppet resource firewall') do |r|
+        r.exit_code.should be_zero
+        # don't check stdout, testing preexisting rules, output is normal
+        r.stderr.should be_empty
+      end
+    end
+  end
+
   context 'when accepts rules with negation' do
     before :all do
       iptables_flush_all_tables
diff --git a/spec/fixtures/iptables/conversion_hash.rb b/spec/fixtures/iptables/conversion_hash.rb
index 60b4367..a169d50 100644
--- a/spec/fixtures/iptables/conversion_hash.rb
+++ b/spec/fixtures/iptables/conversion_hash.rb
@@ -240,6 +240,13 @@ ARGS_TO_HASH = {
       source: '192.168.0.1/32',
     },
   },
+  'multiple_comments' => {
+    line: '-A INPUT -s 192.168.0.1/32 -m comment --comment "000 allow from 192.168.0.1, please" -m comment --comment "another comment"',
+    table: 'filter',
+    params: {
+      name: '000 allow from 192.168.0.1, please;another comment',
+    },
+  },
   'string_escape_sequences' => {
     line: '-A INPUT -m comment --comment "000 parse escaped \\"s, \\\'s, and \\\\s"',
     table: 'filter',