From bf864469a1da81c6b23e9726562b21408764ac8f Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Mon, 15 Jun 2020 20:42:23 -0300
Subject: [PATCH] Add support for attribute `permission` on dnsforwardzone
 module.

Adds missing attribute `permission to dnsforwardzone module, that
enable setting `manageby` for the DNS Forwar Zone.
---
 README-dnsforwardzone.md                     |   1 +
 plugins/modules/ipadnsforwardzone.py         |  71 ++++++++----
 tests/dnsforwardzone/test_dnsforwardzone.yml | 110 +++++++++++++++----
 3 files changed, 136 insertions(+), 46 deletions(-)

diff --git a/README-dnsforwardzone.md b/README-dnsforwardzone.md
index 15b2b574..175e6f8b 100644
--- a/README-dnsforwardzone.md
+++ b/README-dnsforwardzone.md
@@ -104,6 +104,7 @@ Variable | Description | Required
 &nbsp; | `port`: The forwarder IP port. | no
 `forwardpolicy` \| `idnsforwardpolicy` | Per-zone conditional forwarding policy. Possible values are `only`, `first`, `none`. Set to "none" to disable forwarding to global forwarder for this zone. In that case, conditional zone forwarders are disregarded. | no
 `skip_overlap_check` | Force DNS zone creation even if it will overlap with an existing zone. Defaults to False. | no
+`permission` | Allow DNS Forward Zone to be managed. (bool) | no
 `action` | Work on group or member level. It can be on of `member` or `dnsforwardzone` and defaults to `dnsforwardzone`. | no
 `state` | The state to ensure. It can be one of `present`, `absent`, `enabled` or `disabled`, default: `present`. | yes
 
diff --git a/plugins/modules/ipadnsforwardzone.py b/plugins/modules/ipadnsforwardzone.py
index 8e5c3464..a729197b 100644
--- a/plugins/modules/ipadnsforwardzone.py
+++ b/plugins/modules/ipadnsforwardzone.py
@@ -75,6 +75,11 @@ options:
     - Force DNS zone creation even if it will overlap with an existing zone.
     required: false
     default: false
+  permission:
+    description:
+    - Allow DNS Forward Zone to be managed.
+    required: false
+    type: bool
 '''
 
 EXAMPLES = '''
@@ -168,6 +173,8 @@ def main():
                                required=False,
                                choices=['only', 'first', 'none']),
             skip_overlap_check=dict(type='bool', required=False),
+            permission=dict(type='bool', required=False,
+                            aliases=['managedby']),
             action=dict(type="str", default="dnsforwardzone",
                         choices=["member", "dnsforwardzone"]),
             # state
@@ -191,6 +198,7 @@ def main():
     forwardpolicy = module_params_get(ansible_module, "forwardpolicy")
     skip_overlap_check = module_params_get(ansible_module,
                                            "skip_overlap_check")
+    permission = module_params_get(ansible_module, "permission")
     state = module_params_get(ansible_module, "state")
 
     if state == 'present' and len(names) != 1:
@@ -215,7 +223,9 @@ def main():
         wants_enable = True
 
     if operation == "del":
-        invalid = ["forwarders", "forwardpolicy", "skip_overlap_check"]
+        invalid = [
+            "forwarders", "forwardpolicy", "skip_overlap_check", "permission"
+        ]
         for x in invalid:
             if vars()[x] is not None:
                 ansible_module.fail_json(
@@ -241,6 +251,9 @@ def main():
         api_connect()
 
         for name in names:
+            commands = []
+            command = None
+
             # Make sure forwardzone exists
             existing_resource = find_dnsforwardzone(ansible_module, name)
 
@@ -249,6 +262,18 @@ 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 state != "absent":
+                    if forwarders:
+                        forwarders = list(
+                            set(existing_resource["idnsforwarders"]
+                                + forwarders))
+                else:
+                    if forwarders:
+                        forwarders = list(
+                            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
@@ -256,20 +281,17 @@ def main():
                                                          valid""" % (name))
             elif existing_resource is None and operation == "del":
                 # does not exists and should be absent
-                # set command
-                command = None
                 # 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?
-                # set command
-                command = None
                 # enabled or disabled?
                 if existing_resource is not None:
                     is_enabled = existing_resource["idnszoneactive"][0]
@@ -278,23 +300,13 @@ def main():
             elif existing_resource is not None and operation == "update":
                 # exists and is updating
                 # calculate the new forwarders and mod
-                # determine args
-                if state != "absent":
-                    forwarders = list(set(existing_resource["idnsforwarders"]
-                                          + forwarders))
-                else:
-                    forwarders = list(set(existing_resource["idnsforwarders"])
-                                      - set(forwarders))
-                args = gen_args(forwarders, forwardpolicy,
-                                skip_overlap_check)
-                if skip_overlap_check is not None:
+                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"
-                else:
-                    command = None
 
                 # enabled or disabled?
                 is_enabled = existing_resource["idnszoneactive"][0]
@@ -313,21 +325,36 @@ def main():
                 # exists and should be present, has it changed?
                 # determine args
                 args = gen_args(forwarders, forwardpolicy, skip_overlap_check)
-                if skip_overlap_check is not None:
+                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"
-                else:
-                    command = None
 
                 # enabled or disabled?
                 is_enabled = existing_resource["idnszoneactive"][0]
 
-            # if command is set then run it with the args
+            # if command is set...
             if command is not None:
-                api_command(ansible_module, command, name, args)
+                commands.append([name, command, args])
+
+            if permission is not None:
+                if existing_resource is None:
+                    managedby = None
+                else:
+                    managedby = existing_resource.get('managedby', None)
+                if permission and managedby is None:
+                    commands.append(
+                        [name, 'dnsforwardzone_add_permission', {}]
+                    )
+                elif not permission and managedby is not None:
+                    commands.append(
+                        [name, 'dnsforwardzone_remove_permission', {}]
+                    )
+
+            for name, command, args in commands:
+                result = api_command(ansible_module, command, name, args)
                 changed = True
 
             # does the enabled state match what we want (if we care)
diff --git a/tests/dnsforwardzone/test_dnsforwardzone.yml b/tests/dnsforwardzone/test_dnsforwardzone.yml
index 468cd4ce..0386bd48 100644
--- a/tests/dnsforwardzone/test_dnsforwardzone.yml
+++ b/tests/dnsforwardzone/test_dnsforwardzone.yml
@@ -51,8 +51,6 @@
     register: result
     failed_when: not result.changed
 
-  - pause:
-
   - name: ensure forwardzone example.com has one forwarder again
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
@@ -63,7 +61,7 @@
       skip_overlap_check: true
       state: present
     register: result
-    failed_when: not result.changed
+    failed_when: result.changed
 
   - name: skip_overlap_check can only be set on creation so change nothing
     ipadnsforwardzone:
@@ -77,6 +75,22 @@
     register: result
     failed_when: result.changed
 
+  - name: ensure forwardzone example.com is absent.
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      state: absent
+    register: result
+    failed_when: not result.changed
+
+  - name: ensure forwardzone example.com is absent, again.
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      state: absent
+    register: result
+    failed_when: result.changed
+
   - name: change all the things at once
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
@@ -87,11 +101,12 @@
         - ip_address: 4.4.4.4
           port: 8053
       forwardpolicy: only
-      skip_overlap_check: false
+      skip_overlap_check: true
+      permission: yes
     register: result
     failed_when: not result.changed
 
-  - name: ensure forwardzone example.com is absent for next testset
+  - name: ensure forwardzone example.com is absent.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com
@@ -156,43 +171,58 @@
     register: result
     failed_when: result.changed
 
-  - name: ensure forwardzone example.com is absent again
+  - name: Add a permission for per-forward zone access delegation.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com
-      state: absent
+      permission: yes
+      action: member
+    register: result
+    failed_when: not result.changed
 
-  - name: try to create a new forwarder with action=member
+  - name: Add a permission for per-forward zone access delegation, again.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
-      state: present
       name: example.com
-      forwarders:
-        - ip_address: 4.4.4.4
-          port: 8053
+      permission: yes
       action: member
-      skip_overlap_check: true
     register: result
     failed_when: result.changed
 
-  - name: ensure forwardzone example.com is absent - tidy up
+  - name: Remove a permission for per-forward zone access delegation.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com
-      state: absent
+      permission: no
+      action: member
+    register: result
+    failed_when: not result.changed
 
-  - name: try to create a new forwarder is disabled state
+  - name: Remove a permission for per-forward zone access delegation, again.
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
-      state: disabled
       name: example.com
-      forwarders:
-        - ip_address: 4.4.4.4
-          port: 8053
-      skip_overlap_check: true
+      permission: no
+      action: member
+    register: result
+    failed_when: result.changed
+
+  - name: disable the forwarder
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      state: disabled
     register: result
     failed_when: not result.changed
 
+  - name: disable the forwarder again
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      state: disabled
+    register: result
+    failed_when: result.changed
+
   - name: enable the forwarder
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
@@ -201,12 +231,42 @@
     register: result
     failed_when: not result.changed
 
-  - name: disable the forwarder again
+  - name: enable the forwarder, again
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
       name: example.com
-      state: disabled
+      state: enabled
+    register: result
+    failed_when: result.changed
+
+  - name: ensure forwardzone example.com is absent again
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      name: example.com
+      state: absent
+
+  - name: try to create a new forwarder with action=member
+    ipadnsforwardzone:
+      ipaadmin_password: SomeADMINpassword
+      state: present
+      name: example.com
+      forwarders:
+        - ip_address: 4.4.4.4
+          port: 8053
       action: member
+      skip_overlap_check: true
+    register: result
+    failed_when: result.changed
+
+  - 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
 
@@ -228,5 +288,7 @@
   - name: ensure forwardzone example.com is absent - tidy up
     ipadnsforwardzone:
       ipaadmin_password: SomeADMINpassword
-      name: example.com
+      name:
+      - example.com
+      - newfailzone.com
       state: absent
-- 
GitLab