Skip to content

Commit

Permalink
Speed up ipset entries changes
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jfroche committed Apr 18, 2024
1 parent 95507c4 commit 5b66307
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 22 deletions.
20 changes: 13 additions & 7 deletions lib/puppet/provider/firewalld_ipset/firewall_cmd.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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") }

Check failure on line 85 in lib/puppet/provider/firewalld_ipset/firewall_cmd.rb

View workflow job for this annotation

GitHub Actions / Puppet / Static validations

Style/StringConcatenation: Prefer string interpolation to string concatenation. (https://rubystyle.guide#string-interpolation)
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") }

Check failure on line 92 in lib/puppet/provider/firewalld_ipset/firewall_cmd.rb

View workflow job for this annotation

GitHub Actions / Puppet / Static validations

Style/StringConcatenation: Prefer string interpolation to string concatenation. (https://rubystyle.guide#string-interpolation)
f.close
execute_firewall_cmd(["--ipset=#{@resource[:name]}", "--remove-entries-from-file=#{f.path}"], nil)
end

def entries=(should_entries)
Expand All @@ -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
Expand Down
19 changes: 11 additions & 8 deletions spec/unit/puppet/provider/firewalld_ipset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
19 changes: 12 additions & 7 deletions spec/unit/puppet/type/firewalld_ipset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down

0 comments on commit 5b66307

Please sign in to comment.