From e232cd392c9b2252a6a2ddecb1d6d44a5a5a3771 Mon Sep 17 00:00:00 2001 From: Ryan Petrello Date: Tue, 3 Mar 2020 14:46:00 -0500 Subject: [PATCH] make fact saving code more robust to unexpected fact data see: https://github.com/ansible/awx/issues/5935 --- awx/main/models/jobs.py | 6 ++++-- awx/main/tests/unit/models/test_jobs.py | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index eb351dcd53..77b3871626 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -829,8 +829,10 @@ class Job(UnifiedJob, JobOptions, SurveyJobMixin, JobNotificationMixin, TaskMana continue host.ansible_facts = ansible_facts host.ansible_facts_modified = now() - ansible_local_system_id = ansible_facts.get('ansible_local', {}).get('insights', {}).get('system_id', None) - ansible_facts_system_id = ansible_facts.get('insights', {}).get('system_id', None) + ansible_local = ansible_facts.get('ansible_local', {}).get('insights', {}) + ansible_facts = ansible_facts.get('insights', {}) + ansible_local_system_id = ansible_local.get('system_id', None) if isinstance(ansible_local, dict) else None + ansible_facts_system_id = ansible_facts.get('system_id', None) if isinstance(ansible_facts, dict) else None if ansible_local_system_id: print("Setting local {}".format(ansible_local_system_id)) logger.debug("Insights system_id {} found for host <{}, {}> in" diff --git a/awx/main/tests/unit/models/test_jobs.py b/awx/main/tests/unit/models/test_jobs.py index b8964a94f8..f28691f500 100644 --- a/awx/main/tests/unit/models/test_jobs.py +++ b/awx/main/tests/unit/models/test_jobs.py @@ -89,6 +89,27 @@ def test_finish_job_fact_cache_with_existing_data(job, hosts, inventory, mocker, hosts[1].save.assert_called_once_with() +def test_finish_job_fact_cache_with_malformed_fact(job, hosts, inventory, mocker, tmpdir): + fact_cache = os.path.join(tmpdir, 'facts') + modified_times = {} + job.start_job_fact_cache(fact_cache, modified_times, 0) + + for h in hosts: + h.save = mocker.Mock() + + for h in hosts: + filepath = os.path.join(fact_cache, h.name) + with open(filepath, 'w') as f: + json.dump({'ansible_local': {'insights': 'this is an unexpected error from ansible'}}, f) + new_modification_time = time.time() + 3600 + os.utime(filepath, (new_modification_time, new_modification_time)) + + job.finish_job_fact_cache(fact_cache, modified_times) + + for h in hosts: + assert h.insights_system_id is None + + def test_finish_job_fact_cache_with_bad_data(job, hosts, inventory, mocker, tmpdir): fact_cache = os.path.join(tmpdir, 'facts') modified_times = {}