From 2d9f2d36a14e751086eda40045bac6b663737596 Mon Sep 17 00:00:00 2001 From: Gabe Muniz Date: Fri, 10 Feb 2023 12:46:11 -0500 Subject: [PATCH 1/5] remove ability to add constructed inventories to constructed inventories --- awx/api/views/inventory.py | 9 +++++++++ .../test_inventory_input_constructed.py | 17 +++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 awx/main/tests/functional/test_inventory_input_constructed.py diff --git a/awx/api/views/inventory.py b/awx/api/views/inventory.py index 64550e11c5..453f9c6072 100644 --- a/awx/api/views/inventory.py +++ b/awx/api/views/inventory.py @@ -115,6 +115,15 @@ class InventoryInputInventoriesList(SubListAttachDetachAPIView): parent_model = Inventory relationship = 'input_inventories' + # Specifically overriding the post method on this view to disallow constructed inventories as input inventories + def post(self, request, *args, **kwargs): + obj = Inventory.objects.get(id=request.data.get('id')) + if obj.kind == 'constructed': + return Response( + dict(error=_('You cannot add a constructed inventory to another constructed inventory.')), status=status.HTTP_405_METHOD_NOT_ALLOWED + ) + return super(InventoryInputInventoriesList, self).post(request, *args, **kwargs) + class InventoryActivityStreamList(SubListAPIView): model = ActivityStream diff --git a/awx/main/tests/functional/test_inventory_input_constructed.py b/awx/main/tests/functional/test_inventory_input_constructed.py new file mode 100644 index 0000000000..9fa41a9cf6 --- /dev/null +++ b/awx/main/tests/functional/test_inventory_input_constructed.py @@ -0,0 +1,17 @@ +import pytest +from awx.main.models import Inventory +from awx.api.versioning import reverse + + +@pytest.mark.django_db +def test_constructed_inventory_post(post, organization, admin_user): + inventory1 = Inventory.objects.create(name='dummy1', kind='constructed', organization=organization) + inventory2 = Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) + resp = post( + url=reverse('api:inventory_input_inventories', kwargs={'pk': inventory1.id}), + data={'id': inventory2.id}, + user=admin_user, + expect=405, + ) + print(resp) + assert resp.status_code == 405 From e25c767a474807ca13411c33099aec80dd7d47d5 Mon Sep 17 00:00:00 2001 From: Gabe Muniz Date: Sat, 11 Feb 2023 10:57:47 -0500 Subject: [PATCH 2/5] added tests for group/host/source creation --- .../test_inventory_input_constructed.py | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/awx/main/tests/functional/test_inventory_input_constructed.py b/awx/main/tests/functional/test_inventory_input_constructed.py index 9fa41a9cf6..db56521b9a 100644 --- a/awx/main/tests/functional/test_inventory_input_constructed.py +++ b/awx/main/tests/functional/test_inventory_input_constructed.py @@ -3,6 +3,13 @@ from awx.main.models import Inventory from awx.api.versioning import reverse +@pytest.fixture +def constructed_inventory(organization, inventory): + inv_source = Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) + + return inv_source + + @pytest.mark.django_db def test_constructed_inventory_post(post, organization, admin_user): inventory1 = Inventory.objects.create(name='dummy1', kind='constructed', organization=organization) @@ -13,5 +20,37 @@ def test_constructed_inventory_post(post, organization, admin_user): user=admin_user, expect=405, ) - print(resp) assert resp.status_code == 405 + + +@pytest.mark.django_db +def test_add_constructed_inventory_source(post, admin_user, constructed_inventory): + resp = post( + url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': constructed_inventory.id}), + data={'name': 'dummy1', 'source': 'constructed'}, + user=admin_user, + expect=400, + ) + assert resp.status_code == 400 + + +@pytest.mark.django_db +def test_add_constructed_inventory_host(post, admin_user, constructed_inventory): + resp = post( + url=reverse('api:inventory_hosts_list', kwargs={'pk': constructed_inventory.pk}), + data={'name': 'dummy1'}, + user=admin_user, + expect=400, + ) + assert resp.status_code == 400 + + +@pytest.mark.django_db +def test_add_constructed_inventory_group(post, admin_user, constructed_inventory): + resp = post( + reverse('api:inventory_groups_list', kwargs={'pk': constructed_inventory.pk}), + data={'name': 'group-test'}, + user=admin_user, + expect=400, + ) + assert resp.status_code == 400 From 3ff65db2e6a944c91122cbf36c8c193bca4f8af7 Mon Sep 17 00:00:00 2001 From: Gabe Muniz Date: Sat, 11 Feb 2023 14:43:26 -0500 Subject: [PATCH 3/5] block updates to constructed source type --- awx/api/serializers.py | 2 ++ .../test_inventory_input_constructed.py | 28 ++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 7a9721b4e7..7e77b04af0 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2244,6 +2244,8 @@ class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOpt obj = super(InventorySourceSerializer, self).update(obj, validated_data) if deprecated_fields: self._update_deprecated_fields(deprecated_fields, obj) + if obj.source == 'constructed': + raise serializers.ValidationError({'error': _("Cannot edit source of type constructed.")}) return obj # TODO: remove when old 'credential' fields are removed diff --git a/awx/main/tests/functional/test_inventory_input_constructed.py b/awx/main/tests/functional/test_inventory_input_constructed.py index db56521b9a..e677d46f46 100644 --- a/awx/main/tests/functional/test_inventory_input_constructed.py +++ b/awx/main/tests/functional/test_inventory_input_constructed.py @@ -4,10 +4,11 @@ from awx.api.versioning import reverse @pytest.fixture -def constructed_inventory(organization, inventory): - inv_source = Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) - - return inv_source +def constructed_inventory(organization): + """ + creates a new constructed inventory source + """ + return Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) @pytest.mark.django_db @@ -15,8 +16,8 @@ def test_constructed_inventory_post(post, organization, admin_user): inventory1 = Inventory.objects.create(name='dummy1', kind='constructed', organization=organization) inventory2 = Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) resp = post( - url=reverse('api:inventory_input_inventories', kwargs={'pk': inventory1.id}), - data={'id': inventory2.id}, + url=reverse('api:inventory_input_inventories', kwargs={'pk': inventory1.pk}), + data={'id': inventory2.pk}, user=admin_user, expect=405, ) @@ -26,7 +27,7 @@ def test_constructed_inventory_post(post, organization, admin_user): @pytest.mark.django_db def test_add_constructed_inventory_source(post, admin_user, constructed_inventory): resp = post( - url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': constructed_inventory.id}), + url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': constructed_inventory.pk}), data={'name': 'dummy1', 'source': 'constructed'}, user=admin_user, expect=400, @@ -54,3 +55,16 @@ def test_add_constructed_inventory_group(post, admin_user, constructed_inventory expect=400, ) assert resp.status_code == 400 + + +@pytest.mark.django_db +def test_edit_constructed_inventory_source(patch, admin_user, inventory): + inventory.inventory_sources.create(name="dummysrc", source="constructed") + inv_id = inventory.inventory_sources.get(name='dummysrc').id + resp = patch( + reverse('api:inventory_source_detail', kwargs={'pk': inv_id}), + data={'description': inventory.name}, + user=admin_user, + expect=400, + ) + assert resp.status_code == 400 From d55af032f759643deb4cb1e2e971b8a27c9ebefe Mon Sep 17 00:00:00 2001 From: Gabe Muniz Date: Tue, 14 Feb 2023 00:07:49 -0500 Subject: [PATCH 4/5] refactored to use is_valid_relation instead of post --- awx/api/views/inventory.py | 12 ++---- .../test_inventory_input_constructed.py | 42 ++++++++++++------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/awx/api/views/inventory.py b/awx/api/views/inventory.py index 453f9c6072..4085cf9bff 100644 --- a/awx/api/views/inventory.py +++ b/awx/api/views/inventory.py @@ -14,6 +14,7 @@ from django.utils.translation import gettext_lazy as _ from rest_framework.exceptions import PermissionDenied from rest_framework.response import Response from rest_framework import status +from rest_framework import serializers # AWX from awx.main.models import ActivityStream, Inventory, JobTemplate, Role, User, InstanceGroup, InventoryUpdateEvent, InventoryUpdate @@ -115,14 +116,9 @@ class InventoryInputInventoriesList(SubListAttachDetachAPIView): parent_model = Inventory relationship = 'input_inventories' - # Specifically overriding the post method on this view to disallow constructed inventories as input inventories - def post(self, request, *args, **kwargs): - obj = Inventory.objects.get(id=request.data.get('id')) - if obj.kind == 'constructed': - return Response( - dict(error=_('You cannot add a constructed inventory to another constructed inventory.')), status=status.HTTP_405_METHOD_NOT_ALLOWED - ) - return super(InventoryInputInventoriesList, self).post(request, *args, **kwargs) + def is_valid_relation(self, parent, sub, created=False): + if sub.kind == 'constructed': + raise serializers.ValidationError({'error': 'You cannot add a constructed inventory to another constructed inventory.'}) class InventoryActivityStreamList(SubListAPIView): diff --git a/awx/main/tests/functional/test_inventory_input_constructed.py b/awx/main/tests/functional/test_inventory_input_constructed.py index e677d46f46..ea1efb7d64 100644 --- a/awx/main/tests/functional/test_inventory_input_constructed.py +++ b/awx/main/tests/functional/test_inventory_input_constructed.py @@ -8,26 +8,35 @@ def constructed_inventory(organization): """ creates a new constructed inventory source """ - return Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) + + def factory(name, org=organization): + try: + inv = Inventory.objects.get(name=name, organization=org, kind='constructed') + except Inventory.DoesNotExist: + inv = Inventory.objects.create(name=name, organization=org, kind='constructed') + return inv + + return factory @pytest.mark.django_db -def test_constructed_inventory_post(post, organization, admin_user): - inventory1 = Inventory.objects.create(name='dummy1', kind='constructed', organization=organization) - inventory2 = Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) +def test_constructed_inventory_post(post, admin_user, constructed_inventory): + inv1 = constructed_inventory(name='dummy1') + inv2 = constructed_inventory(name='dummy2') resp = post( - url=reverse('api:inventory_input_inventories', kwargs={'pk': inventory1.pk}), - data={'id': inventory2.pk}, + url=reverse('api:inventory_input_inventories', kwargs={'pk': inv1.pk}), + data={'id': inv2.pk}, user=admin_user, - expect=405, + expect=400, ) - assert resp.status_code == 405 + assert resp.status_code == 400 @pytest.mark.django_db def test_add_constructed_inventory_source(post, admin_user, constructed_inventory): + inv1 = constructed_inventory(name='dummy1') resp = post( - url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': constructed_inventory.pk}), + url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': inv1.pk}), data={'name': 'dummy1', 'source': 'constructed'}, user=admin_user, expect=400, @@ -37,8 +46,9 @@ def test_add_constructed_inventory_source(post, admin_user, constructed_inventor @pytest.mark.django_db def test_add_constructed_inventory_host(post, admin_user, constructed_inventory): + inv1 = constructed_inventory(name='dummy1') resp = post( - url=reverse('api:inventory_hosts_list', kwargs={'pk': constructed_inventory.pk}), + url=reverse('api:inventory_hosts_list', kwargs={'pk': inv1.pk}), data={'name': 'dummy1'}, user=admin_user, expect=400, @@ -48,8 +58,9 @@ def test_add_constructed_inventory_host(post, admin_user, constructed_inventory) @pytest.mark.django_db def test_add_constructed_inventory_group(post, admin_user, constructed_inventory): + inv1 = constructed_inventory(name='dummy1') resp = post( - reverse('api:inventory_groups_list', kwargs={'pk': constructed_inventory.pk}), + reverse('api:inventory_groups_list', kwargs={'pk': inv1.pk}), data={'name': 'group-test'}, user=admin_user, expect=400, @@ -58,12 +69,11 @@ def test_add_constructed_inventory_group(post, admin_user, constructed_inventory @pytest.mark.django_db -def test_edit_constructed_inventory_source(patch, admin_user, inventory): - inventory.inventory_sources.create(name="dummysrc", source="constructed") - inv_id = inventory.inventory_sources.get(name='dummysrc').id +def test_edit_constructed_inventory_source(patch, admin_user, inventory_source_factory): + inv_src = inventory_source_factory(name='dummy1', source='constructed') resp = patch( - reverse('api:inventory_source_detail', kwargs={'pk': inv_id}), - data={'description': inventory.name}, + reverse('api:inventory_source_detail', kwargs={'pk': inv_src.id}), + data={'description': inv_src.name}, user=admin_user, expect=400, ) From 7f2933c43c84b2725be63995f130c1d487c0f632 Mon Sep 17 00:00:00 2001 From: Gabe Muniz Date: Tue, 14 Feb 2023 17:54:54 -0500 Subject: [PATCH 5/5] moved fixture to conftest.py and reverted some test changes --- awx/main/tests/functional/conftest.py | 8 +++++ .../test_inventory_input_constructed.py | 33 ++++--------------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 4f8b6bc83c..e1284ce87c 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -511,6 +511,14 @@ def group(inventory): return inventory.groups.create(name='single-group') +@pytest.fixture +def constructed_inventory(organization): + """ + creates a new constructed inventory source + """ + return Inventory.objects.create(name='dummy1', kind='constructed', organization=organization) + + @pytest.fixture def inventory_source(inventory): # by making it ec2, the credential is not required diff --git a/awx/main/tests/functional/test_inventory_input_constructed.py b/awx/main/tests/functional/test_inventory_input_constructed.py index ea1efb7d64..2602cf2947 100644 --- a/awx/main/tests/functional/test_inventory_input_constructed.py +++ b/awx/main/tests/functional/test_inventory_input_constructed.py @@ -3,26 +3,10 @@ from awx.main.models import Inventory from awx.api.versioning import reverse -@pytest.fixture -def constructed_inventory(organization): - """ - creates a new constructed inventory source - """ - - def factory(name, org=organization): - try: - inv = Inventory.objects.get(name=name, organization=org, kind='constructed') - except Inventory.DoesNotExist: - inv = Inventory.objects.create(name=name, organization=org, kind='constructed') - return inv - - return factory - - @pytest.mark.django_db -def test_constructed_inventory_post(post, admin_user, constructed_inventory): - inv1 = constructed_inventory(name='dummy1') - inv2 = constructed_inventory(name='dummy2') +def test_constructed_inventory_post(post, admin_user, organization): + inv1 = Inventory.objects.create(name='dummy1', kind='constructed', organization=organization) + inv2 = Inventory.objects.create(name='dummy2', kind='constructed', organization=organization) resp = post( url=reverse('api:inventory_input_inventories', kwargs={'pk': inv1.pk}), data={'id': inv2.pk}, @@ -34,9 +18,8 @@ def test_constructed_inventory_post(post, admin_user, constructed_inventory): @pytest.mark.django_db def test_add_constructed_inventory_source(post, admin_user, constructed_inventory): - inv1 = constructed_inventory(name='dummy1') resp = post( - url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': inv1.pk}), + url=reverse('api:inventory_inventory_sources_list', kwargs={'pk': constructed_inventory.pk}), data={'name': 'dummy1', 'source': 'constructed'}, user=admin_user, expect=400, @@ -46,9 +29,8 @@ def test_add_constructed_inventory_source(post, admin_user, constructed_inventor @pytest.mark.django_db def test_add_constructed_inventory_host(post, admin_user, constructed_inventory): - inv1 = constructed_inventory(name='dummy1') resp = post( - url=reverse('api:inventory_hosts_list', kwargs={'pk': inv1.pk}), + url=reverse('api:inventory_hosts_list', kwargs={'pk': constructed_inventory.pk}), data={'name': 'dummy1'}, user=admin_user, expect=400, @@ -58,9 +40,8 @@ def test_add_constructed_inventory_host(post, admin_user, constructed_inventory) @pytest.mark.django_db def test_add_constructed_inventory_group(post, admin_user, constructed_inventory): - inv1 = constructed_inventory(name='dummy1') resp = post( - reverse('api:inventory_groups_list', kwargs={'pk': inv1.pk}), + reverse('api:inventory_groups_list', kwargs={'pk': constructed_inventory.pk}), data={'name': 'group-test'}, user=admin_user, expect=400, @@ -72,7 +53,7 @@ def test_add_constructed_inventory_group(post, admin_user, constructed_inventory def test_edit_constructed_inventory_source(patch, admin_user, inventory_source_factory): inv_src = inventory_source_factory(name='dummy1', source='constructed') resp = patch( - reverse('api:inventory_source_detail', kwargs={'pk': inv_src.id}), + reverse('api:inventory_source_detail', kwargs={'pk': inv_src.pk}), data={'description': inv_src.name}, user=admin_user, expect=400,