From da732a39410a978200d9057389e71607b5c4904c Mon Sep 17 00:00:00 2001 From: beeankha Date: Tue, 25 Aug 2020 14:57:02 -0400 Subject: [PATCH] Add unit test, update lookup method to favor ID over name --- .../plugins/module_utils/tower_api.py | 20 ++++------------ awx_collection/test/awx/test_module_utils.py | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py index d2ef54b113..36e3d045f0 100644 --- a/awx_collection/plugins/module_utils/tower_api.py +++ b/awx_collection/plugins/module_utils/tower_api.py @@ -127,24 +127,14 @@ class TowerAPIModule(TowerModule): if response['json']['count'] == 1: return response['json']['results'][0] elif response['json']['count'] > 1: - object_to_return = None for tower_object in response['json']['results']: - # Name takes priority, so we match on name first - # If we already have an object to return and it has the right name, we need to fail - if object_to_return and object_to_return['name'] == name_or_id: - self.fail_json(msg="The requested name or id was ambiguous and resulted in too many items") - # If we found an object ID with this name (which has to be unique) and we don't already have - # something to return, select this object for now - elif tower_object['id'] == name_or_id and not object_to_return: - object_to_return = tower_object - # Otherwise this is the first named object we found, so record that as what to return - else: - object_to_return = tower_object - return object_to_return + # 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: 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'] diff --git a/awx_collection/test/awx/test_module_utils.py b/awx_collection/test/awx/test_module_utils.py index e93b6ee939..2903621768 100644 --- a/awx_collection/test/awx/test_module_utils.py +++ b/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'