Merge pull request #399 from gnfzdz/main

Updat ZoneTransaction to support operations when firewalld is offline

SUMMARY

Fixes #398 by checking the flag self.fw_offline and calling the offline specific APIs when the flag is true.
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
ansible.posix.firewalld
ADDITIONAL INFORMATION
When attempting to add or remove zones, the ansible.posix.firewalld module would always call APIs as if it was online.
Specifically, the ZoneTransaction class assumed that self.fw  was a FirewallClient, but when the daemon is offline, it is instead either a Firewall or Firewall_test based on the version of firewalld installed.
See #398  for additional background.

Sample task
- name: 'Zone example - Create new zone'
  ansible.posix.firewalld:
    zone: "example"
    state: "present"
    permanent: Yes
Before
The full traceback is:
  File "/tmp/ansible_ansible.posix.firewalld_payload_04lptorx/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/module_utils/firewalld.py", line 111, in action_handler
    return action_func(*action_func_args)
  File "/tmp/ansible_ansible.posix.firewalld_payload_04lptorx/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/modules/firewalld.py", line 678, in get_enabled_permanent
fatal: [boot]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "icmp_block": null,
            "icmp_block_inversion": null,
            "immediate": false,
            "interface": null,
            "masquerade": null,
            "offline": null,
            "permanent": true,
            "port": null,
            "port_forward": null,
            "rich_rule": null,
            "service": null,
            "source": null,
            "state": "present",
            "target": null,
            "timeout": 0,
            "zone": "example"
        }
    },
    "msg": "ERROR: Exception caught: 'FirewallConfig' object is not callable"
}

After
changed: [boot] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "icmp_block": null,
            "icmp_block_inversion": null,
            "immediate": false,
            "interface": null,
            "masquerade": null,
            "offline": null,
            "permanent": true,
            "port": null,
            "port_forward": null,
            "rich_rule": null,
            "service": null,
            "source": null,
            "state": "present",
            "target": null,
            "timeout": 0,
            "zone": "example"
        }
    },
    "msg": "Permanent operation, Added zone example, Changed zone example to present, (offline operation: only on-disk configs were altered)"

Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net>

changelogs/fragments/399_firewalld_create_remove_zone_when_offline.yml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - Fixed a bug where firewalld module fails to create/remove zones when the daemon is stopped

plugins/modules/firewalld.py

@@ -675,25 +675,33 @@ class ZoneTransaction(FirewallTransaction):
         self.module.fail_json(msg=self.tx_not_permanent_error_msg)
 
     def get_enabled_permanent(self):
-        zones = self.fw.config().listZones()
-        zone_names = [self.fw.config().getZone(z).get_property("name") for z in zones]
-        if self.zone in zone_names:
-            return True
+        if self.fw_offline:
+            zones = self.fw.config.get_zones()
+            zone_names = [self.fw.config.get_zone(z).name for z in zones]
         else:
-            return False
+            zones = self.fw.config().listZones()
+            zone_names = [self.fw.config().getZone(z).get_property("name") for z in zones]
+        return self.zone in zone_names
 
     def set_enabled_immediate(self):
         self.module.fail_json(msg=self.tx_not_permanent_error_msg)
 
     def set_enabled_permanent(self):
-        self.fw.config().addZone(self.zone, FirewallClientZoneSettings())
+        if self.fw_offline:
+            self.fw.config.new_zone(self.zone, FirewallClientZoneSettings().settings)
+        else:
+            self.fw.config().addZone(self.zone, FirewallClientZoneSettings())
 
     def set_disabled_immediate(self):
         self.module.fail_json(msg=self.tx_not_permanent_error_msg)
 
     def set_disabled_permanent(self):
-        zone_obj = self.fw.config().getZoneByName(self.zone)
-        zone_obj.remove()
+        if self.fw_offline:
+            zone = self.fw.config.get_zone(self.zone)
+            self.fw.config.remove_zone(zone)
+        else:
+            zone_obj = self.fw.config().getZoneByName(self.zone)
+            zone_obj.remove()
 
 
 class ForwardPortTransaction(FirewallTransaction):

tests/integration/targets/firewalld/tasks/run_all_tests.yml

@@ -16,6 +16,9 @@
 # firewalld source operation test cases
 - include_tasks: source_test_cases.yml
 
+# firewalld zone operation test cases
+- include_tasks: zone_test_cases.yml
+
 # firewalld zone target operation test cases
 - include_tasks: zone_target_test_cases.yml
 

tests/integration/targets/firewalld/tasks/zone_test_cases.yml

@@ -0,0 +1,47 @@
+- name: firewalld create zone custom
+  firewalld:
+    zone: custom
+    permanent: True
+    state: present
+  register: result
+
+- name: assert firewalld custom zone created worked
+  assert:
+    that:
+    - result is changed
+
+- name: firewalld create zone custom rerun (verify not changed)
+  firewalld:
+    zone: custom
+    permanent: True
+    state: present
+  register: result
+
+- name: assert firewalld custom zone created worked (verify not changed)
+  assert:
+    that:
+    - result is not changed
+
+- name: firewalld remove zone custom
+  firewalld:
+    zone: custom
+    permanent: True
+    state: absent
+  register: result
+
+- name: assert firewalld custom zone removed worked
+  assert:
+    that:
+    - result is changed
+
+- name: firewalld remove custom zone rerun (verify not changed)
+  firewalld:
+    zone: custom
+    permanent: True
+    state: absent
+  register: result
+
+- name: assert firewalld custom zone removed worked (verify not changed)
+  assert:
+    that:
+    - result is not changed