]> review.fuel-infra Code Review - puppet-modules/puppetlabs-firewall.git/commitdiff
(MODULES-2119) further tweaking to that logic
authorAlex Harvey <alexharv074@gmail.com>
Sat, 12 Jan 2019 08:03:50 +0000 (19:03 +1100)
committerAlex Harvey <alexharv074@gmail.com>
Sat, 12 Jan 2019 08:03:50 +0000 (19:03 +1100)
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.

lib/puppet/provider/firewall/iptables.rb
spec/unit/puppet/provider/iptables_spec.rb

index a70d7af5be9d3818840e2c63940cf25b0b32a2f8..fde6e9740109c6cafd82225f70918ea332948eba 100644 (file)
@@ -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
 
index a7a23d0954e6122d1e9f165b37eba553ef84d222..4bebd37f1e93031b35e913edfe9c990b4b79990f 100644 (file)
@@ -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