From ebd645a4fe9b0f844b64815c8d89397022d0e37b Mon Sep 17 00:00:00 2001 From: nicholasmhughes Date: Tue, 18 Jul 2023 18:49:54 -0400 Subject: [PATCH] fixes saltstack/salt#64665 add follow_symlinks to file.symlink exec module --- changelog/64665.added.md | 1 + salt/modules/file.py | 316 +++++++++--------- salt/states/file.py | 19 +- .../unit/modules/file/test_file_basics.py | 22 ++ 4 files changed, 191 insertions(+), 167 deletions(-) create mode 100644 changelog/64665.added.md diff --git a/changelog/64665.added.md b/changelog/64665.added.md new file mode 100644 index 00000000000..1f320613efe --- /dev/null +++ b/changelog/64665.added.md @@ -0,0 +1 @@ +Add follow_symlinks to file.symlink exec module to switch to os.path.lexists when False diff --git a/salt/modules/file.py b/salt/modules/file.py index 69d7992f5ab..47bdf410ac4 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -435,7 +435,7 @@ def set_mode(path, mode): if not mode: mode = "0" if not os.path.exists(path): - raise CommandExecutionError("{}: File not found".format(path)) + raise CommandExecutionError(f"{path}: File not found") try: os.chmod(path, int(mode, 8)) except Exception: # pylint: disable=broad-except @@ -678,14 +678,14 @@ def chattr(*files, **kwargs): cmd = ["chattr"] if operator == "add": - attrs = "+{}".format(attributes) + attrs = f"+{attributes}" elif operator == "remove": - attrs = "-{}".format(attributes) + attrs = f"-{attributes}" cmd.append(attrs) if flags is not None: - cmd.append("-{}".format(flags)) + cmd.append(f"-{flags}") if version is not None: cmd.extend(["-v", version]) @@ -828,12 +828,7 @@ def get_source_sum( "following are supported hash types and lengths: {}.".format( source_hash, ", ".join(salt.utils.files.VALID_PROTOS), - ", ".join( - [ - "{} ({})".format(HASHES_REVMAP[x], x) - for x in sorted(HASHES_REVMAP) - ] - ), + ", ".join([f"{HASHES_REVMAP[x]} ({x})" for x in sorted(HASHES_REVMAP)]), ) ) @@ -849,7 +844,7 @@ def get_source_sum( ) if not hash_fn: raise CommandExecutionError( - "Source hash file {} not found".format(source_hash) + f"Source hash file {source_hash} not found" ) else: if proto != "": @@ -879,7 +874,7 @@ def get_source_sum( _invalid_source_hash_format() except ValueError: # No hash type, try to figure out by hash length - if not re.match("^[{}]+$".format(string.hexdigits), source_hash): + if not re.match(f"^[{string.hexdigits}]+$", source_hash): _invalid_source_hash_format() ret["hsum"] = source_hash source_hash_len = len(source_hash) @@ -964,10 +959,7 @@ def check_hash(path, file_hash): file_hash, hash_len, ", ".join( - [ - "{} ({})".format(HASHES_REVMAP[x], x) - for x in sorted(HASHES_REVMAP) - ] + [f"{HASHES_REVMAP[x]} ({x})" for x in sorted(HASHES_REVMAP)] ), ) ) @@ -1097,7 +1089,7 @@ def find(path, *args, **kwargs): try: finder = salt.utils.find.Finder(kwargs) except ValueError as ex: - return "error: {}".format(ex) + return f"error: {ex}" ret = [ item @@ -1192,11 +1184,11 @@ def sed( options = options.replace("-r", "-E") cmd = ["sed"] - cmd.append("-i{}".format(backup) if backup else "-i") + cmd.append(f"-i{backup}" if backup else "-i") cmd.extend(salt.utils.args.shlex_split(options)) cmd.append( r"{limit}{negate_match}s/{before}/{after}/{flags}".format( - limit="/{}/ ".format(limit) if limit else "", + limit=f"/{limit}/ " if limit else "", negate_match="!" if negate_match else "", before=before, after=after, @@ -1240,9 +1232,9 @@ def sed_contains(path, text, limit="", flags="g"): cmd.extend(salt.utils.args.shlex_split(options)) cmd.append( r"{limit}s/{before}/$/{flags}".format( - limit="/{}/ ".format(limit) if limit else "", + limit=f"/{limit}/ " if limit else "", before=before, - flags="p{}".format(flags), + flags=f"p{flags}", ) ) cmd.append(path) @@ -1323,10 +1315,10 @@ def psed( # The pattern to replace with does not need to be escaped limit = _sed_esc(limit, escape_all) - shutil.copy2(path, "{}{}".format(path, backup)) + shutil.copy2(path, f"{path}{backup}") with salt.utils.files.fopen(path, "w") as ofile: - with salt.utils.files.fopen("{}{}".format(path, backup), "r") as ifile: + with salt.utils.files.fopen(f"{path}{backup}", "r") as ifile: if multi is True: for line in ifile.readline(): ofile.write( @@ -1509,12 +1501,12 @@ def comment_line(path, regex, char="#", cmnt=True, backup=".bak"): # Make sure the file exists if not os.path.isfile(path): - raise SaltInvocationError("File not found: {}".format(path)) + raise SaltInvocationError(f"File not found: {path}") # Make sure it is a text file if not __utils__["files.is_text"](path): raise SaltInvocationError( - "Cannot perform string replacements on a binary file: {}".format(path) + f"Cannot perform string replacements on a binary file: {path}" ) # First check the whole file, determine whether to make the replacement @@ -1536,14 +1528,12 @@ def comment_line(path, regex, char="#", cmnt=True, backup=".bak"): # Load lines into dictionaries, set found to True orig_file.append(line) if cmnt: - new_file.append("{}{}".format(char, line)) + new_file.append(f"{char}{line}") else: new_file.append(line.lstrip(char)) found = True except OSError as exc: - raise CommandExecutionError( - "Unable to open file '{}'. Exception: {}".format(path, exc) - ) + raise CommandExecutionError(f"Unable to open file '{path}'. Exception: {exc}") # We've searched the whole file. If we didn't find anything, return False if not found: @@ -1558,7 +1548,7 @@ def comment_line(path, regex, char="#", cmnt=True, backup=".bak"): try: temp_file = _mkstemp_copy(path=path, preserve_inode=False) except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") try: # Open the file in write mode @@ -1577,7 +1567,7 @@ def comment_line(path, regex, char="#", cmnt=True, backup=".bak"): if re.match(regex, line): # Write the new line if cmnt: - wline = "{}{}".format(char, line) + wline = f"{char}{line}" else: wline = line.lstrip(char) else: @@ -1593,13 +1583,13 @@ def comment_line(path, regex, char="#", cmnt=True, backup=".bak"): "Exception: {}".format(path, temp_file, exc) ) except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") if backup: # Move the backup file to the original directory - backup_name = "{}{}".format(path, backup) + backup_name = f"{path}{backup}" try: shutil.move(temp_file, backup_name) except OSError as exc: @@ -1686,9 +1676,7 @@ def _mkstemp_copy(path, preserve_inode=True): try: temp_file = salt.utils.files.mkstemp(prefix=salt.utils.files.TEMPFILE_PREFIX) except OSError as exc: - raise CommandExecutionError( - "Unable to create temp file. Exception: {}".format(exc) - ) + raise CommandExecutionError(f"Unable to create temp file. Exception: {exc}") # use `copy` to preserve the inode of the # original file, and thus preserve hardlinks # to the inode. otherwise, use `move` to @@ -1727,7 +1715,7 @@ def _regex_to_static(src, regex): compiled = re.compile(regex, re.DOTALL) src = [line for line in src if compiled.search(line) or line.count(regex)] except Exception as ex: # pylint: disable=broad-except - raise CommandExecutionError("{}: '{}'".format(_get_error_message(ex), regex)) + raise CommandExecutionError(f"{_get_error_message(ex)}: '{regex}'") return src @@ -1748,7 +1736,7 @@ def _assert_occurrence(probe, target, amount=1): if msg: raise CommandExecutionError( - 'Found {} expected occurrences in "{}" expression'.format(msg, target) + f'Found {msg} expected occurrences in "{target}" expression' ) return occ @@ -1859,7 +1847,7 @@ def _set_line( "Mode was not defined. How to process the file?" ) else: - raise CommandExecutionError("Unknown mode: {}".format(mode)) + raise CommandExecutionError(f"Unknown mode: {mode}") if mode != "delete" and content is None: raise CommandExecutionError("Content can only be empty if mode is delete") @@ -2235,7 +2223,7 @@ def line( if not os.path.isfile(path): if not quiet: raise CommandExecutionError( - 'File "{}" does not exists or is not a file.'.format(path) + f'File "{path}" does not exists or is not a file.' ) return False # No changes had happened @@ -2246,7 +2234,7 @@ def line( "Mode was not defined. How to process the file?" ) else: - raise CommandExecutionError('Unknown mode: "{}"'.format(mode)) + raise CommandExecutionError(f'Unknown mode: "{mode}"') # We've set the content to be empty in the function params but we want to make sure # it gets passed when needed. Feature #37092 @@ -2494,11 +2482,11 @@ def replace( if ignore_if_missing: return False else: - raise SaltInvocationError("File not found: {}".format(path)) + raise SaltInvocationError(f"File not found: {path}") if not __utils__["files.is_text"](path): raise SaltInvocationError( - "Cannot perform string replacements on a binary file: {}".format(path) + f"Cannot perform string replacements on a binary file: {path}" ) if search_only and (append_if_not_found or prepend_if_not_found): @@ -2577,7 +2565,7 @@ def replace( # content if it was pre/appended in a previous run. if re.search( salt.utils.stringutils.to_bytes( - "^{}($|(?=\r\n))".format(re.escape(content)) + f"^{re.escape(content)}($|(?=\r\n))" ), r_data, flags=re_flags, @@ -2595,9 +2583,7 @@ def replace( has_changes = False except OSError as exc: - raise CommandExecutionError( - "Unable to open file '{}'. Exception: {}".format(path, exc) - ) + raise CommandExecutionError(f"Unable to open file '{path}'. Exception: {exc}") finally: if r_data and isinstance(r_data, mmap.mmap): r_data.close() @@ -2608,7 +2594,7 @@ def replace( # Create a copy to read from and to use as a backup later temp_file = _mkstemp_copy(path=path, preserve_inode=preserve_inode) except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") r_data = None try: @@ -2636,12 +2622,12 @@ def replace( "Exception: {}".format(path, temp_file, exc) ) except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") finally: if r_data and isinstance(r_data, mmap.mmap): r_data.close() except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") if not found and (append_if_not_found or prepend_if_not_found): if not_found_content is None: @@ -2667,7 +2653,7 @@ def replace( # Create a copy to read from and for later use as a backup temp_file = _mkstemp_copy(path=path, preserve_inode=preserve_inode) except OSError as exc: - raise CommandExecutionError("Exception: {}".format(exc)) + raise CommandExecutionError(f"Exception: {exc}") # write new content in the file while avoiding partial reads try: fh_ = salt.utils.atomicfile.atomic_open(path, "wb") @@ -2679,7 +2665,7 @@ def replace( if backup and has_changes and not dry_run: # keep the backup only if it was requested # and only if there were any changes - backup_name = "{}{}".format(path, backup) + backup_name = f"{path}{backup}" try: shutil.move(temp_file, backup_name) except OSError as exc: @@ -2689,8 +2675,8 @@ def replace( "Exception: {}".format(path, temp_file, exc) ) if symlink: - symlink_backup = "{}{}".format(given_path, backup) - target_backup = "{}{}".format(target_path, backup) + symlink_backup = f"{given_path}{backup}" + target_backup = f"{target_path}{backup}" # Always clobber any existing symlink backup # to match the behaviour of the 'backup' option try: @@ -2709,7 +2695,7 @@ def replace( os.remove(temp_file) except OSError as exc: raise CommandExecutionError( - "Unable to delete temp file '{}'. Exception: {}".format(temp_file, exc) + f"Unable to delete temp file '{temp_file}'. Exception: {exc}" ) if not dry_run and not salt.utils.platform.is_windows(): @@ -2848,7 +2834,7 @@ def blockreplace( path = os.path.expanduser(path) if not os.path.exists(path): - raise SaltInvocationError("File not found: {}".format(path)) + raise SaltInvocationError(f"File not found: {path}") try: file_encoding = __utils__["files.get_encoding"](path) @@ -2858,7 +2844,7 @@ def blockreplace( if __utils__["files.is_binary"](path): if not file_encoding: raise SaltInvocationError( - "Cannot perform string replacements on a binary file: {}".format(path) + f"Cannot perform string replacements on a binary file: {path}" ) if insert_before_match or insert_after_match: @@ -2970,7 +2956,7 @@ def blockreplace( new_file.append(line) except OSError as exc: - raise CommandExecutionError("Failed to read from {}: {}".format(path, exc)) + raise CommandExecutionError(f"Failed to read from {path}: {exc}") finally: if linesep is None: # If the file was empty, we will not have set linesep yet. Assume @@ -3035,7 +3021,7 @@ def blockreplace( # backup old content if backup is not False: - backup_path = "{}{}".format(path, backup) + backup_path = f"{path}{backup}" shutil.copy2(path, backup_path) # copy2 does not preserve ownership if salt.utils.platform.is_windows(): @@ -3070,7 +3056,7 @@ def blockreplace( # backup old content if backup is not False: - backup_path = "{}{}".format(path, backup) + backup_path = f"{path}{backup}" shutil.copy2(path, backup_path) # copy2 does not preserve ownership if salt.utils.platform.is_windows(): @@ -3403,11 +3389,9 @@ def append(path, *args, **kwargs): # Append lines in text mode with salt.utils.files.fopen(path, "a") as ofile: for new_line in args: - ofile.write( - salt.utils.stringutils.to_str("{}{}".format(new_line, os.linesep)) - ) + ofile.write(salt.utils.stringutils.to_str(f"{new_line}{os.linesep}")) - return 'Wrote {} lines to "{}"'.format(len(args), path) + return f'Wrote {len(args)} lines to "{path}"' def prepend(path, *args, **kwargs): @@ -3461,12 +3445,12 @@ def prepend(path, *args, **kwargs): preface = [] for line in args: - preface.append("{}\n".format(line)) + preface.append(f"{line}\n") with salt.utils.files.fopen(path, "w") as ofile: contents = preface + contents ofile.write(salt.utils.stringutils.to_str("".join(contents))) - return 'Prepended {} lines to "{}"'.format(len(args), path) + return f'Prepended {len(args)} lines to "{path}"' def write(path, *args, **kwargs): @@ -3511,10 +3495,10 @@ def write(path, *args, **kwargs): contents = [] for line in args: - contents.append("{}\n".format(line)) + contents.append(f"{line}\n") with salt.utils.files.fopen(path, "w") as ofile: ofile.write(salt.utils.stringutils.to_str("".join(contents))) - return 'Wrote {} lines to "{}"'.format(len(contents), path) + return f'Wrote {len(contents)} lines to "{path}"' def touch(name, atime=None, mtime=None): @@ -3675,7 +3659,7 @@ def link(src, path): os.link(src, path) return True except OSError as E: - raise CommandExecutionError("Could not create '{}': {}".format(path, E)) + raise CommandExecutionError(f"Could not create '{path}': {E}") return False @@ -3715,7 +3699,7 @@ def is_link(path): return os.path.islink(os.path.expanduser(path)) -def symlink(src, path, force=False, atomic=False): +def symlink(src, path, force=False, atomic=False, follow_symlinks=True): """ Create a symbolic link (symlink, soft link) to a file @@ -3733,6 +3717,11 @@ def symlink(src, path, force=False, atomic=False): Use atomic file operations to create the symlink .. versionadded:: 3006.0 + follow_symlinks (bool): + If set to ``False``, use ``os.path.lexists()`` for existence checks + instead of ``os.path.exists()``. + .. versionadded:: 3007.0 + Returns: bool: ``True`` if successful, otherwise raises ``CommandExecutionError`` @@ -3744,8 +3733,13 @@ def symlink(src, path, force=False, atomic=False): """ path = os.path.expanduser(path) + if follow_symlinks: + exists = os.path.exists + else: + exists = os.path.lexists + if not os.path.isabs(path): - raise SaltInvocationError("Link path must be absolute: {}".format(path)) + raise SaltInvocationError(f"Link path must be absolute: {path}") if os.path.islink(path): try: @@ -3758,14 +3752,14 @@ def symlink(src, path, force=False, atomic=False): pass if not force and not atomic: - msg = "Found existing symlink: {}".format(path) + msg = f"Found existing symlink: {path}" raise CommandExecutionError(msg) - if os.path.exists(path) and not force and not atomic: - msg = "Existing path is not a symlink: {}".format(path) + if exists(path) and not force and not atomic: + msg = f"Existing path is not a symlink: {path}" raise CommandExecutionError(msg) - if (os.path.islink(path) or os.path.exists(path)) and force and not atomic: + if (os.path.islink(path) or exists(path)) and force and not atomic: os.unlink(path) elif atomic: link_dir = os.path.dirname(path) @@ -3782,13 +3776,13 @@ def symlink(src, path, force=False, atomic=False): return True except OSError: os.remove(temp_link) - raise CommandExecutionError("Could not create '{}'".format(path)) + raise CommandExecutionError(f"Could not create '{path}'") try: os.symlink(src, path) return True except OSError: - raise CommandExecutionError("Could not create '{}'".format(path)) + raise CommandExecutionError(f"Could not create '{path}'") def rename(src, dst): @@ -3811,7 +3805,7 @@ def rename(src, dst): os.rename(src, dst) return True except OSError: - raise CommandExecutionError("Could not rename '{}' to '{}'".format(src, dst)) + raise CommandExecutionError(f"Could not rename '{src}' to '{dst}'") return False @@ -3849,7 +3843,7 @@ def copy(src, dst, recurse=False, remove_existing=False): raise SaltInvocationError("File path must be absolute.") if not os.path.exists(src): - raise CommandExecutionError("No such file or directory '{}'".format(src)) + raise CommandExecutionError(f"No such file or directory '{src}'") if not salt.utils.platform.is_windows(): pre_user = get_user(src) @@ -3872,7 +3866,7 @@ def copy(src, dst, recurse=False, remove_existing=False): else: shutil.copyfile(src, dst) except OSError: - raise CommandExecutionError("Could not copy '{}' to '{}'".format(src, dst)) + raise CommandExecutionError(f"Could not copy '{src}' to '{dst}'") if not salt.utils.platform.is_windows(): check_perms(dst, None, pre_user, pre_group, pre_mode) @@ -4014,10 +4008,10 @@ def readlink(path, canonicalize=False): path = os.path.expandvars(path) if not os.path.isabs(path): - raise SaltInvocationError("Path to link must be absolute: {}".format(path)) + raise SaltInvocationError(f"Path to link must be absolute: {path}") if not os.path.islink(path): - raise SaltInvocationError("A valid link was not specified: {}".format(path)) + raise SaltInvocationError(f"A valid link was not specified: {path}") if canonicalize: return os.path.realpath(path) @@ -4026,7 +4020,7 @@ def readlink(path, canonicalize=False): return salt.utils.path.readlink(path) except OSError as exc: if exc.errno == errno.EINVAL: - raise CommandExecutionError("Not a symbolic link: {}".format(path)) + raise CommandExecutionError(f"Not a symbolic link: {path}") raise CommandExecutionError(exc.__str__()) @@ -4090,7 +4084,7 @@ def statvfs(path): ) } except OSError: - raise CommandExecutionError("Could not statvfs '{}'".format(path)) + raise CommandExecutionError(f"Could not statvfs '{path}'") return False @@ -4118,7 +4112,7 @@ def stats(path, hash_type=None, follow_symlinks=True): # message in this exception. Any changes made to the message for this # exception will reflect the file.directory state as well, and will # likely require changes there. - raise CommandExecutionError("Path not found: {}".format(path)) + raise CommandExecutionError(f"Path not found: {path}") else: if follow_symlinks: pstat = os.stat(path) @@ -4260,7 +4254,7 @@ def remove(path, **kwargs): path = os.path.expanduser(path) if not os.path.isabs(path): - raise SaltInvocationError("File path must be absolute: {}".format(path)) + raise SaltInvocationError(f"File path must be absolute: {path}") try: if os.path.islink(path) or (os.path.exists(path) and not os.path.isdir(path)): @@ -4270,7 +4264,7 @@ def remove(path, **kwargs): shutil.rmtree(path) return True except OSError as exc: - raise CommandExecutionError("Could not remove '{}': {}".format(path, exc)) + raise CommandExecutionError(f"Could not remove '{path}': {exc}") return False @@ -4352,7 +4346,7 @@ def get_selinux_context(path): if cmd_ret["retcode"] == 0: ret = cmd_ret["stdout"] else: - ret = "No selinux context information is available for {}".format(path) + ret = f"No selinux context information is available for {path}" return ret @@ -4388,9 +4382,7 @@ def set_selinux_context( ) if fcontext_result.get("retcode", None) != 0: # Problem setting fcontext policy - raise CommandExecutionError( - "Problem setting fcontext: {}".format(fcontext_result) - ) + raise CommandExecutionError(f"Problem setting fcontext: {fcontext_result}") cmd = ["chcon"] if user: @@ -4420,7 +4412,7 @@ def source_list(source, source_hash, saltenv): salt '*' file.source_list salt://http/httpd.conf '{hash_type: 'md5', 'hsum': }' base """ - contextkey = "{}_|-{}_|-{}".format(source, source_hash, saltenv) + contextkey = f"{source}_|-{source_hash}_|-{saltenv}" if contextkey in __context__: return __context__[contextkey] @@ -4700,12 +4692,12 @@ def get_managed( if parsed_scheme == "": parsed_path = sfn = source if not os.path.exists(sfn): - msg = "Local file source {} does not exist".format(sfn) + msg = f"Local file source {sfn} does not exist" return "", {}, msg elif parsed_scheme == "file": sfn = parsed_path if not os.path.exists(sfn): - msg = "Local file source {} does not exist".format(sfn) + msg = f"Local file source {sfn} does not exist" return "", {}, msg if parsed_scheme and parsed_scheme.lower() in string.ascii_lowercase: @@ -4718,7 +4710,7 @@ def get_managed( return ( "", {}, - "Source file {} not found in saltenv '{}'".format(source, saltenv), + f"Source file {source} not found in saltenv '{saltenv}'", ) elif not source_hash and unix_local_source: source_sum = _get_local_file_source_sum(parsed_path) @@ -4782,13 +4774,13 @@ def get_managed( # A 404 or other error code may raise an exception, catch it # and return a comment that will fail the calling state. _source = salt.utils.url.redact_http_basic_auth(source) - return "", {}, "Failed to cache {}: {}".format(_source, exc) + return "", {}, f"Failed to cache {_source}: {exc}" # If cache failed, sfn will be False, so do a truth check on sfn first # as invoking os.path.exists() on a bool raises a TypeError. if not sfn or not os.path.exists(sfn): _source = salt.utils.url.redact_http_basic_auth(source) - return sfn, {}, "Source file '{}' not found".format(_source) + return sfn, {}, f"Source file '{_source}' not found" if sfn == name: raise SaltInvocationError("Source file cannot be the same as destination") @@ -4817,7 +4809,7 @@ def get_managed( return ( sfn, {}, - "Specified template format {} is not supported".format(template), + f"Specified template format {template} is not supported", ) if data["result"]: @@ -4890,7 +4882,7 @@ def extract_hash( hash_type, ) hash_type = "" - hash_len_expr = "{},{}".format(min(HASHES_REVMAP), max(HASHES_REVMAP)) + hash_len_expr = f"{min(HASHES_REVMAP)},{max(HASHES_REVMAP)}" else: hash_len_expr = str(hash_len) @@ -5195,7 +5187,7 @@ def check_perms( ret["changes"]["user"] = user else: ret["result"] = False - ret["comment"].append("Failed to change user to {}".format(user)) + ret["comment"].append(f"Failed to change user to {user}") elif "cuser" in perms: ret["changes"]["user"] = user @@ -5211,7 +5203,7 @@ def check_perms( ret["changes"]["group"] = group else: ret["result"] = False - ret["comment"].append("Failed to change group to {}".format(group)) + ret["comment"].append(f"Failed to change group to {group}") elif "cgroup" in perms: ret["changes"]["group"] = group @@ -5224,7 +5216,7 @@ def check_perms( ret["changes"]["mode"] = mode else: ret["result"] = False - ret["comment"].append("Failed to change mode to {}".format(mode)) + ret["comment"].append(f"Failed to change mode to {mode}") elif "cmode" in perms: ret["changes"]["mode"] = mode @@ -5257,9 +5249,7 @@ def check_perms( cmp_attrs = _cmp_attrs(name, attrs) if any(attr for attr in cmp_attrs): ret["result"] = False - ret["comment"].append( - "Failed to change attributes to {}".format(attrs) - ) + ret["comment"].append(f"Failed to change attributes to {attrs}") changes["new"] = "".join(lsattr(name)[name]) else: changes["new"] = attrs @@ -5314,17 +5304,17 @@ def check_perms( selinux_change_new = "" selinux_change_orig = "" if requested_seuser: - selinux_change_new += "User: {} ".format(requested_seuser) - selinux_change_orig += "User: {} ".format(current_seuser) + selinux_change_new += f"User: {requested_seuser} " + selinux_change_orig += f"User: {current_seuser} " if requested_serole: - selinux_change_new += "Role: {} ".format(requested_serole) - selinux_change_orig += "Role: {} ".format(current_serole) + selinux_change_new += f"Role: {requested_serole} " + selinux_change_orig += f"Role: {current_serole} " if requested_setype: - selinux_change_new += "Type: {} ".format(requested_setype) - selinux_change_orig += "Type: {} ".format(current_setype) + selinux_change_new += f"Type: {requested_setype} " + selinux_change_orig += f"Type: {current_setype} " if requested_serange: - selinux_change_new += "Range: {} ".format(requested_serange) - selinux_change_orig += "Range: {} ".format(current_serange) + selinux_change_new += f"Range: {requested_serange} " + selinux_change_orig += f"Range: {current_serange} " if __opts__["test"]: ret["comment"] = "File {} selinux context to be updated".format( @@ -5364,9 +5354,7 @@ def check_perms( selinux_error = True if not selinux_error: - ret["comment"].append( - "The file {} is set to be changed".format(name) - ) + ret["comment"].append(f"The file {name} is set to be changed") if requested_seuser: if current_seuser != requested_seuser: @@ -5498,9 +5486,9 @@ def check_managed( if changes: log.info(changes) comments = ["The following values are set to be changed:\n"] - comments.extend("{}: {}\n".format(key, val) for key, val in changes.items()) + comments.extend(f"{key}: {val}\n" for key, val in changes.items()) return None, "".join(comments) - return True, "The file {} is in the correct state".format(name) + return True, f"The file {name} is in the correct state" def check_managed_changes( @@ -6148,7 +6136,7 @@ def manage_file( # File is not present, cache it sfn = __salt__["cp.cache_file"](source, saltenv, verify_ssl=verify_ssl) if not sfn: - return _error(ret, "Source file '{}' not found".format(source)) + return _error(ret, f"Source file '{source}' not found") htype = source_sum.get("hash_type", __opts__["hash_type"]) # Recalculate source sum now that file has been cached source_sum = {"hash_type": htype, "hsum": get_hash(sfn, form=htype)} @@ -6185,7 +6173,7 @@ def manage_file( source, saltenv, verify_ssl=verify_ssl, use_etag=use_etag ) if not sfn: - return _error(ret, "Source file '{}' not found".format(source)) + return _error(ret, f"Source file '{source}' not found") # If the downloaded file came from a non salt server or local # source, and we are not skipping checksum verification, then # verify that it matches the specified checksum. @@ -6227,7 +6215,7 @@ def manage_file( ) except OSError as io_error: __clean_tmp(sfn) - return _error(ret, "Failed to commit change: {}".format(io_error)) + return _error(ret, f"Failed to commit change: {io_error}") if contents is not None: # Write the static contents to a temporary file @@ -6258,7 +6246,7 @@ def manage_file( except CommandExecutionError as exc: ret.setdefault("warnings", []).append( - "Failed to detect changes to file: {}".format(exc.strerror) + f"Failed to detect changes to file: {exc.strerror}" ) differences = "" @@ -6275,7 +6263,7 @@ def manage_file( ) except OSError as io_error: __clean_tmp(tmp) - return _error(ret, "Failed to commit change: {}".format(io_error)) + return _error(ret, f"Failed to commit change: {io_error}") __clean_tmp(tmp) # Check for changing symlink to regular file here @@ -6283,7 +6271,7 @@ def manage_file( if not sfn: sfn = __salt__["cp.cache_file"](source, saltenv, verify_ssl=verify_ssl) if not sfn: - return _error(ret, "Source file '{}' not found".format(source)) + return _error(ret, f"Source file '{source}' not found") # If the downloaded file came from a non salt server source verify # that it matches the intended sum value if check_web_source_hash: @@ -6307,7 +6295,7 @@ def manage_file( ) except OSError as io_error: __clean_tmp(sfn) - return _error(ret, "Failed to commit change: {}".format(io_error)) + return _error(ret, f"Failed to commit change: {io_error}") ret["changes"]["diff"] = "Replace symbolic link with regular file" @@ -6341,7 +6329,7 @@ def manage_file( ) if ret["changes"]: - ret["comment"] = "File {} updated".format(salt.utils.data.decode(name)) + ret["comment"] = f"File {salt.utils.data.decode(name)} updated" elif not ret["changes"] and ret["result"]: ret["comment"] = "File {} is in the correct state".format( @@ -6359,7 +6347,7 @@ def manage_file( drive, _ = os.path.splitdrive(name) if drive and not os.path.exists(drive): __clean_tmp(sfn) - return _error(ret, "{} drive not present".format(drive)) + return _error(ret, f"{drive} drive not present") if dir_mode is None and mode is not None: # Add execute bit to each nonzero digit in the mode, if # dir_mode was not specified. Otherwise, any @@ -6392,7 +6380,7 @@ def manage_file( if not sfn: sfn = __salt__["cp.cache_file"](source, saltenv, verify_ssl=verify_ssl) if not sfn: - return _error(ret, "Source file '{}' not found".format(source)) + return _error(ret, f"Source file '{source}' not found") # If the downloaded file came from a non salt server source verify # that it matches the intended sum value if check_web_source_hash: @@ -6433,16 +6421,16 @@ def manage_file( if contents is None: if not __opts__["test"]: if touch(name): - ret["changes"]["new"] = "file {} created".format(name) + ret["changes"]["new"] = f"file {name} created" ret["comment"] = "Empty file" else: - return _error(ret, "Empty file {} not created".format(name)) + return _error(ret, f"Empty file {name} not created") else: if not __opts__["test"]: if touch(name): ret["changes"]["diff"] = "New file" else: - return _error(ret, "File {} not created".format(name)) + return _error(ret, f"File {name} not created") if contents is not None: # Write the static contents to a temporary file @@ -6575,12 +6563,12 @@ def makedirs_(path, user=None, group=None, mode=None): if os.path.isdir(dirname): # There's nothing for us to do - msg = "Directory '{}' already exists".format(dirname) + msg = f"Directory '{dirname}' already exists" log.debug(msg) return msg if os.path.exists(dirname): - msg = "The path '{}' already exists and is not a directory".format(dirname) + msg = f"The path '{dirname}' already exists and is not a directory" log.debug(msg) return msg @@ -6634,7 +6622,7 @@ def makedirs_perms(name, user=None, group=None, mode="0755"): if tail == os.curdir: # xxx/newdir/. exists if xxx/newdir exists return os.mkdir(name) - check_perms(name, None, user, group, int("{}".format(mode)) if mode else None) + check_perms(name, None, user, group, int(f"{mode}") if mode else None) def get_devmm(name): @@ -6704,7 +6692,7 @@ def mknod_chrdev(name, major, minor, user=None, group=None, mode="0660"): ) try: if __opts__["test"]: - ret["changes"] = {"new": "Character device {} created.".format(name)} + ret["changes"] = {"new": f"Character device {name} created."} ret["result"] = None else: if ( @@ -6715,7 +6703,7 @@ def mknod_chrdev(name, major, minor, user=None, group=None, mode="0660"): ) is None ): - ret["changes"] = {"new": "Character device {} created.".format(name)} + ret["changes"] = {"new": f"Character device {name} created."} ret["result"] = True except OSError as exc: # be happy it is already there....however, if you are trying to change the @@ -6723,9 +6711,9 @@ def mknod_chrdev(name, major, minor, user=None, group=None, mode="0660"): if exc.errno != errno.EEXIST: raise else: - ret["comment"] = "File {} exists and cannot be overwritten".format(name) + ret["comment"] = f"File {name} exists and cannot be overwritten" # quick pass at verifying the permissions of the newly created character device - check_perms(name, None, user, group, int("{}".format(mode)) if mode else None) + check_perms(name, None, user, group, int(f"{mode}") if mode else None) return ret @@ -6777,7 +6765,7 @@ def mknod_blkdev(name, major, minor, user=None, group=None, mode="0660"): ) try: if __opts__["test"]: - ret["changes"] = {"new": "Block device {} created.".format(name)} + ret["changes"] = {"new": f"Block device {name} created."} ret["result"] = None else: if ( @@ -6788,7 +6776,7 @@ def mknod_blkdev(name, major, minor, user=None, group=None, mode="0660"): ) is None ): - ret["changes"] = {"new": "Block device {} created.".format(name)} + ret["changes"] = {"new": f"Block device {name} created."} ret["result"] = True except OSError as exc: # be happy it is already there....however, if you are trying to change the @@ -6796,9 +6784,9 @@ def mknod_blkdev(name, major, minor, user=None, group=None, mode="0660"): if exc.errno != errno.EEXIST: raise else: - ret["comment"] = "File {} exists and cannot be overwritten".format(name) + ret["comment"] = f"File {name} exists and cannot be overwritten" # quick pass at verifying the permissions of the newly created block device - check_perms(name, None, user, group, int("{}".format(mode)) if mode else None) + check_perms(name, None, user, group, int(f"{mode}") if mode else None) return ret @@ -6844,20 +6832,20 @@ def mknod_fifo(name, user=None, group=None, mode="0660"): log.debug("Creating FIFO name: %s", name) try: if __opts__["test"]: - ret["changes"] = {"new": "Fifo pipe {} created.".format(name)} + ret["changes"] = {"new": f"Fifo pipe {name} created."} ret["result"] = None else: if os.mkfifo(name, int(str(mode).lstrip("0Oo"), 8)) is None: - ret["changes"] = {"new": "Fifo pipe {} created.".format(name)} + ret["changes"] = {"new": f"Fifo pipe {name} created."} ret["result"] = True except OSError as exc: # be happy it is already there if exc.errno != errno.EEXIST: raise else: - ret["comment"] = "File {} exists and cannot be overwritten".format(name) + ret["comment"] = f"File {name} exists and cannot be overwritten" # quick pass at verifying the permissions of the newly created fifo - check_perms(name, None, user, group, int("{}".format(mode)) if mode else None) + check_perms(name, None, user, group, int(f"{mode}") if mode else None) return ret @@ -6939,9 +6927,9 @@ def list_backups(path, limit=None): ]: if salt.utils.platform.is_windows(): # ':' is an illegal filesystem path character on Windows - strpfmt = "{}_%a_%b_%d_%H-%M-%S_%f_%Y".format(basename) + strpfmt = f"{basename}_%a_%b_%d_%H-%M-%S_%f_%Y" else: - strpfmt = "{}_%a_%b_%d_%H:%M:%S_%f_%Y".format(basename) + strpfmt = f"{basename}_%a_%b_%d_%H:%M:%S_%f_%Y" try: timestamp = datetime.datetime.strptime(fname, strpfmt) except ValueError: @@ -7017,7 +7005,7 @@ def list_backups_dir(path, limit=None): for x in sorted(ff): basename = x.split("_")[0] if i == basename: - strpfmt = "{}_%a_%b_%d_%H:%M:%S_%f_%Y".format(basename) + strpfmt = f"{basename}_%a_%b_%d_%H:%M:%S_%f_%Y" try: timestamp = datetime.datetime.strptime(x, strpfmt) except ValueError: @@ -7067,7 +7055,7 @@ def restore_backup(path, backup_id): # Note: This only supports minion backups, so this function will need to be # modified if/when master backups are implemented. - ret = {"result": False, "comment": "Invalid backup_id '{}'".format(backup_id)} + ret = {"result": False, "comment": f"Invalid backup_id '{backup_id}'"} try: if len(str(backup_id)) == len(str(int(backup_id))): backup = list_backups(path)[int(backup_id)] @@ -7076,7 +7064,7 @@ def restore_backup(path, backup_id): except ValueError: return ret except KeyError: - ret["comment"] = "backup_id '{}' does not exist for {}".format(backup_id, path) + ret["comment"] = f"backup_id '{backup_id}' does not exist for {path}" return ret salt.utils.files.backup_minion(path, _get_bkroot()) @@ -7126,7 +7114,7 @@ def delete_backup(path, backup_id): """ path = os.path.expanduser(path) - ret = {"result": False, "comment": "Invalid backup_id '{}'".format(backup_id)} + ret = {"result": False, "comment": f"Invalid backup_id '{backup_id}'"} try: if len(str(backup_id)) == len(str(int(backup_id))): backup = list_backups(path)[int(backup_id)] @@ -7135,7 +7123,7 @@ def delete_backup(path, backup_id): except ValueError: return ret except KeyError: - ret["comment"] = "backup_id '{}' does not exist for {}".format(backup_id, path) + ret["comment"] = f"backup_id '{backup_id}' does not exist for {path}" return ret try: @@ -7253,9 +7241,9 @@ def open_files(by_pid=False): # Then we look at the open files for each PID files = {} for pid in pids: - ppath = "/proc/{}".format(pid) + ppath = f"/proc/{pid}" try: - tids = os.listdir("{}/task".format(ppath)) + tids = os.listdir(f"{ppath}/task") except OSError: continue @@ -7267,17 +7255,17 @@ def open_files(by_pid=False): # except Exception: # pylint: disable=broad-except # pass - for fpath in os.listdir("{}/fd".format(ppath)): - fd_.append("{}/fd/{}".format(ppath, fpath)) + for fpath in os.listdir(f"{ppath}/fd"): + fd_.append(f"{ppath}/fd/{fpath}") for tid in tids: try: - fd_.append(os.path.realpath("{}/task/{}/exe".format(ppath, tid))) + fd_.append(os.path.realpath(f"{ppath}/task/{tid}/exe")) except OSError: continue - for tpath in os.listdir("{}/task/{}/fd".format(ppath, tid)): - fd_.append("{}/task/{}/fd/{}".format(ppath, tid, tpath)) + for tpath in os.listdir(f"{ppath}/task/{tid}/fd"): + fd_.append(f"{ppath}/task/{tid}/fd/{tpath}") fd_ = sorted(set(fd_)) @@ -7454,15 +7442,13 @@ def move(src, dst, disallow_copy_and_unlink=False): ret = { "result": True, - "comment": "'{}' moved to '{}'".format(src, dst), + "comment": f"'{src}' moved to '{dst}'", } try: shutil.move(src, dst) except OSError as exc: - raise CommandExecutionError( - "Unable to move '{}' to '{}': {}".format(src, dst, exc) - ) + raise CommandExecutionError(f"Unable to move '{src}' to '{dst}': {exc}") return ret diff --git a/salt/states/file.py b/salt/states/file.py index fdde331c3c9..bc35c42c4ce 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -1542,6 +1542,7 @@ def symlink( atomic=False, disallow_copy_and_unlink=False, inherit_user_and_group=False, + follow_symlinks=False, **kwargs, ): """ @@ -1647,6 +1648,13 @@ def symlink( override this behavior. .. versionadded:: 3006.0 + + follow_symlinks (bool): + If set to ``False``, the underlying ``file.symlink`` execution module + and any checks in this state will use ``os.path.lexists()`` for + existence checks instead of ``os.path.exists()``. + + .. versionadded:: 3007.0 """ name = os.path.expanduser(name) @@ -1654,6 +1662,11 @@ def symlink( if not name: return _error(ret, "Must provide name to file.symlink") + if follow_symlinks: + exists = os.path.exists + else: + exists = os.path.lexists + # Make sure that leading zeros stripped by YAML loader are added back mode = salt.utils.files.normalize_mode(mode) @@ -1834,7 +1847,7 @@ def symlink( ) return ret - elif os.path.exists(name): + elif exists(name): # It is not a link, but a file, dir, socket, FIFO etc. if backupname is not None: if not os.path.isabs(backupname): @@ -1892,7 +1905,9 @@ def symlink( ) try: - __salt__["file.symlink"](target, name, force=force, atomic=atomic) + __salt__["file.symlink"]( + target, name, force=force, atomic=atomic, follow_symlinks=follow_symlinks + ) except (CommandExecutionError, OSError) as exc: ret["result"] = False ret["comment"] = "Unable to create new symlink {} -> {}: {}".format( diff --git a/tests/pytests/unit/modules/file/test_file_basics.py b/tests/pytests/unit/modules/file/test_file_basics.py index cee60da2fab..5da99d6e47a 100644 --- a/tests/pytests/unit/modules/file/test_file_basics.py +++ b/tests/pytests/unit/modules/file/test_file_basics.py @@ -272,3 +272,25 @@ def test_source_list_for_list_returns_local_file_proto_from_dict(myfile): ): ret = filemod.source_list([{"file://" + myfile: ""}], "filehash", "base") assert list(ret) == ["file://" + myfile, "filehash"] + + +def test_symlink_lexists_called_follow_symlinks_false(): + tfile = "/tmp/file-basics-test-file" + a_link = "/tmp/a_link" + + exists = MagicMock(return_value=False) + lexists = MagicMock(return_value=False) + + with patch("os.path.exists", exists), patch("os.path.lexists", lexists), patch( + "os.symlink", MagicMock(return_value=True) + ): + filemod.symlink(tfile, a_link) + lexists.assert_not_called() + exists.assert_called() + + lexists.reset_mock() + exists.reset_mock() + + filemod.symlink(tfile, a_link, follow_symlinks=False) + lexists.assert_called() + exists.assert_not_called()