Merge 1328ef0c0a into 2c52f969e1

firewalld: make offline do something

SUMMARY

ansible.posix.firewalld has an offline flag, but it currently does not do anything. What most people expect it to do is allow the task to proceed even when firewalld is offline, so it makes the most sense for it to override the immediate flag and prevent the module from throwing an error in that case.
Fixes #81.
ISSUE TYPE


Feature Pull Request

COMPONENT NAME

firewalld
ADDITIONAL INFORMATION

Reviewed-by: Adam Miller <admiller@redhat.com>

changelogs/fragments/484-firewalld-offline.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - firewalld - added offline flag implementation (https://github.com/ansible-collections/ansible.posix/pull/484)

changelogs/fragments/496_seboolean-make-it-wrk-with-SELinux-disabled.yaml

@@ -0,0 +1,3 @@
+---
+bugfixes:
+  - seboolean - make it work with disabled SELinux

plugins/modules/firewalld.py

@@ -84,13 +84,15 @@ options:
     type: str
   permanent:
     description:
-      - Should this configuration be in the running firewalld configuration or persist across reboots.
+      - Whether to apply this change to the permanent firewalld configuration.
       - As of Ansible 2.3, permanent operations can operate on firewalld configs when it is not running (requires firewalld >= 0.3.9).
-      - Note that if this is C(false), immediate is assumed C(true).
+      - Note that if this is C(false), I(immediate) defaults to C(true).
     type: bool
+    default: false
   immediate:
     description:
-      - Should this configuration be applied immediately, if set as permanent.
+      - Whether to apply this change to the runtime firewalld configuration.
+      - Defaults to C(true) if I(permanent=false).
     type: bool
     default: false
   state:
@@ -112,8 +114,9 @@ options:
     type: str
   offline:
     description:
-      - Whether to run this module even when firewalld is offline.
+      - Ignores I(immediate) if I(permanent=true) and firewalld is not running.
     type: bool
+    default: false
   target:
     description:
       - firewalld Zone target
@@ -142,6 +145,14 @@ author:
 '''
 
 EXAMPLES = r'''
+- name: permanently enable https service, also enable it immediately if possible
+  ansible.posix.firewalld:
+    service: https
+    state: enabled
+    permanent: true
+    immediate: true
+    offline: true
+
 - name: permit traffic in default zone for https service
   ansible.posix.firewalld:
     service: https
@@ -806,12 +817,12 @@ def main():
             zone=dict(type='str'),
             immediate=dict(type='bool', default=False),
             source=dict(type='str'),
-            permanent=dict(type='bool'),
+            permanent=dict(type='bool', default=False),
             state=dict(type='str', required=True, choices=['absent', 'disabled', 'enabled', 'present']),
             timeout=dict(type='int', default=0),
             interface=dict(type='str'),
             masquerade=dict(type='str'),
-            offline=dict(type='bool'),
+            offline=dict(type='bool', default=False),
             target=dict(type='str', choices=['default', 'ACCEPT', 'DROP', '%%REJECT%%']),
         ),
         supports_check_mode=True,
@@ -832,19 +843,29 @@ def main():
     timeout = module.params['timeout']
     interface = module.params['interface']
     masquerade = module.params['masquerade']
+    offline = module.params['offline']
 
     # Sanity checks
     FirewallTransaction.sanity_check(module)
 
-    # If neither permanent or immediate is provided, assume immediate (as
+    # `offline`, `immediate`, and `permanent` have a weird twisty relationship.
-    # written in the module's docs)
+    if offline:
+        # specifying offline without permanent makes no sense
+        if not permanent:
+            module.fail_json(msg='offline cannot be enabled unless permanent changes are allowed')
+
+        # offline overrides immediate to false if firewalld is offline
+        if fw_offline:
+            immediate = False
+
+    # immediate defaults to true if permanent is not enabled
     if not permanent and not immediate:
         immediate = True
 
-    # Verify required params are provided
     if immediate and fw_offline:
         module.fail_json(msg='firewall is not currently running, unable to perform immediate actions without a running firewall daemon')
 
+    # Verify required params are provided
     changed = False
     msgs = []
     icmp_block = module.params['icmp_block']

plugins/modules/seboolean.py

@@ -82,23 +82,6 @@ def get_runtime_status(ignore_selinux_state=False):
     return True if ignore_selinux_state is True else selinux.is_selinux_enabled()
 
 
-def has_boolean_value(module, name):
-    bools = []
-    try:
-        rc, bools = selinux.security_get_boolean_names()
-    except OSError:
-        module.fail_json(msg="Failed to get list of boolean names")
-    # work around for selinux who changed its API, see
-    # https://github.com/ansible/ansible/issues/25651
-    if len(bools) > 0:
-        if isinstance(bools[0], binary_type):
-            name = to_bytes(name)
-    if name in bools:
-        return True
-    else:
-        return False
-
-
 def get_boolean_value(module, name):
     state = 0
     try:
@@ -174,7 +157,10 @@ def semanage_set_boolean_value(module, handle, name, value):
         semanage.semanage_handle_destroy(handle)
         module.fail_json(msg="Failed to modify boolean key with semanage")
 
-    if semanage.semanage_bool_set_active(handle, boolkey, sebool) < 0:
+    if (
+            selinux.is_selinux_enabled()
+            and semanage.semanage_bool_set_active(handle, boolkey, sebool) < 0
+    ):
         semanage.semanage_handle_destroy(handle)
         module.fail_json(msg="Failed to set boolean key active with semanage")
 
@@ -315,12 +301,9 @@ def main():
         # Feature only available in selinux library since 2012.
         name = selinux.selinux_boolean_sub(name)
 
-    if not has_boolean_value(module, name):
-        module.fail_json(msg="SELinux boolean %s does not exist." % name)
-
     if persistent:
         changed = semanage_boolean_value(module, name, state)
-    else:
+    elif selinux.is_selinux_enabled():
         cur_value = get_boolean_value(module, name)
         if cur_value != state:
             changed = True

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

@@ -21,6 +21,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: enabled
   register: result
 
@@ -33,6 +35,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: enabled
   register: result
 
@@ -45,6 +49,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: disabled
   register: result
 
@@ -57,6 +63,8 @@
   ansible.posix.firewalld:
     service: https
     permanent: true
+    immediate: true
+    offline: true
     state: disabled
   register: result