AAP-17690 Inventory variables sourced from git project are not getting deleted after being removed from source

awx/api/serializers.py

@@ -116,6 +116,7 @@
 from awx.main.utils.filters import SmartFilter
 from awx.main.utils.plugins import load_combined_inventory_source_options
 from awx.main.utils.named_url_graph import reset_counters
+from awx.main.utils.inventory_vars import update_group_variables
 from awx.main.scheduler.task_manager_models import TaskManagerModels
 from awx.main.redact import UriCleaner, REPLACE_STR
 from awx.main.signals import update_inventory_computed_fields
@@ -1614,7 +1615,15 @@ def validate(self, attrs):
 
         if kind == 'smart' and not host_filter:
             raise serializers.ValidationError({'host_filter': _('Smart inventories must specify host_filter')})
-        return super(InventorySerializer, self).validate(attrs)
+        #
+        attrs = super(InventorySerializer, self).validate(attrs)
+        # The variables field contains vars from the all-group. Since this is
+        # not an update from an inventory source, we update the variables when
+        # the inventory object is saved.
+        vars = parse_yaml_or_json(attrs.get("variables"), silent_failure=False)  # TODO: silent_failure or not?
+        update_group_variables("all", vars, None, 0)
+        #
+        return attrs
 
 
 class ConstructedFieldMixin(serializers.Field):
@@ -1909,7 +1918,14 @@ def validate(self, attrs):
         inventory = attrs.get('inventory', self.instance and self.instance.inventory or '')
         if Host.objects.filter(name=name, inventory=inventory).exists():
             raise serializers.ValidationError(_('A Host with that name already exists.'))
-        return super(GroupSerializer, self).validate(attrs)
+        attrs = super(GroupSerializer, self).validate(attrs)
+        # The variables field contains vars from the inventory group dialog.
+        # Since this is not an update from an inventory source, we update the
+        # variables when the inventory group object is saved.
+        vars = parse_yaml_or_json(attrs.get("variables"), silent_failure=False)  # TODO: silent_failure or not?
+        update_group_variables(name, vars, None, 0)
+        #
+        return attrs
 
     def validate_name(self, value):
         if value in ('all', '_meta'):

awx/main/management/commands/inventory_import.py

@@ -33,6 +33,7 @@
 from awx.main.models.rbac import batch_role_ancestor_rebuilding
 from awx.main.utils import ignore_inventory_computed_fields, get_licenser
 from awx.main.utils.execution_environments import get_default_execution_environment
+from awx.main.utils.inventory_vars import update_group_variables
 from awx.main.signals import disable_activity_stream
 from awx.main.constants import STANDARD_INVENTORY_UPDATE_ENV
 
@@ -457,19 +458,19 @@ def _update_inventory(self):
         """
         Update inventory variables from "all" group.
         """
-        # TODO: We disable variable overwrite here in case user-defined inventory variables get
-        # mangled. But we still need to figure out a better way of processing multiple inventory
-        # update variables mixing with each other.
-        # issue for this: https://github.com/ansible/awx/issues/11623
-
         if self.inventory.kind == 'constructed' and self.inventory_source.overwrite_vars:
             # NOTE: we had to add a exception case to not merge variables
             # to make constructed inventory coherent
             db_variables = self.all_group.variables
         else:
-            db_variables = self.inventory.variables_dict
-            db_variables.update(self.all_group.variables)
-
+            db_variables = update_group_variables(
+                group="all",
+                newvars=self.all_group.variables,
+                dbvars=self.inventory.variables_dict,
+                invsrc_id=self.inventory_source.id,
+                overwrite=self.overwrite_vars,
+            )
+        #
         if db_variables != self.inventory.variables_dict:
             self.inventory.variables = json.dumps(db_variables)
             self.inventory.save(update_fields=['variables'])
@@ -499,10 +500,13 @@ def _create_update_groups(self):
             for group in self.inventory.groups.filter(name__in=group_names):
                 mem_group = self.all_group.all_groups[group.name]
                 db_variables = group.variables_dict
-                if self.overwrite_vars:
-                    db_variables = mem_group.variables
-                else:
-                    db_variables.update(mem_group.variables)
+                db_variables = update_group_variables(
+                    group=group.name,
+                    newvars=mem_group.variables,
+                    dbvars=group.variables_dict,
+                    invsrc_id=self.inventory_source.id,
+                    overwrite=self.overwrite_vars,
+                )
                 if db_variables != group.variables_dict:
                     group.variables = json.dumps(db_variables)
                     group.save(update_fields=['variables'])

awx/main/migrations/0203_inventorygroupvariableswithhistory.py

@@ -0,0 +1,21 @@
+# Generated by Django 4.2.20 on 2025-04-16 10:44
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('main', '0202_delete_token_cleanup_job'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='InventoryGroupVariablesWithHistory',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('group_name', models.CharField(max_length=256)),
+                ('variables', models.JSONField()),
+            ],
+        ),
+    ]

