Ansible facts with firewalld configuration

[richm]

fyi - here is an example of a facts module from the selinux role - https://github.com/linux-system-roles/selinux/blob/master/library/selinux_modules_facts.py and here is an example of one from the timesync role that is a shell script (Ansible modules do not have to be python): https://github.com/linux-system-roles/timesync/blob/master/library/timesync_provider.sh

[vrindle]

The structure of the files and the work you've done so far looks good. It definitely needs testing, but it does look good so far :D

[lgtm-com[bot]]

This pull request introduces 2 alerts when merging a53c0bb92cb211e96b20f76bec9812d84376f48e into cf526b7330df774e9decba3aba572d698d048883 - view on LGTM.com

new alerts:

• 2 for Unused local variable

[lgtm-com[bot]]

This pull request introduces 1 alert when merging 825c5db8a15f071059be0707d9b0dc975c402669 into cf526b7330df774e9decba3aba572d698d048883 - view on LGTM.com

new alerts:

• 1 for Unused local variable

[BrennanPaciorek]

There are two ways of going about parsing information for each option (service, zone, etc.), you can directly parse the xml for each one, or you can get the information using the firewalld client. I personally prefer the first one, as the code for doing so will be arguably cleaner with less conditionals for each setting. Which would be preferred?

[richm]

ok - this is looking pretty good - couple of things

• need a test for this e.g. tests/tests_firewall_facts.yml
• need to document this in README.md

[BrennanPaciorek]

Added features discussed, ran into a sanity test issue where the test cannot import firewall, does anyone know why this is showing for firewall_lib_facts.py but not firewall_lib.py? I do not see any ignores on this particular test. @vrindle @richm. EDIT: This was because I tried using an import in a try-except block under the assumption HAS_FIREWALLD was True, was fixed by nesting an if statement in second try-except block. Will push passing sanity tests here in a couple minutes

[BrennanPaciorek]

Looks like getSettingsDict() is not available on the latest supported version of firewalld for RHEL 7. working on fixing that now

[BrennanPaciorek]

Note: while working on the RHEL 7 compatibility, I realized that the add/delete services feature does not have support for the non-deprecated element helper, and only supports the deprecated element module: https://firewalld.org/documentation/man-pages/firewalld.service.html

I'll look into this after this feature is done, but it will most likely need a low-priority Bugzilla, particularly for when RHEL 7 reaches end of life

[richm]

This lgtm - @vrindle ?

[vrindle]

@richm lgtm