Add new feature to firewalld module allowing the default zone to be set.

[gnfzdz]

SUMMARY

Implements #296 adding support for setting the default zone in the firewalld module.

This pull request is meant to help visualize the changes mentioned in the linked issue and I'm more interested in feedback than actually having it merged.

Related to my concerns from comments in that issue:

1. For consistency with the rest of the module, the state parameter is retained although it may have unclear semantics when considering negative values.
2. When the indicated zone is currently the default AND the combination of the state and default parameters would suggest that the zone should NOT be, the module reverts the default zone back to the upstream default, public
3. As far as i can tell firewalld only supports making the change permanent. When the daemon is running, the firewalld client used will also make the change immediately. The current implementation requires the permanent and immediate parameters to explicitly match the required combinations above (based on whether the daemon is running).

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

ansible.posix.firewalld

ADDITIONAL INFORMATION

  - name: Update default zone to trusted
    ansible.posix.firewalld:
      zone: trusted
      default: True
      permanent: True
      state: enabled

TASK [firewalld : Update default zone to trusted] ******************************
task path: /root/ansible_collections/ansible/posix/tests/output/.tmp/integration/firewalld-6xmdfto8-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/zone_default_test_cases.yml:7
Using module file /root/ansible_collections/ansible/posix/plugins/modules/firewalld.py
Pipelining is enabled.
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<testhost> EXEC /bin/sh -c '/usr/bin/python3.10 && sleep 0'
changed: [testhost] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "default": true,
            "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": "enabled",
            "target": null,
            "timeout": 0,
            "zone": "trusted"
        }
    },
    "msg": "Permanent operation, Updated default zone to trusted, (offline operation: only on-disk configs were altered)"
}

Note: The new integration test cases currently fail for the scenario where the daemon is up AND they are run in a container (as with CI/CD). In that scenario, it correctly updates the default zone to /etc/firewalld/firewalld.conf but fails when attempting to immediately apply the change to nftables. In my tests running on bare metal and a full VM they appear to work without issue.

TASK [firewalld : Update default zone to trusted] ******************************
task path: /root/ansible_collections/ansible/posix/tests/output/.tmp/integration/firewalld-8puctpub-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/zone_default_test_cases.yml:7
Using module file /root/ansible_collections/ansible/posix/plugins/modules/firewalld.py
Pipelining is enabled.
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<testhost> EXEC /bin/sh -c '/usr/bin/python3.10 && sleep 0'
The full traceback is:
  File "/tmp/ansible_ansible.posix.firewalld_payload_t7hmnzkc/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/module_utils/firewalld.py", line 112, in action_handler
    return action_func(*action_func_args)
  File "/tmp/ansible_ansible.posix.firewalld_payload_t7hmnzkc/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/modules/firewalld.py", line 728, in set_enabled_permanent
  File "/usr/lib/python3.10/site-packages/firewall/client.py", line 50, in _impl
    return func(*args, **kwargs)
  File "/usr/lib/python3.10/site-packages/firewall/client.py", line 3257, in setDefaultZone
    self.fw.setDefaultZone(zone)
  File "/usr/lib64/python3.10/site-packages/dbus/proxies.py", line 141, in __call__
    return self._connection.call_blocking(self._named_service,
  File "/usr/lib64/python3.10/site-packages/dbus/connection.py", line 652, in call_blocking
    reply_message = self.send_message_with_reply_and_block(
fatal: [testhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "default": true,
            "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": "enabled",
            "target": null,
            "timeout": 0,
            "zone": "trusted"
        }
    },
    "msg": "ERROR: Exception caught: org.fedoraproject.FirewallD1.Exception: COMMAND_FAILED: '{\"chain\": \"filter_INPUT_ZONES\", \"expr\": [{\"goto\": {\"target\": \"filter_IN_public\"}}], \"family\": \"inet\", \"table\": \"firewalld\"}' Permanent operation"
}

