From 0781a7f68c1918059cd8f3ffe7a8ce98e7ff48af Mon Sep 17 00:00:00 2001 From: Strahinja Kustudic Date: Fri, 25 May 2018 23:06:11 +0200 Subject: [PATCH] Fixed check_mode status to be the same as normal execution (#40721) * Fixed check_mode status to be the same as normal execution * Now when setting the status to `disabled` in check_mode it correctly returns the state changed and prints a warning like it does in normal model. Before it always returned changed even if everything was set correctly and a reboot was required. * Add changelog entry Co-authored by: Strahinja Kustudic --- changelogs/fragments/selinux-check-mode.yaml | 2 + lib/ansible/modules/system/selinux.py | 14 +- .../targets/selinux/tasks/selinux.yml | 157 ++++++++++++++++++ 3 files changed, 166 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/selinux-check-mode.yaml diff --git a/changelogs/fragments/selinux-check-mode.yaml b/changelogs/fragments/selinux-check-mode.yaml new file mode 100644 index 0000000000..bfea08c058 --- /dev/null +++ b/changelogs/fragments/selinux-check-mode.yaml @@ -0,0 +1,2 @@ +bugfixes: + - selinux - correct check mode behavior to report same changes as normal mode (https://github.com/ansible/ansible/pull/40721) diff --git a/lib/ansible/modules/system/selinux.py b/lib/ansible/modules/system/selinux.py index 134ee2d4b9..897596c152 100644 --- a/lib/ansible/modules/system/selinux.py +++ b/lib/ansible/modules/system/selinux.py @@ -228,19 +228,20 @@ def main(): changed = True if state != runtime_state: - if module.check_mode: - module.exit_json(changed=True) if runtime_enabled: if state == 'disabled': if runtime_state != 'permissive': # Temporarily set state to permissive - set_state(module, 'permissive') + if not module.check_mode: + set_state(module, 'permissive') module.warn('SELinux state temporarily changed from \'%s\' to \'permissive\'. State change will take effect next reboot.' % (runtime_state)) + changed = True else: module.warn('SELinux state change will take effect next reboot') reboot_required = True else: - set_state(module, state) + if not module.check_mode: + set_state(module, state) msgs.append('SELinux state changed from \'%s\' to \'%s\'' % (runtime_state, state)) # Only report changes if the file is changed. @@ -251,10 +252,9 @@ def main(): reboot_required = True if state != config_state: - if module.check_mode: - module.exit_json(changed=True) + if not module.check_mode: + set_config_state(module, state, configfile) msgs.append('Config SELinux state changed from \'%s\' to \'%s\'' % (config_state, state)) - set_config_state(module, state, configfile) changed = True module.exit_json(changed=changed, msg=', '.join(msgs), configfile=configfile, policy=policy, state=state, reboot_required=reboot_required) diff --git a/test/integration/targets/selinux/tasks/selinux.yml b/test/integration/targets/selinux/tasks/selinux.yml index 59aec7a1ad..7fcba899cf 100644 --- a/test/integration/targets/selinux/tasks/selinux.yml +++ b/test/integration/targets/selinux/tasks/selinux.yml @@ -205,3 +205,160 @@ - _state_test1.msg == 'Policy non-existing-selinux-policy does not exist in /etc/selinux/' - ansible_selinux.config_mode == 'enforcing' - ansible_selinux.type == 'targeted' + + +# Fourth Test +# ############################################################################## +# Test if check mode returns correct changed values and +# doesn't make any changes + + +- name: TEST 4 | Set SELinux to enforcing + selinux: + state: enforcing + policy: targeted + register: _check_mode_test1 + +- debug: + var: _check_mode_test1 + verbosity: 1 + +- name: TEST 4 | Set SELinux to enforcing in check mode + selinux: + state: enforcing + policy: targeted + register: _check_mode_test1 + check_mode: yes + +- name: TEST 4 | Re-gather facts + setup: + +- debug: + var: ansible_selinux + verbosity: 1 + tags: debug + +- name: TEST 4 | Assert that check mode is idempotent + assert: + that: + - _check_mode_test1 is success + - not _check_mode_test1.reboot_required + - ansible_selinux.config_mode == 'enforcing' + - ansible_selinux.type == 'targeted' + +- name: TEST 4 | Set SELinux to permissive in check mode + selinux: + state: permissive + policy: targeted + register: _check_mode_test2 + check_mode: yes + +- name: TEST 4 | Re-gather facts + setup: + +- debug: + var: ansible_selinux + verbosity: 1 + tags: debug + +- name: TEST 4 | Assert that check mode doesn't set state permissive and returns changed + assert: + that: + - _check_mode_test2 is changed + - not _check_mode_test2.reboot_required + - ansible_selinux.config_mode == 'enforcing' + - ansible_selinux.type == 'targeted' + +- name: TEST 4 | Disable SELinux in check mode + selinux: + state: disabled + register: _check_mode_test3 + check_mode: yes + +- name: TEST 4 | Re-gather facts + setup: + +- debug: + var: ansible_selinux + verbosity: 1 + tags: debug + +- name: TEST 4 | Assert that check mode didn't change anything, status is changed, reboot_required is True, a warning was displayed + assert: + that: + - _check_mode_test3 is changed + - _check_mode_test3.reboot_required + - (_check_mode_test3.warnings | length ) >= 1 + - ansible_selinux.config_mode == 'enforcing' + - ansible_selinux.type == 'targeted' + +- name: TEST 4 | Set SELinux to permissive + selinux: + state: permissive + policy: targeted + register: _check_mode_test4 + +- debug: + var: _check_mode_test4 + verbosity: 1 + +- name: TEST 4 | Disable SELinux in check mode + selinux: + state: disabled + register: _check_mode_test4 + check_mode: yes + +- name: TEST 4 | Re-gather facts + setup: + +- debug: + var: ansible_selinux + verbosity: 1 + tags: debug + +- name: TEST 4 | Assert that check mode didn't change anything, status is changed, reboot_required is True, a warning was displayed + assert: + that: + - _check_mode_test4 is changed + - _check_mode_test4.reboot_required + - (_check_mode_test3.warnings | length ) >= 1 + - ansible_selinux.config_mode == 'permissive' + - ansible_selinux.type == 'targeted' + +- name: TEST 4 | Set SELinux to enforcing + selinux: + state: enforcing + policy: targeted + register: _check_mode_test5 + +- debug: + var: _check_mode_test5 + verbosity: 1 + +- name: TEST 4 | Disable SELinux + selinux: + state: disabled + register: _check_mode_test5 + +- name: TEST 4 | Disable SELinux in check mode + selinux: + state: disabled + register: _check_mode_test5 + check_mode: yes + +- name: TEST 4 | Re-gather facts + setup: + +- debug: + var: ansible_selinux + verbosity: 1 + tags: debug + +- name: TEST 4 | Assert that in check mode status was not changed, reboot_required is True, a warning was displayed, and SELinux is configured properly + assert: + that: + - _check_mode_test5 is success + - _check_mode_test5.reboot_required + - (_check_mode_test5.warnings | length ) >= 1 + - ansible_selinux.config_mode == 'disabled' + - ansible_selinux.type == 'targeted'