puppet-firewall assumes that rule is deleted when iptables command fails

[stek29]

Describe the Bug

When iptables command to delete a rule fails, puppetlabs-firewall assumes that rule was deleted successfully, even if it wasn't (correctively).

Expected Behavior

If execution of iptables delete_args fails for any other reason except rule not being found, module should raise a failure.

Steps to Reproduce

Run puppet agent which tries to remove a rule, but make sure iptables -D command fails (for example, because it wasn't able to grab xtables.lock). Notice that puppet agent reports that ensure absent was successful. run iptables-save and see that the rule is still present.

Environment

Version: commit 62be29194a3450dbf1e732bcac4408b72deb1a5e Platform: CentOS 7

Additional Context

The only case in which it's safe to assume that rule wasn't present is if iptables exits with status code 1 and message ip(6)tables: No chain/target/match by that name.. If it exits with a different error -- execution should fail.

[stek29]

this issue is being amplified by #1131, which makes it reproduce almost every time in my environment, where iptables is manipulated by Kubernetes CNI, kube-proxy and other daemons.

[stek29]

an example of how this could be patched from my undestanding of puppet:

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
@@ -380,6 +380,9 @@ Puppet::Type.type(:firewall).provide :iptables, parent: Puppet::Provider::Firewa
     begin
       iptables delete_args
     rescue Puppet::ExecutionFailure => e
+      # see https://github.com/puppetlabs/puppetlabs-firewall/issues/1132
+      raise e unless %r{ returned 1: ip6?tables: No chain/target/match by that name.}.match?(e.message)
+
       # There is currently a bug in ip6tables where delete rules do not match rules using any protocol
       # if '-p' all is missing.
       #
@@ -390,7 +393,9 @@ Puppet::Type.type(:firewall).provide :iptables, parent: Puppet::Provider::Firewa
       if self.class.instance_variable_get(:@protocol) == 'IPv6' && properties[:proto] == 'all'
         begin
           iptables delete_args.concat(['-p', 'all'])
-        rescue Puppet::ExecutionFailure => e # rubocop:disable Lint/SuppressedException
+        rescue Puppet::ExecutionFailure => e
+          # see https://github.com/puppetlabs/puppetlabs-firewall/issues/1132
+          raise e unless %r{ returned 1: ip6?tables: No chain/target/match by that name.}.match?(e.message)
         end
       end