From 69ec272982bd6dd4cef762cde7dc35cf93d87229 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Fri, 2 Sep 2016 11:25:31 -0700 Subject: [PATCH] Python3 fixes to copy, file, and stat so that the connection integration tests can be run (#4632) * Python3 fixes to copy, file, and stat so that the connection integration tests can be run * Forgot to audit the helper functions as well. * Fix dest to refledt b_dest (found by @mattclay) --- lib/ansible/modules/files/copy.py | 98 ++++++++------- lib/ansible/modules/files/file.py | 196 ++++++++++++++++-------------- lib/ansible/modules/files/stat.py | 8 +- 3 files changed, 163 insertions(+), 139 deletions(-) diff --git a/lib/ansible/modules/files/copy.py b/lib/ansible/modules/files/copy.py index 9ec8c659525..9066226540a 100644 --- a/lib/ansible/modules/files/copy.py +++ b/lib/ansible/modules/files/copy.py @@ -18,9 +18,6 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . -import os -import tempfile - DOCUMENTATION = ''' --- module: copy @@ -183,16 +180,28 @@ state: sample: "file" ''' +import os +import shutil +import tempfile +import traceback + +# import module snippets +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.pycompat24 import get_exception +from ansible.module_utils._text import to_bytes, to_native + + def split_pre_existing_dir(dirname): ''' Return the first pre-existing directory and a list of the new directories that will be created. ''' head, tail = os.path.split(dirname) - if not os.path.exists(head): + b_head = to_bytes(head, errors='surrogate_or_strict') + if not os.path.exists(b_head): (pre_existing_dir, new_directory_list) = split_pre_existing_dir(head) else: - return (head, [ tail ]) + return (head, [tail]) new_directory_list.append(tail) return (pre_existing_dir, new_directory_list) @@ -215,10 +224,10 @@ def main(): module = AnsibleModule( # not checking because of daisy chain to file module argument_spec = dict( - src = dict(required=False), - original_basename = dict(required=False), # used to handle 'dest is a directory' via template, a slight hack + src = dict(required=False, type='path'), + original_basename = dict(required=False), # used to handle 'dest is a directory' via template, a slight hack content = dict(required=False, no_log=True), - dest = dict(required=True), + dest = dict(required=True, type='path'), backup = dict(default=False, type='bool'), force = dict(default=True, aliases=['thirsty'], type='bool'), validate = dict(required=False, type='str'), @@ -229,21 +238,23 @@ def main(): supports_check_mode=True, ) - src = os.path.expanduser(module.params['src']) - dest = os.path.expanduser(module.params['dest']) + src = module.params['src'] + b_src = to_bytes(src, errors='surrogate_or_strict') + dest = module.params['dest'] + b_dest = to_bytes(dest, errors='surrogate_or_strict') backup = module.params['backup'] - force = module.params['force'] - original_basename = module.params.get('original_basename',None) - validate = module.params.get('validate',None) + force = module.params['force'] + original_basename = module.params.get('original_basename', None) + validate = module.params.get('validate', None) follow = module.params['follow'] - mode = module.params['mode'] + mode = module.params['mode'] remote_src = module.params['remote_src'] - if not os.path.exists(src): + if not os.path.exists(b_src): module.fail_json(msg="Source %s not found" % (src)) - if not os.access(src, os.R_OK): + if not os.access(b_src, os.R_OK): module.fail_json(msg="Source %s not readable" % (src)) - if os.path.isdir(src): + if os.path.isdir(b_src): module.fail_json(msg="Remote copy does not support recursive copy of directory: %s" % (src)) checksum_src = module.sha1(src) @@ -259,10 +270,12 @@ def main(): # Special handling for recursive copy - create intermediate dirs if original_basename and dest.endswith(os.sep): dest = os.path.join(dest, original_basename) + b_dest = to_bytes(dest, errors='surrogate_or_strict') dirname = os.path.dirname(dest) - if not os.path.exists(dirname) and os.path.isabs(dirname): + b_dirname = to_bytes(dirname, errors='surrogate_or_strict') + if not os.path.exists(b_dirname) and os.path.isabs(b_dirname): (pre_existing_dir, new_directory_list) = split_pre_existing_dir(dirname) - os.makedirs(dirname) + os.makedirs(b_dirname) directory_args = module.load_file_common_arguments(module.params) directory_mode = module.params["directory_mode"] if directory_mode is not None: @@ -271,45 +284,47 @@ def main(): directory_args['mode'] = None adjust_recursive_directory_permissions(pre_existing_dir, new_directory_list, module, directory_args, changed) - if os.path.exists(dest): - if os.path.islink(dest) and follow: - dest = os.path.realpath(dest) + if os.path.exists(b_dest): + if os.path.islink(b_dest) and follow: + b_dest = os.path.realpath(b_dest) + dest = to_native(b_dest, errors='surrogate_or_strict') if not force: module.exit_json(msg="file already exists", src=src, dest=dest, changed=False) - if (os.path.isdir(dest)): + if os.path.isdir(b_dest): basename = os.path.basename(src) if original_basename: basename = original_basename dest = os.path.join(dest, basename) - if os.access(dest, os.R_OK): + b_dest = to_bytes(dest, errors='surrogate_or_strict') + if os.access(b_dest, os.R_OK): checksum_dest = module.sha1(dest) else: - if not os.path.exists(os.path.dirname(dest)): + if not os.path.exists(os.path.dirname(b_dest)): try: # os.path.exists() can return false in some # circumstances where the directory does not have # the execute bit for the current user set, in # which case the stat() call will raise an OSError - os.stat(os.path.dirname(dest)) + os.stat(os.path.dirname(b_dest)) except OSError: e = get_exception() - if "permission denied" in str(e).lower(): + if "permission denied" in to_native(e).lower(): module.fail_json(msg="Destination directory %s is not accessible" % (os.path.dirname(dest))) module.fail_json(msg="Destination directory %s does not exist" % (os.path.dirname(dest))) - if not os.access(os.path.dirname(dest), os.W_OK): + if not os.access(os.path.dirname(b_dest), os.W_OK): module.fail_json(msg="Destination %s not writable" % (os.path.dirname(dest))) backup_file = None - if checksum_src != checksum_dest or os.path.islink(dest): + if checksum_src != checksum_dest or os.path.islink(b_dest): if not module.check_mode: try: if backup: - if os.path.exists(dest): + if os.path.exists(b_dest): backup_file = module.backup_local(dest) # allow for conversion from symlink. - if os.path.islink(dest): - os.unlink(dest) - open(dest, 'w').close() + if os.path.islink(b_dest): + os.unlink(b_dest) + open(b_dest, 'w').close() if validate: # if we have a mode, make sure we set it on the temporary # file source as some validations may require it @@ -318,14 +333,14 @@ def main(): module.set_mode_if_different(src, mode, False) if "%s" not in validate: module.fail_json(msg="validate must contain %%s: %s" % (validate)) - (rc,out,err) = module.run_command(validate % src) + (rc, out, err) = module.run_command(validate % src) if rc != 0: module.fail_json(msg="failed to validate", exit_status=rc, stdout=out, stderr=err) - mysrc = src + b_mysrc = b_src if remote_src: - _, mysrc = tempfile.mkstemp(dir=os.path.dirname(dest)) - shutil.copy2(src, mysrc) - module.atomic_move(mysrc, dest, unsafe_writes=module.params['unsafe_writes']) + _, b_mysrc = tempfile.mkstemp(dir=os.path.dirname(b_dest)) + shutil.copy2(b_src, b_mysrc) + module.atomic_move(b_mysrc, dest, unsafe_writes=module.params['unsafe_writes']) except IOError: module.fail_json(msg="failed to copy: %s to %s" % (src, dest), traceback=traceback.format_exc()) changed = True @@ -333,7 +348,7 @@ def main(): changed = False res_args = dict( - dest = dest, src = src, md5sum = md5sum_src, checksum = checksum_src, changed = changed + dest=dest, src=src, md5sum=md5sum_src, checksum=checksum_src, changed=changed ) if backup_file: res_args['backup_file'] = backup_file @@ -345,6 +360,5 @@ def main(): module.exit_json(**res_args) -# import module snippets -from ansible.module_utils.basic import * -main() +if __name__ == '__main__': + main() diff --git a/lib/ansible/modules/files/file.py b/lib/ansible/modules/files/file.py index f23c1d51e9e..d9203a2a73f 100644 --- a/lib/ansible/modules/files/file.py +++ b/lib/ansible/modules/files/file.py @@ -18,12 +18,6 @@ # You should have received a copy of the GNU General Public License # along with Ansible. If not, see . -import errno -import shutil -import stat -import grp -import pwd - DOCUMENTATION = ''' --- module: file @@ -37,7 +31,7 @@ description: notes: - See also M(copy), M(template), M(assemble) requirements: [ ] -author: +author: - "Ansible Core Team" - "Michael DeHaan" options: @@ -112,16 +106,27 @@ EXAMPLES = ''' ''' +import errno +import os +import shutil +import time -def get_state(path): +# import module snippets +from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.pycompat24 import get_exception +from ansible.module_utils.six import b +from ansible.module_utils._text import to_bytes, to_native + + +def get_state(b_path): ''' Find out current state ''' - if os.path.lexists(path): - if os.path.islink(path): + if os.path.lexists(b_path): + if os.path.islink(b_path): return 'link' - elif os.path.isdir(path): + elif os.path.isdir(b_path): return 'directory' - elif os.stat(path).st_nlink > 1: + elif os.stat(b_path).st_nlink > 1: return 'hard' else: # could be many other things, but defaulting to file @@ -129,70 +134,73 @@ def get_state(path): return 'absent' -def recursive_set_attributes(module, path, follow, file_args): + +def recursive_set_attributes(module, b_path, follow, file_args): changed = False - for root, dirs, files in os.walk(path): - for fsobj in dirs + files: - fsname = os.path.join(root, fsobj) - if not os.path.islink(fsname): + for b_root, b_dirs, b_files in os.walk(b_path): + for b_fsobj in b_dirs + b_files: + b_fsname = os.path.join(b_root, b_fsobj) + if not os.path.islink(b_fsname): tmp_file_args = file_args.copy() - tmp_file_args['path']=fsname + tmp_file_args['path'] = to_native(b_fsname, errors='surrogate_or_strict') changed |= module.set_fs_attributes_if_different(tmp_file_args, changed) else: tmp_file_args = file_args.copy() - tmp_file_args['path']=fsname + tmp_file_args['path'] = to_native(b_fsname, errors='surrogate_or_strict') changed |= module.set_fs_attributes_if_different(tmp_file_args, changed) if follow: - fsname = os.path.join(root, os.readlink(fsname)) - if os.path.isdir(fsname): - changed |= recursive_set_attributes(module, fsname, follow, file_args) + b_fsname = os.path.join(b_root, os.readlink(b_fsname)) + if os.path.isdir(b_fsname): + changed |= recursive_set_attributes(module, b_fsname, follow, file_args) tmp_file_args = file_args.copy() - tmp_file_args['path']=fsname + tmp_file_args['path'] = to_native(b_fsname, errors='surrogate_or_strict') changed |= module.set_fs_attributes_if_different(tmp_file_args, changed) return changed + def main(): module = AnsibleModule( - argument_spec = dict( - state = dict(choices=['file','directory','link','hard','touch','absent'], default=None), - path = dict(aliases=['dest', 'name'], required=True), - original_basename = dict(required=False), # Internal use only, for recursive ops - recurse = dict(default=False, type='bool'), - force = dict(required=False, default=False, type='bool'), - diff_peek = dict(default=None), # Internal use only, for internal checks in the action plugins - validate = dict(required=False, default=None), # Internal use only, for template and copy - src = dict(required=False, default=None), + argument_spec=dict( + state=dict(choices=['file', 'directory', 'link', 'hard', 'touch', 'absent'], default=None), + path=dict(aliases=['dest', 'name'], required=True, type='path'), + original_basename=dict(required=False), # Internal use only, for recursive ops + recurse=dict(default=False, type='bool'), + force=dict(required=False, default=False, type='bool'), + diff_peek=dict(default=None), # Internal use only, for internal checks in the action plugins + validate=dict(required=False, default=None), # Internal use only, for template and copy + src=dict(required=False, default=None, type='path'), ), add_file_common_args=True, supports_check_mode=True ) params = module.params - state = params['state'] + state = params['state'] force = params['force'] diff_peek = params['diff_peek'] src = params['src'] + b_src = to_bytes(src, errors='surrogate_or_strict') follow = params['follow'] # modify source as we later reload and pass, specially relevant when used by other modules. - params['path'] = path = os.path.expanduser(params['path']) + path = params['path'] + b_path = to_bytes(path, errors='surrogate_or_strict') # short-circuit for diff_peek if diff_peek is not None: appears_binary = False try: - f = open(path) - b = f.read(8192) + f = open(b_path, 'rb') + head = f.read(8192) f.close() - if "\x00" in b: + if b("\x00") in head: appears_binary = True except: pass module.exit_json(path=path, changed=False, appears_binary=appears_binary) - prev_state = get_state(path) - + prev_state = get_state(b_path) # state should default to file, but since that creates many conflicts, # default to 'current' when it exists. @@ -204,18 +212,16 @@ def main(): # 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 not None: - src = os.path.expanduser(src) - else: - if state in ['link','hard']: + if src is None: + if state in ('link', 'hard'): if follow and state == 'link': # use the current target of the link as the source - src = os.path.realpath(path) + src = to_native(os.path.realpath(b_path), errors='strict') else: module.fail_json(msg='src and dest are required for creating links') # original_basename is used by other modules that depend on file. - if os.path.isdir(path) and state not in ["link", "absent"]: + if os.path.isdir(b_path) and state not in ("link", "absent"): basename = None if params['original_basename']: basename = params['original_basename'] @@ -223,6 +229,7 @@ def main(): basename = os.path.basename(src) if basename: params['path'] = path = os.path.join(path, basename) + b_path = to_bytes(path, errors='surrogate_or_strict') # make sure the target path is a directory when we're doing a recursive operation recurse = params['recurse'] @@ -232,11 +239,8 @@ def main(): file_args = module.load_file_common_arguments(params) changed = False - diff = {'before': - {'path': path} - , - 'after': - {'path': path} + diff = {'before': {'path': path}, + 'after': {'path': path}, } state_change = False @@ -250,13 +254,13 @@ def main(): if not module.check_mode: if prev_state == 'directory': try: - shutil.rmtree(path, ignore_errors=False) + shutil.rmtree(b_path, ignore_errors=False) except Exception: e = get_exception() module.fail_json(msg="rmtree failed: %s" % str(e)) else: try: - os.unlink(path) + os.unlink(b_path) except Exception: e = get_exception() module.fail_json(path=path, msg="unlinking failed: %s " % str(e)) @@ -269,11 +273,12 @@ def main(): if state_change: if follow and prev_state == 'link': # follow symlink and operate on original - path = os.path.realpath(path) - prev_state = get_state(path) + b_path = os.path.realpath(b_path) + path = to_native(b_path, errors='strict') + prev_state = get_state(b_path) file_args['path'] = path - if prev_state not in ['file','hard']: + if prev_state not in ('file', 'hard'): # file is not absent and any other state is a conflict module.fail_json(path=path, msg='file (%s) is %s, cannot continue' % (path, prev_state)) @@ -282,8 +287,9 @@ def main(): elif state == 'directory': if follow and prev_state == 'link': - path = os.path.realpath(path) - prev_state = get_state(path) + b_path = os.path.realpath(b_path) + path = to_native(b_path, errors='strict') + prev_state = get_state(b_path) if prev_state == 'absent': if module.check_mode: @@ -301,17 +307,18 @@ def main(): # Remove leading slash if we're creating a relative path if not os.path.isabs(path): curpath = curpath.lstrip('/') - if not os.path.exists(curpath): + b_curpath = to_bytes(curpath, errors='surrogate_or_strict') + if not os.path.exists(b_curpath): try: - os.mkdir(curpath) + os.mkdir(b_curpath) except OSError: ex = get_exception() # Possibly something else created the dir since the os.path.exists # check above. As long as it's a dir, we don't need to error out. - if not (ex.errno == errno.EEXIST and os.path.isdir(curpath)): + if not (ex.errno == errno.EEXIST and os.path.isdir(b_curpath)): raise tmp_file_args = file_args.copy() - tmp_file_args['path']=curpath + tmp_file_args['path'] = curpath changed = module.set_fs_attributes_if_different(tmp_file_args, changed, diff) except Exception: e = get_exception() @@ -324,45 +331,47 @@ def main(): changed = module.set_fs_attributes_if_different(file_args, changed, diff) if recurse: - changed |= recursive_set_attributes(module, file_args['path'], follow, file_args) + changed |= recursive_set_attributes(module, to_bytes(file_args['path'], errors='surrogate_or_strict'), follow, file_args) module.exit_json(path=path, changed=changed, diff=diff) - elif state in ['link','hard']: + elif state in ('link', 'hard'): - if os.path.isdir(path) and not os.path.islink(path): + if os.path.isdir(b_path) and not os.path.islink(b_path): relpath = path else: - relpath = os.path.dirname(path) + b_relpath = os.path.dirname(b_path) + relpath = to_native(b_relpath, errors='strict') absrc = os.path.join(relpath, src) - if not os.path.exists(absrc) and not force: + b_absrc = to_bytes(absrc, errors='surrogate_or_strict') + if not os.path.exists(b_absrc) and not force: module.fail_json(path=path, src=src, msg='src file does not exist, use "force=yes" if you really want to create the link: %s' % absrc) if state == 'hard': - if not os.path.isabs(src): + if not os.path.isabs(b_src): module.fail_json(msg="absolute paths are required") elif prev_state == 'directory': if not force: module.fail_json(path=path, msg='refusing to convert between %s and %s for %s' % (prev_state, state, path)) - elif len(os.listdir(path)) > 0: + elif len(os.listdir(b_path)) > 0: # refuse to replace a directory that has files in it module.fail_json(path=path, msg='the directory %s is not empty, refusing to convert it' % path) - elif prev_state in ['file', 'hard'] and not force: + elif prev_state in ('file', 'hard') and not force: module.fail_json(path=path, msg='refusing to convert between %s and %s for %s' % (prev_state, state, path)) if prev_state == 'absent': changed = True elif prev_state == 'link': - old_src = os.readlink(path) - if old_src != src: + b_old_src = os.readlink(b_path) + if b_old_src != b_src: changed = True elif prev_state == 'hard': - if not (state == 'hard' and os.stat(path).st_ino == os.stat(src).st_ino): + if not (state == 'hard' and os.stat(b_path).st_ino == os.stat(b_src).st_ino): changed = True if not force: module.fail_json(dest=path, src=src, msg='Cannot link, different hard link exists at destination') - elif prev_state in ['file', 'directory']: + elif prev_state in ('file', 'directory'): changed = True if not force: module.fail_json(dest=path, src=src, msg='Cannot link, %s exists at destination' % prev_state) @@ -372,31 +381,33 @@ def main(): if changed and not module.check_mode: if prev_state != 'absent': # try to replace atomically - tmppath = '/'.join([os.path.dirname(path), ".%s.%s.tmp" % (os.getpid(),time.time())]) + b_tmppath = to_bytes(os.path.sep).join( + [os.path.dirname(b_path), to_bytes(".%s.%s.tmp" % (os.getpid(), time.time()))] + ) try: if prev_state == 'directory' and (state == 'hard' or state == 'link'): - os.rmdir(path) + os.rmdir(b_path) if state == 'hard': - os.link(src,tmppath) + os.link(b_src, b_tmppath) else: - os.symlink(src, tmppath) - os.rename(tmppath, path) + os.symlink(b_src, b_tmppath) + os.rename(b_tmppath, b_path) except OSError: e = get_exception() - if os.path.exists(tmppath): - os.unlink(tmppath) - module.fail_json(path=path, msg='Error while replacing: %s' % str(e)) + if os.path.exists(b_tmppath): + os.unlink(b_tmppath) + module.fail_json(path=path, msg='Error while replacing: %s' % to_native(e, nonstring='simplerepr')) else: try: if state == 'hard': - os.link(src,path) + os.link(b_src, b_path) else: - os.symlink(src, path) + os.symlink(b_src, b_path) except OSError: e = get_exception() - module.fail_json(path=path, msg='Error while linking: %s' % str(e)) + module.fail_json(path=path, msg='Error while linking: %s' % to_native(e, nonstring='simplerepr')) - if module.check_mode and not os.path.exists(path): + if module.check_mode and not os.path.exists(b_path): module.exit_json(dest=path, src=src, changed=changed, diff=diff) changed = module.set_fs_attributes_if_different(file_args, changed, diff) @@ -407,16 +418,16 @@ def main(): if prev_state == 'absent': try: - open(path, 'w').close() + open(b_path, 'wb').close() except OSError: e = get_exception() - module.fail_json(path=path, msg='Error, could not touch target: %s' % str(e)) - elif prev_state in ['file', 'directory', 'hard']: + module.fail_json(path=path, msg='Error, could not touch target: %s' % to_native(e, nonstring='simplerepr')) + elif prev_state in ('file', 'directory', 'hard'): try: - os.utime(path, None) + os.utime(b_path, None) except OSError: e = get_exception() - module.fail_json(path=path, msg='Error while touching existing target: %s' % str(e)) + module.fail_json(path=path, msg='Error while touching existing target: %s' % to_native(e, nonstring='simplerepr')) else: module.fail_json(msg='Cannot touch other than files, directories, and hardlinks (%s is %s)' % (path, prev_state)) try: @@ -428,15 +439,12 @@ def main(): # somewhere in basic.py if prev_state == 'absent': # If we just created the file we can safely remove it - os.remove(path) + os.remove(b_path) raise e module.exit_json(dest=path, changed=True, diff=diff) module.fail_json(path=path, msg='unexpected position reached') -# import module snippets -from ansible.module_utils.basic import * if __name__ == '__main__': main() - diff --git a/lib/ansible/modules/files/stat.py b/lib/ansible/modules/files/stat.py index f28ddcc0cc1..38ede211643 100644 --- a/lib/ansible/modules/files/stat.py +++ b/lib/ansible/modules/files/stat.py @@ -328,6 +328,7 @@ import stat # import module snippets from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.pycompat24 import get_exception +from ansible.module_utils._text import to_bytes def format_output(module, path, st, follow, get_md5, get_checksum, @@ -369,7 +370,7 @@ def format_output(module, path, st, follow, get_md5, get_checksum, readable=os.access(path, os.R_OK), writeable=os.access(path, os.W_OK), excutable=os.access(path, os.X_OK), - ) + ) if stat.S_ISLNK(mode): output['lnk_source'] = os.path.realpath(path) @@ -417,6 +418,7 @@ def main(): ) path = module.params.get('path') + b_path = to_bytes(path, errors='surrogate_or_strict') follow = module.params.get('follow') get_mime = module.params.get('mime') get_md5 = module.params.get('get_md5') @@ -425,9 +427,9 @@ def main(): try: if follow: - st = os.stat(path) + st = os.stat(b_path) else: - st = os.lstat(path) + st = os.lstat(b_path) except OSError: e = get_exception() if e.errno == errno.ENOENT: