From 9d014778adcdbbfeca6531c2c4db4121e362c8d2 Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 9 Oct 2019 18:33:25 -0400 Subject: [PATCH] cronvar - use correct binary name (#63279) Fixes regression introduced by #62554 Add integration tests for cronvar --- .../cronvar-correct-binary-name.yaml | 2 + lib/ansible/modules/system/cronvar.py | 2 +- .../targets/cron/defaults/main.yml | 1 - test/integration/targets/cron/meta/main.yml | 2 + test/integration/targets/cron/tasks/main.yml | 236 ++++++------------ test/integration/targets/cronvar/aliases | 3 + .../targets/cronvar/defaults/main.yml | 1 + .../integration/targets/cronvar/meta/main.yml | 2 + .../targets/cronvar/tasks/main.yml | 109 ++++++++ .../targets/setup_cron/defaults/main.yml | 1 + .../targets/setup_cron/tasks/main.yml | 70 ++++++ .../vars/debian.yml} | 1 - .../targets/setup_cron/vars/default.yml | 0 .../vars/fedora.yml} | 1 - .../vars/freebsd.yml} | 1 - .../vars/redhat.yml} | 1 - .../vars/suse.yml} | 1 - 17 files changed, 273 insertions(+), 161 deletions(-) create mode 100644 changelogs/fragments/cronvar-correct-binary-name.yaml create mode 100644 test/integration/targets/cron/meta/main.yml create mode 100644 test/integration/targets/cronvar/aliases create mode 100644 test/integration/targets/cronvar/defaults/main.yml create mode 100644 test/integration/targets/cronvar/meta/main.yml create mode 100644 test/integration/targets/cronvar/tasks/main.yml create mode 100644 test/integration/targets/setup_cron/defaults/main.yml create mode 100644 test/integration/targets/setup_cron/tasks/main.yml rename test/integration/targets/{cron/defaults/cron.debian.yml => setup_cron/vars/debian.yml} (93%) create mode 100644 test/integration/targets/setup_cron/vars/default.yml rename test/integration/targets/{cron/defaults/cron.fedora.yml => setup_cron/vars/fedora.yml} (93%) rename test/integration/targets/{cron/defaults/cron.freebsd.yml => setup_cron/vars/freebsd.yml} (94%) rename test/integration/targets/{cron/defaults/cron.redhat.yml => setup_cron/vars/redhat.yml} (94%) rename test/integration/targets/{cron/defaults/cron.suse.yml => setup_cron/vars/suse.yml} (93%) diff --git a/changelogs/fragments/cronvar-correct-binary-name.yaml b/changelogs/fragments/cronvar-correct-binary-name.yaml new file mode 100644 index 00000000000..4472bfa9dbb --- /dev/null +++ b/changelogs/fragments/cronvar-correct-binary-name.yaml @@ -0,0 +1,2 @@ +bugfixes: + - cronvar - use correct binary name (https://github.com/ansible/ansible/issues/63274) diff --git a/lib/ansible/modules/system/cronvar.py b/lib/ansible/modules/system/cronvar.py index eb7c7a6b7b0..f66320bc44b 100644 --- a/lib/ansible/modules/system/cronvar.py +++ b/lib/ansible/modules/system/cronvar.py @@ -126,7 +126,7 @@ class CronVar(object): self.user = user self.lines = None self.wordchars = ''.join(chr(x) for x in range(128) if chr(x) not in ('=', "'", '"',)) - self.cron_cmd = self.module.get_bin_path('cronvar', required=True) + self.cron_cmd = self.module.get_bin_path('crontab', required=True) if cron_file: self.cron_file = "" diff --git a/test/integration/targets/cron/defaults/main.yml b/test/integration/targets/cron/defaults/main.yml index 2abb0ebf5dc..37e6fc3714c 100644 --- a/test/integration/targets/cron/defaults/main.yml +++ b/test/integration/targets/cron/defaults/main.yml @@ -1,2 +1 @@ ---- faketime_pkg: libfaketime diff --git a/test/integration/targets/cron/meta/main.yml b/test/integration/targets/cron/meta/main.yml new file mode 100644 index 00000000000..2d2436a168f --- /dev/null +++ b/test/integration/targets/cron/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - setup_cron diff --git a/test/integration/targets/cron/tasks/main.yml b/test/integration/targets/cron/tasks/main.yml index fb7fd42d6ac..cc892c8e06e 100644 --- a/test/integration/targets/cron/tasks/main.yml +++ b/test/integration/targets/cron/tasks/main.yml @@ -1,173 +1,101 @@ -- include_vars: "{{ lookup('first_found', search) }}" - vars: - search: - files: - - 'cron.{{ ansible_system | lower }}.yml' - - 'cron.{{ ansible_distribution | lower }}.yml' - - 'cron.{{ ansible_os_family | lower }}.yml' - paths: - - '../defaults/' +- name: add cron task (check mode enabled, cron task not already created) + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + check_mode: yes + register: check_mode_enabled_state_present -- vars: - remote_dir: "{{ lookup('env', 'OUTPUT_DIR') }}" - block: - - name: install cron package - package: - name: '{{ cron_pkg }}' - when: cron_pkg|default(false, true) - register: cron_package_installed - until: cron_package_installed is success +- assert: + that: check_mode_enabled_state_present is changed - - when: faketime_pkg|default(false, true) - block: - - name: install cron and faketime packages - package: - name: '{{ faketime_pkg }}' - register: faketime_package_installed - until: faketime_package_installed is success +- name: add cron task (check mode disabled, task hasn't already been created) + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + register: add_cron_task - - name: Find libfaketime path - shell: '{{ list_pkg_files }} {{ faketime_pkg }} | grep -F libfaketime.so.1' - args: - warn: false - register: libfaketime_path +- assert: + that: add_cron_task is changed - - when: ansible_service_mgr == 'systemd' - block: - - name: create directory for cron drop-in file - file: - path: '/etc/systemd/system/{{ cron_service }}.service.d' - state: directory - owner: root - group: root - mode: 0755 +- name: add cron task (check mode enabled, cron task already exists) + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + check_mode: yes + register: check_mode_enabled_state_present_cron_task_already_exists - - name: Use faketime with cron service - copy: - content: |- - [Service] - Environment=LD_PRELOAD={{ libfaketime_path.stdout_lines[0].strip() }} - Environment="FAKETIME=+0y x10" - Environment=RANDOM_DELAY=0 - dest: '/etc/systemd/system/{{ cron_service }}.service.d/faketime.conf' - owner: root - group: root - mode: 0644 +- assert: + that: check_mode_enabled_state_present_cron_task_already_exists is not changed - - when: ansible_system == 'FreeBSD' - name: Use faketime with cron service - copy: - content: |- - cron_env='LD_PRELOAD={{ libfaketime_path.stdout_lines[0].strip() }} FAKETIME="+0y x10"' - dest: '/etc/rc.conf.d/cron' - owner: root - group: wheel - mode: 0644 +- name: add cron task (check mode disabled, cron task already created) + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + register: cron_task_already_created - - name: enable cron service - service: - daemon-reload: "{{ (ansible_service_mgr == 'systemd')|ternary(true, omit) }}" - name: '{{ cron_service }}' - state: restarted +- assert: + that: cron_task_already_created is not changed - - name: add cron task (check mode enabled, cron task not already created) - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - check_mode: yes - register: check_mode_enabled_state_present +- block: + - name: wait for canary creation + wait_for: + path: '{{ remote_dir }}/cron_canary1' + timeout: '{{ 20 if faketime_pkg else 70 }}' + register: wait_canary + always: + - name: display some logs in case of failure + command: 'journalctl -u {{ cron_service }}' + when: wait_canary is failed and ansible_service_mgr == 'systemd' - - assert: - that: check_mode_enabled_state_present is changed +- debug: + msg: 'elapsed time waiting for canary: {{ wait_canary.elapsed }}' - - name: add cron task (check mode disabled, task hasn't already been created) - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - register: add_cron_task +- name: Check check_mode + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + state: absent + check_mode: yes + register: check_check_mode - - assert: - that: add_cron_task is changed +- assert: + that: check_check_mode is changed - - name: add cron task (check mode enabled, cron task already exists) - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - check_mode: yes - register: check_mode_enabled_state_present_cron_task_already_exists +- name: Remove a cron task + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + state: absent + register: remove_task - - assert: - that: check_mode_enabled_state_present_cron_task_already_exists is not changed +- assert: + that: remove_task is changed - - name: add cron task (check mode disabled, cron task already created) - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - register: cron_task_already_created +- name: 'cron task missing: check idempotence (check mode enabled, state=absent)' + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + state: absent + register: check_mode_enabled_remove_task_idempotence - - assert: - that: cron_task_already_created is not changed +- assert: + that: check_mode_enabled_remove_task_idempotence is not changed - - block: - - name: wait for canary creation - wait_for: - path: '{{ remote_dir }}/cron_canary1' - timeout: '{{ 20 if faketime_pkg else 70 }}' - register: wait_canary - always: - - name: display some logs in case of failure - command: 'journalctl -u {{ cron_service }}' - when: wait_canary is failed and ansible_service_mgr == 'systemd' +- name: 'cron task missing: check idempotence (check mode disabled, state=absent)' + cron: + name: test cron task + job: 'date > {{ remote_dir }}/cron_canary1' + state: absent + register: remove_task_idempotence - - debug: - msg: 'elapsed time waiting for canary: {{ wait_canary.elapsed }}' +- assert: + that: remove_task_idempotence is not changed - - name: Check check_mode - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - state: absent - check_mode: yes - register: check_check_mode +- name: Check that removing a cron task with cron_file and without specifying an user is allowed (#58493) + cron: + cron_file: unexistent_cron_file + state: absent + register: remove_cron_file - - assert: - that: check_check_mode is changed - - - name: Remove a cron task - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - state: absent - register: remove_task - - - assert: - that: remove_task is changed - - - name: 'cron task missing: check idempotence (check mode enabled, state=absent)' - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - state: absent - register: check_mode_enabled_remove_task_idempotence - - - assert: - that: check_mode_enabled_remove_task_idempotence is not changed - - - name: 'cron task missing: check idempotence (check mode disabled, state=absent)' - cron: - name: test cron task - job: 'date > {{ remote_dir }}/cron_canary1' - state: absent - register: remove_task_idempotence - - - assert: - that: remove_task_idempotence is not changed - - - name: Check that removing a cron task with cron_file and without specifying an user is allowed (#58493) - cron: - cron_file: unexistent_cron_file - state: absent - register: remove_cron_file - - - assert: - that: remove_cron_file is not changed +- assert: + that: remove_cron_file is not changed diff --git a/test/integration/targets/cronvar/aliases b/test/integration/targets/cronvar/aliases new file mode 100644 index 00000000000..fe75653cadc --- /dev/null +++ b/test/integration/targets/cronvar/aliases @@ -0,0 +1,3 @@ +destructive +shippable/posix/group4 +skip/osx diff --git a/test/integration/targets/cronvar/defaults/main.yml b/test/integration/targets/cronvar/defaults/main.yml new file mode 100644 index 00000000000..a22230ab66b --- /dev/null +++ b/test/integration/targets/cronvar/defaults/main.yml @@ -0,0 +1 @@ +cron_config_path: /etc/cron.d diff --git a/test/integration/targets/cronvar/meta/main.yml b/test/integration/targets/cronvar/meta/main.yml new file mode 100644 index 00000000000..2d2436a168f --- /dev/null +++ b/test/integration/targets/cronvar/meta/main.yml @@ -0,0 +1,2 @@ +dependencies: + - setup_cron diff --git a/test/integration/targets/cronvar/tasks/main.yml b/test/integration/targets/cronvar/tasks/main.yml new file mode 100644 index 00000000000..e6fff1d9a91 --- /dev/null +++ b/test/integration/targets/cronvar/tasks/main.yml @@ -0,0 +1,109 @@ +- name: Create EMAIL cron var + cronvar: + name: EMAIL + value: doug@ansibmod.con.com + register: create_cronvar1 + +- name: Create EMAIL cron var again + cronvar: + name: EMAIL + value: doug@ansibmod.con.com + register: create_cronvar2 + +- name: Check cron var value + shell: crontab -l -u root | grep -c EMAIL=doug@ansibmod.con.com + register: varcheck1 + +- name: Modify EMAIL cron var + cronvar: + name: EMAIL + value: jane@ansibmod.con.com + register: create_cronvar3 + +- name: Check cron var value again + shell: crontab -l -u root | grep -c EMAIL=jane@ansibmod.con.com + register: varcheck2 + +- name: Remove EMAIL cron var + cronvar: + name: EMAIL + state: absent + register: remove_cronvar1 + +- name: Remove EMAIL cron var again + cronvar: + name: EMAIL + state: absent + register: remove_cronvar2 + +- name: Check cron var value again + shell: crontab -l -u root | grep -c EMAIL + register: varcheck3 + failed_when: varcheck3.rc == 0 + +- name: Add cron var to custom file + cronvar: + name: TESTVAR + value: somevalue + cron_file: cronvar_test + register: custom_cronfile1 + +- name: Add cron var to custom file again + cronvar: + name: TESTVAR + value: somevalue + cron_file: cronvar_test + register: custom_cronfile2 + +- name: Check cron var value in custom file + command: grep -c TESTVAR=somevalue {{ cron_config_path }}/cronvar_test + register: custom_varcheck1 + +- name: Change cron var in custom file + cronvar: + name: TESTVAR + value: newvalue + cron_file: cronvar_test + register: custom_cronfile3 + +- name: Check cron var value in custom file + command: grep -c TESTVAR=newvalue {{ cron_config_path }}/cronvar_test + register: custom_varcheck2 + +- name: Remove cron var from custom file + cronvar: + name: TESTVAR + value: newvalue + cron_file: cronvar_test + state: absent + register: custom_remove_cronvar1 + +- name: Remove cron var from custom file again + cronvar: + name: TESTVAR + value: newvalue + cron_file: cronvar_test + state: absent + register: custom_remove_cronvar2 + +- name: Check cron var value + command: grep -c TESTVAR=newvalue {{ cron_config_path }}/cronvar_test + register: custom_varcheck3 + failed_when: custom_varcheck3.rc == 0 + +- name: Esure cronvar tasks did the right thing + assert: + that: + - create_cronvar1 is changed + - create_cronvar2 is not changed + - create_cronvar3 is changed + - remove_cronvar1 is changed + - remove_cronvar2 is not changed + - varcheck1.stdout == '1' + - varcheck2.stdout == '1' + - varcheck3.stdout == '0' + - custom_remove_cronvar1 is changed + - custom_remove_cronvar2 is not changed + - custom_varcheck1.stdout == '1' + - custom_varcheck2.stdout == '1' + - custom_varcheck3.stdout == '0' diff --git a/test/integration/targets/setup_cron/defaults/main.yml b/test/integration/targets/setup_cron/defaults/main.yml new file mode 100644 index 00000000000..e4b0123da2a --- /dev/null +++ b/test/integration/targets/setup_cron/defaults/main.yml @@ -0,0 +1 @@ +remote_dir: "{{ lookup('env', 'OUTPUT_DIR') }}" diff --git a/test/integration/targets/setup_cron/tasks/main.yml b/test/integration/targets/setup_cron/tasks/main.yml new file mode 100644 index 00000000000..93dcefa56e3 --- /dev/null +++ b/test/integration/targets/setup_cron/tasks/main.yml @@ -0,0 +1,70 @@ +- 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 + +- name: install cron package + package: + name: '{{ cron_pkg }}' + when: cron_pkg | default(false, true) + register: cron_package_installed + until: cron_package_installed is success + +- when: faketime_pkg | default(false, true) + block: + - name: install cron and faketime packages + package: + name: '{{ faketime_pkg }}' + register: faketime_package_installed + until: faketime_package_installed is success + + - name: Find libfaketime path + shell: '{{ list_pkg_files }} {{ faketime_pkg }} | grep -F libfaketime.so.1' + args: + warn: false + register: libfaketime_path + + - when: ansible_service_mgr == 'systemd' + block: + - name: create directory for cron drop-in file + file: + path: '/etc/systemd/system/{{ cron_service }}.service.d' + state: directory + owner: root + group: root + mode: 0755 + + - name: Use faketime with cron service + copy: + content: |- + [Service] + Environment=LD_PRELOAD={{ libfaketime_path.stdout_lines[0].strip() }} + Environment="FAKETIME=+0y x10" + Environment=RANDOM_DELAY=0 + dest: '/etc/systemd/system/{{ cron_service }}.service.d/faketime.conf' + owner: root + group: root + mode: 0644 + + - when: ansible_system == 'FreeBSD' + name: Use faketime with cron service + copy: + content: |- + cron_env='LD_PRELOAD={{ libfaketime_path.stdout_lines[0].strip() }} FAKETIME="+0y x10"' + dest: '/etc/rc.conf.d/cron' + owner: root + group: wheel + mode: 0644 + +- name: enable cron service + service: + daemon-reload: "{{ (ansible_service_mgr == 'systemd') | ternary(true, omit) }}" + name: '{{ cron_service }}' + state: restarted diff --git a/test/integration/targets/cron/defaults/cron.debian.yml b/test/integration/targets/setup_cron/vars/debian.yml similarity index 93% rename from test/integration/targets/cron/defaults/cron.debian.yml rename to test/integration/targets/setup_cron/vars/debian.yml index 49abf6995fb..cd04871c5d1 100644 --- a/test/integration/targets/cron/defaults/cron.debian.yml +++ b/test/integration/targets/setup_cron/vars/debian.yml @@ -1,4 +1,3 @@ ---- cron_pkg: cron cron_service: cron list_pkg_files: dpkg -L diff --git a/test/integration/targets/setup_cron/vars/default.yml b/test/integration/targets/setup_cron/vars/default.yml new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/integration/targets/cron/defaults/cron.fedora.yml b/test/integration/targets/setup_cron/vars/fedora.yml similarity index 93% rename from test/integration/targets/cron/defaults/cron.fedora.yml rename to test/integration/targets/setup_cron/vars/fedora.yml index 3507db46ac2..b80a51b54d4 100644 --- a/test/integration/targets/cron/defaults/cron.fedora.yml +++ b/test/integration/targets/setup_cron/vars/fedora.yml @@ -1,4 +1,3 @@ ---- cron_pkg: cronie cron_service: crond list_pkg_files: rpm -ql diff --git a/test/integration/targets/cron/defaults/cron.freebsd.yml b/test/integration/targets/setup_cron/vars/freebsd.yml similarity index 94% rename from test/integration/targets/cron/defaults/cron.freebsd.yml rename to test/integration/targets/setup_cron/vars/freebsd.yml index 07063dc0faf..41ed4493959 100644 --- a/test/integration/targets/cron/defaults/cron.freebsd.yml +++ b/test/integration/targets/setup_cron/vars/freebsd.yml @@ -1,4 +1,3 @@ ---- cron_pkg: cron_service: cron list_pkg_files: pkg info --list-files diff --git a/test/integration/targets/cron/defaults/cron.redhat.yml b/test/integration/targets/setup_cron/vars/redhat.yml similarity index 94% rename from test/integration/targets/cron/defaults/cron.redhat.yml rename to test/integration/targets/setup_cron/vars/redhat.yml index 40a79f37c3d..2dff13de099 100644 --- a/test/integration/targets/cron/defaults/cron.redhat.yml +++ b/test/integration/targets/setup_cron/vars/redhat.yml @@ -1,4 +1,3 @@ ---- cron_pkg: cronie cron_service: crond faketime_pkg: diff --git a/test/integration/targets/cron/defaults/cron.suse.yml b/test/integration/targets/setup_cron/vars/suse.yml similarity index 93% rename from test/integration/targets/cron/defaults/cron.suse.yml rename to test/integration/targets/setup_cron/vars/suse.yml index 5e57a6e514c..cd3677a6e80 100644 --- a/test/integration/targets/cron/defaults/cron.suse.yml +++ b/test/integration/targets/setup_cron/vars/suse.yml @@ -1,4 +1,3 @@ ---- cron_pkg: cron cron_service: cron list_pkg_files: rpm -ql