diff --git a/changelog/64433.added.md b/changelog/64433.added.md new file mode 100644 index 000000000000..9989fc59ed3a --- /dev/null +++ b/changelog/64433.added.md @@ -0,0 +1 @@ +Added dynamic loading of file_roots, pillar_roots, and thorium_roots to salt config diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 3cb473072f0c..5b787d12b789 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -3654,10 +3654,40 @@ is equivalent to this static configuration: prod: - /srv/prod/salt +As of 3008.0, the ``file_roots`` option is dynamically expanded on each use, +meaning directories or files may be added without restarting the Salt master. +For instance, this configuration: + +.. code-block:: yaml + + file_roots: + base: + - /srv/salt/*-formula + +may be used to dynamically add new formula directories to the ``file_roots`` +without restarting the master, as long as their name matches ``*-formula``. + .. note:: For masterless Salt, this parameter must be specified in the minion config file. +.. conf_master:: dynamic_roots_ttl + +``dynamic_roots_ttl`` +********************** + +.. versionadded:: 3008.0 + +Default: ``5.0`` + +The number of seconds a dynamically-expanded (globbed) entry in +:conf_master:`file_roots`, :conf_master:`pillar_roots`, or ``thorium_roots`` +is cached before the directory is re-scanned. Since globbed roots are +re-evaluated on every access, environments with many saltenvs or a busy +master accessing these options repeatedly (e.g. during a highstate) can +incur significant, avoidable disk I/O without this cache. Set to ``0`` to +disable caching and always re-scan on every access. + .. conf_master:: roots_update_interval ``roots_update_interval`` @@ -4841,6 +4871,19 @@ is equivalent to this static configuration: prod: - /srv/prod/pillar +As of 3008.0, the ``pillar_roots`` option is dynamically expanded on each use, +meaning directories or files may be added without restarting the Salt master. +For instance, this configuration: + +.. code-block:: yaml + + pillar_roots: + base: + - /srv/salt/*-pillar + +may be used to dynamically add new pillar directories to the ``pillar_roots`` +without restarting the master, as long as their name matches ``*-pillar``. + .. conf_master:: on_demand_ext_pillar ``on_demand_ext_pillar`` diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 6e32516235a3..547e3af05250 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -20,6 +20,7 @@ import salt.syspaths import salt.utils.data import salt.utils.dictupdate +import salt.utils.dynamic_dict import salt.utils.files import salt.utils.immutabletypes as immutabletypes import salt.utils.network @@ -314,6 +315,9 @@ def _gather_buffer_space(): "file_roots": dict, # A map of saltenvs and fileserver backend locations "pillar_roots": dict, + # Number of seconds to cache the dynamically-expanded (globbed) entries + # of file_roots, pillar_roots, and thorium_roots before re-scanning disk + "dynamic_roots_ttl": (float, int), # The external pillars permitted to be used on-demand using pillar.ext "on_demand_ext_pillar": list, # A map of glob paths to be used @@ -1201,6 +1205,7 @@ def _gather_buffer_space(): "file_roots": { "base": [salt.syspaths.BASE_FILE_ROOTS_DIR, salt.syspaths.SPM_FORMULA_PATH] }, + "dynamic_roots_ttl": 5.0, "top_file_merging_strategy": "merge", "env_order": [], "default_top": "base", @@ -1544,6 +1549,7 @@ def _gather_buffer_space(): "file_roots": { "base": [salt.syspaths.BASE_FILE_ROOTS_DIR, salt.syspaths.SPM_FORMULA_PATH] }, + "dynamic_roots_ttl": 5.0, "master_roots": {"base": [salt.syspaths.BASE_MASTER_ROOTS_DIR]}, "pillar_roots": { "base": [salt.syspaths.BASE_PILLAR_ROOTS_DIR, salt.syspaths.SPM_PILLAR_PATH] @@ -2095,55 +2101,45 @@ def _gather_buffer_space(): # <---- Salt Cloud Configuration Defaults ------------------------------------ -def _normalize_roots(file_roots): +def _validate_roots(opts, prop_name): """ - Normalize file or pillar roots. + If the roots option has a key that is None then we will log a warning and + use the defaults instead. """ - for saltenv, dirs in file_roots.items(): - normalized_saltenv = str(saltenv) - if normalized_saltenv != saltenv: - file_roots[normalized_saltenv] = file_roots.pop(saltenv) - if not isinstance(dirs, (list, tuple)): - file_roots[normalized_saltenv] = [] - file_roots[normalized_saltenv] = _expand_glob_path( - file_roots[normalized_saltenv] - ) - return file_roots - - -def _validate_pillar_roots(pillar_roots): - """ - If the pillar_roots option has a key that is None then we will error out, - just replace it with an empty list - """ - if not isinstance(pillar_roots, dict): + roots = opts.get(prop_name) + if not isinstance(roots, dict): log.warning( - "The pillar_roots parameter is not properly formatted, using defaults" + "The %s parameter is not properly formatted, using defaults", + prop_name, ) - return {"base": _expand_glob_path([salt.syspaths.BASE_PILLAR_ROOTS_DIR])} - return _normalize_roots(pillar_roots) + return {"base": _expand_glob_path(DEFAULT_MASTER_OPTS.get(prop_name)["base"])} + # Use a dynamic dict to resolve file roots dynamically + result = salt.utils.dynamic_dict.DynamicDict( + ttl=opts.get("dynamic_roots_ttl", DEFAULT_MASTER_OPTS["dynamic_roots_ttl"]) + ) + for saltenv, dirs in roots.items(): + normalized_saltenv = str(saltenv) + if not isinstance(dirs, (list, tuple)): + dirs = [] + result.add_dyn(normalized_saltenv, _expand_glob_path, dirs) + return result -def _validate_file_roots(file_roots): +def _expand_glob_path(dirs, dyn_dict=None, key=None): """ - If the file_roots option has a key that is None then we will error out, - just replace it with an empty list + A dynamic dict function that applies shell globbing to a set of + directories and returns the expanded paths """ - if not isinstance(file_roots, dict): - log.warning( - "The file_roots parameter is not properly formatted, using defaults" - ) - return {"base": _expand_glob_path([salt.syspaths.BASE_FILE_ROOTS_DIR])} - return _normalize_roots(file_roots) + # Unused arguments from dynamic dict + _ = dyn_dict + _ = key + # At times, this method is called with non-iterable objects, + # so just return them directly + if not isinstance(dirs, (list, tuple)): + return dirs - -def _expand_glob_path(file_roots): - """ - Applies shell globbing to a set of directories and returns - the expanded paths - """ unglobbed_path = [] - for path in file_roots: + for path in dirs: try: if glob.has_magic(path): unglobbed_path.extend(glob.glob(path)) @@ -2899,12 +2895,16 @@ def apply_sdb(opts, sdb_opts=None, _visited=None): return salt.utils.sdb.sdb_get(sdb_opts, opts) elif isinstance(sdb_opts, dict): + is_dyn_dict = isinstance(sdb_opts, salt.utils.dynamic_dict.DynamicDict) # Create a list of items to avoid modifying dict during iteration # This is especially important for OptsDict which has special iteration behavior items = list(sdb_opts.items()) for key, value in items: if value is None: continue + if is_dyn_dict and sdb_opts.is_dyn_key(key): + # Nothing to do for dynamically generated elements + continue sdb_opts[key] = apply_sdb(opts, value, _visited) elif isinstance(sdb_opts, list): for key, value in enumerate(sdb_opts): @@ -4181,8 +4181,8 @@ def apply_minion_config( # Enabling open mode requires that the value be set to True, and # nothing else! opts["open_mode"] = opts["open_mode"] is True - opts["file_roots"] = _validate_file_roots(opts["file_roots"]) - opts["pillar_roots"] = _validate_pillar_roots(opts["pillar_roots"]) + opts["file_roots"] = _validate_roots(opts, "file_roots") + opts["pillar_roots"] = _validate_roots(opts, "pillar_roots") # Make sure ext_mods gets set if it is an untrue value # (here to catch older bad configs) opts["extension_modules"] = opts.get("extension_modules") or os.path.join( @@ -4481,8 +4481,8 @@ def apply_master_config(overrides=None, defaults=None): # nothing else! opts["open_mode"] = opts["open_mode"] is True opts["auto_accept"] = opts["auto_accept"] is True - opts["file_roots"] = _validate_file_roots(opts["file_roots"]) - opts["pillar_roots"] = _validate_file_roots(opts["pillar_roots"]) + opts["file_roots"] = _validate_roots(opts, "file_roots") + opts["pillar_roots"] = _validate_roots(opts, "pillar_roots") if opts["file_ignore_regex"]: # If file_ignore_regex was given, make sure it's wrapped in a list. diff --git a/salt/thorium/__init__.py b/salt/thorium/__init__.py index 82351e448584..79e8bcf9e6ae 100644 --- a/salt/thorium/__init__.py +++ b/salt/thorium/__init__.py @@ -19,6 +19,7 @@ import salt.loader import salt.payload import salt.state +from salt.config import _validate_roots from salt.exceptions import SaltRenderError log = logging.getLogger(__name__) @@ -36,7 +37,7 @@ def __init__( self.grain_keys = grain_keys self.pillar = pillar self.pillar_keys = pillar_keys - opts["file_roots"] = opts["thorium_roots"] + opts["file_roots"] = _validate_roots(opts, "thorium_roots") opts["saltenv"] = opts["thoriumenv"] opts["state_top"] = opts["thorium_top"] opts["file_client"] = "local" diff --git a/salt/utils/dynamic_dict.py b/salt/utils/dynamic_dict.py new file mode 100644 index 000000000000..15231153ada3 --- /dev/null +++ b/salt/utils/dynamic_dict.py @@ -0,0 +1,111 @@ +""" +A dictionary with optionally dynamic values, used for dynamic configuration such +as file roots. +""" + +import copy +import time + +__all__ = ["DynamicDict"] + +#: Default number of seconds a dynamic value is cached before being +#: re-evaluated. Callers (e.g. ``salt.config``) may override this per +#: instance via the ``ttl`` argument. +DEFAULT_TTL = 5.0 + + +class DynamicDict(dict): + """ + A dictionary that can mix static and dynamic values. + """ + + def __init__(self, *args, ttl=DEFAULT_TTL, **argv): + self._func_dict = {} + self._cache = {} + self._ttl = ttl + super().__init__(*args, **argv) + + def __getitem__(self, key): + val = super().__getitem__(key) + if key in self._func_dict: + now = time.time() + cached = self._cache.get(key) + if self._ttl and cached is not None and (now - cached[1]) < self._ttl: + return cached[0] + val = self._func_dict[key](val, dyn_dict=self, key=key) + self._cache[key] = (val, now) + return val + + def __delitem__(self, key): + if key in self._func_dict: + del self._func_dict[key] + self._cache.pop(key, None) + super().__delitem__(key) + + def get(self, key, default=None): + if key not in self: + return default + return self[key] + + def pop(self, key, default=None): + if key in self: + val = self[key] + del self[key] + else: + val = default + return val + + def values(self): + keys = super().keys() + for key in keys: + yield self[key] + + def items(self): + keys = super().keys() + for key in keys: + yield key, self[key] + + def copy(self): + new_dd = DynamicDict(ttl=self._ttl) + for key, val in super().items(): + if key in self._func_dict: + func = self._func_dict[key] + data = super().__getitem__(key) + new_dd.add_dyn(key, func, data) + else: + new_dd[key] = val + return new_dd + + def __copy__(self): + return self.copy() + + def __deepcopy__(self, memo): + rdd = DynamicDict(ttl=self._ttl) + memo[id(self)] = rdd + for key in super().keys(): + if key in self._func_dict: + func = self._func_dict[key] + data = copy.deepcopy(super().__getitem__(key), memo) + rdd.add_dyn(key, func, data) + else: + copied_key = copy.deepcopy(key, memo) + copied_value = copy.deepcopy(super().__getitem__(key), memo) + rdd[copied_key] = copied_value + return rdd + + def static_dict(self): + new_dict = {} + for key in super().keys(): + new_dict[key] = self[key] + return new_dict + + def is_dyn_key(self, key): + return key in self._func_dict + + def add_dyn(self, key, func, data=None): + if not hasattr(func, "__call__"): + raise ValueError(f"Value for key '{key}' is not a function") + self._func_dict[key] = func + self._cache.pop(key, None) + if data is not None or key not in self: + self[key] = data diff --git a/salt/utils/yamldumper.py b/salt/utils/yamldumper.py index 9aa49d7966b6..56f604fec2ac 100644 --- a/salt/utils/yamldumper.py +++ b/salt/utils/yamldumper.py @@ -15,6 +15,7 @@ import salt.utils.context from salt.utils.datastructures import HashableOrderedDict +from salt.utils.dynamic_dict import DynamicDict from salt.utils.optsdict import DictProxy, ListProxy, OptsDict from salt.utils.secret import MaskedDict, MaskedList @@ -162,6 +163,9 @@ def represent_listproxy(dumper, data): "tag:yaml.org,2002:timestamp", SafeOrderedDumper.represent_scalar ) +OrderedDumper.add_representer(DynamicDict, represent_ordereddict) +SafeOrderedDumper.add_representer(DynamicDict, represent_ordereddict) + def get_dumper(dumper_name): return { diff --git a/tests/filename_map.yml b/tests/filename_map.yml index c775d2ebe7e8..f79d0cd16227 100644 --- a/tests/filename_map.yml +++ b/tests/filename_map.yml @@ -177,6 +177,9 @@ salt/state.py: salt/utils/decorators/*: - integration.modules.test_decorators +salt/utils/dynamic_dict.py: + - pytests.unit.utils.test_dynamic_dict + salt/(utils|renderers)/jinja\.py: - pytests.functional.modules.state.test_jinja_filters - integration.states.test_renderers @@ -221,6 +224,7 @@ salt/client/ssh/wrapper/*: salt/config/*: - unit.test_config - pytests.unit.config.test__validate_opts + - pytests.integration.master.test_dynamic_config salt/loader/*: - integration.loader.test_ext_modules diff --git a/tests/pytests/functional/conftest.py b/tests/pytests/functional/conftest.py index d7ba6ec3c7e2..a29356c4338f 100644 --- a/tests/pytests/functional/conftest.py +++ b/tests/pytests/functional/conftest.py @@ -140,7 +140,10 @@ def master_opts( @pytest.fixture(scope="module") def loaders(minion_opts): - return Loaders(minion_opts, loaded_base_name=f"{__name__}.loaded") + result = Loaders(minion_opts, loaded_base_name=f"{__name__}.loaded") + # Convert the file roots to a static dict before appending any roots + result.opts["file_roots"] = result.opts["file_roots"].static_dict() + return result @pytest.fixture(autouse=True) diff --git a/tests/pytests/integration/cli/test_salt.py b/tests/pytests/integration/cli/test_salt.py index 141534cc3c3a..ad0eb53fd911 100644 --- a/tests/pytests/integration/cli/test_salt.py +++ b/tests/pytests/integration/cli/test_salt.py @@ -32,8 +32,9 @@ def salt_minion_2(salt_master): """ A running salt-minion fixture """ + _minion_id = "minion-2" factory = salt_master.salt_minion_daemon( - "minion-2", + _minion_id, overrides={ "fips_mode": FIPS_TESTRUN, "encryption_algorithm": "OAEP-SHA224" if FIPS_TESTRUN else "OAEP-SHA1", @@ -57,7 +58,7 @@ def salt_minion_2(salt_master): salt_master.salt_key_cli().run("-d", factory.id, "-y") # Clean up the key so it doesn't affect subsequent tests like test_salt_key.py - key_file = os.path.join(salt_master.config["pki_dir"], "minions", "minion-2") + key_file = os.path.join(salt_master.config["pki_dir"], "minions", _minion_id) if os.path.exists(key_file): os.remove(key_file) diff --git a/tests/pytests/integration/cli/test_salt_call.py b/tests/pytests/integration/cli/test_salt_call.py index 60dcf61ff261..5cd237758dbd 100644 --- a/tests/pytests/integration/cli/test_salt_call.py +++ b/tests/pytests/integration/cli/test_salt_call.py @@ -508,7 +508,9 @@ def test_syslog_file_not_found(salt_minion, salt_call_cli, tmp_path): minion_config = copy.deepcopy(salt_minion.config) minion_config["log_file"] = "file:///dev/doesnotexist" with salt.utils.files.fopen(str(config_dir / "minion"), "w") as fh_: - fh_.write(salt.utils.yaml.dump(minion_config, default_flow_style=False)) + fh_.write( + salt.utils.yaml.safe_dump(minion_config, default_flow_style=False) + ) ret = salt_call_cli.run( f"--config-dir={config_dir}", "--log-level=debug", diff --git a/tests/pytests/integration/cli/test_salt_key.py b/tests/pytests/integration/cli/test_salt_key.py index 73b19e2b1c40..776f3b5fb362 100644 --- a/tests/pytests/integration/cli/test_salt_key.py +++ b/tests/pytests/integration/cli/test_salt_key.py @@ -185,7 +185,9 @@ def test_list_all_no_check_files( 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)) + fh_.write( + salt.utils.yaml.safe_dump(master_config, default_flow_style=False) + ) ret = salt_key_cli.run( f"--config-dir={config_dir}", "-L", diff --git a/tests/pytests/integration/master/test_dynamic_config.py b/tests/pytests/integration/master/test_dynamic_config.py new file mode 100644 index 000000000000..7e7f95172986 --- /dev/null +++ b/tests/pytests/integration/master/test_dynamic_config.py @@ -0,0 +1,161 @@ +""" +Tests for dynamically loading configuration +""" + +import json + +import pytest + + +@pytest.fixture(scope="module") +def sls_contents(): + return """ + test: + test.nop + """ + + +def get_pillar_top_file(*pillar_files): + top = """ + base: + '*': + """ + for file_name in pillar_files: + top = f"{top}\n - {file_name}" + return top + + +def get_pillar_contents(prop_name): + return f""" + {prop_name}: true + """ + + +@pytest.fixture(scope="module") +def formula_path(tmp_path_factory, sls_contents): + path = tmp_path_factory.mktemp("formulas") + + # Create the initial formula files + with pytest.helpers.temp_file( + "initial-formula/initial-test.sls", + sls_contents, + path, + ): + yield path + + +@pytest.fixture(scope="module") +def pillar_path(tmp_path_factory): + path = tmp_path_factory.mktemp("pillars") + with pytest.helpers.temp_file( + "initial-pillar/initial-pillar.sls", + get_pillar_contents("initial"), + path, + ): + yield path + + +@pytest.fixture(scope="module") +def runner_master_config(formula_path, pillar_path): + return { + "auto_accept": True, + "env_order": ["base"], + "file_roots": {"base": [str(formula_path / "*-formula")]}, + "pillar_roots": {"base": [str(pillar_path / "*-pillar")]}, + } + + +@pytest.fixture(scope="module") +def runner_salt_master( + salt_factories, runner_master_config, formula_path, sls_contents, pillar_path +): + factory = salt_factories.salt_master_daemon( + "runner-master", defaults=runner_master_config + ) + with factory.started(): + # Create base test files + with pytest.helpers.temp_file( + "base-test.sls", + sls_contents, + factory.state_tree.base.paths[-1], + ), pytest.helpers.temp_file( + "base-pillar.sls", + get_pillar_contents("base"), + factory.pillar_tree.base.paths[-1], + ), factory.pillar_tree.base.temp_file( + "top.sls", + get_pillar_top_file("base-pillar", "initial-pillar"), + ): + yield factory + + +@pytest.fixture(scope="module") +def runner_salt_minion(runner_salt_master): + assert runner_salt_master.is_running() + factory = runner_salt_master.salt_minion_daemon("runner-minion") + # Don't actually start the minion since we only need the salt call API + yield factory + + +@pytest.fixture(scope="module") +def runner_salt_call_cli(runner_salt_minion): + return runner_salt_minion.salt_call_cli() + + +def test_initial_formulas(runner_salt_call_cli): + # Base state tree files work + ret = runner_salt_call_cli.run("state.apply", "base-test") + assert ret.returncode == 0 + assert "No matching sls found for 'base-test'" not in ret.stdout + + # The initial formula exists + ret = runner_salt_call_cli.run("state.apply", "initial-test") + assert ret.returncode == 0 + assert "No matching sls found for 'initial-test'" not in ret.stdout + + # The new formula does not + ret = runner_salt_call_cli.run("state.apply", "new-test") + assert ret.returncode != 0 + assert "No matching sls found for 'new-test'" in ret.stdout + + +def test_dynamic_formula(runner_salt_call_cli, formula_path, sls_contents): + with pytest.helpers.temp_file( + "new-formula/new-test.sls", + sls_contents, + formula_path, + ): + ret = runner_salt_call_cli.run("state.apply", "new-test") + assert ret.returncode == 0 + assert "No matching sls found for 'new-test'" not in ret.stdout + + +def test_initial_pillars(runner_salt_call_cli, runner_salt_master): + # Base pillar files work + ret = runner_salt_call_cli.run("pillar.get", "base") + assert ret.returncode == 0 + assert json.loads(ret.stdout) == {"local": True} + + # The initial pillar exists + ret = runner_salt_call_cli.run("pillar.get", "initial") + assert ret.returncode == 0 + assert json.loads(ret.stdout) == {"local": True} + + # The new pillar does not + ret = runner_salt_call_cli.run("pillar.get", "new") + assert ret.returncode == 0 + assert json.loads(ret.stdout) == {"local": ""} + + +def test_dynamic_pillar(runner_salt_call_cli, runner_salt_master, pillar_path): + with pytest.helpers.temp_file( + "new-pillar/new-pillar.sls", + get_pillar_contents("new"), + pillar_path, + ), runner_salt_master.pillar_tree.base.temp_file( + "top.sls", + get_pillar_top_file("base-pillar", "initial-pillar", "new-pillar"), + ): + ret = runner_salt_call_cli.run("pillar.get", "new") + assert ret.returncode == 0 + assert json.loads(ret.stdout) == {"local": True} diff --git a/tests/pytests/unit/utils/test_dynamic_dict.py b/tests/pytests/unit/utils/test_dynamic_dict.py new file mode 100644 index 000000000000..4c7b155e32dc --- /dev/null +++ b/tests/pytests/unit/utils/test_dynamic_dict.py @@ -0,0 +1,193 @@ +""" + tests.unit.utils.test_dynamic_dict + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + + Test the dynamic dict class +""" + +import copy + +import pytest + +from salt.utils.dynamic_dict import DynamicDict + + +@pytest.fixture(name="base_dict") +def fixture_base_dict(): + return { + "a": 1, + "b": 2, + "c": 3, + } + + +@pytest.fixture(name="ddict") +def fixture_ddict(base_dict): + return DynamicDict(**base_dict) + + +@pytest.fixture(name="copied_ddict") +def fixture_copied_ddict(ddict): + return ddict.copy() + + +@pytest.fixture(name="dyn_func") +def fixture_dyn_func(): + def dyn_func(data=None, dyn_dict=None, key=None): + return range(3) + + return dyn_func + + +def test_constructor_items(base_dict): + ddict = DynamicDict(base_dict.items()) + for key, val in base_dict.items(): + assert ( + key in ddict + ), f"Missing key '{key}' in DynamicDict: Tuple __init__() failed" + assert ( + ddict[key] == val + ), "Value of key '{}':{} != {}: Tuple __init__() failed".format( + key, ddict[key], val + ) + + +def test_constructor_expansion(base_dict): + ddict = DynamicDict(**base_dict) + for key, val in base_dict.items(): + assert ( + key in ddict + ), f"Missing key '{key}' in DynamicDict: Tuple __init__() failed" + assert ( + ddict[key] == val + ), "Value of key '{}':{} != {}: Tuple __init__() failed".format( + key, ddict[key], val + ) + + +def test_static(ddict, base_dict): + ddict["foo"] = "FOO" + copied_ddict = base_dict.copy() + copied_ddict["foo"] = "FOO" + assert "foo" in ddict, "Failed to add static key 'foo'" + assert ddict["foo"] == "FOO", "Static key 'foo':{} != 'FOO'".format(ddict["foo"]) + assert ddict.get("foo") == "FOO", "Static key 'foo':{} != 'FOO'".format( + ddict["foo"] + ) + assert not ddict.is_dyn_key("foo"), "Static key 'foo' should not be a dynamic key" + del ddict["foo"] + del copied_ddict["foo"] + assert "foo" not in ddict, "failed to delete static key 'foo'" + + +def test_dynamic(ddict, copied_ddict, dyn_func): + ddict.add_dyn("foo", dyn_func) + copied_ddict["foo"] = dyn_func() + assert "foo" in ddict, "Failed to add dynamic key 'foo'" + assert not set(copied_ddict.keys()).difference( + ddict.keys() + ), "Unexpected keys in DynamicDict" + assert ddict["foo"] == copied_ddict["foo"] + assert ddict["foo"] == copied_ddict.get("foo") + assert ddict.get("foo") == dyn_func() + assert ddict.get("foo") == copied_ddict["foo"] + + +def test_iterating(base_dict, ddict, copied_ddict, dyn_func): + for key in ddict: + assert key in copied_ddict, f"Found an unexpected key: {key}" + del copied_ddict[key] + assert ( + not copied_ddict + ), "Failed to iterate across all keys in DynamicDict - remaining: {}".format( + copied_ddict + ) + + ddict.add_dyn("foo", dyn_func) + copied_ddict = base_dict.copy() + copied_ddict["foo"] = dyn_func() + for key in ddict.keys(): + assert key in copied_ddict, f"Found an unexpected key: {key}" + del copied_ddict[key] + assert ( + not copied_ddict + ), "Failed to iterate across all keys in DynamicDict - remaining: {}".format( + copied_ddict + ) + + +def test_copy(ddict, base_dict, dyn_func): + ddict.add_dyn("foo", dyn_func) + ddict2 = ddict.copy() + copied_dict = base_dict.copy() + copied_dict["foo"] = dyn_func() + for key in ddict2.keys(): + assert key in copied_dict, f"Found an unexpected key in the copy: {key}" + del copied_dict[key] + assert ( + not copied_dict + ), "Failed to iterate across all keys in DynamicDict copy - remaining: {}".format( + copied_dict + ) + assert hasattr( + ddict2._func_dict.get("foo"), "__call__" + ), "Failed to copy dynamic key" + + ddict2 = {"ddict": ddict} + copied_dict = {"ddict": base_dict.copy()} + copied_dict["ddict"]["foo"] = dyn_func() + ddict3 = copy.deepcopy(ddict2) + assert ddict3["ddict"].is_dyn_key("foo"), "Dyn key 'foo' is no longer a dynamic key" + assert hasattr( + ddict3["ddict"]._func_dict.get("foo"), "__call__" + ), "Failed to copy dynamic key" + + copied_dict = base_dict.copy() + copied_dict["foo"] = dyn_func() + for key, val in ddict.items(): + assert key in copied_dict, f"Found an unexpected key: {key}" + del copied_dict[key] + assert ( + not copied_dict + ), "Failed to iterate across all keys in DynamicDict - remaining: {}".format( + copied_dict + ) + + +def test_static_dict(ddict, base_dict, dyn_func): + ddict.add_dyn("foo", dyn_func) + static_dict = ddict.static_dict() + assert not isinstance(static_dict, DynamicDict) + assert set(static_dict.keys()) == set(ddict.keys()) + for key, val in static_dict.items(): + assert val == ddict[key] + + +def test_copy_iterator(base_dict, ddict, dyn_func): + ddict.add_dyn("foo", dyn_func) + copied_dict = base_dict.copy() + copied_dict["foo"] = dyn_func() + xvals = [val for val in copied_dict.values()] + for val in ddict.values(): + assert val in xvals, f"Found an unexpected value: {val}" + xvals.remove(val) + assert ( + not xvals + ), "Failed to iterate across all values in DynamicDict - remaining: {}".format( + xvals + ) + + +def _dyn_func(data=None, dyn_dict=None, key=None): + return 7 + + +def test_delete_dyn_key(ddict): + ddict.add_dyn("foo", _dyn_func) + val = ddict.pop("foo") + assert val == _dyn_func(), f"Failed pop(): {val} != {_dyn_func()}" + assert "foo" not in ddict, "Failed to remove key 'foo' when pop()ed" + + ddict.add_dyn("foo", _dyn_func) + del ddict["foo"] + assert "foo" not in ddict, "Failed to delete dynamic key 'foo'" diff --git a/tests/pytests/unit/utils/test_yamldumper.py b/tests/pytests/unit/utils/test_yamldumper.py index 526b23969fb1..ec18e25acf24 100644 --- a/tests/pytests/unit/utils/test_yamldumper.py +++ b/tests/pytests/unit/utils/test_yamldumper.py @@ -7,6 +7,7 @@ import salt.utils.yamldumper from salt.utils.context import NamespacedDictWrapper from salt.utils.datastructures import HashableOrderedDict +from salt.utils.dynamic_dict import DynamicDict def test_yaml_dump(): @@ -47,6 +48,18 @@ def test_yaml_ordered_dump(): ) +def test_yaml_dynamic_dict_dump(): + """ + Test yaml.dump with DynamicDict + """ + data = DynamicDict([("foo", "bar"), ("baz", "qux")]) + exp_yaml = "{foo: bar, baz: qux}\n" + assert ( + salt.utils.yamldumper.dump(data, Dumper=salt.utils.yamldumper.OrderedDumper) + == exp_yaml + ) + + def test_yaml_safe_ordered_dump(): """ Test yaml.safe_dump with OrderedDict diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 991b141d2f9b..a7a911e1f2ad 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -12,6 +12,7 @@ import salt.crypt import salt.minion import salt.syspaths +import salt.utils.dynamic_dict import salt.utils.files import salt.utils.network import salt.utils.platform @@ -493,11 +494,83 @@ def test_master_file_roots_glob(self, tempdir, fpath): }, ) + def test_validate_roots(self): + with patch("salt.config._expand_glob_path") as egp: + egp.side_effect = [["dir1"], ["dir2"]] + ret = salt.config._validate_roots( + { + "file_roots": { + "env1": ["/tmp/dir1", "/tmp/dir2"], + "env2": ["/tmp/dir3"], + } + }, + "file_roots", + ) + + assert isinstance(ret, salt.utils.dynamic_dict.DynamicDict) + egp.assert_not_called() + assert ret["env1"] == ["dir1"] + egp.assert_called_once_with( + ["/tmp/dir1", "/tmp/dir2"], dyn_dict=ret, key="env1" + ) + egp.reset_mock() + assert ret.get("env2") == ["dir2"] + egp.assert_called_once_with(["/tmp/dir3"], dyn_dict=ret, key="env2") + + @with_tempdir() + def test_dynamic_roots(self, tempdir): + # Create some files for roots + for file_name in ("formula1", "formula2", "other1", "other2"): + with salt.utils.files.fopen(os.path.join(tempdir, file_name), "w") as fobj: + fobj.write(file_name) + + ret = salt.config._validate_roots( + { + "file_roots": { + "env1": [os.path.join(tempdir, "formula*")], + "env2": [os.path.join(tempdir, "other*")], + } + }, + "file_roots", + ) + + assert set(ret["env1"]) == { + os.path.join(tempdir, "formula1"), + os.path.join(tempdir, "formula2"), + } + assert set(ret["env2"]) == { + os.path.join(tempdir, "other1"), + os.path.join(tempdir, "other2"), + } + + # Create more files + for file_name in ("formula3", "other3"): + with salt.utils.files.fopen(os.path.join(tempdir, file_name), "w") as fobj: + fobj.write(file_name) + + # Roots are resolved again + assert set(ret["env1"]) == { + os.path.join(tempdir, "formula1"), + os.path.join(tempdir, "formula2"), + os.path.join(tempdir, "formula3"), + } + assert set(ret["env2"]) == { + os.path.join(tempdir, "other1"), + os.path.join(tempdir, "other2"), + os.path.join(tempdir, "other3"), + } + def test_validate_bad_file_roots(self): - expected = salt.config._expand_glob_path([salt.syspaths.BASE_FILE_ROOTS_DIR]) - with patch("salt.config._normalize_roots") as mk: - ret = salt.config._validate_file_roots(None) - assert not mk.called + expected = ["dir1"] + with patch("salt.config._expand_glob_path") as egp: + egp.return_value = expected + ret = salt.config._validate_roots({}, "file_roots") + assert not isinstance(ret, salt.utils.dynamic_dict.DynamicDict) + # Immutable lists do not support direct comparison, so cannot use assert_called_once_with() + egp.assert_called_once() + assert list(egp.call_args[0][0]) == list( + salt.config.DEFAULT_MASTER_OPTS["file_roots"]["base"] + ) assert ret == {"base": expected} @with_tempfile() @@ -525,10 +598,16 @@ def test_master_pillar_roots_glob(self, tempdir, fpath): ) def test_validate_bad_pillar_roots(self): - expected = salt.config._expand_glob_path([salt.syspaths.BASE_PILLAR_ROOTS_DIR]) - with patch("salt.config._normalize_roots") as mk: - ret = salt.config._validate_pillar_roots(None) - assert not mk.called + expected = ["dir1"] + with patch("salt.config._expand_glob_path") as egp: + egp.return_value = expected + ret = salt.config._validate_roots({}, "pillar_roots") + assert not isinstance(ret, salt.utils.dynamic_dict.DynamicDict) + # Immutable lists do not support direct comparison, so cannot use assert_called_once_with() + egp.assert_called_once() + assert list(egp.call_args[0][0]) == list( + salt.config.DEFAULT_MASTER_OPTS["pillar_roots"]["base"] + ) assert ret == {"base": expected} @with_tempdir()