[softwarefactory-project-zuul[bot]]

Build failed.

:x: ansible-changelog-fragment FAILURE in 16s :x: ansible-test-sanity-docker-devel FAILURE in 7m 08s (non-voting) :x: ansible-test-sanity-docker-milestone FAILURE in 7m 04s (non-voting) :x: ansible-test-sanity-docker-stable-2.9 FAILURE in 9m 23s :x: ansible-test-sanity-docker-stable-2.10 FAILURE in 8m 19s :x: ansible-test-sanity-docker-stable-2.11 FAILURE in 10m 41s :x: ansible-test-sanity-docker-stable-2.12 FAILURE in 7m 46s :x: ansible-test-sanity-docker-stable-2.13 FAILURE in 7m 52s :heavy_check_mark: ansible-test-units-posix-python39 SUCCESS in 5m 22s :heavy_check_mark: ansible-test-units-posix-python310 SUCCESS in 5m 47s :heavy_check_mark: ansible-galaxy-importer SUCCESS in 3m 57s :heavy_check_mark: build-ansible-collection SUCCESS in 3m 15s

[softwarefactory-project-zuul[bot]]

Build failed.

:x: ansible-changelog-fragment FAILURE in 17s :heavy_check_mark: ansible-test-sanity-docker-devel SUCCESS in 9m 53s (non-voting) :x: ansible-test-sanity-docker-milestone FAILURE in 7m 07s (non-voting) :heavy_check_mark: ansible-test-sanity-docker-stable-2.9 SUCCESS in 10m 02s :heavy_check_mark: ansible-test-sanity-docker-stable-2.10 SUCCESS in 8m 23s :heavy_check_mark: ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 21s :heavy_check_mark: ansible-test-sanity-docker-stable-2.12 SUCCESS in 7m 42s :heavy_check_mark: ansible-test-sanity-docker-stable-2.13 SUCCESS in 7m 30s :heavy_check_mark: ansible-test-units-posix-python39 SUCCESS in 5m 23s :heavy_check_mark: ansible-test-units-posix-python310 SUCCESS in 5m 11s :heavy_check_mark: ansible-galaxy-importer SUCCESS in 4m 11s :heavy_check_mark: build-ansible-collection SUCCESS in 3m 12s

[saito-hideki]

recheck

[softwarefactory-project-zuul[bot]]

Build failed. https://ansible.softwarefactory-project.io/zuul/buildset/f9f67cb750f446ee8e10f5995f81ac9c

:x: ansible-changelog-fragment FAILURE in 12s :x: ansible-test-sanity-docker-devel FAILURE in 11m 05s (non-voting) :heavy_check_mark: ansible-test-sanity-docker-milestone SUCCESS in 8m 19s (non-voting) :heavy_check_mark: ansible-test-sanity-docker-stable-2.9 SUCCESS in 10m 30s :heavy_check_mark: ansible-test-sanity-docker-stable-2.10 SUCCESS in 9m 11s :heavy_check_mark: ansible-test-sanity-docker-stable-2.11 SUCCESS in 12m 53s :x: ansible-test-sanity-docker-stable-2.12 FAILURE in 6m 20s :x: ansible-test-sanity-docker-stable-2.13 FAILURE in 6m 20s :heavy_check_mark: ansible-test-units-posix-python39 SUCCESS in 5m 11s :heavy_check_mark: ansible-test-units-posix-python310 SUCCESS in 4m 59s :heavy_check_mark: ansible-galaxy-importer SUCCESS in 4m 14s :heavy_check_mark: build-ansible-collection SUCCESS in 6m 47s

[gnfzdz]

@maxamillion Can you take a look at the proposal here as well? There's a bit of clean up required, but I just want to confirm that you don't object to the description of the implementation above (bullet points 2 & 3).

[maxamillion]

@gnfzdz this is great, I love the idea. +1

[maxamillion]

@gnfzdz it appears we have some merge conflicts :(