From fb18792730f5b63206bab3651ddc2b4a8b905a77 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 6 Nov 2023 12:53:50 +0100 Subject: [PATCH 1/7] Add `slsutil` SSH wrapper (cherry picked from commit ddc119764707a355069ff93350ab8e06f2a688ea) --- changelog/50196.fixed.md | 1 + changelog/61143.fixed.md | 1 + changelog/65067.fixed.md | 1 + salt/client/ssh/wrapper/slsutil.py | 450 ++++++++++++++++++ tests/pytests/integration/ssh/test_slsutil.py | 94 ++++ .../unit/client/ssh/wrapper/test_slsutil.py | 166 +++++++ 6 files changed, 713 insertions(+) create mode 100644 changelog/50196.fixed.md create mode 100644 changelog/61143.fixed.md create mode 100644 changelog/65067.fixed.md create mode 100644 salt/client/ssh/wrapper/slsutil.py create mode 100644 tests/pytests/integration/ssh/test_slsutil.py create mode 100644 tests/pytests/unit/client/ssh/wrapper/test_slsutil.py diff --git a/changelog/50196.fixed.md b/changelog/50196.fixed.md new file mode 100644 index 000000000000..979411a640da --- /dev/null +++ b/changelog/50196.fixed.md @@ -0,0 +1 @@ +Made slsutil.renderer work with salt-ssh diff --git a/changelog/61143.fixed.md b/changelog/61143.fixed.md new file mode 100644 index 000000000000..08a62c9d8b10 --- /dev/null +++ b/changelog/61143.fixed.md @@ -0,0 +1 @@ +Made slsutil.findup work with salt-ssh diff --git a/changelog/65067.fixed.md b/changelog/65067.fixed.md new file mode 100644 index 000000000000..d6de87b5bc15 --- /dev/null +++ b/changelog/65067.fixed.md @@ -0,0 +1 @@ +Fixed slsutil.update with salt-ssh during template rendering diff --git a/salt/client/ssh/wrapper/slsutil.py b/salt/client/ssh/wrapper/slsutil.py new file mode 100644 index 000000000000..e09ca1c29845 --- /dev/null +++ b/salt/client/ssh/wrapper/slsutil.py @@ -0,0 +1,450 @@ +import os.path +import posixpath + +import salt.exceptions +import salt.loader +import salt.template +import salt.utils.args +import salt.utils.dictupdate +import salt.utils.stringio + +CONTEXT_BASE = "slsutil" + + +def update(dest, upd, recursive_update=True, merge_lists=False): + """ + Merge ``upd`` recursively into ``dest`` + + If ``merge_lists=True``, will aggregate list object types instead of + replacing. This behavior is only activated when ``recursive_update=True``. + + CLI Example: + + .. code-block:: shell + + salt '*' slsutil.update '{foo: Foo}' '{bar: Bar}' + + """ + return salt.utils.dictupdate.update(dest, upd, recursive_update, merge_lists) + + +def merge(obj_a, obj_b, strategy="smart", renderer="yaml", merge_lists=False): + """ + Merge a data structure into another by choosing a merge strategy + + Strategies: + + * aggregate + * list + * overwrite + * recurse + * smart + + CLI Example: + + .. code-block:: shell + + salt '*' slsutil.merge '{foo: Foo}' '{bar: Bar}' + """ + return salt.utils.dictupdate.merge(obj_a, obj_b, strategy, renderer, merge_lists) + + +def merge_all(lst, strategy="smart", renderer="yaml", merge_lists=False): + """ + .. versionadded:: 2019.2.0 + + Merge a list of objects into each other in order + + :type lst: Iterable + :param lst: List of objects to be merged. + + :type strategy: String + :param strategy: Merge strategy. See utils.dictupdate. + + :type renderer: String + :param renderer: + Renderer type. Used to determine strategy when strategy is 'smart'. + + :type merge_lists: Bool + :param merge_lists: Defines whether to merge embedded object lists. + + CLI Example: + + .. code-block:: shell + + $ salt-call --output=txt slsutil.merge_all '[{foo: Foo}, {foo: Bar}]' + local: {u'foo': u'Bar'} + """ + + ret = {} + for obj in lst: + ret = salt.utils.dictupdate.merge(ret, obj, strategy, renderer, merge_lists) + + return ret + + +def renderer(path=None, string=None, default_renderer="jinja|yaml", **kwargs): + """ + Parse a string or file through Salt's renderer system + + .. versionchanged:: 2018.3.0 + Add support for Salt fileserver URIs. + + This is an open-ended function and can be used for a variety of tasks. It + makes use of Salt's "renderer pipes" system to run a string or file through + a pipe of any of the loaded renderer modules. + + :param path: The path to a file on Salt's fileserver (any URIs supported by + :py:func:`cp.get_url `) or on the local file + system. + :param string: An inline string to be used as the file to send through the + renderer system. Note, not all renderer modules can work with strings; + the 'py' renderer requires a file, for example. + :param default_renderer: The renderer pipe to send the file through; this + is overridden by a "she-bang" at the top of the file. + :param kwargs: Keyword args to pass to Salt's compile_template() function. + + Keep in mind the goal of each renderer when choosing a render-pipe; for + example, the Jinja renderer processes a text file and produces a string, + however the YAML renderer processes a text file and produces a data + structure. + + One possible use is to allow writing "map files", as are commonly seen in + Salt formulas, but without tying the renderer of the map file to the + renderer used in the other sls files. In other words, a map file could use + the Python renderer and still be included and used by an sls file that uses + the default 'jinja|yaml' renderer. + + For example, the two following map files produce identical results but one + is written using the normal 'jinja|yaml' and the other is using 'py': + + .. code-block:: jinja + + #!jinja|yaml + {% set apache = salt.grains.filter_by({ + ...normal jinja map file here... + }, merge=salt.pillar.get('apache:lookup')) %} + {{ apache | yaml() }} + + .. code-block:: python + + #!py + def run(): + apache = __salt__.grains.filter_by({ + ...normal map here but as a python dict... + }, merge=__salt__.pillar.get('apache:lookup')) + return apache + + Regardless of which of the above map files is used, it can be accessed from + any other sls file by calling this function. The following is a usage + example in Jinja: + + .. code-block:: jinja + + {% set apache = salt.slsutil.renderer('map.sls') %} + + CLI Example: + + .. code-block:: bash + + salt '*' slsutil.renderer salt://path/to/file + salt '*' slsutil.renderer /path/to/file + salt '*' slsutil.renderer /path/to/file.jinja default_renderer='jinja' + salt '*' slsutil.renderer /path/to/file.sls default_renderer='jinja|yaml' + salt '*' slsutil.renderer string='Inline template! {{ saltenv }}' + salt '*' slsutil.renderer string='Hello, {{ name }}.' name='world' + """ + if not path and not string: + raise salt.exceptions.SaltInvocationError("Must pass either path or string") + + renderers = salt.loader.render(__opts__, __salt__) + + if path: + path_or_string = __context__["fileclient"].get_url( + path, "", saltenv=kwargs.get("saltenv", "base") + ) + elif string: + path_or_string = ":string:" + kwargs["input_data"] = string + + ret = salt.template.compile_template( + path_or_string, + renderers, + default_renderer, + __opts__["renderer_blacklist"], + __opts__["renderer_whitelist"], + **kwargs + ) + return ret.read() if salt.utils.stringio.is_readable(ret) else ret + + +def _get_serialize_fn(serializer, fn_name): + serializers = salt.loader.serializers(__opts__) + fns = getattr(serializers, serializer, None) + fn = getattr(fns, fn_name, None) + + if not fns: + raise salt.exceptions.CommandExecutionError( + "Serializer '{}' not found.".format(serializer) + ) + + if not fn: + raise salt.exceptions.CommandExecutionError( + "Serializer '{}' does not implement {}.".format(serializer, fn_name) + ) + + return fn + + +def serialize(serializer, obj, **mod_kwargs): + """ + Serialize a Python object using one of the available + :ref:`all-salt.serializers`. + + CLI Example: + + .. code-block:: bash + + salt '*' --no-parse=obj slsutil.serialize 'json' obj="{'foo': 'Foo!'} + + Jinja Example: + + .. code-block:: jinja + + {% set json_string = salt.slsutil.serialize('json', + {'foo': 'Foo!'}) %} + """ + kwargs = salt.utils.args.clean_kwargs(**mod_kwargs) + return _get_serialize_fn(serializer, "serialize")(obj, **kwargs) + + +def deserialize(serializer, stream_or_string, **mod_kwargs): + """ + Deserialize a Python object using one of the available + :ref:`all-salt.serializers`. + + CLI Example: + + .. code-block:: bash + + salt '*' slsutil.deserialize 'json' '{"foo": "Foo!"}' + salt '*' --no-parse=stream_or_string slsutil.deserialize 'json' \\ + stream_or_string='{"foo": "Foo!"}' + + Jinja Example: + + .. code-block:: jinja + + {% set python_object = salt.slsutil.deserialize('json', + '{"foo": "Foo!"}') %} + """ + kwargs = salt.utils.args.clean_kwargs(**mod_kwargs) + return _get_serialize_fn(serializer, "deserialize")(stream_or_string, **kwargs) + + +def boolstr(value, true="true", false="false"): + """ + Convert a boolean value into a string. This function is + intended to be used from within file templates to provide + an easy way to take boolean values stored in Pillars or + Grains, and write them out in the appropriate syntax for + a particular file template. + + :param value: The boolean value to be converted + :param true: The value to return if ``value`` is ``True`` + :param false: The value to return if ``value`` is ``False`` + + In this example, a pillar named ``smtp:encrypted`` stores a boolean + value, but the template that uses that value needs ``yes`` or ``no`` + to be written, based on the boolean value. + + *Note: this is written on two lines for clarity. The same result + could be achieved in one line.* + + .. code-block:: jinja + + {% set encrypted = salt[pillar.get]('smtp:encrypted', false) %} + use_tls: {{ salt['slsutil.boolstr'](encrypted, 'yes', 'no') }} + + Result (assuming the value is ``True``): + + .. code-block:: none + + use_tls: yes + + """ + + if value: + return true + + return false + + +def _set_context(keys, function, fun_args=None, fun_kwargs=None, force=False): + """ + Convenience function to set a value in the ``__context__`` dictionary. + + .. versionadded:: 3004 + + :param keys: The list of keys specifying the dictionary path to set. This + list can be of arbitrary length and the path will be created + in the dictionary if it does not exist. + + :param function: A python function to be called if the specified path does + not exist, if the force parameter is ``True``. + + :param fun_args: A list of positional arguments to the function. + + :param fun_kwargs: A dictionary of keyword arguments to the function. + + :param force: If ``True``, force the ```__context__`` path to be updated. + Otherwise, only create it if it does not exist. + """ + + target = __context__ + + # Build each level of the dictionary as needed + for key in keys[:-1]: + if key not in target: + target[key] = {} + target = target[key] + + # Call the supplied function to populate the dictionary + if force or keys[-1] not in target: + if not fun_args: + fun_args = [] + + if not fun_kwargs: + fun_kwargs = {} + + target[keys[-1]] = function(*fun_args, *fun_kwargs) + + +def file_exists(path, saltenv="base"): + """ + Return ``True`` if a file exists in the state tree, ``False`` otherwise. + + .. versionadded:: 3004 + + :param str path: The fully qualified path to a file in the state tree. + :param str saltenv: The fileserver environment to search. Default: ``base`` + + CLI Example: + + .. code-block:: bash + + salt '*' slsutil.file_exists nginx/defaults.yaml + """ + + _set_context( + [CONTEXT_BASE, saltenv, "file_list"], __salt__["cp.list_master"], [saltenv] + ) + return path in __context__[CONTEXT_BASE][saltenv]["file_list"] + + +def dir_exists(path, saltenv="base"): + """ + Return ``True`` if a directory exists in the state tree, ``False`` otherwise. + + :param str path: The fully qualified path to a directory in the state tree. + :param str saltenv: The fileserver environment to search. Default: ``base`` + + .. versionadded:: 3004 + + CLI Example: + + .. code-block:: bash + + salt '*' slsutil.dir_exists nginx/files + """ + + _set_context( + [CONTEXT_BASE, saltenv, "dir_list"], __salt__["cp.list_master_dirs"], [saltenv] + ) + return path in __context__[CONTEXT_BASE][saltenv]["dir_list"] + + +def path_exists(path, saltenv="base"): + """ + Return ``True`` if a path exists in the state tree, ``False`` otherwise. The path + could refer to a file or directory. + + .. versionadded:: 3004 + + :param str path: The fully qualified path to a file or directory in the state tree. + :param str saltenv: The fileserver environment to search. Default: ``base`` + + CLI Example: + + .. code-block:: bash + + salt '*' slsutil.path_exists nginx/defaults.yaml + """ + + return file_exists(path, saltenv) or dir_exists(path, saltenv) + + +def findup(startpath, filenames, saltenv="base"): + """ + Find the first path matching a filename or list of filenames in a specified + directory or the nearest ancestor directory. Returns the full path to the + first file found. + + .. versionadded:: 3004 + + :param str startpath: The fileserver path from which to begin the search. + An empty string refers to the state tree root. + :param filenames: A filename or list of filenames to search for. Searching for + directory names is also supported. + :param str saltenv: The fileserver environment to search. Default: ``base`` + + Example: return the path to ``defaults.yaml``, walking up the tree from the + state file currently being processed. + + .. code-block:: jinja + + {{ salt["slsutil.findup"](tplfile, "defaults.yaml") }} + + CLI Example: + + .. code-block:: bash + + salt '*' slsutil.findup formulas/shared/nginx map.jinja + """ + + # Normalize the path + if startpath: + startpath = posixpath.normpath(startpath) + + # Verify the cwd is a valid path in the state tree + if startpath and not path_exists(startpath, saltenv): + raise salt.exceptions.SaltInvocationError( + "Starting path not found in the state tree: {}".format(startpath) + ) + + # Ensure that patterns is a string or list of strings + if isinstance(filenames, str): + filenames = [filenames] + if not isinstance(filenames, list): + raise salt.exceptions.SaltInvocationError( + "Filenames argument must be a string or list of strings" + ) + + while True: + + # Loop over filenames, looking for one at the current path level + for filename in filenames: + fullname = salt.utils.path.join( + startpath or "", filename, use_posixpath=True + ) + if path_exists(fullname, saltenv): + return fullname + + # If the root path was just checked, raise an error + if not startpath: + raise salt.exceptions.CommandExecutionError( + "File pattern(s) not found in path ancestry" + ) + + # Move up one level in the ancestry + startpath = os.path.dirname(startpath) diff --git a/tests/pytests/integration/ssh/test_slsutil.py b/tests/pytests/integration/ssh/test_slsutil.py new file mode 100644 index 000000000000..4ac9ed59a825 --- /dev/null +++ b/tests/pytests/integration/ssh/test_slsutil.py @@ -0,0 +1,94 @@ +import json + +import pytest + + +@pytest.mark.usefixtures("state_tree") +def test_renderer_file(salt_ssh_cli): + ret = salt_ssh_cli.run("slsutil.renderer", "salt://test.sls") + assert ret.returncode == 0 + assert isinstance(ret.data, dict) + assert "Ok with def" in ret.data + + +def test_renderer_string(salt_ssh_cli): + rend = "{{ salt['test.echo']('foo') }}: {{ pillar['ext_spam'] }}" + ret = salt_ssh_cli.run("slsutil.renderer", string=rend) + assert ret.returncode == 0 + assert isinstance(ret.data, dict) + assert ret.data == {"foo": "eggs"} + + +def test_serialize(salt_ssh_cli): + obj = {"foo": "bar"} + ret = salt_ssh_cli.run("slsutil.serialize", "json", obj) + assert ret.returncode == 0 + assert isinstance(ret.data, str) + assert ret.data == json.dumps(obj) + + +def test_deserialize(salt_ssh_cli): + obj = {"foo": "bar"} + data = json.dumps(obj) + # Need to quote it, otherwise it's deserialized by the + # test wrapper + ret = salt_ssh_cli.run("slsutil.deserialize", "json", f"'{data}'") + assert ret.returncode == 0 + assert isinstance(ret.data, type(obj)) + assert ret.data == obj + + +@pytest.mark.parametrize( + "path,expected", + [ + ("test_deep", True), + ("test_deep/test.sls", False), + ("test_deep/b/2", True), + ("does_not/ex/ist", False), + ], +) +def test_dir_exists(salt_ssh_cli, path, expected): + ret = salt_ssh_cli.run("slsutil.dir_exists", path) + assert ret.returncode == 0 + assert isinstance(ret.data, bool) + assert ret.data is expected + + +@pytest.mark.parametrize( + "path,expected", [("test_deep", False), ("test_deep/test.sls", True)] +) +def test_file_exists(salt_ssh_cli, path, expected): + ret = salt_ssh_cli.run("slsutil.file_exists", path) + assert ret.returncode == 0 + assert isinstance(ret.data, bool) + assert ret.data is expected + + +@pytest.mark.parametrize( + "start,name,expected", + [ + ("test_deep/b/2", "test.sls", "test_deep/b/2/test.sls"), + ("test_deep/b/2", "cheese", "cheese"), + ], +) +def test_findup(salt_ssh_cli, start, name, expected): + ret = salt_ssh_cli.run("slsutil.findup", start, name) + assert ret.returncode == 0 + assert isinstance(ret.data, str) + assert ret.data == expected + + +@pytest.mark.parametrize( + "path,expected", + [ + ("test_deep", True), + ("test_deep/test.sls", True), + ("test_deep/b/2", True), + ("does_not/ex/ist", False), + ], +) +def test_path_exists(salt_ssh_cli, path, expected): + ret = salt_ssh_cli.run("slsutil.path_exists", path) + assert ret.returncode == 0 + assert isinstance(ret.data, bool) + assert ret.data is expected diff --git a/tests/pytests/unit/client/ssh/wrapper/test_slsutil.py b/tests/pytests/unit/client/ssh/wrapper/test_slsutil.py new file mode 100644 index 000000000000..558d5ee74dcf --- /dev/null +++ b/tests/pytests/unit/client/ssh/wrapper/test_slsutil.py @@ -0,0 +1,166 @@ +import contextlib +import logging + +import pytest + +import salt.client.ssh.wrapper.slsutil as slsutil +from salt.exceptions import CommandExecutionError, SaltInvocationError +from tests.support.mock import MagicMock + +log = logging.getLogger(__name__) + + +# --- These tests are adapted from tests.pytests.unit.utils.slsutil + + +@pytest.fixture +def configure_loader_modules(master_dirs, master_files): + return { + slsutil: { + "__salt__": { + "cp.list_master": MagicMock(return_value=master_files), + "cp.list_master_dirs": MagicMock(return_value=master_dirs), + }, + "__opts__": { + "renderer": "jinja|yaml", + "renderer_blacklist": [], + "renderer_whitelist": [], + }, + } + } + + +@pytest.fixture +def master_dirs(): + return ["red", "red/files", "blue", "blue/files"] + + +@pytest.fixture +def master_files(): + return [ + "top.sls", + "red/init.sls", + "red/files/default.conf", + "blue/init.sls", + "blue/files/default.conf", + ] + + +@pytest.mark.parametrize("inpt,expected", ((True, "yes"), (False, "no"))) +def test_boolstr(inpt, expected): + assert slsutil.boolstr(inpt, true="yes", false="no") == expected + + +@pytest.mark.parametrize( + "inpt,expected", (("red/init.sls", True), ("green/init.sls", False)) +) +def test_file_exists(inpt, expected): + assert slsutil.file_exists(inpt) is expected + + +@pytest.mark.parametrize("inpt,expected", (("red", True), ("green", False))) +def test_dir_exists(inpt, expected): + assert slsutil.dir_exists(inpt) is expected + + +@pytest.mark.parametrize( + "inpt,expected", + ( + ("red", True), + ("green", False), + ("red/init.sls", True), + ("green/init.sls", False), + ), +) +def test_path_exists(inpt, expected): + assert slsutil.path_exists(inpt) is expected + + +@pytest.mark.parametrize( + "inpt,expected,raises", + [ + (("red/files", "init.sls"), "red/init.sls", None), + (("red/files", ["top.sls"]), "top.sls", None), + (("", "top.sls"), "top.sls", None), + ((None, "top.sls"), "top.sls", None), + (("red/files", ["top.sls", "init.sls"]), "red/init.sls", None), + ( + ("red/files", "notfound"), + None, + pytest.raises( + CommandExecutionError, match=r"File pattern\(s\) not found.*" + ), + ), + ( + ("red", "default.conf"), + None, + pytest.raises( + CommandExecutionError, match=r"File pattern\(s\) not found.*" + ), + ), + ( + ("green", "notfound"), + None, + pytest.raises(SaltInvocationError, match="Starting path not found.*"), + ), + ( + ("red", 1234), + None, + pytest.raises( + SaltInvocationError, match=".*must be a string or list of strings.*" + ), + ), + ], +) +def test_findup(inpt, expected, raises): + if raises is None: + raises = contextlib.nullcontext() + with raises: + res = slsutil.findup(*inpt) + assert res == expected + + +@pytest.mark.parametrize( + "a,b,merge_lists,expected", + [ + ( + {"foo": {"bar": "baz", "hi": "there", "some": ["list"]}}, + {"foo": {"baz": "quux", "bar": "hi", "some": ["other_list"]}}, + False, + { + "foo": { + "baz": "quux", + "bar": "hi", + "hi": "there", + "some": ["other_list"], + } + }, + ), + ( + {"foo": {"bar": "baz", "hi": "there", "some": ["list"]}}, + {"foo": {"baz": "quux", "bar": "hi", "some": ["other_list"]}}, + True, + { + "foo": { + "baz": "quux", + "bar": "hi", + "hi": "there", + "some": ["list", "other_list"], + } + }, + ), + ], +) +@pytest.mark.parametrize("func", ("update", "merge", "merge_all")) +def test_update_merge(a, b, merge_lists, expected, func): + arg = (a, b) + if func == "merge_all": + arg = ([a, b],) + res = getattr(slsutil, func)(*arg, merge_lists=merge_lists) + assert res == expected + assert (a is res) is (func == "update") + + +def test_renderer_requires_either_path_or_string(): + with pytest.raises(SaltInvocationError, match=".*either path or string.*"): + slsutil.renderer() From 63e0b5b0ec5b04f33d94e530eb6e20c9c6b86324 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Tue, 7 Nov 2023 00:02:30 +0100 Subject: [PATCH 2/7] Add `defaults` SSH wrapper module This is a 1:1 copy of the execution module, incl. tests... (cherry picked from commit 47a609fab058105109159536d94b577d452155a0) --- changelog/51605.fixed.md | 1 + salt/client/ssh/wrapper/defaults.py | 240 ++++++++++++++++++ .../unit/client/ssh/wrapper/test_defaults.py | 215 ++++++++++++++++ 3 files changed, 456 insertions(+) create mode 100644 changelog/51605.fixed.md create mode 100644 salt/client/ssh/wrapper/defaults.py create mode 100644 tests/pytests/unit/client/ssh/wrapper/test_defaults.py diff --git a/changelog/51605.fixed.md b/changelog/51605.fixed.md new file mode 100644 index 000000000000..990b34413d95 --- /dev/null +++ b/changelog/51605.fixed.md @@ -0,0 +1 @@ +Fixed defaults.merge is not available when using salt-ssh diff --git a/salt/client/ssh/wrapper/defaults.py b/salt/client/ssh/wrapper/defaults.py new file mode 100644 index 000000000000..d03990b87980 --- /dev/null +++ b/salt/client/ssh/wrapper/defaults.py @@ -0,0 +1,240 @@ +""" +SSH wrapper module to work with salt formula defaults files + +""" + +import copy +import logging +import os + +import salt.fileclient +import salt.utils.data +import salt.utils.dictupdate as dictupdate +import salt.utils.files +import salt.utils.json +import salt.utils.url +import salt.utils.yaml + +__virtualname__ = "defaults" + +log = logging.getLogger(__name__) + + +def _mk_client(): + """ + Create a file client and add it to the context + """ + return salt.fileclient.get_file_client(__opts__) + + +def _load(formula): + """ + Generates a list of salt:///defaults.(json|yaml) files + and fetches them from the Salt master. + + Returns first defaults file as python dict. + """ + + # Compute possibilities + paths = [] + for ext in ("yaml", "json"): + source_url = salt.utils.url.create(formula + "/defaults." + ext) + paths.append(source_url) + # Fetch files from master + with _mk_client() as client: + defaults_files = client.cache_files(paths) + + for file_ in defaults_files: + if not file_: + # Skip empty string returned by cp.fileclient.cache_files. + continue + + suffix = file_.rsplit(".", 1)[-1] + if suffix == "yaml": + loader = salt.utils.yaml.safe_load + elif suffix == "json": + loader = salt.utils.json.load + else: + log.debug("Failed to determine loader for %r", file_) + continue + + if os.path.exists(file_): + log.debug("Reading defaults from %r", file_) + with salt.utils.files.fopen(file_) as fhr: + defaults = loader(fhr) + log.debug("Read defaults %r", defaults) + + return defaults or {} + + +def get(key, default=""): + """ + defaults.get is used much like pillar.get except that it will read + a default value for a pillar from defaults.json or defaults.yaml + files that are stored in the root of a salt formula. + + CLI Example: + + .. code-block:: bash + + salt '*' defaults.get core:users:root + + The defaults is computed from pillar key. The first entry is considered as + the formula namespace. + + For example, querying ``core:users:root`` will try to load + ``salt://core/defaults.yaml`` and ``salt://core/defaults.json``. + """ + + # Determine formula namespace from query + if ":" in key: + namespace, key = key.split(":", 1) + else: + namespace, key = key, None + + # Fetch and load defaults formula files from states. + defaults = _load(namespace) + + # Fetch value + if key: + return salt.utils.data.traverse_dict_and_list(defaults, key, default) + else: + return defaults + + +def merge(dest, src, merge_lists=False, in_place=True, convert_none=True): + """ + defaults.merge + Allows deep merging of dicts in formulas. + + merge_lists : False + If True, it will also merge lists instead of replace their items. + + in_place : True + If True, it will merge into dest dict, + if not it will make a new copy from that dict and return it. + + convert_none : True + If True, it will convert src and dest to empty dicts if they are None. + If True and dest is None but in_place is True, raises TypeError. + If False it will make a new copy from that dict and return it. + + .. versionadded:: 3005 + + CLI Example: + + .. code-block:: bash + + salt '*' defaults.merge '{a: b}' '{d: e}' + + It is more typical to use this in a templating language in formulas, + instead of directly on the command-line. + """ + # Force empty dicts if applicable (useful for cleaner templating) + src = {} if (src is None and convert_none) else src + if dest is None and convert_none: + if in_place: + raise TypeError("Can't perform in-place merge into NoneType") + else: + dest = {} + + if in_place: + merged = dest + else: + merged = copy.deepcopy(dest) + return dictupdate.update(merged, src, merge_lists=merge_lists) + + +def deepcopy(source): + """ + defaults.deepcopy + Allows deep copy of objects in formulas. + + By default, Python does not copy objects, + it creates bindings between a target and an object. + + It is more typical to use this in a templating language in formulas, + instead of directly on the command-line. + """ + return copy.deepcopy(source) + + +def update(dest, defaults, merge_lists=True, in_place=True, convert_none=True): + """ + defaults.update + Allows setting defaults for group of data set e.g. group for nodes. + + This function is a combination of defaults.merge + and defaults.deepcopy to avoid redundant in jinja. + + Example: + + .. code-block:: yaml + + group01: + defaults: + enabled: True + extra: + - test + - stage + nodes: + host01: + index: foo + upstream: bar + host02: + index: foo2 + upstream: bar2 + + .. code-block:: jinja + + {% do salt['defaults.update'](group01.nodes, group01.defaults) %} + + Each node will look like the following: + + .. code-block:: yaml + + host01: + enabled: True + index: foo + upstream: bar + extra: + - test + - stage + + merge_lists : True + If True, it will also merge lists instead of replace their items. + + in_place : True + If True, it will merge into dest dict. + if not it will make a new copy from that dict and return it. + + convert_none : True + If True, it will convert src and dest to empty dicts if they are None. + If True and dest is None but in_place is True, raises TypeError. + If False it will make a new copy from that dict and return it. + + .. versionadded:: 3005 + + It is more typical to use this in a templating language in formulas, + instead of directly on the command-line. + """ + # Force empty dicts if applicable here + if in_place: + if dest is None: + raise TypeError("Can't perform in-place update into NoneType") + else: + nodes = dest + else: + dest = {} if (dest is None and convert_none) else dest + nodes = deepcopy(dest) + + defaults = {} if (defaults is None and convert_none) else defaults + + for node_name, node_vars in nodes.items(): + defaults_vars = deepcopy(defaults) + node_vars = merge( + defaults_vars, node_vars, merge_lists=merge_lists, convert_none=convert_none + ) + nodes[node_name] = node_vars + + return nodes diff --git a/tests/pytests/unit/client/ssh/wrapper/test_defaults.py b/tests/pytests/unit/client/ssh/wrapper/test_defaults.py new file mode 100644 index 000000000000..12d07bc2a854 --- /dev/null +++ b/tests/pytests/unit/client/ssh/wrapper/test_defaults.py @@ -0,0 +1,215 @@ +""" +Test cases for salt.client.ssh.wrapper.defaults + +This has been copied 1:1 from tests.pytests.unit.modules.test_defaults +""" + +import inspect + +import pytest + +import salt.client.ssh.wrapper.defaults as defaults +from tests.support.mock import MagicMock, patch + + +@pytest.fixture() +def configure_loader_modules(): + return {defaults: {}} + + +def test_get_mock(): + """ + Test if it execute a defaults client run and return a dict + """ + with patch.object(inspect, "stack", MagicMock(return_value=[])), patch( + "salt.client.ssh.wrapper.defaults.get", + MagicMock(return_value={"users": {"root": [0]}}), + ): + assert defaults.get("core:users:root") == {"users": {"root": [0]}} + + +def test_merge_with_list_merging(): + """ + Test deep merging of dicts with merge_lists enabled. + """ + + src_dict = { + "string_key": "string_val_src", + "list_key": ["list_val_src"], + "dict_key": {"dict_key_src": "dict_val_src"}, + } + + dest_dict = { + "string_key": "string_val_dest", + "list_key": ["list_val_dest"], + "dict_key": {"dict_key_dest": "dict_val_dest"}, + } + + merged_dict = { + "string_key": "string_val_src", + "list_key": ["list_val_dest", "list_val_src"], + "dict_key": { + "dict_key_dest": "dict_val_dest", + "dict_key_src": "dict_val_src", + }, + } + + defaults.merge(dest_dict, src_dict, merge_lists=True) + assert dest_dict == merged_dict + + +def test_merge_without_list_merging(): + """ + Test deep merging of dicts with merge_lists disabled. + """ + + src = { + "string_key": "string_val_src", + "list_key": ["list_val_src"], + "dict_key": {"dict_key_src": "dict_val_src"}, + } + + dest = { + "string_key": "string_val_dest", + "list_key": ["list_val_dest"], + "dict_key": {"dict_key_dest": "dict_val_dest"}, + } + + merged = { + "string_key": "string_val_src", + "list_key": ["list_val_src"], + "dict_key": { + "dict_key_dest": "dict_val_dest", + "dict_key_src": "dict_val_src", + }, + } + + defaults.merge(dest, src, merge_lists=False) + assert dest == merged + + +def test_merge_not_in_place(): + """ + Test deep merging of dicts not in place. + """ + + src = {"nested_dict": {"A": "A"}} + + dest = {"nested_dict": {"B": "B"}} + + dest_orig = {"nested_dict": {"B": "B"}} + + merged = {"nested_dict": {"A": "A", "B": "B"}} + + final = defaults.merge(dest, src, in_place=False) + assert dest == dest_orig + assert final == merged + + +def test_merge_src_is_none(): + """ + Test deep merging of dicts not in place. + """ + + dest = {"nested_dict": {"B": "B"}} + + dest_orig = {"nested_dict": {"B": "B"}} + + final = defaults.merge(dest, None, in_place=False) + assert dest == dest_orig + assert final == dest_orig + + +def test_merge_dest_is_none(): + """ + Test deep merging of dicts not in place. + """ + + src = {"nested_dict": {"B": "B"}} + + src_orig = {"nested_dict": {"B": "B"}} + + final = defaults.merge(None, src, in_place=False) + assert src == src_orig + assert final == src_orig + + +def test_merge_in_place_dest_is_none(): + """ + Test deep merging of dicts not in place. + """ + + src = {"nested_dict": {"B": "B"}} + + pytest.raises(TypeError, defaults.merge, None, src) + + +def test_deepcopy(): + """ + Test a deep copy of object. + """ + + src = {"A": "A", "B": "B"} + + dist = defaults.deepcopy(src) + dist.update({"C": "C"}) + + result = {"A": "A", "B": "B", "C": "C"} + + assert src != dist + assert dist == result + + +def test_update_in_place(): + """ + Test update with defaults values in place. + """ + + group01 = { + "defaults": {"enabled": True, "extra": ["test", "stage"]}, + "nodes": {"host01": {"index": "foo", "upstream": "bar"}}, + } + + host01 = { + "enabled": True, + "index": "foo", + "upstream": "bar", + "extra": ["test", "stage"], + } + + defaults.update(group01["nodes"], group01["defaults"]) + assert group01["nodes"]["host01"] == host01 + + +def test_update_with_defaults_none(): + group01 = { + "defaults": {"enabled": True, "extra": ["test", "stage"]}, + "nodes": {"host01": {"index": "foo", "upstream": "bar"}}, + } + + host01 = { + "index": "foo", + "upstream": "bar", + } + + defaults.update(group01["nodes"], None) + assert group01["nodes"]["host01"] == host01 + + +def test_update_with_dest_none(): + group01 = { + "defaults": {"enabled": True, "extra": ["test", "stage"]}, + "nodes": {"host01": {"index": "foo", "upstream": "bar"}}, + } + + ret = defaults.update(None, group01["defaults"], in_place=False) + assert ret == {} + + +def test_update_in_place_with_dest_none(): + group01 = { + "defaults": {"enabled": True, "extra": ["test", "stage"]}, + "nodes": {"host01": {"index": "foo", "upstream": "bar"}}, + } + + pytest.raises(TypeError, defaults.update, None, group01["defaults"]) From 8ddc648b3be6e22281a1d3934b46350c14ac083f Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 6 Nov 2023 12:42:55 +0100 Subject: [PATCH 3/7] Sync config SSH wrapper with execution module The wrapper has diverged significantly from the module. * `option` did not check grains * `option` did not have `omit_all` and `wildcard` parameters * `get` missed several parameters: `delimiter`, `merge` and all `omit_*` * There was no wrapping function for `items`. (cherry picked from commit 8356be888bf32e2f4b081c54a6a56b21d5cf833c) --- changelog/56441.fixed.md | 1 + salt/client/ssh/wrapper/config.py | 377 +++++++++++++++--- tests/pytests/integration/ssh/test_config.py | 66 +++ .../unit/client/ssh/wrapper/test_config.py | 219 ++++++++++ 4 files changed, 614 insertions(+), 49 deletions(-) create mode 100644 changelog/56441.fixed.md create mode 100644 tests/pytests/integration/ssh/test_config.py create mode 100644 tests/pytests/unit/client/ssh/wrapper/test_config.py diff --git a/changelog/56441.fixed.md b/changelog/56441.fixed.md new file mode 100644 index 000000000000..489ad80f7709 --- /dev/null +++ b/changelog/56441.fixed.md @@ -0,0 +1 @@ +Fixed config.get does not support merge option with salt-ssh diff --git a/salt/client/ssh/wrapper/config.py b/salt/client/ssh/wrapper/config.py index dcc00ceb2c36..a6db176453c2 100644 --- a/salt/client/ssh/wrapper/config.py +++ b/salt/client/ssh/wrapper/config.py @@ -2,17 +2,22 @@ Return config information """ +import copy +import fnmatch +import logging import os -import re +import urllib.parse import salt.syspaths as syspaths import salt.utils.data import salt.utils.files +import salt.utils.sdb as sdb + +log = logging.getLogger(__name__) # Set up the default values for all systems DEFAULTS = { "mongo.db": "salt", - "mongo.host": "salt", "mongo.password": "", "mongo.port": 27017, "mongo.user": "", @@ -38,9 +43,12 @@ "solr.num_backups": 1, "poudriere.config": "/usr/local/etc/poudriere.conf", "poudriere.config_dir": "/usr/local/etc/poudriere.d", + "ldap.uri": "", "ldap.server": "localhost", "ldap.port": "389", "ldap.tls": False, + "ldap.no_verify": False, + "ldap.anonymous": True, "ldap.scope": 2, "ldap.attrs": None, "ldap.binddn": "", @@ -51,6 +59,11 @@ "tunnel": False, "images": os.path.join(syspaths.SRV_ROOT_DIR, "salt-images"), }, + "docker.exec_driver": "docker-exec", + "docker.compare_container_networks": { + "static": ["Aliases", "Links", "IPAMConfig"], + "automatic": ["IPAddress", "Gateway", "GlobalIPv6Address", "IPv6Gateway"], + }, } @@ -96,15 +109,66 @@ def valid_fileproto(uri): salt '*' config.valid_fileproto salt://path/to/file """ - try: - return bool(re.match("^(?:salt|https?|ftp)://", uri)) - except Exception: # pylint: disable=broad-except - return False + return urllib.parse.urlparse(uri).scheme in salt.utils.files.VALID_PROTOS + + +def option( + value, + default=None, + omit_opts=False, + omit_grains=False, + omit_pillar=False, + omit_master=False, + omit_all=False, + wildcard=False, +): + """ + Returns the setting for the specified config value. The priority for + matches is the same as in :py:func:`config.get `, + only this function does not recurse into nested data structures. Another + difference between this function and :py:func:`config.get + ` is that it comes with a set of "sane defaults". + To view these, you can run the following command: + .. code-block:: bash -def option(value, default="", omit_opts=False, omit_master=False, omit_pillar=False): - """ - Pass in a generic option and receive the value that will be assigned + salt '*' config.option '*' omit_all=True wildcard=True + + default + The default value if no match is found. If not specified, then the + fallback default will be an empty string, unless ``wildcard=True``, in + which case the return will be an empty dictionary. + + omit_opts : False + Pass as ``True`` to exclude matches from the minion configuration file + + omit_grains : False + Pass as ``True`` to exclude matches from the grains + + omit_pillar : False + Pass as ``True`` to exclude matches from the pillar data + + omit_master : False + Pass as ``True`` to exclude matches from the master configuration file + + omit_all : True + Shorthand to omit all of the above and return matches only from the + "sane defaults". + + .. versionadded:: 3000 + + wildcard : False + If used, this will perform pattern matching on keys. Note that this + will also significantly change the return data. Instead of only a value + being returned, a dictionary mapping the matched keys to their values + is returned. For example, using ``wildcard=True`` with a ``key`` of + ``'foo.ba*`` could return a dictionary like so: + + .. code-block:: python + + {'foo.bar': True, 'foo.baz': False} + + .. versionadded:: 3000 CLI Example: @@ -112,18 +176,48 @@ def option(value, default="", omit_opts=False, omit_master=False, omit_pillar=Fa salt '*' config.option redis.host """ - if not omit_opts: - if value in __opts__: - return __opts__[value] - if not omit_master: - if value in __pillar__.get("master", {}): - return __pillar__["master"][value] - if not omit_pillar: - if value in __pillar__: - return __pillar__[value] - if value in DEFAULTS: - return DEFAULTS[value] - return default + if omit_all: + omit_opts = omit_grains = omit_pillar = omit_master = True + + if default is None: + default = "" if not wildcard else {} + + if not wildcard: + if not omit_opts: + if value in __opts__: + return __opts__[value] + if not omit_grains: + if value in __grains__: + return __grains__[value] + if not omit_pillar: + if value in __pillar__: + return __pillar__[value] + if not omit_master: + if value in __pillar__.get("master", {}): + return __pillar__["master"][value] + if value in DEFAULTS: + return DEFAULTS[value] + + # No match + return default + else: + # We need to do the checks in the reverse order so that minion opts + # takes precedence + ret = {} + for omit, data in ( + (omit_master, __pillar__.get("master", {})), + (omit_pillar, __pillar__), + (omit_grains, __grains__), + (omit_opts, __opts__), + ): + if not omit: + ret.update({x: data[x] for x in fnmatch.filter(data, value)}) + # Check the DEFAULTS as well to see if the pattern matches it + for item in (x for x in fnmatch.filter(DEFAULTS, value) if x not in ret): + ret[item] = DEFAULTS[item] + + # If no matches, return the default + return ret or default def merge(value, default="", omit_opts=False, omit_master=False, omit_pillar=False): @@ -171,54 +265,223 @@ def merge(value, default="", omit_opts=False, omit_master=False, omit_pillar=Fal ret = list(ret) + list(tmp) if ret is None and value in DEFAULTS: return DEFAULTS[value] - return ret or default + if ret is None: + return default + return ret -def get(key, default=""): +def get( + key, + default="", + delimiter=":", + merge=None, + omit_opts=False, + omit_pillar=False, + omit_master=False, + omit_grains=False, +): """ .. versionadded:: 0.14.0 - Attempt to retrieve the named value from opts, pillar, grains of the master - config, if the named value is not available return the passed default. - The default return is an empty string. + Attempt to retrieve the named value from the minion config file, pillar, + grains or the master config. If the named value is not available, return + the value specified by the ``default`` argument. If this argument is not + specified, ``default`` falls back to an empty string. - The value can also represent a value in a nested dict using a ":" delimiter - for the dict. This means that if a dict looks like this:: + Values can also be retrieved from nested dictionaries. Assume the below + data structure: + + .. code-block:: python {'pkg': {'apache': 'httpd'}} - To retrieve the value associated with the apache key in the pkg dict this - key can be passed:: + To retrieve the value associated with the ``apache`` key, in the + sub-dictionary corresponding to the ``pkg`` key, the following command can + be used: + + .. code-block:: bash + + salt myminion config.get pkg:apache + + The ``:`` (colon) is used to represent a nested dictionary level. - pkg:apache + .. versionchanged:: 2015.5.0 + The ``delimiter`` argument was added, to allow delimiters other than + ``:`` to be used. - This routine traverses these data stores in this order: + This function traverses these data stores in this order, returning the + first match found: - - Local minion config (opts) + - Minion configuration - Minion's grains - - Minion's pillar - - Master config + - Minion's pillar data + - Master configuration (requires :conf_minion:`pillar_opts` to be set to + ``True`` in Minion config file in order to work) + + This means that if there is a value that is going to be the same for the + majority of minions, it can be configured in the Master config file, and + then overridden using the grains, pillar, or Minion config file. + + Adding config options to the Master or Minion configuration file is easy: + + .. code-block:: yaml + + my-config-option: value + cafe-menu: + - egg and bacon + - egg sausage and bacon + - egg and spam + - egg bacon and spam + - egg bacon sausage and spam + - spam bacon sausage and spam + - spam egg spam spam bacon and spam + - spam sausage spam spam bacon spam tomato and spam + + .. note:: + Minion configuration options built into Salt (like those defined + :ref:`here `) will *always* be defined in + the Minion configuration and thus *cannot be overridden by grains or + pillar data*. However, additional (user-defined) configuration options + (as in the above example) will not be in the Minion configuration by + default and thus can be overridden using grains/pillar data by leaving + the option out of the minion config file. + + **Arguments** + + delimiter + .. versionadded:: 2015.5.0 + + Override the delimiter used to separate nested levels of a data + structure. + + merge + .. versionadded:: 2015.5.0 + + If passed, this parameter will change the behavior of the function so + that, instead of traversing each data store above in order and + returning the first match, the data stores are first merged together + and then searched. The pillar data is merged into the master config + data, then the grains are merged, followed by the Minion config data. + The resulting data structure is then searched for a match. This allows + for configurations to be more flexible. + + .. note:: + + The merging described above does not mean that grain data will end + up in the Minion's pillar data, or pillar data will end up in the + master config data, etc. The data is just combined for the purposes + of searching an amalgam of the different data stores. + + The supported merge strategies are as follows: + + - **recurse** - If a key exists in both dictionaries, and the new value + is not a dictionary, it is replaced. Otherwise, the sub-dictionaries + are merged together into a single dictionary, recursively on down, + following the same criteria. For example: + + .. code-block:: python + + >>> dict1 = {'foo': {'bar': 1, 'qux': True}, + 'hosts': ['a', 'b', 'c'], + 'only_x': None} + >>> dict2 = {'foo': {'baz': 2, 'qux': False}, + 'hosts': ['d', 'e', 'f'], + 'only_y': None} + >>> merged + {'foo': {'bar': 1, 'baz': 2, 'qux': False}, + 'hosts': ['d', 'e', 'f'], + 'only_dict1': None, + 'only_dict2': None} + + - **overwrite** - If a key exists in the top level of both + dictionaries, the new value completely overwrites the old. For + example: + + .. code-block:: python + + >>> dict1 = {'foo': {'bar': 1, 'qux': True}, + 'hosts': ['a', 'b', 'c'], + 'only_x': None} + >>> dict2 = {'foo': {'baz': 2, 'qux': False}, + 'hosts': ['d', 'e', 'f'], + 'only_y': None} + >>> merged + {'foo': {'baz': 2, 'qux': False}, + 'hosts': ['d', 'e', 'f'], + 'only_dict1': None, + 'only_dict2': None} CLI Example: .. code-block:: bash salt '*' config.get pkg:apache + salt '*' config.get lxc.container_profile:centos merge=recurse """ - ret = salt.utils.data.traverse_dict_and_list(__opts__, key, "_|-") - if ret != "_|-": - return ret - ret = salt.utils.data.traverse_dict_and_list(__grains__, key, "_|-") - if ret != "_|-": - return ret - ret = salt.utils.data.traverse_dict_and_list(__pillar__, key, "_|-") - if ret != "_|-": - return ret - ret = salt.utils.data.traverse_dict_and_list( - __pillar__.get("master", {}), key, "_|-" - ) - if ret != "_|-": - return ret + if merge is None: + if not omit_opts: + ret = salt.utils.data.traverse_dict_and_list( + __opts__, key, "_|-", delimiter=delimiter + ) + if ret != "_|-": + return sdb.sdb_get(ret, __opts__) + + if not omit_grains: + ret = salt.utils.data.traverse_dict_and_list( + __grains__, key, "_|-", delimiter + ) + if ret != "_|-": + return sdb.sdb_get(ret, __opts__) + + if not omit_pillar: + ret = salt.utils.data.traverse_dict_and_list( + __pillar__, key, "_|-", delimiter=delimiter + ) + if ret != "_|-": + return sdb.sdb_get(ret, __opts__) + + if not omit_master: + ret = salt.utils.data.traverse_dict_and_list( + __pillar__.get("master", {}), key, "_|-", delimiter=delimiter + ) + if ret != "_|-": + return sdb.sdb_get(ret, __opts__) + + ret = salt.utils.data.traverse_dict_and_list( + DEFAULTS, key, "_|-", delimiter=delimiter + ) + if ret != "_|-": + return sdb.sdb_get(ret, __opts__) + else: + if merge not in ("recurse", "overwrite"): + log.warning( + "Unsupported merge strategy '%s'. Falling back to 'recurse'.", merge + ) + merge = "recurse" + + merge_lists = salt.config.master_config("/etc/salt/master").get( + "pillar_merge_lists" + ) + + data = copy.copy(DEFAULTS) + data = salt.utils.dictupdate.merge( + data, __pillar__.get("master", {}), strategy=merge, merge_lists=merge_lists + ) + data = salt.utils.dictupdate.merge( + data, __pillar__, strategy=merge, merge_lists=merge_lists + ) + data = salt.utils.dictupdate.merge( + data, __grains__, strategy=merge, merge_lists=merge_lists + ) + data = salt.utils.dictupdate.merge( + data, __opts__, strategy=merge, merge_lists=merge_lists + ) + ret = salt.utils.data.traverse_dict_and_list( + data, key, "_|-", delimiter=delimiter + ) + if ret != "_|-": + return sdb.sdb_get(ret, __opts__) + return default @@ -241,3 +504,19 @@ def dot_vals(value): if key.startswith(f"{value}."): ret[key] = val return ret + + +def items(): + """ + Return the complete config from the currently running minion process. + This includes defaults for values not set in the config file. + + CLI Example: + + .. code-block:: bash + + salt '*' config.items + """ + # This would otherwise be parsed as just the value of "local" in opts. + # In case the wfunc parsing is improved, this can be removed. + return {"local": {"return": __opts__.copy()}} diff --git a/tests/pytests/integration/ssh/test_config.py b/tests/pytests/integration/ssh/test_config.py new file mode 100644 index 000000000000..d3ae2b03a3e4 --- /dev/null +++ b/tests/pytests/integration/ssh/test_config.py @@ -0,0 +1,66 @@ +import pytest + +pytestmark = [pytest.mark.slow_test] + + +def test_items(salt_ssh_cli): + ret = salt_ssh_cli.run("config.items") + assert ret.returncode == 0 + assert isinstance(ret.data, dict) + assert "id" in ret.data + assert "grains" in ret.data + assert "__master_opts__" in ret.data + assert "cachedir" in ret.data + + +@pytest.mark.parametrize("omit", (False, True)) +def test_option_minion_opt(salt_ssh_cli, omit): + # Minion opt + ret = salt_ssh_cli.run("config.option", "id", omit_opts=omit, omit_grains=True) + assert ret.returncode == 0 + assert (ret.data != salt_ssh_cli.get_minion_tgt()) is omit + assert (ret.data == "") is omit + + +@pytest.mark.parametrize("omit", (False, True)) +def test_option_pillar(salt_ssh_cli, omit): + ret = salt_ssh_cli.run("config.option", "ext_spam", omit_pillar=omit) + assert ret.returncode == 0 + assert (ret.data != "eggs") is omit + assert (ret.data == "") is omit + + +@pytest.mark.parametrize("omit", (False, True)) +def test_option_grain(salt_ssh_cli, omit): + ret = salt_ssh_cli.run("config.option", "kernel", omit_grains=omit) + assert ret.returncode == 0 + assert ( + ret.data not in ("Darwin", "Linux", "FreeBSD", "OpenBSD", "Windows") + ) is omit + assert (ret.data == "") is omit + + +@pytest.mark.parametrize("omit", (False, True)) +def test_get_minion_opt(salt_ssh_cli, omit): + ret = salt_ssh_cli.run("config.get", "cachedir", omit_master=True, omit_opts=omit) + assert ret.returncode == 0 + assert (ret.data == "") is omit + assert ("minion" not in ret.data) is omit + + +@pytest.mark.parametrize("omit", (False, True)) +def test_get_pillar(salt_ssh_cli, omit): + ret = salt_ssh_cli.run("config.get", "ext_spam", omit_pillar=omit) + assert ret.returncode == 0 + assert (ret.data != "eggs") is omit + assert (ret.data == "") is omit + + +@pytest.mark.parametrize("omit", (False, True)) +def test_get_grain(salt_ssh_cli, omit): + ret = salt_ssh_cli.run("config.get", "kernel", omit_grains=omit) + assert ret.returncode == 0 + assert ( + ret.data not in ("Darwin", "Linux", "FreeBSD", "OpenBSD", "Windows") + ) is omit + assert (ret.data == "") is omit diff --git a/tests/pytests/unit/client/ssh/wrapper/test_config.py b/tests/pytests/unit/client/ssh/wrapper/test_config.py new file mode 100644 index 000000000000..64e89c762ad2 --- /dev/null +++ b/tests/pytests/unit/client/ssh/wrapper/test_config.py @@ -0,0 +1,219 @@ +""" + Taken 1:1 from test cases for salt.modules.config + This tests the SSH wrapper module. +""" + + +import fnmatch + +import pytest + +import salt.client.ssh.wrapper.config as config +from tests.support.mock import patch + + +@pytest.fixture +def defaults(): + return { + "test.option.foo": "value of test.option.foo in defaults", + "test.option.bar": "value of test.option.bar in defaults", + "test.option.baz": "value of test.option.baz in defaults", + "test.option": "value of test.option in defaults", + } + + +@pytest.fixture +def no_match(): + return "test.option.nope" + + +@pytest.fixture +def opt_name(): + return "test.option.foo" + + +@pytest.fixture +def wildcard_opt_name(): + return "test.option.b*" + + +@pytest.fixture +def configure_loader_modules(): + return { + config: { + "__opts__": { + "test.option.foo": "value of test.option.foo in __opts__", + "test.option.bar": "value of test.option.bar in __opts__", + "test.option.baz": "value of test.option.baz in __opts__", + }, + "__pillar__": { + "test.option.foo": "value of test.option.foo in __pillar__", + "test.option.bar": "value of test.option.bar in __pillar__", + "test.option.baz": "value of test.option.baz in __pillar__", + "master": { + "test.option.foo": "value of test.option.foo in master", + "test.option.bar": "value of test.option.bar in master", + "test.option.baz": "value of test.option.baz in master", + }, + }, + "__grains__": { + "test.option.foo": "value of test.option.foo in __grains__", + "test.option.bar": "value of test.option.bar in __grains__", + "test.option.baz": "value of test.option.baz in __grains__", + }, + } + } + + +def _wildcard_match(data, wildcard_opt_name): + return {x: data[x] for x in fnmatch.filter(data, wildcard_opt_name)} + + +def test_defaults_only_name(defaults): + with patch.dict(config.DEFAULTS, defaults): + opt_name = "test.option" + opt = config.option(opt_name) + assert opt == config.DEFAULTS[opt_name] + + +def test_no_match(defaults, no_match, wildcard_opt_name): + """ + Make sure that the defa + """ + with patch.dict(config.DEFAULTS, defaults): + ret = config.option(no_match) + assert ret == "", ret + + default = "wat" + ret = config.option(no_match, default=default) + assert ret == default, ret + + ret = config.option(no_match, wildcard=True) + assert ret == {}, ret + + default = {"foo": "bar"} + ret = config.option(no_match, default=default, wildcard=True) + assert ret == default, ret + + # Should be no match since wildcard=False + ret = config.option(wildcard_opt_name) + assert ret == "", ret + + +def test_omits(defaults, opt_name, wildcard_opt_name): + with patch.dict(config.DEFAULTS, defaults): + + # ********** OMIT NOTHING ********** + + # Match should be in __opts__ dict + ret = config.option(opt_name) + assert ret == config.__opts__[opt_name], ret + + # Wildcard match + ret = config.option(wildcard_opt_name, wildcard=True) + assert ret == _wildcard_match(config.__opts__, wildcard_opt_name), ret + + # ********** OMIT __opts__ ********** + + # Match should be in __grains__ dict + ret = config.option(opt_name, omit_opts=True) + assert ret == config.__grains__[opt_name], ret + + # Wildcard match + ret = config.option(wildcard_opt_name, omit_opts=True, wildcard=True) + assert ret == _wildcard_match(config.__grains__, wildcard_opt_name), ret + + # ********** OMIT __opts__, __grains__ ********** + + # Match should be in __pillar__ dict + ret = config.option(opt_name, omit_opts=True, omit_grains=True) + assert ret == config.__pillar__[opt_name], ret + + # Wildcard match + ret = config.option( + wildcard_opt_name, omit_opts=True, omit_grains=True, wildcard=True + ) + assert ret == _wildcard_match(config.__pillar__, wildcard_opt_name), ret + + # ********** OMIT __opts__, __grains__, __pillar__ ********** + + # Match should be in master opts + ret = config.option( + opt_name, omit_opts=True, omit_grains=True, omit_pillar=True + ) + assert ret == config.__pillar__["master"][opt_name], ret + + # Wildcard match + ret = config.option( + wildcard_opt_name, + omit_opts=True, + omit_grains=True, + omit_pillar=True, + wildcard=True, + ) + assert ret == _wildcard_match( + config.__pillar__["master"], wildcard_opt_name + ), ret + + # ********** OMIT ALL THE THINGS ********** + + # Match should be in master opts + ret = config.option( + opt_name, + omit_opts=True, + omit_grains=True, + omit_pillar=True, + omit_master=True, + ) + assert ret == config.DEFAULTS[opt_name], ret + + # Wildcard match + ret = config.option( + wildcard_opt_name, + omit_opts=True, + omit_grains=True, + omit_pillar=True, + omit_master=True, + wildcard=True, + ) + assert ret == _wildcard_match(config.DEFAULTS, wildcard_opt_name), ret + + # Match should be in master opts + ret = config.option(opt_name, omit_all=True) + assert ret == config.DEFAULTS[opt_name], ret + + # Wildcard match + ret = config.option(wildcard_opt_name, omit_all=True, wildcard=True) + assert ret == _wildcard_match(config.DEFAULTS, wildcard_opt_name), ret + + +# --- Additional tests not found in the execution module tests + + +@pytest.mark.parametrize("backup", ("", "minion", "master", "both")) +def test_backup_mode(backup): + res = config.backup_mode(backup) + assert res == backup or "minion" + + +@pytest.mark.parametrize( + "uri,expected", + (("salt://my/foo.txt", True), ("mysql://foo:bar@foo.bar/baz", False)), +) +def test_valid_fileproto(uri, expected): + res = config.valid_fileproto(uri) + assert res is expected + + +def test_dot_vals(): + extra_master_opt = ("test.option.baah", "value of test.option.baah in master") + with patch.dict(config.__pillar__, {"master": dict((extra_master_opt,))}): + res = config.dot_vals("test") + assert isinstance(res, dict) + assert res + for var in ("foo", "bar", "baz"): + key = f"test.option.{var}" + assert key in res + assert res[key] == f"value of test.option.{var} in __opts__" + assert extra_master_opt[0] in res + assert res[extra_master_opt[0]] == extra_master_opt[1] From 0fdcbfd274c86847038f8df0fe5ac1ed714f1d05 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 6 Nov 2023 12:37:23 +0100 Subject: [PATCH 4/7] Add state.test to SSH wrapper (cherry picked from commit 82f90e2f15cb93fd7094a04b1493f74450b39d0c) --- changelog/61100.fixed.md | 1 + salt/client/ssh/wrapper/state.py | 15 ++++ tests/pytests/integration/ssh/conftest.py | 68 +++++++++++++++++++ .../integration/ssh/state/test_state.py | 10 +++ 4 files changed, 94 insertions(+) create mode 100644 changelog/61100.fixed.md diff --git a/changelog/61100.fixed.md b/changelog/61100.fixed.md new file mode 100644 index 000000000000..d7ac2b6bc3f6 --- /dev/null +++ b/changelog/61100.fixed.md @@ -0,0 +1 @@ +Fixed state.test does not work with salt-ssh diff --git a/salt/client/ssh/wrapper/state.py b/salt/client/ssh/wrapper/state.py index aa61e07f81e8..ece4ee92c3a0 100644 --- a/salt/client/ssh/wrapper/state.py +++ b/salt/client/ssh/wrapper/state.py @@ -1317,3 +1317,18 @@ def single(fun, name, test=None, **kwargs): # If for some reason the json load fails, return the stdout return stdout + + +def test(*args, **kwargs): + """ + .. versionadded:: 3001 + + Alias for `state.apply` with the kwarg `test` forced to `True`. + + This is a nicety to avoid the need to type out `test=True` and the possibility of + a typo causing changes you do not intend. + """ + kwargs["test"] = True + ret = apply_(*args, **kwargs) + + return ret diff --git a/tests/pytests/integration/ssh/conftest.py b/tests/pytests/integration/ssh/conftest.py index 7c1ceeba7ca3..b0028efee174 100644 --- a/tests/pytests/integration/ssh/conftest.py +++ b/tests/pytests/integration/ssh/conftest.py @@ -25,3 +25,71 @@ def _reap_stray_processes(): with reap_stray_processes(): # Run test yield + + +@pytest.fixture(scope="module") +def state_tree(base_env_state_tree_root_dir): + top_file = """ + {%- from "map.jinja" import abc with context %} + base: + 'localhost': + - basic + '127.0.0.1': + - basic + """ + map_file = """ + {%- set abc = "def" %} + """ + state_file = """ + {%- from "map.jinja" import abc with context %} + Ok with {{ abc }}: + test.succeed_with_changes + """ + top_tempfile = pytest.helpers.temp_file( + "top.sls", top_file, base_env_state_tree_root_dir + ) + map_tempfile = pytest.helpers.temp_file( + "map.jinja", map_file, base_env_state_tree_root_dir + ) + state_tempfile = pytest.helpers.temp_file( + "test.sls", state_file, base_env_state_tree_root_dir + ) + with top_tempfile, map_tempfile, state_tempfile: + yield + + +@pytest.fixture(scope="module") +def state_tree_dir(base_env_state_tree_root_dir): + """ + State tree with files to test salt-ssh + when the map.jinja file is in another directory + """ + top_file = """ + {%- from "test/map.jinja" import abc with context %} + base: + 'localhost': + - test + '127.0.0.1': + - test + """ + map_file = """ + {%- set abc = "def" %} + """ + state_file = """ + {%- from "test/map.jinja" import abc with context %} + + Ok with {{ abc }}: + test.succeed_without_changes + """ + top_tempfile = pytest.helpers.temp_file( + "top.sls", top_file, base_env_state_tree_root_dir + ) + map_tempfile = pytest.helpers.temp_file( + "test/map.jinja", map_file, base_env_state_tree_root_dir + ) + state_tempfile = pytest.helpers.temp_file( + "test.sls", state_file, base_env_state_tree_root_dir + ) + + with top_tempfile, map_tempfile, state_tempfile: + yield diff --git a/tests/pytests/integration/ssh/state/test_state.py b/tests/pytests/integration/ssh/state/test_state.py index 62e8cbf513b1..a7ebb22a601b 100644 --- a/tests/pytests/integration/ssh/state/test_state.py +++ b/tests/pytests/integration/ssh/state/test_state.py @@ -101,3 +101,13 @@ def test_state_high(salt_ssh_cli): """ ret = salt_ssh_cli.run("state.high", '{"echo blah": {"cmd": ["run"]}}') assert ret.data["cmd_|-echo blah_|-echo blah_|-run"]["changes"]["stdout"] == "blah" + + +def test_state_test(salt_ssh_cli, state_tree): + ret = salt_ssh_cli.run("state.test", "test") + assert ret.returncode == 0 + assert ret.data + assert ( + ret.data["test_|-Ok with def_|-Ok with def_|-succeed_with_changes"]["result"] + is None + ) From e60fa42ee78735a53073f48036990022f5a4b828 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Tue, 28 Nov 2023 21:46:14 +0100 Subject: [PATCH 5/7] Add logmod SSH wrapper (cherry picked from commit 18bc40c77af75c491d2009ad851e86287fc3f6dd) --- changelog/65630.fixed.md | 1 + salt/client/ssh/wrapper/logmod.py | 79 +++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) create mode 100644 changelog/65630.fixed.md create mode 100644 salt/client/ssh/wrapper/logmod.py diff --git a/changelog/65630.fixed.md b/changelog/65630.fixed.md new file mode 100644 index 000000000000..e8650abcdc1c --- /dev/null +++ b/changelog/65630.fixed.md @@ -0,0 +1 @@ +Added SSH wrapper for logmod diff --git a/salt/client/ssh/wrapper/logmod.py b/salt/client/ssh/wrapper/logmod.py new file mode 100644 index 000000000000..911fd7a1d4c7 --- /dev/null +++ b/salt/client/ssh/wrapper/logmod.py @@ -0,0 +1,79 @@ +""" +On-demand logging +================= + +.. versionadded:: 2017.7.0 + +The sole purpose of this module is logging messages in the minion. +It comes very handy when debugging complex Jinja templates, for example: + +.. code-block:: jinja + + {%- for var in range(10) %} + {%- do salt["log.info"](var) -%} + {%- endfor %} + +CLI Example: + +.. code-block:: bash + + salt '*' log.error "Please don't do that, this module is not for CLI use!" +""" + +import logging + +log = logging.getLogger(__name__) + +__virtualname__ = "log" + + +def __virtual__(): + return __virtualname__ + + +def debug(message): + """ + Log message at level DEBUG. + """ + log.debug(message) + return True + + +def info(message): + """ + Log message at level INFO. + """ + log.info(message) + return True + + +def warning(message): + """ + Log message at level WARNING. + """ + log.warning(message) + return True + + +def error(message): + """ + Log message at level ERROR. + """ + log.error(message) + return True + + +def critical(message): + """ + Log message at level CRITICAL. + """ + log.critical(message) + return True + + +def exception(message): + """ + Log message at level EXCEPTION. + """ + log.exception(message) + return True From 76fae4365dd085fc26b493fc5251af949182ca5b Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 6 Nov 2023 15:27:17 +0100 Subject: [PATCH 6/7] Fix unused var in `grains.get` wrapper `ordered=False` would not have worked before (cherry picked from commit 5e16d8483458d4f58dee2d968a781c8e006633ee) --- salt/client/ssh/wrapper/grains.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/salt/client/ssh/wrapper/grains.py b/salt/client/ssh/wrapper/grains.py index 400131e15171..70d2c7e72dab 100644 --- a/salt/client/ssh/wrapper/grains.py +++ b/salt/client/ssh/wrapper/grains.py @@ -72,9 +72,7 @@ def get(key, default="", delimiter=DEFAULT_TARGET_DELIM, ordered=True): grains = __grains__.value() else: grains = salt.utils.json.loads(salt.utils.json.dumps(__grains__.value())) - return salt.utils.data.traverse_dict_and_list( - __grains__.value(), key, default, delimiter - ) + return salt.utils.data.traverse_dict_and_list(grains, key, default, delimiter) def has_value(key): From ffd8cf23aeb589c151905e81f6080fac2667a188 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Mon, 24 Jun 2024 09:02:26 +0200 Subject: [PATCH 7/7] Fix state.test test, run pre-commit The test fix was part of 6a715107fa89b060175ef0c0ae0827b409411543 --- salt/client/ssh/wrapper/slsutil.py | 8 ++++---- tests/pytests/integration/ssh/state/conftest.py | 2 +- tests/pytests/integration/ssh/test_config.py | 4 +++- tests/pytests/unit/client/ssh/wrapper/test_config.py | 1 - 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/salt/client/ssh/wrapper/slsutil.py b/salt/client/ssh/wrapper/slsutil.py index e09ca1c29845..586d09ad2d61 100644 --- a/salt/client/ssh/wrapper/slsutil.py +++ b/salt/client/ssh/wrapper/slsutil.py @@ -173,7 +173,7 @@ def run(): default_renderer, __opts__["renderer_blacklist"], __opts__["renderer_whitelist"], - **kwargs + **kwargs, ) return ret.read() if salt.utils.stringio.is_readable(ret) else ret @@ -185,12 +185,12 @@ def _get_serialize_fn(serializer, fn_name): if not fns: raise salt.exceptions.CommandExecutionError( - "Serializer '{}' not found.".format(serializer) + f"Serializer '{serializer}' not found." ) if not fn: raise salt.exceptions.CommandExecutionError( - "Serializer '{}' does not implement {}.".format(serializer, fn_name) + f"Serializer '{serializer}' does not implement {fn_name}." ) return fn @@ -419,7 +419,7 @@ def findup(startpath, filenames, saltenv="base"): # Verify the cwd is a valid path in the state tree if startpath and not path_exists(startpath, saltenv): raise salt.exceptions.SaltInvocationError( - "Starting path not found in the state tree: {}".format(startpath) + f"Starting path not found in the state tree: {startpath}" ) # Ensure that patterns is a string or list of strings diff --git a/tests/pytests/integration/ssh/state/conftest.py b/tests/pytests/integration/ssh/state/conftest.py index 14d645ae8e8a..9de0d6bcad81 100644 --- a/tests/pytests/integration/ssh/state/conftest.py +++ b/tests/pytests/integration/ssh/state/conftest.py @@ -17,7 +17,7 @@ def state_tree(base_env_state_tree_root_dir): state_file = """ {%- from "map.jinja" import abc with context %} Ok with {{ abc }}: - test.succeed_without_changes + test.succeed_with_changes """ top_tempfile = pytest.helpers.temp_file( "top.sls", top_file, base_env_state_tree_root_dir diff --git a/tests/pytests/integration/ssh/test_config.py b/tests/pytests/integration/ssh/test_config.py index d3ae2b03a3e4..7f38ec5a0a87 100644 --- a/tests/pytests/integration/ssh/test_config.py +++ b/tests/pytests/integration/ssh/test_config.py @@ -16,7 +16,9 @@ def test_items(salt_ssh_cli): @pytest.mark.parametrize("omit", (False, True)) def test_option_minion_opt(salt_ssh_cli, omit): # Minion opt - ret = salt_ssh_cli.run("config.option", "id", omit_opts=omit, omit_grains=True) + ret = salt_ssh_cli.run( + "config.option", "id", omit_opts=omit, omit_grains=True, omit_master=True + ) assert ret.returncode == 0 assert (ret.data != salt_ssh_cli.get_minion_tgt()) is omit assert (ret.data == "") is omit diff --git a/tests/pytests/unit/client/ssh/wrapper/test_config.py b/tests/pytests/unit/client/ssh/wrapper/test_config.py index 64e89c762ad2..a708b925fdfc 100644 --- a/tests/pytests/unit/client/ssh/wrapper/test_config.py +++ b/tests/pytests/unit/client/ssh/wrapper/test_config.py @@ -3,7 +3,6 @@ This tests the SSH wrapper module. """ - import fnmatch import pytest