From 18f2ed63e057667a21ea7194e3c100cb0af80026 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Wed, 5 Jun 2019 15:26:40 +0200 Subject: [PATCH] file: fix setting relative paths for src for hard links (#56055) --- .../55971-file-relative-path-hard-link.yaml | 2 + lib/ansible/modules/files/file.py | 37 ++-------- test/integration/targets/file/tasks/main.yml | 74 +++++++++++++++++-- 3 files changed, 77 insertions(+), 36 deletions(-) create mode 100644 changelogs/fragments/55971-file-relative-path-hard-link.yaml diff --git a/changelogs/fragments/55971-file-relative-path-hard-link.yaml b/changelogs/fragments/55971-file-relative-path-hard-link.yaml new file mode 100644 index 00000000000..0d06c14c34c --- /dev/null +++ b/changelogs/fragments/55971-file-relative-path-hard-link.yaml @@ -0,0 +1,2 @@ +bugfixes: + - file - Fix setting relative paths for hard links (https://github.com/ansible/ansible/issues/55971) diff --git a/lib/ansible/modules/files/file.py b/lib/ansible/modules/files/file.py index 0656dce4df9..f64046f8cba 100644 --- a/lib/ansible/modules/files/file.py +++ b/lib/ansible/modules/files/file.py @@ -52,8 +52,7 @@ options: description: - Path of the file to link to. - This applies only to C(state=link) and C(state=hard). - - Will accept absolute and non-existing paths. - - Will accept relative paths unless state=hard. + - For C(state=link), this will also accept a non-existing path. - Relative paths are relative to the file being created (C(path)) which is how the Unix command C(ln -s SRC DEST) treats relative paths. type: path @@ -758,35 +757,13 @@ def ensure_hardlink(path, src, follow, force, 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']) - # 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): - raise AnsibleModuleError(results={'msg': 'src and dest are required for creating new hardlinks'}) - - # Toshio: Begin suspect block - # I believe that this block of code is wrong for hardlinks. - # src may be relative. - # If it is relative, it should be relative to the cwd (so just use abspath). - # This is different from symlinks where src is relative to the symlink's path. - - # Why must src be an absolute path? - if not os.path.isabs(b_src): - raise AnsibleModuleError(results={'msg': "src must be an absolute path"}) - - # If this is a link, then it can't be a dir so why is it in the conditional? - if not os.path.islink(b_path) and os.path.isdir(b_path): - relpath = path - else: - b_relpath = os.path.dirname(b_path) - relpath = to_native(b_relpath, errors='strict') + # src is the source of a hardlink. We require it if we are creating a new hardlink. + # We require path in the argument_spec so we know it is present at this point. + if src is None: + raise AnsibleModuleError(results={'msg': 'src is required for creating new hardlinks'}) - # Why? This does nothing because src was checked to be absolute above? - absrc = os.path.join(relpath, src) - b_absrc = to_bytes(absrc, errors='surrogate_or_strict') - if not force and not os.path.exists(b_absrc): - raise AnsibleModuleError(results={'msg': 'src file does not exist, use "force=yes" if you' - ' really want to create the link: %s' % absrc, - 'path': path, 'src': src}) - # Toshio: end suspect block + if not os.path.exists(b_src): + raise AnsibleModuleError(results={'msg': 'src does not exist', 'dest': path, 'src': src}) diff = initial_diff(path, 'hard', prev_state) changed = False diff --git a/test/integration/targets/file/tasks/main.yml b/test/integration/targets/file/tasks/main.yml index 52957fff054..2c473329d27 100644 --- a/test/integration/targets/file/tasks/main.yml +++ b/test/integration/targets/file/tasks/main.yml @@ -563,9 +563,6 @@ stat: path={{output_dir}}/test_follow_rec_target_dir/foo register: file_in_dir_result -- debug: var=file_result.stat.mode -- debug: var=dir_result.stat.mode -- debug: var=file_in_dir_result.stat.mode - name: assert that the link targets were unmodified assert: that: @@ -588,12 +585,77 @@ stat: path={{output_dir}}/test_follow_rec_target_dir/foo register: file_in_dir_result -- debug: var=file_result.stat.mode -- debug: var=dir_result.stat.mode -- debug: var=file_in_dir_result.stat.mode - name: assert that the link targets were modified assert: that: - file_result.stat.mode == '0600' - dir_result.stat.mode == '0600' - file_in_dir_result.stat.mode == '0600' + +# https://github.com/ansible/ansible/issues/55971 +- name: Test missing src and path + file: + state: hard + register: file_error1 + ignore_errors: yes + +- assert: + that: + - "file_error1 is failed" + - "file_error1.msg == 'missing required arguments: path'" + +- name: Test missing src + file: + dest: "{{ output_dir }}/hard.txt" + state: hard + register: file_error2 + ignore_errors: yes + +- assert: + that: + - "file_error2 is failed" + - "file_error2.msg == 'src is required for creating new hardlinks'" + +- name: Test non-existing src + file: + src: non-existing-file-that-does-not-exist.txt + dest: "{{ output_dir }}/hard.txt" + state: hard + register: file_error3 + ignore_errors: yes + +- assert: + that: + - "file_error3 is failed" + - "file_error3.msg == 'src does not exist'" + - "file_error3.dest == '{{ output_dir }}/hard.txt' | expanduser" + - "file_error3.src == 'non-existing-file-that-does-not-exist.txt'" + +- block: + - name: Create a testing file + file: + dest: original_file.txt + state: touch + + - name: Test relative path with state=hard + file: + src: original_file.txt + dest: hard_link_file.txt + state: hard + register: hard_link_relpath + + - name: Just check if it was successful, we don't care about the actual hard link in this test + assert: + that: + - "hard_link_relpath is success" + + always: + - name: Clean up + file: + path: "{{ item }}" + state: absent + loop: + - original_file.txt + - hard_link_file.txt + +# END #55971