Skip to content

Commit 3ac267d

Browse files
committed
refactor: ensure that using previous replaced does not remove files and reload unless necessary
The old implemenation of `previous: replaced` would remove all files and reload the firewall every time, even if not necessary - it wasn't technically idempotent, although it would check the new files to see if they matched the old files, and report `changed: false` if nothing actually changed state. The new implementation uses an in-memory backend to apply the changes, then checks if anything changed, and then removes the files and reloads firewall only if something actually changed. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
1 parent f9aa699 commit 3ac267d

27 files changed

Lines changed: 4037 additions & 1141 deletions

.sanity-ansible-ignore-2.14.txt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
plugins/modules/firewall_lib.py validate-modules:missing-gplv3-license
2-
plugins/modules/firewall_lib_facts.py validate-modules:missing-gplv3-license
32
plugins/modules/firewall_lib.py no-unicode-literals!skip
43
plugins/modules/firewall_lib.py future-import-boilerplate!skip
54
plugins/modules/firewall_lib.py validate-modules:illegal-future-imports
5+
plugins/modules/firewall_lib_facts.py validate-modules:missing-gplv3-license
6+
plugins/modules/firewall_lib_facts.py no-unicode-literals!skip
7+
plugins/modules/firewall_lib_facts.py future-import-boilerplate!skip
8+
plugins/modules/firewall_lib_facts.py validate-modules:illegal-future-imports
9+
plugins/modules/firewall_lib_facts.py validate-modules:import-before-documentation
610
tests/firewall/unit/test_firewall_lib.py no-unicode-literals!skip
11+
plugins/module_utils/firewall_lsr/get_config.py future-import-boilerplate!skip
12+
plugins/module_utils/firewall_lsr/get_config.py no-unicode-literals!skip

.sanity-ansible-ignore-2.15.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
plugins/modules/firewall_lib.py validate-modules:missing-gplv3-license
22
plugins/modules/firewall_lib_facts.py validate-modules:missing-gplv3-license
3+
plugins/modules/firewall_lib_facts.py future-import-boilerplate!skip
34
plugins/modules/firewall_lib.py no-unicode-literals!skip
45
plugins/modules/firewall_lib.py future-import-boilerplate!skip
56
tests/firewall/unit/test_firewall_lib.py no-unicode-literals!skip
7+
plugins/module_utils/firewall_lsr/get_config.py future-import-boilerplate!skip
8+
plugins/modules/firewall_lib_facts.py no-unicode-literals!skip
9+
plugins/module_utils/firewall_lsr/get_config.py no-unicode-literals!skip

.sanity-ansible-ignore-2.16.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
plugins/modules/firewall_lib.py validate-modules:missing-gplv3-license
22
plugins/modules/firewall_lib_facts.py validate-modules:missing-gplv3-license
3+
plugins/modules/firewall_lib_facts.py no-unicode-literals!skip
4+
plugins/modules/firewall_lib_facts.py future-import-boilerplate!skip
35
plugins/modules/firewall_lib.py no-unicode-literals!skip
46
plugins/modules/firewall_lib.py future-import-boilerplate!skip
57
tests/firewall/unit/test_firewall_lib.py no-unicode-literals!skip
8+
plugins/module_utils/firewall_lsr/get_config.py future-import-boilerplate!skip
9+
plugins/module_utils/firewall_lsr/get_config.py no-unicode-literals!skip

.sanity-ansible-ignore-2.20.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
plugins/modules/firewall_lib.py validate-modules:missing-gplv3-license
2+
plugins/modules/firewall_lib_facts.py validate-modules:missing-gplv3-license

README.md

Lines changed: 89 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,31 @@ For reference, by default, `firewall_config` takes ~3KB when converted to a stri
7070

7171
#### firewall_config
7272

