From 6ff5467c7a1fa2e891733035413dde336ccee764 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 21 Oct 2025 09:34:26 -0600 Subject: [PATCH] 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 --- README.md | 12 +- files/get_files_checksums.sh | 75 - library/firewall_lib.py | 1814 +++++++++++++++-- library/firewall_lib_facts.py | 174 +- module_utils/firewall_lsr/__init__.py | 0 module_utils/firewall_lsr/get_config.py | 1099 ++++++++++ tasks/firewalld.yml | 2 + tasks/main.yml | 207 +- tests/library | 1 + tests/roles/linux-system-roles.firewall/files | 1 - .../linux-system-roles.firewall/module_utils | 1 + tests/tests_firewall_fact.yml | 85 +- tests/tests_firewalld_conf.yml | 2 - tests/tests_forward_port.yml | 167 +- tests/tests_interface_pci.yml | 107 +- tests/tests_ipsets.yml | 153 +- tests/tests_purge_config.yml | 15 +- tests/tests_service.yml | 353 ++-- tests/tests_target.yml | 40 +- tests/tests_zone.yml | 41 +- tests/unit/test_firewall_lib.py | 60 +- tests/unit/test_get_config.py | 469 +++++ tox.ini | 2 + vars/main.yml | 4 - 24 files changed, 3894 insertions(+), 990 deletions(-) delete mode 100755 files/get_files_checksums.sh create mode 100644 module_utils/firewall_lsr/__init__.py create mode 100644 module_utils/firewall_lsr/get_config.py create mode 120000 tests/library delete mode 120000 tests/roles/linux-system-roles.firewall/files create mode 120000 tests/roles/linux-system-roles.firewall/module_utils create mode 100644 tests/unit/test_get_config.py diff --git a/README.md b/README.md index c2314db3..0bb67e61 100644 --- a/README.md +++ b/README.md @@ -818,23 +818,21 @@ NOTE: `service` - to see how to manage services, see the service section. ### runtime -Enable changes in runtime configuration. If `runtime` parameter is not provided, the default will be set to `True`. +Enable changes in runtime configuration. By default, this is `true` if the +system is booted, or `false` if not booted (i.e. `bootc` system). ```yaml -runtime: true +runtime: false ``` ### permanent -Enable changes in permanent configuration. If `permanent` parameter is not provided, the default will be set to `True`. +Enable changes in permanent configuration. By default, this is `true`. ```yaml -permanent: true +permanent: false ``` -The permanent and runtime settings are independent, so you can set only the runtime, or only the permanent. You cannot -set both permanent and runtime to `false`. - ### previous If you want to completely wipe out all existing firewall configuration, add diff --git a/files/get_files_checksums.sh b/files/get_files_checksums.sh deleted file mode 100755 index a98cf164..00000000 --- a/files/get_files_checksums.sh +++ /dev/null @@ -1,75 +0,0 @@ -#!/usr/bin/env bash - -set -euo pipefail - -python_cmd="$1" -firewall_conf_root="${2:-/etc/firewalld}" -remove="${3:-false}" -package="${4:-}" -firewall_usr_lib="${5:-}" -booted="${6:-}" - -listfile=$(mktemp) -firewallconf=$(mktemp) -# shellcheck disable=SC2064 -trap "rm -f $listfile $firewallconf" EXIT - -find "$firewall_conf_root" -name \*.xml | while read -r file; do - cksum=$(xmllint --c14n "$file" | sha256sum | awk '{print $1}') - if [ -n "$firewall_usr_lib" ]; then - usr_lib_file="${firewall_usr_lib}${file##"$firewall_conf_root"}" - if [ -f "$usr_lib_file" ]; then - cksum_usr_lib=$(xmllint --c14n "$usr_lib_file" | sha256sum | awk '{print $1}') - if [ "$cksum" != "$cksum_usr_lib" ]; then - echo "$cksum" "$file" - fi - else - echo "$cksum" "$file" - fi - else - echo "$cksum" "$file" - fi -done > "$listfile" - -set +o pipefail - -orig_conf="$firewall_conf_root/firewalld.conf" -remove_firewall_conf=true -if [ -f "$orig_conf" ]; then - if [ -z "$package" ] || rpm -V "$package" | grep -q "c ${orig_conf}$"; then - cp "$orig_conf" "$firewallconf" - "$python_cmd" -c 'import os, sys -from firewall.core.io.firewalld_conf import firewalld_conf -fc = firewalld_conf(sys.argv[1]) -fc.read(); os.unlink(sys.argv[1]) -fc.write() -' "$firewallconf" - cksum=$(sha256sum "$firewallconf" | awk '{print $1}') - echo "$cksum" "$orig_conf" >> "$listfile" - else - remove_firewall_conf=false - fi -fi - -set -o pipefail - -if [ "${remove:-false}" = true ]; then - find "$firewall_conf_root" -name \*.xml -exec rm -f {} \; - if [ "$remove_firewall_conf" = true ]; then - "$python_cmd" -c 'import sys -from firewall.core.io.firewalld_conf import firewalld_conf -fc = firewalld_conf(None) # open(None, "r") throws an Exception in fc.read() -try: - fc.read() # should populate fallback configuration -except Exception: - pass -fc.filename=sys.argv[1] # Change target firewalld.conf write target -fc.write() # update firewalld.conf -' "$orig_conf" 2>/dev/null - fi - if [ -s "$listfile" ] && [ -n "$booted" ] ; then - firewall-cmd --reload > /dev/null - fi -fi - -cat "$listfile" diff --git a/library/firewall_lib.py b/library/firewall_lib.py index ec8a4e98..11732d75 100644 --- a/library/firewall_lib.py +++ b/library/firewall_lib.py @@ -260,6 +260,14 @@ type: list elements: str default: [] + previous: + description: + The previous state of the firewall configuration. + The value "replaced" means that the entire firewall configuration will be replaced with the new configuration. + required: false + type: str + choices: ["replaced", "kept"] + default: "kept" includes: description: Services to include in this one. @@ -267,12 +275,6 @@ type: list elements: str default: [] - __report_changed: - description: - If false, do not report changed true even if changed. - required: false - type: bool - default: true online: description: When true, use the D-Bus API to query the status from the running system. @@ -281,17 +283,306 @@ type: bool required: false default: true + __called_from_role: + description: + If true, the module is being called from the role. + type: bool + required: false + default: false + config_list: + description: + List of firewall configurations to apply. + Each item in the list is a dictionary containing any of the module's + parameters (except config_list itself). + This allows applying multiple firewall configurations in a single + module call. Cannot be used together with other module parameters. + type: list + elements: dict + required: false + default: [] + suboptions: + firewalld_conf: + description: + Modify firewalld.conf directives + suboptions: + allow_zone_drifting: + description: + Set AllowZoneDrifting directive if not deprecated + required: false + type: bool + required: false + type: dict + service: + description: + List of service name strings. + The service names needs to be defined in firewalld configuration. + services in firewalld configuration can be defined by setting + this option to a single service name and state to present. + required: false + type: list + elements: str + default: [] + port: + description: + List of ports or port range strings. + The format of a port needs to be port=[-]/. + required: false + type: list + elements: str + default: [] + source_port: + description: + List of source port or port range strings. + The format of a source port needs to be port=[-]/. + required: false + type: list + elements: str + default: [] + forward_port: + description: + List of forward port strings or dicts, + or a single string or dict. + The format of a forward port string needs to be + [-]/;[];[]. + aliases: ["port_forward"] + required: false + type: raw + default: [] + masquerade: + description: + The masquerade bool setting. + type: bool + rich_rule: + description: + List of rich rule strings. + For the format see L(Syntax for firewalld rich language rules, + https://firewalld.org/documentation/man-pages/firewalld.richlanguage.html). + required: false + type: list + elements: str + default: [] + source: + description: + List of source address, address range strings, or ipsets + A source address or address range is either an IP address or a network + IP address with a mask for IPv4 or IPv6. For IPv4, the mask can be a + network mask or a plain number. For IPv6 the mask is a plain number. + An ipset is used by prefixing "ipset{{ ":" }}" to the defined ipset's name. + required: false + type: list + elements: str + default: [] + interface: + description: + List of interface name strings. + required: false + type: list + elements: str + default: [] + interface_pci_id: + description: + List of interface PCI device ID strings. + PCI device ID needs to correspond to a named network interface. + required: false + type: list + elements: str + default: [] + icmp_block: + description: + List of ICMP type strings to block. + The ICMP type names needs to be defined in firewalld configuration. + required: false + type: list + elements: str + default: [] + icmp_block_inversion: + description: + ICMP block inversion bool setting. + It enables or disables inversion of ICMP blocks for a zone in firewalld. + required: false + type: bool + timeout: + description: + The amount of time in seconds a setting is in effect. + The timeout is usable for services, ports, source ports, forward ports, + masquerade, rich rules or icmp blocks for runtime only. + required: false + type: int + default: 0 + target: + description: + The firewalld Zone target. + If the state is set to C(absent), this will reset the target to default. + required: false + choices: ["default", "ACCEPT", "DROP", "%%REJECT%%"] + type: str + zone: + description: + The zone name string. + If the zone name is not given, then the default zone will be used. + required: false + type: str + set_default_zone: + description: Sets the default zone. + required: false + type: str + ipset: + description: + Name of the ipset being configured. + Can be used to define, modify, or remove ipsets. + Must set state to C(present) or C(absent) to use this argument. + Must set permanent to C(true) to use this argument. + required: false + type: str + ipset_type: + description: + Type of ipset being defined + Will only do something when ipset argument is defined. + To get the list of supported ipset types, use + firewall-cmd --get-ipset-types. + required: false + type: str + ipset_entries: + description: + List of addresses to add/remove from ipset. + Must be compatible with the ipset type of the `ipset` + being created or modified. + Will only do something when set with ipset. + required: false + type: list + elements: str + default: [] + ipset_options: + description: + Dict of key/value pairs of ipset options for the given ipset. + Will only do something when set with ipset. + required: false + type: dict + default: {} + permanent: + description: + The permanent bool flag. + Ensures settings permanently across system reboots and firewalld + service restarts. + If the permanent flag is not enabled, runtime is assumed. + required: false + type: bool + runtime: + description: + The runtime bool flag. + Ensures settings in the runtime environment that is not persistent + across system reboots and firewalld service restarts. + aliases: ["immediate"] + required: false + type: bool + state: + description: + Ensure presence or absence of entries. Use C(present) and C(absent) only + for zone-only operations, service-only operations, or target operations. + required: false + type: str + choices: ["enabled", "disabled", "present", "absent"] + description: + description: + Creates or updates the description of a new or existing service or ipset. + State needs to be present for the use of this argument. + Supported for ipsets and services. + required: false + type: str + short: + description: + Creates or updates a short description, generally just a full name of a + new or existing service. + Supported for custom services and ipsets while state is present + required: false + type: str + protocol: + description: + List of protocols supported by managed system. + Supported for service configuration only + required: false + type: list + elements: str + default: [] + helper_module: + description: + List of netfiler kernel helper module names + required: false + type: list + elements: str + default: [] + destination: + description: + List of IPv4/IPv6 addresses with optional mask + format - address[/mask] + Currently only supported for service configuration + Only one IPv4 and one IPv6 address allowed in list. + required: false + type: list + elements: str + default: [] + previous: + description: + The previous state of the firewall configuration. + The value "replaced" means that the entire firewall configuration will be replaced with the new configuration. + required: false + type: str + choices: ["replaced", "kept"] + default: "kept" + includes: + description: + Services to include in this one. + required: false + type: list + elements: str + default: [] """ EXAMPLES = """ -firewall: - - port: ['443/tcp', '443/udp'] +# Single configuration (current method) +- name: Configure firewall ports + firewall_lib: + - port: ['443/tcp', '443/udp'] + +# Multiple configurations using config_list (new method) +- name: Configure firewall ports again with config_list + firewall_lib: + config_list: + - port: ['80/tcp'] + state: enabled + permanent: true + - service: ['ssh'] + state: enabled + permanent: true + - port: ['8080/tcp'] + zone: public + state: enabled + runtime: true + +# Each dict in config_list can contain any parameters that the module supports +- name: Configure firewall with config_list again + firewall_lib: + config_list: + - zone: public + target: ACCEPT + state: present + permanent: true + - service: ['http', 'https'] + zone: public + state: enabled + permanent: true """ from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.firewall_lsr.get_config import ( + config_to_dict, + export_config_dict, + recursive_show_diffs, +) from ansible.module_utils.six import string_types import re import os +import copy try: import ipaddress @@ -300,17 +591,6 @@ except ImportError: HAS_IPADDRESS = False -try: - from firewall.functions import check_mac - - HAS_CHECK_MAC = True -except ImportError: - HAS_CHECK_MAC = False - - def check_mac(mac): - return False - - try: import firewall.config @@ -323,11 +603,37 @@ def check_mac(mac): FirewallClientServiceSettings, FirewallClientIPSetSettings, ) + from firewall.core.io.firewalld_conf import firewalld_conf + from firewall.core.io.zone import Zone + from firewall.core.io.service import Service + from firewall.core.io.ipset import IPSet + + FIREWALLD_DIR = firewall.config.ETC_FIREWALLD HAS_FIREWALLD = True except ImportError: + FIREWALLD_DIR = "/etc/firewalld" HAS_FIREWALLD = False +try: + if HAS_FIREWALLD: + firewall.config.FIREWALLD_POLICIES + + HAS_POLICIES = True +except AttributeError: + HAS_POLICIES = False + +try: + from firewall.functions import check_mac + + HAS_CHECK_MAC = True +except ImportError: + HAS_CHECK_MAC = False + + def check_mac(mac): + return False + + try: from firewall.core.fw_nm import ( nm_is_imported, @@ -432,6 +738,29 @@ def normalize_ipset_options(ipset_options): ipset_options[option] = str(value) +def check_and_normalize_ipset(module, ipset, ipset_entries, ipset_options): + addr_type = get_ipset_entries_type(ipset_entries, module) + if addr_type is None and ipset_entries: + module.fail_json( + msg="ipset %s: Invalid IP address - %s " % (ipset, str(ipset_entries)) + ) + copy_ipset_options = copy.deepcopy(ipset_options) + normalize_ipset_options(copy_ipset_options) + if addr_type == "ipv6" and "family" not in ipset_options: + copy_ipset_options["family"] = "inet6" + if addr_type == "ipv4" and copy_ipset_options.get("family") == "inet6": + module.fail_json( + msg="ipset %s: family=inet6 is not supported for IPv4 ipset_entries %s" + % (ipset, ", ".join(ipset_entries)) + ) + if addr_type == "ipv6" and copy_ipset_options.get("family") == "inet": + module.fail_json( + msg="ipset %s: family=inet is not supported for IPv6 ipset_entries %s" + % (ipset, ", ".join(ipset_entries)) + ) + return copy_ipset_options + + # Above: adapted from firewall-cmd source code class OnlineAPIBackend: """Implement operations with the FirewallClient() API. @@ -439,15 +768,14 @@ class OnlineAPIBackend: This requires firewalld to be running. """ - def __init__( - self, module, permanent, runtime, zone_operation, zone, state, timeout - ): + def __init__(self, module, permanent, runtime, zone, state, timeout): self.module = module self.state = state self.permanent = permanent self.runtime = runtime self.zone = zone self.timeout = timeout + self.set_interface_changed = False self.fw = FirewallClient() @@ -464,15 +792,10 @@ def exception_handler(exception_message): zone_exists = False if runtime: zone_exists = zone_exists or zone is None or zone in self.fw.getZones() - err_str = "Runtime" if permanent: zone_exists = ( zone_exists or zone is None or zone in self.fw.config().getZoneNames() ) - err_str = "Permanent" - - if not zone_exists and not zone_operation: - module.fail_json(msg="%s zone '%s' does not exist." % (err_str, zone)) if zone_exists: self.zone = self.zone or self.fw.getDefaultZone() @@ -485,6 +808,9 @@ def exception_handler(exception_message): self.zone_exists = zone_exists + def check_zone_exists(self): + return self.zone_exists + def finalize(self): if self.fw_zone and self.fw_settings: self.fw_zone.update(self.fw_settings) @@ -584,9 +910,14 @@ def set_service( fw_service_settings.addSourcePort(_port, _protocol) self.changed = True for _module in helper_module: - if not fw_service_settings.queryHelper(_module): + if hasattr(fw_service_settings, "queryHelper"): + if not fw_service_settings.queryHelper(_module): + if not self.module.check_mode: + fw_service_settings.addHelper(_module) + self.changed = True + elif not fw_service_settings.queryModule(_module): if not self.module.check_mode: - fw_service_settings.addHelper(_module) + fw_service_settings.addModule(_module) self.changed = True if destination_ipv4: if not fw_service_settings.queryDestination( @@ -608,30 +939,31 @@ def set_service( fw_service_settings.addInclude(_include) self.changed = True if self.state == "absent" and service_exists: - if port: - for _port, _protocol in port: - if fw_service_settings.queryPort(_port, _protocol): - if not self.module.check_mode: - fw_service_settings.removePort(_port, _protocol) - self.changed = True - if source_port: - for _port, _protocol in source_port: - if fw_service_settings.querySourcePort(_port, _protocol): - if not self.module.check_mode: - fw_service_settings.removeSourcePort(_port, _protocol) - self.changed = True - if protocol: - for _protocol in protocol: - if fw_service_settings.queryProtocol(_protocol): - if not self.module.check_mode: - fw_service_settings.removeProtocol(_protocol) - self.changed = True - if helper_module: - for _module in helper_module: - if fw_service_settings.queryModule(_module): + for _port, _protocol in port: + if fw_service_settings.queryPort(_port, _protocol): + if not self.module.check_mode: + fw_service_settings.removePort(_port, _protocol) + self.changed = True + for _port, _protocol in source_port: + if fw_service_settings.querySourcePort(_port, _protocol): + if not self.module.check_mode: + fw_service_settings.removeSourcePort(_port, _protocol) + self.changed = True + for _protocol in protocol: + if fw_service_settings.queryProtocol(_protocol): + if not self.module.check_mode: + fw_service_settings.removeProtocol(_protocol) + self.changed = True + for _module in helper_module: + if hasattr(fw_service_settings, "queryHelper"): + if fw_service_settings.queryHelper(_module): if not self.module.check_mode: - fw_service_settings.removeModule(_module) + fw_service_settings.removeHelper(_module) self.changed = True + elif fw_service_settings.queryModule(_module): + if not self.module.check_mode: + fw_service_settings.removeModule(_module) + self.changed = True if destination_ipv4: if fw_service_settings.queryDestination("ipv4", destination_ipv4): if not self.module.check_mode: @@ -717,24 +1049,9 @@ def _create_ipset(self, ipset, ipset_type): def set_ipset( self, ipset, description, short, ipset_type, ipset_entries, ipset_options ): - addr_type = get_ipset_entries_type(ipset_entries, self.module) - if addr_type is None and ipset_entries: - self.module.fail_json( - msg="ipset %s: Invalid IP address - %s " % (ipset, str(ipset_entries)) - ) - normalize_ipset_options(ipset_options) - if addr_type == "ipv6" and "family" not in ipset_options: - ipset_options["family"] = "inet6" - if addr_type == "ipv4" and ipset_options.get("family") == "inet6": - self.module.fail_json( - msg="ipset %s: family=inet6 is not supported for IPv4 ipset_entries %s" - % (ipset, ", ".join(ipset_entries)) - ) - if addr_type == "ipv6" and ipset_options.get("family") == "inet": - self.module.fail_json( - msg="ipset %s: family=inet is not supported for IPv6 ipset_entries %s" - % (ipset, ", ".join(ipset_entries)) - ) + ipset_options = check_and_normalize_ipset( + self.module, ipset, ipset_entries, ipset_options + ) ipset_exists = ipset in self.fw.config().getIPSetNames() fw_ipset = None fw_ipset_settings = None @@ -1008,6 +1325,7 @@ def set_interface(self, interface): if not self.module.check_mode: self.fw.changeZoneOfInterface(self.zone, item) self.changed = True + self.set_interface_changed = True if self.permanent: nm_used, if_changed = try_set_zone_of_interface( self.module, self.zone, item @@ -1015,6 +1333,7 @@ def set_interface(self, interface): if nm_used: if if_changed: self.changed = True + self.set_interface_changed = True elif not self.fw_settings.queryInterface(item): if not self.module.check_mode: old_zone_name = self.fw.config().getZoneOfInterface(item) @@ -1028,11 +1347,13 @@ def set_interface(self, interface): old_zone_obj.update(old_zone_settings) self.fw_settings.addInterface(item) self.changed = True + self.set_interface_changed = True elif self.state == "disabled": if self.runtime and self.fw.queryInterface(self.zone, item): if not self.module.check_mode: self.fw.removeInterface(self.zone, item) self.changed = True + self.set_interface_changed = True if self.permanent: nm_used, if_changed = try_set_zone_of_interface( self.module, "", item @@ -1040,10 +1361,12 @@ def set_interface(self, interface): if nm_used: if if_changed: self.changed = True + self.set_interface_changed = True elif self.fw_settings.queryInterface(item): if not self.module.check_mode: self.fw_settings.removeInterface(item) self.changed = True + self.set_interface_changed = True def set_icmp_block(self, icmp_block): for item in icmp_block: @@ -1102,18 +1425,647 @@ def set_target(self, target): self.changed = True +class InMemoryBackend: + """Implement operations using in-memory configuration. + + This backend reads the existing configuration into memory and applies + changes to the in-memory representation. It can return both the original + and modified configurations for comparison or deferred application. + """ + + def __init__( + self, + module, + online, + start_empty=False, + ): + self.module = module + self.online = online + self.state = None + self.permanent = None + self.runtime = None + self.zone = None + self.timeout = None + self.changed = False + self.set_interface_changed = False + + # Load the current configuration + self.original_config = config_to_dict(module, detailed=True, online=online) + self.firewalld_conf = copy.deepcopy( + self.original_config.get("firewalld_conf", {}) + ) + + self.working_config_runtime = {} + if start_empty: + # start with the built-in default settings + self.default_zone = self.original_config.get( + "fallback_default_zone", "public" + ) + self.working_config_permanent = copy.deepcopy( + self.original_config["default"] + ) + if self.online: + self.working_config_runtime = copy.deepcopy( + self.original_config["default"] + ) + self._move_interfaces_to_default_zone( + self.original_config["custom_runtime_with_defaults"] + ) + else: + # start with the current permanent and runtime settings + self.default_zone = self.original_config.get("default_zone", "public") + self.working_config_permanent = copy.deepcopy( + self.original_config["custom_permanent_with_defaults"] + ) + if self.online: + self.working_config_runtime = copy.deepcopy( + self.original_config["custom_runtime_with_defaults"] + ) + self.original_default_zone = self.default_zone + + def check_zone_exists(self): + self.zone_exists = False + # Check in permanent config first + if self.permanent and self.zone in self.working_config_permanent.get( + "zones", {} + ): + self.zone_exists = True + if ( + self.online + and not self.zone_exists + and self.runtime + and self.zone in self.working_config_runtime.get("zones", {}) + ): + self.zone_exists = True + return self.zone_exists + + def _ensure_zone_in_working_config(self, create_if_not_exists): + """Ensure the zone exists in working config with all necessary keys.""" + + for flag, working_config in [ + (self.permanent, self.working_config_permanent), + (self.online and self.runtime, self.working_config_runtime), + ]: + if flag: + if self.zone not in working_config["zones"] and create_if_not_exists: + # Create new zone with empty settings + working_config["zones"][self.zone] = {} + else: + raise ValueError("Zone '%s' does not exist" % self.zone) + + def _get_zone_config(self, config_type="permanent"): + """Get the configuration for the current zone. + + Args: + config_type: Either "permanent" or "runtime" + """ + if config_type == "permanent": + config = self.working_config_permanent + elif self.online: + config = self.working_config_runtime + else: + return None + return config["zones"].get(self.zone, None) + + def finalize(self): + """No-op for in-memory backend.""" + pass + + def get_configs(self): + """Return both original and working configurations. + + Returns: + tuple: (original_config, working_config_permanent, working_config_runtime, default_zone, firewalld_conf) + + """ + return ( + self.original_config, + self.working_config_permanent, + self.working_config_runtime, + self.default_zone, + self.firewalld_conf, + ) + + def set_firewalld_conf(self, firewalld_conf, allow_zone_drifting_deprecated): + """Set firewalld.conf options.""" + # Store firewalld_conf settings in permanent config only + # (firewalld.conf is a permanent configuration file) + if ( + not allow_zone_drifting_deprecated + and "allow_zone_drifting" in firewalld_conf + and firewalld_conf.get("allow_zone_drifting") + != self.firewalld_conf.get("allow_zone_drifting") + ): + self.firewalld_conf["allow_zone_drifting"] = firewalld_conf.get( + "allow_zone_drifting" + ) + self.changed = True + + def _new_zone(self): + if HAS_FIREWALLD: + return export_config_dict(Zone()) + else: + return {} + + def set_zone(self): + """Create or remove a zone.""" + # A zone must be present or absent in both permanent and runtime configurations + if ( + self.state == "present" + and self.zone not in self.working_config_permanent["zones"] + ): + self.working_config_permanent["zones"][self.zone] = self._new_zone() + if self.online: + self.working_config_runtime["zones"][self.zone] = self._new_zone() + self.changed = True + elif ( + self.state == "absent" + and self.zone in self.working_config_permanent["zones"] + ): + del self.working_config_permanent["zones"][self.zone] + if self.online: + del self.working_config_runtime["zones"][self.zone] + self.changed = True + # removing a zone online requires a reload - if you remove the default zone, it + # is set back to the original + if self.default_zone == self.zone: + self.default_zone = self.original_default_zone + self.zone = self.default_zone + + def _move_interfaces_to_default_zone(self, working_config_runtime=None): + if self.online: + if not working_config_runtime: + src = self.working_config_runtime + dest = self.working_config_runtime + else: + src = working_config_runtime + dest = self.working_config_runtime + for zone_name, zone_config in src.get("zones", {}).items(): + if ( + src is not dest or zone_name != self.default_zone + ) and "interfaces" in zone_config: + for interface in zone_config["interfaces"]: + if interface not in dest["zones"][self.default_zone].get( + "interfaces", [] + ): + dest["zones"][self.default_zone].setdefault( + "interfaces", [] + ).append(interface) + if src is dest: + del zone_config["interfaces"] + + def set_default_zone(self, zone): + """Set the default zone.""" + # Default zone applies to both permanent and runtime configurations + if self.default_zone != zone: + self.default_zone = zone + self.changed = True + # move all of the runtime interfaces to the new default zone + self._move_interfaces_to_default_zone() + + def get_default_zone(self): + """Get the default zone.""" + return self.default_zone + + def _new_service(self): + if HAS_FIREWALLD: + return export_config_dict(Service()) + else: + return {} + + def set_service( + self, + service_operation, + service, + description, + short, + port, + protocol, + source_port, + helper_module, + destination_ipv4, + destination_ipv6, + includes, + ): + """Configure services.""" + if service_operation and self.permanent: + working_configs = [self.working_config_permanent] + if self.online: + working_configs.append(self.working_config_runtime) + for working_config in working_configs: + if self.state == "present": + if "services" not in working_config: + working_config["services"] = {} + + if service not in working_config["services"]: + working_config["services"][service] = self._new_service() + self.changed = True + + svc = working_config["services"][service] + + if ( + description is not None + and svc.get("description") != description + ): + svc["description"] = description + self.changed = True + + if short is not None and svc.get("short") != short: + svc["short"] = short + self.changed = True + + for port_tuple in port: + if port_tuple not in svc.get("ports", []): + svc.setdefault("ports", []).append(port_tuple) + self.changed = True + + for _protocol in protocol: + if _protocol not in svc.get("protocols", []): + svc.setdefault("protocols", []).append(_protocol) + self.changed = True + + for port_tuple in source_port: + if port_tuple not in svc.get("source_ports", []): + svc.setdefault("source_ports", []).append(port_tuple) + self.changed = True + + for _module in helper_module: + if _module not in svc.get("helpers", []): + svc.setdefault("helpers", []).append(_module) + self.changed = True + + if destination_ipv4: + if svc.get("destination", {}).get("ipv4") != destination_ipv4: + svc.setdefault("destination", {})["ipv4"] = destination_ipv4 + self.changed = True + + if destination_ipv6: + if svc.get("destination", {}).get("ipv6") != destination_ipv6: + svc.setdefault("destination", {})["ipv6"] = destination_ipv6 + self.changed = True + + for _include in includes: + if _include not in svc.get("includes", []): + svc.setdefault("includes", []).append(_include) + self.changed = True + + elif self.state == "absent" and service in working_config.get( + "services", {} + ): + if any( + ( + port, + source_port, + protocol, + helper_module, + destination_ipv4, + destination_ipv6, + includes, + ) + ): + # Remove specific items + svc = working_config["services"][service] + for port_tuple in port: + if port_tuple in svc.get("ports", []): + svc["ports"].remove(port_tuple) + self.changed = True + if "ports" in svc and not svc["ports"]: + del svc["ports"] + + for _protocol in protocol: + if _protocol in svc.get("protocols", []): + svc["protocols"].remove(_protocol) + self.changed = True + if "protocols" in svc and not svc["protocols"]: + del svc["protocols"] + + for port_tuple in source_port: + if port_tuple in svc.get("source_ports", []): + svc["source_ports"].remove(port_tuple) + self.changed = True + if "source_ports" in svc and not svc["source_ports"]: + del svc["source_ports"] + + for _module in helper_module: + if _module in svc.get("helpers", []): + svc["helpers"].remove(_module) + self.changed = True + if "helpers" in svc and not svc["helpers"]: + del svc["helpers"] + + if ( + destination_ipv4 + and svc.get("destination", {}).get("ipv4") + == destination_ipv4 + ): + del svc["destination"]["ipv4"] + self.changed = True + + if ( + destination_ipv6 + and svc.get("destination", {}).get("ipv6") + == destination_ipv6 + ): + del svc["destination"]["ipv6"] + self.changed = True + + if "destination" in svc and not svc["destination"]: + del svc["destination"] + + for _include in includes: + if _include in svc.get("includes", []): + svc["includes"].remove(_include) + self.changed = True + if "includes" in svc and not svc["includes"]: + del svc["includes"] + else: + # Remove entire service + del working_config["services"][service] + self.changed = True + else: + # Zone service operation - applies to both permanent and runtime + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + for item in service: + service_exists = item in self.working_config_permanent.get( + "services", {} + ) or ( + self.online + and item in self.working_config_runtime.get("services", {}) + ) + if service_exists and self.state == "enabled": + if item not in zone_config.get("services", []): + zone_config.setdefault("services", []).append(item) + self.changed = True + elif service_exists and self.state == "disabled": + if item in zone_config.get("services", []): + zone_config["services"].remove(item) + self.changed = True + else: + if self.module.check_mode: + self.module.warn( + "Service does not exist - " + + item + + ". Ensure that you define the service in the playbook before running it in diff mode" + ) + else: + self.module.fail_json(msg="INVALID SERVICE - " + item) + + def _new_ipset(self, ipset_type): + new_ipset = {} + if HAS_FIREWALLD: + new_ipset = export_config_dict(IPSet()) + new_ipset["type"] = ipset_type + return new_ipset + + def set_ipset( + self, ipset, description, short, ipset_type, ipset_entries, ipset_options + ): + """Configure ipsets (permanent only).""" + ipset_options = check_and_normalize_ipset( + self.module, ipset, ipset_entries, ipset_options + ) + existing_ipset = self.working_config_permanent.get("ipsets", {}).get( + ipset, + {}, + ) + + if existing_ipset: + if ipset_type and ipset_type != existing_ipset.get("type"): + self.module.fail_json( + msg="Name conflict when creating ipset - " + "ipset %s of type %s already exists" + % (ipset, existing_ipset.get("type")) + ) + + # set ipset in permanent, and runtime if set + for config_type, working_config in ( + (self.permanent, self.working_config_permanent), + (self.online and self.runtime, self.working_config_runtime), + ): + if config_type and working_config is not None: + if self.state == "present": + if not existing_ipset: + if "ipsets" not in working_config: + working_config["ipsets"] = {} + working_config["ipsets"][ipset] = self._new_ipset(ipset_type) + self.changed = True + + ipset_cfg = working_config["ipsets"][ipset] + + if ( + description is not None + and ipset_cfg.get("description") != description + ): + ipset_cfg["description"] = description + self.changed = True + + if short is not None and ipset_cfg.get("short") != short: + ipset_cfg["short"] = short + self.changed = True + + for entry in ipset_entries: + if entry not in ipset_cfg.get("entries", []): + ipset_cfg.setdefault("entries", []).append(entry) + self.changed = True + + for option, value in ipset_options.items(): + if ipset_cfg.get("options", {}).get(option) != value: + ipset_cfg.setdefault("options", {})[option] = value + self.changed = True + + elif self.state == "absent" and existing_ipset: + if ipset_entries or ipset_options: + # Remove specific entries/options + ipset_cfg = working_config["ipsets"][ipset] + for entry in ipset_entries: + if entry in ipset_cfg.get("entries", []): + ipset_cfg["entries"].remove(entry) + self.changed = True + else: + # Remove entire ipset + del working_config["ipsets"][ipset] + # if no more ipsets, remove the top level + if not working_config["ipsets"]: + del working_config["ipsets"] + self.changed = True + + def _set_ports_or_source_ports(self, port, port_type): + """Configure ports or source_ports in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + for _port, _protocol in port: + # keep as tuple here + port_spec = (_port, _protocol) + if self.state == "enabled": + if port_spec not in zone_config.get(port_type, []): + zone_config.setdefault(port_type, []).append(port_spec) + self.changed = True + elif self.state == "disabled": + if port_spec in zone_config.get(port_type, []): + zone_config[port_type].remove(port_spec) + self.changed = True + + def set_port(self, port): + """Configure ports in a zone.""" + self._set_ports_or_source_ports(port, "ports") + + def set_source_port(self, source_port): + """Configure source ports in a zone.""" + self._set_ports_or_source_ports(source_port, "source_ports") + + def set_forward_port(self, forward_port): + """Configure port forwarding in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + for forward_port_item in forward_port: + if self.state == "enabled": + if forward_port_item not in zone_config.get( + "forward_ports", [] + ): + zone_config.setdefault("forward_ports", []).append( + forward_port_item + ) + self.changed = True + elif self.state == "disabled": + if forward_port_item in zone_config.get("forward_ports", []): + zone_config["forward_ports"].remove(forward_port_item) + self.changed = True + + def set_masquerade(self, masquerade): + """Configure masquerading in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + if zone_config.get("masquerade", False) != masquerade: + zone_config["masquerade"] = masquerade + self.changed = True + + def set_rich_rule(self, rich_rule): + """Configure rich rules in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + for item in rich_rule: + if self.state == "enabled": + if item not in zone_config.get("rich_rules", []): + zone_config.setdefault("rich_rules", []).append(item) + self.changed = True + elif self.state == "disabled": + if item in zone_config.get("rich_rules", []): + zone_config["rich_rules"].remove(item) + self.changed = True + + def set_source(self, source): + """Configure sources in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + for item in source: + if self.state == "enabled": + if item not in zone_config.get("sources", []): + zone_config.setdefault("sources", []).append(item) + self.changed = True + elif self.state == "disabled": + if item in zone_config.get("sources", []): + zone_config["sources"].remove(item) + self.changed = True + + def set_interface(self, interface): + """Configure interfaces in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + # we have no way to know if this will actually change the firewall configuration, + # so set this flag here to let the real backend handle it even if there are + # no other changes. + self.set_interface_changed = True + for item in interface: + if self.state == "enabled": + if item not in zone_config.get("interfaces", []): + zone_config.setdefault("interfaces", []).append(item) + self.changed = True + elif self.state == "disabled": + if item in zone_config.get("interfaces", []): + zone_config["interfaces"].remove(item) + self.changed = True + + def set_icmp_block(self, icmp_block): + """Configure ICMP blocks in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + for item in icmp_block: + if self.state == "enabled": + if item not in zone_config.get("icmp_blocks", []): + zone_config.setdefault("icmp_blocks", []).append(item) + self.changed = True + elif self.state == "disabled": + if item in zone_config.get("icmp_blocks", []): + zone_config["icmp_blocks"].remove(item) + self.changed = True + + def set_icmp_block_inversion(self, icmp_block_inversion): + """Configure ICMP block inversion in a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + if ( + zone_config.get("icmp_block_inversion", False) + != icmp_block_inversion + ): + zone_config["icmp_block_inversion"] = icmp_block_inversion + self.changed = True + + def set_target(self, target): + """Configure the target for a zone.""" + for config_type, zone_config in ( + (self.permanent, self._get_zone_config("permanent")), + (self.online and self.runtime, self._get_zone_config("runtime")), + ): + if config_type and zone_config is not None: + if self.state in ["enabled", "present"]: + if zone_config.get("target", "default") != target: + zone_config["target"] = target + self.changed = True + elif self.state in ["absent", "disabled"]: + if zone_config.get("target", "default") != "default": + zone_config["target"] = "default" + self.changed = True + + class OfflineCLIBackend: """Implement operations with firewall-offline-cmd. This works during container builds and similar environments. """ - def __init__( - self, module, permanent, runtime, zone_operation, zone, state, timeout - ): + def __init__(self, module, permanent, runtime, zone, state, timeout): self.module = module self.state = state self.timeout = timeout + self.set_interface_changed = False self.changed = False @@ -1131,8 +2083,8 @@ def __init__( zones = self.cmd("--get-zones").split() self.zone_exists = zone in zones - if not self.zone_exists and not zone_operation: - module.fail_json(msg="Zone '%s' does not exist." % zone) + def check_zone_exists(self): + return self.zone_exists def _call_offline_cmd(self, args, check_rc=True): argv = ["firewall-offline-cmd"] + list(args) @@ -1584,6 +2536,7 @@ def set_interface(self, interface): # note: --change-interface first removes it from the old zone "--%s-interface=%s" % ("change" if enable else "remove", item), ) + self.set_interface_changed = True def set_icmp_block(self, icmp_block): enable = self.check_state(["enabled", "disabled"], "icmp_block") @@ -1665,6 +2618,10 @@ def parse_pci_id(module, item): module.fail_json(msg="interface_pci_id is not supported in offline mode.") if PCI_REGEX.search(item): + if not NM_IMPORTED: + module.fail_json( + msg="interface_pci_id is only supported with NetworkManager. If you want to use this feature, please install NetworkManager." + ) global pci_ids if not pci_ids: pci_ids = get_interface_pci() @@ -1673,7 +2630,7 @@ def parse_pci_id(module, item): if interface_name: return interface_name - module.warn(msg="No network interfaces found with PCI device ID %s" % item) + module.warn("No network interfaces found with PCI device ID %s" % item) else: module.fail_json( msg="PCI id %s does not match format: XXXX:XXXX (X = hexadecimal number)" @@ -1838,7 +2795,7 @@ def parse_forward_port(module, item): def check_allow_zone_drifting(firewalld_conf): - if firewalld_conf["allow_zone_drifting"] is not None: + if isinstance(firewalld_conf["allow_zone_drifting"], bool): if firewalld_conf["allow_zone_drifting"]: firewalld_conf["allow_zone_drifting"] = "yes" else: @@ -1851,95 +2808,122 @@ def check_firewalld_conf(firewalld_conf): check_allow_zone_drifting(firewalld_conf) -def main(): - module = AnsibleModule( - argument_spec=dict( - firewalld_conf=dict( - required=False, - type="dict", - options=dict( - allow_zone_drifting=dict(required=False, type="bool", default=None), - ), - default=None, - ), - service=dict(required=False, type="list", elements="str", default=[]), - port=dict(required=False, type="list", elements="str", default=[]), - source_port=dict(required=False, type="list", elements="str", default=[]), - forward_port=dict( - required=False, - type="raw", - default=[], - aliases=["port_forward"], - deprecated_aliases=[ - { - "name": "port_forward", - "date": "2021-09-23", - "collection_name": "ansible.posix", - }, - ], - ), - masquerade=dict(required=False, type="bool", default=None), - rich_rule=dict(required=False, type="list", elements="str", default=[]), - source=dict(required=False, type="list", elements="str", default=[]), - interface=dict(required=False, type="list", elements="str", default=[]), - interface_pci_id=dict( - required=False, type="list", elements="str", default=[] +def get_base_argument_spec(): + """Return the base argument spec for firewall configuration parameters.""" + return dict( + firewalld_conf=dict( + required=False, + type="dict", + options=dict( + allow_zone_drifting=dict(required=False, type="bool", default=None), ), - icmp_block=dict(required=False, type="list", elements="str", default=[]), - icmp_block_inversion=dict(required=False, type="bool", default=None), - timeout=dict(required=False, type="int", default=0), - target=dict( - required=False, - type="str", - choices=["default", "ACCEPT", "DROP", "%%REJECT%%"], - default=None, - ), - zone=dict(required=False, type="str", default=None), - set_default_zone=dict(required=False, type="str", default=None), - ipset=dict(required=False, type="str", default=None), - ipset_type=dict(required=False, type="str", default=None), - ipset_entries=dict(required=False, type="list", elements="str", default=[]), - ipset_options=dict(required=False, type="dict", default={}), - permanent=dict(required=False, type="bool", default=None), - runtime=dict( - required=False, - type="bool", - default=None, - aliases=["immediate"], - deprecated_aliases=[ - { - "name": "immediate", - "date": "2021-09-23", - "collection_name": "ansible.posix", - }, - ], - ), - state=dict( - choices=["enabled", "disabled", "present", "absent"], - required=False, - default=None, - ), - description=dict(required=False, type="str", default=None), - short=dict(required=False, type="str", default=None), - protocol=dict(required=False, type="list", elements="str", default=[]), - helper_module=dict(required=False, type="list", elements="str", default=[]), - destination=dict(required=False, type="list", elements="str", default=[]), - includes=dict(required=False, type="list", elements="str", default=[]), - __report_changed=dict(required=False, type="bool", default=True), - online=dict(required=False, type="bool", default=True), + default=None, ), - supports_check_mode=True, - required_if=( - ("state", "present", ("zone", "target", "service"), True), - ("state", "absent", ("zone", "target", "service"), True), + service=dict(required=False, type="list", elements="str", default=[]), + port=dict(required=False, type="list", elements="str", default=[]), + source_port=dict(required=False, type="list", elements="str", default=[]), + forward_port=dict( + required=False, + type="raw", + default=[], + aliases=["port_forward"], + deprecated_aliases=[ + { + "name": "port_forward", + "date": "2021-09-23", + "collection_name": "ansible.posix", + }, + ], ), + masquerade=dict(required=False, type="bool", default=None), + rich_rule=dict(required=False, type="list", elements="str", default=[]), + source=dict(required=False, type="list", elements="str", default=[]), + interface=dict(required=False, type="list", elements="str", default=[]), + interface_pci_id=dict(required=False, type="list", elements="str", default=[]), + icmp_block=dict(required=False, type="list", elements="str", default=[]), + icmp_block_inversion=dict(required=False, type="bool", default=None), + timeout=dict(required=False, type="int", default=0), + target=dict( + required=False, + type="str", + choices=["default", "ACCEPT", "DROP", "%%REJECT%%"], + default=None, + ), + zone=dict(required=False, type="str", default=None), + set_default_zone=dict(required=False, type="str", default=None), + ipset=dict(required=False, type="str", default=None), + ipset_type=dict(required=False, type="str", default=None), + ipset_entries=dict(required=False, type="list", elements="str", default=[]), + ipset_options=dict(required=False, type="dict", default={}), + permanent=dict(required=False, type="bool", default=None), + runtime=dict( + required=False, + type="bool", + default=None, + aliases=["immediate"], + deprecated_aliases=[ + { + "name": "immediate", + "date": "2021-09-23", + "collection_name": "ansible.posix", + }, + ], + ), + state=dict( + choices=["enabled", "disabled", "present", "absent"], + required=False, + default=None, + ), + description=dict(required=False, type="str", default=None), + short=dict(required=False, type="str", default=None), + protocol=dict(required=False, type="list", elements="str", default=[]), + helper_module=dict(required=False, type="list", elements="str", default=[]), + destination=dict(required=False, type="list", elements="str", default=[]), + includes=dict(required=False, type="list", elements="str", default=[]), + previous=dict(required=False, choices=["replaced", "kept"], default="kept"), ) - if not HAS_FIREWALLD: - module.fail_json(msg="No firewall backend could be imported.") + +def get_full_argument_spec(): + full_spec = get_base_argument_spec() + full_spec.update( + dict( + online=dict(required=False, type="bool", default=True), + __called_from_role=dict(required=False, type="bool", default=False), + ) + ) + return full_spec + + +def process_single_config( + module, + config_params=None, + backend=None, + online_param=None, + __called_from_role_param=None, +): + """ + Process a single configuration, either from module.params or from a config dict. + + Args: + module: The Ansible module object + config_params: Optional config dict to use instead of module.params + backend: Optional backend object to use instead of creating a new one + + Returns a tuple of (backend, changed) or None if no action needed. + """ + # Use config_params if provided, otherwise use module.params + if config_params is None: + params = module.params + else: + # Merge config_params with defaults from base argument spec + params = {} + base_spec = get_base_argument_spec() + for key, spec in base_spec.items(): + params[key] = config_params.get(key, spec.get("default")) # Argument parse - firewalld_conf = module.params["firewalld_conf"] + firewalld_conf = params["firewalld_conf"] if firewalld_conf: check_firewalld_conf(firewalld_conf) allow_zone_drifting_deprecated = lsr_parse_version( @@ -1952,34 +2936,43 @@ def main(): else: # CodeQL will produce an error without this line allow_zone_drifting_deprecated = None - service = module.params["service"] - short = module.params["short"] - description = module.params["description"] - protocol = module.params["protocol"] + service = params["service"] + short = params["short"] + description = params["description"] + protocol = params["protocol"] helper_module = [] - for _module in module.params["helper_module"]: + for _module in params["helper_module"]: helper_module.append(parse_helper_module(module, _module)) port = [] - for port_proto in module.params["port"]: + for port_proto in params["port"]: port.append(parse_port(module, port_proto)) source_port = [] - for port_proto in module.params["source_port"]: + for port_proto in params["source_port"]: source_port.append(parse_port(module, port_proto)) forward_port = [] - for item in get_forward_port(module): + # Simulate get_forward_port for config_params + if config_params is None: + forward_port_items = get_forward_port(module) + else: + forward_port_raw = params["forward_port"] + if isinstance(forward_port_raw, list): + forward_port_items = forward_port_raw + else: + forward_port_items = [forward_port_raw] if forward_port_raw else [] + for item in forward_port_items: forward_port.append(parse_forward_port(module, item)) - masquerade = module.params["masquerade"] + masquerade = params["masquerade"] rich_rule = [] - for item in module.params["rich_rule"]: + for item in params["rich_rule"]: try: rule = str(Rich_Rule(rule_str=item)) rich_rule.append(rule) except Exception as e: module.fail_json(msg="Rich Rule '%s' is not valid: %s" % (item, str(e))) - source = module.params["source"] + source = params["source"] destination_ipv4 = None destination_ipv6 = None - for address in module.params["destination"]: + for address in params["destination"]: ip_type = parse_destination_address(module, address) if ip_type == "ipv4" and not destination_ipv4: destination_ipv4 = address @@ -1989,26 +2982,33 @@ def main(): destination_ipv6 = address elif destination_ipv6 and ip_type == "ipv6": module.fail_json(msg="cannot have more than one destination ipv6") - interface = module.params["interface"] - for _interface in module.params["interface_pci_id"]: + interface = params["interface"] + for _interface in params["interface_pci_id"]: for interface_name in parse_pci_id(module, _interface): if interface_name not in interface: interface.append(interface_name) - icmp_block = module.params["icmp_block"] - icmp_block_inversion = module.params["icmp_block_inversion"] - timeout = module.params["timeout"] - target = module.params["target"] - zone = module.params["zone"] - set_default_zone = module.params["set_default_zone"] - ipset = module.params["ipset"] - ipset_type = module.params["ipset_type"] - ipset_entries = module.params["ipset_entries"] - ipset_options = module.params["ipset_options"] - permanent = module.params["permanent"] - runtime = module.params["runtime"] - state = module.params["state"] - includes = module.params["includes"] - online = module.params["online"] + icmp_block = params["icmp_block"] + icmp_block_inversion = params["icmp_block_inversion"] + timeout = params["timeout"] + target = params["target"] + zone = params["zone"] + set_default_zone = params["set_default_zone"] + ipset = params["ipset"] + ipset_type = params["ipset_type"] + ipset_entries = params["ipset_entries"] + ipset_options = params["ipset_options"] + permanent = params["permanent"] + runtime = params["runtime"] + state = params["state"] + includes = params["includes"] + if online_param is None: + online = params["online"] + else: + online = online_param + if __called_from_role_param is None: + __called_from_role = params["__called_from_role"] + else: + __called_from_role = __called_from_role_param # All options that require state to be set state_required = any( @@ -2024,9 +3024,19 @@ def main(): rich_rule, ) ) - if permanent is None: - runtime = True - elif not any((permanent, runtime)): + # NOTE: The old implementation of this role would always set permanent to True if it was not set, + # and would set runtime to True if it was not set when called from the role. + # so replicate that behavior here in order to maintain backwards compatibility in the module + # in case someone is erroneously using the module directly + if __called_from_role: + if permanent is None: + permanent = True + if runtime is None: + runtime = online + else: + if permanent is None: + runtime = True + if not any((permanent, runtime)): module.fail_json(msg="One of permanent, runtime needs to be enabled") if ( @@ -2050,6 +3060,9 @@ def main(): ) ) ): + # Skip this config if no actionable parameters are set + if config_params is not None: + return None module.fail_json( msg="One of service, port, source_port, forward_port, " "masquerade, rich_rule, source, interface, icmp_block, " @@ -2227,10 +3240,21 @@ def main(): msg="Unsupported firewalld version %s, requires >= 0.2.11" % FW_VERSION ) - backendClass = OnlineAPIBackend if online else OfflineCLIBackend - backend = backendClass( - module, permanent, runtime, zone_operation, zone, state, timeout - ) + # Use provided backend or create a new one + if backend is None: + backendClass = OnlineAPIBackend if online else OfflineCLIBackend + backend = backendClass(module, permanent, runtime, zone, state, timeout) + else: + # Update backend state for this config + backend.state = state + backend.permanent = permanent + backend.runtime = runtime + backend.zone = zone or backend.get_default_zone() + backend.timeout = timeout + + # error out if the zone does not exist and this is not a zone operation + if not zone_operation and not backend.check_zone_exists(): + module.fail_json(msg="Zone '%s' does not exist." % backend.zone) # Firewall modification starts here @@ -2280,9 +3304,407 @@ def main(): backend.set_target(target) backend.finalize() + return backend.changed + + +def configs_are_equivalent(config1, config2): + """ + Compare two firewall configurations for equivalence. + Returns True if they represent the same firewall state. + """ + + # Create normalized copies + def normalize_config(cfg): + """Normalize a config dict for comparison.""" + normalized = copy.deepcopy(cfg) + + # Normalize custom section + custom = normalized.get("custom", {}) + + # Sort all list values for consistent comparison + for zone_name, zone_cfg in custom.get("zones", {}).items(): + for key in [ + "services", + "ports", + "source_ports", + "sources", + "interfaces", + "icmp_blocks", + "rich_rules", + "protocols", + ]: + if key in zone_cfg: + zone_cfg[key] = sorted(zone_cfg[key]) + + return normalized + + norm1 = normalize_config(config1) + norm2 = normalize_config(config2) + + # Compare custom sections (the parts users can modify) + return norm1.get("custom") == norm2.get("custom") + + +def get_diffs( + original_permanent, + original_runtime, + original_default_zone, + original_firewalld_conf, + current_permanent, + current_runtime, + current_default_zone, + current_firewalld_conf, + ignore_interface, +): + diff = {} + diff["permanent_diff"] = recursive_show_diffs( + original_permanent, current_permanent, None, ignore_interface=ignore_interface + ) + if original_runtime and current_runtime: + diff["runtime_diff"] = recursive_show_diffs( + original_runtime, current_runtime, None, ignore_interface=ignore_interface + ) + else: + diff["runtime_diff"] = {} + if original_default_zone != current_default_zone: + diff["before_default_zone"] = original_default_zone + diff["after_default_zone"] = current_default_zone + if ( + original_firewalld_conf + and current_firewalld_conf + and original_firewalld_conf.get("allow_zone_drifting") + != current_firewalld_conf.get("allow_zone_drifting") + ): + diff["before_allow_zone_drifting"] = original_firewalld_conf.get( + "allow_zone_drifting" + ) + diff["after_allow_zone_drifting"] = current_firewalld_conf.get( + "allow_zone_drifting" + ) + return diff + + +def check_for_diffs( + module, + config_list, + replaced, + online_param=None, + __called_from_role_param=None, +): + """ + Process config_list with in-memory backend to check for differences. + + This creates a firewall config from scratch and compares it with the + current config. + + The diff is a dict suitable to display in diff mode. + + custom_file_list is a list of xml files to be removed when + replaced is true. + + Returns: (changed, diff, custom_file_list) + """ + # Build the desired config using InMemoryBackend with empty starting config + # We need to process all configs to build the complete desired state + online = online_param + backend = InMemoryBackend(module, online, start_empty=replaced) + + # Build desired config with InMemoryBackend + for config in config_list: + # Create a temporary module params dict for this config + temp_params = copy.deepcopy(module.params) + base_spec = get_base_argument_spec() + for key, spec in base_spec.items(): + temp_params[key] = config.get(key, spec.get("default")) + + # Extract necessary parameters and set in backend for this config item + backend.permanent = temp_params.get("permanent") + backend.runtime = temp_params.get("runtime") + backend.zone = temp_params.get("zone") + backend.state = temp_params.get("state") + backend.timeout = temp_params.get("timeout", 0) + + # Apply this configuration to the backend + # Pass the backend to process_single_config + process_single_config( + module, + config_params=config, + backend=backend, + online_param=online, + __called_from_role_param=__called_from_role_param, + ) + + # Get the original and desired configs + original_config, permanent_config, runtime_config, default_zone, firewalld_conf = ( + backend.get_configs() + ) + + custom_permanent_with_defaults = original_config.get( + "custom_permanent_with_defaults", {} + ) + diff = get_diffs( + original_config["custom_permanent_with_defaults"], + original_config.get("custom_runtime_with_defaults", {}), + original_config["default_zone"], + original_config["firewalld_conf"], + permanent_config, + runtime_config, + default_zone, + firewalld_conf, + True, + ) + changed = any( + ( + diff["permanent_diff"], + diff["runtime_diff"], + diff.get("before_default_zone", False), + diff.get("before_allow_zone_drifting", False), + ) + ) + + need_remove_custom_files = False + # I have seen cases where there are files in /etc/firewalld for custom permanent config, but + # the config is exactly the same as the default - this causes problems because it appears there + # is custom permanent config, but it isn't different - not sure how this happens - I have seen + # it in machine image builds, as if someone copied files from /usr/lib/firewalld + # into /etc/firewalld but didn't change them - the problem is that this causes the tools to + # report custom config where there isn't any actual customization or changes from the defaults + if replaced and (changed or custom_permanent_with_defaults): + default_diffs = {} + if custom_permanent_with_defaults: + # are the custom permanent settings the same as the defaults? + default_diffs = recursive_show_diffs( + custom_permanent_with_defaults, original_config.get("default", {}), None + ) + need_remove_custom_files = changed or not default_diffs + return ( + changed, + diff, + need_remove_custom_files, + original_config, + backend.set_interface_changed, + ) + + +def process_replaced_config( + module, + changed, + need_remove_custom_files, + online, +): + # Remove existing firewalld configuration files + if not module.check_mode and (changed or need_remove_custom_files): + # Remove firewalld.conf + firewalld_conf_path = os.path.join(FIREWALLD_DIR, "firewalld.conf") + if changed and os.path.exists(firewalld_conf_path): + try: + os.remove(firewalld_conf_path) + module.debug("Removed %s" % firewalld_conf_path) + # ensure that firewalld.conf exists + fc = firewalld_conf(None) + try: + fc.read() + except Exception: + pass # the None causes an exception in read(), but this populates the fallback configuration + fc.filename = firewalld_conf_path + fc.write() + module.debug("Wrote fallback configuration to %s" % firewalld_conf_path) + except Exception as e: + module.fail_json( + msg="Failed to remove %s: %s" % (firewalld_conf_path, str(e)) + ) + + # remove custom files even if nothing else has changed in order to clean up cases + # where there is a custom file which is exactly the same as a default file e.g. + # someone copied public.xml from /usr/lib/firewalld/zones to /etc/firewalld/zones + # and did not change it - it messes up the fact gathering because it is reported + # as a customization but it really isn't - in this case we report changed: false + # because the configuration didn't really change + if changed or need_remove_custom_files: + for root, dirs, files in os.walk(FIREWALLD_DIR): + for filename in files: + if filename.endswith(".xml"): + xml_file = os.path.join(root, filename) + if os.path.exists(xml_file): + try: + os.remove(xml_file) + module.debug("Removed %s" % xml_file) + except Exception as e: + module.warn( + "Failed to remove %s: %s" % (xml_file, str(e)) + ) + + if online: + # Use FirewallClient to reload + try: + fw = FirewallClient() + fw.reload() + module.debug("Reloaded firewalld via FirewallClient") + except Exception as e: + module.fail_json(msg="Failed to reload firewalld: %s" % str(e)) + else: + # Use firewall-offline-cmd (no reload needed in offline mode) + module.debug("Offline mode - no reload needed") + + +def main(): + # Create the base argument spec + argument_spec = get_full_argument_spec() + + # Add config_list parameter + argument_spec["config_list"] = dict( + required=False, + type="list", + elements="dict", + options=get_base_argument_spec(), + default=[], + ) + + module = AnsibleModule( + argument_spec=argument_spec, + supports_check_mode=True, + required_if=( + ("state", "present", ("zone", "target", "service"), True), + ("state", "absent", ("zone", "target", "service"), True), + ), + ) + + if not HAS_FIREWALLD: + module.fail_json(msg="No firewall backend could be imported.") + + config_list = module.params["config_list"] + online = module.params["online"] + __called_from_role = module.params["__called_from_role"] + diff = {} + # Check for mutual exclusivity between config_list and other parameters + if config_list: + # Check if any non-config_list parameters are set (except online) + base_params = get_base_argument_spec() + current_params_set = [] + for param_name, param_spec in base_params.items(): + if param_name in ["online", "__called_from_role"]: + continue + param_value = module.params[param_name] + default_value = param_spec.get("default") + if param_value != default_value: + current_params_set.append(param_name) + + if current_params_set: + module.fail_json( + msg="config_list cannot be used together with other module parameters. " + "Found these parameters set: %s" % ", ".join(current_params_set) + ) + + # Pre-scan config_list for previous="replaced" + has_replaced = any( + config.get("previous") == "replaced" for config in config_list + ) + + # Filter out all items with previous (keep items WITHOUT previous) + filtered_config_list = [ + config for config in config_list if config.get("previous") != "replaced" + ] + + # Validate all configs + for i, config in enumerate(filtered_config_list): + if not isinstance(config, dict): + module.fail_json( + msg="config_list item %d must be a dictionary, got %s" + % (i, type(config).__name__) + ) + + # Validate config parameters against argument spec + for key in config: + if key not in base_params: + module.fail_json( + msg="config_list item %d contains invalid parameter '%s'. " + "Valid parameters: %s" % (i, key, ", ".join(base_params.keys())) + ) + + ( + changed, + diff, + need_remove_custom_files, + original_config, + set_interface_changed, + ) = check_for_diffs( + module, + filtered_config_list, + has_replaced, + online_param=online, + __called_from_role_param=__called_from_role, + ) + # NOTE: This means using previous: replaced with an interface change will not be + # completely idempotent because we have to remove the files and reload firewalld in order to see + # if the interface changes will actually change anything. + if has_replaced and ( + changed or need_remove_custom_files or set_interface_changed + ): + # This handles removing the files that need to be removed and reloading firewalld if necessary + process_replaced_config( + module, + changed, + need_remove_custom_files or set_interface_changed, + online, + ) + + if not changed and not set_interface_changed: + # The desired configuration is exactly the same as the current configuration, + # and no interfaces have changed, so we can exit early. + module.exit_json( + changed=False, __firewall_changed=False, diff=diff, short_circuit=True + ) - changed = backend.changed and module.params["__report_changed"] - module.exit_json(changed=changed, __firewall_changed=changed) + # From here on, something has changed, and we need to process those changes + # if set_interface_changed is True, we need to process the changes in order to see + # if the interfaces have changed + + # Process each configuration in the list normally + for config in filtered_config_list: + # Process this configuration + if process_single_config( + module, + config_params=config, + online_param=online, + __called_from_role_param=__called_from_role, + ): + # Something changed + changed = True + + # get the current config after processing the changes, then compare it to the original config + current_config = config_to_dict(module, detailed=True, online=online) + diff = get_diffs( + original_config["custom_permanent_with_defaults"], + original_config.get("custom_runtime_with_defaults", {}), + original_config["default_zone"], + original_config["firewalld_conf"], + current_config["custom_permanent_with_defaults"], + current_config.get("custom_runtime_with_defaults", {}), + current_config["default_zone"], + current_config["firewalld_conf"], + False, + ) + if not changed: + changed = any( + ( + diff["permanent_diff"], + diff["runtime_diff"], + diff.get("before_default_zone", False), + diff.get("before_allow_zone_drifting", False), + ) + ) + module.exit_json( + changed=changed, __firewall_changed=changed, diff=diff, short_circuit=False + ) + + else: + # Original single configuration mode + changed = process_single_config( + module, online_param=online, __called_from_role_param=__called_from_role + ) + module.exit_json( + changed=changed, __firewall_changed=changed, short_circuit=False + ) ################################################# diff --git a/library/firewall_lib_facts.py b/library/firewall_lib_facts.py index 73f453e7..c700126d 100644 --- a/library/firewall_lib_facts.py +++ b/library/firewall_lib_facts.py @@ -19,7 +19,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from __future__ import absolute_import, division, print_function +from __future__ import absolute_import, division, print_function, unicode_literals __metaclass__ = type @@ -80,177 +80,7 @@ from ansible.module_utils.basic import AnsibleModule -import os - -try: - import firewall.config - - from firewall.client import FirewallClient - - HAS_FIREWALLD = True -except ImportError: - HAS_FIREWALLD = False - -try: - if HAS_FIREWALLD: - firewall.config.FIREWALLD_POLICIES - - HAS_POLICIES = True -except AttributeError: - HAS_POLICIES = False - - -def offline_cmd(module, args, defaults=False): - # get the defaults by disabling the --system-config dir (ETC_FIREWALLD) - conf_args = ["--system-config=/nonexisting"] if defaults else [] - return module.run_command( - ["firewall-offline-cmd"] + conf_args + args, - check_rc=True, - )[1].strip() - - -def config_to_dict(module): - detailed = module.params.get("detailed", False) - online = module.params.get("online", True) - config = {} - defaults = {} - custom = {} - setting_list = ["zones", "services", "icmptypes", "helpers", "ipsets"] - - if detailed and not online: - module.fail_json(msg="detailed information not available in offline mode") - - if HAS_POLICIES: - setting_list.append("policies") - - if online: - fw = FirewallClient() - - for setting in setting_list: - default_setting_dir = os.path.join( - firewall.config.USR_LIB_FIREWALLD, setting - ) - custom_setting_dir = os.path.join(firewall.config.ETC_FIREWALLD, setting) - - settings = fetch_settings_from_dir(default_setting_dir, detailed, fw) - if settings: - defaults[setting] = settings - settings = fetch_settings_from_dir(custom_setting_dir, True, fw) - if settings: - custom[setting] = settings - - config["default_zone"] = fw.getDefaultZone() - else: - # get the default settings - for setting in setting_list: - values = offline_cmd(module, ["--get-" + setting], defaults=True).split() - # zone services are important, so make them a dict, not a plain list (same as with online mode) - if setting == "zones": - for zone in values: - services = offline_cmd( - module, ["--zone", zone, "--list-services"], defaults=True - ).split() - defaults.setdefault("zones", {})[zone] = {"services": services} - else: - defaults[setting] = values - - # compute the custom settings by diffing with defaults - for setting in setting_list: - values = offline_cmd(module, ["--get-" + setting]).split() - if setting == "zones": - for zone in values: - services = offline_cmd( - module, ["--zone", zone, "--list-services"] - ).split() - # add zone to custom if the whole zone is new or its services changed - if zone not in defaults["zones"] or ( - set(services) - set(defaults["zones"][zone]["services"]) - ): - custom.setdefault("zones", {})[zone] = {"services": services} - else: - diff = set(values) - set(defaults[setting]) - if diff: - custom[setting] = sorted(diff) - - config["default_zone"] = offline_cmd(module, ["--get-default-zone"]) - - if defaults: - config["default"] = defaults - if custom: - config["custom"] = custom - return config - - -def fetch_settings_from_dir(directory, detailed=False, fw=None): - setting_options = [ - _file[:-4] for _file in os.listdir(directory) if _file.endswith(".xml") - ] - if not detailed: - return setting_options - else: - setting_name = os.path.basename(directory) - settings = {} - for _item in setting_options: - element_settings = {} - if setting_name == "zones": - element = fw.config().getZoneByName(_item).getSettings() - try: - element_settings = element.getSettingsDict() - except AttributeError: - element_settings["version"] = element.getVersion() - element_settings["short"] = element.getShort() - element_settings["description"] = element.getDescription() - element_settings["target"] = element.getTarget() - element_settings["services"] = element.getServices() - element_settings["ports"] = element.getPorts() - element_settings["icmp_blocks"] = element.getIcmpBlocks() - element_settings["masqerade"] = element.getMasquerade() - element_settings["forward_ports"] = element.getForwardPorts() - element_settings["interfaces"] = element.getInterfaces() - element_settings["sources"] = element.getSources() - element_settings["rules_str"] = element.getRichRules() - element_settings["protocols"] = element.getProtocols() - element_settings["source_ports"] = element.getSourcePorts() - element_settings["icmp_block_inversion"] = ( - element.getIcmpBlockInversion() - ) - elif setting_name == "services": - element = fw.config().getServiceByName(_item).getSettings() - try: - element_settings = element.getSettingsDict() - except AttributeError: - element_settings["version"] = element.getVersion() - element_settings["short"] = element.getShort() - element_settings["description"] = element.getDescription() - element_settings["protocols"] = element.getProtocols() - element_settings["source_ports"] = element.getSourcePorts() - element_settings["modules"] = element.getModules() - elif setting_name == "icmptypes": - element = fw.config().getIcmpTypeByName(_item).getSettings() - element_settings["version"] = element.getVersion() - element_settings["short"] = element.getShort() - element_settings["description"] = element.getDescription() - element_settings["destination"] = element.getDestinations() - elif setting_name == "helpers": - element = fw.config().getHelperByName(_item).getSettings() - element_settings["version"] = element.getVersion() - element_settings["short"] = element.getShort() - element_settings["description"] = element.getDescription() - element_settings["family"] = element.getFamily() - element_settings["module"] = element.getModule() - element_settings["port"] = element.getPorts() - elif setting_name == "ipsets": - element = fw.config().getIPSetByName(_item).getSettings() - element_settings["version"] = element.getVersion() - element_settings["short"] = element.getShort() - element_settings["description"] = element.getDescription() - element_settings["options"] = element.getOptions() - element_settings["entries"] = element.getEntries() - elif setting_name == "policies": - element = fw.config().getPolicyByName(_item).getSettings() - element_settings = element.getSettingsDict() - settings[_item] = element_settings - return settings +from ansible.module_utils.firewall_lsr.get_config import config_to_dict, HAS_FIREWALLD def main(): diff --git a/module_utils/firewall_lsr/__init__.py b/module_utils/firewall_lsr/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/module_utils/firewall_lsr/get_config.py b/module_utils/firewall_lsr/get_config.py new file mode 100644 index 00000000..02da5d7b --- /dev/null +++ b/module_utils/firewall_lsr/get_config.py @@ -0,0 +1,1099 @@ +# -*- coding: utf-8 -*- +# +# Copyright (C) 2016,2017,2020,2021,2024 Red Hat, Inc. +# Reusing some firewalld code +# Authors: +# Brennan Paciorek +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import absolute_import, division, print_function, unicode_literals + +__metaclass__ = type + +import copy +import os +import sys + +try: + import firewall.config + + from firewall.client import FirewallClient + + # firewall.core.io modules needed for xml file reading + from firewall.core.io.zone import zone_reader + from firewall.core.io.service import service_reader + from firewall.core.io.icmptype import icmptype_reader + from firewall.core.io.ipset import ipset_reader + from firewall.core.io.helper import helper_reader + from firewall.core.io.firewalld_conf import firewalld_conf + + HAS_FIREWALLD = True + FALLBACK_ZONE = firewall.config.FALLBACK_ZONE +except ImportError: + HAS_FIREWALLD = False + FALLBACK_ZONE = "public" +try: + if HAS_FIREWALLD: + firewall.config.FIREWALLD_POLICIES + from firewall.core.io.policy import policy_reader + + HAS_POLICIES = True +except AttributeError: + HAS_POLICIES = False +except ImportError: + HAS_POLICIES = False + + +def offline_cmd(module, args, defaults=False): + # get the defaults by disabling the --system-config dir (ETC_FIREWALLD) + conf_args = ["--system-config=/nonexisting"] if defaults else [] + cmd = ["firewall-offline-cmd"] + conf_args + args + (rc, out, err) = module.run_command(cmd) + if rc != 0: + if ( + rc == 1 + and "No such file or directory: '/nonexisting/firewalld.conf'" in err + ): + pass + else: + module.fail_json( + msg="Failed to execute firewall-offline-cmd: cmd [%s] rc [%s] out [%s] err [%s]" + % (cmd.join(" "), rc, out, err) + ) + return out.strip() + + +# EL7 does not have this method, so make our own +def export_config_dict(io_object): + if HAS_FIREWALLD: + try: + object_dict = io_object.export_config_dict() + except AttributeError: + object_dict = {} + for key, unused_value in io_object.IMPORT_EXPORT_STRUCTURE: + if hasattr(io_object, key) and key != "UNUSED": + object_dict[key] = getattr(io_object, key) + if isinstance(io_object, firewall.core.io.zone.Zone): + if object_dict.get("target") == firewall.core.base.DEFAULT_ZONE_TARGET: + # to correspond with online getTarget() + object_dict["target"] = "default" + if object_dict.get("forward_ports"): + object_dict["forward_ports"] = normalize_forward_ports( + object_dict["forward_ports"] + ) + return object_dict + else: + return {} + + +def normalize_settings(settings): + # normalize the settings to remove empty values and set default values + # this duplicates the logic in core/fw_zone.py:get_config_with_settings_dict() + for kk, vv in list(settings.items()): + if kk == "target" and vv == firewall.core.base.DEFAULT_ZONE_TARGET: + settings[kk] = "default" + elif vv or isinstance(vv, bool) or isinstance(vv, int): + pass + else: + # remove the key if the value is empty + del settings[kk] + return settings + + +# not used - this method is extremely slow, but kept for reference +def fetch_settings_using_offline_cmd(module, setting_name): + """ + Fetch firewall settings using firewall-offline-cmd. + + This function retrieves firewall configuration using the offline command interface, + which can query either default settings or custom permanent settings. + + Args: + module: Ansible module object (for running commands) + setting_name: Type of setting to fetch ('zones', 'services', 'icmptypes', + 'helpers', 'ipsets', 'policies') + + Returns: + Dictionary with object names as keys and their settings as values + """ + # Get list of items for this setting type + setting_options = offline_cmd( + module, ["--get-" + setting_name], defaults=True + ).split() + + # Get detailed information for each item + settings = {} + + for item in setting_options: + element_settings = {} + + try: + if setting_name == "zones": + # Get zone details + element_settings["services"] = offline_cmd( + module, ["--zone=" + item, "--list-services"], defaults=True + ).split() + element_settings["ports"] = [ + tuple(p.split("/")) + for p in offline_cmd( + module, ["--zone=" + item, "--list-ports"], defaults=True + ).split() + ] + element_settings["protocols"] = offline_cmd( + module, ["--zone=" + item, "--list-protocols"], defaults=True + ).split() + element_settings["source_ports"] = [ + tuple(p.split("/")) + for p in offline_cmd( + module, ["--zone=" + item, "--list-source-ports"], defaults=True + ).split() + ] + element_settings["icmp_blocks"] = offline_cmd( + module, ["--zone=" + item, "--list-icmp-blocks"], defaults=True + ).split() + element_settings["forward_ports"] = offline_cmd( + module, ["--zone=" + item, "--list-forward-ports"], defaults=True + ).split() + element_settings["interfaces"] = offline_cmd( + module, ["--zone=" + item, "--list-interfaces"], defaults=True + ).split() + element_settings["sources"] = offline_cmd( + module, ["--zone=" + item, "--list-sources"], defaults=True + ).split() + element_settings["rules_str"] = ( + offline_cmd( + module, ["--zone=" + item, "--list-rich-rules"], defaults=True + ).split("\n") + if offline_cmd( + module, ["--zone=" + item, "--list-rich-rules"], defaults=True + ) + else [] + ) + + # Query masquerade (returns yes/no or error) + try: + masq_result = offline_cmd( + module, ["--zone=" + item, "--query-masquerade"], defaults=True + ) + element_settings["masquerade"] = masq_result == "yes" + except Exception: + pass # just omit the masquerade setting if it's not available + + # Get target + try: + element_settings["target"] = offline_cmd( + module, ["--zone=" + item, "--get-target"], defaults=True + ) + except Exception: + pass # just omit the target setting if it's not available + + # Get description and short + try: + element_settings["description"] = offline_cmd( + module, ["--zone=" + item, "--get-description"], defaults=True + ) + except Exception: + pass # just omit the description setting if it's not available + + try: + element_settings["short"] = offline_cmd( + module, ["--zone=" + item, "--get-short"], defaults=True + ) + except Exception: + pass # just omit the short setting if it's not available + + elif setting_name == "services": + # Get service details + ports_output = offline_cmd( + module, ["--service=" + item, "--get-ports"], defaults=True + ) + element_settings["ports"] = ( + [tuple(p.split("/")) for p in ports_output.split()] + if ports_output + else [] + ) + + element_settings["protocols"] = offline_cmd( + module, ["--service=" + item, "--get-protocols"], defaults=True + ).split() + + modules_output = offline_cmd( + module, ["--service=" + item, "--get-modules"], defaults=True + ) + element_settings["modules"] = ( + modules_output.split() if modules_output else [] + ) + + source_ports_output = offline_cmd( + module, ["--service=" + item, "--get-source-ports"], defaults=True + ) + element_settings["source_ports"] = ( + [tuple(p.split("/")) for p in source_ports_output.split()] + if source_ports_output + else [] + ) + + try: + element_settings["description"] = offline_cmd( + module, + ["--service=" + item, "--get-description"], + defaults=True, + ) + except Exception: + pass # just omit the description setting if it's not available + + try: + element_settings["short"] = offline_cmd( + module, ["--service=" + item, "--get-short"], defaults=True + ) + except Exception: + pass # just omit the short setting if it's not available + + elif setting_name == "icmptypes": + # Get icmptype details + dest_output = offline_cmd( + module, ["--icmptype=" + item, "--get-destinations"], defaults=True + ) + element_settings["destination"] = ( + dest_output.split() if dest_output else [] + ) + + try: + element_settings["description"] = offline_cmd( + module, + ["--icmptype=" + item, "--get-description"], + defaults=True, + ) + except Exception: + pass # just omit the description setting if it's not available + + try: + element_settings["short"] = offline_cmd( + module, ["--icmptype=" + item, "--get-short"], defaults=True + ) + except Exception: + pass # just omit the short setting if it's not available + + elif setting_name == "helpers": + # Get helper details + element_settings["family"] = offline_cmd( + module, ["--helper=" + item, "--get-family"], defaults=True + ) + + element_settings["module"] = offline_cmd( + module, ["--helper=" + item, "--get-module"], defaults=True + ) + + ports_output = offline_cmd( + module, ["--helper=" + item, "--get-ports"], defaults=True + ) + element_settings["ports"] = ( + [tuple(p.split("/")) for p in ports_output.split()] + if ports_output + else [] + ) + + try: + element_settings["description"] = offline_cmd( + module, ["--helper=" + item, "--get-description"], defaults=True + ) + except Exception: + pass # just omit the description setting if it's not available + + try: + element_settings["short"] = offline_cmd( + module, ["--helper=" + item, "--get-short"], defaults=True + ) + except Exception: + pass # just omit the short setting if it's not available + + elif setting_name == "ipsets": + # Get ipset details + element_settings["type"] = offline_cmd( + module, ["--ipset=" + item, "--get-type"], defaults=True + ) + + entries_output = offline_cmd( + module, ["--ipset=" + item, "--get-entries"], defaults=True + ) + element_settings["entries"] = ( + entries_output.split("\n") if entries_output else [] + ) + + try: + element_settings["description"] = offline_cmd( + module, ["--ipset=" + item, "--get-description"], defaults=True + ) + except Exception: + pass # just omit the description setting if it's not available + + try: + element_settings["short"] = offline_cmd( + module, ["--ipset=" + item, "--get-short"], defaults=True + ) + except Exception: + pass # just omit the short setting if it's not available + + # Get options if available + try: + options_output = offline_cmd( + module, ["--ipset=" + item, "--get-options"], defaults=True + ) + element_settings["options"] = ( + options_output.split() if options_output else [] + ) + except Exception: + pass # just omit the options setting if it's not available + + elif setting_name == "policies": + # Get policy details (similar to zones) + element_settings["services"] = offline_cmd( + module, ["--policy=" + item, "--list-services"], defaults=True + ).split() + + element_settings["ports"] = [ + tuple(p.split("/")) + for p in offline_cmd( + module, ["--policy=" + item, "--list-ports"], defaults=True + ).split() + ] + + element_settings["protocols"] = offline_cmd( + module, ["--policy=" + item, "--list-protocols"], defaults=True + ).split() + + element_settings["icmp_blocks"] = offline_cmd( + module, ["--policy=" + item, "--list-icmp-blocks"], defaults=True + ).split() + + element_settings["forward_ports"] = offline_cmd( + module, ["--policy=" + item, "--list-forward-ports"], defaults=True + ).split() + + element_settings["rules_str"] = ( + offline_cmd( + module, ["--policy=" + item, "--list-rich-rules"], defaults=True + ).split("\n") + if offline_cmd( + module, ["--policy=" + item, "--list-rich-rules"], defaults=True + ) + else [] + ) + + # Get ingress/egress zones + try: + element_settings["ingress_zones"] = offline_cmd( + module, + ["--policy=" + item, "--list-ingress-zones"], + defaults=True, + ).split() + except Exception: + pass # just omit the ingress_zones setting if it's not available + + try: + element_settings["egress_zones"] = offline_cmd( + module, + ["--policy=" + item, "--list-egress-zones"], + defaults=True, + ).split() + except Exception: + pass # just omit the egress_zones setting if it's not available + + try: + element_settings["target"] = offline_cmd( + module, ["--policy=" + item, "--get-target"], defaults=True + ) + except Exception: + pass # just omit the target setting if it's not available + + try: + element_settings["description"] = offline_cmd( + module, ["--policy=" + item, "--get-description"], defaults=True + ) + except Exception: + pass # just omit the description setting if it's not available + + try: + element_settings["short"] = offline_cmd( + module, ["--policy=" + item, "--get-short"], defaults=True + ) + except Exception: + pass # just omit the short setting if it's not available + + settings[item] = normalize_settings(element_settings) + + except Exception as e: + # If we can't get details for an item, log warning and skip + module.warn( + "Failed to get details for " + + setting_name + + " '" + + item + + "': " + + str(e) + ) + continue + + return settings + + +def config_to_dict(module, detailed=None, online=None): + if detailed is None: + detailed = module.params.get("detailed", False) + if online is None: + online = module.params.get("online", True) + config = {} + defaults = {} + custom_permanent = {} + setting_list = ["zones", "services", "icmptypes", "helpers", "ipsets"] + + if HAS_POLICIES: + setting_list.append("policies") + + defaults = fetch_settings_from_xml_files(module, setting_list, defaults=True) + config["default"] = defaults + custom_permanent = fetch_settings_from_xml_files( + module, setting_list, defaults=False + ) + custom_permanent_with_defaults = merge_with_defaults(custom_permanent, defaults) + config["custom_permanent_with_defaults"] = custom_permanent_with_defaults + if custom_permanent: + config["custom_permanent"] = custom_permanent + config["custom"] = custom_permanent # legacy compatibility + # this is the built-in default zone if there is no firewalld.conf + config["fallback_default_zone"] = FALLBACK_ZONE + # get firewalld.conf settings + fc = firewalld_conf(firewall.config.FIREWALLD_CONF) + fc.read() + config["firewalld_conf"] = {"allow_zone_drifting": fc.get("AllowZoneDrifting")} + if online: + fw = FirewallClient() + + current_settings = fetch_online_settings(fw, setting_list, detailed) + # NOTE: In some cases, the current settings may not include the default settings read from the XML files, + # for example, the icmptype beyond-scope cannot be loaded by the firewalld server because it is not + # supported by the kernel, so it will not show up in the online list of icmptypes. Rather than + # trying to duplicate that logic here, and only keep the list of supported icmptypes, we merge the + # the current settings with the defaults to get the full list of settings. + current_settings_with_defaults = merge_with_defaults(current_settings, defaults) + config["current"] = current_settings_with_defaults + config["custom_runtime_with_defaults"] = current_settings_with_defaults + config["default_zone"] = fw.getDefaultZone() + # the current settings include the custom permanent with defaults settings, so we need to diff them to get the custom runtime settings + dict_diff_normalizers = DEFAULT_NORMALIZERS + custom_runtime = recursive_dict_diff( + current_settings_with_defaults, + custom_permanent_with_defaults, + dict_diff_normalizers, + ) + if custom_runtime: + config["runtime_only"] = custom_runtime + else: + config["default_zone"] = offline_cmd(module, ["--get-default-zone"]) + + return config + + +def normalize_value(value, normalizers=None): + """ + Normalize a value using provided normalization functions. + + Args: + value: The value to normalize + normalizers: Optional dict or callable for normalization + - If callable: applied to the value + - If dict: keys are types or paths, values are normalization functions + + Returns: + Normalized value + """ + if normalizers is None: + return value + + if callable(normalizers): + return normalizers(value) + + # For dict-based normalizers, try type-based lookup + if isinstance(normalizers, dict): + value_type = type(value).__name__ + if value_type in normalizers: + return normalizers[value_type](value) + + return value + + +def normalize_list(lst, normalizers=None): + """ + Normalize and sort a list for comparison. + + Args: + lst: List to normalize + normalizers: Optional normalization functions + + Returns: + Sorted list with normalized values + """ + if not isinstance(lst, list): + return lst + + # Normalize each item in the list + normalized = [normalize_value(item, normalizers) for item in lst] + + # Sort the list for comparison (handle mixed types carefully) + try: + return sorted(normalized) + except TypeError: + # If items aren't directly comparable, convert to strings for sorting + return sorted(normalized, key=str) + + +DEFAULT_NORMALIZERS = { + "str": lambda s: s.strip().lower(), + "list": lambda ll: sorted([x.strip() for x in ll]), +} + + +def recursive_dict_diff(dict1, dict2, normalizers, path=""): + """ + Recursively compare two dictionaries and return only the differences. + + This function compares dict1 against dict2 and returns a new dictionary + containing only the keys/values from dict1 that are different from dict2 + or don't exist in dict2. + + Args: + dict1: First dictionary (the one to extract differences from) + dict2: Second dictionary (the reference to compare against) + normalizers: Optional dict of normalization functions for value comparison + Keys can be: + - Type names (e.g., 'str', 'int', 'list') + - Path patterns (e.g., 'zones.*.services') + Values are callables that normalize the value + path: Internal parameter for tracking the current path in nested dicts + + Returns: + Dictionary containing only the differences from dict1 + """ + if dict1 is None and dict2 is None: + return None + if dict1 is None: + return None + if dict2 is None: + return dict1 + + if not isinstance(dict1, dict) or not isinstance(dict2, dict): + # For non-dict values, normalize and compare + norm1 = normalize_value(dict1, normalizers) + norm2 = normalize_value(dict2, normalizers) + + if isinstance(dict1, list) and isinstance(dict2, list): + # Compare lists (normalized for comparison) + normalized1 = normalize_list(dict1, normalizers) + normalized2 = normalize_list(dict2, normalizers) + if normalized1 != normalized2: + return list(set(normalized1) - set(normalized2)) + return None + elif norm1 != norm2: + return dict1 + return None + + diff = {} + + # Check all keys in dict1 + for key in dict1: + current_path = "%s.%s" % (path, key) if path else key + + if key not in dict2: + # Key exists in dict1 but not in dict2 + diff[key] = dict1[key] + else: + # Key exists in both, compare values + value1 = dict1[key] + value2 = dict2[key] + + if isinstance(value1, dict) and isinstance(value2, dict): + # Recursively compare nested dictionaries + nested_diff = recursive_dict_diff( + value1, value2, normalizers, current_path + ) + if nested_diff: + diff[key] = nested_diff + elif isinstance(value1, list) and isinstance(value2, list): + # Compare lists (normalized for comparison) + normalized1 = normalize_list(value1, normalizers) + normalized2 = normalize_list(value2, normalizers) + if normalized1 != normalized2: + diff[key] = list(set(normalized1) - set(normalized2)) + else: + # Compare scalar values (with normalization) + norm1 = normalize_value(value1, normalizers) + norm2 = normalize_value(value2, normalizers) + if norm1 != norm2: + diff[key] = value1 + + return diff if diff else None + + +def recursive_show_diffs(dict1, dict2, normalizers, ignore_interface=False, path=""): + """ + Recursively compare two dictionaries and return before/after differences. + + This function compares dict1 against dict2 and returns a dictionary with + 'before' and 'after' keys showing the values from dict1 and dict2 respectively + for any elements that differ. + + Args: + dict1: First dictionary (the "before" state) + dict2: Second dictionary (the "after" state) + normalizers: Optional dict of normalization functions for value comparison + Keys can be: + - Type names (e.g., 'str', 'int', 'list') + - Path patterns (e.g., 'zones.*.services') + Values are callables that normalize the value + ignore_interface: If True, ignore changes to the interface list + path: Internal parameter for tracking the current path in nested dicts + + Returns: + Dictionary with 'before' and 'after' keys containing the differences, + or None if no differences found + """ + # If ignore_interface is True, we need to ignore changes to the interface list. + # This is because the interface list is not part of the firewall configuration, + # it is managed by the NetworkManager, and the InMemoryBackend has no way to + # know if a change to the interface list is actually a change to the firewall configuration. + # we are lucky because the only key named "interface" is in the zone configuration. If + # in the future there is another key named "interface" in some other object or at some + # other level, we will need to modify this code to use path e.g. "zones.*.interface". + + if dict1 is None and dict2 is None: + return None + if dict1 is None and dict2 is not None: + return {"before": None, "after": dict2} + if dict2 is None and dict1 is not None: + return {"before": dict1, "after": None} + + if not isinstance(dict1, dict) or not isinstance(dict2, dict): + # For non-dict values, normalize and compare + norm1 = normalize_value(dict1, normalizers) + norm2 = normalize_value(dict2, normalizers) + + if isinstance(dict1, list) and isinstance(dict2, list): + # Compare lists (normalized for comparison) + normalized1 = normalize_list(dict1, normalizers) + normalized2 = normalize_list(dict2, normalizers) + if normalized1 != normalized2: + return {"before": dict1, "after": dict2} + return None + elif norm1 != norm2: + return {"before": dict1, "after": dict2} + return None + + before = {} + after = {} + + # Check all keys in dict1 + for key in dict1: + if ignore_interface and key == "interfaces": + continue + current_path = "%s.%s" % (path, key) if path else key + + if key not in dict2: + if dict1[key] in [None, [], {}, ""]: + pass # missing key is same as empty value + else: + # Key exists in dict1 but not in dict2 + before[key] = dict1[key] + elif key == "forward": + pass # we don't allow setting forward in zones, so we don't compare it + else: + # Key exists in both, compare values + value1 = dict1[key] + value2 = dict2[key] + + if isinstance(value1, dict) and isinstance(value2, dict): + # Recursively compare nested dictionaries + nested_result = recursive_show_diffs( + value1, value2, normalizers, current_path + ) + if nested_result: + before[key] = nested_result["before"] + after[key] = nested_result["after"] + elif isinstance(value1, list) and isinstance(value2, list): + # Compare lists (normalized for comparison) + normalized1 = normalize_list(value1, normalizers) + normalized2 = normalize_list(value2, normalizers) + if normalized1 != normalized2: + before[key] = value1 + after[key] = value2 + else: + # Compare scalar values (with normalization) + norm1 = normalize_value(value1, normalizers) + norm2 = normalize_value(value2, normalizers) + if norm1 != norm2: + before[key] = value1 + after[key] = value2 + + # Check for keys only in dict2 + for key in dict2: + if ignore_interface and key == "interfaces": + continue + if key not in dict1: + if dict2[key] in [None, [], {}, ""]: + pass # missing key is same as empty value + else: + # Key exists in dict2 but not in dict1 + after[key] = dict2[key] + + if before or after: + return {"before": before, "after": after} + return None + + +def _recursive_merge_with_defaults(custom, defaults): + if isinstance(custom, dict) and isinstance(defaults, dict): + return_value = {} + for key in defaults: + if key in custom: + return_value[key] = _recursive_merge_with_defaults( + custom[key], defaults[key] + ) + else: + return_value[key] = copy.deepcopy(defaults[key]) + for key in custom: + if key not in defaults: + return_value[key] = copy.deepcopy(custom[key]) + else: + return_value = copy.deepcopy(custom) + return return_value + + +def merge_with_defaults(custom, defaults): + """ + Merge default settings into custom settings. + + This function merges default firewall configuration settings into custom + settings. For each top-level configuration type (zones, services, icmptypes, + helpers, ipsets, policies), if an item exists in defaults but not in custom, + it will be copied from defaults to custom. + + Args: + custom: Dictionary with custom settings (will be modified in place) + defaults: Dictionary with default settings + + Returns: + The modified custom dictionary with defaults merged in + """ + # Top-level keys that contain named items to merge + if not custom: + return defaults + if not defaults: + return custom + if not isinstance(defaults, dict) or not isinstance(custom, dict): + return copy.deepcopy(custom) + return _recursive_merge_with_defaults(custom, defaults) + + +def normalize_forward_ports(forward_ports): + fwd_ports_list_of_tuples = [] + for fwd_port in forward_ports: + fwd_port_tuple = [] + for ii in fwd_port: + if ii == "": + fwd_port_tuple.append(None) + else: + fwd_port_tuple.append(ii) + fwd_ports_list_of_tuples.append(tuple(fwd_port_tuple)) + return fwd_ports_list_of_tuples + + +def fetch_online_settings(fw, setting_list, detailed=False): + """ + Fetch firewall runtime settings using the online FirewallClient API. + + Args: + fw: FirewallClient object + setting_list: List of setting types to fetch (e.g., ['zones', 'services', 'icmptypes', 'helpers', 'ipsets', 'policies']) + detailed: If True, fetch detailed information for each object; if False, return only names + + Returns: + Dictionary with setting types as keys and their data as values + """ + all_settings = {} + + for setting_name in setting_list: + # Get list of items for this setting type using the runtime API + if setting_name == "zones": + setting_options = fw.getZones() + elif setting_name == "services": + setting_options = fw.listServices() + elif setting_name == "icmptypes": + setting_options = fw.listIcmpTypes() + elif setting_name == "helpers": + setting_options = fw.getHelpers() + elif setting_name == "ipsets": + setting_options = fw.getIPSets() + elif setting_name == "policies": + setting_options = fw.getPolicies() + else: + continue + + if not detailed: + all_settings[setting_name] = setting_options + else: + settings = {} + for _item in setting_options: + element_settings = {} + if setting_name == "zones": + element = fw.getZoneSettings(_item) + try: + element_settings = element.getSettingsDict() + except AttributeError: + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["target"] = element.getTarget() + element_settings["services"] = element.getServices() + element_settings["ports"] = element.getPorts() + element_settings["icmp_blocks"] = element.getIcmpBlocks() + element_settings["masquerade"] = element.getMasquerade() + element_settings["forward_ports"] = element.getForwardPorts() + element_settings["interfaces"] = element.getInterfaces() + element_settings["sources"] = element.getSources() + element_settings["rules_str"] = element.getRichRules() + element_settings["protocols"] = element.getProtocols() + element_settings["source_ports"] = element.getSourcePorts() + element_settings["icmp_block_inversion"] = ( + element.getIcmpBlockInversion() + ) + # normalize the elements + if "forward_ports" in element_settings: + element_settings["forward_ports"] = normalize_forward_ports( + element_settings["forward_ports"] + ) + elif setting_name == "services": + element = fw.getServiceSettings(_item) + try: + element_settings = element.getSettingsDict() + except AttributeError: + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["ports"] = element.getPorts() + element_settings["protocols"] = element.getProtocols() + element_settings["source_ports"] = element.getSourcePorts() + element_settings["destination"] = element.getDestinations() + element_settings["modules"] = element.getModules() + if hasattr(element, "getHelpers"): + element_settings["helpers"] = element.getHelpers() + elif setting_name == "icmptypes": + element = fw.getIcmpTypeSettings(_item) + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["destination"] = element.getDestinations() + elif setting_name == "helpers": + element = fw.getHelperSettings(_item) + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["family"] = element.getFamily() + element_settings["module"] = element.getModule() + element_settings["ports"] = element.getPorts() + elif setting_name == "ipsets": + element = fw.getIPSetSettings(_item) + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["options"] = element.getOptions() + element_settings["entries"] = element.getEntries() + element_settings["type"] = element.getType() + elif setting_name == "policies": + element = fw.getPolicySettings(_item) + element_settings = element.getSettingsDict() + settings[_item] = normalize_settings(element_settings) + all_settings[setting_name] = settings + + return all_settings + + +def cvt_str(string_value): + if sys.version_info >= (3, 0): + return string_value + else: + return str(string_value) + + +def fetch_settings_from_xml_files(module, setting_list, defaults=False): + """ + Fetch firewall settings using FirewallConfig reader API to read XML files. + + This function reads firewall configuration files directly using the + firewall.core.io reader functions (zone_reader, service_reader, etc.) + to parse XML configuration files. + + Args: + module: Ansible module object (for error handling) + setting_list: List of setting types to fetch ('zones', 'services', etc.) + defaults: If True, read from USR_LIB_FIREWALLD (default configs); + if False, read from ETC_FIREWALLD (custom configs) + + Returns: + Dictionary with setting types as keys and their data as values + """ + + # Determine root directory based on defaults parameter + root_dir = cvt_str( + firewall.config.USR_LIB_FIREWALLD if defaults else firewall.config.ETC_FIREWALLD + ) + + all_settings = {} + + for setting_name in setting_list: + # Construct the directory path for this setting type + setting_dir = cvt_str(os.path.join(root_dir, setting_name)) + + # Check if directory exists + if not os.path.exists(setting_dir) or not os.path.isdir(setting_dir): + continue + + # Get list of XML files in this directory + try: + xml_files = [ + cvt_str(f) for f in os.listdir(setting_dir) if f.endswith(".xml") + ] + except OSError: + continue + + if not xml_files: + continue + + # Get detailed information for each item + settings = {} + + for xml_file in xml_files: + item_name = xml_file[:-4] # Remove .xml extension + file_path = cvt_str(os.path.join(setting_dir, xml_file)) + + try: + # Read the configuration file using appropriate reader + if setting_name == "zones": + obj = zone_reader(xml_file, setting_dir) + elif setting_name == "services": + obj = service_reader(xml_file, setting_dir) + elif setting_name == "icmptypes": + obj = icmptype_reader(xml_file, setting_dir) + elif setting_name == "ipsets": + obj = ipset_reader(xml_file, setting_dir) + elif setting_name == "helpers": + obj = helper_reader(xml_file, setting_dir) + elif setting_name == "policies": + obj = policy_reader(xml_file, setting_dir) + else: + continue + + # Extract and normalize settings from the object + settings[item_name] = normalize_settings(export_config_dict(obj)) + + except Exception as e: + # If we can't read a file, log warning and continue + module.warn( + "Failed to read " + + setting_name + + " configuration from " + + file_path + + ": " + + str(e) + ) + continue + + if settings: + all_settings[setting_name] = settings + + return all_settings + + +def fetch_settings_from_dir(directory, detailed=False, fw=None): + setting_options = [ + _file[:-4] for _file in os.listdir(directory) if _file.endswith(".xml") + ] + if not detailed: + return setting_options + else: + setting_name = os.path.basename(directory) + settings = {} + for _item in setting_options: + element_settings = {} + if setting_name == "zones": + element = fw.config().getZoneByName(_item).getSettings() + try: + element_settings = element.getSettingsDict() + except AttributeError: + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["target"] = element.getTarget() + element_settings["services"] = element.getServices() + element_settings["ports"] = element.getPorts() + element_settings["icmp_blocks"] = element.getIcmpBlocks() + element_settings["masquerade"] = element.getMasquerade() + element_settings["forward_ports"] = element.getForwardPorts() + element_settings["interfaces"] = element.getInterfaces() + element_settings["sources"] = element.getSources() + element_settings["rules_str"] = element.getRichRules() + element_settings["protocols"] = element.getProtocols() + element_settings["source_ports"] = element.getSourcePorts() + element_settings["icmp_block_inversion"] = ( + element.getIcmpBlockInversion() + ) + elif setting_name == "services": + element = fw.config().getServiceByName(_item).getSettings() + try: + element_settings = element.getSettingsDict() + except AttributeError: + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["protocols"] = element.getProtocols() + element_settings["source_ports"] = element.getSourcePorts() + element_settings["modules"] = element.getModules() + if hasattr(element, "getHelpers"): + element_settings["helpers"] = element.getHelpers() + elif setting_name == "icmptypes": + element = fw.config().getIcmpTypeByName(_item).getSettings() + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["destination"] = element.getDestinations() + elif setting_name == "helpers": + element = fw.config().getHelperByName(_item).getSettings() + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["family"] = element.getFamily() + element_settings["module"] = element.getModule() + element_settings["ports"] = element.getPorts() + elif setting_name == "ipsets": + element = fw.config().getIPSetByName(_item).getSettings() + element_settings["version"] = element.getVersion() + element_settings["short"] = element.getShort() + element_settings["description"] = element.getDescription() + element_settings["options"] = element.getOptions() + element_settings["entries"] = element.getEntries() + element_settings["type"] = element.getType() + elif setting_name == "policies": + element = fw.config().getPolicyByName(_item).getSettings() + element_settings = element.getSettingsDict() + settings[_item] = normalize_settings(element_settings) + return settings diff --git a/tasks/firewalld.yml b/tasks/firewalld.yml index 2b446bf0..1c2f9858 100644 --- a/tasks/firewalld.yml +++ b/tasks/firewalld.yml @@ -38,6 +38,7 @@ register: __is_system_running changed_when: false failed_when: false + check_mode: "{{ not __firewall_test_check_mode | d(not ansible_check_mode) }}" - name: Require installed systemd fail: @@ -56,6 +57,7 @@ use: "{{ (__firewall_is_ostree | d(false)) | ternary('ansible.posix.rhel_rpm_ostree', omit) }}" register: firewall_package_result + check_mode: "{{ not __firewall_test_check_mode | d(not ansible_check_mode) }}" - name: Handle reboot for transactional update systems when: diff --git a/tasks/main.yml b/tasks/main.yml index c36f0818..5a31ffe0 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -10,6 +10,7 @@ failed_when: false loop: "{{ __firewall_conflicting_services }}" when: firewall_disable_conflicting_services | bool + check_mode: "{{ not __firewall_test_check_mode | d(not ansible_check_mode) }}" - name: Attempt to stop and disable conflicting services service: @@ -17,6 +18,7 @@ state: "{{ 'stopped' if __firewall_is_booted else omit }}" enabled: false loop: "{{ __firewall_conflicting_services_status.results }}" + check_mode: "{{ not __firewall_test_check_mode | d(not ansible_check_mode) }}" when: - firewall_disable_conflicting_services | bool - item.rc == 0 @@ -25,6 +27,7 @@ systemd: name: "{{ __firewall_service }}" masked: false + check_mode: "{{ not __firewall_test_check_mode | d(not ansible_check_mode) }}" when: ansible_facts.service_mgr == "systemd" - name: Enable and start firewalld service @@ -32,167 +35,59 @@ name: "{{ __firewall_service }}" enabled: true state: "{{ 'started' if __firewall_is_booted else omit }}" + check_mode: "{{ not __firewall_test_check_mode | d(not ansible_check_mode) }}" -- name: Check if previous replaced is defined - set_fact: - __firewall_previous_replaced: "{{ firewall | d([]) | - selectattr('previous', 'defined') | - selectattr('previous', 'match', '^replaced$') | - list | length > 0 }}" - __firewall_python_cmd: "{{ ansible_python_interpreter | - d(discovered_interpreter_python) }}" - __firewall_report_changed: true - -- name: Handle previous replaced - when: __firewall_previous_replaced | bool - block: - - name: Get config files, checksums before and remove - script: - cmd: > - files/get_files_checksums.sh {{ __firewall_python_cmd | quote }} - {{ __firewall_firewalld_dir | quote }} - {{ ansible_check_mode | ternary('false', 'true') }} - {{ __firewall_package_with_conf | quote }} - {{ __firewall_usr_lib_dir | quote }} - {{ "booted" if __firewall_is_booted else "" }} - register: __firewall_config_files_before - changed_when: false - check_mode: false - - - name: Tell firewall module it is able to report changed - set_fact: - __firewall_report_changed: "{{ - __firewall_config_files_before.stdout == '' }}" - -# Explanation of loop - `firewall` is a list of dict. We want -# to remove any key: value matching previous: replaced from every -# dict in the firewall list. The `difference` filter removes -# items from lists, not dicts. So we use `map` with `dict2items` -# to convert each dict in the `firewall` list to a list of key/value -# pairs. Then, we use `map` with `difference` to remove all -# occurrences of previous: replaced from all dicts. The `select` -# removes any lists which are now empty (e.g. the item contained -# only previous: replaced). Then we use `map` with `items2dict` -# to convert each item in the list back to a dict. -# Also handle the case where firewall is a dict instead of a list by -# "casting" it into a list. -- name: Configure firewall +# NOTE: The old implementation of this role would always set permanent to True if it was not set, +# and would set runtime to True if it was not set +# so replicate that behavior here in order to maintain backwards compatibility in the module +# in case someone is erroneously using the module directly +- name: Configure firewall and/or gather facts vars: - __previous: - - key: previous - value: replaced - __detailed: - - key: detailed - value: true - - key: detailed - value: false - firewall_lib: - firewalld_conf: "{{ item.firewalld_conf | default(omit) }}" - zone: "{{ item.zone | default(omit) }}" - set_default_zone: "{{ item.set_default_zone | default(omit) }}" - service: "{{ item.service | default(omit) }}" - port: "{{ item.port | default(omit) }}" - source_port: "{{ item.source_port | default(omit) }}" - forward_port: "{{ item.forward_port | default(omit) }}" - masquerade: "{{ item.masquerade | default(omit) }}" - rich_rule: "{{ item.rich_rule | default(omit) }}" - source: "{{ item.source | default(omit) }}" - destination: "{{ item.destination | default(omit) }}" - interface: "{{ item.interface | default(omit) }}" - interface_pci_id: "{{ item.interface_pci_id | default(omit) }}" - icmp_block: "{{ item.icmp_block | default(omit) }}" - icmp_block_inversion: "{{ item.icmp_block_inversion | default(omit) }}" - timeout: "{{ item.timeout | default(omit) }}" - target: "{{ item.target | default(omit) }}" - ipset: "{{ item.ipset | default(omit) }}" - ipset_type: "{{ item.ipset_type | default(omit) }}" - ipset_entries: "{{ item.ipset_entries | default(omit) }}" - ipset_options: "{{ item.ipset_options | default(omit) }}" - short: "{{ item.short | default(omit) }}" - description: "{{ item.description | default(omit) }}" - protocol: "{{ item.protocol | default(omit) }}" - helper_module: "{{ item.helper_module | default(omit) }}" - permanent: "{{ item.permanent | default(True) }}" - runtime: "{{ item.runtime | default(__firewall_is_booted) }}" - state: "{{ item.state | default(omit) }}" - includes: "{{ item.includes | default(omit) }}" - __report_changed: "{{ __firewall_report_changed }}" - online: "{{ __firewall_is_booted }}" - loop: "{{ firewall is mapping | ternary([firewall], firewall) | - map('dict2items') | map('difference', __previous) | - map('difference', __detailed) | select | - map('items2dict') | list }}" - register: firewall_lib_result - -- name: Gather firewall ansible fact with args - vars: - fw: "{{ firewall is mapping | ternary([firewall], firewall) }}" - when: - - firewall != None - - firewall | length == 1 - - "'detailed' in fw[0]" + firewall_config_list: "{{ firewall is mapping | ternary([firewall], firewall) | list + if firewall is not none else [] }}" + firewall_lib_config_list: "{{ firewall_config_list | map('dict2items') | map('rejectattr', 'key', 'match', '^detailed$') | + map('list') | map('items2dict') | list }}" + _detailed: + detailed: true + firewall_detailed_facts: "{{ firewall == _detailed or firewall == [_detailed] }}" block: - - name: Gather firewall config information - vars: - __previous: - - key: previous - value: replaced - firewall_lib_facts: - detailed: "{{ item.detailed | default(False) }}" + - name: Configure firewall + firewall_lib: + config_list: "{{ firewall_lib_config_list }}" online: "{{ __firewall_is_booted }}" - loop: "{{ fw | map('dict2items') | map('difference', __previous) | - select | map('items2dict') | list }}" - register: __firewalld_facts + __called_from_role: true + when: firewall_lib_config_list | length > 0 + register: firewall_lib_result - - name: Update firewalld_config fact - vars: - firewalld_facts: "{{ __firewalld_facts.results[0] }}" - set_fact: - firewall_config: "{{ firewalld_facts.firewall_config }}" - -- name: Gather firewall ansible fact if no args - when: firewall == None or firewall | length == 0 - block: - - name: Gather firewall config if no arguments - firewall_lib_facts: - online: "{{ __firewall_is_booted }}" - detailed: false - register: __firewalld_facts - - - name: Update firewalld_config fact - set_fact: - firewall_config: "{{ __firewalld_facts.firewall_config }}" + - name: Show diffs + debug: + msg: "{{ firewall_lib_result.diff | to_nice_json }}" + when: + - ansible_verbosity > 1 + - firewall_lib_result.diff | d({}) | length > 0 -- name: Check config files after removal - when: - - __firewall_previous_replaced | bool - - not ansible_check_mode - block: - - name: Get config files, checksums after - script: - cmd: > - files/get_files_checksums.sh {{ __firewall_python_cmd | quote }} - {{ __firewall_firewalld_dir | quote }} false - {{ __firewall_package_with_conf | quote }} - {{ __firewall_usr_lib_dir | quote }} - {{ __firewall_is_booted | quote }} - register: __firewall_config_files_after - changed_when: false + # If changed is false, short_circuit should be true - this means the module + # was able to determine that the desired configuration is exactly the same as + # the current configuration and no changes are needed. + - name: Check if short circuit is false + fail: + msg: The role reported no changes but short circuit is false + when: + - not __firewall_debug_short_circuit_disable | d(false) + - __firewall_debug_short_circuit | d(false) + - not firewall_lib_result.changed + - firewall_lib_result.short_circuit is defined + - not firewall_lib_result.short_circuit | bool - - name: Calculate what has changed - set_fact: - firewall_lib_result: - changed: "{{ - __firewall_config_files_after.stdout_lines | - symmetric_difference(__firewall_config_files_before.stdout_lines) | - length > 0 }}" - changed_when: firewall_lib_result.changed + - name: Gather firewall ansible facts if no args + when: firewall_lib_config_list | length == 0 + block: + - name: Gather firewall config if no arguments + firewall_lib_facts: + online: "{{ __firewall_is_booted }}" + detailed: "{{ firewall_detailed_facts }}" + register: __firewalld_facts - - name: Show diffs - when: __firewall_debug | d(false) - debug: - msg: | - __firewall_config_files_before {{ - __firewall_config_files_before.stdout_lines | to_nice_json }} - __firewall_config_files_after {{ - __firewall_config_files_after.stdout_lines | to_nice_json }} + - name: Update firewalld_config fact + set_fact: + firewall_config: "{{ __firewalld_facts.firewall_config }}" diff --git a/tests/library b/tests/library new file mode 120000 index 00000000..53bed968 --- /dev/null +++ b/tests/library @@ -0,0 +1 @@ +../library \ No newline at end of file diff --git a/tests/roles/linux-system-roles.firewall/files b/tests/roles/linux-system-roles.firewall/files deleted file mode 120000 index aa291751..00000000 --- a/tests/roles/linux-system-roles.firewall/files +++ /dev/null @@ -1 +0,0 @@ -../../../files \ No newline at end of file diff --git a/tests/roles/linux-system-roles.firewall/module_utils b/tests/roles/linux-system-roles.firewall/module_utils new file mode 120000 index 00000000..bfe5ddc9 --- /dev/null +++ b/tests/roles/linux-system-roles.firewall/module_utils @@ -0,0 +1 @@ +../../../module_utils \ No newline at end of file diff --git a/tests/tests_firewall_fact.yml b/tests/tests_firewall_fact.yml index 6c2d1218..8907a4fd 100644 --- a/tests/tests_firewall_fact.yml +++ b/tests/tests_firewall_fact.yml @@ -114,62 +114,39 @@ # Test detailed mode - - name: Get firewall_config with detailed on - block: - - name: Run role with detailed on - include_role: - name: linux-system-roles.firewall - vars: - firewall: - detailed: true - rescue: - - name: Record role failure - set_fact: - __details_failed: true - always: - - name: Default to not failed - set_fact: - __details_failed: "{{ __details_failed | default(false) }}" - - # offline mode does not currently have details - - name: Check role failure with details in non-booted mode + - name: Run role with detailed on + include_role: + name: linux-system-roles.firewall + vars: + firewall: + detailed: true + + - name: Fail if default settings values not dictionaries fail: - msg: "Role with details did not fail in non-booted mode" - when: not __firewall_is_booted and not __details_failed + msg: >- + detailed firewall_config.default not formatted properly - + see README.md for expected formatting + when: item is not mapping + loop: "{{ firewall_config.default.values() | list }}" - - name: Test details in booted mode - when: __firewall_is_booted - block: - - name: Assert that role succeeded - assert: - that: not __details_failed - - - name: Fail if default settings values not dictionaries - fail: - msg: >- - detailed firewall_config.default not formatted properly - - see README.md for expected formatting - when: item is not mapping - loop: "{{ firewall_config.default.values() | list }}" - - - name: Fail if custom firewall_config is not its previous value - fail: - msg: custom firewall_config changed unexpectedly - when: firewall_config.custom != __previous_firewall_config.custom - - - name: Fail if default zone differs - fail: - msg: default zone should not have changed - when: firewall_config.default_zone != - __previous_firewall_config.default_zone - - - name: Spot-check default entry details and structure - assert: - that: - - firewall_config.default.helpers.snmp.description is string - - '"reachable" in firewall_config.default.icmptypes["echo-request"].description' - - '"HTTP" in firewall_config.default.services.http.description' - - '"public areas" in firewall_config.default.zones.public.description' + - name: Fail if custom firewall_config is not its previous value + fail: + msg: custom firewall_config changed unexpectedly + when: firewall_config.custom != __previous_firewall_config.custom + + - name: Fail if default zone differs + fail: + msg: default zone should not have changed + when: firewall_config.default_zone != + __previous_firewall_config.default_zone + + - name: Spot-check default entry details and structure + assert: + that: + - firewall_config.default.helpers.snmp.module is string + - '"reachable" in firewall_config.default.icmptypes["echo-request"].description' + - '"HTTP" in firewall_config.default.services.http.description' + - '"public areas" in firewall_config.default.zones.public.description' always: - name: Cleanup diff --git a/tests/tests_firewalld_conf.yml b/tests/tests_firewalld_conf.yml index df5fd034..4ed15ac7 100644 --- a/tests/tests_firewalld_conf.yml +++ b/tests/tests_firewalld_conf.yml @@ -27,7 +27,6 @@ firewall: firewalld_conf: allow_zone_drifting: false - permanent: true - name: Check if AllowZoneDrifting is disabled if possible command: grep -Fx AllowZoneDrifting=no /etc/firewalld/firewalld.conf @@ -49,7 +48,6 @@ firewall: firewalld_conf: allow_zone_drifting: true - permanent: true - name: Check if AllowZoneDrifting is enabled if possible command: grep -Fx AllowZoneDrifting=yes /etc/firewalld/firewalld.conf diff --git a/tests/tests_forward_port.yml b/tests/tests_forward_port.yml index 914d2ab9..14fba020 100644 --- a/tests/tests_forward_port.yml +++ b/tests/tests_forward_port.yml @@ -1,8 +1,27 @@ --- - name: Test forward port hosts: all - become: true - + vars: + _test_settings_strings: + - forward_port: 40/tcp;8080;0.0.0.0 + state: "{{ test_state }}" + - forward_port: 50/tcp;8080;0.0.0.0 + state: "{{ test_state }}" + _test_settings_dicts: + - forward_port: + port: 40 + proto: tcp + toport: 8080 + toaddr: 0.0.0.0 + state: "{{ test_state }}" + - forward_port: + port: 50 + proto: tcp + toport: 8080 + toaddr: 0.0.0.0 + state: "{{ test_state }}" + previous_replaced: + - {previous: replaced} tasks: - name: Clean slate include_role: @@ -13,30 +32,134 @@ # Tests string and dict form types - - name: Test forward_port and undo (string) + - name: Test forward_port strings include_role: name: linux-system-roles.firewall vars: - firewall: - - forward_port: 40/tcp;8080;0.0.0.0 - state: enabled - - forward_port: 40/tcp;8080;0.0.0.0 - state: disabled + firewall: "{{ _test_settings_strings }}" + test_state: enabled - - name: Test forward_port (dict) and undo + - name: Verify forward_port strings + fail: + when: firewall_lib_result is failed or firewall_lib_result is not changed + + - name: Test forward_port strings idempotence include_role: name: linux-system-roles.firewall vars: - firewall: - - forward_port: - port: 40 - proto: tcp - toport: 8080 - toaddr: 0.0.0.0 - state: enabled - - forward_port: - - port: 40 - proto: tcp - toport: 8080 - toaddr: 0.0.0.0 - state: disabled + firewall: "{{ _test_settings_strings }}" + test_state: enabled + + - name: Verify forward_port strings idempotence + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port strings idempotence with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_strings + previous_replaced }}" + test_state: enabled + + - name: Verify forward_port strings idempotence with previous replaced + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port strings undo + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_strings }}" + test_state: disabled + + - name: Verify forward_port strings undo + fail: + when: firewall_lib_result is failed or firewall_lib_result is not changed + + - name: Test forward_port strings undo idempotence + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_strings }}" + test_state: disabled + + - name: Verify forward_port strings undo idempotence + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port strings undo idempotence with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_strings + previous_replaced }}" + test_state: disabled + + - name: Verify forward_port strings undo idempotence with previous replaced + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port dicts + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_dicts }}" + test_state: enabled + + - name: Verify forward_port dicts + fail: + when: firewall_lib_result is failed or firewall_lib_result is not changed + + - name: Test forward_port dicts idempotence + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_dicts }}" + test_state: enabled + + - name: Verify forward_port dicts idempotence + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port dicts idempotence with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_dicts + previous_replaced }}" + test_state: enabled + + - name: Verify forward_port dicts idempotence with previous replaced + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port dicts undo + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_dicts }}" + test_state: disabled + + - name: Verify forward_port dicts undo + fail: + when: firewall_lib_result is failed or firewall_lib_result is not changed + + - name: Test forward_port dicts undo idempotence + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_dicts }}" + test_state: disabled + + - name: Verify forward_port dicts undo idempotence + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed + + - name: Test forward_port dicts undo idempotence with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_settings_dicts + previous_replaced }}" + test_state: disabled + + - name: Verify forward_port dicts undo idempotence with previous replaced + fail: + when: firewall_lib_result is failed or firewall_lib_result is changed diff --git a/tests/tests_interface_pci.yml b/tests/tests_interface_pci.yml index 0b006ce0..90fb02e0 100644 --- a/tests/tests_interface_pci.yml +++ b/tests/tests_interface_pci.yml @@ -9,7 +9,11 @@ become: true roles: - linux-system-roles.firewall - + vars: + # we don't want the test short circuit assertion to be used in this test + # because we will check it manually below and we don't want a bunch of + # block/rescue + __firewall_debug_short_circuit_disable: true tasks: - name: Set expected backend set_fact: @@ -67,7 +71,46 @@ register: pci_id changed_when: false - - name: Add pci device ethernet controller + - name: Handle missing NetworkManager + block: + - name: Add pci device ethernet controller + include_role: + name: linux-system-roles.firewall + vars: + firewall: + zone: internal + interface_pci_id: "{{ pci_id.stdout }}" + state: enabled + rescue: + - name: See if the error is because NetworkManager is not installed + fail: + msg: "{{ ansible_failed_result | to_nice_json }}" + when: not ansible_failed_result.msg | regex_search('interface_pci_id is only supported with NetworkManager') + + - name: Install NetworkManager + package: + name: NetworkManager + state: present + + - name: Start NetworkManager service + service: + name: NetworkManager + state: started + + - name: Add pci device ethernet controller again with NetworkManager installed + include_role: + name: linux-system-roles.firewall + vars: + firewall: + zone: internal + interface_pci_id: "{{ pci_id.stdout }}" + state: enabled + + - name: Confirm changes were made + assert: + that: firewall_lib_result is changed + + - name: Add pci device again include_role: name: linux-system-roles.firewall vars: @@ -75,17 +118,28 @@ zone: internal interface_pci_id: "{{ pci_id.stdout }}" state: enabled - permanent: true - - name: Add pci device again + - name: Confirm changes were not made and short circuit is false + assert: + that: + - not firewall_lib_result is changed + - not firewall_lib_result.short_circuit + + - name: Add pci device again with previous replaced include_role: name: linux-system-roles.firewall vars: firewall: - zone: internal - interface_pci_id: "{{ pci_id.stdout }}" - state: enabled - permanent: true + - previous: replaced + - zone: internal + interface_pci_id: "{{ pci_id.stdout }}" + state: enabled + + - name: Confirm changes were not made and short circuit is false + assert: + that: + - not firewall_lib_result is changed + - not firewall_lib_result.short_circuit - name: Debug - get nftables ruleset after and show diff shell: | @@ -129,7 +183,42 @@ zone: internal interface_pci_id: "{{ pci_id.stdout }}" state: disabled - permanent: true + + - name: Confirm removal was made + assert: + that: firewall_lib_result is changed + + - name: Remove interface from internal again + include_role: + name: linux-system-roles.firewall + vars: + firewall: + zone: internal + interface_pci_id: "{{ pci_id.stdout }}" + state: disabled + + - name: Confirm no removal changes were made + assert: + that: + - not firewall_lib_result is changed + - not firewall_lib_result.short_circuit + + - name: Remove interface from internal with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - zone: internal + interface_pci_id: "{{ pci_id.stdout }}" + state: disabled + + - name: Confirm no removal changes were made with previous replaced + assert: + that: + - not firewall_lib_result is changed + - not firewall_lib_result.short_circuit + always: - name: Cleanup tags: diff --git a/tests/tests_ipsets.yml b/tests/tests_ipsets.yml index b12ff678..9b609033 100644 --- a/tests/tests_ipsets.yml +++ b/tests/tests_ipsets.yml @@ -1,7 +1,32 @@ --- - name: Test firewall user defined ipsets hosts: all - become: true + vars: + previous_replaced: + - {previous: replaced} + _test_ipsets: + - ipset: customipset-ipv4 + ipset_type: "hash:ip" + ipset_entries: + - 127.0.0.1 + - 8.8.8.8 + ipset_options: + hashsize: 120 + maxelem: 1000 + short: Custom + description: Custom IPSet for testing purposes + state: present + - ipset: customipset-ipv6 + ipset_type: "hash:ip" + ipset_entries: + - 2001:4860:4860::8844 + - 2001:4860:4860::8888 + ipset_options: + hashsize: 120 + maxelem: 1000 + short: Custom + description: Custom IPSet for testing purposes + state: present tasks: - name: Test firewall user defined ipsets block: @@ -16,59 +41,26 @@ firewall: - previous: replaced - # Verify customipset-ipv4 is not already defined - - name: Get all ipsets - shell: - executable: /bin/bash - cmd: | - set -euo pipefail - firewall-offline-cmd --get-ipsets | grep customipset-ipv4 + # Verify customipset-ipv4 and ipv6 ipsets are not already defined + - name: Check for customipset-ipv4 and ipv6 ipsets + command: firewall-offline-cmd --get-ipsets register: result changed_when: false - failed_when: result.rc != 1 + failed_when: '"customipset-ipv4" in result.stdout or "customipset-ipv6" in result.stdout' # Begin custom ipset tests - - name: Define new ipset + - name: Define new ipsets include_role: name: linux-system-roles.firewall vars: - firewall: - - ipset: customipset-ipv4 - ipset_type: "hash:ip" - ipset_entries: - - 127.0.0.1 - - 8.8.8.8 - ipset_options: - hashsize: 120 - maxelem: 1000 - short: Custom - description: Custom IPSet for testing purposes - state: present - permanent: true - - ipset: customipset-ipv6 - ipset_type: "hash:ip" - ipset_entries: - - 2001:4860:4860::8844 - - 2001:4860:4860::8888 - ipset_options: - hashsize: 120 - maxelem: 1000 - short: Custom - description: Custom IPSet for testing purposes - state: present - permanent: true + firewall: "{{ _test_ipsets }}" - name: Fail if ipsets not added - shell: - executable: /bin/bash - cmd: | - set -euo pipefail - firewall-offline-cmd --get-ipsets | grep {{ item | quote }} - loop: - - customipset-ipv4 - - customipset-ipv6 + command: firewall-offline-cmd --get-ipsets + register: result changed_when: false + failed_when: '"customipset-ipv4" not in result.stdout or "customipset-ipv6" not in result.stdout' - name: Fail if entry not added to ipset command: firewall-offline-cmd --ipset {{ item.name | quote }} --query-entry {{ item.entry | quote }} @@ -103,7 +95,6 @@ short: Custom description: Custom IPSet for testing purposes state: present - permanent: true ipset_options: hashsize: 120 maxelem: 1000 @@ -111,7 +102,6 @@ short: Custom description: Custom IPSet for testing purposes state: present - permanent: true ipset_options: hashsize: 120 maxelem: 1000 @@ -121,6 +111,17 @@ msg: Defining ipsets is not idempotent when: firewall_lib_result is changed # noqa no-handler + - name: Redefine new ipsets with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ _test_ipsets + previous_replaced }}" + + - name: Fail if defining ipset not idempotent with previous replaced + fail: + msg: Defining ipsets is not idempotent + when: firewall_lib_result is changed # noqa no-handler + # modify ipset options not supported in container or image build - name: Modify ipset options when: __firewall_is_booted | d(true) @@ -182,7 +183,6 @@ hashsize: null maxelem: null state: absent - permanent: true - ipset: customipset-ipv6 ipset_entries: - 2001:4860:4860::8844 @@ -191,7 +191,6 @@ hashsize: null maxelem: null state: absent - permanent: true - name: Fail if ipsets removed shell: @@ -240,12 +239,12 @@ short: CustomChanged description: Custom IPSet for testing purposes (changed) state: present - permanent: true + runtime: false - ipset: customipset-ipv6 short: CustomChanged description: Custom IPSet for testing purposes (changed) state: present - permanent: true + runtime: false - name: Verify changes command: "{{ item['command'] }}" @@ -274,10 +273,10 @@ firewall: - source: "ipset:customipset-ipv4" state: enabled - runtime: true + permanent: false - source: "ipset:customipset-ipv6" state: enabled - runtime: true + permanent: false when: __firewall_is_booted - name: Add ipsets to default zone again (runtime) @@ -287,10 +286,10 @@ firewall: - source: "ipset:customipset-ipv4" state: enabled - runtime: true + permanent: false - source: "ipset:customipset-ipv6" state: enabled - runtime: true + permanent: false when: __firewall_is_booted - name: Fail if adding ipsets is not idempotent (runtime) @@ -307,10 +306,15 @@ firewall: - source: "ipset:customipset-ipv4" state: enabled - permanent: true + runtime: false - source: "ipset:customipset-ipv6" state: enabled - permanent: true + runtime: false + + - name: Fail if adding ipsets reports no changes + fail: + msg: "enabling ipsets in zones reports no changes" + when: firewall_lib_result is not changed # noqa no-handler - name: Add ipsets to default zone again (permanent) include_role: @@ -319,10 +323,10 @@ firewall: - source: "ipset:customipset-ipv4" state: enabled - permanent: true + runtime: false - source: "ipset:customipset-ipv6" state: enabled - permanent: true + runtime: false - name: Fail if adding ipsets is not idempotent (permanent) fail: @@ -336,10 +340,13 @@ firewall: - source: "ipset:customipset-ipv4" state: disabled - permanent: true - source: "ipset:customipset-ipv6" state: disabled - permanent: true + + - name: Fail if removing ipsets reports no changes + fail: + msg: "removing ipsets in zones reports no changes" + when: firewall_lib_result is not changed # noqa no-handler - name: Remove custom ipsets include_role: @@ -348,10 +355,8 @@ firewall: - ipset: customipset-ipv4 state: absent - permanent: true - ipset: customipset-ipv6 state: absent - permanent: true - name: Fail if ipsets not removed shell: @@ -373,16 +378,30 @@ firewall: - ipset: customipset-ipv4 state: absent - permanent: true - ipset: customipset-ipv6 state: absent - permanent: true - name: Fail if not idempotent fail: msg: Removing ipsets is not idempotent when: firewall_lib_result is changed # noqa no-handler + - name: Remove custom ipsets again with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - ipset: customipset-ipv4 + state: absent + - ipset: customipset-ipv6 + state: absent + + - name: Fail if not idempotent with previous replaced + fail: + msg: Removing ipsets is not idempotent with previous replaced + when: firewall_lib_result is changed # noqa no-handler + - name: Test ipset with wrong family block: - name: Test ipset with wrong family (ipv4) @@ -397,7 +416,6 @@ ipset_options: family: inet6 state: present - permanent: true - name: This task should not be reached fail: @@ -410,8 +428,7 @@ expected_error_message: >- ipset badipset-v4: family=inet6 is not supported for IPv4 ipset_entries 127.0.0.1, 8.8.8.8 - when: ansible_failed_result.results | - selectattr("msg", "contains", expected_error_message) | list | length == 0 + when: expected_error_message not in ansible_failed_result.msg - name: Test ipset with wrong family (ipv6) block: @@ -427,7 +444,6 @@ ipset_options: family: inet state: present - permanent: true - name: This task should not be reached fail: @@ -440,8 +456,7 @@ expected_error_message: >- ipset badipset-v6: family=inet is not supported for IPv6 ipset_entries 2001:4860:4860::8844, 2001:4860:4860::8888 - when: ansible_failed_result.results | - selectattr("msg", "contains", expected_error_message) | list | length == 0 + when: expected_error_message not in ansible_failed_result.msg always: - name: Cleanup tags: diff --git a/tests/tests_purge_config.yml b/tests/tests_purge_config.yml index 73bd5059..3f49a7d6 100644 --- a/tests/tests_purge_config.yml +++ b/tests/tests_purge_config.yml @@ -7,13 +7,13 @@ state: present - set_default_zone: customzone state: enabled - - service: [tftp, ftp] + - service: [tftp, ftp, ssh] port: ['443/tcp', '443/udp'] forward_port: ['447/tcp;;1.2.3.4', '448/tcp;;1.2.3.5'] state: enabled - zone: internal - service: [tftp, ftp] + service: [tftp, ftp, ssh] port: ['443/tcp', '443/udp'] forward_port: ['447/tcp;;1.2.3.4', '448/tcp;;1.2.3.5'] @@ -73,6 +73,17 @@ msg: The role reported changes when: firewall_lib_result.changed # noqa no-handler + - name: Get config + include_role: + name: linux-system-roles.firewall + vars: + firewall: + detailed: true + + - name: Show config + debug: + msg: facts {{ firewall_config | to_nice_json }} + - name: Use again previous replaced and basic config include_role: name: linux-system-roles.firewall diff --git a/tests/tests_service.yml b/tests/tests_service.yml index a87a0294..e4975fed 100644 --- a/tests/tests_service.yml +++ b/tests/tests_service.yml @@ -2,6 +2,57 @@ - name: Test firewall user defined services hosts: all become: true + vars: + default_http_service: + service: http + short: "WWW (HTTP)" + description: >- + HTTP is the protocol used to serve Web pages. + If you plan to make your Web server publicly available, + enable this option. + This option is not required for + viewing pages locally or developing Web pages. + port: ["80/tcp"] + state: present + custom_http_service: + service: http + description: "linux system role test description" + port: ["8080/tcp"] + destination: + - 1.1.1.1 + - 1::1 + includes: "{{ ['ssh', 'ldaps'] + if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] + and ansible_distribution_major_version is version('8', '>=') + else [] }}" + state: present + custom_lsr_service: + service: systemroletest + short: Test service (Ansible) + description: >- + This should only be available + when firewalld is being configured + by the firewall linux system role + port: ["9999/udp"] + source_port: ["10000/udp"] + destination: + - 1.1.1.1 + - 1::1 + protocol: icmp + # these two don't exist yet in RHEL 7 + helper_module: "{{ 'ftp' + if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] + and ansible_distribution_major_version is version('8', '>=') + else omit }}" + includes: "{{ ['ssh', 'ldaps'] + if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] + and ansible_distribution_major_version is version('8', '>=') + else [] }}" + state: present + service_removal: # to be combine()'d with service definition + state: absent + description: "{{ omit }}" + short: "{{ omit }}" tasks: - name: Test firewall user defined services block: @@ -38,23 +89,12 @@ include_role: name: linux-system-roles.firewall vars: - firewall: - - service: http - short: "WWW (HTTP)" - description: >- - HTTP is the protocol used to serve Web pages. - If you plan to make your Web server publicly available, - enable this option. - This option is not required for - viewing pages locally or developing Web pages. - port: ["80/tcp"] - permanent: true - state: present + firewall: "{{ [default_http_service] }}" - name: Fail if service changed fail: msg: FAILED - Defaults not properly reset for services - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler ## Begin builtin service tests @@ -62,47 +102,57 @@ include_role: name: linux-system-roles.firewall vars: - firewall: - - service: http - description: "linux system role test description" - port: ["8080/tcp"] - destination: - - 1.1.1.1 - - 1::1 - includes: "{{ ['ssh', 'ldaps'] - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else [] }}" - permanent: true - state: present + firewall: "{{ [custom_http_service] }}" - name: Fail when http is unchanged fail: msg: FAILED - http unchanged - when: not firewall_lib_result.changed # noqa no-handler + when: not firewall_lib_result is changed # noqa no-handler - name: Change default service http again include_role: name: linux-system-roles.firewall vars: - firewall: - - service: http - description: "linux system role test description" - port: ["8080/tcp"] - destination: - - 1.1.1.1 - - 1::1 - includes: "{{ ['ssh', 'ldaps'] - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else [] }}" - permanent: true - state: present - - - name: Fail when http changes affect permanent configuration + firewall: "{{ [custom_http_service] }}" + + - name: Fail when http changes are not idempotent fail: msg: http changed when reapplying the same changes - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler + + - name: Change default service http again with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ [{'previous': 'replaced'}] + [custom_http_service] }}" + + - name: Fail when http changes are not idempotent with previous replaced + fail: + msg: http changed when reapplying the same changes with previous replaced + when: firewall_lib_result is changed # noqa no-handler + + - name: Remove http service changes + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ [custom_http_service | combine(service_removal)] }}" + + - name: Fail when http is unchanged after removal + fail: + msg: FAILED - http unchanged after removal + when: not firewall_lib_result is changed # noqa no-handler + + - name: Revert all changes + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + + - name: Fail if not changed + fail: + msg: FAILED - Defaults not properly reset for services + when: not firewall_lib_result is changed # noqa no-handler ## End builtin service tests @@ -112,30 +162,7 @@ include_role: name: linux-system-roles.firewall vars: - firewall: - - service: systemroletest - short: Test service (Ansible) - description: >- - This should only be available - when firewalld is being configured - by the firewall linux system role - port: ["9999/udp"] - source_port: ["10000/udp"] - destination: - - 1.1.1.1 - - 1::1 - protocol: icmp - # these two don't exist yet in RHEL 7 - helper_module: "{{ 'ftp' - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else omit }}" - includes: "{{ ['ssh', 'ldaps'] - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else [] }}" - permanent: true - state: present + firewall: "{{ [custom_lsr_service] }}" # Verify that custom service is added to permanent configuration @@ -144,15 +171,15 @@ register: result changed_when: false - - name: Fail if systemrole test not created + - name: Fail if systemroletest not created fail: - msg: system role test not created by role + msg: systemroletest service not created by role when: result.stdout.find("systemroletest") == -1 - name: Fail when custom service not changed fail: msg: New custom service did not set changed flag - when: not firewall_lib_result.changed # noqa no-handler + when: not firewall_lib_result is changed # noqa no-handler # Verify idempotency @@ -160,55 +187,36 @@ include_role: name: linux-system-roles.firewall vars: - firewall: - - service: systemroletest - short: Test service (Ansible) - description: >- - This should only be available when - firewalld is being configured by the - firewall linux system role - port: 9999/udp - source_port: ["10000/udp"] - destination: - - 1.1.1.1 - - 1::1 - protocol: icmp - # these two don't exist yet in RHEL 7 - helper_module: "{{ 'ftp' - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else omit }}" - includes: "{{ ['ssh', 'ldaps'] - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else [] }}" - permanent: true - state: present + firewall: "{{ [custom_lsr_service] }}" - name: Fail when custom service changed fail: msg: Custom service changed when it should not have - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler + + - name: Add same custom service again with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ [{'previous': 'replaced'}] + [custom_lsr_service] }}" + + - name: Fail when custom service changed with previous replaced + fail: + msg: Custom service changed when it should not have with previous replaced + when: firewall_lib_result is changed # noqa no-handler + + - name: Get all services again + command: firewall-offline-cmd --get-services + register: result + changed_when: false # Verify that custom service not a part of default zone for some reason - name: Query if default zone has custom service added command: firewall-offline-cmd --query-service systemroletest - ignore_errors: true # noqa ignore-errors register: result changed_when: false - - - name: Fail if query return code incorrect - fail: - msg: query should return code 1 - when: result.rc != 1 - - - name: Fail if custom service in default zone - fail: - msg: >- - custom zone should not already in default zone - without being added - when: result.stdout.find("yes") != -1 + failed_when: result is success or "no" not in result.stdout # Test that service can be added to zones @@ -226,11 +234,22 @@ command: firewall-offline-cmd --query-service systemroletest register: result changed_when: false + failed_when: not result is success or "yes" not in result.stdout + + - name: Add custom service to default zone again with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - "{{ custom_lsr_service }}" + - service: systemroletest + state: enabled - - name: Fail if service not added to default zone + - name: Verify that no changes are made with previous replaced fail: - msg: custom service should be in default zone - when: result.stdout.find("yes") == -1 + msg: Custom service changed when it should not have with previous replaced + when: firewall_lib_result is changed # noqa no-handler # Remove custom service from default zone to prevent errors later @@ -242,77 +261,56 @@ - service: systemroletest state: disabled - # Test removing elements from custom service + - name: Verify that custom service is removed from default zone + command: firewall-offline-cmd --query-service systemroletest + register: result + changed_when: false + failed_when: result is success or "no" not in result.stdout - - name: Remove custom service elements + - name: Remove custom service from default zone again to check idempotency include_role: name: linux-system-roles.firewall vars: firewall: - service: systemroletest - port: ["9999/udp"] - source_port: ["10000/udp"] - destination: - - 1.1.1.1 - - 1::1 - protocol: icmp - helper_module: "{{ 'ftp' - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else omit }}" - includes: "{{ ['ssh', 'ldaps'] - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else [] }}" - permanent: true - state: absent + state: disabled + + - name: Verify that no changes are made removing service again + fail: + msg: Custom service changed when it should not have + when: firewall_lib_result is changed # noqa no-handler + + # Test removing elements from custom service + + - name: Remove custom service elements + include_role: + name: linux-system-roles.firewall + vars: + firewall: "{{ [custom_lsr_service | combine(service_removal)] }}" # Verify nothing is removed in this case - name: Fail if custom service elements unchanged fail: msg: Custom service unchanged (should not be) - when: not firewall_lib_result.changed # noqa no-handler + when: not firewall_lib_result is changed # noqa no-handler - name: Remove custom service elements again include_role: name: linux-system-roles.firewall vars: - firewall: - - service: systemroletest - port: ["9999/udp"] - source_port: ["10000/udp"] - destination: - - 1.1.1.1 - - 1::1 - protocol: icmp - helper_module: "{{ 'ftp' - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else omit }}" - includes: "{{ ['ssh', 'ldaps'] - if ansible_distribution in ['RedHat', 'CentOS', 'Fedora'] - and ansible_distribution_major_version is version('8', '>=') - else [] }}" - permanent: true - state: absent + firewall: "{{ [custom_lsr_service | combine(service_removal)] }}" - name: Fail if custom service elements changed fail: msg: Custom service changed when it should not have - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler - - name: Get all services + - name: Check if custom service still defined command: firewall-offline-cmd --get-services register: result changed_when: false - - - name: Fail if systemroletest removed - fail: - msg: >- - custom service incorrectly removed when removing - elements from service - when: result.stdout.find("systemroletest") == -1 + failed_when: result.stdout.find("systemroletest") == -1 # Test remove service @@ -322,7 +320,6 @@ vars: firewall: - service: systemroletest - permanent: true state: absent - name: Get all services @@ -341,13 +338,26 @@ vars: firewall: - service: systemroletest - permanent: true state: absent - name: Fail if second removal changes anything fail: msg: something changed when systemrole test removed - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler + + - name: Remove custom service again with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - service: systemroletest + state: absent + + - name: Fail if second removal changes anything with previous replaced + fail: + msg: something changed when systemrole test removed with previous replaced + when: firewall_lib_result is changed # noqa no-handler - name: Try to add a service that does not exist in check mode check_mode: true @@ -360,14 +370,33 @@ - service: http-alt state: enabled zone: public - permanent: true - name: Fail if enabling service in check mode reports changes fail: msg: >- enabling/disabling non-existent service in check mode resulted in changes - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler + + - name: Try to add a service that does not exist in check mode with previous replaced + check_mode: true + block: + - name: Attempt to enable http-alt, which does not exist + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - service: http-alt + state: enabled + zone: public + + - name: Fail if enabling service in check mode reports changes with previous replaced + fail: + msg: >- + enabling/disabling non-existent service in check mode with previous replaced + resulted in changes + when: firewall_lib_result is changed # noqa no-handler always: - name: Cleanup diff --git a/tests/tests_target.yml b/tests/tests_target.yml index 12073b29..ab0fb551 100644 --- a/tests/tests_target.yml +++ b/tests/tests_target.yml @@ -4,16 +4,21 @@ tasks: - name: Run target tests block: + - name: Ensure starting from clean system + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - name: Call role to change target settings include_role: name: linux-system-roles.firewall vars: firewall: - set_default_zone: public - permanent: true - target: DROP state: enabled - permanent: true - name: Get target setting command: firewall-offline-cmd --info-zone=public @@ -26,6 +31,21 @@ vars: __expected: " target: DROP" + - name: Call role to change target settings with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - set_default_zone: public + - target: DROP + state: enabled + + - name: Verify target setting with previous replaced + fail: + msg: Target setting changed when it should not have with previous replaced + when: firewall_lib_result is changed # noqa no-handler + - name: Call role to reset target settings include_role: name: linux-system-roles.firewall @@ -33,7 +53,6 @@ firewall: target: DROP state: absent - permanent: true - name: Get target setting command: firewall-offline-cmd --info-zone=public @@ -45,6 +64,21 @@ that: __expected not in __result.stdout_lines vars: __expected: " target: DROP" + + - name: Call role to reset target settings with previous replaced + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced + - target: DROP + state: absent + + - name: Verify target setting was reset with previous replaced + fail: + msg: Target setting changed when it should not have with previous replaced + when: firewall_lib_result is changed # noqa no-handler + always: - name: Cleanup tags: diff --git a/tests/tests_zone.yml b/tests/tests_zone.yml index 5a4b40f6..d257a7ba 100644 --- a/tests/tests_zone.yml +++ b/tests/tests_zone.yml @@ -27,7 +27,7 @@ - name: Fail on missing zones fail: msg: "Required zones do not exist" - when: firewall_lib_result.changed # noqa no-handler + when: firewall_lib_result is changed # noqa no-handler # ENSURE STATE @@ -50,10 +50,8 @@ '448/tcp;;1.2.3.5'] state: 'enabled' - zone: customzone - permanent: true state: present - zone: customzone - permanent: true masquerade: true state: enabled - set_default_zone: dmz @@ -68,7 +66,6 @@ short: Custom description: Custom IPSet for testing purposes state: present - permanent: true - ipset: customipset-ipv6 ipset_type: "hash:ip" ipset_entries: @@ -76,7 +73,6 @@ short: Custom description: Custom IPSet for testing purposes state: present - permanent: true when: not __bootc_validation | d(false) - name: Fail if no changes are done @@ -84,7 +80,7 @@ msg: "FAILED" when: - not __bootc_validation | d(false) - - not firewall_lib_result.changed + - not firewall_lib_result is changed # ENSURE STATE AGAIN @@ -107,10 +103,8 @@ '448/tcp;;1.2.3.5'] state: 'enabled' - zone: customzone - permanent: true state: present - zone: customzone - permanent: true masquerade: true state: enabled - set_default_zone: dmz @@ -125,7 +119,6 @@ short: Custom description: Custom IPSet for testing purposes state: present - permanent: true - ipset: customipset-ipv6 ipset_type: "hash:ip" ipset_entries: @@ -133,7 +126,6 @@ short: Custom description: Custom IPSet for testing purposes state: present - permanent: true when: not __bootc_validation | d(false) - name: Fail on newly changes @@ -141,7 +133,7 @@ msg: "FAILED" when: - not __bootc_validation | d(false) - - firewall_lib_result.changed # noqa no-handler + - firewall_lib_result is changed # noqa no-handler - name: Create QEMU deployment during bootc end-to-end test delegate_to: localhost @@ -221,27 +213,10 @@ always: - name: Cleanup - when: not __bootc_validation | d(false) tags: - tests::cleanup - block: - # CLEANUP: RESET TO ZONE DEFAULTS - - - name: Remove customzone zone - command: firewall-offline-cmd --delete-zone=customzone - register: result - failed_when: result.failed and "INVALID_ZONE" not in result.stderr - changed_when: false - - - name: Reset to zone defaults - shell: - cmd: | - firewall-offline-cmd --load-zone-defaults=internal - firewall-offline-cmd --load-zone-defaults=external - firewall-offline-cmd --load-zone-defaults=trusted || true - changed_when: false - - - name: Reload firewall - when: __firewall_is_booted - command: firewall-cmd --reload - changed_when: false + include_role: + name: linux-system-roles.firewall + vars: + firewall: + - previous: replaced diff --git a/tests/unit/test_firewall_lib.py b/tests/unit/test_firewall_lib.py index e16b7498..36e5d354 100644 --- a/tests/unit/test_firewall_lib.py +++ b/tests/unit/test_firewall_lib.py @@ -564,6 +564,26 @@ def test_try_get_connection_of_interface(self, nm_get_connection_of_interface): assert result is None + @patch("firewall_lib.NM_IMPORTED", True) + @patch("firewall_lib.AnsibleModule", new_callable=MockAnsibleModule) + @patch("firewall_lib.HAS_FIREWALLD", True) + @patch("firewall_lib.pci_ids", {"600D:7C1D": ["eth0"]}) + @patch("firewall_lib.get_interface_pci", create=True, return_value={}) + def test_parse_pci_id(self, get_interface_pci, am_class): + am = am_class.return_value + + am.params = {"interface_pci_id": ["123G:1111"]} + with self.assertRaises(MockException): + firewall_lib.main() + am.fail_json.assert_called_with( + msg="PCI id 123G:1111 does not match format: XXXX:XXXX (X = hexadecimal number)" + ) + + am.params = {"interface_pci_id": ["600D:7C1D"]} + with self.assertRaises(MockException): + firewall_lib.main() + am.fail_json.assert_called_with(msg="Options invalid without state option set") + @patch("firewall_lib.NM_IMPORTED", True) @patch("firewall_lib.try_get_connection_of_interface") @patch("firewall_lib.nm_get_zone_of_connection", create=True, return_value="") @@ -659,24 +679,6 @@ def test_parse_forward_port(self): rc = firewall_lib.parse_forward_port(module, item) self.assertEqual(("a", "b", None, None), rc) - @patch("firewall_lib.AnsibleModule", new_callable=MockAnsibleModule) - @patch("firewall_lib.HAS_FIREWALLD", True) - @patch("firewall_lib.pci_ids", {"600D:7C1D": ["eth0"]}) - def test_parse_pci_id(self, am_class): - am = am_class.return_value - - am.params = {"interface_pci_id": ["123G:1111"]} - with self.assertRaises(MockException): - firewall_lib.main() - am.fail_json.assert_called_with( - msg="PCI id 123G:1111 does not match format: XXXX:XXXX (X = hexadecimal number)" - ) - - am.params = {"interface_pci_id": ["600D:7C1D"]} - with self.assertRaises(MockException): - firewall_lib.main() - am.fail_json.assert_called_with(msg="Options invalid without state option set") - @patch("firewall_lib.AnsibleModule", new_callable=MockAnsibleModule) class FirewallLibMain(unittest.TestCase): @@ -951,7 +953,9 @@ def test_allow_zone_drifting_deprecated(self, firewall_class, am_class): am.warn.assert_called_with( "AllowZoneDrifting is deprecated in this version of firewalld and no longer supported" ) - am.exit_json.assert_called_with(changed=False, __firewall_changed=False) + am.exit_json.assert_called_with( + changed=False, __firewall_changed=False, short_circuit=False + ) @patch("firewall_lib.HAS_FIREWALLD", True) @patch("firewall_lib.FW_VERSION", "0.9.0", create=True) @@ -1003,7 +1007,9 @@ def test_nm_integration_interfaces( try_set_zone_of_interface.return_value = rv fw_settings.queryService.return_value = state == "disabled" firewall_lib.main() - am.exit_json.assert_called_with(changed=rv[1], __firewall_changed=rv[1]) + am.exit_json.assert_called_with( + changed=rv[1], __firewall_changed=rv[1], short_circuit=False + ) for state in ["enabled", "disabled"]: am.params["state"] = state @@ -1012,7 +1018,9 @@ def test_nm_integration_interfaces( try_set_zone_of_interface.return_value = rv fw_settings.queryService.return_value = state == "disabled" firewall_lib.main() - am.exit_json.assert_called_with(changed=True, __firewall_changed=True) + am.exit_json.assert_called_with( + changed=True, __firewall_changed=True, short_circuit=False + ) @patch("firewall_lib.HAS_FIREWALLD", True) @patch("firewall_lib.FW_VERSION", "0.9.0", create=True) @@ -1464,6 +1472,8 @@ def test_module_parameters(method, state, input, expected, _offline_cmd): fw_client = fw_client_patcher.start() has_fw_patcher = patch("firewall_lib.HAS_FIREWALLD", True) has_fw_patcher.start() + has_nm_patcher = patch("firewall_lib.NM_IMPORTED", True) + has_nm_patcher.start() fw_ver_patcher = patch("firewall_lib.FW_VERSION", "0.3.8", create=True) fw_ver_patcher.start() rich_rule_patcher = patch("firewall_lib.Rich_Rule", create=True) @@ -1534,7 +1544,9 @@ def test_module_parameters(method, state, input, expected, _offline_cmd): if permanent: called_mock = getattr(fw_settings, called_mock_name) assert expected["permanent"] == called_mock.call_args_list - am.exit_json.assert_called_once_with(changed=True, __firewall_changed=True) + am.exit_json.assert_called_once_with( + changed=True, __firewall_changed=True, short_circuit=False + ) finally: am_class_patcher.stop() fw_client_patcher.stop() @@ -1614,7 +1626,9 @@ def mock_run_command(args, check_rc=False): else: firewall_lib.main() assert called_cmds == expected["offline"] - am.exit_json.assert_called_once_with(changed=True, __firewall_changed=True) + am.exit_json.assert_called_once_with( + changed=True, __firewall_changed=True, short_circuit=False + ) finally: am_class_patcher.stop() has_fw_patcher.stop() diff --git a/tests/unit/test_get_config.py b/tests/unit/test_get_config.py new file mode 100644 index 00000000..d99f733a --- /dev/null +++ b/tests/unit/test_get_config.py @@ -0,0 +1,469 @@ +# -*- coding: utf-8 -*- + +# Copyright: (c) 2024, Rich Megginson +# SPDX-License-Identifier: GPL-2.0-or-later +# +"""Unit tests for get_config module""" + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import sys +import os +import unittest + +# Add module_utils to path for importing +sys.path.insert( + 0, + os.path.join(os.path.dirname(__file__), "..", "..", "module_utils", "firewall_lsr"), +) + +# pylint: disable=import-error, no-name-in-module +from get_config import recursive_dict_diff, recursive_show_diffs, merge_with_defaults + + +class TestRecursiveDictDiff(unittest.TestCase): + """Tests for recursive_dict_diff function""" + + def test_both_none(self): + """Test when both arguments are None""" + result = recursive_dict_diff(None, None, None) + self.assertIsNone(result) + + def test_dict1_none(self): + """Test when dict1 is None""" + result = recursive_dict_diff(None, {"key": "value"}, None) + self.assertIsNone(result) + + def test_dict2_none(self): + """Test when dict2 is None""" + dict1 = {"key": "value"} + result = recursive_dict_diff(dict1, None, None) + self.assertEqual(result, dict1) + + def test_identical_dicts(self): + """Test when both dicts are identical""" + dict1 = {"key": "value", "nested": {"a": 1, "b": 2}} + dict2 = {"key": "value", "nested": {"a": 1, "b": 2}} + result = recursive_dict_diff(dict1, dict2, None) + self.assertIsNone(result) + + def test_simple_diff(self): + """Test simple value difference""" + dict1 = {"key": "value1"} + dict2 = {"key": "value2"} + result = recursive_dict_diff(dict1, dict2, None) + self.assertEqual(result, {"key": "value1"}) + + def test_key_only_in_dict1(self): + """Test key exists only in dict1""" + dict1 = {"key1": "value1", "key2": "value2"} + dict2 = {"key1": "value1"} + result = recursive_dict_diff(dict1, dict2, None) + self.assertEqual(result, {"key2": "value2"}) + + def test_key_only_in_dict2(self): + """Test key exists only in dict2 - should not be in diff""" + dict1 = {"key1": "value1"} + dict2 = {"key1": "value1", "key2": "value2"} + result = recursive_dict_diff(dict1, dict2, None) + self.assertIsNone(result) + + def test_nested_dict_diff(self): + """Test nested dictionary differences""" + dict1 = {"zones": {"internal": {"ports": [("443", "tcp")]}}} + dict2 = {"zones": {"internal": {"ports": [("443", "tcp"), ("8443", "tcp")]}}} + result = recursive_dict_diff(dict1, dict2, None) + # Lists are compared and only the difference from dict1 is returned + self.assertIsNotNone(result) + + def test_deeply_nested_diff(self): + """Test deeply nested dictionary differences""" + dict1 = {"level1": {"level2": {"level3": {"value": "old"}}}} + dict2 = {"level1": {"level2": {"level3": {"value": "new"}}}} + result = recursive_dict_diff(dict1, dict2, None) + self.assertEqual(result, {"level1": {"level2": {"level3": {"value": "old"}}}}) + + def test_list_diff(self): + """Test list differences""" + dict1 = {"items": ["a", "b", "c"]} + dict2 = {"items": ["a", "b"]} + result = recursive_dict_diff(dict1, dict2, None) + self.assertEqual(result, {"items": ["c"]}) + + def test_list_identical(self): + """Test identical lists""" + dict1 = {"items": ["a", "b", "c"]} + dict2 = {"items": ["a", "b", "c"]} + result = recursive_dict_diff(dict1, dict2, None) + self.assertIsNone(result) + + def test_mixed_types(self): + """Test dict with mixed value types""" + dict1 = {"string": "hello", "number": 42, "list": [1, 2, 3], "nested": {"a": 1}} + dict2 = {"string": "hello", "number": 42, "list": [1, 2, 3], "nested": {"a": 1}} + result = recursive_dict_diff(dict1, dict2, None) + self.assertIsNone(result) + + def test_mixed_types_with_diff(self): + """Test dict with mixed value types and differences""" + dict1 = {"string": "hello", "number": 42, "list": [1, 2, 3], "nested": {"a": 1}} + dict2 = {"string": "world", "number": 42, "list": [1, 2], "nested": {"a": 2}} + result = recursive_dict_diff(dict1, dict2, None) + self.assertIn("string", result) + self.assertEqual(result["string"], "hello") + self.assertIn("list", result) + self.assertIn("nested", result) + + def test_with_normalizers(self): + """Test with normalization functions""" + normalizers = {"str": lambda s: s.strip().lower()} + dict1 = {"key": " VALUE "} + dict2 = {"key": "value"} + result = recursive_dict_diff(dict1, dict2, normalizers) + # After normalization, values should be equal + self.assertIsNone(result) + + +class TestRecursiveShowDiffs(unittest.TestCase): + """Tests for recursive_show_diffs function""" + + def test_both_none(self): + """Test when both arguments are None""" + result = recursive_show_diffs(None, None, None) + self.assertIsNone(result) + + def test_dict1_none(self): + """Test when dict1 is None""" + dict2 = {"key": "value"} + result = recursive_show_diffs(None, dict2, None) + self.assertEqual(result, {"before": None, "after": dict2}) + + def test_dict2_none(self): + """Test when dict2 is None""" + dict1 = {"key": "value"} + result = recursive_show_diffs(dict1, None, None) + self.assertEqual(result, {"before": dict1, "after": None}) + + def test_identical_dicts(self): + """Test when both dicts are identical""" + dict1 = {"key": "value", "nested": {"a": 1, "b": 2}} + dict2 = {"key": "value", "nested": {"a": 1, "b": 2}} + result = recursive_show_diffs(dict1, dict2, None) + self.assertIsNone(result) + + def test_simple_diff(self): + """Test simple value difference shows before and after""" + dict1 = {"key": "value1"} + dict2 = {"key": "value2"} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual(result["before"], {"key": "value1"}) + self.assertEqual(result["after"], {"key": "value2"}) + + def test_key_only_in_dict1(self): + """Test key exists only in dict1""" + dict1 = {"key1": "value1", "key2": "value2"} + dict2 = {"key1": "value1"} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual(result["before"], {"key2": "value2"}) + self.assertEqual(result["after"], {}) + + def test_key_only_in_dict2(self): + """Test key exists only in dict2""" + dict1 = {"key1": "value1"} + dict2 = {"key1": "value1", "key2": "value2"} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual(result["before"], {}) + self.assertEqual(result["after"], {"key2": "value2"}) + + def test_nested_dict_diff(self): + """Test nested dictionary differences - the example from the spec""" + dict1 = {"zones": {"internal": {"ports": (("443", "tcp"),)}}} + dict2 = {"zones": {"internal": {"ports": (("443", "tcp"), ("8443", "tcp"))}}} + result = recursive_show_diffs(dict1, dict2, None) + self.assertIsNotNone(result) + self.assertEqual( + result["before"]["zones"]["internal"]["ports"], (("443", "tcp"),) + ) + self.assertEqual( + result["after"]["zones"]["internal"]["ports"], + (("443", "tcp"), ("8443", "tcp")), + ) + + def test_deeply_nested_diff(self): + """Test deeply nested dictionary differences""" + dict1 = {"level1": {"level2": {"level3": {"value": "old"}}}} + dict2 = {"level1": {"level2": {"level3": {"value": "new"}}}} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual( + result["before"], {"level1": {"level2": {"level3": {"value": "old"}}}} + ) + self.assertEqual( + result["after"], {"level1": {"level2": {"level3": {"value": "new"}}}} + ) + + def test_list_diff(self): + """Test list differences""" + dict1 = {"items": ["a", "b"]} + dict2 = {"items": ["a", "b", "c"]} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual(result["before"]["items"], ["a", "b"]) + self.assertEqual(result["after"]["items"], ["a", "b", "c"]) + + def test_list_identical(self): + """Test identical lists""" + dict1 = {"items": ["a", "b", "c"]} + dict2 = {"items": ["a", "b", "c"]} + result = recursive_show_diffs(dict1, dict2, None) + self.assertIsNone(result) + + def test_multiple_differences(self): + """Test multiple differences at same level""" + dict1 = {"a": 1, "b": 2, "c": 3} + dict2 = {"a": 10, "b": 2, "c": 30} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual(result["before"]["a"], 1) + self.assertEqual(result["after"]["a"], 10) + self.assertEqual(result["before"]["c"], 3) + self.assertEqual(result["after"]["c"], 30) + self.assertNotIn("b", result["before"]) + self.assertNotIn("b", result["after"]) + + def test_complex_nested_diff(self): + """Test complex nested structure with multiple differences""" + dict1 = { + "zones": { + "public": { + "services": ["ssh", "http"], + "ports": [("80", "tcp")], + "target": "default", + }, + "internal": {"services": ["ssh"], "ports": []}, + } + } + customzone = { + "ports": [("8080", "tcp"), ("8081", "tcp")], + "services": ["ssh", "http", "https"], + "target": "DROP", + } + dict2 = { + "zones": { + "public": { + "services": ["ssh", "http", "https"], + "ports": [("80", "tcp")], + "target": "ACCEPT", + }, + "internal": {"services": ["ssh"], "ports": []}, + "customzone": customzone, + } + } + result = recursive_show_diffs(dict1, dict2, None) + self.assertIsNotNone(result) + self.assertIn("zones", result["before"]) + self.assertIn("zones", result["after"]) + self.assertIn("public", result["before"]["zones"]) + self.assertIn("public", result["after"]["zones"]) + # services and target changed, ports stayed same + self.assertIn("services", result["before"]["zones"]["public"]) + self.assertIn("services", result["after"]["zones"]["public"]) + self.assertEqual( + result["before"]["zones"]["public"]["services"], ["ssh", "http"] + ) + self.assertEqual( + result["after"]["zones"]["public"]["services"], ["ssh", "http", "https"] + ) + self.assertIn("target", result["before"]["zones"]["public"]) + self.assertIn("target", result["after"]["zones"]["public"]) + self.assertEqual(result["before"]["zones"]["public"]["target"], "default") + self.assertEqual(result["after"]["zones"]["public"]["target"], "ACCEPT") + self.assertNotIn("ports", result["before"]["zones"]["public"]) + self.assertNotIn("ports", result["after"]["zones"]["public"]) + # internal zone should not be in diff + self.assertNotIn("internal", result["before"]["zones"]) + self.assertNotIn("internal", result["after"]["zones"]) + # customzone should be in diff + self.assertNotIn("customzone", result["before"]["zones"]) + self.assertEqual(result["after"]["zones"]["customzone"], customzone) + + def test_with_normalizers(self): + """Test with normalization functions""" + normalizers = {"str": lambda s: s.strip().lower()} + dict1 = {"key": " VALUE "} + dict2 = {"key": "value"} + result = recursive_show_diffs(dict1, dict2, normalizers) + # After normalization, values should be equal + self.assertIsNone(result) + + def test_non_dict_values_with_diff(self): + """Test comparing non-dict values directly""" + # When both inputs are non-dicts, compare them directly + result = recursive_show_diffs("old", "new", None) + self.assertEqual(result, {"before": "old", "after": "new"}) + + def test_non_dict_values_identical(self): + """Test comparing identical non-dict values""" + result = recursive_show_diffs("same", "same", None) + self.assertIsNone(result) + + def test_list_values_directly(self): + """Test comparing lists directly (not as dict values)""" + list1 = [1, 2, 3] + list2 = [1, 2, 3, 4] + result = recursive_show_diffs(list1, list2, None) + self.assertEqual(result, {"before": list1, "after": list2}) + + def test_empty_dicts(self): + """Test comparing empty dicts""" + result = recursive_show_diffs({}, {}, None) + self.assertIsNone(result) + + def test_one_empty_dict(self): + """Test comparing empty dict with non-empty dict""" + dict1 = {} + dict2 = {"key": "value"} + result = recursive_show_diffs(dict1, dict2, None) + self.assertEqual(result["before"], {}) + self.assertEqual(result["after"], {"key": "value"}) + + +class TestMergeWithDefaults(unittest.TestCase): + """Tests for merge_with_defaults function""" + + def test_both_none(self): + """Test when both arguments are None""" + result = merge_with_defaults(None, None) + self.assertIsNone(result) + + def test_custom_none(self): + """Test when custom is None, returns defaults""" + defaults = {"zones": {"public": {"target": "default"}}} + result = merge_with_defaults(None, defaults) + self.assertEqual(result, defaults) + + def test_defaults_none(self): + """Test when defaults is None, returns custom""" + custom = {"zones": {"internal": {"target": "ACCEPT"}}} + result = merge_with_defaults(custom, None) + self.assertEqual(result, custom) + + def test_merge_missing_zone(self): + """Test merging a zone that exists in defaults but not custom""" + custom = {"zones": {"internal": {"target": "ACCEPT"}}} + defaults = { + "zones": { + "public": {"target": "default", "services": ["ssh"]}, + "internal": {"target": "default", "services": []}, + } + } + result = merge_with_defaults(custom, defaults) + # internal should keep custom settings + self.assertEqual(result["zones"]["internal"]["target"], "ACCEPT") + # public should be copied from defaults + self.assertIn("public", result["zones"]) + self.assertEqual(result["zones"]["public"]["target"], "default") + self.assertEqual(result["zones"]["public"]["services"], ["ssh"]) + + def test_merge_missing_service(self): + """Test merging a service that exists in defaults but not custom""" + custom = {"services": {"custom-svc": {"ports": [("8080", "tcp")]}}} + defaults = { + "services": { + "ssh": {"ports": [("22", "tcp")]}, + "http": {"ports": [("80", "tcp")]}, + } + } + result = merge_with_defaults(custom, defaults) + # custom-svc should remain + self.assertIn("custom-svc", result["services"]) + # ssh and http should be added from defaults + self.assertIn("ssh", result["services"]) + self.assertIn("http", result["services"]) + self.assertEqual(result["services"]["ssh"]["ports"], [("22", "tcp")]) + + def test_merge_all_types(self): + """Test merging all configuration types""" + custom = { + "zones": {"myzone": {}}, + "services": {"mysvc": {}}, + } + defaults = { + "zones": {"public": {"target": "default"}}, + "services": {"ssh": {"ports": [("22", "tcp")]}}, + "icmptypes": {"echo-request": {"destination": ["ipv4", "ipv6"]}}, + "helpers": {"ftp": {"module": "nf_conntrack_ftp"}}, + "ipsets": {"myset": {"type": "hash:ip"}}, + "policies": {"mypolicy": {"target": "CONTINUE"}}, + } + result = merge_with_defaults(custom, defaults) + # Custom items preserved + self.assertIn("myzone", result["zones"]) + self.assertIn("mysvc", result["services"]) + # Default items merged + self.assertIn("public", result["zones"]) + self.assertIn("ssh", result["services"]) + self.assertIn("echo-request", result["icmptypes"]) + self.assertIn("ftp", result["helpers"]) + self.assertIn("myset", result["ipsets"]) + self.assertIn("mypolicy", result["policies"]) + + def test_no_overwrite_existing(self): + """Test that existing custom items are not overwritten by defaults""" + custom = { + "zones": {"public": {"target": "ACCEPT", "services": ["http", "https"]}} + } + defaults = {"zones": {"public": {"target": "default", "services": ["ssh"]}}} + result = merge_with_defaults(custom, defaults) + # Custom settings should be preserved, not overwritten + self.assertEqual(result["zones"]["public"]["target"], "ACCEPT") + self.assertEqual(result["zones"]["public"]["services"], ["http", "https"]) + + def test_empty_custom(self): + """Test merging into empty custom dict""" + custom = {} + defaults = { + "zones": {"public": {"target": "default"}}, + "services": {"ssh": {"ports": [("22", "tcp")]}}, + } + result = merge_with_defaults(custom, defaults) + self.assertIn("zones", result) + self.assertIn("public", result["zones"]) + self.assertIn("services", result) + self.assertIn("ssh", result["services"]) + + def test_empty_defaults(self): + """Test merging with empty defaults dict""" + custom = {"zones": {"internal": {"target": "ACCEPT"}}} + defaults = {} + result = merge_with_defaults(custom, defaults) + self.assertEqual(result, custom) + + def test_custom_missing_category(self): + """Test when custom is missing entire category that exists in defaults""" + custom = {"zones": {"internal": {"target": "ACCEPT"}}} + defaults = { + "zones": {"public": {"target": "default"}}, + "services": {"ssh": {"ports": [("22", "tcp")]}}, + } + result = merge_with_defaults(custom, defaults) + # zones should have both + self.assertIn("internal", result["zones"]) + self.assertIn("public", result["zones"]) + # services should be created with default items + self.assertIn("services", result) + self.assertIn("ssh", result["services"]) + + def test_ignores_non_merge_keys(self): + """Test that keys not in merge_keys are ignored""" + custom = {"other_key": "custom_value"} + defaults = {"other_key": "default_value", "zones": {"public": {}}} + result = merge_with_defaults(custom, defaults) + # other_key in custom should remain unchanged + self.assertEqual(result["other_key"], "custom_value") + # zones should be merged + self.assertIn("zones", result) + self.assertIn("public", result["zones"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tox.ini b/tox.ini index 957c4e41..de42ad0c 100644 --- a/tox.ini +++ b/tox.ini @@ -12,3 +12,5 @@ configfile = {toxinidir}/pylintrc setenv = RUN_PYTEST_EXTRA_ARGS = -v LSR_PUBLISH_COVERAGE = normal + RUN_PYTEST_SETUP_MODULE_UTILS = true + RUN_PYLINT_SETUP_MODULE_UTILS = true diff --git a/vars/main.yml b/vars/main.yml index 327e6faa..c742653d 100644 --- a/vars/main.yml +++ b/vars/main.yml @@ -1,8 +1,4 @@ --- -__firewall_firewalld_dir: /etc/firewalld -__firewall_firewalld_conf: "{{ __firewall_firewalld_dir }}/firewalld.conf" -__firewall_usr_lib_dir: /usr/lib/firewalld - # ansible_facts required by the role __firewall_required_facts: - python_version