Add firewall_backend option
#252
Conversation
README.md
Outdated
| @@ -31,6 +31,7 @@ class { 'firewalld': } | |||
| * `service_ensure`: Whether the service should be running or not (default: running) | |||
| * `service_enable`: Whether to enable the service | |||
| * `default_zone`: Optional, set the default zone for interfaces (default: undef) | |||
| * `firewallbackend`: Optional, set the firewall backend for firewalld (default: undef) | |||
There was a problem hiding this comment.
Suggest changing to firewall_backend to match the current pattern and read easier.
spec/classes/init_spec.rb
Outdated
| } | ||
| end | ||
|
|
||
| it do |
There was a problem hiding this comment.
could you please add similar under the context 'with defaults for all parameters' that shows it is_expected.not_to contain_augeas('firewalld::firewallbackend')
|
Great PR, thank you! Have a couple small changes and this should be good to merge. |
|
Could you please add a couple acceptance tests that shows firewalld running with iptables and nftables? That would help ensure this stays supported. |
florianfa
force-pushed
the
master
branch
2 times, most recently
from
5d747b4
to
c7cb589
Compare
| @@ -235,6 +236,15 @@ | |||
| } | |||
| } | |||
|
|
|||
| if $firewall_backend { | |||
| augeas { | |||
| 'firewalld::firewall_backend': | |||
There was a problem hiding this comment.
This is only valid in firewalld >= 0.6.0 and will cause an error in the logs about unsupported options on versions less than that. Take this, it's dangerous to go alone #255
There was a problem hiding this comment.
From a style perspective this should read like
augeas { 'firewalld::firewall_backend':
changes => ["set FirewallBackend \"${firewall_backend}\"",],
}
|
I've added a patch here that should take care of things. Happy to push it directly if @florianfa wants to enable pushes from maintainers. |
|
Hello trevor-vaughan, Regards, |
trevor-vaughan
force-pushed
the
master
branch
from
c7cb589
to
ee66cd3
Compare
trevor-vaughan
force-pushed
the
master
branch
from
ee66cd3
to
8eacc53
Compare
* Added support for firewalld_version fact * Updated spec tests * Aligned parameters in init.pp * Rubocop fixes are in the EL8 PR
trevor-vaughan
force-pushed
the
master
branch
from
8eacc53
to
b4d4ab4
Compare
| Optional[String] $default_service_zone = undef, | ||
| Optional[String] $default_port_zone = undef, | ||
| Optional[String] $default_port_protocol = undef, | ||
| Enum['present','absent','latest','installed'] $package_ensure = 'installed', |
There was a problem hiding this comment.
Given the propensity for data types to be quite long, aligning them can get pretty crazy. It also causes a lot of needless code churn that makes reviews harder to do.
Pull Request (PR) description
add option FirewallBackend to firewalld
This Pull Request (PR) fixes the following issues
Fixes #235