From: Alex Harvey <alexharv074@gmail.com>
Date: Sat, 12 Jan 2019 08:03:50 +0000 (+1100)
Subject: (MODULES-2119) further tweaking to that logic
X-Git-Tag: 1.15.1~6^2~2
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d82777b766447c806803c0e61d78399ea52a5d9c;p=puppet-modules%2Fpuppetlabs-firewall.git

(MODULES-2119) further tweaking to that logic

A fix for Bugzilla #1015 was added in
680738164865a01f554d1e1037c8d8724e39a720. However, it appears that the
bug description at https://bugzilla.netfilter.org/show_bug.cgi?id=1015
is not quite accurate, and the -p all hack is required sometimes, but
not always. I don't know exactly when it is or isn't required.

This patch changes the logic to try both.
---

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index a70d7af..fde6e97 100644
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -319,6 +319,16 @@ Puppet::Type.type(:firewall).provide :iptables, parent: Puppet::Provider::Firewa
     begin
       iptables delete_args
     rescue Puppet::ExecutionFailure => e
+      # There is currently a bug in ip6tables where delete rules do not match rules using any protocol
+      # if '-p' all is missing.
+      #
+      # https://bugzilla.netfilter.org/show_bug.cgi?id=1015
+      #
+      # This tries deleting again with -p all to see if that helps.
+      if self.class.instance_variable_get(:@protocol) == 'IPv6' && properties[:proto] == 'all'
+        iptables delete_args.concat('-p', 'all')
+      end
+
       # Check to see if the iptables rule is already gone. This can sometimes
       # happen as a side effect of other resource changes. If it's not gone,
       # raise the error as per usual.
@@ -685,17 +695,6 @@ Puppet::Type.type(:firewall).provide :iptables, parent: Puppet::Provider::Firewa
   def delete_args
     # Split into arguments
     line = properties[:line].gsub(%r{^\-A }, '-D ').split(%r{\s+(?=(?:[^"]|"[^"]*")*$)}).map { |v| v.gsub(%r{^"}, '').gsub(%r{"$}, '') }
-    if self.class.instance_variable_get(:@protocol) == 'IPv6' && properties[:proto] == 'all'
-      #
-      # There is currently a bug in ip6tables where delete rules do not match rules using any protocol
-      # if '-p' all is missing.
-      #
-      # https://bugzilla.netfilter.org/show_bug.cgi?id=1015
-      #
-      # This check looks for this case, and adds '-p all' to the rule for ipv6.
-      #
-      line = line.concat ['-p', 'all']
-    end
     line.unshift('-t', properties[:table])
   end
 
diff --git a/spec/unit/puppet/provider/iptables_spec.rb b/spec/unit/puppet/provider/iptables_spec.rb
index a7a23d0..4bebd37 100644
--- a/spec/unit/puppet/provider/iptables_spec.rb
+++ b/spec/unit/puppet/provider/iptables_spec.rb
@@ -455,14 +455,5 @@ describe 'ip6tables provider' do
     it 'delete_args is an array' do
       expect(instance.delete_args.class).to eq(Array)
     end
-
-    it 'attempts to match ipv6 rule' do
-      expect(instance.delete_args).to eq(['-t', 'filter', '-D', 'INPUT', '-i', 'lo', '-m', 'comment', '--comment', '001 accept all to lo interface v6', '-j', 'ACCEPT', '-p', 'all'])
-    end
-
-    it 'delete_args is the same as the rule string when joined' do
-      expect(instance.delete_args.join(' ')).to eq(bare_sample_rule.gsub(%r{\-A},
-                                                                         '-t filter -D') + ' -p all')
-    end
   end
 end