diff --git a/changelog/43287.added b/changelog/43287.added new file mode 100644 index 000000000000..90b47e65d443 --- /dev/null +++ b/changelog/43287.added @@ -0,0 +1 @@ +Added pillar templating to vault policies diff --git a/salt/modules/vault.py b/salt/modules/vault.py index 3f0064bd787a..fed044640d57 100644 --- a/salt/modules/vault.py +++ b/salt/modules/vault.py @@ -130,6 +130,12 @@ values, eg ``my-policies/{grains[os]}``. ``{minion}`` is shorthand for ``grains[id]``, eg ``saltstack/minion/{minion}``. + .. versionadded:: 3006 + + Policies can be templated with pillar values as well: ``salt_role_{pillar[roles]}`` + Make sure to only reference pillars that are not sourced from Vault since the latter + ones might be unavailable during policy rendering. + .. important:: See :ref:`Is Targeting using Grain Data Secure? @@ -160,6 +166,29 @@ Optional. If policies is not configured, ``saltstack/minions`` and ``saltstack/{minion}`` are used as defaults. + policies_refresh_pillar + Whether to refresh the pillar data when rendering templated policies. + When unset (=null/None), will only refresh when the cached data + is unavailable, boolean values force one behavior always. + + .. note:: + + Using cached pillar data only (policies_refresh_pillar=False) + might cause the policies to be out of sync. If there is no cached pillar + data available for the minion, pillar templates will fail to render at all. + + If you use pillar values for templating policies and do not disable + refreshing pillar data, make sure the relevant values are not sourced + from Vault (ext_pillar, sdb) or from a pillar sls file that uses the vault + execution module. Although this will often work when cached pillar data is + available, if the master needs to compile the pillar data during policy rendering, + all Vault modules will be broken to prevent an infinite loop. + + policies_cache_time + Policy computation can be heavy in case pillar data is used in templated policies and + it has not been cached. Therefore, a short-lived cache specifically for rendered policies + is used. This specifies the expiration timeout in seconds. Defaults to 60. + keys List of keys to use to unseal vault server with the vault.unseal runner. diff --git a/salt/pillar/vault.py b/salt/pillar/vault.py index 3eb2d0271ddf..f632c74a519f 100644 --- a/salt/pillar/vault.py +++ b/salt/pillar/vault.py @@ -52,6 +52,7 @@ - vault: path=secret/salt - vault: path=secret/root - vault: path=secret/minions/{minion}/pass + - vault: path=secret/roles/{pillar[roles]}/pass You can also use nesting here as well. Identical nesting keys will get merged. @@ -118,6 +119,31 @@ minion-passwd: minionbadpasswd1 +.. versionadded:: 3006 + + Pillar values from previously rendered pillars can be used to template + vault ext_pillar paths. + +Using pillar values to template vault pillar paths requires them to be defined +before the vault ext_pillar is called. Especially consider the significancy +of :conf_master:`ext_pillar_first ` master config setting. + +If a pillar pattern matches multiple paths, the results are merged according to +the master configuration values :conf_master:`pillar_source_merging_strategy ` +and :conf_master:`pillar_merge_lists ` by default. + +If the optional nesting_key was defined, the merged result will be nested below. +There is currently no way to nest multiple results under different keys. + +You can override the merging behavior per defined ext_pillar: + +.. code-block:: yaml + + ext_pillar: + - vault: + conf: path=secret/roles/{pillar[roles]} + merge_strategy: smart + merge_lists: false """ @@ -125,6 +151,8 @@ from requests.exceptions import HTTPError +import salt.utils.dictupdate + log = logging.getLogger(__name__) @@ -140,10 +168,18 @@ def ext_pillar( pillar, # pylint: disable=W0613 conf, nesting_key=None, + merge_strategy=None, + merge_lists=None, + extra_minion_data=None, ): """ Get pillar data from Vault for the configuration ``conf``. """ + extra_minion_data = extra_minion_data or {} + if extra_minion_data.get("_vault_runner_is_compiling_pillar_templates"): + # Disable vault ext_pillar while compiling pillar for vault policy templates + return {} + comps = conf.split() paths = [comp for comp in comps if comp.startswith("path=")] @@ -151,25 +187,55 @@ def ext_pillar( log.error('"%s" is not a valid Vault ext_pillar config', conf) return {} + merge_strategy = merge_strategy or __opts__.get( + "pillar_source_merging_strategy", "smart" + ) + merge_lists = merge_lists or __opts__.get("pillar_merge_lists", False) vault_pillar = {} - try: - path = paths[0].replace("path=", "") - path = path.format(**{"minion": minion_id}) - version2 = __utils__["vault.is_v2"](path) - if version2["v2"]: - path = version2["data"] - - url = "v1/{}".format(path) - response = __utils__["vault.make_request"]("GET", url) - response.raise_for_status() - vault_pillar = response.json().get("data", {}) - - if vault_pillar and version2["v2"]: - vault_pillar = vault_pillar["data"] - except HTTPError: - log.info("Vault secret not found for: %s", path) + path_pattern = paths[0].replace("path=", "") + for path in _get_paths(path_pattern, minion_id, pillar): + try: + version2 = __utils__["vault.is_v2"](path) + if version2["v2"]: + path = version2["data"] + + url = "v1/{}".format(path) + response = __utils__["vault.make_request"]("GET", url) + response.raise_for_status() + vault_pillar_single = response.json().get("data", {}) + + if vault_pillar_single and version2["v2"]: + vault_pillar_single = vault_pillar_single["data"] + + vault_pillar = salt.utils.dictupdate.merge( + vault_pillar, + vault_pillar_single, + strategy=merge_strategy, + merge_lists=merge_lists, + ) + except HTTPError: + log.info("Vault secret not found for: %s", path) if nesting_key: vault_pillar = {nesting_key: vault_pillar} return vault_pillar + + +def _get_paths(path_pattern, minion_id, pillar): + """ + Get the paths that should be merged into the pillar dict + """ + mappings = {"minion": minion_id, "pillar": pillar} + + paths = [] + try: + for expanded_pattern in __utils__["vault.expand_pattern_lists"]( + path_pattern, **mappings + ): + paths.append(expanded_pattern.format(**mappings)) + except KeyError: + log.warning("Could not resolve pillar path pattern %s", path_pattern) + + log.debug(f"{minion_id} vault pillar paths: {paths}") + return paths diff --git a/salt/runners/vault.py b/salt/runners/vault.py index 78c404726016..f7c5ce37f102 100644 --- a/salt/runners/vault.py +++ b/salt/runners/vault.py @@ -8,15 +8,20 @@ """ import base64 +import copy import json import logging -import string import time +from collections.abc import Mapping import requests +import salt.cache import salt.crypt import salt.exceptions +import salt.pillar +from salt.defaults import NOT_SET +from salt.exceptions import SaltRunnerError log = logging.getLogger(__name__) @@ -63,6 +68,8 @@ def generate_token( if not allow_minion_override or ttl is None: ttl = config["auth"].get("ttl", None) storage_type = config["auth"].get("token_backend", "session") + policies_refresh_pillar = config.get("policies_refresh_pillar", None) + policies_cache_time = config.get("policies_cache_time", 60) if config["auth"]["method"] == "approle": if _selftoken_expired(): @@ -93,7 +100,12 @@ def generate_token( "saltstack-user": globals().get("__user__", ""), } payload = { - "policies": _get_policies(minion_id, config), + "policies": _get_policies_cached( + minion_id, + config, + refresh_pillar=policies_refresh_pillar, + expire=policies_cache_time, + ), "num_uses": uses, "meta": audit_data, } @@ -161,12 +173,24 @@ def unseal(): return False -def show_policies(minion_id): +def show_policies(minion_id, refresh_pillar=NOT_SET, expire=None): """ - Show the Vault policies that are applied to tokens for the given minion + Show the Vault policies that are applied to tokens for the given minion. minion_id - The minions id + The minion's id. + + refresh_pillar + Whether to refresh the pillar data when rendering templated policies. + None will only refresh when the cached data is unavailable, boolean values + force one behavior always. + Defaults to config value ``policies_refresh_pillar`` or None. + + expire + Policy computation can be heavy in case pillar data is used in templated policies and + it has not been cached. Therefore, a short-lived cache specifically for rendered policies + is used. This specifies the expiration timeout in seconds. + Defaults to config value ``policies_cache_time`` or 60. CLI Example: @@ -174,8 +198,13 @@ def show_policies(minion_id): salt-run vault.show_policies myminion """ - config = __opts__["vault"] - return _get_policies(minion_id, config) + config = __opts__.get("vault", {}) + if refresh_pillar == NOT_SET: + refresh_pillar = config.get("policies_refresh_pillar") + expire = expire if expire is not None else config.get("policies_cache_time", 60) + return _get_policies_cached( + minion_id, config, refresh_pillar=refresh_pillar, expire=expire + ) def _validate_signature(minion_id, signature, impersonated_by_master): @@ -198,78 +227,108 @@ def _validate_signature(minion_id, signature, impersonated_by_master): log.trace("Signature ok") -def _get_policies(minion_id, config): +# **kwargs because salt.cache.Cache does not pop "expire" from kwargs +def _get_policies( + minion_id, config, refresh_pillar=None, **kwargs +): # pylint: disable=unused-argument """ Get the policies that should be applied to a token for minion_id """ - _, grains, _ = salt.utils.minions.get_minion_data(minion_id, __opts__) + grains, pillar = _get_minion_data(minion_id, refresh_pillar) policy_patterns = config.get( "policies", ["saltstack/minion/{minion}", "saltstack/minions"] ) - mappings = {"minion": minion_id, "grains": grains or {}} + mappings = {"minion": minion_id, "grains": grains, "pillar": pillar} policies = [] for pattern in policy_patterns: try: - for expanded_pattern in _expand_pattern_lists(pattern, **mappings): + for expanded_pattern in __utils__["vault.expand_pattern_lists"]( + pattern, **mappings + ): policies.append( expanded_pattern.format(**mappings).lower() # Vault requirement ) except KeyError: - log.warning("Could not resolve policy pattern %s", pattern) + log.warning( + "Could not resolve policy pattern %s for minion %s", pattern, minion_id + ) log.debug("%s policies: %s", minion_id, policies) return policies -def _expand_pattern_lists(pattern, **mappings): - """ - Expands the pattern for any list-valued mappings, such that for any list of - length N in the mappings present in the pattern, N copies of the pattern are - returned, each with an element of the list substituted. - - pattern: - A pattern to expand, for example ``by-role/{grains[roles]}`` - - mappings: - A dictionary of variables that can be expanded into the pattern. - - Example: Given the pattern `` by-role/{grains[roles]}`` and the below grains - - .. code-block:: yaml +def _get_policies_cached(minion_id, config, refresh_pillar=None, expire=60): + # expiration of 0 disables cache + if not expire: + return _get_policies(minion_id, config, refresh_pillar=refresh_pillar) + cbank = f"minions/{minion_id}/vault" + ckey = "policies" + cache = salt.cache.factory(__opts__) + policies = cache.cache( + cbank, + ckey, + _get_policies, + expire=expire, + minion_id=minion_id, + config=config, + refresh_pillar=refresh_pillar, + ) + if not isinstance(policies, list): + log.warning("Cached vault policies were not formed as a list. Refreshing.") + cache.flush(cbank, ckey) + policies = cache.cache( + cbank, + ckey, + _get_policies, + expire=expire, + minion_id=minion_id, + config=config, + refresh_pillar=refresh_pillar, + ) + return policies - grains: - roles: - - web - - database - This function will expand into two patterns, - ``[by-role/web, by-role/database]``. +def _get_minion_data(minion_id, refresh_pillar=None): + _, grains, pillar = salt.utils.minions.get_minion_data(minion_id, __opts__) + + if grains is None: + # In case no cached minion data is available, make sure the utils module + # can distinguish a pillar refresh run impersonating a minion from running + # on the master. + grains = {"id": minion_id} + # To properly refresh minion grains, something like this could be used: + # __salt__["salt.execute"](minion_id, "saltutil.refresh_grains", refresh_pillar=False) + # This is deliberately not done since grains should not be used to target + # secrets anyways. + + # salt.utils.minions.get_minion_data only returns data from cache or None. + # To make sure the correct policies are available, the pillar needs to be + # refreshed. This can cause an infinite loop if the pillar data itself + # depends on the vault execution module, which relies on this function. + # By default, only refresh when necessary. Boolean values force one way. + if refresh_pillar is True or (refresh_pillar is None and pillar is None): + if __opts__.get("_vault_runner_is_compiling_pillar_templates"): + raise SaltRunnerError( + "Cyclic dependency detected while refreshing pillar for vault policy templating. " + "This is caused by some pillar value relying on the vault execution module. " + "Either remove the dependency from your pillar, disable refreshing pillar data " + "for policy templating or do not use pillar values in policy templates." + ) + local_opts = copy.deepcopy(__opts__) + # Relying on opts for ext_pillars does not work properly (only the first one runs + # correctly). + extra_minion_data = {"_vault_runner_is_compiling_pillar_templates": True} + local_opts.update(extra_minion_data) + pillar = LazyPillar( + local_opts, grains, minion_id, extra_minion_data=extra_minion_data + ) + elif pillar is None: + # Make sure pillar is a dict. Necessary because a check on LazyPillar would + # refresh it unconditionally (even when no pillar values are used) + pillar = {} - Note that this method does not expand any non-list patterns. - """ - expanded_patterns = [] - f = string.Formatter() - - # This function uses a string.Formatter to get all the formatting tokens from - # the pattern, then recursively replaces tokens whose expanded value is a - # list. For a list with N items, it will create N new pattern strings and - # then continue with the next token. In practice this is expected to not be - # very expensive, since patterns will typically involve a handful of lists at - # most. - - for (_, field_name, _, _) in f.parse(pattern): - if field_name is None: - continue - (value, _) = f.get_field(field_name, None, mappings) - if isinstance(value, list): - token = "{{{0}}}".format(field_name) - expanded = [pattern.replace(token, str(elem)) for elem in value] - for expanded_item in expanded: - result = _expand_pattern_lists(expanded_item, **mappings) - expanded_patterns += result - return expanded_patterns - return [pattern] + return grains, pillar def _selftoken_expired(): @@ -305,3 +364,41 @@ def _get_token_create_url(config): auth_path = "/v1/auth/token/create" base_url = config["url"] return "/".join(x.strip("/") for x in (base_url, auth_path, role_name) if x) + + +class LazyPillar(Mapping): + """ + Simulates a pillar dictionary. Only compiles the pillar + once an item is requested. + """ + + def __init__(self, opts, grains, minion_id, extra_minion_data=None): + self.opts = opts + self.grains = grains + self.minion_id = minion_id + self.extra_minion_data = extra_minion_data or {} + self._pillar = None + + def _load(self): + log.info("Refreshing pillar for vault templating.") + self._pillar = salt.pillar.get_pillar( + self.opts, + self.grains, + self.minion_id, + extra_minion_data=self.extra_minion_data, + ).compile_pillar() + + def __getitem__(self, key): + if self._pillar is None: + self._load() + return self._pillar[key] + + def __iter__(self): + if self._pillar is None: + self._load() + yield from self._pillar + + def __len__(self): + if self._pillar is None: + self._load() + return len(self._pillar) diff --git a/salt/utils/vault.py b/salt/utils/vault.py index b480b5e3b152..e33f36f7cd89 100644 --- a/salt/utils/vault.py +++ b/salt/utils/vault.py @@ -10,6 +10,7 @@ import base64 import logging import os +import string import tempfile import time @@ -555,3 +556,53 @@ def _get_secret_path_metadata(path): except Exception as err: # pylint: disable=broad-except log.error("Failed to get secret metadata %s: %s", type(err).__name__, err) return ret + + +def expand_pattern_lists(pattern, **mappings): + """ + Expands the pattern for any list-valued mappings, such that for any list of + length N in the mappings present in the pattern, N copies of the pattern are + returned, each with an element of the list substituted. + + pattern: + A pattern to expand, for example ``by-role/{grains[roles]}`` + + mappings: + A dictionary of variables that can be expanded into the pattern. + + Example: Given the pattern `` by-role/{grains[roles]}`` and the below grains + + .. code-block:: yaml + + grains: + roles: + - web + - database + + This function will expand into two patterns, + ``[by-role/web, by-role/database]``. + + Note that this method does not expand any non-list patterns. + """ + expanded_patterns = [] + f = string.Formatter() + + # This function uses a string.Formatter to get all the formatting tokens from + # the pattern, then recursively replaces tokens whose expanded value is a + # list. For a list with N items, it will create N new pattern strings and + # then continue with the next token. In practice this is expected to not be + # very expensive, since patterns will typically involve a handful of lists at + # most. + + for (_, field_name, _, _) in f.parse(pattern): + if field_name is None: + continue + (value, _) = f.get_field(field_name, None, mappings) + if isinstance(value, list): + token = "{{{0}}}".format(field_name) + expanded = [pattern.replace(token, str(elem)) for elem in value] + for expanded_item in expanded: + result = expand_pattern_lists(expanded_item, **mappings) + expanded_patterns += result + return expanded_patterns + return [pattern] diff --git a/tests/pytests/integration/runners/test_vault.py b/tests/pytests/integration/runners/test_vault.py new file mode 100644 index 000000000000..5f662d551702 --- /dev/null +++ b/tests/pytests/integration/runners/test_vault.py @@ -0,0 +1,394 @@ +""" +Tests for the Vault runner +""" + +import logging +import shutil + +import pytest + +log = logging.getLogger(__name__) + + +pytestmark = [ + pytest.mark.slow_test, +] + + +@pytest.fixture(scope="class") +def pillar_state_tree(tmp_path_factory): + _pillar_state_tree = tmp_path_factory.mktemp("pillar") + try: + yield _pillar_state_tree + finally: + shutil.rmtree(str(_pillar_state_tree), ignore_errors=True) + + +@pytest.fixture(scope="class") +def pillar_salt_master(salt_factories, pillar_state_tree): + config_defaults = { + "pillar_roots": {"base": [str(pillar_state_tree)]}, + "open_mode": True, + "ext_pillar": [{"vault": "path=does/not/matter"}], + "sdbvault": { + "driver": "vault", + }, + "vault": { + "auth": {"method": "token", "token": "testsecret"}, + "policies": [ + "salt_minion", + "salt_minion_{minion}", + "salt_role_{pillar[roles]}", + "salt_unsafe_{grains[foo]}", + ], + "policies_cache_time": 0, + "url": "http://127.0.0.1:8200", + }, + "minion_data_cache": False, + } + factory = salt_factories.salt_master_daemon( + "vault-policy-int-master-uncached", defaults=config_defaults + ) + with factory.started(): + yield factory + + +@pytest.fixture(scope="class") +def pillar_caching_salt_master(salt_factories, pillar_state_tree): + config_defaults = { + "pillar_roots": {"base": [str(pillar_state_tree)]}, + "open_mode": True, + "vault": { + "auth": {"method": "token", "token": "testsecret"}, + "policies": [ + "salt_minion", + "salt_minion_{minion}", + "salt_role_{pillar[roles]}", + "salt_unsafe_{grains[foo]}", + ], + "policies_cache_time": 0, + "url": "http://127.0.0.1:8200", + }, + "minion_data_cache": True, + } + factory = salt_factories.salt_master_daemon( + "vault-policy-int-master-cached", defaults=config_defaults + ) + with factory.started(): + yield factory + + +@pytest.fixture(scope="class") +def pillar_salt_minion(pillar_salt_master): + assert pillar_salt_master.is_running() + factory = pillar_salt_master.salt_minion_daemon( + "vault-policy-int-minion-uncached-1", + defaults={"open_mode": True, "grains": {"foo": "bar"}}, + ) + with factory.started(): + # Sync All + salt_call_cli = factory.salt_call_cli() + ret = salt_call_cli.run("saltutil.sync_all", _timeout=120) + assert ret.returncode == 0, ret + yield factory + + +@pytest.fixture(scope="class") +def pillar_caching_salt_minion(pillar_caching_salt_master): + assert pillar_caching_salt_master.is_running() + factory = pillar_caching_salt_master.salt_minion_daemon( + "vault-policy-int-minion-cached-1", + defaults={"open_mode": True, "grains": {"foo": "bar"}}, + ) + with factory.started(): + # Sync All + salt_call_cli = factory.salt_call_cli() + ret = salt_call_cli.run("saltutil.sync_all", _timeout=120) + assert ret.returncode == 0, ret + yield factory + + +@pytest.fixture(scope="class") +def pillar_salt_run_cli(pillar_salt_master): + return pillar_salt_master.salt_run_cli() + + +@pytest.fixture(scope="class") +def pillar_caching_salt_run_cli(pillar_caching_salt_master): + return pillar_caching_salt_master.salt_run_cli() + + +@pytest.fixture(scope="class") +def pillar_salt_call_cli(pillar_salt_minion): + return pillar_salt_minion.salt_call_cli() + + +@pytest.fixture(scope="class") +def pillar_caching_salt_call_cli(pillar_caching_salt_minion): + return pillar_caching_salt_minion.salt_call_cli() + + +class TestVaultPillarPolicyTemplatesWithoutCache: + @pytest.fixture(autouse=True) + def pillar_policy_tree( + self, + pillar_salt_master, + pillar_salt_minion, + ): + top_pillar_contents = """ + base: + '{}': + - roles + """.format( + pillar_salt_minion.id + ) + roles_pillar_contents = """ + roles: + - minion + - web + """ + top_file = pillar_salt_master.pillar_tree.base.temp_file( + "top.sls", top_pillar_contents + ) + roles_file = pillar_salt_master.pillar_tree.base.temp_file( + "roles.sls", roles_pillar_contents + ) + + with top_file, roles_file: + yield + + @pytest.fixture() + def pillar_exe_loop(self, pillar_state_tree, pillar_salt_minion): + top_file = """ + base: + '{}': + - roles + - exe_loop + """.format( + pillar_salt_minion.id + ) + exe_loop_pillar = r""" + bar: {{ salt["vault.read_secret"]("does/not/matter") }} + """ + top_tempfile = pytest.helpers.temp_file("top.sls", top_file, pillar_state_tree) + exe_loop_tempfile = pytest.helpers.temp_file( + "exe_loop.sls", exe_loop_pillar, pillar_state_tree + ) + + with top_tempfile, exe_loop_tempfile: + yield + + @pytest.fixture() + def pillar_sdb_loop(self, pillar_state_tree, pillar_salt_minion): + top_file = """ + base: + '{}': + - roles + - sdb_loop + """.format( + pillar_salt_minion.id + ) + sdb_loop_pillar = r""" + foo: {{ salt["sdb.get"]("sdb://sdbvault/does/not/matter/val") }} + """ + top_tempfile = pytest.helpers.temp_file("top.sls", top_file, pillar_state_tree) + sdb_loop_tempfile = pytest.helpers.temp_file( + "sdb_loop.sls", sdb_loop_pillar, pillar_state_tree + ) + + with top_tempfile, sdb_loop_tempfile: + yield + + @pytest.fixture(autouse=True) + def minion_data_cache_absent(self, pillar_salt_run_cli, pillar_salt_minion): + ret = pillar_salt_run_cli.run( + "cache.flush", f"minions/{pillar_salt_minion.id}", "data" + ) + assert ret.returncode == 0 + cached = pillar_salt_run_cli.run( + "cache.fetch", f"minions/{pillar_salt_minion.id}", "data" + ) + assert cached.returncode == 0 + assert not cached.data + yield + + def test_show_policies(self, pillar_salt_run_cli, pillar_salt_minion): + """ + Test that pillar data is refreshed correctly before rendering policies when necessary. + This test includes the prevention of loop exceptions by the ext_pillar module + This refresh does not include grains and pillar data targeted by these grains (unsafe anyways!). + """ + ret = pillar_salt_run_cli.run( + "vault.show_policies", pillar_salt_minion.id, expire=0 + ) + assert ret.data == [ + "salt_minion", + f"salt_minion_{pillar_salt_minion.id}", + "salt_role_minion", + "salt_role_web", + ] + assert "Pillar render error: Failed to load ext_pillar vault" not in ret.stderr + + def test_show_policies_uncached_data_no_pillar_refresh( + self, pillar_salt_run_cli, pillar_salt_minion + ): + """ + Test that the pillar is not refreshed when explicitly disabled + """ + ret = pillar_salt_run_cli.run( + "vault.show_policies", pillar_salt_minion.id, refresh_pillar=False, expire=0 + ) + assert ret.data == ["salt_minion", f"salt_minion_{pillar_salt_minion.id}"] + + def test_policy_compilation_prevents_loop_for_execution_module( + self, + pillar_salt_run_cli, + pillar_salt_minion, + pillar_exe_loop, + ): + """ + Test that the runner prevents a recursive cycle from happening + """ + ret = pillar_salt_run_cli.run( + "vault.show_policies", pillar_salt_minion.id, refresh_pillar=True, expire=0 + ) + assert ret.data == [ + "salt_minion", + f"salt_minion_{pillar_salt_minion.id}", + "salt_role_minion", + "salt_role_web", + ] + assert "Pillar render error: Rendering SLS 'exe_loop' failed" in ret.stderr + assert "Cyclic dependency detected while refreshing pillar" in ret.stderr + + def test_policy_compilation_prevents_loop_for_sdb_module( + self, + pillar_salt_run_cli, + pillar_salt_minion, + pillar_sdb_loop, + ): + """ + Test that the runner prevents a recursive cycle from happening + """ + ret = pillar_salt_run_cli.run( + "vault.show_policies", pillar_salt_minion.id, refresh_pillar=True, expire=0 + ) + assert ret.data == [ + "salt_minion", + f"salt_minion_{pillar_salt_minion.id}", + "salt_role_minion", + "salt_role_web", + ] + assert "Pillar render error: Rendering SLS 'sdb_loop' failed" in ret.stderr + assert "Cyclic dependency detected while refreshing pillar" in ret.stderr + + +class TestVaultPillarPolicyTemplatesWithCache: + @pytest.fixture(autouse=True) + def pillar_caching_policy_tree( + self, pillar_caching_salt_master, pillar_caching_salt_minion + ): + top_pillar_contents = """ + base: + '{}': + - roles + """.format( + pillar_caching_salt_minion.id + ) + roles_pillar_contents = """ + roles: + - minion + - web + """ + top_file = pillar_caching_salt_master.pillar_tree.base.temp_file( + "top.sls", top_pillar_contents + ) + roles_file = pillar_caching_salt_master.pillar_tree.base.temp_file( + "roles.sls", roles_pillar_contents + ) + + with top_file, roles_file: + yield + + @pytest.fixture(autouse=True) + def minion_data_cache_present( + self, + pillar_caching_salt_call_cli, + pillar_caching_policy_tree, + ): + ret = pillar_caching_salt_call_cli.run("saltutil.refresh_pillar", wait=True) + assert ret.returncode == 0 + assert ret.data is True + yield + + @pytest.fixture(autouse=True) + def minion_data_cache_outdated( + self, + minion_data_cache_present, + pillar_caching_salt_run_cli, + pillar_caching_salt_master, + pillar_caching_salt_minion, + ): + roles_pillar_new_contents = """ + roles: + - minion + - web + - fresh + """ + roles_file = pillar_caching_salt_master.pillar_tree.base.temp_file( + "roles.sls", roles_pillar_new_contents + ) + + cached = pillar_caching_salt_run_cli.run( + "cache.fetch", f"minions/{pillar_caching_salt_minion.id}", "data" + ) + assert cached.returncode == 0 + assert cached.data + assert "pillar" in cached.data + assert "grains" in cached.data + assert "roles" in cached.data["pillar"] + assert ["minion", "web"] == cached.data["pillar"]["roles"] + with roles_file: + yield + + def test_show_policies_cached_data_no_pillar_refresh( + self, + pillar_caching_salt_run_cli, + pillar_caching_salt_minion, + ): + """ + Test that pillar data from cache is used when it is available + """ + ret = pillar_caching_salt_run_cli.run( + "vault.show_policies", pillar_caching_salt_minion.id, expire=0 + ) + assert ret.data == [ + "salt_minion", + f"salt_minion_{pillar_caching_salt_minion.id}", + "salt_role_minion", + "salt_role_web", + "salt_unsafe_bar", + ] + + def test_show_policies_refresh_pillar( + self, + pillar_caching_salt_run_cli, + pillar_caching_salt_minion, + ): + """ + Test that pillar data is always refreshed when requested. + """ + ret = pillar_caching_salt_run_cli.run( + "vault.show_policies", + pillar_caching_salt_minion.id, + refresh_pillar=True, + expire=0, + ) + assert ret.data == [ + "salt_minion", + f"salt_minion_{pillar_caching_salt_minion.id}", + "salt_role_minion", + "salt_role_web", + "salt_role_fresh", + "salt_unsafe_bar", + ] diff --git a/tests/pytests/unit/pillar/test_vault.py b/tests/pytests/unit/pillar/test_vault.py index ca4281fe53f8..77f56421c34e 100644 --- a/tests/pytests/unit/pillar/test_vault.py +++ b/tests/pytests/unit/pillar/test_vault.py @@ -1,3 +1,4 @@ +import copy import logging import pytest @@ -9,7 +10,15 @@ @pytest.fixture def configure_loader_modules(): - return {vault: {}} + return { + vault: { + "__utils__": { + "vault.expand_pattern_lists": Mock( + side_effect=lambda x, *args, **kwargs: [x] + ) + } + } + } @pytest.fixture @@ -100,3 +109,107 @@ def test_ext_pillar_nesting_key(is_v2_false, vault_kvv1): assert "baz" in ext_pillar assert "foo" in ext_pillar["baz"] assert ext_pillar["baz"]["foo"] == "bar" + + +@pytest.mark.parametrize( + "pattern,expected", + [ + ("no/template/in/use", ["no/template/in/use"]), + ("salt/minions/{minion}", ["salt/minions/test-minion"]), + ("salt/roles/{pillar[role]}", ["salt/roles/foo"]), + ("salt/roles/{pillar[nonexistent]}", []), + ], +) +def test_get_paths(pattern, expected): + """ + Test that templated paths are resolved as expected. + Expansion of lists is tested in the utility module unit test. + """ + previous_pillar = { + "role": "foo", + } + result = vault._get_paths(pattern, "test-minion", previous_pillar) + assert result == expected + + +def test_ext_pillar_merging(is_v2_false): + """ + Test that patterns that result in multiple paths are merged as expected. + """ + + def make_request(method, resource, *args, **kwargs): + vault_data = { + "v1/salt/roles/db": { + "from_db": True, + "pass": "hunter2", + "list": ["a", "b"], + }, + "v1/salt/roles/web": { + "from_web": True, + "pass": "hunter1", + "list": ["c", "d"], + }, + } + res = Mock(status_code=200, ok=True) + res.json.return_value = {"data": copy.deepcopy(vault_data[resource])} + return res + + cases = [ + ( + ["salt/roles/db", "salt/roles/web"], + {"from_db": True, "from_web": True, "list": ["c", "d"], "pass": "hunter1"}, + ), + ( + ["salt/roles/web", "salt/roles/db"], + {"from_db": True, "from_web": True, "list": ["a", "b"], "pass": "hunter2"}, + ), + ] + vaultkv = Mock(side_effect=make_request) + + for expanded_patterns, expected in cases: + with patch.dict( + vault.__utils__, + { + "vault.make_request": vaultkv, + "vault.expand_pattern_lists": Mock(return_value=expanded_patterns), + "vault.is_v2": Mock(return_value=is_v2_false), + }, + ): + ext_pillar = vault.ext_pillar( + "test-minion", + {"roles": ["db", "web"]}, + conf="path=salt/roles/{pillar[roles]}", + merge_strategy="smart", + merge_lists=False, + ) + assert ext_pillar == expected + + +def test_ext_pillar_disabled_during_policy_pillar_rendering(): + """ + Ensure ext_pillar returns an empty dict when called during pillar + template rendering to prevent a cyclic dependency. + """ + mock_version = Mock() + mock_vault = Mock() + extra = {"_vault_runner_is_compiling_pillar_templates": True} + + with patch.dict( + vault.__utils__, {"vault.make_request": mock_vault, "vault.is_v2": mock_version} + ): + assert {} == vault.ext_pillar( + "test-minion", {}, conf="path=secret/path", extra_minion_data=extra + ) + assert mock_version.call_count == 0 + assert mock_vault.call_count == 0 + + +def test_invalid_config(caplog): + """ + Ensure an empty dict is returned and an error is logged in case + the config does not contain path=<...> + """ + with caplog.at_level(logging.ERROR): + ext_pillar = vault.ext_pillar("testminion", {}, "secret/path") + assert ext_pillar == {} + assert "is not a valid Vault ext_pillar config" in caplog.text diff --git a/tests/pytests/unit/runners/vault/test_app_role_auth.py b/tests/pytests/unit/runners/vault/test_app_role_auth.py index a60a8a8bc4b0..d6cfc6b53a1a 100644 --- a/tests/pytests/unit/runners/vault/test_app_role_auth.py +++ b/tests/pytests/unit/runners/vault/test_app_role_auth.py @@ -50,6 +50,10 @@ def configure_loader_modules(): } +@patch( + "salt.runners.vault._get_policies_cached", + Mock(return_value=["saltstack/minion/test-minion", "saltstack/minions"]), +) def test_generate_token(): """ Basic test for test_generate_token with approle (two vault calls) diff --git a/tests/pytests/unit/runners/vault/test_token_auth.py b/tests/pytests/unit/runners/vault/test_token_auth.py index 07c87fcd3c73..8f6dd7f09606 100644 --- a/tests/pytests/unit/runners/vault/test_token_auth.py +++ b/tests/pytests/unit/runners/vault/test_token_auth.py @@ -50,6 +50,10 @@ def configure_loader_modules(): } +@patch( + "salt.runners.vault._get_policies_cached", + Mock(return_value=["saltstack/minion/test-minion", "saltstack/minions"]), +) def test_generate_token(): """ Basic tests for test_generate_token: all exits @@ -109,7 +113,7 @@ def test_generate_token(): assert "error" in result assert result["error"] == "no reason" - with patch("salt.runners.vault._get_policies", MagicMock(return_value=[])): + with patch("salt.runners.vault._get_policies_cached", MagicMock(return_value=[])): result = vault.generate_token("test-minion", "signature") assert isinstance(result, dict) assert "error" in result @@ -124,6 +128,10 @@ def test_generate_token(): assert result["error"] == "Test Exception Reason" +@patch( + "salt.runners.vault._get_policies_cached", + Mock(return_value=["saltstack/minion/test-minion", "saltstack/minions"]), +) def test_generate_token_with_namespace(): """ Basic tests for test_generate_token: all exits diff --git a/tests/pytests/unit/runners/vault/test_vault.py b/tests/pytests/unit/runners/vault/test_vault.py index cbceee05ca18..3634e862e8ea 100644 --- a/tests/pytests/unit/runners/vault/test_vault.py +++ b/tests/pytests/unit/runners/vault/test_vault.py @@ -8,7 +8,7 @@ import pytest import salt.runners.vault as vault -from tests.support.mock import MagicMock, patch +from tests.support.mock import MagicMock, Mock, patch log = logging.getLogger(__name__) @@ -29,53 +29,34 @@ def grains(): } -def test_pattern_list_expander(grains): - """ - Ensure _expand_pattern_lists works as intended: - - Expand list-valued patterns - - Do not change non-list-valued tokens - """ - cases = { - "no-tokens-to-replace": ["no-tokens-to-replace"], - "single-dict:{minion}": ["single-dict:{minion}"], - "single-list:{grains[roles]}": ["single-list:web", "single-list:database"], - "multiple-lists:{grains[roles]}+{grains[aux]}": [ - "multiple-lists:web+foo", - "multiple-lists:web+bar", - "multiple-lists:database+foo", - "multiple-lists:database+bar", - ], - "single-list-with-dicts:{grains[id]}+{grains[roles]}+{grains[id]}": [ - "single-list-with-dicts:{grains[id]}+web+{grains[id]}", - "single-list-with-dicts:{grains[id]}+database+{grains[id]}", - ], - "deeply-nested-list:{grains[deep][foo][bar][baz]}": [ - "deeply-nested-list:hello", - "deeply-nested-list:world", - ], +@pytest.fixture +def pillar(): + return { + "role": "test", } - # The mappings dict is assembled in _get_policies, so emulate here - mappings = {"minion": grains["id"], "grains": grains} - for case, correct_output in cases.items(): - output = vault._expand_pattern_lists( - case, **mappings - ) # pylint: disable=protected-access - diff = set(output).symmetric_difference(set(correct_output)) - if diff: - log.debug("Test %s failed", case) - log.debug("Expected:\n\t%s\nGot\n\t%s", output, correct_output) - log.debug("Difference:\n\t%s", diff) - assert output == correct_output + +@pytest.fixture +def expand_pattern_lists(): + with patch.dict( + vault.__utils__, + { + "vault.expand_pattern_lists": Mock( + side_effect=lambda x, *args, **kwargs: [x] + ) + }, + ): + yield +@pytest.mark.usefixtures("expand_pattern_lists") def test_get_policies_for_nonexisting_minions(): minion_id = "salt_master" # For non-existing minions, or the master-minion, grains will be None cases = { "no-tokens-to-replace": ["no-tokens-to-replace"], "single-dict:{minion}": ["single-dict:{}".format(minion_id)], - "single-list:{grains[roles]}": [], + "single-grain:{grains[os]}": [], } with patch( "salt.utils.minions.get_minion_data", @@ -94,28 +75,15 @@ def test_get_policies_for_nonexisting_minions(): assert output == correct_output +@pytest.mark.usefixtures("expand_pattern_lists") def test_get_policies(grains): """ - Ensure _get_policies works as intended, including expansion of lists + Ensure _get_policies works as intended. + The expansion of lists is tested in the vault utility module unit tests. """ cases = { "no-tokens-to-replace": ["no-tokens-to-replace"], "single-dict:{minion}": ["single-dict:test-minion"], - "single-list:{grains[roles]}": ["single-list:web", "single-list:database"], - "multiple-lists:{grains[roles]}+{grains[aux]}": [ - "multiple-lists:web+foo", - "multiple-lists:web+bar", - "multiple-lists:database+foo", - "multiple-lists:database+bar", - ], - "single-list-with-dicts:{grains[id]}+{grains[roles]}+{grains[id]}": [ - "single-list-with-dicts:test-minion+web+test-minion", - "single-list-with-dicts:test-minion+database+test-minion", - ], - "deeply-nested-list:{grains[deep][foo][bar][baz]}": [ - "deeply-nested-list:hello", - "deeply-nested-list:world", - ], "should-not-cause-an-exception,but-result-empty:{foo}": [], "Case-Should-Be-Lowered:{grains[mixedcase]}": [ "case-should-be-lowered:up-low-up" @@ -139,6 +107,33 @@ def test_get_policies(grains): assert output == correct_output +@pytest.mark.usefixtures("expand_pattern_lists") +@pytest.mark.parametrize( + "pattern,count", + [ + ("salt_minion_{minion}", 0), + ("salt_grain_{grains[id]}", 0), + ("unset_{foo}", 0), + ("salt_pillar_{pillar[role]}", 1), + ], +) +def test_get_policies_does_not_render_pillar_unnecessarily( + pattern, count, grains, pillar +): + """ + The pillar data should only be refreshed in case items are accessed. + """ + with patch("salt.utils.minions.get_minion_data", autospec=True) as get_minion_data: + get_minion_data.return_value = (None, grains, None) + with patch("salt.pillar.get_pillar", autospec=True) as get_pillar: + get_pillar.return_value.compile_pillar.return_value = pillar + test_config = {"policies": [pattern]} + vault._get_policies( + "test-minion", test_config, refresh_pillar=True + ) # pylint: disable=protected-access + assert get_pillar.call_count == count + + def test_get_token_create_url(): """ Ensure _get_token_create_url parses config correctly diff --git a/tests/pytests/unit/utils/test_vault.py b/tests/pytests/unit/utils/test_vault.py index e6abcc135577..0b93589ae1dc 100644 --- a/tests/pytests/unit/utils/test_vault.py +++ b/tests/pytests/unit/utils/test_vault.py @@ -546,6 +546,45 @@ def test_get_secret_path_metadata_no_cache(metadata_v2, cache_uses, cache_secret assert cache_object == expected_cache_object +def test_expand_pattern_lists(): + """ + Ensure expand_pattern_lists works as intended: + - Expand list-valued patterns + - Do not change non-list-valued tokens + """ + cases = { + "no-tokens-to-replace": ["no-tokens-to-replace"], + "single-dict:{minion}": ["single-dict:{minion}"], + "single-list:{grains[roles]}": ["single-list:web", "single-list:database"], + "multiple-lists:{grains[roles]}+{grains[aux]}": [ + "multiple-lists:web+foo", + "multiple-lists:web+bar", + "multiple-lists:database+foo", + "multiple-lists:database+bar", + ], + "single-list-with-dicts:{grains[id]}+{grains[roles]}+{grains[id]}": [ + "single-list-with-dicts:{grains[id]}+web+{grains[id]}", + "single-list-with-dicts:{grains[id]}+database+{grains[id]}", + ], + "deeply-nested-list:{grains[deep][foo][bar][baz]}": [ + "deeply-nested-list:hello", + "deeply-nested-list:world", + ], + } + + pattern_vars = { + "id": "test-minion", + "roles": ["web", "database"], + "aux": ["foo", "bar"], + "deep": {"foo": {"bar": {"baz": ["hello", "world"]}}}, + } + + mappings = {"minion": "test-minion", "grains": pattern_vars} + for case, correct_output in cases.items(): + output = vault.expand_pattern_lists(case, **mappings) + assert output == correct_output + + @pytest.mark.parametrize( "conf_location,called", [("local", False), ("master", True), (None, False), ("doesnotexist", False)],