Skip to content

Commit 4c37812

Browse files
authored
AAP-50805 Ignore role content types not known to system in sync (#811)
The bug here is that AWX `periodic_resource_sync` will result in an error. It seems there was an attempt to log an error and continue on when getting a `ValidationError`, it seems this only targeted the Django validation error exception, leaving the more common DRF ValidationError uncaught. This means that the issue broke the resource sync, globally. That means the periodic sync was effectively broken by the referenced bug. We could change that, but we wanted to have a targeted and more intentional special-case just for roles, and that is what this implements. If the field `RoleDefinition.content_type` references a type that does not exist locally, we don't bother in the _periodic fallback_ sync and just skip over that record. This is very sensible behavior, and it will be reported as a no-op, as one should expect.
1 parent a22e139 commit 4c37812

File tree

3 files changed

+55
-2
lines changed

3 files changed

+55
-2
lines changed

ansible_base/resource_registry/shared_types.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from rest_framework import serializers
2+
from rest_framework.exceptions import ValidationError
23

34
from ansible_base.rbac.models import DABContentType, DABPermission
45
from ansible_base.resource_registry.utils.resource_type_serializers import AnsibleResourceForeignKeyField, SharedResourceTypeSerializer
@@ -104,3 +105,15 @@ class RoleDefinitionType(SharedResourceTypeSerializer):
104105
default=None,
105106
)
106107
permissions = LenientPermissionSlugListField()
108+
109+
def is_valid(self, raise_exception=False):
110+
try:
111+
return super().is_valid(raise_exception=raise_exception)
112+
except ValidationError as e:
113+
if hasattr(e, 'detail') and 'content_type' in e.detail:
114+
fd_detail = e.detail['content_type'][0]
115+
if fd_detail.code == "does_not_exist":
116+
from ansible_base.resource_registry.tasks.sync import SkipResource
117+
118+
raise SkipResource(*e.args)
119+
raise

ansible_base/resource_registry/tasks/sync.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,13 @@ class ResourceSyncHTTPError(HTTPError):
3737
"""Custom catchall error"""
3838

3939

40+
class SkipResource(ValidationError):
41+
"""To raise by serializers if the item should be skipped"""
42+
43+
default_detail = "The specified content_type slug was not found."
44+
default_code = "content_type_does_not_exist"
45+
46+
4047
class SyncStatus(str, Enum):
4148
CREATED = "created"
4249
UPDATED = "updated"
@@ -226,6 +233,8 @@ def _attempt_create_resource(
226233
ansible_id=manifest_item.ansible_id,
227234
service_id=resource_service_id,
228235
)
236+
except SkipResource:
237+
return SyncResult(SyncStatus.NOOP, manifest_item)
229238
except IntegrityError:
230239
# This typically means that there was a duplicate key error. To mitigate this
231240
# we will attempt to hanlde the conflicting resource and perform the operation

test_app/tests/resource_registry/test_resource_sync.py

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,10 @@
77

88
from ansible_base.lib.testing.util import StaticResourceAPIClient
99
from ansible_base.lib.utils.response import get_relative_url
10-
from ansible_base.resource_registry.models import Resource
10+
from ansible_base.rbac.models import RoleDefinition
11+
from ansible_base.resource_registry.models import Resource, ResourceType
1112
from ansible_base.resource_registry.models.service_identifier import service_id
12-
from ansible_base.resource_registry.tasks.sync import ResourceSyncHTTPError, SyncExecutor
13+
from ansible_base.resource_registry.tasks.sync import ManifestItem, ResourceSyncHTTPError, SyncExecutor, _attempt_create_resource
1314

1415

1516
@pytest.fixture(scope="function")
@@ -178,6 +179,36 @@ def test_resource_sync_update_conflict(static_api_client, stdout, resource_to_up
178179
assert Resource.objects.get(ansible_id=new_id).name == "was_renamed"
179180

180181

182+
@pytest.mark.django_db
183+
def test_resource_sync_create_local_role_definition(static_api_client, stdout, resource_to_update):
184+
item_data = {"name": "Organization Inventory Role", "content_type": "shared.organization", "managed": True, "permissions": []}
185+
manifest_item = ManifestItem(str(uuid4()), str(uuid4()), item_data)
186+
result = _attempt_create_resource(
187+
manifest_item=manifest_item,
188+
resource_data=item_data,
189+
resource_type=ResourceType.objects.get(name='shared.roledefinition'),
190+
resource_service_id=str(uuid4()),
191+
api_client=static_api_client, # unused
192+
)
193+
assert result.status == 'created'
194+
195+
196+
@pytest.mark.django_db
197+
def test_resource_sync_create_non_local_role_definition(static_api_client, stdout, resource_to_update):
198+
item_data = {"name": "Remote Role", "content_type": "shared.foo_type", "managed": True, "permissions": []}
199+
manifest_item = ManifestItem(str(uuid4()), str(uuid4()), item_data)
200+
result = _attempt_create_resource(
201+
manifest_item=manifest_item,
202+
resource_data=item_data,
203+
resource_type=ResourceType.objects.get(name='shared.roledefinition'),
204+
resource_service_id=str(uuid4()),
205+
api_client=static_api_client, # unused
206+
)
207+
assert result.status == 'noop'
208+
209+
assert not RoleDefinition.objects.filter(name="Remote Role").exists()
210+
211+
181212
@pytest.mark.django_db
182213
def test_resource_sync_create_conflict(static_api_client, stdout, resource_to_update):
183214
# Update the ansible ID on the local resources so that it causes a conflict to happen.

0 commit comments

Comments
 (0)