73-
This ansible fact shows the permanent configuration of
74-
of firewalld on the managed node in dictionary format.
75-
The top level of the fact is made up of three keys:
76-
77-
* `default`
78-
* `custom`
79-
* `default_zone`
80-
81-
Each dictionaries custom and default have the keys:
73+
This ansible fact shows the configuration of of firewalld on the managed node in
74+
dictionary format. The top level of the fact is made up of the following keys:
75+
76+
* `default` - the default values
77+
* `custom_permanent`, `custom` (deprecated) - the permanent settings not
78+
including the defaults
79+
* `custom_permanent_with_defaults` - the permanent settings including the
80+
defaults
81+
* `custom_runtime_with_defaults`, `current` (deprecated) - the runtime settings
82+
including the defaults
83+
* `runtime_only` - the runtime settings not including the defaults
84+
* `default_zone` - the default zone
85+
* `firewalld_conf` - the firewalld.conf settings
86+
* `fallback_default_zone`- the built-in default zone if there is no
87+
firewalld.conf
88+
89+
For `default`, `custom_permanent`, `custom`, `custom_permanent_with_defaults`,
90+
`custom_runtime_with_defaults`, `current`, `runtime_only` - these are type
91+
`dict` - the key is one of the keywords in the following list. If `detailed:
92+
false`, then the value is a list of names e.g. list of zone names, list of
93+
service names. If `detailed: true`, then the value is a `dict` - the key is the
94+
name of the item e.g. the zone name, service name, etc. - and the value is the
95+
detailed information for that item e.g. for `zone`, it will list the ports,
96+
services, forward ports, etc. for that zone - for `service` it will list the
97+
ports and description for that service.
8298

