[2.10] Do not remove non-empty cron tabs (#74697)

* Only remove crontabs if they are empty (cherry picked from commit fefda12827) * Add integration test to ensure system cron tab doesn't get removed. Increase cron integration tests separation. (cherry picked from commit 1e37fa86b4) * Also detect crontab which only contains whitespace as empty. (cherry picked from commit 4b69c8f501) * cron integration test: Adjust system crontab path to be distribution specific. (cherry picked from commit 70be3730db) * Add changelog fragment for #74497. (cherry picked from commit c606b50a3d) Co-authored-by: Fabian Klemp <fabian.klemp@elara-gmbh.de>
4 years ago · 2b29428f22
parent a6a2af7814
commit 2b29428f22
5 changed files with 121 additions and 20 deletions
--- a/changelogs/fragments/74497-keep-non-empty-crontabs.yml
+++ 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).
--- a/lib/ansible/modules/cron.py
+++ b/lib/ansible/modules/cron.py
@ -291,7 +291,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):
        """
@ -675,18 +678,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")
@ -729,6 +720,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 != '':
--- 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
@ -129,39 +148,114 @@
 # BusyBox does not have /etc/cron.d
 - name: Removing a cron file when the name is specified is allowed (#57471)
  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)
  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
@ -169,13 +263,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:
--- a/test/integration/targets/cron/vars/alpine.yml
+++ b/test/integration/targets/cron/vars/alpine.yml
@ -0,0 +1 @@
+system_crontab: /etc/crontabs/root
--- a/test/integration/targets/cron/vars/default.yml
+++ b/test/integration/targets/cron/vars/default.yml
@ -0,0 +1 @@
+system_crontab: /etc/crontab