From c6ac9de67b50c88a3fe2a6ce6fee8bc25c1c99a3 Mon Sep 17 00:00:00 2001 From: elara-leitstellentechnik Date: Thu, 13 May 2021 19:38:13 +0200 Subject: [PATCH] Do not remove non-empty cron tabs (#74497) * Only remove crontabs if they are empty * Add integration test to ensure system cron tab doesn't get removed. Increase cron integration tests separation. * Also detect crontab which only contains whitespace as empty. * cron integration test: Adjust system crontab path to be distribution specific. Co-authored-by: Fabian Klemp --- .../74497-keep-non-empty-crontabs.yml | 3 + lib/ansible/modules/cron.py | 28 ++--- test/integration/targets/cron/tasks/main.yml | 108 ++++++++++++++++-- test/integration/targets/cron/vars/alpine.yml | 1 + .../integration/targets/cron/vars/default.yml | 1 + 5 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/74497-keep-non-empty-crontabs.yml create mode 100644 test/integration/targets/cron/vars/alpine.yml create mode 100644 test/integration/targets/cron/vars/default.yml diff --git a/changelogs/fragments/74497-keep-non-empty-crontabs.yml b/changelogs/fragments/74497-keep-non-empty-crontabs.yml new file mode 100644 index 00000000000..2e88e6b54cf --- /dev/null +++ b/changelogs/fragments/74497-keep-non-empty-crontabs.yml @@ -0,0 +1,3 @@ +bugfixes: + - ansible.builtin.cron - Keep non-empty crontabs, when removing cron jobs + (https://github.com/ansible/ansible/pull/74497). diff --git a/lib/ansible/modules/cron.py b/lib/ansible/modules/cron.py index 51d6b4eec7d..1854bcd2303 100644 --- a/lib/ansible/modules/cron.py +++ b/lib/ansible/modules/cron.py @@ -286,7 +286,10 @@ class CronTab(object): if len(self.lines) == 0: return True else: - return False + for line in self.lines: + if line.strip(): + return False + return True def write(self, backup_file=None): """ @@ -654,18 +657,6 @@ def main(): (backuph, backup_file) = tempfile.mkstemp(prefix='crontab') crontab.write(backup_file) - if crontab.cron_file and not do_install: - if module._diff: - diff['after'] = '' - diff['after_header'] = '/dev/null' - else: - diff = dict() - if module.check_mode: - changed = os.path.isfile(crontab.cron_file) - else: - changed = crontab.remove_job_file() - module.exit_json(changed=changed, cron_file=cron_file, state=state, diff=diff) - if env: if ' ' in name: module.fail_json(msg="Invalid name for environment variable") @@ -708,6 +699,17 @@ def main(): if len(old_job) > 0: crontab.remove_job(name) changed = True + if crontab.cron_file and crontab.is_empty(): + if module._diff: + diff['after'] = '' + diff['after_header'] = '/dev/null' + else: + diff = dict() + if module.check_mode: + changed = os.path.isfile(crontab.cron_file) + else: + changed = crontab.remove_job_file() + module.exit_json(changed=changed, cron_file=cron_file, state=state, diff=diff) # no changes to env/job, but existing crontab needs a terminating newline if not changed and crontab.n_existing != '': diff --git a/test/integration/targets/cron/tasks/main.yml b/test/integration/targets/cron/tasks/main.yml index c5f22b2dd1b..406d257caaa 100644 --- a/test/integration/targets/cron/tasks/main.yml +++ b/test/integration/targets/cron/tasks/main.yml @@ -1,3 +1,22 @@ +- name: Include distribution specific variables + include_vars: "{{ lookup('first_found', search) }}" + vars: + search: + files: + - '{{ ansible_distribution | lower }}.yml' + - '{{ ansible_os_family | lower }}.yml' + - '{{ ansible_system | lower }}.yml' + - default.yml + paths: + - vars + +# https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=726661 +- name: Work around vixie-cron/PAM issue on old distros + command: sed -i '/pam_loginuid/ s/^/#/' /etc/pam.d/crond + when: + - ansible_distribution in ('RedHat', 'CentOS') + - ansible_distribution_major_version is version('6', '==') + - name: add cron task (check mode enabled, cron task not already created) cron: name: test cron task @@ -131,40 +150,115 @@ - name: Removing a cron file when the name is specified is allowed (#57471) when: ansible_distribution != 'Alpine' block: + - name: Check file does not exist + stat: + path: /etc/cron.d/cron_remove_name + register: cron_file_stats + + - assert: + that: not cron_file_stats.stat.exists + - name: Cron file creation cron: - cron_file: cron_filename + cron_file: cron_remove_name name: "integration test cron" job: 'ls' user: root - name: Cron file deletion cron: - cron_file: cron_filename + cron_file: cron_remove_name name: "integration test cron" state: absent - name: Check file succesfull deletion stat: - path: /etc/cron.d/cron_filename + path: /etc/cron.d/cron_remove_name register: cron_file_stats - assert: that: not cron_file_stats.stat.exists +# BusyBox does not have /etc/cron.d +- name: Removing a cron file, which contains only whitespace + when: ansible_distribution != 'Alpine' + block: + - name: Check file does not exist + stat: + path: /etc/cron.d/cron_remove_whitespace + register: cron_file_stats + + - assert: + that: not cron_file_stats.stat.exists + + - name: Cron file creation + cron: + cron_file: cron_remove_whitespace + name: "integration test cron" + job: 'ls' + user: root + + - name: Add whitespace to cron file + shell: 'printf "\n \n\t\n" >> /etc/cron.d/cron_remove_whitespace' + + - name: Cron file deletion + cron: + cron_file: cron_remove_whitespace + name: "integration test cron" + state: absent + + - name: Check file succesfull deletion + stat: + path: /etc/cron.d/cron_remove_whitespace + register: cron_file_stats + + - assert: + that: not cron_file_stats.stat.exists + +- name: System cron tab does not get removed + block: + - name: Add cron job + cron: + cron_file: "{{ system_crontab }}" + user: root + name: "integration test cron" + job: 'ls' + + - name: Remove cron job + cron: + cron_file: "{{ system_crontab }}" + name: "integration test cron" + state: absent + + - name: Check system crontab still exists + stat: + path: "{{ system_crontab }}" + register: cron_file_stats + + - assert: + that: cron_file_stats.stat.exists + - name: Allow non-ascii chars in job (#69492) when: ansible_distribution != 'Alpine' block: + - name: Check file does not exist + stat: + path: /etc/cron.d/cron_nonascii + register: cron_file_stats + + - assert: + that: not cron_file_stats.stat.exists + - name: Cron file creation cron: - cron_file: cron_filename + cron_file: cron_nonascii name: "cron job that contain non-ascii chars in job (これは日本語です; This is Japanese)" job: 'echo "うどんは好きだがお化け👻は苦手である。"' user: root - name: "Ensure cron_file contains job string" replace: - path: /etc/cron.d/cron_filename + path: /etc/cron.d/cron_nonascii regexp: "うどんは好きだがお化け👻は苦手である。" replace: "それは機密情報🔓です。" register: find_chars @@ -172,13 +266,13 @@ - name: Cron file deletion cron: - cron_file: cron_filename + cron_file: cron_nonascii name: "cron job that contain non-ascii chars in job (これは日本語です; This is Japanese)" state: absent - name: Check file succesfull deletion stat: - path: /etc/cron.d/cron_filename + path: /etc/cron.d/cron_nonascii register: cron_file_stats - assert: diff --git a/test/integration/targets/cron/vars/alpine.yml b/test/integration/targets/cron/vars/alpine.yml new file mode 100644 index 00000000000..29ca3b94744 --- /dev/null +++ b/test/integration/targets/cron/vars/alpine.yml @@ -0,0 +1 @@ +system_crontab: /etc/crontabs/root diff --git a/test/integration/targets/cron/vars/default.yml b/test/integration/targets/cron/vars/default.yml new file mode 100644 index 00000000000..69c5de469f0 --- /dev/null +++ b/test/integration/targets/cron/vars/default.yml @@ -0,0 +1 @@ +system_crontab: /etc/crontab