Merge pull request #7945 from beeankha/tower_role_id_fix

Get tower_role Module to Accept IDs for Related Objects

Reviewed-by: https://github.com/apps/softwarefactory-project-zuul

awx_collection/plugins/module_utils/tower_api.py

@@ -105,32 +105,39 @@ class TowerAPIModule(TowerModule):
 
         return response['json']['results'][0]
 
-    def resolve_name_to_id(self, endpoint, name_or_id):
-        # Try to resolve the object by name
+    def get_one_by_name_or_id(self, endpoint, name_or_id):
         name_field = 'name'
         if endpoint == 'users':
             name_field = 'username'
 
-        response = self.get_endpoint(endpoint, **{'data': {name_field: name_or_id}})
-        if response['status_code'] == 400:
-            self.fail_json(msg="Unable to try and resolve {0} for {1} : {2}".format(endpoint, name_or_id, response['json']['detail']))
+        query_params = {'or__{0}'.format(name_field): name_or_id}
+        try:
+            query_params['or__id'] = int(name_or_id)
+        except ValueError:
+            # If we get a value error, then we didn't have an integer so we can just pass and fall down to the fail
+            pass
+
+        response = self.get_endpoint(endpoint, **{'data': query_params})
+        if response['status_code'] != 200:
+            self.fail_json(
+                msg="Failed to query endpoint {0} for {1} {2} ({3}), see results".format(endpoint, name_field, name_or_id, response['status_code']),
+                resuls=response
+            )
 
         if response['json']['count'] == 1:
-            return response['json']['results'][0]['id']
+            return response['json']['results'][0]
+        elif response['json']['count'] > 1:
+            for tower_object in response['json']['results']:
+                # ID takes priority, so we match on that first
+                if str(tower_object['id']) == name_or_id:
+                    return tower_object
+            # We didn't match on an ID but we found more than 1 object, therefore the results are ambiguous
+            self.fail_json(msg="The requested name or id was ambiguous and resulted in too many items")
         elif response['json']['count'] == 0:
-            try:
-                int(name_or_id)
-                # If we got 0 items by name, maybe they gave us an ID, let's try looking it up by ID
-                response = self.head_endpoint("{0}/{1}".format(endpoint, name_or_id), **{'return_none_on_404': True})
-                if response is not None:
-                    return name_or_id
-            except ValueError:
-                # If we got a value error than we didn't have an integer so we can just pass and fall down to the fail
-                pass
-
             self.fail_json(msg="The {0} {1} was not found on the Tower server".format(endpoint, name_or_id))
-        else:
-            self.fail_json(msg="Found too many names {0} at endpoint {1} try using an ID instead of a name".format(name_or_id, endpoint))
+
+    def resolve_name_to_id(self, endpoint, name_or_id):
+        return self.get_one_by_name_or_id(endpoint, name_or_id)['id']
 
     def make_request(self, method, endpoint, *args, **kwargs):
         # In case someone is calling us directly; make sure we were given a method, let's not just assume a GET

awx_collection/plugins/modules/tower_role.py

@@ -126,11 +126,10 @@ def main():
     resource_data = {}
     for param in resource_param_keys:
         endpoint = module.param_to_endpoint(param)
-        name_field = 'username' if param == 'user' else 'name'
 
         resource_name = params.get(param)
         if resource_name:
-            resource = module.get_one(endpoint, **{'data': {name_field: resource_name}})
+            resource = module.get_one_by_name_or_id(module.param_to_endpoint(param), resource_name)
             if not resource:
                 module.fail_json(
                     msg='Failed to update role, {0} not found in {1}'.format(param, endpoint),
@@ -170,14 +169,14 @@ def main():
                 if response['status_code'] == 204:
                     module.json_output['changed'] = True
                 else:
-                    module.fail_json(msg="Failed to grant role {0}".format(response['json']['detail']))
+                    module.fail_json(msg="Failed to grant role. {0}".format(response['json'].get('detail', response['json'].get('msg', 'unknown'))))
         else:
             for an_id in list(set(existing_associated_ids) & set(new_association_list)):
                 response = module.post_endpoint(association_endpoint, **{'data': {'id': int(an_id), 'disassociate': True}})
                 if response['status_code'] == 204:
                     module.json_output['changed'] = True
                 else:
-                    module.fail_json(msg="Failed to revoke role {0}".format(response['json']['detail']))
+                    module.fail_json(msg="Failed to revoke role. {0}".format(response['json'].get('detail', response['json'].get('msg', 'unknown'))))
 
     module.exit_json(**module.json_output)
 

awx_collection/test/awx/test_module_utils.py

@@ -4,6 +4,7 @@ __metaclass__ = type
 import json
 import sys
 
+from awx.main.models import Organization, Team
 from requests.models import Response
 from unittest import mock
 
@@ -102,3 +103,25 @@ def test_no_templated_values(collection_import):
         'The inventory plugin FQCN is templated when the collection is built '
         'and the code should retain the default of awx.awx.'
     )
