From bedf56a7fddad3e3d04baf0756eb3ec5313865c6 Mon Sep 17 00:00:00 2001 From: Adam Miller Date: Thu, 6 Oct 2016 16:38:39 -0500 Subject: [PATCH] provide useful error when invalid service name provided add offline mode to firewalld permanent operations Signed-off-by: Adam Miller --- .../modules/extras/system/firewalld.py | 396 +++++++++++++++--- 1 file changed, 349 insertions(+), 47 deletions(-) diff --git a/lib/ansible/modules/extras/system/firewalld.py b/lib/ansible/modules/extras/system/firewalld.py index d826d5d6e13..6efae7e5718 100644 --- a/lib/ansible/modules/extras/system/firewalld.py +++ b/lib/ansible/modules/extras/system/firewalld.py @@ -106,7 +106,13 @@ EXAMPLES = ''' from ansible.module_utils.basic import AnsibleModule +import sys + +##################### +# Globals +# fw = None +module = None fw_offline = False Rich_Rule = None FirewallClientZoneSettings = None @@ -114,9 +120,37 @@ FirewallClientZoneSettings = None module = None ##################### -# fw_offline helpers +# exception handling # +def action_handler(action_func, action_func_args): + """ + Function to wrap calls to make actions on firewalld in try/except + logic and emit (hopefully) useful error messages + """ + + msgs = [] + + try: + return action_func(*action_func_args) + except Exception: + # Make python 2.4 shippable ci tests happy + e = sys.exc_info()[1] + + # If there are any commonly known errors that we should provide more + # context for to help the users diagnose what's wrong. Handle that here + if "INVALID_SERVICE" in "%s" % e: + msgs.append("Services are defined by port/tcp relationship and named as they are in /etc/services (on most systems)") + + if len(msgs) > 0: + module.fail_json( + msg='ERROR: Exception caught: %s %s' % (e, ', '.join(msgs)) + ) + else: + module.fail_json(msg='ERROR: Exception caught: %s' % e) +##################### +# fw_offline helpers +# def get_fw_zone_settings(zone): if fw_offline: fw_zone = fw.config.get_zone(zone) @@ -151,7 +185,6 @@ def get_masquerade_enabled_permanent(zone): else: return False - def set_masquerade_enabled(zone): fw.addMasquerade(zone) @@ -364,10 +397,12 @@ def set_rich_rule_disabled_permanent(zone, rule): fw_settings.removeRichRule(rule) update_fw_settings(fw_zone, fw_settings) - def main(): global module + ## make module global so we don't have to pass it to action_handler every + ## function call + global module module = AnsibleModule( argument_spec = dict( service=dict(required=False,default=None), @@ -380,6 +415,7 @@ def main(): timeout=dict(type='int',required=False,default=0), interface=dict(required=False,default=None), masquerade=dict(required=False,default=None), + offline=dict(type='bool',required=False,default=None), ), supports_check_mode=True ) @@ -397,7 +433,6 @@ def main(): from firewall.client import Rich_Rule from firewall.client import FirewallClient - HAS_FIREWALLD = True fw = None fw_offline = False @@ -418,10 +453,9 @@ def main(): fw_offline = True except ImportError: - HAS_FIREWALLD = False - - if not HAS_FIREWALLD: - module.fail_json(msg='firewalld and its python 2 module are required for this module, version 2.0.11 or newer required (3.0.9 or newer for offline operations)') + ## Make python 2.4 shippable ci tests happy + e = sys.exc_info()[1] + module.fail_json(msg='firewalld and its python 2 module are required for this module, version 2.0.11 or newer required (3.0.9 or newer for offline operations) \n %s' % e) if fw_offline: ## Pre-run version checking @@ -495,8 +529,57 @@ def main(): module.fail_json(msg='can only operate on port, service, rich_rule or interface at once') if service != None: - if permanent: - is_enabled = get_service_enabled_permanent(zone, service) + if immediate and permanent: + is_enabled_permanent = action_handler( + get_service_enabled_permanent, + (zone, service) + ) + is_enabled_immediate = action_handler( + get_service_enabled, + (zone, service) + ) + msgs.append('Permanent and Non-Permanent(immediate) operation') + + if desired_state == "enabled": + if not is_enabled_permanent or not is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if not is_enabled_permanent: + action_handler( + set_service_enabled_permanent, + (zone, service) + ) + changed=True + if not is_enabled_immediate: + action_handler( + set_service_enabled, + (zone, service, timeout) + ) + changed=True + + + elif desired_state == "disabled": + if is_enabled_permanent or is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if is_enabled_permanent: + action_handler( + set_service_disabled_permanent, + (zone, service) + ) + changed=True + if is_enabled_immediate: + action_handler( + set_service_disabled, + (zone, service) + ) + changed=True + + elif permanent and not immediate: + is_enabled = action_handler( + get_service_enabled_permanent, + (zone, service) + ) msgs.append('Permanent operation') if desired_state == "enabled": @@ -504,17 +587,26 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_service_enabled_permanent(zone, service) + action_handler( + set_service_enabled_permanent, + (zone, service) + ) changed=True elif desired_state == "disabled": if is_enabled == True: if module.check_mode: module.exit_json(changed=True) - set_service_disabled_permanent(zone, service) + action_handler( + set_service_disabled_permanent, + (zone, service) + ) changed=True - if immediate or not permanent: - is_enabled = get_service_enabled(zone, service) + elif immediate and not permanent: + is_enabled = action_handler( + get_service_enabled, + (zone, service) + ) msgs.append('Non-permanent operation') @@ -523,27 +615,35 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_service_enabled(zone, service, timeout) + action_handler( + set_service_enabled, + (zone, service, timeout) + ) changed=True elif desired_state == "disabled": if is_enabled == True: if module.check_mode: module.exit_json(changed=True) - set_service_disabled(zone, service) + action_handler( + set_service_disabled, + (zone, service) + ) changed=True if changed == True: msgs.append("Changed service %s to %s" % (service, desired_state)) + # FIXME - source type does not handle non-permanent mode, this was an + # oversight in the past. if source != None: - is_enabled = get_source(zone, source) + is_enabled = action_handler(get_source, (zone, source)) if desired_state == "enabled": if is_enabled == False: if module.check_mode: module.exit_json(changed=True) - add_source(zone, source) + action_handler(add_source, (zone, source)) changed=True msgs.append("Added %s to zone %s" % (source, zone)) elif desired_state == "disabled": @@ -551,13 +651,61 @@ def main(): if module.check_mode: module.exit_json(changed=True) - remove_source(zone, source) + action_handler(remove_source, (zone, source)) changed=True msgs.append("Removed %s from zone %s" % (source, zone)) if port != None: - if permanent: - is_enabled = get_port_enabled_permanent(zone, [port, protocol]) + if immediate and permanent: + is_enabled_permanent = action_handler( + get_port_enabled_permanent, + (zone,[port, protocol]) + ) + is_enabled_immediate = action_handler( + get_port_enabled, + (zone, [port, protocol]) + ) + msgs.append('Permanent and Non-Permanent(immediate) operation') + + if desired_state == "enabled": + if not is_enabled_permanent or not is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if not is_enabled_permanent: + action_handler( + set_port_enabled_permanent, + (zone, port, protocol) + ) + changed=True + if not is_enabled_immediate: + action_handler( + set_port_enabled, + (zone, port, protocol, timeout) + ) + changed=True + + elif desired_state == "disabled": + if is_enabled_permanent or is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if is_enabled_permanent: + action_handler( + set_port_disabled_permanent, + (zone, port, protocol) + ) + changed=True + if is_enabled_immediate: + action_handler( + set_port_disabled, + (zone, port, protocol) + ) + changed=True + + elif permanent and not immediate: + is_enabled = action_handler( + get_port_enabled_permanent, + (zone, [port, protocol]) + ) msgs.append('Permanent operation') if desired_state == "enabled": @@ -565,17 +713,26 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_port_enabled_permanent(zone, port, protocol) + action_handler( + set_port_enabled_permanent, + (zone, port, protocol) + ) changed=True elif desired_state == "disabled": if is_enabled == True: if module.check_mode: module.exit_json(changed=True) - set_port_disabled_permanent(zone, port, protocol) + action_handler( + set_port_disabled_permanent, + (zone, port, protocol) + ) changed=True - if immediate or not permanent: - is_enabled = get_port_enabled(zone, [port,protocol]) + if immediate and not permanent: + is_enabled = action_handler( + get_port_enabled, + (zone, [port,protocol]) + ) msgs.append('Non-permanent operation') if desired_state == "enabled": @@ -583,14 +740,20 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_port_enabled(zone, port, protocol, timeout) + action_handler( + set_port_enabled, + (zone, port, protocol, timeout) + ) changed=True elif desired_state == "disabled": if is_enabled == True: if module.check_mode: module.exit_json(changed=True) - set_port_disabled(zone, port, protocol) + action_handler( + set_port_disabled, + (zone, port, protocol) + ) changed=True if changed == True: @@ -598,8 +761,55 @@ def main(): desired_state)) if rich_rule != None: - if permanent: - is_enabled = get_rich_rule_enabled_permanent(zone, rich_rule) + if immediate and permanent: + is_enabled_permanent = action_handler( + get_rich_rule_enabled_permanent, + (zone, rich_rule) + ) + is_enabled_immediate = action_handler( + get_rich_rule_enabled, + (zone, rich_rule) + ) + msgs.append('Permanent and Non-Permanent(immediate) operation') + + if desired_state == "enabled": + if not is_enabled_permanent or not is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if not is_enabled_permanent: + action_handler( + set_rich_rule_enabled_permanent, + (zone, rich_rule) + ) + changed=True + if not is_enabled_immediate: + action_handler( + set_rich_rule_enabled, + (zone, rich_rule, timeout) + ) + changed=True + + elif desired_state == "disabled": + if is_enabled_permanent or is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if is_enabled_permanent: + action_handler( + set_rich_rule_disabled_permanent, + (zone, rich_rule) + ) + changed=True + if is_enabled_immediate: + action_handler( + set_rich_rule_disabled, + (zone, rich_rule) + ) + changed=True + if permanent and not immediate: + is_enabled = action_handler( + get_rich_rule_enabled_permanent, + (zone, rich_rule) + ) msgs.append('Permanent operation') if desired_state == "enabled": @@ -607,17 +817,26 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_rich_rule_enabled_permanent(zone, rich_rule) + action_handler( + set_rich_rule_enabled_permanent, + (zone, rich_rule) + ) changed=True elif desired_state == "disabled": if is_enabled == True: if module.check_mode: module.exit_json(changed=True) - set_rich_rule_disabled_permanent(zone, rich_rule) + action_handler( + set_rich_rule_disabled_permanent, + (zone, rich_rule) + ) changed=True - if immediate or not permanent: - is_enabled = get_rich_rule_enabled(zone, rich_rule) + if immediate and not permanent: + is_enabled = action_handler( + get_rich_rule_enabled, + (zone, rich_rule) + ) msgs.append('Non-permanent operation') if desired_state == "enabled": @@ -625,22 +844,68 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_rich_rule_enabled(zone, rich_rule, timeout) + action_handler( + set_rich_rule_enabled, + (zone, rich_rule, timeout) + ) changed=True elif desired_state == "disabled": if is_enabled == True: if module.check_mode: module.exit_json(changed=True) - set_rich_rule_disabled(zone, rich_rule) + action_handler( + set_rich_rule_disabled, + (zone, rich_rule) + ) changed=True if changed == True: msgs.append("Changed rich_rule %s to %s" % (rich_rule, desired_state)) if interface != None: - if permanent: - is_enabled = get_interface_permanent(zone, interface) + if immediate and permanent: + is_enabled_permanent = action_handler( + get_interface_permanent, + (zone, interface) + ) + is_enabled_immediate = action_handler( + get_interface, + (zone, interface) + ) + msgs.append('Permanent and Non-Permanent(immediate) operation') + + if desired_state == "enabled": + if not is_enabled_permanent or not is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if not is_enabled_permanent: + change_zone_of_interface_permanent(zone, interface) + changed=True + if not is_enabled_immediate: + change_zone_of_interface(zone, interface) + changed=True + if changed: + msgs.append("Changed %s to zone %s" % (interface, zone)) + + elif desired_state == "disabled": + if is_enabled_permanent or is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if is_enabled_permanent: + remove_interface_permanent(zone, interface) + changed=True + if is_enabled_immediate: + remove_interface(zone, interface) + changed=True + if changed: + msgs.append("Removed %s from zone %s" % (interface, zone)) + + elif permanent and not immediate: + is_enabled = action_handler( + get_interface_permanent, + (zone, interface) + ) msgs.append('Permanent operation') if desired_state == "enabled": if is_enabled == False: @@ -658,8 +923,11 @@ def main(): remove_interface_permanent(zone, interface) changed=True msgs.append("Removed %s from zone %s" % (interface, zone)) - if immediate or not permanent: - is_enabled = get_interface(zone, interface) + elif immediate and not permanent: + is_enabled = action_handler( + get_interface, + (zone, interface) + ) msgs.append('Non-permanent operation') if desired_state == "enabled": if is_enabled == False: @@ -680,8 +948,42 @@ def main(): if masquerade != None: - if permanent: - is_enabled = get_masquerade_enabled_permanent(zone) + if immediate and permanent: + is_enabled_permanent = action_handler( + get_masquerade_enabled_permanent, + (zone) + ) + is_enabled_immediate = action_handler(get_masquerade_enabled, (zone)) + msgs.append('Permanent and Non-Permanent(immediate) operation') + + if desired_state == "enabled": + if not is_enabled_permanent or not is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if not is_enabled_permanent: + action_handler(set_masquerade_permanent, (zone, True)) + changed=True + if not is_enabled_immediate: + action_handler(set_masquerade_enabled, (zone)) + changed=True + if changed: + msgs.append("Added masquerade to zone %s" % (zone)) + + elif desired_state == "disabled": + if is_enabled_permanent or is_enabled_immediate: + if module.check_mode: + module.exit_json(changed=True) + if is_enabled_permanent: + action_handler(set_masquerade_permanent, (zone, False)) + changed=True + if is_enabled_immediate: + action_handler(set_masquerade_disabled, (zone)) + changed=True + if changed: + msgs.append("Removed masquerade from zone %s" % (zone)) + + elif permanent and not immediate: + is_enabled = action_handler(get_masquerade_enabled_permanent, (zone)) msgs.append('Permanent operation') if desired_state == "enabled": @@ -689,7 +991,7 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_masquerade_permanent(zone, True) + action_handler(set_masquerade_permanent, (zone, True)) changed=True msgs.append("Added masquerade to zone %s" % (zone)) elif desired_state == "disabled": @@ -697,11 +999,11 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_masquerade_permanent(zone, False) + action_handler(set_masquerade_permanent, (zone, False)) changed=True msgs.append("Removed masquerade from zone %s" % (zone)) - if immediate or not permanent: - is_enabled = get_masquerade_enabled(zone) + elif immediate and not permanent: + is_enabled = action_handler(get_masquerade_enabled, (zone)) msgs.append('Non-permanent operation') if desired_state == "enabled": @@ -709,7 +1011,7 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_masquerade_enabled(zone) + action_handler(set_masquerade_enabled, (zone)) changed=True msgs.append("Added masquerade to zone %s" % (zone)) elif desired_state == "disabled": @@ -717,7 +1019,7 @@ def main(): if module.check_mode: module.exit_json(changed=True) - set_masquerade_disabled(zone) + action_handler(set_masquerade_disabled, (zone)) changed=True msgs.append("Removed masquerade from zone %s" % (zone))