From 3936b5c471068d86c3e51a454a1de2f0d2942845 Mon Sep 17 00:00:00 2001 From: sbettid Date: Tue, 29 Nov 2022 16:17:38 +0100 Subject: Fix file touch check mode result (#79360) (#79422) Fixes #79360 --- .../79422-fix-file-touch-check-mode-status.yaml | 3 + lib/ansible/modules/file.py | 58 +++++++++-------- test/integration/targets/file/tasks/main.yml | 76 ++++++++++++++++++++++ 3 files changed, 110 insertions(+), 27 deletions(-) create mode 100644 changelogs/fragments/79422-fix-file-touch-check-mode-status.yaml diff --git a/changelogs/fragments/79422-fix-file-touch-check-mode-status.yaml b/changelogs/fragments/79422-fix-file-touch-check-mode-status.yaml new file mode 100644 index 0000000000..757cf33e24 --- /dev/null +++ b/changelogs/fragments/79422-fix-file-touch-check-mode-status.yaml @@ -0,0 +1,3 @@ +bugfixes: + - file - touch action in check mode was always returning ok. Fix now evaluates the different conditions and + returns the appropriate changed status. (https://github.com/ansible/ansible/issues/79360) diff --git a/lib/ansible/modules/file.py b/lib/ansible/modules/file.py index b691d3d976..72b510c3ca 100644 --- a/lib/ansible/modules/file.py +++ b/lib/ansible/modules/file.py @@ -553,34 +553,38 @@ def execute_touch(path, follow, timestamps): mtime = get_timestamp_for_time(timestamps['modification_time'], timestamps['modification_time_format']) atime = get_timestamp_for_time(timestamps['access_time'], timestamps['access_time_format']) - if not module.check_mode: - if prev_state == 'absent': - # Create an empty file if the filename did not already exist - try: - open(b_path, 'wb').close() - changed = True - except (OSError, IOError) as e: - raise AnsibleModuleError(results={'msg': 'Error, could not touch target: %s' - % to_native(e, nonstring='simplerepr'), - 'path': path}) - - # Update the attributes on the file - diff = initial_diff(path, 'touch', prev_state) - file_args = module.load_file_common_arguments(module.params) + # If the file did not already exist + if prev_state == 'absent': + # if we are in check mode and the file is absent + # we can set the changed status to True and return + if module.check_mode: + result['changed'] = True + return result + # Create an empty file try: - changed = module.set_fs_attributes_if_different(file_args, changed, diff, expand=False) - changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) - except SystemExit as e: - if e.code: # this is the exit code passed to sys.exit, not a constant -- pylint: disable=using-constant-test - # We take this to mean that fail_json() was called from - # somewhere in basic.py - if prev_state == 'absent': - # If we just created the file we can safely remove it - os.remove(b_path) - raise - - result['changed'] = changed - result['diff'] = diff + open(b_path, 'wb').close() + changed = True + except (OSError, IOError) as e: + raise AnsibleModuleError(results={'msg': 'Error, could not touch target: %s' + % to_native(e, nonstring='simplerepr'), + 'path': path}) + # Update the attributes on the file + diff = initial_diff(path, 'touch', prev_state) + file_args = module.load_file_common_arguments(module.params) + try: + changed = module.set_fs_attributes_if_different(file_args, changed, diff, expand=False) + changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) + except SystemExit as e: + if e.code: # this is the exit code passed to sys.exit, not a constant -- pylint: disable=using-constant-test + # We take this to mean that fail_json() was called from + # somewhere in basic.py + if prev_state == 'absent': + # If we just created the file we can safely remove it + os.remove(b_path) + raise + + result['changed'] = changed + result['diff'] = diff return result diff --git a/test/integration/targets/file/tasks/main.yml b/test/integration/targets/file/tasks/main.yml index 3aed491762..17b0fae68a 100644 --- a/test/integration/targets/file/tasks/main.yml +++ b/test/integration/targets/file/tasks/main.yml @@ -711,6 +711,82 @@ - group_exists.warnings is not defined - group_gid_exists.warnings is not defined +# ensures touching a file returns changed when needed +# issue: https://github.com/ansible/ansible/issues/79360 +- name: touch a file returns changed in check mode if file does not exist + file: + path: '/tmp/touch_check_mode_test' + state: touch + check_mode: yes + register: touch_result_in_check_mode_not_existing + +- name: touch the file + file: + path: '/tmp/touch_check_mode_test' + mode: "0660" + state: touch + +- name: touch an existing file returns changed in check mode + file: + path: '/tmp/touch_check_mode_test' + state: touch + check_mode: yes + register: touch_result_in_check_mode_change_all_attr + +- name: touch an existing file returns changed in check mode when preserving access time + file: + path: '/tmp/touch_check_mode_test' + state: touch + access_time: "preserve" + check_mode: yes + register: touch_result_in_check_mode_preserve_access_time + +- name: touch an existing file returns changed in check mode when only mode changes + file: + path: '/tmp/touch_check_mode_test' + state: touch + access_time: "preserve" + modification_time: "preserve" + mode: "0640" + check_mode: yes + register: touch_result_in_check_mode_change_only_mode + +- name: touch an existing file returns ok if all attributes are preserved + file: + path: '/tmp/touch_check_mode_test' + state: touch + access_time: "preserve" + modification_time: "preserve" + check_mode: yes + register: touch_result_in_check_mode_all_attrs_preserved + +- name: touch an existing file fails in check mode when user does not exist + file: + path: '/tmp/touch_check_mode_test' + state: touch + owner: not-existing-user + check_mode: yes + ignore_errors: true + register: touch_result_in_check_mode_fails_not_existing_user + +- name: touch an existing file fails in check mode when group does not exist + file: + path: '/tmp/touch_check_mode_test' + state: touch + group: not-existing-group + check_mode: yes + ignore_errors: true + register: touch_result_in_check_mode_fails_not_existing_group + +- assert: + that: + - touch_result_in_check_mode_not_existing.changed + - touch_result_in_check_mode_preserve_access_time.changed + - touch_result_in_check_mode_change_only_mode.changed + - not touch_result_in_check_mode_all_attrs_preserved.changed + - touch_result_in_check_mode_fails_not_existing_user.warnings[0] is search('failed to look up user') + - touch_result_in_check_mode_fails_not_existing_group.warnings[0] is search('failed to look up group') + # https://github.com/ansible/ansible/issues/50943 # Need to use /tmp as nobody can't access remote_tmp_dir_test at all - name: create file as root with all write permissions -- cgit v1.2.1