Speed up ipset entries changes

We now use `--add-entries-from-file` and `--remove-entries-from-file` to
change firewalld ipset. Adding or removing entries one by one was really
slow.

This pull request is based on
https://github.com/42wim/puppet-firewalld/blob/04683b46cbe6e6a925c585283941cc363752aceb/lib/puppet/provider/firewalld_ipset/firewall_cmd.rb

Author: jfroche

lib/puppet/provider/firewalld_ipset/firewall_cmd.rb

@@ -60,7 +60,7 @@ def create
       args << ["--option=#{option_name}=#{value}"] if value
     end
     execute_firewall_cmd(args.flatten, nil)
-    @resource[:entries].each { |e| add_entry(e) } if @resource[:manage_entries]
+    add_entries_from_file(@resource[:entries]) if @resource[:manage_entries]
   end
 
   %i[type maxelem family hashsize timeout].each do |method|
@@ -80,12 +80,18 @@ def entries
     end
   end
 
-  def add_entry(entry)
-    execute_firewall_cmd(["--ipset=#{@resource[:name]}", "--add-entry=#{entry}"], nil)
+  def add_entries_from_file(entries)
+    f = Tempfile.new('ipset')
+    entries.each { |e| f.write(e + "\n") }
+    f.close
+    execute_firewall_cmd(["--ipset=#{@resource[:name]}", "--add-entries-from-file=#{f.path}"], nil)
   end
 
-  def remove_entry(entry)
-    execute_firewall_cmd(["--ipset=#{@resource[:name]}", "--remove-entry=#{entry}"], nil)
+  def remove_entries_from_file(entries)
+    f = Tempfile.new('ipset')
+    entries.each { |e| f.write(e + "\n") }
+    f.close
+    execute_firewall_cmd(["--ipset=#{@resource[:name]}", "--remove-entries-from-file=#{f.path}"], nil)
   end
 
   def entries=(should_entries)
@@ -96,8 +102,8 @@ def entries=(should_entries)
     cur_entries = entries
     delete_entries = cur_entries - should_entries
     add_entries = should_entries - cur_entries
-    delete_entries.each { |e| remove_entry(e) }
-    add_entries.each { |e| add_entry(e) }
+    remove_entries_from_file(delete_entries) if delete_entries
+    add_entries_from_file(add_entries) if add_entries
   end
 
   def destroy

spec/unit/puppet/provider/firewalld_ipset_spec.rb

@@ -17,6 +17,13 @@
   let(:provider) { resource.provider }
 
   before do
+    tempfile = stub('tempfile', class: Tempfile,
+                                write: true,
+                                flush: true,
+                                close!: true,
+                                close: true,
+                                path: '/tmp/ipset-rspec')
+    Tempfile.stubs(:new).returns(tempfile)
     provider.class.stubs(:execute_firewall_cmd).with(['--get-ipsets'], nil).returns('white black')
     provider.class.stubs(:execute_firewall_cmd).with(['--state'], nil, false, false, false).returns(Object.any_instance.stubs(exitstatus: 0)) # rubocop:disable RSpec/AnyInstance
     provider.class.stubs(:execute_firewall_cmd).with(['--info-ipset=white'], nil).returns('white
@@ -67,8 +74,7 @@
         resource.expects(:[]).with(:manage_entries).returns(true)
         resource.expects(:[]).with(:entries).returns(['192.168.0/24', '10.0.0/8'])
         provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet', '--option=hashsize=1024', '--option=maxelem=65536'], nil)
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0/24'], nil)
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0/8'], nil)
+        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entries-from-file=/tmp/ipset-rspec'], nil)
         provider.create
       end
     end
@@ -89,8 +95,7 @@
         resource.expects(:[]).with(:entries).returns(['192.168.0/24', '10.0.0/8']).at_least_once
         provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet'], nil)
         provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet', '--option=hashsize=2048'], nil)
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0/24'], nil).at_least_once
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0/8'], nil).at_least_once
+        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entries-from-file=/tmp/ipset-rspec'], nil).at_least_once
         provider.expects(:execute_firewall_cmd).with(['--delete-ipset=white'], nil)
         provider.create
         provider.hashsize = 2048
@@ -110,10 +115,8 @@
         resource.expects(:[]).with(:entries).returns(['192.168.0.0/24', '10.0.0.0/8']).at_least_once
         provider.expects(:entries).returns(['192.168.0.0/24', '10.0.0.0/8'])
         provider.expects(:execute_firewall_cmd).with(['--new-ipset=white', '--type=hash:net', '--option=family=inet'], nil)
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.0.0/24'], nil).at_least_once
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=10.0.0.0/8'], nil).at_least_once
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entry=192.168.14.0/24'], nil)
-        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--remove-entry=192.168.0.0/24'], nil)
+        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--remove-entries-from-file=/tmp/ipset-rspec'], nil)
+        provider.expects(:execute_firewall_cmd).with(['--ipset=white', '--add-entries-from-file=/tmp/ipset-rspec'], nil).at_least_once
         provider.create
         provider.entries = ['192.168.14.0/24', '10.0.0.0/8']
       end

spec/unit/puppet/type/firewalld_ipset_spec.rb

@@ -5,6 +5,13 @@
 describe Puppet::Type.type(:firewalld_ipset) do
   before do
     Puppet::Provider::Firewalld.any_instance.stubs(:state).returns(:true) # rubocop:disable RSpec/AnyInstance
+    tempfile = stub('tempfile', class: Tempfile,
+                                write: true,
+                                flush: true,
+                                close!: true,
+                                close: true,
+                                path: '/tmp/ipset-rspec')
+    Tempfile.stubs(:new).returns(tempfile)
   end
 
   describe 'type' do
@@ -116,8 +123,7 @@
 
     it 'creates' do
       provider.expects(:execute_firewall_cmd).with(['--new-ipset=whitelist', '--type=hash:ip'], nil)
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=192.168.2.2'], nil)
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=10.72.1.100'], nil)
+      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entries-from-file=/tmp/ipset-rspec'], nil)
       provider.create
     end
 
@@ -128,16 +134,15 @@
 
     it 'sets entries' do
       provider.expects(:entries).returns([])
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=192.168.2.2'], nil)
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=10.72.1.100'], nil)
+      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entries-from-file=/tmp/ipset-rspec'], nil)
+      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--remove-entries-from-file=/tmp/ipset-rspec'], nil)
       provider.entries = ['192.168.2.2', '10.72.1.100']
     end
 
     it 'removes unconfigured entries' do
       provider.expects(:entries).returns(['10.9.9.9', '10.8.8.8', '10.72.1.100'])
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entry=192.168.2.2'], nil)
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--remove-entry=10.9.9.9'], nil)
-      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--remove-entry=10.8.8.8'], nil)
+      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--add-entries-from-file=/tmp/ipset-rspec'], nil)
+      provider.expects(:execute_firewall_cmd).with(['--ipset=whitelist', '--remove-entries-from-file=/tmp/ipset-rspec'], nil)
       provider.entries = ['192.168.2.2', '10.72.1.100']
     end
   end