Adapt automount to IPAAnsibleModule and add code review modifications.

README-automountmap.md

@@ -37,14 +37,13 @@ Example inventory file
 ipaserver.test.local
 ```
 
-
 Example playbook to ensure presence of an automount map:
 
 ```yaml
 ---
 - name: Playbook to add an automount map
   hosts: ipaserver
-  become: true
+  become: no
 
   tasks:
   - name: ensure map named auto.DMZ in location DMZ is created
@@ -55,20 +54,20 @@ Example playbook to ensure presence of an automount map:
       desc: "this is a map for servers in the DMZ"
 ```
 
+Example playbook to ensure auto.DMZi is absent:
 
-Example playbook to ensure auto.DMZi does not exist
 ```yaml
 ---
 - name: Playbook to remove an automount map
   hosts: ipaserver
-  become: true
+  become: no
 
   tasks:
   - name: ensure map auto.DMZ has been removed
     ipaautomountmap:
       ipaadmin_password: SomeADMINpassword
       name: auto.DMZ
-      location: DMX
+      location: DMZ
       state: absent
 ```
 
@@ -76,9 +75,6 @@ Example playbook to ensure auto.DMZi does not exist
 Variables
 =========
 
-ipaautomountmap
--------
-
 Variable | Description | Required
 -------- | ----------- | --------
 `ipaadmin_principal` | The admin principal is a string and defaults to `admin` | no
@@ -89,6 +85,11 @@ Variable | Description | Required
 `state` | The state to ensure. It can be one of `present`, or `absent`, default: `present`. | no
 
 
+Notes
+=====
+
+Creation of indirect mount points are not supported.
+
 Authors
 =======
 

playbooks/automount/automount-map-absent.yaml

@@ -1,7 +1,8 @@
 ---
 - name: Automount map absent example
   hosts: ipaserver
-  become: true
+  become: no
+
   tasks:
   - name: ensure map TestMap is absent
     ipaautomountmap:
@@ -9,4 +10,3 @@
       name: TestMap
       location: TestLocation
       state: absent
-

playbooks/automount/automount-map-present.yaml

@@ -1,7 +1,8 @@
 ---
 - name: Automount map present example
   hosts: ipaserver
-  become: true
+  become: no
+
   tasks:
   - name: ensure map TestMap is present
     ipaautomountmap:
@@ -9,4 +10,3 @@
       name: TestMap
       location: TestLocation
       desc: "this is a test map"
-

plugins/modules/ipaautomountmap.py

@@ -19,6 +19,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from __future__ import (absolute_import, division, print_function)
+
+__metaclass__ = type
+
 ANSIBLE_METADATA = {
     "metadata_version": "1.0",
     "supported_by": "community",
@@ -29,27 +33,27 @@ ANSIBLE_METADATA = {
 DOCUMENTATION = '''
 ---
 module: ipaautomountmap
-author: chris procter
+author: Chris Procter
 short_description: Manage FreeIPA autommount map
 description:
 - Add, delete, and modify an IPA automount map
 options:
   ipaadmin_principal:
-    description: The admin principal
+    description: The admin principal.
     default: admin
   ipaadmin_password:
-    description: The admin password
+    description: The admin password.
     required: false
   automountlocation:
     description: automount location map is anchored to
     choices: ["location", "automountlocationcn"]
     required: True
   name:
-    description: automount map to be managed
+    description: automount map to be managed.
     choices: ["mapname", "map", "automountmapname"]
     required: True
   desc:
-    description: description of automount map
+    description: description of automount map.
     choices: ["description"]
     required: false
   state:
@@ -78,84 +82,85 @@ EXAMPLES = '''
 RETURN = '''
 '''
 
-from ansible.module_utils.ansible_freeipa_module import FreeIPABaseModule
+from ansible.module_utils.ansible_freeipa_module import (
+    IPAAnsibleModule, compare_args_ipa
+)
 
 
-class AutomountMap(FreeIPABaseModule):
+class AutomountMap(IPAAnsibleModule):
 
-    ipa_param_mapping = {
-        "automountmapname": "name",
-    }
+    def __init__(self, *args, **kwargs):
+        # pylint: disable=super-with-arguments
+        super(AutomountMap, self).__init__(*args, **kwargs)
+        self.commands = []
 
-    def get_map(self, location, name):
-        response = dict()
+    def get_automountmap(self, location, name):
         try:
-            response = self.api_command("automountmap_show",
+            response = self.ipa_command(
+                "automountmap_show",
                 location,
-                                        {"automountmapname": name})
-        except Exception:
-            pass
-
-        return response.get("result", None)
+                {"automountmapname": name, "all": True}
+            )
+        except Exception:  # pylint: disable=broad-except
+            return None
+        else:
+            return response["result"]
 
     def check_ipa_params(self):
-
-        if self.ipa_params.state == "present":
-            if len(self.ipa_params.name) != 1:
+        invalid = []
+        name = self.params_get("name")
+        state = self.params_get("state")
+        if state == "present":
+            if len(name) != 1:
                 self.fail_json(msg="Exactly one name must be provided \
                                 for state=present.")
-        else:
-            if len(self.ipa_params.name) == 0 :
-                self.fail_json(msg="At least one name must be provided \
-                                when state=absent.")
+        if state == "absent":
+            if len(name) == 0 :
+                self.fail_json(msg="Argument 'map_type' can not be used with "
+                                   "state 'absent'")
+            invalid = ["desc"]
+
+        self.params_fail_used_invalid(invalid, state)
+
+    def get_args(self, mapname, desc):  # pylint: disable=no-self-use
+        _args = {}
+        if mapname:
+            _args["automountmapname"] = mapname
+        if desc:
+            _args["description"] = desc
+        return _args
 
     def define_ipa_commands(self):
-        args = self.get_ipa_command_args()
+        name = self.params_get("name")
+        state = self.params_get("state")
+        location = self.params_get("location")
+        desc = self.params_get("desc")
 
-        if self.ipa_params.state == "present":
-            automountmap = self.get_map(self.ipa_params.location,
-                                        self.ipa_params.name[0])
-            args['automountmapname'] = self.ipa_params.name[0]
+        for mapname in name:
+            automountmap = self.get_automountmap(location, mapname)
+
+            if state == "present":
+                args = self.get_args(mapname, desc)
                 if automountmap is None:
-                # does not exist and is wanted
-                self.add_ipa_command(
-                    "automountmap_add",
-                    name=self.ipa_params.location,
-                    args=args
-                )
+                    self.commands.append([location, "automountmap_add", args])
                 else:
-                # exists and is wanted, check for changes
-                if self.ipa_params.desc != \
-                        automountmap.get('description', [None])[0]:
-                    args['description'] = self.ipa_params.desc
-                    self.add_ipa_command(
-                        "automountmap_mod",
-                        name=self.ipa_params.location,
-                        args=args
+                    if not compare_args_ipa(self, args, automountmap):
+                        self.commands.append(
+                            [location, "automountmap_mod", args]
                         )
-        else:
-            # exists and is not wanted (i.e. self.ipa_params.state == "absent")
-            to_del = [x for x in self.ipa_params.name
-                      if self.get_map(self.ipa_params.location, x) is not None]
 
-            if len(to_del) > 0:
-                self.add_ipa_command(
+            if state == "absent":
+                if automountmap is not None:
+                    self.commands.append([
+                        location,
                         "automountmap_del",
-                    name=self.ipa_params.location,
-                    args={"automountmapname": to_del}
-                )
+                        {"automountmapname": [mapname]}
+                    ])
 
 
 def main():
     ipa_module = AutomountMap(
         argument_spec=dict(
-            ipaadmin_principal=dict(type="str",
-                                    default="admin"
-                                    ),
-            ipaadmin_password=dict(type="str",
-                                   required=False,
-                                   no_log=True
-                                   ),
             state=dict(type='str',
                        default='present',
                        choices=['present', 'absent']
@@ -177,7 +182,13 @@ def main():
                       ),
         ),
     )
-    ipa_module.ipa_run()
+    changed = False
+    ipaapi_context = ipa_module.params_get("ipaapi_context")
+    with ipa_module.ipa_connect(context=ipaapi_context):
+        ipa_module.check_ipa_params()
+        ipa_module.define_ipa_commands()
+        changed = ipa_module.execute_ipa_commands(ipa_module.commands)
+    ipa_module.exit_json(changed=changed)
 
 
 if __name__ == "__main__":

tests/automount/test_automountmap.yml

@@ -1,10 +1,20 @@
 ---
 - name: Test automountmap
   hosts: ipaserver
-  become: true
-  gather_facts: false
+  become: no
+  gather_facts: no
 
   tasks:
+  # setup environment
+  - name: ensure test maps are absent
+    ipaautomountmap:
+      ipaadmin_password: SomeADMINpassword
+      name:
+        - TestMap01
+        - TestMap02
+      location: TestLocation
+      state: absent
+
   - name: ensure location TestLocation is absent
     ipaautomountlocation:
       ipaadmin_password: SomeADMINpassword
@@ -24,6 +34,8 @@
       name: TestLocation
       state: present
 
+  # TESTS
+  - block:
     - name: ensure map TestMap is present
       ipaautomountmap:
         ipaadmin_password: SomeADMINpassword
@@ -50,7 +62,7 @@
       register: result
       failed_when: result.failed or not result.changed
 
-  - name: ensure map TestMap has a different description
+    - name: ensure map TestMap has a different description, again
       ipaautomountmap:
         ipaadmin_password: SomeADMINpassword
         name: TestMap
@@ -117,19 +129,19 @@
       register: result
       failed_when: result.failed or result.changed
 
+    # CLEAN UP
+    always:
+    - name: ensure test maps are absent
+      ipaautomountmap:
+        ipaadmin_password: SomeADMINpassword
+        name:
+          - TestMap01
+          - TestMap02
+        location: TestLocation
+        state: absent
+
     - name: ensure location TestLocation is absent
       ipaautomountlocation:
         ipaadmin_password: SomeADMINpassword
         name: TestLocation
         state: absent
-    register: result
-    failed_when: result.failed or not result.changed
-
-  - name: ensure location TestLocation is absent again
-    ipaautomountlocation:
-      ipaadmin_password: SomeADMINpassword
-      name: TestLocation
-      state: absent
-    register: result
-    failed_when: result.failed or result.changed
-