From d398a4b4f0999e3fc2c22e98a1b5b1253d031fb5 Mon Sep 17 00:00:00 2001 From: Andrew Klychkov Date: Thu, 3 Sep 2020 16:11:50 +0300 Subject: [PATCH] file: module should warn in check_mode when path and owner/group don't exist (#69640) * file: module must fail in check_mode when path and owner/group don't exist --- ...hen_path_and_owner_or_group_dont_exist.yml | 2 + lib/ansible/modules/file.py | 38 ++++++ test/integration/targets/file/tasks/main.yml | 119 ++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 changelogs/fragments/69640-file_should_warn_when_path_and_owner_or_group_dont_exist.yml diff --git a/changelogs/fragments/69640-file_should_warn_when_path_and_owner_or_group_dont_exist.yml b/changelogs/fragments/69640-file_should_warn_when_path_and_owner_or_group_dont_exist.yml new file mode 100644 index 00000000000..97b7c8bd794 --- /dev/null +++ b/changelogs/fragments/69640-file_should_warn_when_path_and_owner_or_group_dont_exist.yml @@ -0,0 +1,2 @@ +bugfixes: +- file - the module should warn in check_mode when path an owner/group don't exist (https://github.com/ansible/ansible/issues/67307). diff --git a/lib/ansible/modules/file.py b/lib/ansible/modules/file.py index 5ffab319224..06921a8d607 100644 --- a/lib/ansible/modules/file.py +++ b/lib/ansible/modules/file.py @@ -225,6 +225,9 @@ import shutil import sys import time +from pwd import getpwnam, getpwuid +from grp import getgrnam, getgrgid + from ansible.module_utils.basic import AnsibleModule from ansible.module_utils._text import to_bytes, to_native @@ -860,6 +863,34 @@ def ensure_hardlink(path, src, follow, force, timestamps): return {'dest': path, 'src': src, 'changed': changed, 'diff': diff} +def check_owner_exists(module, owner): + try: + uid = int(owner) + try: + getpwuid(uid).pw_name + except KeyError: + module.warn('failed to look up user with uid %s. Create user up to this point in real play' % uid) + except ValueError: + try: + getpwnam(owner).pw_uid + except KeyError: + module.warn('failed to look up user %s. Create user up to this point in real play' % owner) + + +def check_group_exists(module, group): + try: + gid = int(group) + try: + getgrgid(gid).gr_name + except KeyError: + module.warn('failed to look up group with gid %s. Create group up to this point in real play' % gid) + except ValueError: + try: + getgrnam(group).gr_gid + except KeyError: + module.warn('failed to look up group %s. Create group up to this point in real play' % group) + + def main(): global module @@ -895,6 +926,13 @@ def main(): path = params['path'] src = params['src'] + if module.check_mode and state != 'absent': + file_args = module.load_file_common_arguments(module.params) + if file_args['owner']: + check_owner_exists(module, file_args['owner']) + if file_args['group']: + check_group_exists(module, file_args['group']) + timestamps = {} timestamps['modification_time'] = keep_backward_compatibility_on_timestamps(params['modification_time'], state) timestamps['modification_time_format'] = params['modification_time_format'] diff --git a/test/integration/targets/file/tasks/main.yml b/test/integration/targets/file/tasks/main.yml index 4e1d88e6bae..4a8811da0bd 100644 --- a/test/integration/targets/file/tasks/main.yml +++ b/test/integration/targets/file/tasks/main.yml @@ -143,6 +143,16 @@ - "file_attributes_result_4 is not changed" when: file_attributes_result_4 is changed +- name: create user + user: + name: test1 + uid: 1234 + +- name: create group + group: + name: test1 + gid: 1234 + - name: change ownership and group file: path={{output_dir}}/baz.txt owner=1234 group=1234 @@ -330,6 +340,11 @@ that: - "file9_result.uid != 1234" +- name: create user + user: + name: test2 + uid: 1235 + - name: change the ownership of a directory with recurse=yes file: path={{output_dir}}/foobar owner=1235 recurse=yes @@ -539,6 +554,110 @@ that: - result.mode == '0444' +# https://github.com/ansible/ansible/issues/67307 +# Test the module fails in check_mode when directory and owner/group do not exist +# I don't use state=touch here intentionally to fail and catch warnings +- name: owner does not exist in check_mode + file: + path: '/tmp/nonexistent' + owner: nonexistent + check_mode: yes + register: owner_no_exist + ignore_errors: yes + +- name: create owner + user: + name: nonexistent + +# I don't use state=touch here intentionally to fail and catch warnings +- name: owner exist in check_mode + file: + path: '/tmp/nonexistent' + owner: nonexistent + check_mode: yes + register: owner_exists + ignore_errors: yes + +# I don't use state=touch here intentionally to fail and catch warnings +- name: owner does not exist in check_mode, using uid + file: + path: '/tmp/nonexistent' + owner: '111111' + check_mode: yes + ignore_errors: yes + register: owner_uid_no_exist + +- name: create owner using uid + user: + name: test_uid + uid: 111111 + +# I don't use state=touch here intentionally to fail and catch warnings +- name: owner exists in check_mode, using uid + file: + path: '/tmp/nonexistent' + owner: '111111' + state: touch + check_mode: yes + ignore_errors: yes + register: owner_uid_exists + +# I don't use state=touch here intentionally to fail and catch warnings +- name: group does not exist in check_mode + file: + path: '/tmp/nonexistent' + group: nonexistent1 + check_mode: yes + register: group_no_exist + ignore_errors: yes + +- name: create group + group: + name: nonexistent1 + +# I don't use state=touch here intentionally to fail and catch warnings +- name: group exists in check_mode + file: + path: '/tmp/nonexistent' + group: nonexistent1 + check_mode: yes + register: group_exists + ignore_errors: yes + +# I don't use state=touch here intentionally to fail and catch warnings +- name: group does not exist in check_mode, using gid + file: + path: '/tmp/nonexistent' + group: '111112' + check_mode: yes + register: group_gid_no_exist + ignore_errors: yes + +- name: create group with gid + group: + name: test_gid + gid: 111112 + +# I don't use state=touch here intentionally to fail and catch warnings +- name: group exists in check_mode, using gid + file: + path: '/tmp/nonexistent' + group: '111112' + check_mode: yes + register: group_gid_exists + ignore_errors: yes + +- assert: + that: + - owner_no_exist.warnings[0] is search('failed to look up user') + - owner_uid_no_exist.warnings[0] is search('failed to look up user with uid') + - group_no_exist.warnings[0] is search('failed to look up group') + - group_gid_no_exist.warnings[0] is search('failed to look up group with gid') + - owner_exists.warnings is not defined + - owner_uid_exists.warnings is not defined + - group_exists.warnings is not defined + - group_gid_exists.warnings is not defined + # https://github.com/ansible/ansible/issues/50943 # Need to use /tmp as nobody can't access output_dir at all - name: create file as root with all write permissions