From 857fb82eb9141a44ffb91331653e1c30b43f671e Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Mon, 15 Jun 2020 23:40:35 -0300
Subject: [PATCH] Allows modification of forward policy in existing DNS Forward
 Zone.

This patch allows the modification of the forward zone policy in
an existing DNS Forward Zone, and fixes some issues with `enable`
and `disable` state that prevented correct behavior of `forwardpolicy`.
---
 plugins/modules/ipadnsforwardzone.py         | 154 ++++++++++---------
 tests/dnsforwardzone/test_dnsforwardzone.yml |  32 ++--
 2 files changed, 97 insertions(+), 89 deletions(-)

diff --git a/plugins/modules/ipadnsforwardzone.py b/plugins/modules/ipadnsforwardzone.py
index a729197b..1f1e85ec 100644
--- a/plugins/modules/ipadnsforwardzone.py
+++ b/plugins/modules/ipadnsforwardzone.py
@@ -217,10 +217,20 @@ def main():
     else:
         operation = "add"
 
-    if state == "disabled":
-        wants_enable = False
-    else:
-        wants_enable = True
+    if state in ["enabled", "disabled"]:
+        if action == "member":
+            ansible_module.fail_json(
+                msg="Action `member` cannot be used with state `%s`"
+                    % (state))
+        invalid = [
+            "forwarders", "forwardpolicy", "skip_overlap_check", "permission"
+        ]
+        for x in invalid:
+            if vars()[x] is not None:
+                ansible_module.fail_json(
+                    msg="Argument '%s' can not be used with action "
+                    "'%s', state `%s`" % (x, action, state))
+        wants_enable = (state == "enabled")
 
     if operation == "del":
         invalid = [
@@ -230,7 +240,7 @@ def main():
             if vars()[x] is not None:
                 ansible_module.fail_json(
                     msg="Argument '%s' can not be used with action "
-                    "'%s'" % (x, action))
+                    "'%s', state `%s`" % (x, action, state))
 
     changed = False
     exit_args = {}
@@ -262,7 +272,27 @@ def main():
                 if existing_resource is None and not forwarders:
                     ansible_module.fail_json(msg='No forwarders specified.')
 
-            if existing_resource is not None:
+            if existing_resource is None:
+                if operation == "add":
+                    # does not exist but should be present
+                    # determine args
+                    args = gen_args(forwarders, forwardpolicy,
+                                    skip_overlap_check)
+                    # set command
+                    command = "dnsforwardzone_add"
+                    # enabled or disabled?
+
+                elif operation == "update":
+                    # does not exist and is updating
+                    # trying to update something that doesn't exist, so error
+                    ansible_module.fail_json(
+                        msg="dnsforwardzone '%s' not found." % (name))
+
+                elif operation == "del":
+                    # there's nothnig to do.
+                    continue
+
+            else:   # existing_resource is not None
                 if state != "absent":
                     if forwarders:
                         forwarders = list(
@@ -274,66 +304,51 @@ def main():
                             set(existing_resource["idnsforwarders"])
                             - set(forwarders))
 
-            if existing_resource is None and operation == "update":
-                # does not exist and is updating
-                # trying to update something that doesn't exist, so error
-                ansible_module.fail_json(msg="""dnsforwardzone '%s' is not
-                                                         valid""" % (name))
-            elif existing_resource is None and operation == "del":
-                # does not exists and should be absent
-                # enabled or disabled?
-                is_enabled = "IGNORE"
-            elif existing_resource is not None and operation == "del":
-                # exists but should be absent
-                # set command
-                command = "dnsforwardzone_del"
-                args = {}
-                # enabled or disabled?
-                is_enabled = "IGNORE"
-            elif forwarders is None:
-                # forwarders are not defined its not a delete, update state?
-                # enabled or disabled?
+                if operation == "add":
+                    # exists and should be present, has it changed?
+                    # determine args
+                    args = gen_args(
+                        forwarders, forwardpolicy, skip_overlap_check)
+                    if 'skip_overlap_check' in args:
+                        del args['skip_overlap_check']
+
+                    # set command
+                    if not compare_args_ipa(
+                            ansible_module, args, existing_resource):
+                        command = "dnsforwardzone_mod"
+
+                elif operation == "del":
+                    # exists but should be absent
+                    # set command
+                    command = "dnsforwardzone_del"
+                    args = {}
+
+                elif operation == "update":
+                    # exists and is updating
+                    # calculate the new forwarders and mod
+                    args = gen_args(
+                        forwarders, forwardpolicy, skip_overlap_check)
+                    if "skip_overlap_check" in args:
+                        del args['skip_overlap_check']
+
+                    # command
+                    if not compare_args_ipa(
+                            ansible_module, args, existing_resource):
+                        command = "dnsforwardzone_mod"
+
+            if state in ['enabled', 'disabled']:
                 if existing_resource is not None:
                     is_enabled = existing_resource["idnszoneactive"][0]
                 else:
