diff --git a/changelog/64260.fixed.md b/changelog/64260.fixed.md new file mode 100644 index 000000000000..6de5c7a72d03 --- /dev/null +++ b/changelog/64260.fixed.md @@ -0,0 +1 @@ +Skipped the `isfile` check to greatly increase speed of reading minion keys for systems with a large number of minions on slow file storage diff --git a/salt/key.py b/salt/key.py index b15b80eca3f2..3d464b46a2b5 100644 --- a/salt/key.py +++ b/salt/key.py @@ -177,7 +177,7 @@ def _run_cmd(self, cmd, args=None): if cmd in ("accept", "reject", "delete") and args is None: args = self.opts.get("match_dict", {}).get("minions") - fstr = "key.{}".format(cmd) + fstr = f"key.{cmd}" fun = self.client.functions[fstr] args, kwargs = self._get_args_kwargs(fun, args) @@ -230,7 +230,7 @@ def _print_no_match(self, cmd, match): stat_str = statuses[0] else: stat_str = "{} or {}".format(", ".join(statuses[:-1]), statuses[-1]) - msg = "The key glob '{}' does not match any {} keys.".format(match, stat_str) + msg = f"The key glob '{match}' does not match any {stat_str} keys." print(msg) def run(self): @@ -291,7 +291,7 @@ def run(self): else: salt.output.display_output({"return": ret}, "key", opts=self.opts) except salt.exceptions.SaltException as exc: - ret = "{}".format(exc) + ret = f"{exc}" if not self.opts.get("quiet", False): salt.output.display_output(ret, "nested", self.opts) return ret @@ -311,7 +311,7 @@ def __init__(self, opts, io_loop=None): self.opts = opts kind = self.opts.get("__role", "") # application kind if kind not in salt.utils.kinds.APPL_KINDS: - emsg = "Invalid application kind = '{}'.".format(kind) + emsg = f"Invalid application kind = '{kind}'." log.error(emsg) raise ValueError(emsg) self.event = salt.utils.event.get_event( @@ -377,7 +377,7 @@ def gen_keys_signature( # check given pub-key if pub: if not os.path.isfile(pub): - return "Public-key {} does not exist".format(pub) + return f"Public-key {pub} does not exist" # default to master.pub else: mpub = self.opts["pki_dir"] + "/" + "master.pub" @@ -387,7 +387,7 @@ def gen_keys_signature( # check given priv-key if priv: if not os.path.isfile(priv): - return "Private-key {} does not exist".format(priv) + return f"Private-key {priv} does not exist" # default to master_sign.pem else: mpriv = self.opts["pki_dir"] + "/" + "master_sign.pem" @@ -467,7 +467,7 @@ def check_minion_cache(self, preserve_minions=None): if clist: for minion in clist: if minion not in minions and minion not in preserve_minions: - cache.flush("{}/{}".format(self.ACC, minion)) + cache.flush(f"{self.ACC}/{minion}") def check_master(self): """ @@ -528,8 +528,7 @@ def local_keys(self): for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(self.opts["pki_dir"])): if fn_.endswith(".pub") or fn_.endswith(".pem"): path = os.path.join(self.opts["pki_dir"], fn_) - if os.path.isfile(path): - ret["local"].append(fn_) + ret["local"].append(fn_) return ret def list_keys(self): @@ -547,10 +546,9 @@ def list_keys(self): try: for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(dir_)): if not fn_.startswith("."): - if os.path.isfile(os.path.join(dir_, fn_)): - ret[os.path.basename(dir_)].append( - salt.utils.stringutils.to_unicode(fn_) - ) + ret[os.path.basename(dir_)].append( + salt.utils.stringutils.to_unicode(fn_) + ) except OSError: # key dir kind is not created yet, just skip continue @@ -574,26 +572,22 @@ def list_status(self, match): ret[os.path.basename(acc)] = [] for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(acc)): if not fn_.startswith("."): - if os.path.isfile(os.path.join(acc, fn_)): - ret[os.path.basename(acc)].append(fn_) + ret[os.path.basename(acc)].append(fn_) elif match.startswith("pre") or match.startswith("un"): ret[os.path.basename(pre)] = [] for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(pre)): if not fn_.startswith("."): - if os.path.isfile(os.path.join(pre, fn_)): - ret[os.path.basename(pre)].append(fn_) + ret[os.path.basename(pre)].append(fn_) elif match.startswith("rej"): ret[os.path.basename(rej)] = [] for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(rej)): if not fn_.startswith("."): - if os.path.isfile(os.path.join(rej, fn_)): - ret[os.path.basename(rej)].append(fn_) + ret[os.path.basename(rej)].append(fn_) elif match.startswith("den") and den is not None: ret[os.path.basename(den)] = [] for fn_ in salt.utils.data.sorted_ignorecase(os.listdir(den)): if not fn_.startswith("."): - if os.path.isfile(os.path.join(den, fn_)): - ret[os.path.basename(den)].append(fn_) + ret[os.path.basename(den)].append(fn_) elif match.startswith("all"): return self.all_keys() return ret @@ -663,7 +657,7 @@ def accept( pass for keydir, key in invalid_keys: matches[keydir].remove(key) - sys.stderr.write("Unable to accept invalid key for {}.\n".format(key)) + sys.stderr.write(f"Unable to accept invalid key for {key}.\n") return self.name_match(match) if match is not None else self.dict_match(matches) def accept_all(self): diff --git a/salt/master.py b/salt/master.py index 97ea8dcb5af9..d2d8110b02a5 100644 --- a/salt/master.py +++ b/salt/master.py @@ -158,7 +158,7 @@ def rotate_secrets(cls, opts=None, event=None, use_lock=True): if "serial" in secret_map: secret_map["serial"].value = 0 if event: - event.fire_event({"rotate_{}_key".format(secret_key): True}, tag="key") + event.fire_event({f"rotate_{secret_key}_key": True}, tag="key") if opts.get("ping_on_rotate"): # Ping all minions to get them to pick up the new key @@ -290,9 +290,7 @@ def handle_key_cache(self): acc = "accepted" for fn_ in os.listdir(os.path.join(self.opts["pki_dir"], acc)): - if not fn_.startswith(".") and os.path.isfile( - os.path.join(self.opts["pki_dir"], acc, fn_) - ): + if not fn_.startswith("."): keys.append(fn_) log.debug("Writing master key cache") # Write a temporary file securely @@ -399,7 +397,7 @@ def fill_buckets(self): update_intervals = self.fileserver.update_intervals() self.buckets = {} for backend in self.fileserver.backends(): - fstr = "{}.update".format(backend) + fstr = f"{backend}.update" try: update_func = self.fileserver.servers[fstr] except KeyError: @@ -429,7 +427,7 @@ def fill_buckets(self): # nothing to pass to the backend's update func, so we'll just # set the value to None. try: - interval_key = "{}_update_interval".format(backend) + interval_key = f"{backend}_update_interval" interval = self.opts[interval_key] except KeyError: interval = DEFAULT_INTERVAL @@ -606,7 +604,7 @@ def _pre_flight(self): try: os.chdir("/") except OSError as err: - errors.append("Cannot change to root directory ({})".format(err)) + errors.append(f"Cannot change to root directory ({err})") if self.opts.get("fileserver_verify_config", True): # Avoid circular import @@ -624,7 +622,7 @@ def _pre_flight(self): try: fileserver.init() except salt.exceptions.FileserverConfigError as exc: - critical_errors.append("{}".format(exc)) + critical_errors.append(f"{exc}") if not self.opts["fileserver_backend"]: errors.append("No fileserver backends are configured") @@ -769,7 +767,7 @@ def start(self): cls = proc.split(".")[-1] _tmp = __import__(mod, globals(), locals(), [cls], -1) cls = _tmp.__getattribute__(cls) - name = "ExtProcess({})".format(cls.__qualname__) + name = f"ExtProcess({cls.__qualname__})" self.process_manager.add_process(cls, args=(self.opts,), name=name) except Exception: # pylint: disable=broad-except log.error("Error creating ext_processes process: %s", proc) @@ -909,7 +907,7 @@ def __bind(self): # signal handlers with salt.utils.process.default_signals(signal.SIGINT, signal.SIGTERM): for ind in range(int(self.opts["worker_threads"])): - name = "MWorker-{}".format(ind) + name = f"MWorker-{ind}" self.process_manager.add_process( MWorker, args=(self.opts, self.master_key, self.key, req_channels), @@ -2107,7 +2105,7 @@ def wheel(self, clear_load): fun = clear_load.pop("fun") tag = tagify(jid, prefix="wheel") data = { - "fun": "wheel.{}".format(fun), + "fun": f"wheel.{fun}", "jid": jid, "tag": tag, "user": username, @@ -2210,7 +2208,7 @@ def publish(self, clear_load): else: auth_list = auth_check.get("auth_list", []) - err_msg = 'Authentication failure of type "{}" occurred.'.format(auth_type) + err_msg = f'Authentication failure of type "{auth_type}" occurred.' if auth_check.get("error"): # Authentication error occurred: do not continue. diff --git a/salt/utils/minions.py b/salt/utils/minions.py index 21d34b7eddd8..71b3d2f0fc0b 100644 --- a/salt/utils/minions.py +++ b/salt/utils/minions.py @@ -109,11 +109,11 @@ def get_minion_data(minion, opts): cache = salt.cache.factory(opts) if minion is None: for id_ in cache.list("minions"): - data = cache.fetch("minions/{}".format(id_), "data") + data = cache.fetch(f"minions/{id_}", "data") if data is None: continue else: - data = cache.fetch("minions/{}".format(minion), "data") + data = cache.fetch(f"minions/{minion}", "data") if data is not None: grains = data.get("grains", None) pillar = data.get("pillar", None) @@ -257,7 +257,7 @@ def _check_pcre_minions(self, expr, greedy): # pylint: disable=unused-argument def _pki_minions(self): """ - Retreive complete minion list from PKI dir. + Retrieve complete minion list from PKI dir. Respects cache if configured """ minions = [] @@ -275,9 +275,7 @@ def _pki_minions(self): for fn_ in salt.utils.data.sorted_ignorecase( os.listdir(os.path.join(self.opts["pki_dir"], self.acc)) ): - if not fn_.startswith(".") and os.path.isfile( - os.path.join(self.opts["pki_dir"], self.acc, fn_) - ): + if not fn_.startswith("."): minions.append(fn_) return minions except OSError as exc: @@ -305,9 +303,7 @@ def list_cached_minions(): for fn_ in salt.utils.data.sorted_ignorecase( os.listdir(os.path.join(self.opts["pki_dir"], self.acc)) ): - if not fn_.startswith(".") and os.path.isfile( - os.path.join(self.opts["pki_dir"], self.acc, fn_) - ): + if not fn_.startswith("."): minions.append(fn_) elif cache_enabled: minions = list_cached_minions() @@ -325,7 +321,7 @@ def list_cached_minions(): for id_ in cminions: if greedy and id_ not in minions: continue - mdata = self.cache.fetch("minions/{}".format(id_), "data") + mdata = self.cache.fetch(f"minions/{id_}", "data") if mdata is None: if not greedy: minions.remove(id_) @@ -410,11 +406,11 @@ def _check_ipcidr_minions(self, expr, greedy): except Exception: # pylint: disable=broad-except log.error("Invalid IP/CIDR target: %s", tgt) return {"minions": [], "missing": []} - proto = "ipv{}".format(tgt.version) + proto = f"ipv{tgt.version}" minions = set(minions) for id_ in cminions: - mdata = self.cache.fetch("minions/{}".format(id_), "data") + mdata = self.cache.fetch(f"minions/{id_}", "data") if mdata is None: if not greedy: minions.remove(id_) @@ -453,9 +449,7 @@ def _check_range_minions(self, expr, greedy): for fn_ in salt.utils.data.sorted_ignorecase( os.listdir(os.path.join(self.opts["pki_dir"], self.acc)) ): - if not fn_.startswith(".") and os.path.isfile( - os.path.join(self.opts["pki_dir"], self.acc, fn_) - ): + if not fn_.startswith("."): mlist.append(fn_) return {"minions": mlist, "missing": []} elif cache_enabled: @@ -651,7 +645,7 @@ def connected_ids(self, subset=None, show_ip=False): search = subset for id_ in search: try: - mdata = self.cache.fetch("minions/{}".format(id_), "data") + mdata = self.cache.fetch(f"minions/{id_}", "data") except SaltCacheError: # If a SaltCacheError is explicitly raised during the fetch operation, # permission was denied to open the cached data.p file. Continue on as @@ -685,9 +679,7 @@ def _all_minions(self, expr=None): for fn_ in salt.utils.data.sorted_ignorecase( os.listdir(os.path.join(self.opts["pki_dir"], self.acc)) ): - if not fn_.startswith(".") and os.path.isfile( - os.path.join(self.opts["pki_dir"], self.acc, fn_) - ): + if not fn_.startswith("."): mlist.append(fn_) return {"minions": mlist, "missing": []} @@ -704,7 +696,7 @@ def check_minions( try: if expr is None: expr = "" - check_func = getattr(self, "_check_{}_minions".format(tgt_type), None) + check_func = getattr(self, f"_check_{tgt_type}_minions", None) if tgt_type in ( "grain", "grain_pcre", @@ -1050,11 +1042,7 @@ def spec_check(self, auth_list, fun, args, form): for ind in auth_list: if isinstance(ind, str): if ind[0] == "@": - if ( - ind[1:] == mod_name - or ind[1:] == form - or ind == "@{}s".format(form) - ): + if ind[1:] == mod_name or ind[1:] == form or ind == f"@{form}s": return True elif isinstance(ind, dict): if len(ind) != 1: @@ -1066,7 +1054,7 @@ def spec_check(self, auth_list, fun, args, form): ind[valid], fun_name, args.get("arg"), args.get("kwarg") ): return True - if valid[1:] == form or valid == "@{}s".format(form): + if valid[1:] == form or valid == f"@{form}s": if self.__fun_check( ind[valid], fun, args.get("arg"), args.get("kwarg") ): diff --git a/tests/pytests/integration/cli/test_matcher.py b/tests/pytests/integration/cli/test_matcher.py index 6b3b21bbaa64..c999c465b2ef 100644 --- a/tests/pytests/integration/cli/test_matcher.py +++ b/tests/pytests/integration/cli/test_matcher.py @@ -85,7 +85,7 @@ def test_list(salt_cli, salt_minion, salt_sub_minion): assert salt_minion.id in ret.stdout assert salt_sub_minion.id not in ret.stdout ret = salt_cli.run( - "-L", "test.ping", minion_tgt="{},{}".format(salt_minion.id, salt_sub_minion.id) + "-L", "test.ping", minion_tgt=f"{salt_minion.id},{salt_sub_minion.id}" ) assert ret.returncode == 0 assert salt_minion.id in ret.data @@ -124,16 +124,14 @@ def test_compound_pcre_grain_and_grain(salt_cli, salt_minion, salt_sub_minion): def test_compound_list_and_pcre_minion(salt_cli, salt_minion, salt_sub_minion): - match = "L@{} and E@.*".format(salt_sub_minion.id) + match = f"L@{salt_sub_minion.id} and E@.*" ret = salt_cli.run("-C", "test.ping", minion_tgt=match) assert salt_sub_minion.id in ret.data assert salt_minion.id not in ret.data def test_compound_not_sub_minion(salt_cli, salt_minion, salt_sub_minion): - ret = salt_cli.run( - "-C", "test.ping", minion_tgt="not {}".format(salt_sub_minion.id) - ) + ret = salt_cli.run("-C", "test.ping", minion_tgt=f"not {salt_sub_minion.id}") assert ret.returncode == 0 assert salt_minion.id in ret.data assert salt_sub_minion.id not in ret.data @@ -183,7 +181,7 @@ def test_compound_nodegroup(salt_cli, salt_minion, salt_sub_minion): assert ret.returncode == 0 assert salt_minion.id in ret.data assert salt_sub_minion.id in ret.data - target = "N@multiline_nodegroup not {}".format(salt_sub_minion.id) + target = f"N@multiline_nodegroup not {salt_sub_minion.id}" ret = salt_cli.run("-C", "test.ping", minion_tgt=target) assert ret.returncode == 0 assert salt_minion.id in ret.data @@ -269,7 +267,7 @@ def test_regex(salt_cli, salt_minion, salt_sub_minion): """ test salt regex matcher """ - ret = salt_cli.run("-E", "test.ping", minion_tgt="^{}$".format(salt_minion.id)) + ret = salt_cli.run("-E", "test.ping", minion_tgt=f"^{salt_minion.id}$") assert ret.returncode == 0 assert salt_minion.id in ret.data assert salt_sub_minion.id not in ret.data @@ -362,12 +360,12 @@ def test_grains_targeting_minion_id_running(salt_cli, salt_minion, salt_sub_mini """ Tests return of each running test minion targeting with minion id grain """ - ret = salt_cli.run("-G", "test.ping", minion_tgt="id:{}".format(salt_minion.id)) + ret = salt_cli.run("-G", "test.ping", minion_tgt=f"id:{salt_minion.id}") assert ret.returncode == 0 assert salt_minion.id in ret.data assert ret.data[salt_minion.id] is True - ret = salt_cli.run("-G", "test.ping", minion_tgt="id:{}".format(salt_sub_minion.id)) + ret = salt_cli.run("-G", "test.ping", minion_tgt=f"id:{salt_sub_minion.id}") assert ret.returncode == 0 assert salt_sub_minion.id in ret.data assert ret.data[salt_sub_minion.id] is True @@ -400,8 +398,8 @@ def test_grains_targeting_minion_id_disconnected(salt_master, salt_minion, salt_ "--log-level=debug", "-G", "test.ping", - minion_tgt="id:{}".format(disconnected_minion_id), - _timeout=15, + minion_tgt=f"id:{disconnected_minion_id}", + _timeout=30, ) assert ret.returncode == 1 assert disconnected_minion_id in ret.data @@ -432,9 +430,7 @@ def test_pillar(salt_cli, salt_minion, salt_sub_minion, pillar_tree): assert salt_minion.id in ret.data assert salt_sub_minion.id in ret.data # First-level pillar (string value, only in sub_minion) - ret = salt_cli.run( - "-I", "test.ping", minion_tgt="sub:{}".format(salt_sub_minion.id) - ) + ret = salt_cli.run("-I", "test.ping", minion_tgt=f"sub:{salt_sub_minion.id}") assert ret.returncode == 0 assert salt_sub_minion.id in ret.data assert salt_minion.id not in ret.data diff --git a/tests/pytests/integration/cli/test_salt_key.py b/tests/pytests/integration/cli/test_salt_key.py index 2e16cd3a96e6..e321a1a29f75 100644 --- a/tests/pytests/integration/cli/test_salt_key.py +++ b/tests/pytests/integration/cli/test_salt_key.py @@ -1,6 +1,8 @@ import ast +import copy import os import re +import shutil import textwrap import pytest @@ -138,7 +140,7 @@ def test_list_accepted_args(salt_key_cli, key_type): assert ret.returncode == 0 assert "error:" not in ret.stdout # Should throw an error now - ret = salt_key_cli.run("-l", "foo-{}".format(key_type)) + ret = salt_key_cli.run("-l", f"foo-{key_type}") assert ret.returncode != 0 assert "error:" in ret.stderr @@ -158,6 +160,49 @@ def test_list_all(salt_key_cli, salt_minion, salt_sub_minion): assert ret.data == expected +def test_list_all_no_check_files( + salt_key_cli, salt_minion, salt_sub_minion, tmp_path, salt_master +): + """ + test salt-key -L + """ + config_dir = tmp_path / "key_no_check_files" + config_dir.mkdir() + pki_dir = config_dir / "pki_dir" + shutil.copytree(salt_master.config["pki_dir"], str(pki_dir)) + with pytest.helpers.change_cwd(str(config_dir)): + master_config = copy.deepcopy(salt_master.config) + master_config["pki_check_files"] = False + master_config["pki_dir"] = "pki_dir" + master_config["root_dir"] = str(config_dir) + with salt.utils.files.fopen(str(config_dir / "master"), "w") as fh_: + fh_.write(salt.utils.yaml.dump(master_config, default_flow_style=False)) + ret = salt_key_cli.run( + f"--config-dir={config_dir}", + "-L", + ) + assert ret.returncode == 0 + expected = { + "minions_rejected": [], + "minions_denied": [], + "minions_pre": [], + "minions": [salt_minion.id, salt_sub_minion.id], + } + assert ret.data == expected + + bad_key = pki_dir / "minions" / "dir1" + bad_key.mkdir() + + ret = salt_key_cli.run( + f"--config-dir={config_dir}", + "-L", + ) + assert ret.returncode == 0 + # The directory will show up since there is no file check + expected["minions"].insert(0, "dir1") + assert ret.data == expected + + def test_list_all_yaml_out(salt_key_cli, salt_minion, salt_sub_minion): """ test salt-key -L --out=yaml @@ -323,7 +368,7 @@ def test_accept_bad_key(salt_master, salt_key_cli): # Check Key ret = salt_key_cli.run("-y", "-a", min_name) assert ret.returncode == 0 - assert "invalid key for {}".format(min_name) in ret.stderr + assert f"invalid key for {min_name}" in ret.stderr finally: if os.path.exists(key): os.remove(key)