+
+
+def test_conflicting_name_and_id(run_module, admin_user):
+    """In the event that 2 related items match our search criteria in this way:
+    one item has an id that matches input
+    one item has a name that matches input
+    We should preference the id over the name.
+    Otherwise, the universality of the tower_api lookup plugin is compromised.
+    """
+    org_by_id = Organization.objects.create(name='foo')
+    slug = str(org_by_id.id)
+    org_by_name = Organization.objects.create(name=slug)
+    result = run_module('tower_team', {
+        'name': 'foo_team', 'description': 'fooin around',
+        'organization': slug
+    }, admin_user)
+    assert not result.get('failed', False), result.get('msg', result)
+    team = Team.objects.filter(name='foo_team').first()
+    assert str(team.organization_id) == slug, (
+        'Lookup by id should be preferenced over name in cases of conflict.'
+    )
+    assert team.organization.name == 'foo'

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

@@ -1,74 +1,112 @@
 ---
+- name: Generate a test id
+  set_fact:
+    test_id: "{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}"
+
 - name: Generate names
   set_fact:
-    username: "AWX-Collection-tests-tower_role-user-{{ lookup('password', '/dev/null chars=ascii_letters length=16') }}"
+    username: "AWX-Collection-tests-tower_role-user-{{ test_id }}"
+    project_name: "AWX-Collection-tests-tower_role-project-{{ test_id }}"
 
-- name: Create a User
-  tower_user:
-    first_name: Joe
-    last_name: User
-    username: "{{ username }}"
-    password: "{{ 65535 | random | to_uuid }}"
-    email: joe@example.org
-    state: present
-  register: result
+- block:
+    - name: Create a User
+      tower_user:
+        first_name: Joe
+        last_name: User
+        username: "{{ username }}"
+        password: "{{ 65535 | random | to_uuid }}"
+        email: joe@example.org
+        state: present
+      register: result
 
-- assert:
-    that:
-      - "result is changed"
+    - assert:
+        that:
+          - "result is changed"
 
-- name: Add Joe to the update role of the default Project
-  tower_role:
-    user: "{{ username }}"
-    role: update
-    project: Demo Project
-    state: "{{ item }}"
-  register: result
-  with_items:
-    - "present"
-    - "absent"
+    - name: Create a project
+      tower_project:
+        name: "{{ project_name }}"
+        organization: Default
+        scm_type: git
+        scm_url: https://github.com/ansible/test-playbooks
+        wait: false
+      register: project_info
 
-- assert:
-    that:
-      - "result is changed"
+    - assert:
+        that:
+          - project_info is changed
 
-- name: Create a workflow
-  tower_workflow_job_template:
-    name: test-role-workflow
-    organization: Default
-    state: present
+    - name: Add Joe to the update role of the default Project
+      tower_role:
+        user: "{{ username }}"
+        role: update
+        project: "Demo Project"
+        state: "{{ item }}"
+      register: result
+      with_items:
+        - "present"
+        - "absent"
 
-- name: Add Joe to workflow execute role
-  tower_role:
-    user: "{{ username }}"
-    role: execute
-    workflow: test-role-workflow
-    state: present
-  register: result
+    - assert:
+        that:
+          - "result is changed"
 
-- assert:
-    that:
-      - "result is changed"
+    - name: Add Joe to the new project by ID
+      tower_role:
+        user: "{{ username }}"
+        role: update
+        project: "{{ project_info['id'] }}"
+        state: "{{ item }}"
+      register: result
+      with_items:
+        - "present"
+        - "absent"
 
-- name: Add Joe to workflow execute role, no-op
-  tower_role:
-    user: "{{ username }}"
-    role: execute
-    workflow: test-role-workflow
-    state: present
-  register: result
+    - assert:
+        that:
+          - "result is changed"
 
-- assert:
-    that:
-      - "result is not changed"
+    - name: Create a workflow
+      tower_workflow_job_template:
+        name: test-role-workflow
+        organization: Default
+        state: present
 
-- name: Delete a User
-  tower_user:
-    username: "{{ username }}"
-    email: joe@example.org
-    state: absent
-  register: result
+    - name: Add Joe to workflow execute role
+      tower_role:
+        user: "{{ username }}"
+        role: execute
+        workflow: test-role-workflow
+        state: present
+      register: result
 
-- assert:
-    that:
-      - "result is changed"
+    - assert:
+        that:
+          - "result is changed"
+
+    - name: Add Joe to workflow execute role, no-op
+      tower_role:
+        user: "{{ username }}"
+        role: execute
+        workflow: test-role-workflow
+        state: present
+      register: result
+
+    - assert:
+        that:
+          - "result is not changed"
+
+  always:
+    - name: Delete a User
+      tower_user:
+        username: "{{ username }}"
+        email: joe@example.org
+        state: absent
+      register: result
+
+    - name: Delete the project
+      tower_project:
+        name: "{{ project_name }}"
+        organization: Default
+        state: absent
+      register: result