Skip the isfile check to increase speed of listing large numbers of keys on slow file storage

This commit is contained in:
saville 2023-06-07 13:59:19 -06:00 committed by Megan Wilhite
parent 7916ffb63e
commit 96c4c441a2
6 changed files with 98 additions and 76 deletions

1
changelog/64260.fixed.md Normal file
View file

@ -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

View file

@ -177,7 +177,7 @@ class KeyCLI:
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 @@ class KeyCLI:
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 @@ class KeyCLI:
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 @@ class Key:
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 @@ class Key:
# 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 @@ class Key:
# 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 @@ class Key:
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 @@ class Key:
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 @@ class Key:
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 @@ class Key:
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 @@ class Key:
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):

View file

@ -158,7 +158,7 @@ class SMaster:
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 @@ class Maintenance(salt.utils.process.SignalHandlingProcess):
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 @@ class FileserverUpdate(salt.utils.process.SignalHandlingProcess):
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 @@ class FileserverUpdate(salt.utils.process.SignalHandlingProcess):
# 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 @@ class Master(SMaster):
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 @@ class Master(SMaster):
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 @@ class Master(SMaster):
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 @@ class ReqServer(salt.utils.process.SignalHandlingProcess):
# 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 @@ class ClearFuncs(TransportMethods):
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 @@ class ClearFuncs(TransportMethods):
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.

View file

@ -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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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 @@ class CkMinions:
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")
):

View file

@ -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

View file

@ -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)