awx/main/models/__init__.py

@@ -33,6 +33,7 @@
     InventorySource,
     InventoryUpdate,
     SmartInventoryMembership,
+    InventoryGroupVariablesWithHistory,
 )
 from awx.main.models.jobs import (  # noqa
     Job,

awx/main/models/inventory.py

@@ -1402,3 +1402,17 @@ class Meta:
 
     def get_absolute_url(self, request=None):
         return reverse('api:inventory_script_detail', kwargs={'pk': self.pk}, request=request)
+
+
+class InventoryGroupVariablesWithHistory(models.Model):
+    """
+    Represents the inventory variables of one inventory group.
+
+    The purpose of this model is to persist the update history of the group
+    variables. The update history is maintained in another class
+    (`InventoryGroupVariables`), this class here is just a container for the
+    database storage.
+    """
+
+    group_name = models.CharField(max_length=256)
+    variables = models.JSONField()  # The group variables, including their history.

awx/main/tests/unit/utils/test_inventory_vars.py

@@ -0,0 +1,105 @@
+"""
+Test utility functions and classes for inventory variable handling.
+"""
+
+import pytest
+
+from awx.main.utils.inventory_vars import InventoryVariable
+from awx.main.utils.inventory_vars import InventoryGroupVariables
+
+
+def test_inventory_variable_update_basic():
+    """Test basic functionality of an inventory variable."""
+    x = InventoryVariable("x")
+    assert x.has_no_source
+    x.update(1, 101)
+    assert str(x) == "1"
+    x.update(2, 102)
+    assert str(x) == "2"
+    x.update(3, 103)
+    assert str(x) == "3"
+    x.update(None, 102)
+    assert str(x) == "3"
+    x.update(None, 103)
+    assert str(x) == "1"
+    x.update(None, 101)
+    assert x.value is None
+    assert x.has_no_source
+
+
+@pytest.mark.parametrize(
+    "updates",  # (<source_id>, <value>, <expected_value>)
+    [
+        ((101, 1, 1),),
+        ((101, 1, 1), (101, None, None)),
+        ((101, 1, 1), (102, 2, 2), (102, None, 1)),
+        ((101, 1, 1), (102, 2, 2), (101, None, 2), (102, None, None)),
+        (
+            (101, 0, 0),
+            (101, 1, 1),
+            (102, 2, 2),
+            (103, 3, 3),
+            (102, None, 3),
+            (103, None, 1),
+            (101, None, None),
+        ),
+    ],
+)
+def test_inventory_variable_update(updates: tuple[int, int | None, int | None]):
+    """
+    Test if the variable value is set correctly on a sequence of updates.
+    """
+    x = InventoryVariable("x")
+    for src_id, value, expected_value in updates:
+        x.update(value, src_id)
+        assert x.value == expected_value
+
+
+def test_inventory_group_variables_update_basic():
+    """Test basic functionality of an inventory variables update."""
+    vars = InventoryGroupVariables("all")
+    vars.update_from_src({"x": 1, "y": 2}, 101)
+    assert vars == {"x": 1, "y": 2}
+
+
+@pytest.mark.parametrize(
+    "updates",  # (<source_id>, <vars>: dict, <expected_vars>: dict)
+    [
+        ((101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),),
+        (
+            (101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),
+            (102, {}, {"x": 1, "y": 1}),
+        ),
+        (
+            (101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),
+            (102, {"x": 2}, {"x": 2, "y": 1}),
+        ),
+        (
+            (101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),
+            (102, {"x": 2, "y": 2}, {"x": 2, "y": 2}),
+        ),
+        (
+            (101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),
+            (102, {"x": 2, "z": 2}, {"x": 2, "y": 1, "z": 2}),
+        ),
+        (
+            (101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),
+            (102, {"x": 2, "z": 2}, {"x": 2, "y": 1, "z": 2}),
+            (102, {}, {"x": 1, "y": 1}),
+        ),
+        (
+            (101, {"x": 1, "y": 1}, {"x": 1, "y": 1}),
+            (102, {"x": 2, "z": 2}, {"x": 2, "y": 1, "z": 2}),
+            (103, {"x": 3}, {"x": 3, "y": 1, "z": 2}),
+            (101, {}, {"x": 3, "z": 2}),
+        ),
+    ],
+)
+def test_inventory_group_variables_update(updates: tuple[int, int | None, int | None]):
+    """
+    Test if the group vars are set correctly on various update sequences.
+    """
+    groupvars = InventoryGroupVariables("test_group")
+    for src_id, vars, expected_vars in updates:
+        groupvars.update_from_src(vars, src_id)
+        assert groupvars == expected_vars