diff --git a/ansible_base/resource_registry/signals/handlers.py b/ansible_base/resource_registry/signals/handlers.py index 0c02dfd0f..a7ee5b4eb 100644 --- a/ansible_base/resource_registry/signals/handlers.py +++ b/ansible_base/resource_registry/signals/handlers.py @@ -50,14 +50,12 @@ def decide_to_sync_update(sender, instance, raw, using, update_fields, **kwargs) return try: - if not getattr(instance, 'resource', None) or not instance.resource.ansible_id: - # We can't sync here, but we want to log that, so let sync_to_resource_server() discard it. - return + resource = Resource.get_resource_for_object(instance) except Resource.DoesNotExist: - # The getattr() will raise a Resource.DoesNotExist if the resource doesn't exist. + # We can't sync here, but we want to log that, so let sync_to_resource_server() discard it. return - fields_that_sync = instance.resource.content_type.resource_type.serializer_class().get_fields().keys() + fields_that_sync = resource.content_type.resource_type.serializer_class().get_fields().keys() if update_fields is None: # If we're not given a useful update_fields, manually calculate the changed fields diff --git a/ansible_base/resource_registry/utils/sync_to_resource_server.py b/ansible_base/resource_registry/utils/sync_to_resource_server.py index e6d30fa55..13dbc5260 100644 --- a/ansible_base/resource_registry/utils/sync_to_resource_server.py +++ b/ansible_base/resource_registry/utils/sync_to_resource_server.py @@ -6,7 +6,7 @@ from django.utils.translation import gettext_lazy as _ from rest_framework.exceptions import ValidationError -from ansible_base.resource_registry.models import Resource +from ansible_base.resource_registry.models import Resource, service_id from ansible_base.resource_registry.rest_client import ResourceRequestBody, get_resource_server_client logger = logging.getLogger('ansible_base.resource_registry.utils.sync_to_resource_server') @@ -39,20 +39,22 @@ def sync_to_resource_server(instance, action, ansible_id=None): logger.info(f"Skipping sync of resource {instance}") return + if action != "delete" and ansible_id is not None: + raise Exception("ansible_id should not be provided for create/update actions") + elif action == "delete" and ansible_id is None: + raise Exception("ansible_id should be provided for delete actions") + try: - if action != "delete" and ansible_id is not None: - raise Exception("ansible_id should not be provided for create/update actions") - elif action == "delete" and ansible_id is None: - raise Exception("ansible_id should be provided for delete actions") - elif not getattr(instance, 'resource', None) or not instance.resource.ansible_id: - # We can't sync if we don't have a resource and an ansible_id. - logger.error(f"Resource {instance} does not have a resource or ansible_id") - return + resource = Resource.get_resource_for_object(instance) except Resource.DoesNotExist: - # The getattr() will raise a Resource.DoesNotExist if the resource doesn't exist. logger.error(f"Resource {instance} does not have a resource") return + if str(resource.service_id) == service_id() and action == "update": + # Don't sync if we're updating a resource that isn't owned by the resource server yet. + logger.info(f"Skipping sync of resource {instance} because its service_id is local") + return + user_ansible_id = None user = get_current_user() if user: @@ -60,8 +62,9 @@ def sync_to_resource_server(instance, action, ansible_id=None): # If they don't have one some how, or if we don't have a user, sync with None and # let the resource server decide what to do. try: - user_ansible_id = user.resource.ansible_id - except AttributeError: + user_resource = Resource.get_resource_for_object(user) + user_ansible_id = user_resource.ansible_id + except (Resource.DoesNotExist, AttributeError): logger.error(f"User {user} does not have a resource") pass else: @@ -74,9 +77,9 @@ def sync_to_resource_server(instance, action, ansible_id=None): ) if action != "delete": - ansible_id = instance.resource.ansible_id + ansible_id = resource.ansible_id - resource_type = instance.resource.content_type.resource_type + resource_type = resource.content_type.resource_type data = resource_type.serializer_class(instance).data body = ResourceRequestBody( resource_type=resource_type.name, @@ -89,8 +92,9 @@ def sync_to_resource_server(instance, action, ansible_id=None): response = client.create_resource(body) json = response.json() if isinstance(json, dict): # Mainly for tests... to avoid getting here with mock - instance.resource.service_id = json['service_id'] - instance.resource.save() + resource.service_id = json['service_id'] + resource.ansible_id = json['ansible_id'] + resource.save() elif action == "update": client.update_resource(ansible_id, body) elif action == "delete": diff --git a/test_app/tests/resource_registry/test_utils.py b/test_app/tests/resource_registry/test_utils.py index d0637f001..e1ebc4dec 100644 --- a/test_app/tests/resource_registry/test_utils.py +++ b/test_app/tests/resource_registry/test_utils.py @@ -1,4 +1,5 @@ import os +import uuid from contextlib import nullcontext from unittest import mock @@ -49,6 +50,9 @@ def test_sync_to_resource_server_happy_path(self, user, action, enable_reverse_s if action == 'create': org = Organization.objects.create(name='Hello') elif action == 'update': + # Fake the service_id to not be local + org.resource.service_id = uuid.uuid4() + org.resource.save() org.name = 'World' org.save() elif action == 'delete': @@ -102,6 +106,20 @@ def test_sync_to_resource_server_no_resource(self, user, nullify_resource, enabl # We bail out if we don't have a resource get_resource_server_client.assert_not_called() + @pytest.mark.django_db + def test_sync_to_resource_server_update_with_local_service_id(self, user, organization, enable_reverse_sync, expected_log): + """ + If the resource's service_id is the local service_id, we should not attempt to sync it on update. + """ + with enable_reverse_sync(): + with mock.patch(f'{utils_path}.get_resource_server_client') as get_resource_server_client: + with impersonate(user): + organization.name = 'World' + with expected_log(f'{utils_path}.logger', 'info', 'service_id is local'): + organization.save() + + get_resource_server_client.assert_not_called() + @pytest.mark.django_db(transaction=True) @pytest.mark.parametrize( 'ansible_reverse_resource_sync, should_sync',