Salt: firewalld: Permanent and runtime configuration.

[hakoerber]

I'm adding new functionality to the firewalld state and module, and while doing so I saw that the handling of permanent and runtime configuration is a bit difficult.

I'm proposing a simplification that would look like this:

• The module retains the permanent parameters.
• The state by default does all changes in runtime and commits them to permanent config in the end.
• The persist switch for the state detemines whether this commit is done, which makes it possible to do runtime-only configuration.

I cannot think of any use case where one would want the state to _only_ update the runtime or permanent configuration. I think this runtime/permanent distinction was mainly added to firewalld for interactive use (so one can try out a new config without danger) and is not that useful for config management.

Those changes would cut the state runtime in half for most use cases, because currently, the whole config is applied twice, once for the runtime and once for the permanent config. A simple firewall-cmd --runtime-to-permanent instead of the latter would be much more efficient.

The only thing that would no longer be possible would be permanent changes without affecting runtime config, but as I said I cannot see a use for this.

Also paging @cmercier because we discussed a related issue before.

EDIT: I just saw that doing it the other way round (permanent changes only, then applying them to runtime with firewall-cmd --reload) would be better, because some options are only available for permanent config. This would also make the whole state execution atomic.

[jfindlay]

@whatevsz, thanks for your analysis. Also ping @dmyerscough.

[cmercier]

Sorry didn't see the notification before.
I agree about the runtime-to-permanent, but my main problem is that i'm using firewalld on ubuntu (14.04) so I don't have the last version and this function is not available for me.
I have a doubt about the --reload if I have access to it, I'm not working on firwalld right now.

[hakoerber]

Ubuntu 14.04LTS uses firewalld 0.3.7-1, which has the --reload option as far as I'm aware, so you should be fine. --runtime-to-permanent doesn't seem to be available though, but is not needed anyway for the changes.

[cmercier]

Ok great. I just hope that --reload is softer compare to a restart of the service because restart breaks connections (that was the case for the connection between my salt-master and salt-minions).

[hakoerber]

The manpage of firewall-cmd says the following:

       Reload firewall rules and keep state information. Current permanent configuration will become new runtime configuration, i.e. all runtime only changes done until reload are lost with reload if they have not been also in
       permanent configuration.

--complete-reload
       Reload firewall completely, even netfilter kernel modules. This will most likely terminate active connections, because state information is lost. This option should only be used in case of severe firewall problems. For
       example if there are state information problems that no connection can be established with correct firewall rules.

As we are using --reload and not --complete-reload, existing connection should be unaffected.

[cmercier]

I was looking to your commit and you should add some comments or information in the documentation part about the new parameter prune_services. For the rest it seems good, that's good the new function for service ports.

[damon-atkins]

I would have thought that prune_services should default to False.

I would have thought the norm would be to have an init.sls or set of them which

• Install the software
• Defines the service for the firewalld if software does not come with it (may be an extra check)
• firewall.present adds it to the existing list.

The default of prune_services sets the expectation firewall rules need to be separate to the configuration of the software. Making setting up a service a two step process.

What is also missing is an absent function. (which would warn if a present exists for the same setting)
Cheers