diff options
-rw-r--r-- | changelogs/fragments/unsafe_writes_env.yml | 2 | ||||
-rw-r--r-- | changelogs/fragments/unsafe_writes_fix.yml | 4 | ||||
-rw-r--r-- | lib/ansible/module_utils/basic.py | 38 | ||||
-rw-r--r-- | lib/ansible/modules/net_tools/basics/get_url.py | 2 | ||||
-rw-r--r-- | test/integration/targets/unsafe_writes/aliases | 6 | ||||
-rw-r--r-- | test/integration/targets/unsafe_writes/basic.yml | 68 | ||||
-rwxr-xr-x | test/integration/targets/unsafe_writes/runme.sh | 12 | ||||
-rw-r--r-- | test/units/module_utils/basic/test_atomic_move.py | 1 |
8 files changed, 113 insertions, 20 deletions
diff --git a/changelogs/fragments/unsafe_writes_env.yml b/changelogs/fragments/unsafe_writes_env.yml new file mode 100644 index 0000000000..38d833d551 --- /dev/null +++ b/changelogs/fragments/unsafe_writes_env.yml @@ -0,0 +1,2 @@ +minor_changes: + - Allow unsafe_writes to be set on target via env var, for those targets that need a blanket setting. diff --git a/changelogs/fragments/unsafe_writes_fix.yml b/changelogs/fragments/unsafe_writes_fix.yml new file mode 100644 index 0000000000..1993ac3853 --- /dev/null +++ b/changelogs/fragments/unsafe_writes_fix.yml @@ -0,0 +1,4 @@ +bugfixes: + - Restored unsafe_writes functionality which was being skipped. + - Added unsafe_writes test. + - Enabled unsafe_writes for get_url which was ignoring the paramter. diff --git a/lib/ansible/module_utils/basic.py b/lib/ansible/module_utils/basic.py index adb95e9706..a531f3032c 100644 --- a/lib/ansible/module_utils/basic.py +++ b/lib/ansible/module_utils/basic.py @@ -228,6 +228,15 @@ _literal_eval = literal_eval # is an internal implementation detail _ANSIBLE_ARGS = None + +def env_fallback(*args, **kwargs): + ''' Load value from environment ''' + for arg in args: + if arg in os.environ: + return os.environ[arg] + raise AnsibleFallbackNotFound + + FILE_COMMON_ARGUMENTS = dict( # These are things we want. About setting metadata (mode, ownership, permissions in general) on # created files (these are used by set_fs_attributes_if_different and included in @@ -255,7 +264,7 @@ FILE_COMMON_ARGUMENTS = dict( regexp=dict(), # used by assemble delimiter=dict(), # used by assemble directory_mode=dict(), # used by copy - unsafe_writes=dict(type='bool'), # should be available to any module using atomic_move + unsafe_writes=dict(type='bool', default=False, fallback=(env_fallback, ['ANSIBLE_UNSAFE_WRITES'])), # should be available to any module using atomic_move ) PASSWD_ARG_RE = re.compile(r'^[-]{0,2}pass[-]?(word|wd)?') @@ -641,14 +650,6 @@ def _load_params(): sys.exit(1) -def env_fallback(*args, **kwargs): - ''' Load value from environment ''' - for arg in args: - if arg in os.environ: - return os.environ[arg] - raise AnsibleFallbackNotFound - - def missing_required_lib(library, reason=None, url=None): hostname = platform.node() msg = "Failed to import the required Python library (%s) on %s's Python %s." % (library, hostname, sys.executable) @@ -2357,8 +2358,7 @@ class AnsibleModule(object): if e.errno not in [errno.EPERM, errno.EXDEV, errno.EACCES, errno.ETXTBSY, errno.EBUSY]: # only try workarounds for errno 18 (cross device), 1 (not permitted), 13 (permission denied) # and 26 (text file busy) which happens on vagrant synced folders and other 'exotic' non posix file systems - self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, to_native(e)), - exception=traceback.format_exc()) + self.fail_json(msg='Could not replace file: %s to %s: %s' % (src, dest, to_native(e)), exception=traceback.format_exc()) else: # Use bytes here. In the shippable CI, this fails with # a UnicodeError with surrogateescape'd strings for an unknown @@ -2368,14 +2368,13 @@ class AnsibleModule(object): error_msg = None tmp_dest_name = None try: - tmp_dest_fd, tmp_dest_name = tempfile.mkstemp(prefix=b'.ansible_tmp', - dir=b_dest_dir, suffix=b_suffix) + tmp_dest_fd, tmp_dest_name = tempfile.mkstemp(prefix=b'.ansible_tmp', dir=b_dest_dir, suffix=b_suffix) except (OSError, IOError) as e: error_msg = 'The destination directory (%s) is not writable by the current user. Error was: %s' % (os.path.dirname(dest), to_native(e)) except TypeError: # We expect that this is happening because python3.4.x and - # below can't handle byte strings in mkstemp(). Traceback - # would end in something like: + # below can't handle byte strings in mkstemp(). + # Traceback would end in something like: # file = _os.path.join(dir, pre + name + suf) # TypeError: can't concat bytes to str error_msg = ('Failed creating tmp file for atomic move. This usually happens when using Python3 less than Python3.5. ' @@ -2419,11 +2418,12 @@ class AnsibleModule(object): self._unsafe_writes(b_tmp_dest_name, b_dest) else: self.fail_json(msg='Unable to make %s into to %s, failed final rename from %s: %s' % - (src, dest, b_tmp_dest_name, to_native(e)), - exception=traceback.format_exc()) + (src, dest, b_tmp_dest_name, to_native(e)), exception=traceback.format_exc()) except (shutil.Error, OSError, IOError) as e: - self.fail_json(msg='Failed to replace file: %s to %s: %s' % (src, dest, to_native(e)), - exception=traceback.format_exc()) + if unsafe_writes: + self._unsafe_writes(b_src, b_dest) + else: + self.fail_json(msg='Failed to replace file: %s to %s: %s' % (src, dest, to_native(e)), exception=traceback.format_exc()) finally: self.cleanup(b_tmp_dest_name) diff --git a/lib/ansible/modules/net_tools/basics/get_url.py b/lib/ansible/modules/net_tools/basics/get_url.py index 3c84cb60e9..2c9e95e415 100644 --- a/lib/ansible/modules/net_tools/basics/get_url.py +++ b/lib/ansible/modules/net_tools/basics/get_url.py @@ -620,7 +620,7 @@ def main(): if backup: if os.path.exists(dest): backup_file = module.backup_local(dest) - module.atomic_move(tmpsrc, dest) + module.atomic_move(tmpsrc, dest, unsafe_writes=module.params['unsafe_writes']) except Exception as e: if os.path.exists(tmpsrc): os.remove(tmpsrc) diff --git a/test/integration/targets/unsafe_writes/aliases b/test/integration/targets/unsafe_writes/aliases new file mode 100644 index 0000000000..4fb7a11640 --- /dev/null +++ b/test/integration/targets/unsafe_writes/aliases @@ -0,0 +1,6 @@ +needs/root +skip/freebsd +skip/osx +skip/macos +skip/aix +shippable/posix/group3 diff --git a/test/integration/targets/unsafe_writes/basic.yml b/test/integration/targets/unsafe_writes/basic.yml new file mode 100644 index 0000000000..410726ad0e --- /dev/null +++ b/test/integration/targets/unsafe_writes/basic.yml @@ -0,0 +1,68 @@ +- hosts: testhost + gather_facts: false + vars: + testudir: '{{output_dir}}/unsafe_writes_test' + testufile: '{{testudir}}/unreplacablefile.txt' + tasks: + - name: test unsafe_writes on immutable dir (file cannot be atomically replaced) + block: + - name: create target dir + file: path={{testudir}} state=directory + - name: setup test file + copy: content=ORIGINAL dest={{testufile}} + - name: make target dir immutable (cannot write to file w/o unsafe_writes) + file: path={{testudir}} state=directory attributes="+i" + become: yes + ignore_errors: true + register: madeimmutable + + - name: only run if immutable dir command worked, some of our test systems don't allow for it + when: madeimmutable is success + block: + - name: test this is actually immmutable working as we expect + file: path={{testufile}} state=absent + register: breakimmutable + ignore_errors: True + + - name: only run if reallyh immutable dir + when: breakimmutable is failed + block: + - name: test overwriting file w/o unsafe + copy: content=NEW dest={{testufile}} unsafe_writes=False + ignore_errors: true + register: copy_without + + - name: ensure we properly failed + assert: + that: + - copy_without is failed + + - name: test overwriting file with unsafe + copy: content=NEWNOREALLY dest={{testufile}} unsafe_writes=True + register: copy_with + + - name: ensure we properly changed + assert: + that: + - copy_with is changed + + - name: test fallback env var + when: lookup('env', 'ANSIBLE_UNSAFE_WRITES') not in ('', None) + vars: + env_enabled: "{{lookup('env', 'ANSIBLE_UNSAFE_WRITES')|bool}}" + block: + - name: test overwriting file with unsafe depending on fallback environment setting + copy: content=NEWBUTNOTDIFFERENT dest={{testufile}} + register: copy_with_env + ignore_errors: True + + - name: ensure we properly follow env var + assert: + msg: "Failed with envvar: {{env_enabled}}, due AUW: to {{q('env', 'ANSIBLE_UNSAFE_WRITES')}}" + that: + - env_enabled and copy_with_env is changed or not env_enabled and copy_with_env is failed + always: + - name: remove immutable flag from dir to prevent issues with cleanup + file: path={{testudir}} state=directory attributes="-i" + ignore_errors: true + become: yes diff --git a/test/integration/targets/unsafe_writes/runme.sh b/test/integration/targets/unsafe_writes/runme.sh new file mode 100755 index 0000000000..791a5676b4 --- /dev/null +++ b/test/integration/targets/unsafe_writes/runme.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +set -eux + +# test w/o fallback env var +ansible-playbook basic.yml -i ../../inventory -e "output_dir=${OUTPUT_DIR}" "$@" + +# test enabled fallback env var +ANSIBLE_UNSAFE_WRITES=1 ansible-playbook basic.yml -i ../../inventory -e "output_dir=${OUTPUT_DIR}" "$@" + +# test disnabled fallback env var +ANSIBLE_UNSAFE_WRITES=0 ansible-playbook basic.yml -i ../../inventory -e "output_dir=${OUTPUT_DIR}" "$@" diff --git a/test/units/module_utils/basic/test_atomic_move.py b/test/units/module_utils/basic/test_atomic_move.py index 7bd9496edd..bbdb051966 100644 --- a/test/units/module_utils/basic/test_atomic_move.py +++ b/test/units/module_utils/basic/test_atomic_move.py @@ -23,6 +23,7 @@ def atomic_am(am, mocker): am.selinux_context = mocker.MagicMock() am.selinux_default_context = mocker.MagicMock() am.set_context_if_different = mocker.MagicMock() + am._unsafe_writes = mocker.MagicMock() yield am |