From a78cc15099d6a31e948eb5aa6f9f3cc82337f5bc Mon Sep 17 00:00:00 2001 From: Stephen SORRIAUX Date: Fri, 24 Aug 2018 02:20:54 +0200 Subject: [PATCH] Add new parameters to manage mtime and atime for file module, fixes #30226 (#43230) * Add new parameters related to mtime and atime for file module, fixes #30226 --- lib/ansible/modules/files/file.py | 198 ++++++++++++++---- .../targets/file/tasks/directory_as_dest.yml | 29 +++ test/integration/targets/file/tasks/main.yml | 18 ++ 3 files changed, 203 insertions(+), 42 deletions(-) diff --git a/lib/ansible/modules/files/file.py b/lib/ansible/modules/files/file.py index 963c6dda60d..64921d14a33 100644 --- a/lib/ansible/modules/files/file.py +++ b/lib/ansible/modules/files/file.py @@ -76,6 +76,30 @@ options: type: bool default: 'yes' version_added: "1.8" + modification_time: + description: + - This parameter indicates the time the file's modification time should be set to + - 'Should be C(preserve) when no modification is required, C(YYYYMMDDHHMM.SS) when using default time format, or C(now)' + - 'Default is None meaning that C(preserve) is the default for C(state=[file,directory,link,hard]) and C(now) is default for C(state=touch)' + version_added: "2.7" + modification_time_format: + description: + - 'When used with C(modification_time), indicates the time format that must be used.' + - 'Based on default Python format (see time.strftime doc)' + default: "%Y%m%d%H%M.%S" + version_added: "2.7" + access_time: + description: + - This parameter indicates the time the file's access time should be set to + - 'Should be C(preserve) when no modification is required, C(YYYYMMDDHHMM.SS) when using default time format, or C(now)' + - 'Default is None meaning that C(preserve) is the default for C(state=[file,directory,link,hard]) and C(now) is default for C(state=touch)' + version_added: "2.7" + access_time_format: + description: + - 'When used with C(access_time), indicates the time format that must be used.' + - 'Based on default Python format (see time.strftime doc)' + default: "%Y%m%d%H%M.%S" + version_added: "2.7" ''' EXAMPLES = ''' @@ -116,11 +140,31 @@ EXAMPLES = ''' state: touch mode: "u+rw,g-wx,o-rwx" +# touch again the same file, but dont change times +# this makes the task idempotents +- file: + path: /etc/foo.conf + state: touch + mode: "u+rw,g-wx,o-rwx" + modification_time: "preserve" + access_time: "preserve" + # create a directory if it doesn't exist - file: path: /etc/some_directory state: directory mode: 0755 + +# updates modification and access time of given file +- file: + path: /etc/some_file + state: file + mode: 0755 + modification_time: now + access_time: now +''' +RETURN = ''' + ''' import errno @@ -233,7 +277,7 @@ def get_state(path): # This should be moved into the common file utilities -def recursive_set_attributes(b_path, follow, file_args): +def recursive_set_attributes(b_path, follow, file_args, mtime, atime): changed = False for b_root, b_dirs, b_files in os.walk(b_path): for b_fsobj in b_dirs + b_files: @@ -242,11 +286,14 @@ def recursive_set_attributes(b_path, follow, file_args): tmp_file_args = file_args.copy() tmp_file_args['path'] = to_native(b_fsname, errors='surrogate_or_strict') changed |= module.set_fs_attributes_if_different(tmp_file_args, changed, expand=False) + changed |= update_timestamp_for_file(tmp_file_args['path'], mtime, atime) + else: # Change perms on the link tmp_file_args = file_args.copy() tmp_file_args['path'] = to_native(b_fsname, errors='surrogate_or_strict') changed |= module.set_fs_attributes_if_different(tmp_file_args, changed, expand=False) + changed |= update_timestamp_for_file(tmp_file_args['path'], mtime, atime) if follow: b_fsname = os.path.join(b_root, os.readlink(b_fsname)) @@ -254,12 +301,13 @@ def recursive_set_attributes(b_path, follow, file_args): if os.path.exists(b_fsname): if os.path.isdir(b_fsname): # Link is a directory so change perms on the directory's contents - changed |= recursive_set_attributes(b_fsname, follow, file_args) + changed |= recursive_set_attributes(b_fsname, follow, file_args, mtime, atime) # Change perms on the file pointed to by the link tmp_file_args = file_args.copy() tmp_file_args['path'] = to_native(b_fsname, errors='surrogate_or_strict') changed |= module.set_fs_attributes_if_different(tmp_file_args, changed, expand=False) + changed |= update_timestamp_for_file(tmp_file_args['path'], mtime, atime) return changed @@ -279,6 +327,70 @@ def initial_diff(path, state, prev_state): # +def get_timestamp_for_time(formatted_time, time_format): + if formatted_time == 'preserve': + return None + elif formatted_time == 'now': + current_time = time.time() + return current_time + else: + try: + struct = time.strptime(formatted_time, time_format) + struct_time = time.mktime(struct) + except (ValueError, OverflowError) as e: + raise AnsibleModuleError(results={'msg': 'Error while obtaining timestamp for time %s using format %s: %s' + % (formatted_time, time_format, to_native(e, nonstring='simplerepr'))}) + + return struct_time + + +def update_timestamp_for_file(path, mtime, atime, diff=None): + # If both parameters are None, nothing to do + if mtime is None and atime is None: + return False + + try: + previous_mtime = os.stat(path).st_mtime + previous_atime = os.stat(path).st_atime + + if mtime is None: + mtime = previous_mtime + + if atime is None: + atime = previous_atime + + # If both timestamps are already ok, nothing to do + if mtime == previous_mtime and atime == previous_atime: + return False + + os.utime(path, (atime, mtime)) + + if diff is not None: + if 'before' not in diff: + diff['before'] = {} + if 'after' not in diff: + diff['after'] = {} + if mtime != previous_mtime: + diff['before']['mtime'] = previous_mtime + diff['after']['mtime'] = mtime + if atime != previous_atime: + diff['before']['atime'] = previous_atime + diff['after']['atime'] = atime + except OSError as e: + raise AnsibleModuleError(results={'msg': 'Error while updating modification or access time: %s' + % to_native(e, nonstring='simplerepr'), 'path': path}) + return True + + +def keep_backward_compatibility_on_timestamps(parameter, state): + if state in ['file', 'hard', 'directory', 'link'] and parameter is None: + return 'preserve' + elif state == 'touch' and parameter is None: + return 'now' + else: + return parameter + + def execute_diff_peek(path): """Take a guess as to whether a file is a binary file""" b_path = to_bytes(path, errors='surrogate_or_strict') @@ -324,51 +436,31 @@ def ensure_absent(path): return result -def execute_touch(path, follow): +def execute_touch(path, follow, timestamps): b_path = to_bytes(path, errors='surrogate_or_strict') prev_state = get_state(b_path) - - # Unfortunately, touch always changes the file because it updates file's timestamp - result = {'dest': path, 'changed': True} + changed = False + result = {'dest': path} + 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}) - elif prev_state in ('file', 'directory', 'hard'): - # Update the timestamp if the file already existed - try: - os.utime(b_path, None) - except OSError as e: - raise AnsibleModuleError(results={'msg': 'Error while touching existing target: %s' - % to_native(e, nonstring='simplerepr'), - 'path': path}) - - elif prev_state == 'link' and follow: - # Update the timestamp of the pointed to file - b_link_target = os.readlink(b_path) - try: - os.utime(b_link_target, None) - except OSError as e: - raise AnsibleModuleError(results={'msg': 'Error while touching existing target: %s' - % to_native(e, nonstring='simplerepr'), - 'path': path}) - - else: - raise AnsibleModuleError(results={'msg': 'Can only touch files, directories, and' - ' hardlinks (%s is %s)' % (path, prev_state)}) - # Update the attributes on the file diff = initial_diff(path, 'touch', prev_state) file_args = module.load_file_common_arguments(module.params) try: - module.set_fs_attributes_if_different(file_args, True, diff, expand=False) + 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: # We take this to mean that fail_json() was called from @@ -378,14 +470,17 @@ def execute_touch(path, follow): os.remove(b_path) raise + result['changed'] = changed result['diff'] = diff return result -def ensure_file_attributes(path, follow): +def ensure_file_attributes(path, follow, timestamps): b_path = to_bytes(path, errors='surrogate_or_strict') prev_state = get_state(b_path) file_args = module.load_file_common_arguments(module.params) + 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 prev_state != 'file': if follow and prev_state == 'link': @@ -402,13 +497,16 @@ def ensure_file_attributes(path, follow): diff = initial_diff(path, 'file', prev_state) changed = module.set_fs_attributes_if_different(file_args, False, diff, expand=False) + changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) return {'path': path, 'changed': changed, 'diff': diff} -def ensure_directory(path, follow, recurse): +def ensure_directory(path, follow, recurse, timestamps): b_path = to_bytes(path, errors='surrogate_or_strict') prev_state = get_state(b_path) file_args = module.load_file_common_arguments(module.params) + mtime = get_timestamp_for_time(timestamps['modification_time'], timestamps['modification_time_format']) + atime = get_timestamp_for_time(timestamps['access_time'], timestamps['access_time_format']) # For followed symlinks, we need to operate on the target of the link if follow and prev_state == 'link': @@ -450,6 +548,7 @@ def ensure_directory(path, follow, recurse): tmp_file_args = file_args.copy() tmp_file_args['path'] = curpath changed = module.set_fs_attributes_if_different(tmp_file_args, changed, diff, expand=False) + changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) except Exception as e: raise AnsibleModuleError(results={'msg': 'There was an issue creating %s as requested:' ' %s' % (curpath, to_native(e)), @@ -466,19 +565,20 @@ def ensure_directory(path, follow, recurse): # changed = module.set_fs_attributes_if_different(file_args, changed, diff, expand=False) - + changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) if recurse: - changed |= recursive_set_attributes(b_path, follow, file_args) + changed |= recursive_set_attributes(b_path, follow, file_args, mtime, atime) return {'path': path, 'changed': changed, 'diff': diff} -def ensure_symlink(path, src, follow, force): +def ensure_symlink(path, src, follow, force, timestamps): b_path = to_bytes(path, errors='surrogate_or_strict') b_src = to_bytes(src, errors='surrogate_or_strict') prev_state = get_state(b_path) file_args = module.load_file_common_arguments(module.params) - + mtime = get_timestamp_for_time(timestamps['modification_time'], timestamps['modification_time_format']) + atime = get_timestamp_for_time(timestamps['access_time'], timestamps['access_time_format']) # source is both the source of a symlink or an informational passing of the src for a template module # or copy module, even if this module never uses it, it is needed to key off some things if src is None: @@ -581,15 +681,18 @@ def ensure_symlink(path, src, follow, force): ' set to False to avoid this.') else: changed = module.set_fs_attributes_if_different(file_args, changed, diff, expand=False) + changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) return {'dest': path, 'src': src, 'changed': changed, 'diff': diff} -def ensure_hardlink(path, src, follow, force): +def ensure_hardlink(path, src, follow, force, timestamps): b_path = to_bytes(path, errors='surrogate_or_strict') b_src = to_bytes(src, errors='surrogate_or_strict') prev_state = get_state(b_path) file_args = module.load_file_common_arguments(module.params) + mtime = get_timestamp_for_time(timestamps['modification_time'], timestamps['modification_time_format']) + atime = get_timestamp_for_time(timestamps['access_time'], timestamps['access_time_format']) # src is the source of a hardlink. We require it if we are creating a new hardlink if src is None and not os.path.exists(b_path): @@ -688,6 +791,7 @@ def ensure_hardlink(path, src, follow, force): return {'dest': path, 'src': src, 'changed': changed, 'diff': diff} changed = module.set_fs_attributes_if_different(file_args, changed, diff, expand=False) + changed |= update_timestamp_for_file(file_args['path'], mtime, atime, diff) return {'dest': path, 'src': src, 'changed': changed, 'diff': diff} @@ -706,6 +810,10 @@ def main(): follow=dict(required=False, default=True, type='bool'), # Note: Different default than file_common_args _diff_peek=dict(default=None), # Internal use only, for internal checks in the action plugins src=dict(required=False, default=None, type='path'), # Note: Should not be in file_common_args in future + modification_time=dict(required=False, default=None), + modification_time_format=dict(required=False, default='%Y%m%d%H%M.%S'), + access_time=dict(required=False, default=None), + access_time_format=dict(required=False, default='%Y%m%d%H%M.%S'), ), add_file_common_args=True, supports_check_mode=True @@ -723,21 +831,27 @@ def main(): path = params['path'] src = params['src'] + timestamps = {} + timestamps['modification_time'] = keep_backward_compatibility_on_timestamps(params['modification_time'], state) + timestamps['modification_time_format'] = params['modification_time_format'] + timestamps['access_time'] = keep_backward_compatibility_on_timestamps(params['access_time'], state) + timestamps['access_time_format'] = params['access_time_format'] + # short-circuit for diff_peek if params['_diff_peek'] is not None: appears_binary = execute_diff_peek(to_bytes(path, errors='surrogate_or_strict')) module.exit_json(path=path, changed=False, appears_binary=appears_binary) if state == 'file': - result = ensure_file_attributes(path, follow) + result = ensure_file_attributes(path, follow, timestamps) elif state == 'directory': - result = ensure_directory(path, follow, recurse) + result = ensure_directory(path, follow, recurse, timestamps) elif state == 'link': - result = ensure_symlink(path, src, follow, force) + result = ensure_symlink(path, src, follow, force, timestamps) elif state == 'hard': - result = ensure_hardlink(path, src, follow, force) + result = ensure_hardlink(path, src, follow, force, timestamps) elif state == 'touch': - result = execute_touch(path, follow) + result = execute_touch(path, follow, timestamps) elif state == 'absent': result = ensure_absent(path) diff --git a/test/integration/targets/file/tasks/directory_as_dest.yml b/test/integration/targets/file/tasks/directory_as_dest.yml index decc3835fb7..b51fa72a7fd 100644 --- a/test/integration/targets/file/tasks/directory_as_dest.yml +++ b/test/integration/targets/file/tasks/directory_as_dest.yml @@ -262,6 +262,35 @@ - 'file8_dir_stat["stat"].isdir' - 'file8_dir_stat["stat"]["mtime"] != file8_initial_dir_stat["stat"]["mtime"]' +- name: Get initial stat info to compare with later + stat: + path: '{{ output_dir }}/sub1' + follow: False + register: file11_initial_dir_stat + +- name: Use touch with directory as dest and keep mtime and atime + file: + dest: '{{output_dir}}/sub1' + state: touch + force: False + modification_time: preserve + access_time: preserve + register: file11_result + +- name: Get stat info to show the directory has not been changed + stat: + path: '{{ output_dir }}/sub1' + follow: False + register: file11_dir_stat + +- name: verify that the directory has not been updated + assert: + that: + - 'file11_result is not changed' + - 'file11_dir_stat["stat"].isdir' + - 'file11_dir_stat["stat"]["mtime"] == file11_initial_dir_stat["stat"]["mtime"]' + - 'file11_dir_stat["stat"]["atime"] == file11_initial_dir_stat["stat"]["atime"]' + # # State=directory realizes that the directory already exists and does nothing # diff --git a/test/integration/targets/file/tasks/main.yml b/test/integration/targets/file/tasks/main.yml index c27a90b5120..a8e903f4be6 100644 --- a/test/integration/targets/file/tasks/main.yml +++ b/test/integration/targets/file/tasks/main.yml @@ -117,6 +117,24 @@ - name: change ownership and group file: path={{output_dir}}/baz.txt owner=1234 group=1234 +- name: Get stat info to check atime later + stat: path={{output_dir}}/baz.txt + register: file_attributes_result_5_before + +- name: updates access time + file: path={{output_dir}}/baz.txt access_time=now + register: file_attributes_result_5 + +- name: Get stat info to check atime later + stat: path={{output_dir}}/baz.txt + register: file_attributes_result_5_after + +- name: verify that the file was marked as changed and atime changed + assert: + that: + - "file_attributes_result_5 is changed" + - "file_attributes_result_5_after['stat']['atime'] != file_attributes_result_5_before['stat']['atime']" + - name: setup a tmp-like directory for ownership test file: path=/tmp/worldwritable mode=1777 state=directory