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, (