Bugfix: Account for rules sorted after unmanaged rules

The offset calculation assumed unmanaged rules are numbered 9000+ and
would be sorted to the end and didn't need to be accounted for. This
caused failures when people used9-numbered rules. This should fix that.

Additionally, for rules that are 9-numbered, they should be ordered
*after* unmanaged rules, so this fixes that too.

So when encountering unmanaged rules, the order will be something like
this:

- Managed rules that begin with 0 through 8
- Unmanaged rules (which are assigned 9-numbers)
- Managed rules that begin with 9 (but not numbered lower than the
  unmanaged rules)

Mixing unmanaged rules with managed rules is still not officially
supported, but at least we can try and behave with them.

diff --git a/lib/puppet/provider/firewall/iptables.rb b/lib/puppet/provider/firewall/iptables.rb
index 0350a10b4925fc6b8aece02fec2a4c6b22f9e683..698e731c939fa0770cd842472aa92e3a856c1bcd 100644 (file)
--- a/lib/puppet/provider/firewall/iptables.rb
+++ b/lib/puppet/provider/firewall/iptables.rb
@@ -482,11 +482,13 @@ Puppet::Type.type(:firewall).provide :iptables, :parent => Puppet::Provider::Fir
       sum + (rule.match(unmanaged_rule_regex) ? 1 : 0)
     end
 
-    # We want our rules to come before unmanaged rules
-    unnamed_offset -= 1 if offset_rule.match(unmanaged_rule_regex)
+    # We want our rule to come before unmanaged rules if it's not a 9-rule
+    if offset_rule.match(unmanaged_rule_regex) and ! my_rule.match(/^9/)
+      unnamed_offset -= 1
+    end
 
     # Insert our new or updated rule in the correct order of named rules, but
     # offset for unnamed rules.
-    rules.sort.index(my_rule) + 1 + unnamed_offset
+    rules.reject{|r|r.match(unmanaged_rule_regex)}.sort.index(my_rule) + 1 + unnamed_offset
   end
 end

diff --git a/spec/unit/puppet/provider/iptables_spec.rb b/spec/unit/puppet/provider/iptables_spec.rb
index 872c1c62e7ec96924f21c6c33007a36018adc650..d6f5b64cfea957be5ed1eeafccf55d33108bbbca 100644 (file)
--- a/spec/unit/puppet/provider/iptables_spec.rb
+++ b/spec/unit/puppet/provider/iptables_spec.rb
@@ -196,6 +196,11 @@ describe 'iptables provider' do
         allow(resource.provider.class).to receive(:instances).and_return(providers)
         expect(resource.provider.insert_order).to eq(9)
       end
+      it 'understands offsets for adding rules at the end' do
+        resource = Puppet::Type.type(:firewall).new({ :name => '950 test', })
+        allow(resource.provider.class).to receive(:instances).and_return(providers)
+        expect(resource.provider.insert_order).to eq(11)
+      end
     end
   end