Speed up ipset entries changes #175
Conversation
|
What is the format of the files that feed this function? |
|
@Rovanion some example: ipset::set:
some_name:
type: hash:net
hashsize: 1024
family: inet6
manage_entries: true
set:
- 2a02:2c40::1
- 2a02:2c40::2
# We have a wrapper around firewalld module, should be similar
wrapped::firewalld::rich_rules:
allow_from_some_name:
zone: public
family: ipv6
source:
ipset: some_name
service: some_service
action: accept
<?xml version="1.0" encoding="utf-8"?>
<ipset type="hash:net">
<option name="hashsize" value="1024"/>
<option name="family" value="inet6"/>
<entry>2a02:2c40::1</entry>
<entry>2a02:2c40::2</entry>
</ipset> |
|
Dear @jfroche, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @jfroche, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
1 similar comment
|
Dear @jfroche, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @jfroche, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
Dear @jfroche, thanks for the PR! This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase? You can find my sourcecode at voxpupuli/vox-pupuli-tasks |
|
@dhoppe would you like us to fix the merge conflicts? |
|
@jovandeginste That would be great. Thank you very much. |
|
rebased on master branch |
| flush: true, | ||
| close!: true, | ||
| close: true, | ||
| path: '/tmp/ipset-rspec') |
There was a problem hiding this comment.
Reading from /tmp is insecure and could introduce a race condition. Suggest using a dedicated directory that is root:root 0700 and outside of temp.
There was a problem hiding this comment.
not sure I need to notify using @ghoneycutt , sorry for the spam
There was a problem hiding this comment.
Why not just use a real tempfile?
This is going to be an issue if a system is trying to run multiple tests at the same time.
A before(:each) with an instance variable should be fine (just tell rubocop to shut up).
|
Are we speaking about using /tmp during unittest not being secure ? |
|
ping @ghoneycutt |
|
I was mistaken, for the test it seems fine. |
|
can we pick this PR up again, @ghoneycutt ? |
jovandeginste
force-pushed
the
speedup
branch
from
d2de4c1 to
078ae2c
Compare
|
Can we get --add-entries-from-file and --remove-entries-from-file functionality in this module (obviously still keep the --add-entry and --remove-entry functionality) over the line please and in the latest version. |
|
Can you rebase off head for the CI? |
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
We now use
--add-entries-from-fileand--remove-entries-from-filetochange 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
first pull request was here: kuleuven#4