diff --git a/awx/api/serializers.py b/awx/api/serializers.py index f3f37a03a1..ad081a34fd 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -126,6 +126,7 @@ from awx.main.utils import ( ) from awx.main.utils.filters import SmartFilter 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 @@ -1890,8 +1891,68 @@ class InventorySerializer(LabelsListMixin, BaseSerializerWithVariables, OpaQuery if kind == 'smart' and not host_filter: raise serializers.ValidationError({'host_filter': _('Smart inventories must specify host_filter')}) + return super(InventorySerializer, self).validate(attrs) + @staticmethod + def _update_variables(variables, inventory_id): + """ + Update the inventory variables of the 'all'-group. + + The variables field contains vars from the inventory dialog, hence + representing the "all"-group variables. + + Since this is not an update from an inventory source, we update the + variables when the inventory details form is saved. + + A user edit on the inventory variables is considered a reset of the + variables update history. Particularly if the user removes a variable by + editing the inventory variables field, the variable is not supposed to + reappear with a value from a previous inventory source update. + + We achieve this by forcing `reset=True` on such an update. + + As a side-effect, variables which have been set by source updates and + have survived a user-edit (i.e. they have not been deleted from the + variables field) will be assumed to originate from the user edit and are + thus no longer deleted from the inventory when they are removed from + their original source! + + Note that we use the inventory source id -1 for user-edit updates + because a regular inventory source cannot have an id of -1 since + PostgreSQL assigns pk's starting from 1 (if this assumption doesn't hold + true, we have to assign another special value for invsrc_id). + + :param str variables: The variables as plain text in yaml or json + format. + :param int inventory_id: The primary key of the related inventory + object. + """ + variables_dict = parse_yaml_or_json(variables, silent_failure=False) + logger.debug(f"InventorySerializer._update_variables: {inventory_id=} {variables_dict=}, {variables=}") + update_group_variables( + group_id=None, # `None` denotes the 'all' group (which doesn't have a pk). + newvars=variables_dict, + dbvars=None, + invsrc_id=-1, + inventory_id=inventory_id, + reset=True, + ) + + def create(self, validated_data): + """Called when a new inventory has to be created.""" + logger.debug(f"InventorySerializer.create({validated_data=}) >>>>") + obj = super().create(validated_data) + self._update_variables(validated_data.get("variables") or "", obj.id) + return obj + + def update(self, obj, validated_data): + """Called when an existing inventory is updated.""" + logger.debug(f"InventorySerializer.update({validated_data=}) >>>>") + obj = super().update(obj, validated_data) + self._update_variables(validated_data.get("variables") or "", obj.id) + return obj + class ConstructedFieldMixin(serializers.Field): def get_attribute(self, instance): @@ -2181,10 +2242,12 @@ class GroupSerializer(BaseSerializerWithVariables): return res def validate(self, attrs): + # Do not allow the group name to conflict with an existing host name. name = force_str(attrs.get('name', self.instance and self.instance.name or '')) 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) def validate_name(self, value): diff --git a/awx/main/management/commands/inventory_import.py b/awx/main/management/commands/inventory_import.py index 582af9d03b..4524b4d5e8 100644 --- a/awx/main/management/commands/inventory_import.py +++ b/awx/main/management/commands/inventory_import.py @@ -30,6 +30,7 @@ from awx.main.utils.safe_yaml import sanitize_jinja 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 from awx.main.utils.pglock import advisory_lock @@ -455,19 +456,19 @@ class Command(BaseCommand): """ 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_id=None, # `None` denotes the 'all' group (which doesn't have a pk). + newvars=self.all_group.variables, + dbvars=self.inventory.variables_dict, + invsrc_id=self.inventory_source.id, + inventory_id=self.inventory.id, + overwrite_vars=self.overwrite_vars, + ) if db_variables != self.inventory.variables_dict: self.inventory.variables = json.dumps(db_variables) self.inventory.save(update_fields=['variables']) diff --git a/awx/main/migrations/0199_inventorygroupvariableswithhistory_and_more.py b/awx/main/migrations/0199_inventorygroupvariableswithhistory_and_more.py new file mode 100644 index 0000000000..98e00925b6 --- /dev/null +++ b/awx/main/migrations/0199_inventorygroupvariableswithhistory_and_more.py @@ -0,0 +1,32 @@ +# Generated by Django 4.2.20 on 2025-04-24 09:08 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0198_alter_inventorysource_source_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='InventoryGroupVariablesWithHistory', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('variables', models.JSONField()), + ('group', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='inventory_group_variables', to='main.group')), + ( + 'inventory', + models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, related_name='inventory_group_variables', to='main.inventory'), + ), + ], + ), + migrations.AddConstraint( + model_name='inventorygroupvariableswithhistory', + constraint=models.UniqueConstraint( + fields=('inventory', 'group'), name='unique_inventory_group', violation_error_message='Inventory/Group combination must be unique.' + ), + ), + ] diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index a799b077f3..54eaab7c59 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -33,6 +33,7 @@ from awx.main.models.inventory import ( # noqa InventorySource, InventoryUpdate, SmartInventoryMembership, + InventoryGroupVariablesWithHistory, ) from awx.main.models.jobs import ( # noqa Job, diff --git a/awx/main/models/inventory.py b/awx/main/models/inventory.py index 56ec5faf76..44fc0d3f2c 100644 --- a/awx/main/models/inventory.py +++ b/awx/main/models/inventory.py @@ -1745,3 +1745,38 @@ class constructed(PluginFileInjector): for cls in PluginFileInjector.__subclasses__(): InventorySourceOptions.injectors[cls.__name__] = cls + + +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. + """ + + class Meta: + constraints = [ + # Do not allow the same inventory/group combination more than once. + models.UniqueConstraint( + fields=["inventory", "group"], + name="unique_inventory_group", + violation_error_message=_("Inventory/Group combination must be unique."), + ), + ] + + inventory = models.ForeignKey( + 'Inventory', + related_name='inventory_group_variables', + null=True, + on_delete=models.CASCADE, + ) + group = models.ForeignKey( # `None` denotes the 'all'-group. + 'Group', + related_name='inventory_group_variables', + null=True, + on_delete=models.CASCADE, + ) + variables = models.JSONField() # The group variables, including their history. diff --git a/awx/main/tests/data/projects/inventory_vars/inventory_var_deleted_in_source.ini b/awx/main/tests/data/projects/inventory_vars/inventory_var_deleted_in_source.ini new file mode 100644 index 0000000000..8dfebcd502 --- /dev/null +++ b/awx/main/tests/data/projects/inventory_vars/inventory_var_deleted_in_source.ini @@ -0,0 +1,3 @@ +[all:vars] +a=value_a +b=value_b diff --git a/awx/main/tests/functional/api/test_inventory.py b/awx/main/tests/functional/api/test_inventory.py index 829662cc2f..9924af82ba 100644 --- a/awx/main/tests/functional/api/test_inventory.py +++ b/awx/main/tests/functional/api/test_inventory.py @@ -8,6 +8,7 @@ from django.core.exceptions import ValidationError from awx.api.versioning import reverse from awx.main.models import InventorySource, Inventory, ActivityStream +from awx.main.utils.inventory_vars import update_group_variables @pytest.fixture @@ -712,3 +713,241 @@ class TestConstructedInventory: assert inv_r.data['url'] != const_r.data['url'] assert inv_r.data['related']['constructed_url'] == url_const assert const_r.data['related']['constructed_url'] == url_const + + +@pytest.mark.django_db +class TestInventoryAllVariables: + + @staticmethod + def simulate_update_from_source(inv_src, variables_dict, overwrite_vars=True): + """ + Update `inventory` with variables `variables_dict` from source + `inv_src`. + """ + # Perform an update from source the same way it is done in + # `inventory_import.Command._update_inventory`. + new_vars = update_group_variables( + group_id=None, # `None` denotes the 'all' group (which doesn't have a pk). + newvars=variables_dict, + dbvars=inv_src.inventory.variables_dict, + invsrc_id=inv_src.id, + inventory_id=inv_src.inventory.id, + overwrite_vars=overwrite_vars, + ) + inv_src.inventory.variables = json.dumps(new_vars) + inv_src.inventory.save(update_fields=["variables"]) + return new_vars + + def update_and_verify(self, inv_src, new_vars, expect=None, overwrite_vars=True, teststep=None): + """ + Helper: Update from source and verify the new inventory variables. + + :param inv_src: An inventory source object with its inventory property + set to the inventory fixture of the called. + :param dict new_vars: The variables of the inventory source `inv_src`. + :param dict expect: (optional) The expected variables state of the + inventory after the update. If not set or None, expect `new_vars`. + :param bool overwrite_vars: The status of the inventory source option + 'overwrite variables'. Default is `True`. + :raise AssertionError: If the inventory does not contain the expected + variables after the update. + """ + self.simulate_update_from_source(inv_src, new_vars, overwrite_vars=overwrite_vars) + if teststep is not None: + assert inv_src.inventory.variables_dict == (expect if expect is not None else new_vars), f"Test step {teststep}" + else: + assert inv_src.inventory.variables_dict == (expect if expect is not None else new_vars) + + def test_set_variables_through_inventory_details_update(self, inventory, patch, admin_user): + """ + Set an inventory variable by changing the inventory details, simulating + a user edit. + """ + # a: x + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'a: x'}, user=admin_user, expect=200) + inventory.refresh_from_db() + assert inventory.variables_dict == {"a": "x"} + + def test_variables_set_by_user_persist_update_from_src(self, inventory, inventory_source, patch, admin_user): + """ + Verify the special behavior that a variable which originates from a user + edit (instead of a source update), is not removed from the inventory + when a source update with overwrite_vars=True does not contain that + variable. This behavior is considered special because a variable which + originates from a source would actually be deleted. + + In addition, verify that an existing variable which was set by a user + edit can be overwritten by a source update. + """ + # Set two variables via user edit. + patch( + url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), + data={'variables': '{"a": "a_from_user", "b": "b_from_user"}'}, + user=admin_user, + expect=200, + ) + inventory.refresh_from_db() + assert inventory.variables_dict == {'a': 'a_from_user', 'b': 'b_from_user'} + # Update from a source which contains only one of the two variables from + # the previous update. + self.simulate_update_from_source(inventory_source, {'a': 'a_from_source'}) + # Verify inventory variables. + assert inventory.variables_dict == {'a': 'a_from_source', 'b': 'b_from_user'} + + def test_variables_set_through_src_get_removed_on_update_from_same_src(self, inventory, inventory_source, patch, admin_user): + """ + Verify that a variable which originates from a source update, is removed + from the inventory when a source update with overwrite_vars=True does + not contain that variable. + + In addition, verify that an existing variable which was set by a user + edit can be overwritten by a source update. + """ + # Set two variables via update from source. + self.simulate_update_from_source(inventory_source, {'a': 'a_from_source', 'b': 'b_from_source'}) + # Verify inventory variables. + assert inventory.variables_dict == {'a': 'a_from_source', 'b': 'b_from_source'} + # Update from the same source which now contains only one of the two + # variables from the previous update. + self.simulate_update_from_source(inventory_source, {'b': 'b_from_source'}) + # Verify the variable has been deleted from the inventory. + assert inventory.variables_dict == {'b': 'b_from_source'} + + def test_overwrite_variables_through_inventory_details_update(self, inventory, patch, admin_user): + """ + Set and update the inventory variables multiple times by changing the + inventory details via api, simulating user edits. + + Any variables update by means of an inventory details update shall + overwright all existing inventory variables. + """ + # a: x + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'a: x'}, user=admin_user, expect=200) + inventory.refresh_from_db() + assert inventory.variables_dict == {"a": "x"} + # a: x2 + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'a: x2'}, user=admin_user, expect=200) + inventory.refresh_from_db() + assert inventory.variables_dict == {"a": "x2"} + # b: y + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'b: y'}, user=admin_user, expect=200) + inventory.refresh_from_db() + assert inventory.variables_dict == {"b": "y"} + + def test_inventory_group_variables_internal_data(self, inventory, patch, admin_user): + """ + Basic verification of how variable updates are stored internally. + + .. Warning:: + + This test verifies a specific implementation of the inventory + variables update business logic. It may deliver false negatives if + the implementation changes. + """ + # x: a + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'a: x'}, user=admin_user, expect=200) + igv = inventory.inventory_group_variables.first() + assert igv.variables == {'a': [[-1, 'x']]} + # b: y + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'b: y'}, user=admin_user, expect=200) + igv = inventory.inventory_group_variables.first() + assert igv.variables == {'b': [[-1, 'y']]} + + def test_update_then_user_change(self, inventory, patch, admin_user, inventory_source): + """ + 1. Update inventory vars by means of an inventory source update. + 2. Update inventory vars by editing the inventory details (aka a 'user + update'), thereby changing variables values and deleting variables + from the inventory. + + .. Warning:: + + This test partly relies on a specific implementation of the + inventory variables update business logic. It may deliver false + negatives if the implementation changes. + """ + assert inventory_source.inventory_id == inventory.pk # sanity + # ---- Test step 1: Set variables by updating from an inventory source. + self.simulate_update_from_source(inventory_source, {'foo': 'foo_from_source', 'bar': 'bar_from_source'}) + # Verify inventory variables. + assert inventory.variables_dict == {'foo': 'foo_from_source', 'bar': 'bar_from_source'} + # Verify internal storage of variables data. Note that this is + # implementation specific + assert inventory.inventory_group_variables.count() == 1 + igv = inventory.inventory_group_variables.first() + assert igv.variables == {'foo': [[inventory_source.id, 'foo_from_source']], 'bar': [[inventory_source.id, 'bar_from_source']]} + # ---- Test step 2: Change the variables by editing the inventory details. + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'foo: foo_from_user'}, user=admin_user, expect=200) + inventory.refresh_from_db() + # Verify that variable `foo` contains the new value, and that variable + # `bar` has been deleted from the inventory. + assert inventory.variables_dict == {"foo": "foo_from_user"} + # Verify internal storage of variables data. Note that this is + # implementation specific + inventory.inventory_group_variables.count() == 1 + igv = inventory.inventory_group_variables.first() + assert igv.variables == {'foo': [[-1, 'foo_from_user']]} + + def test_monotonic_deletions(self, inventory, patch, admin_user): + """ + Verify the variables history logic for monotonic deletions. + + Monotonic in this context means that the variables are deleted in the + reverse order of their creation. + + 1. Set inventory variable x: 0, expect INV={x: 0} + + (The following steps use overwrite_variables=False) + + 2. Update from source A={x: 1}, expect INV={x: 1} + 3. Update from source B={x: 2}, expect INV={x: 2} + 4. Update from source B={}, expect INV={x: 1} + 5. Update from source A={}, expect INV={x: 0} + """ + inv_src_a = InventorySource.objects.create(name="inv-src-A", inventory=inventory, source="ec2") + inv_src_b = InventorySource.objects.create(name="inv-src-B", inventory=inventory, source="ec2") + # Test step 1: + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'x: 0'}, user=admin_user, expect=200) + inventory.refresh_from_db() + assert inventory.variables_dict == {"x": 0} + # Test step 2: Source A overwrites value of var x + self.update_and_verify(inv_src_a, {"x": 1}, teststep=2) + # Test step 3: Source A overwrites value of var x + self.update_and_verify(inv_src_b, {"x": 2}, teststep=3) + # Test step 4: Value of var x from source A reappears + self.update_and_verify(inv_src_b, {}, expect={"x": 1}, teststep=4) + # Test step 5: Value of var x from initial user edit reappears + self.update_and_verify(inv_src_a, {}, expect={"x": 0}, teststep=5) + + def test_interleaved_deletions(self, inventory, patch, admin_user, inventory_source): + """ + Verify the variables history logic for interleaved deletions. + + Interleaved in this context means that the variables are deleted in a + different order than the sequence of their creation. + + 1. Set inventory variable x: 0, expect INV={x: 0} + 2. Update from source A={x: 1}, expect INV={x: 1} + 3. Update from source B={x: 2}, expect INV={x: 2} + 4. Update from source C={x: 3}, expect INV={x: 3} + 5. Update from source B={}, expect INV={x: 3} + 6. Update from source C={}, expect INV={x: 1} + """ + inv_src_a = InventorySource.objects.create(name="inv-src-A", inventory=inventory, source="ec2") + inv_src_b = InventorySource.objects.create(name="inv-src-B", inventory=inventory, source="ec2") + inv_src_c = InventorySource.objects.create(name="inv-src-C", inventory=inventory, source="ec2") + # Test step 1. Set inventory variable x: 0 + patch(url=reverse('api:inventory_detail', kwargs={'pk': inventory.pk}), data={'variables': 'x: 0'}, user=admin_user, expect=200) + inventory.refresh_from_db() + assert inventory.variables_dict == {"x": 0} + # Test step 2: Source A overwrites value of var x + self.update_and_verify(inv_src_a, {"x": 1}, teststep=2) + # Test step 3: Source B overwrites value of var x + self.update_and_verify(inv_src_b, {"x": 2}, teststep=3) + # Test step 4: Source C overwrites value of var x + self.update_and_verify(inv_src_c, {"x": 3}, teststep=4) + # Test step 5: Value of var x from source C remains unchanged + self.update_and_verify(inv_src_b, {}, expect={"x": 3}, teststep=5) + # Test step 6: Value of var x from source A reappears, because the + # latest update from source B did not contain var x. + self.update_and_verify(inv_src_c, {}, expect={"x": 1}, teststep=6) diff --git a/awx/main/tests/live/tests/test_inventory_vars.py b/awx/main/tests/live/tests/test_inventory_vars.py new file mode 100644 index 0000000000..c574d86a56 --- /dev/null +++ b/awx/main/tests/live/tests/test_inventory_vars.py @@ -0,0 +1,224 @@ +import subprocess +import time +import os.path +from urllib.parse import urlsplit + +import pytest +from unittest import mock + +from awx.main.models.projects import Project +from awx.main.models.organization import Organization +from awx.main.models.inventory import Inventory, InventorySource +from awx.main.tests.live.tests.conftest import wait_for_job + + +NAME_PREFIX = "test-ivu" +GIT_REPO_FOLDER = "inventory_vars" + + +def create_new_by_name(model, **kwargs): + """ + Create a new model instance. Delete an existing instance first. + + :param model: The Django model. + :param dict kwargs: The keyword arguments required to create a model + instance. Must contain at least `name`. + :return: The model instance. + """ + name = kwargs["name"] + try: + instance = model.objects.get(name=name) + except model.DoesNotExist: + pass + else: + print(f"FORCE DELETE {name}") + instance.delete() + finally: + instance = model.objects.create(**kwargs) + return instance + + +def wait_for_update(instance, timeout=3.0): + """Wait until the last update of *instance* is finished.""" + start = time.time() + while time.time() - start < timeout: + if instance.current_job or instance.last_job or instance.last_job_run: + break + time.sleep(0.2) + assert instance.current_job or instance.last_job or instance.last_job_run, f'Instance never updated id={instance.id}' + update = instance.current_job or instance.last_job + if update: + wait_for_job(update) + + +def change_source_vars_and_update(invsrc, group_vars): + """ + Change the variables content of an inventory source and update its + inventory. + + Does not return before the inventory update is finished. + + :param invsrc: The inventory source instance. + :param dict group_vars: The variables for various groups. Format:: + + { + : {: , : , ..}, : + {: , : , ..}, .. + } + + :return: None + """ + project = invsrc.source_project + repo_path = urlsplit(project.scm_url).path + filepath = os.path.join(repo_path, invsrc.source_path) + # print(f"change_source_vars_and_update: {project=} {repo_path=} {filepath=}") + with open(filepath, "w") as fp: + for group, variables in group_vars.items(): + fp.write(f"[{group}:vars]\n") + for name, value in variables.items(): + fp.write(f"{name}={value}\n") + subprocess.run('git add .; git commit -m "Update variables in invsrc.source_path"', cwd=repo_path, shell=True) + # Update the project to sync the changed repo contents. + project.update() + wait_for_update(project) + # Update the inventory from the changed source. + invsrc.update() + wait_for_update(invsrc) + + +@pytest.fixture +def organization(): + name = f"{NAME_PREFIX}-org" + instance = create_new_by_name(Organization, name=name, description=f"Description for {name}") + yield instance + instance.delete() + + +@pytest.fixture +def project(organization, live_tmp_folder): + name = f"{NAME_PREFIX}-project" + instance = create_new_by_name( + Project, + name=name, + description=f"Description for {name}", + organization=organization, + scm_url=f"file://{live_tmp_folder}/{GIT_REPO_FOLDER}", + scm_type="git", + ) + yield instance + instance.delete() + + +@pytest.fixture +def inventory(organization): + name = f"{NAME_PREFIX}-inventory" + instance = create_new_by_name( + Inventory, + name=name, + description=f"Description for {name}", + organization=organization, + ) + yield instance + instance.delete() + + +@pytest.fixture +def inventory_source(inventory, project): + name = f"{NAME_PREFIX}-invsrc" + inv_src = InventorySource( + name=name, + source_project=project, + source="scm", + source_path="inventory_var_deleted_in_source.ini", + inventory=inventory, + overwrite_vars=True, + ) + with mock.patch('awx.main.models.unified_jobs.UnifiedJobTemplate.update'): + inv_src.save() + yield inv_src + inv_src.delete() + + +@pytest.fixture +def inventory_source_factory(inventory, project): + """ + Use this fixture if you want to use multiple inventory sources for the same + inventory in your test. + """ + # https://docs.pytest.org/en/stable/how-to/fixtures.html#factories-as-fixtures + + created = [] + # repo_path = f"{live_tmp_folder}/{GIT_REPO_FOLDER}" + + def _factory(inventory_file, name): + # Make sure the inventory file exists before the inventory source + # instance is created. + # + # Note: The current implementation of the inventory source object allows + # to create an instance even when the inventory source file does not + # exist. If this behaviour changes, uncomment the following code block + # and add the fixture `live_tmp_folder` to the factory function + # signature. + # + # inventory_file_path = os.path.join(repo_path, inventory_file) if not + # os.path.isfile(inventory_file_path): with open(inventory_file_path, + # "w") as fp: pass subprocess.run(f'git add .; git commit -m "Create + # {inventory_file_path}"', cwd=repo_path, shell=True) + # + # Create the inventory source instance. + name = f"{NAME_PREFIX}-invsrc-{name}" + inv_src = InventorySource( + name=name, + source_project=project, + source="scm", + source_path=inventory_file, + inventory=inventory, + overwrite_vars=True, + ) + with mock.patch('awx.main.models.unified_jobs.UnifiedJobTemplate.update'): + inv_src.save() + return inv_src + + yield _factory + for instance in created: + instance.delete() + + +def test_inventory_var_deleted_in_source(inventory, inventory_source): + """ + Verify that a variable which is deleted from its (git-)source between two + updates is also deleted from the inventory. + + Verifies https://issues.redhat.com/browse/AAP-17690 + """ + inventory_source.update() + wait_for_update(inventory_source) + assert {"a": "value_a", "b": "value_b"} == Inventory.objects.get(name=inventory.name).variables_dict + # Remove variable `a` from source and verify that it is also removed from + # the inventory variables. + change_source_vars_and_update(inventory_source, {"all": {"b": "value_b"}}) + assert {"b": "value_b"} == Inventory.objects.get(name=inventory.name).variables_dict + + +def test_inventory_vars_with_multiple_sources(inventory, inventory_source_factory): + """ + Verify a sequence of updates from various sources with changing content. + """ + invsrc_a = inventory_source_factory("invsrc_a.ini", "A") + invsrc_b = inventory_source_factory("invsrc_b.ini", "B") + invsrc_c = inventory_source_factory("invsrc_c.ini", "C") + + change_source_vars_and_update(invsrc_a, {"all": {"x": "x_from_a", "y": "y_from_a"}}) + assert {"x": "x_from_a", "y": "y_from_a"} == Inventory.objects.get(name=inventory.name).variables_dict + change_source_vars_and_update(invsrc_b, {"all": {"x": "x_from_b", "y": "y_from_b", "z": "z_from_b"}}) + assert {"x": "x_from_b", "y": "y_from_b", "z": "z_from_b"} == Inventory.objects.get(name=inventory.name).variables_dict + change_source_vars_and_update(invsrc_c, {"all": {"x": "x_from_c", "z": "z_from_c"}}) + assert {"x": "x_from_c", "y": "y_from_b", "z": "z_from_c"} == Inventory.objects.get(name=inventory.name).variables_dict + change_source_vars_and_update(invsrc_b, {"all": {}}) + assert {"x": "x_from_c", "y": "y_from_a", "z": "z_from_c"} == Inventory.objects.get(name=inventory.name).variables_dict + change_source_vars_and_update(invsrc_c, {"all": {"z": "z_from_c"}}) + assert {"x": "x_from_a", "y": "y_from_a", "z": "z_from_c"} == Inventory.objects.get(name=inventory.name).variables_dict + change_source_vars_and_update(invsrc_a, {"all": {}}) + assert {"z": "z_from_c"} == Inventory.objects.get(name=inventory.name).variables_dict + change_source_vars_and_update(invsrc_c, {"all": {}}) + assert {} == Inventory.objects.get(name=inventory.name).variables_dict diff --git a/awx/main/tests/unit/utils/test_inventory_vars.py b/awx/main/tests/unit/utils/test_inventory_vars.py new file mode 100644 index 0000000000..8ba55c900e --- /dev/null +++ b/awx/main/tests/unit/utils/test_inventory_vars.py @@ -0,0 +1,110 @@ +""" +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.delete(102) + assert str(x) == "3" + x.delete(103) + assert str(x) == "1" + x.delete(101) + assert x.value is None + assert x.has_no_source + + +@pytest.mark.parametrize( + "updates", # (, , ) + [ + ((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. + + For this test, the value `None` implies the deletion of the source. + """ + x = InventoryVariable("x") + for src_id, value, expected_value in updates: + if value is None: + x.delete(src_id) + else: + 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(1) + vars.update_from_src({"x": 1, "y": 2}, 101) + assert vars == {"x": 1, "y": 2} + + +@pytest.mark.parametrize( + "updates", # (, : dict, : 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(2) + for src_id, vars, expected_vars in updates: + groupvars.update_from_src(vars, src_id) + assert groupvars == expected_vars diff --git a/awx/main/utils/inventory_vars.py b/awx/main/utils/inventory_vars.py new file mode 100644 index 0000000000..37d9e08f1c --- /dev/null +++ b/awx/main/utils/inventory_vars.py @@ -0,0 +1,277 @@ +import logging +from typing import TypeAlias, Any + +from awx.main.models import InventoryGroupVariablesWithHistory + + +var_value: TypeAlias = Any +update_queue: TypeAlias = list[tuple[int, var_value]] + + +logger = logging.getLogger('awx.api.inventory_import') + + +class InventoryVariable: + """ + Represents an inventory variable. + + This class keeps track of the variable updates from different inventory + sources. + """ + + def __init__(self, name: str) -> None: + """ + :param str name: The variable's name. + :return: None + """ + self.name = name + self._update_queue: update_queue = [] + """ + A queue representing updates from inventory sources in the sequence of + occurrence. + + The queue is realized as a list of two-tuples containing variable values + and their originating inventory source. The last item of the list is + considered the top of the queue, and holds the current value of the + variable. + """ + + def reset(self) -> None: + """Reset the variable by deleting its history.""" + self._update_queue = [] + + def load(self, updates: update_queue) -> "InventoryVariable": + """Load internal state from a list.""" + self._update_queue = updates + return self + + def dump(self) -> update_queue: + """Save internal state to a list.""" + return self._update_queue + + def update(self, value: var_value, invsrc_id: int) -> None: + """ + Update the variable with a new value from an inventory source. + + Updating means that this source is moved to the top of the queue + and `value` becomes the new current value. + + :param value: The new value of the variable. + :param int invsrc_id: The inventory source of the new variable value. + :return: None + """ + logger.debug(f"InventoryVariable().update({value}, {invsrc_id}):") + # Move this source to the front of the queue by first deleting a + # possibly existing entry, and then add the new entry to the front. + self.delete(invsrc_id) + self._update_queue.append((invsrc_id, value)) + + def delete(self, invsrc_id: int) -> None: + """ + Delete an inventory source from the variable. + + :param int invsrc_id: The inventory source id. + :return: None + """ + data_index = self._get_invsrc_index(invsrc_id) + # Remove last update from this source, if there was any. + if data_index is not None: + value = self._update_queue.pop(data_index)[1] + logger.debug(f"InventoryVariable().delete({invsrc_id}): {data_index=} {value=}") + + def _get_invsrc_index(self, invsrc_id: int) -> int | None: + """Return the inventory source's position in the queue, or `None`.""" + for i, entry in enumerate(self._update_queue): + if entry[0] == invsrc_id: + return i + return None + + def _get_current_value(self) -> var_value: + """ + Return the current value of the variable, or None if the variable has no + history. + """ + return self._update_queue[-1][1] if self._update_queue else None + + @property + def value(self) -> var_value: + """Read the current value of the variable.""" + return self._get_current_value() + + @property + def has_no_source(self) -> bool: + """True, if the variable is orphan, i.e. no source contains this var anymore.""" + return not self._update_queue + + def __str__(self): + """Return the string representation of the current value.""" + return str(self.value or "") + + +class InventoryGroupVariables(dict): + """ + Represent all inventory variables from one group. + + This dict contains all variables of a inventory group and their current + value under consideration of the inventory source update history. + + Note that variables values cannot be `None`, use the empty string to + indicate that a variable holds no value. See also `InventoryVariable`. + """ + + def __init__(self, id: int) -> None: + """ + :param int id: The id of the group object. + :return: None + """ + super().__init__() + self.id = id + # In _vars we keep all sources for a given variable. This enables us to + # find the current value for a variable, which is the value from the + # latest update which defined this variable. + self._vars: dict[str, InventoryVariable] = {} + + def _sync_vars(self) -> None: + """ + Copy the current values of all variables into the internal dict. + + Call this everytime the `_vars` structure has been modified. + """ + for name, inv_var in self._vars.items(): + self[name] = inv_var.value + + def load_state(self, state: dict[str, update_queue]) -> "InventoryGroupVariables": + """Load internal state from a dict.""" + for name, updates in state.items(): + self._vars[name] = InventoryVariable(name).load(updates) + self._sync_vars() + return self + + def save_state(self) -> dict[str, update_queue]: + """Return internal state as a dict.""" + state = {} + for name, inv_var in self._vars.items(): + state[name] = inv_var.dump() + return state + + def update_from_src( + self, + new_vars: dict[str, var_value], + source_id: int, + overwrite_vars: bool = True, + reset: bool = False, + ) -> None: + """ + Update with variables from an inventory source. + + Delete all variables for this source which are not in the update vars. + + :param dict new_vars: The variables from the inventory source. + :param int invsrc_id: The id of the inventory source for this update. + :param bool overwrite_vars: If `True`, delete this source's history + entry for variables which are not in this update. If `False`, keep + the old updates in the history for such variables. Default is + `True`. + :param bool reset: If `True`, delete the update history for all existing + variables before updating the new vars. Therewith making this update + overwrite all history. Default is `False`. + :return: None + """ + logger.debug(f"InventoryGroupVariables({self.id}).update_from_src({new_vars=}, {source_id=}, {overwrite_vars=}, {reset=}): {self=}") + # Create variables which are newly introduced by this source. + for name in new_vars: + if name not in self._vars: + self._vars[name] = InventoryVariable(name) + # Combine the names of the existing vars and the new vars from this update. + all_var_names = list(set(list(self.keys()) + list(new_vars.keys()))) + # In reset-mode, delete all existing vars and their history before + # updating. + if reset: + for name in all_var_names: + self._vars[name].reset() + # Go through all variables (the existing ones, and the ones added by + # this update), delete this source from variables which are not in this + # update, and update the value of variables which are part of this + # update. + for name in all_var_names: + # Update or delete source from var (if name not in vars). + if name in new_vars: + self._vars[name].update(new_vars[name], source_id) + elif overwrite_vars: + self._vars[name].delete(source_id) + # Delete vars which have no source anymore. + if self._vars[name].has_no_source: + del self._vars[name] + del self[name] + # After the update, refresh the internal dict with the possibly changed + # current values. + self._sync_vars() + logger.debug(f"InventoryGroupVariables({self.id}).update_from_src(): {self=}") + + +def update_group_variables( + group_id: int | None, + newvars: dict, + dbvars: dict | None, + invsrc_id: int, + inventory_id: int, + overwrite_vars: bool = True, + reset: bool = False, +) -> dict[str, var_value]: + """ + Update the inventory variables of one group. + + Merge the new variables into the existing group variables. + + The update can be triggered either by an inventory update via API, or via a + manual edit of the variables field in the awx inventory form. + + TODO: Can we get rid of the dbvars? This is only needed because the new + update-var mechanism needs to be properly initialized if the db already + contains some variables. + + :param int group_id: The inventory group id (pk). For the 'all'-group use + `None`, because this group is not an actual `Group` object in the + database. + :param dict newvars: The variables contained in this update. + :param dict dbvars: The variables which are already stored in the database + for this inventory and this group. Can be `None`. + :param int invsrc_id: The id of the inventory source. Usually this is the + database primary key of the inventory source object, but there is one + special id -1 which is used for the initial update from the database and + for manual updates via the GUI. + :param int inventory_id: The id of the inventory on which this update is + applied. + :param bool overwrite_vars: If `True`, delete variables which were merged + from the same source in a previous update, but are no longer contained + in that source. If `False`, such variables would not be removed from the + group. Default is `True`. + :param bool reset: If `True`, delete all variables from previous updates, + therewith making this update overwrite all history. Default is `False`. + :return: The variables and their current values as a dict. + :rtype: dict + """ + inv_group_vars = InventoryGroupVariables(group_id) + # Restore the existing variables state. + try: + # Get the object for this group from the database. + model = InventoryGroupVariablesWithHistory.objects.get(inventory_id=inventory_id, group_id=group_id) + except InventoryGroupVariablesWithHistory.DoesNotExist: + # If no previous state exists, create a new database object, and + # initialize it with the current group variables. + model = InventoryGroupVariablesWithHistory(inventory_id=inventory_id, group_id=group_id) + if dbvars: + inv_group_vars.update_from_src(dbvars, -1) # Assume -1 as inv_source_id for existing vars. + else: + # Load the group variables state from the database object. + inv_group_vars.load_state(model.variables) + # + logger.debug(f"update_group_variables: before update_from_src {model.variables=}") + # Apply the new inventory update onto the group variables. + inv_group_vars.update_from_src(newvars, invsrc_id, overwrite_vars, reset) + # Save the new variables state. + model.variables = inv_group_vars.save_state() + model.save() + logger.debug(f"update_group_variables: after update_from_src {model.variables=}") + logger.debug(f"update_group_variables({group_id=}, {newvars}): {inv_group_vars}") + return inv_group_vars