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 <fabian.klemp@elara-gmbh.de>

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).

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 != '':

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:

test/integration/targets/cron/vars/alpine.yml

@@ -0,0 +1 @@
+system_crontab: /etc/crontabs/root

test/integration/targets/cron/vars/default.yml

@@ -0,0 +1 @@
+system_crontab: /etc/crontab