[constructed-inventory] Fix some validation for constructed inv sources (#13727)

- When updating, we need the original object so we can make sure we
  aren't changing things we shouldn't be.
- We want to allow source_vars and limit, but not much else.
- We want to block everything else (at least, if it doesn't match what
  is in the original object...to allow the collection to work properly).
- Add two functional tests.

Signed-off-by: Rick Elrod <rick@elrod.me>

awx/api/serializers.py

@@ -191,6 +191,11 @@ SUMMARIZABLE_FK_FIELDS = {
 }
 
 
+# These fields can be edited on a constructed inventory's generated source (possibly by using the constructed
+# inventory's special API endpoint, but also by using the inventory sources endpoint).
+CONSTRUCTED_INVENTORY_SOURCE_EDITABLE_FIELDS = ('source_vars', 'update_cache_timeout', 'limit', 'verbosity')
+
+
 def reverse_gfk(content_object, request):
     """
     Computes a reverse for a GenericForeignKey field.
@@ -1784,12 +1789,12 @@ class ConstructedInventorySerializer(InventorySerializer):
 
     class Meta:
         model = Inventory
-        fields = ('*', '-host_filter', 'source_vars', 'update_cache_timeout', 'limit', 'verbosity')
+        fields = ('*', '-host_filter') + CONSTRUCTED_INVENTORY_SOURCE_EDITABLE_FIELDS
         read_only_fields = ('*', 'kind')
 
     def pop_inv_src_data(self, data):
         inv_src_data = {}
-        for field in ('source_vars', 'update_cache_timeout', 'limit', 'verbosity'):
+        for field in CONSTRUCTED_INVENTORY_SOURCE_EDITABLE_FIELDS:
             if field in data:
                 # values always need to be removed, as they are not valid for Inventory model
                 value = data.pop(field)
@@ -2361,8 +2366,6 @@ 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
@@ -2386,11 +2389,16 @@ class InventorySourceSerializer(UnifiedJobTemplateSerializer, InventorySourceOpt
         def get_field_from_model_or_attrs(fd):
             return attrs.get(fd, self.instance and getattr(self.instance, fd) or None)
 
-        if get_field_from_model_or_attrs('source') == 'scm':
+        if self.instance and self.instance.source == 'constructed':
+            allowed_fields = CONSTRUCTED_INVENTORY_SOURCE_EDITABLE_FIELDS
+            for field in attrs:
+                if attrs[field] != getattr(self.instance, field) and field not in allowed_fields:
+                    raise serializers.ValidationError({"error": _("Cannot change field '{}' on a constructed inventory source.").format(field)})
+        elif get_field_from_model_or_attrs('source') == 'scm':
             if ('source' in attrs or 'source_project' in attrs) and get_field_from_model_or_attrs('source_project') is None:
                 raise serializers.ValidationError({"source_project": _("Project required for scm type sources.")})
-        elif (get_field_from_model_or_attrs('source') == 'constructed') and (self.instance and self.instance.source != 'constructed'):
-            raise serializers.ValidationError({"Error": _('constructed not a valid source for inventory')})
+        elif get_field_from_model_or_attrs('source') == 'constructed':
+            raise serializers.ValidationError({"error": _('constructed not a valid source for inventory')})
         else:
             redundant_scm_fields = list(filter(lambda x: attrs.get(x, None), ['source_project', 'source_path', 'scm_branch']))
             if redundant_scm_fields:

awx/main/tests/functional/api/test_inventory.py

@@ -624,6 +624,50 @@ class TestConstructedInventory:
         assert inv_src.update_cache_timeout == 54
         assert inv_src.limit == 'foobar'
 
+    def test_patch_constructed_inventory_generated_source_limits_editable_fields(self, constructed_inventory, admin_user, project, patch):
+        inv_src = constructed_inventory.inventory_sources.first()
+        r = patch(
+            url=inv_src.get_absolute_url(),
+            data={
+                'source': 'scm',
+                'source_project': project.pk,
+                'source_path': '',
+                'source_vars': 'plugin: a.b.c',
+            },
+            expect=400,
+            user=admin_user,
+        )
+        assert str(r.data['error'][0]) == "Cannot change field 'source' on a constructed inventory source."
+
+        # Make sure it didn't get updated before we got the error
+        inv_src_after_err = constructed_inventory.inventory_sources.first()
+        assert inv_src.id == inv_src_after_err.id
+        assert inv_src.source == inv_src_after_err.source
+        assert inv_src.source_project == inv_src_after_err.source_project
+        assert inv_src.source_path == inv_src_after_err.source_path
+        assert inv_src.source_vars == inv_src_after_err.source_vars
+
+    def test_patch_constructed_inventory_generated_source_allows_source_vars_edit(self, constructed_inventory, admin_user, patch):
+        inv_src = constructed_inventory.inventory_sources.first()
+        patch(
+            url=inv_src.get_absolute_url(),
+            data={
+                'source_vars': 'plugin: a.b.c',
+            },
+            expect=200,
+            user=admin_user,
+        )
+
+        inv_src_after_patch = constructed_inventory.inventory_sources.first()
+
+        # sanity checks
+        assert inv_src.id == inv_src_after_patch.id
+        assert inv_src.source == 'constructed'
+        assert inv_src_after_patch.source == 'constructed'
+        assert inv_src.source_vars == ''
+
+        assert inv_src_after_patch.source_vars == 'plugin: a.b.c'
+
     def test_create_constructed_inventory(self, constructed_inventory, admin_user, post, organization):
         r = post(
             url=reverse('api:constructed_inventory_list'),