-                    is_enabled = "IGNORE"
-            elif existing_resource is not None and operation == "update":
-                # exists and is updating
-                # calculate the new forwarders and mod
-                args = gen_args(forwarders, forwardpolicy, skip_overlap_check)
-                if "skip_overlap_check" in args:
-                    del args['skip_overlap_check']
-
-                # command
-                if not compare_args_ipa(ansible_module, args, existing_resource):
-                    command = "dnsforwardzone_mod"
-
-                # enabled or disabled?
-                is_enabled = existing_resource["idnszoneactive"][0]
-
-            elif existing_resource is None and operation == "add":
-                # does not exist but should be present
-                # determine args
-                args = gen_args(forwarders, forwardpolicy,
-                                skip_overlap_check)
-                # set command
-                command = "dnsforwardzone_add"
-                # enabled or disabled?
-                is_enabled = "TRUE"
-
-            elif existing_resource is not None and operation == "add":
-                # exists and should be present, has it changed?
-                # determine args
-                args = gen_args(forwarders, forwardpolicy, skip_overlap_check)
-                if 'skip_overlap_check' in args:
-                    del args['skip_overlap_check']
-
-                # set command
-                if not compare_args_ipa(ansible_module, args, existing_resource):
-                    command = "dnsforwardzone_mod"
-
-                # enabled or disabled?
-                is_enabled = existing_resource["idnszoneactive"][0]
+                    ansible_module.fail_json(
+                        msg="dnsforwardzone '%s' not found." % (name))
+
+            # does the enabled state match what we want (if we care)
+            if is_enabled != "IGNORE":
+                if wants_enable and is_enabled != "TRUE":
+                    commands.append([name, "dnsforwardzone_enable", {}])
+                elif not wants_enable and is_enabled != "FALSE":
+                    commands.append([name, "dnsforwardzone_disable", {}])
 
             # if command is set...
             if command is not None:
@@ -354,20 +369,9 @@ def main():
                     )
 
             for name, command, args in commands:
-                result = api_command(ansible_module, command, name, args)
+                api_command(ansible_module, command, name, args)
                 changed = True
 
-            # does the enabled state match what we want (if we care)
-            if is_enabled != "IGNORE":
-                if wants_enable and is_enabled != "TRUE":
-                    api_command(ansible_module, "dnsforwardzone_enable",
-                                name, {})
-                    changed = True
-                elif not wants_enable and is_enabled != "FALSE":
-                    api_command(ansible_module, "dnsforwardzone_disable",
-                                name, {})
-                    changed = True
-
     except Exception as e:
         ansible_module.fail_json(msg=str(e))
 
diff --git a/tests/dnsforwardzone/test_dnsforwardzone.yml b/tests/dnsforwardzone/test_dnsforwardzone.yml
index 0386bd48..223cf3d0 100644
--- a/tests/dnsforwardzone/test_dnsforwardzone.yml
+++ b/tests/dnsforwardzone/test_dnsforwardzone.yml
@@ -106,6 +106,22 @@
     register: result
     failed_when: not result.changed
 
+  - name: change zone forward policy
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      forwardpolicy: first
+    register: result
+    failed_when: not result.changed
+
+  - name: change zone forward policy, again
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      forwardpolicy: first
+    register: result
+    failed_when: result.changed
+
   - name: ensure forwardzone example.com is absent.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
@@ -256,27 +272,15 @@
       action: member
       skip_overlap_check: true
     register: result
-    failed_when: result.changed
+    failed_when: not result.failed or "not found" not in result.msg
 
   - name: try to create a new forwarder with disabled state
-    ipadnsforwardzone:
-      ipaadmin_password: SomeADMINpassword
-      state: disabled
-      name: example.com
-      forwarders:
-        - ip_address: 4.4.4.4
-          port: 8053
-      skip_overlap_check: yes
-    register: result
-    failed_when: not result.changed
-
-  - name: ensure it stays disabled
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com
       state: disabled
     register: result
-    failed_when: result.changed
+    failed_when: not result.failed or "not found" not in result.msg
 
   - name: Ensure forwardzone is not added without forwarders, with correct message.
     ipadnsforwardzone:
-- 
GitLab