From 3f1838bf918a055b491ab25d6caee2e5022db3c1 Mon Sep 17 00:00:00 2001 From: Matthew Donoughe Date: Wed, 10 Aug 2022 16:01:31 -0400 Subject: [PATCH] Update attributes of files that are links without specifying link target (#76167) * update attributes of files that are symlinks * update attributes of files that are hard links * fix default state in documentation * remove unnecessary suppression * add to changelog --- ...ate-attributes-of-files-that-are-links.yml | 2 ++ lib/ansible/modules/file.py | 34 ++++++++++++------- test/integration/targets/file/tasks/main.yml | 2 +- .../targets/file/tasks/state_link.yml | 14 +++++++- test/sanity/ignore.txt | 1 - 5 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 changelogs/fragments/76167-update-attributes-of-files-that-are-links.yml diff --git a/changelogs/fragments/76167-update-attributes-of-files-that-are-links.yml b/changelogs/fragments/76167-update-attributes-of-files-that-are-links.yml new file mode 100644 index 00000000000..083df07dc23 --- /dev/null +++ b/changelogs/fragments/76167-update-attributes-of-files-that-are-links.yml @@ -0,0 +1,2 @@ +bugfixes: + - file - setting attributes of symbolic links or files that are hard linked no longer fails when the link target is unspecified (https://github.com/ansible/ansible/issues/76142). diff --git a/lib/ansible/modules/file.py b/lib/ansible/modules/file.py index 44963778583..55489bcda50 100644 --- a/lib/ansible/modules/file.py +++ b/lib/ansible/modules/file.py @@ -43,8 +43,8 @@ options: - If C(touch) (new in 1.4), an empty file will be created if the file does not exist, while an existing file or directory will receive updated file access and modification times (similar to the way C(touch) works from the command line). + - Default is the current state of the file if it exists, C(directory) if C(recurse=yes), or C(file) otherwise. type: str - default: file choices: [ absent, directory, file, hard, link, touch ] src: description: @@ -689,7 +689,7 @@ def ensure_symlink(path, src, follow, force, timestamps): # 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: - if follow: + if follow and os.path.exists(b_path): # use the current target of the link as the source src = to_native(os.readlink(b_path), errors='strict') b_src = to_bytes(src, errors='surrogate_or_strict') @@ -700,9 +700,14 @@ def ensure_symlink(path, src, follow, force, timestamps): b_relpath = os.path.dirname(b_path) relpath = to_native(b_relpath, errors='strict') - absrc = os.path.join(relpath, src) + # If src is None that means we are expecting to update an existing link. + if src is None: + absrc = None + else: + 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): + if not force and src is not None 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}) @@ -726,13 +731,16 @@ def ensure_symlink(path, src, follow, force, timestamps): changed = False if prev_state in ('hard', 'file', 'directory', 'absent'): + if src is None: + raise AnsibleModuleError(results={'msg': 'src is required for creating new symlinks'}) changed = True elif prev_state == 'link': - b_old_src = os.readlink(b_path) - if b_old_src != b_src: - diff['before']['src'] = to_native(b_old_src, errors='strict') - diff['after']['src'] = src - changed = True + if src is not None: + b_old_src = os.readlink(b_path) + if b_old_src != b_src: + diff['before']['src'] = to_native(b_old_src, errors='strict') + diff['after']['src'] = src + changed = True else: raise AnsibleModuleError(results={'msg': 'unexpected position reached', 'dest': path, 'src': src}) @@ -793,10 +801,12 @@ def ensure_hardlink(path, src, follow, force, timestamps): # 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: + if prev_state != 'hard' and src is None: raise AnsibleModuleError(results={'msg': 'src is required for creating new hardlinks'}) - if not os.path.exists(b_src): + # Even if the link already exists, if src was specified it needs to exist. + # The inode number will be compared to ensure the link has the correct target. + if src is not None and 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) @@ -811,7 +821,7 @@ def ensure_hardlink(path, src, follow, force, timestamps): diff['after']['src'] = src changed = True elif prev_state == 'hard': - if not os.stat(b_path).st_ino == os.stat(b_src).st_ino: + if src is not None and not os.stat(b_path).st_ino == os.stat(b_src).st_ino: changed = True if not force: raise AnsibleModuleError(results={'msg': 'Cannot link, different hard link exists at destination', diff --git a/test/integration/targets/file/tasks/main.yml b/test/integration/targets/file/tasks/main.yml index a74cbc289e5..4d7d942b6d9 100644 --- a/test/integration/targets/file/tasks/main.yml +++ b/test/integration/targets/file/tasks/main.yml @@ -287,7 +287,7 @@ - "hlink_result.changed == False" - name: Change mode on a hard link - file: src={{output_file}} dest={{remote_tmp_dir_test}}/hard.txt mode=0701 + file: dest={{remote_tmp_dir_test}}/hard.txt mode=0701 register: file6_mode_change - name: verify that the hard link was touched diff --git a/test/integration/targets/file/tasks/state_link.yml b/test/integration/targets/file/tasks/state_link.yml index ec6c07127f8..673fe6fd529 100644 --- a/test/integration/targets/file/tasks/state_link.yml +++ b/test/integration/targets/file/tasks/state_link.yml @@ -375,7 +375,6 @@ - name: attempt to modify the permissions of the link itself file: path: '{{remote_tmp_dir_test}}/test_follow_link' - src: './test_follow' state: 'link' mode: 0600 follow: False @@ -445,6 +444,19 @@ - src_state is failed - "'src option requires state to be' in src_state.msg" +- name: Create without src + file: + state: link + dest: "{{ output_dir }}/link.txt" + ignore_errors: yes + register: create_without_src + +- name: Ensure create without src failed + assert: + that: + - create_without_src is failed + - "'src is required' in create_without_src.msg" + # Test creating a symlink when the destination exists and is a file - name: create a test file copy: diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index cc55e6772d2..3ba64b43520 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -32,7 +32,6 @@ lib/ansible/modules/copy.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/copy.py validate-modules:undocumented-parameter lib/ansible/modules/dnf.py validate-modules:doc-required-mismatch lib/ansible/modules/dnf.py validate-modules:parameter-invalid -lib/ansible/modules/file.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/file.py validate-modules:undocumented-parameter lib/ansible/modules/find.py use-argspec-type-path # fix needed lib/ansible/modules/git.py pylint:disallowed-name