diff --git a/awx/api/serializers.py b/awx/api/serializers.py index d02a486f5c..0e0ad53aff 100644 --- a/awx/api/serializers.py +++ b/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: diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index c8139a340a..ab84ff236d 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/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'),