(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 15c99d1d7b9c36fa953da66bfd1f0236834e7fda..fbef28e86262328c42f00aaa90eda178da23527e 100644 (file)
--- 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 6ed69eb294cd7b341fdf5e1f6e7bfac0bff1b7fd..c9462c01b30514bee115a713c3d89f7a509ca57d 100644 (file)
--- 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 60b43674bb8c8e3aa44bdff006b1d4d76f06648f..a169d50f609caa8aead391f7214ad4ff6d8e4448 100644 (file)
--- 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',