Fix username/name issue in tower_user module, update test playbook

awx_collection/plugins/module_utils/tower_api.py

@@ -531,8 +531,11 @@ class TowerModule(AnsibleModule):
             # If we have an item, we can see if it needs an update
             try:
                 item_url = existing_item['url']
-                item_name = existing_item['name']
-                item_type = existing_item['url']
+                item_type = existing_item['type']
+                if item_type == 'user':
+                    item_name = existing_item['username']
+                else:
+                    item_name = existing_item['name']
                 item_id = existing_item['id']
             except KeyError as ke:
                 self.fail_json(msg="Unable to process update of item due to missing data {0}".format(ke))

awx_collection/plugins/modules/tower_user.py

@@ -1,7 +1,7 @@
 #!/usr/bin/python
 # coding: utf-8 -*-
 
-# (c) 2017, Wayne Witzel III <wayne@riotousliving.com>
+# (c) 2020, John Westcott IV <john.westcott.iv@redhat.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
@@ -16,50 +16,58 @@ ANSIBLE_METADATA = {'metadata_version': '1.1',
 DOCUMENTATION = '''
 ---
 module: tower_user
-author: "Wayne Witzel III (@wwitzel3)"
+author: "John Westcott IV (@john-westcott-iv)"
 version_added: "2.3"
-short_description: create, update, or destroy Ansible Tower user.
+short_description: create, update, or destroy Ansible Tower users.
 description:
     - Create, update, or destroy Ansible Tower users. See
       U(https://www.ansible.com/tower) for an overview.
 options:
     username:
       description:
-        - The username of the user.
+        - Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.
       required: True
       type: str
     first_name:
       description:
         - First name of the user.
+      required: False
       type: str
     last_name:
       description:
         - Last name of the user.
+      required: False
       type: str
     email:
       description:
-        - Email address of the user. Required if creating a new user.
+        - Email address of the user.
       required: False
       type: str
-    password:
-      description:
-        - Password of the user.
-      type: str
-    superuser:
+    is_superuser:
       description:
         - User is a system wide administrator.
+      required: False
       type: bool
-      default: 'no'
-    auditor:
+      default: 'False'
+      aliases: ['superuser']
+    is_system_auditor:
       description:
         - User is a system wide auditor.
+      required: False
       type: bool
-      default: 'no'
+      default: 'False'
+      aliases: ['auditor']
+    password:
+      description:
+        - Password of the user; write-only field.
+      required: False
+      type: str
+      default: ''
     state:
       description:
         - Desired state of the resource.
-      default: "present"
       choices: ["present", "absent"]
+      default: "present"
       type: str
     tower_oauthtoken:
       description:
@@ -114,48 +122,72 @@ from ..module_utils.tower_api import TowerModule
 def main():
     # Any additional arguments that are not fields of the item can be added here
     argument_spec = dict(
-        username=dict(required=True),
-        first_name=dict(),
-        last_name=dict(),
-        password=dict(no_log=True),
-        email=dict(required=False, default=''),
-        superuser=dict(type='bool', default=False),
-        auditor=dict(type='bool', default=False),
+        username=dict(required=True, type='str'),
+        first_name=dict(required=False, type='str'),
+        last_name=dict(required=False, type='str'),
+        email=dict(required=False, type='str'),
+        is_superuser=dict(required=False, type='bool', default='False', aliases=['superuser']),
+        is_system_auditor=dict(required=False, type='bool', default='False', aliases=['auditor']),
+        password=dict(required=False, type='str', default=''),
         state=dict(choices=['present', 'absent'], default='present'),
     )
 
+# TODO: Implement a way to also generate proper aliases for backwards compatibility
+# and also add the aliases to documentation!
+
     # Create a module for ourselves
-    module = TowerModule(argument_spec=argument_spec, supports_check_mode=True, required_if=[['state', 'present', ['email']]])
+    module = TowerModule(argument_spec=argument_spec, supports_check_mode=True)
 
     # Extract our parameters
-    state = module.params.get('state')
+    username = module.params.get('username')
+    if username and len(username) > 150:
+        module.fail_msg(msg="The value for username can not be longer than 150")
+    first_name = module.params.get('first_name')
+    if first_name and len(first_name) > 30:
+        module.fail_msg(msg="The value for first_name can not be longer than 30")
+    last_name = module.params.get('last_name')
+    if last_name and len(last_name) > 150:
+        module.fail_msg(msg="The value for last_name can not be longer than 150")
     email = module.params.get('email')
+    if email and len(email) > 254:
+        module.fail_msg(msg="The value for email can not be longer than 254")
+    is_superuser = module.params.get('is_superuser')
+    is_system_auditor = module.params.get('is_system_auditor')
+    password = module.params.get('password')
+    state = module.params.get('state')
 
-    # Create the data that gets sent for create and update
-    user_fields = {
-        'username': module.params.get('username'),
-        'first_name': module.params.get('first_name'),
-        'last_name': module.params.get('last_name'),
-        'password': module.params.get('password'),
-        'superuser': module.params.get('superuser'),
-        'auditor': module.params.get('auditor'),
-    }
-    if email is not None:
-        user_fields['email'] = email
+    # Attempt to look up the related items the user specified (these will fail the module if not found)
 
-    # Attempt to look up user based on the provided username
-    user = module.get_one('users', **{
+    # Attempt to look up an existing item based on the provided data
+    existing_item = module.get_one('users', **{
         'data': {
-            'username': user_fields['username'],
+            'username': username,
         }
     })
 
+    # Create the data that gets sent for create and update
+    new_fields = {}
+    if username:
+        new_fields['username'] = username
+    if first_name:
+        new_fields['first_name'] = first_name
+    if last_name:
+        new_fields['last_name'] = last_name
+    if email:
+        new_fields['email'] = email
+    if is_superuser:
+        new_fields['is_superuser'] = is_superuser
+    if is_system_auditor:
+        new_fields['is_system_auditor'] = is_system_auditor
+    if password:
+        new_fields['password'] = password
+
     if state == 'absent':
         # If the state was absent we can let the module delete it if needed, the module will handle exiting from this
-        module.delete_if_needed(user)
+        module.delete_if_needed(existing_item)
     elif state == 'present':
-        # If the state was present and we can let the module build or update the existing user, this will return on its own
-        module.create_or_update_if_needed(user, user_fields, endpoint='users', item_type='user')
+        # If the state was present and we can let the module build or update the existing item, this will return on its own
+        module.create_or_update_if_needed(existing_item, new_fields, endpoint='users', item_type='user')
 
 
 if __name__ == '__main__':

awx_collection/tests/integration/targets/tower_user/tasks/main.yml

@@ -1,10 +1,20 @@
 ---
 - name: Create a User
   tower_user:
-    first_name: Joe
-    last_name: User
     username: joe
+    first_name: Joe
     password: "{{ 65535 | random | to_uuid }}"
+    state: present
+  register: result
+
+- assert:
+    that:
+      - "result is changed"
+
+- name: Change a User
+  tower_user:
+    username: joe
+    last_name: User
     email: joe@example.org
     state: present
   register: result
@@ -13,6 +23,17 @@
     that:
       - "result is changed"
 
+- name: Check idempotency
+  tower_user:
+    username: joe
+    first_name: Joe
+    last_name: User
+  register: result
+
+- assert:
+    that:
+      - "result is not changed"
+
 - name: Delete a User
   tower_user:
     username: joe