From ff96a750e13b55b41924b963d6b8afe1d2118831 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 29 Aug 2017 16:43:56 -0400 Subject: [PATCH] backend of org-scoped smart inventories --- awx/main/access.py | 13 ++++++++++++- awx/main/managers.py | 2 ++ .../tests/functional/models/test_inventory.py | 15 +++++++++++++++ awx/main/tests/functional/test_rbac_inventory.py | 14 ++++++++++++++ awx/main/tests/unit/api/test_views.py | 4 ++-- 5 files changed, 45 insertions(+), 3 deletions(-) diff --git a/awx/main/access.py b/awx/main/access.py index badd3f415f..a1fa59b157 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -600,9 +600,20 @@ class InventoryAccess(BaseAccess): @check_superuser def can_admin(self, obj, data): + # Host filter may only be modified by org admin level + org_admin_mandatory = False + new_host_filter = data.get('host_filter', None) if data else None + if new_host_filter == '': # Provision for field 'blank' behavior + new_host_filter = None + if new_host_filter != obj.host_filter: + org_admin_mandatory = True # Verify that the user has access to the new organization if moving an # inventory to a new organization. Otherwise, just check for admin permission. - return self.check_related('organization', Organization, data, obj=obj) and self.user in obj.admin_role + return ( + self.check_related('organization', Organization, data, obj=obj, + mandatory=org_admin_mandatory) and + self.user in obj.admin_role + ) @check_superuser def can_update(self, obj): diff --git a/awx/main/managers.py b/awx/main/managers.py index 155a46fa25..4825b33231 100644 --- a/awx/main/managers.py +++ b/awx/main/managers.py @@ -37,6 +37,8 @@ class HostManager(models.Manager): hasattr(self.instance, 'kind')): if self.instance.kind == 'smart' and self.instance.host_filter is not None: q = SmartFilter.query_from_string(self.instance.host_filter) + if self.instance.organization_id: + q = q.filter(inventory__organization=self.instance.organization_id) # If we are using host_filters, disable the core_filters, this allows # us to access all of the available Host entries, not just the ones associated # with a specific FK/relation. diff --git a/awx/main/tests/functional/models/test_inventory.py b/awx/main/tests/functional/models/test_inventory.py index fde2723dc3..85d07c1ce0 100644 --- a/awx/main/tests/functional/models/test_inventory.py +++ b/awx/main/tests/functional/models/test_inventory.py @@ -10,6 +10,7 @@ from awx.main.models import ( InventorySource, InventoryUpdate, ) +from awx.main.utils.filters import SmartFilter @pytest.mark.django_db @@ -109,3 +110,17 @@ class TestHostManager: organization=organization, host_filter='inventory_sources__source=ec2') assert len(smart_inventory.hosts.all()) == 0 + + def test_host_distinctness(self, setup_inventory_groups, organization): + """ + two criteria would both yield the same host, check that we only get 1 copy here + """ + assert ( + list(SmartFilter.query_from_string('name=single_host or name__startswith=single_')) == + [Host.objects.get(name='single_host')] + ) + + # Things we can not easily test due to SQLite backend: + # 2 organizations with host of same name only has 1 entry in smart inventory + # smart inventory in 1 organization does not include host from another + # smart inventory correctly returns hosts in filter in same organization diff --git a/awx/main/tests/functional/test_rbac_inventory.py b/awx/main/tests/functional/test_rbac_inventory.py index 42218905f3..821d8893a8 100644 --- a/awx/main/tests/functional/test_rbac_inventory.py +++ b/awx/main/tests/functional/test_rbac_inventory.py @@ -174,3 +174,17 @@ def test_inventory_source_org_admin_schedule_access(org_admin, inventory_source) assert access.get_queryset() assert access.can_read(schedule) assert access.can_change(schedule, {'rrule': 'DTSTART:20151117T050000Z RRULE:FREQ=DAILY;INTERVAL=1;COUNT=2'}) + + +@pytest.fixture +def smart_inventory(organization): + return organization.inventories.create(name="smart-inv", kind="smart") + + +@pytest.mark.django_db +class TestSmartInventory: + + def test_host_filter_edit(self, smart_inventory, rando, org_admin): + assert InventoryAccess(org_admin).can_admin(smart_inventory, {'host_filter': 'search=foo'}) + smart_inventory.admin_role.members.add(rando) + assert not InventoryAccess(rando).can_admin(smart_inventory, {'host_filter': 'search=foo'}) diff --git a/awx/main/tests/unit/api/test_views.py b/awx/main/tests/unit/api/test_views.py index 2402186606..a2532a75d7 100644 --- a/awx/main/tests/unit/api/test_views.py +++ b/awx/main/tests/unit/api/test_views.py @@ -220,8 +220,8 @@ class TestHostInsights(): class TestInventoryHostsList(object): def test_host_list_smart_inventory(self, mocker): - Inventory = namedtuple('Inventory', ['kind', 'host_filter', 'hosts']) - obj = Inventory(kind='smart', host_filter='localhost', hosts=HostManager()) + Inventory = namedtuple('Inventory', ['kind', 'host_filter', 'hosts', 'organization_id']) + obj = Inventory(kind='smart', host_filter='localhost', hosts=HostManager(), organization_id=None) obj.hosts.instance = obj with mock.patch.object(InventoryHostsList, 'get_parent_object', return_value=obj):