From dad4da74b92ccaf9e5878c957dcb1882f23aa1ee Mon Sep 17 00:00:00 2001 From: Dirk Julich <202084408+djulich@users.noreply.github.com> Date: Wed, 23 Sep 2026 17:34:29 +0200 Subject: [PATCH] Fix 500 on /api/v2/schedules/ from orphaned token cleanup schedule (#16660) * Fix 500 error on /api/v2/schedules/ from orphaned token cleanup schedule delete_clear_tokens_sjt() relied on cascade to remove the associated schedule when deleting the cleanup_tokens SystemJobTemplate. In migration context with polymorphic models, the cascade did not reliably fire, leaving an orphaned schedule whose unified_job_template referenced a missing row. Explicitly delete associated schedules and clear stale next_schedule references before deleting the SJT. Add defensive handling in ScheduleSerializer.get_related() and get_summary_fields() so orphaned schedules degrade gracefully instead of raising RelatedObjectDoesNotExist. Fix the existing test assertion that used a JOIN-based query incapable of detecting orphans, and add coverage for next_schedule reference cleanup. Fixes: AAP-92564 * Add migration to clean up orphaned token cleanup schedules Databases that already ran migration 0204 may have orphaned "Cleanup Expired OAuth 2 Tokens" schedules left behind by the broken cascade. Remove them and clear any stale UnifiedJobTemplate.next_schedule references. Add a test that simulates the orphan scenario by deleting the UJT via raw SQL and verifying the migration repairs it. Ref: AAP-92564 * Re-parent migration 0212 for gateway backport to stable-2.7 Apply the gateway backport migration pattern (AAP-87599): re-parent 0212_cleanup_orphaned_token_schedules to depend on 0207_merge (the common ancestor with tower/stable-2.7) instead of 0211. Add 0213_merge_0211_0212 to converge the parallel paths on devel. This allows the backport to tower/stable-2.7 to use the identical 0212 migration file with no changes. Ref: AAP-92564 * Test next_schedule cleanup on a surviving template Use the cleanup_sessions SystemJobTemplate (which survives the deletion) as the next_schedule reference holder, so the assertion actually proves the UPDATE ... SET next_schedule = NULL logic works rather than being masked by the referenced UJT itself being deleted. Ref: AAP-92564 * Fix test: create surviving SJT instead of relying on migration data The CI test DB does not have the cleanup_sessions SystemJobTemplate from migration 0078. Use get_or_create to ensure the surviving UJT exists within the test itself. Ref: AAP-92564 * Renumber cleanup migration to 0208 and bump 0208-0211 to 0209-0212 Replace the gateway backport pattern (re-parented migration + merge migration) with a simpler approach: slot the cleanup migration as 0208 directly after the merge point, and renumber the subsequent migrations. This keeps the migration graph linear on all branches and preserves the ability to backport 0209-0212 individually in the future. Ref: AAP-92564 --------- Co-authored-by: Claude Opus 4.6 --- awx/api/serializers.py | 29 ++++--- .../0208_cleanup_orphaned_token_schedules.py | 39 ++++++++++ ...y => 0209_alter_skip_tags_to_textfield.py} | 2 +- ...y => 0210_fix_system_auditor_migration.py} | 2 +- ...py => 0211_remove_host_last_job_fields.py} | 2 +- ... 0212_create_metrics_utility_functions.py} | 2 +- awx/main/migrations/_create_system_jobs.py | 6 ++ .../migrations/test_token_sjt_removal.py | 76 ++++++++++++++++++- awx/main/tests/functional/test_migrations.py | 8 +- 9 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 awx/main/migrations/0208_cleanup_orphaned_token_schedules.py rename awx/main/migrations/{0208_alter_skip_tags_to_textfield.py => 0209_alter_skip_tags_to_textfield.py} (89%) rename awx/main/migrations/{0209_fix_system_auditor_migration.py => 0210_fix_system_auditor_migration.py} (96%) rename awx/main/migrations/{0210_remove_host_last_job_fields.py => 0211_remove_host_last_job_fields.py} (86%) rename awx/main/migrations/{0211_create_metrics_utility_functions.py => 0212_create_metrics_utility_functions.py} (96%) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 8474021495..3670082934 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -5792,24 +5792,33 @@ class ScheduleSerializer(PromptFieldCleanTextMixin, LaunchConfigurationBaseSeria def get_related(self, obj): res = super(ScheduleSerializer, self).get_related(obj) res.update(dict(unified_jobs=self.reverse('api:schedule_unified_jobs_list', kwargs={'pk': obj.pk}))) - if obj.unified_job_template: - res['unified_job_template'] = obj.unified_job_template.get_absolute_url(self.context.get('request')) + try: + ujt = obj.unified_job_template + except ObjectDoesNotExist: + ujt = None + if ujt: + res['unified_job_template'] = ujt.get_absolute_url(self.context.get('request')) try: - if obj.unified_job_template.project: - res['project'] = obj.unified_job_template.project.get_absolute_url(self.context.get('request')) + if ujt.project: + res['project'] = ujt.project.get_absolute_url(self.context.get('request')) except ObjectDoesNotExist: pass if obj.inventory: res['inventory'] = obj.inventory.get_absolute_url(self.context.get('request')) - elif obj.unified_job_template and getattr(obj.unified_job_template, 'inventory', None): - res['inventory'] = obj.unified_job_template.inventory.get_absolute_url(self.context.get('request')) + elif ujt and getattr(ujt, 'inventory', None): + res['inventory'] = ujt.inventory.get_absolute_url(self.context.get('request')) return res def get_summary_fields(self, obj): summary_fields = super(ScheduleSerializer, self).get_summary_fields(obj) - if isinstance(obj.unified_job_template, SystemJobTemplate): - summary_fields['unified_job_template']['job_type'] = obj.unified_job_template.job_type + try: + ujt = obj.unified_job_template + except ObjectDoesNotExist: + return summary_fields + + if isinstance(ujt, SystemJobTemplate): + summary_fields['unified_job_template']['job_type'] = ujt.job_type # We are not showing instance groups on summary fields because JTs don't either @@ -5817,8 +5826,8 @@ class ScheduleSerializer(PromptFieldCleanTextMixin, LaunchConfigurationBaseSeria return summary_fields inventory = None - if obj.unified_job_template and getattr(obj.unified_job_template, 'inventory', None): - inventory = obj.unified_job_template.inventory + if ujt and getattr(ujt, 'inventory', None): + inventory = ujt.inventory else: return summary_fields diff --git a/awx/main/migrations/0208_cleanup_orphaned_token_schedules.py b/awx/main/migrations/0208_cleanup_orphaned_token_schedules.py new file mode 100644 index 0000000000..7868607821 --- /dev/null +++ b/awx/main/migrations/0208_cleanup_orphaned_token_schedules.py @@ -0,0 +1,39 @@ +import logging + +from django.db import migrations + +logger = logging.getLogger('awx.main.migrations') + + +def cleanup_orphaned_token_schedules(apps, schema_editor): + """Remove orphaned 'Cleanup Expired OAuth 2 Tokens' schedules left behind + by migration 0204 when cascade delete did not fire for the associated + SystemJobTemplate. Also clears any stale next_schedule references.""" + Schedule = apps.get_model('main', 'Schedule') + UnifiedJobTemplate = apps.get_model('main', 'UnifiedJobTemplate') + + orphaned = Schedule.objects.filter( + name='Cleanup Expired OAuth 2 Tokens', + ).exclude( + unified_job_template_id__in=UnifiedJobTemplate.objects.values('pk'), + ) + + orphan_ids = list(orphaned.values_list('pk', flat=True)) + if not orphan_ids: + return + + logger.info(f'Cleaning up {len(orphan_ids)} orphaned token cleanup schedule(s): {orphan_ids}') + stale_refs = UnifiedJobTemplate.objects.filter(next_schedule_id__in=orphan_ids).update(next_schedule=None) + if stale_refs: + logger.info(f'Cleared {stale_refs} stale next_schedule reference(s)') + orphaned.delete() + + +class Migration(migrations.Migration): + dependencies = [ + ('main', '0207_merge_0205_0206'), + ] + + operations = [ + migrations.RunPython(cleanup_orphaned_token_schedules, migrations.RunPython.noop), + ] diff --git a/awx/main/migrations/0208_alter_skip_tags_to_textfield.py b/awx/main/migrations/0209_alter_skip_tags_to_textfield.py similarity index 89% rename from awx/main/migrations/0208_alter_skip_tags_to_textfield.py rename to awx/main/migrations/0209_alter_skip_tags_to_textfield.py index da386f4bf2..59d3961605 100644 --- a/awx/main/migrations/0208_alter_skip_tags_to_textfield.py +++ b/awx/main/migrations/0209_alter_skip_tags_to_textfield.py @@ -5,7 +5,7 @@ from django.db import migrations, models class Migration(migrations.Migration): dependencies = [ - ('main', '0207_merge_0205_0206'), + ('main', '0208_cleanup_orphaned_token_schedules'), ] operations = [ diff --git a/awx/main/migrations/0209_fix_system_auditor_migration.py b/awx/main/migrations/0210_fix_system_auditor_migration.py similarity index 96% rename from awx/main/migrations/0209_fix_system_auditor_migration.py rename to awx/main/migrations/0210_fix_system_auditor_migration.py index c9a122eee9..6583ebce61 100644 --- a/awx/main/migrations/0209_fix_system_auditor_migration.py +++ b/awx/main/migrations/0210_fix_system_auditor_migration.py @@ -37,7 +37,7 @@ def fix_system_auditor_assignments(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('main', '0208_alter_skip_tags_to_textfield'), + ('main', '0209_alter_skip_tags_to_textfield'), ] operations = [ diff --git a/awx/main/migrations/0210_remove_host_last_job_fields.py b/awx/main/migrations/0211_remove_host_last_job_fields.py similarity index 86% rename from awx/main/migrations/0210_remove_host_last_job_fields.py rename to awx/main/migrations/0211_remove_host_last_job_fields.py index 45192e7012..2588bf682d 100644 --- a/awx/main/migrations/0210_remove_host_last_job_fields.py +++ b/awx/main/migrations/0211_remove_host_last_job_fields.py @@ -3,7 +3,7 @@ from django.db import migrations class Migration(migrations.Migration): dependencies = [ - ('main', '0209_fix_system_auditor_migration'), + ('main', '0210_fix_system_auditor_migration'), ] operations = [ diff --git a/awx/main/migrations/0211_create_metrics_utility_functions.py b/awx/main/migrations/0212_create_metrics_utility_functions.py similarity index 96% rename from awx/main/migrations/0211_create_metrics_utility_functions.py rename to awx/main/migrations/0212_create_metrics_utility_functions.py index ef2a149acb..f754ddb5ab 100644 --- a/awx/main/migrations/0211_create_metrics_utility_functions.py +++ b/awx/main/migrations/0212_create_metrics_utility_functions.py @@ -47,7 +47,7 @@ DROP_FUNCTIONS_SQL = """ class Migration(migrations.Migration): dependencies = [ - ('main', '0210_remove_host_last_job_fields'), + ('main', '0211_remove_host_last_job_fields'), ] operations = [ diff --git a/awx/main/migrations/_create_system_jobs.py b/awx/main/migrations/_create_system_jobs.py index 10901a8095..d911fe998d 100644 --- a/awx/main/migrations/_create_system_jobs.py +++ b/awx/main/migrations/_create_system_jobs.py @@ -48,6 +48,12 @@ def create_clearsessions_jt(apps, schema_editor): def delete_clear_tokens_sjt(apps, schema_editor): SystemJobTemplate = apps.get_model('main', 'SystemJobTemplate') + Schedule = apps.get_model('main', 'Schedule') + UnifiedJobTemplate = apps.get_model('main', 'UnifiedJobTemplate') for sjt in SystemJobTemplate.objects.filter(job_type='cleanup_tokens'): logger.info(f'Deleting system job template id={sjt.id} due to removal of local OAuth2 tokens') + schedule_ids = list(Schedule.objects.filter(unified_job_template_id=sjt.pk).values_list('pk', flat=True)) + if schedule_ids: + UnifiedJobTemplate.objects.filter(next_schedule_id__in=schedule_ids).update(next_schedule=None) + Schedule.objects.filter(pk__in=schedule_ids).delete() sjt.delete() diff --git a/awx/main/tests/functional/migrations/test_token_sjt_removal.py b/awx/main/tests/functional/migrations/test_token_sjt_removal.py index 35770fd4ca..0eace67ab9 100644 --- a/awx/main/tests/functional/migrations/test_token_sjt_removal.py +++ b/awx/main/tests/functional/migrations/test_token_sjt_removal.py @@ -1,9 +1,14 @@ +import importlib + import pytest from django.apps import apps from django.utils.timezone import now from awx.main.migrations._create_system_jobs import delete_clear_tokens_sjt +_migration_0208 = importlib.import_module('awx.main.migrations.0208_cleanup_orphaned_token_schedules') +cleanup_orphaned_token_schedules = _migration_0208.cleanup_orphaned_token_schedules + SJT_NAME = 'Cleanup Expired OAuth 2 Tokens' @@ -49,10 +54,73 @@ def test_clear_token_sjt(): assert qs.count() == 1 sjt = qs.first() assert Schedule.objects.filter(unified_job_template=sjt).count() == 1 - assert Schedule.objects.filter(unified_job_template__systemjobtemplate__name=SJT_NAME).count() == 1 - # Now run the migration logic to remove delete_clear_tokens_sjt(apps, None) assert SystemJobTemplate.objects.filter(name=SJT_NAME).count() == 0 - # Making sure that the schedule is cleaned up is the main point of this test - assert Schedule.objects.filter(unified_job_template__systemjobtemplate__name=SJT_NAME).count() == 0 + assert Schedule.objects.filter(name=SJT_NAME).count() == 0 + + +def _get_or_create_surviving_ujt(): + """Return a SystemJobTemplate that will survive cleanup_tokens deletion.""" + from django.contrib.contenttypes.models import ContentType + + SystemJobTemplate = apps.get_model('main', 'SystemJobTemplate') + sjt_ct = ContentType.objects.get_for_model(SystemJobTemplate) + now_dt = now() + sjt, _ = SystemJobTemplate.objects.get_or_create( + job_type='cleanup_sessions', + defaults=dict( + name='Cleanup Expired Sessions', + description='Cleans out expired browser sessions', + polymorphic_ctype=sjt_ct, + created=now_dt, + modified=now_dt, + ), + ) + return sjt + + +@pytest.mark.django_db +def test_clear_token_sjt_clears_next_schedule(): + Schedule = apps.get_model('main', 'Schedule') + UnifiedJobTemplate = apps.get_model('main', 'UnifiedJobTemplate') + create_cleartokens_jt(apps, None) + + sched = Schedule.objects.get(name=SJT_NAME) + surviving_ujt = _get_or_create_surviving_ujt() + UnifiedJobTemplate.objects.filter(pk=surviving_ujt.pk).update(next_schedule=sched) + + delete_clear_tokens_sjt(apps, None) + assert Schedule.objects.filter(name=SJT_NAME).count() == 0 + surviving_ujt.refresh_from_db() + assert surviving_ujt.next_schedule is None, 'Stale next_schedule on surviving template should be cleared' + + +@pytest.mark.django_db +def test_cleanup_orphaned_token_schedules(): + """Simulate an orphaned schedule (UJT row missing) and verify the + cleanup migration removes it and clears stale next_schedule on a + surviving template.""" + Schedule = apps.get_model('main', 'Schedule') + UnifiedJobTemplate = apps.get_model('main', 'UnifiedJobTemplate') + create_cleartokens_jt(apps, None) + + sched = Schedule.objects.get(name=SJT_NAME) + sched_id = sched.pk + ujt_id = sched.unified_job_template_id + + surviving_ujt = _get_or_create_surviving_ujt() + UnifiedJobTemplate.objects.filter(pk=surviving_ujt.pk).update(next_schedule=sched) + + from django.db import connection + + with connection.cursor() as cursor: + cursor.execute('DELETE FROM main_systemjobtemplate WHERE unifiedjobtemplate_ptr_id = %s', [ujt_id]) + cursor.execute('DELETE FROM main_unifiedjobtemplate WHERE id = %s', [ujt_id]) + + assert Schedule.objects.filter(pk=sched_id).exists(), 'Orphaned schedule should still exist' + + cleanup_orphaned_token_schedules(apps, None) + assert not Schedule.objects.filter(pk=sched_id).exists(), 'Orphaned schedule should be removed' + surviving_ujt.refresh_from_db() + assert surviving_ujt.next_schedule is None, 'Stale next_schedule on surviving template should be cleared' diff --git a/awx/main/tests/functional/test_migrations.py b/awx/main/tests/functional/test_migrations.py index 7bb25d4fc5..0eeeca099d 100644 --- a/awx/main/tests/functional/test_migrations.py +++ b/awx/main/tests/functional/test_migrations.py @@ -251,7 +251,7 @@ class TestSystemAuditorMigration: def test_fix_system_auditor_migration_backfills_missing(self, migrator): """ - Migration 0209 should backfill Platform Auditor for any + Migration 0210 should backfill Platform Auditor for any system_auditor members who were missed by the original migration. """ state = migrator.apply_initial_migration(('main', '0190_alter_inventorysource_source_and_more')) @@ -273,7 +273,7 @@ class TestSystemAuditorMigration: "Setup: user should not have Platform Auditor before corrective migration" ) - state = migrator.apply_tested_migration(('main', '0209_fix_system_auditor_migration')) + state = migrator.apply_tested_migration(('main', '0210_fix_system_auditor_migration')) RoleUserAssignment = state.apps.get_model('dab_rbac', 'RoleUserAssignment') assert RoleUserAssignment.objects.filter(user=user.id, role_definition__name='Platform Auditor').exists(), ( @@ -282,7 +282,7 @@ class TestSystemAuditorMigration: def test_fix_system_auditor_migration_no_duplicates(self, migrator): """ - Migration 0209 must be idempotent — users who already have + Migration 0210 must be idempotent — users who already have Platform Auditor should not receive a duplicate assignment. """ state = migrator.apply_initial_migration(('main', '0190_alter_inventorysource_source_and_more')) @@ -299,7 +299,7 @@ class TestSystemAuditorMigration: # Confirm the user already has the assignment (from the fixed 0192) assert RoleUserAssignment.objects.filter(user=user.id, role_definition__name='Platform Auditor').exists() - state = migrator.apply_tested_migration(('main', '0209_fix_system_auditor_migration')) + state = migrator.apply_tested_migration(('main', '0210_fix_system_auditor_migration')) RoleUserAssignment = state.apps.get_model('dab_rbac', 'RoleUserAssignment') assert RoleUserAssignment.objects.filter(user=user.id, role_definition__name='Platform Auditor').count() == 1, (