Add new properties to firewalld_ipset type and improve logging of changes
#170
Conversation
Define firewalld ipset options using properties: - family - hashsize - maxelem - timeout This change is backward compatible with the definition of options using the `options` property.
/32 is stripped by ipset when the rules is created.
When you have a long list of ips in a set, it is easier to only show what actually changed (removal and addition).
We parse actual ipset rules to define existing resource. We avoid declaration of getter.
Sometimes firewalld ipset entries are modified by another process than puppet. If you want to declare the ipset with puppet but not manage the entries, we add the `manage_entries` option. Obviously, by default entries are managed by puppet. We throw an error if you try to define ipset entries with puppet and don't want to manage entries (`manage_entries = false`)
Add new firewalld_ipset option: manage_entries
From documentation: Parameters change how Puppet manages a resource, but do not necessarily map directly to something measurable.
This option is useful if you want to control ipsets only with puppet.
Add option to remove unknown ipsets
If ipset is created with `instances` and `prefetch`, there is no need to verify if the set exists using firewalld-cmd
Do not check if ipset exists again
| @@ -0,0 +1,15 @@ | |||
| module PuppetX | |||
There was a problem hiding this comment.
Is it a bit overkill to create a PuppetX class for this property rather than just using a validate block in the type? This would fall outside of environment isolation provided by puppet generate types.....
There was a problem hiding this comment.
Indeed, it is more code but validate is not specific to a field/property. We might want to reuse that validation for other fields and avoid code duplication
self.instances method on the provider class did not verify if firewalld had a correct state before issuing `firewall-cmd` commands. This was a problem as we were trying to collect ipset informations without checking if firewalld was ready. So we move the `available?` implementation to the class method to be able to verify the current firewalld state.
Verify if firewalld is online before self.execute_firewall_cmd
alexjfisher
changed the title
firewalld_ipset type and improve logging of changes
|
|
||
| it 'removes /32 in set members' do | ||
| expect(resource[:entries]).to eq ['8.8.8.8', '9.9.9.9'] |
There was a problem hiding this comment.
For future reference I'd avoid using public IPs. RFC5735 describes subnets exactly for this use case:
192.0.2.0/24 - This block is assigned as "TEST-NET-1" for use in documentation and example code.
There's also TEST-NET-2 (198.51.100.0/24) and TEST-NET-3 (203.0.113.0/24).
|
Also... @jfroche sorry for the notifications I may be spamming you with. We're adjusting PR titles for the benefit of the changelog that is about to accompany a new 4.0.0 release :) |
Improve
firewalld_ipsettype:Add more tests