8399
* `zones`
84100
* `services`
@@ -87,129 +103,15 @@ Each dictionaries custom and default have the keys:
87103
* `ipsets`
88104
* `policies` (if supported by remote host's firewalld installation)
89105

90-
Each of the keys contains a list of elements present in
91-
permanent configuration for each respective option.
92-
93-
`custom` will have a list of subdictionaries for each key,
94-
providing a more detailed description.
95-
96-
`default` will have only the names of each setting,
97-
unless the detailed option is supplied, in which case
98-
it will be structured in the same manner as custom.
99-
100-
`default_zone` contains the configured default zone
101-
for the managed node's firewalld installation. It
102-
is a string value.
103-
104106
JSON representation of the structure of firewall_config fact:
105107

106108
```json
107109
{
108-
"default": {...},
109-
"custom": {...},
110+
"default": {"zones": ..., "services": ...},
111+
"custom_permanent_with_defaults": {"zones": ..., "services": ...},
112+
"custom_runtime_with_defaults": {"zones": ..., "services": ...},
110113
"default_zone": "public",
111-
}
112-
```
113-
114-
#### default
115-
116-
The default subdictionary of firewall_config contains the default
117-
configuration for the managed node's firewalld configuration.
118-
This subdictionary only changes with changes to the managed node's
119-
firewalld installation.
120-
121-
default without detailed parameter set to true
122-
123-
```json
124-
"default": {
125-
"zones": ["public",...],
126-
"services": ["amanda_client",...],
127-
"icmptypes": [...],
128-
"helpers": [...],
129-
"ipsets": [...],
130-
"policies": [...],
131-
}
132-
```
133-
134-
default when parameter set to true
135-
136-
```json
137-
"default": {
138-
"zones": {
139-
"public": {
140-
...
141-
},
142-
...
143-
},
144-
"services": {
145-
"amanda_client":{
146-
...
147-
},
148-
...
149-
},
150-
"icmptypes": {
151-
...
152-
},
153-
"helpers": {
154-
...
155-
},
156-
"ipsets": {
157-
...
158-
},
159-
"policies": {
160-
...
161-
},
162-
}
163-
```
164-
165-
#### custom
166-
167-
The custom subdictionary contains any differences from the default
168-
firewalld configuration. This includes a repeat for a default
169-
element if that element has been modified in any way, and any new
170-
elements introduced in addition to the defaults.
171-
172-
This subdictionary will be modified by any changes to the
173-
firewalld installation done locally or remotely via the
174-
firewall system role.
175-
176-
If the managed nodes firewalld settings are not different from the defaults,
177-
the custom key and subdictionary will not be present in firewall_config.
178-
Additionally, if any of firewalld's settings have not changed from the default,
179-
there will not be a key-value pair for that setting in custom.
180-
181-
Below is the state of the custom subdictionary where at least one
182-
permanent change was made to each setting:
183-
184-
```json
185-
"custom": {
186-
"zones": {
187-
"custom_zone": {
188-
...
189-
},
190-
...
191-
},
192-
"services": {
193-
"custom_service": {
194-
...
195-
},
196-
...
197-
},
198-
"icmptypes": {
199-
"custom": {
200-
...
201-
},
202-
...
203-
},
204-
"helpers": {
205-
...
206-
},
207-
"ipsets": {
208-
...
209-
},
210-
"policies": {
211-
...
212-
},
114+
... other fields ...
213115
}
214116
```
215117

@@ -279,7 +181,7 @@ bound/assigned to another zone.
279181

280182
That means that if there is no zone assigned to a connection, interface or
281183
source, only the default zone is used. The zone should exist before setting
282-
it as the default zone.
184+
it as the default zone. This affects both permanent and runtime.
283185

284186
```yaml
285187
firewall:
@@ -667,6 +569,8 @@ avoid confusing behavior.
667569

668570
### interface_pci_id
669571

572+
NOTE: This requires NetworkManager.
573+
670574
String or list of interface PCI device IDs.
671575
Accepts PCI IDs if the wildcard `XXXX:YYYY` applies
672576
where:
@@ -818,23 +722,21 @@ NOTE: `service` - to see how to manage services, see the service section.
818722

819723
### runtime
820724

821-
Enable changes in runtime configuration. If `runtime` parameter is not provided, the default will be set to `True`.
725+
Enable changes in runtime configuration. By default, this is `true` if the
726+
system is booted, or `false` if not booted (i.e. `bootc` system).
822727

823728
```yaml
824-
runtime: true
729+
runtime: false
825730
```
826731

827732
### permanent
828733

829-
Enable changes in permanent configuration. If `permanent` parameter is not provided, the default will be set to `True`.
734+
Enable changes in permanent configuration. By default, this is `true`.
830735

831736
```yaml
832-
permanent: true
737+
permanent: false
833738
```
834739

835-
The permanent and runtime settings are independent, so you can set only the runtime, or only the permanent. You cannot
836-
set both permanent and runtime to `false`.
837-
838740
### previous
839741

840742
If you want to completely wipe out all existing firewall configuration, add
@@ -949,6 +851,58 @@ firewall:
949851
state: enabled
950852
```
951853

854+
## Check mode/Diff mode
855+
856+
If check mode or diff mode is used, the role will report the difference between the desired
857+
configuration and the actual configuration. The difference will be reported as a `dict` with
858+
two top level keys - `permanent` for the permanent settings, and `runtime` for the runtime settings.
859+
Each of these will have a key `added` for the settings that would be added to the system, and
860+
`removed` for the settings that would be removed from the system. Each one of these is a `dict`
861+
where the key is one of the settings categories described above like `zones`, `services`, etc.
862+
863+
For example:
864+
865+
```json
866+
{
867+
"permanent": {
868+
"added": {
869+
"default_zone": "dmz",
870+
"ipsets": {
871+
"customipset-ipv4": {
872+
"description": "Custom IPSet for testing purposes",
873+
"entries": [
874+
"127.0.0.1",
875+
"8.8.8.8"
876+
],
877+
"short": "Custom",
878+
"type": "hash:ip"
879+
},
880+
{
881+
...
882+
}
883+
},
884+
"zones": {
885+
"dmz": {
886+
"services": [
887+
"http"
888+
]
889+
},
890+
{
891+
...
892+
}
893+
},
894+
...
895+
},
896+
"removed": {
897+
...
898+
}
899+
},
900+
"runtime": {
901+
...
902+
}
903+
}
904+
```
905+
952906
## Example Playbooks
953907

954908
Erase all existing configuration, and enable ssh service:

files/get_files_checksums.sh

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)